Experiment some more with exception handling.

codingchili · Jan 20, 2017 · 0196a02 · 0196a02
1 parent 63c8a4c
commit 0196a02
Show file tree

Hide file tree

Showing 7 changed files with 32 additions and 35 deletions.
diff --git a/core/main/com/codingchili/core/protocol/ExceptionHandler.java b/core/main/com/codingchili/core/protocol/ExceptionHandler.java
@@ -6,6 +6,6 @@
  * Protocol exception handler.
  */
 @FunctionalInterface
-interface ExceptionHandler {
+public interface ExceptionHandler {
     void handle(Request request, Throwable exception);
 }
diff --git a/core/main/com/codingchili/core/security/HashHelper.java b/core/main/com/codingchili/core/security/HashHelper.java
@@ -72,7 +72,7 @@ public void hash(Future<String> future, String password, String salt) {
      *
      * @return A generated salt of 512 bits.
      */
-    public static String salt() {
+    public String generateSalt() {
         byte[] salt = new byte[SALT_BYTES];
         new SecureRandom().nextBytes(salt);
         return Base64.getEncoder().encodeToString(salt);

diff --git a/core/test/com/codingchili/core/security/HashHelperTest.java b/core/test/com/codingchili/core/security/HashHelperTest.java
@@ -43,7 +43,7 @@ public void generateUniqueSaltTest() {
         HashMap<String, Boolean> salts = new HashMap<>();
 
         for (int i = 0; i < 1000; i++) {
-            String salt = HashHelper.salt();
+            String salt = HashHelper.generateSalt();
 
             Assert.assertFalse(salts.containsKey(salt));
             salts.put(salt, true);

diff --git a/services/authentication/main/com/codingchili/authentication/controller/ClientHandler.java b/services/authentication/main/com/codingchili/authentication/controller/ClientHandler.java
@@ -1,22 +1,19 @@
 package com.codingchili.authentication.controller;
 
+import com.codingchili.authentication.configuration.AuthenticationContext;
+import com.codingchili.authentication.model.*;
 import io.vertx.core.Future;
 
 import com.codingchili.core.context.CoreException;
 import com.codingchili.core.protocol.*;
 import com.codingchili.core.security.Account;
 
-import com.codingchili.authentication.configuration.AuthenticationContext;
-import com.codingchili.authentication.model.*;
-
-import java.util.HashMap;
-
-import static com.codingchili.core.protocol.Access.PUBLIC;
 import static com.codingchili.common.Strings.*;
+import static com.codingchili.core.protocol.Access.PUBLIC;
 
 /**
  * @author Robin Duda
- *         Routing used to authenticate users and create/delete characters.
+ *         Routing used to register/authenticate accounts.
  */
 public class ClientHandler<T extends AuthenticationContext> extends AbstractHandler<T> {
     private final Protocol<RequestHandler<ClientRequest>> protocol = new Protocol<>();
@@ -83,4 +80,4 @@ private void sendAuthentication(Account account, ClientRequest request, boolean
         else
             context.onAuthenticated(account.getUsername(), request.sender());
     }
-}
+}
diff --git a/services/authentication/main/com/codingchili/authentication/model/AccountDB.java b/services/authentication/main/com/codingchili/authentication/model/AccountDB.java
@@ -1,7 +1,6 @@
 package com.codingchili.authentication.model;
 
-import io.vertx.core.Future;
-import io.vertx.core.Vertx;
+import io.vertx.core.*;
 
 import com.codingchili.core.security.*;
 import com.codingchili.core.storage.AsyncStorage;
@@ -10,8 +9,8 @@
 
 /**
  * @author Robin Duda
- *
- * Account storage logic.
+ *         <p>
+ *         Account storage logic.
  */
 public class AccountDB implements AsyncAccountStore {
     private final AsyncStorage<String, AccountMapping> accounts;
@@ -39,48 +38,48 @@ public void authenticate(Future<Account> future, Account account) {
             if (map.succeeded()) {
                 authenticate(future, map.result(), account);
             } else {
-                if (map.cause() instanceof ValueMissingException) {
-                    future.fail(new AccountMissingException());
-                } else {
-                    future.fail(map.cause());
-                }
+                fail(future, map);
             }
         });
     }
 
     @Override
     public void register(Future<Account> future, Account account) {
         Future<String> hashing = Future.future();
-        String salt = HashHelper.salt();
-        AccountMapping mapping = new AccountMapping(account);
+        AccountMapping mapping = new AccountMapping(account)
+                .setSalt(hasher.generateSalt());
 
         hashing.setHandler(hash -> {
-            mapping.setSalt(salt);
             mapping.setHash(hash.result());
 
             accounts.putIfAbsent(account.getUsername(), mapping, map -> {
                 if (map.succeeded()) {
                     future.complete(filter(account));
                 } else {
-                    if (map.cause() instanceof ValueAlreadyPresentException) {
-                        future.fail(new AccountExistsException());
-                    } else {
-                        future.fail(map.cause());
-                    }
+                    fail(future, map);
                 }
             });
         });
+        hasher.hash(hashing, account.getPassword(), mapping.getSalt());
+    }
 
-        hasher.hash(hashing, account.getPassword(), salt);
+    private void fail(Future future, AsyncResult result) {
+        Throwable cause = result.cause();
+
+        if (cause instanceof ValueAlreadyPresentException) {
+            future.fail(new AccountExistsException());
+        } else if (cause instanceof ValueMissingException) {
+            future.fail(new AccountMissingException());
+        } else {
+            future.fail(cause);
+        }
     }
 
     private void authenticate(Future<Account> future, AccountMapping authenticated, Account unauthenticated) {
         Future<String> hashing = Future.future();
 
         hashing.setHandler(hash -> {
-            boolean verified = ByteComparator.compare(hash.result(), authenticated.getHash());
-
-            if (verified) {
+            if (ByteComparator.compare(hash.result(), authenticated.getHash())) {
                 future.complete(filter(authenticated));
             } else {
                 future.fail(new AccountPasswordException());
@@ -99,4 +98,4 @@ private Account filter(AccountMapping account) {
     private Account filter(Account account) {
         return account.setPassword(null);
     }
-}
+}
diff --git a/services/authentication/main/com/codingchili/authentication/model/AccountMapping.java b/services/authentication/main/com/codingchili/authentication/model/AccountMapping.java
@@ -26,8 +26,9 @@ public String getSalt() {
         return salt;
     }
 
-    public void setSalt(String salt) {
+    public AccountMapping setSalt(String salt) {
         this.salt = salt;
+        return this;
     }
 
     public String getUsername() {

diff --git a/services/authentication/main/com/codingchili/authentication/model/AsyncAccountStore.java b/services/authentication/main/com/codingchili/authentication/model/AsyncAccountStore.java
@@ -13,7 +13,7 @@ public interface AsyncAccountStore {
     /**
      * Finds an account in the store.
      *
-     * @param username username of the account to findByUsername.
+     * @param username username of the account to find by username.
      */
     void get(Future<Account> future, String username);