Cleanup access checks in account REST API implementations

The more common pattern is to compare IdentifiedUser using reference
equality. If they are the same object instance in the JVM then its
the same user. This happens when the URL was "/accounts/self/..." and
thus is very clearly for the calling user.

Don't allow registering new emails if the realm doesn't permit it.
Some realms may disable this feature because they want only the
address from the LDAP directory to be used on the server.

Load emails from the cached AccountState, rather than pulling them
from the database. This is the set the user's commits are verified
against so its a more accurate view to show to the user. If the
server cache is behind the database table the API will now reflect
its actually behind.

Shorten a few error messages, avoiding some line wrapping.

Change-Id: I3d644735117ccea3a120c49ace85b992f3ead6d6

gerrit-server/src/main/java/com/google/gerrit/server/account/CreateEmail.java

@@ -18,13 +18,14 @@ import com.google.gerrit.common.errors.EmailException;
 import com.google.gerrit.extensions.restapi.AuthException;
 import com.google.gerrit.extensions.restapi.BadRequestException;
 import com.google.gerrit.extensions.restapi.DefaultInput;
+import com.google.gerrit.extensions.restapi.MethodNotAllowedException;
 import com.google.gerrit.extensions.restapi.ResourceConflictException;
 import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
 import com.google.gerrit.extensions.restapi.Response;
 import com.google.gerrit.extensions.restapi.RestModifyView;
 import com.google.gerrit.reviewdb.client.AuthType;
+import com.google.gerrit.reviewdb.client.Account.FieldName;
 import com.google.gerrit.server.CurrentUser;
-import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.account.CreateEmail.Input;
 import com.google.gerrit.server.account.GetEmails.EmailInfo;
 import com.google.gerrit.server.config.AuthConfig;
@@ -52,6 +53,7 @@ public class CreateEmail implements RestModifyView<AccountResource, Input> {
   }
 
   private final Provider<CurrentUser> self;
+  private final Realm realm;
   private final AuthConfig authConfig;
   private final AccountManager accountManager;
   private final RegisterNewEmailSender.Factory registerNewEmailFactory;
@@ -59,11 +61,15 @@ public class CreateEmail implements RestModifyView<AccountResource, Input> {
   private final String email;
 
   @Inject
-  CreateEmail(Provider<CurrentUser> self, AuthConfig authConfig,
+  CreateEmail(Provider<CurrentUser> self,
+      Realm realm,
+      AuthConfig authConfig,
       AccountManager accountManager,
       RegisterNewEmailSender.Factory registerNewEmailFactory,
-      Provider<PutPreferred> putPreferredProvider, @Assisted String email) {
+      Provider<PutPreferred> putPreferredProvider,
+      @Assisted String email) {
     this.self = self;
+    this.realm = realm;
     this.authConfig = authConfig;
     this.accountManager = accountManager;
     this.registerNewEmailFactory = registerNewEmailFactory;
@@ -74,21 +80,28 @@ public class CreateEmail implements RestModifyView<AccountResource, Input> {
   @Override
   public Object apply(AccountResource rsrc, Input input) throws AuthException,
       BadRequestException, ResourceConflictException,
-      ResourceNotFoundException, OrmException, EmailException {
+      ResourceNotFoundException, OrmException, EmailException,
-    IdentifiedUser s = (IdentifiedUser) self.get();
+      MethodNotAllowedException {
-    if (s.getAccountId().get() != rsrc.getUser().getAccountId().get()
+    if (self.get() != rsrc.getUser()
         && !self.get().getCapabilities().canAdministrateServer()) {
       throw new AuthException("not allowed to add email address");
     }
+
+    if (!realm.allowsEdit(FieldName.REGISTER_NEW_EMAIL)) {
+      throw new MethodNotAllowedException("realm does not allow adding emails");
+    }
+
     if (input == null) {
       input = new Input();
     }
+
     if (input.email != null && !email.equals(input.email)) {
       throw new BadRequestException("email address must match URL");
     }
-    if (input.noConfirmation && !self.get().getCapabilities().canAdministrateServer()) {
+
-      throw new AuthException("not allowed to add email address without confirmation, "
+    if (input.noConfirmation
-          + "need to be Gerrit administrator");
+        && !self.get().getCapabilities().canAdministrateServer()) {
+      throw new AuthException("must be administrator to use no_confirmation");
     }
 
     EmailInfo info = new EmailInfo();
@@ -105,7 +118,7 @@ public class CreateEmail implements RestModifyView<AccountResource, Input> {
         putPreferredProvider.get().apply(
             new AccountResource.Email(rsrc.getUser(), email),
             null);
-        info.setPreferred(true);
+        info.preferred = true;
       }
     } else {
       try {

gerrit-server/src/main/java/com/google/gerrit/server/account/Emails.java

@@ -17,13 +17,11 @@ package com.google.gerrit.server.account;
 import com.google.common.base.Strings;
 import com.google.gerrit.extensions.registration.DynamicMap;
 import com.google.gerrit.extensions.restapi.AcceptsCreate;
-import com.google.gerrit.extensions.restapi.AuthException;
 import com.google.gerrit.extensions.restapi.ChildCollection;
 import com.google.gerrit.extensions.restapi.IdString;
 import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
 import com.google.gerrit.extensions.restapi.RestView;
 import com.google.gerrit.server.CurrentUser;
-import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.account.AccountResource.Email;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
@@ -53,27 +51,24 @@ public class Emails implements
   }
 
   @Override
-  public AccountResource.Email parse(AccountResource parent, IdString id)
+  public AccountResource.Email parse(AccountResource rsrc, IdString id)
-      throws AuthException, ResourceNotFoundException {
+      throws ResourceNotFoundException {
-    if ("preferred".equals(id.get())) {
+    if (self.get() != rsrc.getUser()
-      String preferredEmail = parent.getUser().getAccount().getPreferredEmail();
+        && !self.get().getCapabilities().canAdministrateServer()) {
-      if (!Strings.isNullOrEmpty(preferredEmail)) {
-        return new AccountResource.Email(parent.getUser(), preferredEmail);
-      }
       throw new ResourceNotFoundException();
     }
 
-    if (!(self.get() instanceof IdentifiedUser)) {
+    if ("preferred".equals(id.get())) {
-      throw new AuthException("Authentication required");
+      String email = rsrc.getUser().getAccount().getPreferredEmail();
-    }
+      if (Strings.isNullOrEmpty(email)) {
-    IdentifiedUser s = (IdentifiedUser) self.get();
+        throw new ResourceNotFoundException();
-    if (s.getAccountId().equals(parent.getUser().getAccountId())
-        || s.getCapabilities().canAdministrateServer()) {
-      if (parent.getUser().getEmailAddresses().contains(id.get())) {
-        return new AccountResource.Email(parent.getUser(), id.get());
       }
+      return new AccountResource.Email(rsrc.getUser(), email);
+    } else if (rsrc.getUser().getEmailAddresses().contains(id.get())) {
+      return new AccountResource.Email(rsrc.getUser(), id.get());
+    } else {
+      throw new ResourceNotFoundException();
     }
-    throw new ResourceNotFoundException();
   }
 
   @Override

gerrit-server/src/main/java/com/google/gerrit/server/account/GetEmail.java

@@ -18,13 +18,11 @@ import com.google.gerrit.extensions.restapi.RestReadView;
 import com.google.gerrit.server.account.GetEmails.EmailInfo;
 
 public class GetEmail implements RestReadView<AccountResource.Email> {
-
   @Override
   public EmailInfo apply(AccountResource.Email rsrc) {
     EmailInfo e = new EmailInfo();
     e.email = rsrc.getEmail();
-    e.setPreferred(rsrc.getEmail().equals(
+    e.preferred(rsrc.getUser().getAccount().getPreferredEmail());
-        rsrc.getUser().getAccount().getPreferredEmail()));
     return e;
   }
 }

gerrit-server/src/main/java/com/google/gerrit/server/account/GetEmails.java

@@ -17,54 +17,46 @@ package com.google.gerrit.server.account;
 import com.google.common.collect.Lists;
 import com.google.gerrit.extensions.restapi.AuthException;
 import com.google.gerrit.extensions.restapi.RestReadView;
-import com.google.gerrit.reviewdb.client.AccountExternalId;
-import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.CurrentUser;
-import com.google.gerrit.server.IdentifiedUser;
 import com.google.gwtorm.server.OrmException;
-import com.google.gwtorm.server.ResultSet;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
 
+import java.util.Collections;
+import java.util.Comparator;
 import java.util.List;
 
 public class GetEmails implements RestReadView<AccountResource> {
-
   private final Provider<CurrentUser> self;
-  private final Provider<ReviewDb> dbProvider;
 
   @Inject
-  public GetEmails(Provider<CurrentUser> self, Provider<ReviewDb> dbProvider) {
+  public GetEmails(Provider<CurrentUser> self) {
     this.self = self;
-    this.dbProvider = dbProvider;
   }
 
   @Override
   public List<EmailInfo> apply(AccountResource rsrc) throws AuthException,
       OrmException {
-    if (!(self.get() instanceof IdentifiedUser)) {
+    if (self.get() != rsrc.getUser()
-      throw new AuthException("Authentication required");
+        && !self.get().getCapabilities().canAdministrateServer()) {
-    }
-    IdentifiedUser s = (IdentifiedUser) self.get();
-    if (s.getAccountId().get() != rsrc.getUser().getAccountId().get()
-        && !s.getCapabilities().canAdministrateServer()) {
       throw new AuthException("not allowed to list email addresses");
     }
 
     List<EmailInfo> emails = Lists.newArrayList();
-    ResultSet<AccountExternalId> ids =
+    for (String email : rsrc.getUser().getEmailAddresses()) {
-        dbProvider.get().accountExternalIds()
-            .byAccount(rsrc.getUser().getAccountId());
-    for (AccountExternalId extId : ids) {
-      String email = extId.getEmailAddress();
       if (email != null) {
         EmailInfo e = new EmailInfo();
         e.email = email;
-        e.setPreferred(email.equals(rsrc.getUser().getAccount()
+        e.preferred(rsrc.getUser().getAccount().getPreferredEmail());
-            .getPreferredEmail()));
         emails.add(e);
       }
     }
+    Collections.sort(emails, new Comparator<EmailInfo>() {
+      @Override
+      public int compare(EmailInfo a, EmailInfo b) {
+        return a.email.compareTo(b.email);
+      }
+    });
     return emails;
   }
 
@@ -73,8 +65,8 @@ public class GetEmails implements RestReadView<AccountResource> {
     public Boolean preferred;
     public Boolean pendingConfirmation;
 
-    void setPreferred(boolean preferred) {
+    void preferred(String e) {
-      this.preferred = preferred ? true : null;
+      this.preferred = e != null && e.equals(email) ? true : null;
     }
   }
 }

gerrit-server/src/main/java/com/google/gerrit/server/account/PutAccount.java

@@ -22,7 +22,6 @@ public class PutAccount implements RestModifyView<AccountResource, Input> {
   @Override
   public Object apply(AccountResource resource, Input input)
       throws ResourceConflictException {
-    throw new ResourceConflictException("Account \"" + resource.getUser().getNameEmail()
+    throw new ResourceConflictException("account exists");
-        + "\" already exists");
   }
 }

gerrit-server/src/main/java/com/google/gerrit/server/account/PutEmail.java

@@ -19,11 +19,9 @@ import com.google.gerrit.extensions.restapi.RestModifyView;
 import com.google.gerrit.server.account.CreateEmail.Input;
 
 public class PutEmail implements RestModifyView<AccountResource.Email, Input> {
-
   @Override
   public Object apply(AccountResource.Email rsrc, Input input)
       throws ResourceConflictException {
-    throw new ResourceConflictException("Email \"" + rsrc.getEmail()
+    throw new ResourceConflictException("email exists");
-        + "\" already exists");
   }
 }

gerrit-server/src/main/java/com/google/gerrit/server/account/PutName.java

@@ -25,7 +25,6 @@ import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.Account.FieldName;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.CurrentUser;
-import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.account.PutName.Input;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
@@ -54,16 +53,16 @@ public class PutName implements RestModifyView<AccountResource, Input> {
   }
 
   @Override
-  public Object apply(AccountResource rsrc, Input input) throws AuthException,
+  public Response<String> apply(AccountResource rsrc, Input input)
-      MethodNotAllowedException, ResourceNotFoundException, OrmException {
+      throws AuthException, MethodNotAllowedException,
-    IdentifiedUser s = (IdentifiedUser) self.get();
+      ResourceNotFoundException, OrmException {
-    if (s.getAccountId().get() != rsrc.getUser().getAccountId().get()
+    if (self.get() != rsrc.getUser()
         && !self.get().getCapabilities().canAdministrateServer()) {
       throw new AuthException("not allowed to change name");
     }
 
     if (!realm.allowsEdit(FieldName.FULL_NAME)) {
-      throw new MethodNotAllowedException("The realm doesn't allow editing names");
+      throw new MethodNotAllowedException("realm does not allow editing name");
     }
 
     if (input == null) {
@@ -72,13 +71,13 @@ public class PutName implements RestModifyView<AccountResource, Input> {
 
     Account a = dbProvider.get().accounts().get(rsrc.getUser().getAccountId());
     if (a == null) {
-      throw new ResourceNotFoundException("No such account: "
+      throw new ResourceNotFoundException("account not found");
-          + rsrc.getUser().getAccountId());
     }
     a.setFullName(input.name);
     dbProvider.get().accounts().update(Collections.singleton(a));
     byIdCache.evict(a.getId());
-    return Strings.isNullOrEmpty(a.getFullName()) ?
+    return Strings.isNullOrEmpty(a.getFullName())
-        Response.none() : a.getFullName();
+        ? Response.<String> none()
+        : Response.ok(a.getFullName());
   }
 }

gerrit-server/src/main/java/com/google/gerrit/server/account/PutPreferred.java

@@ -21,7 +21,6 @@ import com.google.gerrit.extensions.restapi.RestModifyView;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.CurrentUser;
-import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.account.PutPreferred.Input;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
@@ -47,17 +46,16 @@ public class PutPreferred implements
   }
 
   @Override
-  public Object apply(AccountResource.Email rsrc, Input input)
+  public Response<String> apply(AccountResource.Email rsrc, Input input)
       throws AuthException, ResourceNotFoundException, OrmException {
-    IdentifiedUser s = (IdentifiedUser) self.get();
+    if (self.get() != rsrc.getUser()
-    if (s.getAccountId().get() != rsrc.getUser().getAccountId().get()
         && !self.get().getCapabilities().canAdministrateServer()) {
       throw new AuthException("not allowed to set preferred email address");
     }
+
     Account a = dbProvider.get().accounts().get(rsrc.getUser().getAccountId());
     if (a == null) {
-      throw new ResourceNotFoundException("No such account: "
+      throw new ResourceNotFoundException("account not found");
-          + rsrc.getUser().getAccountId());
     }
     if (rsrc.getEmail().equals(a.getPreferredEmail())) {
       return Response.ok("");