Skip to content

Commit

Permalink
[Backend] Fix password reset for users who are not logged in (#1963)
Browse files Browse the repository at this point in the history
  • Loading branch information
savacano28 authored Jan 3, 2025
1 parent f257e8e commit 3430fe2
Show file tree
Hide file tree
Showing 18 changed files with 150 additions and 29 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,8 @@ public Document updateDocumentInformation(
!exercise.isUserHasAccess(
userRepository
.findById(currentUser().getId())
.orElseThrow(ElementNotFoundException::new)))
.orElseThrow(
() -> new ElementNotFoundException("Current user not found"))))
.map(Exercise::getId);
List<String> askExerciseIds =
Stream.concat(askExerciseIdsStream, input.getExerciseIds().stream()).distinct().toList();
Expand All @@ -370,7 +371,8 @@ public Document updateDocumentInformation(
!scenario.isUserHasAccess(
userRepository
.findById(currentUser().getId())
.orElseThrow(ElementNotFoundException::new)))
.orElseThrow(
() -> new ElementNotFoundException("Current user not found"))))
.map(Scenario::getId);
List<String> askScenarioIds =
Stream.concat(askScenarioIdsStream, input.getScenarioIds().stream()).distinct().toList();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,9 @@ public Log createLog(@PathVariable String exerciseId, @Valid @RequestBody LogCre
log.setExercise(exercise);
log.setTags(iterableToSet(tagRepository.findAllById(input.getTagIds())));
log.setUser(
userRepository.findById(currentUser().getId()).orElseThrow(ElementNotFoundException::new));
userRepository
.findById(currentUser().getId())
.orElseThrow(() -> new ElementNotFoundException("Current user not found")));
return exerciseLogRepository.save(log);
}

Expand Down Expand Up @@ -172,7 +174,7 @@ public Dryrun createDryrun(
? List.of(
userRepository
.findById(currentUser().getId())
.orElseThrow(ElementNotFoundException::new))
.orElseThrow(() -> new ElementNotFoundException("Current user not found")))
: fromIterable(userRepository.findAllById(userIds));
return dryrunService.provisionDryrun(exercise, users, input.getName());
}
Expand Down
25 changes: 18 additions & 7 deletions openbas-api/src/main/java/io/openbas/rest/helper/RestBehavior.java
Original file line number Diff line number Diff line change
Expand Up @@ -163,21 +163,29 @@ public ResponseEntity<ErrorMessage> handleAlreadyExistingException(AlreadyExisti

// --- Open channel access
public User impersonateUser(UserRepository userRepository, Optional<String> userId) {
if (currentUser().getId().equals(ANONYMOUS)) {
if (userId.isPresent()) {
return userRepository.findById(userId.get()).orElseThrow();
if (ANONYMOUS.equals(currentUser().getId())) {
if (userId.isEmpty()) {
throw new UnsupportedOperationException(
"User must be logged or dynamic player is required");
}
throw new UnsupportedOperationException("User must be logged or dynamic player is required");
return userRepository
.findById(userId.get())
.orElseThrow(() -> new ElementNotFoundException("User not found"));
}
return userRepository.findById(currentUser().getId()).orElseThrow();
return userRepository
.findById(currentUser().getId())
.orElseThrow(() -> new ElementNotFoundException("Current user not found"));
}

public void checkUserAccess(UserRepository userRepository, String userId) {
User askedUser = userRepository.findById(userId).orElseThrow();
if (askedUser.getOrganization() != null) {
OpenBASPrincipal currentUser = currentUser();
if (!currentUser.isAdmin()) {
User local = userRepository.findById(currentUser.getId()).orElseThrow();
User local =
userRepository
.findById(currentUser.getId())
.orElseThrow(() -> new ElementNotFoundException("Current user not found"));
List<String> localOrganizationIds =
local.getGroups().stream()
.flatMap(group -> group.getOrganizations().stream())
Expand All @@ -194,7 +202,10 @@ public void checkOrganizationAccess(UserRepository userRepository, String organi
if (organizationId != null) {
OpenBASPrincipal currentUser = currentUser();
if (!currentUser.isAdmin()) {
User local = userRepository.findById(currentUser.getId()).orElseThrow();
User local =
userRepository
.findById(currentUser.getId())
.orElseThrow(() -> new ElementNotFoundException("Current user not found"));
List<String> localOrganizationIds =
local.getGroups().stream()
.flatMap(group -> group.getOrganizations().stream())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,9 @@ public Inject createInjectForExercise(
// Get common attributes
Inject inject = input.toInject(injectorContract);
inject.setUser(
userRepository.findById(currentUser().getId()).orElseThrow(ElementNotFoundException::new));
userRepository
.findById(currentUser().getId())
.orElseThrow(() -> new ElementNotFoundException("Current user not found")));
inject.setExercise(exercise);
// Set dependencies
if (input.getDependsOn() != null) {
Expand Down Expand Up @@ -489,7 +491,8 @@ public List<Inject> nextInjectsToExecute(@RequestParam Optional<Integer> size) {
.isUserHasAccess(
userRepository
.findById(currentUser().getId())
.orElseThrow(ElementNotFoundException::new)))
.orElseThrow(
() -> new ElementNotFoundException("Current user not found"))))
// Order by near execution
.sorted(Inject.executionComparator)
// Keep only the expected size
Expand Down Expand Up @@ -552,7 +555,7 @@ public Inject createInjectForScenario(
inject.setUser(
this.userRepository
.findById(currentUser().getId())
.orElseThrow(ElementNotFoundException::new));
.orElseThrow(() -> new ElementNotFoundException("Current user not found")));
inject.setScenario(scenario);
// Set dependencies
if (input.getDependsOn() != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,9 @@ public Evaluation createEvaluation(
Objective objective = resolveRelation(objectiveId, objectiveRepository);
evaluation.setObjective(objective);
evaluation.setUser(
userRepository.findById(currentUser().getId()).orElseThrow(ElementNotFoundException::new));
userRepository
.findById(currentUser().getId())
.orElseThrow(() -> new ElementNotFoundException("Current user not found")));
Evaluation result = evaluationRepository.save(evaluation);
objective.setUpdatedAt(now());
objectiveRepository.save(objective);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,9 @@ public Evaluation createEvaluation(
Objective objective = resolveRelation(objectiveId, objectiveRepository);
evaluation.setObjective(objective);
evaluation.setUser(
userRepository.findById(currentUser().getId()).orElseThrow(ElementNotFoundException::new));
userRepository
.findById(currentUser().getId())
.orElseThrow(() -> new ElementNotFoundException("Current user not found")));
Evaluation result = evaluationRepository.save(evaluation);
objective.setUpdatedAt(now());
objectiveRepository.save(objective);
Expand Down
4 changes: 3 additions & 1 deletion openbas-api/src/main/java/io/openbas/rest/team/TeamApi.java
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,9 @@ public Iterable<TeamSimple> getTeams() {
} else {
// We get the teams that are linked to the organizations we are part of
User local =
userRepository.findById(currentUser.getId()).orElseThrow(ElementNotFoundException::new);
userRepository
.findById(currentUser.getId())
.orElseThrow(() -> new ElementNotFoundException("Current user not found"));
List<String> organizationIds =
local.getGroups().stream()
.flatMap(group -> group.getOrganizations().stream())
Expand Down
18 changes: 13 additions & 5 deletions openbas-api/src/main/java/io/openbas/rest/user/MeApi.java
Original file line number Diff line number Diff line change
Expand Up @@ -71,14 +71,16 @@ public ResponseEntity<Object> logout() {
public User me() {
return userRepository
.findById(currentUser().getId())
.orElseThrow(ElementNotFoundException::new);
.orElseThrow(() -> new ElementNotFoundException("Current user not found"));
}

@Secured(ROLE_USER)
@PutMapping("/api/me/profile")
public User updateProfile(@Valid @RequestBody UpdateProfileInput input) {
User user =
userRepository.findById(currentUser().getId()).orElseThrow(ElementNotFoundException::new);
userRepository
.findById(currentUser().getId())
.orElseThrow(() -> new ElementNotFoundException("Current user not found"));
user.setUpdateAttributes(input);
user.setOrganization(
updateRelation(input.getOrganizationId(), user.getOrganization(), organizationRepository));
Expand All @@ -91,7 +93,9 @@ public User updateProfile(@Valid @RequestBody UpdateProfileInput input) {
@PutMapping("/api/me/information")
public User updateInformation(@Valid @RequestBody UpdateUserInfoInput input) {
User user =
userRepository.findById(currentUser().getId()).orElseThrow(ElementNotFoundException::new);
userRepository
.findById(currentUser().getId())
.orElseThrow(() -> new ElementNotFoundException("Current user not found"));
user.setUpdateAttributes(input);
User savedUser = userRepository.save(user);
sessionManager.refreshUserSessions(savedUser);
Expand All @@ -103,7 +107,9 @@ public User updateInformation(@Valid @RequestBody UpdateUserInfoInput input) {
public User updatePassword(@Valid @RequestBody UpdateMePasswordInput input)
throws InputValidationException {
User user =
userRepository.findById(currentUser().getId()).orElseThrow(ElementNotFoundException::new);
userRepository
.findById(currentUser().getId())
.orElseThrow(() -> new ElementNotFoundException("Current user not found"));
if (userService.isUserPasswordValid(user, input.getCurrentPassword())) {
user.setPassword(userService.encodeUserPassword(input.getPassword()));
return userRepository.save(user);
Expand All @@ -118,7 +124,9 @@ public User updatePassword(@Valid @RequestBody UpdateMePasswordInput input)
public Token renewToken(@Valid @RequestBody RenewTokenInput input)
throws InputValidationException {
User user =
userRepository.findById(currentUser().getId()).orElseThrow(ElementNotFoundException::new);
userRepository
.findById(currentUser().getId())
.orElseThrow(() -> new ElementNotFoundException("Current user not found"));
Token token =
tokenRepository.findById(input.getTokenId()).orElseThrow(ElementNotFoundException::new);
if (!user.equals(token.getUser())) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,9 @@ public Iterable<RawPlayer> players() {
players = fromIterable(userRepository.rawAllPlayers());
} else {
User local =
userRepository.findById(currentUser.getId()).orElseThrow(ElementNotFoundException::new);
userRepository
.findById(currentUser.getId())
.orElseThrow(() -> new ElementNotFoundException("Current user not found"));
List<String> organizationIds =
local.getGroups().stream()
.flatMap(group -> group.getOrganizations().stream())
Expand Down
5 changes: 4 additions & 1 deletion openbas-api/src/main/java/io/openbas/rest/user/UserApi.java
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import org.springframework.data.domain.Page;
import org.springframework.data.domain.Pageable;
import org.springframework.data.jpa.domain.Specification;
import org.springframework.http.ResponseEntity;
import org.springframework.security.access.AccessDeniedException;
import org.springframework.security.access.annotation.Secured;
import org.springframework.security.authentication.BadCredentialsException;
Expand Down Expand Up @@ -87,7 +88,7 @@ public User login(@Valid @RequestBody LoginUserInput input) {
}

@PostMapping("/api/reset")
public void passwordReset(@Valid @RequestBody ResetUserInput input) {
public ResponseEntity<?> passwordReset(@Valid @RequestBody ResetUserInput input) {
Optional<User> optionalUser = userRepository.findByEmailIgnoreCase(input.getLogin());
if (optionalUser.isPresent()) {
User user = optionalUser.get();
Expand Down Expand Up @@ -116,7 +117,9 @@ public void passwordReset(@Valid @RequestBody ResetUserInput input) {
}
// Store in memory reset token
resetTokenMap.put(resetToken, user.getId());
return ResponseEntity.ok().build();
}
return ResponseEntity.badRequest().build();
}

@PostMapping("/api/reset/{token}")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,10 @@ public InjectResultOverviewOutput createOrUpdate(AtomicTestingInput input, Strin
injectToSave.setAllTeams(input.isAllTeams());
injectToSave.setDescription(input.getDescription());
injectToSave.setDependsDuration(0L);
injectToSave.setUser(userRepository.findById(currentUser().getId()).orElseThrow());
injectToSave.setUser(
userRepository
.findById(currentUser().getId())
.orElseThrow(() -> new ElementNotFoundException("Current user not found")));
injectToSave.setExercise(null);

// Set dependencies
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -710,7 +710,10 @@ private ImportRow importRow(
mapPatternByAllTeams));
});
// The user is the one doing the import
inject.setUser(userRepository.findById(currentUser().getId()).orElseThrow());
inject.setUser(
userRepository
.findById(currentUser().getId())
.orElseThrow(() -> new ElementNotFoundException("Current user not found")));
// No exercise yet
inject.setExercise(null);
// No dependencies
Expand Down
11 changes: 10 additions & 1 deletion openbas-api/src/main/java/io/openbas/service/MailingService.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package io.openbas.service;

import static io.openbas.config.OpenBASAnonymous.ANONYMOUS;
import static io.openbas.config.SessionHelper.currentUser;

import com.fasterxml.jackson.databind.ObjectMapper;
Expand Down Expand Up @@ -74,7 +75,15 @@ public void sendEmail(
.ifPresent(
injectorContract -> {
inject.setContent(this.mapper.valueToTree(emailContent));
inject.setUser(this.userRepository.findById(currentUser().getId()).orElseThrow());

// When resetting the password, the user is not logged in (anonymous),
// so there's no need to add the user to the inject.
if (!ANONYMOUS.equals(currentUser().getId())) {
inject.setUser(
this.userRepository
.findById(currentUser().getId())
.orElseThrow(() -> new ElementNotFoundException("Current user not found")));
}

exercise.ifPresent(inject::setExercise);

Expand Down
53 changes: 53 additions & 0 deletions openbas-api/src/test/java/io/openbas/rest/UserApiTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,12 @@

import static io.openbas.utils.JsonUtils.asJsonString;
import static io.openbas.utils.fixtures.UserFixture.EMAIL;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.TestInstance.Lifecycle.PER_CLASS;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
Expand All @@ -11,10 +16,15 @@
import io.openbas.database.model.User;
import io.openbas.database.repository.UserRepository;
import io.openbas.rest.user.form.login.LoginUserInput;
import io.openbas.rest.user.form.login.ResetUserInput;
import io.openbas.rest.user.form.user.CreateUserInput;
import io.openbas.service.MailingService;
import io.openbas.utils.fixtures.UserFixture;
import java.util.List;
import org.junit.jupiter.api.*;
import org.mockito.ArgumentCaptor;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.mock.mockito.MockBean;
import org.springframework.http.MediaType;
import org.springframework.security.test.context.support.WithMockUser;
import org.springframework.test.web.servlet.MockMvc;
Expand All @@ -28,6 +38,8 @@ class UserApiTest extends IntegrationTest {

@Autowired private UserRepository userRepository;

@MockBean private MailingService mailingService;

@BeforeAll
public void setup() {
// Create user
Expand Down Expand Up @@ -143,4 +155,45 @@ void given_known_create_user_in_uppercase_input_should_return_conflict() throws
.andExpect(status().isConflict());
}
}

@Nested
@DisplayName("Reset Password from I forget my pwd option")
class ResetPassword {
@DisplayName("With a known email")
@Test
void resetPassword() throws Exception {
// -- PREPARE --
ResetUserInput input = UserFixture.getResetUserInput();

// -- EXECUTE --
mvc.perform(
post("/api/reset")
.contentType(MediaType.APPLICATION_JSON)
.content(asJsonString(input)))
.andExpect(status().isOk());

// -- ASSERT --
ArgumentCaptor<List<User>> userCaptor = ArgumentCaptor.forClass(List.class);
verify(mailingService).sendEmail(anyString(), anyString(), userCaptor.capture());
assertEquals(EMAIL, userCaptor.getValue().get(0).getEmail());
}

@DisplayName("With a unknown email")
@Test
void resetPasswordWithUnknownEmail() throws Exception {
// -- PREPARE --
ResetUserInput input = UserFixture.getResetUserInput();
input.setLogin("[email protected]");

// -- EXECUTE --
mvc.perform(
post("/api/reset")
.contentType(MediaType.APPLICATION_JSON)
.content(asJsonString(input)))
.andExpect(status().isBadRequest());

// -- ASSERT --
verify(mailingService, never()).sendEmail(anyString(), anyString(), any(List.class));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import io.openbas.database.model.User;
import io.openbas.rest.user.form.login.LoginUserInput;
import io.openbas.rest.user.form.login.ResetUserInput;
import org.springframework.security.crypto.argon2.Argon2PasswordEncoder;

public class UserFixture {
Expand Down Expand Up @@ -45,4 +46,11 @@ public static User getSavedUser() {
user.setId("saved-user-id");
return user;
}

public static ResetUserInput getResetUserInput() {
ResetUserInput resetUserInput = new ResetUserInput();
resetUserInput.setLogin(EMAIL);

return resetUserInput;
}
}
Loading

0 comments on commit 3430fe2

Please sign in to comment.