Merge "Split context group tests"
diff --git a/Documentation/access-control.txt b/Documentation/access-control.txt
index cdf6b30..d333347 100644
--- a/Documentation/access-control.txt
+++ b/Documentation/access-control.txt
@@ -727,13 +727,29 @@
A user must have this access granted in order to see a project, its
changes, or any of its data.
-This category has a special behavior, where the per-project ACL is
-evaluated before the global all projects ACL. If the per-project
-ACL has granted `Read` with 'DENY', and does not otherwise grant
-`Read` with 'ALLOW', then a `Read` in the all projects ACL
-is ignored. This behavior is useful to hide a handful of projects
+[[read_special_behaviors]]
+==== Special behaviors
+
+This category has multiple special behaviors:
+
+The per-project ACL is evaluated before the global all projects ACL.
+If the per-project ACL has granted `Read` with 'DENY', and does not
+otherwise grant `Read` with 'ALLOW', then a `Read` in the all projects
+ACL is ignored. This behavior is useful to hide a handful of projects
on an otherwise public server.
+You cannot grant `Read` on the `refs/tags/` namespace. Visibility to
+`refs/tags/` is derived from `Read` grants on refs namespaces other than
+`refs/tags/`, `refs/changes/`, and `refs/cache-automerge/` by finding
+tags reachable from those refs. For example, if a tag `refs/tags/test`
+points to a commit on the branch `refs/heads/master`, then allowing
+`Read` access to `refs/heads/master` would also allow access to
+`refs/tags/test`. If a tag is reachable from multiple refs, allowing
+access to any of those refs allows access to the tag.
+
+[[read_typical_usage]]
+==== Typical usage
+
For an open source, public Gerrit installation it is common to grant
`Read` to `Anonymous Users` in the `All-Projects` ACL, enabling
casual browsing of any project's changes, as well as fetching any
@@ -911,7 +927,7 @@
Suggested access rights to grant:
-* xref:category_read[`Read`] on 'refs/heads/\*' and 'refs/tags/*'
+* xref:category_read[`Read`] on 'refs/heads/\*'
* xref:category_push[`Push`] to 'refs/for/refs/heads/*'
* link:config-labels.html#label_Code-Review[`Code-Review`] with range '-1' to '+1' for 'refs/heads/*'
@@ -939,7 +955,7 @@
Suggested access rights to grant:
-* xref:category_read[`Read`] on 'refs/heads/\*' and 'refs/tags/*'
+* xref:category_read[`Read`] on 'refs/heads/\*'
* xref:category_push[`Push`] to 'refs/for/refs/heads/*'
* xref:category_push_merge[`Push merge commit`] to 'refs/for/refs/heads/*'
* xref:category_forge_author[`Forge Author Identity`] to 'refs/heads/*'
@@ -994,7 +1010,7 @@
Suggested access rights to grant, that won't block changes:
-* xref:category_read[`Read`] on 'refs/heads/\*' and 'refs/tags/*'
+* xref:category_read[`Read`] on 'refs/heads/\*'
* link:config-labels.html#label_Code-Review[`Label: Code-Review`] with range '-1' to '0' for 'refs/heads/*'
* link:config-labels.html#label_Verified[`Label: Verified`] with range '0' to '+1' for 'refs/heads/*'
diff --git a/java/com/google/gerrit/server/group/testing/InternalGroupSubject.java b/java/com/google/gerrit/server/group/testing/InternalGroupSubject.java
index f5e8fca..0b24f8c 100644
--- a/java/com/google/gerrit/server/group/testing/InternalGroupSubject.java
+++ b/java/com/google/gerrit/server/group/testing/InternalGroupSubject.java
@@ -18,7 +18,6 @@
import com.google.common.truth.BooleanSubject;
import com.google.common.truth.ComparableSubject;
-import com.google.common.truth.DefaultSubject;
import com.google.common.truth.FailureMetadata;
import com.google.common.truth.IterableSubject;
import com.google.common.truth.StringSubject;
@@ -60,7 +59,7 @@
return check("getName()").that(group.getName());
}
- public Subject<DefaultSubject, Object> id() {
+ public Subject<?, ?> id() {
isNotNull();
return check("getId()").that(group.getId());
}
diff --git a/javatests/com/google/gerrit/acceptance/git/RefAdvertisementIT.java b/javatests/com/google/gerrit/acceptance/git/RefAdvertisementIT.java
index 4f55046..811ef35 100644
--- a/javatests/com/google/gerrit/acceptance/git/RefAdvertisementIT.java
+++ b/javatests/com/google/gerrit/acceptance/git/RefAdvertisementIT.java
@@ -74,6 +74,7 @@
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.RefUpdate;
import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.revwalk.RevCommit;
import org.junit.Before;
import org.junit.Test;
@@ -89,6 +90,7 @@
private AccountGroup.UUID nonInteractiveUsers;
private ChangeData cd1;
+ private RevCommit rc1;
private String psRef1;
private String metaRef1;
@@ -156,6 +158,7 @@
pushFactory.create(admin.newIdent(), testRepo).to("refs/for/master%submit");
mr.assertOkStatus();
cd1 = mr.getChange();
+ rc1 = mr.getCommit();
psRef1 = cd1.currentPatchSet().id().toRefName();
metaRef1 = RefNames.changeMetaRef(cd1.getId());
PushOneCommit.Result br =
@@ -189,10 +192,18 @@
btu.setExpectedOldObjectId(ObjectId.zeroId());
btu.setNewObjectId(repo.exactRef("refs/heads/branch").getObjectId());
assertThat(btu.update()).isEqualTo(RefUpdate.Result.NEW);
+
+ // Create a tag for the tree of the commit on 'master'
+ // tree-tag -> master.tree
+ RefUpdate ttu = repo.updateRef("refs/tags/tree-tag");
+ ttu.setExpectedOldObjectId(ObjectId.zeroId());
+ ttu.setNewObjectId(rc1.getTree().toObjectId());
+ assertThat(ttu.update()).isEqualTo(RefUpdate.Result.NEW);
}
}
@Test
+ @GerritConfig(name = "auth.skipFullRefEvaluationIfAllRefsAreVisible", value = "false")
public void uploadPackAllRefsVisibleNoRefsMetaConfig() throws Exception {
projectOperations
.project(project)
@@ -217,6 +228,36 @@
"refs/heads/master",
"refs/tags/branch-tag",
"refs/tags/master-tag");
+ // tree-tag not visible. See comment in subsetOfBranchesVisibleIncludingHead.
+ }
+
+ @Test
+ @GerritConfig(name = "auth.skipFullRefEvaluationIfAllRefsAreVisible", value = "true")
+ public void uploadPackAllRefsVisibleNoRefsMetaConfigSkipFullRefEval() throws Exception {
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(allow(Permission.READ).ref("refs/*").group(REGISTERED_USERS))
+ .add(allow(Permission.READ).ref(RefNames.REFS_CONFIG).group(admins))
+ .setExclusiveGroup(permissionKey(Permission.READ).ref(RefNames.REFS_CONFIG), true)
+ .update();
+
+ requestScopeOperations.setApiUser(user.id());
+ assertUploadPackRefs(
+ "HEAD",
+ psRef1,
+ metaRef1,
+ psRef2,
+ metaRef2,
+ psRef3,
+ metaRef3,
+ psRef4,
+ metaRef4,
+ "refs/heads/branch",
+ "refs/heads/master",
+ "refs/tags/branch-tag",
+ "refs/tags/master-tag",
+ "refs/tags/tree-tag");
}
@Test
@@ -242,7 +283,20 @@
"refs/heads/master",
RefNames.REFS_CONFIG,
"refs/tags/branch-tag",
- "refs/tags/master-tag");
+ "refs/tags/master-tag",
+ "refs/tags/tree-tag");
+ }
+
+ @Test
+ public void grantReadOnRefsTagsIsNoOp() throws Exception {
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(allow(Permission.READ).ref("refs/tags/*").group(REGISTERED_USERS))
+ .update();
+
+ requestScopeOperations.setApiUser(user.id());
+ assertUploadPackRefs(); // We expect no refs returned
}
@Test
@@ -257,6 +311,8 @@
requestScopeOperations.setApiUser(user.id());
assertUploadPackRefs(
"HEAD", psRef1, metaRef1, psRef3, metaRef3, "refs/heads/master", "refs/tags/master-tag");
+ // tree-tag is not visible because we don't look at trees reachable from
+ // refs
}
@Test
@@ -279,6 +335,7 @@
// master branch is not visible but master-tag is reachable from branch
// (since PushOneCommit always bases changes on each other).
"refs/tags/master-tag");
+ // tree-tag not visible. See comment in subsetOfBranchesVisibleIncludingHead.
}
@Test
@@ -306,6 +363,7 @@
"refs/heads/master",
"refs/tags/master-tag",
"refs/users/01/1000001/edit-" + cd3.getId() + "/1");
+ // tree-tag not visible. See comment in subsetOfBranchesVisibleIncludingHead.
}
@Test
@@ -338,6 +396,7 @@
"refs/tags/master-tag",
"refs/users/00/1000000/edit-" + cd3.getId() + "/1",
"refs/users/01/1000001/edit-" + cd3.getId() + "/1");
+ // tree-tag not visible. See comment in subsetOfBranchesVisibleIncludingHead.
}
@Test
@@ -374,6 +433,7 @@
"refs/tags/master-tag",
// All edits are visible due to accessDatabase capability.
"refs/users/00/1000000/edit-" + cd3.getId() + "/1");
+ // tree-tag not visible. See comment in subsetOfBranchesVisibleIncludingHead.
}
@Test
@@ -413,6 +473,7 @@
"refs/heads/master",
"refs/tags/branch-tag",
"refs/tags/master-tag");
+ // tree-tag not visible. See comment in subsetOfBranchesVisibleIncludingHead.
}
@Test
@@ -449,6 +510,35 @@
metaRef3,
"refs/heads/master",
"refs/tags/branch-tag",
+ "refs/tags/master-tag",
+ "refs/tags/tree-tag");
+ }
+
+ @Test
+ public void uploadPackSubsetRefsVisibleOrphanedTagInvisible() throws Exception {
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(allow(Permission.READ).ref("refs/heads/branch").group(REGISTERED_USERS))
+ .update();
+ // Create a tag for the pending change on 'branch' so that the tag is orphaned
+ try (Repository repo = repoManager.openRepository(project)) {
+ // change4-tag -> psRef4
+ RefUpdate ctu = repo.updateRef("refs/tags/change4-tag");
+ ctu.setExpectedOldObjectId(ObjectId.zeroId());
+ ctu.setNewObjectId(repo.exactRef(psRef4).getObjectId());
+ assertThat(ctu.update()).isEqualTo(RefUpdate.Result.NEW);
+ }
+
+ requestScopeOperations.setApiUser(user.id());
+ assertUploadPackRefs(
+ psRef2,
+ metaRef2,
+ psRef4,
+ metaRef4,
+ "refs/heads/branch",
+ "refs/tags/branch-tag",
+ // See comment in subsetOfBranchesVisibleNotIncludingHead.
"refs/tags/master-tag");
}
@@ -463,7 +553,8 @@
"refs/heads/master",
"refs/meta/config",
"refs/tags/branch-tag",
- "refs/tags/master-tag");
+ "refs/tags/master-tag",
+ "refs/tags/tree-tag");
assertThat(r.additionalHaves()).containsExactly(obj(cd3, 1), obj(cd4, 1));
}
diff --git a/package.json b/package.json
index f096e2a..8a18318f 100644
--- a/package.json
+++ b/package.json
@@ -12,6 +12,7 @@
"web-component-tester": "^6.5.0"
},
"scripts": {
+ "start": "polygerrit-ui/run-server.sh",
"test": "WCT_HEADLESS_MODE=1 WCT_ARGS='--verbose -l chrome' ./polygerrit-ui/app/run_test.sh",
"eslint": "./node_modules/eslint/bin/eslint.js --ignore-pattern 'bower_components/' --ignore-pattern 'gr-linked-text' --ignore-pattern 'scripts/vendor' --ext .html,.js polygerrit-ui/app || exit 0",
"test-template": "./polygerrit-ui/app/run_template_test.sh"
diff --git a/polygerrit-ui/README.md b/polygerrit-ui/README.md
index 7699808..9ceda8c 100644
--- a/polygerrit-ui/README.md
+++ b/polygerrit-ui/README.md
@@ -89,7 +89,7 @@
```sh
$(bazel info output_base)/external/local_jdk/bin/java \
- -DsourceRoot=/path/to/my/checkout \
+ -DsourceRoot=$(bazel info workspace) \
-jar bazel-bin/gerrit.war daemon \
-d $GERRIT_SITE \
--console-log \
diff --git a/polygerrit-ui/app/elements/plugins/gr-endpoint-decorator/gr-endpoint-decorator.js b/polygerrit-ui/app/elements/plugins/gr-endpoint-decorator/gr-endpoint-decorator.js
index 35e4292..69d7188 100644
--- a/polygerrit-ui/app/elements/plugins/gr-endpoint-decorator/gr-endpoint-decorator.js
+++ b/polygerrit-ui/app/elements/plugins/gr-endpoint-decorator/gr-endpoint-decorator.js
@@ -115,7 +115,8 @@
},
_initModule({moduleName, plugin, type, domHook}) {
- if (this._initializedPlugins.get(plugin.getPluginName())) {
+ const name = plugin.getPluginName() + '.' + moduleName;
+ if (this._initializedPlugins.get(name)) {
return;
}
let initPromise;
@@ -128,10 +129,9 @@
break;
}
if (!initPromise) {
- console.warn('Unable to initialize module' +
- `${moduleName} from ${plugin.getPluginName()}`);
+ console.warn('Unable to initialize module ' + name);
}
- this._initializedPlugins.set(plugin.getPluginName(), true);
+ this._initializedPlugins.set(name, true);
initPromise.then(el => {
domHook.handleInstanceAttached(el);
this._domHooks.set(el, domHook);
diff --git a/polygerrit-ui/app/elements/plugins/gr-endpoint-decorator/gr-endpoint-decorator_test.html b/polygerrit-ui/app/elements/plugins/gr-endpoint-decorator/gr-endpoint-decorator_test.html
index 65a5f08..fa097ac 100644
--- a/polygerrit-ui/app/elements/plugins/gr-endpoint-decorator/gr-endpoint-decorator_test.html
+++ b/polygerrit-ui/app/elements/plugins/gr-endpoint-decorator/gr-endpoint-decorator_test.html
@@ -129,6 +129,22 @@
});
});
+ test('two modules', done => {
+ plugin.registerCustomComponent('banana', 'mod-one');
+ plugin.registerCustomComponent('banana', 'mod-two');
+ flush(() => {
+ const element =
+ container.querySelector('gr-endpoint-decorator[name="banana"]');
+ const module1 = Polymer.dom(element.root).children.find(
+ element => element.nodeName === 'MOD-ONE');
+ assert.isOk(module1);
+ const module2 = Polymer.dom(element.root).children.find(
+ element => element.nodeName === 'MOD-TWO');
+ assert.isOk(module2);
+ done();
+ });
+ });
+
test('late param setup', done => {
const element =
container.querySelector('gr-endpoint-decorator[name="banana"]');
