Skip to content

Commit

Permalink
fix: P4ADEV-639 temporarily storing User PII (#41)
Browse files Browse the repository at this point in the history
* P4ADEV-639 temporarily storing User PII

* P4ADEV-639 temporarily storing User PII

* fix code smell on tests
  • Loading branch information
antonioT90 authored Jul 1, 2024
1 parent 75166d8 commit 64a1667
Show file tree
Hide file tree
Showing 15 changed files with 63 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
6 changes: 6 additions & 0 deletions src/main/java/it/gov/pagopa/payhub/auth/model/User.java
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ public IamUserInfoDTO apply(Map<String, Claim> 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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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())) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<String> roles);
UserInfo getUserInfo(String accessToken);
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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(){
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand All @@ -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()),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ void givenIDTokenClaimsThenOk(boolean isOrganizationAccess) {
.userId("e1d9c534-86a9-4039-80da-8aa7a33ac9e7")
.name("demo")
.familyName("demosurname")
.email("[email protected]")
.fiscalCode("DMEMPY15L21L736U")
.issuer("https://dev.selfcare.pagopa.it")
.organizationAccess(IamUserOrganizationRolesDTO.builder()
Expand Down Expand Up @@ -74,6 +75,7 @@ void givenIDTokenNoOrganizationAndNoOrganizationAccessModeThenOk() {
.name("demo")
.familyName("demosurname")
.fiscalCode("DMEMPY15L21L736U")
.email("[email protected]")
.issuer("https://dev.selfcare.pagopa.it")
.build();
Map<String, Claim> claims = JWT.decode("eyJraWQiOiJqd3QtZXhjaGFuZ2VfZTA6OTQ6M2Q6NWI6YWY6ODY6YWU6YWM6YzM6ZGI6OWM6MzI6NTc6NWE6YTA6NDciLCJ0eXAiOiJKV1QiLCJhbGciOiJSUzI1NiJ9.eyJ0eXAiOiJJRCIsImZhbWlseV9uYW1lIjoiZGVtb3N1cm5hbWUiLCJmaXNjYWxfbnVtYmVyIjoiRE1FTVBZMTVMMjFMNzM2VSIsIm5hbWUiOiJkZW1vIiwic3BpZF9sZXZlbCI6Imh0dHBzOi8vd3d3LnNwaWQuZ292Lml0L1NwaWRMMiIsImZyb21fYWEiOmZhbHNlLCJzdWIiOiJlMWQ5YzUzNC04NmE5LTQwMzktODBkYS04YWE3YTMzYWM5ZTciLCJ1aWQiOiJlMWQ5YzUzNC04NmE5LTQwMzktODBkYS04YWE3YTMzYWM5ZTciLCJsZXZlbCI6IkwyIiwiaWF0IjoxNzE0OTgwNTY4LCJleHAiOjU3MTQ5ODE0NjgsImF1ZCI6ImRldi5waWF0dGFmb3JtYXVuaXRhcmlhLnBhZ29wYS5pdCIsImlzcyI6Imh0dHBzOi8vZGV2LnNlbGZjYXJlLnBhZ29wYS5pdCIsImp0aSI6IjkyYzQ3OWI1LTUxMTUtNGQzMS1iMDliLTVjZmFmNTQ1Mzc3NCIsImVtYWlsIjoiZWVAZWUuaXQiLCJkZXNpcmVkX2V4cCI6NTcxNTAxMjg5NH0.aJIhnNEOXNx8wDTSmAVv7NJio_J9muU2Jq9bY3AX0rL09LIvoGoxH1-u2-gdz_2LpxfZjluS0PUNKvAVgAbtK7o_vNPnThbCaQjVvtBPPBSWcjXSxbi7uJ9r89CMyu1CGim5tXz9cPB9vTBsdPElQ4xGnWGUBl8nYvUZ9oayYxDpPBdEjbHUEAvjjiaEHFjt5bmzYeyOnh4g_qe6m_j6JzsijRNXh87Ple4Awb72CPaf4gwTXIPQqoHIISSKfBYOIU_zl5mg5e-p3pTdsNNBR30vKViiolde4EIuY33IB5hSAjk5Pvw_5TD26sEI581Zra9ccaUsoPW2watGAFa6kg").getClaims();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<String> roles) {
Expand All @@ -104,6 +104,9 @@ private Pair<IamUserInfoDTO, User> configureUserServiceMock() {
IamUserInfoDTO userInfo = IamUserInfoDTO.builder()
.userId("EXTERNALUSERID")
.fiscalCode("FISCALCODE")
.name("NAME")
.familyName("FAMILYNAME")
.email("EMAIL")
.issuer("IAMISSUER")
.organizationAccess(IamUserOrganizationRolesDTO.builder()
.organizationIpaCode("ORG")
Expand All @@ -114,7 +117,7 @@ private Pair<IamUserInfoDTO, User> 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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand All @@ -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);
Expand Down

0 comments on commit 64a1667

Please sign in to comment.