From 0dcf07891a12168c64fe6cd572373516ee50c379 Mon Sep 17 00:00:00 2001 From: Sebastian Becker Date: Mon, 18 Nov 2024 14:47:35 +0100 Subject: [PATCH] feat(service): update default policy package and improve tests Signed-off-by: Sebastian Becker --- amphora-service/charts/amphora/values.yaml | 2 +- .../amphora/service/opa/OpaClient.java | 11 +- .../amphora/service/opa/OpaRequestBody.java | 8 - .../persistence/metadata/StorageService.java | 15 -- .../amphora/service/opa/JwtReaderTest.java | 24 +++ .../amphora/service/opa/OpaResultTest.java | 31 ++++ .../metadata/StorageServiceTest.java | 147 ++++++++++++++++++ 7 files changed, 210 insertions(+), 28 deletions(-) create mode 100644 amphora-service/src/test/java/io/carbynestack/amphora/service/opa/OpaResultTest.java diff --git a/amphora-service/charts/amphora/values.yaml b/amphora-service/charts/amphora/values.yaml index 9c49182..556700b 100644 --- a/amphora-service/charts/amphora/values.yaml +++ b/amphora-service/charts/amphora/values.yaml @@ -78,5 +78,5 @@ auth: userIdFieldName: "sub" opa: - defaultPolicyPackage: "default" + defaultPolicyPackage: "carbynestack.def" endpoint: "http://opa.default.svc.cluster.local:8081/" \ No newline at end of file diff --git a/amphora-service/src/main/java/io/carbynestack/amphora/service/opa/OpaClient.java b/amphora-service/src/main/java/io/carbynestack/amphora/service/opa/OpaClient.java index 1fb544f..30ba7b9 100644 --- a/amphora-service/src/main/java/io/carbynestack/amphora/service/opa/OpaClient.java +++ b/amphora-service/src/main/java/io/carbynestack/amphora/service/opa/OpaClient.java @@ -48,10 +48,13 @@ public boolean isAllowed(String policyPackage, String action, String subject, Li .tags(tags) .build(); try { - return csHttpClient.postForObject( - opaServiceUri.resolve(String.format("/v1/data/%s/%s", policyPackage, action)), - new OpaRequest(body), - OpaResult.class).isAllowed(); + return csHttpClient.postForObject(opaServiceUri.resolve( + String.format("/v1/data/%s/%s", + policyPackage.replace(".", "/"), + action)), + new OpaRequest(body), + OpaResult.class) + .isAllowed(); } catch (CsHttpClientException e) { log.error("Error occurred while evaluating OPA policy package: {}", e.getMessage()); } diff --git a/amphora-service/src/main/java/io/carbynestack/amphora/service/opa/OpaRequestBody.java b/amphora-service/src/main/java/io/carbynestack/amphora/service/opa/OpaRequestBody.java index 9992d96..ffd591b 100644 --- a/amphora-service/src/main/java/io/carbynestack/amphora/service/opa/OpaRequestBody.java +++ b/amphora-service/src/main/java/io/carbynestack/amphora/service/opa/OpaRequestBody.java @@ -16,19 +16,11 @@ @Value public class OpaRequestBody { String subject; - long currentTime; List tags; @Builder public OpaRequestBody(String subject, List tags) { this.subject = subject; - this.currentTime = System.currentTimeMillis(); - this.tags = tags; - } - - OpaRequestBody(String subject, long currentTime, List tags) { - this.subject = subject; - this.currentTime = currentTime; this.tags = tags; } } diff --git a/amphora-service/src/main/java/io/carbynestack/amphora/service/persistence/metadata/StorageService.java b/amphora-service/src/main/java/io/carbynestack/amphora/service/persistence/metadata/StorageService.java index cfa0445..c572822 100644 --- a/amphora-service/src/main/java/io/carbynestack/amphora/service/persistence/metadata/StorageService.java +++ b/amphora-service/src/main/java/io/carbynestack/amphora/service/persistence/metadata/StorageService.java @@ -535,20 +535,5 @@ public void deleteTag(UUID secretId, String key, String authorizedUserId) throws NO_TAG_WITH_KEY_EXISTS_FOR_SECRET_WITH_ID_EXCEPTION_MSG, key, secretId)))); -// SecretEntity secretEntityReference = secretEntityRepository.findById(secretId.toString()) -// .orElseThrow(() -> -// new NotFoundException( -// String.format(NO_SECRET_WITH_ID_EXISTS_EXCEPTION_MSG, secretId))); -// if (!opaService.canDeleteTags(authorizedUserId, setToTagList(secretEntityReference.getTags()))) { -// throw new UnauthorizedException("User is not authorized to delete tags for this secret"); -// } -// TagEntity tagEntity = tagRepository.findBySecretAndKey(secretEntityReference, key) -// .orElseThrow(() -> -// new NotFoundException( -// String.format( -// NO_TAG_WITH_KEY_EXISTS_FOR_SECRET_WITH_ID_EXCEPTION_MSG, -// key, -// secretId))); -// tagRepository.delete(tagEntity); } } diff --git a/amphora-service/src/test/java/io/carbynestack/amphora/service/opa/JwtReaderTest.java b/amphora-service/src/test/java/io/carbynestack/amphora/service/opa/JwtReaderTest.java index fe9ddbb..7f90826 100644 --- a/amphora-service/src/test/java/io/carbynestack/amphora/service/opa/JwtReaderTest.java +++ b/amphora-service/src/test/java/io/carbynestack/amphora/service/opa/JwtReaderTest.java @@ -15,6 +15,7 @@ import static org.junit.Assert.assertThrows; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; class JwtReaderTest { @@ -41,4 +42,27 @@ void givenNoTokenProvided_whenExtractSubject_thenThrowUnauthorizedException() th "Bearer header.%s.signature", Base64.toBase64String(invalidToken.getBytes(StandardCharsets.UTF_8))))); } + + @Test + void givenJwtOfInvalidFormat_whenExtractSubject_thenThrowUnauthorizedException() { + JwtReader jwtReader = new JwtReader("sub"); + assertThrows(UnauthorizedException.class, () -> + jwtReader.extractUserIdFromAuthHeader("Bearer invalid.jwt_missing_dot_token")); + } + + @Test + void givenJwtDataFieldOfInvalidFormat_whenExtractSubject_thenThrowUnauthorizedException() { + JwtReader jwtReader = new JwtReader("sub"); + String invalidDataJson = "{"; + assertThrows(UnauthorizedException.class, () -> + jwtReader.extractUserIdFromAuthHeader( + String.format("header.%s.signature", Base64.toBase64String(invalidDataJson.getBytes(StandardCharsets.UTF_8))))); + } + + @Test + void givenInvalidAuthHeader_whenExtractSubject_thenThrowUnauthorizedException() { + JwtReader jwtReader = new JwtReader("sub"); + assertThrows(UnauthorizedException.class, () -> + jwtReader.extractUserIdFromAuthHeader("invalid_auth_header missing Bearer prefix")); + } } \ No newline at end of file diff --git a/amphora-service/src/test/java/io/carbynestack/amphora/service/opa/OpaResultTest.java b/amphora-service/src/test/java/io/carbynestack/amphora/service/opa/OpaResultTest.java new file mode 100644 index 0000000..a87c03d --- /dev/null +++ b/amphora-service/src/test/java/io/carbynestack/amphora/service/opa/OpaResultTest.java @@ -0,0 +1,31 @@ +/* + * Copyright (c) 2024 - for information on the respective copyright owner + * see the NOTICE file and/or the repository https://github.com/carbynestack/amphora. + * + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.carbynestack.amphora.service.opa; + +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.ObjectMapper; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +class OpaResultTest { + private final ObjectMapper objectMapper = new ObjectMapper(); + + @Test + void givenResultIsTrue_whenParsed_thenIsAllowedShouldBeTrue() throws JsonProcessingException { + OpaResult result = objectMapper.readValue("{\"result\": true}", OpaResult.class); + assertTrue(result.isAllowed(), "isAllowed must be true"); + } + + @Test + void givenResultIsEmpty_whenParsed_thenIsAllowedShouldBeFalse() throws JsonProcessingException { + OpaResult result = objectMapper.readValue("{}", OpaResult.class); + assertFalse(result.isAllowed(), "isAllowed must be false"); + } +} \ No newline at end of file diff --git a/amphora-service/src/test/java/io/carbynestack/amphora/service/persistence/metadata/StorageServiceTest.java b/amphora-service/src/test/java/io/carbynestack/amphora/service/persistence/metadata/StorageServiceTest.java index a5617b2..4eceeea 100644 --- a/amphora-service/src/test/java/io/carbynestack/amphora/service/persistence/metadata/StorageServiceTest.java +++ b/amphora-service/src/test/java/io/carbynestack/amphora/service/persistence/metadata/StorageServiceTest.java @@ -398,6 +398,18 @@ void givenNoSecretShareWithGivenIdInDatabase_whenGetSecretShare_thenThrowNotFoun String.format(NO_SECRET_WITH_ID_EXISTS_EXCEPTION_MSG, testSecretId), nfe.getMessage()); } + @Test + void givenSubjectHasNoAccess_whenGetSecretShare_thenThrowUnauthorizedException() throws CsOpaException { + SecretEntity secretEntity = new SecretEntity(testSecretId.toString(), emptySet()); + when(secretEntityRepository.findById(testSecretId.toString())) + .thenReturn(Optional.of(secretEntity)); + when(opaService.canReadSecret(authorizedUserId, emptyList())).thenReturn(false); + + UnauthorizedException ue = + assertThrows(UnauthorizedException.class, () -> storageService.getSecretShare(testSecretId, authorizedUserId)); + assertEquals("User is not authorized to read this secret", ue.getMessage()); + } + @Test void givenDataCannotBeRetrieved_whenGetSecretShare_thenThrowAmphoraServiceException() throws CsOpaException { AmphoraServiceException expectedAse = new AmphoraServiceException("Expected this one"); @@ -437,6 +449,52 @@ void givenSuccessfulRequest_whenGetSecretShare_thenReturnContent() throws CsOpaE assertEquals(expectedData, actualSecretShare.getData()); } + @Test + void givenNoSecretShareWithGivenIdInDatabase_whenGetSecretShareAuthorized_thenThrowNotFoundException() { + when(secretEntityRepository.findById(testSecretId.toString())).thenReturn(Optional.empty()); + + NotFoundException nfe = + assertThrows(NotFoundException.class, () -> storageService.getSecretShareAuthorized(testSecretId)); + assertEquals( + String.format(NO_SECRET_WITH_ID_EXISTS_EXCEPTION_MSG, testSecretId), nfe.getMessage()); + } + + @Test + void givenDataCannotBeRetrieved_whenGetSecretShareAuthorized_thenThrowAmphoraServiceException() { + AmphoraServiceException expectedAse = new AmphoraServiceException("Expected this one"); + TagEntity tagEntity = TagEntity.fromTag(testTag); + SecretEntity secretEntity = new SecretEntity(testSecretId.toString(), singleton(tagEntity)); + when(secretEntityRepository.findById(testSecretId.toString())) + .thenReturn(Optional.of(secretEntity)); + when(secretShareDataStore.getSecretShareData(testSecretId)).thenThrow(expectedAse); + + assertThrows( + AmphoraServiceException.class, + () -> storageService.getSecretShareAuthorized(testSecretId), + expectedAse.getMessage()); + } + + @Test + void givenSuccessfulRequest_whenGetSecretShareAuthorized_thenReturnContent() { + List expectedTags = singletonList(testTag); + byte[] expectedData = RandomUtils.nextBytes(MpSpdzIntegrationUtils.SHARE_WIDTH); + SecretEntity existingSecretEntity = + new SecretEntity(testSecretId.toString(), TagEntity.setFromTagList(expectedTags)); + + when(secretEntityRepository.findById(existingSecretEntity.getSecretId())) + .thenReturn(Optional.of(existingSecretEntity)); + when(secretShareDataStore.getSecretShareData( + UUID.fromString(existingSecretEntity.getSecretId()))) + .thenReturn(expectedData); + + SecretShare actualSecretShare = + storageService.getSecretShareAuthorized(UUID.fromString(existingSecretEntity.getSecretId())); + assertEquals( + UUID.fromString(existingSecretEntity.getSecretId()), actualSecretShare.getSecretId()); + assertEquals(expectedTags, actualSecretShare.getTags()); + assertEquals(expectedData, actualSecretShare.getData()); + } + @Test void givenNoSecretShareWithGivenIdInDatabase_whenDeleteObject_thenThrowNotFoundException() { NotFoundException nfe = assertThrows(NotFoundException.class, () -> storageService.deleteSecret(testSecretId, authorizedUserId)); @@ -444,6 +502,17 @@ void givenNoSecretShareWithGivenIdInDatabase_whenDeleteObject_thenThrowNotFoundE String.format(NO_SECRET_WITH_ID_EXISTS_EXCEPTION_MSG, testSecretId), nfe.getMessage()); } + @Test + void givenSubjectHasNoAccess_whenDeleteObject_thenThrowUnauthorizedException() throws CsOpaException { + SecretEntity secretEntity = new SecretEntity(testSecretId.toString(), emptySet()); + when(secretEntityRepository.findById(testSecretId.toString())) + .thenReturn(Optional.of(secretEntity)); + + UnauthorizedException ue = + assertThrows(UnauthorizedException.class, () -> storageService.deleteSecret(testSecretId, authorizedUserId)); + assertEquals("User is not authorized to delete this secret", ue.getMessage()); + } + @Test void givenDeleteObjectDataFails_whenDeleteObject_thenThrowGivenException() throws CsOpaException { AmphoraServiceException expectedAse = new AmphoraServiceException("Expected this one"); @@ -516,6 +585,18 @@ void givenObjectAlreadyHasTagWithGivenKey_whenStoreTag_thenThrowAlreadyExistsExc aee.getMessage()); } + @Test + void givenSubjectHasNoAccess_whenStoreTag_thenThrowUnauthorizedException() throws CsOpaException { + SecretEntity secretEntity = new SecretEntity(testSecretId.toString(), emptySet()); + when(secretEntityRepository.findById(testSecretId.toString())) + .thenReturn(Optional.of(secretEntity)); + + UnauthorizedException ue = + assertThrows(UnauthorizedException.class, () -> + storageService.storeTag(UUID.fromString(secretEntity.getSecretId()), testTag, authorizedUserId)); + assertEquals("User is not authorized to create tags for this secret", ue.getMessage()); + } + @Test void givenSuccessfulRequest_whenStoreTag_thenPersistTag() throws CsOpaException, UnauthorizedException { SecretEntity existingSecretEntity = new SecretEntity().setSecretId(testSecretId.toString()); @@ -566,6 +647,20 @@ void givenNoSecretShareWithGivenIdInDatabase_whenReplaceTags_thenThrowNotFoundEx String.format(NO_SECRET_WITH_ID_EXISTS_EXCEPTION_MSG, testSecretId), nfe.getMessage()); } + @Test + void givenSubjectHasNoAccess_whenReplaceTags_thenThrowUnauthorizedException() throws CsOpaException { + List tagListWithReservedKey = asList(testTag, testTagReservedCreationDateKey); + SecretEntity secretEntity = new SecretEntity(testSecretId.toString(), emptySet()); + when(secretEntityRepository.findById(testSecretId.toString())) + .thenReturn(Optional.of(secretEntity)); + + UnauthorizedException ue = + assertThrows(UnauthorizedException.class, () -> + storageService.replaceTags( + UUID.fromString(secretEntity.getSecretId()), tagListWithReservedKey, authorizedUserId)); + assertEquals("User is not authorized to update tags for this secret", ue.getMessage()); + } + @Test void givenListHasTagWithReservedKey_whenReplaceTags_thenReplaceByExistingTagAndPersist() throws CsOpaException, UnauthorizedException { List tagListWithReservedKey = asList(testTag, testTagReservedCreationDateKey); @@ -609,6 +704,18 @@ void givenNoSecretShareWithGivenIdInDatabase_whenRetrieveTags_thenThrowNotFoundE String.format(NO_SECRET_WITH_ID_EXISTS_EXCEPTION_MSG, testSecretId), nfe.getMessage()); } + @Test + void givenSubjectHasNoAccess_whenRetrieveTags_thenThrowUnauthorizedException() throws CsOpaException { + SecretEntity secretEntity = new SecretEntity(testSecretId.toString(), emptySet()); + when(secretEntityRepository.findById(testSecretId.toString())) + .thenReturn(Optional.of(secretEntity)); + + UnauthorizedException ue = + assertThrows(UnauthorizedException.class, () -> + storageService.retrieveTags(UUID.fromString(secretEntity.getSecretId()), authorizedUserId)); + assertEquals("User is not authorized to read tags for this secret", ue.getMessage()); + } + @Test void givenSuccessfulRequest_whenRetrieveTags_thenReturnContent() throws CsOpaException, UnauthorizedException { List expectedTags = asList(testTag, testTag2); @@ -647,6 +754,18 @@ void givenNoTagWithGivenKeyInDatabaseForGivenObject_whenRetrieveTag_thenThrowNot existingSecretEntity.getSecretId())); } + @Test + void givenSubjectHasNoAccess_whenRetrieveTag_thenThrowUnauthorizedException() throws CsOpaException { + SecretEntity secretEntity = new SecretEntity(testSecretId.toString(), emptySet()); + when(secretEntityRepository.findById(testSecretId.toString())) + .thenReturn(Optional.of(secretEntity)); + + UnauthorizedException ue = + assertThrows(UnauthorizedException.class, () -> + storageService.retrieveTag(testSecretId, testTag.getKey(), authorizedUserId)); + assertEquals("User is not authorized to read tags for this secret", ue.getMessage()); + } + @Test void givenSuccessfulRequest_whenRetrieveTag_thenReturnContent() throws CsOpaException, UnauthorizedException { TagEntity existingTagEntity = TagEntity.fromTag(testTag); @@ -694,6 +813,20 @@ void givenNoTagWithGivenKeyInDatabaseForGivenObject_whenUpdateTag_thenThrowNotFo existingSecretEntity.getSecretId())); } + @Test + void givenSubjectHasNoAccess_whenUpdateTag_thenThrowUnauthorizedException() throws CsOpaException { + Tag newTag = + Tag.builder().key(testTag.getKey()).value("123").valueType(TagValueType.LONG).build(); + SecretEntity secretEntity = new SecretEntity(testSecretId.toString(), emptySet()); + when(secretEntityRepository.findById(testSecretId.toString())) + .thenReturn(Optional.of(secretEntity)); + + UnauthorizedException ue = + assertThrows(UnauthorizedException.class, () -> + storageService.updateTag(UUID.fromString(secretEntity.getSecretId()), newTag, authorizedUserId)); + assertEquals("User is not authorized to update tags for this secret", ue.getMessage()); + } + @Test void givenSuccessfulRequest_whenUpdateTag_thenUpdateTag() throws CsOpaException, UnauthorizedException { Tag newTag = @@ -751,6 +884,20 @@ void givenNoTagWithGivenKeyInDatabaseForGivenObject_whenDeleteTag_thenThrowNotFo existingSecretEntity.getSecretId())); } + @Test + void givenSubjectHasNoAccess_whenDeleteTag_thenThrowUnauthorizedException() throws CsOpaException { + TagEntity tagEntityToDelete = TagEntity.fromTag(testTag); + SecretEntity secretEntity = new SecretEntity(testSecretId.toString(), emptySet()); + when(secretEntityRepository.findById(testSecretId.toString())) + .thenReturn(Optional.of(secretEntity)); + + UnauthorizedException ue = + assertThrows(UnauthorizedException.class, () -> + storageService.deleteTag( + UUID.fromString(secretEntity.getSecretId()), tagEntityToDelete.getKey(), authorizedUserId)); + assertEquals("User is not authorized to delete tags for this secret", ue.getMessage()); + } + @Test void givenSuccessfulRequest_whenDeleteTag_thenDelete() throws CsOpaException, UnauthorizedException { TagEntity tagEntityToDelete = TagEntity.fromTag(testTag);