Skip to content

Commit

Permalink
Merge pull request #1090 from bprize15/fix-token-renewed-on-update-user
Browse files Browse the repository at this point in the history 
Only renew tokens when activating user for PUT endpoint
  • Loading branch information
@bprize15
bprize15 authored Feb 29, 2024
2 parents 520d4f5 + de1f675 commit 8675344
Show file tree
Hide file tree
Showing 8 changed files with 24 additions and 19 deletions.
16 changes: 8 additions & 8 deletions src/main/java/org/mskcc/cbio/oncokb/service/UserService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -376,7 +376,7 @@ public User createUser(UserDTO userDTO, Optional<Integer> tokenValidDays, Option
return user;
}

private Optional<UserDTO> updateUserFromUserDTO(UserDTO userDTO) {
public Optional<UserDTO> updateUserFromUserDTO(UserDTO userDTO) {
Optional<UserDTO> updatedUserDTO = Optional.of(userRepository
.findById(userDTO.getId()))
.filter(Optional::isPresent)
Expand Down Expand Up @@ -423,7 +423,7 @@ private Optional<UserDTO> updateUserFromUserDTO(UserDTO userDTO) {
* @param userDTO user to update.
* @return updated user.
*/
public Optional<UserDTO> updateUser(UserDTO userDTO) {
public Optional<UserDTO> updateUserAndTokens(UserDTO userDTO) {
Optional<UserDTO> updatedUserDTO = updateUserFromUserDTO(userDTO);

if (updatedUserDTO.isPresent()) {
Expand Down Expand Up @@ -579,7 +579,7 @@ public Optional<UserDTO> approveUser(UserDTO userDTO, Boolean isTrial) {
if (!userDTO.isActivated()) {
userDTO.setActivated(true);
}
Optional<UserDTO> updatedUserDTO = updateUser(userDTO);
Optional<UserDTO> updatedUserDTO = updateUserAndTokens(userDTO);
if (updatedUserDTO.isPresent()) {
List<Token> tokens =
generateTokenForUserIfNotExist(
Expand Down Expand Up @@ -616,7 +616,7 @@ public void convertUserToRegular(UserDTO userDTO) {
userDTO.setActivated(true);
}

Optional<UserDTO> updatedUserDTO = updateUser(userDTO);
Optional<UserDTO> updatedUserDTO = updateUserAndTokens(userDTO);
if (!updatedUserDTO.isPresent()) {
return;
}
Expand Down Expand Up @@ -723,7 +723,7 @@ public Optional<UserDTO> updateUserWithCompanyLicense(UserDTO userDTO, Company c
userDTO.setCompanyName(company.getName());
userDTO.setCompany(companyMapper.toDto(company));
userDTO.setLicenseType(company.getLicenseType());
Optional<UserDTO> updatedUserDTO = updateUser(userDTO);
Optional<UserDTO> updatedUserDTO = updateUserAndTokens(userDTO);

if (updatedUserDTO.isPresent()) {
// Update the user with the company's license
Expand All @@ -747,7 +747,7 @@ public Optional<UserDTO> 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)) {
Expand All @@ -765,7 +765,7 @@ public Optional<UserDTO> 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);
Expand Down Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/mskcc/cbio/oncokb/service/impl/CompanyServiceImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/mskcc/cbio/oncokb/web/rest/AccountResource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/mskcc/cbio/oncokb/web/rest/CompanyResource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ public ResponseEntity<CompanyDTO> updateCompany(@Valid @RequestBody CompanyVM co
Set<String> userAuthorities = user.getAuthorities();
if (!userAuthorities.contains(AuthoritiesConstants.API)) {
userAuthorities.add(AuthoritiesConstants.API);
userService.updateUser(user);
userService.updateUserAndTokens(user);
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/org/mskcc/cbio/oncokb/web/rest/SlackController.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ public ResponseEntity<String> approveUser(@RequestParam("payload") String action

updateUserWithRoleApiIfRequested(userDTO);

Optional<UserDTO> updatedUser = userService.updateUser(userDTO);
Optional<UserDTO> updatedUser = userService.updateUserAndTokens(userDTO);
if (updatedUser.isPresent() && updatedUser.get().isActivated() && !userAlreadyHasRoleApi) {
mailService.sendApiAccessApprovalEmail(userDTO);
}
Expand All @@ -142,7 +142,7 @@ public ResponseEntity<String> 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);
Expand Down
7 changes: 6 additions & 1 deletion src/main/java/org/mskcc/cbio/oncokb/web/rest/UserResource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,12 @@ public ResponseEntity<UserDTO> updateUser(
userDTO.setCompany(null);
}

Optional<UserDTO> updatedUser = userService.updateUser(userDTO);
Optional<UserDTO> 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());
Expand Down
2 changes: 1 addition & 1 deletion src/test/java/org/mskcc/cbio/oncokb/service/CompanyServiceIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions src/test/java/org/mskcc/cbio/oncokb/service/UserServiceIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ public void assertThatInactivateUserWillRemoveActivationKey() {

UserDTO userDTO = userMapper.userToUserDTO(user);
userDTO.setActivated(false);
Optional<UserDTO> updatedUserDto = userService.updateUser(userDTO);
Optional<UserDTO> updatedUserDto = userService.updateUserAndTokens(userDTO);
assertThat(updatedUserDto).isPresent();
assertThat(updatedUserDto.get().isActivated()).isFalse();
assertThat(updatedUserDto.get().getActivationKey()).isNullOrEmpty();
Expand All @@ -149,7 +149,7 @@ public void assertThatInactivateUserWillRemoveResetKeyAndDate() {

UserDTO userDTO = userMapper.userToUserDTO(user);
userDTO.setActivated(false);
Optional<UserDTO> updatedUserDto = userService.updateUser(userDTO);
Optional<UserDTO> updatedUserDto = userService.updateUserAndTokens(userDTO);
assertThat(updatedUserDto).isPresent();
assertThat(updatedUserDto.get().isActivated()).isFalse();
assertThat(updatedUserDto.get().getResetKey()).isNullOrEmpty();
Expand Down Expand Up @@ -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();
Expand Down

0 comments on commit 8675344

Please sign in to comment.