Merge "Only accept pushes from service users if at least one owner is active"
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();
+ }
+}
