From 832abecbb3385558162a63f026ad1c2338b43c17 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Milo=C5=A1=20=C4=8Cuturi=C4=87?= Date: Mon, 16 Feb 2026 17:54:46 +0100 Subject: [PATCH] fix: Fix user auth to match other services --- build.gradle.kts | 5 +- .../user/config/PasswordEncoderConfig.java | 15 +++ .../com/devoops/user/config/RequireRole.java | 12 +++ .../config/RoleAuthorizationInterceptor.java | 50 ++++++++++ .../devoops/user/config/SecurityConfig.java | 94 ------------------- .../com/devoops/user/config/UserContext.java | 5 + .../user/config/UserContextResolver.java | 41 ++++++++ .../com/devoops/user/config/WebConfig.java | 27 ++++++ .../user/controller/UserController.java | 22 ++--- .../java/com/devoops/user/entity/User.java | 34 +------ .../user/exception/ForbiddenException.java | 8 ++ .../exception/GlobalExceptionHandler.java | 18 ++-- .../user/exception/UnauthorizedException.java | 8 ++ .../security/JwtAuthenticationFilter.java | 61 ------------ .../user/service/AuthenticationService.java | 10 +- .../user/service/UserDetailsServiceImpl.java | 21 ----- .../com/devoops/user/service/UserService.java | 23 +++-- .../user/controller/UserControllerTest.java | 87 +++++++++-------- .../service/AuthenticationServiceTest.java | 19 +--- .../devoops/user/service/UserServiceTest.java | 90 ++++++++++-------- 20 files changed, 306 insertions(+), 344 deletions(-) create mode 100644 src/main/java/com/devoops/user/config/PasswordEncoderConfig.java create mode 100644 src/main/java/com/devoops/user/config/RequireRole.java create mode 100644 src/main/java/com/devoops/user/config/RoleAuthorizationInterceptor.java delete mode 100644 src/main/java/com/devoops/user/config/SecurityConfig.java create mode 100644 src/main/java/com/devoops/user/config/UserContext.java create mode 100644 src/main/java/com/devoops/user/config/UserContextResolver.java create mode 100644 src/main/java/com/devoops/user/config/WebConfig.java create mode 100644 src/main/java/com/devoops/user/exception/ForbiddenException.java create mode 100644 src/main/java/com/devoops/user/exception/UnauthorizedException.java delete mode 100644 src/main/java/com/devoops/user/security/JwtAuthenticationFilter.java delete mode 100644 src/main/java/com/devoops/user/service/UserDetailsServiceImpl.java diff --git a/build.gradle.kts b/build.gradle.kts index 39bcd1a..42b7452 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -34,8 +34,8 @@ dependencies { implementation("org.flywaydb:flyway-database-postgresql") runtimeOnly("org.postgresql:postgresql") - // Security - implementation("org.springframework.boot:spring-boot-starter-security") + // Password Encoding + implementation("org.springframework.security:spring-security-crypto") // RabbitMQ implementation("org.springframework.boot:spring-boot-starter-amqp") @@ -66,7 +66,6 @@ dependencies { // Test testImplementation("org.springframework.boot:spring-boot-starter-test") - testImplementation("org.springframework.security:spring-security-test") testImplementation("org.testcontainers:junit-jupiter:1.20.4") testImplementation("org.testcontainers:postgresql:1.20.4") testImplementation("org.springframework.amqp:spring-rabbit-test") diff --git a/src/main/java/com/devoops/user/config/PasswordEncoderConfig.java b/src/main/java/com/devoops/user/config/PasswordEncoderConfig.java new file mode 100644 index 0000000..967b702 --- /dev/null +++ b/src/main/java/com/devoops/user/config/PasswordEncoderConfig.java @@ -0,0 +1,15 @@ +package com.devoops.user.config; + +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.security.crypto.bcrypt.BCryptPasswordEncoder; +import org.springframework.security.crypto.password.PasswordEncoder; + +@Configuration +public class PasswordEncoderConfig { + + @Bean + public PasswordEncoder passwordEncoder() { + return new BCryptPasswordEncoder(); + } +} diff --git a/src/main/java/com/devoops/user/config/RequireRole.java b/src/main/java/com/devoops/user/config/RequireRole.java new file mode 100644 index 0000000..fbd33c6 --- /dev/null +++ b/src/main/java/com/devoops/user/config/RequireRole.java @@ -0,0 +1,12 @@ +package com.devoops.user.config; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +@Target({ElementType.METHOD, ElementType.TYPE}) +@Retention(RetentionPolicy.RUNTIME) +public @interface RequireRole { + String[] value(); +} diff --git a/src/main/java/com/devoops/user/config/RoleAuthorizationInterceptor.java b/src/main/java/com/devoops/user/config/RoleAuthorizationInterceptor.java new file mode 100644 index 0000000..e72a31a --- /dev/null +++ b/src/main/java/com/devoops/user/config/RoleAuthorizationInterceptor.java @@ -0,0 +1,50 @@ +package com.devoops.user.config; + +import com.devoops.user.exception.ForbiddenException; +import com.devoops.user.exception.UnauthorizedException; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; +import org.jspecify.annotations.NonNull; +import org.springframework.stereotype.Component; +import org.springframework.web.method.HandlerMethod; +import org.springframework.web.servlet.HandlerInterceptor; + +import java.util.Arrays; + +@Component +public class RoleAuthorizationInterceptor implements HandlerInterceptor { + + @Override + public boolean preHandle( + @NonNull HttpServletRequest request, + @NonNull HttpServletResponse response, + @NonNull Object handler + ) + { + if (!(handler instanceof HandlerMethod handlerMethod)) { + return true; + } + + RequireRole methodAnnotation = handlerMethod.getMethodAnnotation(RequireRole.class); + RequireRole classAnnotation = handlerMethod.getBeanType().getAnnotation(RequireRole.class); + + RequireRole requireRole = methodAnnotation != null ? methodAnnotation : classAnnotation; + if (requireRole == null) { + return true; + } + + String role = request.getHeader("X-User-Role"); + if (role == null) { + throw new UnauthorizedException("Missing authentication headers"); + } + + boolean hasRole = Arrays.stream(requireRole.value()) + .anyMatch(r -> r.equalsIgnoreCase(role)); + + if (!hasRole) { + throw new ForbiddenException("Insufficient permissions"); + } + + return true; + } +} diff --git a/src/main/java/com/devoops/user/config/SecurityConfig.java b/src/main/java/com/devoops/user/config/SecurityConfig.java deleted file mode 100644 index 412127b..0000000 --- a/src/main/java/com/devoops/user/config/SecurityConfig.java +++ /dev/null @@ -1,94 +0,0 @@ -package com.devoops.user.config; - -import com.devoops.user.security.JwtAuthenticationFilter; -import lombok.RequiredArgsConstructor; -import org.springframework.beans.factory.annotation.Value; -import org.springframework.context.annotation.Bean; -import org.springframework.context.annotation.Configuration; -import org.springframework.security.authentication.AuthenticationManager; -import org.springframework.security.authentication.AuthenticationProvider; -import org.springframework.security.authentication.dao.DaoAuthenticationProvider; -import org.springframework.security.config.annotation.authentication.configuration.AuthenticationConfiguration; -import org.springframework.security.config.annotation.method.configuration.EnableMethodSecurity; -import org.springframework.security.config.annotation.web.builders.HttpSecurity; -import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity; -import org.springframework.security.config.annotation.web.configurers.AbstractHttpConfigurer; -import org.springframework.security.config.http.SessionCreationPolicy; -import org.springframework.security.core.userdetails.UserDetailsService; -import org.springframework.security.crypto.bcrypt.BCryptPasswordEncoder; -import org.springframework.security.crypto.password.PasswordEncoder; -import org.springframework.security.web.SecurityFilterChain; -import org.springframework.security.web.authentication.UsernamePasswordAuthenticationFilter; -import org.springframework.web.cors.CorsConfiguration; -import org.springframework.web.cors.CorsConfigurationSource; -import org.springframework.web.cors.UrlBasedCorsConfigurationSource; - -import java.util.List; - -@Configuration -@EnableWebSecurity -@EnableMethodSecurity -@RequiredArgsConstructor -public class SecurityConfig { - - private final JwtAuthenticationFilter jwtAuthenticationFilter; - private final UserDetailsService userDetailsService; - - @Value("${cors.allowed-origins:http://localhost:4200}") - private String allowedOrigins; - - @Bean - public SecurityFilterChain securityFilterChain(HttpSecurity http) throws Exception { - http - .cors(cors -> cors.configurationSource(corsConfigurationSource())) - .csrf(AbstractHttpConfigurer::disable) - .authorizeHttpRequests(auth -> auth - .requestMatchers( - "/api/user/auth/register", - "/api/user/auth/login", - "/api/user/test", - "/actuator/**" - ).permitAll() - .anyRequest().authenticated() - ) - .sessionManagement(session -> session - .sessionCreationPolicy(SessionCreationPolicy.STATELESS) - ) - .authenticationProvider(authenticationProvider()) - .addFilterBefore(jwtAuthenticationFilter, UsernamePasswordAuthenticationFilter.class); - - return http.build(); - } - - @Bean - public CorsConfigurationSource corsConfigurationSource() { - CorsConfiguration configuration = new CorsConfiguration(); - configuration.setAllowedOrigins(List.of(allowedOrigins.split(","))); - configuration.setAllowedMethods(List.of("GET", "POST", "PUT", "DELETE", "PATCH", "OPTIONS")); - configuration.setAllowedHeaders(List.of("Authorization", "Content-Type", "X-Requested-With")); - configuration.setExposedHeaders(List.of("Authorization")); - configuration.setAllowCredentials(true); - configuration.setMaxAge(3600L); - - UrlBasedCorsConfigurationSource source = new UrlBasedCorsConfigurationSource(); - source.registerCorsConfiguration("/**", configuration); - return source; - } - - @Bean - public AuthenticationProvider authenticationProvider() { - DaoAuthenticationProvider authProvider = new DaoAuthenticationProvider(userDetailsService); - authProvider.setPasswordEncoder(passwordEncoder()); - return authProvider; - } - - @Bean - public AuthenticationManager authenticationManager(AuthenticationConfiguration config) throws Exception { - return config.getAuthenticationManager(); - } - - @Bean - public PasswordEncoder passwordEncoder() { - return new BCryptPasswordEncoder(); - } -} diff --git a/src/main/java/com/devoops/user/config/UserContext.java b/src/main/java/com/devoops/user/config/UserContext.java new file mode 100644 index 0000000..268c5f9 --- /dev/null +++ b/src/main/java/com/devoops/user/config/UserContext.java @@ -0,0 +1,5 @@ +package com.devoops.user.config; + +import java.util.UUID; + +public record UserContext(UUID userId, String role) { } diff --git a/src/main/java/com/devoops/user/config/UserContextResolver.java b/src/main/java/com/devoops/user/config/UserContextResolver.java new file mode 100644 index 0000000..462ef51 --- /dev/null +++ b/src/main/java/com/devoops/user/config/UserContextResolver.java @@ -0,0 +1,41 @@ +package com.devoops.user.config; + +import com.devoops.user.exception.UnauthorizedException; +import org.jspecify.annotations.NonNull; +import org.springframework.core.MethodParameter; +import org.springframework.web.bind.support.WebDataBinderFactory; +import org.springframework.web.context.request.NativeWebRequest; +import org.springframework.web.method.support.HandlerMethodArgumentResolver; +import org.springframework.web.method.support.ModelAndViewContainer; + +import java.util.UUID; + +public class UserContextResolver implements HandlerMethodArgumentResolver { + + @Override + public boolean supportsParameter(MethodParameter parameter) { + return UserContext.class.isAssignableFrom(parameter.getParameterType()); + } + + @Override + public Object resolveArgument( + @NonNull MethodParameter parameter, + ModelAndViewContainer mavContainer, + NativeWebRequest webRequest, + WebDataBinderFactory binderFactory + ) + { + String userId = webRequest.getHeader("X-User-Id"); + String role = webRequest.getHeader("X-User-Role"); + + if (userId == null || role == null) { + throw new UnauthorizedException("Missing authentication headers"); + } + + try { + return new UserContext(UUID.fromString(userId), role); + } catch (IllegalArgumentException e) { + throw new UnauthorizedException("Invalid user ID format"); + } + } +} diff --git a/src/main/java/com/devoops/user/config/WebConfig.java b/src/main/java/com/devoops/user/config/WebConfig.java new file mode 100644 index 0000000..1d764b5 --- /dev/null +++ b/src/main/java/com/devoops/user/config/WebConfig.java @@ -0,0 +1,27 @@ +package com.devoops.user.config; + +import lombok.RequiredArgsConstructor; +import org.springframework.context.annotation.Configuration; +import org.springframework.web.method.support.HandlerMethodArgumentResolver; +import org.springframework.web.servlet.config.annotation.InterceptorRegistry; +import org.springframework.web.servlet.config.annotation.WebMvcConfigurer; + +import java.util.List; + +@Configuration +@RequiredArgsConstructor +public class WebConfig implements WebMvcConfigurer { + + private final RoleAuthorizationInterceptor roleAuthorizationInterceptor; + + @Override + public void addArgumentResolvers(List resolvers) { + resolvers.add(new UserContextResolver()); + } + + @Override + public void addInterceptors(InterceptorRegistry registry) { + registry.addInterceptor(roleAuthorizationInterceptor); + } + +} diff --git a/src/main/java/com/devoops/user/controller/UserController.java b/src/main/java/com/devoops/user/controller/UserController.java index f9c872a..6d355f3 100644 --- a/src/main/java/com/devoops/user/controller/UserController.java +++ b/src/main/java/com/devoops/user/controller/UserController.java @@ -1,5 +1,7 @@ package com.devoops.user.controller; +import com.devoops.user.config.RequireRole; +import com.devoops.user.config.UserContext; import com.devoops.user.dto.request.ChangePasswordRequest; import com.devoops.user.dto.request.UpdateUserRequest; import com.devoops.user.dto.response.AuthenticationResponse; @@ -8,8 +10,6 @@ import jakarta.validation.Valid; import lombok.RequiredArgsConstructor; import org.springframework.http.ResponseEntity; -import org.springframework.security.access.prepost.PreAuthorize; -import org.springframework.security.core.Authentication; import org.springframework.web.bind.annotation.*; @RestController @@ -20,25 +20,25 @@ public class UserController { private final UserService userService; @GetMapping - @PreAuthorize("hasAnyRole('HOST', 'GUEST')") - public ResponseEntity getProfile(Authentication auth) { - return ResponseEntity.ok(userService.getProfile(auth)); + @RequireRole({"HOST", "GUEST"}) + public ResponseEntity getProfile(UserContext userContext) { + return ResponseEntity.ok(userService.getProfile(userContext.userId())); } @PutMapping - @PreAuthorize("hasAnyRole('HOST', 'GUEST')") + @RequireRole({"HOST", "GUEST"}) public ResponseEntity updateProfile( - Authentication auth, + UserContext userContext, @RequestBody @Valid UpdateUserRequest request) { - return ResponseEntity.ok(userService.updateProfile(auth, request)); + return ResponseEntity.ok(userService.updateProfile(userContext.userId(), request)); } @PutMapping("/password") - @PreAuthorize("hasAnyRole('HOST', 'GUEST')") + @RequireRole({"HOST", "GUEST"}) public ResponseEntity changePassword( - Authentication auth, + UserContext userContext, @RequestBody @Valid ChangePasswordRequest request) { - userService.changePassword(auth, request); + userService.changePassword(userContext.userId(), request); return ResponseEntity.noContent().build(); } } diff --git a/src/main/java/com/devoops/user/entity/User.java b/src/main/java/com/devoops/user/entity/User.java index d9f0934..eb11852 100644 --- a/src/main/java/com/devoops/user/entity/User.java +++ b/src/main/java/com/devoops/user/entity/User.java @@ -6,12 +6,6 @@ import org.hibernate.annotations.JdbcTypeCode; import org.hibernate.annotations.SQLRestriction; import org.hibernate.type.SqlTypes; -import org.springframework.security.core.GrantedAuthority; -import org.springframework.security.core.authority.SimpleGrantedAuthority; -import org.springframework.security.core.userdetails.UserDetails; - -import java.util.Collection; -import java.util.List; @Entity @Table(name = "users") @@ -21,7 +15,7 @@ @NoArgsConstructor @AllArgsConstructor @SuperBuilder -public class User extends BaseEntity implements UserDetails { +public class User extends BaseEntity { @Column(nullable = false, unique = true, length = 50) private String username; @@ -45,30 +39,4 @@ public class User extends BaseEntity implements UserDetails { @Column(nullable = false, columnDefinition = "user_role") @JdbcTypeCode(SqlTypes.NAMED_ENUM) private Role role; - - @Override - @NonNull - public Collection getAuthorities() { - return List.of(new SimpleGrantedAuthority("ROLE_" + role.name())); - } - - @Override - public boolean isAccountNonExpired() { - return true; - } - - @Override - public boolean isAccountNonLocked() { - return true; - } - - @Override - public boolean isCredentialsNonExpired() { - return true; - } - - @Override - public boolean isEnabled() { - return true; - } } diff --git a/src/main/java/com/devoops/user/exception/ForbiddenException.java b/src/main/java/com/devoops/user/exception/ForbiddenException.java new file mode 100644 index 0000000..a17ecf5 --- /dev/null +++ b/src/main/java/com/devoops/user/exception/ForbiddenException.java @@ -0,0 +1,8 @@ +package com.devoops.user.exception; + +public class ForbiddenException extends RuntimeException { + + public ForbiddenException(String message) { + super(message); + } +} diff --git a/src/main/java/com/devoops/user/exception/GlobalExceptionHandler.java b/src/main/java/com/devoops/user/exception/GlobalExceptionHandler.java index 9e36899..96d1083 100644 --- a/src/main/java/com/devoops/user/exception/GlobalExceptionHandler.java +++ b/src/main/java/com/devoops/user/exception/GlobalExceptionHandler.java @@ -4,8 +4,6 @@ import io.jsonwebtoken.security.SignatureException; import org.springframework.http.HttpStatus; import org.springframework.http.ProblemDetail; -import org.springframework.security.authentication.BadCredentialsException; -import org.springframework.security.core.userdetails.UsernameNotFoundException; import org.springframework.validation.FieldError; import org.springframework.web.bind.MethodArgumentNotValidException; import org.springframework.web.bind.annotation.ExceptionHandler; @@ -50,10 +48,18 @@ public ProblemDetail handleInvalidPassword(InvalidPasswordException ex) { return problemDetail; } - @ExceptionHandler({BadCredentialsException.class, UsernameNotFoundException.class}) - public ProblemDetail handleAuthenticationFailure(Exception ex) { - ProblemDetail problemDetail = ProblemDetail.forStatusAndDetail(HttpStatus.UNAUTHORIZED, "Invalid credentials"); - problemDetail.setTitle("Authentication Failed"); + @ExceptionHandler(UnauthorizedException.class) + public ProblemDetail handleUnauthorized(UnauthorizedException ex) { + ProblemDetail problemDetail = ProblemDetail.forStatusAndDetail(HttpStatus.UNAUTHORIZED, ex.getMessage()); + problemDetail.setTitle("Unauthorized"); + problemDetail.setProperty("timestamp", Instant.now()); + return problemDetail; + } + + @ExceptionHandler(ForbiddenException.class) + public ProblemDetail handleForbidden(ForbiddenException ex) { + ProblemDetail problemDetail = ProblemDetail.forStatusAndDetail(HttpStatus.FORBIDDEN, ex.getMessage()); + problemDetail.setTitle("Forbidden"); problemDetail.setProperty("timestamp", Instant.now()); return problemDetail; } diff --git a/src/main/java/com/devoops/user/exception/UnauthorizedException.java b/src/main/java/com/devoops/user/exception/UnauthorizedException.java new file mode 100644 index 0000000..4c9dd85 --- /dev/null +++ b/src/main/java/com/devoops/user/exception/UnauthorizedException.java @@ -0,0 +1,8 @@ +package com.devoops.user.exception; + +public class UnauthorizedException extends RuntimeException { + + public UnauthorizedException(String message) { + super(message); + } +} diff --git a/src/main/java/com/devoops/user/security/JwtAuthenticationFilter.java b/src/main/java/com/devoops/user/security/JwtAuthenticationFilter.java deleted file mode 100644 index ff37bc0..0000000 --- a/src/main/java/com/devoops/user/security/JwtAuthenticationFilter.java +++ /dev/null @@ -1,61 +0,0 @@ -package com.devoops.user.security; - -import com.devoops.user.entity.User; -import jakarta.servlet.FilterChain; -import jakarta.servlet.ServletException; -import jakarta.servlet.http.HttpServletRequest; -import jakarta.servlet.http.HttpServletResponse; -import lombok.RequiredArgsConstructor; -import org.springframework.lang.NonNull; -import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; -import org.springframework.security.core.context.SecurityContextHolder; -import org.springframework.security.core.userdetails.UserDetails; -import org.springframework.security.core.userdetails.UserDetailsService; -import org.springframework.security.web.authentication.WebAuthenticationDetailsSource; -import org.springframework.stereotype.Component; -import org.springframework.web.filter.OncePerRequestFilter; -import org.springframework.web.servlet.HandlerExceptionResolver; - -import java.io.IOException; - -@Component -@RequiredArgsConstructor -public class JwtAuthenticationFilter extends OncePerRequestFilter { - - private final JwtService jwtService; - private final UserDetailsService userDetailsService; - private final HandlerExceptionResolver handlerExceptionResolver; - - @Override - protected void doFilterInternal(@NonNull HttpServletRequest request, - @NonNull HttpServletResponse response, - @NonNull FilterChain filterChain) throws ServletException, IOException { - - final String authHeader = request.getHeader("Authorization"); - - if (authHeader == null || !authHeader.startsWith("Bearer ")) { - filterChain.doFilter(request, response); - return; - } - - try { - final String jwt = authHeader.substring(7); - final String username = jwtService.extractUsername(jwt); - - if (username != null && SecurityContextHolder.getContext().getAuthentication() == null) { - UserDetails userDetails = this.userDetailsService.loadUserByUsername(username); - - if (jwtService.isTokenValid(jwt, (User) userDetails)) { - UsernamePasswordAuthenticationToken authToken = new UsernamePasswordAuthenticationToken( - userDetails, null, userDetails.getAuthorities()); - authToken.setDetails(new WebAuthenticationDetailsSource().buildDetails(request)); - SecurityContextHolder.getContext().setAuthentication(authToken); - } - } - - filterChain.doFilter(request, response); - } catch (Exception exception) { - handlerExceptionResolver.resolveException(request, response, null, exception); - } - } -} diff --git a/src/main/java/com/devoops/user/service/AuthenticationService.java b/src/main/java/com/devoops/user/service/AuthenticationService.java index 7d94d19..bddfdc8 100644 --- a/src/main/java/com/devoops/user/service/AuthenticationService.java +++ b/src/main/java/com/devoops/user/service/AuthenticationService.java @@ -11,9 +11,6 @@ import com.devoops.user.security.JwtService; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; -import org.springframework.security.authentication.AuthenticationManager; -import org.springframework.security.authentication.BadCredentialsException; -import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; import org.springframework.security.crypto.password.PasswordEncoder; import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.Transactional; @@ -26,7 +23,6 @@ public class AuthenticationService { private final UserRepository userRepository; private final PasswordEncoder passwordEncoder; private final JwtService jwtService; - private final AuthenticationManager authenticationManager; private final UserMapper userMapper; private final UserEventPublisherService userEventPublisherService; @@ -67,11 +63,7 @@ public AuthenticationResponse login(LoginRequest request) { return new InvalidCredentialsException("Invalid username/email or password"); }); - try { - authenticationManager.authenticate( - new UsernamePasswordAuthenticationToken(user.getUsername(), request.password()) - ); - } catch (BadCredentialsException e) { + if (!passwordEncoder.matches(request.password(), user.getPassword())) { log.warn("Login failed - invalid password for user: {}", user.getUsername()); throw new InvalidCredentialsException("Invalid username/email or password"); } diff --git a/src/main/java/com/devoops/user/service/UserDetailsServiceImpl.java b/src/main/java/com/devoops/user/service/UserDetailsServiceImpl.java deleted file mode 100644 index 859c08b..0000000 --- a/src/main/java/com/devoops/user/service/UserDetailsServiceImpl.java +++ /dev/null @@ -1,21 +0,0 @@ -package com.devoops.user.service; - -import com.devoops.user.repository.UserRepository; -import lombok.RequiredArgsConstructor; -import org.springframework.security.core.userdetails.UserDetails; -import org.springframework.security.core.userdetails.UserDetailsService; -import org.springframework.security.core.userdetails.UsernameNotFoundException; -import org.springframework.stereotype.Service; - -@Service -@RequiredArgsConstructor -public class UserDetailsServiceImpl implements UserDetailsService { - - private final UserRepository userRepository; - - @Override - public UserDetails loadUserByUsername(String username) throws UsernameNotFoundException { - return userRepository.findByUsername(username) - .orElseThrow(() -> new UsernameNotFoundException("User not found: " + username)); - } -} diff --git a/src/main/java/com/devoops/user/service/UserService.java b/src/main/java/com/devoops/user/service/UserService.java index 36c6221..5028c3d 100644 --- a/src/main/java/com/devoops/user/service/UserService.java +++ b/src/main/java/com/devoops/user/service/UserService.java @@ -5,7 +5,6 @@ import com.devoops.user.dto.response.AuthenticationResponse; import com.devoops.user.dto.response.UserResponse; import com.devoops.user.entity.User; -import com.devoops.user.exception.InvalidCredentialsException; import com.devoops.user.exception.InvalidPasswordException; import com.devoops.user.exception.UserAlreadyExistsException; import com.devoops.user.exception.UserNotFoundException; @@ -13,10 +12,11 @@ import com.devoops.user.repository.UserRepository; import com.devoops.user.security.JwtService; import lombok.RequiredArgsConstructor; -import org.springframework.security.core.Authentication; import org.springframework.security.crypto.password.PasswordEncoder; import org.springframework.stereotype.Service; +import java.util.UUID; + @Service @RequiredArgsConstructor public class UserService { @@ -26,15 +26,15 @@ public class UserService { private final PasswordEncoder passwordEncoder; private final JwtService jwtService; - public UserResponse getProfile(Authentication auth) { - User user = (User) auth.getPrincipal(); + public UserResponse getProfile(UUID userId) { + User user = userRepository.findById(userId) + .orElseThrow(() -> new UserNotFoundException("User does not exist")); return userMapper.toUserResponse(user); } - public AuthenticationResponse updateProfile(Authentication auth, UpdateUserRequest request) { - User user = (User) auth.getPrincipal(); - if (user == null) - throw new UserNotFoundException("User does not exist"); + public AuthenticationResponse updateProfile(UUID userId, UpdateUserRequest request) { + User user = userRepository.findById(userId) + .orElseThrow(() -> new UserNotFoundException("User does not exist")); if (request.username() != null && !request.username().equals(user.getUsername())) { if (userRepository.existsByUsername(request.username())) { @@ -58,10 +58,9 @@ public AuthenticationResponse updateProfile(Authentication auth, UpdateUserReque return new AuthenticationResponse(jwtService.generateToken(saved), jwtService.getExpirationTime(), userMapper.toUserResponse(saved)); } - public void changePassword(Authentication auth, ChangePasswordRequest request) { - User user = (User) auth.getPrincipal(); - if (user == null) - throw new UserNotFoundException("User does not exist"); + public void changePassword(UUID userId, ChangePasswordRequest request) { + User user = userRepository.findById(userId) + .orElseThrow(() -> new UserNotFoundException("User does not exist")); if (!passwordEncoder.matches(request.currentPassword(), user.getPassword())) { throw new InvalidPasswordException("Current password is incorrect"); diff --git a/src/test/java/com/devoops/user/controller/UserControllerTest.java b/src/test/java/com/devoops/user/controller/UserControllerTest.java index fccc6d5..90705da 100644 --- a/src/test/java/com/devoops/user/controller/UserControllerTest.java +++ b/src/test/java/com/devoops/user/controller/UserControllerTest.java @@ -1,5 +1,7 @@ package com.devoops.user.controller; +import com.devoops.user.config.RoleAuthorizationInterceptor; +import com.devoops.user.config.UserContextResolver; import com.devoops.user.dto.request.ChangePasswordRequest; import com.devoops.user.dto.request.UpdateUserRequest; import com.devoops.user.dto.response.AuthenticationResponse; @@ -19,15 +21,14 @@ import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; import org.springframework.http.MediaType; -import org.springframework.security.core.Authentication; import org.springframework.test.web.servlet.MockMvc; import org.springframework.test.web.servlet.setup.MockMvcBuilders; import java.util.UUID; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.doThrow; -import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.put; @@ -45,6 +46,7 @@ class UserControllerTest { private MockMvc mockMvc; private ObjectMapper objectMapper; + private UUID userId; private UserResponse userResponse; private AuthenticationResponse authenticationResponse; @@ -52,11 +54,15 @@ class UserControllerTest { void setUp() { mockMvc = MockMvcBuilders.standaloneSetup(userController) .setControllerAdvice(new GlobalExceptionHandler()) + .setCustomArgumentResolvers(new UserContextResolver()) + .addInterceptors(new RoleAuthorizationInterceptor()) .build(); objectMapper = new ObjectMapper(); + userId = UUID.randomUUID(); + userResponse = new UserResponse( - UUID.randomUUID(), + userId, "testuser", "test@example.com", "Test", @@ -77,15 +83,15 @@ void setUp() { class GetProfileTests { @Test - @DisplayName("Should return 200 OK with UserResponse when auth is valid") - void getProfile_WithValidAuth_ReturnsUserResponse() throws Exception { + @DisplayName("Should return 200 OK with UserResponse when auth headers are valid") + void getProfile_WithValidHeaders_ReturnsUserResponse() throws Exception { // Given - Authentication auth = mock(Authentication.class); - when(userService.getProfile(any(Authentication.class))).thenReturn(userResponse); + when(userService.getProfile(eq(userId))).thenReturn(userResponse); // When/Then mockMvc.perform(get("/api/user/me") - .principal(auth)) + .header("X-User-Id", userId.toString()) + .header("X-User-Role", "GUEST")) .andExpect(status().isOk()) .andExpect(content().contentType(MediaType.APPLICATION_JSON)) .andExpect(jsonPath("$.username").value("testuser")) @@ -94,18 +100,21 @@ void getProfile_WithValidAuth_ReturnsUserResponse() throws Exception { } @Test - @DisplayName("Should return 401 when service throws InvalidCredentialsException") - void getProfile_WithoutAuth_Returns401() throws Exception { - // Given - Authentication auth = mock(Authentication.class); - when(userService.getProfile(any(Authentication.class))) - .thenThrow(new InvalidCredentialsException("Unauthorized")); + @DisplayName("Should return 401 when auth headers are missing") + void getProfile_WithoutHeaders_Returns401() throws Exception { + // When/Then + mockMvc.perform(get("/api/user/me")) + .andExpect(status().isUnauthorized()); + } + @Test + @DisplayName("Should return 403 when role is not allowed") + void getProfile_WithWrongRole_Returns403() throws Exception { // When/Then mockMvc.perform(get("/api/user/me") - .principal(auth)) - .andExpect(status().isUnauthorized()) - .andExpect(jsonPath("$.title").value("Invalid Credentials")); + .header("X-User-Id", userId.toString()) + .header("X-User-Role", "ADMIN")) + .andExpect(status().isForbidden()); } } @@ -120,13 +129,13 @@ void updateProfile_WithValidRequest_ReturnsAuthenticationResponse() throws Excep UpdateUserRequest request = new UpdateUserRequest( "newusername", "new@example.com", "New", "Name", "New City" ); - Authentication auth = mock(Authentication.class); - when(userService.updateProfile(any(Authentication.class), any(UpdateUserRequest.class))) + when(userService.updateProfile(eq(userId), any(UpdateUserRequest.class))) .thenReturn(authenticationResponse); // When/Then mockMvc.perform(put("/api/user/me") - .principal(auth) + .header("X-User-Id", userId.toString()) + .header("X-User-Role", "GUEST") .contentType(MediaType.APPLICATION_JSON) .content(objectMapper.writeValueAsString(request))) .andExpect(status().isOk()) @@ -143,13 +152,13 @@ void updateProfile_WithUsernameTaken_Returns409() throws Exception { UpdateUserRequest request = new UpdateUserRequest( "takenuser", null, null, null, null ); - Authentication auth = mock(Authentication.class); - when(userService.updateProfile(any(Authentication.class), any(UpdateUserRequest.class))) + when(userService.updateProfile(eq(userId), any(UpdateUserRequest.class))) .thenThrow(new UserAlreadyExistsException("Username already taken")); // When/Then mockMvc.perform(put("/api/user/me") - .principal(auth) + .header("X-User-Id", userId.toString()) + .header("X-User-Role", "GUEST") .contentType(MediaType.APPLICATION_JSON) .content(objectMapper.writeValueAsString(request))) .andExpect(status().isConflict()) @@ -164,13 +173,13 @@ void updateProfile_WithEmailTaken_Returns409() throws Exception { UpdateUserRequest request = new UpdateUserRequest( null, "taken@example.com", null, null, null ); - Authentication auth = mock(Authentication.class); - when(userService.updateProfile(any(Authentication.class), any(UpdateUserRequest.class))) + when(userService.updateProfile(eq(userId), any(UpdateUserRequest.class))) .thenThrow(new UserAlreadyExistsException("Email already taken")); // When/Then mockMvc.perform(put("/api/user/me") - .principal(auth) + .header("X-User-Id", userId.toString()) + .header("X-User-Role", "GUEST") .contentType(MediaType.APPLICATION_JSON) .content(objectMapper.writeValueAsString(request))) .andExpect(status().isConflict()) @@ -186,11 +195,11 @@ void updateProfile_WithInvalidEmail_Returns400() throws Exception { "email": "not-an-email" } """; - Authentication auth = mock(Authentication.class); // When/Then mockMvc.perform(put("/api/user/me") - .principal(auth) + .header("X-User-Id", userId.toString()) + .header("X-User-Role", "GUEST") .contentType(MediaType.APPLICATION_JSON) .content(invalidRequest)) .andExpect(status().isBadRequest()); @@ -205,11 +214,11 @@ void updateProfile_WithUsernameTooShort_Returns400() throws Exception { "username": "ab" } """; - Authentication auth = mock(Authentication.class); // When/Then mockMvc.perform(put("/api/user/me") - .principal(auth) + .header("X-User-Id", userId.toString()) + .header("X-User-Role", "GUEST") .contentType(MediaType.APPLICATION_JSON) .content(invalidRequest)) .andExpect(status().isBadRequest()); @@ -225,11 +234,11 @@ class ChangePasswordTests { void changePassword_WithValidRequest_Returns204() throws Exception { // Given ChangePasswordRequest request = new ChangePasswordRequest("currentPass1", "newPassword123"); - Authentication auth = mock(Authentication.class); // When/Then mockMvc.perform(put("/api/user/me/password") - .principal(auth) + .header("X-User-Id", userId.toString()) + .header("X-User-Role", "GUEST") .contentType(MediaType.APPLICATION_JSON) .content(objectMapper.writeValueAsString(request))) .andExpect(status().isNoContent()); @@ -240,13 +249,13 @@ void changePassword_WithValidRequest_Returns204() throws Exception { void changePassword_WithInvalidCurrentPassword_Returns401() throws Exception { // Given ChangePasswordRequest request = new ChangePasswordRequest("wrongPass", "newPassword123"); - Authentication auth = mock(Authentication.class); doThrow(new InvalidCredentialsException("Current password is incorrect")) - .when(userService).changePassword(any(Authentication.class), any(ChangePasswordRequest.class)); + .when(userService).changePassword(eq(userId), any(ChangePasswordRequest.class)); // When/Then mockMvc.perform(put("/api/user/me/password") - .principal(auth) + .header("X-User-Id", userId.toString()) + .header("X-User-Role", "GUEST") .contentType(MediaType.APPLICATION_JSON) .content(objectMapper.writeValueAsString(request))) .andExpect(status().isUnauthorized()) @@ -263,11 +272,11 @@ void changePassword_WithBlankCurrentPassword_Returns400() throws Exception { "newPassword": "newPassword123" } """; - Authentication auth = mock(Authentication.class); // When/Then mockMvc.perform(put("/api/user/me/password") - .principal(auth) + .header("X-User-Id", userId.toString()) + .header("X-User-Role", "GUEST") .contentType(MediaType.APPLICATION_JSON) .content(invalidRequest)) .andExpect(status().isBadRequest()); @@ -283,11 +292,11 @@ void changePassword_WithShortNewPassword_Returns400() throws Exception { "newPassword": "short" } """; - Authentication auth = mock(Authentication.class); // When/Then mockMvc.perform(put("/api/user/me/password") - .principal(auth) + .header("X-User-Id", userId.toString()) + .header("X-User-Role", "GUEST") .contentType(MediaType.APPLICATION_JSON) .content(invalidRequest)) .andExpect(status().isBadRequest()); diff --git a/src/test/java/com/devoops/user/service/AuthenticationServiceTest.java b/src/test/java/com/devoops/user/service/AuthenticationServiceTest.java index 15d1970..404b2b5 100644 --- a/src/test/java/com/devoops/user/service/AuthenticationServiceTest.java +++ b/src/test/java/com/devoops/user/service/AuthenticationServiceTest.java @@ -19,9 +19,6 @@ import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; -import org.springframework.security.authentication.AuthenticationManager; -import org.springframework.security.authentication.BadCredentialsException; -import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; import org.springframework.security.crypto.password.PasswordEncoder; import java.util.Optional; @@ -45,9 +42,6 @@ class AuthenticationServiceTest { @Mock private JwtService jwtService; - @Mock - private AuthenticationManager authenticationManager; - @Mock private UserMapper userMapper; @@ -172,8 +166,7 @@ void login_WithValidCredentials_ReturnsAuthenticationResponse() { // Given when(userRepository.findByUsernameOrEmail("testuser", "testuser")) .thenReturn(Optional.of(user)); - when(authenticationManager.authenticate(any(UsernamePasswordAuthenticationToken.class))) - .thenReturn(new UsernamePasswordAuthenticationToken(user, null)); + when(passwordEncoder.matches("password123", "encodedPassword")).thenReturn(true); when(jwtService.generateToken(any(User.class))).thenReturn("jwt-token"); when(jwtService.getExpirationTime()).thenReturn(86400000L); when(userMapper.toUserResponse(any(User.class))).thenReturn(userResponse); @@ -186,7 +179,7 @@ void login_WithValidCredentials_ReturnsAuthenticationResponse() { assertThat(response.accessToken()).isEqualTo("jwt-token"); assertThat(response.user().username()).isEqualTo("testuser"); - verify(authenticationManager).authenticate(any(UsernamePasswordAuthenticationToken.class)); + verify(passwordEncoder).matches("password123", "encodedPassword"); } @Test @@ -196,8 +189,7 @@ void login_WithEmail_ReturnsAuthenticationResponse() { LoginRequest emailLoginRequest = new LoginRequest("test@example.com", "password123"); when(userRepository.findByUsernameOrEmail("test@example.com", "test@example.com")) .thenReturn(Optional.of(user)); - when(authenticationManager.authenticate(any(UsernamePasswordAuthenticationToken.class))) - .thenReturn(new UsernamePasswordAuthenticationToken(user, null)); + when(passwordEncoder.matches("password123", "encodedPassword")).thenReturn(true); when(jwtService.generateToken(any(User.class))).thenReturn("jwt-token"); when(jwtService.getExpirationTime()).thenReturn(86400000L); when(userMapper.toUserResponse(any(User.class))).thenReturn(userResponse); @@ -222,7 +214,7 @@ void login_WithNonExistentUser_ThrowsInvalidCredentialsException() { .isInstanceOf(InvalidCredentialsException.class) .hasMessageContaining("Invalid username/email or password"); - verify(authenticationManager, never()).authenticate(any()); + verify(passwordEncoder, never()).matches(anyString(), anyString()); } @Test @@ -231,8 +223,7 @@ void login_WithWrongPassword_ThrowsInvalidCredentialsException() { // Given when(userRepository.findByUsernameOrEmail("testuser", "testuser")) .thenReturn(Optional.of(user)); - when(authenticationManager.authenticate(any(UsernamePasswordAuthenticationToken.class))) - .thenThrow(new BadCredentialsException("Bad credentials")); + when(passwordEncoder.matches("password123", "encodedPassword")).thenReturn(false); // When/Then assertThatThrownBy(() -> authenticationService.login(loginRequest)) diff --git a/src/test/java/com/devoops/user/service/UserServiceTest.java b/src/test/java/com/devoops/user/service/UserServiceTest.java index ea901a2..f621d4b 100644 --- a/src/test/java/com/devoops/user/service/UserServiceTest.java +++ b/src/test/java/com/devoops/user/service/UserServiceTest.java @@ -20,9 +20,9 @@ import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; -import org.springframework.security.core.Authentication; import org.springframework.security.crypto.password.PasswordEncoder; +import java.util.Optional; import java.util.UUID; import static org.assertj.core.api.Assertions.assertThat; @@ -50,14 +50,16 @@ class UserServiceTest { private UserService userService; private User testUser; + private UUID testUserId; private UserResponse userResponse; @BeforeEach void setUp() { + testUserId = UUID.randomUUID(); testUser = buildTestUser(); userResponse = new UserResponse( - UUID.randomUUID(), + testUserId, "testuser", "test@example.com", "Test", @@ -69,6 +71,7 @@ void setUp() { private User buildTestUser() { return User.builder() + .id(testUserId) .username("testuser") .password("encoded_password") .email("test@example.com") @@ -84,22 +87,35 @@ private User buildTestUser() { class GetProfileTests { @Test - @DisplayName("Should return UserResponse when auth is valid") - void getProfile_WithValidAuth_ReturnsUserResponse() { + @DisplayName("Should return UserResponse when user exists") + void getProfile_WithValidUserId_ReturnsUserResponse() { // Given - Authentication auth = mock(Authentication.class); - when(auth.getPrincipal()).thenReturn(testUser); + when(userRepository.findById(testUserId)).thenReturn(Optional.of(testUser)); when(userMapper.toUserResponse(testUser)).thenReturn(userResponse); // When - UserResponse result = userService.getProfile(auth); + UserResponse result = userService.getProfile(testUserId); // Then assertThat(result).isNotNull(); assertThat(result.username()).isEqualTo("testuser"); assertThat(result.email()).isEqualTo("test@example.com"); + verify(userRepository).findById(testUserId); verify(userMapper).toUserResponse(testUser); } + + @Test + @DisplayName("Should throw UserNotFoundException when user does not exist") + void getProfile_WithInvalidUserId_ThrowsUserNotFoundException() { + // Given + UUID unknownId = UUID.randomUUID(); + when(userRepository.findById(unknownId)).thenReturn(Optional.empty()); + + // When/Then + assertThatThrownBy(() -> userService.getProfile(unknownId)) + .isInstanceOf(UserNotFoundException.class) + .hasMessageContaining("User does not exist"); + } } @Nested @@ -110,8 +126,7 @@ class UpdateProfileTests { @DisplayName("Should update username and return new token when username is new") void updateProfile_WithNewUsername_UpdatesAndReturnsToken() { // Given - Authentication auth = mock(Authentication.class); - when(auth.getPrincipal()).thenReturn(testUser); + when(userRepository.findById(testUserId)).thenReturn(Optional.of(testUser)); UpdateUserRequest request = new UpdateUserRequest("newuser", null, null, null, null); when(userRepository.existsByUsername("newuser")).thenReturn(false); when(userRepository.save(any(User.class))).thenReturn(testUser); @@ -120,7 +135,7 @@ void updateProfile_WithNewUsername_UpdatesAndReturnsToken() { when(userMapper.toUserResponse(any(User.class))).thenReturn(userResponse); // When - AuthenticationResponse result = userService.updateProfile(auth, request); + AuthenticationResponse result = userService.updateProfile(testUserId, request); // Then assertThat(result).isNotNull(); @@ -133,13 +148,12 @@ void updateProfile_WithNewUsername_UpdatesAndReturnsToken() { @DisplayName("Should throw UserAlreadyExistsException when username is taken") void updateProfile_WithExistingUsername_ThrowsUserAlreadyExistsException() { // Given - Authentication auth = mock(Authentication.class); - when(auth.getPrincipal()).thenReturn(testUser); + when(userRepository.findById(testUserId)).thenReturn(Optional.of(testUser)); UpdateUserRequest request = new UpdateUserRequest("takenuser", null, null, null, null); when(userRepository.existsByUsername("takenuser")).thenReturn(true); // When/Then - assertThatThrownBy(() -> userService.updateProfile(auth, request)) + assertThatThrownBy(() -> userService.updateProfile(testUserId, request)) .isInstanceOf(UserAlreadyExistsException.class) .hasMessageContaining("Username already taken"); @@ -150,8 +164,7 @@ void updateProfile_WithExistingUsername_ThrowsUserAlreadyExistsException() { @DisplayName("Should update email and return new token when email is new") void updateProfile_WithNewEmail_UpdatesAndReturnsToken() { // Given - Authentication auth = mock(Authentication.class); - when(auth.getPrincipal()).thenReturn(testUser); + when(userRepository.findById(testUserId)).thenReturn(Optional.of(testUser)); UpdateUserRequest request = new UpdateUserRequest(null, "new@example.com", null, null, null); when(userRepository.existsByEmail("new@example.com")).thenReturn(false); when(userRepository.save(any(User.class))).thenReturn(testUser); @@ -160,7 +173,7 @@ void updateProfile_WithNewEmail_UpdatesAndReturnsToken() { when(userMapper.toUserResponse(any(User.class))).thenReturn(userResponse); // When - AuthenticationResponse result = userService.updateProfile(auth, request); + AuthenticationResponse result = userService.updateProfile(testUserId, request); // Then assertThat(result).isNotNull(); @@ -172,13 +185,12 @@ void updateProfile_WithNewEmail_UpdatesAndReturnsToken() { @DisplayName("Should throw UserAlreadyExistsException when email is taken") void updateProfile_WithExistingEmail_ThrowsUserAlreadyExistsException() { // Given - Authentication auth = mock(Authentication.class); - when(auth.getPrincipal()).thenReturn(testUser); + when(userRepository.findById(testUserId)).thenReturn(Optional.of(testUser)); UpdateUserRequest request = new UpdateUserRequest(null, "taken@example.com", null, null, null); when(userRepository.existsByEmail("taken@example.com")).thenReturn(true); // When/Then - assertThatThrownBy(() -> userService.updateProfile(auth, request)) + assertThatThrownBy(() -> userService.updateProfile(testUserId, request)) .isInstanceOf(UserAlreadyExistsException.class) .hasMessageContaining("Email already taken"); @@ -189,8 +201,7 @@ void updateProfile_WithExistingEmail_ThrowsUserAlreadyExistsException() { @DisplayName("Should skip username check when username is the same as current") void updateProfile_WithSameUsername_SkipsUsernameCheck() { // Given - Authentication auth = mock(Authentication.class); - when(auth.getPrincipal()).thenReturn(testUser); + when(userRepository.findById(testUserId)).thenReturn(Optional.of(testUser)); UpdateUserRequest request = new UpdateUserRequest("testuser", null, null, null, null); when(userRepository.save(any(User.class))).thenReturn(testUser); when(jwtService.generateToken(any(User.class))).thenReturn("token"); @@ -198,7 +209,7 @@ void updateProfile_WithSameUsername_SkipsUsernameCheck() { when(userMapper.toUserResponse(any(User.class))).thenReturn(userResponse); // When - userService.updateProfile(auth, request); + userService.updateProfile(testUserId, request); // Then verify(userRepository, never()).existsByUsername(anyString()); @@ -208,8 +219,7 @@ void updateProfile_WithSameUsername_SkipsUsernameCheck() { @DisplayName("Should only update non-null fields") void updateProfile_WithNullFields_OnlyUpdatesNonNullFields() { // Given - Authentication auth = mock(Authentication.class); - when(auth.getPrincipal()).thenReturn(testUser); + when(userRepository.findById(testUserId)).thenReturn(Optional.of(testUser)); UpdateUserRequest request = new UpdateUserRequest(null, null, "UpdatedFirst", null, null); when(userRepository.save(any(User.class))).thenReturn(testUser); when(jwtService.generateToken(any(User.class))).thenReturn("token"); @@ -217,7 +227,7 @@ void updateProfile_WithNullFields_OnlyUpdatesNonNullFields() { when(userMapper.toUserResponse(any(User.class))).thenReturn(userResponse); // When - userService.updateProfile(auth, request); + userService.updateProfile(testUserId, request); // Then assertThat(testUser.getUsername()).isEqualTo("testuser"); @@ -227,15 +237,15 @@ void updateProfile_WithNullFields_OnlyUpdatesNonNullFields() { } @Test - @DisplayName("Should throw UserNotFoundException when principal is null") - void updateProfile_WithNullPrincipal_ThrowsUserNotFoundException() { + @DisplayName("Should throw UserNotFoundException when user does not exist") + void updateProfile_WithNonExistentUser_ThrowsUserNotFoundException() { // Given - Authentication auth = mock(Authentication.class); - when(auth.getPrincipal()).thenReturn(null); + UUID unknownId = UUID.randomUUID(); + when(userRepository.findById(unknownId)).thenReturn(Optional.empty()); UpdateUserRequest request = new UpdateUserRequest("newuser", null, null, null, null); // When/Then - assertThatThrownBy(() -> userService.updateProfile(auth, request)) + assertThatThrownBy(() -> userService.updateProfile(unknownId, request)) .isInstanceOf(UserNotFoundException.class) .hasMessageContaining("User does not exist"); } @@ -249,14 +259,13 @@ class ChangePasswordTests { @DisplayName("Should encode and save new password when current password matches") void changePassword_WithValidPassword_EncodesAndSaves() { // Given - Authentication auth = mock(Authentication.class); - when(auth.getPrincipal()).thenReturn(testUser); + when(userRepository.findById(testUserId)).thenReturn(Optional.of(testUser)); ChangePasswordRequest request = new ChangePasswordRequest("current", "newpassword123"); when(passwordEncoder.matches("current", "encoded_password")).thenReturn(true); when(passwordEncoder.encode("newpassword123")).thenReturn("new_encoded"); // When - userService.changePassword(auth, request); + userService.changePassword(testUserId, request); // Then assertThat(testUser.getPassword()).isEqualTo("new_encoded"); @@ -267,13 +276,12 @@ void changePassword_WithValidPassword_EncodesAndSaves() { @DisplayName("Should throw InvalidPasswordException when current password is wrong") void changePassword_WithIncorrectCurrentPassword_ThrowsInvalidPasswordException() { // Given - Authentication auth = mock(Authentication.class); - when(auth.getPrincipal()).thenReturn(testUser); + when(userRepository.findById(testUserId)).thenReturn(Optional.of(testUser)); ChangePasswordRequest request = new ChangePasswordRequest("wrong", "newpassword123"); when(passwordEncoder.matches("wrong", "encoded_password")).thenReturn(false); // When/Then - assertThatThrownBy(() -> userService.changePassword(auth, request)) + assertThatThrownBy(() -> userService.changePassword(testUserId, request)) .isInstanceOf(InvalidPasswordException.class) .hasMessageContaining("Current password is incorrect"); @@ -281,15 +289,15 @@ void changePassword_WithIncorrectCurrentPassword_ThrowsInvalidPasswordException( } @Test - @DisplayName("Should throw UserNotFoundException when principal is null") - void changePassword_WithNullPrincipal_ThrowsUserNotFoundException() { + @DisplayName("Should throw UserNotFoundException when user does not exist") + void changePassword_WithNonExistentUser_ThrowsUserNotFoundException() { // Given - Authentication auth = mock(Authentication.class); - when(auth.getPrincipal()).thenReturn(null); + UUID unknownId = UUID.randomUUID(); + when(userRepository.findById(unknownId)).thenReturn(Optional.empty()); ChangePasswordRequest request = new ChangePasswordRequest("current", "newpassword123"); // When/Then - assertThatThrownBy(() -> userService.changePassword(auth, request)) + assertThatThrownBy(() -> userService.changePassword(unknownId, request)) .isInstanceOf(UserNotFoundException.class) .hasMessageContaining("User does not exist"); }