From de1f675fcc65624e9d7564863a0406db17f96298 Mon Sep 17 00:00:00 2001 From: Benjamin Preiser Date: Thu, 29 Feb 2024 11:59:22 -0500 Subject: [PATCH] only renew tokens when activating user --- .../mskcc/cbio/oncokb/service/UserService.java | 16 ++++++++-------- .../oncokb/service/impl/CompanyServiceImpl.java | 2 +- .../cbio/oncokb/web/rest/AccountResource.java | 2 +- .../cbio/oncokb/web/rest/CompanyResource.java | 2 +- .../cbio/oncokb/web/rest/SlackController.java | 4 ++-- .../mskcc/cbio/oncokb/web/rest/UserResource.java | 7 ++++++- .../cbio/oncokb/service/CompanyServiceIT.java | 2 +- .../mskcc/cbio/oncokb/service/UserServiceIT.java | 8 ++++---- 8 files changed, 24 insertions(+), 19 deletions(-) diff --git a/src/main/java/org/mskcc/cbio/oncokb/service/UserService.java b/src/main/java/org/mskcc/cbio/oncokb/service/UserService.java index 2969d0d8b..729a0b518 100644 --- a/src/main/java/org/mskcc/cbio/oncokb/service/UserService.java +++ b/src/main/java/org/mskcc/cbio/oncokb/service/UserService.java @@ -376,7 +376,7 @@ public User createUser(UserDTO userDTO, Optional tokenValidDays, Option return user; } - private Optional updateUserFromUserDTO(UserDTO userDTO) { + public Optional updateUserFromUserDTO(UserDTO userDTO) { Optional updatedUserDTO = Optional.of(userRepository .findById(userDTO.getId())) .filter(Optional::isPresent) @@ -423,7 +423,7 @@ private Optional updateUserFromUserDTO(UserDTO userDTO) { * @param userDTO user to update. * @return updated user. */ - public Optional updateUser(UserDTO userDTO) { + public Optional updateUserAndTokens(UserDTO userDTO) { Optional updatedUserDTO = updateUserFromUserDTO(userDTO); if (updatedUserDTO.isPresent()) { @@ -579,7 +579,7 @@ public Optional approveUser(UserDTO userDTO, Boolean isTrial) { if (!userDTO.isActivated()) { userDTO.setActivated(true); } - Optional updatedUserDTO = updateUser(userDTO); + Optional updatedUserDTO = updateUserAndTokens(userDTO); if (updatedUserDTO.isPresent()) { List tokens = generateTokenForUserIfNotExist( @@ -616,7 +616,7 @@ public void convertUserToRegular(UserDTO userDTO) { userDTO.setActivated(true); } - Optional updatedUserDTO = updateUser(userDTO); + Optional updatedUserDTO = updateUserAndTokens(userDTO); if (!updatedUserDTO.isPresent()) { return; } @@ -723,7 +723,7 @@ public Optional updateUserWithCompanyLicense(UserDTO userDTO, Company c userDTO.setCompanyName(company.getName()); userDTO.setCompany(companyMapper.toDto(company)); userDTO.setLicenseType(company.getLicenseType()); - Optional updatedUserDTO = updateUser(userDTO); + Optional updatedUserDTO = updateUserAndTokens(userDTO); if (updatedUserDTO.isPresent()) { // Update the user with the company's license @@ -747,7 +747,7 @@ public Optional updateUserWithCompanyLicense(UserDTO userDTO, Company c if (!isAccountCreation) { mailService.sendActiveTrialMail(userMapper.userToUserDTO(updatedUser.get()), false); } - updatedUserDTO = updateUser(userMapper.userToUserDTO(updatedUser.get())); + updatedUserDTO = updateUserAndTokens(userMapper.userToUserDTO(updatedUser.get())); } } }else if (companyLicenseStatus.equals(LicenseStatus.TRIAL_EXPIRED) || companyLicenseStatus.equals(LicenseStatus.EXPIRED)) { @@ -765,7 +765,7 @@ public Optional updateUserWithCompanyLicense(UserDTO userDTO, Company c private void expireUserAccount(UserDTO userDTO) { if(userDTO.isActivated()) { userDTO.setActivated(false); - updateUser(userDTO); + updateUserAndTokens(userDTO); } else { if (userHasUnactivatedTrial(userDTO)) { clearTrialAccountInformation(userDTO); @@ -897,7 +897,7 @@ public void sendUserToRocReview(UserDTO userDTO) throws MessagingException { userDTO.setAdditionalInfo(new AdditionalInfoDTO()); } userDTO.getAdditionalInfo().setSentToRocReview(true); - updateUser(userDTO); + updateUserAndTokens(userDTO); } public boolean isAdmin(String userLogin) { diff --git a/src/main/java/org/mskcc/cbio/oncokb/service/impl/CompanyServiceImpl.java b/src/main/java/org/mskcc/cbio/oncokb/service/impl/CompanyServiceImpl.java index 88d5799a7..e74afd2e2 100644 --- a/src/main/java/org/mskcc/cbio/oncokb/service/impl/CompanyServiceImpl.java +++ b/src/main/java/org/mskcc/cbio/oncokb/service/impl/CompanyServiceImpl.java @@ -95,7 +95,7 @@ public CompanyDTO updateCompany(CompanyVM companyVm) { if(isLicenseUpdateRequired){ companyUsers.forEach(userDTO -> userService.updateUserWithCompanyLicense(userDTO, company, false, false)); }else{ - companyUsers.forEach(userDTO -> userService.updateUser(userDTO)); + companyUsers.forEach(userDTO -> userService.updateUserAndTokens(userDTO)); } // Update the licenses for new users being added to this company diff --git a/src/main/java/org/mskcc/cbio/oncokb/web/rest/AccountResource.java b/src/main/java/org/mskcc/cbio/oncokb/web/rest/AccountResource.java index d2e7818c2..dd2c17aef 100644 --- a/src/main/java/org/mskcc/cbio/oncokb/web/rest/AccountResource.java +++ b/src/main/java/org/mskcc/cbio/oncokb/web/rest/AccountResource.java @@ -257,7 +257,7 @@ public void saveAccount(@Valid @RequestBody UserDTO userDTO) { if (!user.isPresent()) { throw new CustomMessageRuntimeException("User could not be found"); } - userService.updateUser(userDTO); + userService.updateUserAndTokens(userDTO); } /** diff --git a/src/main/java/org/mskcc/cbio/oncokb/web/rest/CompanyResource.java b/src/main/java/org/mskcc/cbio/oncokb/web/rest/CompanyResource.java index e12986959..9bb8d9af8 100644 --- a/src/main/java/org/mskcc/cbio/oncokb/web/rest/CompanyResource.java +++ b/src/main/java/org/mskcc/cbio/oncokb/web/rest/CompanyResource.java @@ -112,7 +112,7 @@ public ResponseEntity updateCompany(@Valid @RequestBody CompanyVM co Set userAuthorities = user.getAuthorities(); if (!userAuthorities.contains(AuthoritiesConstants.API)) { userAuthorities.add(AuthoritiesConstants.API); - userService.updateUser(user); + userService.updateUserAndTokens(user); } } } diff --git a/src/main/java/org/mskcc/cbio/oncokb/web/rest/SlackController.java b/src/main/java/org/mskcc/cbio/oncokb/web/rest/SlackController.java index be5bda2ff..549c8167a 100644 --- a/src/main/java/org/mskcc/cbio/oncokb/web/rest/SlackController.java +++ b/src/main/java/org/mskcc/cbio/oncokb/web/rest/SlackController.java @@ -120,7 +120,7 @@ public ResponseEntity approveUser(@RequestParam("payload") String action updateUserWithRoleApiIfRequested(userDTO); - Optional updatedUser = userService.updateUser(userDTO); + Optional updatedUser = userService.updateUserAndTokens(userDTO); if (updatedUser.isPresent() && updatedUser.get().isActivated() && !userAlreadyHasRoleApi) { mailService.sendApiAccessApprovalEmail(userDTO); } @@ -142,7 +142,7 @@ public ResponseEntity approveUser(@RequestParam("payload") String action String value = action.getSelectedOption().getValue(); LicenseType newLicenseType = LicenseType.valueOf(this.slackService.getOptionValueArgument(value)); userDTO.setLicenseType(newLicenseType); - this.userService.updateUser(userDTO); + this.userService.updateUserAndTokens(userDTO); break; case CONVERT_TO_REGULAR_ACCOUNT: userService.convertUserToRegular(userDTO); diff --git a/src/main/java/org/mskcc/cbio/oncokb/web/rest/UserResource.java b/src/main/java/org/mskcc/cbio/oncokb/web/rest/UserResource.java index 653da60f1..f7bd51c97 100644 --- a/src/main/java/org/mskcc/cbio/oncokb/web/rest/UserResource.java +++ b/src/main/java/org/mskcc/cbio/oncokb/web/rest/UserResource.java @@ -180,7 +180,12 @@ public ResponseEntity updateUser( userDTO.setCompany(null); } - Optional updatedUser = userService.updateUser(userDTO); + Optional updatedUser; + if (existingUser.isPresent() && !existingUser.get().getActivated() && userDTO.isActivated()) { + updatedUser = userService.updateUserAndTokens(userDTO); + } else { + updatedUser = userService.updateUserFromUserDTO(userDTO); + } if(updatedUser.isPresent() && sendEmail && updatedUser.get().isActivated()) { mailService.sendApprovalEmail(updatedUser.get()); diff --git a/src/test/java/org/mskcc/cbio/oncokb/service/CompanyServiceIT.java b/src/test/java/org/mskcc/cbio/oncokb/service/CompanyServiceIT.java index b491fd83a..29f895010 100644 --- a/src/test/java/org/mskcc/cbio/oncokb/service/CompanyServiceIT.java +++ b/src/test/java/org/mskcc/cbio/oncokb/service/CompanyServiceIT.java @@ -416,7 +416,7 @@ public void assertThatExpiredUserStatusWillChangeToTrialAfterLinkedWithTrialComp User user = userService.createUser(userDTO, Optional.of(DEFAULT_TOKEN_EXPIRATION_IN_DAYS), Optional.of(Boolean.TRUE)); userDTO = userMapper.userToUserDTO(user); userDTO.setActivated(false); - userService.updateUser(userDTO); + userService.updateUserAndTokens(userDTO); // The user expired at this moment // Create ta trial company diff --git a/src/test/java/org/mskcc/cbio/oncokb/service/UserServiceIT.java b/src/test/java/org/mskcc/cbio/oncokb/service/UserServiceIT.java index 371f9c7fe..213aa66d5 100644 --- a/src/test/java/org/mskcc/cbio/oncokb/service/UserServiceIT.java +++ b/src/test/java/org/mskcc/cbio/oncokb/service/UserServiceIT.java @@ -133,7 +133,7 @@ public void assertThatInactivateUserWillRemoveActivationKey() { UserDTO userDTO = userMapper.userToUserDTO(user); userDTO.setActivated(false); - Optional updatedUserDto = userService.updateUser(userDTO); + Optional updatedUserDto = userService.updateUserAndTokens(userDTO); assertThat(updatedUserDto).isPresent(); assertThat(updatedUserDto.get().isActivated()).isFalse(); assertThat(updatedUserDto.get().getActivationKey()).isNullOrEmpty(); @@ -149,7 +149,7 @@ public void assertThatInactivateUserWillRemoveResetKeyAndDate() { UserDTO userDTO = userMapper.userToUserDTO(user); userDTO.setActivated(false); - Optional updatedUserDto = userService.updateUser(userDTO); + Optional updatedUserDto = userService.updateUserAndTokens(userDTO); assertThat(updatedUserDto).isPresent(); assertThat(updatedUserDto.get().isActivated()).isFalse(); assertThat(updatedUserDto.get().getResetKey()).isNullOrEmpty(); @@ -365,13 +365,13 @@ public void assertThatUserTokenStatusIsExpected() { User savedUser = userService.createUser(userDTO, Optional.empty(), Optional.of(Boolean.TRUE)); userDTO = userMapper.userToUserDTO(savedUser); userDTO.setActivated(false); - userDTO = userService.updateUser(userDTO).get(); + userDTO = userService.updateUserAndTokens(userDTO).get(); assertThat(tokenService.findByUser(savedUser).get(0).getExpiration()).isBefore(Instant.now()); assertThat(tokenService.findByUser(savedUser).get(0).isRenewable()).isTrue(); userDTO.setActivated(true); - userService.updateUser(userDTO); + userService.updateUserAndTokens(userDTO); assertThat(tokenService.findByUser(savedUser).get(0).getExpiration()).isAfter(Instant.now()); assertThat(tokenService.findByUser(savedUser).get(0).isRenewable()).isTrue();