opendev/gerrit
Code Issues Proposed changes

Verify labels in Prolog using PermissionBackend

Browse Source 
user_label_range/4 needs to compute the min and max values, so this
predicate uses test(LabelType) to determine the set of allowed values
and picks the min and max.

The new check_user_label/3 takes a specific value and uses the
backend's check method to assert the label can be set to that value.
This improves logic at submit time to use check just before the change
is submitted to the destination branch.

Change-Id: Ia29f16ffc50712ed1b57ec964b9e7930b35a5673
This commit is contained in:
Shawn Pearce
2017-02-19 14:28:18 -08:00
committed by David Pursehouse
parent 1486d3757a
commit 1feb1d1914
9 changed files with 185 additions and 21 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
gerrit-server/src/main/java/com/google/gerrit/rules/PrologEnvironment.java
View File

@@ -20,6 +20,7 @@ import com.google.gerrit.server.config.GerritServerConfig;
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 @@ public class PrologEnvironment extends BufferingPrologControl {
}
private final ProjectCache projectCache;
private final PermissionBackend permissionBackend;
private final GitRepositoryManager repositoryManager;
private final PatchListCache patchListCache;
private final PatchSetInfoFactory patchSetInfoFactory;
@@ -177,6 +179,7 @@ public class PrologEnvironment extends BufferingPrologControl {
@Inject
Args(
ProjectCache projectCache,
PermissionBackend permissionBackend,
GitRepositoryManager repositoryManager,
PatchListCache patchListCache,
PatchSetInfoFactory patchSetInfoFactory,
@@ -184,6 +187,7 @@ public class PrologEnvironment extends BufferingPrologControl {
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 @@ public class PrologEnvironment extends BufferingPrologControl {
return projectCache;
}
public PermissionBackend getPermissionBackend() {
return permissionBackend;
}
public GitRepositoryManager getGitRepositoryManager() {
return repositoryManager;
}

10
gerrit-server/src/main/java/com/google/gerrit/rules/StoredValues.java
View File

@@ -33,6 +33,7 @@ import com.google.gerrit.server.patch.PatchListKey;
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 final class StoredValues {
}
};
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

5
gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java
View File

@@ -33,6 +33,7 @@ import com.google.common.collect.ListMultimap;
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 @@ public class ChangeData {
return change;
}
public LabelTypes getLabelTypes() {
return changeControl.getLabelTypes();
}
public ChangeNotes notes() throws OrmException {
if (notes == null) {
if (!lazyLoad) {

104
gerrit-server/src/main/java/gerrit/PRED__check_user_label_3.java Normal file
View File

@@ -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;
}
}
}

2
gerrit-server/src/main/java/gerrit/PRED__load_commit_labels_1.java
View File

@@ -38,7 +38,7 @@ class PRED__load_commit_labels_1 extends Predicate.P1 {
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());

38
gerrit-server/src/main/java/gerrit/PRED__user_label_range_4.java
View File

@@ -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.Prolog;
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 @@ class PRED__user_label_range_4 extends Predicate.P4 {
}
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();
}

2
gerrit-server/src/main/java/gerrit/PRED_get_legacy_label_types_1.java
View File

@@ -51,7 +51,7 @@ class PRED_get_legacy_label_types_1 extends Predicate.P1 {
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);

24
gerrit-server/src/main/prolog/gerrit_common.pl
View File

@@ -90,6 +90,27 @@ index_commit_labels([_ | Rs]) :-
index_commit_labels(Rs).
%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
%%
%% 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:
@@ -319,8 +340,7 @@ max_no_block(Label, Max, need(Max)) :-
%%
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)) :-

13
gerrit-server/src/test/java/com/google/gerrit/rules/GerritCommonTest.java
View File

@@ -17,8 +17,8 @@ package com.google.gerrit.rules;
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 @@ public class GerritCommonTest extends PrologTestCase {
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 @@ public class GerritCommonTest extends PrologTestCase {
@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