Allow users to delete OpenID identities no longer used

A user may desire to delete an OpenID identity they had previously
stored into their account, such as if they no longer trust that
provider, or are no longer affiliated with that provider.

To prevent the user from locking themselves out of their own user
account in Gerrit we forbid deleting the identity they last used
to login to the site under.

Signed-off-by: Shawn O. Pearce <sop@google.com>

6 files changed, 237 insertions, 11 deletions

diff --git a/src/main/java/com/google/gerrit/client/account/AccountConstants.java b/src/main/java/com/google/gerrit/client/account/AccountConstants.java
index 3b8415449f..e02b03d006 100644
--- a/src/main/java/com/google/gerrit/client/account/AccountConstants.java
+++ b/src/main/java/com/google/gerrit/client/account/AccountConstants.java
@@ -53,6 +53,7 @@ public interface AccountConstants extends Constants {
   String webIdLastUsed();
   String webIdEmail();
   String webIdIdentity();
+  String buttonDeleteIdentity();
   String buttonLinkIdentity();
 
   String watchedProjects();
diff --git a/src/main/java/com/google/gerrit/client/account/AccountConstants.properties b/src/main/java/com/google/gerrit/client/account/AccountConstants.properties
index 83f6fb1d01..a24641b4e3 100644
--- a/src/main/java/com/google/gerrit/client/account/AccountConstants.properties
+++ b/src/main/java/com/google/gerrit/client/account/AccountConstants.properties
@@ -29,6 +29,7 @@ sshKeyStored = Stored
 webIdLastUsed = Last Login
 webIdEmail = Email Address
 webIdIdentity = Identity
+buttonDeleteIdentity = Delete
 buttonLinkIdentity = Link Another Identity
 
 addSshKeyPanelHeader = Add SSH Public Key
diff --git a/src/main/java/com/google/gerrit/client/account/AccountSecurity.java b/src/main/java/com/google/gerrit/client/account/AccountSecurity.java
index af6ab67aab..3742127ec9 100644
--- a/src/main/java/com/google/gerrit/client/account/AccountSecurity.java
+++ b/src/main/java/com/google/gerrit/client/account/AccountSecurity.java
@@ -42,6 +42,10 @@ public interface AccountSecurity extends RemoteJsonService {
   void myExternalIds(AsyncCallback<List<AccountExternalId>> callback);
 
   @SignInRequired
+  void deleteExternalIds(Set<AccountExternalId.Key> keys,
+      AsyncCallback<Set<AccountExternalId.Key>> callback);
+
+  @SignInRequired
   void updateContact(String fullName, String emailAddr,
       ContactInformation info, AsyncCallback<Account> callback);
 
diff --git a/src/main/java/com/google/gerrit/client/account/ExternalIdPanel.java b/src/main/java/com/google/gerrit/client/account/ExternalIdPanel.java
index 7748688fbc..35585daa47 100644
--- a/src/main/java/com/google/gerrit/client/account/ExternalIdPanel.java
+++ b/src/main/java/com/google/gerrit/client/account/ExternalIdPanel.java
@@ -21,6 +21,7 @@ import com.google.gerrit.client.rpc.Common;
 import com.google.gerrit.client.rpc.GerritCallback;
 import com.google.gerrit.client.ui.FancyFlexTable;
 import com.google.gwt.user.client.ui.Button;
+import com.google.gwt.user.client.ui.CheckBox;
 import com.google.gwt.user.client.ui.ClickListener;
 import com.google.gwt.user.client.ui.Composite;
 import com.google.gwt.user.client.ui.FlowPanel;
@@ -29,10 +30,13 @@ import com.google.gwt.user.client.ui.TableListener;
 import com.google.gwt.user.client.ui.Widget;
 import com.google.gwt.user.client.ui.FlexTable.FlexCellFormatter;
 
+import java.util.HashSet;
 import java.util.List;
+import java.util.Set;
 
 class ExternalIdPanel extends Composite {
   private IdTable identites;
+  private Button deleteIdentity;
 
   ExternalIdPanel() {
     final FlowPanel body = new FlowPanel();
@@ -40,6 +44,14 @@ class ExternalIdPanel extends Composite {
     identites = new IdTable();
     body.add(identites);
 
+    deleteIdentity = new Button(Util.C.buttonDeleteIdentity());
+    deleteIdentity.addClickListener(new ClickListener() {
+      public void onClick(final Widget sender) {
+        identites.deleteChecked();
+      }
+    });
+    body.add(deleteIdentity);
+
     switch (Common.getGerritConfig().getLoginType()) {
       case OPENID: {
         final Button linkIdentity = new Button(Util.C.buttonLinkIdentity());
@@ -85,19 +97,22 @@ class ExternalIdPanel extends Composite {
 
   private class IdTable extends FancyFlexTable<AccountExternalId> {
     IdTable() {
-      table.setText(0, 1, Util.C.webIdLastUsed());
-      table.setText(0, 2, Util.C.webIdEmail());
-      table.setText(0, 3, Util.C.webIdIdentity());
+      table.setText(0, 2, Util.C.webIdLastUsed());
+      table.setText(0, 3, Util.C.webIdEmail());
+      table.setText(0, 4, Util.C.webIdIdentity());
       table.addTableListener(new TableListener() {
         public void onCellClicked(SourcesTableEvents sender, int row, int cell) {
-          movePointerTo(row);
+          if (cell != 1 && getRowItem(row) != null) {
+            movePointerTo(row);
+          }
         }
       });
 
       final FlexCellFormatter fmt = table.getFlexCellFormatter();
-      fmt.addStyleName(0, 1, S_DATA_HEADER);
+      fmt.addStyleName(0, 1, S_ICON_HEADER);
       fmt.addStyleName(0, 2, S_DATA_HEADER);
       fmt.addStyleName(0, 3, S_DATA_HEADER);
+      fmt.addStyleName(0, 4, S_DATA_HEADER);
     }
 
     @Override
@@ -105,6 +120,75 @@ class ExternalIdPanel extends Composite {
       return item.getKey();
     }
 
+    @Override
+    protected boolean onKeyPress(final char keyCode, final int modifiers) {
+      if (super.onKeyPress(keyCode, modifiers)) {
+        return true;
+      }
+      if (modifiers == 0) {
+        switch (keyCode) {
+          case 's':
+          case 'c':
+            toggleCurrentRow();
+            return true;
+        }
+      }
+      return false;
+    }
+
+    @Override
+    protected void onOpenItem(final AccountExternalId item) {
+      toggleCurrentRow();
+    }
+
+    private void toggleCurrentRow() {
+      final CheckBox cb = (CheckBox) table.getWidget(getCurrentRow(), 1);
+      if (cb != null) {
+        cb.setChecked(!cb.isChecked());
+      }
+    }
+
+    void deleteChecked() {
+      final HashSet<AccountExternalId.Key> keys =
+          new HashSet<AccountExternalId.Key>();
+      for (int row = 1; row < table.getRowCount(); row++) {
+        final AccountExternalId k = getRowItem(row);
+        if (k == null) {
+          continue;
+        }
+        final CheckBox cb = (CheckBox) table.getWidget(row, 1);
+        if (cb == null) {
+          continue;
+        }
+        if (cb.isChecked()) {
+          keys.add(k.getKey());
+        }
+      }
+      if (!keys.isEmpty()) {
+        deleteIdentity.setEnabled(false);
+        Util.ACCOUNT_SEC.deleteExternalIds(keys,
+            new GerritCallback<Set<AccountExternalId.Key>>() {
+              public void onSuccess(final Set<AccountExternalId.Key> removed) {
+                deleteIdentity.setEnabled(true);
+                for (int row = 1; row < table.getRowCount();) {
+                  final AccountExternalId k = getRowItem(row);
+                  if (k != null && removed.contains(k.getKey())) {
+                    table.removeRow(row);
+                  } else {
+                    row++;
+                  }
+                }
+              }
+
+              @Override
+              public void onFailure(Throwable caught) {
+                deleteIdentity.setEnabled(true);
+                super.onFailure(caught);
+              }
+            });
+      }
+    }
+
     void display(final List<AccountExternalId> result) {
       while (1 < table.getRowCount())
         table.removeRow(table.getRowCount() - 1);
@@ -112,6 +196,20 @@ class ExternalIdPanel extends Composite {
       for (final AccountExternalId k : result) {
         addOneId(k);
       }
+
+      final AccountExternalId mostRecent = AccountExternalId.mostRecent(result);
+      if (mostRecent != null) {
+        for (int row = 1; row < table.getRowCount(); row++) {
+          if (getRowItem(row) == mostRecent) {
+            // Remove the box from the most recent row, this prevents
+            // the user from trying to delete the identity they last used
+            // to login, possibly locking themselves out of the account.
+            //
+            table.setText(row, 1, "");
+            break;
+          }
+        }
+      }
     }
 
     void addOneId(final AccountExternalId k) {
@@ -119,15 +217,20 @@ class ExternalIdPanel extends Composite {
       table.insertRow(row);
       applyDataRowStyle(row);
 
-      table.setText(row, 1, FormatUtil.mediumFormat(k.getLastUsedOn()));
-      table.setText(row, 2, k.getEmailAddress());
-      table.setText(row, 3, k.getExternalId());
+      if (k.canUserDelete()) {
+        table.setWidget(row, 1, new CheckBox());
+      } else {
+        table.setText(row, 1, "");
+      }
+      table.setText(row, 2, FormatUtil.mediumFormat(k.getLastUsedOn()));
+      table.setText(row, 3, k.getEmailAddress());
+      table.setText(row, 4, k.getExternalId());
 
       final FlexCellFormatter fmt = table.getFlexCellFormatter();
-      fmt.addStyleName(row, 1, S_DATA_CELL);
-      fmt.addStyleName(row, 1, "C_LAST_UPDATE");
-      fmt.addStyleName(row, 2, S_DATA_CELL);
+      fmt.addStyleName(row, 1, S_ICON_CELL);
+      fmt.addStyleName(row, 2, "C_LAST_UPDATE");
       fmt.addStyleName(row, 3, S_DATA_CELL);
+      fmt.addStyleName(row, 4, S_DATA_CELL);
 
       setRowItem(row, k);
     }
diff --git a/src/main/java/com/google/gerrit/client/reviewdb/AccountExternalId.java b/src/main/java/com/google/gerrit/client/reviewdb/AccountExternalId.java
index 4d380527dc..ecbb07b662 100644
--- a/src/main/java/com/google/gerrit/client/reviewdb/AccountExternalId.java
+++ b/src/main/java/com/google/gerrit/client/reviewdb/AccountExternalId.java
@@ -14,10 +14,12 @@
 
 package com.google.gerrit.client.reviewdb;
 
+import com.google.gerrit.client.rpc.Common;
 import com.google.gwtorm.client.Column;
 import com.google.gwtorm.client.StringKey;
 
 import java.sql.Timestamp;
+import java.util.Collection;
 
 /** Association of an external account identifier to a local {@link Account}. */
 public final class AccountExternalId {
@@ -53,6 +55,37 @@ public final class AccountExternalId {
     }
   }
 
+  /**
+   * Select the most recently used identity from a list of identities.
+   * 
+   * @param all all known identities
+   * @return most recently used login identity; null if none matches.
+   */
+  public static AccountExternalId mostRecent(Collection<AccountExternalId> all) {
+    AccountExternalId mostRecent = null;
+    for (final AccountExternalId e : all) {
+      final Timestamp lastUsed = e.getLastUsedOn();
+      if (lastUsed == null) {
+        // Identities without logins have never been used, so
+        // they can't be the most recent.
+        //
+        continue;
+      }
+
+      if (e.getExternalId().startsWith("mailto:")) {
+        // Don't ever consider an email address as a "recent login"
+        //
+        continue;
+      }
+
+      if (mostRecent == null
+          || lastUsed.getTime() > mostRecent.getLastUsedOn().getTime()) {
+        mostRecent = e;
+      }
+    }
+    return mostRecent;
+  }
+
   @Column(name = Column.NONE)
   protected Key key;
 
@@ -102,4 +135,28 @@ public final class AccountExternalId {
   public void setLastUsedOn() {
     lastUsedOn = new Timestamp(System.currentTimeMillis());
   }
+
+  public boolean canUserDelete() {
+    switch (Common.getGerritConfig().getLoginType()) {
+      case OPENID:
+        if (getExternalId().startsWith("Google Account ")) {
+          // Don't allow users to delete legacy google account tokens.
+          // Administrators will do it when cleaning the database.
+          //
+          return false;
+        }
+        break;
+
+      case HTTP:
+        if (getExternalId().startsWith("gerrit:")) {
+          // Don't allow users to delete a gerrit: token, as this is
+          // a Gerrit generated value for single-sign-on configurations
+          // not using OpenID.
+          //
+          return false;
+        }
+        break;
+    }
+    return true;
+  }
 }
diff --git a/src/main/java/com/google/gerrit/server/AccountSecurityImpl.java b/src/main/java/com/google/gerrit/server/AccountSecurityImpl.java
index 0e0c08b689..52a4808403 100644
--- a/src/main/java/com/google/gerrit/server/AccountSecurityImpl.java
+++ b/src/main/java/com/google/gerrit/server/AccountSecurityImpl.java
@@ -22,6 +22,7 @@ import com.google.gerrit.client.reviewdb.AccountSshKey;
 import com.google.gerrit.client.reviewdb.ContactInformation;
 import com.google.gerrit.client.reviewdb.ContributorAgreement;
 import com.google.gerrit.client.reviewdb.ReviewDb;
+import com.google.gerrit.client.reviewdb.SystemConfig;
 import com.google.gerrit.client.rpc.BaseServiceImplementation;
 import com.google.gerrit.client.rpc.Common;
 import com.google.gerrit.client.rpc.ContactInformationStoreException;
@@ -45,9 +46,12 @@ import java.io.UnsupportedEncodingException;
 import java.security.NoSuchAlgorithmException;
 import java.security.NoSuchProviderException;
 import java.security.spec.InvalidKeySpecException;
+import java.util.ArrayList;
 import java.util.Collections;
 import java.util.Date;
+import java.util.HashSet;
 import java.util.List;
+import java.util.Map;
 import java.util.Set;
 
 import javax.mail.Message;
@@ -140,6 +144,62 @@ class AccountSecurityImpl extends BaseServiceImplementation implements
     });
   }
 
+  public void deleteExternalIds(final Set<AccountExternalId.Key> keys,
+      final AsyncCallback<Set<AccountExternalId.Key>> callback) {
+    run(callback, new Action<Set<AccountExternalId.Key>>() {
+      public Set<AccountExternalId.Key> run(final ReviewDb db)
+          throws OrmException, Failure {
+        // Don't permit deletes unless they are for our own account
+        //
+        final Account.Id me = Common.getAccountId();
+        for (final AccountExternalId.Key keyId : keys) {
+          if (!me.equals(keyId.getParentKey()))
+            throw new Failure(new NoSuchEntityException());
+        }
+
+        // Determine the records we will allow the user to remove.
+        //
+        final Map<AccountExternalId.Key, AccountExternalId> all =
+            db.accountExternalIds()
+                .toMap(db.accountExternalIds().byAccount(me));
+        final AccountExternalId mostRecent =
+            AccountExternalId.mostRecent(all.values());
+        final SystemConfig.LoginType loginType =
+            Common.getGerritConfig().getLoginType();
+        final Set<AccountExternalId.Key> removed =
+            new HashSet<AccountExternalId.Key>();
+        final List<AccountExternalId> toDelete =
+            new ArrayList<AccountExternalId>();
+        for (final AccountExternalId.Key k : keys) {
+          final AccountExternalId e = all.get(k);
+          if (e == null) {
+            // Its already gone, tell the client its gone
+            //
+            removed.add(k);
+
+          } else if (e == mostRecent) {
+            // Don't delete the most recently accessed identity; the
+            // user might lock themselves out of the account.
+            //
+            continue;
+
+          } else if (e.canUserDelete()) {
+            toDelete.add(e);
+            removed.add(e.getKey());
+          }
+        }
+
+        if (!toDelete.isEmpty()) {
+          final Transaction txn = db.beginTransaction();
+          db.accountExternalIds().delete(toDelete, txn);
+          txn.commit();
+        }
+
+        return removed;
+      }
+    });
+  }
+
   public void updateContact(final String fullName, final String emailAddr,
       final ContactInformation info, final AsyncCallback<Account> callback) {
     run(callback, new Action<Account>() {