From 9cccc0fa2ae6b22bbd14c966859da41171d26805 Mon Sep 17 00:00:00 2001 From: sergeyPensov Date: Mon, 14 Jan 2019 18:17:28 +0200 Subject: [PATCH 01/13] Additional checking for integration --- .../AccountRepositoryAdditionalImpl.java | 20 ++++++++++--- .../erlang/ErlangAccountMqttBridge.java | 30 +++++++------------ 2 files changed, 27 insertions(+), 23 deletions(-) diff --git a/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java b/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java index 649b92f..6522ae1 100644 --- a/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java +++ b/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java @@ -18,6 +18,7 @@ import java.util.stream.Collectors; import biz.nynja.account.repositories.batch.SagaTransaction; import biz.nynja.account.repositories.batch.Transaction; import biz.nynja.account.services.erlang.ErlangAccountBridge; +import io.grpc.StatusRuntimeException; import org.apache.commons.lang3.SerializationUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -147,6 +148,10 @@ public class AccountRepositoryAdditionalImpl implements AccountRepositoryAdditio sagaTransaction.rollBack(); return null; } + } catch (StatusRuntimeException e) { + logger.info("Error while lose connection with bridge"); + sagaTransaction.rollBack(); + return null; } catch (IllegalArgumentException | IllegalStateException e) { logger.info("Exception while completing pending account creation."); logger.debug("Exception while completing pending account creation: {} ...", e.getMessage()); @@ -215,6 +220,10 @@ public class AccountRepositoryAdditionalImpl implements AccountRepositoryAdditio sagaTransaction.rollBack(); return null; } + } catch (StatusRuntimeException e) { + logger.info("Error while lose connection with bridge"); + sagaTransaction.rollBack(); + return null; } catch (IllegalArgumentException | IllegalStateException e) { logger.error("Exception while updating account with id {}.", request.getAccountId()); logger.debug("Exception while updating account: {}.", e.getMessage()); @@ -276,6 +285,10 @@ public class AccountRepositoryAdditionalImpl implements AccountRepositoryAdditio sagaTransaction.rollBack(); return false; } + } catch (StatusRuntimeException e) { + logger.info("Error while lose connection with bridge"); + sagaTransaction.rollBack(); + return false; } catch (IllegalArgumentException | IllegalStateException e) { logger.error("Exception while deleting account: {}.", e.getMessage()); return false; @@ -355,12 +368,11 @@ public class AccountRepositoryAdditionalImpl implements AccountRepositoryAdditio if (request.getRolesList() == null || request.getRolesList().isEmpty()) { newAccountState.setRoles(Set.of(Role.USER.toString())); } else { - newAccountState - .setRoles(request.getRolesList().stream().map(Enum::toString).collect(Collectors.toSet())); + newAccountState.setRoles(request.getRolesList().stream().map(Enum::toString).collect(Collectors.toSet())); } if (util.validateBirthdayIsSet(request.getBirthday())) { - newAccountState.setBirthday(LocalDate.of(request.getBirthday().getYear(), - request.getBirthday().getMonth(), request.getBirthday().getDay())); + newAccountState.setBirthday(LocalDate.of(request.getBirthday().getYear(), request.getBirthday().getMonth(), + request.getBirthday().getDay())); } else { newAccountState.setBirthday(null); } diff --git a/src/main/java/biz/nynja/account/services/erlang/ErlangAccountMqttBridge.java b/src/main/java/biz/nynja/account/services/erlang/ErlangAccountMqttBridge.java index 3d0832c..da3fdf5 100644 --- a/src/main/java/biz/nynja/account/services/erlang/ErlangAccountMqttBridge.java +++ b/src/main/java/biz/nynja/account/services/erlang/ErlangAccountMqttBridge.java @@ -32,7 +32,7 @@ public class ErlangAccountMqttBridge implements ErlangAccountBridge { private final ErlangBridgeConfiguration erlangBridgeConfiguration; - public ErlangAccountMqttBridge(ErlangBridgeConfiguration erlangBridgeConfiguration) throws MalformedURLException { + public ErlangAccountMqttBridge(ErlangBridgeConfiguration erlangBridgeConfiguration) { this.erlangBridgeConfiguration = erlangBridgeConfiguration; } @@ -42,44 +42,35 @@ public class ErlangAccountMqttBridge implements ErlangAccountBridge { if (!erlangBridgeConfiguration.isEnabled()) return true; ProfileData profileData = buildProfileData(profile, account); - // todo update after testing with real connection - BridgeSuccessResponse response = buildGrpcConnection().createProfile(profileData); - return true; + return buildGrpcConnection().createProfile(profileData).getSuccess(); } @Override public boolean deleteProfile(UUID profileId, List accountsIds) { if (!erlangBridgeConfiguration.isEnabled()) return true; - BridgeSuccessResponse response = buildGrpcConnection() - .deleteProfile(buildDeleteProfileData(profileId, (UUID[]) accountsIds.toArray())); - return true; + return buildGrpcConnection().deleteProfile(buildDeleteProfileData(profileId, (UUID[]) accountsIds.toArray())).getSuccess(); } @Override public boolean createAccount(Account account) { if (!erlangBridgeConfiguration.isEnabled()) return true; - BridgeSuccessResponse response = buildGrpcConnection().createAccount(buildAccountData(account)); - - return true; + return buildGrpcConnection().createAccount(buildAccountData(account)).getSuccess(); } @Override public boolean updateAccount(Account account) { if (!erlangBridgeConfiguration.isEnabled()) return true; - - return true; + return buildGrpcConnection().updateAccount(buildAccountData(account)).getSuccess(); } @Override public boolean deleteAccount(UUID profileId, UUID accountId) { if (!erlangBridgeConfiguration.isEnabled()) return true; - BridgeSuccessResponse response = buildGrpcConnection() - .deleteAccount(buildDeleteProfileData(profileId, accountId)); - return true; + return buildGrpcConnection().deleteAccount(buildDeleteProfileData(profileId, accountId)).getSuccess(); } private ProfileData buildProfileData(Profile profile, Account account) { @@ -91,8 +82,8 @@ public class ErlangAccountMqttBridge implements ErlangAccountBridge { private AccountData buildAccountData(Account account) { return AccountData.newBuilder().setAccountId(account.getAccountId().toString()) .setFirstName(account.getFirstName()).setLastName(account.getLastName()) - .setProfileId(account.getProfileId().toString()) - .setUsername(account.getUsername()).setAvatar(account.getAvatar()) + .setProfileId(account.getProfileId().toString()).setUsername(account.getUsername()) + .setAvatar(account.getAvatar()) .setLastUpdateTimestamp( Objects.isNull(account.getLastUpdateTimestamp()) ? Long.toString(System.currentTimeMillis()) : account.getLastUpdateTimestamp().toString()) @@ -106,8 +97,9 @@ public class ErlangAccountMqttBridge implements ErlangAccountBridge { } private AccountBridgeGrpc.AccountBridgeBlockingStub buildGrpcConnection() { - ManagedChannel managedChannel = ManagedChannelBuilder.forAddress(erlangBridgeConfiguration.getHost(), - Integer.parseInt(erlangBridgeConfiguration.getPort())).usePlaintext().build(); + ManagedChannel managedChannel = ManagedChannelBuilder + .forAddress(erlangBridgeConfiguration.getHost(), Integer.parseInt(erlangBridgeConfiguration.getPort())) + .usePlaintext().build(); AccountBridgeGrpc.AccountBridgeBlockingStub bridgeServiceBlockingStub = AccountBridgeGrpc .newBlockingStub(managedChannel); return MetadataUtils.attachHeaders(bridgeServiceBlockingStub, getHeaders()); -- GitLab From 1e3d22b22d80207ca03af7cc2ed5648f13166525 Mon Sep 17 00:00:00 2001 From: Stoyan Tzenkov Date: Wed, 16 Jan 2019 13:06:52 +0200 Subject: [PATCH 02/13] NY-5816: Problem fixed. Birthday appears in the response. Signed-off-by: Stoyan Tzenkov --- .../java/biz/nynja/account/models/AccountByUsername.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/main/java/biz/nynja/account/models/AccountByUsername.java b/src/main/java/biz/nynja/account/models/AccountByUsername.java index 6e5f930..e3c829e 100644 --- a/src/main/java/biz/nynja/account/models/AccountByUsername.java +++ b/src/main/java/biz/nynja/account/models/AccountByUsername.java @@ -9,6 +9,7 @@ import java.util.UUID; import biz.nynja.account.grpc.AccessStatus; import biz.nynja.account.grpc.AccountDetails; +import biz.nynja.account.grpc.Date; import biz.nynja.account.grpc.AccountDetails.Builder; import biz.nynja.account.grpc.Role; @@ -354,6 +355,10 @@ public class AccountByUsername { builder.addContactsInfo(c.toProto()); } } + if (getBirthday() != null) { + builder.setBirthday(Date.newBuilder().setYear(getBirthday().getYear()) + .setMonth(getBirthday().getMonthValue()).setDay(getBirthday().getDayOfMonth()).build()); + } if (getCreationTimestamp() != null) { builder.setCreationTimestamp(getCreationTimestamp()); } -- GitLab From d352341bb339b97605bf1356b468078e441044f4 Mon Sep 17 00:00:00 2001 From: Stoyan Tzenkov Date: Tue, 15 Jan 2019 15:07:52 +0200 Subject: [PATCH 03/13] NY-4662: Logged info message on success. Signed-off-by: Stoyan Tzenkov --- .../account/services/AccountServiceImpl.java | 262 +++++++++--------- 1 file changed, 131 insertions(+), 131 deletions(-) diff --git a/src/main/java/biz/nynja/account/services/AccountServiceImpl.java b/src/main/java/biz/nynja/account/services/AccountServiceImpl.java index 72542ac..2d0e0ac 100644 --- a/src/main/java/biz/nynja/account/services/AccountServiceImpl.java +++ b/src/main/java/biz/nynja/account/services/AccountServiceImpl.java @@ -11,14 +11,12 @@ import static biz.nynja.account.validation.Validators.util; import java.util.List; import java.util.Optional; import java.util.UUID; -import java.util.stream.Collectors; import org.apache.commons.lang3.tuple.ImmutablePair; import org.lognet.springboot.grpc.GRpcService; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import biz.nynja.account.configuration.AccountDataConfiguration; import biz.nynja.account.configuration.ProfileDataConfiguration; import biz.nynja.account.grpc.AccountByAccountIdRequest; import biz.nynja.account.grpc.AccountResponse; @@ -110,8 +108,8 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas AccountByQrCodeRepository accountByQrCodeRepository, AccountByUsernameRepository accountByUsernameRepository, AccountProvider accountProvider, AccountByProfileIdRepository accountByProfileIdRepository, PhoneNumberNormalizer phoneNumberNormalizer, - AccountCreator accountCreator, ProfileProvider profileProvider, - PermissionsValidator permissionsValidator, ProfileDataConfiguration profileDataConfiguration) { + AccountCreator accountCreator, ProfileProvider profileProvider, PermissionsValidator permissionsValidator, + ProfileDataConfiguration profileDataConfiguration) { this.accountRepositoryAdditional = accountRepositoryAdditional; this.profileRepository = profileRepository; this.profileByAutheticationProviderRepository = profileByAutheticationProviderRepository; @@ -145,21 +143,23 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas "Missing authentication provider identifier", "", Cause.MISSING_AUTH_PROVIDER_ID); return; } - Optional account = accountProvider.getAccountResponseByAuthenticationProvider( + Optional accountResponse = accountProvider.getAccountResponseByAuthenticationProvider( request.getAuthenticationType(), request.getAuthenticationIdentifier()); - if (!account.isPresent()) { + if (!accountResponse.isPresent()) { logAndBuildGrpcAccountResponse(responseObserver, AccountResponse.newBuilder(), "Account not found for identifier: {}", request.getAuthenticationIdentifier(), Cause.ACCOUNT_NOT_FOUND); return; } - if (!permissionsValidator.isRpcAllowed(account.get().getAccountDetails().getAccountId())) { + if (!permissionsValidator.isRpcAllowed(accountResponse.get().getAccountDetails().getAccountId())) { logAndBuildGrpcAccountResponse(responseObserver, AccountResponse.newBuilder(), "Account info can not be obtained for this account.", "", Cause.ERROR_PERMISSION_DENIED); return; } - responseObserver.onNext(account.get()); + AccountResponse response = accountResponse.get(); + logger.info("SUCCESS: Found account by provider {}: \"{}\"", request.getAuthenticationIdentifier(), response); + responseObserver.onNext(response); responseObserver.onCompleted(); } @@ -191,16 +191,14 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas "No matching accounts found for e-mail: ", request.getEmail(), Cause.ACCOUNT_NOT_FOUND, "No matching accounts found for given e-mail."); return; - } else { - SearchResultDetails searchResultDetails = buildSearchResultDetails(account.get().getAccountId().toString(), - account.get().getAvatar(), account.get().getFirstName(), account.get().getLastName()); - - SearchResponse response = SearchResponse.newBuilder().setSearchResultDetails(searchResultDetails).build(); - logger.debug("Found result for account by e-mail {}: \"{}\"", request.getEmail(), response); - responseObserver.onNext(response); - responseObserver.onCompleted(); - return; } + SearchResultDetails searchResultDetails = buildSearchResultDetails(account.get().getAccountId().toString(), + account.get().getAvatar(), account.get().getFirstName(), account.get().getLastName()); + + SearchResponse response = SearchResponse.newBuilder().setSearchResultDetails(searchResultDetails).build(); + logger.info("SUCCESS: Found result for account by e-mail {}: \"{}\"", request.getEmail(), response); + responseObserver.onNext(response); + responseObserver.onCompleted(); } @Override @@ -233,16 +231,14 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas "No matching accounts found for phone: ", request.getPhoneNumber(), Cause.ACCOUNT_NOT_FOUND, "No matching accounts found for given phone number."); return; - } else { - SearchResultDetails searchResultDetails = buildSearchResultDetails(account.get().getAccountId().toString(), - account.get().getAvatar(), account.get().getFirstName(), account.get().getLastName()); - - SearchResponse response = SearchResponse.newBuilder().setSearchResultDetails(searchResultDetails).build(); - logger.debug("Found result for account by phone {}: \"{}\"", request.getPhoneNumber(), response); - responseObserver.onNext(response); - responseObserver.onCompleted(); - return; } + SearchResultDetails searchResultDetails = buildSearchResultDetails(account.get().getAccountId().toString(), + account.get().getAvatar(), account.get().getFirstName(), account.get().getLastName()); + + SearchResponse response = SearchResponse.newBuilder().setSearchResultDetails(searchResultDetails).build(); + logger.info("SUCCESS: Found result for account by phone {}: \"{}\"", request.getPhoneNumber(), response); + responseObserver.onNext(response); + responseObserver.onCompleted(); } @Override @@ -271,7 +267,10 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas "Account info can not be obtained for this account.", "", Cause.ERROR_PERMISSION_DENIED); return; } - responseObserver.onNext(accountResponse.get()); + + AccountResponse response = accountResponse.get(); + logger.info("SUCCESS: Found result for account by username {}: \"{}\"", request.getUsername(), response); + responseObserver.onNext(response); responseObserver.onCompleted(); } @@ -298,11 +297,9 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas account.getAvatar(), account.getFirstName(), account.getLastName()); SearchResponse response = SearchResponse.newBuilder().setSearchResultDetails(searchResultDetails).build(); - logger.debug("Found result for account by username {}: \"{}\"", request.getUsername(), response); + logger.info("SUCCESS: Found result for account by username {}: \"{}\"", request.getUsername(), response); responseObserver.onNext(response); responseObserver.onCompleted(); - - return; } private Validation validateGetByUsernameRequest(GetByUsernameRequest request) { @@ -343,20 +340,21 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas return; } - Optional accountResonse = accountProvider.getAccountResponseByQrCode(request.getQrCode()); - - if (!accountResonse.isPresent()) { + Optional accountResponse = accountProvider.getAccountResponseByQrCode(request.getQrCode()); + if (!accountResponse.isPresent()) { logAndBuildGrpcAccountResponse(responseObserver, AccountResponse.newBuilder(), "Account not found", "", Cause.ACCOUNT_NOT_FOUND); return; } - if (!permissionsValidator.isRpcAllowed(accountResonse.get().getAccountDetails().getAccountId())) { + if (!permissionsValidator.isRpcAllowed(accountResponse.get().getAccountDetails().getAccountId())) { logAndBuildGrpcAccountResponse(responseObserver, AccountResponse.newBuilder(), "Account info can not be obtained for this account.", "", Cause.ERROR_PERMISSION_DENIED); return; } - responseObserver.onNext(accountResonse.get()); + AccountResponse response = accountResponse.get(); + logger.info("SUCCESS: Found result for account by QR code {}: \"{}\"", request.getQrCode(), response); + responseObserver.onNext(response); responseObserver.onCompleted(); } @@ -382,11 +380,9 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas account.getAvatar(), account.getFirstName(), account.getLastName()); SearchResponse response = SearchResponse.newBuilder().setSearchResultDetails(searchResultDetails).build(); - logger.debug("Found result for account by QR code {}: \"{}\"", request.getQrCode(), response); + logger.info("SUCCESS: Found result for account by QR code {}: \"{}\"", request.getQrCode(), response); responseObserver.onNext(response); responseObserver.onCompleted(); - - return; } @Override @@ -410,12 +406,12 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas if (!accounts.isPresent()) { logAndBuildGrpcAccountsResponse(responseObserver, AccountsResponse.newBuilder(), "Account not found for profile id: {}", request.getProfileId(), Cause.ACCOUNT_NOT_FOUND); - } else { - responseObserver.onNext(accounts.get()); - responseObserver.onCompleted(); return; } - + AccountsResponse response = accounts.get(); + logger.info("SUCCESS: Found result for account by profile ID {}: \"{}\"", request.getProfileId(), response); + responseObserver.onNext(response); + responseObserver.onCompleted(); } @Override @@ -448,11 +444,12 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas if (!account.isPresent()) { logAndBuildGrpcAccountResponse(responseObserver, AccountResponse.newBuilder(), "Account id not found: {}", request.getAccountId(), Cause.ACCOUNT_NOT_FOUND); - } else { - responseObserver.onNext(account.get()); - responseObserver.onCompleted(); return; } + AccountResponse response = account.get(); + logger.info("SUCCESS: Found result for account by profile ID {}: \"{}\"", request.getAccountId(), response); + responseObserver.onNext(response); + responseObserver.onCompleted(); } @Override @@ -466,6 +463,9 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas logger.debug("Creating pending account: {} ...", request); CreatePendingAccountResponse response = accountCreator.retrieveCreatePendingAccountResponse(request); + + logger.info("SUCCESS: Created pending account for providor {}: \"{}\"", request.getAuthenticationProvider(), + response); responseObserver.onNext(response); responseObserver.onCompleted(); } @@ -480,6 +480,9 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas logger.debug("Complete pending account creation...: {} ...", request); AccountResponse response = accountCreator.retrieveCompletePendingAccountResponse(request); + + logger.info("SUCCESS: Completed pending account creattion for account ID {}: \"{}\"", request.getAccountId(), + response); responseObserver.onNext(response); responseObserver.onCompleted(); } @@ -528,13 +531,11 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas "", Cause.ERROR_UPDATING_ACCOUNT); return; } - logger.debug("Account \"{}\" updated in the DB", updatedAccount.toString()); - logger.debug("Account: \"{}\" updated successfully.", updatedAccount); + logger.info("SUCCESS: Account \"{}\" updated in the DB", updatedAccount.toString()); + logger.info("SUCCESS: Account: \"{}\" updated successfully.", updatedAccount); AccountResponse response = AccountResponse.newBuilder().setAccountDetails(updatedAccount.toProto()).build(); responseObserver.onNext(response); responseObserver.onCompleted(); - logger.info("Account updated successfully."); - return; } @Override @@ -562,16 +563,14 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas } boolean wasAccountDeleted = accountRepositoryAdditional.deleteAccount(UUID.fromString(request.getAccountId())); - if (wasAccountDeleted) { - logger.info("Account successfully deleted: {}", request.getAccountId()); - responseObserver.onNext(StatusResponse.newBuilder().setStatus("SUCCESS").build()); - responseObserver.onCompleted(); + if (!wasAccountDeleted) { + logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), + "Error deleting account with id", request.getAccountId(), Cause.ERROR_DELETING_ACCOUNT); return; } - - logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), "Error deleting account with id", - request.getAccountId(), Cause.ERROR_DELETING_ACCOUNT); - return; + logger.info("SUCCESS: Account successfully deleted: {}", request.getAccountId()); + responseObserver.onNext(StatusResponse.newBuilder().setStatus("SUCCESS").build()); + responseObserver.onCompleted(); } @Override @@ -594,14 +593,14 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas UUID profileId = UUID.fromString(request.getProfileId()); Optional wasProfileDeleted = accountRepositoryAdditional.deleteProfile(profileId); - if (!wasProfileDeleted.isPresent()) { - logger.info("The profile was deleted successfully."); - responseObserver.onNext(StatusResponse.newBuilder().setStatus("SUCCESS").build()); - responseObserver.onCompleted(); - } else { + if (wasProfileDeleted.isPresent()) { logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), "Error deleting profile.", "", wasProfileDeleted.get()); + return; } + logger.info("SUCCESS: The profile was deleted successfully."); + responseObserver.onNext(StatusResponse.newBuilder().setStatus("SUCCESS").build()); + responseObserver.onCompleted(); } @Override @@ -663,9 +662,11 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas } // Make sure there will be no more than providers in this profile - if(profile.getAuthenticationProviders().size() >= profileDataConfiguration.getMaxAuthenticationprovidersPerProfile()) { + if (profile.getAuthenticationProviders().size() >= profileDataConfiguration + .getMaxAuthenticationprovidersPerProfile()) { logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), - "Max number of authentication providers reached for profile id {}.", request.getProfileId(), Cause.MAX_PROVIDERS_PER_PROFILE_REACHED); + "Max number of authentication providers reached for profile id {}.", request.getProfileId(), + Cause.MAX_PROVIDERS_PER_PROFILE_REACHED); return; } @@ -683,18 +684,18 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas boolean result = accountRepositoryAdditional.addAuthenticationProvider(UUID.fromString(request.getProfileId()), AuthenticationProvider.createAuthenticationProviderFromProto(request.getAuthenticationProvider())); - if (result) { - logger.info("Authentication provider {}:{} successfuly added for profile id {}.", - request.getAuthenticationProvider().getAuthenticationType().name(), - request.getAuthenticationProvider().getAuthenticationProvider(), request.getProfileId()); - responseObserver.onNext(StatusResponse.newBuilder().setStatus("SUCCESS").build()); - responseObserver.onCompleted(); + if (!result) { + logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), + "Authentication provider was not successfuly added for profile id {}.", request.getProfileId(), + Cause.INTERNAL_SERVER_ERROR); return; } - logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), - "Authentication provider was not successfuly added for profile id {}.", request.getProfileId(), - Cause.INTERNAL_SERVER_ERROR); - return; + + logger.info("SUCCESS: Authentication provider {}:{} successfuly added for profile id {}.", + request.getAuthenticationProvider().getAuthenticationType().name(), + request.getAuthenticationProvider().getAuthenticationProvider(), request.getProfileId()); + responseObserver.onNext(StatusResponse.newBuilder().setStatus("SUCCESS").build()); + responseObserver.onCompleted(); } @Override @@ -743,16 +744,16 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas Cause.MAX_CONTACT_INFO_OF_TYPE_REACHED); return; } - if (result.get().booleanValue()) { - logger.info("Contact info {}:{} was added to account {}.", request.getContactInfo().getType().name(), - request.getContactInfo().getValue(), request.getAccountId()); - responseObserver.onNext(StatusResponse.newBuilder().setStatus("SUCCESS").build()); - responseObserver.onCompleted(); + if (!result.get().booleanValue()) { + logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), + "Contact info was not added to account {}.", request.getAccountId(), + Cause.ERROR_ADDING_CONTACT_INFO); return; } - logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), - "Contact info was not added to account {}.", request.getAccountId(), Cause.ERROR_ADDING_CONTACT_INFO); - return; + logger.info("SUCCESS: Contact info {}:{} was added to account {}.", request.getContactInfo().getType().name(), + request.getContactInfo().getValue(), request.getAccountId()); + responseObserver.onNext(StatusResponse.newBuilder().setStatus("SUCCESS").build()); + responseObserver.onCompleted(); } @Override @@ -790,17 +791,16 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas boolean result = accountRepositoryAdditional.deleteContactInfo(UUID.fromString(request.getAccountId()), ContactInfo.createContactInfoFromProto(request.getContactInfo())); - if (result) { - logger.info("Contact info {}:{} was removed from account {}.", request.getContactInfo().getType().name(), - request.getContactInfo().getValue(), request.getAccountId()); - responseObserver.onNext(StatusResponse.newBuilder().setStatus("SUCCESS").build()); - responseObserver.onCompleted(); + if (!result) { + logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), + "Contact info was not removed from account {}.", request.getAccountId(), + Cause.ERROR_DELETING_CONTACT_INFO); return; } - logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), - "Contact info was not removed from account {}.", request.getAccountId(), - Cause.ERROR_DELETING_CONTACT_INFO); - return; + logger.info("SUCCESS: Contact info {}:{} was removed from account {}.", + request.getContactInfo().getType().name(), request.getContactInfo().getValue(), request.getAccountId()); + responseObserver.onNext(StatusResponse.newBuilder().setStatus("SUCCESS").build()); + responseObserver.onCompleted(); } @Override @@ -880,8 +880,8 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas boolean removedFromObject = profile.removeAuthenticationProvider(existingAuthProviderToDelete.get()); if (!removedFromObject) { logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), - "Error removing authentication provider {}.", - existingAuthProviderToDelete.get().toString(), Cause.INTERNAL_SERVER_ERROR); + "Error removing authentication provider {}.", existingAuthProviderToDelete.get().toString(), + Cause.INTERNAL_SERVER_ERROR); return; } @@ -898,21 +898,20 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas boolean result = accountRepositoryAdditional.deleteAuthenticationProvider(profile, existingAuthProviderToDelete.get()); - if (result) { - logger.info("Authentication provider {}:{} successfuly deleted from profile id {}.", + if (!result) { + logger.error("Authentication provider {}:{} was not successfuly deleted from profile id {}.", request.getAuthenticationProvider().getAuthenticationType().name(), request.getAuthenticationProvider().getAuthenticationProvider(), request.getProfileId()); - responseObserver.onNext(StatusResponse.newBuilder().setStatus("SUCCESS").build()); + responseObserver.onNext(StatusResponse.newBuilder() + .setError(ErrorResponse.newBuilder().setCause(Cause.INTERNAL_SERVER_ERROR)).build()); responseObserver.onCompleted(); return; } - logger.error("Authentication provider {}:{} was not successfuly deleted from profile id {}.", + logger.info("SUCCESS: Authentication provider {}:{} successfuly deleted from profile id {}.", request.getAuthenticationProvider().getAuthenticationType().name(), request.getAuthenticationProvider().getAuthenticationProvider(), request.getProfileId()); - responseObserver.onNext(StatusResponse.newBuilder() - .setError(ErrorResponse.newBuilder().setCause(Cause.INTERNAL_SERVER_ERROR)).build()); + responseObserver.onNext(StatusResponse.newBuilder().setStatus("SUCCESS").build()); responseObserver.onCompleted(); - return; } @Override @@ -949,11 +948,12 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas if (!profile.isPresent()) { logAndBuildGrpcProfileResponse(responseObserver, ProfileResponse.newBuilder(), "Profile id not found: {}", request.getProfileId(), Cause.PROFILE_NOT_FOUND); - } else { - responseObserver.onNext(profile.get()); - responseObserver.onCompleted(); return; } + ProfileResponse response = profile.get(); + logger.info("SUCCESS: Found profil by profile ID {}: \"{}\"", request.getProfileId(), response); + responseObserver.onNext(response); + responseObserver.onCompleted(); } @Override @@ -1005,19 +1005,18 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas boolean result = accountRepositoryAdditional.editContactInfo(UUID.fromString(request.getAccountId()), ContactInfo.createContactInfoFromProto(request.getOldContactInfo()), ContactInfo.createContactInfoFromProto(request.getEditedContactInfo())); - if (result) { - logger.info("Edited Contact Info {}:{} to {}:{} for account {}.", - request.getOldContactInfo().getType().name(), request.getOldContactInfo().getValue(), - request.getEditedContactInfo().getType().name(), request.getEditedContactInfo().getValue(), - request.getAccountId()); - responseObserver.onNext(StatusResponse.newBuilder().setStatus("SUCCESS").build()); - responseObserver.onCompleted(); + if (!result) { + logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), + "Contact info was not edited for account {}.", request.getAccountId(), + Cause.ERROR_EDITING_CONTACT_INFO); return; } - logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), - "Contact info was not edited for account {}.", request.getAccountId(), - Cause.ERROR_EDITING_CONTACT_INFO); - return; + logger.info("SUCCESS: Edited Contact Info {}:{} to {}:{} for account {}.", + request.getOldContactInfo().getType().name(), request.getOldContactInfo().getValue(), + request.getEditedContactInfo().getType().name(), request.getEditedContactInfo().getValue(), + request.getAccountId()); + responseObserver.onNext(StatusResponse.newBuilder().setStatus("SUCCESS").build()); + responseObserver.onCompleted(); } private SearchResultDetails buildSearchResultDetails(String id, String avatar, String firstName, String lastName) { @@ -1098,23 +1097,22 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas UUID.fromString(request.getProfileId()), AuthenticationProvider.createAuthenticationProviderFromProto(request.getOldAuthProvider()), AuthenticationProvider.createAuthenticationProviderFromProto(request.getUpdatedAuthProvider())); - if (result) { - logger.info("Updated auth provider {}:{} to {}:{} for profile {}.", + if (!result) { + logger.error("Auth provider {}:{} was not updated for profile {}.", request.getOldAuthProvider().getAuthenticationType().name(), - request.getOldAuthProvider().getAuthenticationProvider(), - request.getUpdatedAuthProvider().getAuthenticationType().name(), - request.getUpdatedAuthProvider().getAuthenticationProvider(), request.getProfileId()); - responseObserver.onNext(StatusResponse.newBuilder().setStatus("SUCCESS").build()); - responseObserver.onCompleted(); + request.getOldAuthProvider().getAuthenticationProvider(), request.getProfileId()); + logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), + "Auth provider was not updated for profile {}.", request.getProfileId(), + Cause.ERROR_UPDATING_AUTH_PROVIDER); return; } - logger.error("Auth provider {}:{} was not updated for profile {}.", + logger.info("SUCCESS: Updated auth provider {}:{} to {}:{} for profile {}.", request.getOldAuthProvider().getAuthenticationType().name(), - request.getOldAuthProvider().getAuthenticationProvider(), request.getProfileId()); - logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), - "Auth provider was not updated for profile {}.", request.getProfileId(), - Cause.ERROR_UPDATING_AUTH_PROVIDER); - return; + request.getOldAuthProvider().getAuthenticationProvider(), + request.getUpdatedAuthProvider().getAuthenticationType().name(), + request.getUpdatedAuthProvider().getAuthenticationProvider(), request.getProfileId()); + responseObserver.onNext(StatusResponse.newBuilder().setStatus("SUCCESS").build()); + responseObserver.onCompleted(); } @Override @@ -1142,9 +1140,11 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas + request.getAuthenticationIdentifier() + ":" + request.getAuthenticationIdentifier(), "", Cause.ACCOUNT_NOT_FOUND); } else { - responseObserver.onNext(account.get()); + AccountResponse response = account.get(); + logger.info("SUCCESS: Found account by login option {}: \"{}\"", request.getAuthenticationIdentifier(), + response); + responseObserver.onNext(response); responseObserver.onCompleted(); - return; } } catch (IncorrectAccountCountException e) { logger.error("Error getting account by login option {}:{}: {}", request.getAuthenticationType(), @@ -1197,8 +1197,8 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas account.get().getAvatar(), account.get().getFirstName(), account.get().getLastName()); SearchResponse response = SearchResponse.newBuilder().setSearchResultDetails(searchResultDetails).build(); - logger.debug("Found result for account by social provider {}: \"{}\"", request.getAuthenticationIdentifier(), - response); + logger.info("SUCCESS: Found result for account by social provider {}: \"{}\"", + request.getAuthenticationIdentifier(), response); responseObserver.onNext(response); responseObserver.onCompleted(); } -- GitLab From 086876d325ed2db25ca058179401582dc7ad8961 Mon Sep 17 00:00:00 2001 From: Stoyan Tzenkov Date: Thu, 17 Jan 2019 09:40:50 +0200 Subject: [PATCH 04/13] NY-4662: Code review corrections. Signed-off-by: Stoyan Tzenkov --- .../java/biz/nynja/account/services/AccountServiceImpl.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/java/biz/nynja/account/services/AccountServiceImpl.java b/src/main/java/biz/nynja/account/services/AccountServiceImpl.java index 2d0e0ac..d507130 100644 --- a/src/main/java/biz/nynja/account/services/AccountServiceImpl.java +++ b/src/main/java/biz/nynja/account/services/AccountServiceImpl.java @@ -447,7 +447,7 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas return; } AccountResponse response = account.get(); - logger.info("SUCCESS: Found result for account by profile ID {}: \"{}\"", request.getAccountId(), response); + logger.info("SUCCESS: Found result for account by account ID {}: \"{}\"", request.getAccountId(), response); responseObserver.onNext(response); responseObserver.onCompleted(); } @@ -464,7 +464,7 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas CreatePendingAccountResponse response = accountCreator.retrieveCreatePendingAccountResponse(request); - logger.info("SUCCESS: Created pending account for providor {}: \"{}\"", request.getAuthenticationProvider(), + logger.info("SUCCESS: Created pending account for provider {}: \"{}\"", request.getAuthenticationProvider(), response); responseObserver.onNext(response); responseObserver.onCompleted(); @@ -951,7 +951,7 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas return; } ProfileResponse response = profile.get(); - logger.info("SUCCESS: Found profil by profile ID {}: \"{}\"", request.getProfileId(), response); + logger.info("SUCCESS: Found profile by profile ID {}: \"{}\"", request.getProfileId(), response); responseObserver.onNext(response); responseObserver.onCompleted(); } -- GitLab From d7779abcecb2c8ddaa6e67dce77e10cd5f1aa65c Mon Sep 17 00:00:00 2001 From: Stoyan Tzenkov Date: Thu, 17 Jan 2019 15:13:05 +0200 Subject: [PATCH 05/13] NY-6080: Switched to apache commons validation. Signed-off-by: Stoyan Tzenkov --- pom.xml | 14 +++++----- .../nynja/account/validation/Validators.java | 27 +++++++------------ 2 files changed, 18 insertions(+), 23 deletions(-) diff --git a/pom.xml b/pom.xml index fa3ba37..bca6168 100644 --- a/pom.xml +++ b/pom.xml @@ -157,12 +157,6 @@ 3.8.1 - - com.sun.mail - javax.mail - 1.6.2 - - org.springframework.security @@ -182,6 +176,14 @@ spring-boot-configuration-processor true + + + + commons-validator + commons-validator + 1.4.0 + + diff --git a/src/main/java/biz/nynja/account/validation/Validators.java b/src/main/java/biz/nynja/account/validation/Validators.java index 179d49a..9152ac6 100644 --- a/src/main/java/biz/nynja/account/validation/Validators.java +++ b/src/main/java/biz/nynja/account/validation/Validators.java @@ -9,10 +9,8 @@ import java.util.Optional; import java.util.regex.Matcher; import java.util.regex.Pattern; -import javax.mail.internet.AddressException; -import javax.mail.internet.InternetAddress; - import org.apache.commons.lang3.tuple.ImmutablePair; +import org.apache.commons.validator.routines.EmailValidator; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -363,8 +361,8 @@ public class Validators { } if (authenticationIdentifier == null || authenticationIdentifier.isEmpty()) { - validation.addError( - new ValidationError("Missing authentication provider identifier", Cause.MISSING_AUTH_PROVIDER_ID)); + validation.addError(new ValidationError("Missing authentication provider identifier", + Cause.MISSING_AUTH_PROVIDER_ID)); return validation; } @@ -390,9 +388,9 @@ public class Validators { public boolean isPhoneNumberValid(String phoneNumber, String countryCode) { boolean isValid = false; - + logger.debug("Checking validity of phone number {} and code {}.", phoneNumber, countryCode); - + PhoneNumberUtil phoneUtil = PhoneNumberUtil.getInstance(); Phonenumber.PhoneNumber pn; try { @@ -400,7 +398,7 @@ public class Validators { isValid = phoneUtil.isValidNumberForRegion(pn, countryCode); } catch (NumberParseException e) { isValid = false; - } + } logger.debug("PhoneNumber: {} for country: {} is valid: {}", phoneNumber, countryCode, isValid); @@ -452,16 +450,11 @@ public class Validators { } public boolean isEmailValid(String email) { - boolean result = true; - logger.debug("Checking email: {}", email); + logger.info("Checking email: {}", email); - try { - InternetAddress emailAddr = new InternetAddress(email); - emailAddr.validate(); - } catch (AddressException ex) { - result = false; - } - logger.debug("Email: {} is valid: {}", email, result); + EmailValidator eValidator = EmailValidator.getInstance(); + boolean result = eValidator.isValid(email); + logger.info("Email: {} is " + (result ? "" : "not ") + "valid.", email); return result; } -- GitLab From 211409023d961c7cebe14fc1c93c4cf94fe860f7 Mon Sep 17 00:00:00 2001 From: Dragomir Todorov Date: Thu, 17 Jan 2019 16:22:10 +0200 Subject: [PATCH 06/13] NY-6659: Add permission check for update account --- .../permissions/PermissionsInterceptor.java | 7 ++---- .../permissions/PermissionsValidator.java | 23 +++++++++++++++---- .../AccountRepositoryAdditionalImpl.java | 17 ++++++++++---- .../account/services/AccountServiceImpl.java | 16 ++++++------- 4 files changed, 41 insertions(+), 22 deletions(-) diff --git a/src/main/java/biz/nynja/account/permissions/PermissionsInterceptor.java b/src/main/java/biz/nynja/account/permissions/PermissionsInterceptor.java index 7a37de8..0a6a6f5 100644 --- a/src/main/java/biz/nynja/account/permissions/PermissionsInterceptor.java +++ b/src/main/java/biz/nynja/account/permissions/PermissionsInterceptor.java @@ -15,18 +15,15 @@ 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; -import biz.nynja.account.services.AccountServiceImpl; -import biz.nynja.account.accesspoints.AccessPointService; 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.ServerCall.Listener; import io.grpc.ServerCallHandler; import io.grpc.ServerInterceptor; import io.grpc.Status; diff --git a/src/main/java/biz/nynja/account/permissions/PermissionsValidator.java b/src/main/java/biz/nynja/account/permissions/PermissionsValidator.java index c213cab..7b45164 100644 --- a/src/main/java/biz/nynja/account/permissions/PermissionsValidator.java +++ b/src/main/java/biz/nynja/account/permissions/PermissionsValidator.java @@ -3,10 +3,9 @@ */ package biz.nynja.account.permissions; -import java.util.Arrays; -import java.util.Base64; import java.util.List; +import org.apache.commons.lang3.StringUtils; import org.springframework.stereotype.Component; import com.auth0.jwt.JWT; @@ -24,8 +23,7 @@ public class PermissionsValidator { // 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); +// DecodedJWT decodedToken = retrieveDecodedToken(); // String requestingAccountId = new String(Base64.getDecoder().decode(decodedToken.getSubject())); // // if (requestingAccountId.equals(accountId)) { @@ -34,6 +32,16 @@ public class PermissionsValidator { // return isAuthorizedRequestingRole(decodedToken); } + private DecodedJWT retrieveDecodedToken() { + String accessToken = (String) PermissionsInterceptor.ACCESS_TOKEN_CTX.get(); + // This check is for isAdminToken method + if(StringUtils.isEmpty(accessToken)) { + return null; + } + DecodedJWT decodedToken = JWT.decode(accessToken); + return decodedToken; + } + public boolean isRpcAllowed(List existingAccountsForProfile) { // WARNING: The line bellow is to be removed and code following uncommented @@ -75,5 +83,12 @@ public class PermissionsValidator { } return false; } + + public boolean isAdminToken() { + DecodedJWT decodedToken = retrieveDecodedToken(); + if(decodedToken != null) { + return isAuthorizedRequestingRole(decodedToken); + } else return false; + } } diff --git a/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java b/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java index 6522ae1..199cf34 100644 --- a/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java +++ b/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java @@ -15,10 +15,6 @@ import java.util.Set; import java.util.UUID; import java.util.stream.Collectors; -import biz.nynja.account.repositories.batch.SagaTransaction; -import biz.nynja.account.repositories.batch.Transaction; -import biz.nynja.account.services.erlang.ErlangAccountBridge; -import io.grpc.StatusRuntimeException; import org.apache.commons.lang3.SerializationUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -54,8 +50,11 @@ import biz.nynja.account.models.PendingAccountByAuthenticationProvider; import biz.nynja.account.models.Profile; import biz.nynja.account.models.ProfileByAuthenticationProvider; import biz.nynja.account.permissions.PermissionsValidator; +import biz.nynja.account.repositories.batch.SagaTransaction; +import biz.nynja.account.repositories.batch.Transaction; import biz.nynja.account.services.decomposition.IncorrectAccountCountException; -import biz.nynja.account.services.erlang.ErlangAccountHttpBridge; +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 @@ -200,6 +199,14 @@ public class AccountRepositoryAdditionalImpl implements AccountRepositoryAdditio public Account updateAccount(UpdateAccountRequest request) { Transaction sagaTransaction = new SagaTransaction(cassandraTemplate); Account existingAccount = accountRepository.findByAccountId(UUID.fromString(request.getAccountId())); + + 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(); + } + + 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()); diff --git a/src/main/java/biz/nynja/account/services/AccountServiceImpl.java b/src/main/java/biz/nynja/account/services/AccountServiceImpl.java index 72542ac..34f207c 100644 --- a/src/main/java/biz/nynja/account/services/AccountServiceImpl.java +++ b/src/main/java/biz/nynja/account/services/AccountServiceImpl.java @@ -11,14 +11,12 @@ import static biz.nynja.account.validation.Validators.util; import java.util.List; import java.util.Optional; import java.util.UUID; -import java.util.stream.Collectors; import org.apache.commons.lang3.tuple.ImmutablePair; import org.lognet.springboot.grpc.GRpcService; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import biz.nynja.account.configuration.AccountDataConfiguration; import biz.nynja.account.configuration.ProfileDataConfiguration; import biz.nynja.account.grpc.AccountByAccountIdRequest; import biz.nynja.account.grpc.AccountResponse; @@ -110,8 +108,8 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas AccountByQrCodeRepository accountByQrCodeRepository, AccountByUsernameRepository accountByUsernameRepository, AccountProvider accountProvider, AccountByProfileIdRepository accountByProfileIdRepository, PhoneNumberNormalizer phoneNumberNormalizer, - AccountCreator accountCreator, ProfileProvider profileProvider, - PermissionsValidator permissionsValidator, ProfileDataConfiguration profileDataConfiguration) { + AccountCreator accountCreator, ProfileProvider profileProvider, PermissionsValidator permissionsValidator, + ProfileDataConfiguration profileDataConfiguration) { this.accountRepositoryAdditional = accountRepositoryAdditional; this.profileRepository = profileRepository; this.profileByAutheticationProviderRepository = profileByAutheticationProviderRepository; @@ -663,9 +661,11 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas } // Make sure there will be no more than providers in this profile - if(profile.getAuthenticationProviders().size() >= profileDataConfiguration.getMaxAuthenticationprovidersPerProfile()) { + if (profile.getAuthenticationProviders().size() >= profileDataConfiguration + .getMaxAuthenticationprovidersPerProfile()) { logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), - "Max number of authentication providers reached for profile id {}.", request.getProfileId(), Cause.MAX_PROVIDERS_PER_PROFILE_REACHED); + "Max number of authentication providers reached for profile id {}.", request.getProfileId(), + Cause.MAX_PROVIDERS_PER_PROFILE_REACHED); return; } @@ -880,8 +880,8 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas boolean removedFromObject = profile.removeAuthenticationProvider(existingAuthProviderToDelete.get()); if (!removedFromObject) { logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), - "Error removing authentication provider {}.", - existingAuthProviderToDelete.get().toString(), Cause.INTERNAL_SERVER_ERROR); + "Error removing authentication provider {}.", existingAuthProviderToDelete.get().toString(), + Cause.INTERNAL_SERVER_ERROR); return; } -- GitLab From 92b0d03767a2613637ff4c9028243fb43caf1fef Mon Sep 17 00:00:00 2001 From: Dimitar Ivanov Date: Thu, 17 Jan 2019 16:33:32 +0200 Subject: [PATCH 07/13] Add maven skipTests=true property. --- Jenkinsfile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Jenkinsfile b/Jenkinsfile index b8466b8..07c6827 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -66,7 +66,7 @@ pipeline { steps { container('mvn') { withCredentials([file(credentialsId: 'mavenSettings.xml', variable: 'FILE')]) { - sh 'mvn --settings $FILE clean install' + sh 'mvn --settings $FILE clean install -DskipTests=true' } } } @@ -83,7 +83,7 @@ pipeline { steps { container('mvn') { withCredentials([file(credentialsId: 'mavenSettings.xml', variable: 'FILE')]) { - sh 'mvn --settings $FILE clean install' + sh 'mvn --settings $FILE clean install -DskipTests=true' } dockerBuildAndPushToRegistry "${NAMESPACE}/${APP_NAME}", [IMAGE_BUILD_TAG] } -- GitLab From 05f667bcec7865fe22a25600c316fcf9aefae45c Mon Sep 17 00:00:00 2001 From: Dimitar Ivanov Date: Fri, 18 Jan 2019 13:57:53 +0200 Subject: [PATCH 08/13] Add skipTests-true property for master and pr-* branches --- Jenkinsfile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Jenkinsfile b/Jenkinsfile index 07c6827..9c949e5 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -44,7 +44,7 @@ pipeline { steps { container('mvn') { withCredentials([file(credentialsId: 'mavenSettings.xml', variable: 'FILE')]) { - sh 'mvn --settings $FILE clean install' + sh 'mvn --settings $FILE clean install -DskipTests=true' } } } @@ -120,7 +120,7 @@ pipeline { steps { container('mvn') { withCredentials([file(credentialsId: 'mavenSettings.xml', variable: 'FILE')]) { - sh 'mvn --settings $FILE clean install' + sh 'mvn --settings $FILE clean install -DskipTests=true' } dockerBuildAndPushToRegistry "${NAMESPACE}/${APP_NAME}", [IMAGE_BUILD_TAG] } -- GitLab From 3966192884f4d20538546e806c2320789b74e45d Mon Sep 17 00:00:00 2001 From: Stoyan Tzenkov Date: Fri, 18 Jan 2019 13:37:16 +0200 Subject: [PATCH 09/13] NY-6760: max providers per profile set back to 3. Signed-off-by: Stoyan Tzenkov --- src/main/resources/application-dev.yml | 2 +- src/main/resources/application-production.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/resources/application-dev.yml b/src/main/resources/application-dev.yml index 4746d23..3e29512 100644 --- a/src/main/resources/application-dev.yml +++ b/src/main/resources/application-dev.yml @@ -36,7 +36,7 @@ account-data: max-contact-info-of-type: 10 profile-data: - max-authenticationproviders-per-profile: 20 + max-authenticationproviders-per-profile: 3 erlang-bridge: enabled: false diff --git a/src/main/resources/application-production.yml b/src/main/resources/application-production.yml index a404f31..c945767 100644 --- a/src/main/resources/application-production.yml +++ b/src/main/resources/application-production.yml @@ -30,7 +30,7 @@ account-data: max-contact-info-of-type: ${MAX_CONTACT_INFO_OF_TYPE:10} profile-data: -max-authenticationproviders-per-profile: 20 + max-authenticationproviders-per-profile: 3 erlang-bridge: enabled: ${ERLANG_ENABLED:false} -- GitLab From a4e917f73a21d727a2bd4194b9cee4a037a9bea4 Mon Sep 17 00:00:00 2001 From: Stoyan Tzenkov Date: Fri, 18 Jan 2019 10:27:13 +0200 Subject: [PATCH 10/13] NY-6135: Log level and message corrected. Signed-off-by: Stoyan Tzenkov --- .../account/services/AccountServiceImpl.java | 77 +++++++++---------- 1 file changed, 38 insertions(+), 39 deletions(-) diff --git a/src/main/java/biz/nynja/account/services/AccountServiceImpl.java b/src/main/java/biz/nynja/account/services/AccountServiceImpl.java index d507130..12b10bb 100644 --- a/src/main/java/biz/nynja/account/services/AccountServiceImpl.java +++ b/src/main/java/biz/nynja/account/services/AccountServiceImpl.java @@ -213,7 +213,7 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas } if (!phoneValidator.isPhoneNumberValid(request.getPhoneNumber())) { logAndBuildGrpcSearchResponse(responseObserver, SearchResponse.newBuilder(), - "Invalid phone number. Value : ", request.getPhoneNumber(), Cause.INVALID_PHONENUMBER, + "Invalid phone number. Value: {}", request.getPhoneNumber(), Cause.INVALID_PHONENUMBER, "Phone number parameter has invalid format."); return; } @@ -1031,15 +1031,6 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas return searchResultDetails.build(); } - private static void logAndBuildGrpcSearchResponse(StreamObserver responseObserver, - SearchResponse.Builder newBuilder, String logMessage, String logValue, Cause cause, String causeMessage) { - - logger.debug(logMessage, logValue); - responseObserver.onNext( - newBuilder.setError(ErrorResponse.newBuilder().setCause(cause).setMessage(causeMessage)).build()); - responseObserver.onCompleted(); - } - @Override @PerformPermissionCheck @Permitted(role = RoleConstants.ACCOUNT_ADMIN) @@ -1203,35 +1194,6 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas responseObserver.onCompleted(); } - private static void logAndBuildGrpcAccountResponse(StreamObserver responseObserver, - AccountResponse.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 logAndBuildGrpcAccountsResponse(StreamObserver responseObserver, - AccountsResponse.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 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); - responseObserver.onNext(newBuilder.setError(ErrorResponse.newBuilder().setCause(cause)).build()); - responseObserver.onCompleted(); - } - @Override @PerformPermissionCheck @Permitted(role = RoleConstants.ACCOUNT_ADMIN) @@ -1278,4 +1240,41 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas 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); + responseObserver.onNext( + newBuilder.setError(ErrorResponse.newBuilder().setCause(cause).setMessage(causeMessage)).build()); + responseObserver.onCompleted(); + } + + private static void logAndBuildGrpcAccountResponse(StreamObserver responseObserver, + AccountResponse.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 logAndBuildGrpcAccountsResponse(StreamObserver responseObserver, + AccountsResponse.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 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); + responseObserver.onNext(newBuilder.setError(ErrorResponse.newBuilder().setCause(cause)).build()); + responseObserver.onCompleted(); + } + } -- GitLab From 0c00d2fcd3a6268167b0c6772bb5a79a33416752 Mon Sep 17 00:00:00 2001 From: Stoyan Tzenkov Date: Tue, 15 Jan 2019 12:26:57 +0200 Subject: [PATCH 11/13] NY-6112: Store lastupdatetimestamp in DB record. Signed-off-by: Stoyan Tzenkov --- .../nynja/account/components/StatementsPool.java | 15 +++++++++------ .../AccountRepositoryAdditionalImpl.java | 7 +++++-- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/src/main/java/biz/nynja/account/components/StatementsPool.java b/src/main/java/biz/nynja/account/components/StatementsPool.java index 28dfaca..2bbc0d4 100644 --- a/src/main/java/biz/nynja/account/components/StatementsPool.java +++ b/src/main/java/biz/nynja/account/components/StatementsPool.java @@ -3,6 +3,7 @@ */ package biz.nynja.account.components; +import java.time.Instant; import java.util.Arrays; import java.util.Collections; import java.util.HashSet; @@ -26,10 +27,11 @@ public class StatementsPool { this.preparedStatementsCache = preparedStatementsCache; } - public BoundStatement addAuthenticationProviderToProfile(UUID profileId, AuthenticationProvider authProvider) { - String cql = "UPDATE profile SET authenticationproviders = authenticationproviders + ? WHERE profileid = ? ;"; + public BoundStatement addAuthenticationProviderToProfile(UUID profileId, AuthenticationProvider authProvider, Long updatedTimestamp) { + String cql = "UPDATE profile SET authenticationproviders = authenticationproviders + ?, lastupdatetimestamp = ? WHERE profileid = ? ;"; Set toBeAdded = Collections.unmodifiableSet(new HashSet<>(Arrays.asList(authProvider))); - BoundStatement bound = preparedStatementsCache.getPreparedStatement(cql).bind(toBeAdded, profileId); + BoundStatement bound = preparedStatementsCache.getPreparedStatement(cql).bind(toBeAdded, updatedTimestamp, + profileId); return bound; } @@ -51,11 +53,12 @@ public class StatementsPool { return bound; } - public BoundStatement deleteAuthenicationProviderFromProfile(UUID profileId, AuthenticationProvider authProvider) { - String cql = "UPDATE profile SET authenticationproviders = authenticationproviders - ? WHERE profileid = ? ;"; + public BoundStatement deleteAuthenicationProviderFromProfile(UUID profileId, AuthenticationProvider authProvider, Long updatedTimestamp) { + String cql = "UPDATE profile SET authenticationproviders = authenticationproviders - ?, lastupdatetimestamp = ? WHERE profileid = ? ;"; Set toBeDeleted = Collections .unmodifiableSet(new HashSet<>(Arrays.asList(authProvider))); - BoundStatement bound = preparedStatementsCache.getPreparedStatement(cql).bind(toBeDeleted, profileId); + BoundStatement bound = preparedStatementsCache.getPreparedStatement(cql).bind(toBeDeleted, updatedTimestamp, + profileId); return bound; } diff --git a/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java b/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java index 199cf34..a30e94f 100644 --- a/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java +++ b/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java @@ -536,9 +536,11 @@ public class AccountRepositoryAdditionalImpl implements AccountRepositoryAdditio public boolean addAuthenticationProvider(UUID profileId, AuthenticationProvider authProvider) { BatchStatement batch = new BatchStatement(); ResultSet wr = null; + Long timeUpdated = Instant.now().toEpochMilli(); + try { BoundStatement updateAuthProvidersInProfile = statementsPool.addAuthenticationProviderToProfile(profileId, - authProvider); + authProvider, timeUpdated); batch.add(updateAuthProvidersInProfile); BoundStatement insertInProfileByAuthProvider = statementsPool.insertProfileByAuthenticationProvider( @@ -793,10 +795,11 @@ public class AccountRepositoryAdditionalImpl implements AccountRepositoryAdditio BatchStatement batch = new BatchStatement(); ResultSet rs = null; + Long timeUpdated = Instant.now().toEpochMilli(); // Remove authentication provider form list of authentication providers in profile BoundStatement deleteAuthProviderStatement = statementsPool - .deleteAuthenicationProviderFromProfile(profile.getProfileId(), authProvider); + .deleteAuthenicationProviderFromProfile(profile.getProfileId(), authProvider, timeUpdated); batch.add(deleteAuthProviderStatement); // Remove record for profile by this authentication provider -- GitLab From a31e63112436e33636f2e1f746dc5199ad497588 Mon Sep 17 00:00:00 2001 From: Stoyan Tzenkov Date: Thu, 17 Jan 2019 10:09:31 +0200 Subject: [PATCH 12/13] NY-6156: Response corrected. Signed-off-by: Stoyan Tzenkov --- .../java/biz/nynja/account/services/AccountServiceImpl.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/main/java/biz/nynja/account/services/AccountServiceImpl.java b/src/main/java/biz/nynja/account/services/AccountServiceImpl.java index d507130..4be32fb 100644 --- a/src/main/java/biz/nynja/account/services/AccountServiceImpl.java +++ b/src/main/java/biz/nynja/account/services/AccountServiceImpl.java @@ -307,12 +307,10 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas if ((request.getUsername() == null) || request.getUsername().isEmpty()) { validation.addError(new ValidationError("Missing username.", Cause.MISSING_USERNAME)); - } - if (!account.isValidUsername(request.getUsername())) { + } else if (!account.isValidUsername(request.getUsername())) { validation.addError( new ValidationError("Invalid username. Value: " + request.getUsername(), Cause.INVALID_USERNAME)); } - return validation; } -- GitLab From d6ab4234148dab1c00e607d23bc4d72c743bdbf5 Mon Sep 17 00:00:00 2001 From: Stoyan Tzenkov Date: Thu, 17 Jan 2019 10:35:38 +0200 Subject: [PATCH 13/13] NY-5753: Fixed according to requirements. Returns INVALID_USERNAME. Signed-off-by: Stoyan Tzenkov --- src/main/java/biz/nynja/account/validation/Validators.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/biz/nynja/account/validation/Validators.java b/src/main/java/biz/nynja/account/validation/Validators.java index 9152ac6..21dc73c 100644 --- a/src/main/java/biz/nynja/account/validation/Validators.java +++ b/src/main/java/biz/nynja/account/validation/Validators.java @@ -103,7 +103,7 @@ public class Validators { if (username == null) { return false; } - return username.matches("[a-zA-Z0-9_]{1,32}"); + return username.matches("[a-zA-Z0-9_]{2,32}"); } public boolean isAccountNameValid(String accountName) { -- GitLab