Merge "Don't use #test(Permission) when #check(permission) is expected"
diff --git a/java/com/google/gerrit/httpd/rpc/project/ProjectAccessHandler.java b/java/com/google/gerrit/httpd/rpc/project/ProjectAccessHandler.java
index 0ae9c4c..0bcd8f8 100644
--- a/java/com/google/gerrit/httpd/rpc/project/ProjectAccessHandler.java
+++ b/java/com/google/gerrit/httpd/rpc/project/ProjectAccessHandler.java
@@ -127,8 +127,12 @@
replace(config, toDelete, section);
} else if (AccessSection.isValid(name)) {
- if (checkIfOwner && !forProject.ref(name).test(RefPermission.WRITE_CONFIG)) {
- continue;
+ if (checkIfOwner) {
+ try {
+ forProject.ref(name).check(RefPermission.WRITE_CONFIG);
+ } catch (AuthException e) {
+ continue;
+ }
}
RefPattern.validate(name);
@@ -143,8 +147,15 @@
config.remove(config.getAccessSection(name));
}
- } else if (!checkIfOwner || forProject.ref(name).test(RefPermission.WRITE_CONFIG)) {
+ } else if (!checkIfOwner) {
config.remove(config.getAccessSection(name));
+ } else {
+ try {
+ forProject.ref(name).check(RefPermission.WRITE_CONFIG);
+ config.remove(config.getAccessSection(name));
+ } catch (AuthException e) {
+ // Do nothing.
+ }
}
}
diff --git a/java/com/google/gerrit/server/ApprovalsUtil.java b/java/com/google/gerrit/server/ApprovalsUtil.java
index 0d12eca..3625de6 100644
--- a/java/com/google/gerrit/server/ApprovalsUtil.java
+++ b/java/com/google/gerrit/server/ApprovalsUtil.java
@@ -263,12 +263,17 @@
private boolean canSee(ReviewDb db, ChangeNotes notes, Account.Id accountId) {
try {
- return projectCache.checkedGet(notes.getProjectName()).statePermitsRead()
- && permissionBackend
- .absentUser(accountId)
- .change(notes)
- .database(db)
- .test(ChangePermission.READ);
+ if (!projectCache.checkedGet(notes.getProjectName()).statePermitsRead()) {
+ return false;
+ }
+ permissionBackend
+ .absentUser(accountId)
+ .change(notes)
+ .database(db)
+ .check(ChangePermission.READ);
+ return true;
+ } catch (AuthException e) {
+ return false;
} catch (IOException | PermissionBackendException e) {
logger.atWarning().withCause(e).log(
"Failed to check if account %d can see change %d",
diff --git a/java/com/google/gerrit/server/change/ChangeInserter.java b/java/com/google/gerrit/server/change/ChangeInserter.java
index 8ca364b..e8c55e8 100644
--- a/java/com/google/gerrit/server/change/ChangeInserter.java
+++ b/java/com/google/gerrit/server/change/ChangeInserter.java
@@ -29,6 +29,7 @@
import com.google.gerrit.common.data.LabelTypes;
import com.google.gerrit.extensions.api.changes.NotifyHandling;
import com.google.gerrit.extensions.api.changes.RecipientType;
+import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.reviewdb.client.Account;
@@ -459,18 +460,24 @@
.stream()
.filter(
accountId -> {
+ if (!projectState.statePermitsRead()) {
+ return false;
+ }
+
try {
- return permissionBackend
- .absentUser(accountId)
- .change(notes)
- .database(db)
- .test(ChangePermission.READ)
- && projectState.statePermitsRead();
+ permissionBackend
+ .absentUser(accountId)
+ .change(notes)
+ .database(db)
+ .check(ChangePermission.READ);
+ return true;
} catch (PermissionBackendException e) {
logger.atWarning().withCause(e).log(
"Failed to check if account %d can see change %d",
accountId.get(), notes.getChangeId().get());
return false;
+ } catch (AuthException e) {
+ return false;
}
})
.collect(toSet());
diff --git a/java/com/google/gerrit/server/events/EventBroker.java b/java/com/google/gerrit/server/events/EventBroker.java
index 7d35070..1aff0fa 100644
--- a/java/com/google/gerrit/server/events/EventBroker.java
+++ b/java/com/google/gerrit/server/events/EventBroker.java
@@ -171,21 +171,31 @@
return false;
}
ReviewDb db = dbProvider.get();
- return permissionBackend
- .user(user)
- .change(notesFactory.createChecked(db, change))
- .database(db)
- .test(ChangePermission.READ);
+ try {
+ permissionBackend
+ .user(user)
+ .change(notesFactory.createChecked(db, change))
+ .database(db)
+ .check(ChangePermission.READ);
+ return true;
+ } catch (AuthException e) {
+ return false;
+ }
}
protected boolean isVisibleTo(Branch.NameKey branchName, CurrentUser user)
throws PermissionBackendException {
ProjectState pe = projectCache.get(branchName.getParentKey());
- if (pe == null) {
+ if (pe == null || !pe.statePermitsRead()) {
return false;
}
- return pe.statePermitsRead()
- && permissionBackend.user(user).ref(branchName).test(RefPermission.READ);
+
+ try {
+ permissionBackend.user(user).ref(branchName).check(RefPermission.READ);
+ return true;
+ } catch (AuthException e) {
+ return false;
+ }
}
protected boolean isVisibleTo(Event event, CurrentUser user)
diff --git a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
index d5b1966..e93b2c57 100644
--- a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
+++ b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
@@ -2528,11 +2528,23 @@
if (magicBranch != null
&& (magicBranch.workInProgress || magicBranch.ready)
&& magicBranch.workInProgress != change.isWorkInProgress()
- && (!user.getAccountId().equals(change.getOwner())
- && !permissions.test(ProjectPermission.WRITE_CONFIG)
- && !permissionBackend.user(user).test(GlobalPermission.ADMINISTRATE_SERVER))) {
- reject(inputCommand, ONLY_CHANGE_OWNER_OR_PROJECT_OWNER_CAN_MODIFY_WIP);
- return false;
+ && !user.getAccountId().equals(change.getOwner())) {
+ boolean hasWriteConfigPermission = false;
+ try {
+ permissions.check(ProjectPermission.WRITE_CONFIG);
+ hasWriteConfigPermission = true;
+ } catch (AuthException e) {
+ // Do nothing.
+ }
+
+ if (!hasWriteConfigPermission) {
+ try {
+ permissionBackend.user(user).check(GlobalPermission.ADMINISTRATE_SERVER);
+ } catch (AuthException e1) {
+ reject(inputCommand, ONLY_CHANGE_OWNER_OR_PROJECT_OWNER_CAN_MODIFY_WIP);
+ return false;
+ }
+ }
}
if (magicBranch != null && (magicBranch.edit || magicBranch.draft)) {
diff --git a/java/com/google/gerrit/server/mail/send/ChangeEmail.java b/java/com/google/gerrit/server/mail/send/ChangeEmail.java
index 503fbd0..95ecbba 100644
--- a/java/com/google/gerrit/server/mail/send/ChangeEmail.java
+++ b/java/com/google/gerrit/server/mail/send/ChangeEmail.java
@@ -403,12 +403,19 @@
@Override
protected boolean isVisibleTo(Account.Id to) throws PermissionBackendException {
- return projectState.statePermitsRead()
- && args.permissionBackend
- .absentUser(to)
- .change(changeData)
- .database(args.db)
- .test(ChangePermission.READ);
+ if (!projectState.statePermitsRead()) {
+ return false;
+ }
+ try {
+ args.permissionBackend
+ .absentUser(to)
+ .change(changeData)
+ .database(args.db)
+ .check(ChangePermission.READ);
+ return true;
+ } catch (AuthException e) {
+ return false;
+ }
}
/** Find all users who are authors of any part of this change. */
diff --git a/java/com/google/gerrit/server/permissions/DefaultRefFilter.java b/java/com/google/gerrit/server/permissions/DefaultRefFilter.java
index 3b88080..1e5b3c8 100644
--- a/java/com/google/gerrit/server/permissions/DefaultRefFilter.java
+++ b/java/com/google/gerrit/server/permissions/DefaultRefFilter.java
@@ -299,9 +299,14 @@
Map<Change.Id, Branch.NameKey> visibleChanges = new HashMap<>();
for (ChangeData cd : changeCache.getChangeData(db.get(), project)) {
ChangeNotes notes = changeNotesFactory.createFromIndexedChange(cd.change());
- if (projectState.statePermitsRead()
- && permissionBackendForProject.indexedChange(cd, notes).test(ChangePermission.READ)) {
+ if (!projectState.statePermitsRead()) {
+ continue;
+ }
+ try {
+ permissionBackendForProject.indexedChange(cd, notes).check(ChangePermission.READ);
visibleChanges.put(cd.getId(), cd.change().getDest());
+ } catch (AuthException e) {
+ // Do nothing.
}
}
return visibleChanges;
@@ -334,11 +339,16 @@
"Failed to load change %s in %s", r.id(), projectState.getName());
return null;
}
+
+ if (!projectState.statePermitsRead()) {
+ return null;
+ }
+
try {
- if (projectState.statePermitsRead()
- && permissionBackendForProject.change(r.notes()).test(ChangePermission.READ)) {
- return r.notes();
- }
+ permissionBackendForProject.change(r.notes()).check(ChangePermission.READ);
+ return r.notes();
+ } catch (AuthException e) {
+ // Skip.
} catch (PermissionBackendException e) {
logger.atSevere().withCause(e).log(
"Failed to check permission for %s in %s", r.id(), projectState.getName());
diff --git a/java/com/google/gerrit/server/query/account/AccountQueryBuilder.java b/java/com/google/gerrit/server/query/account/AccountQueryBuilder.java
index e7ffd5e..41d53a2 100644
--- a/java/com/google/gerrit/server/query/account/AccountQueryBuilder.java
+++ b/java/com/google/gerrit/server/query/account/AccountQueryBuilder.java
@@ -19,6 +19,7 @@
import com.google.common.flogger.FluentLogger;
import com.google.common.primitives.Ints;
import com.google.gerrit.common.errors.NotSignedInException;
+import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.index.Index;
import com.google.gerrit.index.Schema;
import com.google.gerrit.index.query.LimitPredicate;
@@ -120,12 +121,17 @@
public Predicate<AccountState> cansee(String change)
throws QueryParseException, OrmException, PermissionBackendException {
ChangeNotes changeNotes = args.changeFinder.findOne(change);
- if (changeNotes == null
- || !args.permissionBackend
- .user(args.getUser())
- .database(args.db)
- .change(changeNotes)
- .test(ChangePermission.READ)) {
+ if (changeNotes == null) {
+ throw error(String.format("change %s not found", change));
+ }
+
+ try {
+ args.permissionBackend
+ .user(args.getUser())
+ .database(args.db)
+ .change(changeNotes)
+ .check(ChangePermission.READ);
+ } catch (AuthException e) {
throw error(String.format("change %s not found", change));
}
@@ -218,7 +224,12 @@
}
private boolean canSeeSecondaryEmails() throws PermissionBackendException, QueryParseException {
- return args.permissionBackend.user(args.getUser()).test(GlobalPermission.MODIFY_ACCOUNT);
+ try {
+ args.permissionBackend.user(args.getUser()).check(GlobalPermission.MODIFY_ACCOUNT);
+ return true;
+ } catch (AuthException e) {
+ return false;
+ }
}
private boolean checkedCanSeeSecondaryEmails() {
diff --git a/java/com/google/gerrit/server/query/account/CanSeeChangePredicate.java b/java/com/google/gerrit/server/query/account/CanSeeChangePredicate.java
index b008092..fb3549c 100644
--- a/java/com/google/gerrit/server/query/account/CanSeeChangePredicate.java
+++ b/java/com/google/gerrit/server/query/account/CanSeeChangePredicate.java
@@ -14,6 +14,7 @@
package com.google.gerrit.server.query.account;
+import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.index.query.PostFilterPredicate;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.account.AccountState;
@@ -40,13 +41,16 @@
@Override
public boolean match(AccountState accountState) throws OrmException {
try {
- return permissionBackend
+ permissionBackend
.absentUser(accountState.getAccount().getId())
.database(db)
.change(changeNotes)
- .test(ChangePermission.READ);
+ .check(ChangePermission.READ);
+ return true;
} catch (PermissionBackendException e) {
throw new OrmException("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 346ac8e..27baef1 100644
--- a/java/com/google/gerrit/server/query/change/ChangeIsVisibleToPredicate.java
+++ b/java/com/google/gerrit/server/query/change/ChangeIsVisibleToPredicate.java
@@ -15,6 +15,7 @@
package com.google.gerrit.server.query.change;
import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.index.query.IsVisibleToPredicate;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.server.ReviewDb;
@@ -83,13 +84,12 @@
throw new OrmException("unable to read project state", e);
}
- boolean visible;
PermissionBackend.WithUser withUser =
user.isIdentifiedUser()
? permissionBackend.absentUser(user.getAccountId())
: permissionBackend.user(anonymousUserProvider.get());
try {
- visible = withUser.indexedChange(cd, notes).database(db).test(ChangePermission.READ);
+ withUser.indexedChange(cd, notes).database(db).check(ChangePermission.READ);
} catch (PermissionBackendException e) {
Throwable cause = e.getCause();
if (cause instanceof RepositoryNotFoundException) {
@@ -98,12 +98,12 @@
return false;
}
throw new OrmException("unable to check permissions on change " + cd.getId(), e);
+ } catch (AuthException e) {
+ return false;
}
- if (visible) {
- cd.cacheVisibleTo(user);
- return true;
- }
- return false;
+
+ cd.cacheVisibleTo(user);
+ return true;
}
@Override
diff --git a/java/com/google/gerrit/server/query/change/EqualsLabelPredicate.java b/java/com/google/gerrit/server/query/change/EqualsLabelPredicate.java
index 19fd4a1..54e22f3 100644
--- a/java/com/google/gerrit/server/query/change/EqualsLabelPredicate.java
+++ b/java/com/google/gerrit/server/query/change/EqualsLabelPredicate.java
@@ -16,6 +16,7 @@
import com.google.gerrit.common.data.LabelType;
import com.google.gerrit.common.data.LabelTypes;
+import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.client.Change;
@@ -125,10 +126,13 @@
PermissionBackend.ForChange perm =
permissionBackend.absentUser(approver).database(dbProvider).change(cd);
ProjectState projectState = projectCache.checkedGet(cd.project());
- return projectState != null
- && projectState.statePermitsRead()
- && perm.test(ChangePermission.READ);
- } catch (PermissionBackendException | IOException e) {
+ if (projectState == null || !projectState.statePermitsRead()) {
+ return false;
+ }
+
+ perm.check(ChangePermission.READ);
+ return true;
+ } catch (PermissionBackendException | IOException | AuthException e) {
return false;
}
}
diff --git a/java/com/google/gerrit/server/restapi/account/Capabilities.java b/java/com/google/gerrit/server/restapi/account/Capabilities.java
index ec16e2b..85f9da0 100644
--- a/java/com/google/gerrit/server/restapi/account/Capabilities.java
+++ b/java/com/google/gerrit/server/restapi/account/Capabilities.java
@@ -71,10 +71,12 @@
}
GlobalOrPluginPermission perm = parse(id);
- if (permissionBackend.user(target).test(perm)) {
+ try {
+ permissionBackend.user(target).check(perm);
return new AccountResource.Capability(target, globalOrPluginPermissionName(perm));
+ } catch (AuthException e) {
+ throw new ResourceNotFoundException(id);
}
- throw new ResourceNotFoundException(id);
}
private GlobalOrPluginPermission parse(IdString id) throws ResourceNotFoundException {
diff --git a/java/com/google/gerrit/server/restapi/account/GetDetail.java b/java/com/google/gerrit/server/restapi/account/GetDetail.java
index c8f99b3..e8d5395 100644
--- a/java/com/google/gerrit/server/restapi/account/GetDetail.java
+++ b/java/com/google/gerrit/server/restapi/account/GetDetail.java
@@ -16,6 +16,7 @@
import com.google.common.collect.Sets;
import com.google.gerrit.extensions.common.AccountDetailInfo;
+import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.RestReadView;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.server.CurrentUser;
@@ -56,12 +57,19 @@
AccountDetailInfo info = new AccountDetailInfo(a.getId().get());
info.registeredOn = a.getRegisteredOn();
info.inactive = !a.isActive() ? true : null;
- Set<FillOptions> fillOptions =
- self.get().hasSameAccountId(rsrc.getUser())
- || permissionBackend.currentUser().test(GlobalPermission.MODIFY_ACCOUNT)
- ? EnumSet.allOf(FillOptions.class)
- : Sets.difference(
+ Set<FillOptions> fillOptions;
+ if (self.get().hasSameAccountId(rsrc.getUser())) {
+ fillOptions = EnumSet.allOf(FillOptions.class);
+ } else {
+ try {
+ permissionBackend.currentUser().check(GlobalPermission.MODIFY_ACCOUNT);
+ fillOptions = EnumSet.allOf(FillOptions.class);
+ } catch (AuthException e) {
+ fillOptions =
+ Sets.difference(
EnumSet.allOf(FillOptions.class), EnumSet.of(FillOptions.SECONDARY_EMAILS));
+ }
+ }
directory.fillAccountInfo(Collections.singleton(info), fillOptions);
return info;
}
diff --git a/java/com/google/gerrit/server/restapi/account/QueryAccounts.java b/java/com/google/gerrit/server/restapi/account/QueryAccounts.java
index 8784d23..2c0512c 100644
--- a/java/com/google/gerrit/server/restapi/account/QueryAccounts.java
+++ b/java/com/google/gerrit/server/restapi/account/QueryAccounts.java
@@ -19,6 +19,7 @@
import com.google.gerrit.extensions.client.ListAccountsOption;
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.RestApiException;
@@ -171,9 +172,15 @@
fillOptions.addAll(AccountLoader.DETAILED_OPTIONS);
fillOptions.add(FillOptions.EMAIL);
- if (modifyAccountCapabilityChecked
- || permissionBackend.currentUser().test(GlobalPermission.MODIFY_ACCOUNT)) {
+ if (modifyAccountCapabilityChecked) {
fillOptions.add(FillOptions.SECONDARY_EMAILS);
+ } else {
+ try {
+ permissionBackend.currentUser().check(GlobalPermission.MODIFY_ACCOUNT);
+ fillOptions.add(FillOptions.SECONDARY_EMAILS);
+ } catch (AuthException e) {
+ // Do nothing.
+ }
}
}
accountLoader = accountLoaderFactory.create(fillOptions);
diff --git a/java/com/google/gerrit/server/restapi/change/PostReviewers.java b/java/com/google/gerrit/server/restapi/change/PostReviewers.java
index 4991c18..a398764 100644
--- a/java/com/google/gerrit/server/restapi/change/PostReviewers.java
+++ b/java/com/google/gerrit/server/restapi/change/PostReviewers.java
@@ -352,14 +352,17 @@
NotifyHandling notify,
ListMultimap<RecipientType, Account.Id> accountsToNotify)
throws PermissionBackendException {
- if (!permissionBackend
- .user(anonymousProvider.get())
- .change(rsrc.getNotes())
- .database(dbProvider)
- .test(ChangePermission.READ)) {
+ try {
+ permissionBackend
+ .user(anonymousProvider.get())
+ .change(rsrc.getNotes())
+ .database(dbProvider)
+ .check(ChangePermission.READ);
+ } catch (AuthException e) {
return fail(
reviewer, MessageFormat.format(ChangeMessages.get().reviewerCantSeeChange, reviewer));
}
+
if (!migration.readChanges()) {
// addByEmail depends on NoteDb.
return fail(
diff --git a/java/com/google/gerrit/server/restapi/change/ReviewerJson.java b/java/com/google/gerrit/server/restapi/change/ReviewerJson.java
index c891a65..29c5649 100644
--- a/java/com/google/gerrit/server/restapi/change/ReviewerJson.java
+++ b/java/com/google/gerrit/server/restapi/change/ReviewerJson.java
@@ -22,6 +22,7 @@
import com.google.gerrit.common.data.LabelTypes;
import com.google.gerrit.common.data.SubmitRecord;
import com.google.gerrit.extensions.api.changes.ReviewerInfo;
+import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.PatchSetApproval;
@@ -132,10 +133,15 @@
for (SubmitRecord.Label label : rec.labels) {
String name = label.label;
LabelType type = labelTypes.byLabel(name);
- if (!out.approvals.containsKey(name)
- && type != null
- && perm.test(new LabelPermission(type))) {
+ if (out.approvals.containsKey(name) || type == null) {
+ continue;
+ }
+
+ try {
+ perm.check(new LabelPermission(type));
out.approvals.put(name, formatValue((short) 0));
+ } catch (AuthException e) {
+ // Do nothing.
}
}
}
diff --git a/java/com/google/gerrit/server/restapi/change/ReviewersUtil.java b/java/com/google/gerrit/server/restapi/change/ReviewersUtil.java
index 779ee5a..c800bde 100644
--- a/java/com/google/gerrit/server/restapi/change/ReviewersUtil.java
+++ b/java/com/google/gerrit/server/restapi/change/ReviewersUtil.java
@@ -27,6 +27,7 @@
import com.google.gerrit.common.data.GroupReference;
import com.google.gerrit.extensions.common.GroupBaseInfo;
import com.google.gerrit.extensions.common.SuggestedReviewerInfo;
+import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.Url;
import com.google.gerrit.index.IndexConfig;
import com.google.gerrit.index.QueryOptions;
@@ -300,10 +301,14 @@
private List<SuggestedReviewerInfo> loadAccounts(List<Account.Id> accountIds)
throws PermissionBackendException {
- Set<FillOptions> fillOptions =
- permissionBackend.currentUser().test(GlobalPermission.MODIFY_ACCOUNT)
- ? EnumSet.of(FillOptions.SECONDARY_EMAILS)
- : EnumSet.noneOf(FillOptions.class);
+ Set<FillOptions> fillOptions;
+ try {
+ permissionBackend.currentUser().check(GlobalPermission.MODIFY_ACCOUNT);
+ fillOptions = EnumSet.of(FillOptions.SECONDARY_EMAILS);
+ } catch (AuthException e) {
+ fillOptions = EnumSet.noneOf(FillOptions.class);
+ }
+
fillOptions.addAll(AccountLoader.DETAILED_OPTIONS);
AccountLoader accountLoader = accountLoaderFactory.create(fillOptions);
diff --git a/java/com/google/gerrit/server/restapi/change/SetReadyForReview.java b/java/com/google/gerrit/server/restapi/change/SetReadyForReview.java
index 8fe5612..faf946f 100644
--- a/java/com/google/gerrit/server/restapi/change/SetReadyForReview.java
+++ b/java/com/google/gerrit/server/restapi/change/SetReadyForReview.java
@@ -65,16 +65,28 @@
protected Response<?> applyImpl(
BatchUpdate.Factory updateFactory, ChangeResource rsrc, Input input)
throws RestApiException, UpdateException, PermissionBackendException {
- Change change = rsrc.getChange();
- if (!rsrc.isUserOwner()
- && !permissionBackend.currentUser().test(GlobalPermission.ADMINISTRATE_SERVER)
- && !permissionBackend
- .currentUser()
- .project(rsrc.getProject())
- .test(ProjectPermission.WRITE_CONFIG)) {
- throw new AuthException("not allowed to set ready for review");
+ if (!rsrc.isUserOwner()) {
+ boolean hasAdministrateServerPermission = false;
+ try {
+ permissionBackend.currentUser().check(GlobalPermission.ADMINISTRATE_SERVER);
+ hasAdministrateServerPermission = true;
+ } catch (AuthException e) {
+ // Skip.
+ }
+
+ if (!hasAdministrateServerPermission) {
+ try {
+ permissionBackend
+ .currentUser()
+ .project(rsrc.getProject())
+ .check(ProjectPermission.WRITE_CONFIG);
+ } catch (AuthException exp) {
+ throw new AuthException("not allowed to set ready for review");
+ }
+ }
}
+ Change change = rsrc.getChange();
if (change.getStatus() != Status.NEW) {
throw new ResourceConflictException("change is " + ChangeUtil.status(change));
}
diff --git a/java/com/google/gerrit/server/restapi/change/SetWorkInProgress.java b/java/com/google/gerrit/server/restapi/change/SetWorkInProgress.java
index 9524903..1f9d81f 100644
--- a/java/com/google/gerrit/server/restapi/change/SetWorkInProgress.java
+++ b/java/com/google/gerrit/server/restapi/change/SetWorkInProgress.java
@@ -65,17 +65,28 @@
protected Response<?> applyImpl(
BatchUpdate.Factory updateFactory, ChangeResource rsrc, Input input)
throws RestApiException, UpdateException, PermissionBackendException {
- Change change = rsrc.getChange();
+ if (!rsrc.isUserOwner()) {
+ boolean hasAdministrateServerPermission = false;
+ try {
+ permissionBackend.currentUser().check(GlobalPermission.ADMINISTRATE_SERVER);
+ hasAdministrateServerPermission = true;
+ } catch (AuthException e) {
+ // Skip.
+ }
- if (!rsrc.isUserOwner()
- && !permissionBackend.currentUser().test(GlobalPermission.ADMINISTRATE_SERVER)
- && !permissionBackend
- .currentUser()
- .project(rsrc.getProject())
- .test(ProjectPermission.WRITE_CONFIG)) {
- throw new AuthException("not allowed to set work in progress");
+ if (!hasAdministrateServerPermission) {
+ try {
+ permissionBackend
+ .currentUser()
+ .project(rsrc.getProject())
+ .check(ProjectPermission.WRITE_CONFIG);
+ } catch (AuthException exp) {
+ throw new AuthException("not allowed to set work in progress");
+ }
+ }
}
+ Change change = rsrc.getChange();
if (change.getStatus() != Status.NEW) {
throw new ResourceConflictException("change is " + ChangeUtil.status(change));
}
diff --git a/java/com/google/gerrit/server/restapi/project/GetAccess.java b/java/com/google/gerrit/server/restapi/project/GetAccess.java
index 6a50c2f..d545f92 100644
--- a/java/com/google/gerrit/server/restapi/project/GetAccess.java
+++ b/java/com/google/gerrit/server/restapi/project/GetAccess.java
@@ -237,11 +237,15 @@
}
}
- if (info.ownerOf.isEmpty()
- && permissionBackend.currentUser().test(GlobalPermission.ADMINISTRATE_SERVER)) {
- // Special case: If the section list is empty, this project has no current
- // access control information. Fall back to site administrators.
- info.ownerOf.add(AccessSection.ALL);
+ if (info.ownerOf.isEmpty()) {
+ try {
+ permissionBackend.currentUser().check(GlobalPermission.ADMINISTRATE_SERVER);
+ // Special case: If the section list is empty, this project has no current
+ // access control information. Fall back to site administrators.
+ info.ownerOf.add(AccessSection.ALL);
+ } catch (AuthException e) {
+ // Do nothing.
+ }
}
if (config.getRevision() != null) {
diff --git a/java/com/google/gerrit/server/restapi/project/ListBranches.java b/java/com/google/gerrit/server/restapi/project/ListBranches.java
index 0bdf979..bf4a547 100644
--- a/java/com/google/gerrit/server/restapi/project/ListBranches.java
+++ b/java/com/google/gerrit/server/restapi/project/ListBranches.java
@@ -24,6 +24,7 @@
import com.google.gerrit.extensions.common.ActionInfo;
import com.google.gerrit.extensions.common.WebLinkInfo;
import com.google.gerrit.extensions.registration.DynamicMap;
+import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestReadView;
@@ -185,9 +186,13 @@
// showing the resolved value, show the name it references.
//
String target = ref.getTarget().getName();
- if (!perm.ref(target).test(RefPermission.READ)) {
+
+ try {
+ perm.ref(target).check(RefPermission.READ);
+ } catch (AuthException e) {
continue;
}
+
if (target.startsWith(Constants.R_HEADS)) {
target = target.substring(Constants.R_HEADS.length());
}
@@ -212,10 +217,13 @@
continue;
}
- if (perm.ref(ref.getName()).test(RefPermission.READ)) {
+ try {
+ perm.ref(ref.getName()).check(RefPermission.READ);
branches.add(
createBranchInfo(
perm.ref(ref.getName()), ref, rsrc.getProjectState(), rsrc.getUser(), targets));
+ } catch (AuthException e) {
+ // Do nothing.
}
}
Collections.sort(branches, new BranchComparator());
diff --git a/java/com/google/gerrit/server/restapi/project/ListDashboards.java b/java/com/google/gerrit/server/restapi/project/ListDashboards.java
index 4ff46cf..06dbdb0 100644
--- a/java/com/google/gerrit/server/restapi/project/ListDashboards.java
+++ b/java/com/google/gerrit/server/restapi/project/ListDashboards.java
@@ -18,6 +18,7 @@
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.extensions.api.projects.DashboardInfo;
+import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.RestReadView;
import com.google.gerrit.reviewdb.client.Project;
@@ -104,8 +105,15 @@
RevWalk rw = new RevWalk(git)) {
List<DashboardInfo> all = new ArrayList<>();
for (Ref ref : git.getRefDatabase().getRefsByPrefix(REFS_DASHBOARDS)) {
- if (perm.ref(ref.getName()).test(RefPermission.READ) && state.statePermitsRead()) {
+ if (!state.statePermitsRead()) {
+ continue;
+ }
+
+ try {
+ perm.ref(ref.getName()).check(RefPermission.READ);
all.addAll(scanDashboards(state.getProject(), git, rw, ref, project, setDefault));
+ } catch (AuthException e) {
+ // Do nothing.
}
}
return all;
diff --git a/java/com/google/gerrit/server/submit/LocalMergeSuperSetComputation.java b/java/com/google/gerrit/server/submit/LocalMergeSuperSetComputation.java
index 06f57b5..9efb976 100644
--- a/java/com/google/gerrit/server/submit/LocalMergeSuperSetComputation.java
+++ b/java/com/google/gerrit/server/submit/LocalMergeSuperSetComputation.java
@@ -25,6 +25,7 @@
import com.google.gerrit.common.data.SubmitTypeRecord;
import com.google.gerrit.extensions.client.SubmitType;
import com.google.gerrit.extensions.registration.DynamicItem;
+import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.reviewdb.client.Branch;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.server.ReviewDb;
@@ -182,14 +183,19 @@
changeSet.ids().contains(cd.getId())
&& (projectState != null)
&& projectState.statePermitsRead();
- if (visible
- && !permissionBackend.user(user).change(cd).database(db).test(ChangePermission.READ)) {
+ if (!visible) {
+ return false;
+ }
+
+ try {
+ permissionBackend.user(user).change(cd).database(db).check(ChangePermission.READ);
+ return true;
+ } catch (AuthException e) {
// We thought the change was visible, but it isn't.
// This can happen if the ACL changes during the
// completeChangeSet computation, for example.
- visible = false;
+ return false;
}
- return visible;
}
private SubmitType submitType(ChangeData cd) throws OrmException {
diff --git a/java/com/google/gerrit/server/submit/MergeSuperSet.java b/java/com/google/gerrit/server/submit/MergeSuperSet.java
index 3e9f068..31bcc2a 100644
--- a/java/com/google/gerrit/server/submit/MergeSuperSet.java
+++ b/java/com/google/gerrit/server/submit/MergeSuperSet.java
@@ -20,6 +20,7 @@
import com.google.common.base.Strings;
import com.google.common.collect.Iterables;
import com.google.gerrit.extensions.registration.DynamicItem;
+import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.CurrentUser;
@@ -100,17 +101,22 @@
List<ChangeData> cds = queryProvider.get().byLegacyChangeId(change.getId());
checkState(cds.size() == 1, "Expected exactly one ChangeData, got " + cds.size());
ChangeData cd = Iterables.getFirst(cds, null);
- ProjectState projectState = projectCache.checkedGet(cd.project());
- ChangeSet changeSet =
- new ChangeSet(
- cd,
- projectState != null
- && projectState.statePermitsRead()
- && permissionBackend
- .user(user)
- .change(cd)
- .database(db)
- .test(ChangePermission.READ));
+
+ boolean visible = false;
+ if (cd != null) {
+ ProjectState projectState = projectCache.checkedGet(cd.project());
+
+ if (projectState.statePermitsRead()) {
+ try {
+ permissionBackend.user(user).change(cd).database(db).check(ChangePermission.READ);
+ visible = true;
+ } catch (AuthException e) {
+ // Do nothing.
+ }
+ }
+ }
+
+ ChangeSet changeSet = new ChangeSet(cd, visible);
if (wholeTopicEnabled(cfg)) {
return completeChangeSetIncludingTopics(db, changeSet, user);
}
@@ -203,8 +209,15 @@
private boolean canRead(ReviewDb db, CurrentUser user, ChangeData cd)
throws PermissionBackendException, IOException {
ProjectState projectState = projectCache.checkedGet(cd.project());
- return projectState != null
- && projectState.statePermitsRead()
- && permissionBackend.user(user).change(cd).database(db).test(ChangePermission.READ);
+ if (projectState == null || !projectState.statePermitsRead()) {
+ return false;
+ }
+
+ try {
+ permissionBackend.user(user).change(cd).database(db).check(ChangePermission.READ);
+ return true;
+ } catch (AuthException e) {
+ return false;
+ }
}
}
diff --git a/java/com/google/gerrit/sshd/ChangeArgumentParser.java b/java/com/google/gerrit/sshd/ChangeArgumentParser.java
index 9fc4cda..2ab13ee 100644
--- a/java/com/google/gerrit/sshd/ChangeArgumentParser.java
+++ b/java/com/google/gerrit/sshd/ChangeArgumentParser.java
@@ -86,15 +86,22 @@
}
for (ChangeNotes notes : matched) {
if (!changes.containsKey(notes.getChangeId())
- && inProject(projectState, notes.getProjectName())
- && (canMaintainServer
- || (permissionBackend
- .currentUser()
- .change(notes)
- .database(db)
- .test(ChangePermission.READ)
- && projectState.statePermitsRead()))) {
- toAdd.add(notes);
+ && inProject(projectState, notes.getProjectName())) {
+ if (canMaintainServer) {
+ toAdd.add(notes);
+ continue;
+ }
+
+ if (!projectState.statePermitsRead()) {
+ continue;
+ }
+
+ try {
+ permissionBackend.currentUser().change(notes).database(db).check(ChangePermission.READ);
+ toAdd.add(notes);
+ } catch (AuthException e) {
+ // Do nothing.
+ }
}
}
