Disallow current_user/1 predicate in submit type rules

Submit type is used as part of the mergeability bit computation, which
is persistent and so needs a consistent view of submit type. An
informal poll of repo-discuss[1] indicated that current_user/1 is
pretty much only useful for submit rules, not submit type rules.

[1] https://groups.google.com/d/topic/repo-discuss/vW6XhUOkqik/discussion

Change-Id: Ia938a685409025b36e16979fcafd92afbed2d91f

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

@@ -25,6 +25,7 @@ import com.google.gerrit.reviewdb.client.PatchSetInfo;
 import com.google.gerrit.reviewdb.client.Project;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.AnonymousUser;
+import com.google.gerrit.server.CurrentUser;
 import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.git.GitRepositoryManager;
 import com.google.gerrit.server.patch.PatchList;
@@ -50,7 +51,13 @@ public final class StoredValues {
   public static final StoredValue<ReviewDb> REVIEW_DB = create(ReviewDb.class);
   public static final StoredValue<ChangeData> CHANGE_DATA = create(ChangeData.class);
   public static final StoredValue<PatchSet> PATCH_SET = create(PatchSet.class);
+
+  // Note: no guarantees are made about the user passed in the ChangeControl; do
+  // not depend on this directly. Either use .forUser(otherUser) to get a
+  // control for a specific known user, or use CURRENT_USER, which may be null
+  // for rule types that may not depend on the current user.
   public static final StoredValue<ChangeControl> CHANGE_CONTROL = create(ChangeControl.class);
+  public static final StoredValue<CurrentUser> CURRENT_USER = create(CurrentUser.class);
 
   public static Change getChange(Prolog engine) throws SystemException {
     ChangeData cd = CHANGE_DATA.get(engine);

gerrit-server/src/main/java/com/google/gerrit/server/project/SubmitRuleEvaluator.java

@@ -28,6 +28,7 @@ import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.PatchSet;
 import com.google.gerrit.rules.PrologEnvironment;
 import com.google.gerrit.rules.StoredValues;
+import com.google.gerrit.server.CurrentUser;
 import com.google.gerrit.server.query.change.ChangeData;
 import com.google.gwtorm.server.OrmException;
 
@@ -209,7 +210,8 @@ public class SubmitRuleEvaluator {
     List<Term> results;
     try {
       results = evaluateImpl("locate_submit_rule", "can_submit",
-          "locate_submit_filter", "filter_submit_results");
+          "locate_submit_filter", "filter_submit_results",
+          control.getCurrentUser());
     } catch (RuleEvalException e) {
       return ruleError(e.getMessage(), e);
     }
@@ -386,7 +388,11 @@ public class SubmitRuleEvaluator {
     List<Term> results;
     try {
       results = evaluateImpl("locate_submit_type", "get_submit_type",
-          "locate_submit_type_filter", "filter_submit_type_results");
+          "locate_submit_type_filter", "filter_submit_type_results",
+          // Do not include current user in submit type evaluation. This is used
+          // for mergeability checks, which are stored persistently and so must
+          // have a consistent view of the submit type.
+          null);
     } catch (RuleEvalException e) {
       return typeError(e.getMessage(), e);
     }
@@ -436,8 +442,9 @@ public class SubmitRuleEvaluator {
       String userRuleLocatorName,
       String userRuleWrapperName,
       String filterRuleLocatorName,
-      String filterRuleWrapperName) throws RuleEvalException {
+      String filterRuleWrapperName,
-    PrologEnvironment env = getPrologEnvironment();
+      CurrentUser user) throws RuleEvalException {
+    PrologEnvironment env = getPrologEnvironment(user);
     try {
       Term sr = env.once("gerrit", userRuleLocatorName, new VariableTerm());
       if (fastEvalLabels) {
@@ -479,7 +486,8 @@ public class SubmitRuleEvaluator {
     }
   }
 
-  private PrologEnvironment getPrologEnvironment() throws RuleEvalException {
+  private PrologEnvironment getPrologEnvironment(CurrentUser user)
+      throws RuleEvalException {
     checkState(patchSet != null,
         "getPrologEnvironment() called before initPatchSet()");
     ProjectState projectState = control.getProjectControl().getProjectState();
@@ -499,6 +507,9 @@ public class SubmitRuleEvaluator {
     env.set(StoredValues.CHANGE_DATA, cd);
     env.set(StoredValues.PATCH_SET, patchSet);
     env.set(StoredValues.CHANGE_CONTROL, control);
+    if (user != null) {
+      env.set(StoredValues.CURRENT_USER, user);
+    }
     return env;
   }
 

gerrit-server/src/main/java/gerrit/PRED_current_user_1.java

@@ -20,7 +20,6 @@ import com.google.gerrit.server.AnonymousUser;
 import com.google.gerrit.server.CurrentUser;
 import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.PeerDaemonUser;
-import com.google.gerrit.server.project.ChangeControl;
 
 import com.googlecode.prolog_cafe.lang.EvaluationException;
 import com.googlecode.prolog_cafe.lang.IntegerTerm;
@@ -47,8 +46,11 @@ public class PRED_current_user_1 extends Predicate.P1 {
     engine.setB0();
     Term a1 = arg1.dereference();
 
-    ChangeControl cControl = StoredValues.CHANGE_CONTROL.get(engine);
+    CurrentUser curUser = StoredValues.CURRENT_USER.getOrNull(engine);
-    CurrentUser curUser = cControl.getCurrentUser();
+    if (curUser == null) {
+      throw new EvaluationException(
+          "Current user not available in this rule type");
+    }
     Term resultTerm;
 
     if (curUser.isIdentifiedUser()) {
@@ -67,4 +69,4 @@ public class PRED_current_user_1 extends Predicate.P1 {
     }
     return cont;
   }
-}
+}