From 8a6165e05bbdfbb682b156b385aad04971aebdf1 Mon Sep 17 00:00:00 2001 From: Stanimir Penkov Date: Wed, 9 Jan 2019 11:52:23 +0200 Subject: [PATCH 1/2] NY-4867: Fix errors with accountId/profileId - updated existing checks for accountId/profileId; - updated log messages; - added check for used accountId/profileId. Signed-off-by: Stanimir Penkov --- .../AccountRepositoryAdditional.java | 4 ++ .../AccountRepositoryAdditionalImpl.java | 24 ++++++++ .../account/services/AccountServiceImpl.java | 60 ++++++++++++++++--- .../decomposition/AccountCreator.java | 11 ++++ 4 files changed, 90 insertions(+), 9 deletions(-) diff --git a/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditional.java b/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditional.java index dd43e40..800a1cd 100644 --- a/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditional.java +++ b/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditional.java @@ -37,6 +37,10 @@ public interface AccountRepositoryAdditional { PendingAccountByAuthenticationProvider findSameAuthenticationProviderInPendingAccount(AuthenticationProvider authProvider); + boolean accountIdAlreadyUsed(UUID accountId); + + boolean profileIdAlreadyUsed(UUID profileId); + boolean authenticationProviderAlreadyUsedInProfile(AuthenticationProvider authProvider); boolean addAuthenticationProvider(UUID profileId, AuthenticationProvider authProvider); diff --git a/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java b/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java index 77a6745..c09021f 100644 --- a/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java +++ b/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java @@ -120,6 +120,16 @@ public class AccountRepositoryAdditionalImpl implements AccountRepositoryAdditio pendingAccount.getAuthenticationProviderType(), pendingAccount.getAuthenticationProvider()); return null; } + if (accountRepository.findByAccountId(pendingAccount.getAccountId()) != null) { + logger.error("The account id {} is already used.", pendingAccount.getAccountId()); + pendingAccountRepository.deleteById(pendingAccount.getAccountId()); + return null; + } + if (profileRepository.findByProfileId(pendingAccount.getProfileId()) != null) { + logger.error("The profile id {} is already used.", pendingAccount.getProfileId()); + pendingAccountRepository.deleteById(pendingAccount.getAccountId()); + return null; + } Long timeCreated = Instant.now().toEpochMilli(); WriteResult wr; @@ -469,6 +479,20 @@ public class AccountRepositoryAdditionalImpl implements AccountRepositoryAdditio return false; } + public boolean accountIdAlreadyUsed(UUID accountId) { + if (accountRepository.findByAccountId(accountId) != null) { + return true; + } + return false; + } + + public boolean profileIdAlreadyUsed(UUID profileId) { + if (profileRepository.findByProfileId(profileId) != null) { + return true; + } + return false; + } + @Override public boolean addAuthenticationProvider(UUID profileId, AuthenticationProvider authProvider) { BatchStatement batch = new BatchStatement(); diff --git a/src/main/java/biz/nynja/account/services/AccountServiceImpl.java b/src/main/java/biz/nynja/account/services/AccountServiceImpl.java index 4d68518..72542ac 100644 --- a/src/main/java/biz/nynja/account/services/AccountServiceImpl.java +++ b/src/main/java/biz/nynja/account/services/AccountServiceImpl.java @@ -476,7 +476,7 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas public void completePendingAccountCreation(CompletePendingAccountCreationRequest request, StreamObserver responseObserver) { - logger.info("Complete pending account creation..."); + logger.info("Complete pending account creation for account id {} ...", request.getAccountId()); logger.debug("Complete pending account creation...: {} ...", request); AccountResponse response = accountCreator.retrieveCompletePendingAccountResponse(request); @@ -497,6 +497,12 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas Cause.MISSING_ACCOUNT_ID); return; } + if (!util.isValidUuid(request.getAccountId())) { + logAndBuildGrpcAccountResponse(responseObserver, AccountResponse.newBuilder(), "Invalid account id: {}", + request.getAccountId(), Cause.INVALID_ACCOUNT_ID); + return; + } + if (!permissionsValidator.isRpcAllowed(request.getAccountId())) { logAndBuildGrpcAccountResponse(responseObserver, AccountResponse.newBuilder(), "Can not update account {}.", request.getAccountId(), Cause.ERROR_PERMISSION_DENIED); @@ -543,18 +549,17 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas Cause.MISSING_ACCOUNT_ID); return; } - if (!permissionsValidator.isRpcAllowed(request.getAccountId())) { - logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), "Can not delete account {}.", - request.getAccountId(), Cause.ERROR_PERMISSION_DENIED); - 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); return; } + if (!permissionsValidator.isRpcAllowed(request.getAccountId())) { + logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), "Can not delete account {}.", + request.getAccountId(), Cause.ERROR_PERMISSION_DENIED); + return; + } boolean wasAccountDeleted = accountRepositoryAdditional.deleteAccount(UUID.fromString(request.getAccountId())); if (wasAccountDeleted) { @@ -614,6 +619,11 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas Cause.MISSING_PROFILE_ID); return; } + if (!util.isValidUuid(request.getProfileId())) { + logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), "Invalid profile id: {}", + request.getProfileId(), Cause.INVALID_PROFILE_ID); + return; + } List existingAccountsForProfile = accountByProfileIdRepository .findAllByProfileId(UUID.fromString(request.getProfileId())); @@ -703,6 +713,11 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas validationResult.get().getValue(), "", validationResult.get().getKey()); return; } + if (!util.isValidUuid(request.getAccountId())) { + logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), "Invalid account id: {}", + request.getAccountId(), Cause.INVALID_ACCOUNT_ID); + return; + } if (!permissionsValidator.isRpcAllowed(request.getAccountId())) { logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), @@ -756,6 +771,11 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas validationResult.get().getRight(), validationResult.get().getLeft()); return; } + if (!util.isValidUuid(request.getAccountId())) { + logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), "Invalid account id: {}", + request.getAccountId(), Cause.INVALID_ACCOUNT_ID); + return; + } if (!permissionsValidator.isRpcAllowed(request.getAccountId())) { logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), @@ -796,6 +816,11 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas Cause.MISSING_PROFILE_ID); return; } + if (!util.isValidUuid(request.getProfileId())) { + logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), "Invalid profile id: {}", + request.getProfileId(), Cause.INVALID_PROFILE_ID); + return; + } if (request.getAuthenticationProvider().getAuthenticationTypeValue() == 0) { logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), "Missing auth provider type.", @@ -937,8 +962,8 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas @Permitted(role = RoleConstants.USER) public void editContactInfoForAccount(EditContactInfoRequest request, StreamObserver responseObserver) { - logger.info("Removing contact info from account requested."); - logger.debug("Removing contact info from account requested: {}", request); + logger.info("Editing contact info for account requested."); + logger.debug("Editing contact info for account requested: {}", request); Optional> validationResultEditContactInfo = account.validateEditContactInfoRequest( request.getAccountId(), request.getOldContactInfo(), request.getEditedContactInfo()); @@ -948,6 +973,11 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas validationResultEditContactInfo.get().getKey()); return; } + if (!util.isValidUuid(request.getAccountId())) { + logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), "Invalid account id: {}", + request.getAccountId(), Cause.INVALID_ACCOUNT_ID); + return; + } if (!permissionsValidator.isRpcAllowed(request.getAccountId())) { logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), @@ -1030,6 +1060,11 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas validationResultUpdateAuthProvider.get().getKey()); return; } + if (!util.isValidUuid(request.getProfileId())) { + logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), "Invalid profile id: {}.", + request.getProfileId(), validationResultUpdateAuthProvider.get().getKey()); + return; + } List existingAccountsForProfile = accountByProfileIdRepository .findAllByProfileId(UUID.fromString(request.getProfileId())); @@ -1203,6 +1238,8 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas @Permitted(role = RoleConstants.USER) public void updateSearchableOption(UpdateSearchableOptionRequest request, StreamObserver responseObserver) { + logger.info("Updating searchable option for profile {} requested.", request.getProfileId()); + logger.debug("Updating searchable option for profile requested: {}", request); Validation validation = authenticationProvider.validateUpdateSearchableOptionRequest(request.getProfileId(), request.getAuthenticationType(), request.getAuthenticationIdentifier()); @@ -1211,6 +1248,11 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas "", validation.getCause().get()); return; } + if (!util.isValidUuid(request.getProfileId())) { + logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), "Invalid profile id: {}", + request.getProfileId(), Cause.INVALID_PROFILE_ID); + return; + } List existingAccountsForProfile = accountByProfileIdRepository .findAllByProfileId(UUID.fromString(request.getProfileId())); 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 d4d507d..9123af3 100644 --- a/src/main/java/biz/nynja/account/services/decomposition/AccountCreator.java +++ b/src/main/java/biz/nynja/account/services/decomposition/AccountCreator.java @@ -80,6 +80,17 @@ public class AccountCreator { pendingAccount.setAccountId(UUID.randomUUID()); pendingAccount.setProfileId(UUID.randomUUID()); + if (pendingAccountRepository.findByAccountId(pendingAccount.getAccountId()) != null + || accountRepositoryAdditional.accountIdAlreadyUsed(pendingAccount.getAccountId())) { + logger.error("Can not create pending account. The account id {} is already used.", pendingAccount.getAccountId()); + return CreatePendingAccountResponse.newBuilder() + .setError(ErrorResponse.newBuilder().setCause(Cause.INTERNAL_SERVER_ERROR)).build(); + } + if (accountRepositoryAdditional.profileIdAlreadyUsed(pendingAccount.getProfileId())) { + logger.error("Can not create pending account. The profile id {} is already used.", pendingAccount.getProfileId()); + return CreatePendingAccountResponse.newBuilder() + .setError(ErrorResponse.newBuilder().setCause(Cause.INTERNAL_SERVER_ERROR)).build(); + } pendingAccount.setCreationTimestamp(Instant.now().toEpochMilli()); try { -- GitLab From 257c116ccc1a5fb08c2d61386deed355230a22d3 Mon Sep 17 00:00:00 2001 From: Stanimir Penkov Date: Wed, 9 Jan 2019 12:19:39 +0200 Subject: [PATCH 2/2] NY-4867: Remove unused method's parameter Signed-off-by: Stanimir Penkov --- .../account/repositories/AccountRepositoryAdditionalImpl.java | 4 ++-- 1 file changed, 2 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 c09021f..79ca704 100644 --- a/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java +++ b/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java @@ -442,7 +442,7 @@ public class AccountRepositoryAdditionalImpl implements AccountRepositoryAdditio WriteResult wr = null; try { - deleteProfileAccountsWhenDeletingProfile(profileId, batchOperations, existingAccountsForProfile); + deleteProfileAccountsWhenDeletingProfile(batchOperations, existingAccountsForProfile); if (existingProfile.getAuthenticationProviders() != null && existingProfile.getAuthenticationProviders().size() > 0) { deleteAuthenticationProvidersFromProfile(batchOperations, existingProfile.getProfileId(), @@ -461,7 +461,7 @@ public class AccountRepositoryAdditionalImpl implements AccountRepositoryAdditio return Optional.of(Cause.ERROR_DELETING_PROFILE); } - private void deleteProfileAccountsWhenDeletingProfile(UUID profileId, CassandraBatchOperations batchOperations, + private void deleteProfileAccountsWhenDeletingProfile(CassandraBatchOperations batchOperations, List existingAccountsForProfile) { for (AccountByProfileId accountByProfileId : existingAccountsForProfile) { Account existingAccount = accountRepository.findByAccountId(accountByProfileId.getAccountId()); -- GitLab