Don't store SSH keys we know to be invalid

Instead of storing an invalid key, tell the user its invalid and
store nothing at all.  This helps users who might be trying to
use an SSH 1 style key and insert it, we'll fail right away and
tell them its not a valid key.

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

5 files changed, 50 insertions, 5 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 6ee84fc966..81dd7f41bf 100644
--- a/src/main/java/com/google/gerrit/client/account/AccountConstants.java
+++ b/src/main/java/com/google/gerrit/client/account/AccountConstants.java
@@ -45,6 +45,7 @@ public interface AccountConstants extends Constants {
 
   String addSshKeyPanelHeader();
   String addSshKeyHelp();
+  String invalidSshKeyError();
 
   String webIdLastUsed();
   String webIdEmail();
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 a4feca1628..c81d7cfc30 100644
--- a/src/main/java/com/google/gerrit/client/account/AccountConstants.properties
+++ b/src/main/java/com/google/gerrit/client/account/AccountConstants.properties
@@ -31,6 +31,7 @@ buttonLinkIdentity = Link Another Identity
 
 addSshKeyPanelHeader = Add SSH Public Key
 addSshKeyHelp = (<a href="http://github.com/guides/providing-your-ssh-key" target="_blank">GitHub's Guide to SSH Keys</a>)
+invalidSshKeyError = Invalid SSH Key
 
 watchedProjects = Watched Projects
 buttonWatchProject = Watch
diff --git a/src/main/java/com/google/gerrit/client/account/SshKeyPanel.java b/src/main/java/com/google/gerrit/client/account/SshKeyPanel.java
index 6b00b3bd50..4638507a8b 100644
--- a/src/main/java/com/google/gerrit/client/account/SshKeyPanel.java
+++ b/src/main/java/com/google/gerrit/client/account/SshKeyPanel.java
@@ -14,9 +14,11 @@
 
 package com.google.gerrit.client.account;
 
+import com.google.gerrit.client.ErrorDialog;
 import com.google.gerrit.client.FormatUtil;
 import com.google.gerrit.client.reviewdb.AccountSshKey;
 import com.google.gerrit.client.rpc.GerritCallback;
+import com.google.gerrit.client.rpc.InvalidSshKeyException;
 import com.google.gerrit.client.ui.FancyFlexTable;
 import com.google.gerrit.client.ui.SmallHeading;
 import com.google.gwt.user.client.ui.Button;
@@ -31,6 +33,7 @@ import com.google.gwt.user.client.ui.TextArea;
 import com.google.gwt.user.client.ui.VerticalPanel;
 import com.google.gwt.user.client.ui.Widget;
 import com.google.gwt.user.client.ui.FlexTable.FlexCellFormatter;
+import com.google.gwtjsonrpc.client.RemoteJsonException;
 import com.google.gwtjsonrpc.client.VoidResult;
 
 import java.util.HashSet;
@@ -98,7 +101,21 @@ class SshKeyPanel extends Composite {
         @Override
         public void onFailure(final Throwable caught) {
           addNew.setEnabled(true);
-          super.onFailure(caught);
+
+          if (isInvalidSshKey(caught)) {
+            new ErrorDialog(Util.C.invalidSshKeyError()).center();
+
+          } else {
+            super.onFailure(caught);
+          }
+        }
+
+        private boolean isInvalidSshKey(final Throwable caught) {
+          if (caught instanceof InvalidSshKeyException) {
+            return true;
+          }
+          return caught instanceof RemoteJsonException
+              && InvalidSshKeyException.MESSAGE.equals(caught.getMessage());
         }
       });
     }
diff --git a/src/main/java/com/google/gerrit/client/rpc/InvalidSshKeyException.java b/src/main/java/com/google/gerrit/client/rpc/InvalidSshKeyException.java
new file mode 100644
index 0000000000..7fb01ab6d8
--- /dev/null
+++ b/src/main/java/com/google/gerrit/client/rpc/InvalidSshKeyException.java
@@ -0,0 +1,24 @@
+// Copyright 2009 Google Inc.
+//
+// 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 com.google.gerrit.client.rpc;
+
+/** Error indicating the SSH key string is invalid as supplied. */
+public class InvalidSshKeyException extends Exception {
+  public static final String MESSAGE = "Invalid SSH Key";
+
+  public InvalidSshKeyException() {
+    super(MESSAGE);
+  }
+}
diff --git a/src/main/java/com/google/gerrit/server/AccountSecurityImpl.java b/src/main/java/com/google/gerrit/server/AccountSecurityImpl.java
index 4907245008..912a7fad9a 100644
--- a/src/main/java/com/google/gerrit/server/AccountSecurityImpl.java
+++ b/src/main/java/com/google/gerrit/server/AccountSecurityImpl.java
@@ -24,6 +24,7 @@ import com.google.gerrit.client.reviewdb.ContributorAgreement;
 import com.google.gerrit.client.reviewdb.ReviewDb;
 import com.google.gerrit.client.rpc.BaseServiceImplementation;
 import com.google.gerrit.client.rpc.Common;
+import com.google.gerrit.client.rpc.InvalidSshKeyException;
 import com.google.gerrit.client.rpc.NoSuchEntityException;
 import com.google.gerrit.server.ssh.SshUtil;
 import com.google.gwt.user.client.rpc.AsyncCallback;
@@ -75,7 +76,7 @@ class AccountSecurityImpl extends BaseServiceImplementation implements
   public void addSshKey(final String keyText,
       final AsyncCallback<AccountSshKey> callback) {
     run(callback, new Action<AccountSshKey>() {
-      public AccountSshKey run(final ReviewDb db) throws OrmException {
+      public AccountSshKey run(final ReviewDb db) throws OrmException, Failure {
         int max = 0;
         final Account.Id me = Common.getAccountId();
         for (final AccountSshKey k : db.accountSshKeys().byAccount(me)) {
@@ -92,11 +93,12 @@ class AccountSecurityImpl extends BaseServiceImplementation implements
         try {
           SshUtil.parse(newKey);
         } catch (NoSuchAlgorithmException e) {
-          newKey.setInvalid();
+          throw new Failure(new InvalidSshKeyException());
         } catch (InvalidKeySpecException e) {
-          newKey.setInvalid();
+          throw new Failure(new InvalidSshKeyException());
         } catch (NoSuchProviderException e) {
-          newKey.setInvalid();
+          log.error("Cannot parse SSH key", e);
+          throw new Failure(new InvalidSshKeyException());
         }
         db.accountSshKeys().insert(Collections.singleton(newKey));
         SshUtil.invalidate(Common.getAccountCache().get(me, db));