Skip to content

MOSU-88 feat: OAuth 처리 절차 수정 #92

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.

Sign up for GitHub

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

Merged
wlgns12370 merged 5 commits into from
Jul 14, 2025
Merged

MOSU-88 feat: OAuth 처리 절차 수정 #92

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6797f67
MOSU-88 chore: AccessTokenFilter 비활성화
wlgns12370 Jul 14, 2025
da199f3
MOSU-88 feat: OAuth, Auth 로그인 반환 토큰 쿠키로 반환
wlgns12370 Jul 14, 2025
91387c1
MOSU-88 feat: Util class 생성자 파괴
wlgns12370 Jul 14, 2025
0a8882f
Merge remote-tracking branch 'origin/develop' into feature/mosu-88
wlgns12370 Jul 14, 2025
d21d60c
MOSU-88 fix: code 리뷰 반영
wlgns12370 Jul 14, 2025
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
16 changes: 11 additions & 5 deletions src/main/java/life/mosu/mosuserver/application/auth/AuthService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
package life.mosu.mosuserver.application.auth;

import jakarta.servlet.http.HttpServletRequest;
import life.mosu.mosuserver.domain.profile.ProfileJpaRepository;
import life.mosu.mosuserver.domain.user.UserJpaEntity;
import life.mosu.mosuserver.global.exception.CustomRuntimeException;
import life.mosu.mosuserver.global.exception.ErrorCode;
import life.mosu.mosuserver.presentation.auth.dto.LoginRequest;
import life.mosu.mosuserver.presentation.auth.dto.LoginResponse;
import life.mosu.mosuserver.presentation.auth.dto.Token;
import lombok.RequiredArgsConstructor;
import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
Expand All @@ -16,21 +18,25 @@
@Service
@RequiredArgsConstructor
public class AuthService {

private final AuthenticationManagerBuilder authenticationManagerBuilder;
private final AuthTokenManager authTokenManager;
private final ProfileJpaRepository profileJpaRepository;

@Transactional
public Token login(final LoginRequest request) {
public LoginResponse login(final LoginRequest request) {
try {
final UsernamePasswordAuthenticationToken authenticationToken =
new UsernamePasswordAuthenticationToken(request.id(), request.password());
new UsernamePasswordAuthenticationToken(request.id(), request.password());
final Authentication authentication = authenticationManagerBuilder
.getObject()
.authenticate(authenticationToken);
.getObject()
.authenticate(authenticationToken);
final PrincipalDetails principalDetails = (PrincipalDetails) authentication.getPrincipal();
final UserJpaEntity user = principalDetails.user();

return authTokenManager.generateAuthToken(user);
Boolean isProfileRegistered = profileJpaRepository.existsByUserId(user.getId());

return LoginResponse.of(authTokenManager.generateAuthToken(user), isProfileRegistered);
} catch (final Exception e) {
throw new CustomRuntimeException(ErrorCode.INCORRECT_ID_OR_PASSWORD);
}
Comment on lines 40 to 42
Copy link

@gemini-code-assist gemini-code-assist bot Jul 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The catch (final Exception e) block is too broad. It catches all exceptions, including potential RuntimeExceptions, and masks them as an INCORRECT_ID_OR_PASSWORD error. This can make debugging difficult as it hides the true cause of an error.

It's better to catch the more specific AuthenticationException that is thrown by authenticate() on login failure. Other unexpected exceptions should be allowed to propagate to be handled by a global exception handler.

Suggested change
} catch (final Exception e) {
throw new CustomRuntimeException(ErrorCode.INCORRECT_ID_OR_PASSWORD);
}
} catch (final org.springframework.security.core.AuthenticationException e) {
throw new CustomRuntimeException(ErrorCode.INCORRECT_ID_OR_PASSWORD);
}

Expand Down
41 changes: 8 additions & 33 deletions src/main/java/life/mosu/mosuserver/application/oauth/OAuthUser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,26 +5,31 @@
import java.util.Map;
import life.mosu.mosuserver.domain.user.UserJpaEntity;
import lombok.Getter;
import lombok.extern.slf4j.Slf4j;
import org.springframework.security.core.GrantedAuthority;
import org.springframework.security.core.authority.SimpleGrantedAuthority;
import org.springframework.security.core.userdetails.UserDetails;
import org.springframework.security.oauth2.core.user.OAuth2User;

public class OAuthUser implements OAuth2User, UserDetails {
@Slf4j
public class OAuthUser implements OAuth2User {

@Getter
private final UserJpaEntity user;
private final Map<String, Object> attributes;
private final String attributeKey;
@Getter
Boolean isProfileRegistered;
Comment on lines +20 to +21
Copy link

@coderabbitai coderabbitai bot Jul 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Fix field naming convention and consider type choice.

The field has several issues:

  1. Naming: isProfileRegistered doesn't follow Java conventions. Boolean fields should not use "is" prefix as it creates awkward getter names (getIsProfileRegistered()). Consider profileRegistered instead.
  2. Type: Using Boolean instead of boolean introduces potential null values. If nullability is intentional, document it; otherwise, use primitive boolean.
  3. Modifiers: The field should be private and final for better encapsulation and immutability.
-    @Getter
-    Boolean isProfileRegistered;
+    @Getter
+    private final boolean profileRegistered;

Then update the constructor parameter accordingly.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
@Getter
Boolean isProfileRegistered;
@Getter
private final boolean profileRegistered;
🤖 Prompt for AI Agents 
In src/main/java/life/mosu/mosuserver/application/oauth/OAuthUser.java around
lines 20 to 21, rename the field from isProfileRegistered to profileRegistered
to follow Java boolean naming conventions and avoid awkward getter names. Change
its type from Boolean to primitive boolean unless null values are required, in
which case document the nullability. Also, make the field private and final to
improve encapsulation and immutability. Finally, update the constructor
parameter to match the new field name and type.

Comment on lines +20 to +21
Copy link

@coderabbitai coderabbitai bot Jul 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider using primitive boolean type for better null safety.

Using Boolean wrapper type allows null values which could lead to null pointer exceptions. If the profile registration status should always have a definite true/false value, consider using boolean primitive type instead.

 @Getter
-Boolean isProfileRegistered;
+boolean isProfileRegistered;

If null values are intentionally allowed for this field, please add documentation explaining when and why this field would be null.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
@Getter
Boolean isProfileRegistered;
@Getter
boolean isProfileRegistered;
🤖 Prompt for AI Agents 
In src/main/java/life/mosu/mosuserver/application/oauth/OAuthUser.java at lines
20-21, the field isProfileRegistered is declared as a Boolean wrapper type which
allows null values and may cause null pointer exceptions. Change the type from
Boolean to the primitive boolean to ensure it always holds a definite true or
false value. If null is intentionally allowed, add clear documentation
explaining the circumstances under which this field can be null.


public OAuthUser(
final UserJpaEntity user,
final Map<String, Object> attributes,
final String attributeKey
final String attributeKey,
final Boolean isProfileRegistered
) {
Comment on lines +26 to 28
Copy link

@coderabbitai coderabbitai bot Jul 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add parameter validation and maintain consistency with field naming.

If the field naming is updated as suggested, ensure the constructor parameter name is also updated for consistency. Consider adding validation for the new parameter.

     public OAuthUser(
             final UserJpaEntity user,
             final Map<String, Object> attributes,
-            final String attributeKey,
-            final Boolean isProfileRegistered
+            final String attributeKey,
+            final boolean profileRegistered
     ) {
         this.user = user;
         this.attributes = attributes;
         this.attributeKey = attributeKey;
-        this.isProfileRegistered = isProfileRegistered;
+        this.profileRegistered = profileRegistered;
     }

Also applies to: 32-32

🤖 Prompt for AI Agents 
In src/main/java/life/mosu/mosuserver/application/oauth/OAuthUser.java around
lines 26 to 28 and line 32, update the constructor parameter name to match the
new field naming for consistency. Additionally, add validation logic for the new
parameter to ensure it meets expected criteria before assignment, such as
checking for null or invalid values, to maintain robustness.

this.user = user;
this.attributes = attributes;
this.attributeKey = attributeKey;
this.isProfileRegistered = isProfileRegistered;
}

@Override
Expand All @@ -44,34 +49,4 @@ public Collection<? extends GrantedAuthority> getAuthorities() {
new SimpleGrantedAuthority(role)
);
}

@Override
public String getPassword() {
return null;
}

@Override
public String getUsername() {
return user.getName();
}

@Override
public boolean isAccountNonExpired() {
return UserDetails.super.isAccountNonExpired();
}

@Override
public boolean isAccountNonLocked() {
return UserDetails.super.isAccountNonLocked();
}

@Override
public boolean isCredentialsNonExpired() {
return UserDetails.super.isCredentialsNonExpired();
}

@Override
public boolean isEnabled() {
return UserDetails.super.isEnabled();
}
}
9 changes: 6 additions & 3 deletions src/main/java/life/mosu/mosuserver/application/oauth/OAuthUserService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import java.time.LocalDate;
import java.util.Map;
import life.mosu.mosuserver.domain.profile.Gender;
import life.mosu.mosuserver.domain.profile.ProfileJpaRepository;
import life.mosu.mosuserver.domain.user.UserJpaEntity;
import life.mosu.mosuserver.domain.user.UserJpaRepository;
import life.mosu.mosuserver.domain.user.UserRole;
Expand All @@ -19,14 +20,14 @@
public class OAuthUserService extends DefaultOAuth2UserService {

private final UserJpaRepository userRepository;
private final ProfileJpaRepository profileRepository;

@Override
public OAuth2User loadUser(final OAuth2UserRequest userRequest)
throws OAuth2AuthenticationException {
final OAuth2User user = super.loadUser(userRequest);

final Map<String, Object> oAuth2UserAttributes = user.getAttributes();
System.out.println("OAuth2User Attributes: " + oAuth2UserAttributes.toString());
final String registrationId = userRequest.getClientRegistration().getRegistrationId();
final String userNameAttributeName = userRequest.getClientRegistration()
.getProviderDetails()
Expand All @@ -38,11 +39,13 @@ public OAuth2User loadUser(final OAuth2UserRequest userRequest)

final UserJpaEntity oAuthUser = updateOrWrite(userInfo);

return new OAuthUser(oAuthUser, oAuth2UserAttributes, userNameAttributeName);
Boolean isProfileRegistered = profileRepository.existsByUserId(oAuthUser.getId());

return new OAuthUser(oAuthUser, oAuth2UserAttributes, userNameAttributeName,
isProfileRegistered);
}

private UserJpaEntity updateOrWrite(final OAuthUserInfo info) {

return userRepository.findByLoginId(info.email())
.map(existingUser -> {
existingUser.updateOAuthUser(info.gender(), info.name(),
Expand Down
1 change: 0 additions & 1 deletion src/main/java/life/mosu/mosuserver/domain/profile/ProfileJpaRepository.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,4 @@ public interface ProfileJpaRepository extends JpaRepository<ProfileJpaEntity, Lo
boolean existsByUserId(Long userId);

Optional<ProfileJpaEntity> findByUserId(Long userId);

}
29 changes: 15 additions & 14 deletions src/main/java/life/mosu/mosuserver/global/handler/OAuth2LoginSuccessHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,26 +1,23 @@
package life.mosu.mosuserver.global.handler;

import jakarta.servlet.http.Cookie;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
import java.io.IOException;
import life.mosu.mosuserver.application.auth.AccessTokenService;
import life.mosu.mosuserver.application.oauth.OAuthUser;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.security.core.Authentication;
import org.springframework.security.web.authentication.AuthenticationSuccessHandler;
import org.springframework.stereotype.Component;
import org.springframework.web.util.UriComponentsBuilder;

@Slf4j
@Component
@RequiredArgsConstructor
public class OAuth2LoginSuccessHandler implements AuthenticationSuccessHandler {

private final AccessTokenService accessTokenService;
@Value("${target.url}")
private String targetUrl;

@Override
public void onAuthenticationSuccess(
Expand All @@ -29,16 +26,20 @@ public void onAuthenticationSuccess(
final Authentication authentication
) throws IOException {
final OAuthUser oAuthUser = (OAuthUser) authentication.getPrincipal();

final String accessToken = accessTokenService.generateAccessToken(oAuthUser.getUser());

final String redirectUrlWithToken = UriComponentsBuilder.fromUriString(targetUrl)
.queryParam("token", accessToken)
.build()
.toUriString();

log.info("로그인 성공. 리다이렉트 URL: {}", redirectUrlWithToken);

response.sendRedirect(redirectUrlWithToken);
String jsonResponse =
"{\"isProfileRegistered\": \"" + oAuthUser.getIsProfileRegistered() + "\"}";
Comment on lines +30 to +31
Copy link

@gemini-code-assist gemini-code-assist bot Jul 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Manually constructing JSON strings is error-prone. The boolean value isProfileRegistered is being serialized as a string (e.g., "true") instead of a boolean literal (true) because it's wrapped in quotes. This will likely cause parsing issues on the client-side.

While the best long-term solution is to use a JSON serialization library like Jackson (e.g., via ObjectMapper) to serialize a DTO or a Map, a quick fix is to remove the quotes around the boolean value.

Suggested change
String jsonResponse =
"{\"isProfileRegistered\": \"" + oAuthUser.getIsProfileRegistered() + "\"}";
String jsonResponse =
"{\"isProfileRegistered\": " + oAuthUser.getIsProfileRegistered() + "}";


Cookie cookie = new Cookie("accessToken", accessToken);
cookie.setHttpOnly(true);
cookie.setSecure(true);
cookie.setPath("/");
cookie.setMaxAge(3600);
response.addCookie(cookie);

response.setStatus(HttpServletResponse.SC_OK);
response.setContentType("application/json");
response.setCharacterEncoding("UTF-8");
response.getWriter().write(jsonResponse);
}
}
5 changes: 5 additions & 0 deletions src/main/java/life/mosu/mosuserver/global/util/EncodeUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,11 @@
import org.springframework.security.crypto.password.PasswordEncoder;

public class EncodeUtil {

private EncodeUtil() {
throw new UnsupportedOperationException();
}

public static String passwordEncode(final PasswordEncoder encoder, final String password) {
return encoder.encode(password);
}
Expand Down
10 changes: 6 additions & 4 deletions src/main/java/life/mosu/mosuserver/presentation/auth/AuthController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import life.mosu.mosuserver.application.auth.AuthTokenManager;
import life.mosu.mosuserver.global.util.ApiResponseWrapper;
import life.mosu.mosuserver.presentation.auth.dto.LoginRequest;
import life.mosu.mosuserver.presentation.auth.dto.LoginResponse;
import life.mosu.mosuserver.presentation.auth.dto.Token;
import lombok.RequiredArgsConstructor;
import org.springframework.http.HttpHeaders;
Expand All @@ -32,11 +33,12 @@ public class AuthController implements AuthControllerDocs {
* @return 로그인 한 회원의 Access Token과 Refresh Token
*/
@PostMapping("/login")
public ResponseEntity<ApiResponseWrapper<Token>> login(
public ResponseEntity<ApiResponseWrapper<LoginResponse>> login(
@RequestBody @Valid final LoginRequest request) {
final Token token = authService.login(request);
final LoginResponse response = authService.login(request);

final ResponseCookie cookie = ResponseCookie.from("accessToken", token.accessToken())
final ResponseCookie cookie = ResponseCookie.from("accessToken",
response.token().accessToken())
.httpOnly(true)
.secure(true)
.path("/")
Expand All @@ -45,7 +47,7 @@ public ResponseEntity<ApiResponseWrapper<Token>> login(

return ResponseEntity.status(HttpStatus.CREATED)
.header(HttpHeaders.SET_COOKIE, cookie.toString())
.body(ApiResponseWrapper.success(HttpStatus.CREATED, token));
.body(ApiResponseWrapper.success(HttpStatus.CREATED, response));
}

/**
Expand Down
5 changes: 3 additions & 2 deletions src/main/java/life/mosu/mosuserver/presentation/auth/AuthControllerDocs.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,16 @@
import jakarta.validation.Valid;
import life.mosu.mosuserver.global.util.ApiResponseWrapper;
import life.mosu.mosuserver.presentation.auth.dto.LoginRequest;
import life.mosu.mosuserver.presentation.auth.dto.LoginResponse;
import life.mosu.mosuserver.presentation.auth.dto.Token;
import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.annotation.RequestBody;

@Tag(description = "인증 API", name = "Auth API")
public interface AuthControllerDocs {

@Operation(description = "로그인 API 지금은 쿠키와 response 둘다 반환하는데 곧 쿠키로만 작동하게 할 것 입니다.", summary = "사용자가 로그인합니다.")
public ResponseEntity<ApiResponseWrapper<Token>> login(
@Operation(description = "로그인 API 지금은 쿠키와 response 둘다 반환하는데 곧 쿠키로만 작동하게 할 것 입니다. <프론트하고 변경하려고 Response 이렇게 만들었는데 나중에 같이 맞춥시다!>", summary = "사용자가 로그인합니다.")
public ResponseEntity<ApiResponseWrapper<LoginResponse>> login(
@RequestBody @Valid final LoginRequest request);

@Operation(description = "수정될 예정 입니다.")
Expand Down
11 changes: 11 additions & 0 deletions src/main/java/life/mosu/mosuserver/presentation/auth/dto/LoginResponse.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package life.mosu.mosuserver.presentation.auth.dto;

public record LoginResponse(
Token token,
Boolean isProfileRegistered
) {

public static LoginResponse of(final Token token, final Boolean isProfileRegistered) {
return new LoginResponse(token, isProfileRegistered);
}
}
3 changes: 2 additions & 1 deletion src/main/java/life/mosu/mosuserver/presentation/oauth/AccessTokenFilter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@
// return;
// }
//
// if (request.getRequestURI().startsWith("/api/v1/oauth2")) {
// if (request.getRequestURI().startsWith("/api/v1/oauth2") || request.getRequestURI()
// .startsWith("/api/v1/oauth")) {
// filterChain.doFilter(request, response);
// return;
// }
Expand Down
39 changes: 13 additions & 26 deletions src/main/java/life/mosu/mosuserver/presentation/oauth/OAuthController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
package life.mosu.mosuserver.presentation.oauth;

import static life.mosu.mosuserver.global.resolver.AuthorizationRequestRedirectResolver.REDIRECT_PARAM_KEY;

import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.springframework.http.HttpHeaders;
import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.PathVariable;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RequestParam;
import org.springframework.web.bind.annotation.RestController;
import org.springframework.web.servlet.view.RedirectView;
import org.springframework.web.util.UriComponentsBuilder;

@Slf4j
Expand All @@ -17,30 +18,16 @@
@RequestMapping("/oauth")
public class OAuthController {

@GetMapping("/authorize-code")
public ResponseEntity<Void> handleOAuthCodeFromFrontend(
@RequestParam String code,
@RequestParam(required = false) String state
@GetMapping("/login/{registrationId}")
public RedirectView login(
@PathVariable String registrationId,
@RequestParam(REDIRECT_PARAM_KEY) String redirect
) {
UriComponentsBuilder uriBuilder = UriComponentsBuilder
.fromPath("/api/v1/oauth2/callback/kakao")
.queryParam("code", code);

if (state != null && !state.isEmpty()) {
uriBuilder.queryParam("state", state);
}

String redirectUrl = UriComponentsBuilder.newInstance()
.scheme("http")
.host("localhost")
.port(8080)
.path(uriBuilder.toUriString())
.build()
final String url = UriComponentsBuilder
.fromPath("/api/v1/oauth2/authorization/{registrationId}")
.queryParam(REDIRECT_PARAM_KEY, redirect)
.buildAndExpand(registrationId)
.toUriString();

log.info("클라이언트를 Spring Security OAuth2 콜백 URL로 리다이렉트합니다: {}", redirectUrl);
HttpHeaders headers = new HttpHeaders();
headers.add(HttpHeaders.LOCATION, redirectUrl);
return new ResponseEntity<>(headers, HttpStatus.FOUND);
return new RedirectView(url);
}
}
3 changes: 1 addition & 2 deletions src/main/java/life/mosu/mosuserver/presentation/profile/ProfileController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
import lombok.extern.slf4j.Slf4j;
import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity;
import org.springframework.security.access.prepost.PreAuthorize;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.PostMapping;
import org.springframework.web.bind.annotation.PutMapping;
Expand All @@ -38,7 +37,7 @@ public ResponseEntity<ApiResponseWrapper<Void>> create(
}

@PutMapping
@PreAuthorize("isAuthenticated()")
// @PreAuthorize("isAuthenticated()")
Copy link

@gemini-code-assist gemini-code-assist bot Jul 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

The @PreAuthorize("isAuthenticated()") annotation on the update method has been commented out. This disables authentication for the profile update endpoint, allowing any unauthenticated user to modify any user's profile by simply providing a userId. This is a critical security vulnerability.

Please reinstate this annotation to ensure that only authenticated users can modify profiles.

Suggested change
// @PreAuthorize("isAuthenticated()")
@PreAuthorize("isAuthenticated()")

Copy link

@coderabbitai coderabbitai bot Jul 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Critical: Authentication check removed from profile update endpoint.

The @PreAuthorize("isAuthenticated()") annotation was commented out, which removes authentication requirements from the profile update endpoint. This could allow unauthenticated users to modify profiles.

Please verify this change is intentional and that alternative authentication mechanisms are in place.

🏁 Script executed:

#!/bin/bash
# Verify if there are alternative authentication mechanisms for this endpoint
ast-grep --pattern 'class ProfileController {
  $$$
  @PutMapping
  $$$
  public ResponseEntity<$_> update($_) {
    $$$
  }
  $$$
}'

Length of output: 135

🏁 Script executed:

#!/bin/bash
# Locate and inspect ProfileController for authentication annotations on the update endpoint
FILE="src/main/java/life/mosu/mosuserver/presentation/profile/ProfileController.java"
if [[ -f "$FILE" ]]; then
  echo "----- Begin ProfileController.java (lines 1–200) -----"
  sed -n '1,200p' "$FILE"
  echo "----- End excerpt -----"
  echo
  echo "----- Security annotations in ProfileController.java -----"
  rg -n "@PreAuthorize|@Secured|@RolesAllowed" "$FILE" || echo "(none found)"
  echo
  echo "----- @PutMapping occurrences in ProfileController.java -----"
  rg -n "@PutMapping" "$FILE" || echo "(none found)"
else
  echo "Error: File not found: $FILE"
  exit 1
fi

Length of output: 3303

Critical: Profile update endpoint is unprotected and accepts arbitrary user IDs

Observations:

  • In ProfileController.java (line 40), the @PreAuthorize("isAuthenticated()") annotation on the update method is commented out.
  • The update method still takes @RequestParam Long userId, allowing any caller to specify and modify another user’s profile.
  • No other security annotations (e.g. @Secured, @RolesAllowed) or global config protect this endpoint.

Recommended fixes:

  • Reinstate the authentication check on the update method (e.g. restore @PreAuthorize("isAuthenticated()")).
  • Replace @RequestParam Long userId with @UserId Long userId so the user ID is derived from the authenticated principal.
  • Review your Spring Security configuration to ensure all /profile endpoints require authentication.
🤖 Prompt for AI Agents 
In
src/main/java/life/mosu/mosuserver/presentation/profile/ProfileController.java
at line 40, the update method is unprotected and accepts a userId parameter from
the request, allowing unauthorized profile modifications. To fix this, uncomment
and restore the @PreAuthorize("isAuthenticated()") annotation on the update
method to enforce authentication. Replace the @RequestParam Long userId
parameter with @UserId Long userId to derive the user ID from the authenticated
principal instead of the request. Additionally, verify the Spring Security
configuration to ensure all /profile endpoints require authentication.

public ResponseEntity<ApiResponseWrapper<Void>> update(
@RequestParam Long userId,
@Valid @RequestBody EditProfileRequest request
Expand Down