Changed according to review.

Now changing API Key after creation should also work Signed-off-by: Thomas Schauer-Köckeis <[email protected]>
stevespringett · Oct 21, 2024 · d053abe · d053abe
1 parent cd2132d
commit d053abe
Show file tree

Hide file tree

Showing 4 changed files with 39 additions and 34 deletions.
diff --git a/alpine-infra/pom.xml b/alpine-infra/pom.xml
@@ -71,10 +71,6 @@
             <groupId>io.jsonwebtoken</groupId>
             <artifactId>jjwt</artifactId>
         </dependency>
-        <dependency>
-            <groupId>org.mindrot</groupId>
-            <artifactId>jbcrypt</artifactId>
-        </dependency>
         <!-- Unit Tests -->
         <dependency>
             <groupId>org.junit.jupiter</groupId>

diff --git a/alpine-infra/src/main/java/alpine/persistence/AlpineQueryManager.java b/alpine-infra/src/main/java/alpine/persistence/AlpineQueryManager.java
@@ -18,7 +18,6 @@
  */
 package alpine.persistence;
 
-import alpine.Config;
 import alpine.common.logging.Logger;
 import alpine.event.LdapSyncEvent;
 import alpine.event.framework.EventService;
@@ -42,12 +41,12 @@
 import javax.jdo.PersistenceManager;
 import javax.jdo.Query;
 
-import org.mindrot.jbcrypt.BCrypt;
-
+import java.nio.charset.StandardCharsets;
+import java.security.MessageDigest;
 import java.sql.Timestamp;
 import java.util.ArrayList;
-import java.util.Base64;
 import java.util.Date;
+import java.util.HexFormat;
 import java.util.LinkedHashSet;
 import java.util.List;
 
@@ -61,7 +60,7 @@
 public class AlpineQueryManager extends AbstractAlpineQueryManager {
 
     private static final Logger LOGGER = Logger.getLogger(AlpineQueryManager.class);
-    private static final int ROUNDS = Config.getInstance().getPropertyAsInt(Config.AlpineKey.BCRYPT_ROUNDS);
+    private static final String HASH_METHOD = "SHA3-256";
     private static final int SUFFIX_LENGTH = 5;
 
     /**
@@ -108,7 +107,9 @@ public ApiKey getApiKey(final String key) {
             final Query<ApiKey> query = pm.newQuery(ApiKey.class, "suffix == :suffix");
             query.setParameters(key.substring(key.length() - SUFFIX_LENGTH));
             ApiKey apiKey = executeAndCloseUnique(query);
-            return BCrypt.checkpw(key.substring(0, key.length() - SUFFIX_LENGTH), apiKey.getKey()) ? apiKey : null;
+            MessageDigest digest = MessageDigest.getInstance(HASH_METHOD);
+            byte[] hashedKey = digest.digest(key.getBytes(StandardCharsets.UTF_8));
+            return apiKey != null && MessageDigest.isEqual(hashedKey, apiKey.getKey().getBytes()) ? apiKey : null;
         });
     }
 
@@ -123,12 +124,12 @@ public ApiKey getApiKey(final String key) {
     public ApiKey regenerateApiKey(final ApiKey apiKey) {
         return callInTransaction(() -> {
             String clearKey = ApiKeyGenerator.generate();
-            String hashedKey = BCrypt.hashpw(new String(Base64.getDecoder().decode(clearKey)), BCrypt.gensalt(ROUNDS))
-                    .toCharArray().toString();
+            MessageDigest digest = MessageDigest.getInstance(HASH_METHOD);
+            String hashedKey = HexFormat.of().formatHex(digest.digest(clearKey.getBytes(StandardCharsets.UTF_8)));
             apiKey.setKey(hashedKey);
-            apiKey.setSuffix(ApiKeyGenerator.generate(SUFFIX_LENGTH));
+            apiKey.setSuffix(clearKey.substring(clearKey.length() - SUFFIX_LENGTH));
             pm.makeTransient(apiKey);
-            apiKey.setKey(clearKey + apiKey.getSuffix());
+            apiKey.setKey(clearKey);
             return apiKey;
         });
     }
@@ -140,20 +141,21 @@ public ApiKey regenerateApiKey(final ApiKey apiKey) {
      * @return an ApiKey
      */
     public ApiKey createApiKey(final Team team) {
-        return callInTransaction(() -> {
-            final var apiKey = new ApiKey();
-            String clearKey = ApiKeyGenerator.generate();
-            String hashedKey = BCrypt.hashpw(new String(Base64.getDecoder().decode(clearKey)), BCrypt.gensalt(ROUNDS))
-                    .toCharArray().toString();
-            apiKey.setKey(hashedKey);
-            apiKey.setSuffix(ApiKeyGenerator.generate(SUFFIX_LENGTH));
-            apiKey.setCreated(new Date());
-            apiKey.setTeams(List.of(team));
-            pm.makePersistent(apiKey);
-            pm.makeTransient(apiKey);
-            apiKey.setKey(clearKey + apiKey.getSuffix());
-            return apiKey;
+        String clearKey = ApiKeyGenerator.generate();
+        ApiKey apiKey = callInTransaction(() -> {
+            final var apiKeyPers = new ApiKey();
+            MessageDigest digest = MessageDigest.getInstance(HASH_METHOD);
+            String hashedKey = HexFormat.of().formatHex(digest.digest(clearKey.getBytes(StandardCharsets.UTF_8)));
+            apiKeyPers.setKey(hashedKey);
+            apiKeyPers.setSuffix(clearKey.substring(clearKey.length() - SUFFIX_LENGTH));
+            apiKeyPers.setCreated(new Date());
+            apiKeyPers.setTeams(List.of(team));
+            pm.makePersistent(apiKeyPers);
+            return apiKeyPers;
         });
+            ApiKey copiedApiKey = pm.detachCopy(apiKey);
+            copiedApiKey.setKey(clearKey);
+            return copiedApiKey;
     }
 
     public ApiKey updateApiKey(final ApiKey transientApiKey) {

diff --git a/alpine-model/src/main/java/alpine/model/ApiKey.java b/alpine-model/src/main/java/alpine/model/ApiKey.java
@@ -90,6 +90,7 @@ public class ApiKey implements Serializable, Principal {
     @Persistent
     @Unique
     @Column(name = "SUFFIX")
+    @JsonIgnore
     private String suffix;
 
     public long getId() {

diff --git a/alpine-model/src/test/java/alpine/model/ApiKeyTest.java b/alpine-model/src/test/java/alpine/model/ApiKeyTest.java
@@ -40,14 +40,16 @@ public void keyTest() {
         {
             ApiKey key = new ApiKey();
             key.setKey("12345678901234567890");
+            key.setSuffix(key.getKey().substring(key.getKey().length() - 5));
             Assertions.assertEquals("12345678901234567890", key.getKey());
-            Assertions.assertEquals("****************7890", key.getName());
+            Assertions.assertEquals("***************67890", key.getName());
         }
         {
             ApiKey key = new ApiKey();
             key.setKey(prefix + "12345678901234567890");
+            key.setSuffix(key.getKey().substring(key.getKey().length() - 5));
             Assertions.assertEquals(prefix + "12345678901234567890", key.getKey());
-            Assertions.assertEquals(prefix + "****************7890", key.getName());
+            Assertions.assertEquals(prefix + "***************67890", key.getName());
         }
     }
 
@@ -56,23 +58,27 @@ public void maskTest() {
         {
             ApiKey key = new ApiKey();
             key.setKey("12345678901234567890");
-            Assertions.assertEquals("****************7890", key.getMaskedKey());
+            key.setSuffix(key.getKey().substring(key.getKey().length() - 5));
+            Assertions.assertEquals("***************67890", key.getMaskedKey());
         }
         {
             ApiKey key = new ApiKey();
             key.setKey("1234ABCabc+_=!?-*");
-            Assertions.assertEquals("*************!?-*", key.getMaskedKey());
+            key.setSuffix(key.getKey().substring(key.getKey().length() - 5));
+            Assertions.assertEquals("************=!?-*", key.getMaskedKey());
         }
         {
             ApiKey key = new ApiKey();
-            key.setKey("1234");
-            Assertions.assertEquals("1234", key.getMaskedKey());
+            key.setKey("12345");
+            key.setSuffix(key.getKey().substring(key.getKey().length() - 5));
+            Assertions.assertEquals("12345", key.getMaskedKey());
         }
         {
             // test with prefix
             ApiKey key = new ApiKey();
             key.setKey(prefix + "1234567890");
-            Assertions.assertEquals(prefix + "******7890", key.getMaskedKey());
+            key.setSuffix(key.getKey().substring(key.getKey().length() - 5));
+            Assertions.assertEquals(prefix + "*****67890", key.getMaskedKey());
         }
     }