Hide ChangeControl#isVisible and migrate callers to PermissionBackend
Change-Id: I8cec3c3c182e0ca2b684370e5beeae11a95b69a1
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SubmitResolvingMergeCommitIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SubmitResolvingMergeCommitIT.java
index 308c9a5..bb7da11 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SubmitResolvingMergeCommitIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SubmitResolvingMergeCommitIT.java
@@ -26,6 +26,7 @@
import com.google.gerrit.server.change.Submit;
import com.google.gerrit.server.git.ChangeSet;
import com.google.gerrit.server.git.MergeSuperSet;
+import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.testutil.ConfigSuite;
import com.google.gwtorm.server.OrmException;
@@ -304,7 +305,8 @@
}
private void assertChangeSetMergeable(ChangeData change, boolean expected)
- throws MissingObjectException, IncorrectObjectTypeException, IOException, OrmException {
+ throws MissingObjectException, IncorrectObjectTypeException, IOException, OrmException,
+ PermissionBackendException {
ChangeSet cs = mergeSuperSet.get().completeChangeSet(db, change.change(), user(admin));
assertThat(submit.unmergeableChanges(cs).isEmpty()).isEqualTo(expected);
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/common/EventBroker.java b/gerrit-server/src/main/java/com/google/gerrit/common/EventBroker.java
index 3cc7335..8bfd880 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/common/EventBroker.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/common/EventBroker.java
@@ -29,6 +29,7 @@
import com.google.gerrit.server.events.ProjectEvent;
import com.google.gerrit.server.events.RefEvent;
import com.google.gerrit.server.notedb.ChangeNotes;
+import com.google.gerrit.server.permissions.ChangePermission;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.permissions.ProjectPermission;
@@ -86,7 +87,8 @@
}
@Override
- public void postEvent(Change change, ChangeEvent event) throws OrmException {
+ public void postEvent(Change change, ChangeEvent event)
+ throws OrmException, PermissionBackendException {
fireEvent(change, event);
}
@@ -101,7 +103,7 @@
}
@Override
- public void postEvent(Event event) throws OrmException {
+ public void postEvent(Event event) throws OrmException, PermissionBackendException {
fireEvent(event);
}
@@ -111,7 +113,8 @@
}
}
- protected void fireEvent(Change change, ChangeEvent event) throws OrmException {
+ protected void fireEvent(Change change, ChangeEvent event)
+ throws OrmException, PermissionBackendException {
for (UserScopedEventListener listener : listeners) {
if (isVisibleTo(change, listener.getUser())) {
listener.onEvent(event);
@@ -138,7 +141,7 @@
fireEventForUnrestrictedListeners(event);
}
- protected void fireEvent(Event event) throws OrmException {
+ protected void fireEvent(Event event) throws OrmException, PermissionBackendException {
for (UserScopedEventListener listener : listeners) {
if (isVisibleTo(event, listener.getUser())) {
listener.onEvent(event);
@@ -156,7 +159,8 @@
}
}
- protected boolean isVisibleTo(Change change, CurrentUser user) throws OrmException {
+ protected boolean isVisibleTo(Change change, CurrentUser user)
+ throws OrmException, PermissionBackendException {
if (change == null) {
return false;
}
@@ -164,9 +168,12 @@
if (pe == null) {
return false;
}
- ProjectControl pc = pe.controlFor(user);
ReviewDb db = dbProvider.get();
- return pc.controlFor(db, change).isVisible(db);
+ return permissionBackend
+ .user(user)
+ .change(notesFactory.createChecked(db, change))
+ .database(db)
+ .test(ChangePermission.READ);
}
protected boolean isVisibleTo(Branch.NameKey branchName, CurrentUser user) {
@@ -178,7 +185,8 @@
return pc.controlForRef(branchName).isVisible();
}
- protected boolean isVisibleTo(Event event, CurrentUser user) throws OrmException {
+ protected boolean isVisibleTo(Event event, CurrentUser user)
+ throws OrmException, PermissionBackendException {
if (event instanceof RefEvent) {
RefEvent refEvent = (RefEvent) event;
String ref = refEvent.getRefName();
diff --git a/gerrit-server/src/main/java/com/google/gerrit/common/EventDispatcher.java b/gerrit-server/src/main/java/com/google/gerrit/common/EventDispatcher.java
index 20d55d6..947b656 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/common/EventDispatcher.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/common/EventDispatcher.java
@@ -21,6 +21,7 @@
import com.google.gerrit.server.events.Event;
import com.google.gerrit.server.events.ProjectEvent;
import com.google.gerrit.server.events.RefEvent;
+import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gwtorm.server.OrmException;
/** Interface for posting (dispatching) Events */
@@ -31,8 +32,9 @@
* @param change The change that the event is related to
* @param event The event to post
* @throws OrmException on failure to post the event due to DB error
+ * @throws PermissionBackendException on failure of permission checks
*/
- void postEvent(Change change, ChangeEvent event) throws OrmException;
+ void postEvent(Change change, ChangeEvent event) throws OrmException, PermissionBackendException;
/**
* Post a stream event that is related to a branch
@@ -58,6 +60,7 @@
*
* @param event The event to post.
* @throws OrmException on failure to post the event due to DB error
+ * @throws PermissionBackendException on failure of permission checks
*/
- void postEvent(Event event) throws OrmException;
+ void postEvent(Event event) throws OrmException, PermissionBackendException;
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/ApprovalsUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/ApprovalsUtil.java
index e4cca59..3b8723b 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/ApprovalsUtil.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/ApprovalsUtil.java
@@ -47,6 +47,9 @@
import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.notedb.NotesMigration;
import com.google.gerrit.server.notedb.ReviewerStateInternal;
+import com.google.gerrit.server.permissions.ChangePermission;
+import com.google.gerrit.server.permissions.PermissionBackend;
+import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.util.LabelVote;
import com.google.gwtorm.server.OrmException;
@@ -108,6 +111,7 @@
private final IdentifiedUser.GenericFactory userFactory;
private final ChangeControl.GenericFactory changeControlFactory;
private final ApprovalCopier copier;
+ private final PermissionBackend permissionBackend;
@VisibleForTesting
@Inject
@@ -115,11 +119,13 @@
NotesMigration migration,
IdentifiedUser.GenericFactory userFactory,
ChangeControl.GenericFactory changeControlFactory,
- ApprovalCopier copier) {
+ ApprovalCopier copier,
+ PermissionBackend permissionBackend) {
this.migration = migration;
this.userFactory = userFactory;
this.changeControlFactory = changeControlFactory;
this.copier = copier;
+ this.permissionBackend = permissionBackend;
}
/**
@@ -262,8 +268,8 @@
private boolean canSee(ReviewDb db, ChangeNotes notes, Account.Id accountId) {
try {
IdentifiedUser user = userFactory.create(accountId);
- return changeControlFactory.controlFor(notes, user).isVisible(db);
- } catch (OrmException e) {
+ return permissionBackend.user(user).change(notes).database(db).test(ChangePermission.READ);
+ } catch (PermissionBackendException e) {
log.warn(
String.format(
"Failed to check if account %d can see change %d",
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/StarredChanges.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/StarredChanges.java
index 995aaa5..a6c844b 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/StarredChanges.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/StarredChanges.java
@@ -32,6 +32,7 @@
import com.google.gerrit.server.StarredChangesUtil;
import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.change.ChangesCollection;
+import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.query.change.QueryChanges;
import com.google.gwtorm.server.OrmDuplicateKeyException;
import com.google.gwtorm.server.OrmException;
@@ -67,7 +68,7 @@
@Override
public AccountResource.StarredChange parse(AccountResource parent, IdString id)
- throws ResourceNotFoundException, OrmException {
+ throws ResourceNotFoundException, OrmException, PermissionBackendException {
IdentifiedUser user = parent.getUser();
ChangeResource change = changes.parse(TopLevelResource.INSTANCE, id);
if (starredChangesUtil
@@ -104,7 +105,7 @@
return createProvider.get().setChange(changes.parse(TopLevelResource.INSTANCE, id));
} catch (ResourceNotFoundException e) {
throw new UnprocessableEntityException(String.format("change %s not found", id.get()));
- } catch (OrmException e) {
+ } catch (OrmException | PermissionBackendException e) {
log.error("cannot resolve change", e);
throw new UnprocessableEntityException("internal server error");
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/Stars.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/Stars.java
index 52c6cdf..860f396 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/Stars.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/Stars.java
@@ -33,6 +33,7 @@
import com.google.gerrit.server.account.AccountResource.Star;
import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.change.ChangesCollection;
+import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.query.change.QueryChanges;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
@@ -65,7 +66,7 @@
@Override
public Star parse(AccountResource parent, IdString id)
- throws ResourceNotFoundException, OrmException {
+ throws ResourceNotFoundException, OrmException, PermissionBackendException {
IdentifiedUser user = parent.getUser();
ChangeResource change = changes.parse(TopLevelResource.INSTANCE, id);
Set<String> labels = starredChangesUtil.getLabels(user.getAccountId(), change.getId());
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java
index 3e8a146..d400999 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java
@@ -53,7 +53,9 @@
import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.notedb.NotesMigration;
import com.google.gerrit.server.patch.PatchSetInfoFactory;
+import com.google.gerrit.server.permissions.ChangePermission;
import com.google.gerrit.server.permissions.PermissionBackend;
+import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.NoSuchProjectException;
import com.google.gerrit.server.project.ProjectControl;
@@ -475,8 +477,12 @@
accountId -> {
try {
IdentifiedUser user = userFactory.create(accountId);
- return changeControlFactory.controlFor(notes, user).isVisible(db);
- } catch (OrmException e) {
+ return permissionBackend
+ .user(user)
+ .change(notes)
+ .database(db)
+ .test(ChangePermission.READ);
+ } catch (PermissionBackendException e) {
log.warn(
String.format(
"Failed to check if account %d can see change %d",
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangesCollection.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangesCollection.java
index eeb1ab3..d967ab8 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangesCollection.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangesCollection.java
@@ -26,6 +26,9 @@
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.ChangeFinder;
import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.permissions.ChangePermission;
+import com.google.gerrit.server.permissions.PermissionBackend;
+import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.query.change.QueryChanges;
import com.google.gwtorm.server.OrmException;
@@ -44,6 +47,7 @@
private final ChangeFinder changeFinder;
private final CreateChange createChange;
private final ChangeResource.Factory changeResourceFactory;
+ private final PermissionBackend permissionBackend;
@Inject
ChangesCollection(
@@ -53,7 +57,8 @@
DynamicMap<RestView<ChangeResource>> views,
ChangeFinder changeFinder,
CreateChange createChange,
- ChangeResource.Factory changeResourceFactory) {
+ ChangeResource.Factory changeResourceFactory,
+ PermissionBackend permissionBackend) {
this.db = db;
this.user = user;
this.queryFactory = queryFactory;
@@ -61,6 +66,7 @@
this.changeFinder = changeFinder;
this.createChange = createChange;
this.changeResourceFactory = changeResourceFactory;
+ this.permissionBackend = permissionBackend;
}
@Override
@@ -75,7 +81,7 @@
@Override
public ChangeResource parse(TopLevelResource root, IdString id)
- throws ResourceNotFoundException, OrmException {
+ throws ResourceNotFoundException, OrmException, PermissionBackendException {
List<ChangeControl> ctls = changeFinder.find(id.encoded(), user.get());
if (ctls.isEmpty()) {
throw new ResourceNotFoundException(id);
@@ -84,13 +90,14 @@
}
ChangeControl ctl = ctls.get(0);
- if (!ctl.isVisible(db.get())) {
+ if (!canRead(ctl)) {
throw new ResourceNotFoundException(id);
}
return changeResourceFactory.create(ctl);
}
- public ChangeResource parse(Change.Id id) throws ResourceNotFoundException, OrmException {
+ public ChangeResource parse(Change.Id id)
+ throws ResourceNotFoundException, OrmException, PermissionBackendException {
List<ChangeControl> ctls = changeFinder.find(id, user.get());
if (ctls.isEmpty()) {
throw new ResourceNotFoundException(toIdString(id));
@@ -99,7 +106,7 @@
}
ChangeControl ctl = ctls.get(0);
- if (!ctl.isVisible(db.get())) {
+ if (!canRead(ctl)) {
throw new ResourceNotFoundException(toIdString(id));
}
return changeResourceFactory.create(ctl);
@@ -118,4 +125,12 @@
public CreateChange post(TopLevelResource parent) throws RestApiException {
return createChange;
}
+
+ private boolean canRead(ChangeControl ctl) throws PermissionBackendException {
+ return permissionBackend
+ .user(user)
+ .change(ctl.getNotes())
+ .database(db)
+ .test(ChangePermission.READ);
+ }
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/CreateChange.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/CreateChange.java
index 92ad46d..39eb8d4 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/CreateChange.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/CreateChange.java
@@ -52,6 +52,7 @@
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.MergeUtil;
+import com.google.gerrit.server.permissions.ChangePermission;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.permissions.RefPermission;
@@ -195,7 +196,11 @@
throw new UnprocessableEntityException("Base change not found: " + input.baseChange);
}
ChangeControl ctl = Iterables.getOnlyElement(ctls);
- if (!ctl.isVisible(db.get())) {
+ if (!permissionBackend
+ .user(user)
+ .change(ctl.getNotes())
+ .database(db)
+ .test(ChangePermission.READ)) {
throw new UnprocessableEntityException("Base change not found: " + input.baseChange);
}
PatchSet ps = psUtil.current(db.get(), ctl.getNotes());
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/GetRevisionActions.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/GetRevisionActions.java
index 8a6a1ab..cb77fd1 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/GetRevisionActions.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/GetRevisionActions.java
@@ -24,6 +24,7 @@
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.git.ChangeSet;
import com.google.gerrit.server.git.MergeSuperSet;
+import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gwtorm.server.OrmException;
import com.google.gwtorm.server.OrmRuntimeException;
@@ -74,7 +75,7 @@
changeResourceFactory.create(cd.changeControl()).prepareETag(h, user);
}
h.putBoolean(cs.furtherHiddenChanges());
- } catch (IOException | OrmException e) {
+ } catch (IOException | OrmException | PermissionBackendException e) {
throw new OrmRuntimeException(e);
}
return h.hash().toString();
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java
index a7711b6..fb40b41 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java
@@ -56,6 +56,7 @@
import com.google.gerrit.server.mail.send.OutgoingEmailValidator;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.NotesMigration;
+import com.google.gerrit.server.permissions.ChangePermission;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.permissions.RefPermission;
@@ -338,8 +339,12 @@
ReviewerState state,
NotifyHandling notify,
ListMultimap<RecipientType, Account.Id> accountsToNotify)
- throws OrmException {
- if (!rsrc.getControl().forUser(anonymousProvider.get()).isVisible(dbProvider.get())) {
+ throws OrmException, PermissionBackendException {
+ if (!permissionBackend
+ .user(anonymousProvider)
+ .change(rsrc.getNotes())
+ .database(dbProvider)
+ .test(ChangePermission.READ)) {
return fail(
reviewer, MessageFormat.format(ChangeMessages.get().reviewerCantSeeChange, reviewer));
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PreviewSubmit.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PreviewSubmit.java
index 9ef445d..5724941 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PreviewSubmit.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PreviewSubmit.java
@@ -34,6 +34,7 @@
import com.google.gerrit.server.git.MergeOp;
import com.google.gerrit.server.git.MergeOpRepoManager;
import com.google.gerrit.server.git.MergeOpRepoManager.OpenRepo;
+import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.NoSuchProjectException;
import com.google.gerrit.server.update.UpdateException;
@@ -83,7 +84,8 @@
@Override
public BinaryResult apply(RevisionResource rsrc)
- throws OrmException, RestApiException, UpdateException, IOException, ConfigInvalidException {
+ throws OrmException, RestApiException, UpdateException, IOException, ConfigInvalidException,
+ PermissionBackendException {
if (Strings.isNullOrEmpty(format)) {
throw new BadRequestException("format is not specified");
}
@@ -111,7 +113,8 @@
}
private BinaryResult getBundles(RevisionResource rsrc, ArchiveFormat f)
- throws OrmException, RestApiException, UpdateException, IOException, ConfigInvalidException {
+ throws OrmException, RestApiException, UpdateException, IOException, ConfigInvalidException,
+ PermissionBackendException {
ReviewDb db = dbProvider.get();
ChangeControl control = rsrc.getControl();
IdentifiedUser caller = control.getUser().asIdentifiedUser();
@@ -131,7 +134,8 @@
| UpdateException
| IOException
| ConfigInvalidException
- | RuntimeException e) {
+ | RuntimeException
+ | PermissionBackendException e) {
op.close();
throw e;
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Submit.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Submit.java
index 3dd467a..81f830c 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Submit.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Submit.java
@@ -217,7 +217,8 @@
}
public Change mergeChange(RevisionResource rsrc, IdentifiedUser submitter, SubmitInput input)
- throws OrmException, RestApiException, IOException, UpdateException, ConfigInvalidException {
+ throws OrmException, RestApiException, IOException, UpdateException, ConfigInvalidException,
+ PermissionBackendException {
Change change = rsrc.getChange();
if (!change.getStatus().isOpen()) {
throw new ResourceConflictException("change is " + ChangeUtil.status(change));
@@ -340,7 +341,7 @@
ChangeSet cs;
try {
cs = mergeSuperSet.get().completeChangeSet(db, cd.change(), resource.getControl().getUser());
- } catch (OrmException | IOException e) {
+ } catch (OrmException | IOException | PermissionBackendException e) {
throw new OrmRuntimeException(
"Could not determine complete set of changes to be submitted", e);
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/SubmittedTogether.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/SubmittedTogether.java
index 568b50a..ea53dc3 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/SubmittedTogether.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/SubmittedTogether.java
@@ -29,6 +29,7 @@
import com.google.gerrit.server.change.WalkSorter.PatchSetData;
import com.google.gerrit.server.git.ChangeSet;
import com.google.gerrit.server.git.MergeSuperSet;
+import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.query.change.InternalChangeQuery;
import com.google.gwtorm.server.OrmException;
@@ -107,7 +108,7 @@
@Override
public Object apply(ChangeResource resource)
throws AuthException, BadRequestException, ResourceConflictException, IOException,
- OrmException {
+ OrmException, PermissionBackendException {
SubmittedTogetherInfo info = applyInfo(resource);
if (options.isEmpty()) {
return info.changes;
@@ -116,7 +117,7 @@
}
public SubmittedTogetherInfo applyInfo(ChangeResource resource)
- throws AuthException, IOException, OrmException {
+ throws AuthException, IOException, OrmException, PermissionBackendException {
Change c = resource.getChange();
try {
List<ChangeData> cds;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/events/StreamEventsApiListener.java b/gerrit-server/src/main/java/com/google/gerrit/server/events/StreamEventsApiListener.java
index b79f137..318c251 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/events/StreamEventsApiListener.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/events/StreamEventsApiListener.java
@@ -53,6 +53,7 @@
import com.google.gerrit.server.data.RefUpdateAttribute;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.notedb.ChangeNotes;
+import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gwtorm.server.OrmException;
@@ -261,7 +262,7 @@
event.oldAssignee = accountAttributeSupplier(ev.getOldAssignee());
dispatcher.get().postEvent(change, event);
- } catch (OrmException e) {
+ } catch (OrmException | PermissionBackendException e) {
log.error("Failed to dispatch event", e);
}
}
@@ -277,7 +278,7 @@
event.oldTopic = ev.getOldTopic();
dispatcher.get().postEvent(change, event);
- } catch (OrmException e) {
+ } catch (OrmException | PermissionBackendException e) {
log.error("Failed to dispatch event", e);
}
}
@@ -295,7 +296,7 @@
event.uploader = accountAttributeSupplier(ev.getWho());
dispatcher.get().postEvent(change, event);
- } catch (OrmException e) {
+ } catch (OrmException | PermissionBackendException e) {
log.error("Failed to dispatch event", e);
}
}
@@ -315,7 +316,7 @@
approvalsAttributeSupplier(change, ev.getNewApprovals(), ev.getOldApprovals());
dispatcher.get().postEvent(change, event);
- } catch (OrmException e) {
+ } catch (OrmException | PermissionBackendException e) {
log.error("Failed to dispatch event", e);
}
}
@@ -333,7 +334,7 @@
event.reviewer = accountAttributeSupplier(reviewer);
dispatcher.get().postEvent(change, event);
}
- } catch (OrmException e) {
+ } catch (OrmException | PermissionBackendException e) {
log.error("Failed to dispatch event", e);
}
}
@@ -360,7 +361,7 @@
event.removed = hashtagArray(ev.getRemovedHashtags());
dispatcher.get().postEvent(change, event);
- } catch (OrmException e) {
+ } catch (OrmException | PermissionBackendException e) {
log.error("Failed to dispatch event", e);
}
}
@@ -399,7 +400,7 @@
event.uploader = accountAttributeSupplier(ev.getWho());
dispatcher.get().postEvent(change, event);
- } catch (OrmException e) {
+ } catch (OrmException | PermissionBackendException e) {
log.error("Failed to dispatch event", e);
}
}
@@ -419,7 +420,7 @@
event.approvals = approvalsAttributeSupplier(change, ev.getApprovals(), ev.getOldApprovals());
dispatcher.get().postEvent(change, event);
- } catch (OrmException e) {
+ } catch (OrmException | PermissionBackendException e) {
log.error("Failed to dispatch event", e);
}
}
@@ -437,7 +438,7 @@
event.reason = ev.getReason();
dispatcher.get().postEvent(change, event);
- } catch (OrmException e) {
+ } catch (OrmException | PermissionBackendException e) {
log.error("Failed to dispatch event", e);
}
}
@@ -455,7 +456,7 @@
event.newRev = ev.getNewRevisionId();
dispatcher.get().postEvent(change, event);
- } catch (OrmException e) {
+ } catch (OrmException | PermissionBackendException e) {
log.error("Failed to dispatch event", e);
}
}
@@ -473,7 +474,7 @@
event.reason = ev.getReason();
dispatcher.get().postEvent(change, event);
- } catch (OrmException e) {
+ } catch (OrmException | PermissionBackendException e) {
log.error("Failed to dispatch event", e);
}
}
@@ -493,7 +494,7 @@
event.approvals = approvalsAttributeSupplier(change, ev.getApprovals(), ev.getOldApprovals());
dispatcher.get().postEvent(change, event);
- } catch (OrmException e) {
+ } catch (OrmException | PermissionBackendException e) {
log.error("Failed to dispatch event", e);
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java
index ea28fa9..9d2286e 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java
@@ -63,6 +63,7 @@
import com.google.gerrit.server.git.strategy.SubmitStrategyListener;
import com.google.gerrit.server.git.validators.MergeValidationException;
import com.google.gerrit.server.git.validators.MergeValidators;
+import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.NoSuchProjectException;
import com.google.gerrit.server.project.SubmitRuleOptions;
@@ -423,6 +424,7 @@
* @param submitInput parameters regarding the merge
* @throws OrmException an error occurred reading or writing the database.
* @throws RestApiException if an error occurred.
+ * @throws PermissionBackendException if permissions can't be checked
*/
public void merge(
ReviewDb db,
@@ -431,7 +433,8 @@
boolean checkSubmitRules,
SubmitInput submitInput,
boolean dryrun)
- throws OrmException, RestApiException, UpdateException, IOException, ConfigInvalidException {
+ throws OrmException, RestApiException, UpdateException, IOException, ConfigInvalidException,
+ PermissionBackendException {
this.submitInput = submitInput;
this.accountsToNotify = notifyUtil.resolveAccounts(submitInput.notifyDetails);
this.dryrun = dryrun;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeSuperSet.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeSuperSet.java
index e880543..72e422b 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeSuperSet.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeSuperSet.java
@@ -37,6 +37,9 @@
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.git.MergeOpRepoManager.OpenRepo;
import com.google.gerrit.server.index.change.ChangeField;
+import com.google.gerrit.server.permissions.ChangePermission;
+import com.google.gerrit.server.permissions.PermissionBackend;
+import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gerrit.server.project.NoSuchProjectException;
import com.google.gerrit.server.project.SubmitRuleEvaluator;
@@ -101,6 +104,7 @@
private final ChangeData.Factory changeDataFactory;
private final Provider<InternalChangeQuery> queryProvider;
private final Provider<MergeOpRepoManager> repoManagerProvider;
+ private final PermissionBackend permissionBackend;
private final Config cfg;
private final Map<QueryKey, List<ChangeData>> queryCache;
private final Map<Branch.NameKey, Optional<RevCommit>> heads;
@@ -115,13 +119,15 @@
Accounts accounts,
ChangeData.Factory changeDataFactory,
Provider<InternalChangeQuery> queryProvider,
- Provider<MergeOpRepoManager> repoManagerProvider) {
+ Provider<MergeOpRepoManager> repoManagerProvider,
+ PermissionBackend permissionBackend) {
this.cfg = cfg;
this.accountCache = accountCache;
this.accounts = accounts;
this.changeDataFactory = changeDataFactory;
this.queryProvider = queryProvider;
this.repoManagerProvider = repoManagerProvider;
+ this.permissionBackend = permissionBackend;
queryCache = new HashMap<>();
heads = new HashMap<>();
}
@@ -134,11 +140,13 @@
}
public ChangeSet completeChangeSet(ReviewDb db, Change change, CurrentUser user)
- throws IOException, OrmException {
+ throws IOException, OrmException, PermissionBackendException {
try {
ChangeData cd = changeDataFactory.create(db, change.getProject(), change.getId());
cd.changeControl(user);
- ChangeSet cs = new ChangeSet(cd, cd.changeControl().isVisible(db, cd));
+ ChangeSet cs =
+ new ChangeSet(
+ cd, permissionBackend.user(user).change(cd).database(db).test(ChangePermission.READ));
if (Submit.wholeTopicEnabled(cfg)) {
return completeChangeSetIncludingTopics(db, cs, user);
}
@@ -212,7 +220,7 @@
}
private ChangeSet completeChangeSetWithoutTopic(ReviewDb db, ChangeSet changes, CurrentUser user)
- throws IOException, OrmException {
+ throws IOException, OrmException, PermissionBackendException {
Collection<ChangeData> visibleChanges = new ArrayList<>();
Collection<ChangeData> nonVisibleChanges = new ArrayList<>();
@@ -231,7 +239,7 @@
+ " at ChangeData creation time");
boolean visible = changes.ids().contains(cd.getId());
- if (visible && !cd.changeControl().isVisible(db, cd)) {
+ if (visible && !canRead(db, user, cd)) {
// We thought the change was visible, but it isn't.
// This can happen if the ACL changes during the
// completeChangeSet computation, for example.
@@ -357,7 +365,7 @@
CurrentUser user,
Set<String> topicsSeen,
Set<String> visibleTopicsSeen)
- throws OrmException {
+ throws OrmException, PermissionBackendException {
List<ChangeData> visibleChanges = new ArrayList<>();
List<ChangeData> nonVisibleChanges = new ArrayList<>();
@@ -370,7 +378,7 @@
for (ChangeData topicCd : query().byTopicOpen(topic)) {
try {
topicCd.changeControl(user);
- if (topicCd.changeControl().isVisible(db, topicCd)) {
+ if (canRead(db, user, topicCd)) {
visibleChanges.add(topicCd);
} else {
nonVisibleChanges.add(topicCd);
@@ -402,7 +410,8 @@
}
private ChangeSet completeChangeSetIncludingTopics(
- ReviewDb db, ChangeSet changes, CurrentUser user) throws IOException, OrmException {
+ ReviewDb db, ChangeSet changes, CurrentUser user)
+ throws IOException, OrmException, PermissionBackendException {
Set<String> topicsSeen = new HashSet<>();
Set<String> visibleTopicsSeen = new HashSet<>();
int oldSeen;
@@ -443,4 +452,9 @@
logError(msg);
throw new OrmException(msg);
}
+
+ private boolean canRead(ReviewDb db, CurrentUser user, ChangeData cd)
+ throws PermissionBackendException {
+ return permissionBackend.user(user).change(cd).database(db).test(ChangePermission.READ);
+ }
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java
index 42fb1b3..40120df 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java
@@ -819,7 +819,8 @@
| OrmException
| UpdateException
| IOException
- | ConfigInvalidException e) {
+ | ConfigInvalidException
+ | PermissionBackendException e) {
logError("Error submitting changes to " + project.getName(), e);
reject(magicBranchCmd, "error during submit");
}
@@ -2264,7 +2265,8 @@
}
private void submit(Collection<CreateRequest> create, Collection<ReplaceRequest> replace)
- throws OrmException, RestApiException, UpdateException, IOException, ConfigInvalidException {
+ throws OrmException, RestApiException, UpdateException, IOException, ConfigInvalidException,
+ PermissionBackendException {
Map<ObjectId, Change> bySha = Maps.newHashMapWithExpectedSize(create.size() + replace.size());
for (CreateRequest r : create) {
checkNotNull(r.change, "cannot submit new change %s; op may not have run", r.changeId);
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 93aa361..2afea25 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,8 +30,10 @@
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.ChangePermission;
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.project.ProjectControl;
import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.server.query.change.ChangeData;
@@ -262,12 +264,16 @@
try {
Map<Change.Id, Branch.NameKey> visibleChanges = new HashMap<>();
for (ChangeData cd : changeCache.getChangeData(db.get(), project.getNameKey())) {
- if (projectCtl.controlForIndexedChange(cd.change()).isVisible(db.get(), cd)) {
+ if (permissionBackend
+ .user(user)
+ .indexedChange(cd, changeNotesFactory.createFromIndexedChange(cd.change()))
+ .database(db)
+ .test(ChangePermission.READ)) {
visibleChanges.put(cd.getId(), cd.change().getDest());
}
}
return visibleChanges;
- } catch (OrmException e) {
+ } catch (OrmException | PermissionBackendException e) {
log.error(
"Cannot load changes for project "
+ project.getName()
@@ -282,12 +288,12 @@
try {
Map<Change.Id, Branch.NameKey> visibleChanges = new HashMap<>();
for (ChangeNotes cn : changeNotesFactory.scan(git, db.get(), project)) {
- if (projectCtl.controlFor(cn).isVisible(db.get())) {
+ if (permissionBackend.user(user).change(cn).database(db).test(ChangePermission.READ)) {
visibleChanges.put(cn.getChangeId(), cn.getChange().getDest());
}
}
return visibleChanges;
- } catch (IOException | OrmException e) {
+ } catch (IOException | OrmException | PermissionBackendException e) {
log.error(
"Cannot load changes for project " + project + ", assuming no changes are visible", e);
return Collections.emptyMap();
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/ChangeEmail.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/ChangeEmail.java
index 15a4b13..7bef0a6 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/ChangeEmail.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/ChangeEmail.java
@@ -38,6 +38,7 @@
import com.google.gerrit.server.patch.PatchListEntry;
import com.google.gerrit.server.patch.PatchListNotAvailableException;
import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException;
+import com.google.gerrit.server.permissions.ChangePermission;
import com.google.gerrit.server.permissions.GlobalPermission;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.ProjectState;
@@ -405,12 +406,12 @@
}
@Override
- protected boolean isVisibleTo(Account.Id to) throws OrmException {
- return projectState == null
- || projectState
- .controlFor(args.identifiedUserFactory.create(to))
- .controlFor(args.db.get(), change)
- .isVisible(args.db.get());
+ protected boolean isVisibleTo(Account.Id to) throws OrmException, PermissionBackendException {
+ return args.permissionBackend
+ .user(args.identifiedUserFactory.create(to))
+ .change(changeData)
+ .database(args.db.get())
+ .test(ChangePermission.READ);
}
/** Find all users who are authors of any part of this change. */
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/OutgoingEmail.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/OutgoingEmail.java
index 4e204ce..e569adf 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/OutgoingEmail.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/OutgoingEmail.java
@@ -31,6 +31,7 @@
import com.google.gerrit.server.account.AccountState;
import com.google.gerrit.server.mail.Address;
import com.google.gerrit.server.mail.send.EmailHeader.AddressList;
+import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.validators.OutgoingEmailValidationListener;
import com.google.gerrit.server.validators.ValidationException;
import com.google.gwtorm.server.OrmException;
@@ -479,7 +480,7 @@
rcptTo.add(to);
add(rt, toAddress(to), override);
}
- } catch (OrmException e) {
+ } catch (OrmException | PermissionBackendException e) {
log.error("Error reading database for account: " + to, e);
}
}
@@ -487,9 +488,10 @@
/**
* @param to account.
* @throws OrmException
+ * @throws PermissionBackendException
* @return whether this email is visible to the given account.
*/
- protected boolean isVisibleTo(Account.Id to) throws OrmException {
+ protected boolean isVisibleTo(Account.Id to) throws OrmException, PermissionBackendException {
return true;
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/permissions/FailedPermissionBackend.java b/gerrit-server/src/main/java/com/google/gerrit/server/permissions/FailedPermissionBackend.java
index 24f5164..4c6e6753 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/permissions/FailedPermissionBackend.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/permissions/FailedPermissionBackend.java
@@ -119,7 +119,12 @@
}
@Override
- public ForChange change(ChangeNotes cd) {
+ public ForChange change(ChangeNotes notes) {
+ return new FailedChange(message, cause);
+ }
+
+ @Override
+ public ForChange indexedChange(ChangeData cd, ChangeNotes notes) {
return new FailedChange(message, cause);
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/permissions/PermissionBackend.java b/gerrit-server/src/main/java/com/google/gerrit/server/permissions/PermissionBackend.java
index 93db963..522eccb 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/permissions/PermissionBackend.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/permissions/PermissionBackend.java
@@ -139,6 +139,15 @@
return ref(notes.getChange().getDest()).change(notes);
}
+ /**
+ * @return instance scoped for the change loaded from index, and its destination ref and
+ * project. This method should only be used when database access is harmful and potentially
+ * stale data from the index is acceptable.
+ */
+ public ForChange indexedChange(ChangeData cd, ChangeNotes notes) {
+ return ref(notes.getChange().getDest()).indexedChange(cd, notes);
+ }
+
/** Verify scoped user can {@code perm}, throwing if denied. */
public abstract void check(GlobalOrPluginPermission perm)
throws AuthException, PermissionBackendException;
@@ -269,6 +278,12 @@
/** @return instance scoped to change. */
public abstract ForChange change(ChangeNotes notes);
+ /**
+ * @return instance scoped to change loaded from index. This method should only be used when
+ * database access is harmful and potentially stale data from the index is acceptable.
+ */
+ public abstract ForChange indexedChange(ChangeData cd, ChangeNotes notes);
+
/** Verify scoped user can {@code perm}, throwing if denied. */
public abstract void check(RefPermission perm) throws AuthException, PermissionBackendException;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java
index 94f5ebf..d0b7ad4 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java
@@ -136,18 +136,6 @@
return create(refControl, notesFactory.create(db, project, changeId));
}
- /**
- * Create a change control for a change that was loaded from index. This method should only be
- * used when database access is harmful and potentially stale data from the index is acceptable.
- *
- * @param refControl ref control
- * @param change change loaded from secondary index
- * @return change control
- */
- ChangeControl createForIndexedChange(RefControl refControl, Change change) {
- return create(refControl, notesFactory.createFromIndexedChange(change));
- }
-
ChangeControl create(RefControl refControl, ChangeNotes notes) {
return new ChangeControl(changeDataFactory, approvalsUtil, refControl, notes, patchSetUtil);
}
@@ -209,12 +197,12 @@
}
/** Can this user see this change? */
- public boolean isVisible(ReviewDb db) throws OrmException {
+ boolean isVisible(ReviewDb db) throws OrmException {
return isVisible(db, null);
}
/** Can this user see this change? */
- public boolean isVisible(ReviewDb db, @Nullable ChangeData cd) throws OrmException {
+ private boolean isVisible(ReviewDb db, @Nullable ChangeData cd) throws OrmException {
if (getChange().isPrivate() && !isPrivateVisible(db, cd)) {
return false;
}
@@ -226,6 +214,7 @@
/** Can this user see the given patchset? */
public boolean isPatchVisible(PatchSet ps, ReviewDb db) throws OrmException {
+ // TODO(hiesel) These don't need to be migrated, just remove after support for drafts is removed
if (ps != null && ps.isDraft() && !isDraftVisible(db, null)) {
return false;
}
@@ -234,6 +223,7 @@
/** Can this user see the given patchset? */
public boolean isPatchVisible(PatchSet ps, ChangeData cd) throws OrmException {
+ // TODO(hiesel) These don't need to be migrated, just remove after support for drafts is removed
checkArgument(
cd.getId().equals(ps.getId().getParentKey()), "%s not for change %s", ps, cd.getId());
if (ps.isDraft() && !isDraftVisible(cd.db(), cd)) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectControl.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectControl.java
index a1ab177..c7e6e83 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectControl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectControl.java
@@ -190,17 +190,6 @@
controlForRef(change.getDest()), db, change.getProject(), change.getId());
}
- /**
- * Create a change control for a change that was loaded from index. This method should only be
- * used when database access is harmful and potentially stale data from the index is acceptable.
- *
- * @param change change loaded from secondary index
- * @return change control
- */
- public ChangeControl controlForIndexedChange(Change change) {
- return changeControlFactory.createForIndexedChange(controlForRef(change.getDest()), change);
- }
-
public ChangeControl controlFor(ChangeNotes notes) {
return changeControlFactory.create(controlForRef(notes.getChange().getDest()), notes);
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java
index 4bb823e..b12db61 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java
@@ -713,6 +713,11 @@
}
@Override
+ public ForChange indexedChange(ChangeData cd, ChangeNotes notes) {
+ return getProjectControl().controlFor(notes).asForChange(cd, db);
+ }
+
+ @Override
public void check(RefPermission perm) throws AuthException, PermissionBackendException {
if (!can(perm)) {
throw new AuthException(perm.describeForException() + " not permitted");
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeIsVisibleToPredicate.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeIsVisibleToPredicate.java
index fa08f53..15fd190 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeIsVisibleToPredicate.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeIsVisibleToPredicate.java
@@ -18,6 +18,9 @@
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.notedb.ChangeNotes;
+import com.google.gerrit.server.permissions.ChangePermission;
+import com.google.gerrit.server.permissions.PermissionBackend;
+import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gerrit.server.query.IsVisibleToPredicate;
@@ -29,17 +32,20 @@
protected final ChangeNotes.Factory notesFactory;
protected final ChangeControl.GenericFactory changeControl;
protected final CurrentUser user;
+ protected final PermissionBackend permissionBackend;
public ChangeIsVisibleToPredicate(
Provider<ReviewDb> db,
ChangeNotes.Factory notesFactory,
ChangeControl.GenericFactory changeControlFactory,
- CurrentUser user) {
+ CurrentUser user,
+ PermissionBackend permissionBackend) {
super(ChangeQueryBuilder.FIELD_VISIBLETO, describe(user));
this.db = db;
this.notesFactory = notesFactory;
this.changeControl = changeControlFactory;
this.user = user;
+ this.permissionBackend = permissionBackend;
}
@Override
@@ -47,21 +53,35 @@
if (cd.fastIsVisibleTo(user)) {
return true;
}
+ Change change;
try {
- Change c = cd.change();
- if (c == null) {
+ change = cd.change();
+ if (change == null) {
return false;
}
-
- ChangeNotes notes = notesFactory.createFromIndexedChange(c);
- ChangeControl cc = changeControl.controlFor(notes, user);
- if (cc.isVisible(db.get(), cd)) {
- cd.cacheVisibleTo(cc);
- return true;
- }
} catch (NoSuchChangeException e) {
// Ignored
+ return false;
}
+
+ ChangeNotes notes = notesFactory.createFromIndexedChange(change);
+ ChangeControl cc = changeControl.controlFor(notes, user);
+ boolean visible;
+ try {
+ visible =
+ permissionBackend
+ .user(user)
+ .indexedChange(cd, notes)
+ .database(db)
+ .test(ChangePermission.READ);
+ } catch (PermissionBackendException e) {
+ throw new OrmException("unable to check permissions", e);
+ }
+ if (visible) {
+ cd.cacheVisibleTo(cc);
+ return true;
+ }
+
return false;
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
index aecfc42..980b0dc 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
@@ -940,7 +940,7 @@
public Predicate<ChangeData> visibleto(CurrentUser user) {
return new ChangeIsVisibleToPredicate(
- args.db, args.notesFactory, args.changeControlGenericFactory, user);
+ args.db, args.notesFactory, args.changeControlGenericFactory, user, args.permissionBackend);
}
public Predicate<ChangeData> is_visible() throws QueryParseException {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryProcessor.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryProcessor.java
index f4064f5..eeb6d01 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryProcessor.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryProcessor.java
@@ -30,6 +30,7 @@
import com.google.gerrit.server.index.change.ChangeSchemaDefinitions;
import com.google.gerrit.server.index.change.IndexedChangeQuery;
import com.google.gerrit.server.notedb.ChangeNotes;
+import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.query.Predicate;
import com.google.gerrit.server.query.QueryProcessor;
@@ -55,6 +56,7 @@
private final ChangeControl.GenericFactory changeControlFactory;
private final ChangeNotes.Factory notesFactory;
private final DynamicMap<ChangeAttributeFactory> attributeFactories;
+ private final PermissionBackend permissionBackend;
static {
// It is assumed that basic rewrites do not touch visibleto predicates.
@@ -74,7 +76,8 @@
Provider<ReviewDb> db,
ChangeControl.GenericFactory changeControlFactory,
ChangeNotes.Factory notesFactory,
- DynamicMap<ChangeAttributeFactory> attributeFactories) {
+ DynamicMap<ChangeAttributeFactory> attributeFactories,
+ PermissionBackend permissionBackend) {
super(
userProvider,
limitsFactory,
@@ -88,6 +91,7 @@
this.changeControlFactory = changeControlFactory;
this.notesFactory = notesFactory;
this.attributeFactories = attributeFactories;
+ this.permissionBackend = permissionBackend;
}
@Override
@@ -130,7 +134,8 @@
protected Predicate<ChangeData> enforceVisibility(Predicate<ChangeData> pred) {
return new AndChangeSource(
pred,
- new ChangeIsVisibleToPredicate(db, notesFactory, changeControlFactory, userProvider.get()),
+ new ChangeIsVisibleToPredicate(
+ db, notesFactory, changeControlFactory, userProvider.get(), permissionBackend),
start);
}
}
diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/ChangeArgumentParser.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/ChangeArgumentParser.java
index e64ab0e..0801447 100644
--- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/ChangeArgumentParser.java
+++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/ChangeArgumentParser.java
@@ -25,6 +25,7 @@
import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.change.ChangesCollection;
import com.google.gerrit.server.notedb.ChangeNotes;
+import com.google.gerrit.server.permissions.ChangePermission;
import com.google.gerrit.server.permissions.GlobalPermission;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
@@ -68,13 +69,13 @@
}
public void addChange(String id, Map<Change.Id, ChangeResource> changes)
- throws UnloggedFailure, OrmException {
+ throws UnloggedFailure, OrmException, PermissionBackendException {
addChange(id, changes, null);
}
public void addChange(
String id, Map<Change.Id, ChangeResource> changes, ProjectControl projectControl)
- throws UnloggedFailure, OrmException {
+ throws UnloggedFailure, OrmException, PermissionBackendException {
addChange(id, changes, projectControl, true);
}
@@ -83,7 +84,7 @@
Map<Change.Id, ChangeResource> changes,
ProjectControl projectControl,
boolean useIndex)
- throws UnloggedFailure, OrmException {
+ throws UnloggedFailure, OrmException, PermissionBackendException {
List<ChangeControl> matched =
useIndex ? changeFinder.find(id, currentUser) : changeFromNotesFactory(id, currentUser);
List<ChangeControl> toAdd = new ArrayList<>(changes.size());
@@ -97,7 +98,12 @@
for (ChangeControl ctl : matched) {
if (!changes.containsKey(ctl.getId())
&& inProject(projectControl, ctl.getProject())
- && (canMaintainServer || ctl.isVisible(db))) {
+ && (canMaintainServer
+ || permissionBackend
+ .user(currentUser)
+ .change(ctl.getNotes())
+ .database(db)
+ .test(ChangePermission.READ))) {
toAdd.add(ctl);
}
}
diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/IndexChangesCommand.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/IndexChangesCommand.java
index 86209fe..821257c 100644
--- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/IndexChangesCommand.java
+++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/IndexChangesCommand.java
@@ -17,6 +17,7 @@
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.change.Index;
+import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.sshd.ChangeArgumentParser;
import com.google.gerrit.sshd.CommandMetaData;
import com.google.gerrit.sshd.SshCommand;
@@ -42,7 +43,7 @@
void addChange(String token) {
try {
changeArgumentParser.addChange(token, changes, null, false);
- } catch (UnloggedFailure | OrmException e) {
+ } catch (UnloggedFailure | OrmException | PermissionBackendException e) {
writeError("warning", e.getMessage());
}
}
diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/SetReviewersCommand.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/SetReviewersCommand.java
index 5ea5bf7..026f9b7 100644
--- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/SetReviewersCommand.java
+++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/SetReviewersCommand.java
@@ -23,6 +23,7 @@
import com.google.gerrit.server.change.DeleteReviewer;
import com.google.gerrit.server.change.PostReviewers;
import com.google.gerrit.server.change.ReviewerResource;
+import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.ProjectControl;
import com.google.gerrit.sshd.ChangeArgumentParser;
import com.google.gerrit.sshd.CommandMetaData;
@@ -79,6 +80,8 @@
throw new IllegalArgumentException(e.getMessage(), e);
} catch (OrmException e) {
throw new IllegalArgumentException("database is down", e);
+ } catch (PermissionBackendException e) {
+ throw new IllegalArgumentException("can't check permissions", e);
}
}
diff --git a/plugins/replication b/plugins/replication
index fae5360..0721b20 160000
--- a/plugins/replication
+++ b/plugins/replication
@@ -1 +1 @@
-Subproject commit fae5360380023e8351f39be3d4effd4bb2cd8906
+Subproject commit 0721b208ad863ff2f2c7fe1c89950dc2b921abaa
