Skip to content
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 @@ -3,35 +3,31 @@
import jakarta.servlet.ServletException;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
import org.example.visacasemanagementsystem.user.UserAuthorization;
import org.example.visacasemanagementsystem.user.entity.User;
import org.example.visacasemanagementsystem.user.repository.UserRepository;
import org.example.visacasemanagementsystem.user.security.UserPrincipal;
import org.example.visacasemanagementsystem.user.service.UserService;
import org.springframework.context.annotation.Configuration;
import org.springframework.dao.DataIntegrityViolationException;
import org.springframework.security.authentication.BadCredentialsException;
import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
import org.springframework.security.core.Authentication;
import org.springframework.security.core.context.SecurityContext;
import org.springframework.security.core.context.SecurityContextHolder;
import org.springframework.security.crypto.password.PasswordEncoder;
import org.springframework.security.oauth2.core.oidc.user.DefaultOidcUser;
import org.springframework.security.web.authentication.SimpleUrlAuthenticationSuccessHandler;
import org.springframework.security.web.context.SecurityContextRepository;

import java.io.IOException;
import java.util.Objects;
import java.util.UUID;

@Configuration
public class OauthSuccessHandler extends SimpleUrlAuthenticationSuccessHandler {

private final UserRepository userRepository;
private final PasswordEncoder passwordEncoder;
private final UserService userService;
private final SecurityContextRepository securityContextRepository;

public OauthSuccessHandler(UserRepository userRepository, PasswordEncoder passwordEncoder, SecurityContextRepository securityContextRepository) {
this.userRepository = userRepository;
this.passwordEncoder = passwordEncoder;
public OauthSuccessHandler(UserService userService,
SecurityContextRepository securityContextRepository) {
this.userService = userService;
this.securityContextRepository = securityContextRepository;
}

Expand All @@ -49,19 +45,9 @@ public void onAuthenticationSuccess(HttpServletRequest request,
throw new BadCredentialsException("OAuth account missing verified email");
}

User user = userRepository.findByEmail(email).orElseGet(() -> {
User newUser = new User();
newUser.setFullName(name);
newUser.setUsername(email);
newUser.setEmail(email);
newUser.setPassword(passwordEncoder.encode(UUID.randomUUID().toString()));
newUser.setUserAuthorization(UserAuthorization.USER);
try {
return userRepository.saveAndFlush(newUser);
} catch (DataIntegrityViolationException e) {
return userRepository.findByEmail(email).orElseThrow();
}
});
// Persistence + audit run inside a @Transactional service method so the
// transaction commits before we touch the SecurityContext or issue the redirect.
User user = userService.findOrCreateOauthUser(email, name);

UserPrincipal principal = new UserPrincipal(user);
Authentication auth = new UsernamePasswordAuthenticationToken(principal, null, principal.getAuthorities());
Expand All @@ -72,7 +58,7 @@ public void onAuthenticationSuccess(HttpServletRequest request,

clearAuthenticationAttributes(request);

String targetUrl = "/dashboard";
String targetUrl = "/home";
getRedirectStrategy().sendRedirect(request, response, targetUrl);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,12 @@ public String userSignupForm(@AuthenticationPrincipal UserPrincipal principal, M
public String createUser(@RequestParam String fullName,
@RequestParam String email,
@RequestParam String password,
@RequestParam String confirmPassword,
Model model) {
try {
if (!password.equals(confirmPassword)) {
throw new IllegalArgumentException("Passwords do not match");
}
CreateUserDTO dto = new CreateUserDTO(fullName, email, password, UserAuthorization.USER);
userService.createUser(dto);
return "redirect:/user/login";
Expand Down Expand Up @@ -144,12 +148,19 @@ public String showProfileEditForm(@AuthenticationPrincipal UserPrincipal princip
public String updateProfile(@AuthenticationPrincipal UserPrincipal principal,
@PathVariable Long userId,
@RequestParam String fullName,
@RequestParam String email,
@RequestParam String password,
@RequestParam String confirmPassword,
Model model) {
userService.validateProfileAccess(principal, userId);

try {
UpdateUserDTO dto = new UpdateUserDTO(userId, fullName, email);
if (!password.equals(confirmPassword)) {
throw new IllegalArgumentException("Passwords do not match");
}
if (!password.isBlank() && password.length() < 8) {
throw new IllegalArgumentException("Password must be at least 8 characters");
}
UpdateUserDTO dto = new UpdateUserDTO(userId, fullName, password);
userService.updateUser(dto, principal.getUserId());
Comment thread
coderabbitai[bot] marked this conversation as resolved.
return "redirect:/profile/view/" + userId;
} catch (IllegalArgumentException e) {
Expand All @@ -158,7 +169,7 @@ public String updateProfile(@AuthenticationPrincipal UserPrincipal principal,
UserDTO existing = userService.findById(userId)
.orElseThrow(() -> new EntityNotFoundException("User not found"));
model.addAttribute("error", e.getMessage());
model.addAttribute("user", new UserDTO(userId, fullName, email, existing.userAuthorization()));
model.addAttribute("user", new UserDTO(userId, fullName, existing.email(), existing.userAuthorization()));
addAuthorizationFormAttributes(model, principal, userId);
return "profile/edit";
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@

import jakarta.validation.constraints.NotBlank;
import jakarta.validation.constraints.NotNull;
import jakarta.validation.constraints.Pattern;

public record UpdateUserDTO(
@NotNull Long id,
@NotBlank(message = "Full name must be specified") String fullName,
@NotBlank(message = "Email must be specified") String email) {
@NotNull(message = "Password must be provided (use empty string to skip change)")
@Pattern(regexp = "^$|.{8,}", message = "Password must be at least 8 characters") String password) {
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,5 @@ public void updateEntityFromDTO(UpdateUserDTO dto, User user) {
if (dto == null || user == null) return;

user.setFullName(dto.fullName());
user.setEmail(dto.email());
user.setUsername(dto.email());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,14 @@
import org.example.visacasemanagementsystem.user.security.UserPrincipal;
import org.springframework.dao.DataIntegrityViolationException;
import org.springframework.security.access.prepost.PreAuthorize;
import org.springframework.security.core.context.SecurityContextHolder;
import org.springframework.security.crypto.password.PasswordEncoder;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;
import java.util.List;
import java.util.Objects;
import java.util.Optional;
import java.util.UUID;

@PreAuthorize("isAuthenticated()")
@Service
Expand Down Expand Up @@ -89,26 +91,64 @@ public UserDTO createUser(@Valid CreateUserDTO dto) {

@Transactional
public UserDTO updateUser(UpdateUserDTO dto, Long actorUserId) {
validateProfileAccess(getUserPrincipal(), dto.id());
// Check if user and email exists
User user = userRepository.findById(dto.id())
.orElseThrow(() -> new EntityNotFoundException(USER_NOT_FOUND));

if (!dto.password().isBlank()) {
user.setPassword(passwordEncoder.encode(dto.password()));
}

Comment thread
coderabbitai[bot] marked this conversation as resolved.
User savedUser;
try {
userMapper.updateEntityFromDTO(dto, user);
savedUser = userRepository.saveAndFlush(user);
} catch (DataIntegrityViolationException e) {
throw new IllegalArgumentException("A user with this email already exists", e);
// Email is no longer mutable on this path, but other unique/integrity constraints
// (now or in the future) could still trip here, so the message stays generic.
throw new IllegalArgumentException("Data integrity violation while updating user", e);
}
userLogService.createUserLog(
actorUserId,
savedUser.getId(),
UserEventType.UPDATED,
"User profile updated (fullName/email)."
"User profile updated (fullName/password)."
);
return userMapper.toDTO(savedUser);
}

/**
* OAuth login lookup-or-create. Used by OauthSuccessHandler to keep the persistence work
* inside a single short-lived transaction that commits before the success handler builds
* the SecurityContext and issues the redirect.
*/
@PreAuthorize("permitAll()")
@Transactional
public User findOrCreateOauthUser(String email, String fullName) {
return userRepository.findByEmail(email).orElseGet(() -> {
User newUser = new User();
newUser.setFullName(fullName);
newUser.setUsername(email);
newUser.setEmail(email);
newUser.setPassword(passwordEncoder.encode(UUID.randomUUID().toString()));
newUser.setUserAuthorization(UserAuthorization.USER);
try {
User savedUser = userRepository.saveAndFlush(newUser);
userLogService.createUserLog(
savedUser.getId(),
savedUser.getId(),
UserEventType.CREATED,
"User account created via OAUTH2."
);
return savedUser;
} catch (DataIntegrityViolationException e) {
// Race: another concurrent OAuth login created the row first. Re-read.
return userRepository.findByEmail(email).orElseThrow();
}
});
}

@PreAuthorize("hasRole('SYSADMIN')")
@Transactional
public UserDTO updateUserAuthorization(Long actorUserId, Long targetUserId, UserAuthorization newAuth) {
Expand Down Expand Up @@ -154,4 +194,15 @@ public void validateProfileAccess(UserPrincipal principal, Long userId) {
throw new UnauthorizedException("You do not have permission to edit this profile.");
}
}

private static UserPrincipal getUserPrincipal() {
// Class-level @PreAuthorize("isAuthenticated()") guarantees an authentication is present.
Object principal = SecurityContextHolder.getContext().getAuthentication().getPrincipal();
if (!(principal instanceof UserPrincipal userPrincipal)) {
throw new IllegalStateException(
"Expected UserPrincipal in SecurityContext but got: "
+ (principal == null ? "null" : principal.getClass().getName()));
}
return userPrincipal;
}
}
36 changes: 31 additions & 5 deletions src/main/resources/templates/profile/edit.html
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,16 @@ <h1>Edit Profile</h1>
required>
</div>
<div class="form-group">
<label>Email</label>
<input type="email" name="email"
th:value="${user.email()}"
placeholder="you@example.com"
required>
<label>New Password <span style="color: var(--color-text-dim); font-weight: normal;">(leave blank to keep current)</span></label>
<input type="password" name="password" id="password"
placeholder="Min. 8 characters"
minlength="8">
</div>
<div class="form-group">
<label>Confirm New Password</label>
<input type="password" name="confirmPassword" id="confirmPassword"
placeholder="Repeat new password"
minlength="8">
</div>
<!-- Read-only role display for any viewer who can't change it
(the user themselves, or any non-sysadmin). Sysadmins viewing
Expand Down Expand Up @@ -94,5 +99,26 @@ <h1>Authorization</h1>
</div>
</div>

<script>
// Attach the mismatch-check to the form that actually owns the password fields,
// not to whatever 'form' querySelector picks first — the sysadmin authorization
// form on the same page would otherwise trigger this handler too.
(function () {
const pw = document.getElementById('password');
const cpw = document.getElementById('confirmPassword');
if (!pw || !cpw) return;
const form = pw.form || cpw.form;
if (!form) return;
form.addEventListener('submit', function (e) {
if (pw.value !== cpw.value) {
cpw.setCustomValidity('Passwords do not match');
cpw.reportValidity();
e.preventDefault();
} else {
cpw.setCustomValidity('');
}
});
})();
</script>
</body>
</html>
32 changes: 30 additions & 2 deletions src/main/resources/templates/user/signup.html
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,17 @@ <h1>Create Account</h1>

<div class="form-group">
<label>Password</label>
<input type="password" name="password"
placeholder="Choose a password"
<input type="password" name="password" id="password"
placeholder="Min. 8 characters"
minlength="8"
required>
</div>

<div class="form-group">
<label>Confirm Password</label>
<input type="password" name="confirmPassword" id="confirmPassword"
placeholder="Repeat your password"
minlength="8"
required>
</div>

Expand All @@ -65,5 +74,24 @@ <h1>Create Account</h1>
</form>
</div>

<script>
(function () {
const pw = document.getElementById('password');
const cpw = document.getElementById('confirmPassword');
if (!pw || !cpw) return;
const form = pw.form || cpw.form;
if (!form) return;
form.addEventListener('submit', function (e) {
if (pw.value !== cpw.value) {
cpw.setCustomValidity('Passwords do not match');
cpw.reportValidity();
e.preventDefault();
} else {
cpw.setCustomValidity('');
}
});
})();
</script>

</body>
</html>
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ void createUser_WithValidData_ShouldRedirectToLogin() throws Exception {
.param("fullName", "New Applicant")
.param("email", "new@test.com")
.param("password", "securePass1")
.param("confirmPassword", "securePass1")
.with(csrf()))
.andExpect(status().is3xxRedirection())
.andExpect(redirectedUrl("/user/login"));
Expand All @@ -113,6 +114,7 @@ void createUser_WithDuplicateEmail_ShouldReturnSignupViewWithError() throws Exce
.param("fullName", "Duplicate User")
.param("email", "existing@test.com")
.param("password", "password123")
.param("confirmPassword", "password123")
.with(csrf()))
.andExpect(status().isOk())
.andExpect(view().name("user/signup"))
Expand All @@ -131,6 +133,7 @@ void createUser_WithShortPassword_ShouldReturnSignupViewWithError() throws Excep
.param("fullName", "Short Pass")
.param("email", "short@test.com")
.param("password", "abc")
.param("confirmPassword", "abc")
.with(csrf()))
.andExpect(status().isOk())
.andExpect(view().name("user/signup"))
Expand Down Expand Up @@ -259,7 +262,8 @@ void updateProfile_WithValidData_ShouldRedirectToProfileView() throws Exception
// Act & Assert
mockMvc.perform(post("/profile/edit/" + userId)
.param("fullName", "Updated Name")
.param("email", "updated@test.com")
.param("password", "newPassword")
.param("confirmPassword", "newPassword")
.with(authentication(authFor(userId, "Test User", "user@test.com", UserAuthorization.USER)))
.with(csrf()))
.andExpect(status().is3xxRedirection())
Expand All @@ -281,7 +285,8 @@ void updateProfile_WithDuplicateEmail_ShouldReturnEditViewWithError() throws Exc
// Act & Assert
mockMvc.perform(post("/profile/edit/" + userId)
.param("fullName", "Test User")
.param("email", "taken@test.com")
.param("password", "newPassword")
.param("confirmPassword", "newPassword")
.with(authentication(authFor(userId, "Test User", "user@test.com", UserAuthorization.USER)))
.with(csrf()))
.andExpect(status().isOk())
Expand All @@ -302,7 +307,8 @@ void updateProfile_WhenUserConcurrentlyDeleted_ShouldReturnNotFound() throws Exc
// Act & Assert — handled by GlobalExceptionHandler → 404
mockMvc.perform(post("/profile/edit/" + userId)
.param("fullName", "Test User")
.param("email", "taken@test.com")
.param("password", "newPassword")
.param("confirmPassword", "newPassword")
.with(authentication(authFor(userId, "Test User", "user@test.com", UserAuthorization.USER)))
.with(csrf()))
.andExpect(status().isNotFound());
Expand All @@ -320,7 +326,8 @@ void updateProfile_AsUnauthorizedUser_ShouldReturnForbidden() throws Exception {
// Act & Assert
mockMvc.perform(post("/profile/edit/" + targetUserId)
.param("fullName", "Hacked")
.param("email", "hacked@test.com")
.param("password", "newPassword")
.param("confirmPassword", "newPassword")
.with(authentication(authFor(1L, "Test User", "user@test.com", UserAuthorization.USER)))
.with(csrf()))
.andExpect(status().isForbidden());
Expand Down
Loading