diff options
author | Orgad Shaneh <orgad.shaneh@audiocodes.com> | 2021-11-16 07:36:15 +0200 |
---|---|---|
committer | Orgad Shaneh <orgad.shaneh@audiocodes.com> | 2021-11-16 10:00:16 +0200 |
commit | b2dfc4b25153e25ad040173279a994b9fd5c43ff (patch) | |
tree | befa7f15ab596c8d35cb386aac4d90e4215f413f | |
parent | 7c31335361872efa9f02848d3874c8b83ddea580 (diff) | |
parent | 9661f6309ec8a564878a108e93a56ff98daf8c2d (diff) |
Merge branch 'stable-3.1' into stable-3.2
* stable-3.1:
Fix tests compilation
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: Ie10708a20d272364ff1336acb37d575e8c04c7a7
4 files changed, 99 insertions, 3 deletions
diff --git a/java/com/google/gerrit/server/git/meta/VersionedMetaData.java b/java/com/google/gerrit/server/git/meta/VersionedMetaData.java index c0f11f9830..ea594dd0c3 100644 --- a/java/com/google/gerrit/server/git/meta/VersionedMetaData.java +++ b/java/com/google/gerrit/server/git/meta/VersionedMetaData.java @@ -428,7 +428,10 @@ public abstract class VersionedMetaData { 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 5a3dbcd28e..ba4cda57e9 100644 --- a/java/com/google/gerrit/server/restapi/project/GetAccess.java +++ b/java/com/google/gerrit/server/restapi/project/GetAccess.java @@ -331,7 +331,7 @@ public class GetAccess implements RestReadView<ProjectResource> { } 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 0514e03540..269fb3c394 100644 --- a/javatests/com/google/gerrit/acceptance/rest/project/AccessIT.java +++ b/javatests/com/google/gerrit/acceptance/rest/project/AccessIT.java @@ -23,6 +23,8 @@ import static com.google.gerrit.server.schema.AclUtil.grant; import static com.google.gerrit.testing.GerritJUnit.assertThrows; import static com.google.gerrit.truth.ConfigSubject.assertThat; import static com.google.gerrit.truth.MapSubject.assertThatMap; +import static java.util.Arrays.asList; +import static org.junit.Assert.assertEquals; import com.google.gerrit.acceptance.AbstractDaemonTest; import com.google.gerrit.acceptance.ExtensionRegistry; @@ -62,6 +64,7 @@ import com.google.gerrit.server.project.ProjectConfig; import com.google.gerrit.server.schema.GrantRevertPermission; 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; @@ -934,6 +937,90 @@ public class AccessIT extends AbstractDaemonTest { 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( + admin.newIdent(), 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( + admin.newIdent(), 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 0567468e8c..54c366148d 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> |