Finish moving ACCESS_DATABASE checks to PermissionBackend

Change-Id: I3b8c6fdb1cc23cee4ec0f0e88c3614366b4b4275
This commit is contained in:
Shawn Pearce
2017-04-29 12:23:00 -07:00
committed by David Pursehouse
parent 8edeb9e69c
commit 2a64b9acce
7 changed files with 65 additions and 45 deletions

View File

@@ -115,7 +115,7 @@ public class ExternalIdIT extends AbstractDaemonTest {
public void getExternalIdsOfOtherUserNotAllowed() throws Exception { public void getExternalIdsOfOtherUserNotAllowed() throws Exception {
setApiUser(user); setApiUser(user);
exception.expect(AuthException.class); exception.expect(AuthException.class);
exception.expectMessage("not allowed to get external IDs"); exception.expectMessage("access database not permitted");
gApi.accounts().id(admin.id.get()).getExternalIds(); gApi.accounts().id(admin.id.get()).getExternalIds();
} }
@@ -170,7 +170,7 @@ public class ExternalIdIT extends AbstractDaemonTest {
List<AccountExternalIdInfo> extIds = gApi.accounts().self().getExternalIds(); List<AccountExternalIdInfo> extIds = gApi.accounts().self().getExternalIds();
setApiUser(user); setApiUser(user);
exception.expect(AuthException.class); exception.expect(AuthException.class);
exception.expectMessage("not allowed to delete external IDs"); exception.expectMessage("access database not permitted");
gApi.accounts() gApi.accounts()
.id(admin.id.get()) .id(admin.id.get())
.deleteExternalIds(extIds.stream().map(e -> e.identity).collect(toList())); .deleteExternalIds(extIds.stream().map(e -> e.identity).collect(toList()));
@@ -428,7 +428,7 @@ public class ExternalIdIT extends AbstractDaemonTest {
@Test @Test
public void checkConsistencyNotAllowed() throws Exception { public void checkConsistencyNotAllowed() throws Exception {
exception.expect(AuthException.class); exception.expect(AuthException.class);
exception.expectMessage("not allowed to run consistency checks"); exception.expectMessage("access database not permitted");
gApi.config().server().checkConsistency(new ConsistencyCheckInput()); gApi.config().server().checkConsistency(new ConsistencyCheckInput());
} }

View File

@@ -63,11 +63,6 @@ public class CapabilityControl {
effective = new HashMap<>(); effective = new HashMap<>();
} }
/** Identity of the user the control will compute for. */
public CurrentUser getUser() {
return user;
}
/** /**
* <b>Do not use.</b> Determine if the user can administer this server. * <b>Do not use.</b> Determine if the user can administer this server.
* *
@@ -112,15 +107,6 @@ public class CapabilityControl {
return canPerform(GlobalCapability.VIEW_ALL_ACCOUNTS) || isAdmin_DoNotUse(); return canPerform(GlobalCapability.VIEW_ALL_ACCOUNTS) || isAdmin_DoNotUse();
} }
/** @return true if the user can access the database (with gsql). */
public boolean canAccessDatabase() {
try {
return doCanForDefaultPermissionBackend(GlobalPermission.ACCESS_DATABASE);
} catch (PermissionBackendException e) {
return false;
}
}
/** @return which priority queue the user's tasks should be submitted to. */ /** @return which priority queue the user's tasks should be submitted to. */
public QueueProvider.QueueType getQueueType() { public QueueProvider.QueueType getQueueType() {
// If a non-generic group (that is not Anonymous Users or Registered Users) // If a non-generic group (that is not Anonymous Users or Registered Users)

View File

@@ -17,7 +17,6 @@ package com.google.gerrit.server.account;
import static com.google.gerrit.server.account.externalids.ExternalId.SCHEME_USERNAME; import static com.google.gerrit.server.account.externalids.ExternalId.SCHEME_USERNAME;
import static java.util.stream.Collectors.toMap; import static java.util.stream.Collectors.toMap;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException; import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.ResourceConflictException; import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.Response; import com.google.gerrit.extensions.restapi.Response;
@@ -27,6 +26,9 @@ import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.account.externalids.ExternalId; import com.google.gerrit.server.account.externalids.ExternalId;
import com.google.gerrit.server.account.externalids.ExternalIds; import com.google.gerrit.server.account.externalids.ExternalIds;
import com.google.gerrit.server.permissions.GlobalPermission;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject; import com.google.inject.Inject;
import com.google.inject.Provider; import com.google.inject.Provider;
@@ -39,13 +41,18 @@ import org.eclipse.jgit.errors.ConfigInvalidException;
@Singleton @Singleton
public class DeleteExternalIds implements RestModifyView<AccountResource, List<String>> { public class DeleteExternalIds implements RestModifyView<AccountResource, List<String>> {
private final PermissionBackend permissionBackend;
private final AccountManager accountManager; private final AccountManager accountManager;
private final ExternalIds externalIds; private final ExternalIds externalIds;
private final Provider<CurrentUser> self; private final Provider<CurrentUser> self;
@Inject @Inject
DeleteExternalIds( DeleteExternalIds(
AccountManager accountManager, ExternalIds externalIds, Provider<CurrentUser> self) { PermissionBackend permissionBackend,
AccountManager accountManager,
ExternalIds externalIds,
Provider<CurrentUser> self) {
this.permissionBackend = permissionBackend;
this.accountManager = accountManager; this.accountManager = accountManager;
this.externalIds = externalIds; this.externalIds = externalIds;
this.self = self; this.self = self;
@@ -53,9 +60,10 @@ public class DeleteExternalIds implements RestModifyView<AccountResource, List<S
@Override @Override
public Response<?> apply(AccountResource resource, List<String> extIds) public Response<?> apply(AccountResource resource, List<String> extIds)
throws RestApiException, IOException, OrmException, ConfigInvalidException { throws RestApiException, IOException, OrmException, ConfigInvalidException,
if (self.get() != resource.getUser() && !self.get().getCapabilities().canAccessDatabase()) { PermissionBackendException {
throw new AuthException("not allowed to delete external IDs"); if (self.get() != resource.getUser()) {
permissionBackend.user(self).check(GlobalPermission.ACCESS_DATABASE);
} }
if (extIds == null || extIds.size() == 0) { if (extIds == null || extIds.size() == 0) {

View File

@@ -19,13 +19,15 @@ import static com.google.gerrit.server.account.externalids.ExternalId.SCHEME_USE
import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableList;
import com.google.common.collect.Lists; import com.google.common.collect.Lists;
import com.google.gerrit.extensions.common.AccountExternalIdInfo; import com.google.gerrit.extensions.common.AccountExternalIdInfo;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.RestApiException; import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestReadView; import com.google.gerrit.extensions.restapi.RestReadView;
import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.account.externalids.ExternalId; import com.google.gerrit.server.account.externalids.ExternalId;
import com.google.gerrit.server.account.externalids.ExternalIds; import com.google.gerrit.server.account.externalids.ExternalIds;
import com.google.gerrit.server.config.AuthConfig; import com.google.gerrit.server.config.AuthConfig;
import com.google.gerrit.server.permissions.GlobalPermission;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject; import com.google.inject.Inject;
import com.google.inject.Provider; import com.google.inject.Provider;
@@ -37,12 +39,18 @@ import java.util.List;
@Singleton @Singleton
public class GetExternalIds implements RestReadView<AccountResource> { public class GetExternalIds implements RestReadView<AccountResource> {
private final PermissionBackend permissionBackend;
private final ExternalIds externalIds; private final ExternalIds externalIds;
private final Provider<CurrentUser> self; private final Provider<CurrentUser> self;
private final AuthConfig authConfig; private final AuthConfig authConfig;
@Inject @Inject
GetExternalIds(ExternalIds externalIds, Provider<CurrentUser> self, AuthConfig authConfig) { GetExternalIds(
PermissionBackend permissionBackend,
ExternalIds externalIds,
Provider<CurrentUser> self,
AuthConfig authConfig) {
this.permissionBackend = permissionBackend;
this.externalIds = externalIds; this.externalIds = externalIds;
this.self = self; this.self = self;
this.authConfig = authConfig; this.authConfig = authConfig;
@@ -50,9 +58,9 @@ public class GetExternalIds implements RestReadView<AccountResource> {
@Override @Override
public List<AccountExternalIdInfo> apply(AccountResource resource) public List<AccountExternalIdInfo> apply(AccountResource resource)
throws RestApiException, IOException, OrmException { throws RestApiException, IOException, OrmException, PermissionBackendException {
if (self.get() != resource.getUser() && !self.get().getCapabilities().canAccessDatabase()) { if (self.get() != resource.getUser()) {
throw new AuthException("not allowed to get external IDs"); permissionBackend.user(self).check(GlobalPermission.ACCESS_DATABASE);
} }
Collection<ExternalId> ids = externalIds.byAccount(resource.getUser().getAccountId()); Collection<ExternalId> ids = externalIds.byAccount(resource.getUser().getAccountId());

View File

@@ -18,13 +18,15 @@ import com.google.gerrit.extensions.api.config.ConsistencyCheckInfo;
import com.google.gerrit.extensions.api.config.ConsistencyCheckInfo.CheckAccountExternalIdsResultInfo; import com.google.gerrit.extensions.api.config.ConsistencyCheckInfo.CheckAccountExternalIdsResultInfo;
import com.google.gerrit.extensions.api.config.ConsistencyCheckInfo.CheckAccountsResultInfo; import com.google.gerrit.extensions.api.config.ConsistencyCheckInfo.CheckAccountsResultInfo;
import com.google.gerrit.extensions.api.config.ConsistencyCheckInput; import com.google.gerrit.extensions.api.config.ConsistencyCheckInput;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException; import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.RestApiException; import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestModifyView; import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.account.AccountsConsistencyChecker; import com.google.gerrit.server.account.AccountsConsistencyChecker;
import com.google.gerrit.server.account.externalids.ExternalIdsConsistencyChecker; import com.google.gerrit.server.account.externalids.ExternalIdsConsistencyChecker;
import com.google.gerrit.server.permissions.GlobalPermission;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject; import com.google.inject.Inject;
import com.google.inject.Provider; import com.google.inject.Provider;
@@ -33,30 +35,27 @@ import java.io.IOException;
@Singleton @Singleton
public class CheckConsistency implements RestModifyView<ConfigResource, ConsistencyCheckInput> { public class CheckConsistency implements RestModifyView<ConfigResource, ConsistencyCheckInput> {
private final Provider<IdentifiedUser> userProvider; private final PermissionBackend permissionBackend;
private final Provider<CurrentUser> user;
private final AccountsConsistencyChecker accountsConsistencyChecker; private final AccountsConsistencyChecker accountsConsistencyChecker;
private final ExternalIdsConsistencyChecker externalIdsConsistencyChecker; private final ExternalIdsConsistencyChecker externalIdsConsistencyChecker;
@Inject @Inject
CheckConsistency( CheckConsistency(
Provider<IdentifiedUser> currentUser, PermissionBackend permissionBackend,
Provider<CurrentUser> user,
AccountsConsistencyChecker accountsConsistencyChecker, AccountsConsistencyChecker accountsConsistencyChecker,
ExternalIdsConsistencyChecker externalIdsConsistencyChecker) { ExternalIdsConsistencyChecker externalIdsConsistencyChecker) {
this.userProvider = currentUser; this.permissionBackend = permissionBackend;
this.user = user;
this.accountsConsistencyChecker = accountsConsistencyChecker; this.accountsConsistencyChecker = accountsConsistencyChecker;
this.externalIdsConsistencyChecker = externalIdsConsistencyChecker; this.externalIdsConsistencyChecker = externalIdsConsistencyChecker;
} }
@Override @Override
public ConsistencyCheckInfo apply(ConfigResource resource, ConsistencyCheckInput input) public ConsistencyCheckInfo apply(ConfigResource resource, ConsistencyCheckInput input)
throws RestApiException, IOException, OrmException { throws RestApiException, IOException, OrmException, PermissionBackendException {
IdentifiedUser user = userProvider.get(); permissionBackend.user(user).check(GlobalPermission.ACCESS_DATABASE);
if (!user.isIdentifiedUser()) {
throw new AuthException("Authentication required");
}
if (!user.getCapabilities().canAccessDatabase()) {
throw new AuthException("not allowed to run consistency checks");
}
if (input == null || (input.checkAccounts == null && input.checkAccountExternalIds == null)) { if (input == null || (input.checkAccounts == null && input.checkAccountExternalIds == null)) {
throw new BadRequestException("input required"); throw new BadRequestException("input required");

View File

@@ -30,6 +30,8 @@ import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.notedb.ChangeNotes; import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.permissions.GlobalPermission;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.project.ProjectControl; import com.google.gerrit.server.project.ProjectControl;
import com.google.gerrit.server.project.ProjectState; import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.server.query.change.ChangeData; import com.google.gerrit.server.query.change.ChangeData;
@@ -66,6 +68,7 @@ public class VisibleRefFilter extends AbstractAdvertiseRefsHook {
@Nullable private final SearchingChangeCacheImpl changeCache; @Nullable private final SearchingChangeCacheImpl changeCache;
private final Provider<ReviewDb> db; private final Provider<ReviewDb> db;
private final Provider<CurrentUser> user; private final Provider<CurrentUser> user;
private final PermissionBackend permissionBackend;
private final ProjectState projectState; private final ProjectState projectState;
private final Repository git; private final Repository git;
private ProjectControl projectCtl; private ProjectControl projectCtl;
@@ -80,6 +83,7 @@ public class VisibleRefFilter extends AbstractAdvertiseRefsHook {
@Nullable SearchingChangeCacheImpl changeCache, @Nullable SearchingChangeCacheImpl changeCache,
Provider<ReviewDb> db, Provider<ReviewDb> db,
Provider<CurrentUser> user, Provider<CurrentUser> user,
PermissionBackend permissionBackend,
@Assisted ProjectState projectState, @Assisted ProjectState projectState,
@Assisted Repository git) { @Assisted Repository git) {
this.tagCache = tagCache; this.tagCache = tagCache;
@@ -87,6 +91,7 @@ public class VisibleRefFilter extends AbstractAdvertiseRefsHook {
this.changeCache = changeCache; this.changeCache = changeCache;
this.db = db; this.db = db;
this.user = user; this.user = user;
this.permissionBackend = permissionBackend;
this.projectState = projectState; this.projectState = projectState;
this.git = git; this.git = git;
} }
@@ -110,9 +115,9 @@ public class VisibleRefFilter extends AbstractAdvertiseRefsHook {
Account.Id userId; Account.Id userId;
boolean viewMetadata; boolean viewMetadata;
if (user.get().isIdentifiedUser()) { if (user.get().isIdentifiedUser()) {
viewMetadata = permissionBackend.user(user).testOrFalse(GlobalPermission.ACCESS_DATABASE);
IdentifiedUser u = user.get().asIdentifiedUser(); IdentifiedUser u = user.get().asIdentifiedUser();
userId = u.getAccountId(); userId = u.getAccountId();
viewMetadata = u.getCapabilities().canAccessDatabase();
userEditPrefix = RefNames.refsEditPrefix(userId); userEditPrefix = RefNames.refsEditPrefix(userId);
} else { } else {
userId = null; userId = null;

View File

@@ -17,12 +17,16 @@ import com.google.common.base.Predicate;
import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables; import com.google.common.collect.Iterables;
import com.google.gerrit.extensions.registration.DynamicSet; import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames; import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.config.AllUsersName; import com.google.gerrit.server.config.AllUsersName;
import com.google.gerrit.server.events.RefReceivedEvent; import com.google.gerrit.server.events.RefReceivedEvent;
import com.google.gerrit.server.permissions.GlobalPermission;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.validators.ValidationException; import com.google.gerrit.server.validators.ValidationException;
import com.google.inject.Inject; import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted; import com.google.inject.assistedinject.Assisted;
@@ -46,17 +50,20 @@ public class RefOperationValidators {
update.getExpectedOldObjectId(), update.getNewObjectId(), update.getName(), type); update.getExpectedOldObjectId(), update.getNewObjectId(), update.getName(), type);
} }
private final PermissionBackend.WithUser perm;
private final AllUsersName allUsersName; private final AllUsersName allUsersName;
private final DynamicSet<RefOperationValidationListener> refOperationValidationListeners; private final DynamicSet<RefOperationValidationListener> refOperationValidationListeners;
private final RefReceivedEvent event; private final RefReceivedEvent event;
@Inject @Inject
RefOperationValidators( RefOperationValidators(
PermissionBackend permissionBackend,
AllUsersName allUsersName, AllUsersName allUsersName,
DynamicSet<RefOperationValidationListener> refOperationValidationListeners, DynamicSet<RefOperationValidationListener> refOperationValidationListeners,
@Assisted Project project, @Assisted Project project,
@Assisted IdentifiedUser user, @Assisted IdentifiedUser user,
@Assisted ReceiveCommand cmd) { @Assisted ReceiveCommand cmd) {
this.perm = permissionBackend.user(user);
this.allUsersName = allUsersName; this.allUsersName = allUsersName;
this.refOperationValidationListeners = refOperationValidationListeners; this.refOperationValidationListeners = refOperationValidationListeners;
event = new RefReceivedEvent(); event = new RefReceivedEvent();
@@ -69,7 +76,7 @@ public class RefOperationValidators {
List<ValidationMessage> messages = new ArrayList<>(); List<ValidationMessage> messages = new ArrayList<>();
boolean withException = false; boolean withException = false;
List<RefOperationValidationListener> listeners = new ArrayList<>(); List<RefOperationValidationListener> listeners = new ArrayList<>();
listeners.add(new DisallowCreationAndDeletionOfUserBranches(allUsersName)); listeners.add(new DisallowCreationAndDeletionOfUserBranches(perm, allUsersName));
refOperationValidationListeners.forEach(l -> listeners.add(l)); refOperationValidationListeners.forEach(l -> listeners.add(l));
try { try {
for (RefOperationValidationListener listener : listeners) { for (RefOperationValidationListener listener : listeners) {
@@ -107,9 +114,12 @@ public class RefOperationValidators {
private static class DisallowCreationAndDeletionOfUserBranches private static class DisallowCreationAndDeletionOfUserBranches
implements RefOperationValidationListener { implements RefOperationValidationListener {
private final PermissionBackend.WithUser perm;
private final AllUsersName allUsersName; private final AllUsersName allUsersName;
DisallowCreationAndDeletionOfUserBranches(AllUsersName allUsersName) { DisallowCreationAndDeletionOfUserBranches(
PermissionBackend.WithUser perm, AllUsersName allUsersName) {
this.perm = perm;
this.allUsersName = allUsersName; this.allUsersName = allUsersName;
} }
@@ -120,7 +130,9 @@ public class RefOperationValidators {
&& (refEvent.command.getRefName().startsWith(RefNames.REFS_USERS) && (refEvent.command.getRefName().startsWith(RefNames.REFS_USERS)
&& !refEvent.command.getRefName().equals(RefNames.REFS_USERS_DEFAULT))) { && !refEvent.command.getRefName().equals(RefNames.REFS_USERS_DEFAULT))) {
if (refEvent.command.getType().equals(ReceiveCommand.Type.CREATE)) { if (refEvent.command.getType().equals(ReceiveCommand.Type.CREATE)) {
if (!refEvent.user.getCapabilities().canAccessDatabase()) { try {
perm.check(GlobalPermission.ACCESS_DATABASE);
} catch (AuthException | PermissionBackendException e) {
throw new ValidationException("Not allowed to create user branch."); throw new ValidationException("Not allowed to create user branch.");
} }
if (Account.Id.fromRef(refEvent.command.getRefName()) == null) { if (Account.Id.fromRef(refEvent.command.getRefName()) == null) {
@@ -129,7 +141,9 @@ public class RefOperationValidators {
"Not allowed to create non-user branch under %s.", RefNames.REFS_USERS)); "Not allowed to create non-user branch under %s.", RefNames.REFS_USERS));
} }
} else if (refEvent.command.getType().equals(ReceiveCommand.Type.DELETE)) { } else if (refEvent.command.getType().equals(ReceiveCommand.Type.DELETE)) {
if (!refEvent.user.getCapabilities().canAccessDatabase()) { try {
perm.check(GlobalPermission.ACCESS_DATABASE);
} catch (AuthException | PermissionBackendException e) {
throw new ValidationException("Not allowed to delete user branch."); throw new ValidationException("Not allowed to delete user branch.");
} }
} }