From 9c9de8cea0516e056e7c83b0440e897920e6af18 Mon Sep 17 00:00:00 2001 From: macacia Date: Thu, 17 Oct 2024 16:50:12 +0200 Subject: [PATCH] P4ADEV-1274 added parameters validation --- .../payhub/auth/service/AuthnServiceImpl.java | 20 ++++-- .../service/a2a/ClientCredentialService.java | 7 ++ .../a2a/ClientCredentialServiceImpl.java | 23 +++++++ .../auth/service/a2a/ClientService.java | 3 + .../auth/service/a2a/ClientServiceImpl.java | 7 +- .../a2a/ValidateClientCredentialsService.java | 50 ++++++++++++++ .../a2a/retrieve/ClientRetrieverService.java | 3 + .../payhub/auth/service/AuthnServiceTest.java | 56 ++++++++++++++-- .../auth/service/a2a/ClientServiceTest.java | 14 ++++ .../ValidateClientCredentialsServiceTest.java | 67 +++++++++++++++++++ .../retrieve/ClientRetrieverServiceTest.java | 20 +++++- .../ValidateExternalTokenServiceTest.java | 2 +- 12 files changed, 259 insertions(+), 13 deletions(-) create mode 100644 src/main/java/it/gov/pagopa/payhub/auth/service/a2a/ClientCredentialService.java create mode 100644 src/main/java/it/gov/pagopa/payhub/auth/service/a2a/ClientCredentialServiceImpl.java create mode 100644 src/main/java/it/gov/pagopa/payhub/auth/service/a2a/ValidateClientCredentialsService.java create mode 100644 src/test/java/it/gov/pagopa/payhub/auth/service/a2a/ValidateClientCredentialsServiceTest.java diff --git a/src/main/java/it/gov/pagopa/payhub/auth/service/AuthnServiceImpl.java b/src/main/java/it/gov/pagopa/payhub/auth/service/AuthnServiceImpl.java index f2ca498b..6e3c9dbd 100644 --- a/src/main/java/it/gov/pagopa/payhub/auth/service/AuthnServiceImpl.java +++ b/src/main/java/it/gov/pagopa/payhub/auth/service/AuthnServiceImpl.java @@ -1,6 +1,10 @@ package it.gov.pagopa.payhub.auth.service; +import it.gov.pagopa.payhub.auth.exception.custom.InvalidGrantTypeException; +import it.gov.pagopa.payhub.auth.service.a2a.ClientCredentialService; +import it.gov.pagopa.payhub.auth.service.a2a.ValidateClientCredentialsService; import it.gov.pagopa.payhub.auth.service.exchange.ExchangeTokenService; +import it.gov.pagopa.payhub.auth.service.exchange.ValidateExternalTokenService; import it.gov.pagopa.payhub.auth.service.logout.LogoutService; import it.gov.pagopa.payhub.auth.service.user.UserService; import it.gov.pagopa.payhub.model.generated.AccessToken; @@ -11,19 +15,25 @@ @Slf4j @Service public class AuthnServiceImpl implements AuthnService { + private final ClientCredentialService clientCredentialService; private final ExchangeTokenService exchangeTokenService; private final UserService userService; private final LogoutService logoutService; - public AuthnServiceImpl(ExchangeTokenService exchangeTokenService, UserService userService, LogoutService logoutService) { - this.exchangeTokenService = exchangeTokenService; - this.userService = userService; - this.logoutService = logoutService; + public AuthnServiceImpl(ClientCredentialService clientCredentialService, ExchangeTokenService exchangeTokenService, UserService userService, LogoutService logoutService) { + this.clientCredentialService = clientCredentialService; + this.exchangeTokenService = exchangeTokenService; + this.userService = userService; + this.logoutService = logoutService; } @Override public AccessToken postToken(String clientId, String grantType, String scope, String subjectToken, String subjectIssuer, String subjectTokenType, String clientSecret) { - return exchangeTokenService.postToken(clientId, grantType, subjectToken, subjectIssuer, subjectTokenType, scope); + return switch (grantType) { + case ValidateExternalTokenService.ALLOWED_GRANT_TYPE -> exchangeTokenService.postToken(clientId, grantType, subjectToken, subjectIssuer, subjectTokenType, scope); + case ValidateClientCredentialsService.ALLOWED_GRANT_TYPE -> clientCredentialService.postToken(clientId, grantType, scope, clientSecret); + default -> throw new InvalidGrantTypeException("Invalid grantType " + grantType); + }; } @Override diff --git a/src/main/java/it/gov/pagopa/payhub/auth/service/a2a/ClientCredentialService.java b/src/main/java/it/gov/pagopa/payhub/auth/service/a2a/ClientCredentialService.java new file mode 100644 index 00000000..da334914 --- /dev/null +++ b/src/main/java/it/gov/pagopa/payhub/auth/service/a2a/ClientCredentialService.java @@ -0,0 +1,7 @@ +package it.gov.pagopa.payhub.auth.service.a2a; + +import it.gov.pagopa.payhub.model.generated.AccessToken; + +public interface ClientCredentialService { + AccessToken postToken(String clientId, String grantType, String scope, String clientSecret); +} diff --git a/src/main/java/it/gov/pagopa/payhub/auth/service/a2a/ClientCredentialServiceImpl.java b/src/main/java/it/gov/pagopa/payhub/auth/service/a2a/ClientCredentialServiceImpl.java new file mode 100644 index 00000000..6b89b07f --- /dev/null +++ b/src/main/java/it/gov/pagopa/payhub/auth/service/a2a/ClientCredentialServiceImpl.java @@ -0,0 +1,23 @@ +package it.gov.pagopa.payhub.auth.service.a2a; + +import it.gov.pagopa.payhub.model.generated.AccessToken; +import lombok.extern.slf4j.Slf4j; +import org.springframework.stereotype.Service; + +@Service +@Slf4j +public class ClientCredentialServiceImpl implements ClientCredentialService { + + private final ValidateClientCredentialsService validateClientCredentialsService; + + public ClientCredentialServiceImpl(ValidateClientCredentialsService validateClientCredentialsService) { + this.validateClientCredentialsService = validateClientCredentialsService; + } + + @Override + public AccessToken postToken(String clientId, String grantType, String scope, String clientSecret) { + validateClientCredentialsService.validate(clientId, grantType, scope, clientSecret); + //TODO return real AccessToken implementation + return new AccessToken("token", "bearer", 7); + } +} diff --git a/src/main/java/it/gov/pagopa/payhub/auth/service/a2a/ClientService.java b/src/main/java/it/gov/pagopa/payhub/auth/service/a2a/ClientService.java index c4356153..1abb977a 100644 --- a/src/main/java/it/gov/pagopa/payhub/auth/service/a2a/ClientService.java +++ b/src/main/java/it/gov/pagopa/payhub/auth/service/a2a/ClientService.java @@ -1,13 +1,16 @@ package it.gov.pagopa.payhub.auth.service.a2a; +import it.gov.pagopa.payhub.auth.model.Client; import it.gov.pagopa.payhub.model.generated.ClientDTO; import it.gov.pagopa.payhub.model.generated.ClientNoSecretDTO; import java.util.List; +import java.util.Optional; public interface ClientService { ClientDTO registerClient(String clientName, String organizationIpaCode); String getClientSecret(String organizationIpaCode, String clientId); List getClients(String organizationIpaCode); + Optional getClientByClientId(String clientId); } diff --git a/src/main/java/it/gov/pagopa/payhub/auth/service/a2a/ClientServiceImpl.java b/src/main/java/it/gov/pagopa/payhub/auth/service/a2a/ClientServiceImpl.java index ebda1341..d4ba9be7 100644 --- a/src/main/java/it/gov/pagopa/payhub/auth/service/a2a/ClientServiceImpl.java +++ b/src/main/java/it/gov/pagopa/payhub/auth/service/a2a/ClientServiceImpl.java @@ -10,6 +10,7 @@ import org.springframework.stereotype.Service; import java.util.List; +import java.util.Optional; @Service @Slf4j @@ -27,7 +28,6 @@ public ClientServiceImpl(ClientRegistrationService clientRegistrationService, Cl @Override public ClientDTO registerClient(String clientName, String organizationIpaCode) { - Client client = clientRegistrationService.registerClient(clientName, organizationIpaCode); return clientMapper.mapToDTO(client); } @@ -44,4 +44,9 @@ public List getClients(String organizationIpaCode) { return clientRetrieverService.getClients(organizationIpaCode); } + public Optional getClientByClientId(String clientId) { + log.info("Retrieving client for {}", clientId); + return clientRetrieverService.getClientByClientId(clientId); + } + } diff --git a/src/main/java/it/gov/pagopa/payhub/auth/service/a2a/ValidateClientCredentialsService.java b/src/main/java/it/gov/pagopa/payhub/auth/service/a2a/ValidateClientCredentialsService.java new file mode 100644 index 00000000..a036961e --- /dev/null +++ b/src/main/java/it/gov/pagopa/payhub/auth/service/a2a/ValidateClientCredentialsService.java @@ -0,0 +1,50 @@ +package it.gov.pagopa.payhub.auth.service.a2a; + +import it.gov.pagopa.payhub.auth.exception.custom.InvalidExchangeClientException; +import it.gov.pagopa.payhub.auth.exception.custom.InvalidExchangeRequestException; +import it.gov.pagopa.payhub.auth.exception.custom.InvalidGrantTypeException; +import it.gov.pagopa.payhub.auth.model.Client; +import lombok.extern.slf4j.Slf4j; +import org.springframework.stereotype.Service; +import org.springframework.util.StringUtils; + +@Service +@Slf4j +public class ValidateClientCredentialsService { + private final ClientService clientService; + public static final String ALLOWED_GRANT_TYPE = "client_credentials"; + public static final String ALLOWED_SCOPE = "openid"; + + public ValidateClientCredentialsService(ClientService clientService) { + this.clientService = clientService; + } + + public void validate(String clientId, String grantType, String scope, String clientSecret) { + validateClient(clientId); + validateProtocolConfiguration(grantType, scope); + validateClientSecret(clientSecret); + log.info("authorization granted"); + } + + //TODO Client will be used to verify clientSecret and assign roles with organizationIpaCode + private Client validateClient(String clientId) { + return clientService.getClientByClientId(clientId) + .orElseThrow(() -> new InvalidExchangeClientException("Invalid clientId:"+ clientId)); + } + + private void validateProtocolConfiguration(String grantType, String scope) { + if (!ALLOWED_GRANT_TYPE.equals(grantType)) { + throw new InvalidGrantTypeException("Invalid grantType " + grantType); + } + if (!ALLOWED_SCOPE.equals(scope)){ + throw new InvalidExchangeRequestException("Invalid scope " + scope); + } + } + + private void validateClientSecret(String clientSecret) { + if (!StringUtils.hasText(clientSecret)) { + throw new InvalidExchangeRequestException("clientSecret is mandatory with client-credentials grant type"); + } + } + +} diff --git a/src/main/java/it/gov/pagopa/payhub/auth/service/a2a/retrieve/ClientRetrieverService.java b/src/main/java/it/gov/pagopa/payhub/auth/service/a2a/retrieve/ClientRetrieverService.java index 0d9c9418..d9d46de0 100644 --- a/src/main/java/it/gov/pagopa/payhub/auth/service/a2a/retrieve/ClientRetrieverService.java +++ b/src/main/java/it/gov/pagopa/payhub/auth/service/a2a/retrieve/ClientRetrieverService.java @@ -10,6 +10,7 @@ import org.springframework.stereotype.Service; import java.util.List; +import java.util.Optional; @Service @Slf4j @@ -41,4 +42,6 @@ public List getClients(String organizationIpaCode) { .toList(); } + public Optional getClientByClientId(String clientId) { return clientRepository.findById(clientId); } + } diff --git a/src/test/java/it/gov/pagopa/payhub/auth/service/AuthnServiceTest.java b/src/test/java/it/gov/pagopa/payhub/auth/service/AuthnServiceTest.java index 343f6d57..3e1c6d0d 100644 --- a/src/test/java/it/gov/pagopa/payhub/auth/service/AuthnServiceTest.java +++ b/src/test/java/it/gov/pagopa/payhub/auth/service/AuthnServiceTest.java @@ -1,6 +1,10 @@ package it.gov.pagopa.payhub.auth.service; +import it.gov.pagopa.payhub.auth.exception.custom.InvalidGrantTypeException; +import it.gov.pagopa.payhub.auth.service.a2a.ClientCredentialService; +import it.gov.pagopa.payhub.auth.service.a2a.ValidateClientCredentialsService; import it.gov.pagopa.payhub.auth.service.exchange.ExchangeTokenService; +import it.gov.pagopa.payhub.auth.service.exchange.ValidateExternalTokenService; import it.gov.pagopa.payhub.auth.service.logout.LogoutService; import it.gov.pagopa.payhub.auth.service.user.UserService; import it.gov.pagopa.payhub.model.generated.AccessToken; @@ -14,9 +18,13 @@ import org.mockito.Mockito; import org.mockito.junit.jupiter.MockitoExtension; +import static org.junit.jupiter.api.Assertions.assertThrows; + @ExtendWith(MockitoExtension.class) class AuthnServiceTest { + @Mock + private ClientCredentialService clientCredentialService; @Mock private ExchangeTokenService exchangeTokenServiceMock; @Mock @@ -28,15 +36,16 @@ class AuthnServiceTest { @BeforeEach void init(){ - service = new AuthnServiceImpl(exchangeTokenServiceMock, userServiceMock, logoutServiceMock); + service = new AuthnServiceImpl(clientCredentialService, exchangeTokenServiceMock, userServiceMock, logoutServiceMock); } @AfterEach void verifyNotMoreInteractions(){ Mockito.verifyNoMoreInteractions( - exchangeTokenServiceMock, - userServiceMock, - logoutServiceMock + clientCredentialService, + exchangeTokenServiceMock, + userServiceMock, + logoutServiceMock ); } @@ -44,13 +53,13 @@ void verifyNotMoreInteractions(){ void whenPostTokenThenCallExchangeService(){ // Given String clientId="CLIENT_ID"; - String grantType="GRANT_TYPE"; String subjectToken="SUBJECT_TOKEN"; String subjectIssuer="SUBJECT_ISSUER"; String subjectTokenType="SUBJECT_TOKEN_TYPE"; String scope="SCOPE"; String clientSecret = "CLIENT_SECRET"; + String grantType= ValidateExternalTokenService.ALLOWED_GRANT_TYPE; AccessToken expectedResult = new AccessToken(); Mockito.when(exchangeTokenServiceMock.postToken(clientId, grantType, subjectToken, subjectIssuer, subjectTokenType, scope)) .thenReturn(expectedResult); @@ -62,6 +71,43 @@ void whenPostTokenThenCallExchangeService(){ Assertions.assertSame(expectedResult, result); } + @Test + void whenPostTokenThenCallClientCredentialService(){ + // Given + String clientId="CLIENT_ID"; + String subjectToken="SUBJECT_TOKEN"; + String subjectIssuer="SUBJECT_ISSUER"; + String subjectTokenType="SUBJECT_TOKEN_TYPE"; + String scope="SCOPE"; + String clientSecret = "CLIENT_SECRET"; + + String grantType= ValidateClientCredentialsService.ALLOWED_GRANT_TYPE; + AccessToken expectedResult = new AccessToken(); + Mockito.when(clientCredentialService.postToken(clientId, grantType, scope, clientSecret)).thenReturn(expectedResult); + + // When + AccessToken result = service.postToken(clientId, grantType, scope, subjectToken, subjectIssuer, subjectTokenType, clientSecret); + + // Then + Assertions.assertSame(expectedResult, result); + } + + @Test + void whenPostTokenWhenCallClientCredentialServiceThenInvalidGrantTypeException(){ + // Given + String clientId="CLIENT_ID"; + String subjectToken="SUBJECT_TOKEN"; + String subjectIssuer="SUBJECT_ISSUER"; + String subjectTokenType="SUBJECT_TOKEN_TYPE"; + String scope="SCOPE"; + String clientSecret = "CLIENT_SECRET"; + + String grantType="UNEXPECTED_GRANT_TYPE"; + // When, Then + assertThrows(InvalidGrantTypeException.class, () -> + service.postToken(clientId, grantType, scope, subjectToken, subjectIssuer, subjectTokenType, clientSecret)); + } + @Test void whenGetUserInfoThenCallUserService(){ // Given diff --git a/src/test/java/it/gov/pagopa/payhub/auth/service/a2a/ClientServiceTest.java b/src/test/java/it/gov/pagopa/payhub/auth/service/a2a/ClientServiceTest.java index cab22443..4020d07e 100644 --- a/src/test/java/it/gov/pagopa/payhub/auth/service/a2a/ClientServiceTest.java +++ b/src/test/java/it/gov/pagopa/payhub/auth/service/a2a/ClientServiceTest.java @@ -16,6 +16,7 @@ import org.mockito.junit.jupiter.MockitoExtension; import java.util.List; +import java.util.Optional; import java.util.UUID; @ExtendWith(MockitoExtension.class) @@ -103,4 +104,17 @@ void givenOrganizationIpaCodeWhenGetClientsThenGetClientNoSecretDTOList() { Assertions.assertEquals(List.of(dto1, dto2), result); } + @Test + void givenClientIdWhenGetClientByClientIdThenInvokeClientService() { + // Given + String clientId = "clientId"; + Client expectedClient = new Client(); + + Mockito.when(clientRetrieverServiceMock.getClientByClientId(clientId)).thenReturn(Optional.of(expectedClient)); + //When + Optional result = service.getClientByClientId(clientId); + // Then + Assertions.assertEquals(Optional.of(expectedClient), result); + } + } diff --git a/src/test/java/it/gov/pagopa/payhub/auth/service/a2a/ValidateClientCredentialsServiceTest.java b/src/test/java/it/gov/pagopa/payhub/auth/service/a2a/ValidateClientCredentialsServiceTest.java new file mode 100644 index 00000000..051aa60f --- /dev/null +++ b/src/test/java/it/gov/pagopa/payhub/auth/service/a2a/ValidateClientCredentialsServiceTest.java @@ -0,0 +1,67 @@ +package it.gov.pagopa.payhub.auth.service.a2a; + +import it.gov.pagopa.payhub.auth.exception.custom.InvalidExchangeClientException; +import it.gov.pagopa.payhub.auth.exception.custom.InvalidExchangeRequestException; +import it.gov.pagopa.payhub.auth.exception.custom.InvalidGrantTypeException; +import it.gov.pagopa.payhub.auth.model.Client; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.junit.jupiter.MockitoExtension; + +import java.util.Optional; + +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertThrows; + +@ExtendWith(MockitoExtension.class) +public class ValidateClientCredentialsServiceTest { + @Mock + private ClientService clientServiceMock; + private ValidateClientCredentialsService service; + + private static final String ALLOWED_CLIENT_ID = "CLIENTID"; + private static final String ALLOWED_CLIENT_SECRET = "CLIENTSECRET"; + + @BeforeEach + void setup(){ + service = new ValidateClientCredentialsService(clientServiceMock); + } + + @Test + void givenValidRequestThenOk() { + Mockito.doReturn(Optional.of(new Client())).when(clientServiceMock).getClientByClientId(ALLOWED_CLIENT_ID); + assertDoesNotThrow(() -> + service.validate(ALLOWED_CLIENT_ID, ValidateClientCredentialsService.ALLOWED_GRANT_TYPE, ValidateClientCredentialsService.ALLOWED_SCOPE, ALLOWED_CLIENT_SECRET)); + } + + @Test + void givenInvalidExchangeClientException() { + assertThrows(InvalidExchangeClientException.class, () -> + service.validate("UNEXPECTED_CLIENT_ID", ValidateClientCredentialsService.ALLOWED_GRANT_TYPE, ValidateClientCredentialsService.ALLOWED_SCOPE, ALLOWED_CLIENT_SECRET)); + } + + @Test + void givenInvalidGrantTypeException() { + Mockito.doReturn(Optional.of(new Client())).when(clientServiceMock).getClientByClientId(ALLOWED_CLIENT_ID); + assertThrows(InvalidGrantTypeException.class, () -> + service.validate(ALLOWED_CLIENT_ID, "UNEXPECTED_GRANT_TYPE", ValidateClientCredentialsService.ALLOWED_SCOPE, ALLOWED_CLIENT_SECRET)); + } + + @Test + void givenInvalidScopeThenInvalidExchangeRequestException() { + Mockito.doReturn(Optional.of(new Client())).when(clientServiceMock).getClientByClientId(ALLOWED_CLIENT_ID); + assertThrows(InvalidExchangeRequestException.class, () -> + service.validate(ALLOWED_CLIENT_ID, ValidateClientCredentialsService.ALLOWED_GRANT_TYPE, "UNEXPECTED_SCOPE", ALLOWED_CLIENT_SECRET)); + } + + @Test + void givenNullClientSecretThenInvalidExchangeRequestException() { + Mockito.doReturn(Optional.of(new Client())).when(clientServiceMock).getClientByClientId(ALLOWED_CLIENT_ID); + assertThrows(InvalidExchangeRequestException.class, () -> + service.validate(ALLOWED_CLIENT_ID, ValidateClientCredentialsService.ALLOWED_GRANT_TYPE, ValidateClientCredentialsService.ALLOWED_SCOPE, null)); + } + +} diff --git a/src/test/java/it/gov/pagopa/payhub/auth/service/a2a/retrieve/ClientRetrieverServiceTest.java b/src/test/java/it/gov/pagopa/payhub/auth/service/a2a/retrieve/ClientRetrieverServiceTest.java index bedd29e2..7cba6235 100644 --- a/src/test/java/it/gov/pagopa/payhub/auth/service/a2a/retrieve/ClientRetrieverServiceTest.java +++ b/src/test/java/it/gov/pagopa/payhub/auth/service/a2a/retrieve/ClientRetrieverServiceTest.java @@ -103,5 +103,23 @@ void givenOrganizationIpaCodeWhenGetClientsThenInvokeClientRetrieverService(){ // Then Assertions.assertEquals(List.of(expectedDto1, expectedDto2), result); } - + + @Test + void givenClientIdWhenFindByIdThenInvokeClientRetrieverService(){ + // Given + String organizationIpaCode = "organizationIpaCode"; + String clientName = "clientName"; + String clientId = organizationIpaCode + clientName; + byte[] encryptedClientSecret = new byte[16]; + new Random().nextBytes(encryptedClientSecret); + Client storedClient = new Client(clientId, clientName, organizationIpaCode, encryptedClientSecret); + + Mockito.when(clientRepositoryMock.findById(clientId)).thenReturn(Optional.of(storedClient)); + + // When + Optional result = service.getClientByClientId(clientId); + + // Then + Assertions.assertEquals(Optional.of(storedClient), result); + } } diff --git a/src/test/java/it/gov/pagopa/payhub/auth/service/exchange/ValidateExternalTokenServiceTest.java b/src/test/java/it/gov/pagopa/payhub/auth/service/exchange/ValidateExternalTokenServiceTest.java index f760a9e6..bfe15b99 100644 --- a/src/test/java/it/gov/pagopa/payhub/auth/service/exchange/ValidateExternalTokenServiceTest.java +++ b/src/test/java/it/gov/pagopa/payhub/auth/service/exchange/ValidateExternalTokenServiceTest.java @@ -174,7 +174,7 @@ void givenNullSubjectTypeThenIllegalArgumentException() throws Exception { } @Test - void givenNullSubjectTokenThenIllegalArgumentException() throws Exception { + void givenNullSubjectTokenThenInvalidExchangeRequestException() throws Exception { String subjectToken = utils.generateJWK(EXPIRES_AT); Map claimsMap = createJWKClaims(ALLOWED_SUBECJECT_ISSUER, ALLOWED_AUDIENCE);