From 4b70e62296c406732065d91a16f35ea90b1ca067 Mon Sep 17 00:00:00 2001 From: Stoyan Tzenkov Date: Mon, 21 Jan 2019 16:57:50 +0200 Subject: [PATCH 01/20] NY-6574: Got rid of warning message. Signed-off-by: Stoyan Tzenkov --- .../AccountRepositoryAdditionalImpl.java | 38 ++++++++++++------- 1 file changed, 25 insertions(+), 13 deletions(-) diff --git a/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java b/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java index d7bffb5..3542ea2 100644 --- a/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java +++ b/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java @@ -10,11 +10,15 @@ import java.time.Instant; import java.time.LocalDate; import java.util.HashSet; import java.util.List; +import java.util.Map; import java.util.Optional; import java.util.Set; import java.util.UUID; +import java.util.concurrent.ConcurrentHashMap; import java.util.stream.Collectors; +import javax.annotation.PostConstruct; + import org.apache.commons.lang3.SerializationUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -22,10 +26,12 @@ import org.springframework.data.cassandra.core.CassandraBatchOperations; import org.springframework.data.cassandra.core.CassandraTemplate; import org.springframework.data.cassandra.core.UpdateOptions; import org.springframework.data.cassandra.core.WriteResult; +import org.springframework.data.cassandra.core.cql.CqlOperations; import org.springframework.stereotype.Service; import com.datastax.driver.core.BatchStatement; import com.datastax.driver.core.BoundStatement; +import com.datastax.driver.core.PreparedStatement; import com.datastax.driver.core.ResultSet; import com.datastax.driver.core.Session; @@ -56,9 +62,9 @@ import biz.nynja.account.services.decomposition.IncorrectAccountCountException; import biz.nynja.account.services.erlang.ErlangAccountBridge; import io.grpc.StatusRuntimeException; -// TODO: 11/19/2018 refactor this class and adding rolback cassandra data if erlang return fail state @Service public class AccountRepositoryAdditionalImpl implements AccountRepositoryAdditional { + private PreparedStatement preparedCql; private static final Logger logger = LoggerFactory.getLogger(AccountRepositoryAdditionalImpl.class); private final PendingAccountConfiguration pendingAccountConfiguration; @@ -106,6 +112,15 @@ public class AccountRepositoryAdditionalImpl implements AccountRepositoryAdditio this.accountDataConfiguration = accountDataConfiguration; } + @PostConstruct + public void initStatements() { + + preparedCql = session.prepare( + "INSERT INTO pendingAccount (accountId, profileId, authenticationProvider, authenticationProviderType, creationTimestamp)" + + " VALUES (?, ?, ? ,?, ?) " + "USING TTL " + pendingAccountConfiguration.getTtl() * 60 + ";"); + + } + public Account completePendingAccountCreation(CompletePendingAccountCreationRequest request) { Transaction sagaTransaction = new SagaTransaction(cassandraTemplate); PendingAccount pendingAccount = pendingAccountRepository @@ -198,8 +213,8 @@ public class AccountRepositoryAdditionalImpl implements AccountRepositoryAdditio public Account updateAccount(UpdateAccountRequest request) { Transaction sagaTransaction = new SagaTransaction(cassandraTemplate); - Account existingAccount = accountRepository.findByAccountId(UUID.fromString(request.getAccountId())); + Account existingAccount = accountRepository.findByAccountId(UUID.fromString(request.getAccountId())); if (existingAccount == null) { logger.error("Existing account with the provided id {} was not found.", request.getAccountId()); logger.debug("Existing account with the provided id {} was not found.", request.getAccountId()); @@ -211,7 +226,6 @@ public class AccountRepositoryAdditionalImpl implements AccountRepositoryAdditio Set roles = existingAccount.getRoles().stream().map(Role::valueOf).collect(Collectors.toSet()); request = UpdateAccountRequest.newBuilder(request).clearRoles().addAllRoles(roles).build(); } - Long timeUpdated = Instant.now().toEpochMilli(); WriteResult wr = null; try { @@ -998,20 +1012,18 @@ public class AccountRepositoryAdditionalImpl implements AccountRepositoryAdditio return false; } - public PendingAccount savePendingAccount(PendingAccount pendingAccount) { + public PendingAccount savePendingAccount(PendingAccount pendingAccount) throws RuntimeException { // for saving back compatibility. need refactoring in the future - if (!createSavePendingAccount(pendingAccount)) { - throw new RuntimeException("Exception for save save pending account id=" + pendingAccount.getAccountId()); - } - return pendingAccount; - } - private boolean createSavePendingAccount(PendingAccount pendingAccount) { - String cql = "INSERT INTO pendingAccount (accountId, profileId, authenticationProvider, authenticationProviderType, creationTimestamp)" - + " VALUES (?, ?, ? ,?, ?) " + "USING TTL " + pendingAccountConfiguration.getTtl() * 60 + ";"; - return cassandraTemplate.getCqlOperations().execute(cql, pendingAccount.getAccountId(), + BoundStatement boundStatement = new BoundStatement(preparedCql).bind(pendingAccount.getAccountId(), pendingAccount.getProfileId(), pendingAccount.getAuthenticationProvider(), pendingAccount.getAuthenticationProviderType(), pendingAccount.getCreationTimestamp()); + try { + ResultSet result = session.execute(boundStatement); + } catch (Exception ex) { + throw new RuntimeException("Exception for save save pending account id=" + pendingAccount.getAccountId()); + } + return pendingAccount; } public void removeNullsForSearchableOption() { -- GitLab From 8dde85a24ad983eafde79802f1ab24a8976f8376 Mon Sep 17 00:00:00 2001 From: Stoyan Tzenkov Date: Wed, 23 Jan 2019 10:21:13 +0200 Subject: [PATCH 02/20] NY-6574: Small message fix. Signed-off-by: Stoyan Tzenkov --- .../account/repositories/AccountRepositoryAdditionalImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java b/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java index 3542ea2..1737eff 100644 --- a/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java +++ b/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java @@ -1021,7 +1021,7 @@ public class AccountRepositoryAdditionalImpl implements AccountRepositoryAdditio try { ResultSet result = session.execute(boundStatement); } catch (Exception ex) { - throw new RuntimeException("Exception for save save pending account id=" + pendingAccount.getAccountId()); + throw new RuntimeException("Exception during save pending account id=" + pendingAccount.getAccountId()); } return pendingAccount; } -- GitLab From 4940f27c40d0cad32f7db2dfbb8b49f52196c6e9 Mon Sep 17 00:00:00 2001 From: Dragomir Todorov Date: Wed, 23 Jan 2019 13:21:52 +0200 Subject: [PATCH 03/20] NY-6523: Removed the requirement to have country code --- .../account/phone/PhoneNumberNormalizer.java | 10 +++--- .../nynja/account/validation/Validators.java | 36 +++++++++++++------ .../account/services/AccountServiceTests.java | 4 +-- 3 files changed, 33 insertions(+), 17 deletions(-) diff --git a/src/main/java/biz/nynja/account/phone/PhoneNumberNormalizer.java b/src/main/java/biz/nynja/account/phone/PhoneNumberNormalizer.java index d5834b4..6c09984 100644 --- a/src/main/java/biz/nynja/account/phone/PhoneNumberNormalizer.java +++ b/src/main/java/biz/nynja/account/phone/PhoneNumberNormalizer.java @@ -28,7 +28,7 @@ public class PhoneNumberNormalizer { public AddContactInfoRequest normalizePhoneNumber(AddContactInfoRequest request) { // Get the normalized phone number from libphone ContactDetails newContactDetails = ContactDetails.newBuilder().setType(request.getContactInfo().getType()) - .setValue(getNormalizedPhoneNumberContactInfo(request.getContactInfo().getValue())) + .setValue(getNormalizedPhoneNumberWithSelector(request.getContactInfo().getValue())) .setLabel(request.getContactInfo().getLabel()).build(); AddContactInfoRequest newRequest = AddContactInfoRequest.newBuilder().setAccountId(request.getAccountId()) .setContactInfo(newContactDetails).build(); @@ -39,7 +39,7 @@ public class PhoneNumberNormalizer { public DeleteContactInfoRequest normalizePhoneNumber(DeleteContactInfoRequest request) { // Get the normalized phone number from libphone ContactDetails newContactDetails = ContactDetails.newBuilder().setType(request.getContactInfo().getType()) - .setValue(getNormalizedPhoneNumberContactInfo(request.getContactInfo().getValue())) + .setValue(getNormalizedPhoneNumberWithSelector(request.getContactInfo().getValue())) .setLabel(request.getContactInfo().getLabel()).build(); DeleteContactInfoRequest newRequest = DeleteContactInfoRequest.newBuilder().setAccountId(request.getAccountId()) .setContactInfo(newContactDetails).build(); @@ -51,12 +51,12 @@ public class PhoneNumberNormalizer { // Get the normalized phone number from libphone for the old number ContactDetails contactDetailsOldNumber = ContactDetails.newBuilder() .setType(request.getOldContactInfo().getType()) - .setValue(getNormalizedPhoneNumberContactInfo(request.getOldContactInfo().getValue())) + .setValue(getNormalizedPhoneNumberWithSelector(request.getOldContactInfo().getValue())) .setLabel(request.getOldContactInfo().getLabel()).build(); // Get the normalized phone number from libphone for the edited number ContactDetails contactDetailsEditedNumber = ContactDetails.newBuilder() .setType(request.getEditedContactInfo().getType()) - .setValue(getNormalizedPhoneNumberContactInfo(request.getEditedContactInfo().getValue())) + .setValue(getNormalizedPhoneNumberWithSelector(request.getEditedContactInfo().getValue())) .setLabel(request.getEditedContactInfo().getLabel()).build(); EditContactInfoRequest newRequest = EditContactInfoRequest.newBuilder().setAccountId(request.getAccountId()) .setOldContactInfo(contactDetailsOldNumber).setEditedContactInfo(contactDetailsEditedNumber).build(); @@ -135,7 +135,7 @@ public class PhoneNumberNormalizer { return normalizedPhoneNumber; } - public String getNormalizedPhoneNumberContactInfo(String rawPhoneNumber) throws InvalidPhoneNumberException { + public String getNormalizedPhoneNumberWithSelector(String rawPhoneNumber) throws InvalidPhoneNumberException { String normalizedPhoneNumber = getNormalizedPhoneNumber(rawPhoneNumber); String country; diff --git a/src/main/java/biz/nynja/account/validation/Validators.java b/src/main/java/biz/nynja/account/validation/Validators.java index 51ff766..61ca996 100644 --- a/src/main/java/biz/nynja/account/validation/Validators.java +++ b/src/main/java/biz/nynja/account/validation/Validators.java @@ -9,6 +9,7 @@ import java.util.Optional; import java.util.regex.Matcher; import java.util.regex.Pattern; +import org.apache.commons.lang3.StringUtils; import org.apache.commons.lang3.tuple.ImmutablePair; import org.apache.commons.validator.routines.EmailValidator; import org.slf4j.Logger; @@ -142,8 +143,8 @@ public class Validators { Validation validation = new Validation(); if (!isContactInfoPhoneLabelValid(oldPhoneNumberLabel)) { - validation.addError( - new ValidationError("Invalid old phone label: " + oldPhoneNumberLabel, Cause.INVALID_PHONE_LABEL)); + validation.addError(new ValidationError("Invalid old phone label: " + oldPhoneNumberLabel, + Cause.INVALID_PHONE_LABEL)); } if (!isContactInfoPhoneLabelValid(editedPhoneNumberLabel)) { validation.addError(new ValidationError("Invalid new phone label: " + editedPhoneNumberLabel, @@ -276,6 +277,11 @@ public class Validators { } public Optional validateAuthenticationProvider(AuthenticationType type, String authProvider) { + return validateAuthenticationProvider(type, authProvider, false); + } + + public Optional validateAuthenticationProvider(AuthenticationType type, String authProvider, + boolean isDeleteOperation) { if (authProvider == null || authProvider.trim().isEmpty()) { return Optional.of(Cause.MISSING_AUTH_PROVIDER_ID); } @@ -285,14 +291,24 @@ public class Validators { case PHONE: // We expect to receive phone number in the following format : // ":" - String[] provider = authProvider.split(":"); - if (provider == null || provider.length != 2) { - return Optional.of(Cause.INVALID_PHONENUMBER); + if(isDeleteOperation) { + if (StringUtils.isEmpty(authProvider)) { + return Optional.of(Cause.INVALID_PHONENUMBER); + } + if (!phoneValidator.isPhoneNumberValid(authProvider)) { + return Optional.of(Cause.INVALID_PHONENUMBER); + } + break; + } else { + String[] provider = authProvider.split(":"); + if (provider == null || provider.length != 2) { + return Optional.of(Cause.INVALID_PHONENUMBER); + } + if (!phoneValidator.isPhoneNumberValid(provider[1], provider[0])) { + return Optional.of(Cause.INVALID_PHONENUMBER); + } + break; } - if (!phoneValidator.isPhoneNumberValid(provider[1], provider[0])) { - return Optional.of(Cause.INVALID_PHONENUMBER); - } - break; case EMAIL: if (!util.isEmailValid(authProvider)) { return Optional.of(Cause.INVALID_EMAIL); @@ -320,7 +336,7 @@ public class Validators { return Optional.of(Cause.INVALID_PROFILE_ID); } return validateAuthenticationProvider(request.getAuthenticationProvider().getAuthenticationType(), - request.getAuthenticationProvider().getAuthenticationProvider()); + request.getAuthenticationProvider().getAuthenticationProvider(), true); } public Optional validateAddAuthenticationProviderRequest(AddAuthenticationProviderRequest request) { diff --git a/src/test/java/biz/nynja/account/services/AccountServiceTests.java b/src/test/java/biz/nynja/account/services/AccountServiceTests.java index 919803c..9526411 100644 --- a/src/test/java/biz/nynja/account/services/AccountServiceTests.java +++ b/src/test/java/biz/nynja/account/services/AccountServiceTests.java @@ -589,7 +589,7 @@ public class AccountServiceTests extends GrpcServerTestBase { verify(accountRepositoryAdditional).completePendingAccountCreation(argument.capture()); assertNotNull(argument.getValue().getRolesList()); assertNotNull(Role.USER.equals(argument.getValue().getRoles(0))); - assertNotNull(Role.USER.equals(argument.getValue().getRoles(1))); + assertEquals(argument.getAllValues().size(),1); } @Test @@ -975,7 +975,7 @@ public class AccountServiceTests extends GrpcServerTestBase { final DeleteAuthenticationProviderRequest request = DeleteAuthenticationProviderRequest.newBuilder() .setProfileId(Util.PROFILE_ID.toString()) .setAuthenticationProvider(AuthProviderDetails.newBuilder() - .setAuthenticationProvider(Util.PHONE_PROVIDER).setAuthenticationType(AuthenticationType.PHONE)) + .setAuthenticationProvider(Util.PHONE_NUMBER_STREIGHT).setAuthenticationType(AuthenticationType.PHONE)) .build(); given(profileRepository.findByProfileId(Util.PROFILE_ID)).willReturn(profile1AuthProviderNormalized); -- GitLab From 5a7091680cca97886bb490b938a59dd22ccaae14 Mon Sep 17 00:00:00 2001 From: Stoyan Tzenkov Date: Wed, 23 Jan 2019 12:57:05 +0200 Subject: [PATCH 04/20] NY-6436: Error log fixed. Signed-off-by: Stoyan Tzenkov --- .../repositories/AccountRepositoryAdditionalImpl.java | 6 +++++- .../java/biz/nynja/account/services/AccountServiceImpl.java | 3 ++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java b/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java index 1737eff..4077e46 100644 --- a/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java +++ b/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java @@ -945,9 +945,13 @@ public class AccountRepositoryAdditionalImpl implements AccountRepositoryAdditio ProfileByAuthenticationProvider profileByAuthProvider = profileByAuthenticationProviderRepository .findByAuthenticationProviderAndAuthenticationProviderType(loginOption.getValue(), loginOption.getType()); - if (profileByAuthProvider == null || !profileByAuthProvider.getSearchable().booleanValue()) { + if (profileByAuthProvider == null) { return Optional.empty(); } + if (!profileByAuthProvider.getSearchable().booleanValue()) { + throw new IncorrectAccountCountException("Authentication provider is not searchable for the profileId: " + + profileByAuthProvider.getProfileId()); + } List listAccountsByProfileId = accountByProfileIdRepository .findAllByProfileId(profileByAuthProvider.getProfileId()); diff --git a/src/main/java/biz/nynja/account/services/AccountServiceImpl.java b/src/main/java/biz/nynja/account/services/AccountServiceImpl.java index d33e624..cd5b80e 100644 --- a/src/main/java/biz/nynja/account/services/AccountServiceImpl.java +++ b/src/main/java/biz/nynja/account/services/AccountServiceImpl.java @@ -184,7 +184,8 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas account = accountProvider.searchAccountByLoginOption(AuthenticationType.EMAIL, request.getEmail()); } catch (IncorrectAccountCountException e) { logAndBuildGrpcSearchResponse(responseObserver, SearchResponse.newBuilder(), - "Error while searching for e-mail: ", request.getEmail(), Cause.INTERNAL_SERVER_ERROR, ""); + "Error while searching for e-mail. {}.", e.getMessage(), Cause.INTERNAL_SERVER_ERROR, ""); + return; } if (!account.isPresent()) { -- GitLab From 91ed21ac3923f7de2882fa7888849525bffab93e Mon Sep 17 00:00:00 2001 From: Stoyan Tzenkov Date: Wed, 23 Jan 2019 16:36:05 +0200 Subject: [PATCH 05/20] NY-6436: InternalError used instead of IncorrectAccountCountException. Signed-off-by: Stoyan Tzenkov --- .../account/repositories/AccountRepositoryAdditionalImpl.java | 4 ++-- .../java/biz/nynja/account/services/AccountServiceImpl.java | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java b/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java index 4077e46..7f51e10 100644 --- a/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java +++ b/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java @@ -949,8 +949,8 @@ public class AccountRepositoryAdditionalImpl implements AccountRepositoryAdditio return Optional.empty(); } if (!profileByAuthProvider.getSearchable().booleanValue()) { - throw new IncorrectAccountCountException("Authentication provider is not searchable for the profileId: " - + profileByAuthProvider.getProfileId()); + logger.error("Record found in DB but is not searchable for the profileId: {}", profileByAuthProvider.getProfileId()); + throw new InternalError("Record found in DB but is not searchable"); } List listAccountsByProfileId = accountByProfileIdRepository diff --git a/src/main/java/biz/nynja/account/services/AccountServiceImpl.java b/src/main/java/biz/nynja/account/services/AccountServiceImpl.java index cd5b80e..71e8dde 100644 --- a/src/main/java/biz/nynja/account/services/AccountServiceImpl.java +++ b/src/main/java/biz/nynja/account/services/AccountServiceImpl.java @@ -182,9 +182,9 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas Optional account = Optional.empty(); try { account = accountProvider.searchAccountByLoginOption(AuthenticationType.EMAIL, request.getEmail()); - } catch (IncorrectAccountCountException e) { + } catch (IncorrectAccountCountException | InternalError e) { logAndBuildGrpcSearchResponse(responseObserver, SearchResponse.newBuilder(), - "Error while searching for e-mail. {}.", e.getMessage(), Cause.INTERNAL_SERVER_ERROR, ""); + "Error while searching for e-mail. {}.", e.getMessage(), Cause.INTERNAL_SERVER_ERROR, "Error while searching for e-mail."); return; } -- GitLab From 9d174e46af915f58c2bd2b4c23098ef4dc1a378c Mon Sep 17 00:00:00 2001 From: Stoyan Tzenkov Date: Wed, 23 Jan 2019 18:03:02 +0200 Subject: [PATCH 06/20] NY-6436: Got rid of InternalError, logging at info level. Signed-off-by: Stoyan Tzenkov --- .../account/repositories/AccountRepositoryAdditionalImpl.java | 4 ++-- .../java/biz/nynja/account/services/AccountServiceImpl.java | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java b/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java index 7f51e10..6518636 100644 --- a/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java +++ b/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java @@ -949,8 +949,8 @@ public class AccountRepositoryAdditionalImpl implements AccountRepositoryAdditio return Optional.empty(); } if (!profileByAuthProvider.getSearchable().booleanValue()) { - logger.error("Record found in DB but is not searchable for the profileId: {}", profileByAuthProvider.getProfileId()); - throw new InternalError("Record found in DB but is not searchable"); + logger.info("Record found in DB but is not searchable for the profileId: {}", profileByAuthProvider.getProfileId()); + return Optional.empty(); } List listAccountsByProfileId = accountByProfileIdRepository diff --git a/src/main/java/biz/nynja/account/services/AccountServiceImpl.java b/src/main/java/biz/nynja/account/services/AccountServiceImpl.java index 71e8dde..8f06131 100644 --- a/src/main/java/biz/nynja/account/services/AccountServiceImpl.java +++ b/src/main/java/biz/nynja/account/services/AccountServiceImpl.java @@ -182,7 +182,7 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas Optional account = Optional.empty(); try { account = accountProvider.searchAccountByLoginOption(AuthenticationType.EMAIL, request.getEmail()); - } catch (IncorrectAccountCountException | InternalError e) { + } catch (IncorrectAccountCountException e) { logAndBuildGrpcSearchResponse(responseObserver, SearchResponse.newBuilder(), "Error while searching for e-mail. {}.", e.getMessage(), Cause.INTERNAL_SERVER_ERROR, "Error while searching for e-mail."); return; -- GitLab From 700bf22f86750d747f48c8cc2d6b12b7edcce145 Mon Sep 17 00:00:00 2001 From: Dragomir Todorov Date: Thu, 24 Jan 2019 14:35:49 +0200 Subject: [PATCH 07/20] NY-6523: Simplify logic to delete auth provider phone --- .../nynja/account/validation/Validators.java | 21 ++++++------------- 1 file changed, 6 insertions(+), 15 deletions(-) diff --git a/src/main/java/biz/nynja/account/validation/Validators.java b/src/main/java/biz/nynja/account/validation/Validators.java index 61ca996..801949e 100644 --- a/src/main/java/biz/nynja/account/validation/Validators.java +++ b/src/main/java/biz/nynja/account/validation/Validators.java @@ -290,25 +290,16 @@ public class Validators { return Optional.of(Cause.MISSING_AUTH_PROVIDER_TYPE); case PHONE: // We expect to receive phone number in the following format : - // ":" - if(isDeleteOperation) { - if (StringUtils.isEmpty(authProvider)) { - return Optional.of(Cause.INVALID_PHONENUMBER); - } + // " + if (StringUtils.isEmpty(authProvider)) { + return Optional.of(Cause.INVALID_PHONENUMBER); + } + if (!isDeleteOperation) { if (!phoneValidator.isPhoneNumberValid(authProvider)) { return Optional.of(Cause.INVALID_PHONENUMBER); } - break; - } else { - String[] provider = authProvider.split(":"); - if (provider == null || provider.length != 2) { - return Optional.of(Cause.INVALID_PHONENUMBER); - } - if (!phoneValidator.isPhoneNumberValid(provider[1], provider[0])) { - return Optional.of(Cause.INVALID_PHONENUMBER); - } - break; } + break; case EMAIL: if (!util.isEmailValid(authProvider)) { return Optional.of(Cause.INVALID_EMAIL); -- GitLab From 13154c15dce58db53da924998cf96aea61f68ad0 Mon Sep 17 00:00:00 2001 From: Stoyan Tzenkov Date: Wed, 23 Jan 2019 19:45:26 +0200 Subject: [PATCH 08/20] NY-6797: Turned permissions on. Signed-off-by: Stoyan Tzenkov --- .../permissions/PermissionsInterceptor.java | 80 ++++++++++--------- .../permissions/PermissionsValidator.java | 43 +++++----- .../account/services/AccountServiceTests.java | 6 +- 3 files changed, 67 insertions(+), 62 deletions(-) diff --git a/src/main/java/biz/nynja/account/permissions/PermissionsInterceptor.java b/src/main/java/biz/nynja/account/permissions/PermissionsInterceptor.java index 0a6a6f5..807dd02 100644 --- a/src/main/java/biz/nynja/account/permissions/PermissionsInterceptor.java +++ b/src/main/java/biz/nynja/account/permissions/PermissionsInterceptor.java @@ -15,6 +15,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; +import com.auth0.jwt.JWT; import com.auth0.jwt.interfaces.Claim; import com.auth0.jwt.interfaces.DecodedJWT; @@ -22,6 +23,7 @@ import biz.nynja.account.accesspoints.AccessPoint; import biz.nynja.account.accesspoints.AccessPointService; import biz.nynja.account.services.AccountServiceImpl; import io.grpc.Context; +import io.grpc.Contexts; import io.grpc.Metadata; import io.grpc.ServerCall; import io.grpc.ServerCallHandler; @@ -64,50 +66,50 @@ public class PermissionsInterceptor implements ServerInterceptor { // WARNING: THe line bellow is to be removed and code following uncommented // when Istio starts sending an access token with each and every request - return next.startCall(call, headers); + // return next.startCall(call, headers); /* * Expected metadata is "Authorization" : "Bearer --accessTokenValue--" so we can skip validation as istio won't * allow this request through */ -// String accessToken = (headers.get(ACCESS_TOKEN_METADATA).split(" "))[1]; -// String rpc = getRpcName(call); -// -// boolean permitted = false; -// Context ctx = null; -// String[] requestingRoles = null; -// -// if (accessToken == null && accessToken.isEmpty()) { -// permissionDenied(call, headers, "Permission denied for rpc {}. Access token not in headers", rpc ); -// } -// ctx = Context.current().withValue(ACCESS_TOKEN_CTX, accessToken); -// DecodedJWT decodedToken = JWT.decode(accessToken); -// -// if (!accessPointAvailable(accessToken, decodedToken, rpc)) { -// permissionDenied(call, headers, "Permission denied for rpc {}. No access point available for this account and access token.", rpc ); -// } -// -// requestingRoles = getRolesFromAccessToken(decodedToken); -// if (requestingRoles == null) { -// permissionDenied(call, headers, "Permission denied for rpc {}. No roles found for requesting account in access token.", rpc ); -// } -// -// Method method = getMethod(rpc); -// if (method == null) { -// permissionDenied(call, headers, "Permission denied for rpc {}. Could not identify the method implementing this rpc.", rpc ); -// } -// -// Permitted[] permittedRoles = method.getAnnotationsByType(Permitted.class); -// permitted = checkPermissions(requestingRoles, permittedRoles); -// if (permitted) { -// logger.info("Permission granted to rpc {}.", rpc); -// return Contexts.interceptCall(ctx, call, headers, next); -// } else { -// logger.error("Permission denied for rpc {}, roles {}.", rpc, requestingRoles); -// call.close(Status.PERMISSION_DENIED.withDescription("An unauthorized call was made to " + rpc + "."), -// headers); -// return NOOP_LISTENER; -// } + String accessToken = (headers.get(ACCESS_TOKEN_METADATA).split(" "))[1]; + String rpc = getRpcName(call); + + boolean permitted = false; + Context ctx = null; + String[] requestingRoles = null; + + if (accessToken == null && accessToken.isEmpty()) { + permissionDenied(call, headers, "Permission denied for rpc {}. Access token not in headers", rpc ); + } + ctx = Context.current().withValue(ACCESS_TOKEN_CTX, accessToken); + DecodedJWT decodedToken = JWT.decode(accessToken); + + if (!accessPointAvailable(accessToken, decodedToken, rpc)) { + permissionDenied(call, headers, "Permission denied for rpc {}. No access point available for this account and access token.", rpc ); + } + + requestingRoles = getRolesFromAccessToken(decodedToken); + if (requestingRoles == null) { + permissionDenied(call, headers, "Permission denied for rpc {}. No roles found for requesting account in access token.", rpc ); + } + + Method method = getMethod(rpc); + if (method == null) { + permissionDenied(call, headers, "Permission denied for rpc {}. Could not identify the method implementing this rpc.", rpc ); + } + + Permitted[] permittedRoles = method.getAnnotationsByType(Permitted.class); + permitted = checkPermissions(requestingRoles, permittedRoles); + if (permitted) { + logger.info("Permission granted to rpc {}.", rpc); + return Contexts.interceptCall(ctx, call, headers, next); + } else { + logger.error("Permission denied for rpc {}, roles {}.", rpc, requestingRoles); + call.close(Status.PERMISSION_DENIED.withDescription("An unauthorized call was made to " + rpc + "."), + headers); + return NOOP_LISTENER; + } } private String getRpcName(ServerCall call) { diff --git a/src/main/java/biz/nynja/account/permissions/PermissionsValidator.java b/src/main/java/biz/nynja/account/permissions/PermissionsValidator.java index 7b45164..44edf68 100644 --- a/src/main/java/biz/nynja/account/permissions/PermissionsValidator.java +++ b/src/main/java/biz/nynja/account/permissions/PermissionsValidator.java @@ -3,6 +3,7 @@ */ package biz.nynja.account.permissions; +import java.util.Base64; import java.util.List; import org.apache.commons.lang3.StringUtils; @@ -21,15 +22,15 @@ public class PermissionsValidator { // WARNING: THe line bellow is to be removed and code following uncommented // when Istio starts sending an access token with each and every request - return true; - -// DecodedJWT decodedToken = retrieveDecodedToken(); -// String requestingAccountId = new String(Base64.getDecoder().decode(decodedToken.getSubject())); -// -// if (requestingAccountId.equals(accountId)) { -// return true; -// } -// return isAuthorizedRequestingRole(decodedToken); + // return true; + + DecodedJWT decodedToken = retrieveDecodedToken(); + String requestingAccountId = new String(Base64.getDecoder().decode(decodedToken.getSubject())); + + if (requestingAccountId.equals(accountId)) { + return true; + } + return isAuthorizedRequestingRole(decodedToken); } private DecodedJWT retrieveDecodedToken() { @@ -46,18 +47,18 @@ public class PermissionsValidator { // WARNING: The line bellow is to be removed and code following uncommented // when Istio starts sending an access token with each and every request - return true; - -// String accessToken = (String) PermissionsInterceptor.ACCESS_TOKEN_CTX.get(); -// DecodedJWT decodedToken = JWT.decode(accessToken); -// String requestingAccountId = new String(Base64.getDecoder().decode(decodedToken.getSubject())); -// -// for (AccountByProfileId account : existingAccountsForProfile) { -// if (requestingAccountId.equals(account.getAccountId().toString())) { -// return true; -// } -// } -// return isAuthorizedRequestingRole(decodedToken); + // return true; + + String accessToken = (String) PermissionsInterceptor.ACCESS_TOKEN_CTX.get(); + DecodedJWT decodedToken = JWT.decode(accessToken); + String requestingAccountId = new String(Base64.getDecoder().decode(decodedToken.getSubject())); + + for (AccountByProfileId account : existingAccountsForProfile) { + if (requestingAccountId.equals(account.getAccountId().toString())) { + return true; + } + } + return isAuthorizedRequestingRole(decodedToken); } private static boolean isAuthorizedRequestingRole(DecodedJWT decodedToken) { diff --git a/src/test/java/biz/nynja/account/services/AccountServiceTests.java b/src/test/java/biz/nynja/account/services/AccountServiceTests.java index 9526411..6bd124f 100644 --- a/src/test/java/biz/nynja/account/services/AccountServiceTests.java +++ b/src/test/java/biz/nynja/account/services/AccountServiceTests.java @@ -13,6 +13,7 @@ import java.util.ArrayList; import java.util.LinkedList; import java.util.List; import java.util.Optional; +import java.util.Set; import java.util.UUID; import java.util.concurrent.ExecutionException; import java.util.stream.Collectors; @@ -242,7 +243,7 @@ public class AccountServiceTests extends GrpcServerTestBase { public void setupTest() { AccessPoint accessPoint = new AccessPoint(); String accountId = "0994566e-ac7b-45b2-b6ef-36440e44a15a"; - String adminAccessToken = "eyJraWQiOiIyMDE4MTEzMCIsInR5cCI6IkpXVCIsImFsZyI6IkVTMjU2In0.eyJzdWIiOiJNRGs1TkRVMk5tVXRZV00zWWkwME5XSXlMV0kyWldZdE16WTBOREJsTkRSaE1UVmgiLCJhdWQiOiJibmx1YW1FPTpueW5qYTpueW5qYSIsInNjb3BlIjoiYWNjZXNzIiwicm9sZXMiOlsiQURNSU4iLCJVU0VSIl0sImlzcyI6Imh0dHBzOi8vYXV0aC5ueW5qYS5iaXovIiwiZXhwIjoxNTQ1MTQzNTY0LCJpYXQiOjE1NDUxMzk5NjR9.w9LZEKB8GLRHDRpa80Lp2EFlGOjmJA4mWP6__Fk0JDu8HlJPk5GyXAm7081s6BqH99ixsKXsGGea6CuT0NHKqQ"; + String adminAccessToken = "eyJraWQiOiIyMDE4MTEzMCIsInR5cCI6IkpXVCIsImFsZyI6IkVTMjU2In0.eyJzdWIiOiJNRGs1TkRVMk5tVXRZV00zWWkwME5XSXlMV0kyWldZdE16WTBOREJsTkRSaE1UVmgiLCJhdWQiOiJTVTVUVkVGT1EwVXdNRGM9Om55bmphOk55bmphIiwic2NvcGUiOiJhY2Nlc3MiLCJyb2xlcyI6WyJVU0VSIiwiQUNDT1VOVF9BRE1JTiJdLCJpc3MiOiJodHRwczovL2F1dGgubnluamEuYml6LyIsImV4cCI6MTU0ODI2NzExMCwiaWF0IjoxNTQ4MjYzNTEwfQ.W9JrZsCXyMCbAvGwOfLbZJNO4PPf0j_nxrUCPX-Q395l2y6SXzGbJYw8ItlpAmZrtQg4ZrY9q2jXvJEoHdi0qg"; accountServiceBlockingStub = AccountServiceGrpc .newBlockingStub(Optional.ofNullable(channel).orElse(inProcChannel)); @@ -578,9 +579,10 @@ public class AccountServiceTests extends GrpcServerTestBase { @Test public void testCompletePendingAccountCreationOkMultipleRoles() { + Set roles = Set.of(Role.USER, Role.ACCOUNT_ADMIN); final CompletePendingAccountCreationRequest request = CompletePendingAccountCreationRequest.newBuilder() .setAccountId(Util.ACCOUNT_ID.toString()).setUsername(Util.USERNAME).setFirstName(Util.FIRST_NAME) - .addRoles(Role.USER).addRoles(Role.ACCOUNT_ADMIN).build(); + .addAllRoles(roles).build(); accountServiceBlockingStub.completePendingAccountCreation(request); -- GitLab From e3546a5109090175772f4e1285c22b51cba46e1b Mon Sep 17 00:00:00 2001 From: Stoyan Tzenkov Date: Thu, 24 Jan 2019 09:43:17 +0200 Subject: [PATCH 09/20] NY-4662: No account details in the log Signed-off-by: Stoyan Tzenkov --- .../account/services/AccountServiceImpl.java | 26 +++++++++---------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/src/main/java/biz/nynja/account/services/AccountServiceImpl.java b/src/main/java/biz/nynja/account/services/AccountServiceImpl.java index 8f06131..59bff97 100644 --- a/src/main/java/biz/nynja/account/services/AccountServiceImpl.java +++ b/src/main/java/biz/nynja/account/services/AccountServiceImpl.java @@ -159,7 +159,7 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas return; } AccountResponse response = accountResponse.get(); - logger.info("SUCCESS: Found account by provider {}: \"{}\"", request.getAuthenticationIdentifier(), response); + logger.info("SUCCESS: Found account by provider {}: ", request.getAuthenticationIdentifier()); responseObserver.onNext(response); responseObserver.onCompleted(); } @@ -271,7 +271,7 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas } AccountResponse response = accountResponse.get(); - logger.info("SUCCESS: Found result for account by username {}: \"{}\"", request.getUsername(), response); + logger.info("SUCCESS: Found result for account by username {}.", request.getUsername()); responseObserver.onNext(response); responseObserver.onCompleted(); } @@ -353,7 +353,7 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas return; } AccountResponse response = accountResponse.get(); - logger.info("SUCCESS: Found result for account by QR code {}: \"{}\"", request.getQrCode(), response); + logger.info("SUCCESS: Found result for account by QR code {}.", request.getQrCode()); responseObserver.onNext(response); responseObserver.onCompleted(); } @@ -409,7 +409,7 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas return; } AccountsResponse response = accounts.get(); - logger.info("SUCCESS: Found result for account by profile ID {}: \"{}\"", request.getProfileId(), response); + logger.info("SUCCESS: Found result for account by profile ID {}.", request.getProfileId()); responseObserver.onNext(response); responseObserver.onCompleted(); } @@ -447,7 +447,7 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas return; } AccountResponse response = account.get(); - logger.info("SUCCESS: Found result for account by account ID {}: \"{}\"", request.getAccountId(), response); + logger.info("SUCCESS: Found result for account by account ID {}.", request.getAccountId()); responseObserver.onNext(response); responseObserver.onCompleted(); } @@ -471,6 +471,7 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas response = accountCreator.retrieveCreatePendingAccountResponse(request); } + logger.info("SUCCESS: Created pending account for provider {}.", request.getAuthenticationProvider()); responseObserver.onNext(response); responseObserver.onCompleted(); } @@ -486,8 +487,7 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas AccountResponse response = accountCreator.retrieveCompletePendingAccountResponse(request); - logger.info("SUCCESS: Completed pending account creattion for account ID {}: \"{}\"", request.getAccountId(), - response); + logger.info("SUCCESS: Completed pending account creattion for account ID {}.", request.getAccountId()); responseObserver.onNext(response); responseObserver.onCompleted(); } @@ -536,8 +536,7 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas "", Cause.ERROR_UPDATING_ACCOUNT); return; } - logger.info("SUCCESS: Account \"{}\" updated in the DB", updatedAccount.toString()); - logger.info("SUCCESS: Account: \"{}\" updated successfully.", updatedAccount); + logger.info("SUCCESS: Account \"{}\" updated in the DB", updatedAccount.getAccountId()); AccountResponse response = AccountResponse.newBuilder().setAccountDetails(updatedAccount.toProto()).build(); responseObserver.onNext(response); responseObserver.onCompleted(); @@ -956,7 +955,7 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas return; } ProfileResponse response = profile.get(); - logger.info("SUCCESS: Found profile by profile ID {}: \"{}\"", request.getProfileId(), response); + logger.info("SUCCESS: Found profile by profile ID {}.", request.getProfileId()); responseObserver.onNext(response); responseObserver.onCompleted(); } @@ -1137,8 +1136,7 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas "", Cause.ACCOUNT_NOT_FOUND); } else { AccountResponse response = account.get(); - logger.info("SUCCESS: Found account by login option {}: \"{}\"", request.getAuthenticationIdentifier(), - response); + logger.info("SUCCESS: Found account by login option {}.", request.getAuthenticationIdentifier()); responseObserver.onNext(response); responseObserver.onCompleted(); } @@ -1193,8 +1191,8 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas account.get().getAvatar(), account.get().getFirstName(), account.get().getLastName()); SearchResponse response = SearchResponse.newBuilder().setSearchResultDetails(searchResultDetails).build(); - logger.info("SUCCESS: Found result for account by social provider {}: \"{}\"", - request.getAuthenticationIdentifier(), response); + logger.info("SUCCESS: Found result for account by social provider {}.", + request.getAuthenticationIdentifier()); responseObserver.onNext(response); responseObserver.onCompleted(); } -- GitLab From 3928a82637c61fbf520ce7b64b715fd0bf053943 Mon Sep 17 00:00:00 2001 From: Stoyan Tzenkov Date: Fri, 25 Jan 2019 16:06:46 +0200 Subject: [PATCH 10/20] NY-4662: Account Id added to log whereever possible. Signed-off-by: Stoyan Tzenkov --- .../nynja/account/services/AccountServiceImpl.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/main/java/biz/nynja/account/services/AccountServiceImpl.java b/src/main/java/biz/nynja/account/services/AccountServiceImpl.java index 59bff97..4b42261 100644 --- a/src/main/java/biz/nynja/account/services/AccountServiceImpl.java +++ b/src/main/java/biz/nynja/account/services/AccountServiceImpl.java @@ -271,7 +271,7 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas } AccountResponse response = accountResponse.get(); - logger.info("SUCCESS: Found result for account by username {}.", request.getUsername()); + logger.info("SUCCESS: Found result for account by username {}. Account Id = {}.", request.getUsername(), response.getAccountDetails().getAccountId()); responseObserver.onNext(response); responseObserver.onCompleted(); } @@ -353,7 +353,7 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas return; } AccountResponse response = accountResponse.get(); - logger.info("SUCCESS: Found result for account by QR code {}.", request.getQrCode()); + logger.info("SUCCESS: Found result for account by QR code {}. Account Id={}.", request.getQrCode(), response.getAccountDetails().getAccountId()); responseObserver.onNext(response); responseObserver.onCompleted(); } @@ -471,7 +471,7 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas response = accountCreator.retrieveCreatePendingAccountResponse(request); } - logger.info("SUCCESS: Created pending account for provider {}.", request.getAuthenticationProvider()); + logger.info("SUCCESS: Created pending account for provider {} and accoutn Id {}.", request.getAuthenticationProvider(), response.getPendingAccountDetails().getAccountId()); responseObserver.onNext(response); responseObserver.onCompleted(); } @@ -487,7 +487,7 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas AccountResponse response = accountCreator.retrieveCompletePendingAccountResponse(request); - logger.info("SUCCESS: Completed pending account creattion for account ID {}.", request.getAccountId()); + logger.info("SUCCESS: Completed pending account creation for account ID {}.", request.getAccountId()); responseObserver.onNext(response); responseObserver.onCompleted(); } @@ -602,7 +602,7 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas wasProfileDeleted.get()); return; } - logger.info("SUCCESS: The profile was deleted successfully."); + logger.info("SUCCESS: Profile with Id {} was deleted successfully.", request.getProfileId()); responseObserver.onNext(StatusResponse.newBuilder().setStatus("SUCCESS").build()); responseObserver.onCompleted(); } @@ -1136,7 +1136,7 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas "", Cause.ACCOUNT_NOT_FOUND); } else { AccountResponse response = account.get(); - logger.info("SUCCESS: Found account by login option {}.", request.getAuthenticationIdentifier()); + logger.info("SUCCESS: Found account by login option {}. Account Id={}.", request.getAuthenticationIdentifier(), response.getAccountDetails().getAccountId()); responseObserver.onNext(response); responseObserver.onCompleted(); } -- GitLab From 76a241ea044b4a1f930a0839992d317afa8104ce Mon Sep 17 00:00:00 2001 From: Ralitsa Todorova Date: Mon, 28 Jan 2019 16:43:23 +0200 Subject: [PATCH 11/20] Add TODO notes for future implementation in addAuthenticationProviderToProfile Signed-off-by: Ralitsa Todorova --- .../java/biz/nynja/account/services/AccountServiceImpl.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/main/java/biz/nynja/account/services/AccountServiceImpl.java b/src/main/java/biz/nynja/account/services/AccountServiceImpl.java index 4b42261..acbb793 100644 --- a/src/main/java/biz/nynja/account/services/AccountServiceImpl.java +++ b/src/main/java/biz/nynja/account/services/AccountServiceImpl.java @@ -607,6 +607,10 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas responseObserver.onCompleted(); } + // TODO: In future this endpoint should be exposed so that it is accessible for end users. An additional call to + // Authentication Service should be made in order to verify the authentication provider that is to be added. + // See this post for more information: + // https://nynjadev.atlassian.net/wiki/spaces/NM/pages/595132817/Verify+Authentication+Provider+Flow @Override @PerformPermissionCheck @Permitted(role = RoleConstants.ACCOUNT_ADMIN) -- GitLab From 45760def3866d38d2a874bf80da55fa1b78c2f75 Mon Sep 17 00:00:00 2001 From: Dragomir Todorov Date: Tue, 29 Jan 2019 16:12:54 +0200 Subject: [PATCH 12/20] Add missed interceptor check --- .../account/permissions/PermissionsInterceptor.java | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/main/java/biz/nynja/account/permissions/PermissionsInterceptor.java b/src/main/java/biz/nynja/account/permissions/PermissionsInterceptor.java index 807dd02..9b731b2 100644 --- a/src/main/java/biz/nynja/account/permissions/PermissionsInterceptor.java +++ b/src/main/java/biz/nynja/account/permissions/PermissionsInterceptor.java @@ -21,6 +21,7 @@ import com.auth0.jwt.interfaces.DecodedJWT; import biz.nynja.account.accesspoints.AccessPoint; import biz.nynja.account.accesspoints.AccessPointService; +import biz.nynja.account.grid.ag.AdminServiceImpl; import biz.nynja.account.services.AccountServiceImpl; import io.grpc.Context; import io.grpc.Contexts; @@ -49,6 +50,7 @@ public class PermissionsInterceptor implements ServerInterceptor { private static final Logger logger = LoggerFactory.getLogger(PermissionsInterceptor.class); private static final Class SERVICE_CLASS = AccountServiceImpl.class; + private static final Class ADMIN_SERVICE_CLASS = AdminServiceImpl.class; public static final Metadata.Key ACCESS_TOKEN_METADATA = Metadata.Key.of("Authorization", ASCII_STRING_MARSHALLER); @@ -141,12 +143,20 @@ public class PermissionsInterceptor implements ServerInterceptor { private Method getMethod(String rpc) { // Get the rpc method called Method[] allMethods = SERVICE_CLASS.getDeclaredMethods(); - + Method[] adminMethods = ADMIN_SERVICE_CLASS.getDeclaredMethods(); + for (Method method : allMethods) { if (method.getName().equals(rpc)) { return method; } } + + for (Method method : adminMethods) { + if (method.getName().equals(rpc)) { + return method; + } + } + return null; } -- GitLab From 1e52609b2dbe7c41fb1bfaec4c52ab332e24718a Mon Sep 17 00:00:00 2001 From: Stoyan Tzenkov Date: Tue, 29 Jan 2019 17:15:06 +0200 Subject: [PATCH 13/20] NY-6877: Delete accesspoints when status changed to DISABLED/SUSPENDED. Signed-off-by: Stoyan Tzenkov --- .../account/accesspoints/AccessPointRepository.java | 2 ++ .../account/accesspoints/AccessPointService.java | 12 ++++++++++++ .../AccountRepositoryAdditionalImpl.java | 12 +++++++++++- 3 files changed, 25 insertions(+), 1 deletion(-) diff --git a/src/main/java/biz/nynja/account/accesspoints/AccessPointRepository.java b/src/main/java/biz/nynja/account/accesspoints/AccessPointRepository.java index 9f2c3c0..84b4c0d 100644 --- a/src/main/java/biz/nynja/account/accesspoints/AccessPointRepository.java +++ b/src/main/java/biz/nynja/account/accesspoints/AccessPointRepository.java @@ -22,4 +22,6 @@ public interface AccessPointRepository @Query("select * from auth.accesspoint where accountid=:accountId;") List findAllByAccountId(@Param("accountId") UUID accountId); + @Query("delete from auth.accesspoint where accountid=:accountId;") + void deleteById(@Param("accountId") UUID accountId); } diff --git a/src/main/java/biz/nynja/account/accesspoints/AccessPointService.java b/src/main/java/biz/nynja/account/accesspoints/AccessPointService.java index 2ac4a44..74cc4af 100644 --- a/src/main/java/biz/nynja/account/accesspoints/AccessPointService.java +++ b/src/main/java/biz/nynja/account/accesspoints/AccessPointService.java @@ -33,6 +33,18 @@ public class AccessPointService { return Optional.empty(); } + public boolean deleteAccessPointsForAccount(UUID accountId) { + try { + accessPointRepository.deleteById(accountId); + logger.info("AccessPoint successfully deleted from the DB."); + return true; + } catch (Exception e) { + logger.error("Error deleting accesspoints for account {}. Message: {}, cause: {}", accountId, + e.getMessage(), e.getCause()); + return false; + } + } + public AccessPoint buildAccessPoint(String deviceId, String accessToken, String accountId, long expiresIn) { AccessPoint accessPoint = new AccessPoint(); accessPoint.setDeviceId(deviceId); diff --git a/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java b/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java index 6518636..48aaa80 100644 --- a/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java +++ b/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java @@ -35,10 +35,12 @@ import com.datastax.driver.core.PreparedStatement; import com.datastax.driver.core.ResultSet; import com.datastax.driver.core.Session; +import biz.nynja.account.accesspoints.AccessPointService; import biz.nynja.account.components.AccountServiceHelper; import biz.nynja.account.components.StatementsPool; import biz.nynja.account.configuration.AccountDataConfiguration; import biz.nynja.account.configuration.PendingAccountConfiguration; +import biz.nynja.account.grpc.AccessStatus; import biz.nynja.account.grpc.CompletePendingAccountCreationRequest; import biz.nynja.account.grpc.ContactType; import biz.nynja.account.grpc.ErrorResponse.Cause; @@ -83,6 +85,7 @@ public class AccountRepositoryAdditionalImpl implements AccountRepositoryAdditio private final Session session; private final ErlangAccountBridge erlangAccountBridge; private final PermissionsValidator permissionsValidator; + private final AccessPointService accessPointService; public AccountRepositoryAdditionalImpl(PendingAccountConfiguration pendingAccountConfiguration, CassandraTemplate cassandraTemplate, StatementsPool statementsPool, AccountRepository accountRepository, @@ -93,7 +96,7 @@ public class AccountRepositoryAdditionalImpl implements AccountRepositoryAdditio PendingAccountByAuthenticationProviderRepository pendingAccountByAuthenticationProviderRepository, PendingAccountRepository pendingAccountRepository, AccountServiceHelper accountServiceHelper, Session session, ErlangAccountBridge erlangAccountBridge, PermissionsValidator permissionsValidator, - AccountDataConfiguration accountDataConfiguration) { + AccountDataConfiguration accountDataConfiguration, AccessPointService accessPointService) { this.pendingAccountConfiguration = pendingAccountConfiguration; this.cassandraTemplate = cassandraTemplate; this.statementsPool = statementsPool; @@ -110,6 +113,7 @@ public class AccountRepositoryAdditionalImpl implements AccountRepositoryAdditio this.erlangAccountBridge = erlangAccountBridge; this.permissionsValidator = permissionsValidator; this.accountDataConfiguration = accountDataConfiguration; + this.accessPointService= accessPointService; } @PostConstruct @@ -254,6 +258,12 @@ public class AccountRepositoryAdditionalImpl implements AccountRepositoryAdditio if (wr != null) { boolean applied = wr.wasApplied(); if (applied) { + if (request.getAccessStatus().equals(AccessStatus.DISABLED) + || request.getAccessStatus().equals(AccessStatus.SUSPENDED)) { + if (!accessPointService.deleteAccessPointsForAccount(UUID.fromString(request.getAccountId()))) { + logger.error("Error deleting accesspoints from the DB for account {}.", request.getAccountId()); + } + } Account updatedAccount = accountRepository.findByAccountId(UUID.fromString(request.getAccountId())); return updatedAccount; } -- GitLab From 9b8cb21fa31c801f05778678234dcb8d46b01130 Mon Sep 17 00:00:00 2001 From: Dragomir Todorov Date: Wed, 30 Jan 2019 15:03:31 +0200 Subject: [PATCH 14/20] NY-6842: Added error messages to most responses --- .../account/services/AccountServiceImpl.java | 225 ++++++++++-------- .../decomposition/AccountCreator.java | 26 +- .../nynja/account/validation/Validation.java | 17 +- .../nynja/account/validation/Validators.java | 75 +++--- .../account/components/ValidatorTests.java | 29 +-- 5 files changed, 218 insertions(+), 154 deletions(-) diff --git a/src/main/java/biz/nynja/account/services/AccountServiceImpl.java b/src/main/java/biz/nynja/account/services/AccountServiceImpl.java index acbb793..191f704 100644 --- a/src/main/java/biz/nynja/account/services/AccountServiceImpl.java +++ b/src/main/java/biz/nynja/account/services/AccountServiceImpl.java @@ -89,6 +89,7 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas private static final Logger logger = LoggerFactory.getLogger(AccountServiceImpl.class); private static final byte MIN_NUMBER_OF_AUTH_PROVIDERS_IN_PROFILE = 1; + private static final String EMPTY_ERROR_MESSAGE = ""; private final AccountRepositoryAdditional accountRepositoryAdditional; private final ProfileRepository profileRepository; @@ -136,12 +137,13 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas logger.info("Getting account by authentication provider: {}", request); if (request.getAuthenticationTypeValue() == 0) { logAndBuildGrpcAccountResponse(responseObserver, AccountResponse.newBuilder(), - "Missing authentication provider", "", Cause.MISSING_AUTH_PROVIDER_TYPE); + "Missing authentication provider", "", Cause.MISSING_AUTH_PROVIDER_TYPE, EMPTY_ERROR_MESSAGE); return; } if (request.getAuthenticationIdentifier() == null || request.getAuthenticationIdentifier().isEmpty()) { logAndBuildGrpcAccountResponse(responseObserver, AccountResponse.newBuilder(), - "Missing authentication provider identifier", "", Cause.MISSING_AUTH_PROVIDER_ID); + "Missing authentication provider identifier", "", Cause.MISSING_AUTH_PROVIDER_ID, + EMPTY_ERROR_MESSAGE); return; } Optional accountResponse = accountProvider.getAccountResponseByAuthenticationProvider( @@ -150,12 +152,13 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas if (!accountResponse.isPresent()) { logAndBuildGrpcAccountResponse(responseObserver, AccountResponse.newBuilder(), "Account not found for identifier: {}", request.getAuthenticationIdentifier(), - Cause.ACCOUNT_NOT_FOUND); + Cause.ACCOUNT_NOT_FOUND, "Account not found"); return; } if (!permissionsValidator.isRpcAllowed(accountResponse.get().getAccountDetails().getAccountId())) { logAndBuildGrpcAccountResponse(responseObserver, AccountResponse.newBuilder(), - "Account info can not be obtained for this account.", "", Cause.ERROR_PERMISSION_DENIED); + "Account info can not be obtained for this account.", "", Cause.ERROR_PERMISSION_DENIED, + "Permission denied"); return; } AccountResponse response = accountResponse.get(); @@ -184,7 +187,8 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas account = accountProvider.searchAccountByLoginOption(AuthenticationType.EMAIL, request.getEmail()); } catch (IncorrectAccountCountException e) { logAndBuildGrpcSearchResponse(responseObserver, SearchResponse.newBuilder(), - "Error while searching for e-mail. {}.", e.getMessage(), Cause.INTERNAL_SERVER_ERROR, "Error while searching for e-mail."); + "Error while searching for e-mail. {}.", e.getMessage(), Cause.INTERNAL_SERVER_ERROR, + "Error while searching for e-mail."); return; } @@ -252,7 +256,7 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas Validation validation = validateGetByUsernameRequest(request); if (validation.hasErrors()) { logAndBuildGrpcAccountResponse(responseObserver, AccountResponse.newBuilder(), validation.getErrorMessage(), - "", validation.getCause().get()); + "", validation.getCause().get(), validation.getErrorMessage()); return; } @@ -260,18 +264,20 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas if (!accountResponse.isPresent()) { logAndBuildGrpcAccountResponse(responseObserver, AccountResponse.newBuilder(), "Account not found", "", - Cause.ACCOUNT_NOT_FOUND); + Cause.ACCOUNT_NOT_FOUND, "Account not found"); return; } if (!permissionsValidator.isRpcAllowed(accountResponse.get().getAccountDetails().getAccountId())) { logAndBuildGrpcAccountResponse(responseObserver, AccountResponse.newBuilder(), - "Account info can not be obtained for this account.", "", Cause.ERROR_PERMISSION_DENIED); + "Account info can not be obtained for this account.", "", Cause.ERROR_PERMISSION_DENIED, + "Permission denied"); return; } AccountResponse response = accountResponse.get(); - logger.info("SUCCESS: Found result for account by username {}. Account Id = {}.", request.getUsername(), response.getAccountDetails().getAccountId()); + logger.info("SUCCESS: Found result for account by username {}. Account Id = {}.", request.getUsername(), + response.getAccountDetails().getAccountId()); responseObserver.onNext(response); responseObserver.onCompleted(); } @@ -336,24 +342,26 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas if ((request.getQrCode() == null) || request.getQrCode().isEmpty()) { logAndBuildGrpcAccountResponse(responseObserver, AccountResponse.newBuilder(), "Missing QR code.", "", - Cause.MISSING_QR_CODE); + Cause.MISSING_QR_CODE, "Missing QR code"); return; } Optional accountResponse = accountProvider.getAccountResponseByQrCode(request.getQrCode()); if (!accountResponse.isPresent()) { logAndBuildGrpcAccountResponse(responseObserver, AccountResponse.newBuilder(), "Account not found", "", - Cause.ACCOUNT_NOT_FOUND); + Cause.ACCOUNT_NOT_FOUND, "Account not found"); return; } if (!permissionsValidator.isRpcAllowed(accountResponse.get().getAccountDetails().getAccountId())) { logAndBuildGrpcAccountResponse(responseObserver, AccountResponse.newBuilder(), - "Account info can not be obtained for this account.", "", Cause.ERROR_PERMISSION_DENIED); + "Account info can not be obtained for this account.", "", Cause.ERROR_PERMISSION_DENIED, + "Permission denied"); return; } AccountResponse response = accountResponse.get(); - logger.info("SUCCESS: Found result for account by QR code {}. Account Id={}.", request.getQrCode(), response.getAccountDetails().getAccountId()); + logger.info("SUCCESS: Found result for account by QR code {}. Account Id={}.", request.getQrCode(), + response.getAccountDetails().getAccountId()); responseObserver.onNext(response); responseObserver.onCompleted(); } @@ -424,26 +432,26 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas logger.info("Getting account by account id: {}", request.getAccountId()); if ((request.getAccountId() == null) || (request.getAccountId().isEmpty())) { - logAndBuildGrpcAccountResponse(responseObserver, AccountResponse.newBuilder(), "Missing account id", "", - Cause.MISSING_ACCOUNT_ID); + logAndBuildGrpcAccountResponse(responseObserver, AccountResponse.newBuilder(), "Missing account ID", "", + Cause.MISSING_ACCOUNT_ID, "Missing account ID"); return; } if (!util.isValidUuid(request.getAccountId())) { logAndBuildGrpcAccountResponse(responseObserver, AccountResponse.newBuilder(), "Invalid account id: {}", - request.getAccountId(), Cause.INVALID_ACCOUNT_ID); + request.getAccountId(), Cause.INVALID_ACCOUNT_ID, "Invalid Account ID"); return; } if (!permissionsValidator.isRpcAllowed(request.getAccountId())) { logAndBuildGrpcAccountResponse(responseObserver, AccountResponse.newBuilder(), "Account info can not be obtained for account {}.", request.getAccountId(), - Cause.ERROR_PERMISSION_DENIED); + Cause.ERROR_PERMISSION_DENIED, "Permission denied"); return; } Optional account = accountProvider.getAccountByAccountId(request); if (!account.isPresent()) { logAndBuildGrpcAccountResponse(responseObserver, AccountResponse.newBuilder(), "Account id not found: {}", - request.getAccountId(), Cause.ACCOUNT_NOT_FOUND); + request.getAccountId(), Cause.ACCOUNT_NOT_FOUND, "Account not found"); return; } AccountResponse response = account.get(); @@ -465,13 +473,14 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas Optional cause = pendingAccount.validateCreatePendingAccountRequest(request); if (cause.isPresent()) { - response = CreatePendingAccountResponse.newBuilder().setError(ErrorResponse.newBuilder().setCause(cause.get())) - .build(); + response = CreatePendingAccountResponse.newBuilder() + .setError(ErrorResponse.newBuilder().setCause(cause.get())).build(); } else { response = accountCreator.retrieveCreatePendingAccountResponse(request); } - logger.info("SUCCESS: Created pending account for provider {} and accoutn Id {}.", request.getAuthenticationProvider(), response.getPendingAccountDetails().getAccountId()); + logger.info("SUCCESS: Created pending account for provider {} and accoutn Id {}.", + request.getAuthenticationProvider(), response.getPendingAccountDetails().getAccountId()); responseObserver.onNext(response); responseObserver.onCompleted(); } @@ -501,39 +510,41 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas logger.debug("Updating account...: {}", request); if ((request.getAccountId() == null) || (request.getAccountId().isEmpty())) { - logAndBuildGrpcAccountResponse(responseObserver, AccountResponse.newBuilder(), "Missing account id", "", - Cause.MISSING_ACCOUNT_ID); + logAndBuildGrpcAccountResponse(responseObserver, AccountResponse.newBuilder(), "Missing account ID", "", + Cause.MISSING_ACCOUNT_ID, "Missing account ID"); return; } if (!util.isValidUuid(request.getAccountId())) { logAndBuildGrpcAccountResponse(responseObserver, AccountResponse.newBuilder(), "Invalid account id: {}", - request.getAccountId(), Cause.INVALID_ACCOUNT_ID); + request.getAccountId(), Cause.INVALID_ACCOUNT_ID, "Invalid Account ID"); return; } if (!permissionsValidator.isRpcAllowed(request.getAccountId())) { logAndBuildGrpcAccountResponse(responseObserver, AccountResponse.newBuilder(), "Can not update account {}.", - request.getAccountId(), Cause.ERROR_PERMISSION_DENIED); + request.getAccountId(), Cause.ERROR_PERMISSION_DENIED, "Permission denied"); return; } - Optional cause = account.validateUpdateAccountRequest(request); - if (cause.isPresent()) { - logAndBuildGrpcAccountResponse(responseObserver, AccountResponse.newBuilder(), "Validation failed", "", - cause.get()); + Validation validation = account.validateUpdateAccountRequest(request); + if (validation.hasErrors()) { + logAndBuildGrpcAccountResponse(responseObserver, AccountResponse.newBuilder(), + "Validation failed: " + validation.getErrorMessage(), "", validation.getFirstCause().get(), + validation.getFirstMessage().get()); return; } if (request.getUsername() != null && !request.getUsername().trim().isEmpty() && accountRepositoryAdditional .foundExistingNotOwnUsername(UUID.fromString(request.getAccountId()), request.getUsername())) { logAndBuildGrpcAccountResponse(responseObserver, AccountResponse.newBuilder(), - "User name already in use: {}", request.getUsername(), Cause.USERNAME_ALREADY_USED); + "User name already in use: {}", request.getUsername(), Cause.USERNAME_ALREADY_USED, + "Username is already used"); return; } Account updatedAccount = accountRepositoryAdditional.updateAccount(request); if (updatedAccount == null) { logAndBuildGrpcAccountResponse(responseObserver, AccountResponse.newBuilder(), "Error updating account.", - "", Cause.ERROR_UPDATING_ACCOUNT); + "", Cause.ERROR_UPDATING_ACCOUNT, "Failed to update account"); return; } logger.info("SUCCESS: Account \"{}\" updated in the DB", updatedAccount.getAccountId()); @@ -551,25 +562,26 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas if ((request.getAccountId() == null) || (request.getAccountId().isEmpty())) { logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), "Missing account id", "", - Cause.MISSING_ACCOUNT_ID); + Cause.MISSING_ACCOUNT_ID, "Missing account ID"); return; } if (!util.isValidUuid(request.getAccountId())) { logger.info("Can not delete account. Invalid account id: {}", request.getAccountId()); logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), "Invalid account id: {}", - request.getAccountId(), Cause.INVALID_ACCOUNT_ID); + request.getAccountId(), Cause.INVALID_ACCOUNT_ID, "Invalid Account ID"); return; } if (!permissionsValidator.isRpcAllowed(request.getAccountId())) { logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), "Can not delete account {}.", - request.getAccountId(), Cause.ERROR_PERMISSION_DENIED); + request.getAccountId(), Cause.ERROR_PERMISSION_DENIED, "Permission denied"); return; } boolean wasAccountDeleted = accountRepositoryAdditional.deleteAccount(UUID.fromString(request.getAccountId())); if (!wasAccountDeleted) { logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), - "Error deleting account with id", request.getAccountId(), Cause.ERROR_DELETING_ACCOUNT); + "Error deleting account with id", request.getAccountId(), Cause.ERROR_DELETING_ACCOUNT, + "Failed to delete account"); return; } logger.info("SUCCESS: Account successfully deleted: {}", request.getAccountId()); @@ -585,12 +597,12 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas logger.debug("Deleting profile: {}", request); if ((request.getProfileId() == null) || (request.getProfileId().isEmpty())) { logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), "Missing profile id", "", - Cause.MISSING_PROFILE_ID); + Cause.MISSING_PROFILE_ID, "Missing profile id"); return; } if (!util.isValidUuid(request.getProfileId())) { logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), "Invalid profile id: {}", - request.getProfileId(), Cause.INVALID_PROFILE_ID); + request.getProfileId(), Cause.INVALID_PROFILE_ID, "Invalid profile id"); return; } @@ -599,7 +611,7 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas if (wasProfileDeleted.isPresent()) { logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), "Error deleting profile.", "", - wasProfileDeleted.get()); + wasProfileDeleted.get(), "Failed to delete profile"); return; } logger.info("SUCCESS: Profile with Id {} was deleted successfully.", request.getProfileId()); @@ -623,12 +635,12 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas if ((request.getProfileId() == null) || (request.getProfileId().isEmpty())) { logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), "Missing profile id", "", - Cause.MISSING_PROFILE_ID); + Cause.MISSING_PROFILE_ID, "Missing profile id"); return; } if (!util.isValidUuid(request.getProfileId())) { logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), "Invalid profile id: {}", - request.getProfileId(), Cause.INVALID_PROFILE_ID); + request.getProfileId(), Cause.INVALID_PROFILE_ID, "Invalid profile id"); return; } @@ -638,14 +650,14 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas if (!permissionsValidator.isRpcAllowed(existingAccountsForProfile)) { logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), "Can not add authentication provider to profile {}.", request.getProfileId(), - Cause.ERROR_PERMISSION_DENIED); + Cause.ERROR_PERMISSION_DENIED, "Permission denied"); return; } - Optional cause = authenticationProvider.validateAddAuthenticationProviderRequest(request); - if (cause.isPresent()) { + Validation validation = authenticationProvider.validateAddAuthenticationProviderRequest(request); + if (validation.hasErrors()) { logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), "Validation failed", "", - cause.get()); + validation.getFirstCause().get(), validation.getFirstMessage().get()); return; } @@ -665,7 +677,8 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas Profile profile = profileRepository.findByProfileId(UUID.fromString(request.getProfileId())); if (profile == null) { logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), - "Profile id {} not found in DB.", request.getProfileId(), Cause.PROFILE_NOT_FOUND); + "Profile id {} not found in DB.", request.getProfileId(), Cause.PROFILE_NOT_FOUND, + "Profile not found"); return; } @@ -674,7 +687,7 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas .getMaxAuthenticationprovidersPerProfile()) { logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), "Max number of authentication providers reached for profile id {}.", request.getProfileId(), - Cause.MAX_PROVIDERS_PER_PROFILE_REACHED); + Cause.MAX_PROVIDERS_PER_PROFILE_REACHED, "You have reached the max number of login options"); return; } @@ -686,7 +699,7 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas if (profileByAuthProvider != null) { logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), "Authentication provider: {} already used.", request.getAuthenticationProvider().toString(), - Cause.AUTH_PROVIDER_ALREADY_USED); + Cause.AUTH_PROVIDER_ALREADY_USED, "Login option is already used"); return; } @@ -695,7 +708,7 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas if (!result) { logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), "Authentication provider was not successfuly added for profile id {}.", request.getProfileId(), - Cause.INTERNAL_SERVER_ERROR); + Cause.INTERNAL_SERVER_ERROR, "Failed to add login option to profile"); return; } @@ -715,22 +728,25 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas logger.info("Adding contact info to account requested."); logger.debug("Adding contact info to account requested: {}", request); + // TODO use Validation object Optional> validationResult = account .validateContactInfoRequest(request.getAccountId(), request.getContactInfo()); if (validationResult.isPresent()) { logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), - validationResult.get().getValue(), "", validationResult.get().getKey()); + validationResult.get().getValue(), "", validationResult.get().getKey(), + validationResult.get().getValue()); return; } if (!util.isValidUuid(request.getAccountId())) { logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), "Invalid account id: {}", - request.getAccountId(), Cause.INVALID_ACCOUNT_ID); + request.getAccountId(), Cause.INVALID_ACCOUNT_ID, "Invalid Account ID"); return; } if (!permissionsValidator.isRpcAllowed(request.getAccountId())) { logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), - "Can not add contact info to account {}.", request.getAccountId(), Cause.ERROR_PERMISSION_DENIED); + "Can not add contact info to account {}.", request.getAccountId(), Cause.ERROR_PERMISSION_DENIED, + "Permission denied"); return; } @@ -738,7 +754,8 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas Validation validation = account.validateContactInfoPhoneLabel(request.getContactInfo().getLabel()); if (validation.hasErrors()) { logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), - validation.getErrorMessage(), "", validation.getCause().get()); + "Validation failed: " + validation.getErrorMessage(), "", validation.getFirstCause().get(), + validation.getFirstMessage().get()); return; } request = phoneNumberNormalizer.normalizePhoneNumber(request); @@ -749,13 +766,13 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas if (!result.isPresent()) { logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), "Reached max count of contact info for type {}.", request.getContactInfo().getType().name(), - Cause.MAX_CONTACT_INFO_OF_TYPE_REACHED); + Cause.MAX_CONTACT_INFO_OF_TYPE_REACHED, "You have reached the max number of records for this type"); return; } if (!result.get().booleanValue()) { logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), "Contact info was not added to account {}.", request.getAccountId(), - Cause.ERROR_ADDING_CONTACT_INFO); + Cause.ERROR_ADDING_CONTACT_INFO, "Failed to add contact info"); return; } logger.info("SUCCESS: Contact info {}:{} was added to account {}.", request.getContactInfo().getType().name(), @@ -777,19 +794,20 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas .validateContactInfoRequest(request.getAccountId(), request.getContactInfo()); if (validationResult.isPresent()) { logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), "Validation failed. {}.", - validationResult.get().getRight(), validationResult.get().getLeft()); + validationResult.get().getRight(), validationResult.get().getLeft(), + validationResult.get().getRight()); return; } if (!util.isValidUuid(request.getAccountId())) { logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), "Invalid account id: {}", - request.getAccountId(), Cause.INVALID_ACCOUNT_ID); + request.getAccountId(), Cause.INVALID_ACCOUNT_ID, "Invalid Account ID"); return; } if (!permissionsValidator.isRpcAllowed(request.getAccountId())) { logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), "Can not remove contact info from account {}.", request.getAccountId(), - Cause.ERROR_PERMISSION_DENIED); + Cause.ERROR_PERMISSION_DENIED, "Permission denied"); return; } @@ -802,7 +820,7 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas if (!result) { logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), "Contact info was not removed from account {}.", request.getAccountId(), - Cause.ERROR_DELETING_CONTACT_INFO); + Cause.ERROR_DELETING_CONTACT_INFO, "Failed to delete conact info"); return; } logger.info("SUCCESS: Contact info {}:{} was removed from account {}.", @@ -821,31 +839,31 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas if ((request.getProfileId() == null) || (request.getProfileId().isEmpty())) { logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), "Missing profile id.", "", - Cause.MISSING_PROFILE_ID); + Cause.MISSING_PROFILE_ID, "Missing profile id"); return; } if (!util.isValidUuid(request.getProfileId())) { logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), "Invalid profile id: {}", - request.getProfileId(), Cause.INVALID_PROFILE_ID); + request.getProfileId(), Cause.INVALID_PROFILE_ID, "Invalid profile id"); return; } if (request.getAuthenticationProvider().getAuthenticationTypeValue() == 0) { logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), "Missing auth provider type.", - "", Cause.MISSING_AUTH_PROVIDER_TYPE); + "", Cause.MISSING_AUTH_PROVIDER_TYPE, "Missing login option type"); return; } if (request.getAuthenticationProvider().getAuthenticationProvider() == null || request.getAuthenticationProvider().getAuthenticationProvider().isEmpty()) { logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), "Missing auth provider id.", - "", Cause.MISSING_AUTH_PROVIDER_ID); + "", Cause.MISSING_AUTH_PROVIDER_ID, "Missing login option value (id)"); return; } - Optional cause = authenticationProvider.validateDeleteAuthenticationProviderRequest(request); - if (cause.isPresent()) { + Validation validation = authenticationProvider.validateDeleteAuthenticationProviderRequest(request); + if (validation.hasErrors()) { logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), "Validation failed", "", - cause.get()); + validation.getFirstCause().get(), validation.getFirstMessage().get()); return; } @@ -855,7 +873,7 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas if (!permissionsValidator.isRpcAllowed(existingAccountsForProfile)) { logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), "Can not delete authentication provider from profile {}.", request.getProfileId(), - Cause.ERROR_PERMISSION_DENIED); + Cause.ERROR_PERMISSION_DENIED, "Permission denied"); return; } @@ -863,7 +881,7 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas Profile profile = profileRepository.findByProfileId(UUID.fromString(request.getProfileId())); if (profile == null) { logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), "Profile id {} missing in DB.", - request.getProfileId(), Cause.PROFILE_NOT_FOUND); + request.getProfileId(), Cause.PROFILE_NOT_FOUND, "Profile not found"); return; } @@ -881,7 +899,8 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas if (!existingAuthProviderToDelete.isPresent()) { logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), "Authentication provider {} is not used by this profile.", - authenticationProviderToDelete.toString(), Cause.AUTH_PROVIDER_NOT_FOUND); + authenticationProviderToDelete.toString(), Cause.AUTH_PROVIDER_NOT_FOUND, + "Login option to delete not found"); return; } @@ -889,7 +908,7 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas if (!removedFromObject) { logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), "Error removing authentication provider {}.", existingAuthProviderToDelete.get().toString(), - Cause.INTERNAL_SERVER_ERROR); + Cause.INTERNAL_SERVER_ERROR, "Error when removing login option"); return; } @@ -973,30 +992,33 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas logger.info("Editing contact info for account requested."); logger.debug("Editing contact info for account requested: {}", request); + // TODO use Validation object Optional> validationResultEditContactInfo = account.validateEditContactInfoRequest( request.getAccountId(), request.getOldContactInfo(), request.getEditedContactInfo()); if (validationResultEditContactInfo.isPresent()) { logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), validationResultEditContactInfo.get().getValue(), "", - validationResultEditContactInfo.get().getKey()); + validationResultEditContactInfo.get().getKey(), validationResultEditContactInfo.get().getValue()); return; } if (!util.isValidUuid(request.getAccountId())) { logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), "Invalid account id: {}", - request.getAccountId(), Cause.INVALID_ACCOUNT_ID); + request.getAccountId(), Cause.INVALID_ACCOUNT_ID, "Invalid Account ID"); return; } if (!permissionsValidator.isRpcAllowed(request.getAccountId())) { logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), - "Can not edit contact info for account {}.", request.getAccountId(), Cause.ERROR_PERMISSION_DENIED); + "Can not edit contact info for account {}.", request.getAccountId(), Cause.ERROR_PERMISSION_DENIED, + "Permission denied"); return; } if (!request.getOldContactInfo().getType().equals(request.getEditedContactInfo().getType())) { + // TODO we must extend log method to support multiple parameters logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), "Error editing Contact info for account {}. Different types: {} and {}.", request.getAccountId(), - Cause.ERROR_EDITING_CONTACT_INFO); + Cause.ERROR_EDITING_CONTACT_INFO, "Failed to update contact info - wrong type."); return; } if (request.getOldContactInfo().getType() == ContactType.PHONE_CONTACT) { @@ -1004,7 +1026,8 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas request.getEditedContactInfo().getLabel()); if (validation.hasErrors()) { logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), - validation.getErrorMessage(), "", validation.getCause().get()); + validation.getErrorMessage(), "", validation.getFirstCause().get(), + validation.getFirstMessage().get()); return; } request = phoneNumberNormalizer.normalizePhoneNumbers(request); @@ -1016,7 +1039,7 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas if (!result) { logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), "Contact info was not edited for account {}.", request.getAccountId(), - Cause.ERROR_EDITING_CONTACT_INFO); + Cause.ERROR_EDITING_CONTACT_INFO, "Failed to update contact info."); return; } logger.info("SUCCESS: Edited Contact Info {}:{} to {}:{} for account {}.", @@ -1049,18 +1072,20 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas logger.info("Updating auth provider for profile {} requested.", request.getProfileId()); logger.debug("Updating auth provider for profile requested: {}", request); + // Use validation object Optional> validationResultUpdateAuthProvider = authenticationProvider .validateUpdateAuthProviderRequest(request.getProfileId(), request.getOldAuthProvider(), request.getUpdatedAuthProvider()); if (validationResultUpdateAuthProvider.isPresent()) { logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), "Auth provider was not updated: {}.", validationResultUpdateAuthProvider.get().getValue(), - validationResultUpdateAuthProvider.get().getKey()); + validationResultUpdateAuthProvider.get().getKey(), + validationResultUpdateAuthProvider.get().getValue()); return; } if (!util.isValidUuid(request.getProfileId())) { logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), "Invalid profile id: {}.", - request.getProfileId(), validationResultUpdateAuthProvider.get().getKey()); + request.getProfileId(), validationResultUpdateAuthProvider.get().getKey(), "Invalid profile id."); return; } @@ -1070,21 +1095,22 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas if (!permissionsValidator.isRpcAllowed(existingAccountsForProfile)) { logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), "Can not update authentication provider in profile {}.", request.getProfileId(), - Cause.ERROR_PERMISSION_DENIED); + Cause.ERROR_PERMISSION_DENIED, "Permission denied"); return; } if (!authenticationProvider .canUseAsAdditionalLoginOption(request.getUpdatedAuthProvider().getAuthenticationType())) { logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), - "Invalid additional login option type used.", "", Cause.INVALID_ADDITIONAL_LOGIN_OPTION_TYPE); + "Invalid additional login option type used.", "", Cause.INVALID_ADDITIONAL_LOGIN_OPTION_TYPE, + "Invalid additional login option type used."); return; } if (request.getOldAuthProvider().equals(request.getUpdatedAuthProvider())) { logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), "The same old and new auth providers requested to update for profile {}.", request.getProfileId(), - Cause.ERROR_UPDATING_AUTH_PROVIDER); + Cause.ERROR_UPDATING_AUTH_PROVIDER, "Failed to update login option."); return; } if (request.getOldAuthProvider().getAuthenticationTypeValue() == AuthenticationType.PHONE_VALUE @@ -1102,7 +1128,7 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas request.getOldAuthProvider().getAuthenticationProvider(), request.getProfileId()); logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), "Auth provider was not updated for profile {}.", request.getProfileId(), - Cause.ERROR_UPDATING_AUTH_PROVIDER); + Cause.ERROR_UPDATING_AUTH_PROVIDER, ""); return; } logger.info("SUCCESS: Updated auth provider {}:{} to {}:{} for profile {}.", @@ -1126,7 +1152,7 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas Validation validation = validateGetAccountByLoginOptionRequest(request); if (validation.hasErrors()) { logAndBuildGrpcAccountResponse(responseObserver, AccountResponse.newBuilder(), validation.getErrorMessage(), - "", validation.getCause().get()); + "", validation.getCause().get(), validation.getErrorMessage()); return; } @@ -1134,13 +1160,14 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas Optional account = accountProvider.getAccountResponseByLoginOption( request.getAuthenticationType(), request.getAuthenticationIdentifier()); if (!account.isPresent()) { - logAndBuildGrpcAccountResponse( - responseObserver, AccountResponse.newBuilder(), "Account not found for login option: " - + request.getAuthenticationIdentifier() + ":" + request.getAuthenticationIdentifier(), - "", Cause.ACCOUNT_NOT_FOUND); + logAndBuildGrpcAccountResponse(responseObserver, AccountResponse.newBuilder(), + "Account not found for login option: " + request.getAuthenticationIdentifier() + ":" + + request.getAuthenticationIdentifier(), + "", Cause.ACCOUNT_NOT_FOUND, "Account not found"); } else { AccountResponse response = account.get(); - logger.info("SUCCESS: Found account by login option {}. Account Id={}.", request.getAuthenticationIdentifier(), response.getAccountDetails().getAccountId()); + logger.info("SUCCESS: Found account by login option {}. Account Id={}.", + request.getAuthenticationIdentifier(), response.getAccountDetails().getAccountId()); responseObserver.onNext(response); responseObserver.onCompleted(); } @@ -1150,7 +1177,7 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas logger.debug("Error getting account by login option {}:{}: {}", request.getAuthenticationType(), request.getAuthenticationIdentifier(), e.getCause()); logAndBuildGrpcAccountResponse(responseObserver, AccountResponse.newBuilder(), - Cause.INTERNAL_SERVER_ERROR.toString(), "", Cause.INTERNAL_SERVER_ERROR); + Cause.INTERNAL_SERVER_ERROR.toString(), "", Cause.INTERNAL_SERVER_ERROR, "Internal server error"); } } @@ -1195,8 +1222,7 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas account.get().getAvatar(), account.get().getFirstName(), account.get().getLastName()); SearchResponse response = SearchResponse.newBuilder().setSearchResultDetails(searchResultDetails).build(); - logger.info("SUCCESS: Found result for account by social provider {}.", - request.getAuthenticationIdentifier()); + logger.info("SUCCESS: Found result for account by social provider {}.", request.getAuthenticationIdentifier()); responseObserver.onNext(response); responseObserver.onCompleted(); } @@ -1214,12 +1240,12 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas request.getAuthenticationType(), request.getAuthenticationIdentifier()); if (validation.hasErrors()) { logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), validation.getErrorMessage(), - "", validation.getCause().get()); + "", validation.getFirstCause().get(), validation.getFirstMessage().get()); return; } if (!util.isValidUuid(request.getProfileId())) { logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), "Invalid profile id: {}", - request.getProfileId(), Cause.INVALID_PROFILE_ID); + request.getProfileId(), Cause.INVALID_PROFILE_ID, "Invalid profile id"); return; } @@ -1229,7 +1255,7 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas if (!permissionsValidator.isRpcAllowed(existingAccountsForProfile)) { logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), "Can not update searchable option for authentication provider in profile {}.", - request.getProfileId(), Cause.ERROR_PERMISSION_DENIED); + request.getProfileId(), Cause.ERROR_PERMISSION_DENIED, "Permission denied"); return; } @@ -1237,7 +1263,8 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas request.getAuthenticationType(), request.getAuthenticationIdentifier(), request.getSearchOption())) { logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), "Can not update searchable option for authentication provider in profile {}.", - request.getProfileId(), Cause.ERROR_UPDATING_SEARCHABLE_OPTION); + request.getProfileId(), Cause.ERROR_UPDATING_SEARCHABLE_OPTION, + "Failed to update searchable option"); return; } @@ -1257,9 +1284,10 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas } private static void logAndBuildGrpcAccountResponse(StreamObserver responseObserver, - AccountResponse.Builder newBuilder, String logMessage, String logValue, Cause cause) { + AccountResponse.Builder newBuilder, String logMessage, String logValue, Cause cause, String errorMessage) { logger.error(logMessage, logValue); - responseObserver.onNext(newBuilder.setError(ErrorResponse.newBuilder().setCause(cause)).build()); + responseObserver.onNext( + newBuilder.setError(ErrorResponse.newBuilder().setCause(cause).setMessage(errorMessage)).build()); responseObserver.onCompleted(); } @@ -1278,9 +1306,10 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas } private static void logAndBuildGrpcStatusResponse(StreamObserver responseObserver, - StatusResponse.Builder newBuilder, String logMessage, String logValue, Cause cause) { + StatusResponse.Builder newBuilder, String logMessage, String logValue, Cause cause, String errorMessage) { logger.error(logMessage, logValue); - responseObserver.onNext(newBuilder.setError(ErrorResponse.newBuilder().setCause(cause)).build()); + responseObserver.onNext( + newBuilder.setError(ErrorResponse.newBuilder().setCause(cause).setMessage(errorMessage)).build()); responseObserver.onCompleted(); } diff --git a/src/main/java/biz/nynja/account/services/decomposition/AccountCreator.java b/src/main/java/biz/nynja/account/services/decomposition/AccountCreator.java index 794177b..8d31c91 100644 --- a/src/main/java/biz/nynja/account/services/decomposition/AccountCreator.java +++ b/src/main/java/biz/nynja/account/services/decomposition/AccountCreator.java @@ -6,7 +6,6 @@ package biz.nynja.account.services.decomposition; import static biz.nynja.account.validation.Validators.pendingAccount; import java.time.Instant; -import java.util.Optional; import java.util.UUID; import org.slf4j.Logger; @@ -29,6 +28,7 @@ import biz.nynja.account.models.PendingAccountByAuthenticationProvider; import biz.nynja.account.phone.PhoneNumberNormalizer; import biz.nynja.account.repositories.AccountRepositoryAdditional; import biz.nynja.account.repositories.PendingAccountRepository; +import biz.nynja.account.validation.Validation; @Service public class AccountCreator { @@ -129,7 +129,8 @@ public class AccountCreator { request.getAuthenticationType().name(), request.getAuthenticationProvider()))) { return logAndBuildGrpcPendingAccountResponse(CreatePendingAccountResponse.newBuilder(), "Error creating pending account. Authentication provider: {} already used.", - request.getAuthenticationProvider(), Cause.AUTH_PROVIDER_ALREADY_USED); + request.getAuthenticationProvider(), Cause.AUTH_PROVIDER_ALREADY_USED, + "Login option is already used"); } return null; } @@ -152,7 +153,8 @@ public class AccountCreator { if (createdAccount == null) { return logAndBuildGrpcAccountResponse(AccountResponse.newBuilder(), - "Error creating account with useraname: {}", request.getUsername(), Cause.ERROR_CREATING_ACCOUNT); + "Error creating account with useraname: {}", request.getUsername(), Cause.ERROR_CREATING_ACCOUNT, + "Failed to create account"); } else { logger.debug("Account \"{}\" saved into the DB", createdAccount); AccountDetails details = createdAccount.toProto(); @@ -163,28 +165,30 @@ public class AccountCreator { private AccountResponse validateCompletePendingAccountCreationRequest( CompletePendingAccountCreationRequest request) { - Cause cause = pendingAccount.validateCompletePendingAccountCreationRequest(request); - if (cause != null) { - return logAndBuildGrpcAccountResponse(AccountResponse.newBuilder(), "Validation failed", "", cause); + Validation validation = pendingAccount.validateCompletePendingAccountCreationRequest(request); + if (validation.hasErrors()) { + return logAndBuildGrpcAccountResponse(AccountResponse.newBuilder(), "Validation failed: " + validation.getErrorMessage(), + "", validation.getFirstCause().get(), validation.getFirstMessage().get()); } if (request.getUsername() != null && !request.getUsername().trim().isEmpty() && accountRepositoryAdditional .foundExistingNotOwnUsername(UUID.fromString(request.getAccountId()), request.getUsername())) { return logAndBuildGrpcAccountResponse(AccountResponse.newBuilder(), "User name already in use: {}", - request.getUsername(), Cause.USERNAME_ALREADY_USED); + request.getUsername(), Cause.USERNAME_ALREADY_USED, "Username is already used"); } return null; } private static CreatePendingAccountResponse logAndBuildGrpcPendingAccountResponse( - CreatePendingAccountResponse.Builder newBuilder, String logMessage, String logValue, Cause cause) { + CreatePendingAccountResponse.Builder newBuilder, String logMessage, String logValue, Cause cause, + String errorMessage) { logger.error(logMessage, logValue); - return newBuilder.setError(ErrorResponse.newBuilder().setCause(cause)).build(); + return newBuilder.setError(ErrorResponse.newBuilder().setCause(cause).setMessage(errorMessage)).build(); } private static AccountResponse logAndBuildGrpcAccountResponse(AccountResponse.Builder newBuilder, String logMessage, - String logValue, Cause cause) { + String logValue, Cause cause, String errorMessage) { logger.error(logMessage, logValue); - return newBuilder.setError(ErrorResponse.newBuilder().setCause(cause)).build(); + return newBuilder.setError(ErrorResponse.newBuilder().setCause(cause).setMessage(errorMessage)).build(); } } diff --git a/src/main/java/biz/nynja/account/validation/Validation.java b/src/main/java/biz/nynja/account/validation/Validation.java index 9ae0982..c157b4e 100644 --- a/src/main/java/biz/nynja/account/validation/Validation.java +++ b/src/main/java/biz/nynja/account/validation/Validation.java @@ -39,7 +39,7 @@ public class Validation { for (ValidationError error : errors) { builder.append("Message: ").append(error.getMessage()).append(". "); if (error.getCause() != null) { - builder.append("Cause: ").append(error.getCause()); + builder.append("Cause:").append(error.getCause()).append(";"); } } @@ -59,4 +59,19 @@ public class Validation { } + // TODO this is a workaround for now as we have a single cause and error + public Optional getFirstCause() { + if (!hasErrors()) { + return Optional.empty(); + } + return Optional.of(errors.get(0).getCause()); + } + + public Optional getFirstMessage() { + if (!hasErrors()) { + return Optional.empty(); + } + return Optional.of(errors.get(0).getMessage()); + } + } diff --git a/src/main/java/biz/nynja/account/validation/Validators.java b/src/main/java/biz/nynja/account/validation/Validators.java index 801949e..38ab7a8 100644 --- a/src/main/java/biz/nynja/account/validation/Validators.java +++ b/src/main/java/biz/nynja/account/validation/Validators.java @@ -154,30 +154,32 @@ public class Validators { return validation; } - public Optional validateUpdateAccountRequest(UpdateAccountRequest request) { + public Validation validateUpdateAccountRequest(UpdateAccountRequest request) { + Validation validation = new Validation(); + if (!util.isValidUuid(request.getAccountId())) { - return Optional.of(Cause.INVALID_ACCOUNT_ID); + validation.addError(new ValidationError("Invalid Account ID", Cause.INVALID_ACCOUNT_ID)); } if (request.getUsername() != null && !request.getUsername().trim().isEmpty() && !isUsernameValid(request.getUsername())) { - return Optional.of(Cause.INVALID_USERNAME); + validation.addError(new ValidationError("Invalid username", Cause.INVALID_USERNAME)); } if (request.getFirstName() != null && request.getFirstName().trim().isEmpty()) { - return Optional.of(Cause.MISSING_FIRST_NAME); + validation.addError(new ValidationError("Missing first name", Cause.MISSING_FIRST_NAME)); } else if (!isFirstNameValid(request.getFirstName())) { - return Optional.of(Cause.INVALID_FIRST_NAME); + validation.addError(new ValidationError("Invalid first name", Cause.INVALID_FIRST_NAME)); } if (request.getLastName() != null && !request.getLastName().trim().isEmpty() && !isLastNameValid(request.getLastName())) { - return Optional.of(Cause.INVALID_LAST_NAME); + validation.addError(new ValidationError("Invalid last name", Cause.INVALID_LAST_NAME)); } if (request.getAccountName() != null && !request.getAccountName().trim().isEmpty() && !isAccountNameValid(request.getAccountName())) { - return Optional.of(Cause.INVALID_ACCOUNT_NAME); + validation.addError(new ValidationError("Invalid account name", Cause.INVALID_ACCOUNT_NAME)); } if (util.validateBirthdayIsSet(request.getBirthday())) { @@ -185,10 +187,10 @@ public class Validators { LocalDate.of(request.getBirthday().getYear(), request.getBirthday().getMonth(), request.getBirthday().getDay()); } catch (DateTimeException e) { - return Optional.of(Cause.INVALID_BIRTHDAY_DATE); + validation.addError(new ValidationError("Invalid birthday format", Cause.INVALID_BIRTHDAY_DATE)); } } - return Optional.empty(); + return validation; } public Optional> validateContactInfo(ContactType type, String contactInfoValue) { @@ -233,7 +235,7 @@ public class Validators { public Optional> validateContactInfoRequest(String accountId, ContactDetails contactDetails) { if ((accountId == null) || (accountId.isEmpty())) { - return Optional.of(new ImmutablePair<>(Cause.MISSING_ACCOUNT_ID, "Missing account id")); + return Optional.of(new ImmutablePair<>(Cause.MISSING_ACCOUNT_ID, "Missing account ID")); } if (contactDetails.getTypeValue() == 0) { return Optional.of(new ImmutablePair<>(Cause.MISSING_CONTACT_INFO_TYPE, "Missing contact info type")); @@ -321,32 +323,47 @@ public class Validators { } } - public Optional validateDeleteAuthenticationProviderRequest( - DeleteAuthenticationProviderRequest request) { + public Validation validateDeleteAuthenticationProviderRequest(DeleteAuthenticationProviderRequest request) { + Validation validation = new Validation(); if (!isValidUuid(request.getProfileId())) { - return Optional.of(Cause.INVALID_PROFILE_ID); + validation.addError(new ValidationError("Invalid profile id", Cause.INVALID_PROFILE_ID)); } - return validateAuthenticationProvider(request.getAuthenticationProvider().getAuthenticationType(), + + Optional authenticationProviderValidation = validateAuthenticationProvider( + request.getAuthenticationProvider().getAuthenticationType(), request.getAuthenticationProvider().getAuthenticationProvider(), true); + if (authenticationProviderValidation.isPresent()) { + validation.addError(new ValidationError( + "Failed deleting a login option due to error" + authenticationProviderValidation.get(), + authenticationProviderValidation.get())); + } + + return validation; } - public Optional validateAddAuthenticationProviderRequest(AddAuthenticationProviderRequest request) { + public Validation validateAddAuthenticationProviderRequest(AddAuthenticationProviderRequest request) { + Validation validation = new Validation(); if (!isValidUuid(request.getProfileId())) { - return Optional.of(Cause.INVALID_PROFILE_ID); + validation.addError(new ValidationError("Invalid profile id", Cause.INVALID_PROFILE_ID)); } Optional authenticationProviderValidation = validateAuthenticationProvider( request.getAuthenticationProvider().getAuthenticationType(), request.getAuthenticationProvider().getAuthenticationProvider()); if (authenticationProviderValidation.isPresent()) { - return authenticationProviderValidation; + validation.addError(new ValidationError( + "Failed adding a login option due to error: " + authenticationProviderValidation.get(), + authenticationProviderValidation.get())); + return validation; } boolean typeCanBeUsed = authenticationProvider .canUseAsAdditionalLoginOption(request.getAuthenticationProvider().getAuthenticationType()); if (typeCanBeUsed) { - return Optional.empty(); + return validation; } else { - return Optional.of(Cause.INVALID_ADDITIONAL_LOGIN_OPTION_TYPE); + validation.addError(new ValidationError("Invalid additional login option type", + Cause.INVALID_ADDITIONAL_LOGIN_OPTION_TYPE)); } + return validation; } public Validation validateUpdateSearchableOptionRequest(String profileId, String authenticationType, @@ -479,36 +496,38 @@ public class Validators { request.getAuthenticationProvider()); } - public Cause validateCompletePendingAccountCreationRequest(CompletePendingAccountCreationRequest request) { + public Validation validateCompletePendingAccountCreationRequest(CompletePendingAccountCreationRequest request) { + Validation validation = new Validation(); + if (request.getAccountId() == null || request.getAccountId().trim().isEmpty()) { - return Cause.MISSING_ACCOUNT_ID; + validation.addError(new ValidationError("Missing Account ID", Cause.MISSING_ACCOUNT_ID)); } if (!Validators.util.isValidUuid(request.getAccountId())) { - return Cause.INVALID_ACCOUNT_ID; + validation.addError(new ValidationError("Invalid Account ID", Cause.INVALID_ACCOUNT_ID)); } if (request.getFirstName() != null && request.getFirstName().trim().isEmpty()) { - return Cause.MISSING_FIRST_NAME; + validation.addError(new ValidationError("Missing first name", Cause.MISSING_FIRST_NAME)); } else if (!account.isFirstNameValid(request.getFirstName())) { - return Cause.INVALID_FIRST_NAME; + validation.addError(new ValidationError("Invalid first name", Cause.INVALID_FIRST_NAME)); } if (request.getLastName() != null && !request.getLastName().trim().isEmpty() && !account.isLastNameValid(request.getLastName())) { - return Cause.INVALID_LAST_NAME; + validation.addError(new ValidationError("Invalid last name", Cause.INVALID_LAST_NAME)); } if (request.getUsername() != null && !request.getUsername().trim().isEmpty() && !account.isUsernameValid(request.getUsername())) { - return Cause.INVALID_USERNAME; + validation.addError(new ValidationError("Invalid username", Cause.INVALID_USERNAME)); } if (request.getAccountName() != null && !request.getAccountName().trim().isEmpty() && !account.isAccountNameValid(request.getAccountName())) { - return Cause.INVALID_ACCOUNT_NAME; + validation.addError(new ValidationError("Invalid account name", Cause.INVALID_ACCOUNT_NAME)); } - return null; + return validation; } } diff --git a/src/test/java/biz/nynja/account/components/ValidatorTests.java b/src/test/java/biz/nynja/account/components/ValidatorTests.java index ec44798..2c4c4a3 100644 --- a/src/test/java/biz/nynja/account/components/ValidatorTests.java +++ b/src/test/java/biz/nynja/account/components/ValidatorTests.java @@ -11,7 +11,6 @@ import static biz.nynja.account.validation.Validators.util; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotEquals; -import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import java.util.NoSuchElementException; @@ -188,8 +187,8 @@ public class ValidatorTests { @Test(expected = NoSuchElementException.class) public void validateAuthProviderValidTest() { - authenticationProvider.validateAuthenticationProvider(AuthenticationType.EMAIL, - "valid.E-mail1@domain-sub.test.com").get(); + authenticationProvider + .validateAuthenticationProvider(AuthenticationType.EMAIL, "valid.E-mail1@domain-sub.test.com").get(); } @Test @@ -201,26 +200,22 @@ public class ValidatorTests { @Test public void validateAdditionalLoginOptionGoogleplusInvalidTest() { - assertEquals(authenticationProvider - .canUseAsAdditionalLoginOption(AuthenticationType.GOOGLEPLUS), false); + assertEquals(authenticationProvider.canUseAsAdditionalLoginOption(AuthenticationType.GOOGLEPLUS), false); } @Test public void validateAdditionalLoginOptionFacebookInvalidTest() { - assertEquals(authenticationProvider - .canUseAsAdditionalLoginOption(AuthenticationType.FACEBOOK), false); + assertEquals(authenticationProvider.canUseAsAdditionalLoginOption(AuthenticationType.FACEBOOK), false); } @Test public void validateAdditionalLoginOptionPhoneValidTest() { - assertEquals(authenticationProvider - .canUseAsAdditionalLoginOption(AuthenticationType.PHONE), true); + assertEquals(authenticationProvider.canUseAsAdditionalLoginOption(AuthenticationType.PHONE), true); } @Test public void validateAdditionalLoginOptionEmailValidTest() { - assertEquals(authenticationProvider - .canUseAsAdditionalLoginOption(AuthenticationType.EMAIL), true); + assertEquals(authenticationProvider.canUseAsAdditionalLoginOption(AuthenticationType.EMAIL), true); } @Test @@ -259,7 +254,7 @@ public class ValidatorTests { CompletePendingAccountCreationRequest request = CompletePendingAccountCreationRequest.newBuilder() .setAccountId(Util.ACCOUNT_ID.toString()).setFirstName(Util.FIRST_NAME).setLastName(Util.LAST_NAME) .setUsername(Util.USERNAME).build(); - assertNull(pendingAccount.validateCompletePendingAccountCreationRequest(request)); + assertFalse(pendingAccount.validateCompletePendingAccountCreationRequest(request).getFirstCause().isPresent()); } @Test @@ -267,14 +262,16 @@ public class ValidatorTests { CompletePendingAccountCreationRequest request = CompletePendingAccountCreationRequest.newBuilder() .setAccountId(Util.ACCOUNT_ID.toString()).setFirstName(Util.FIRST_NAME).setLastName(Util.LAST_NAME) .setUsername("@alabala").build(); - assertEquals(Cause.INVALID_USERNAME, pendingAccount.validateCompletePendingAccountCreationRequest(request)); + assertEquals(Cause.INVALID_USERNAME, + pendingAccount.validateCompletePendingAccountCreationRequest(request).getFirstCause().get()); } @Test public void validateCompletePendingAccountCreationRequestMissingAccountIdTest() { CompletePendingAccountCreationRequest request = CompletePendingAccountCreationRequest.newBuilder() .setFirstName(Util.FIRST_NAME).setLastName(Util.LAST_NAME).setUsername(Util.USERNAME).build(); - assertEquals(Cause.MISSING_ACCOUNT_ID, pendingAccount.validateCompletePendingAccountCreationRequest(request)); + assertEquals(Cause.MISSING_ACCOUNT_ID, + pendingAccount.validateCompletePendingAccountCreationRequest(request).getFirstCause().get()); } @Test(expected = NoSuchElementException.class) @@ -292,7 +289,7 @@ public class ValidatorTests { .setAuthenticationProvider("invalid.E-mail1.@domain_test.com1").build(); assertEquals(Cause.INVALID_EMAIL, pendingAccount.validateCreatePendingAccountRequest(request).get()); } - + @Test public void testCountryCodeGetterValidNumber() { String countryCode = PhoneNumberUtils.getCountrySelector("359888888888"); // Bulgaria @@ -300,7 +297,7 @@ public class ValidatorTests { countryCode = PhoneNumberUtils.getCountrySelector("1888888888"); // United states assertEquals("US", countryCode); - + countryCode = PhoneNumberUtils.getCountrySelector("380888888888"); // Ukrain assertEquals("UA", countryCode); } -- GitLab From 65d950c9201f28ce42fbf719f005c573d5829e78 Mon Sep 17 00:00:00 2001 From: Stoyan Tzenkov Date: Wed, 30 Jan 2019 15:33:29 +0200 Subject: [PATCH 15/20] NY-6804: Delete ACCOUNT_ADMIN accesspoints when ACCUNT_ADMIN role is taken away from user. Signed-off-by: Stoyan Tzenkov --- .../accesspoints/AccessPointRepository.java | 9 +++ .../accesspoints/AccessPointService.java | 41 +++++++++++++ .../AccountRepositoryAdditionalImpl.java | 57 +++++++++++++++++-- 3 files changed, 101 insertions(+), 6 deletions(-) diff --git a/src/main/java/biz/nynja/account/accesspoints/AccessPointRepository.java b/src/main/java/biz/nynja/account/accesspoints/AccessPointRepository.java index 84b4c0d..3fe188d 100644 --- a/src/main/java/biz/nynja/account/accesspoints/AccessPointRepository.java +++ b/src/main/java/biz/nynja/account/accesspoints/AccessPointRepository.java @@ -24,4 +24,13 @@ public interface AccessPointRepository @Query("delete from auth.accesspoint where accountid=:accountId;") void deleteById(@Param("accountId") UUID accountId); + + @Query("delete from auth.accesspoint where accountid=:accountId and accesstoken=:accessToken;") + void deleteByPrimary(@Param("accountId") UUID accountId, @Param("accessToken") String accessToken); + + @Query("INSERT INTO auth.accesspoint (accountid, deviceid, accesstoken, creationtimestamp, expirationtimestamp) VALUES ( :accountId, :deviceId, :accessToken, :creationTimestamp, :expirationTimestamp ) USING TTL :expiresIn;") + void save(@Param("accountId") UUID accountId, @Param("deviceId") String deviceId, + @Param("accessToken") String accessToken, @Param("creationTimestamp") long creationTimestamp, + @Param("expirationTimestamp") long expirationTimestamp, @Param("expiresIn") int expiresIn); + } diff --git a/src/main/java/biz/nynja/account/accesspoints/AccessPointService.java b/src/main/java/biz/nynja/account/accesspoints/AccessPointService.java index 74cc4af..3c02e91 100644 --- a/src/main/java/biz/nynja/account/accesspoints/AccessPointService.java +++ b/src/main/java/biz/nynja/account/accesspoints/AccessPointService.java @@ -4,6 +4,7 @@ package biz.nynja.account.accesspoints; import java.util.Date; +import java.util.List; import java.util.Optional; import java.util.UUID; @@ -11,6 +12,8 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.stereotype.Service; +import biz.nynja.account.accesspoints.AccessPoint; + @Service public class AccessPointService { private final Logger logger = LoggerFactory.getLogger(AccessPointService.class); @@ -20,6 +23,19 @@ public class AccessPointService { this.accessPointRepository = accessPointRepository; } + public Optional saveAccessPoint(AccessPoint accessPoint) { + int expiresIn = (int) (accessPoint.getExpirationTimestamp() - accessPoint.getCreationTimestamp()); + try { + accessPointRepository.save(accessPoint.getAccountId(), accessPoint.getDeviceId(), + accessPoint.getAccessToken(), accessPoint.getCreationTimestamp(), + accessPoint.getExpirationTimestamp(), expiresIn); + } catch (Exception e) { + logger.error("Error saving accesspoint record for accoint ID {}. Error message: {}", + accessPoint.getAccountId(), e.getMessage()); + } + return Optional.empty(); + } + public Optional getAccessPoint(UUID accountId, String accessToken) { try { AccessPoint accessPoint = accessPointRepository.findByAccountIdAndAccessToken(accountId, accessToken); @@ -33,6 +49,31 @@ public class AccessPointService { return Optional.empty(); } + public Optional> getAllAccessPointsForAccount(UUID accountId) { + try { + List accessPointsForAccount = accessPointRepository.findAllByAccountId(accountId); + if (accessPointsForAccount != null) { + return Optional.of(accessPointsForAccount); + } + } catch (Exception e) { + logger.error("Error getting accesspoints for account {}. Message: {}, cause: {}", accountId, e.getMessage(), + e.getCause()); + } + return Optional.empty(); + } + + public boolean deleteAccessPoint(UUID accountId, String accessToken) { + try { + accessPointRepository.deleteByPrimary(accountId, accessToken); + logger.info("AccessPoint successfully deleted from the DB."); + return true; + } catch (Exception e) { + logger.error("Error deleting access point record for account {} from DB. Message: {}, cause: {}.", accountId, + e.getMessage(), e.getCause()); + return false; + } + } + public boolean deleteAccessPointsForAccount(UUID accountId) { try { accessPointRepository.deleteById(accountId); diff --git a/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java b/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java index 48aaa80..d5639c4 100644 --- a/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java +++ b/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java @@ -29,12 +29,16 @@ import org.springframework.data.cassandra.core.WriteResult; import org.springframework.data.cassandra.core.cql.CqlOperations; import org.springframework.stereotype.Service; +import com.auth0.jwt.JWT; +import com.auth0.jwt.interfaces.Claim; +import com.auth0.jwt.interfaces.DecodedJWT; import com.datastax.driver.core.BatchStatement; import com.datastax.driver.core.BoundStatement; import com.datastax.driver.core.PreparedStatement; import com.datastax.driver.core.ResultSet; import com.datastax.driver.core.Session; +import biz.nynja.account.accesspoints.AccessPoint; import biz.nynja.account.accesspoints.AccessPointService; import biz.nynja.account.components.AccountServiceHelper; import biz.nynja.account.components.StatementsPool; @@ -113,7 +117,7 @@ public class AccountRepositoryAdditionalImpl implements AccountRepositoryAdditio this.erlangAccountBridge = erlangAccountBridge; this.permissionsValidator = permissionsValidator; this.accountDataConfiguration = accountDataConfiguration; - this.accessPointService= accessPointService; + this.accessPointService = accessPointService; } @PostConstruct @@ -217,18 +221,18 @@ public class AccountRepositoryAdditionalImpl implements AccountRepositoryAdditio public Account updateAccount(UpdateAccountRequest request) { Transaction sagaTransaction = new SagaTransaction(cassandraTemplate); - + Account existingAccount = accountRepository.findByAccountId(UUID.fromString(request.getAccountId())); if (existingAccount == null) { logger.error("Existing account with the provided id {} was not found.", request.getAccountId()); logger.debug("Existing account with the provided id {} was not found.", request.getAccountId()); return null; } - + + Set existingRoles = existingAccount.getRoles().stream().map(Role::valueOf).collect(Collectors.toSet()); if (!permissionsValidator.isAdminToken()) { // No permission to update roles, load old ones - Set roles = existingAccount.getRoles().stream().map(Role::valueOf).collect(Collectors.toSet()); - request = UpdateAccountRequest.newBuilder(request).clearRoles().addAllRoles(roles).build(); +// request = UpdateAccountRequest.newBuilder(request).clearRoles().addAllRoles(existingRoles).build(); } Long timeUpdated = Instant.now().toEpochMilli(); WriteResult wr = null; @@ -262,6 +266,14 @@ public class AccountRepositoryAdditionalImpl implements AccountRepositoryAdditio || request.getAccessStatus().equals(AccessStatus.SUSPENDED)) { if (!accessPointService.deleteAccessPointsForAccount(UUID.fromString(request.getAccountId()))) { logger.error("Error deleting accesspoints from the DB for account {}.", request.getAccountId()); + return null; + } + } + Set newRoles = new HashSet(request.getRolesList()); + if (existingRoles.contains(Role.ACCOUNT_ADMIN) && newRoles.size() == 1 + && newRoles.contains(Role.USER)) { + if (!deleteADMINAccessPoints(request.getAccountId())) { + return null; } } Account updatedAccount = accountRepository.findByAccountId(UUID.fromString(request.getAccountId())); @@ -271,6 +283,38 @@ public class AccountRepositoryAdditionalImpl implements AccountRepositoryAdditio return null; } + private boolean deleteADMINAccessPoints(String accountId) { + Optional> accessPointsForAccount = accessPointService + .getAllAccessPointsForAccount(UUID.fromString(accountId)); + if (accessPointsForAccount.isPresent()) { + for (AccessPoint accessPoint : accessPointsForAccount.get()) { + String accessToken = accessPoint.getAccessToken(); + DecodedJWT decodedToken = JWT.decode(accessToken); + List accesspointRoles = getRolesFromAccessToken(decodedToken); + if (accesspointRoles.contains(Role.ACCOUNT_ADMIN.name())) { + if (!accessPointService.deleteAccessPoint(UUID.fromString(accountId), accessToken)) { + logger.error("Error deleting accesspoint record from DB for account with id {} and role {}.", + accountId, Role.ACCOUNT_ADMIN.name()); + return false; + } + + } + } + } + return true; + } + + private List getRolesFromAccessToken(DecodedJWT decodedToken) { + // Get roles from access token + List requestingRoles = null; + + Claim claim = decodedToken.getClaim("roles"); + if (claim != null) { + requestingRoles = claim.asList(String.class); + } + return requestingRoles; + } + public boolean deleteAccount(UUID accountId) { Transaction sagaTransaction = new SagaTransaction(cassandraTemplate); Account existingAccount = accountRepository.findByAccountId(accountId); @@ -959,7 +1003,8 @@ public class AccountRepositoryAdditionalImpl implements AccountRepositoryAdditio return Optional.empty(); } if (!profileByAuthProvider.getSearchable().booleanValue()) { - logger.info("Record found in DB but is not searchable for the profileId: {}", profileByAuthProvider.getProfileId()); + logger.info("Record found in DB but is not searchable for the profileId: {}", + profileByAuthProvider.getProfileId()); return Optional.empty(); } -- GitLab From 7c75713c89684c927dda9e544d3c7add48e33908 Mon Sep 17 00:00:00 2001 From: Stoyan Tzenkov Date: Wed, 30 Jan 2019 16:02:50 +0200 Subject: [PATCH 16/20] NY-6804: Some corrections. Signed-off-by: Stoyan Tzenkov --- .../repositories/AccountRepositoryAdditionalImpl.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java b/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java index d5639c4..dccb8f8 100644 --- a/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java +++ b/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java @@ -232,7 +232,7 @@ public class AccountRepositoryAdditionalImpl implements AccountRepositoryAdditio Set existingRoles = existingAccount.getRoles().stream().map(Role::valueOf).collect(Collectors.toSet()); if (!permissionsValidator.isAdminToken()) { // No permission to update roles, load old ones -// request = UpdateAccountRequest.newBuilder(request).clearRoles().addAllRoles(existingRoles).build(); + request = UpdateAccountRequest.newBuilder(request).clearRoles().addAllRoles(existingRoles).build(); } Long timeUpdated = Instant.now().toEpochMilli(); WriteResult wr = null; @@ -270,8 +270,8 @@ public class AccountRepositoryAdditionalImpl implements AccountRepositoryAdditio } } Set newRoles = new HashSet(request.getRolesList()); - if (existingRoles.contains(Role.ACCOUNT_ADMIN) && newRoles.size() == 1 - && newRoles.contains(Role.USER)) { + if (existingRoles.size() > 1 && existingRoles.contains(Role.USER) + && newRoles.size() == 1 && newRoles.contains(Role.USER)) { if (!deleteADMINAccessPoints(request.getAccountId())) { return null; } @@ -291,7 +291,7 @@ public class AccountRepositoryAdditionalImpl implements AccountRepositoryAdditio String accessToken = accessPoint.getAccessToken(); DecodedJWT decodedToken = JWT.decode(accessToken); List accesspointRoles = getRolesFromAccessToken(decodedToken); - if (accesspointRoles.contains(Role.ACCOUNT_ADMIN.name())) { + if (accesspointRoles.contains(Role.ACCOUNT_ADMIN.name()) || accesspointRoles.contains(Role.AUTHENTICATION_ADMIN.name())) { if (!accessPointService.deleteAccessPoint(UUID.fromString(accountId), accessToken)) { logger.error("Error deleting accesspoint record from DB for account with id {} and role {}.", accountId, Role.ACCOUNT_ADMIN.name()); -- GitLab From 0572d204a100f5ac1767480b8545e318c797200c Mon Sep 17 00:00:00 2001 From: Stoyan Tzenkov Date: Wed, 30 Jan 2019 16:33:46 +0200 Subject: [PATCH 17/20] NY-6804: Condition refined. Signed-off-by: Stoyan Tzenkov --- .../account/repositories/AccountRepositoryAdditionalImpl.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java b/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java index dccb8f8..5b1c412 100644 --- a/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java +++ b/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java @@ -270,8 +270,7 @@ public class AccountRepositoryAdditionalImpl implements AccountRepositoryAdditio } } Set newRoles = new HashSet(request.getRolesList()); - if (existingRoles.size() > 1 && existingRoles.contains(Role.USER) - && newRoles.size() == 1 && newRoles.contains(Role.USER)) { + if (newRoles.size() != existingRoles.size() || !existingRoles.containsAll(newRoles)) { if (!deleteADMINAccessPoints(request.getAccountId())) { return null; } -- GitLab From 5235ed6fd8d5cda4eac9100bf2b28531115f1a38 Mon Sep 17 00:00:00 2001 From: Stoyan Tzenkov Date: Thu, 31 Jan 2019 09:41:05 +0200 Subject: [PATCH 18/20] Ny-6879: Accesspoints for account deleted whenever the account is deleted. Signed-off-by: Stoyan Tzenkov --- .../biz/nynja/account/services/AccountServiceImpl.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/main/java/biz/nynja/account/services/AccountServiceImpl.java b/src/main/java/biz/nynja/account/services/AccountServiceImpl.java index 191f704..e988a60 100644 --- a/src/main/java/biz/nynja/account/services/AccountServiceImpl.java +++ b/src/main/java/biz/nynja/account/services/AccountServiceImpl.java @@ -18,6 +18,7 @@ import org.lognet.springboot.grpc.GRpcService; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import biz.nynja.account.accesspoints.AccessPointService; import biz.nynja.account.configuration.ProfileDataConfiguration; import biz.nynja.account.grpc.AccountByAccountIdRequest; import biz.nynja.account.grpc.AccountResponse; @@ -103,6 +104,7 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas private final ProfileProvider profileProvider; private final PermissionsValidator permissionsValidator; private final ProfileDataConfiguration profileDataConfiguration; + private final AccessPointService accessPointService; public AccountServiceImpl(AccountRepositoryAdditional accountRepositoryAdditional, ProfileRepository profileRepository, @@ -111,7 +113,7 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas AccountByUsernameRepository accountByUsernameRepository, AccountProvider accountProvider, AccountByProfileIdRepository accountByProfileIdRepository, PhoneNumberNormalizer phoneNumberNormalizer, AccountCreator accountCreator, ProfileProvider profileProvider, PermissionsValidator permissionsValidator, - ProfileDataConfiguration profileDataConfiguration) { + ProfileDataConfiguration profileDataConfiguration, AccessPointService accessPointService) { this.accountRepositoryAdditional = accountRepositoryAdditional; this.profileRepository = profileRepository; this.profileByAutheticationProviderRepository = profileByAutheticationProviderRepository; @@ -124,6 +126,7 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas this.profileProvider = profileProvider; this.permissionsValidator = permissionsValidator; this.profileDataConfiguration = profileDataConfiguration; + this.accessPointService = accessPointService; } @Override @@ -584,6 +587,9 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas "Failed to delete account"); return; } + if (!accessPointService.deleteAccessPointsForAccount(UUID.fromString(request.getAccountId()))) { + logger.error("Error deleting accesspoints from the DB for account {}.", request.getAccountId()); + } logger.info("SUCCESS: Account successfully deleted: {}", request.getAccountId()); responseObserver.onNext(StatusResponse.newBuilder().setStatus("SUCCESS").build()); responseObserver.onCompleted(); -- GitLab From 01fa79ffda12adba20d5fa827b7f279511dbcceb Mon Sep 17 00:00:00 2001 From: Stoyan Tzenkov Date: Thu, 31 Jan 2019 12:15:21 +0200 Subject: [PATCH 19/20] NY-6879: Accesspoints deleted for account whenever profile deleted. Signed-off-by: Stoyan Tzenkov --- .../account/repositories/AccountRepositoryAdditionalImpl.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java b/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java index 5b1c412..b541a36 100644 --- a/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java +++ b/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java @@ -582,6 +582,10 @@ public class AccountRepositoryAdditionalImpl implements AccountRepositoryAdditio for (AccountByProfileId accountByProfileId : existingAccountsForProfile) { Account existingAccount = accountRepository.findByAccountId(accountByProfileId.getAccountId()); deleteAccountData(batchOperations, existingAccount); + if (!accessPointService.deleteAccessPointsForAccount(accountByProfileId.getAccountId())) { + logger.error("Error deleting accesspoints from the DB for account {}.", accountByProfileId.getAccountId()); + } + } } -- GitLab From 259c5dccd541854e573f30986ab65fe4bcb951a0 Mon Sep 17 00:00:00 2001 From: Dragomir Todorov Date: Thu, 31 Jan 2019 16:37:23 +0200 Subject: [PATCH 20/20] NY-6840: Delete user avatar --- .../biz/nynja/account/models/Account.java | 43 ++++++++++++++++- .../account/services/AccountServiceImpl.java | 47 +++++++++++++++++++ .../decomposition/AccountProvider.java | 11 ++++- 3 files changed, 99 insertions(+), 2 deletions(-) diff --git a/src/main/java/biz/nynja/account/models/Account.java b/src/main/java/biz/nynja/account/models/Account.java index 34807d5..f6e26c8 100644 --- a/src/main/java/biz/nynja/account/models/Account.java +++ b/src/main/java/biz/nynja/account/models/Account.java @@ -4,7 +4,6 @@ package biz.nynja.account.models; import java.io.Serializable; -import java.nio.ByteBuffer; import java.time.LocalDate; import java.util.Set; import java.util.UUID; @@ -18,6 +17,7 @@ import biz.nynja.account.grpc.AccountDetails.Builder; import biz.nynja.account.grpc.CreatePendingAccountRequest; import biz.nynja.account.grpc.Date; import biz.nynja.account.grpc.Role; +import biz.nynja.account.grpc.UpdateAccountRequest; @Table public class Account implements Serializable { @@ -385,4 +385,45 @@ public class Account implements Serializable { } + public UpdateAccountRequest toUpdateAccountProto() { + + biz.nynja.account.grpc.UpdateAccountRequest.Builder builder = UpdateAccountRequest.newBuilder(); + + if (getAccountId() != null) { + builder.setAccountId(getAccountId().toString()); + } + if (getAccountMark() != null) { + builder.setAccountMark(getAccountMark()); + } + if (getAccountName() != null) { + builder.setAccountName(getAccountName()); + } + if (getFirstName() != null) { + builder.setFirstName(getFirstName()); + } + if (getLastName() != null) { + builder.setLastName(getLastName()); + } + if (getUsername() != null) { + builder.setUsername(getUsername()); + } + if (getAvatar() != null) { + builder.setAvatar(avatar); + } + if (getRoles() != null) { + for (String role : getRoles()) { + builder.addRoles(Role.valueOf(role)); + } + } + if (getAccessStatus() != null) { + builder.setAccessStatus(AccessStatus.valueOf(getAccessStatus())); + } + if (getBirthday() != null) { + builder.setBirthday(Date.newBuilder().setYear(getBirthday().getYear()) + .setMonth(getBirthday().getMonthValue()).setDay(getBirthday().getDayOfMonth()).build()); + } + + return builder.build(); + } + } diff --git a/src/main/java/biz/nynja/account/services/AccountServiceImpl.java b/src/main/java/biz/nynja/account/services/AccountServiceImpl.java index 191f704..5c6575a 100644 --- a/src/main/java/biz/nynja/account/services/AccountServiceImpl.java +++ b/src/main/java/biz/nynja/account/services/AccountServiceImpl.java @@ -33,6 +33,7 @@ import biz.nynja.account.grpc.CompletePendingAccountCreationRequest; import biz.nynja.account.grpc.ContactType; import biz.nynja.account.grpc.CreatePendingAccountRequest; import biz.nynja.account.grpc.CreatePendingAccountResponse; +import biz.nynja.account.grpc.DeleteAccountAvatarRequest; import biz.nynja.account.grpc.DeleteAccountRequest; import biz.nynja.account.grpc.DeleteAuthenticationProviderRequest; import biz.nynja.account.grpc.DeleteContactInfoRequest; @@ -1275,6 +1276,52 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas responseObserver.onCompleted(); } + @Override + @PerformPermissionCheck + @Permitted(role = RoleConstants.ACCOUNT_ADMIN) + @Permitted(role = RoleConstants.USER) + public void deleteAccountAvatar(DeleteAccountAvatarRequest request, + StreamObserver responseObserver) { + logger.info("Updating account..."); + logger.debug("Updating account...: {}", request); + + if ((request.getAccountId() == null) || (request.getAccountId().isEmpty())) { + logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), "Missing account ID", "", + Cause.MISSING_ACCOUNT_ID, "Missing account ID"); + return; + } + if (!util.isValidUuid(request.getAccountId())) { + logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), "Invalid account id: {}", + request.getAccountId(), Cause.INVALID_ACCOUNT_ID, "Invalid Account ID"); + return; + } + + if (!permissionsValidator.isRpcAllowed(request.getAccountId())) { + logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), "Can not update account {}.", + request.getAccountId(), Cause.ERROR_PERMISSION_DENIED, "Permission denied"); + return; + } + + Optional account = accountProvider.retrieveAccountById(request.getAccountId()); + if (!account.isPresent()) { + logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), + "Failed to delete avatar. Account {} not found.", request.getAccountId(), Cause.ACCOUNT_NOT_FOUND, + "Failed to delete avatar. Account not found."); + } + + UpdateAccountRequest updateRequest = UpdateAccountRequest.newBuilder(account.get().toUpdateAccountProto()) + .setAvatar("").build(); + + Account updatedAccount = accountRepositoryAdditional.updateAccount(updateRequest); + if (updatedAccount == null) { + logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), + "Failed to delete avatar of account {}.", request.getAccountId(), Cause.INTERNAL_SERVER_ERROR, + "Failed to delete avatar"); + } + responseObserver.onNext(StatusResponse.newBuilder().setStatus("SUCCESS").build()); + responseObserver.onCompleted(); + } + private static void logAndBuildGrpcSearchResponse(StreamObserver responseObserver, SearchResponse.Builder newBuilder, String logMessage, String logValue, Cause cause, String causeMessage) { logger.error(logMessage, logValue); diff --git a/src/main/java/biz/nynja/account/services/decomposition/AccountProvider.java b/src/main/java/biz/nynja/account/services/decomposition/AccountProvider.java index 0e5bd08..a062706 100644 --- a/src/main/java/biz/nynja/account/services/decomposition/AccountProvider.java +++ b/src/main/java/biz/nynja/account/services/decomposition/AccountProvider.java @@ -8,7 +8,6 @@ import java.util.Optional; import java.util.UUID; import java.util.stream.Collectors; -import biz.nynja.account.phone.PhoneNumberNormalizer; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.stereotype.Service; @@ -27,6 +26,7 @@ import biz.nynja.account.models.AccountByProfileId; import biz.nynja.account.models.AccountByQrCode; import biz.nynja.account.models.AccountByUsername; import biz.nynja.account.models.AuthenticationProvider; +import biz.nynja.account.phone.PhoneNumberNormalizer; import biz.nynja.account.repositories.AccountByProfileIdRepository; import biz.nynja.account.repositories.AccountByQrCodeRepository; import biz.nynja.account.repositories.AccountByUsernameRepository; @@ -180,4 +180,13 @@ public class AccountProvider { phoneNumberNormalizer.getNormalizedPhoneNumber(request.getAuthenticationIdentifier())) .build(); } + + public Optional retrieveAccountById(String accountId) { + Account account = accountRepository.findByAccountId(UUID.fromString(accountId)); + if (account == null || account.getAccountId() == null) { + return Optional.empty(); + } + + return Optional.of(account); + } } -- GitLab