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