diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/account/ExternalIdIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/account/ExternalIdIT.java index 1976edc6be..462f9afb50 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/account/ExternalIdIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/account/ExternalIdIT.java @@ -115,7 +115,7 @@ public class ExternalIdIT extends AbstractDaemonTest { public void getExternalIdsOfOtherUserNotAllowed() throws Exception { setApiUser(user); 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(); } @@ -170,7 +170,7 @@ public class ExternalIdIT extends AbstractDaemonTest { List extIds = gApi.accounts().self().getExternalIds(); setApiUser(user); exception.expect(AuthException.class); - exception.expectMessage("not allowed to delete external IDs"); + exception.expectMessage("access database not permitted"); gApi.accounts() .id(admin.id.get()) .deleteExternalIds(extIds.stream().map(e -> e.identity).collect(toList())); @@ -428,7 +428,7 @@ public class ExternalIdIT extends AbstractDaemonTest { @Test public void checkConsistencyNotAllowed() throws Exception { exception.expect(AuthException.class); - exception.expectMessage("not allowed to run consistency checks"); + exception.expectMessage("access database not permitted"); gApi.config().server().checkConsistency(new ConsistencyCheckInput()); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/CapabilityControl.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/CapabilityControl.java index fa4c35b9cd..bb40c26d3d 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/account/CapabilityControl.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/CapabilityControl.java @@ -63,11 +63,6 @@ public class CapabilityControl { effective = new HashMap<>(); } - /** Identity of the user the control will compute for. */ - public CurrentUser getUser() { - return user; - } - /** * Do not use. Determine if the user can administer this server. * @@ -112,15 +107,6 @@ public class CapabilityControl { 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. */ public QueueProvider.QueueType getQueueType() { // If a non-generic group (that is not Anonymous Users or Registered Users) diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/DeleteExternalIds.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/DeleteExternalIds.java index 20c152ecb4..31679a64bd 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/account/DeleteExternalIds.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/DeleteExternalIds.java @@ -17,7 +17,6 @@ package com.google.gerrit.server.account; import static com.google.gerrit.server.account.externalids.ExternalId.SCHEME_USERNAME; 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.ResourceConflictException; 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.account.externalids.ExternalId; 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.inject.Inject; import com.google.inject.Provider; @@ -39,13 +41,18 @@ import org.eclipse.jgit.errors.ConfigInvalidException; @Singleton public class DeleteExternalIds implements RestModifyView> { + private final PermissionBackend permissionBackend; private final AccountManager accountManager; private final ExternalIds externalIds; private final Provider self; @Inject DeleteExternalIds( - AccountManager accountManager, ExternalIds externalIds, Provider self) { + PermissionBackend permissionBackend, + AccountManager accountManager, + ExternalIds externalIds, + Provider self) { + this.permissionBackend = permissionBackend; this.accountManager = accountManager; this.externalIds = externalIds; this.self = self; @@ -53,9 +60,10 @@ public class DeleteExternalIds implements RestModifyView apply(AccountResource resource, List extIds) - throws RestApiException, IOException, OrmException, ConfigInvalidException { - if (self.get() != resource.getUser() && !self.get().getCapabilities().canAccessDatabase()) { - throw new AuthException("not allowed to delete external IDs"); + throws RestApiException, IOException, OrmException, ConfigInvalidException, + PermissionBackendException { + if (self.get() != resource.getUser()) { + permissionBackend.user(self).check(GlobalPermission.ACCESS_DATABASE); } if (extIds == null || extIds.size() == 0) { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/GetExternalIds.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/GetExternalIds.java index e115e022f9..709bfc3c4f 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/account/GetExternalIds.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/GetExternalIds.java @@ -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.Lists; 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.RestReadView; import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.account.externalids.ExternalId; import com.google.gerrit.server.account.externalids.ExternalIds; 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.inject.Inject; import com.google.inject.Provider; @@ -37,12 +39,18 @@ import java.util.List; @Singleton public class GetExternalIds implements RestReadView { + private final PermissionBackend permissionBackend; private final ExternalIds externalIds; private final Provider self; private final AuthConfig authConfig; @Inject - GetExternalIds(ExternalIds externalIds, Provider self, AuthConfig authConfig) { + GetExternalIds( + PermissionBackend permissionBackend, + ExternalIds externalIds, + Provider self, + AuthConfig authConfig) { + this.permissionBackend = permissionBackend; this.externalIds = externalIds; this.self = self; this.authConfig = authConfig; @@ -50,9 +58,9 @@ public class GetExternalIds implements RestReadView { @Override public List apply(AccountResource resource) - throws RestApiException, IOException, OrmException { - if (self.get() != resource.getUser() && !self.get().getCapabilities().canAccessDatabase()) { - throw new AuthException("not allowed to get external IDs"); + throws RestApiException, IOException, OrmException, PermissionBackendException { + if (self.get() != resource.getUser()) { + permissionBackend.user(self).check(GlobalPermission.ACCESS_DATABASE); } Collection ids = externalIds.byAccount(resource.getUser().getAccountId()); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/CheckConsistency.java b/gerrit-server/src/main/java/com/google/gerrit/server/config/CheckConsistency.java index 416ba37476..eaf45bec17 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/config/CheckConsistency.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/config/CheckConsistency.java @@ -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.CheckAccountsResultInfo; 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.RestApiException; 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.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.inject.Inject; import com.google.inject.Provider; @@ -33,30 +35,27 @@ import java.io.IOException; @Singleton public class CheckConsistency implements RestModifyView { - private final Provider userProvider; + private final PermissionBackend permissionBackend; + private final Provider user; private final AccountsConsistencyChecker accountsConsistencyChecker; private final ExternalIdsConsistencyChecker externalIdsConsistencyChecker; @Inject CheckConsistency( - Provider currentUser, + PermissionBackend permissionBackend, + Provider user, AccountsConsistencyChecker accountsConsistencyChecker, ExternalIdsConsistencyChecker externalIdsConsistencyChecker) { - this.userProvider = currentUser; + this.permissionBackend = permissionBackend; + this.user = user; this.accountsConsistencyChecker = accountsConsistencyChecker; this.externalIdsConsistencyChecker = externalIdsConsistencyChecker; } @Override public ConsistencyCheckInfo apply(ConfigResource resource, ConsistencyCheckInput input) - throws RestApiException, IOException, OrmException { - IdentifiedUser user = userProvider.get(); - if (!user.isIdentifiedUser()) { - throw new AuthException("Authentication required"); - } - if (!user.getCapabilities().canAccessDatabase()) { - throw new AuthException("not allowed to run consistency checks"); - } + throws RestApiException, IOException, OrmException, PermissionBackendException { + permissionBackend.user(user).check(GlobalPermission.ACCESS_DATABASE); if (input == null || (input.checkAccounts == null && input.checkAccountExternalIds == null)) { throw new BadRequestException("input required"); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/VisibleRefFilter.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/VisibleRefFilter.java index e4d78c46c1..93aa3617b9 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/VisibleRefFilter.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/VisibleRefFilter.java @@ -30,6 +30,8 @@ import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.IdentifiedUser; 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.ProjectState; import com.google.gerrit.server.query.change.ChangeData; @@ -66,6 +68,7 @@ public class VisibleRefFilter extends AbstractAdvertiseRefsHook { @Nullable private final SearchingChangeCacheImpl changeCache; private final Provider db; private final Provider user; + private final PermissionBackend permissionBackend; private final ProjectState projectState; private final Repository git; private ProjectControl projectCtl; @@ -80,6 +83,7 @@ public class VisibleRefFilter extends AbstractAdvertiseRefsHook { @Nullable SearchingChangeCacheImpl changeCache, Provider db, Provider user, + PermissionBackend permissionBackend, @Assisted ProjectState projectState, @Assisted Repository git) { this.tagCache = tagCache; @@ -87,6 +91,7 @@ public class VisibleRefFilter extends AbstractAdvertiseRefsHook { this.changeCache = changeCache; this.db = db; this.user = user; + this.permissionBackend = permissionBackend; this.projectState = projectState; this.git = git; } @@ -110,9 +115,9 @@ public class VisibleRefFilter extends AbstractAdvertiseRefsHook { Account.Id userId; boolean viewMetadata; if (user.get().isIdentifiedUser()) { + viewMetadata = permissionBackend.user(user).testOrFalse(GlobalPermission.ACCESS_DATABASE); IdentifiedUser u = user.get().asIdentifiedUser(); userId = u.getAccountId(); - viewMetadata = u.getCapabilities().canAccessDatabase(); userEditPrefix = RefNames.refsEditPrefix(userId); } else { userId = null; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/RefOperationValidators.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/RefOperationValidators.java index 6d5824f1e4..8047a99a7d 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/RefOperationValidators.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/RefOperationValidators.java @@ -17,12 +17,16 @@ import com.google.common.base.Predicate; import com.google.common.collect.ImmutableList; import com.google.common.collect.Iterables; 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.Project; import com.google.gerrit.reviewdb.client.RefNames; import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.config.AllUsersName; 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.inject.Inject; import com.google.inject.assistedinject.Assisted; @@ -46,17 +50,20 @@ public class RefOperationValidators { update.getExpectedOldObjectId(), update.getNewObjectId(), update.getName(), type); } + private final PermissionBackend.WithUser perm; private final AllUsersName allUsersName; private final DynamicSet refOperationValidationListeners; private final RefReceivedEvent event; @Inject RefOperationValidators( + PermissionBackend permissionBackend, AllUsersName allUsersName, DynamicSet refOperationValidationListeners, @Assisted Project project, @Assisted IdentifiedUser user, @Assisted ReceiveCommand cmd) { + this.perm = permissionBackend.user(user); this.allUsersName = allUsersName; this.refOperationValidationListeners = refOperationValidationListeners; event = new RefReceivedEvent(); @@ -69,7 +76,7 @@ public class RefOperationValidators { List messages = new ArrayList<>(); boolean withException = false; List listeners = new ArrayList<>(); - listeners.add(new DisallowCreationAndDeletionOfUserBranches(allUsersName)); + listeners.add(new DisallowCreationAndDeletionOfUserBranches(perm, allUsersName)); refOperationValidationListeners.forEach(l -> listeners.add(l)); try { for (RefOperationValidationListener listener : listeners) { @@ -107,9 +114,12 @@ public class RefOperationValidators { private static class DisallowCreationAndDeletionOfUserBranches implements RefOperationValidationListener { + private final PermissionBackend.WithUser perm; private final AllUsersName allUsersName; - DisallowCreationAndDeletionOfUserBranches(AllUsersName allUsersName) { + DisallowCreationAndDeletionOfUserBranches( + PermissionBackend.WithUser perm, AllUsersName allUsersName) { + this.perm = perm; this.allUsersName = allUsersName; } @@ -120,7 +130,9 @@ public class RefOperationValidators { && (refEvent.command.getRefName().startsWith(RefNames.REFS_USERS) && !refEvent.command.getRefName().equals(RefNames.REFS_USERS_DEFAULT))) { 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."); } 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)); } } 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."); } }