From bda5aa13df72084e41efbaa5f352b5e514cace36 Mon Sep 17 00:00:00 2001 From: Stanimir Penkov Date: Thu, 6 Dec 2018 16:39:25 +0200 Subject: [PATCH 1/3] NY-5166: Social providers can not be added as login option Signed-off-by: Stanimir Penkov --- .../account/services/AccountServiceImpl.java | 15 +++++ .../account/components/ValidatorTests.java | 24 +++++++ .../account/services/AccountServiceTests.java | 64 +++++++++++++++++++ 3 files changed, 103 insertions(+) diff --git a/src/main/java/biz/nynja/account/services/AccountServiceImpl.java b/src/main/java/biz/nynja/account/services/AccountServiceImpl.java index e900c6c..0f477bf 100644 --- a/src/main/java/biz/nynja/account/services/AccountServiceImpl.java +++ b/src/main/java/biz/nynja/account/services/AccountServiceImpl.java @@ -615,6 +615,14 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas "", Cause.MISSING_AUTH_PROVIDER_TYPE); return; } + + if (!authenticationProvider + .canUseAsAdditionalLoginOption(request.getAuthenticationProvider().getAuthenticationType())) { + logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), + "Invalid additional login option type used.", "", Cause.INVALID_ADDITIONAL_LOGIN_OPTION_TYPE); + return; + } + if (request.getAuthenticationProvider().getAuthenticationProvider() == null || request.getAuthenticationProvider().getAuthenticationProvider().isEmpty()) { logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), @@ -1019,6 +1027,13 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas return; } + if (!authenticationProvider + .canUseAsAdditionalLoginOption(request.getUpdatedAuthProvider().getAuthenticationType())) { + logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), + "Invalid additional login option type used.", "", Cause.INVALID_ADDITIONAL_LOGIN_OPTION_TYPE); + 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(), diff --git a/src/test/java/biz/nynja/account/components/ValidatorTests.java b/src/test/java/biz/nynja/account/components/ValidatorTests.java index 068449e..2617128 100644 --- a/src/test/java/biz/nynja/account/components/ValidatorTests.java +++ b/src/test/java/biz/nynja/account/components/ValidatorTests.java @@ -195,6 +195,30 @@ public class ValidatorTests { Cause.INVALID_EMAIL); } + @Test + public void validateAdditionalLoginOptionGoogleplusInvalidTest() { + assertEquals(authenticationProvider + .canUseAsAdditionalLoginOption(AuthenticationType.GOOGLEPLUS), false); + } + + @Test + public void validateAdditionalLoginOptionFacebookInvalidTest() { + assertEquals(authenticationProvider + .canUseAsAdditionalLoginOption(AuthenticationType.FACEBOOK), false); + } + + @Test + public void validateAdditionalLoginOptionPhoneValidTest() { + assertEquals(authenticationProvider + .canUseAsAdditionalLoginOption(AuthenticationType.PHONE), true); + } + + @Test + public void validateAdditionalLoginOptionEmailValidTest() { + assertEquals(authenticationProvider + .canUseAsAdditionalLoginOption(AuthenticationType.EMAIL), true); + } + @Test public void validateAuthProviderEmptyProviderIdentifierTest() { assertEquals(authenticationProvider.validateAuthenticationProvider(AuthenticationType.EMAIL, null).get(), diff --git a/src/test/java/biz/nynja/account/services/AccountServiceTests.java b/src/test/java/biz/nynja/account/services/AccountServiceTests.java index d8df165..75dcb45 100644 --- a/src/test/java/biz/nynja/account/services/AccountServiceTests.java +++ b/src/test/java/biz/nynja/account/services/AccountServiceTests.java @@ -809,6 +809,36 @@ public class AccountServiceTests extends GrpcServerTestBase { reply.getError().getCause().equals(Cause.INTERNAL_SERVER_ERROR)); } + @Test + public void testAddAuthenticationProviderToProfileInvalidTypeFacebook() { + final AddAuthenticationProviderRequest request = AddAuthenticationProviderRequest.newBuilder() + .setProfileId(Util.PROFILE_ID.toString()) + .setAuthenticationProvider( + AuthProviderDetails.newBuilder().setAuthenticationProvider("Facebook_provider") + .setAuthenticationType(AuthenticationType.FACEBOOK)) + .build(); + + final StatusResponse reply = accountServiceBlockingStub.addAuthenticationProviderToProfile(request); + assertNotNull("Reply should not be null", reply); + assertTrue(String.format("Reply should contain cause '%s'", Cause.INVALID_ADDITIONAL_LOGIN_OPTION_TYPE), + reply.getError().getCause().equals(Cause.INVALID_ADDITIONAL_LOGIN_OPTION_TYPE)); + } + + @Test + public void testAddAuthenticationProviderToProfileInvalidTypeGoogleplus() { + final AddAuthenticationProviderRequest request = AddAuthenticationProviderRequest.newBuilder() + .setProfileId(Util.PROFILE_ID.toString()) + .setAuthenticationProvider( + AuthProviderDetails.newBuilder().setAuthenticationProvider("Googleplus_provider") + .setAuthenticationType(AuthenticationType.GOOGLEPLUS)) + .build(); + + final StatusResponse reply = accountServiceBlockingStub.addAuthenticationProviderToProfile(request); + assertNotNull("Reply should not be null", reply); + assertTrue(String.format("Reply should contain cause '%s'", Cause.INVALID_ADDITIONAL_LOGIN_OPTION_TYPE), + reply.getError().getCause().equals(Cause.INVALID_ADDITIONAL_LOGIN_OPTION_TYPE)); + } + @Test public void testAddAuthenticationProviderToProfileNotFound() { final AddAuthenticationProviderRequest request = AddAuthenticationProviderRequest.newBuilder() @@ -1796,6 +1826,40 @@ public class AccountServiceTests extends GrpcServerTestBase { reply.getError().getCause().equals(Cause.INVALID_EMAIL)); } + @Test + public void testUpdateAuthProviderForProfileInvalidTypeFacebook() { + final UpdateAuthenticationProviderRequest request = UpdateAuthenticationProviderRequest.newBuilder() + .setProfileId(Util.PROFILE_ID.toString()) + .setOldAuthProvider(AuthProviderDetails.newBuilder().setAuthenticationType(AuthenticationType.EMAIL) + .setAuthenticationProvider(Util.EMAIL).build()) + .setUpdatedAuthProvider( + AuthProviderDetails.newBuilder().setAuthenticationType(AuthenticationType.FACEBOOK) + .setAuthenticationProvider("Facebook_provider").build()) + .build(); + + final StatusResponse reply = accountServiceBlockingStub.updateAuthenticationProviderForProfile(request); + assertNotNull("Reply should not be null", reply); + assertTrue(String.format("Reply should contain cause '%s'", Cause.INVALID_ADDITIONAL_LOGIN_OPTION_TYPE), + reply.getError().getCause().equals(Cause.INVALID_ADDITIONAL_LOGIN_OPTION_TYPE)); + } + + @Test + public void testUpdateAuthProviderForProfileInvalidTypeGoogleplus() { + final UpdateAuthenticationProviderRequest request = UpdateAuthenticationProviderRequest.newBuilder() + .setProfileId(Util.PROFILE_ID.toString()) + .setOldAuthProvider(AuthProviderDetails.newBuilder().setAuthenticationType(AuthenticationType.EMAIL) + .setAuthenticationProvider(Util.EMAIL).build()) + .setUpdatedAuthProvider( + AuthProviderDetails.newBuilder().setAuthenticationType(AuthenticationType.GOOGLEPLUS) + .setAuthenticationProvider("Googleplus_provider").build()) + .build(); + + final StatusResponse reply = accountServiceBlockingStub.updateAuthenticationProviderForProfile(request); + assertNotNull("Reply should not be null", reply); + assertTrue(String.format("Reply should contain cause '%s'", Cause.INVALID_ADDITIONAL_LOGIN_OPTION_TYPE), + reply.getError().getCause().equals(Cause.INVALID_ADDITIONAL_LOGIN_OPTION_TYPE)); + } + @Test public void testGetAccountByLoginOptionPhone() throws IncorrectAccountCountException { final AuthenticationProviderRequest request = AuthenticationProviderRequest.newBuilder() -- GitLab From 5ba3b139a614aeae72368c16d3b25da2589cb433 Mon Sep 17 00:00:00 2001 From: Stanimir Penkov Date: Thu, 6 Dec 2018 17:10:44 +0200 Subject: [PATCH 2/3] NY-5166: Additional method added Signed-off-by: Stanimir Penkov --- .../java/biz/nynja/account/validation/Validators.java | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/main/java/biz/nynja/account/validation/Validators.java b/src/main/java/biz/nynja/account/validation/Validators.java index b15b7bd..ca59555 100644 --- a/src/main/java/biz/nynja/account/validation/Validators.java +++ b/src/main/java/biz/nynja/account/validation/Validators.java @@ -296,6 +296,16 @@ public class Validators { return Optional.empty(); } + public boolean canUseAsAdditionalLoginOption(AuthenticationType authenticationType) { + switch (authenticationType) { + case PHONE: + case EMAIL: + return true; + default: + return false; + } + } + public Optional validateDeleteAuthenticationProviderRequest( DeleteAuthenticationProviderRequest request) { if (!isValidUuid(request.getProfileId())) { -- GitLab From ce75a896e9f6ea7ca8ebf69fb87ef88b4fda5f64 Mon Sep 17 00:00:00 2001 From: Stanimir Penkov Date: Mon, 10 Dec 2018 14:29:00 +0200 Subject: [PATCH 3/3] NY-5166: Code Review Feedback - moved validation check related to cause INVALID_ADDITIONAL_LOGIN_OPTION_TYPE to existing method; - removed duplicated validation checks (checks related to causes MISSING_AUTH_PROVIDER_TYPE and MISSING_AUTH_PROVIDER_ID are already added in validateAuthenticationProvider()); Signed-off-by: Stanimir Penkov --- .../account/services/AccountServiceImpl.java | 20 ------------------- .../nynja/account/validation/Validators.java | 13 +++++++++++- 2 files changed, 12 insertions(+), 21 deletions(-) diff --git a/src/main/java/biz/nynja/account/services/AccountServiceImpl.java b/src/main/java/biz/nynja/account/services/AccountServiceImpl.java index 0f477bf..4057f14 100644 --- a/src/main/java/biz/nynja/account/services/AccountServiceImpl.java +++ b/src/main/java/biz/nynja/account/services/AccountServiceImpl.java @@ -610,26 +610,6 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas return; } - if (request.getAuthenticationProvider().getAuthenticationTypeValue() == 0) { - logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), "Missing auth provider type", - "", Cause.MISSING_AUTH_PROVIDER_TYPE); - return; - } - - if (!authenticationProvider - .canUseAsAdditionalLoginOption(request.getAuthenticationProvider().getAuthenticationType())) { - logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), - "Invalid additional login option type used.", "", Cause.INVALID_ADDITIONAL_LOGIN_OPTION_TYPE); - return; - } - - if (request.getAuthenticationProvider().getAuthenticationProvider() == null - || request.getAuthenticationProvider().getAuthenticationProvider().isEmpty()) { - logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), - "Missing auth provider identifier", "", Cause.MISSING_AUTH_PROVIDER_ID); - return; - } - Optional cause = authenticationProvider.validateAddAuthenticationProviderRequest(request); if (cause.isPresent()) { logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), "Validation failed", "", diff --git a/src/main/java/biz/nynja/account/validation/Validators.java b/src/main/java/biz/nynja/account/validation/Validators.java index ca59555..55010bd 100644 --- a/src/main/java/biz/nynja/account/validation/Validators.java +++ b/src/main/java/biz/nynja/account/validation/Validators.java @@ -319,8 +319,19 @@ public class Validators { if (!isValidUuid(request.getProfileId())) { return Optional.of(Cause.INVALID_PROFILE_ID); } - return validateAuthenticationProvider(request.getAuthenticationProvider().getAuthenticationType(), + Optional authenticationProviderValidation = validateAuthenticationProvider( + request.getAuthenticationProvider().getAuthenticationType(), request.getAuthenticationProvider().getAuthenticationProvider()); + if (authenticationProviderValidation.isPresent()) { + return authenticationProviderValidation; + } + boolean typeCanBeUsed = authenticationProvider + .canUseAsAdditionalLoginOption(request.getAuthenticationProvider().getAuthenticationType()); + if (typeCanBeUsed) { + return Optional.empty(); + } else { + return Optional.of(Cause.INVALID_ADDITIONAL_LOGIN_OPTION_TYPE); + } } private boolean isValidUuid(String id) { -- GitLab