diff --git a/scm-webapp/src/main/java/sonia/scm/security/ApiKeyService.java b/scm-webapp/src/main/java/sonia/scm/security/ApiKeyService.java index 097ad98e51..63994b3fd9 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/ApiKeyService.java +++ b/scm-webapp/src/main/java/sonia/scm/security/ApiKeyService.java @@ -26,6 +26,7 @@ package sonia.scm.security; import com.google.common.util.concurrent.Striped; import org.apache.shiro.authc.credential.PasswordService; +import org.apache.shiro.authz.AuthorizationException; import org.apache.shiro.util.ThreadContext; import sonia.scm.ContextEntry; import sonia.scm.store.ConfigurationEntryStore; @@ -52,18 +53,20 @@ public class ApiKeyService { private final PasswordService passwordService; private final KeyGenerator keyGenerator; private final Supplier passphraseGenerator; + private final ApiKeyTokenHandler tokenHandler; private final Striped locks = Striped.readWriteLock(10); @Inject - ApiKeyService(ConfigurationEntryStoreFactory storeFactory, KeyGenerator keyGenerator, PasswordService passwordService) { - this(storeFactory, passwordService, keyGenerator, () -> random(KEY_LENGTH, 0, 0, true, true, null, new SecureRandom())); + ApiKeyService(ConfigurationEntryStoreFactory storeFactory, KeyGenerator keyGenerator, PasswordService passwordService, ApiKeyTokenHandler tokenHandler) { + this(storeFactory, passwordService, keyGenerator, tokenHandler, () -> random(KEY_LENGTH, 0, 0, true, true, null, new SecureRandom())); } - ApiKeyService(ConfigurationEntryStoreFactory storeFactory, PasswordService passwordService, KeyGenerator keyGenerator, Supplier passphraseGenerator) { + ApiKeyService(ConfigurationEntryStoreFactory storeFactory, PasswordService passwordService, KeyGenerator keyGenerator, ApiKeyTokenHandler tokenHandler, Supplier passphraseGenerator) { this.store = storeFactory.withType(ApiKeyCollection.class).withName("apiKeys").build(); this.passwordService = passwordService; this.keyGenerator = keyGenerator; + this.tokenHandler = tokenHandler; this.passphraseGenerator = passphraseGenerator; } @@ -71,6 +74,7 @@ public class ApiKeyService { String user = currentUser(); String passphrase = passphraseGenerator.get(); String hashedPassphrase = passwordService.encryptPassword(passphrase); + ApiKeyWithPassphrase key = new ApiKeyWithPassphrase(keyGenerator.createKey(), name, role, hashedPassphrase); Lock lock = locks.get(user).writeLock(); lock.lock(); try { @@ -78,12 +82,12 @@ public class ApiKeyService { throw alreadyExists(ContextEntry.ContextBuilder.entity(ApiKeyWithPassphrase.class, name)); } final ApiKeyCollection apiKeyCollection = store.getOptional(user).orElse(new ApiKeyCollection(emptyList())); - final ApiKeyCollection newApiKeyCollection = apiKeyCollection.add(new ApiKeyWithPassphrase(keyGenerator.createKey(), name, role, hashedPassphrase)); + final ApiKeyCollection newApiKeyCollection = apiKeyCollection.add(key); store.put(user, newApiKeyCollection); } finally { lock.unlock(); } - return passphrase; + return tokenHandler.createToken(user, new ApiKey(key), passphrase); } public void remove(String id) { @@ -105,6 +109,15 @@ public class ApiKeyService { } } + Optional check(String tokenAsString) { + return check(tokenHandler.readToken(tokenAsString) + .orElseThrow(AuthorizationException::new)); + } + + private Optional check(ApiKeyTokenHandler.Token token) { + return check(token.getUser(), token.getApiKeyId(), token.getPassphrase()); + } + Optional check(String user, String id, String passphrase) { Lock lock = locks.get(user).readLock(); lock.lock(); diff --git a/scm-webapp/src/main/java/sonia/scm/security/ApiKeyTokenHandler.java b/scm-webapp/src/main/java/sonia/scm/security/ApiKeyTokenHandler.java index 33ea38a593..34ee392060 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/ApiKeyTokenHandler.java +++ b/scm-webapp/src/main/java/sonia/scm/security/ApiKeyTokenHandler.java @@ -59,9 +59,9 @@ class ApiKeyTokenHandler { } } - Optional readToken(String asString) { + Optional readToken(String token) { try { - return of(OBJECT_MAPPER.readValue(decoder.decode(asString), Token.class)); + return of(OBJECT_MAPPER.readValue(decoder.decode(token), Token.class)); } catch (IOException | DecodingException e) { LOG.warn("error reading api token", e); return empty(); diff --git a/scm-webapp/src/test/java/sonia/scm/security/ApiKeyServiceTest.java b/scm-webapp/src/test/java/sonia/scm/security/ApiKeyServiceTest.java index df69eee8df..28a2f16659 100644 --- a/scm-webapp/src/test/java/sonia/scm/security/ApiKeyServiceTest.java +++ b/scm-webapp/src/test/java/sonia/scm/security/ApiKeyServiceTest.java @@ -54,10 +54,10 @@ class ApiKeyServiceTest { PasswordService passwordService = mock(PasswordService.class); Supplier passphraseGenerator = () -> Integer.toString(nextKey++); KeyGenerator keyGenerator = () -> Integer.toString(nextId++); + ApiKeyTokenHandler tokenHandler = new ApiKeyTokenHandler(); ConfigurationEntryStoreFactory storeFactory = new InMemoryConfigurationEntryStoreFactory(); ConfigurationEntryStore store = storeFactory.withType(ApiKeyCollection.class).withName("apiKeys").build(); - ApiKeyService service = new ApiKeyService(storeFactory, passwordService, keyGenerator, passphraseGenerator); - + ApiKeyService service = new ApiKeyService(storeFactory, passwordService, keyGenerator, tokenHandler, passphraseGenerator); @BeforeEach void mockPasswordService() { @@ -85,7 +85,7 @@ class ApiKeyServiceTest { @Test void shouldCreateNewKeyAndStoreItHashed() { - String newKey = service.createNewKey("1", "READ"); + service.createNewKey("1", "READ"); ApiKeyCollection apiKeys = store.get("dent"); @@ -93,7 +93,6 @@ class ApiKeyServiceTest { ApiKeyWithPassphrase key = apiKeys.getKeys().iterator().next(); assertThat(key.getRole()).isEqualTo("READ"); assertThat(key.getPassphrase()).isEqualTo("1-hashed"); - assertThat(newKey).isEqualTo("1"); Optional role = service.check("dent", "1", "1-hashed"); @@ -104,7 +103,7 @@ class ApiKeyServiceTest { void shouldReturnRoleForKey() { String newKey = service.createNewKey("1", "READ"); - Optional role = service.check("dent", "1", newKey); + Optional role = service.check(newKey); assertThat(role).contains("READ"); } @@ -127,8 +126,8 @@ class ApiKeyServiceTest { assertThat(apiKeys.getKeys()).hasSize(2); - assertThat(service.check("dent", "1", firstKey)).contains("READ"); - assertThat(service.check("dent", "2", secondKey)).contains("WRITE"); + assertThat(service.check(firstKey)).contains("READ"); + assertThat(service.check(secondKey)).contains("WRITE"); assertThat(service.getKeys()).extracting("id").contains("1", "2"); } @@ -140,8 +139,8 @@ class ApiKeyServiceTest { service.remove("1"); - assertThat(service.check("dent", "1", firstKey)).isEmpty(); - assertThat(service.check("dent", "2", secondKey)).contains("WRITE"); + assertThat(service.check(firstKey)).isEmpty(); + assertThat(service.check(secondKey)).contains("WRITE"); } @Test @@ -150,7 +149,7 @@ class ApiKeyServiceTest { assertThrows(AlreadyExistsException.class, () -> service.createNewKey("1", "WRITE")); - assertThat(service.check("dent", "1", firstKey)).contains("READ"); + assertThat(service.check(firstKey)).contains("READ"); } @Test