Merge branch 'stable-2.16' into stable-3.0
* stable-2.16:
VersionMetaData: Don't close passed in RevWalk
AccessIT: Add tests for when group appears twice for same rule
Prevent infinite loops with GWT UI and HTTP auth
Change bouncycastle urls
ListAccess: Fix incorrect behavior when group appears twice for same rule
Change-Id: Ie13fb6b21cf3478ecd9321bfc97c52f74416a4cf
diff --git a/java/com/google/gerrit/server/git/meta/VersionedMetaData.java b/java/com/google/gerrit/server/git/meta/VersionedMetaData.java
index a4cfcbf..5cda238 100644
--- a/java/com/google/gerrit/server/git/meta/VersionedMetaData.java
+++ b/java/com/google/gerrit/server/git/meta/VersionedMetaData.java
@@ -424,7 +424,10 @@
public void close() {
newTree = null;
- rw.close();
+ if (revWalk == null) {
+ rw.close();
+ }
+
if (objInserter == null && inserter != null) {
inserter.close();
inserter = null;
diff --git a/java/com/google/gerrit/server/restapi/project/GetAccess.java b/java/com/google/gerrit/server/restapi/project/GetAccess.java
index d6c07dd..171c2e5 100644
--- a/java/com/google/gerrit/server/restapi/project/GetAccess.java
+++ b/java/com/google/gerrit/server/restapi/project/GetAccess.java
@@ -332,7 +332,7 @@
}
AccountGroup.UUID group = r.getGroup().getUUID();
if (group != null) {
- pInfo.rules.put(group.get(), info);
+ pInfo.rules.putIfAbsent(group.get(), info); // First entry for the group wins
loadGroup(groups, group);
}
}
diff --git a/javatests/com/google/gerrit/acceptance/rest/project/AccessIT.java b/javatests/com/google/gerrit/acceptance/rest/project/AccessIT.java
index 72af075..eac911d 100644
--- a/javatests/com/google/gerrit/acceptance/rest/project/AccessIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/project/AccessIT.java
@@ -17,6 +17,9 @@
import static com.google.common.truth.Truth8.assertThat;
import static com.google.gerrit.extensions.client.ListChangesOption.MESSAGES;
import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
+import static java.util.Arrays.asList;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.GitUtil;
@@ -25,6 +28,7 @@
import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
import com.google.gerrit.common.data.AccessSection;
import com.google.gerrit.common.data.GlobalCapability;
+import com.google.gerrit.common.data.GroupReference;
import com.google.gerrit.common.data.Permission;
import com.google.gerrit.extensions.api.access.AccessSectionInfo;
import com.google.gerrit.extensions.api.access.PermissionInfo;
@@ -51,6 +55,7 @@
import com.google.gerrit.server.project.ProjectConfig;
import com.google.inject.Inject;
import java.util.HashMap;
+import java.util.List;
import java.util.Map;
import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
import org.eclipse.jgit.junit.TestRepository;
@@ -673,6 +678,90 @@
return gApi.projects().name(newProjectName.get());
}
+ @Test
+ public void grantAllowAndDenyForSameGroup() throws Exception {
+ GroupReference registeredUsers = systemGroupBackend.getGroup(REGISTERED_USERS);
+ String access = "access";
+ List<String> allowThenDeny =
+ asList(registeredUsers.toConfigValue(), "deny " + registeredUsers.toConfigValue());
+ // Clone repository to forcefully add permission
+ TestRepository<InMemoryRepository> allProjectsRepo = cloneProject(allProjects, admin);
+
+ // Fetch permission ref
+ GitUtil.fetch(allProjectsRepo, "refs/meta/config:cfg");
+ allProjectsRepo.reset("cfg");
+
+ // Load current permissions
+ String config =
+ gApi.projects()
+ .name(allProjects.get())
+ .branch(RefNames.REFS_CONFIG)
+ .file(ProjectConfig.PROJECT_CONFIG)
+ .asString();
+
+ // Append and push allowThenDeny permissions
+ Config cfg = new Config();
+ cfg.fromText(config);
+ cfg.setStringList(access, AccessSection.HEADS, Permission.READ, allowThenDeny);
+ config = cfg.toText();
+ PushOneCommit push =
+ pushFactory.create(
+ db, admin.getIdent(), allProjectsRepo, "Subject", ProjectConfig.PROJECT_CONFIG, config);
+ push.to(RefNames.REFS_CONFIG).assertOkStatus();
+
+ ProjectAccessInfo pai = gApi.projects().name(allProjects.get()).access();
+ Map<String, AccessSectionInfo> local = pai.local;
+ AccessSectionInfo heads = local.get(AccessSection.HEADS);
+ Map<String, PermissionInfo> permissions = heads.permissions;
+ PermissionInfo read = permissions.get(Permission.READ);
+ Map<String, PermissionRuleInfo> rules = read.rules;
+ assertEquals(
+ rules.get(registeredUsers.getUUID().get()),
+ new PermissionRuleInfo(PermissionRuleInfo.Action.ALLOW, false));
+ }
+
+ @Test
+ public void grantDenyAndAllowForSameGroup() throws Exception {
+ GroupReference registeredUsers = systemGroupBackend.getGroup(REGISTERED_USERS);
+ String access = "access";
+ List<String> denyThenAllow =
+ asList("deny " + registeredUsers.toConfigValue(), registeredUsers.toConfigValue());
+ // Clone repository to forcefully add permission
+ TestRepository<InMemoryRepository> allProjectsRepo = cloneProject(allProjects, admin);
+
+ // Fetch permission ref
+ GitUtil.fetch(allProjectsRepo, "refs/meta/config:cfg");
+ allProjectsRepo.reset("cfg");
+
+ // Load current permissions
+ String config =
+ gApi.projects()
+ .name(allProjects.get())
+ .branch(RefNames.REFS_CONFIG)
+ .file(ProjectConfig.PROJECT_CONFIG)
+ .asString();
+
+ // Append and push denyThenAllow permissions
+ Config cfg = new Config();
+ cfg.fromText(config);
+ cfg.setStringList(access, AccessSection.HEADS, Permission.READ, denyThenAllow);
+ config = cfg.toText();
+ PushOneCommit push =
+ pushFactory.create(
+ db, admin.getIdent(), allProjectsRepo, "Subject", ProjectConfig.PROJECT_CONFIG, config);
+ push.to(RefNames.REFS_CONFIG).assertOkStatus();
+
+ ProjectAccessInfo pai = gApi.projects().name(allProjects.get()).access();
+ Map<String, AccessSectionInfo> local = pai.local;
+ AccessSectionInfo heads = local.get(AccessSection.HEADS);
+ Map<String, PermissionInfo> permissions = heads.permissions;
+ PermissionInfo read = permissions.get(Permission.READ);
+ Map<String, PermissionRuleInfo> rules = read.rules;
+ assertEquals(
+ rules.get(registeredUsers.getUUID().get()),
+ new PermissionRuleInfo(PermissionRuleInfo.Action.DENY, false));
+ }
+
private ProjectAccessInput newProjectAccessInput() {
ProjectAccessInput p = new ProjectAccessInput();
p.add = new HashMap<>();
diff --git a/resources/com/google/gerrit/httpd/auth/container/LoginRedirect.html b/resources/com/google/gerrit/httpd/auth/container/LoginRedirect.html
index 0567468..54c3661 100644
--- a/resources/com/google/gerrit/httpd/auth/container/LoginRedirect.html
+++ b/resources/com/google/gerrit/httpd/auth/container/LoginRedirect.html
@@ -4,6 +4,12 @@
<title>Gerrit Code Review</title>
<script type="text/javascript">
var href = window.location.href;
+ var query = "";
+ var q = href.indexOf('?');
+ if (q >= 0) {
+ query = href.substring(q);
+ href = href.substring(0,q);
+ }
var p = href.indexOf('#');
var token;
if (p >= 0) {
@@ -12,7 +18,7 @@
} else {
token = '';
}
- window.location.replace(href + 'login/' + token);
+ window.location.replace(href + 'login/' + token + query);
</script>
</head>
<body>
