From 553562f3a8d1b9e32263aa8b4b8a5c77c6c80dfd Mon Sep 17 00:00:00 2001 From: Stoyan Tzenkov Date: Wed, 14 Nov 2018 16:02:23 +0200 Subject: [PATCH 1/4] NY-4867: Added validation for accountId and ProfileId to all endpoints Signed-off-by: Stoyan Tzenkov --- .../nynja/account/components/Validator.java | 5 ++ .../account/services/AccountServiceImpl.java | 49 ++++++++++++------- 2 files changed, 36 insertions(+), 18 deletions(-) diff --git a/src/main/java/biz/nynja/account/components/Validator.java b/src/main/java/biz/nynja/account/components/Validator.java index 3a473e1..b086d28 100644 --- a/src/main/java/biz/nynja/account/components/Validator.java +++ b/src/main/java/biz/nynja/account/components/Validator.java @@ -17,6 +17,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.stereotype.Component; +import biz.nynja.account.grpc.AccountResponse; import biz.nynja.account.grpc.AddAuthenticationProviderRequest; import biz.nynja.account.grpc.AuthProviderDetails; import biz.nynja.account.grpc.AuthenticationType; @@ -184,6 +185,10 @@ public class Validator { public Cause validateUpdateAccountRequest(UpdateAccountRequest request) { + if (!isValidUuid(request.getAccountId())) { + return Cause.INVALID_ACCOUNT_ID; + } + if (request.getUsername() != null && !request.getUsername().trim().isEmpty() && !isUsernameValid(request.getUsername())) { return Cause.INVALID_USERNAME; diff --git a/src/main/java/biz/nynja/account/services/AccountServiceImpl.java b/src/main/java/biz/nynja/account/services/AccountServiceImpl.java index e0ae5f7..6833092 100644 --- a/src/main/java/biz/nynja/account/services/AccountServiceImpl.java +++ b/src/main/java/biz/nynja/account/services/AccountServiceImpl.java @@ -181,7 +181,6 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas Cause.MISSING_PHONENUMBER); return; } - if (!phoneNumberValidator.isPhoneNumberValid(request.getPhoneNumber())) { logAndBuildGrpcSearchResponse(responseObserver, SearchResponse.newBuilder(), "Invalid phone number. Value : ", request.getPhoneNumber(), Cause.INVALID_PHONENUMBER); @@ -337,8 +336,8 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas request.getProfileId(), Cause.INVALID_PROFILE_ID); return; } - Optional accounts = accountProvider.getAllAccountsByProfileId(request); + Optional accounts = accountProvider.getAllAccountsByProfileId(request); if (!accounts.isPresent()) { logAndBuildGrpcAccountsResponse(responseObserver, AccountsResponse.newBuilder(), "Account not found for profile id: {}", request.getProfileId(), Cause.ACCOUNT_NOT_FOUND); @@ -360,7 +359,6 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas Cause.MISSING_ACCOUNT_ID); return; } - if (!validator.isValidUuid(request.getAccountId())) { logAndBuildGrpcAccountResponse(responseObserver, AccountResponse.newBuilder(), "Invalid account id: {}", request.getAccountId(), Cause.INVALID_ACCOUNT_ID); @@ -368,7 +366,6 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas } Optional account = accountProvider.getAccountByAccountId(request); - if (!account.isPresent()) { logAndBuildGrpcAccountResponse(responseObserver, AccountResponse.newBuilder(), "Account id not found: {}", request.getAccountId(), Cause.ACCOUNT_NOT_FOUND); @@ -413,13 +410,13 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas Cause.MISSING_ACCOUNT_ID); return; } + Cause cause = validator.validateUpdateAccountRequest(request); if (cause != null) { logAndBuildGrpcAccountResponse(responseObserver, AccountResponse.newBuilder(), "Validation failed", "", cause); return; } - if (request.getUsername() != null && !request.getUsername().trim().isEmpty() && accountRepositoryAdditional .foundExistingNotOwnUsername(UUID.fromString(request.getAccountId()), request.getUsername())) { logAndBuildGrpcAccountResponse(responseObserver, AccountResponse.newBuilder(), @@ -428,7 +425,6 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas } Account updatedAccount = accountRepositoryAdditional.updateAccount(request); - if (updatedAccount == null) { logAndBuildGrpcAccountResponse(responseObserver, AccountResponse.newBuilder(), "Error updating account.", "", Cause.ERROR_UPDATING_ACCOUNT); @@ -452,6 +448,11 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas Cause.MISSING_ACCOUNT_ID); return; } + if (!validator.isValidUuid(request.getAccountId())) { + logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), "Invalid account id: {}", + request.getAccountId(), Cause.INVALID_ACCOUNT_ID); + return; + } boolean wasAccountDeleted = accountRepositoryAdditional.deleteAccount(UUID.fromString(request.getAccountId())); if (wasAccountDeleted) { @@ -474,6 +475,11 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas Cause.MISSING_PROFILE_ID); return; } + if (!validator.isValidUuid(request.getProfileId())) { + logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), "Invalid profile id: {}", + request.getProfileId(), Cause.INVALID_PROFILE_ID); + return; + } boolean wasProfileDeleted = accountRepositoryAdditional.deleteProfile(UUID.fromString(request.getProfileId())); if (wasProfileDeleted) { @@ -498,6 +504,11 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas Cause.MISSING_PROFILE_ID); return; } + if (!validator.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", "", Cause.MISSING_AUTH_PROVIDER_TYPE); @@ -509,13 +520,13 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas "Missing auth provider identifier", "", Cause.MISSING_AUTH_PROVIDER_ID); return; } + Cause cause = validator.validateAddAuthenticationProviderRequest(request); if (cause != null) { logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), "Validation failed", "", cause); return; } - if (request.getAuthenticationProvider().getAuthenticationType() == AuthenticationType.PHONE) { // Get the normalized phone number from libphone AuthProviderDetails newAuthProviderDetails = AuthProviderDetails.newBuilder() @@ -535,6 +546,7 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas "Profile id {} misnot found in DB.", request.getProfileId(), Cause.PROFILE_NOT_FOUND); return; } + // Make sure that the requested authentication provider is not already used in the system. ProfileByAuthenticationProvider profileByAuthProvider = profileByAutheticationProviderRepository .findByAuthenticationProviderAndAuthenticationProviderType( @@ -546,6 +558,7 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas Cause.AUTH_PROVIDER_ALREADY_USED); return; } + boolean result = accountRepositoryAdditional.addAuthenticationProvider(UUID.fromString(request.getProfileId()), AuthenticationProvider.createAuthenticationProviderFromProto(request.getAuthenticationProvider())); if (result) { @@ -567,6 +580,7 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas StreamObserver responseObserver) { logger.info("Adding contact info to account requested."); logger.debug("Adding contact info to account requested: {}", request); + Optional> validationResult = validator .validateContactInfoRequest(request.getAccountId(), request.getContactInfo()); if (validationResult.isPresent()) { @@ -577,6 +591,7 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas if (request.getContactInfo().getType() == ContactType.PHONE_CONTACT) { request = phoneNumberNormalizer.normalizePhoneNumber(request); } + boolean result = accountRepositoryAdditional.addContactInfo(UUID.fromString(request.getAccountId()), ContactInfo.createContactInfoFromProto(request.getContactInfo())); if (result) { @@ -596,6 +611,7 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas StreamObserver responseObserver) { logger.info("Removing contact info from account requested."); logger.debug("Removing contact info from account requested: {}", request); + Optional> validationResult = validator .validateContactInfoRequest(request.getAccountId(), request.getContactInfo()); if (validationResult.isPresent()) { @@ -606,6 +622,7 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas if (request.getContactInfo().getType() == ContactType.PHONE_CONTACT) { request = phoneNumberNormalizer.normalizePhoneNumber(request); } + boolean result = accountRepositoryAdditional.deleteContactInfo(UUID.fromString(request.getAccountId()), ContactInfo.createContactInfoFromProto(request.getContactInfo())); if (result) { @@ -631,6 +648,11 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas Cause.MISSING_PROFILE_ID); return; } + if (!validator.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.", "", Cause.MISSING_AUTH_PROVIDER_TYPE); @@ -736,6 +758,7 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas StreamObserver responseObserver) { logger.info("Removing contact info from account requested."); logger.debug("Removing contact info from account requested: {}", request); + Optional> validationResultEditContactInfo = validator .validateEditContactInfoRequest(request.getAccountId(), request.getOldContactInfo(), request.getEditedContactInfo()); @@ -745,17 +768,16 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas validationResultEditContactInfo.get().getKey()); return; } - if (!request.getOldContactInfo().getType().equals(request.getEditedContactInfo().getType())) { logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), "Error editing Contact info for account {}. Different types: {} and {}.", request.getAccountId(), Cause.ERROR_EDITING_CONTACT_INFO); return; } - if (request.getOldContactInfo().getType() == ContactType.PHONE_CONTACT) { request = phoneNumberNormalizer.normalizePhoneNumbers(request); } + boolean result = accountRepositoryAdditional.editContactInfo(UUID.fromString(request.getAccountId()), ContactInfo.createContactInfoFromProto(request.getOldContactInfo()), ContactInfo.createContactInfoFromProto(request.getEditedContactInfo())); @@ -807,14 +829,12 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas validationResultUpdateAuthProvider.get().getKey()); 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); return; } - if (request.getOldAuthProvider().getAuthenticationTypeValue() == AuthenticationType.PHONE_VALUE || request.getUpdatedAuthProvider().getAuthenticationTypeValue() == AuthenticationType.PHONE_VALUE) { request = phoneNumberNormalizer.normalizePhoneNumbers(request); @@ -858,13 +878,6 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas responseObserver.onCompleted(); } - private static void logAndBuildGrpcProfileResponse(StreamObserver responseObserver, - ProfileResponse.Builder newBuilder, String logMessage, String logValue, Cause cause) { - logger.error(logMessage, logValue); - responseObserver.onNext(newBuilder.setError(ErrorResponse.newBuilder().setCause(cause)).build()); - responseObserver.onCompleted(); - } - private static void logAndBuildGrpcStatusResponse(StreamObserver responseObserver, StatusResponse.Builder newBuilder, String logMessage, String logValue, Cause cause) { logger.error(logMessage, logValue); -- GitLab From 986811cf24231d4da0582794dd9df35989f6ba6a Mon Sep 17 00:00:00 2001 From: Stoyan Tzenkov Date: Thu, 15 Nov 2018 12:54:12 +0200 Subject: [PATCH 2/4] NY-4867: Check removed from addAuthenticationProviderToProfile() since it is already in validateAddAuthenticationProviderRequest() Signed-off-by: Stoyan Tzenkov --- .../java/biz/nynja/account/services/AccountServiceImpl.java | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/main/java/biz/nynja/account/services/AccountServiceImpl.java b/src/main/java/biz/nynja/account/services/AccountServiceImpl.java index 6833092..b317537 100644 --- a/src/main/java/biz/nynja/account/services/AccountServiceImpl.java +++ b/src/main/java/biz/nynja/account/services/AccountServiceImpl.java @@ -504,11 +504,6 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas Cause.MISSING_PROFILE_ID); return; } - if (!validator.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", "", Cause.MISSING_AUTH_PROVIDER_TYPE); -- GitLab From e9e534549467157bf96ffdb42e9015eaabbbd2cf Mon Sep 17 00:00:00 2001 From: Stoyan Tzenkov Date: Fri, 16 Nov 2018 09:20:55 +0200 Subject: [PATCH 3/4] NY-4867: Validation added for completePendingAccountCreation Signed-off-by: Stoyan Tzenkov --- .../biz/nynja/account/components/PendingAccountValidator.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/main/java/biz/nynja/account/components/PendingAccountValidator.java b/src/main/java/biz/nynja/account/components/PendingAccountValidator.java index 96a74cb..c10e8f7 100644 --- a/src/main/java/biz/nynja/account/components/PendingAccountValidator.java +++ b/src/main/java/biz/nynja/account/components/PendingAccountValidator.java @@ -27,6 +27,10 @@ public class PendingAccountValidator { return Cause.MISSING_ACCOUNT_ID; } + if (!validator.isValidUuid(request.getAccountId())) { + return Cause.INVALID_ACCOUNT_ID; + } + if (request.getFirstName() != null && request.getFirstName().trim().isEmpty()) { return Cause.MISSING_FIRST_NAME; } else if (!validator.isFirstNameValid(request.getFirstName())) { -- GitLab From fd1b43ead89a6c4c59a2d1fb0e2adbe32de07783 Mon Sep 17 00:00:00 2001 From: Stoyan Tzenkov Date: Tue, 20 Nov 2018 09:37:54 +0200 Subject: [PATCH 4/4] NY-4867: prifleId validation removed from deleteAuthenticationProviderFromProfile (it is done in validateDeleteAuthenticationProviderRequest) Signed-off-by: Stoyan Tzenkov --- .../nynja/account/services/AccountServiceImpl.java | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/main/java/biz/nynja/account/services/AccountServiceImpl.java b/src/main/java/biz/nynja/account/services/AccountServiceImpl.java index b317537..cf62db6 100644 --- a/src/main/java/biz/nynja/account/services/AccountServiceImpl.java +++ b/src/main/java/biz/nynja/account/services/AccountServiceImpl.java @@ -643,11 +643,6 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas Cause.MISSING_PROFILE_ID); return; } - if (!validator.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.", "", Cause.MISSING_AUTH_PROVIDER_TYPE); @@ -873,6 +868,13 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas responseObserver.onCompleted(); } + private static void logAndBuildGrpcProfileResponse(StreamObserver responseObserver, + ProfileResponse.Builder newBuilder, String logMessage, String logValue, Cause cause) { + logger.error(logMessage, logValue); + responseObserver.onNext(newBuilder.setError(ErrorResponse.newBuilder().setCause(cause)).build()); + responseObserver.onCompleted(); + } + private static void logAndBuildGrpcStatusResponse(StreamObserver responseObserver, StatusResponse.Builder newBuilder, String logMessage, String logValue, Cause cause) { logger.error(logMessage, logValue); -- GitLab