Only accept pushes from service users if at least one owner is active
This adds a commit validator that rejects pushes of commits where the
committer is a service user when this service user has no active
owner. The idea of having owners for a service user is that these
owners are responsible for the commits that are created by the service
user. If the accounts of all owners are inactive (e.g. because they
have left the company) the service must not be able anymore to push
further commits because then nobody would be responsible for these
commits.
Change-Id: I15d2130b6c51ff383ccc79d7d842f9846a47c0ab
Signed-off-by: Edwin Kempin <edwin.kempin@sap.com>
diff --git a/src/main/java/com/googlesource/gerrit/plugins/serviceuser/CreateServiceUserNotes.java b/src/main/java/com/googlesource/gerrit/plugins/serviceuser/CreateServiceUserNotes.java
index 600a9b1..41551e3 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/serviceuser/CreateServiceUserNotes.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/serviceuser/CreateServiceUserNotes.java
@@ -17,29 +17,13 @@
import static com.googlesource.gerrit.plugins.serviceuser.CreateServiceUser.KEY_CREATED_BY;
import static com.googlesource.gerrit.plugins.serviceuser.CreateServiceUser.KEY_OWNER;
-import com.google.gerrit.extensions.restapi.MethodNotAllowedException;
-import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
-import com.google.gerrit.reviewdb.client.Account;
-import com.google.gerrit.reviewdb.client.AccountGroup;
-import com.google.gerrit.reviewdb.client.AccountProjectWatch;
-import com.google.gerrit.reviewdb.client.Change.Id;
import com.google.gerrit.reviewdb.client.Project;
-import com.google.gerrit.reviewdb.server.ReviewDb;
-import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.GerritPersonIdent;
-import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.AccountInfo;
-import com.google.gerrit.server.account.AccountResolver;
-import com.google.gerrit.server.account.GroupMembership;
import com.google.gerrit.server.config.AnonymousCowardName;
import com.google.gerrit.server.git.NotesBranchUtil;
-import com.google.gerrit.server.group.ListMembers;
-import com.google.gerrit.server.util.RequestContext;
-import com.google.gerrit.server.util.ThreadLocalRequestContext;
import com.google.gwtorm.server.OrmException;
-import com.google.gwtorm.server.SchemaFactory;
import com.google.inject.Inject;
-import com.google.inject.Provider;
import com.google.inject.assistedinject.Assisted;
import com.googlesource.gerrit.plugins.serviceuser.GetServiceUser.ServiceUserInfo;
@@ -58,11 +42,6 @@
import org.slf4j.LoggerFactory;
import java.io.IOException;
-import java.util.ArrayList;
-import java.util.Collection;
-import java.util.Collections;
-import java.util.List;
-import java.util.Set;
class CreateServiceUserNotes {
private static final Logger log =
@@ -76,13 +55,8 @@
private final PersonIdent gerritServerIdent;
private final NotesBranchUtil.Factory notesBranchUtilFactory;
- private final AccountResolver resolver;
- private final IdentifiedUser.GenericFactory genericUserFactory;
- private final Provider<GetServiceUser> getServiceUser;
- private final Provider<ListMembers> listMembers;
+ private final ServiceUserResolver serviceUserResolver;
private final @AnonymousCowardName String anonymousCowardName;
- private final ThreadLocalRequestContext tl;
- private final SchemaFactory<ReviewDb> schema;
private final Project.NameKey project;
private final Repository git;
@@ -93,24 +67,14 @@
@Inject
CreateServiceUserNotes(@GerritPersonIdent PersonIdent gerritIdent,
NotesBranchUtil.Factory notesBranchUtilFactory,
- AccountResolver resolver,
- IdentifiedUser.GenericFactory genericUserFactory,
- Provider<GetServiceUser> getServiceUser,
- Provider<ListMembers> listMembers,
+ ServiceUserResolver serviceUserResolver,
@AnonymousCowardName String anonymousCowardName,
- ThreadLocalRequestContext tl,
- SchemaFactory<ReviewDb> schema,
@Assisted Project.NameKey project,
@Assisted Repository git) {
this.gerritServerIdent = gerritIdent;
this.notesBranchUtilFactory = notesBranchUtilFactory;
- this.resolver = resolver;
- this.genericUserFactory = genericUserFactory;
- this.getServiceUser = getServiceUser;
- this.listMembers = listMembers;
+ this.serviceUserResolver = serviceUserResolver;
this.anonymousCowardName = anonymousCowardName;
- this.tl = tl;
- this.schema = schema;
this.project = project;
this.git = git;
}
@@ -137,7 +101,8 @@
try {
for (RevCommit c : rw) {
- ServiceUserInfo serviceUser = getAsServiceUser(c.getCommitterIdent());
+ ServiceUserInfo serviceUser =
+ serviceUserResolver.getAsServiceUser(c.getCommitterIdent());
if (serviceUser != null) {
ObjectId content = createNoteContent(branch, serviceUser);
getNotes().set(c, content);
@@ -149,26 +114,6 @@
}
}
- private ServiceUserInfo getAsServiceUser(PersonIdent committerIdent)
- throws OrmException {
- StringBuilder committer = new StringBuilder();
- committer.append(committerIdent.getName());
- committer.append(" <");
- committer.append(committerIdent.getEmailAddress());
- committer.append("> ");
-
- Account account = resolver.find(committer.toString());
- if (account == null) {
- return null;
- }
- try {
- return getServiceUser.get().apply(
- new ServiceUserResource(genericUserFactory.create(account.getId())));
- } catch (ResourceNotFoundException e) {
- return null;
- }
- }
-
void commitNotes() throws IOException, ConcurrentRefUpdateException {
try {
if (serviceUserNotes == null) {
@@ -221,91 +166,12 @@
fmt.append("Project", project.get());
fmt.append("Branch", branch);
fmt.appendUser(KEY_CREATED_BY, serviceUser.createdBy);
- for (AccountInfo owner : listOwners(serviceUser)) {
+ for (AccountInfo owner : serviceUserResolver.listOwners(serviceUser)) {
fmt.appendUser(KEY_OWNER, owner);
}
return fmt.toString();
}
- private List<AccountInfo> listOwners(ServiceUserInfo serviceUser) throws OrmException {
- if (serviceUser.owner == null) {
- return Collections.emptyList();
- }
-
- final ReviewDb db = schema.open();
- try {
- RequestContext context = new RequestContext() {
- @Override
- public CurrentUser getCurrentUser() {
- return new CurrentUser(null) {
-
- @Override
- public Set<Id> getStarredChanges() {
- return null;
- }
-
- @Override
- public Collection<AccountProjectWatch> getNotificationFilters() {
- return null;
- }
-
- @Override
- public GroupMembership getEffectiveGroups() {
- return new GroupMembership() {
- @Override
- public Set<AccountGroup.UUID> intersection(Iterable<AccountGroup.UUID> groupIds) {
- return null;
- }
-
- @Override
- public Set<AccountGroup.UUID> getKnownGroups() {
- return null;
- }
-
- @Override
- public boolean containsAnyOf(Iterable<AccountGroup.UUID> groupIds) {
- return true;
- }
-
- @Override
- public boolean contains(AccountGroup.UUID groupId) {
- return true;
- }
- };
- }
- };
- }
-
- @Override
- public Provider<ReviewDb> getReviewDbProvider() {
- return new Provider<ReviewDb>() {
- @Override
- public ReviewDb get() {
- return db;
- }};
- }
- };
- RequestContext old = tl.setContext(context);
- try {
- ListMembers lm = listMembers.get();
- lm.setRecursive(true);
- List<AccountInfo> owners = new ArrayList<>();
- for (AccountInfo a : lm.apply(new AccountGroup.UUID(serviceUser.owner.id))) {
- owners.add(a);
- }
- return owners;
- } catch (MethodNotAllowedException e) {
- log.error(String.format("Failed to list members of owner group %s for service user %s.",
- serviceUser.owner.name, serviceUser.username));
- return Collections.emptyList();
- } finally {
- tl.setContext(old);
- }
- } finally {
- db.close();
- }
- }
-
private ObjectInserter getInserter() {
if (inserter == null) {
inserter = git.newObjectInserter();
diff --git a/src/main/java/com/googlesource/gerrit/plugins/serviceuser/Module.java b/src/main/java/com/googlesource/gerrit/plugins/serviceuser/Module.java
index 085e05d..1282925 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/serviceuser/Module.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/serviceuser/Module.java
@@ -25,6 +25,7 @@
import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.extensions.restapi.RestApiModule;
import com.google.gerrit.extensions.webui.TopMenu;
+import com.google.gerrit.server.git.validators.CommitValidationListener;
import com.google.inject.AbstractModule;
import com.google.inject.assistedinject.FactoryModuleBuilder;
@@ -38,6 +39,8 @@
DynamicSet.bind(binder(), TopMenu.class).to(ServiceUserMenu.class);
DynamicSet.bind(binder(), GitReferenceUpdatedListener.class)
.to(RefUpdateListener.class);
+ DynamicSet.bind(binder(), CommitValidationListener.class)
+ .to(ValidateServiceUserCommits.class);
install(new FactoryModuleBuilder().build(CreateServiceUserNotes.Factory.class));
install(new RestApiModule() {
@Override
diff --git a/src/main/java/com/googlesource/gerrit/plugins/serviceuser/ServiceUserResolver.java b/src/main/java/com/googlesource/gerrit/plugins/serviceuser/ServiceUserResolver.java
new file mode 100644
index 0000000..e29c304
--- /dev/null
+++ b/src/main/java/com/googlesource/gerrit/plugins/serviceuser/ServiceUserResolver.java
@@ -0,0 +1,188 @@
+// Copyright (C) 2014 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.googlesource.gerrit.plugins.serviceuser;
+
+import com.google.gerrit.extensions.restapi.MethodNotAllowedException;
+import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
+import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.reviewdb.client.AccountGroup;
+import com.google.gerrit.reviewdb.client.AccountProjectWatch;
+import com.google.gerrit.reviewdb.client.Change.Id;
+import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.account.AccountCache;
+import com.google.gerrit.server.account.AccountInfo;
+import com.google.gerrit.server.account.AccountResolver;
+import com.google.gerrit.server.account.AccountState;
+import com.google.gerrit.server.account.GroupMembership;
+import com.google.gerrit.server.group.ListMembers;
+import com.google.gerrit.server.util.RequestContext;
+import com.google.gerrit.server.util.ThreadLocalRequestContext;
+import com.google.gwtorm.server.OrmException;
+import com.google.gwtorm.server.SchemaFactory;
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+
+import com.googlesource.gerrit.plugins.serviceuser.GetServiceUser.ServiceUserInfo;
+
+import org.eclipse.jgit.lib.PersonIdent;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.List;
+import java.util.Set;
+
+public class ServiceUserResolver {
+ private static final Logger log =
+ LoggerFactory.getLogger(ServiceUserResolver.class);
+
+ private final AccountResolver resolver;
+ private final IdentifiedUser.GenericFactory genericUserFactory;
+ private final Provider<GetServiceUser> getServiceUser;
+ private final Provider<ListMembers> listMembers;
+ private final SchemaFactory<ReviewDb> schema;
+ private final ThreadLocalRequestContext tl;
+ private final AccountCache accountCache;
+
+ @Inject
+ ServiceUserResolver(AccountResolver resolver,
+ IdentifiedUser.GenericFactory genericUserFactory,
+ Provider<GetServiceUser> getServiceUser,
+ Provider<ListMembers> listMembers, SchemaFactory<ReviewDb> schema,
+ ThreadLocalRequestContext tl, AccountCache accountCache) {
+ this.resolver = resolver;
+ this.genericUserFactory = genericUserFactory;
+ this.getServiceUser = getServiceUser;
+ this.listMembers = listMembers;
+ this.schema = schema;
+ this.tl = tl;
+ this.accountCache = accountCache;
+ }
+
+ ServiceUserInfo getAsServiceUser(PersonIdent committerIdent)
+ throws OrmException {
+ StringBuilder committer = new StringBuilder();
+ committer.append(committerIdent.getName());
+ committer.append(" <");
+ committer.append(committerIdent.getEmailAddress());
+ committer.append("> ");
+
+ Account account = resolver.find(committer.toString());
+ if (account == null) {
+ return null;
+ }
+ try {
+ return getServiceUser.get().apply(
+ new ServiceUserResource(genericUserFactory.create(account.getId())));
+ } catch (ResourceNotFoundException e) {
+ return null;
+ }
+ }
+
+ List<AccountInfo> listOwners(ServiceUserInfo serviceUser) throws OrmException {
+ if (serviceUser.owner == null) {
+ return Collections.emptyList();
+ }
+
+ final ReviewDb db = schema.open();
+ try {
+ RequestContext context = new RequestContext() {
+ @Override
+ public CurrentUser getCurrentUser() {
+ return new CurrentUser(null) {
+
+ @Override
+ public Set<Id> getStarredChanges() {
+ return null;
+ }
+
+ @Override
+ public Collection<AccountProjectWatch> getNotificationFilters() {
+ return null;
+ }
+
+ @Override
+ public GroupMembership getEffectiveGroups() {
+ return new GroupMembership() {
+ @Override
+ public Set<AccountGroup.UUID> intersection(Iterable<AccountGroup.UUID> groupIds) {
+ return null;
+ }
+
+ @Override
+ public Set<AccountGroup.UUID> getKnownGroups() {
+ return null;
+ }
+
+ @Override
+ public boolean containsAnyOf(Iterable<AccountGroup.UUID> groupIds) {
+ return true;
+ }
+
+ @Override
+ public boolean contains(AccountGroup.UUID groupId) {
+ return true;
+ }
+ };
+ }
+ };
+ }
+
+ @Override
+ public Provider<ReviewDb> getReviewDbProvider() {
+ return new Provider<ReviewDb>() {
+ @Override
+ public ReviewDb get() {
+ return db;
+ }};
+ }
+ };
+ RequestContext old = tl.setContext(context);
+ try {
+ ListMembers lm = listMembers.get();
+ lm.setRecursive(true);
+ List<AccountInfo> owners = new ArrayList<>();
+ for (AccountInfo a : lm.apply(new AccountGroup.UUID(serviceUser.owner.id))) {
+ owners.add(a);
+ }
+ return owners;
+ } catch (MethodNotAllowedException e) {
+ log.error(String.format("Failed to list members of owner group %s for service user %s.",
+ serviceUser.owner.name, serviceUser.username));
+ return Collections.emptyList();
+ } finally {
+ tl.setContext(old);
+ }
+ } finally {
+ db.close();
+ }
+ }
+
+ List<AccountInfo> listActiveOwners(ServiceUserInfo serviceUser)
+ throws OrmException {
+ List<AccountInfo> activeOwners = new ArrayList<>();
+ for (AccountInfo owner : listOwners(serviceUser)) {
+ AccountState accountState = accountCache.get(owner._id);
+ if (accountState != null && accountState.getAccount().isActive()) {
+ activeOwners.add(owner);
+ }
+ }
+ return activeOwners;
+ }
+}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/serviceuser/ValidateServiceUserCommits.java b/src/main/java/com/googlesource/gerrit/plugins/serviceuser/ValidateServiceUserCommits.java
new file mode 100644
index 0000000..4cabde3
--- /dev/null
+++ b/src/main/java/com/googlesource/gerrit/plugins/serviceuser/ValidateServiceUserCommits.java
@@ -0,0 +1,74 @@
+// Copyright (C) 2014 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.googlesource.gerrit.plugins.serviceuser;
+
+import com.google.gerrit.server.account.AccountCache;
+import com.google.gerrit.server.account.AccountState;
+import com.google.gerrit.server.events.CommitReceivedEvent;
+import com.google.gerrit.server.git.validators.CommitValidationException;
+import com.google.gerrit.server.git.validators.CommitValidationListener;
+import com.google.gerrit.server.git.validators.CommitValidationMessage;
+import com.google.gwtorm.server.OrmException;
+import com.google.inject.Inject;
+
+import com.googlesource.gerrit.plugins.serviceuser.GetServiceUser.ServiceUserInfo;
+
+import org.eclipse.jgit.lib.PersonIdent;
+
+import java.util.Collections;
+import java.util.List;
+
+public class ValidateServiceUserCommits implements CommitValidationListener {
+ private final ServiceUserResolver serviceUserResolver;
+ private final AccountCache accountCache;
+
+ @Inject
+ ValidateServiceUserCommits(ServiceUserResolver serviceUserResolver,
+ AccountCache accountCache) {
+ this.serviceUserResolver = serviceUserResolver;
+ this.accountCache = accountCache;
+ }
+
+ @Override
+ public List<CommitValidationMessage> onCommitReceived(
+ CommitReceivedEvent receiveEvent) throws CommitValidationException {
+ try {
+ PersonIdent committer = receiveEvent.commit.getCommitterIdent();
+ ServiceUserInfo serviceUser =
+ serviceUserResolver.getAsServiceUser(committer);
+ if (serviceUser.owner != null
+ && serviceUserResolver.listActiveOwners(serviceUser).isEmpty()) {
+ throw new CommitValidationException(String.format(
+ "Commit %s of service user %s (%s) is rejected because "
+ + "all service user owner accounts are inactive.",
+ receiveEvent.commit.getId().getName(), committer.getName(),
+ committer.getEmailAddress()));
+ } else {
+ AccountState creator = accountCache.get(serviceUser.createdBy._id);
+ if (creator == null || !creator.getAccount().isActive()) {
+ throw new CommitValidationException(String.format(
+ "Commit %s of service user %s (%s) is rejected because "
+ + "the account of the service creator is inactive.",
+ receiveEvent.commit.getId().getName(), committer.getName(),
+ committer.getEmailAddress()));
+ }
+ }
+ } catch (OrmException e) {
+ throw new CommitValidationException(
+ "Internal error while checking for service user commits.", e);
+ }
+ return Collections.emptyList();
+ }
+}
