From 67a33309508b3236494898c52176b868c913112d Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Thu, 11 Aug 2016 11:51:59 -0700 Subject: [PATCH] Add capability.administrateServer to gerrit.config This provides a fail-safe in case an administrator deletes all administrateServer entries from All-Projects. In a default installation of Gerrit Code Review all administrators are locked out and recovery must happen by directly modifying the refs/meta/config branch in All-Projects "behind Gerrit's back". In some hosted installations this option may not be available to the administrators, as the UNIX account is managed by a different group. By adding a fail-safe group to gerrit.config the administrators are always able to use the REST API and web interface to manage the server, even if all entries were accidentally removed or incorrectly specified in All-Projects. Change-Id: I3083dc2997061ce7be095fb53f187856ab8ed0df --- Documentation/config-gerrit.txt | 27 ++++++++ .../gerrit/pgm/util/BatchProgramModule.java | 8 +++ .../server/account/CapabilityCollection.java | 69 ++++++++++++++----- .../config/AdministrateServerGroups.java | 34 +++++++++ .../AdministrateServerGroupsProvider.java | 65 +++++++++++++++++ .../server/config/GerritGlobalModule.java | 2 + .../server/config/GroupSetProvider.java | 10 +-- .../server/project/AccessControlModule.java | 23 +++++-- .../gerrit/server/project/ProjectState.java | 3 +- .../gerrit/server/project/RefControlTest.java | 28 ++++---- 10 files changed, 226 insertions(+), 43 deletions(-) create mode 100644 gerrit-server/src/main/java/com/google/gerrit/server/config/AdministrateServerGroups.java create mode 100644 gerrit-server/src/main/java/com/google/gerrit/server/config/AdministrateServerGroupsProvider.java diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt index cf230f0321..fb81be6a8b 100644 --- a/Documentation/config-gerrit.txt +++ b/Documentation/config-gerrit.txt @@ -892,6 +892,33 @@ threads will die out after the cache is loaded. + Default is the number of CPUs. + +[[capability]] +=== Section capability + +[[capability.administrateServer]]capability.administrateServer:: ++ +Names of groups of users that are allowed to exercise the +administrateServer capability, in addition to those listed in +All-Projects. Configuring this option can be a useful fail-safe +to recover a server in the event an administrator removed all +groups from the administrateServer capability, or to ensure that +specific groups always have administration capabilities. ++ +---- +[capability] + administrateServer = group Fail Safe Admins +---- ++ +The configuration file uses group names, not UUIDs. If a group is +renamed the gerrit.config file must be updated to reflect the new +name. If a group cannot be found for the configured name a warning +is logged and the server will continue normal startup. ++ +If not specified (default), only the groups listed by All-Projects +may use the administrateServer capability. + + [[change]] === Section change diff --git a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/util/BatchProgramModule.java b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/util/BatchProgramModule.java index eebf8e05a5..f076e5403b 100644 --- a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/util/BatchProgramModule.java +++ b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/util/BatchProgramModule.java @@ -17,6 +17,8 @@ package com.google.gerrit.pgm.util; import static com.google.inject.Scopes.SINGLETON; import com.google.common.cache.Cache; +import com.google.common.collect.ImmutableSet; +import com.google.gerrit.common.data.GroupReference; import com.google.gerrit.extensions.api.projects.CommentLinkInfo; import com.google.gerrit.extensions.config.FactoryModule; import com.google.gerrit.extensions.registration.DynamicMap; @@ -29,6 +31,7 @@ import com.google.gerrit.server.account.AccountByEmailCacheImpl; import com.google.gerrit.server.account.AccountCacheImpl; import com.google.gerrit.server.account.AccountVisibility; import com.google.gerrit.server.account.AccountVisibilityProvider; +import com.google.gerrit.server.account.CapabilityCollection; import com.google.gerrit.server.account.CapabilityControl; import com.google.gerrit.server.account.FakeRealm; import com.google.gerrit.server.account.GroupCacheImpl; @@ -41,6 +44,7 @@ import com.google.gerrit.server.change.ChangeKindCacheImpl; import com.google.gerrit.server.change.MergeabilityCacheImpl; import com.google.gerrit.server.change.PatchSetInserter; import com.google.gerrit.server.change.RebaseChangeOp; +import com.google.gerrit.server.config.AdministrateServerGroups; import com.google.gerrit.server.config.CanonicalWebUrl; import com.google.gerrit.server.config.CanonicalWebUrlProvider; import com.google.gerrit.server.config.DisableReverseDnsLookup; @@ -130,6 +134,9 @@ public class BatchProgramModule extends FactoryModule { bind(SearchingChangeCacheImpl.class).toProvider( Providers.of(null)); + bind(new TypeLiteral>() {}) + .annotatedWith(AdministrateServerGroups.class) + .toInstance(ImmutableSet. of()); bind(new TypeLiteral>() {}) .annotatedWith(GitUploadPackGroups.class) .toInstance(Collections. emptySet()); @@ -153,6 +160,7 @@ public class BatchProgramModule extends FactoryModule { install(ChangeKindCacheImpl.module()); install(MergeabilityCacheImpl.module()); install(TagCache.module()); + factory(CapabilityCollection.Factory.class); factory(CapabilityControl.Factory.class); factory(ChangeData.Factory.class); factory(ProjectState.Factory.class); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/CapabilityCollection.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/CapabilityCollection.java index 3017f73cc1..4bf4214011 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/account/CapabilityCollection.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/CapabilityCollection.java @@ -14,32 +14,46 @@ package com.google.gerrit.server.account; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; +import com.google.gerrit.common.Nullable; 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.common.data.PermissionRange; import com.google.gerrit.common.data.PermissionRule; +import com.google.gerrit.server.config.AdministrateServerGroups; import com.google.gerrit.server.group.SystemGroupBackend; +import com.google.inject.Inject; +import com.google.inject.assistedinject.Assisted; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Set; /** Caches active {@link GlobalCapability} set for a site. */ public class CapabilityCollection { - private final Map> permissions; + public interface Factory { + CapabilityCollection create(@Nullable AccessSection section); + } - public final List administrateServer; - public final List batchChangesLimit; - public final List emailReviewers; - public final List priority; - public final List queryLimit; + private final ImmutableMap> permissions; - public CapabilityCollection(AccessSection section) { + public final ImmutableList administrateServer; + public final ImmutableList batchChangesLimit; + public final ImmutableList emailReviewers; + public final ImmutableList priority; + public final ImmutableList queryLimit; + + @Inject + CapabilityCollection( + @AdministrateServerGroups ImmutableSet admins, + @Assisted @Nullable AccessSection section) { if (section == null) { section = new AccessSection(AccessSection.GLOBAL_CAPABILITIES); } @@ -61,18 +75,19 @@ public class CapabilityCollection { } } configureDefaults(tmp, section); + if (!tmp.containsKey(GlobalCapability.ADMINISTRATE_SERVER) && !admins.isEmpty()) { + tmp.put(GlobalCapability.ADMINISTRATE_SERVER, ImmutableList.of()); + } - Map> res = new HashMap<>(); + ImmutableMap.Builder> m = ImmutableMap.builder(); for (Map.Entry> e : tmp.entrySet()) { List rules = e.getValue(); - if (rules.size() == 1) { - res.put(e.getKey(), Collections.singletonList(rules.get(0))); - } else { - res.put(e.getKey(), Collections.unmodifiableList( - Arrays.asList(rules.toArray(new PermissionRule[rules.size()])))); + if (GlobalCapability.ADMINISTRATE_SERVER.equals(e.getKey())) { + rules = mergeAdmin(admins, rules); } + m.put(e.getKey(), ImmutableList.copyOf(rules)); } - permissions = Collections.unmodifiableMap(res); + permissions = m.build(); administrateServer = getPermission(GlobalCapability.ADMINISTRATE_SERVER); batchChangesLimit = getPermission(GlobalCapability.BATCH_CHANGES_LIMIT); @@ -81,9 +96,27 @@ public class CapabilityCollection { queryLimit = getPermission(GlobalCapability.QUERY_LIMIT); } - public List getPermission(String permissionName) { - List r = permissions.get(permissionName); - return r != null ? r : Collections. emptyList(); + private static List mergeAdmin(Set admins, + List rules) { + if (admins.isEmpty()) { + return rules; + } + + List r = new ArrayList<>(admins.size() + rules.size()); + for (GroupReference g : admins) { + r.add(new PermissionRule(g)); + } + for (PermissionRule rule : rules) { + if (!admins.contains(rule.getGroup())) { + r.add(rule); + } + } + return r; + } + + public ImmutableList getPermission(String permissionName) { + ImmutableList r = permissions.get(permissionName); + return r != null ? r : ImmutableList. of(); } private static final GroupReference anonymous = SystemGroupBackend diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/AdministrateServerGroups.java b/gerrit-server/src/main/java/com/google/gerrit/server/config/AdministrateServerGroups.java new file mode 100644 index 0000000000..caeb771e98 --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/config/AdministrateServerGroups.java @@ -0,0 +1,34 @@ +// Copyright (C) 2010 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License + +package com.google.gerrit.server.config; + +import static java.lang.annotation.RetentionPolicy.RUNTIME; + +import com.google.inject.BindingAnnotation; + +import java.lang.annotation.Retention; + +/** + * Groups that can always exercise {@code administrateServer} capability. + * + * 
+ * [capability]
+ *     administrateServer = group Administrators
+ *
+ */ +@Retention(RUNTIME) +@BindingAnnotation +public @interface AdministrateServerGroups { +} diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/AdministrateServerGroupsProvider.java b/gerrit-server/src/main/java/com/google/gerrit/server/config/AdministrateServerGroupsProvider.java new file mode 100644 index 0000000000..dd3b83292a --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/config/AdministrateServerGroupsProvider.java @@ -0,0 +1,65 @@ +// Copyright (C) 2010 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License + +package com.google.gerrit.server.config; + +import com.google.common.collect.ImmutableSet; +import com.google.gerrit.common.data.GroupReference; +import com.google.gerrit.common.data.PermissionRule; +import com.google.gerrit.server.account.GroupBackend; +import com.google.gerrit.server.account.GroupBackends; +import com.google.gerrit.server.util.RequestContext; +import com.google.gerrit.server.util.ServerRequestContext; +import com.google.gerrit.server.util.ThreadLocalRequestContext; +import com.google.inject.Inject; +import com.google.inject.Provider; + +import org.eclipse.jgit.lib.Config; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** Loads {@link AdministrateServerGroups} from {@code gerrit.config}. */ +public class AdministrateServerGroupsProvider implements Provider> { + private final ImmutableSet groups; + + @Inject + public AdministrateServerGroupsProvider(GroupBackend groupBackend, + @GerritServerConfig Config config, + ThreadLocalRequestContext threadContext, + ServerRequestContext serverCtx) { + RequestContext ctx = threadContext.setContext(serverCtx); + try { + ImmutableSet.Builder builder = ImmutableSet.builder(); + for (String value : config.getStringList("capability", null, "administrateServer")) { + PermissionRule rule = PermissionRule.fromString(value, false); + String name = rule.getGroup().getName(); + GroupReference g = GroupBackends.findBestSuggestion(groupBackend, name); + if (g != null) { + builder.add(g); + } else { + Logger log = LoggerFactory.getLogger(getClass()); + log.warn("Group \"{}\" not available, skipping.", name); + } + } + groups = builder.build(); + } finally { + threadContext.setContext(ctx); + } + } + + @Override + public ImmutableSet get() { + return groups; + } +} diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java b/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java index c6bc09fc3e..da1f9a6862 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java @@ -79,6 +79,7 @@ import com.google.gerrit.server.account.AccountManager; import com.google.gerrit.server.account.AccountResolver; import com.google.gerrit.server.account.AccountVisibility; import com.google.gerrit.server.account.AccountVisibilityProvider; +import com.google.gerrit.server.account.CapabilityCollection; import com.google.gerrit.server.account.CapabilityControl; import com.google.gerrit.server.account.ChangeUserName; import com.google.gerrit.server.account.EmailExpander; @@ -233,6 +234,7 @@ public class GerritGlobalModule extends FactoryModule { factory(DeleteReviewerSender.Factory.class); factory(AddKeySender.Factory.class); factory(BatchUpdate.Factory.class); + factory(CapabilityCollection.Factory.class); factory(CapabilityControl.Factory.class); factory(ChangeData.Factory.class); factory(ChangeJson.Factory.class); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/GroupSetProvider.java b/gerrit-server/src/main/java/com/google/gerrit/server/config/GroupSetProvider.java index af92c89ce9..78af1ae7e0 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/config/GroupSetProvider.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/config/GroupSetProvider.java @@ -30,10 +30,9 @@ import org.slf4j.LoggerFactory; import java.util.List; import java.util.Set; +/** Parses groups referenced in the {@code gerrit.config} file. */ public abstract class GroupSetProvider implements Provider> { - private static final Logger log = - LoggerFactory.getLogger(GroupSetProvider.class); protected Set groupIds; @@ -45,10 +44,11 @@ public abstract class GroupSetProvider implements ImmutableSet.Builder builder = ImmutableSet.builder(); for (String n : groupNames) { GroupReference g = GroupBackends.findBestSuggestion(groupBackend, n); - if (g == null) { - log.warn("Group \"{}\" not in database, skipping.", n); - } else { + if (g != null) { builder.add(g.getUUID()); + } else { + Logger log = LoggerFactory.getLogger(getClass()); + log.warn("Group \"{}\" not available, skipping.", n); } } groupIds = builder.build(); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/AccessControlModule.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/AccessControlModule.java index bc35d279c2..8e85fd0a6f 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/project/AccessControlModule.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/AccessControlModule.java @@ -16,8 +16,12 @@ package com.google.gerrit.server.project; import static com.google.inject.Scopes.SINGLETON; +import com.google.common.collect.ImmutableSet; +import com.google.gerrit.common.data.GroupReference; import com.google.gerrit.extensions.config.FactoryModule; import com.google.gerrit.reviewdb.client.AccountGroup; +import com.google.gerrit.server.config.AdministrateServerGroups; +import com.google.gerrit.server.config.AdministrateServerGroupsProvider; import com.google.gerrit.server.config.GitReceivePackGroups; import com.google.gerrit.server.config.GitReceivePackGroupsProvider; import com.google.gerrit.server.config.GitUploadPackGroups; @@ -29,13 +33,20 @@ import java.util.Set; public class AccessControlModule extends FactoryModule { @Override protected void configure() { - bind(new TypeLiteral>() {}) // - .annotatedWith(GitUploadPackGroups.class) // - .toProvider(GitUploadPackGroupsProvider.class).in(SINGLETON); + bind(new TypeLiteral>() {}) + .annotatedWith(AdministrateServerGroups.class) + .toProvider(AdministrateServerGroupsProvider.class) + .in(SINGLETON); - bind(new TypeLiteral>() {}) // - .annotatedWith(GitReceivePackGroups.class) // - .toProvider(GitReceivePackGroupsProvider.class).in(SINGLETON); + bind(new TypeLiteral>() {}) + .annotatedWith(GitUploadPackGroups.class) + .toProvider(GitUploadPackGroupsProvider.class) + .in(SINGLETON); + + bind(new TypeLiteral>() {}) + .annotatedWith(GitReceivePackGroups.class) + .toProvider(GitReceivePackGroupsProvider.class) + .in(SINGLETON); bind(ChangeControl.Factory.class); factory(ProjectControl.AssistedFactory.class); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectState.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectState.java index b391da790c..9e5c35f850 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectState.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectState.java @@ -123,6 +123,7 @@ public class ProjectState { final GitRepositoryManager gitMgr, final RulesCache rulesCache, final List commentLinks, + final CapabilityCollection.Factory capabilityFactory, @Assisted final ProjectConfig config) { this.sitePaths = sitePaths; this.projectCache = projectCache; @@ -137,7 +138,7 @@ public class ProjectState { this.config = config; this.configs = new HashMap<>(); this.capabilities = isAllProjects - ? new CapabilityCollection(config.getAccessSection(AccessSection.GLOBAL_CAPABILITIES)) + ? capabilityFactory.create(config.getAccessSection(AccessSection.GLOBAL_CAPABILITIES)) : null; if (isAllProjects && !Permission.canBeOnAllProjects(AccessSection.ALL, Permission.OWNER)) { diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java index 153fd9412e..d4d77bddc8 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java @@ -47,6 +47,7 @@ import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.rules.PrologEnvironment; import com.google.gerrit.rules.RulesCache; import com.google.gerrit.server.CurrentUser; +import com.google.gerrit.server.account.CapabilityCollection; import com.google.gerrit.server.account.CapabilityControl; import com.google.gerrit.server.account.GroupMembership; import com.google.gerrit.server.account.ListGroupMembership; @@ -235,6 +236,7 @@ public class RefControlTest { private ChangeControl.Factory changeControlFactory; private ReviewDb db; + @Inject private CapabilityCollection.Factory capabilityCollectionFactory; @Inject private CapabilityControl.Factory capabilityControlFactory; @Inject private SchemaCreator schemaCreator; @Inject private InMemoryDatabase schemaFactory; @@ -243,18 +245,6 @@ public class RefControlTest { @Before public void setUp() throws Exception { repoManager = new InMemoryRepositoryManager(); - try { - Repository repo = repoManager.createRepository(allProjectsName); - ProjectConfig allProjects = - new ProjectConfig(new Project.NameKey(allProjectsName.get())); - allProjects.load(repo); - LabelType cr = Util.codeReview(); - allProjects.getLabelSections().put(cr.getName(), cr); - add(allProjects); - } catch (IOException | ConfigInvalidException e) { - throw new RuntimeException(e); - } - projectCache = new ProjectCache() { @Override public ProjectState getAllProjects() { @@ -312,6 +302,18 @@ public class RefControlTest { Injector injector = Guice.createInjector(new InMemoryModule()); injector.injectMembers(this); + try { + Repository repo = repoManager.createRepository(allProjectsName); + ProjectConfig allProjects = + new ProjectConfig(new Project.NameKey(allProjectsName.get())); + allProjects.load(repo); + LabelType cr = Util.codeReview(); + allProjects.getLabelSections().put(cr.getName(), cr); + add(allProjects); + } catch (IOException | ConfigInvalidException e) { + throw new RuntimeException(e); + } + db = schemaFactory.open(); schemaCreator.create(db); @@ -865,7 +867,7 @@ public class RefControlTest { all.put(pc.getName(), new ProjectState(sitePaths, projectCache, allProjectsName, allUsersName, projectControlFactory, envFactory, repoManager, rulesCache, - commentLinks, pc)); + commentLinks, capabilityCollectionFactory, pc)); return repo; }