Merge changes Ic6ac567a,Ifddea6fa
* changes:
dev-contributing: Specify the version of buildifier to use
Format gerrit-acceptance-tests/BUILD with buildifier
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/account/ImpersonationIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/account/ImpersonationIT.java
index 82eae1b..b221ec5 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/account/ImpersonationIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/account/ImpersonationIT.java
@@ -314,7 +314,7 @@
in.label("Code-Review", 1);
exception.expect(UnprocessableEntityException.class);
- exception.expectMessage("on_behalf_of account " + user.id + " cannot see destination ref");
+ exception.expectMessage("on_behalf_of account " + user.id + " cannot see change");
revision.review(in);
}
diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/project/ReviewProjectAccess.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/project/ReviewProjectAccess.java
index 1f7848d..8ba0978 100644
--- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/project/ReviewProjectAccess.java
+++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/project/ReviewProjectAccess.java
@@ -38,6 +38,7 @@
import com.google.gerrit.server.git.MetaDataUpdate;
import com.google.gerrit.server.git.ProjectConfig;
import com.google.gerrit.server.group.SystemGroupBackend;
+import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectControl;
import com.google.gerrit.server.project.RefControl;
@@ -174,7 +175,11 @@
AddReviewerInput input = new AddReviewerInput();
input.reviewer = projectOwners;
reviewersProvider.get().apply(rsrc, input);
- } catch (IOException | OrmException | RestApiException | UpdateException e) {
+ } catch (IOException
+ | OrmException
+ | RestApiException
+ | UpdateException
+ | PermissionBackendException e) {
// one of the owner groups is not visible to the user and this it why it
// can't be added as reviewer
}
@@ -193,7 +198,11 @@
AddReviewerInput input = new AddReviewerInput();
input.reviewer = r.getGroup().getUUID().get();
reviewersProvider.get().apply(rsrc, input);
- } catch (IOException | OrmException | RestApiException | UpdateException e) {
+ } catch (IOException
+ | OrmException
+ | RestApiException
+ | UpdateException
+ | PermissionBackendException e) {
// ignore
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/rules/PrologEnvironment.java b/gerrit-server/src/main/java/com/google/gerrit/rules/PrologEnvironment.java
index 9538121a4..23c59f5 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/rules/PrologEnvironment.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/rules/PrologEnvironment.java
@@ -20,6 +20,7 @@
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.patch.PatchListCache;
import com.google.gerrit.server.patch.PatchSetInfoFactory;
+import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.project.ProjectCache;
import com.google.inject.Inject;
import com.google.inject.Provider;
@@ -166,6 +167,7 @@
}
private final ProjectCache projectCache;
+ private final PermissionBackend permissionBackend;
private final GitRepositoryManager repositoryManager;
private final PatchListCache patchListCache;
private final PatchSetInfoFactory patchSetInfoFactory;
@@ -177,6 +179,7 @@
@Inject
Args(
ProjectCache projectCache,
+ PermissionBackend permissionBackend,
GitRepositoryManager repositoryManager,
PatchListCache patchListCache,
PatchSetInfoFactory patchSetInfoFactory,
@@ -184,6 +187,7 @@
Provider<AnonymousUser> anonymousUser,
@GerritServerConfig Config config) {
this.projectCache = projectCache;
+ this.permissionBackend = permissionBackend;
this.repositoryManager = repositoryManager;
this.patchListCache = patchListCache;
this.patchSetInfoFactory = patchSetInfoFactory;
@@ -213,6 +217,10 @@
return projectCache;
}
+ public PermissionBackend getPermissionBackend() {
+ return permissionBackend;
+ }
+
public GitRepositoryManager getGitRepositoryManager() {
return repositoryManager;
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/rules/StoredValues.java b/gerrit-server/src/main/java/com/google/gerrit/rules/StoredValues.java
index 98ec569..e35171b 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/rules/StoredValues.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/rules/StoredValues.java
@@ -33,6 +33,7 @@
import com.google.gerrit.server.patch.PatchListNotAvailableException;
import com.google.gerrit.server.patch.PatchSetInfoFactory;
import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException;
+import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gwtorm.server.OrmException;
@@ -130,6 +131,15 @@
}
};
+ public static final StoredValue<PermissionBackend> PERMISSION_BACKEND =
+ new StoredValue<PermissionBackend>() {
+ @Override
+ protected PermissionBackend createValue(Prolog engine) {
+ PrologEnvironment env = (PrologEnvironment) engine.control;
+ return env.getArgs().getPermissionBackend();
+ }
+ };
+
public static final StoredValue<AnonymousUser> ANONYMOUS_USER =
new StoredValue<AnonymousUser>() {
@Override
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java
index 4698a80..d534c5a 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java
@@ -416,7 +416,7 @@
public void addReviewer(AddReviewerInput in) throws RestApiException {
try {
postReviewers.apply(change, in);
- } catch (OrmException | IOException | UpdateException e) {
+ } catch (OrmException | IOException | UpdateException | PermissionBackendException e) {
throw new RestApiException("Cannot add change reviewer", e);
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/RevisionApiImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/RevisionApiImpl.java
index 2af7b90..d934f6c 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/RevisionApiImpl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/RevisionApiImpl.java
@@ -214,7 +214,7 @@
public void review(ReviewInput in) throws RestApiException {
try {
review.apply(revision, in);
- } catch (OrmException | UpdateException | IOException e) {
+ } catch (OrmException | UpdateException | IOException | PermissionBackendException e) {
throw new RestApiException("Cannot post review", e);
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java
index 190f59b..c86714a 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java
@@ -59,8 +59,6 @@
import com.google.gerrit.common.data.LabelType;
import com.google.gerrit.common.data.LabelTypes;
import com.google.gerrit.common.data.LabelValue;
-import com.google.gerrit.common.data.Permission;
-import com.google.gerrit.common.data.PermissionRange;
import com.google.gerrit.common.data.SubmitRecord;
import com.google.gerrit.common.data.SubmitTypeRecord;
import com.google.gerrit.extensions.api.changes.FixInput;
@@ -104,7 +102,6 @@
import com.google.gerrit.server.api.accounts.AccountInfoComparator;
import com.google.gerrit.server.api.accounts.GpgApiAdapter;
import com.google.gerrit.server.git.GitRepositoryManager;
-import com.google.gerrit.server.git.LabelNormalizer;
import com.google.gerrit.server.git.MergeUtil;
import com.google.gerrit.server.index.change.ChangeField;
import com.google.gerrit.server.index.change.ChangeIndexCollection;
@@ -112,6 +109,9 @@
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.ReviewerStateInternal;
import com.google.gerrit.server.patch.PatchListNotAvailableException;
+import com.google.gerrit.server.permissions.LabelPermission;
+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.ProjectCache;
import com.google.gerrit.server.project.SubmitRuleOptions;
@@ -186,9 +186,9 @@
}
private final Provider<ReviewDb> db;
- private final LabelNormalizer labelNormalizer;
private final Provider<CurrentUser> userProvider;
private final AnonymousUser anonymous;
+ private final PermissionBackend permissionBackend;
private final GitRepositoryManager repoManager;
private final ProjectCache projectCache;
private final MergeUtil.Factory mergeUtilFactory;
@@ -217,9 +217,9 @@
@Inject
ChangeJson(
Provider<ReviewDb> db,
- LabelNormalizer ln,
Provider<CurrentUser> user,
AnonymousUser au,
+ PermissionBackend permissionBackend,
GitRepositoryManager repoManager,
ProjectCache projectCache,
MergeUtil.Factory mergeUtilFactory,
@@ -241,10 +241,10 @@
ApprovalsUtil approvalsUtil,
@Assisted Iterable<ListChangesOption> options) {
this.db = db;
- this.labelNormalizer = ln;
this.userProvider = user;
this.anonymous = au;
this.changeDataFactory = cdf;
+ this.permissionBackend = permissionBackend;
this.repoManager = repoManager;
this.userFactory = uf;
this.projectCache = projectCache;
@@ -316,6 +316,7 @@
| GpgException
| OrmException
| IOException
+ | PermissionBackendException
| RuntimeException e) {
if (!has(CHECK)) {
Throwables.throwIfInstanceOf(e, OrmException.class);
@@ -393,6 +394,7 @@
| GpgException
| OrmException
| IOException
+ | PermissionBackendException
| RuntimeException e) {
if (has(CHECK)) {
i = checkOnly(cd);
@@ -450,7 +452,8 @@
}
private ChangeInfo toChangeInfo(ChangeData cd, Optional<PatchSet.Id> limitToPsId)
- throws PatchListNotAvailableException, GpgException, OrmException, IOException {
+ throws PatchListNotAvailableException, GpgException, OrmException, IOException,
+ PermissionBackendException {
ChangeInfo out = new ChangeInfo();
CurrentUser user = userProvider.get();
ChangeControl ctl = cd.changeControl().forUser(user);
@@ -466,6 +469,7 @@
}
}
+ PermissionBackend.ForChange perm = permissionBackend.user(user).database(db).change(cd);
Change in = cd.change();
out.project = in.getProject().get();
out.branch = in.getDest().getShortName();
@@ -514,16 +518,17 @@
out.reviewed = cd.reviewedBy().contains(accountId) ? true : null;
}
- out.labels = labelsFor(ctl, cd, has(LABELS), has(DETAILED_LABELS));
+ out.labels = labelsFor(perm, ctl, cd, has(LABELS), has(DETAILED_LABELS));
out.submitted = getSubmittedOn(cd);
if (out.labels != null && has(DETAILED_LABELS)) {
// If limited to specific patch sets but not the current patch set, don't
// list permitted labels, since users can't vote on those patch sets.
- if (!limitToPsId.isPresent() || limitToPsId.get().equals(in.currentPatchSetId())) {
+ if (user.isIdentifiedUser()
+ && (!limitToPsId.isPresent() || limitToPsId.get().equals(in.currentPatchSetId()))) {
out.permittedLabels =
cd.change().getStatus() != Change.Status.ABANDONED
- ? permittedLabels(ctl, cd)
+ ? permittedLabels(perm, cd)
: ImmutableMap.of();
}
@@ -603,7 +608,12 @@
}
private Map<String, LabelInfo> labelsFor(
- ChangeControl ctl, ChangeData cd, boolean standard, boolean detailed) throws OrmException {
+ PermissionBackend.ForChange perm,
+ ChangeControl ctl,
+ ChangeData cd,
+ boolean standard,
+ boolean detailed)
+ throws OrmException, PermissionBackendException {
if (!standard && !detailed) {
return null;
}
@@ -612,20 +622,24 @@
return null;
}
- LabelTypes labelTypes = ctl.getLabelTypes();
+ LabelTypes labelTypes = cd.getLabelTypes();
Map<String, LabelWithStatus> withStatus =
cd.change().getStatus().isOpen()
- ? labelsForOpenChange(ctl, cd, labelTypes, standard, detailed)
- : labelsForClosedChange(ctl, cd, labelTypes, standard, detailed);
+ ? labelsForOpenChange(perm, cd, labelTypes, standard, detailed)
+ : labelsForClosedChange(perm, cd, labelTypes, standard, detailed);
return ImmutableMap.copyOf(Maps.transformValues(withStatus, LabelWithStatus::label));
}
private Map<String, LabelWithStatus> labelsForOpenChange(
- ChangeControl ctl, ChangeData cd, LabelTypes labelTypes, boolean standard, boolean detailed)
- throws OrmException {
+ PermissionBackend.ForChange perm,
+ ChangeData cd,
+ LabelTypes labelTypes,
+ boolean standard,
+ boolean detailed)
+ throws OrmException, PermissionBackendException {
Map<String, LabelWithStatus> labels = initLabels(cd, labelTypes, standard);
if (detailed) {
- setAllApprovals(ctl, cd, labels);
+ setAllApprovals(perm, cd, labels);
}
for (Map.Entry<String, LabelWithStatus> e : labels.entrySet()) {
LabelType type = labelTypes.byLabel(e.getKey());
@@ -712,8 +726,8 @@
}
private void setAllApprovals(
- ChangeControl baseCtrl, ChangeData cd, Map<String, LabelWithStatus> labels)
- throws OrmException {
+ PermissionBackend.ForChange basePerm, ChangeData cd, Map<String, LabelWithStatus> labels)
+ throws OrmException, PermissionBackendException {
Change.Status status = cd.change().getStatus();
checkState(status.isOpen(), "should not call setAllApprovals on %s change", status);
@@ -727,17 +741,17 @@
}
Table<Account.Id, String, PatchSetApproval> current =
- HashBasedTable.create(allUsers.size(), baseCtrl.getLabelTypes().getLabelTypes().size());
+ HashBasedTable.create(allUsers.size(), cd.getLabelTypes().getLabelTypes().size());
for (PatchSetApproval psa : cd.currentApprovals()) {
current.put(psa.getAccountId(), psa.getLabel(), psa);
}
+ LabelTypes labelTypes = cd.getLabelTypes();
for (Account.Id accountId : allUsers) {
- IdentifiedUser user = userFactory.create(accountId);
- ChangeControl ctl = baseCtrl.forUser(user);
- Map<String, VotingRangeInfo> pvr = getPermittedVotingRanges(permittedLabels(ctl, cd));
+ PermissionBackend.ForChange perm = basePerm.user(userFactory.create(accountId));
+ Map<String, VotingRangeInfo> pvr = getPermittedVotingRanges(permittedLabels(perm, cd));
for (Map.Entry<String, LabelWithStatus> e : labels.entrySet()) {
- LabelType lt = ctl.getLabelTypes().byLabel(e.getKey());
+ LabelType lt = labelTypes.byLabel(e.getKey());
if (lt == null) {
// Ignore submit record for undefined label; likely the submit rule
// author didn't intend for the label to show up in the table.
@@ -754,7 +768,7 @@
// This may be a dummy approval that was inserted when the reviewer
// was added. Explicitly check whether the user can vote on this
// label.
- value = labelNormalizer.canVote(ctl, lt, accountId) ? 0 : null;
+ value = perm.test(new LabelPermission(lt)) ? 0 : null;
}
tag = psa.getTag();
date = psa.getGranted();
@@ -765,7 +779,7 @@
// Either the user cannot vote on this label, or they were added as a
// reviewer but have not responded yet. Explicitly check whether the
// user can vote on this label.
- value = labelNormalizer.canVote(ctl, lt, accountId) ? 0 : null;
+ value = perm.test(new LabelPermission(lt)) ? 0 : null;
}
addApproval(
e.getValue().label(), approvalInfo(accountId, value, permittedVotingRange, tag, date));
@@ -813,12 +827,12 @@
}
private Map<String, LabelWithStatus> labelsForClosedChange(
- ChangeControl baseCtrl,
+ PermissionBackend.ForChange basePerm,
ChangeData cd,
LabelTypes labelTypes,
boolean standard,
boolean detailed)
- throws OrmException {
+ throws OrmException, PermissionBackendException {
Set<Account.Id> allUsers = new HashSet<>();
if (detailed) {
// Users expect to see all reviewers on closed changes, even if they
@@ -886,8 +900,8 @@
Map<String, ApprovalInfo> byLabel = Maps.newHashMapWithExpectedSize(labels.size());
Map<String, VotingRangeInfo> pvr = Collections.emptyMap();
if (detailed) {
- ChangeControl ctl = baseCtrl.forUser(userFactory.create(accountId));
- pvr = getPermittedVotingRanges(permittedLabels(ctl, cd));
+ PermissionBackend.ForChange perm = basePerm.user(userFactory.create(accountId));
+ pvr = getPermittedVotingRanges(permittedLabels(perm, cd));
for (Map.Entry<String, LabelWithStatus> entry : labels.entrySet()) {
ApprovalInfo ai = approvalInfo(accountId, 0, null, null, null);
byLabel.put(entry.getKey(), ai);
@@ -961,15 +975,25 @@
}
}
- private Map<String, Collection<String>> permittedLabels(ChangeControl ctl, ChangeData cd)
- throws OrmException {
- if (ctl == null || !ctl.getUser().isIdentifiedUser()) {
- return null;
+ private Map<String, Collection<String>> permittedLabels(
+ PermissionBackend.ForChange perm, ChangeData cd)
+ throws OrmException, PermissionBackendException {
+ boolean isMerged = cd.change().getStatus() == Change.Status.MERGED;
+ LabelTypes labelTypes = cd.getLabelTypes();
+ Map<String, LabelType> toCheck = new HashMap<>();
+ for (SubmitRecord rec : submitRecords(cd)) {
+ if (rec.labels != null) {
+ for (SubmitRecord.Label r : rec.labels) {
+ LabelType type = labelTypes.byLabel(r.label);
+ if (type != null && (!isMerged || type.allowPostSubmit())) {
+ toCheck.put(type.getName(), type);
+ }
+ }
+ }
}
Map<String, Short> labels = null;
- boolean isMerged = ctl.getChange().getStatus() == Change.Status.MERGED;
- LabelTypes labelTypes = ctl.getLabelTypes();
+ Set<LabelPermission.WithValue> can = perm.testLabels(toCheck.values());
SetMultimap<String, String> permitted = LinkedHashMultimap.create();
for (SubmitRecord rec : submitRecords(cd)) {
if (rec.labels == null) {
@@ -980,12 +1004,12 @@
if (type == null || (isMerged && !type.allowPostSubmit())) {
continue;
}
- PermissionRange range = ctl.getRange(Permission.forLabel(r.label));
+
for (LabelValue v : type.getValues()) {
- boolean ok = range.contains(v.getValue());
+ boolean ok = can.contains(new LabelPermission.WithValue(type, v));
if (isMerged) {
if (labels == null) {
- labels = currentLabels(ctl);
+ labels = currentLabels(perm, cd);
}
short prev = labels.getOrDefault(type.getName(), (short) 0);
ok &= v.getValue() >= prev;
@@ -996,6 +1020,7 @@
}
}
}
+
List<String> toClear = Lists.newArrayListWithCapacity(permitted.keySet().size());
for (Map.Entry<String, Collection<String>> e : permitted.asMap().entrySet()) {
if (isOnlyZero(e.getValue())) {
@@ -1008,11 +1033,14 @@
return permitted.asMap();
}
- private Map<String, Short> currentLabels(ChangeControl ctl) throws OrmException {
+ private Map<String, Short> currentLabels(PermissionBackend.ForChange perm, ChangeData cd)
+ throws OrmException {
+ IdentifiedUser user = perm.user().asIdentifiedUser();
+ ChangeControl ctl = cd.changeControl().forUser(user);
Map<String, Short> result = new HashMap<>();
for (PatchSetApproval psa :
approvalsUtil.byPatchSetUser(
- db.get(), ctl, ctl.getChange().currentPatchSetId(), ctl.getUser().getAccountId())) {
+ db.get(), ctl, cd.change().currentPatchSetId(), user.getAccountId())) {
result.put(psa.getLabel(), psa.getValue());
}
return result;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/GetReviewer.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/GetReviewer.java
index aa0b339..db9af1d 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/GetReviewer.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/GetReviewer.java
@@ -16,6 +16,7 @@
import com.google.gerrit.extensions.api.changes.ReviewerInfo;
import com.google.gerrit.extensions.restapi.RestReadView;
+import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Singleton;
@@ -31,7 +32,8 @@
}
@Override
- public List<ReviewerInfo> apply(ReviewerResource rsrc) throws OrmException {
+ public List<ReviewerInfo> apply(ReviewerResource rsrc)
+ throws OrmException, PermissionBackendException {
return json.format(rsrc);
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ListReviewers.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ListReviewers.java
index 1dba58c..ba2a10b 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ListReviewers.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ListReviewers.java
@@ -20,6 +20,7 @@
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.mail.Address;
+import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
@@ -48,7 +49,8 @@
}
@Override
- public List<ReviewerInfo> apply(ChangeResource rsrc) throws OrmException {
+ public List<ReviewerInfo> apply(ChangeResource rsrc)
+ throws OrmException, PermissionBackendException {
Map<String, ReviewerResource> reviewers = new LinkedHashMap<>();
ReviewDb db = dbProvider.get();
for (Account.Id accountId : approvalsUtil.getReviewers(db, rsrc.getNotes()).all()) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ListRevisionReviewers.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ListRevisionReviewers.java
index 5aaee56..6d9dc79 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ListRevisionReviewers.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ListRevisionReviewers.java
@@ -21,6 +21,7 @@
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.mail.Address;
+import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
@@ -50,7 +51,7 @@
@Override
public List<ReviewerInfo> apply(RevisionResource rsrc)
- throws OrmException, MethodNotAllowedException {
+ throws OrmException, MethodNotAllowedException, PermissionBackendException {
if (!rsrc.isCurrent()) {
throw new MethodNotAllowedException("Cannot list reviewers on non-current patch set");
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java
index 81ab39e..6365fd9 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java
@@ -18,6 +18,7 @@
import static com.google.common.base.Preconditions.checkState;
import static com.google.gerrit.server.CommentsUtil.setCommentRevId;
import static com.google.gerrit.server.notedb.ReviewerStateInternal.REVIEWER;
+import static com.google.gerrit.server.permissions.LabelPermission.ForUser.ON_BEHALF_OF;
import static java.nio.charset.StandardCharsets.UTF_8;
import static java.util.stream.Collectors.groupingBy;
import static java.util.stream.Collectors.joining;
@@ -39,8 +40,6 @@
import com.google.gerrit.common.TimeUtil;
import com.google.gerrit.common.data.LabelType;
import com.google.gerrit.common.data.LabelTypes;
-import com.google.gerrit.common.data.Permission;
-import com.google.gerrit.common.data.PermissionRange;
import com.google.gerrit.extensions.api.changes.AddReviewerInput;
import com.google.gerrit.extensions.api.changes.AddReviewerResult;
import com.google.gerrit.extensions.api.changes.NotifyHandling;
@@ -83,6 +82,7 @@
import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.CommentsUtil;
+import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.OutputFormat;
import com.google.gerrit.server.PatchSetUtil;
@@ -95,6 +95,10 @@
import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.notedb.NotesMigration;
import com.google.gerrit.server.patch.PatchListCache;
+import com.google.gerrit.server.permissions.ChangePermission;
+import com.google.gerrit.server.permissions.LabelPermission;
+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.ChangeData;
import com.google.gerrit.server.update.BatchUpdate;
@@ -188,12 +192,14 @@
@Override
public Response<ReviewResult> apply(RevisionResource revision, ReviewInput input)
- throws RestApiException, UpdateException, OrmException, IOException {
+ throws RestApiException, UpdateException, OrmException, IOException,
+ PermissionBackendException {
return apply(revision, input, TimeUtil.nowTs());
}
public Response<ReviewResult> apply(RevisionResource revision, ReviewInput input, Timestamp ts)
- throws RestApiException, UpdateException, OrmException, IOException {
+ throws RestApiException, UpdateException, OrmException, IOException,
+ PermissionBackendException {
// Respect timestamp, but truncate at change created-on time.
ts = Ordering.natural().max(ts, revision.getChange().getCreatedOn());
if (revision.getEdit().isPresent()) {
@@ -348,7 +354,8 @@
}
private RevisionResource onBehalfOf(RevisionResource rev, ReviewInput in)
- throws BadRequestException, AuthException, UnprocessableEntityException, OrmException {
+ throws BadRequestException, AuthException, UnprocessableEntityException, OrmException,
+ PermissionBackendException {
if (in.labels == null || in.labels.isEmpty()) {
throw new AuthException(
String.format("label required to post review on behalf of \"%s\"", in.onBehalfOf));
@@ -360,11 +367,13 @@
throw new AuthException("not allowed to modify other user's drafts");
}
- ChangeControl caller = rev.getControl();
+ CurrentUser caller = rev.getUser();
+ PermissionBackend.ForChange perm = rev.permissions().database(db);
+ LabelTypes labelTypes = rev.getControl().getLabelTypes();
Iterator<Map.Entry<String, Short>> itr = in.labels.entrySet().iterator();
while (itr.hasNext()) {
Map.Entry<String, Short> ent = itr.next();
- LabelType type = caller.getLabelTypes().byLabel(ent.getKey());
+ LabelType type = labelTypes.byLabel(ent.getKey());
if (type == null && in.strictLabels) {
throw new BadRequestException(
String.format("label \"%s\" is not a configured label", ent.getKey()));
@@ -373,16 +382,15 @@
continue;
}
- if (caller.getUser().isInternalUser()) {
- continue;
- }
-
- PermissionRange r = caller.getRange(Permission.forLabelAs(type.getName()));
- if (r == null || r.isEmpty() || !r.contains(ent.getValue())) {
- throw new AuthException(
- String.format(
- "not permitted to modify label \"%s\" on behalf of \"%s\"",
- ent.getKey(), in.onBehalfOf));
+ if (!caller.isInternalUser()) {
+ try {
+ perm.check(new LabelPermission.WithValue(ON_BEHALF_OF, type, ent.getValue()));
+ } catch (AuthException e) {
+ throw new AuthException(
+ String.format(
+ "not permitted to modify label \"%s\" on behalf of \"%s\"",
+ type.getName(), in.onBehalfOf));
+ }
}
}
if (in.labels.isEmpty()) {
@@ -390,25 +398,26 @@
String.format("label required to post review on behalf of \"%s\"", in.onBehalfOf));
}
- ChangeControl target =
- caller.forUser(accounts.parseOnBehalfOf(caller.getUser(), in.onBehalfOf));
- if (!target.getRefControl().isVisible()) {
+ IdentifiedUser reviewer = accounts.parseOnBehalfOf(caller, in.onBehalfOf);
+ try {
+ perm.user(reviewer).check(ChangePermission.READ);
+ } catch (AuthException e) {
throw new UnprocessableEntityException(
- String.format(
- "on_behalf_of account %s cannot see destination ref",
- target.getUser().getAccountId()));
+ String.format("on_behalf_of account %s cannot see change", reviewer.getAccountId()));
}
- return new RevisionResource(changes.parse(target), rev.getPatchSet());
+
+ ChangeControl ctl = rev.getControl().forUser(reviewer);
+ return new RevisionResource(changes.parse(ctl), rev.getPatchSet());
}
- private void checkLabels(RevisionResource revision, boolean strict, Map<String, Short> labels)
- throws BadRequestException, AuthException {
- ChangeControl ctl = revision.getControl();
+ private void checkLabels(RevisionResource rsrc, boolean strict, Map<String, Short> labels)
+ throws BadRequestException, AuthException, PermissionBackendException {
+ LabelTypes types = rsrc.getControl().getLabelTypes();
+ PermissionBackend.ForChange perm = rsrc.permissions();
Iterator<Map.Entry<String, Short>> itr = labels.entrySet().iterator();
while (itr.hasNext()) {
Map.Entry<String, Short> ent = itr.next();
-
- LabelType lt = revision.getControl().getLabelTypes().byLabel(ent.getKey());
+ LabelType lt = types.byLabel(ent.getKey());
if (lt == null) {
if (strict) {
throw new BadRequestException(
@@ -433,18 +442,15 @@
continue;
}
- String name = lt.getName();
- PermissionRange range = ctl.getRange(Permission.forLabel(name));
- if (range == null || !range.contains(ent.getValue())) {
+ short val = ent.getValue();
+ try {
+ perm.check(new LabelPermission.WithValue(lt, val));
+ } catch (AuthException e) {
if (strict) {
throw new AuthException(
- String.format(
- "Applying label \"%s\": %d is restricted", ent.getKey(), ent.getValue()));
- } else if (range == null || range.isEmpty()) {
- ent.setValue((short) 0);
- } else {
- ent.setValue((short) range.squash(ent.getValue()));
+ String.format("Applying label \"%s\": %d is restricted", lt.getName(), val));
}
+ ent.setValue(perm.squashThenCheck(lt, val));
}
}
}
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 b7fe986..cc759fd 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
@@ -54,6 +54,8 @@
import com.google.gerrit.server.mail.Address;
import com.google.gerrit.server.mail.send.OutgoingEmailValidator;
import com.google.gerrit.server.notedb.NotesMigration;
+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.ProjectCache;
@@ -79,6 +81,7 @@
private final AccountsCollection accounts;
private final ReviewerResource.Factory reviewerFactory;
+ private final PermissionBackend permissionBackend;
private final GroupsCollection groupsCollection;
private final GroupMembers.Factory groupMembersFactory;
@@ -98,6 +101,7 @@
PostReviewers(
AccountsCollection accounts,
ReviewerResource.Factory reviewerFactory,
+ PermissionBackend permissionBackend,
GroupsCollection groupsCollection,
GroupMembers.Factory groupMembersFactory,
AccountLoader.Factory accountLoaderFactory,
@@ -113,6 +117,7 @@
PostReviewersOp.Factory postReviewersOpFactory) {
this.accounts = accounts;
this.reviewerFactory = reviewerFactory;
+ this.permissionBackend = permissionBackend;
this.groupsCollection = groupsCollection;
this.groupMembersFactory = groupMembersFactory;
this.accountLoaderFactory = accountLoaderFactory;
@@ -130,7 +135,8 @@
@Override
public AddReviewerResult apply(ChangeResource rsrc, AddReviewerInput input)
- throws IOException, OrmException, RestApiException, UpdateException {
+ throws IOException, OrmException, RestApiException, UpdateException,
+ PermissionBackendException {
if (input.reviewer == null) {
throw new BadRequestException("missing reviewer field");
}
@@ -398,14 +404,17 @@
rsrc, this.reviewers, this.reviewersByEmail, state, notify, accountsToNotify);
}
- void gatherResults() throws OrmException {
+ void gatherResults() throws OrmException, PermissionBackendException {
// Generate result details and fill AccountLoader. This occurs outside
// the Op because the accounts are in a different table.
PostReviewersOp.Result opResult = op.getResult();
if (migration.readChanges() && state == CC) {
result.ccs = Lists.newArrayListWithCapacity(opResult.addedCCs().size());
for (Account.Id accountId : opResult.addedCCs()) {
- result.ccs.add(json.format(new ReviewerInfo(accountId.get()), reviewers.get(accountId)));
+ ChangeControl ctl = reviewers.get(accountId);
+ PermissionBackend.ForChange perm =
+ permissionBackend.user(ctl.getUser()).database(dbProvider).change(ctl.getNotes());
+ result.ccs.add(json.format(new ReviewerInfo(accountId.get()), perm, ctl));
}
accountLoaderFactory.create(true).fill(result.ccs);
for (Address a : reviewersByEmail) {
@@ -415,11 +424,12 @@
result.reviewers = Lists.newArrayListWithCapacity(opResult.addedReviewers().size());
for (PatchSetApproval psa : opResult.addedReviewers()) {
// New reviewers have value 0, don't bother normalizing.
+ ChangeControl ctl = reviewers.get(psa.getAccountId());
+ PermissionBackend.ForChange perm =
+ permissionBackend.user(ctl.getUser()).database(dbProvider).change(ctl.getNotes());
result.reviewers.add(
json.format(
- new ReviewerInfo(psa.getAccountId().get()),
- reviewers.get(psa.getAccountId()),
- ImmutableList.of(psa)));
+ new ReviewerInfo(psa.getAccountId().get()), perm, ctl, ImmutableList.of(psa)));
}
accountLoaderFactory.create(true).fill(result.reviewers);
for (Address a : reviewersByEmail) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ReviewerJson.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ReviewerJson.java
index ac7f15e..d6ef2a7 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ReviewerJson.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ReviewerJson.java
@@ -20,7 +20,6 @@
import com.google.common.collect.Lists;
import com.google.gerrit.common.data.LabelType;
import com.google.gerrit.common.data.LabelTypes;
-import com.google.gerrit.common.data.Permission;
import com.google.gerrit.common.data.PermissionRange;
import com.google.gerrit.common.data.SubmitRecord;
import com.google.gerrit.extensions.api.changes.ReviewerInfo;
@@ -30,6 +29,9 @@
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.account.AccountLoader;
+import com.google.gerrit.server.permissions.LabelPermission;
+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.SubmitRuleEvaluator;
import com.google.gerrit.server.query.change.ChangeData;
@@ -44,6 +46,7 @@
@Singleton
public class ReviewerJson {
private final Provider<ReviewDb> db;
+ private final PermissionBackend permissionBackend;
private final ChangeData.Factory changeDataFactory;
private final ApprovalsUtil approvalsUtil;
private final AccountLoader.Factory accountLoaderFactory;
@@ -51,22 +54,29 @@
@Inject
ReviewerJson(
Provider<ReviewDb> db,
+ PermissionBackend permissionBackend,
ChangeData.Factory changeDataFactory,
ApprovalsUtil approvalsUtil,
AccountLoader.Factory accountLoaderFactory) {
this.db = db;
+ this.permissionBackend = permissionBackend;
this.changeDataFactory = changeDataFactory;
this.approvalsUtil = approvalsUtil;
this.accountLoaderFactory = accountLoaderFactory;
}
- public List<ReviewerInfo> format(Collection<ReviewerResource> rsrcs) throws OrmException {
+ public List<ReviewerInfo> format(Collection<ReviewerResource> rsrcs)
+ throws OrmException, PermissionBackendException {
List<ReviewerInfo> infos = Lists.newArrayListWithCapacity(rsrcs.size());
AccountLoader loader = accountLoaderFactory.create(true);
for (ReviewerResource rsrc : rsrcs) {
ReviewerInfo info =
format(
new ReviewerInfo(rsrc.getReviewerUser().getAccountId().get()),
+ permissionBackend
+ .user(rsrc.getReviewerUser())
+ .database(db)
+ .change(rsrc.getChangeResource().getNotes()),
rsrc.getReviewerControl());
loader.put(info);
infos.add(info);
@@ -75,22 +85,29 @@
return infos;
}
- public List<ReviewerInfo> format(ReviewerResource rsrc) throws OrmException {
+ public List<ReviewerInfo> format(ReviewerResource rsrc)
+ throws OrmException, PermissionBackendException {
return format(ImmutableList.<ReviewerResource>of(rsrc));
}
- public ReviewerInfo format(ReviewerInfo out, ChangeControl ctl) throws OrmException {
+ public ReviewerInfo format(ReviewerInfo out, PermissionBackend.ForChange perm, ChangeControl ctl)
+ throws OrmException, PermissionBackendException {
PatchSet.Id psId = ctl.getChange().currentPatchSetId();
return format(
out,
+ perm,
ctl,
approvalsUtil.byPatchSetUser(db.get(), ctl, psId, new Account.Id(out._accountId)));
}
public ReviewerInfo format(
- ReviewerInfo out, ChangeControl ctl, Iterable<PatchSetApproval> approvals)
- throws OrmException {
- LabelTypes labelTypes = ctl.getLabelTypes();
+ ReviewerInfo out,
+ PermissionBackend.ForChange perm,
+ ChangeControl ctl,
+ Iterable<PatchSetApproval> approvals)
+ throws OrmException, PermissionBackendException {
+ ChangeData cd = changeDataFactory.create(db.get(), ctl);
+ LabelTypes labelTypes = cd.getLabelTypes();
// Don't use Maps.newTreeMap(Comparator) due to OpenJDK bug 100167.
out.approvals = new TreeMap<>(labelTypes.nameComparator());
@@ -107,7 +124,6 @@
// Add dummy approvals for all permitted labels for the user even if they
// do not exist in the DB.
- ChangeData cd = changeDataFactory.create(db.get(), ctl);
PatchSet ps = cd.currentPatchSet();
if (ps != null) {
for (SubmitRecord rec :
@@ -117,8 +133,10 @@
}
for (SubmitRecord.Label label : rec.labels) {
String name = label.label;
+ LabelType type = labelTypes.byLabel(name);
if (!out.approvals.containsKey(name)
- && !ctl.getRange(Permission.forLabel(name)).isEmpty()) {
+ && type != null
+ && perm.test(new LabelPermission(type))) {
out.approvals.put(name, formatValue((short) 0));
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/LabelNormalizer.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/LabelNormalizer.java
index 6a05d22..323f352 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/LabelNormalizer.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/LabelNormalizer.java
@@ -142,16 +142,6 @@
return Result.create(unchanged, updated, deleted);
}
- /**
- * @param ctl change control (for any user).
- * @param lt label type.
- * @param id account ID.
- * @return whether the given account ID has any permissions to vote on this label for this change.
- */
- public boolean canVote(ChangeControl ctl, LabelType lt, Account.Id id) {
- return !getRange(ctl, lt, id).isEmpty();
- }
-
private PatchSetApproval copy(PatchSetApproval src) {
return new PatchSetApproval(src.getPatchSetId(), src);
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/MergedSender.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/MergedSender.java
index 47115af..4d48990 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/MergedSender.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/MergedSender.java
@@ -41,7 +41,7 @@
public MergedSender(EmailArguments ea, @Assisted Project.NameKey project, @Assisted Change.Id id)
throws OrmException {
super(ea, "merged", newChangeData(ea, project, id));
- labelTypes = changeData.changeControl().getLabelTypes();
+ labelTypes = changeData.getLabelTypes();
}
@Override
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 4945879..24f5164 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
@@ -164,5 +164,10 @@
throws PermissionBackendException {
throw new PermissionBackendException(message, cause);
}
+
+ @Override
+ public CurrentUser user() {
+ throw new UnsupportedOperationException("FailedPermissionBackend is not scoped to user");
+ }
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/permissions/LabelPermission.java b/gerrit-server/src/main/java/com/google/gerrit/server/permissions/LabelPermission.java
index 61f7330..747c997 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/permissions/LabelPermission.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/permissions/LabelPermission.java
@@ -15,25 +15,69 @@
package com.google.gerrit.server.permissions;
import static com.google.common.base.Preconditions.checkNotNull;
+import static com.google.gerrit.server.permissions.LabelPermission.ForUser.ON_BEHALF_OF;
+import static com.google.gerrit.server.permissions.LabelPermission.ForUser.SELF;
import com.google.gerrit.common.data.LabelType;
+import com.google.gerrit.common.data.LabelValue;
import com.google.gerrit.common.data.Permission;
import com.google.gerrit.server.util.LabelVote;
import java.util.Optional;
/** Permission representing a label. */
public class LabelPermission implements ChangePermissionOrLabel {
+ public enum ForUser {
+ SELF,
+ ON_BEHALF_OF;
+ }
+
+ private final ForUser forUser;
private final String name;
/**
* Construct a reference to a label permission.
*
+ * @param type type description of the label.
+ */
+ public LabelPermission(LabelType type) {
+ this(SELF, type);
+ }
+
+ /**
+ * Construct a reference to a label permission.
+ *
+ * @param forUser {@code SELF} (default) or {@code ON_BEHALF_OF} for labelAs behavior.
+ * @param type type description of the label.
+ */
+ public LabelPermission(ForUser forUser, LabelType type) {
+ this(forUser, type.getName());
+ }
+
+ /**
+ * Construct a reference to a label permission.
+ *
* @param name name of the label, e.g. {@code "Code-Review"} or {@code "Verified"}.
*/
public LabelPermission(String name) {
+ this(SELF, name);
+ }
+
+ /**
+ * Construct a reference to a label permission.
+ *
+ * @param forUser {@code SELF} (default) or {@code ON_BEHALF_OF} for labelAs behavior.
+ * @param name name of the label, e.g. {@code "Code-Review"} or {@code "Verified"}.
+ */
+ public LabelPermission(ForUser forUser, String name) {
+ this.forUser = checkNotNull(forUser, "ForUser");
this.name = LabelType.checkName(name);
}
+ /** @return {@code SELF} or {@code ON_BEHALF_OF} (or labelAs). */
+ public ForUser forUser() {
+ return forUser;
+ }
+
/** @return name of the label, e.g. {@code "Code-Review"}. */
public String label() {
return name;
@@ -42,12 +86,21 @@
/** @return name used in {@code project.config} permissions. */
@Override
public Optional<String> permissionName() {
- return Optional.of(Permission.forLabel(label()));
+ switch (forUser) {
+ case SELF:
+ return Optional.of(Permission.forLabel(name));
+ case ON_BEHALF_OF:
+ return Optional.of(Permission.forLabelAs(name));
+ }
+ return Optional.empty();
}
@Override
public String describeForException() {
- return "label " + label();
+ if (forUser == ON_BEHALF_OF) {
+ return "labelAs " + name;
+ }
+ return "label " + name;
}
@Override
@@ -57,26 +110,87 @@
@Override
public boolean equals(Object other) {
- return other instanceof LabelPermission && name.equals(((LabelPermission) other).name);
+ if (other instanceof LabelPermission) {
+ LabelPermission b = (LabelPermission) other;
+ return forUser == b.forUser && name.equals(b.name);
+ }
+ return false;
}
@Override
public String toString() {
+ if (forUser == ON_BEHALF_OF) {
+ return "LabelAs[" + name + ']';
+ }
return "Label[" + name + ']';
}
/** A {@link LabelPermission} at a specific value. */
public static class WithValue implements ChangePermissionOrLabel {
+ private final ForUser forUser;
private final LabelVote label;
/**
* Construct a reference to a label at a specific value.
*
+ * @param type description of the label.
+ * @param value numeric score assigned to the label.
+ */
+ public WithValue(LabelType type, LabelValue value) {
+ this(SELF, type, value);
+ }
+
+ /**
+ * Construct a reference to a label at a specific value.
+ *
+ * @param type description of the label.
+ * @param value numeric score assigned to the label.
+ */
+ public WithValue(LabelType type, short value) {
+ this(SELF, type.getName(), value);
+ }
+
+ /**
+ * Construct a reference to a label at a specific value.
+ *
+ * @param forUser {@code SELF} (default) or {@code ON_BEHALF_OF} for labelAs behavior.
+ * @param type description of the label.
+ * @param value numeric score assigned to the label.
+ */
+ public WithValue(ForUser forUser, LabelType type, LabelValue value) {
+ this(forUser, type.getName(), value.getValue());
+ }
+
+ /**
+ * Construct a reference to a label at a specific value.
+ *
+ * @param forUser {@code SELF} (default) or {@code ON_BEHALF_OF} for labelAs behavior.
+ * @param type description of the label.
+ * @param value numeric score assigned to the label.
+ */
+ public WithValue(ForUser forUser, LabelType type, short value) {
+ this(forUser, type.getName(), value);
+ }
+
+ /**
+ * Construct a reference to a label at a specific value.
+ *
* @param name name of the label, e.g. {@code "Code-Review"} or {@code "Verified"}.
* @param value numeric score assigned to the label.
*/
public WithValue(String name, short value) {
- this(LabelVote.create(name, value));
+ this(SELF, name, value);
+ }
+
+ /**
+ * Construct a reference to a label at a specific value.
+ *
+ * @param forUser {@code SELF} (default) or {@code ON_BEHALF_OF} for labelAs behavior.
+ * @param name name of the label, e.g. {@code "Code-Review"} or {@code "Verified"}.
+ * @param value numeric score assigned to the label.
+ */
+ public WithValue(ForUser forUser, String name, short value) {
+ this(forUser, LabelVote.create(name, value));
}
/**
@@ -85,9 +199,25 @@
* @param label label name and vote.
*/
public WithValue(LabelVote label) {
+ this(SELF, label);
+ }
+
+ /**
+ * Construct a reference to a label at a specific value.
+ *
+ * @param forUser {@code SELF} (default) or {@code ON_BEHALF_OF} for labelAs behavior.
+ * @param label label name and vote.
+ */
+ public WithValue(ForUser forUser, LabelVote label) {
+ this.forUser = checkNotNull(forUser, "ForUser");
this.label = checkNotNull(label, "LabelVote");
}
+ /** @return {@code SELF} or {@code ON_BEHALF_OF} (or labelAs). */
+ public ForUser forUser() {
+ return forUser;
+ }
+
/** @return name of the label, e.g. {@code "Code-Review"}. */
public String label() {
return label.label();
@@ -101,11 +231,20 @@
/** @return name used in {@code project.config} permissions. */
@Override
public Optional<String> permissionName() {
- return Optional.of(Permission.forLabel(label()));
+ switch (forUser) {
+ case SELF:
+ return Optional.of(Permission.forLabel(label()));
+ case ON_BEHALF_OF:
+ return Optional.of(Permission.forLabelAs(label()));
+ }
+ return Optional.empty();
}
@Override
public String describeForException() {
+ if (forUser == ON_BEHALF_OF) {
+ return "labelAs " + label.formatWithEquals();
+ }
return "label " + label.formatWithEquals();
}
@@ -116,11 +255,18 @@
@Override
public boolean equals(Object other) {
- return other instanceof WithValue && label.equals(((WithValue) other).label);
+ if (other instanceof WithValue) {
+ WithValue b = (WithValue) other;
+ return forUser == b.forUser && label.equals(b.label);
+ }
+ return false;
}
@Override
public String toString() {
+ if (forUser == ON_BEHALF_OF) {
+ return "LabelAs[" + label.format() + ']';
+ }
return "Label[" + label.format() + ']';
}
}
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 9e5350b..7a76949 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
@@ -15,7 +15,9 @@
package com.google.gerrit.server.permissions;
import static com.google.common.base.Preconditions.checkNotNull;
+import static java.util.stream.Collectors.toSet;
+import com.google.gerrit.common.data.LabelType;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.reviewdb.client.Branch;
import com.google.gerrit.reviewdb.client.Project;
@@ -174,6 +176,9 @@
/** PermissionBackend scoped to a user, project, reference and change. */
public abstract static class ForChange extends AcceptsReviewDb<ForChange> {
+ /** @return user this instance is scoped to. */
+ public abstract CurrentUser user();
+
/** @return new instance rescoped to same change, but different {@code user}. */
public abstract ForChange user(CurrentUser user);
@@ -207,5 +212,97 @@
return false;
}
}
+
+ /**
+ * Test which values of a label the user may be able to set.
+ *
+ * @param label definition of the label to test values of.
+ * @return set containing values the user may be able to use; may be empty if none.
+ * @throws PermissionBackendException if failure consulting backend configuration.
+ */
+ public Set<LabelPermission.WithValue> test(LabelType label) throws PermissionBackendException {
+ return test(valuesOf(checkNotNull(label, "LabelType")));
+ }
+
+ /**
+ * Test which values of a group of labels the user may be able to set.
+ *
+ * @param types definition of the labels to test values of.
+ * @return set containing values the user may be able to use; may be empty if none.
+ * @throws PermissionBackendException if failure consulting backend configuration.
+ */
+ public Set<LabelPermission.WithValue> testLabels(Collection<LabelType> types)
+ throws PermissionBackendException {
+ checkNotNull(types, "LabelType");
+ return test(types.stream().flatMap((t) -> valuesOf(t).stream()).collect(toSet()));
+ }
+
+ private static Set<LabelPermission.WithValue> valuesOf(LabelType label) {
+ return label
+ .getValues()
+ .stream()
+ .map((v) -> new LabelPermission.WithValue(label, v))
+ .collect(toSet());
+ }
+
+ /**
+ * Squash a label value to the nearest allowed value.
+ *
+ * <p>For multi-valued labels like Code-Review with values -2..+2 a user may try to use +2, but
+ * only have permission for the -1..+1 range. The caller should have already tried:
+ *
+ * <pre>
+ * check(new LabelPermission.WithValue("Code-Review", 2));
+ * </pre>
+ *
+ * and caught {@link AuthException}. {@code squashThenCheck} will use {@link #test(LabelType)}
+ * to determine potential values of Code-Review the user can use, and select the nearest value
+ * along the same sign, e.g. -1 for -2 and +1 for +2.
+ *
+ * @param label definition of the label to test values of.
+ * @param val previously denied value the user attempted.
+ * @return nearest allowed value, or {@code 0} if no value was allowed.
+ * @throws PermissionBackendException backend cannot run test or check.
+ */
+ public short squashThenCheck(LabelType label, short val) throws PermissionBackendException {
+ short s = squashByTest(label, val);
+ if (s == 0 || s == val) {
+ return 0;
+ }
+ try {
+ check(new LabelPermission.WithValue(label, s));
+ return s;
+ } catch (AuthException e) {
+ return 0;
+ }
+ }
+
+ /**
+ * Squash a label value to the nearest allowed value using only test methods.
+ *
+ * <p>Tests all possible values and selects the closet available to {@code val} while matching
+ * the sign of {@code val}. Unlike {@code #squashThenCheck(LabelType, short)} this method only
+ * uses {@code test} methods and should not be used in contexts like a review handler without
+ * checking the resulting score.
+ *
+ * @param label definition of the label to test values of.
+ * @param val previously denied value the user attempted.
+ * @return nearest likely allowed value, or {@code 0} if no value was identified.
+ * @throws PermissionBackendException backend cannot run test.
+ */
+ public short squashByTest(LabelType label, short val) throws PermissionBackendException {
+ return nearest(test(label), val);
+ }
+
+ private static short nearest(Iterable<LabelPermission.WithValue> possible, short wanted) {
+ short s = 0;
+ for (LabelPermission.WithValue v : possible) {
+ if ((wanted < 0 && v.value() < 0 && wanted <= v.value() && v.value() < s)
+ || (wanted > 0 && v.value() > 0 && wanted >= v.value() && v.value() > s)) {
+ s = v.value();
+ }
+ }
+ return s;
+ }
}
}
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 af89d94..3e5eba3 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
@@ -16,6 +16,7 @@
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkState;
+import static com.google.gerrit.server.permissions.LabelPermission.ForUser.ON_BEHALF_OF;
import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
@@ -527,8 +528,13 @@
}
@Override
+ public CurrentUser user() {
+ return getUser();
+ }
+
+ @Override
public ForChange user(CurrentUser user) {
- return getUser().equals(user) ? this : forUser(user).asForChange(cd, db);
+ return user().equals(user) ? this : forUser(user).asForChange(cd, db);
}
@Override
@@ -603,7 +609,11 @@
}
private boolean can(LabelPermission.WithValue perm) {
- return label(perm.permissionName().get()).contains(perm.value());
+ PermissionRange r = label(perm.permissionName().get());
+ if (perm.forUser() == ON_BEHALF_OF && r.isEmpty()) {
+ return false;
+ }
+ return r.contains(perm.value());
}
private PermissionRange label(String permission) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java
index 1dbe5cd..64ef091 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java
@@ -33,6 +33,7 @@
import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
import com.google.gerrit.common.Nullable;
+import com.google.gerrit.common.data.LabelTypes;
import com.google.gerrit.common.data.SubmitRecord;
import com.google.gerrit.common.data.SubmitTypeRecord;
import com.google.gerrit.reviewdb.client.Account;
@@ -757,6 +758,10 @@
return change;
}
+ public LabelTypes getLabelTypes() {
+ return changeControl.getLabelTypes();
+ }
+
public ChangeNotes notes() throws OrmException {
if (notes == null) {
if (!lazyLoad) {
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 6b66c41..1ae54ee 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
@@ -66,6 +66,7 @@
import com.google.gerrit.server.notedb.NotesMigration;
import com.google.gerrit.server.notedb.ReviewerStateInternal;
import com.google.gerrit.server.patch.PatchListCache;
+import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.ListChildProjects;
import com.google.gerrit.server.project.ProjectCache;
@@ -182,6 +183,7 @@
final AccountResolver accountResolver;
final AllProjectsName allProjectsName;
final AllUsersName allUsersName;
+ final PermissionBackend permissionBackend;
final CapabilityControl.Factory capabilityControlFactory;
final ChangeControl.GenericFactory changeControlGenericFactory;
final ChangeData.Factory changeDataFactory;
@@ -221,6 +223,7 @@
DynamicMap<ChangeHasOperandFactory> hasOperands,
IdentifiedUser.GenericFactory userFactory,
Provider<CurrentUser> self,
+ PermissionBackend permissionBackend,
CapabilityControl.Factory capabilityControlFactory,
ChangeControl.GenericFactory changeControlGenericFactory,
ChangeNotes.Factory notesFactory,
@@ -253,6 +256,7 @@
hasOperands,
userFactory,
self,
+ permissionBackend,
capabilityControlFactory,
changeControlGenericFactory,
notesFactory,
@@ -287,6 +291,7 @@
DynamicMap<ChangeHasOperandFactory> hasOperands,
IdentifiedUser.GenericFactory userFactory,
Provider<CurrentUser> self,
+ PermissionBackend permissionBackend,
CapabilityControl.Factory capabilityControlFactory,
ChangeControl.GenericFactory changeControlGenericFactory,
ChangeNotes.Factory notesFactory,
@@ -317,6 +322,7 @@
this.opFactories = opFactories;
this.userFactory = userFactory;
this.self = self;
+ this.permissionBackend = permissionBackend;
this.capabilityControlFactory = capabilityControlFactory;
this.notesFactory = notesFactory;
this.changeControlGenericFactory = changeControlGenericFactory;
@@ -353,6 +359,7 @@
hasOperands,
userFactory,
Providers.of(otherUser),
+ permissionBackend,
capabilityControlFactory,
changeControlGenericFactory,
notesFactory,
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/EqualsLabelPredicate.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/EqualsLabelPredicate.java
index bb251cb..a5814fe 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/EqualsLabelPredicate.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/EqualsLabelPredicate.java
@@ -16,7 +16,6 @@
import com.google.gerrit.common.data.LabelType;
import com.google.gerrit.common.data.LabelTypes;
-import com.google.gerrit.common.data.Permission;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.client.Change;
@@ -24,8 +23,9 @@
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.index.change.ChangeField;
-import com.google.gerrit.server.project.ChangeControl;
-import com.google.gerrit.server.project.NoSuchChangeException;
+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.ProjectCache;
import com.google.gerrit.server.project.ProjectState;
import com.google.gwtorm.server.OrmException;
@@ -33,7 +33,7 @@
class EqualsLabelPredicate extends ChangeIndexPredicate {
private final ProjectCache projectCache;
- private final ChangeControl.GenericFactory ccFactory;
+ private final PermissionBackend permissionBackend;
private final IdentifiedUser.GenericFactory userFactory;
private final Provider<ReviewDb> dbProvider;
private final String label;
@@ -43,7 +43,7 @@
EqualsLabelPredicate(LabelPredicate.Args args, String label, int expVal, Account.Id account) {
super(ChangeField.LABEL, ChangeField.formatLabel(label, expVal, account));
- this.ccFactory = args.ccFactory;
+ this.permissionBackend = args.permissionBackend;
this.projectCache = args.projectCache;
this.userFactory = args.userFactory;
this.dbProvider = args.dbProvider;
@@ -78,7 +78,7 @@
for (PatchSetApproval p : object.currentApprovals()) {
if (labelType.matches(p)) {
hasVote = true;
- if (match(c, p.getValue(), p.getAccountId(), labelType)) {
+ if (match(object, p.getValue(), p.getAccountId(), labelType)) {
return true;
}
}
@@ -104,40 +104,28 @@
return null;
}
- private boolean match(Change change, int value, Account.Id approver, LabelType type)
- throws OrmException {
- int psVal = value;
- if (psVal == expVal) {
- // Double check the value is still permitted for the user.
- //
- IdentifiedUser reviewer = userFactory.create(approver);
- try {
- ChangeControl cc = ccFactory.controlFor(dbProvider.get(), change, reviewer);
- if (!cc.isVisible(dbProvider.get())) {
- // The user can't see the change anymore.
- //
- return false;
- }
- psVal = cc.getRange(Permission.forLabel(type.getName())).squash(psVal);
- } catch (NoSuchChangeException e) {
- // The project has disappeared.
- //
- return false;
- }
-
- if (account != null && !account.equals(approver)) {
- return false;
- }
-
- if (group != null && !reviewer.getEffectiveGroups().contains(group)) {
- return false;
- }
-
- if (psVal == expVal) {
- return true;
- }
+ private boolean match(ChangeData cd, short value, Account.Id approver, LabelType type) {
+ if (value != expVal) {
+ return false;
}
- return false;
+
+ if (account != null && !account.equals(approver)) {
+ return false;
+ }
+
+ IdentifiedUser reviewer = userFactory.create(approver);
+ if (group != null && !reviewer.getEffectiveGroups().contains(group)) {
+ return false;
+ }
+
+ // Double check the value is still permitted for the user.
+ try {
+ PermissionBackend.ForChange perm =
+ permissionBackend.user(reviewer).database(dbProvider).change(cd);
+ return perm.test(ChangePermission.READ) && expVal == perm.squashByTest(type, value);
+ } catch (PermissionBackendException e) {
+ return false;
+ }
}
@Override
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/LabelPredicate.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/LabelPredicate.java
index 9fdbcef..3fe6a6f 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/LabelPredicate.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/LabelPredicate.java
@@ -19,6 +19,7 @@
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.query.OrPredicate;
@@ -36,6 +37,7 @@
static class Args {
final ProjectCache projectCache;
+ final PermissionBackend permissionBackend;
final ChangeControl.GenericFactory ccFactory;
final IdentifiedUser.GenericFactory userFactory;
final Provider<ReviewDb> dbProvider;
@@ -45,6 +47,7 @@
private Args(
ProjectCache projectCache,
+ PermissionBackend permissionBackend,
ChangeControl.GenericFactory ccFactory,
IdentifiedUser.GenericFactory userFactory,
Provider<ReviewDb> dbProvider,
@@ -52,6 +55,7 @@
Set<Account.Id> accounts,
AccountGroup.UUID group) {
this.projectCache = projectCache;
+ this.permissionBackend = permissionBackend;
this.ccFactory = ccFactory;
this.userFactory = userFactory;
this.dbProvider = dbProvider;
@@ -84,6 +88,7 @@
predicates(
new Args(
a.projectCache,
+ a.permissionBackend,
a.changeControlGenericFactory,
a.userFactory,
a.db,
diff --git a/gerrit-server/src/main/java/gerrit/PRED__check_user_label_3.java b/gerrit-server/src/main/java/gerrit/PRED__check_user_label_3.java
new file mode 100644
index 0000000..b2b9890
--- /dev/null
+++ b/gerrit-server/src/main/java/gerrit/PRED__check_user_label_3.java
@@ -0,0 +1,104 @@
+// Copyright (C) 2017 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package gerrit;
+
+import com.google.gerrit.common.data.LabelType;
+import com.google.gerrit.extensions.restapi.AuthException;
+import com.google.gerrit.rules.StoredValues;
+import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.permissions.LabelPermission;
+import com.google.gerrit.server.permissions.PermissionBackendException;
+import com.google.gerrit.server.query.change.ChangeData;
+import com.googlecode.prolog_cafe.exceptions.IllegalTypeException;
+import com.googlecode.prolog_cafe.exceptions.PInstantiationException;
+import com.googlecode.prolog_cafe.exceptions.PrologException;
+import com.googlecode.prolog_cafe.exceptions.SystemException;
+import com.googlecode.prolog_cafe.lang.IntegerTerm;
+import com.googlecode.prolog_cafe.lang.JavaObjectTerm;
+import com.googlecode.prolog_cafe.lang.Operation;
+import com.googlecode.prolog_cafe.lang.Predicate;
+import com.googlecode.prolog_cafe.lang.Prolog;
+import com.googlecode.prolog_cafe.lang.SymbolTerm;
+import com.googlecode.prolog_cafe.lang.Term;
+import com.googlecode.prolog_cafe.lang.VariableTerm;
+
+/**
+ * Checks user can set label to val.
+ *
+ * <pre>
+ * '_check_user_label'(+Label, +CurrentUser, +Val)
+ * </pre>
+ */
+class PRED__check_user_label_3 extends Predicate.P3 {
+ PRED__check_user_label_3(Term a1, Term a2, Term a3, Operation n) {
+ arg1 = a1;
+ arg2 = a2;
+ arg3 = a3;
+ cont = n;
+ }
+
+ @Override
+ public Operation exec(Prolog engine) throws PrologException {
+ engine.setB0();
+ Term a1 = arg1.dereference();
+ Term a2 = arg2.dereference();
+ Term a3 = arg3.dereference();
+
+ if (a1 instanceof VariableTerm) {
+ throw new PInstantiationException(this, 1);
+ }
+ if (!(a1 instanceof SymbolTerm)) {
+ throw new IllegalTypeException(this, 1, "atom", a1);
+ }
+ String label = a1.name();
+
+ if (a2 instanceof VariableTerm) {
+ throw new PInstantiationException(this, 2);
+ }
+ if (!(a2 instanceof JavaObjectTerm) || !a2.convertible(CurrentUser.class)) {
+ throw new IllegalTypeException(this, 2, "CurrentUser)", a2);
+ }
+ CurrentUser user = (CurrentUser) ((JavaObjectTerm) a2).object();
+
+ if (a3 instanceof VariableTerm) {
+ throw new PInstantiationException(this, 3);
+ }
+ if (!(a3 instanceof IntegerTerm)) {
+ throw new IllegalTypeException(this, 3, "integer", a3);
+ }
+ short val = (short) ((IntegerTerm) a3).intValue();
+
+ ChangeData cd = StoredValues.CHANGE_DATA.get(engine);
+ LabelType type = cd.getLabelTypes().byLabel(label);
+ if (type == null) {
+ return engine.fail();
+ }
+
+ try {
+ StoredValues.PERMISSION_BACKEND
+ .get(engine)
+ .user(user)
+ .change(cd)
+ .check(new LabelPermission.WithValue(type, val));
+ return cont;
+ } catch (AuthException err) {
+ return engine.fail();
+ } catch (PermissionBackendException err) {
+ SystemException se = new SystemException(err.getMessage());
+ se.initCause(err);
+ throw se;
+ }
+ }
+}
diff --git a/gerrit-server/src/main/java/gerrit/PRED__load_commit_labels_1.java b/gerrit-server/src/main/java/gerrit/PRED__load_commit_labels_1.java
index 8b5a33d..5a3d656 100644
--- a/gerrit-server/src/main/java/gerrit/PRED__load_commit_labels_1.java
+++ b/gerrit-server/src/main/java/gerrit/PRED__load_commit_labels_1.java
@@ -38,7 +38,7 @@
Term listHead = Prolog.Nil;
try {
ChangeData cd = StoredValues.CHANGE_DATA.get(engine);
- LabelTypes types = StoredValues.CHANGE_CONTROL.get(engine).getLabelTypes();
+ LabelTypes types = cd.getLabelTypes();
for (PatchSetApproval a : cd.currentApprovals()) {
LabelType t = types.byLabel(a.getLabelId());
diff --git a/gerrit-server/src/main/java/gerrit/PRED__user_label_range_4.java b/gerrit-server/src/main/java/gerrit/PRED__user_label_range_4.java
index d06664e..5c61007 100644
--- a/gerrit-server/src/main/java/gerrit/PRED__user_label_range_4.java
+++ b/gerrit-server/src/main/java/gerrit/PRED__user_label_range_4.java
@@ -14,14 +14,16 @@
package gerrit;
-import com.google.gerrit.common.data.Permission;
-import com.google.gerrit.common.data.PermissionRange;
+import com.google.gerrit.common.data.LabelType;
import com.google.gerrit.rules.StoredValues;
import com.google.gerrit.server.CurrentUser;
-import com.google.gerrit.server.project.ChangeControl;
+import com.google.gerrit.server.permissions.LabelPermission;
+import com.google.gerrit.server.permissions.PermissionBackendException;
+import com.google.gerrit.server.query.change.ChangeData;
import com.googlecode.prolog_cafe.exceptions.IllegalTypeException;
import com.googlecode.prolog_cafe.exceptions.PInstantiationException;
import com.googlecode.prolog_cafe.exceptions.PrologException;
+import com.googlecode.prolog_cafe.exceptions.SystemException;
import com.googlecode.prolog_cafe.lang.IntegerTerm;
import com.googlecode.prolog_cafe.lang.JavaObjectTerm;
import com.googlecode.prolog_cafe.lang.Operation;
@@ -30,12 +32,13 @@
import com.googlecode.prolog_cafe.lang.SymbolTerm;
import com.googlecode.prolog_cafe.lang.Term;
import com.googlecode.prolog_cafe.lang.VariableTerm;
+import java.util.Set;
/**
* Resolves the valid range for a label on a CurrentUser.
*
* <pre>
- * '$user_label_range'(+Label, +CurrentUser, -Min, -Max)
+ * '_user_label_range'(+Label, +CurrentUser, -Min, -Max)
* </pre>
*/
class PRED__user_label_range_4 extends Predicate.P4 {
@@ -71,20 +74,33 @@
}
CurrentUser user = (CurrentUser) ((JavaObjectTerm) a2).object();
- ChangeControl ctl = StoredValues.CHANGE_CONTROL.get(engine).forUser(user);
- PermissionRange range = ctl.getRange(Permission.LABEL + label);
- if (range == null) {
+ ChangeData cd = StoredValues.CHANGE_DATA.get(engine);
+ LabelType type = cd.getLabelTypes().byLabel(label);
+ if (type == null) {
return engine.fail();
}
- IntegerTerm min = new IntegerTerm(range.getMin());
- IntegerTerm max = new IntegerTerm(range.getMax());
+ Set<LabelPermission.WithValue> can;
+ try {
+ can = StoredValues.PERMISSION_BACKEND.get(engine).user(user).change(cd).test(type);
+ } catch (PermissionBackendException err) {
+ SystemException se = new SystemException(err.getMessage());
+ se.initCause(err);
+ throw se;
+ }
- if (!a3.unify(min, engine.trail)) {
+ int min = 0;
+ int max = 0;
+ for (LabelPermission.WithValue v : can) {
+ min = Math.min(min, v.value());
+ max = Math.max(max, v.value());
+ }
+
+ if (!a3.unify(new IntegerTerm(min), engine.trail)) {
return engine.fail();
}
- if (!a4.unify(max, engine.trail)) {
+ if (!a4.unify(new IntegerTerm(max), engine.trail)) {
return engine.fail();
}
diff --git a/gerrit-server/src/main/java/gerrit/PRED_get_legacy_label_types_1.java b/gerrit-server/src/main/java/gerrit/PRED_get_legacy_label_types_1.java
index ea3fb17..33d63c4 100644
--- a/gerrit-server/src/main/java/gerrit/PRED_get_legacy_label_types_1.java
+++ b/gerrit-server/src/main/java/gerrit/PRED_get_legacy_label_types_1.java
@@ -51,7 +51,7 @@
public Operation exec(Prolog engine) throws PrologException {
engine.setB0();
Term a1 = arg1.dereference();
- List<LabelType> list = StoredValues.CHANGE_CONTROL.get(engine).getLabelTypes().getLabelTypes();
+ List<LabelType> list = StoredValues.CHANGE_DATA.get(engine).getLabelTypes().getLabelTypes();
Term head = Prolog.Nil;
for (int idx = list.size() - 1; 0 <= idx; idx--) {
head = new ListTerm(export(list.get(idx)), head);
diff --git a/gerrit-server/src/main/prolog/gerrit_common.pl b/gerrit-server/src/main/prolog/gerrit_common.pl
index 59c926f..4671e0d 100644
--- a/gerrit-server/src/main/prolog/gerrit_common.pl
+++ b/gerrit-server/src/main/prolog/gerrit_common.pl
@@ -92,6 +92,27 @@
%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
%%
+%% check_user_label/3:
+%%
+%% Check Who can set Label to Val.
+%%
+check_user_label(Label, Who, Val) :-
+ hash_get(commit_labels, '$fast_range', true), !,
+ atom(Label),
+ assume_range_from_label(Label, Who, Min, Max),
+ Min @=< Val, Val @=< Max.
+check_user_label(Label, Who, Val) :-
+ Who = user(_), !,
+ atom(Label),
+ current_user(Who, User),
+ '_check_user_label'(Label, User, Val).
+check_user_label(Label, test_user(Name), Val) :-
+ clause(user:test_grant(Label, test_user(Name), range(Min, Max)), _),
+ Min @=< Val, Val @=< Max
+ .
+
+%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
+%%
%% user_label_range/4:
%%
%% Lookup the range allowed to be used.
@@ -319,8 +340,7 @@
%%
check_label_range_permission(Label, ExpValue, ok(Who)) :-
commit_label(label(Label, ExpValue), Who),
- user_label_range(Label, Who, Min, Max),
- Min @=< ExpValue, ExpValue @=< Max
+ check_user_label(Label, Who, ExpValue)
.
%TODO Uncomment this clause when group suggesting is possible.
%check_label_range_permission(Label, ExpValue, ask(Group)) :-
diff --git a/gerrit-server/src/test/java/com/google/gerrit/rules/GerritCommonTest.java b/gerrit-server/src/test/java/com/google/gerrit/rules/GerritCommonTest.java
index fa4a951..3dfcaec 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/rules/GerritCommonTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/rules/GerritCommonTest.java
@@ -17,8 +17,8 @@
import static org.easymock.EasyMock.expect;
import com.google.gerrit.common.data.LabelTypes;
-import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.Util;
+import com.google.gerrit.server.query.change.ChangeData;
import com.google.inject.AbstractModule;
import com.googlecode.prolog_cafe.exceptions.CompileException;
import com.googlecode.prolog_cafe.exceptions.ReductionLimitException;
@@ -47,7 +47,8 @@
cfg.setInt("rules", null, "reductionLimit", 1300);
cfg.setInt("rules", null, "compileReductionLimit", (int) 1e6);
bind(PrologEnvironment.Args.class)
- .toInstance(new PrologEnvironment.Args(null, null, null, null, null, null, cfg));
+ .toInstance(
+ new PrologEnvironment.Args(null, null, null, null, null, null, null, cfg));
}
});
}
@@ -55,10 +56,10 @@
@Override
protected void setUpEnvironment(PrologEnvironment env) {
LabelTypes labelTypes = new LabelTypes(Arrays.asList(Util.codeReview(), Util.verified()));
- ChangeControl ctl = EasyMock.createMock(ChangeControl.class);
- expect(ctl.getLabelTypes()).andStubReturn(labelTypes);
- EasyMock.replay(ctl);
- env.set(StoredValues.CHANGE_CONTROL, ctl);
+ ChangeData cd = EasyMock.createMock(ChangeData.class);
+ expect(cd.getLabelTypes()).andStubReturn(labelTypes);
+ EasyMock.replay(cd);
+ env.set(StoredValues.CHANGE_DATA, cd);
}
@Test
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/index/change/FakeQueryBuilder.java b/gerrit-server/src/test/java/com/google/gerrit/server/index/change/FakeQueryBuilder.java
index 6fda100..3bbd335 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/index/change/FakeQueryBuilder.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/index/change/FakeQueryBuilder.java
@@ -27,8 +27,8 @@
new FakeQueryBuilder.Definition<>(FakeQueryBuilder.class),
new ChangeQueryBuilder.Arguments(
null, null, null, null, null, null, null, null, null, null, null, null, null, null,
- null, null, null, null, null, null, null, indexes, null, null, null, null, null, null,
- null, null, null));
+ null, null, null, null, null, null, null, null, indexes, null, null, null, null, null,
+ null, null, null, null));
}
@Operator
