Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Backend] Fix password reset for users who are not logged in #2141

Merged
merged 5 commits into from
Jan 3, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,8 @@
!exercise.isUserHasAccess(
userRepository
.findById(currentUser().getId())
.orElseThrow(ElementNotFoundException::new)))
.orElseThrow(
() -> new ElementNotFoundException("Current user not found"))))

Check warning on line 353 in openbas-api/src/main/java/io/openbas/rest/document/DocumentApi.java

View check run for this annotation

Codecov / codecov/patch

openbas-api/src/main/java/io/openbas/rest/document/DocumentApi.java#L352-L353

Added lines #L352 - L353 were not covered by tests
.map(Exercise::getId);
List<String> askExerciseIds =
Stream.concat(askExerciseIdsStream, input.getExerciseIds().stream()).distinct().toList();
Expand All @@ -370,7 +371,8 @@
!scenario.isUserHasAccess(
userRepository
.findById(currentUser().getId())
.orElseThrow(ElementNotFoundException::new)))
.orElseThrow(
() -> new ElementNotFoundException("Current user not found"))))

Check warning on line 375 in openbas-api/src/main/java/io/openbas/rest/document/DocumentApi.java

View check run for this annotation

Codecov / codecov/patch

openbas-api/src/main/java/io/openbas/rest/document/DocumentApi.java#L374-L375

Added lines #L374 - L375 were not covered by tests
.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 @@
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")));

Check warning on line 132 in openbas-api/src/main/java/io/openbas/rest/exercise/ExerciseApi.java

View check run for this annotation

Codecov / codecov/patch

openbas-api/src/main/java/io/openbas/rest/exercise/ExerciseApi.java#L131-L132

Added lines #L131 - L132 were not covered by tests
return exerciseLogRepository.save(log);
}

Expand Down Expand Up @@ -172,7 +174,7 @@
? List.of(
userRepository
.findById(currentUser().getId())
.orElseThrow(ElementNotFoundException::new))
.orElseThrow(() -> new ElementNotFoundException("Current user not found")))

Check warning on line 177 in openbas-api/src/main/java/io/openbas/rest/exercise/ExerciseApi.java

View check run for this annotation

Codecov / codecov/patch

openbas-api/src/main/java/io/openbas/rest/exercise/ExerciseApi.java#L177

Added line #L177 was not covered by tests
: 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 @@

// --- 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(

Check warning on line 168 in openbas-api/src/main/java/io/openbas/rest/helper/RestBehavior.java

View check run for this annotation

Codecov / codecov/patch

openbas-api/src/main/java/io/openbas/rest/helper/RestBehavior.java#L168

Added line #L168 was not covered by tests
"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"));

Check warning on line 173 in openbas-api/src/main/java/io/openbas/rest/helper/RestBehavior.java

View check run for this annotation

Codecov / codecov/patch

openbas-api/src/main/java/io/openbas/rest/helper/RestBehavior.java#L171-L173

Added lines #L171 - L173 were not covered by tests
}
return userRepository.findById(currentUser().getId()).orElseThrow();
return userRepository
.findById(currentUser().getId())
.orElseThrow(() -> new ElementNotFoundException("Current user not found"));

Check warning on line 177 in openbas-api/src/main/java/io/openbas/rest/helper/RestBehavior.java

View check run for this annotation

Codecov / codecov/patch

openbas-api/src/main/java/io/openbas/rest/helper/RestBehavior.java#L175-L177

Added lines #L175 - L177 were not covered by tests
}

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 =

Check warning on line 185 in openbas-api/src/main/java/io/openbas/rest/helper/RestBehavior.java

View check run for this annotation

Codecov / codecov/patch

openbas-api/src/main/java/io/openbas/rest/helper/RestBehavior.java#L185

Added line #L185 was not covered by tests
userRepository
.findById(currentUser.getId())
.orElseThrow(() -> new ElementNotFoundException("Current user not found"));

Check warning on line 188 in openbas-api/src/main/java/io/openbas/rest/helper/RestBehavior.java

View check run for this annotation

Codecov / codecov/patch

openbas-api/src/main/java/io/openbas/rest/helper/RestBehavior.java#L187-L188

Added lines #L187 - L188 were not covered by tests
List<String> localOrganizationIds =
local.getGroups().stream()
.flatMap(group -> group.getOrganizations().stream())
Expand All @@ -194,7 +202,10 @@
if (organizationId != null) {
OpenBASPrincipal currentUser = currentUser();
if (!currentUser.isAdmin()) {
User local = userRepository.findById(currentUser.getId()).orElseThrow();
User local =

Check warning on line 205 in openbas-api/src/main/java/io/openbas/rest/helper/RestBehavior.java

View check run for this annotation

Codecov / codecov/patch

openbas-api/src/main/java/io/openbas/rest/helper/RestBehavior.java#L205

Added line #L205 was not covered by tests
userRepository
.findById(currentUser.getId())
.orElseThrow(() -> new ElementNotFoundException("Current user not found"));

Check warning on line 208 in openbas-api/src/main/java/io/openbas/rest/helper/RestBehavior.java

View check run for this annotation

Codecov / codecov/patch

openbas-api/src/main/java/io/openbas/rest/helper/RestBehavior.java#L207-L208

Added lines #L207 - L208 were not covered by tests
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 @@
// 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 @@
.isUserHasAccess(
userRepository
.findById(currentUser().getId())
.orElseThrow(ElementNotFoundException::new)))
.orElseThrow(
() -> new ElementNotFoundException("Current user not found"))))

Check warning on line 495 in openbas-api/src/main/java/io/openbas/rest/inject/InjectApi.java

View check run for this annotation

Codecov / codecov/patch

openbas-api/src/main/java/io/openbas/rest/inject/InjectApi.java#L494-L495

Added lines #L494 - L495 were not covered by tests
// Order by near execution
.sorted(Inject.executionComparator)
// Keep only the expected size
Expand Down Expand Up @@ -552,7 +555,7 @@
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 @@
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")));

Check warning on line 106 in openbas-api/src/main/java/io/openbas/rest/objective/ExerciseObjectiveApi.java

View check run for this annotation

Codecov / codecov/patch

openbas-api/src/main/java/io/openbas/rest/objective/ExerciseObjectiveApi.java#L105-L106

Added lines #L105 - L106 were not covered by tests
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 @@
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")));

Check warning on line 106 in openbas-api/src/main/java/io/openbas/rest/objective/ScenarioObjectiveApi.java

View check run for this annotation

Codecov / codecov/patch

openbas-api/src/main/java/io/openbas/rest/objective/ScenarioObjectiveApi.java#L105-L106

Added lines #L105 - L106 were not covered by tests
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 @@
} 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"));

Check warning on line 74 in openbas-api/src/main/java/io/openbas/rest/team/TeamApi.java

View check run for this annotation

Codecov / codecov/patch

openbas-api/src/main/java/io/openbas/rest/team/TeamApi.java#L73-L74

Added lines #L73 - L74 were not covered by tests
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 User me() {
return userRepository
.findById(currentUser().getId())
.orElseThrow(ElementNotFoundException::new);
.orElseThrow(() -> new ElementNotFoundException("Current user not found"));

Check warning on line 74 in openbas-api/src/main/java/io/openbas/rest/user/MeApi.java

View check run for this annotation

Codecov / codecov/patch

openbas-api/src/main/java/io/openbas/rest/user/MeApi.java#L74

Added line #L74 was not covered by tests
}

@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"));

Check warning on line 83 in openbas-api/src/main/java/io/openbas/rest/user/MeApi.java

View check run for this annotation

Codecov / codecov/patch

openbas-api/src/main/java/io/openbas/rest/user/MeApi.java#L82-L83

Added lines #L82 - L83 were not covered by tests
user.setUpdateAttributes(input);
user.setOrganization(
updateRelation(input.getOrganizationId(), user.getOrganization(), organizationRepository));
Expand All @@ -91,7 +93,9 @@
@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"));

Check warning on line 98 in openbas-api/src/main/java/io/openbas/rest/user/MeApi.java

View check run for this annotation

Codecov / codecov/patch

openbas-api/src/main/java/io/openbas/rest/user/MeApi.java#L97-L98

Added lines #L97 - L98 were not covered by tests
user.setUpdateAttributes(input);
User savedUser = userRepository.save(user);
sessionManager.refreshUserSessions(savedUser);
Expand All @@ -103,7 +107,9 @@
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"));

Check warning on line 112 in openbas-api/src/main/java/io/openbas/rest/user/MeApi.java

View check run for this annotation

Codecov / codecov/patch

openbas-api/src/main/java/io/openbas/rest/user/MeApi.java#L111-L112

Added lines #L111 - L112 were not covered by tests
if (userService.isUserPasswordValid(user, input.getCurrentPassword())) {
user.setPassword(userService.encodeUserPassword(input.getPassword()));
return userRepository.save(user);
Expand All @@ -118,7 +124,9 @@
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"));

Check warning on line 129 in openbas-api/src/main/java/io/openbas/rest/user/MeApi.java

View check run for this annotation

Codecov / codecov/patch

openbas-api/src/main/java/io/openbas/rest/user/MeApi.java#L128-L129

Added lines #L128 - L129 were not covered by tests
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 @@
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"));

Check warning on line 62 in openbas-api/src/main/java/io/openbas/rest/user/PlayerApi.java

View check run for this annotation

Codecov / codecov/patch

openbas-api/src/main/java/io/openbas/rest/user/PlayerApi.java#L61-L62

Added lines #L61 - L62 were not covered by tests
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
Loading