Fix the buggy task visibility
TaskCofigFactory is looking at the wrong methods to define
if a ref is visibile to user. Fix to call RefControl.isVisible()
rather than RefControl.canRead(). canRead() only returns the
project permissions but isVisible() takes the user and ref
into account. Also modify a supporting test.
Originally-Authored-By: Adithya Chakilam
Change-Id: I89ff399e66f06133212825a0fb3f1239734fb1a9
diff --git a/src/main/java/com/googlesource/gerrit/plugins/task/TaskConfigFactory.java b/src/main/java/com/googlesource/gerrit/plugins/task/TaskConfigFactory.java
index 908ff0b..dd6ab9d 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/task/TaskConfigFactory.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/task/TaskConfigFactory.java
@@ -88,7 +88,7 @@
boolean visible = true; // invisible psas are filtered out by commandline
boolean isMasqueraded = false;
if (psa == null) {
- visible = canRead(branch);
+ visible = isVisible(branch);
} else {
isMasqueraded = true;
branch = BranchNameKey.create(psa.change.getProject(), psa.patchSet.refName());
@@ -110,7 +110,7 @@
return cfg;
}
- public boolean canRead(BranchNameKey branch) {
+ public boolean isVisible(BranchNameKey branch) {
try {
PermissionBackend.ForProject permissions =
permissionBackend.user(user).project(branch.project());
diff --git a/src/main/resources/Documentation/test/preview.md b/src/main/resources/Documentation/test/preview.md
index 510f073..2a436ae 100644
--- a/src/main/resources/Documentation/test/preview.md
+++ b/src/main/resources/Documentation/test/preview.md
@@ -292,22 +292,28 @@
"status" : "WAITING",
"subTasks" : [
{
- "applicable" : true,
- "hasPass" : true,
- "name" : "userfile task/special.config PASS",
- "status" : "PASS"
+ "applicable" : true, # Only Test Suite: !untrusted
+ "hasPass" : true, # Only Test Suite: !untrusted
+ "name" : "userfile task/special.config PASS", # Only Test Suite: !untrusted
+ "status" : "PASS" # Only Test Suite: !untrusted
+ "name" : "UNKNOWN", # Only Test Suite: untrusted
+ "status" : "UNKNOWN" # Only Test Suite: untrusted
},
{
- "applicable" : true,
- "hasPass" : true,
- "name" : "userfile task/special.config FAIL",
- "status" : "FAIL"
+ "applicable" : true, # Only Test Suite: !untrusted
+ "hasPass" : true, # Only Test Suite: !untrusted
+ "name" : "userfile task/special.config FAIL", # Only Test Suite: !untrusted
+ "status" : "FAIL" # Only Test Suite: !untrusted
+ "name" : "UNKNOWN", # Only Test Suite: untrusted
+ "status" : "UNKNOWN" # Only Test Suite: untrusted
},
{
- "applicable" : true,
- "hasPass" : true,
- "name" : "file task/common.config Preload PASS",
- "status" : "PASS"
+ "applicable" : true, # Only Test Suite: !untrusted
+ "hasPass" : true, # Only Test Suite: !untrusted
+ "name" : "file task/common.config Preload PASS", # Only Test Suite: !untrusted
+ "status" : "PASS" # Only Test Suite: !untrusted
+ "name" : "UNKNOWN", # Only Test Suite: untrusted
+ "status" : "UNKNOWN" # Only Test Suite: untrusted
}
]
}
diff --git a/test/check_task_statuses.sh b/test/check_task_statuses.sh
index 1d3efbd..9fb939f 100755
--- a/test/check_task_statuses.sh
+++ b/test/check_task_statuses.sh
@@ -60,12 +60,12 @@
# These comments look like: "# Only Test Suite: <suite>"
#
-remove_suite() { # suite < pre_json > json
- grep -v "# Only Test Suite: $1" | \
- sed -e's/# Only Test Suite:.*$//; s/ *$//'
+remove_suites() { # suites... < pre_json > json
+ grep -vE "# Only Test Suite: ($(echo "$@" | sed "s/ /|/g"))" | \
+ sed -e's/# Only Test Suite:.*$//; s/ *$//'
}
-remove_not_suite() { remove_suite !"$1" ; } # suite < pre_json > json
+remove_not_suite() { remove_suites !"$1" ; } # suite < pre_json > json
# -------- Test Doc Format --------
#
@@ -144,7 +144,12 @@
json_val_by_key() { json_val_by "$1" "'$2'" ; } # json key > value
# --------
-gssh() { ssh -x -p "$PORT" "$SERVER" gerrit "$@" ; } # cmd [args]...
+
+gssh() { # [-l user] cmd [args]...
+ local user_args=()
+ [ "-l" = "$1" ] && { user_args=("-l" "$2") ; shift 2 ; }
+ ssh -x -p "$PORT" "${user_args[@]}" "$SERVER" gerrit "$@"
+}
q() { "$@" > /dev/null 2>&1 ; } # cmd [args...] # quiet a command
@@ -270,7 +275,11 @@
)
}
-query() { gssh query "$@" --format json ; } # query > json lines
+query() { # [-l user] query > json lines
+ local user_args=()
+ [ "-l" = "$1" ] && { user_args=("-l" "$2") ; shift 2 ; }
+ gssh "${user_args[@]}" query "$@" --format json
+}
# N < json lines > changeN_json
change_plugins() { awk "NR==$1" | get_plugins | json_pp ; }
@@ -297,23 +306,30 @@
results_suite "$name 2nd change" "$EXPECTED.$name"2 "$(echo "$out" | change_plugins 2)"
}
-test_generated() { # name task_args...
+test_generated() { # name [-l query_user] task_args...
local name=$1 ; shift
query "$@" | change_plugins 1 > "$ACTUAL.$name"
results_suite "$name" "$EXPECTED.$name" "$( < "$ACTUAL.$name")"
}
-test_file() { # name task_args...
- local name=$1 ; shift
- local expected=$MYDIR/$name output=$STATUSES.$name
+usage() { # [error_message]
+ cat <<-EOF
+Usage:
+ "$MYPROG" --server <gerrit_host> --untrusted-user <untrusted user>
- query "$@" | awk '$0==" \"plugins\" : [",$0==" ],"' > "$output"
- out=$(diff "$expected" "$output")
- result "$name" "$out"
+ --help|-h help text
+ --server|-s gerrit host
+ --untrusted-user user who don't have permission
+ to view other user refs.
+EOF
+
+ [ -n "$1" ] && { echo "Error: $1" ; exit 1 ; }
+ exit 0
}
readlink -f / &> /dev/null || readlink() { greadlink "$@" ; } # for MacOS
MYDIR=$(dirname -- "$(readlink -f -- "$0")")
+MYPROG=$(basename -- "$0")
DOCS=$MYDIR/.././src/main/resources/Documentation/test
OUT=$MYDIR/../target/tests
@@ -333,8 +349,20 @@
USER_SPECIAL_CFG=$USER_TASKS/special.config
# --- Args ----
-SERVER=$1
-[ -z "$SERVER" ] && { echo "You must specify a server" ; exit ; }
+
+while (( "$#" )) ; do
+ case "$1" in
+ --help|-h) usage ;;
+ --server|-s) shift ; SERVER=$1 ;;
+ --untrusted-user) shift ; UNTRUSTED_USER=$1 ;;
+ *) usage "invalid argument $1" ;;
+ esac
+ shift
+done
+
+[ -z "$SERVER" ] && usage "You must specify --server"
+[ -z "$UNTRUSTED_USER" ] && usage "You must specify --untrusted-user"
+
PORT=29418
PROJECT=test
@@ -395,8 +423,8 @@
"NEW" \
"")
-no_all_json=$(echo "$all_pjson" | remove_suite all)
-no_all2_json=$(echo "$all2_pjson" | remove_suite all)
+no_all_json=$(echo "$all_pjson" | remove_suites all)
+no_all2_json=$(echo "$all2_pjson" | remove_suites all)
echo "$no_all_json" | strip_non_applicable | \
grep -v "\"applicable\" :" > "$EXPECTED".applicable
@@ -411,8 +439,12 @@
preview_pjson=$(testdoc_2_pjson < "$DOC_PREVIEW" | replace_default_changes)
-echo "$preview_pjson" | remove_suite invalid | ensure json_pp > "$EXPECTED".preview
-echo "$preview_pjson" | remove_not_suite invalid | strip_non_invalid > "$EXPECTED".preview-invalid
+echo "$preview_pjson" | remove_suites "invalid" "!untrusted" | \
+ ensure json_pp > "$EXPECTED".preview-untrusted
+echo "$preview_pjson" | remove_suites "invalid" "untrusted" | \
+ ensure json_pp > "$EXPECTED".preview-admin
+echo "$preview_pjson" | remove_suites "!untrusted" "!invalid" | \
+ strip_non_invalid > "$EXPECTED".preview-invalid
testdoc_2_cfg < "$DOC_PREVIEW" | replace_user > "$ROOT_CFG"
cnum=$(create_repo_change "$ALL" "$REMOTE_ALL" "$REF_ALL")
@@ -428,6 +460,8 @@
test_generated invalid-applicable --task--applicable --task--invalid "$query"
ROOTS=$PREVIEW_ROOTS
-test_generated preview --task--preview "$cnum,1" --task--all "$query"
+test_generated preview-admin --task--preview "$cnum,1" --task--all "$query"
+test_generated preview-untrusted -l "$UNTRUSTED_USER" --task--preview "$cnum,1" --task--all "$query"
test_generated preview-invalid --task--preview "$cnum,1" --task--invalid "$query"
+
exit $RESULT
diff --git a/test/docker/run_tests/start.sh b/test/docker/run_tests/start.sh
index 73d69c3..5104677 100755
--- a/test/docker/run_tests/start.sh
+++ b/test/docker/run_tests/start.sh
@@ -28,4 +28,9 @@
./"$USER_RUN_TESTS_DIR"/update-all-users-project.sh
echo "Running Task plugin tests ..."
-cd "$USER_RUN_TESTS_DIR"/../../ && ./check_task_statuses.sh "$GERRIT_HOST"
+untrusted_user="untrusted_user"
+ssh -p 29418 "$GERRIT_HOST" gerrit create-account "$untrusted_user" --full-name "$untrusted_user" \
+ --email "$untrusted_user"@example.com --ssh-key - < ~/.ssh/id_rsa.pub
+
+cd "$USER_RUN_TESTS_DIR"/../../ && ./check_task_statuses.sh \
+ --server "$GERRIT_HOST" --untrusted-user "$untrusted_user"
diff --git a/test/docker/run_tests/update-all-users-project.sh b/test/docker/run_tests/update-all-users-project.sh
index d0e1527..55e879a 100755
--- a/test/docker/run_tests/update-all-users-project.sh
+++ b/test/docker/run_tests/update-all-users-project.sh
@@ -7,4 +7,18 @@
git config -f project.config access."refs/users/*".read "group Administrators"
git config -f project.config access."refs/users/*".push "group Administrators"
git config -f project.config access."refs/users/*".create "group Administrators"
+
+git config -f project.config access.'refs/users/${shardeduserid}'.read "group Registered Users"
+git config -f project.config access.'refs/users/${shardeduserid}'.push "group Registered Users"
+git config -f project.config access.'refs/users/${shardeduserid}'.create "group Registered Users"
+git config -f "project.config" \
+ access."refs/*".read "deny group Anonymous Users"
+echo -e "global:Registered-Users\tRegistered Users" >> groups
+echo -e "global:Anonymous-Users\tAnonymous Users" >> groups
git add . && git commit -m "project config update" && git push origin HEAD:refs/meta/config
+
+echo "Modifying All-Projects project ..."
+cd "$WORKSPACE" && git clone ssh://"$GERRIT_HOST":29418/All-Projects allProjects && cd allProjects
+git fetch origin refs/meta/config && git checkout FETCH_HEAD
+git config -f "project.config" --add access."refs/meta/config".read "group Registered Users"
+git add . && git commit -m "project config update" && git push origin HEAD:refs/meta/config
\ No newline at end of file
