Remove exception from api key verification

This removes repeated exceptions from the api key service when checking requests without an api key. Despite of throwing an exception, the service now simply returns `null`, when the authentication was not successful.

Pushed-by: Rene Pfeuffer<rene.pfeuffer@cloudogu.com>
Co-authored-by: René Pfeuffer<rene.pfeuffer@cloudogu.com>
This commit is contained in:
Rene Pfeuffer
2023-11-21 08:13:07 +01:00
parent 77a285063e
commit 14f1cb0218
4 changed files with 16 additions and 13 deletions

View File

@@ -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) {

View File

@@ -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<CheckResult> check(String tokenAsString) {
return tokenHandler.readToken(tokenAsString).map(this::check);
}
private CheckResult check(ApiKeyTokenHandler.Token token) {

View File

@@ -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");

View File

@@ -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<ApiKeyService.CheckResult> 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