Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: P4ADEV-1102-create-client-revoke-API #103

Merged
merged 6 commits into from
Oct 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions openapi/p4pa-auth.openapi.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,34 @@ paths:
'429':
description: Too Many Requests
/auth/clients/{organizationIpaCode}/{clientId}:
delete:
tags:
- authz
operationId: revokeClient
parameters:
- name: organizationIpaCode
in: path
required: true
schema:
type: string
- name: clientId
in: path
required: true
schema:
type: string
responses:
'204':
description: OK
'400':
description: Invalid request
'401':
description: Unauthorized
'403':
description: Forbidden
'412':
description: ToS acceptance missing
'429':
description: Too Many Requests
get:
tags:
- authz
Expand Down
56 changes: 56 additions & 0 deletions postman/p4pa-auth-E2E.postman_collection.json
Original file line number Diff line number Diff line change
Expand Up @@ -1129,6 +1129,62 @@
}
},
"response": []
},
{
"name": "revokeClient",
"event": [
{
"listen": "test",
"script": {
"exec": [
"pm.test(\"Auth - revokeClient - Responses with 200\", function () {\r",
" pm.response.to.have.status(200);\r",
"});"
],
"type": "text/javascript",
"packages": {}
}
},
{
"listen": "prerequest",
"script": {
"exec": [
" pm.environment.get(\"clientId\")\r",
" pm.environment.get(\"organizationIpaCode\")"
],
"type": "text/javascript",
"packages": {}
}
}
],
"request": {
"auth": {
"type": "bearer",
"bearer": [
{
"key": "token",
"value": "{{accessToken}}",
"type": "string"
}
]
},
"method": "DELETE",
"header": [],
"url": {
"raw": "{{p4paAuthBaseUrl}}/payhub/auth/clients/{{organizationIpaCode}}/{{clientId}}",
"host": [
"{{p4paAuthBaseUrl}}"
],
"path": [
"payhub",
"auth",
"clients",
"{{organizationIpaCode}}",
"{{clientId}}"
]
}
},
"response": []
}
],
"event": [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,4 +122,13 @@ public ResponseEntity<List<ClientNoSecretDTO>> getClients(String organizationIpa
}
return ResponseEntity.ok(authzService.getClients(organizationIpaCode));
}

@Override
public ResponseEntity<Void> revokeClient(String organizationIpaCode, String clientId) {
if(!SecurityUtils.isPrincipalAdmin(organizationIpaCode)){
throw new UserUnauthorizedException("User not allowed to delete client with clientId " + clientId);
}
authzService.revokeClient(organizationIpaCode, clientId);
return ResponseEntity.ok(null);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,6 @@
import java.util.List;

public interface ClientRepository extends MongoRepository<Client, String> {

List<Client> findAllByOrganizationIpaCode(String organizationIpaCode);
long deleteByClientIdAndOrganizationIpaCode(String clientId, String organizationIpaCode);
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,5 @@ public interface AuthzService {
ClientDTO registerClient(String organizationIpaCode, CreateClientRequest createClientRequest);
String getClientSecret(String organizationIpaCode, String clientId);
List<ClientNoSecretDTO> getClients(String organizationIpaCode);
void revokeClient(String organizationIpaCode, String clientId);
}
Original file line number Diff line number Diff line change
Expand Up @@ -117,4 +117,9 @@ public String getClientSecret(String organizationIpaCode, String clientId) {
public List<ClientNoSecretDTO> getClients(String organizationIpaCode) {
return clientService.getClients(organizationIpaCode);
}

@Override
public void revokeClient(String organizationIpaCode, String clientId) {
clientService.revokeClient(organizationIpaCode, clientId);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,5 @@ public interface ClientService {
String getClientSecret(String organizationIpaCode, String clientId);
List<ClientNoSecretDTO> getClients(String organizationIpaCode);
Optional<Client> getClientByClientId(String clientId);
void revokeClient(String organizationIpaCode, String clientId);
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,10 @@
import it.gov.pagopa.payhub.auth.model.Client;
import it.gov.pagopa.payhub.auth.service.a2a.registration.ClientRegistrationService;
import it.gov.pagopa.payhub.auth.service.a2a.retrieve.ClientRetrieverService;
import it.gov.pagopa.payhub.auth.service.a2a.revoke.ClientRemovalService;
import it.gov.pagopa.payhub.model.generated.ClientDTO;
import it.gov.pagopa.payhub.model.generated.ClientNoSecretDTO;
import jakarta.transaction.Transactional;
import lombok.extern.slf4j.Slf4j;
import org.springframework.stereotype.Service;

Expand All @@ -15,12 +17,13 @@
@Service
@Slf4j
public class ClientServiceImpl implements ClientService {

private final ClientRemovalService clientRemovalService;
private final ClientRegistrationService clientRegistrationService;
private final ClientRetrieverService clientRetrieverService;
private final ClientMapper clientMapper;

public ClientServiceImpl(ClientRegistrationService clientRegistrationService, ClientRetrieverService clientRetrieverService, ClientMapper clientMapper) {
public ClientServiceImpl(ClientRemovalService clientRemovalService, ClientRegistrationService clientRegistrationService, ClientRetrieverService clientRetrieverService, ClientMapper clientMapper) {
this.clientRemovalService = clientRemovalService;
this.clientRegistrationService = clientRegistrationService;
this.clientRetrieverService = clientRetrieverService;
this.clientMapper = clientMapper;
Expand Down Expand Up @@ -49,4 +52,9 @@ public Optional<Client> getClientByClientId(String clientId) {
return clientRetrieverService.getClientByClientId(clientId);
}

@Override
public void revokeClient(String organizationIpaCode, String clientId) {
clientRemovalService.revokeClient(organizationIpaCode, clientId);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package it.gov.pagopa.payhub.auth.service.a2a.revoke;

import it.gov.pagopa.payhub.auth.repository.ClientRepository;
import lombok.extern.slf4j.Slf4j;
import org.springframework.stereotype.Service;

@Service
@Slf4j
public class ClientRemovalService {

private final ClientRepository clientRepository;

public ClientRemovalService(ClientRepository clientRepository) {
this.clientRepository = clientRepository;
}

public void revokeClient(String organizationIpaCode, String clientId) {
clientRepository.deleteByClientIdAndOrganizationIpaCode(clientId, organizationIpaCode);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,10 @@
import java.util.UUID;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.mockito.BDDMockito.willDoNothing;
import static org.mockito.Mockito.doReturn;
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post;
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.*;
import static org.springframework.test.web.servlet.result.MockMvcResultHandlers.print;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.content;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;

Expand Down Expand Up @@ -467,5 +468,55 @@ void givenAlreadyExistentClientWhenRegisterClientThenConflict() throws Exception
.content(objectMapper.writeValueAsString(createClientRequest))
).andExpect(status().isConflict());
}
//end region

//region revokeClient tests
@Test
void givenAuthorizedUserWhenRevokeClientThenOk() throws Exception {
String organizationIpaCode = "IPA_TEST_2";
String clientId = "CLIENTID";

UserInfo expectedUser = UserInfo.builder()
.userId("USERID")
.organizationAccess(organizationIpaCode)
.organizations(List.of(UserOrganizationRoles.builder()
.organizationIpaCode(organizationIpaCode)
.roles(List.of(Constants.ROLE_ADMIN))
.build()))
.build();

Mockito.when(authnServiceMock.getUserInfo("accessToken"))
.thenReturn(expectedUser);

willDoNothing().given(authzServiceMock).revokeClient(organizationIpaCode, clientId);

mockMvc.perform(
delete("/payhub/auth/clients/{organizationIpaCode}/{clientId}", organizationIpaCode, clientId)
.header(HttpHeaders.AUTHORIZATION, "Bearer accessToken")
).andExpect(status().isOk())
.andDo(print());
}

@Test
void givenUnauthorizedUserWhenRevokeClientThenException() throws Exception {
//Given
String organizationIpaCode = "IPA_TEST_2";
String clientId = "CLIENTID";

//When
Mockito.when(authnServiceMock.getUserInfo("accessToken"))
.thenReturn(UserInfo.builder()
.organizations(List.of(UserOrganizationRoles.builder()
.organizationIpaCode("ORG")
.roles(List.of(Constants.ROLE_OPER))
.build()))
.build());

//Then
mockMvc.perform(
delete("/payhub/auth/clients/{organizationIpaCode}/{clientId}", organizationIpaCode, clientId)
.header(HttpHeaders.AUTHORIZATION, "Bearer accessToken")
).andExpect(status().isUnauthorized());
}
//end region
}
Original file line number Diff line number Diff line change
Expand Up @@ -299,4 +299,14 @@ void givenOrganizationIpaCodeWhenGetClientsThenInvokeClientService() {
Assertions.assertEquals(expectedDTOList, result);
}

@Test
void givenClientIdWhenRevokeClientThenVerifyRevoke() {
//Given
String organizationIpaCode = "organizationIpaCode";
String clientId = "clientId";
//When
service.revokeClient(organizationIpaCode, clientId);
//Then
Mockito.verify(clientServiceMock).revokeClient(organizationIpaCode, clientId);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import it.gov.pagopa.payhub.auth.model.Client;
import it.gov.pagopa.payhub.auth.service.a2a.registration.ClientRegistrationService;
import it.gov.pagopa.payhub.auth.service.a2a.retrieve.ClientRetrieverService;
import it.gov.pagopa.payhub.auth.service.a2a.revoke.ClientRemovalService;
import it.gov.pagopa.payhub.model.generated.ClientDTO;
import it.gov.pagopa.payhub.model.generated.ClientNoSecretDTO;
import org.junit.jupiter.api.AfterEach;
Expand All @@ -22,6 +23,9 @@
@ExtendWith(MockitoExtension.class)
class ClientServiceTest {

@Mock
private ClientRemovalService clientRemovalServiceMock;

@Mock
private ClientRegistrationService clientRegistrationServiceMock;

Expand All @@ -35,12 +39,13 @@ class ClientServiceTest {

@BeforeEach
void init(){
service = new ClientServiceImpl(clientRegistrationServiceMock, clientRetrieverServiceMock, clientMapperMock);
service = new ClientServiceImpl(clientRemovalServiceMock, clientRegistrationServiceMock, clientRetrieverServiceMock, clientMapperMock);
}

@AfterEach
void verifyNotMoreInteractions(){
Mockito.verifyNoMoreInteractions(
clientRemovalServiceMock,
clientRegistrationServiceMock,
clientRetrieverServiceMock,
clientMapperMock
Expand Down Expand Up @@ -117,4 +122,14 @@ void givenClientIdWhenGetClientByClientIdThenInvokeClientService() {
Assertions.assertEquals(Optional.of(expectedClient), result);
}

@Test
void givenClientIdWhenRevokeClientThenVerifyRevoke() {
// Given
String organizationIpaCode = "organizationIpaCode";
String clientId = "clientId";
//When
service.revokeClient(organizationIpaCode, clientId);
//Then
Mockito.verify(clientRemovalServiceMock).revokeClient(organizationIpaCode, clientId);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
package it.gov.pagopa.payhub.auth.service.a2a.revoke;

import it.gov.pagopa.payhub.auth.repository.ClientRepository;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.Mockito;
import org.mockito.junit.jupiter.MockitoExtension;

@ExtendWith(MockitoExtension.class)
class ClientRemovalServiceTest {

@Mock
private ClientRepository clientRepository;
@InjectMocks
private ClientRemovalService service;

@Test
void givenClientIdWhenRevokeClientThenVerifyRevoke() {
// Given
String organizationIpaCode = "organizationIpaCode";
String clientId = "clientId";
//When
service.revokeClient(organizationIpaCode, clientId);
//Then
Mockito.verify(clientRepository).deleteByClientIdAndOrganizationIpaCode(clientId, organizationIpaCode);
}
}