diff --git a/scm-webapp/src/main/java/sonia/scm/security/ApiKeyRealm.java b/scm-webapp/src/main/java/sonia/scm/security/ApiKeyRealm.java index e1ece3788c..b4860cb60d 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/ApiKeyRealm.java +++ b/scm-webapp/src/main/java/sonia/scm/security/ApiKeyRealm.java @@ -85,8 +85,9 @@ public class ApiKeyRealm extends AuthenticatingRealm { token instanceof BearerToken || token instanceof UsernamePasswordToken, "%s is required", BearerToken.class); String password = getPassword(token); - ApiKeyService.CheckResult check = apiKeyService.check(password); - return buildAuthenticationInfo(token, check); + return apiKeyService.check(password) + .map(check -> buildAuthenticationInfo(token, check)) + .orElse(null); } private AuthenticationInfo buildAuthenticationInfo(AuthenticationToken token, ApiKeyService.CheckResult check) { 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 419a394e39..59e8d706d7 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/ApiKeyService.java +++ b/scm-webapp/src/main/java/sonia/scm/security/ApiKeyService.java @@ -43,6 +43,7 @@ import sonia.scm.user.UserPermissions; import javax.inject.Inject; import java.security.SecureRandom; import java.util.Collection; +import java.util.Optional; import java.util.concurrent.locks.Lock; import java.util.concurrent.locks.ReadWriteLock; import java.util.function.Supplier; @@ -127,9 +128,8 @@ public class ApiKeyService { }); } - CheckResult check(String tokenAsString) { - return check(tokenHandler.readToken(tokenAsString) - .orElseThrow(AuthorizationException::new)); + Optional check(String tokenAsString) { + return tokenHandler.readToken(tokenAsString).map(this::check); } private CheckResult check(ApiKeyTokenHandler.Token token) { diff --git a/scm-webapp/src/test/java/sonia/scm/security/ApiKeyRealmTest.java b/scm-webapp/src/test/java/sonia/scm/security/ApiKeyRealmTest.java index 6cc5401f7e..b93ad715bf 100644 --- a/scm-webapp/src/test/java/sonia/scm/security/ApiKeyRealmTest.java +++ b/scm-webapp/src/test/java/sonia/scm/security/ApiKeyRealmTest.java @@ -38,6 +38,7 @@ import sonia.scm.repository.RepositoryRole; import sonia.scm.repository.RepositoryRoleManager; import static java.util.Collections.singleton; +import static java.util.Optional.of; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.ArgumentMatchers.any; @@ -75,7 +76,7 @@ class ApiKeyRealmTest { @Test void shouldCreateAuthenticationWithScope() { - when(apiKeyService.check("towel")).thenReturn(new ApiKeyService.CheckResult("ford", "READ")); + when(apiKeyService.check("towel")).thenReturn(of(new ApiKeyService.CheckResult("ford", "READ"))); when(repositoryRoleManager.get("READ")).thenReturn(new RepositoryRole("guide", singleton("read"), "system")); realm.doGetAuthenticationInfo(valueOf("towel")); @@ -93,7 +94,7 @@ class ApiKeyRealmTest { @Test void shouldFailWithUnknownRole() { - when(apiKeyService.check("towel")).thenReturn(new ApiKeyService.CheckResult("ford", "READ")); + when(apiKeyService.check("towel")).thenReturn(of(new ApiKeyService.CheckResult("ford", "READ"))); when(repositoryRoleManager.get("READ")).thenReturn(null); BearerToken token = valueOf("towel"); 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 c23bb5bb7a..bfe69b5a95 100644 --- a/scm-webapp/src/test/java/sonia/scm/security/ApiKeyServiceTest.java +++ b/scm-webapp/src/test/java/sonia/scm/security/ApiKeyServiceTest.java @@ -43,6 +43,7 @@ import sonia.scm.store.InMemoryDataStoreFactory; import sonia.scm.user.User; import sonia.scm.user.UserEvent; +import java.util.Optional; import java.util.function.Supplier; import static org.assertj.core.api.Assertions.assertThat; @@ -109,9 +110,9 @@ class ApiKeyServiceTest { void shouldReturnRoleForKey() { String newKey = service.createNewKey("dent","1", "READ").getToken(); - ApiKeyService.CheckResult role = service.check(newKey); + Optional role = service.check(newKey); - assertThat(role).extracting("permissionRole").isEqualTo("READ"); + assertThat(role).get().extracting("permissionRole").isEqualTo("READ"); } @Test @@ -135,8 +136,8 @@ class ApiKeyServiceTest { assertThat(apiKeys.getKeys()).hasSize(2); - assertThat(service.check(firstKey.getToken())).extracting("permissionRole").isEqualTo("READ"); - assertThat(service.check(secondKey.getToken())).extracting("permissionRole").isEqualTo("WRITE"); + assertThat(service.check(firstKey.getToken())).get().extracting("permissionRole").isEqualTo("READ"); + assertThat(service.check(secondKey.getToken())).get().extracting("permissionRole").isEqualTo("WRITE"); assertThat(service.getKeys("dent")).extracting("id") .contains(firstKey.getId(), secondKey.getId()); @@ -150,7 +151,7 @@ class ApiKeyServiceTest { service.remove("dent","1"); assertThrows(AuthorizationException.class, () -> service.check(firstKey)); - assertThat(service.check(secondKey)).extracting("permissionRole").isEqualTo("WRITE"); + assertThat(service.check(secondKey)).get().extracting("permissionRole").isEqualTo("WRITE"); } @Test @@ -159,7 +160,7 @@ class ApiKeyServiceTest { assertThrows(AlreadyExistsException.class, () -> service.createNewKey("dent","1", "WRITE")); - assertThat(service.check(firstKey)).extracting("permissionRole").isEqualTo("READ"); + assertThat(service.check(firstKey)).get().extracting("permissionRole").isEqualTo("READ"); } @Test