From 333a1e5c3fe1d8e187ddb93ccb18e564b1f3e8a3 Mon Sep 17 00:00:00 2001 From: Simon Hwang Date: Fri, 20 Nov 2015 13:26:01 -0500 Subject: Fetch parent groups of the authGroups An authGroup could be included in other groups and should be granted the same permission as its parents. The result of this change will be that the core is now able to check the permission of the parent groups of the authGroups. Moreover, the parent groups are fetched recursively so that multi dimensional inclusion could also work as expected. The plugin needs to be reloaded in order to perceive: - a newly created group - a change in group settings such as adding/removing groups in 'Included Groups' or a change in 'Members' Workflow considered: - user 'ua' is a member of group 'ga' - user 'ub' is a member of group 'gb' - group 'ga' is included in group 'gb' - group 'gb' is included in group 'gc' - group 'gc' is given READ permission of project 'p' - user 'ua' replicates the project 'p' successfully - user 'ub' replicates the project 'p' successfully Change-Id: Iaf9bd3502c1846c7b4bf89f5e2c068045647efef --- .../plugins/replication/AutoReloadConfigDecorator.java | 11 +++++++---- .../gerrit/plugins/replication/Destination.java | 17 ++++++++++++++++- .../plugins/replication/ReplicationFileBasedConfig.java | 9 +++++++-- 3 files changed, 30 insertions(+), 7 deletions(-) diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/AutoReloadConfigDecorator.java b/src/main/java/com/googlesource/gerrit/plugins/replication/AutoReloadConfigDecorator.java index 62cad2c..14db7a3 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/replication/AutoReloadConfigDecorator.java +++ b/src/main/java/com/googlesource/gerrit/plugins/replication/AutoReloadConfigDecorator.java @@ -15,6 +15,7 @@ package com.googlesource.gerrit.plugins.replication; import com.google.gerrit.server.PluginUser; import com.google.gerrit.server.account.GroupBackend; +import com.google.gerrit.server.account.GroupIncludeCache; import com.google.gerrit.server.config.SitePaths; import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.git.WorkQueue; @@ -43,12 +44,14 @@ public class AutoReloadConfigDecorator implements ReplicationConfig { private final GitRepositoryManager gitRepositoryManager; private final GroupBackend groupBackend; private final WorkQueue workQueue; + private final GroupIncludeCache groupIncludeCache; @Inject public AutoReloadConfigDecorator(Injector injector, SitePaths site, RemoteSiteUser.Factory ruf, PluginUser pu, GitRepositoryManager grm, GroupBackend gb, - WorkQueue workQueue) throws ConfigInvalidException, + WorkQueue workQueue, + GroupIncludeCache groupIncludeCache) throws ConfigInvalidException, IOException { this.injector = injector; this.site = site; @@ -56,6 +59,7 @@ public class AutoReloadConfigDecorator implements ReplicationConfig { this.pluginUser = pu; this.gitRepositoryManager = grm; this.groupBackend = gb; + this.groupIncludeCache = groupIncludeCache; this.currentConfig = loadConfig(); this.currentConfigTs = currentConfig.getCfgPath().lastModified(); this.workQueue = workQueue; @@ -63,9 +67,8 @@ public class AutoReloadConfigDecorator implements ReplicationConfig { private ReplicationFileBasedConfig loadConfig() throws ConfigInvalidException, IOException { - return new ReplicationFileBasedConfig(injector, site, - remoteSiteUserFactory, pluginUser, gitRepositoryManager, - groupBackend); + return new ReplicationFileBasedConfig(injector, site, remoteSiteUserFactory, + pluginUser, gitRepositoryManager, groupBackend, groupIncludeCache); } private synchronized boolean isAutoReload() { diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/Destination.java b/src/main/java/com/googlesource/gerrit/plugins/replication/Destination.java index dfeb174..544a285 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/replication/Destination.java +++ b/src/main/java/com/googlesource/gerrit/plugins/replication/Destination.java @@ -17,6 +17,7 @@ package com.googlesource.gerrit.plugins.replication; import com.google.common.base.MoreObjects; import com.google.common.base.Throwables; import com.google.common.collect.ImmutableSet; +import com.google.common.collect.ImmutableSet.Builder; import com.google.common.collect.Lists; import com.google.gerrit.common.data.GroupReference; import com.google.gerrit.reviewdb.client.AccountGroup; @@ -27,6 +28,7 @@ import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.PluginUser; import com.google.gerrit.server.account.GroupBackend; import com.google.gerrit.server.account.GroupBackends; +import com.google.gerrit.server.account.GroupIncludeCache; import com.google.gerrit.server.account.ListGroupMembership; import com.google.gerrit.server.config.ConfigUtil; import com.google.gerrit.server.config.FactoryModule; @@ -101,7 +103,8 @@ class Destination { final RemoteSiteUser.Factory replicationUserFactory, final PluginUser pluginUser, final GitRepositoryManager gitRepositoryManager, - final GroupBackend groupBackend) { + final GroupBackend groupBackend, + final GroupIncludeCache groupIncludeCache) { remote = rc; gitManager = gitRepositoryManager; delay = Math.max(0, @@ -131,6 +134,7 @@ class Destination { GroupReference g = GroupBackends.findExactSuggestion(groupBackend, name); if (g != null) { builder.add(g.getUUID()); + addRecursiveParents(g.getUUID(), builder, groupIncludeCache); } else { repLog.warn(String.format( "Group \"%s\" not recognized, removing from authGroup", name)); @@ -183,6 +187,17 @@ class Destination { threadScoper = child.getInstance(PerThreadRequestScope.Scoper.class); } + private void addRecursiveParents(AccountGroup.UUID g, + Builder builder, GroupIncludeCache groupIncludeCache) { + for (AccountGroup.UUID p : groupIncludeCache.parentGroupsOf(g)) { + if (builder.build().contains(p)) { + continue; + } + builder.add(p); + addRecursiveParents(p, builder, groupIncludeCache); + } + } + void start(WorkQueue workQueue) { pool = workQueue.createQueue(poolThreads, poolName); } diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationFileBasedConfig.java b/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationFileBasedConfig.java index f56185d..e796c9f 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationFileBasedConfig.java +++ b/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationFileBasedConfig.java @@ -19,6 +19,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.Lists; import com.google.gerrit.server.PluginUser; import com.google.gerrit.server.account.GroupBackend; +import com.google.gerrit.server.account.GroupIncludeCache; import com.google.gerrit.server.config.SitePaths; import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.git.WorkQueue; @@ -55,13 +56,17 @@ public class ReplicationFileBasedConfig implements ReplicationConfig { private final GitRepositoryManager gitRepositoryManager; private final GroupBackend groupBackend; private final FileBasedConfig config; + private final GroupIncludeCache groupIncludeCache; @Inject public ReplicationFileBasedConfig(final Injector injector, final SitePaths site, final RemoteSiteUser.Factory ruf, final PluginUser pu, final GitRepositoryManager grm, - final GroupBackend gb) throws ConfigInvalidException, IOException { + final GroupBackend gb, + final GroupIncludeCache groupIncludeCache) throws ConfigInvalidException, + IOException { this.cfgPath = new File(site.etc_dir, "replication.config"); + this.groupIncludeCache = groupIncludeCache; this.injector = injector; this.replicationUserFactory = ruf; this.pluginUser = pu; @@ -159,7 +164,7 @@ public class ReplicationFileBasedConfig implements ReplicationConfig { Destination destination = new Destination(injector, c, config, replicationUserFactory, - pluginUser, gitRepositoryManager, groupBackend); + pluginUser, gitRepositoryManager, groupBackend, groupIncludeCache); if (!destination.isSingleProjectMatch()) { for (URIish u : c.getURIs()) { -- cgit v1.2.3