Replace PermissionBackend#check calls with #test where appropriate
Calls to #check throw an AuthException and are useful when that
exception is propagated. Most callers got this wrong and called
check where a call to test is the better choice because it returns
a boolean.
This commit is not fixing all callers. It's making iterative progress.
Change-Id: I91958f8780e98db5f3a0a5116341998b72741800
Release-Notes: n/a
diff --git a/java/com/google/gerrit/acceptance/InProcessProtocol.java b/java/com/google/gerrit/acceptance/InProcessProtocol.java
index 15be85c..17ce595 100644
--- a/java/com/google/gerrit/acceptance/InProcessProtocol.java
+++ b/java/com/google/gerrit/acceptance/InProcessProtocol.java
@@ -235,9 +235,9 @@
PermissionBackend.ForProject perm = permissionBackend.currentUser().project(req.project);
try {
- perm.check(ProjectPermission.RUN_UPLOAD_PACK);
- } catch (AuthException e) {
- throw new ServiceNotAuthorizedException(e.getMessage(), e);
+ if (!perm.test(ProjectPermission.RUN_UPLOAD_PACK)) {
+ throw new ServiceNotAuthorizedException("upload pack not permitted");
+ }
} catch (PermissionBackendException e) {
throw new RuntimeException(e);
}
diff --git a/java/com/google/gerrit/server/account/AccountControl.java b/java/com/google/gerrit/server/account/AccountControl.java
index 3f7f3f2..f5f9b3d 100644
--- a/java/com/google/gerrit/server/account/AccountControl.java
+++ b/java/com/google/gerrit/server/account/AccountControl.java
@@ -24,7 +24,6 @@
import com.google.gerrit.entities.PermissionRule;
import com.google.gerrit.exceptions.NoSuchGroupException;
import com.google.gerrit.extensions.common.AccountVisibility;
-import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.group.SystemGroupBackend;
@@ -259,10 +258,7 @@
private boolean viewAll() {
if (viewAll == null) {
try {
- perm.check(GlobalPermission.VIEW_ALL_ACCOUNTS);
- viewAll = true;
- } catch (AuthException e) {
- viewAll = false;
+ viewAll = perm.test(GlobalPermission.VIEW_ALL_ACCOUNTS);
} catch (PermissionBackendException e) {
logger.atFine().withCause(e).log(
"Failed to check %s global capability for user %s",
diff --git a/java/com/google/gerrit/server/account/AccountResolver.java b/java/com/google/gerrit/server/account/AccountResolver.java
index 1eb65ec..8824d56 100644
--- a/java/com/google/gerrit/server/account/AccountResolver.java
+++ b/java/com/google/gerrit/server/account/AccountResolver.java
@@ -27,7 +27,6 @@
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Streams;
import com.google.gerrit.entities.Account;
-import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
import com.google.gerrit.index.Schema;
import com.google.gerrit.server.CurrentUser;
@@ -443,9 +442,10 @@
// more strict here.
boolean canSeeSecondaryEmails = false;
try {
- permissionBackend.user(self.get()).check(GlobalPermission.MODIFY_ACCOUNT);
- canSeeSecondaryEmails = true;
- } catch (AuthException | PermissionBackendException e) {
+ if (permissionBackend.user(self.get()).test(GlobalPermission.MODIFY_ACCOUNT)) {
+ canSeeSecondaryEmails = true;
+ }
+ } catch (PermissionBackendException e) {
// remains false
}
return accountQueryProvider.get().enforceVisibility(true)
diff --git a/java/com/google/gerrit/server/account/GroupControl.java b/java/com/google/gerrit/server/account/GroupControl.java
index d42db60..fd18d3e 100644
--- a/java/com/google/gerrit/server/account/GroupControl.java
+++ b/java/com/google/gerrit/server/account/GroupControl.java
@@ -19,7 +19,6 @@
import com.google.gerrit.entities.AccountGroup;
import com.google.gerrit.entities.GroupDescription;
import com.google.gerrit.exceptions.NoSuchGroupException;
-import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.permissions.GlobalPermission;
import com.google.gerrit.server.permissions.PermissionBackend;
@@ -189,10 +188,7 @@
private boolean canAdministrateServer() {
try {
- perm.check(GlobalPermission.ADMINISTRATE_SERVER);
- return true;
- } catch (AuthException e) {
- return false;
+ return perm.test(GlobalPermission.ADMINISTRATE_SERVER);
} catch (PermissionBackendException e) {
logger.atFine().log(
"Failed to check %s global capability for user %s",
diff --git a/java/com/google/gerrit/server/approval/ApprovalsUtil.java b/java/com/google/gerrit/server/approval/ApprovalsUtil.java
index 8d227f7..227ff70 100644
--- a/java/com/google/gerrit/server/approval/ApprovalsUtil.java
+++ b/java/com/google/gerrit/server/approval/ApprovalsUtil.java
@@ -223,10 +223,7 @@
.statePermitsRead()) {
return false;
}
- permissionBackend.absentUser(accountId).change(notes).check(ChangePermission.READ);
- return true;
- } catch (AuthException e) {
- return false;
+ return permissionBackend.absentUser(accountId).change(notes).test(ChangePermission.READ);
} catch (PermissionBackendException e) {
logger.atWarning().withCause(e).log(
"Failed to check if account %d can see change %d",
@@ -330,11 +327,9 @@
for (Map.Entry<String, Short> vote : approvals.entrySet()) {
String name = vote.getKey();
Short value = vote.getValue();
- try {
- forChange.check(new LabelPermission.WithValue(name, value));
- } catch (AuthException e) {
+ if (!forChange.test(new LabelPermission.WithValue(name, value))) {
throw new AuthException(
- String.format("applying label \"%s\": %d is restricted", name, value), e);
+ String.format("applying label \"%s\": %d is restricted", name, value));
}
}
}
diff --git a/java/com/google/gerrit/server/change/ReviewerJson.java b/java/com/google/gerrit/server/change/ReviewerJson.java
index 6189708..f0f3a8f 100644
--- a/java/com/google/gerrit/server/change/ReviewerJson.java
+++ b/java/com/google/gerrit/server/change/ReviewerJson.java
@@ -26,7 +26,6 @@
import com.google.gerrit.entities.PatchSetApproval;
import com.google.gerrit.entities.SubmitRecord;
import com.google.gerrit.extensions.api.changes.ReviewerInfo;
-import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.server.account.AccountLoader;
import com.google.gerrit.server.approval.ApprovalsUtil;
import com.google.gerrit.server.permissions.LabelPermission;
@@ -129,11 +128,8 @@
continue;
}
- try {
- perm.check(new LabelPermission(type.get()));
+ if (perm.test(new LabelPermission(type.get()))) {
out.approvals.put(name, formatValue((short) 0));
- } catch (AuthException e) {
- // Do nothing.
}
}
}
diff --git a/java/com/google/gerrit/server/change/ReviewerModifier.java b/java/com/google/gerrit/server/change/ReviewerModifier.java
index bfc7841..c98fcaa 100644
--- a/java/com/google/gerrit/server/change/ReviewerModifier.java
+++ b/java/com/google/gerrit/server/change/ReviewerModifier.java
@@ -48,7 +48,6 @@
import com.google.gerrit.extensions.api.changes.ReviewerResult;
import com.google.gerrit.extensions.client.ReviewerState;
import com.google.gerrit.extensions.common.AccountInfo;
-import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
import com.google.gerrit.server.AnonymousUser;
@@ -378,9 +377,10 @@
@Nullable
private ReviewerModification addByEmail(ReviewerInput input, ChangeNotes notes, CurrentUser user)
throws PermissionBackendException {
- try {
- permissionBackend.user(anonymousProvider.get()).change(notes).check(ChangePermission.READ);
- } catch (AuthException e) {
+ if (!permissionBackend
+ .user(anonymousProvider.get())
+ .change(notes)
+ .test(ChangePermission.READ)) {
return fail(
input,
FailureType.OTHER,
@@ -399,15 +399,10 @@
private boolean isValidReviewer(BranchNameKey branch, Account member)
throws PermissionBackendException {
- try {
- // Check ref permission instead of change permission, since change permissions take into
- // account the private bit, whereas adding a user as a reviewer is explicitly allowing them to
- // see private changes.
- permissionBackend.absentUser(member.id()).ref(branch).check(RefPermission.READ);
- return true;
- } catch (AuthException e) {
- return false;
- }
+ // Check ref permission instead of change permission, since change permissions take into
+ // account the private bit, whereas adding a user as a reviewer is explicitly allowing them to
+ // see private changes.
+ return permissionBackend.absentUser(member.id()).ref(branch).test(RefPermission.READ);
}
private ReviewerModification fail(ReviewerInput input, FailureType failureType, String error) {
diff --git a/java/com/google/gerrit/server/change/RevisionJson.java b/java/com/google/gerrit/server/change/RevisionJson.java
index 5a2a0eb..0321fcb 100644
--- a/java/com/google/gerrit/server/change/RevisionJson.java
+++ b/java/com/google/gerrit/server/change/RevisionJson.java
@@ -48,7 +48,6 @@
import com.google.gerrit.extensions.config.DownloadScheme;
import com.google.gerrit.extensions.registration.DynamicMap;
import com.google.gerrit.extensions.registration.Extension;
-import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.server.AnonymousUser;
import com.google.gerrit.server.CurrentUser;
@@ -353,9 +352,7 @@
}
private boolean isWorldReadable(ChangeData cd) throws PermissionBackendException {
- try {
- permissionBackend.user(anonymous).change(cd).check(ChangePermission.READ);
- } catch (AuthException ae) {
+ if (!permissionBackend.user(anonymous).change(cd).test(ChangePermission.READ)) {
return false;
}
ProjectState projectState =
diff --git a/java/com/google/gerrit/server/edit/ChangeEditModifier.java b/java/com/google/gerrit/server/edit/ChangeEditModifier.java
index 81f98e6..232aa6a 100644
--- a/java/com/google/gerrit/server/edit/ChangeEditModifier.java
+++ b/java/com/google/gerrit/server/edit/ChangeEditModifier.java
@@ -407,14 +407,15 @@
// Not allowed to edit if the current patch set is locked.
patchSetUtil.checkPatchSetNotLocked(notes);
- try {
- permissionBackend.currentUser().change(notes).check(ChangePermission.ADD_PATCH_SET);
- projectCache
- .get(notes.getProjectName())
- .orElseThrow(illegalState(notes.getProjectName()))
- .checkStatePermitsWrite();
- } catch (AuthException denied) {
- throw new AuthException("edit not permitted", denied);
+ boolean canEdit =
+ permissionBackend.currentUser().change(notes).test(ChangePermission.ADD_PATCH_SET);
+ canEdit &=
+ projectCache
+ .get(notes.getProjectName())
+ .orElseThrow(illegalState(notes.getProjectName()))
+ .statePermitsWrite();
+ if (!canEdit) {
+ throw new AuthException("edit not permitted");
}
}
diff --git a/java/com/google/gerrit/server/events/EventBroker.java b/java/com/google/gerrit/server/events/EventBroker.java
index 4001a48..2697da5 100644
--- a/java/com/google/gerrit/server/events/EventBroker.java
+++ b/java/com/google/gerrit/server/events/EventBroker.java
@@ -22,7 +22,6 @@
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.entities.Project;
import com.google.gerrit.extensions.registration.DynamicItem;
-import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.lifecycle.LifecycleModule;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.config.GerritInstanceId;
@@ -170,9 +169,8 @@
return false;
}
- permissionBackend.user(user).project(project).check(ProjectPermission.ACCESS);
- return true;
- } catch (AuthException | PermissionBackendException e) {
+ return permissionBackend.user(user).project(project).test(ProjectPermission.ACCESS);
+ } catch (PermissionBackendException e) {
return false;
}
}
@@ -185,15 +183,10 @@
if (!pe.isPresent() || !pe.get().statePermitsRead()) {
return false;
}
- try {
- permissionBackend
- .user(user)
- .change(notesFactory.createChecked(change))
- .check(ChangePermission.READ);
- return true;
- } catch (AuthException e) {
- return false;
- }
+ return permissionBackend
+ .user(user)
+ .change(notesFactory.createChecked(change))
+ .test(ChangePermission.READ);
}
protected boolean isVisibleTo(BranchNameKey branchName, CurrentUser user)
@@ -203,12 +196,7 @@
return false;
}
- try {
- permissionBackend.user(user).ref(branchName).check(RefPermission.READ);
- return true;
- } catch (AuthException e) {
- return false;
- }
+ return permissionBackend.user(user).ref(branchName).test(RefPermission.READ);
}
protected boolean isVisibleTo(Event event, CurrentUser user) throws PermissionBackendException {
diff --git a/java/com/google/gerrit/server/git/receive/AsyncReceiveCommits.java b/java/com/google/gerrit/server/git/receive/AsyncReceiveCommits.java
index 85d7db0..718eec2 100644
--- a/java/com/google/gerrit/server/git/receive/AsyncReceiveCommits.java
+++ b/java/com/google/gerrit/server/git/receive/AsyncReceiveCommits.java
@@ -325,9 +325,7 @@
/** Determine if the user can upload commits. */
public Capable canUpload() throws IOException, PermissionBackendException {
- try {
- perm.check(ProjectPermission.PUSH_AT_LEAST_ONE_REF);
- } catch (AuthException e) {
+ if (!perm.test(ProjectPermission.PUSH_AT_LEAST_ONE_REF)) {
return new Capable("Upload denied for project '" + projectState.getName() + "'");
}
diff --git a/java/com/google/gerrit/server/git/validators/CommitValidators.java b/java/com/google/gerrit/server/git/validators/CommitValidators.java
index 8049df4..5f19758 100644
--- a/java/com/google/gerrit/server/git/validators/CommitValidators.java
+++ b/java/com/google/gerrit/server/git/validators/CommitValidators.java
@@ -639,10 +639,10 @@
}
if (!sboAuthor && !sboCommitter && !sboMe) {
try {
- perm.check(RefPermission.FORGE_COMMITTER);
- } catch (AuthException denied) {
- throw new CommitValidationException(
- "not Signed-off-by author/committer/uploader in message footer", denied);
+ if (!perm.test(RefPermission.FORGE_COMMITTER)) {
+ throw new CommitValidationException(
+ "not Signed-off-by author/committer/uploader in message footer");
+ }
} catch (PermissionBackendException e) {
logger.atSevere().withCause(e).log("cannot check FORGE_COMMITTER");
throw new CommitValidationException("internal auth error");
@@ -673,11 +673,11 @@
return Collections.emptyList();
}
try {
- perm.check(RefPermission.FORGE_AUTHOR);
+ if (!perm.test(RefPermission.FORGE_AUTHOR)) {
+ throw new CommitValidationException(
+ "invalid author", invalidEmail("author", author, user, urlFormatter));
+ }
return Collections.emptyList();
- } catch (AuthException e) {
- throw new CommitValidationException(
- "invalid author", invalidEmail("author", author, user, urlFormatter), e);
} catch (PermissionBackendException e) {
logger.atSevere().withCause(e).log("cannot check FORGE_AUTHOR");
throw new CommitValidationException("internal auth error");
@@ -706,11 +706,11 @@
return Collections.emptyList();
}
try {
- perm.check(RefPermission.FORGE_COMMITTER);
+ if (!perm.test(RefPermission.FORGE_COMMITTER)) {
+ throw new CommitValidationException(
+ "invalid committer", invalidEmail("committer", committer, user, urlFormatter));
+ }
return Collections.emptyList();
- } catch (AuthException e) {
- throw new CommitValidationException(
- "invalid committer", invalidEmail("committer", committer, user, urlFormatter), e);
} catch (PermissionBackendException e) {
logger.atSevere().withCause(e).log("cannot check FORGE_COMMITTER");
throw new CommitValidationException("internal auth error");
diff --git a/java/com/google/gerrit/server/git/validators/MergeValidators.java b/java/com/google/gerrit/server/git/validators/MergeValidators.java
index c514969..40ce671 100644
--- a/java/com/google/gerrit/server/git/validators/MergeValidators.java
+++ b/java/com/google/gerrit/server/git/validators/MergeValidators.java
@@ -195,9 +195,9 @@
if (!oldParent.equals(newParent)) {
if (!allowProjectOwnersToChangeParent) {
try {
- permissionBackend.user(caller).check(GlobalPermission.ADMINISTRATE_SERVER);
- } catch (AuthException e) {
- throw new MergeValidationException(SET_BY_ADMIN, e);
+ if (!permissionBackend.user(caller).test(GlobalPermission.ADMINISTRATE_SERVER)) {
+ throw new MergeValidationException(SET_BY_ADMIN);
+ }
} catch (PermissionBackendException e) {
logger.atWarning().withCause(e).log("Cannot check ADMINISTRATE_SERVER");
throw new MergeValidationException("validation unavailable", e);
diff --git a/java/com/google/gerrit/server/permissions/DefaultRefFilter.java b/java/com/google/gerrit/server/permissions/DefaultRefFilter.java
index 39b0f90..4f10528 100644
--- a/java/com/google/gerrit/server/permissions/DefaultRefFilter.java
+++ b/java/com/google/gerrit/server/permissions/DefaultRefFilter.java
@@ -27,7 +27,6 @@
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.RefNames;
import com.google.gerrit.exceptions.StorageException;
-import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.metrics.Counter0;
import com.google.gerrit.metrics.Description;
import com.google.gerrit.metrics.MetricMaker;
@@ -319,17 +318,14 @@
}
if (visibleChangesCache.isVisible(id)) {
- try {
- // Default to READ_PRIVATE_CHANGES as there is no special permission for reading edits.
- permissionBackendForProject
- .ref(visibleChangesCache.getBranchNameKey(id).branch())
- .check(RefPermission.READ_PRIVATE_CHANGES);
- logger.atFinest().log("Foreign change edit ref is visible: %s", name);
- return true;
- } catch (AuthException e) {
- logger.atFinest().log("Foreign change edit ref is not visible: %s", name);
- return false;
- }
+ // Default to READ_PRIVATE_CHANGES as there is no special permission for reading edits.
+ boolean canRead =
+ permissionBackendForProject
+ .ref(visibleChangesCache.getBranchNameKey(id).branch())
+ .test(RefPermission.READ_PRIVATE_CHANGES);
+ logger.atFinest().log(
+ "Foreign change edit ref is " + (canRead ? "visible" : "invisible") + ": %s", name);
+ return canRead;
}
logger.atFinest().log("Change %d of change edit ref %s is not visible", id.get(), name);
@@ -347,23 +343,14 @@
}
private boolean canReadRef(String ref) throws PermissionBackendException {
- try {
- permissionBackendForProject.ref(ref).check(RefPermission.READ);
- } catch (AuthException e) {
- return false;
- }
- return projectState.statePermitsRead();
+ return permissionBackendForProject.ref(ref).test(RefPermission.READ)
+ && projectState.statePermitsRead();
}
private boolean checkProjectPermission(
PermissionBackend.ForProject forProject, ProjectPermission perm)
throws PermissionBackendException {
- try {
- forProject.check(perm);
- } catch (AuthException e) {
- return false;
- }
- return true;
+ return forProject.test(perm);
}
/**
@@ -394,12 +381,7 @@
} catch (StorageException e) {
throw new PermissionBackendException("can't construct change notes", e);
}
- try {
- permissionBackendForProject.change(notes).check(ChangePermission.READ);
- return true;
- } catch (AuthException e) {
- return false;
- }
+ return permissionBackendForProject.change(notes).test(ChangePermission.READ);
}
@AutoValue
diff --git a/java/com/google/gerrit/server/permissions/PermissionBackend.java b/java/com/google/gerrit/server/permissions/PermissionBackend.java
index 1191db8..fea2827 100644
--- a/java/com/google/gerrit/server/permissions/PermissionBackend.java
+++ b/java/com/google/gerrit/server/permissions/PermissionBackend.java
@@ -246,10 +246,9 @@
Set<Project.NameKey> allowed = Sets.newHashSetWithExpectedSize(projects.size());
for (Project.NameKey project : projects) {
try {
- project(project).check(perm);
- allowed.add(project);
- } catch (AuthException e) {
- // Do not include this project in allowed.
+ if (project(project).test(perm)) {
+ allowed.add(project);
+ }
} catch (PermissionBackendException e) {
if (e.getCause() instanceof RepositoryNotFoundException) {
logger.atWarning().withCause(e).log(
diff --git a/java/com/google/gerrit/server/permissions/ProjectControl.java b/java/com/google/gerrit/server/permissions/ProjectControl.java
index 1203049..664d867 100644
--- a/java/com/google/gerrit/server/permissions/ProjectControl.java
+++ b/java/com/google/gerrit/server/permissions/ProjectControl.java
@@ -165,9 +165,8 @@
boolean isAdmin() {
try {
- permissionBackend.user(user).check(GlobalPermission.ADMINISTRATE_SERVER);
- return true;
- } catch (AuthException | PermissionBackendException e) {
+ return permissionBackend.user(user).test(GlobalPermission.ADMINISTRATE_SERVER);
+ } catch (PermissionBackendException e) {
return false;
}
}
diff --git a/java/com/google/gerrit/server/permissions/RefVisibilityControl.java b/java/com/google/gerrit/server/permissions/RefVisibilityControl.java
index cc6387b..c2d1139 100644
--- a/java/com/google/gerrit/server/permissions/RefVisibilityControl.java
+++ b/java/com/google/gerrit/server/permissions/RefVisibilityControl.java
@@ -25,7 +25,6 @@
import com.google.gerrit.entities.RefNames;
import com.google.gerrit.exceptions.NoSuchGroupException;
import com.google.gerrit.exceptions.StorageException;
-import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.account.GroupControl;
import com.google.gerrit.server.project.NoSuchChangeException;
@@ -170,17 +169,14 @@
return true;
}
- try {
- // Default to READ_PRIVATE_CHANGES as there is no special permission for reading edits.
- projectControl
- .asForProject()
- .ref(cd.change().getDest().branch())
- .check(RefPermission.READ_PRIVATE_CHANGES);
- logger.atFinest().log("Foreign change edit ref is visible: %s", refName);
- return true;
- } catch (AuthException e) {
- logger.atFinest().log("Foreign change edit ref is not visible: %s", refName);
- return false;
- }
+ // Default to READ_PRIVATE_CHANGES as there is no special permission for reading edits.
+ boolean canRead =
+ projectControl
+ .asForProject()
+ .ref(cd.change().getDest().branch())
+ .test(RefPermission.READ_PRIVATE_CHANGES);
+ logger.atFinest().log(
+ "Foreign change edit ref is " + (canRead ? "visible" : "invisible") + ": %s", refName);
+ return canRead;
}
}
diff --git a/java/com/google/gerrit/server/permissions/VisibleChangesCache.java b/java/com/google/gerrit/server/permissions/VisibleChangesCache.java
index 2e47576..552f4f6 100644
--- a/java/com/google/gerrit/server/permissions/VisibleChangesCache.java
+++ b/java/com/google/gerrit/server/permissions/VisibleChangesCache.java
@@ -23,7 +23,6 @@
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.Project;
import com.google.gerrit.exceptions.StorageException;
-import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.server.git.SearchingChangeCacheImpl;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.ChangeNotes.Factory.ChangeNotesResult;
@@ -113,11 +112,8 @@
if (!projectState.statePermitsRead()) {
continue;
}
- try {
- permissionBackendForProject.change(cd).check(ChangePermission.READ);
+ if (permissionBackendForProject.change(cd).test(ChangePermission.READ)) {
visibleChanges.put(cd.getId(), cd.change().getDest());
- } catch (AuthException e) {
- // Do nothing.
}
}
} catch (StorageException e) {
@@ -158,11 +154,8 @@
return null;
}
- try {
- permissionBackendForProject.change(r.notes()).check(ChangePermission.READ);
+ if (permissionBackendForProject.change(r.notes()).test(ChangePermission.READ)) {
return r.notes();
- } catch (AuthException e) {
- // Skip.
}
return null;
}
diff --git a/java/com/google/gerrit/server/project/CreateRefControl.java b/java/com/google/gerrit/server/project/CreateRefControl.java
index 2e76949..0d015d4 100644
--- a/java/com/google/gerrit/server/project/CreateRefControl.java
+++ b/java/com/google/gerrit/server/project/CreateRefControl.java
@@ -124,13 +124,10 @@
Project.NameKey project,
PermissionBackend.ForRef forRef)
throws AuthException, PermissionBackendException, IOException {
- try {
- // If the user has update (push) permission, they can create the ref regardless
- // of whether they are pushing any new objects along with the create.
- forRef.check(RefPermission.UPDATE);
+ // If the user has update (push) permission, they can create the ref regardless
+ // of whether they are pushing any new objects along with the create.
+ if (forRef.test(RefPermission.UPDATE)) {
return;
- } catch (AuthException denied) {
- // Fall through to check reachability.
}
if (reachable.fromRefs(
project,
diff --git a/java/com/google/gerrit/server/project/RemoveReviewerControl.java b/java/com/google/gerrit/server/project/RemoveReviewerControl.java
index 0336e8e..1bc309c 100644
--- a/java/com/google/gerrit/server/project/RemoveReviewerControl.java
+++ b/java/com/google/gerrit/server/project/RemoveReviewerControl.java
@@ -108,30 +108,10 @@
// owner and site admin can remove anyone
PermissionBackend.WithUser withUser = permissionBackend.user(currentUser);
PermissionBackend.ForProject forProject = withUser.project(change.getProject());
- if (check(forProject.ref(change.getDest().branch()), RefPermission.WRITE_CONFIG)
- || check(withUser, GlobalPermission.ADMINISTRATE_SERVER)) {
+ if (forProject.ref(change.getDest().branch()).test(RefPermission.WRITE_CONFIG)
+ || withUser.test(GlobalPermission.ADMINISTRATE_SERVER)) {
return true;
}
return false;
}
-
- private static boolean check(PermissionBackend.ForRef forRef, RefPermission perm)
- throws PermissionBackendException {
- try {
- forRef.check(perm);
- return true;
- } catch (AuthException e) {
- return false;
- }
- }
-
- private static boolean check(PermissionBackend.WithUser withUser, GlobalPermission perm)
- throws PermissionBackendException {
- try {
- withUser.check(perm);
- return true;
- } catch (AuthException e) {
- return false;
- }
- }
}
diff --git a/java/com/google/gerrit/server/query/account/CanSeeChangePredicate.java b/java/com/google/gerrit/server/query/account/CanSeeChangePredicate.java
index 0252a06..e293285 100644
--- a/java/com/google/gerrit/server/query/account/CanSeeChangePredicate.java
+++ b/java/com/google/gerrit/server/query/account/CanSeeChangePredicate.java
@@ -15,7 +15,6 @@
package com.google.gerrit.server.query.account;
import com.google.gerrit.exceptions.StorageException;
-import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.index.query.PostFilterPredicate;
import com.google.gerrit.server.account.AccountState;
import com.google.gerrit.server.notedb.ChangeNotes;
@@ -36,15 +35,12 @@
@Override
public boolean match(AccountState accountState) {
try {
- permissionBackend
+ return permissionBackend
.absentUser(accountState.account().id())
.change(changeNotes)
- .check(ChangePermission.READ);
- return true;
+ .test(ChangePermission.READ);
} catch (PermissionBackendException e) {
throw new StorageException("Failed to check if account can see change", e);
- } catch (AuthException e) {
- return false;
}
}
diff --git a/java/com/google/gerrit/server/query/change/ChangeIsVisibleToPredicate.java b/java/com/google/gerrit/server/query/change/ChangeIsVisibleToPredicate.java
index e3e0312..ac72d15 100644
--- a/java/com/google/gerrit/server/query/change/ChangeIsVisibleToPredicate.java
+++ b/java/com/google/gerrit/server/query/change/ChangeIsVisibleToPredicate.java
@@ -17,7 +17,6 @@
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.entities.Change;
import com.google.gerrit.exceptions.StorageException;
-import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.index.query.IsVisibleToPredicate;
import com.google.gerrit.server.AnonymousUser;
import com.google.gerrit.server.CurrentUser;
@@ -86,8 +85,12 @@
Optional.of(user)
.filter(u -> u instanceof GroupBackedUser || u instanceof InternalUser)
.orElseGet(anonymousUserProvider::get));
+
try {
- withUser.change(cd).check(ChangePermission.READ);
+ if (!withUser.change(cd).test(ChangePermission.READ)) {
+ logger.atFine().log("Filter out non-visisble change: %s", cd);
+ return false;
+ }
} catch (PermissionBackendException e) {
Throwable cause = e.getCause();
if (cause instanceof RepositoryNotFoundException) {
@@ -97,9 +100,6 @@
return false;
}
throw new StorageException("unable to check permissions on change " + cd.getId(), e);
- } catch (AuthException e) {
- logger.atFine().log("Filter out non-visisble change: %s", cd);
- return false;
}
cd.cacheVisibleTo(user);
diff --git a/java/com/google/gerrit/server/restapi/account/Capabilities.java b/java/com/google/gerrit/server/restapi/account/Capabilities.java
index 3d719ff9..469f05d 100644
--- a/java/com/google/gerrit/server/restapi/account/Capabilities.java
+++ b/java/com/google/gerrit/server/restapi/account/Capabilities.java
@@ -71,12 +71,10 @@
}
GlobalOrPluginPermission perm = parse(id);
- try {
- permissionBackend.absentUser(target.getAccountId()).check(perm);
+ if (permissionBackend.absentUser(target.getAccountId()).test(perm)) {
return new AccountResource.Capability(target, globalOrPluginPermissionName(perm));
- } catch (AuthException e) {
- throw new ResourceNotFoundException(id, e);
}
+ throw new ResourceNotFoundException(id);
}
private GlobalOrPluginPermission parse(IdString id) throws ResourceNotFoundException {
diff --git a/java/com/google/gerrit/server/restapi/account/QueryAccounts.java b/java/com/google/gerrit/server/restapi/account/QueryAccounts.java
index e6b4eee..c671562 100644
--- a/java/com/google/gerrit/server/restapi/account/QueryAccounts.java
+++ b/java/com/google/gerrit/server/restapi/account/QueryAccounts.java
@@ -21,7 +21,6 @@
import com.google.gerrit.extensions.client.ListOption;
import com.google.gerrit.extensions.common.AccountInfo;
import com.google.gerrit.extensions.common.AccountVisibility;
-import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.MethodNotAllowedException;
import com.google.gerrit.extensions.restapi.Response;
@@ -186,11 +185,8 @@
if (modifyAccountCapabilityChecked) {
fillOptions.add(FillOptions.SECONDARY_EMAILS);
} else {
- try {
- permissionBackend.currentUser().check(GlobalPermission.MODIFY_ACCOUNT);
+ if (permissionBackend.currentUser().test(GlobalPermission.MODIFY_ACCOUNT)) {
fillOptions.add(FillOptions.SECONDARY_EMAILS);
- } catch (AuthException e) {
- // Do nothing.
}
}
}
diff --git a/java/com/google/gerrit/server/restapi/change/ChangesCollection.java b/java/com/google/gerrit/server/restapi/change/ChangesCollection.java
index 572f704..a0c5b16 100644
--- a/java/com/google/gerrit/server/restapi/change/ChangesCollection.java
+++ b/java/com/google/gerrit/server/restapi/change/ChangesCollection.java
@@ -17,7 +17,6 @@
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.Project;
import com.google.gerrit.extensions.registration.DynamicMap;
-import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.IdString;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
@@ -123,9 +122,7 @@
}
private boolean canRead(ChangeNotes notes) throws PermissionBackendException {
- try {
- permissionBackend.currentUser().change(notes).check(ChangePermission.READ);
- } catch (AuthException e) {
+ if (!permissionBackend.currentUser().change(notes).test(ChangePermission.READ)) {
return false;
}
Optional<ProjectState> projectState = projectCache.get(notes.getProjectName());
