Hide ChangeControl.isOwner

Check can directly verify the change owner. DeletePrivate and
PutPrivate are altering the private bit on the change and can
also directly verify the change owner.

While here fix the UiAction predicates for private changes to reflect
the state of the resource and only offer the correct action.  E.g. a
merged change cannot be marked private.

Index should only be run by those with maintainServer permission, as
its otherwise a possible avenue for a change owner to easily DoS the
index system by requesting unnecessary reindexing of the same change.

Change-Id: Ic071b375bd8e8882d5facddee1f8fea785155d29

gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeResource.java

@@ -22,6 +22,7 @@ import com.google.common.hash.Hashing;
 import com.google.gerrit.extensions.restapi.RestResource;
 import com.google.gerrit.extensions.restapi.RestResource.HasETag;
 import com.google.gerrit.extensions.restapi.RestView;
+import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.AccountGroup;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.Project;
@@ -84,6 +85,13 @@ public class ChangeResource implements RestResource, HasETag {
     return getControl().getId();
   }
 
+  /** @return true if {@link #getUser()} is the change's owner. */
+  public boolean isUserOwner() {
+    CurrentUser user = getControl().getUser();
+    Account.Id owner = getChange().getOwner();
+    return user.isIdentifiedUser() && user.asIdentifiedUser().getAccountId().equals(owner);
+  }
+
   public Change getChange() {
     return getControl().getChange();
   }

gerrit-server/src/main/java/com/google/gerrit/server/change/Check.java

@@ -25,7 +25,6 @@ import com.google.gerrit.server.CurrentUser;
 import com.google.gerrit.server.permissions.GlobalPermission;
 import com.google.gerrit.server.permissions.PermissionBackend;
 import com.google.gerrit.server.permissions.PermissionBackendException;
-import com.google.gerrit.server.project.ChangeControl;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
@@ -51,8 +50,7 @@ public class Check
   @Override
   public Response<ChangeInfo> apply(ChangeResource rsrc, FixInput input)
       throws RestApiException, OrmException, PermissionBackendException {
-    ChangeControl ctl = rsrc.getControl();
+    if (!rsrc.isUserOwner() && !rsrc.getControl().getProjectControl().isOwner()) {
-    if (!ctl.isOwner() && !ctl.getProjectControl().isOwner()) {
       permissionBackend.user(user).check(GlobalPermission.MAINTAIN_SERVER);
     }
     return Response.withMustRevalidate(newChangeJson().fix(input).format(rsrc));

gerrit-server/src/main/java/com/google/gerrit/server/change/DeletePrivate.java

@@ -46,7 +46,7 @@ public class DeletePrivate
   @Override
   public Response<String> apply(ChangeResource rsrc, DeletePrivate.Input input)
       throws RestApiException, UpdateException {
-    if (!rsrc.getControl().isOwner()) {
+    if (!rsrc.isUserOwner()) {
       throw new AuthException("not allowed to unmark private");
     }
 
@@ -73,6 +73,6 @@ public class DeletePrivate
     return new UiAction.Description()
         .setLabel("Unmark private")
         .setTitle("Unmark change as private")
-        .setVisible(rsrc.getControl().isOwner());
+        .setVisible(rsrc.getChange().isPrivate() && rsrc.isUserOwner());
   }
 }

gerrit-server/src/main/java/com/google/gerrit/server/change/Index.java

@@ -24,7 +24,6 @@ import com.google.gerrit.server.index.change.ChangeIndexer;
 import com.google.gerrit.server.permissions.GlobalPermission;
 import com.google.gerrit.server.permissions.PermissionBackend;
 import com.google.gerrit.server.permissions.PermissionBackendException;
-import com.google.gerrit.server.project.ChangeControl;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
@@ -55,10 +54,7 @@ public class Index implements RestModifyView<ChangeResource, Input> {
   @Override
   public Response<?> apply(ChangeResource rsrc, Input input)
       throws IOException, AuthException, OrmException, PermissionBackendException {
-    ChangeControl ctl = rsrc.getControl();
+    permissionBackend.user(user).check(GlobalPermission.MAINTAIN_SERVER);
-    if (!ctl.isOwner()) {
-      permissionBackend.user(user).check(GlobalPermission.MAINTAIN_SERVER);
-    }
     indexer.index(db.get(), rsrc.getChange());
     return Response.none();
   }

gerrit-server/src/main/java/com/google/gerrit/server/change/PutPrivate.java

@@ -20,6 +20,7 @@ import com.google.gerrit.extensions.restapi.Response;
 import com.google.gerrit.extensions.restapi.RestApiException;
 import com.google.gerrit.extensions.restapi.RestModifyView;
 import com.google.gerrit.extensions.webui.UiAction;
+import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.project.ChangeControl;
 import com.google.gerrit.server.update.BatchUpdate;
@@ -45,7 +46,7 @@ public class PutPrivate
   @Override
   public Response<String> apply(ChangeResource rsrc, Input input)
       throws RestApiException, UpdateException {
-    if (!rsrc.getControl().isOwner()) {
+    if (!rsrc.isUserOwner()) {
       throw new AuthException("not allowed to mark private");
     }
 
@@ -69,9 +70,13 @@ public class PutPrivate
 
   @Override
   public Description getDescription(ChangeResource rsrc) {
+    Change change = rsrc.getChange();
     return new UiAction.Description()
         .setLabel("Mark private")
         .setTitle("Mark change as private")
-        .setVisible(rsrc.getControl().isOwner());
+        .setVisible(
+            !change.isPrivate()
+                && change.getStatus() != Change.Status.MERGED
+                && rsrc.isUserOwner());
   }
 }

gerrit-server/src/main/java/com/google/gerrit/server/change/SetPrivateOp.java

@@ -14,6 +14,7 @@
 
 package com.google.gerrit.server.change;
 
+import com.google.gerrit.extensions.restapi.ResourceConflictException;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.server.notedb.ChangeUpdate;
 import com.google.gerrit.server.update.BatchUpdateOp;
@@ -27,8 +28,11 @@ class SetPrivateOp implements BatchUpdateOp {
   }
 
   @Override
-  public boolean updateChange(ChangeContext ctx) {
+  public boolean updateChange(ChangeContext ctx) throws ResourceConflictException {
     Change change = ctx.getChange();
+    if (change.getStatus() == Change.Status.MERGED) {
+      throw new ResourceConflictException("change is merged");
+    }
     ChangeUpdate update = ctx.getUpdate(change.currentPatchSetId());
     change.setPrivate(isPrivate);
     change.setLastUpdatedOn(ctx.getWhen());

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

@@ -360,7 +360,7 @@ public class ChangeControl {
   }
 
   /** Is this user the owner of the change? */
-  public boolean isOwner() {
+  private boolean isOwner() {
     if (getUser().isIdentifiedUser()) {
       Account.Id id = getUser().asIdentifiedUser().getAccountId();
       return id.equals(getChange().getOwner());