From 64a1667fd2f78460a17f854884a0b582134ed4c7 Mon Sep 17 00:00:00 2001 From: antonioT90 <34568575+antonioT90@users.noreply.github.com> Date: Mon, 1 Jul 2024 12:44:28 +0200 Subject: [PATCH] fix: P4ADEV-639 temporarily storing User PII (#41) * P4ADEV-639 temporarily storing User PII * P4ADEV-639 temporarily storing User PII * fix code smell on tests --- .../gov/pagopa/payhub/auth/dto/IamUserInfoDTO.java | 1 + .../java/it/gov/pagopa/payhub/auth/model/User.java | 6 ++++++ .../auth/repository/UsersRepositoryExtImpl.java | 11 ++++++++--- .../exchange/IDTokenClaims2UserInfoMapper.java | 1 + .../service/exchange/IamUserRegistrationService.java | 2 +- .../pagopa/payhub/auth/service/user/UserService.java | 2 +- .../payhub/auth/service/user/UserServiceImpl.java | 4 ++-- .../user/registration/UserRegistrationService.java | 12 +++++++++--- .../pagopa/payhub/auth/config/RedisConfigTest.java | 4 ++-- .../auth/repository/UsersRepositoryExtImplTest.java | 12 ++++++++++-- .../exchange/AccessTokenBuilderServiceTest.java | 4 ++-- .../exchange/IDTokenClaims2UserInfoMapperTest.java | 2 ++ .../exchange/IamUserRegistrationServiceTest.java | 7 +++++-- .../payhub/auth/service/user/UserServiceTest.java | 7 +++++-- .../registration/UserRegistrationServiceTest.java | 9 ++++++++- 15 files changed, 63 insertions(+), 21 deletions(-) diff --git a/src/main/java/it/gov/pagopa/payhub/auth/dto/IamUserInfoDTO.java b/src/main/java/it/gov/pagopa/payhub/auth/dto/IamUserInfoDTO.java index 85138961..80d4b75e 100644 --- a/src/main/java/it/gov/pagopa/payhub/auth/dto/IamUserInfoDTO.java +++ b/src/main/java/it/gov/pagopa/payhub/auth/dto/IamUserInfoDTO.java @@ -15,6 +15,7 @@ public class IamUserInfoDTO { private String fiscalCode; private String familyName; private String name; + private String email; private String issuer; private IamUserOrganizationRolesDTO organizationAccess; diff --git a/src/main/java/it/gov/pagopa/payhub/auth/model/User.java b/src/main/java/it/gov/pagopa/payhub/auth/model/User.java index 7977a166..fb965624 100644 --- a/src/main/java/it/gov/pagopa/payhub/auth/model/User.java +++ b/src/main/java/it/gov/pagopa/payhub/auth/model/User.java @@ -26,4 +26,10 @@ public class User { private LocalDateTime lastLogin; private boolean tosAccepted; +//region PII temporarily stored + private String fiscalCode; + private String firstName; + private String lastName; + private String email; +//endregion } diff --git a/src/main/java/it/gov/pagopa/payhub/auth/repository/UsersRepositoryExtImpl.java b/src/main/java/it/gov/pagopa/payhub/auth/repository/UsersRepositoryExtImpl.java index 1cc57a0d..dff8e3ed 100644 --- a/src/main/java/it/gov/pagopa/payhub/auth/repository/UsersRepositoryExtImpl.java +++ b/src/main/java/it/gov/pagopa/payhub/auth/repository/UsersRepositoryExtImpl.java @@ -22,10 +22,15 @@ public User registerUser(User user) { return mongoTemplate.findAndModify( Query.query(Criteria.where(User.Fields.mappedExternalUserId).is(user.getMappedExternalUserId())), new Update() - .setOnInsert(User.Fields.userCode, user.getUserCode()) - .setOnInsert(User.Fields.iamIssuer, user.getIamIssuer()) + .set(User.Fields.userCode, user.getUserCode()) + .set(User.Fields.iamIssuer, user.getIamIssuer()) .setOnInsert(User.Fields.tosAccepted, false) - .set(User.Fields.lastLogin, LocalDateTime.now()), + .set(User.Fields.lastLogin, LocalDateTime.now()) + + .set(User.Fields.fiscalCode, user.getFiscalCode()) + .set(User.Fields.firstName, user.getFirstName()) + .set(User.Fields.lastName, user.getLastName()) + .set(User.Fields.email, user.getEmail()), FindAndModifyOptions.options() .returnNew(true) .upsert(true), diff --git a/src/main/java/it/gov/pagopa/payhub/auth/service/exchange/IDTokenClaims2UserInfoMapper.java b/src/main/java/it/gov/pagopa/payhub/auth/service/exchange/IDTokenClaims2UserInfoMapper.java index b51a1b57..14439b5c 100644 --- a/src/main/java/it/gov/pagopa/payhub/auth/service/exchange/IDTokenClaims2UserInfoMapper.java +++ b/src/main/java/it/gov/pagopa/payhub/auth/service/exchange/IDTokenClaims2UserInfoMapper.java @@ -32,6 +32,7 @@ public IamUserInfoDTO apply(Map claims) { .userId(claims.get("uid").asString()) .name(claims.get("name").asString()) .familyName(claims.get("family_name").asString()) + .email(claims.get("email").asString()) .fiscalCode(claims.get("fiscal_number").asString()) .organizationAccess(buildUserOrganizationRoles(claims)) .build(); diff --git a/src/main/java/it/gov/pagopa/payhub/auth/service/exchange/IamUserRegistrationService.java b/src/main/java/it/gov/pagopa/payhub/auth/service/exchange/IamUserRegistrationService.java index 3e34ee3d..77b14ad4 100644 --- a/src/main/java/it/gov/pagopa/payhub/auth/service/exchange/IamUserRegistrationService.java +++ b/src/main/java/it/gov/pagopa/payhub/auth/service/exchange/IamUserRegistrationService.java @@ -27,7 +27,7 @@ public IamUserRegistrationService( } User registerUser(IamUserInfoDTO userInfo) { - User user = userService.registerUser(userInfo.getUserId(), userInfo.getFiscalCode(), userInfo.getIssuer()); + User user = userService.registerUser(userInfo.getUserId(), userInfo.getFiscalCode(), userInfo.getIssuer(), userInfo.getName(), userInfo.getFamilyName(), userInfo.getEmail()); if (organizationAccessMode) { if (CollectionUtils.isEmpty(userInfo.getOrganizationAccess().getRoles())) { diff --git a/src/main/java/it/gov/pagopa/payhub/auth/service/user/UserService.java b/src/main/java/it/gov/pagopa/payhub/auth/service/user/UserService.java index cf914d2b..1441af29 100644 --- a/src/main/java/it/gov/pagopa/payhub/auth/service/user/UserService.java +++ b/src/main/java/it/gov/pagopa/payhub/auth/service/user/UserService.java @@ -7,7 +7,7 @@ import java.util.Set; public interface UserService { - User registerUser(String externalUserId, String fiscalCode, String iamIssuer); + User registerUser(String externalUserId, String fiscalCode, String iamIssuer, String firstName, String lastName, String email); Operator registerOperator(String userId, String organizationIpaCode, Set roles); UserInfo getUserInfo(String accessToken); } diff --git a/src/main/java/it/gov/pagopa/payhub/auth/service/user/UserServiceImpl.java b/src/main/java/it/gov/pagopa/payhub/auth/service/user/UserServiceImpl.java index cdb70b92..b61cbd78 100644 --- a/src/main/java/it/gov/pagopa/payhub/auth/service/user/UserServiceImpl.java +++ b/src/main/java/it/gov/pagopa/payhub/auth/service/user/UserServiceImpl.java @@ -30,8 +30,8 @@ public UserServiceImpl(TokenStoreService tokenStoreService, UserRegistrationServ } @Override - public User registerUser(String externalUserId, String fiscalCode, String iamIssuer) { - return userRegistrationService.registerUser(externalUserId, fiscalCode, iamIssuer); + public User registerUser(String externalUserId, String fiscalCode, String iamIssuer, String firstName, String lastName, String email) { + return userRegistrationService.registerUser(externalUserId, fiscalCode, iamIssuer, firstName, lastName, email); } @Override diff --git a/src/main/java/it/gov/pagopa/payhub/auth/service/user/registration/UserRegistrationService.java b/src/main/java/it/gov/pagopa/payhub/auth/service/user/registration/UserRegistrationService.java index f8a26cfe..095cf4e6 100644 --- a/src/main/java/it/gov/pagopa/payhub/auth/service/user/registration/UserRegistrationService.java +++ b/src/main/java/it/gov/pagopa/payhub/auth/service/user/registration/UserRegistrationService.java @@ -19,17 +19,23 @@ public UserRegistrationService(ExternalUserIdObfuscatorService externalUserIdObf this.usersRepository = usersRepository; } - public User registerUser(String externalUserId, String fiscalCode, String iamIssuer){ - User user = buildUser(externalUserId, fiscalCode, iamIssuer); + public User registerUser(String externalUserId, String fiscalCode, String iamIssuer, String firstName, String lastName, String email){ + User user = buildUser(externalUserId, fiscalCode, iamIssuer, firstName, lastName, email); log.info("Registering user having mappedExternalUserId {}", user.getMappedExternalUserId()); return usersRepository.registerUser(user); } - private User buildUser(String externalUserId, String fiscalCode, String iamIssuer){ + private User buildUser(String externalUserId, String fiscalCode, String iamIssuer, String firstName, String lastName, String email){ return User.builder() .iamIssuer(iamIssuer) .mappedExternalUserId(externalUserIdObfuscatorService.obfuscate(externalUserId)) .userCode(fiscalCodeObfuscatorService.obfuscate(fiscalCode)) + + .fiscalCode(fiscalCode) + .firstName(firstName) + .lastName(lastName) + .email(email) + .build(); } } diff --git a/src/test/java/it/gov/pagopa/payhub/auth/config/RedisConfigTest.java b/src/test/java/it/gov/pagopa/payhub/auth/config/RedisConfigTest.java index 173af1cf..0addbd5c 100644 --- a/src/test/java/it/gov/pagopa/payhub/auth/config/RedisConfigTest.java +++ b/src/test/java/it/gov/pagopa/payhub/auth/config/RedisConfigTest.java @@ -9,8 +9,8 @@ class RedisConfigTest { - private final static ObjectMapper objectMapper = new ObjectMapper(); - private final static RedisConfig redisConfig = new RedisConfig(); + private static final ObjectMapper objectMapper = new ObjectMapper(); + private static final RedisConfig redisConfig = new RedisConfig(); @Test void testCustomizer(){ diff --git a/src/test/java/it/gov/pagopa/payhub/auth/repository/UsersRepositoryExtImplTest.java b/src/test/java/it/gov/pagopa/payhub/auth/repository/UsersRepositoryExtImplTest.java index ace05ddd..12ae9663 100644 --- a/src/test/java/it/gov/pagopa/payhub/auth/repository/UsersRepositoryExtImplTest.java +++ b/src/test/java/it/gov/pagopa/payhub/auth/repository/UsersRepositoryExtImplTest.java @@ -42,6 +42,10 @@ void whenRegisterUserThenReturnStoredUser() { .iamIssuer("IAMISSUER") .mappedExternalUserId("MAPPEDEXTERNALUSERID") .userCode("USERCODE") + .firstName("FIRSTNAME") + .lastName("LASTNAME") + .fiscalCode("FISCALCODE") + .email("EMAIL") .build(); User storedUser = new User(); @@ -54,10 +58,14 @@ void whenRegisterUserThenReturnStoredUser() { Assertions.assertTrue(lastLogin.isAfter(LocalDateTime.now().minusMinutes(1))); return u.equals(new Update() - .setOnInsert(User.Fields.userCode, user.getUserCode()) - .setOnInsert(User.Fields.iamIssuer, user.getIamIssuer()) + .set(User.Fields.userCode, user.getUserCode()) + .set(User.Fields.iamIssuer, user.getIamIssuer()) .setOnInsert(User.Fields.tosAccepted, false) .set(User.Fields.lastLogin, lastLogin) + .set(User.Fields.firstName, user.getFirstName()) + .set(User.Fields.lastName, user.getLastName()) + .set(User.Fields.email, user.getEmail()) + .set(User.Fields.fiscalCode, user.getFiscalCode()) ); }), Mockito.argThat(opt -> opt.isReturnNew() && opt.isUpsert() && !opt.isRemove()), diff --git a/src/test/java/it/gov/pagopa/payhub/auth/service/exchange/AccessTokenBuilderServiceTest.java b/src/test/java/it/gov/pagopa/payhub/auth/service/exchange/AccessTokenBuilderServiceTest.java index 88665644..d5ad2858 100644 --- a/src/test/java/it/gov/pagopa/payhub/auth/service/exchange/AccessTokenBuilderServiceTest.java +++ b/src/test/java/it/gov/pagopa/payhub/auth/service/exchange/AccessTokenBuilderServiceTest.java @@ -14,7 +14,7 @@ public class AccessTokenBuilderServiceTest { public static final int EXPIRE_IN = 3600; - private final static String PRIVATE_KEY= """ + private static final String PRIVATE_KEY= """ -----BEGIN RSA PRIVATE KEY----- MIIEogIBAAKCAQEA2ovm/rd3g69dq9PisinQ6mWy8ZttT8D+GKXCsHZycsGnN7b7 4TPyYy+4+h+9cgJeizp8RDRrufHjiBrqi/2reOk/rD7ZHbpfQvHK8MYfgIVdtTxY @@ -44,7 +44,7 @@ public class AccessTokenBuilderServiceTest { -----END RSA PRIVATE KEY----- """; - private final static String PUBLIC_KEY= """ + private static final String PUBLIC_KEY= """ -----BEGIN PUBLIC KEY----- MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA2ovm/rd3g69dq9PisinQ 6mWy8ZttT8D+GKXCsHZycsGnN7b74TPyYy+4+h+9cgJeizp8RDRrufHjiBrqi/2r diff --git a/src/test/java/it/gov/pagopa/payhub/auth/service/exchange/IDTokenClaims2UserInfoMapperTest.java b/src/test/java/it/gov/pagopa/payhub/auth/service/exchange/IDTokenClaims2UserInfoMapperTest.java index 19cb079b..fd0817e3 100644 --- a/src/test/java/it/gov/pagopa/payhub/auth/service/exchange/IDTokenClaims2UserInfoMapperTest.java +++ b/src/test/java/it/gov/pagopa/payhub/auth/service/exchange/IDTokenClaims2UserInfoMapperTest.java @@ -39,6 +39,7 @@ void givenIDTokenClaimsThenOk(boolean isOrganizationAccess) { .userId("e1d9c534-86a9-4039-80da-8aa7a33ac9e7") .name("demo") .familyName("demosurname") + .email("ee@ee.it") .fiscalCode("DMEMPY15L21L736U") .issuer("https://dev.selfcare.pagopa.it") .organizationAccess(IamUserOrganizationRolesDTO.builder() @@ -74,6 +75,7 @@ void givenIDTokenNoOrganizationAndNoOrganizationAccessModeThenOk() { .name("demo") .familyName("demosurname") .fiscalCode("DMEMPY15L21L736U") + .email("ee@ee.it") .issuer("https://dev.selfcare.pagopa.it") .build(); Map claims = JWT.decode("eyJraWQiOiJqd3QtZXhjaGFuZ2VfZTA6OTQ6M2Q6NWI6YWY6ODY6YWU6YWM6YzM6ZGI6OWM6MzI6NTc6NWE6YTA6NDciLCJ0eXAiOiJKV1QiLCJhbGciOiJSUzI1NiJ9.eyJ0eXAiOiJJRCIsImZhbWlseV9uYW1lIjoiZGVtb3N1cm5hbWUiLCJmaXNjYWxfbnVtYmVyIjoiRE1FTVBZMTVMMjFMNzM2VSIsIm5hbWUiOiJkZW1vIiwic3BpZF9sZXZlbCI6Imh0dHBzOi8vd3d3LnNwaWQuZ292Lml0L1NwaWRMMiIsImZyb21fYWEiOmZhbHNlLCJzdWIiOiJlMWQ5YzUzNC04NmE5LTQwMzktODBkYS04YWE3YTMzYWM5ZTciLCJ1aWQiOiJlMWQ5YzUzNC04NmE5LTQwMzktODBkYS04YWE3YTMzYWM5ZTciLCJsZXZlbCI6IkwyIiwiaWF0IjoxNzE0OTgwNTY4LCJleHAiOjU3MTQ5ODE0NjgsImF1ZCI6ImRldi5waWF0dGFmb3JtYXVuaXRhcmlhLnBhZ29wYS5pdCIsImlzcyI6Imh0dHBzOi8vZGV2LnNlbGZjYXJlLnBhZ29wYS5pdCIsImp0aSI6IjkyYzQ3OWI1LTUxMTUtNGQzMS1iMDliLTVjZmFmNTQ1Mzc3NCIsImVtYWlsIjoiZWVAZWUuaXQiLCJkZXNpcmVkX2V4cCI6NTcxNTAxMjg5NH0.aJIhnNEOXNx8wDTSmAVv7NJio_J9muU2Jq9bY3AX0rL09LIvoGoxH1-u2-gdz_2LpxfZjluS0PUNKvAVgAbtK7o_vNPnThbCaQjVvtBPPBSWcjXSxbi7uJ9r89CMyu1CGim5tXz9cPB9vTBsdPElQ4xGnWGUBl8nYvUZ9oayYxDpPBdEjbHUEAvjjiaEHFjt5bmzYeyOnh4g_qe6m_j6JzsijRNXh87Ple4Awb72CPaf4gwTXIPQqoHIISSKfBYOIU_zl5mg5e-p3pTdsNNBR30vKViiolde4EIuY33IB5hSAjk5Pvw_5TD26sEI581Zra9ccaUsoPW2watGAFa6kg").getClaims(); diff --git a/src/test/java/it/gov/pagopa/payhub/auth/service/exchange/IamUserRegistrationServiceTest.java b/src/test/java/it/gov/pagopa/payhub/auth/service/exchange/IamUserRegistrationServiceTest.java index ac766eea..f27d1738 100644 --- a/src/test/java/it/gov/pagopa/payhub/auth/service/exchange/IamUserRegistrationServiceTest.java +++ b/src/test/java/it/gov/pagopa/payhub/auth/service/exchange/IamUserRegistrationServiceTest.java @@ -93,7 +93,7 @@ void givenInvalidOrganizationAccessModeWhenRegisterUserThenThrowInvalidOrganizat } private void verifyRegisterUserInvocation(IamUserInfoDTO userInfo) { - Mockito.verify(userServiceMock).registerUser(userInfo.getUserId(), userInfo.getFiscalCode(), userInfo.getIssuer()); + Mockito.verify(userServiceMock).registerUser(userInfo.getUserId(), userInfo.getFiscalCode(), userInfo.getIssuer(), userInfo.getName(), userInfo.getFamilyName(), userInfo.getEmail()); } private void verifyRegisterOperatorInvocation(User user, String organizationIpaCode, Set roles) { @@ -104,6 +104,9 @@ private Pair configureUserServiceMock() { IamUserInfoDTO userInfo = IamUserInfoDTO.builder() .userId("EXTERNALUSERID") .fiscalCode("FISCALCODE") + .name("NAME") + .familyName("FAMILYNAME") + .email("EMAIL") .issuer("IAMISSUER") .organizationAccess(IamUserOrganizationRolesDTO.builder() .organizationIpaCode("ORG") @@ -114,7 +117,7 @@ private Pair configureUserServiceMock() { User registeredUser = User.builder() .userId("INTERNALID") .build(); - Mockito.when(userServiceMock.registerUser(userInfo.getUserId(), userInfo.getFiscalCode(), userInfo.getIssuer())) + Mockito.when(userServiceMock.registerUser(userInfo.getUserId(), userInfo.getFiscalCode(), userInfo.getIssuer(), userInfo.getName(), userInfo.getFamilyName(), userInfo.getEmail())) .thenReturn(registeredUser); return Pair.of(userInfo, registeredUser); diff --git a/src/test/java/it/gov/pagopa/payhub/auth/service/user/UserServiceTest.java b/src/test/java/it/gov/pagopa/payhub/auth/service/user/UserServiceTest.java index 2f7605ad..67426b65 100644 --- a/src/test/java/it/gov/pagopa/payhub/auth/service/user/UserServiceTest.java +++ b/src/test/java/it/gov/pagopa/payhub/auth/service/user/UserServiceTest.java @@ -81,13 +81,16 @@ void whenRegisterUserThenReturnStoredUser() { String externalUserId = "EXTERNALUSERID"; String fiscalCode = "FISCALCODE"; String iamIssuer = "IAMISSUER"; + String name = "NAME"; + String familyName = "FAMILYNAME"; + String email = "EMAIL"; User storedUser = new User(); - Mockito.when(userRegistrationServiceMock.registerUser(externalUserId, fiscalCode, iamIssuer)) + Mockito.when(userRegistrationServiceMock.registerUser(externalUserId, fiscalCode, iamIssuer, name, familyName, email)) .thenReturn(storedUser); // When - User result = service.registerUser(externalUserId, fiscalCode, iamIssuer); + User result = service.registerUser(externalUserId, fiscalCode, iamIssuer, name, familyName, email); // Then Assertions.assertSame(storedUser, result); diff --git a/src/test/java/it/gov/pagopa/payhub/auth/service/user/registration/UserRegistrationServiceTest.java b/src/test/java/it/gov/pagopa/payhub/auth/service/user/registration/UserRegistrationServiceTest.java index b87c4142..1ea625fd 100644 --- a/src/test/java/it/gov/pagopa/payhub/auth/service/user/registration/UserRegistrationServiceTest.java +++ b/src/test/java/it/gov/pagopa/payhub/auth/service/user/registration/UserRegistrationServiceTest.java @@ -47,11 +47,18 @@ void whenRegisterUserThenReturnStoredUser(){ String fiscalCode = "FISCALCODE"; String obfuscatedFiscalCode = "OBFUSCATEDFISCALCODE"; String iamIssuer = "IAMISSUER"; + String name = "NAME"; + String familyName = "FAMILYNAME"; + String email = "EMAIL"; User user = User.builder() .mappedExternalUserId(obfuscatedExternalUserId) .userCode(obfuscatedFiscalCode) .iamIssuer(iamIssuer) + .fiscalCode(fiscalCode) + .firstName(name) + .lastName(familyName) + .email(email) .build(); User storedUser = new User(); @@ -60,7 +67,7 @@ void whenRegisterUserThenReturnStoredUser(){ Mockito.when(usersRepositoryMock.registerUser(user)).thenReturn(storedUser); // When - User result = service.registerUser(externalUserId, fiscalCode, iamIssuer); + User result = service.registerUser(externalUserId, fiscalCode, iamIssuer, name, familyName, email); // Then Assertions.assertSame(storedUser, result);