Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
Walkthrough패키지 구조를 대규모로 재배치하고 인증·OAuth·사용자 관리 관련 서비스(AuthService, OAuthService, UserService), OAuth 클라이언트, Redis 저장소, gRPC/HTTP 핸들러 및 여러 DTO/결과 타입과 README/Dockerfile 변경을 추가했습니다. Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant AuthController as "AuthController"
participant AuthService as "AuthService"
participant UserRepo as "UserRepository"
participant Jwt as "JwtProvider"
participant Blacklist as "TokenBlacklistRepository"
Client->>AuthController: POST /auth/login (email,password)
AuthController->>AuthService: login(LoginCommand)
AuthService->>UserRepo: find active user by email
alt user not found or inactive
AuthService-->>Client: BizException(INVALID_CREDENTIALS)
else user found
AuthService->>AuthService: verify password
alt password mismatch
AuthService-->>Client: BizException(INVALID_CREDENTIALS)
else password ok
AuthService->>Jwt: issue access & refresh tokens
AuthService-->>Client: TokenPair(access, refresh)
end
end
Client->>AuthController: POST /auth/token/refresh (refreshToken)
AuthController->>AuthService: refreshToken(refreshToken)
AuthService->>Blacklist: is blacklisted?
alt blacklisted
AuthService-->>Client: BizException(BLACKLISTED_TOKEN)
else valid
AuthService->>Jwt: issue new TokenPair
AuthService->>Blacklist: blacklist old refresh token (TTL)
AuthService-->>Client: TokenPair(new)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
시
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (7)
src/main/java/flipnote/user/domain/common/BizException.java (1)
3-3: 동일 패키지 내 불필요한 import
ErrorCode는BizException과 동일한 패키지(flipnote.user.domain.common)에 있으므로 이 import 문은 불필요합니다.♻️ 제안된 수정
package flipnote.user.domain.common; -import flipnote.user.domain.common.ErrorCode; import lombok.AllArgsConstructor; import lombok.Getter;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/flipnote/user/domain/common/BizException.java` at line 3, Remove the unnecessary import of ErrorCode from BizException since ErrorCode is in the same package (flipnote.user.domain.common); in the BizException class delete the line "import flipnote.user.domain.common.ErrorCode" and ensure the class still compiles (no other missing imports) and build passes for BizException and any usages.src/main/java/flipnote/user/infrastructure/jwt/JwtProvider.java (1)
1-6: 불필요한 import 제거 권장Line 6의
flipnote.user.infrastructure.jwt.JwtPropertiesimport는 동일 패키지 내 클래스이므로 불필요합니다.♻️ 수정 제안
package flipnote.user.infrastructure.jwt; import flipnote.user.domain.TokenClaims; import flipnote.user.domain.TokenPair; import flipnote.user.domain.entity.User; -import flipnote.user.infrastructure.jwt.JwtProperties; import io.jsonwebtoken.Claims;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/flipnote/user/infrastructure/jwt/JwtProvider.java` around lines 1 - 6, Remove the redundant import of JwtProperties in JwtProvider.java (the line importing flipnote.user.infrastructure.jwt.JwtProperties) since JwtProperties is in the same package; delete that import statement and run a quick compile to ensure no remaining unresolved references in the JwtProvider class or other classes in this file.src/main/java/flipnote/user/application/AuthService.java (1)
22-27: 계층 간 의존성 방향 검토 권장Application 계층(
flipnote.user.application)에서 Interfaces 계층(flipnote.user.interfaces.http.dto.*)의 DTO를 직접 참조하고 있습니다. Clean Architecture 관점에서 내부 계층이 외부 계층에 의존하는 것은 의존성 역전 원칙에 위배됩니다.권장 방안:
- Application 계층에 별도의 Command/Query DTO를 정의하고, Interfaces 계층에서 변환하거나
- 공유 DTO 모듈을 별도로 분리하는 것을 고려해 주세요.
현재 동작에는 문제가 없으나, 향후 확장성과 테스트 용이성을 위해 검토가 필요합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/flipnote/user/application/AuthService.java` around lines 22 - 27, AuthService currently imports external HTTP DTOs (ChangePasswordRequest, LoginRequest, SignupRequest, SocialLinksResponse, TokenValidateResponse, UserResponse) from flipnote.user.interfaces.http.dto.*, violating dependency direction; to fix, introduce application-layer DTOs/commands/queries (e.g., AuthSignUpCommand, AuthLoginCommand, ChangePasswordCommand, AuthUserDto, TokenValidationResult) and update AuthService method signatures to use those internal types, then move the conversion/mapping between HTTP DTOs and application DTOs into the Interfaces layer (controllers/assemblers) or a shared mapping component so flipnote.user.application no longer depends on flipnote.user.interfaces.src/main/java/flipnote/user/domain/AuthErrorCode.java (1)
25-27: 에러 코드 순번 확인 권장
AUTH_014코드가 누락되어 있습니다 (AUTH_013→AUTH_015). 의도된 것인지 확인이 필요합니다. 이전에 제거된 코드가 있었다면 향후 혼란 방지를 위해 주석으로 기록해두는 것이 좋습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/flipnote/user/domain/AuthErrorCode.java` around lines 25 - 27, The enum in AuthErrorCode has a missing sequential code (AUTH_014) between INVALID_OAUTH_PROVIDER ("AUTH_013") and BLACKLISTED_TOKEN ("AUTH_015"); either renumber BLACKLISTED_TOKEN to "AUTH_014" (update the string in the BLACKLISTED_TOKEN enum entry) or add a commented placeholder enum entry labeled AUTH_014 with a short note about why it was removed, so the enum sequence and history are clear; refer to the enum constants INVALID_OAUTH_PROVIDER, BLACKLISTED_TOKEN, and INVALIDATED_SESSION when making the change.src/main/java/flipnote/user/application/UserService.java (1)
45-48:getUserInfo와findActiveUser간 코드 중복 고려.
getUserInfo메서드(Line 46-47)와findActiveUser메서드(Line 103-104)가 동일한 로직을 수행합니다.getUserInfo에서findActiveUser를 재사용하면 중복을 줄일 수 있습니다.♻️ 제안된 리팩토링
public UserInfoResponse getUserInfo(Long userId) { - User user = userRepository.findByIdAndStatus(userId, User.Status.ACTIVE) - .orElseThrow(() -> new BizException(UserErrorCode.USER_NOT_FOUND)); + User user = findActiveUser(userId); return UserInfoResponse.from(user); }Also applies to: 102-105
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/flipnote/user/application/UserService.java` around lines 45 - 48, Refactor getUserInfo to reuse the existing findActiveUser method to remove duplicated lookup/error handling: replace the direct repository call in UserService.getUserInfo with a call to findActiveUser(userId) and then convert the returned User to UserInfoResponse via UserInfoResponse.from(...), keeping existing exceptions (BizException with UserErrorCode.USER_NOT_FOUND) intact by relying on findActiveUser’s behavior.src/main/java/flipnote/user/infrastructure/mail/ResendMailService.java (1)
7-7: 불필요한 동일 패키지 import
ResendProperties는 현재 파일과 동일한 패키지(flipnote.user.infrastructure.mail)에 있으므로 이 import 문은 불필요합니다.♻️ 제안된 수정
import flipnote.user.domain.common.EmailSendException; -import flipnote.user.infrastructure.mail.ResendProperties; import lombok.RequiredArgsConstructor;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/flipnote/user/infrastructure/mail/ResendMailService.java` at line 7, ResendMailService에 있는 불필요한 import 문을 제거하세요: 현재 파일과 동일한 패키지인 flipnote.user.infrastructure.mail에서 가져오는 ResendProperties import는 제거해도 됩니다; ResendMailService 클래스의 import 목록에서 해당 import 줄을 삭제하여 컴파일과 동작에 영향이 없도록 정리하세요.src/main/java/flipnote/user/interfaces/http/AuthController.java (1)
1-1: 패키지 경로 불일치 가능성
AuthController가flipnote.user.interfaces패키지에 있지만,UserController도 동일한 패키지에 있고 HTTP 관련 유틸리티는flipnote.user.interfaces.http.common에 있습니다. HTTP 컨트롤러들을flipnote.user.interfaces.http패키지로 이동하는 것이 레이어드 아키텍처 구조와 더 일관성 있을 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/flipnote/user/interfaces/http/AuthController.java` at line 1, AuthController is currently declared in package flipnote.user.interfaces but HTTP controllers belong under flipnote.user.interfaces.http; update the package declaration in AuthController to flipnote.user.interfaces.http, relocate the source file to the corresponding package directory, and adjust any imports/usages (e.g., references from UserController or common utilities in flipnote.user.interfaces.http.common) to the new package so compilation and layered architecture remain consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@README.md`:
- Around line 46-81: Several fenced code blocks in the README are missing a
language specifier (causing MD040 lint warnings); update each triple-backtick
block that contains shell/env/text or file listings to include an appropriate
language tag such as bash or text (e.g., change ``` to ```bash for environment
variable blocks and to ```text for file path examples). Locate the blocks around
the env vars and the file/path examples (the blocks containing
DB_URL/REDIS/GIT/JWT/APP_CLIENT_URL and the blocks showing src/main/java/... and
FlipNote-User/) and add the language tokens consistently to silence the linter.
- Around line 95-97: The README's stated local port (http://localhost:8081 and
Swagger URL /users/swagger-ui.html) conflicts with the Docker example which maps
8081:8080; update the Docker run/compose example to map the same internal port
(e.g., change 8081:8080 to 8081:8081) or change the README URLs so they match
the container mapping, and apply the same fix to the other occurrence of the
port mismatch later in the file (the other block that references the local port
and Docker mapping).
- Around line 17-18: The TOC fragment link may break because the heading uses
emoji and GitHub's slug can differ; fix by adding an explicit HTML anchor before
the heading (e.g., add <a name="execution-and-deployment"></a> immediately above
the "## 🖥️ 실행 및 배포" heading) and update the TOC entry link to point to that
anchor (change the link target in the list entry that currently references
"#-실행-및-배포" to the new "#execution-and-deployment"); do the same for "## 프로젝트
구조" if needed, using clear anchor names to ensure stable TOC links.
In `@src/main/java/flipnote/user/application/OAuthService.java`:
- Around line 93-106: 현재 로직은 특정 사용자(userId)에 대한 중복 링크만 확인하므로 동일한 (provider,
providerId)를 다른 사용자에게도 다시 연결할 수 있습니다;
oAuthLinkRepository.existsByUser_IdAndProviderAndProviderId(...) 체크에 더해 먼저
oAuthLinkRepository.existsByProviderAndProviderId(userInfo.getProvider(),
userInfo.getProviderId())로 전역 중복을 확인하고 중복인 경우
BizException(AuthErrorCode.ALREADY_LINKED_SOCIAL_ACCOUNT)을 던지도록 수정하고, OAuthLink
엔티티(DB 스키마)에 (provider, providerId)에 대한 unique 제약을 추가하며 동시에 save 시 DB 충돌(유니크 위반)
발생 가능성을 고려해 save 호출 주위에서 데이터베이스 고유 제약 위반 예외를 잡아 적절히
AuthErrorCode.ALREADY_LINKED_SOCIAL_ACCOUNT으로 변환하도록 처리하세요 (참조:
oAuthLinkRepository.existsByProviderAndProviderId, oAuthLinkRepository.save,
OAuthLink builder, UserRepository.findByIdAndStatus).
In `@src/main/java/flipnote/user/infrastructure/oauth/OAuthApiClient.java`:
- Around line 33-34: The code reconstructs the redirect URI from the incoming
request (via buildRedirectUri(request, provider.getRedirectUri())), which is
unsafe; change both the token request (where params.add("redirect_uri",
buildRedirectUri(...)) is used) and the authorization request construction (the
block around lines building the auth URL) to use a fixed, pre-registered
redirect URI instead (e.g., provider.getRedirectUri() or a dedicated config
value), and remove/replace any logic in buildRedirectUri that uses
ServletUriComponentsBuilder.fromRequestUri(request) so the redirect URI is not
derived from the request host/scheme.
- Around line 17-18: OAuthApiClient.java의 잘못된 Jackson 패키지 import를 수정하세요: 현재
import lines에서 tools.jackson.core.type.TypeReference와
tools.jackson.databind.ObjectMapper를
com.fasterxml.jackson.core.type.TypeReference와
com.fasterxml.jackson.databind.ObjectMapper로 변경하여 Spring Boot 4 / Jackson 2.x와
일치시키고 다른 클래스(SocialLinkResponse, MyInfoResponse)에서 사용하는
com.fasterxml.jackson.annotation.JsonFormat와 패키지명을 일관되게 유지하세요.
In `@src/main/java/flipnote/user/infrastructure/oauth/OAuthProperties.java`:
- Around line 11-27: Annotate the OAuthProperties configuration so Spring
validates values at startup: add `@Validated` to the OAuthProperties class and
apply javax.validation constraints to Provider fields (e.g., mark clientId,
clientSecret, redirectUri, authorizationUri, tokenUri, userInfoUri with
`@NotBlank` and scope with `@NotEmpty`) so missing or empty values fail fast; keep
the nested class name Provider and the
OAuthProperties/@ConfigurationProperties(prefix = "app.oauth2") setup so
validation triggers on application start.
In `@src/main/java/flipnote/user/interfaces/grpc/GrpcExceptionHandlerImpl.java`:
- Around line 26-32: When handling gRPC errors in GrpcExceptionHandlerImpl,
avoid converting StatusRuntimeException to a new StatusException (which drops
trailers) and also handle StatusException explicitly; instead, if t is a
StatusRuntimeException return the original exception (preserving getTrailers()),
if t is a StatusException return it as-is, otherwise log and return
Status.INTERNAL.withDescription(...).asException(); update the instanceof
branches around the StatusRuntimeException check to return the original
exception objects rather than calling e.getStatus().asException().
In `@src/main/java/flipnote/user/interfaces/http/OAuthController.java`:
- Around line 45-60: The controller currently declares the code RequestParam and
the cookie (HttpConstants.OAUTH_VERIFIER_COOKIE) as required which causes Spring
to return 400 before handleCallback runs; change the signature of handleCallback
so code and codeVerifier are optional (make the `@RequestParam` and `@CookieValue`
non-required / nullable) and add an explicit branch at the top of handleCallback
that detects a missing code or missing codeVerifier and immediately redirects to
your login/link failure URL (reuse the same failure redirect logic used in
handleSocialLogin and handleSocialLink) before deleting the cookie and before
calling handleSocialLogin or handleSocialLink so the existing failure handling
is executed.
---
Nitpick comments:
In `@src/main/java/flipnote/user/application/AuthService.java`:
- Around line 22-27: AuthService currently imports external HTTP DTOs
(ChangePasswordRequest, LoginRequest, SignupRequest, SocialLinksResponse,
TokenValidateResponse, UserResponse) from flipnote.user.interfaces.http.dto.*,
violating dependency direction; to fix, introduce application-layer
DTOs/commands/queries (e.g., AuthSignUpCommand, AuthLoginCommand,
ChangePasswordCommand, AuthUserDto, TokenValidationResult) and update
AuthService method signatures to use those internal types, then move the
conversion/mapping between HTTP DTOs and application DTOs into the Interfaces
layer (controllers/assemblers) or a shared mapping component so
flipnote.user.application no longer depends on flipnote.user.interfaces.
In `@src/main/java/flipnote/user/application/UserService.java`:
- Around line 45-48: Refactor getUserInfo to reuse the existing findActiveUser
method to remove duplicated lookup/error handling: replace the direct repository
call in UserService.getUserInfo with a call to findActiveUser(userId) and then
convert the returned User to UserInfoResponse via UserInfoResponse.from(...),
keeping existing exceptions (BizException with UserErrorCode.USER_NOT_FOUND)
intact by relying on findActiveUser’s behavior.
In `@src/main/java/flipnote/user/domain/AuthErrorCode.java`:
- Around line 25-27: The enum in AuthErrorCode has a missing sequential code
(AUTH_014) between INVALID_OAUTH_PROVIDER ("AUTH_013") and BLACKLISTED_TOKEN
("AUTH_015"); either renumber BLACKLISTED_TOKEN to "AUTH_014" (update the string
in the BLACKLISTED_TOKEN enum entry) or add a commented placeholder enum entry
labeled AUTH_014 with a short note about why it was removed, so the enum
sequence and history are clear; refer to the enum constants
INVALID_OAUTH_PROVIDER, BLACKLISTED_TOKEN, and INVALIDATED_SESSION when making
the change.
In `@src/main/java/flipnote/user/domain/common/BizException.java`:
- Line 3: Remove the unnecessary import of ErrorCode from BizException since
ErrorCode is in the same package (flipnote.user.domain.common); in the
BizException class delete the line "import
flipnote.user.domain.common.ErrorCode" and ensure the class still compiles (no
other missing imports) and build passes for BizException and any usages.
In `@src/main/java/flipnote/user/infrastructure/jwt/JwtProvider.java`:
- Around line 1-6: Remove the redundant import of JwtProperties in
JwtProvider.java (the line importing
flipnote.user.infrastructure.jwt.JwtProperties) since JwtProperties is in the
same package; delete that import statement and run a quick compile to ensure no
remaining unresolved references in the JwtProvider class or other classes in
this file.
In `@src/main/java/flipnote/user/infrastructure/mail/ResendMailService.java`:
- Line 7: ResendMailService에 있는 불필요한 import 문을 제거하세요: 현재 파일과 동일한 패키지인
flipnote.user.infrastructure.mail에서 가져오는 ResendProperties import는 제거해도 됩니다;
ResendMailService 클래스의 import 목록에서 해당 import 줄을 삭제하여 컴파일과 동작에 영향이 없도록 정리하세요.
In `@src/main/java/flipnote/user/interfaces/http/AuthController.java`:
- Line 1: AuthController is currently declared in package
flipnote.user.interfaces but HTTP controllers belong under
flipnote.user.interfaces.http; update the package declaration in AuthController
to flipnote.user.interfaces.http, relocate the source file to the corresponding
package directory, and adjust any imports/usages (e.g., references from
UserController or common utilities in flipnote.user.interfaces.http.common) to
the new package so compilation and layered architecture remain consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4122d2f3-c3b4-43be-ba8a-88093b06b6b1
📒 Files selected for processing (72)
README.mdsrc/main/java/flipnote/user/application/AuthService.javasrc/main/java/flipnote/user/application/OAuthService.javasrc/main/java/flipnote/user/application/UserService.javasrc/main/java/flipnote/user/domain/AuthErrorCode.javasrc/main/java/flipnote/user/domain/ImageErrorCode.javasrc/main/java/flipnote/user/domain/PasswordResetConstants.javasrc/main/java/flipnote/user/domain/TokenClaims.javasrc/main/java/flipnote/user/domain/TokenPair.javasrc/main/java/flipnote/user/domain/UserErrorCode.javasrc/main/java/flipnote/user/domain/VerificationConstants.javasrc/main/java/flipnote/user/domain/common/BizException.javasrc/main/java/flipnote/user/domain/common/EmailSendException.javasrc/main/java/flipnote/user/domain/common/ErrorCode.javasrc/main/java/flipnote/user/domain/entity/BaseEntity.javasrc/main/java/flipnote/user/domain/entity/OAuthLink.javasrc/main/java/flipnote/user/domain/entity/User.javasrc/main/java/flipnote/user/domain/event/EmailVerificationSendEvent.javasrc/main/java/flipnote/user/domain/event/PasswordResetCreateEvent.javasrc/main/java/flipnote/user/domain/repository/OAuthLinkRepository.javasrc/main/java/flipnote/user/domain/repository/UserRepository.javasrc/main/java/flipnote/user/infrastructure/config/AppConfig.javasrc/main/java/flipnote/user/infrastructure/config/ClientProperties.javasrc/main/java/flipnote/user/infrastructure/config/GrpcClientConfig.javasrc/main/java/flipnote/user/infrastructure/config/JpaAuditingConfig.javasrc/main/java/flipnote/user/infrastructure/config/SwaggerConfig.javasrc/main/java/flipnote/user/infrastructure/jwt/JwtProperties.javasrc/main/java/flipnote/user/infrastructure/jwt/JwtProvider.javasrc/main/java/flipnote/user/infrastructure/listener/EmailVerificationEventListener.javasrc/main/java/flipnote/user/infrastructure/listener/PasswordResetEventListener.javasrc/main/java/flipnote/user/infrastructure/mail/MailService.javasrc/main/java/flipnote/user/infrastructure/mail/ResendConfig.javasrc/main/java/flipnote/user/infrastructure/mail/ResendMailService.javasrc/main/java/flipnote/user/infrastructure/mail/ResendProperties.javasrc/main/java/flipnote/user/infrastructure/oauth/GoogleUserInfo.javasrc/main/java/flipnote/user/infrastructure/oauth/OAuth2UserInfo.javasrc/main/java/flipnote/user/infrastructure/oauth/OAuthApiClient.javasrc/main/java/flipnote/user/infrastructure/oauth/OAuthProperties.javasrc/main/java/flipnote/user/infrastructure/oauth/PkceUtil.javasrc/main/java/flipnote/user/infrastructure/redis/EmailVerificationRepository.javasrc/main/java/flipnote/user/infrastructure/redis/PasswordResetRepository.javasrc/main/java/flipnote/user/infrastructure/redis/PasswordResetTokenGenerator.javasrc/main/java/flipnote/user/infrastructure/redis/SessionInvalidationRepository.javasrc/main/java/flipnote/user/infrastructure/redis/SocialLinkTokenRepository.javasrc/main/java/flipnote/user/infrastructure/redis/TokenBlacklistRepository.javasrc/main/java/flipnote/user/infrastructure/redis/VerificationCodeGenerator.javasrc/main/java/flipnote/user/interfaces/grpc/GrpcExceptionHandlerImpl.javasrc/main/java/flipnote/user/interfaces/grpc/GrpcUserQueryService.javasrc/main/java/flipnote/user/interfaces/http/AuthController.javasrc/main/java/flipnote/user/interfaces/http/OAuthController.javasrc/main/java/flipnote/user/interfaces/http/UserController.javasrc/main/java/flipnote/user/interfaces/http/common/ApiResponse.javasrc/main/java/flipnote/user/interfaces/http/common/ApiResponseAdvice.javasrc/main/java/flipnote/user/interfaces/http/common/CookieUtil.javasrc/main/java/flipnote/user/interfaces/http/common/GlobalExceptionHandler.javasrc/main/java/flipnote/user/interfaces/http/common/HttpConstants.javasrc/main/java/flipnote/user/interfaces/http/dto/request/ChangePasswordRequest.javasrc/main/java/flipnote/user/interfaces/http/dto/request/EmailVerificationRequest.javasrc/main/java/flipnote/user/interfaces/http/dto/request/EmailVerifyRequest.javasrc/main/java/flipnote/user/interfaces/http/dto/request/LoginRequest.javasrc/main/java/flipnote/user/interfaces/http/dto/request/PasswordResetCreateRequest.javasrc/main/java/flipnote/user/interfaces/http/dto/request/PasswordResetRequest.javasrc/main/java/flipnote/user/interfaces/http/dto/request/SignupRequest.javasrc/main/java/flipnote/user/interfaces/http/dto/request/TokenValidateRequest.javasrc/main/java/flipnote/user/interfaces/http/dto/request/UpdateProfileRequest.javasrc/main/java/flipnote/user/interfaces/http/dto/response/MyInfoResponse.javasrc/main/java/flipnote/user/interfaces/http/dto/response/SocialLinkResponse.javasrc/main/java/flipnote/user/interfaces/http/dto/response/SocialLinksResponse.javasrc/main/java/flipnote/user/interfaces/http/dto/response/TokenValidateResponse.javasrc/main/java/flipnote/user/interfaces/http/dto/response/UserInfoResponse.javasrc/main/java/flipnote/user/interfaces/http/dto/response/UserResponse.javasrc/main/java/flipnote/user/interfaces/http/dto/response/UserUpdateResponse.java
| if (oAuthLinkRepository.existsByUser_IdAndProviderAndProviderId( | ||
| userId, userInfo.getProvider(), userInfo.getProviderId())) { | ||
| throw new BizException(AuthErrorCode.ALREADY_LINKED_SOCIAL_ACCOUNT); | ||
| } | ||
|
|
||
| User user = userRepository.findByIdAndStatus(userId, User.Status.ACTIVE) | ||
| .orElseThrow(() -> new BizException(UserErrorCode.USER_NOT_FOUND)); | ||
|
|
||
| OAuthLink link = OAuthLink.builder() | ||
| .provider(userInfo.getProvider()) | ||
| .providerId(userInfo.getProviderId()) | ||
| .user(user) | ||
| .build(); | ||
| oAuthLinkRepository.save(link); |
There was a problem hiding this comment.
같은 소셜 계정을 다른 사용자에게 다시 연결할 수 있습니다.
지금 체크는 "이 사용자에게 이미 같은 링크가 있나"만 봅니다. 다른 사용자에게 이미 연결된 (provider, providerId) 는 서비스 레벨에서 막지 못해서, DB 제약이 없으면 중복 링크가 생성되고 제약이 있어도 여기서는 ALREADY_LINKED_SOCIAL_ACCOUNT 대신 DB 예외로 떨어질 가능성이 큽니다. 여기서는 provider + providerId 전역 기준으로 검사하고, DB에도 같은 unique constraint를 두는 편이 맞습니다.
수정 방향
- if (oAuthLinkRepository.existsByUser_IdAndProviderAndProviderId(
- userId, userInfo.getProvider(), userInfo.getProviderId())) {
+ if (oAuthLinkRepository.existsByProviderAndProviderId(
+ userInfo.getProvider(), userInfo.getProviderId())) {
throw new BizException(AuthErrorCode.ALREADY_LINKED_SOCIAL_ACCOUNT);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/flipnote/user/application/OAuthService.java` around lines 93 -
106, 현재 로직은 특정 사용자(userId)에 대한 중복 링크만 확인하므로 동일한 (provider, providerId)를 다른
사용자에게도 다시 연결할 수 있습니다;
oAuthLinkRepository.existsByUser_IdAndProviderAndProviderId(...) 체크에 더해 먼저
oAuthLinkRepository.existsByProviderAndProviderId(userInfo.getProvider(),
userInfo.getProviderId())로 전역 중복을 확인하고 중복인 경우
BizException(AuthErrorCode.ALREADY_LINKED_SOCIAL_ACCOUNT)을 던지도록 수정하고, OAuthLink
엔티티(DB 스키마)에 (provider, providerId)에 대한 unique 제약을 추가하며 동시에 save 시 DB 충돌(유니크 위반)
발생 가능성을 고려해 save 호출 주위에서 데이터베이스 고유 제약 위반 예외를 잡아 적절히
AuthErrorCode.ALREADY_LINKED_SOCIAL_ACCOUNT으로 변환하도록 처리하세요 (참조:
oAuthLinkRepository.existsByProviderAndProviderId, oAuthLinkRepository.save,
OAuthLink builder, UserRepository.findByIdAndStatus).
| import tools.jackson.core.type.TypeReference; | ||
| import tools.jackson.databind.ObjectMapper; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -e
fd -a 'pom\.xml|build\.gradle(\.kts)?|settings\.gradle(\.kts)?|gradle\.properties' . \
-x sh -c 'echo "### $1"; sed -n "1,220p" "$1"' sh {}
rg -n -C2 'tools\.jackson|com\.fasterxml\.jackson|jackson' .Repository: FlipNoteTeam/FlipNote-User
Length of output: 4918
🌐 Web query:
Spring Boot 4.0.2 Jackson dependency version
💡 Result:
Spring Boot 4.0.2 manages Jackson 3 (default) at version 2.21.2 for core modules like jackson-core, jackson-databind, and jackson-annotations at 2.21. Jackson 2 support is available separately via spring-boot-jackson2 (deprecated) with BOM upgrades tracked in issues like #48910 (2.20.2 for 4.0.2) and later #49389 (2.21.1).
Citations:
- 1: https://docs.spring.io/spring-boot/4.0/appendix/dependency-versions/coordinates.html
- 2: https://docs.spring.io/spring-boot/4.0/4.1-SNAPSHOT/appendix/dependency-versions/coordinates.html
- 3: Upgrade to Jackson 2 Bom 2.20.2 spring-projects/spring-boot#48910
- 4: https://github.com/spring-projects/spring-boot/wiki/Spring-Boot-4.0-Release-Notes
- 5: https://docs.spring.io/spring-boot/4.0/4.1/reference/features/json.html
🏁 Script executed:
rg -i 'jackson' build.gradle.kts pom.xml --no-heading -B2 -A2Repository: FlipNoteTeam/FlipNote-User
Length of output: 393
올바른 Jackson 패키지명으로 변경해 주세요.
OAuthApiClient.java 17-18번 줄의 import 문이 잘못되었습니다. 프로젝트는 Spring Boot 4.0.2를 사용하며, 이는 Jackson 2.21.2를 포함합니다. Jackson 2.x의 패키지명은 com.fasterxml.jackson.*입니다.
현재 코드는 Jackson 3.x 패키지명인 tools.jackson.*을 사용하고 있어서 컴파일되지 않습니다. 다른 파일들(SocialLinkResponse.java, MyInfoResponse.java)에서도 com.fasterxml.jackson.annotation.JsonFormat를 사용하고 있으니 일관성을 맞춰야 합니다.
수정 필요 부분
- import tools.jackson.core.type.TypeReference;
- import tools.jackson.databind.ObjectMapper;
+ import com.fasterxml.jackson.core.type.TypeReference;
+ import com.fasterxml.jackson.databind.ObjectMapper;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/flipnote/user/infrastructure/oauth/OAuthApiClient.java` around
lines 17 - 18, OAuthApiClient.java의 잘못된 Jackson 패키지 import를 수정하세요: 현재 import
lines에서 tools.jackson.core.type.TypeReference와
tools.jackson.databind.ObjectMapper를
com.fasterxml.jackson.core.type.TypeReference와
com.fasterxml.jackson.databind.ObjectMapper로 변경하여 Spring Boot 4 / Jackson 2.x와
일치시키고 다른 클래스(SocialLinkResponse, MyInfoResponse)에서 사용하는
com.fasterxml.jackson.annotation.JsonFormat와 패키지명을 일관되게 유지하세요.
src/main/java/flipnote/user/infrastructure/oauth/OAuthApiClient.java
Outdated
Show resolved
Hide resolved
src/main/java/flipnote/user/infrastructure/oauth/OAuthProperties.java
Outdated
Show resolved
Hide resolved
| if (t instanceof StatusRuntimeException e) { | ||
| log.warn("gRPC StatusRuntimeException: status={}, description={}", | ||
| e.getStatus().getCode(), e.getStatus().getDescription()); | ||
| return e.getStatus().asException(); | ||
| } | ||
| log.error("gRPC Unhandled exception", t); | ||
| return Status.INTERNAL.withDescription("Internal server error").asException(); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find and inspect the actual file
find . -type f -name "GrpcExceptionHandlerImpl.java" 2>/dev/nullRepository: FlipNoteTeam/FlipNote-User
Length of output: 145
🏁 Script executed:
# Let's also search for any gRPC related files to understand the codebase structure
fd -e java | grep -i grpc | head -20Repository: FlipNoteTeam/FlipNote-User
Length of output: 285
🏁 Script executed:
# Search for imports and usage of StatusException and StatusRuntimeException
rg "StatusException|StatusRuntimeException" --type java -B 2 -A 5Repository: FlipNoteTeam/FlipNote-User
Length of output: 3126
🏁 Script executed:
# Let's read the complete file to understand the full context
cat -n src/main/java/flipnote/user/interfaces/grpc/GrpcExceptionHandlerImpl.javaRepository: FlipNoteTeam/FlipNote-User
Length of output: 2208
🌐 Web query:
gRPC StatusRuntimeException getTrailers() method API documentation
💡 Result:
The gRPC Java class io.grpc.StatusRuntimeException has a method getTrailers that returns the received trailers as a Metadata object. Method signature: @Nullable public final Metadata getTrailers Description: Returns the received trailers. This method was introduced in gRPC Java version 1.0.0 and is available in recent versions like 1.80.0. This exception is used to propagate gRPC status information, including trailers, via exceptions. Trailers can be provided at construction: public StatusRuntimeException(Status status, @Nullable Metadata trailers) To extract trailers from a caught exception: try { // gRPC call } catch (StatusRuntimeException e) { Metadata trailers = e.getTrailers; } Note: When throwing StatusRuntimeException on the server side, trailers may not propagate correctly unless handled via ServerCall.onError; direct throws lose trailers due to exception handling mechanics (as discussed in grpc/grpc-java issue #6683).
Citations:
- 1: https://grpc.github.io/grpc-java/javadoc/io/grpc/StatusRuntimeException.html
- 2: https://javadoc.io/doc/io.grpc/grpc-api/1.28.0/io/grpc/StatusRuntimeException.html
- 3: https://grpc.io/grpc-java/javadoc/io/grpc/Status.html
- 4: StatusRuntimeException trailers not sent to client grpc/grpc-java#6683
🌐 Web query:
gRPC Status.asException() method with Metadata trailers parameter
💡 Result:
The gRPC Java library provides the Status.asException(Metadata trailers) method in the io.grpc.Status class. This static factory method converts a Status object to a StatusException (a checked exception) that includes the provided Metadata as trailers. There is also a corresponding Status.asRuntimeException(Metadata trailers) method that returns a StatusRuntimeException (unchecked exception) with the trailers. Usage example from official patterns: Metadata trailers = new Metadata; trailers.put(Metadata.Key.of("custom-key", Metadata.ASCII_STRING_MARSHALLER), "custom-value"); Status status = Status.INVALID_ARGUMENT.withDescription("Invalid input"); throw status.asException(trailers); // or .asRuntimeException(trailers) On the receiving end (client or server error handling), extract trailers with exception.getTrailers or Status.trailersFromThrowable(exception). This is available since gRPC-Java 1.0.0 and stable as of recent versions (1.80.0 API docs). Trailers are sent as HTTP/2 trailers in error responses, useful for additional error details, tracing, or metrics. Note: When throwing these exceptions from server implementations, gRPC extracts and transmits the trailers correctly if handled properly (e.g., via ServerCall.onError or direct exception propagation in supported contexts). Some frameworks like Quarkus had early bugs but are resolved.
Citations:
- 1: https://grpc.github.io/grpc-java/javadoc/io/grpc/StatusException.html
- 2: https://grpc.io/grpc-java/javadoc/io/grpc/StatusException.html
- 3: https://grpc.github.io/grpc-java/javadoc/io/grpc/StatusRuntimeException.html
- 4: https://grpc.github.io/grpc-java/javadoc/io/grpc/class-use/StatusRuntimeException.html
- 5: https://grpc.github.io/grpc-java/javadoc/io/grpc/class-use/StatusException.html
- 6: https://grpc.github.io/grpc-java/javadoc/io/grpc/Status.html
- 7: https://www.baeldung.com/grpcs-error-handling
- 8: https://yidongnan.github.io/grpc-spring-boot-starter/en/server/exception-handling.html
- 9: Adding trailer metadata in gRPC server error cause the response to fail with UNKNOWN status quarkusio/quarkus#31110
gRPC 상태와 메타데이터가 손실되고 있습니다.
StatusRuntimeException을 처리할 때 e.getStatus().asException()으로 변환하면서 trailers(메타데이터)가 사라집니다. 또한 StatusException을 별도로 처리하지 않아서, 이미 구성된 gRPC 상태도 라인 32의 INTERNAL 오류로 덮어씌워집니다.
수정 예시:
권장 수정
+ if (t instanceof StatusException e) {
+ log.warn("gRPC StatusException: status={}, description={}",
+ e.getStatus().getCode(), e.getStatus().getDescription());
+ return e;
+ }
if (t instanceof StatusRuntimeException e) {
log.warn("gRPC StatusRuntimeException: status={}, description={}",
e.getStatus().getCode(), e.getStatus().getDescription());
- return e.getStatus().asException();
+ return e.getStatus().asException(e.getTrailers());
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/flipnote/user/interfaces/grpc/GrpcExceptionHandlerImpl.java`
around lines 26 - 32, When handling gRPC errors in GrpcExceptionHandlerImpl,
avoid converting StatusRuntimeException to a new StatusException (which drops
trailers) and also handle StatusException explicitly; instead, if t is a
StatusRuntimeException return the original exception (preserving getTrailers()),
if t is a StatusException return it as-is, otherwise log and return
Status.INTERNAL.withDescription(...).asException(); update the instanceof
branches around the StatusRuntimeException check to return the original
exception objects rather than calling e.getStatus().asException().
| @GetMapping("/oauth2/callback/{provider}") | ||
| public ResponseEntity<Void> handleCallback( | ||
| @PathVariable String provider, | ||
| @RequestParam String code, | ||
| @RequestParam(required = false) String state, | ||
| @CookieValue(HttpConstants.OAUTH_VERIFIER_COOKIE) String codeVerifier, | ||
| HttpServletRequest request, | ||
| HttpServletResponse response) { | ||
|
|
||
| CookieUtil.deleteCookie(response, HttpConstants.OAUTH_VERIFIER_COOKIE); | ||
|
|
||
| boolean isSocialLinkRequest = StringUtils.hasText(state); | ||
| if (isSocialLinkRequest) { | ||
| return handleSocialLink(provider, code, state, codeVerifier, request); | ||
| } | ||
| return handleSocialLogin(provider, code, codeVerifier, request, response); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate the OAuthController file and examine its structure
find . -name "OAuthController.java" -type f 2>/dev/nullRepository: FlipNoteTeam/FlipNote-User
Length of output: 136
🏁 Script executed:
# Check the file size before reading
wc -l src/main/java/flipnote/user/interfaces/http/OAuthController.java 2>/dev/null || echo "File not found in this path, searching..."Repository: FlipNoteTeam/FlipNote-User
Length of output: 138
🏁 Script executed:
# Search for OAuthController across the repository
rg "class OAuthController" --type javaRepository: FlipNoteTeam/FlipNote-User
Length of output: 165
🏁 Script executed:
cat -n src/main/java/flipnote/user/interfaces/http/OAuthController.javaRepository: FlipNoteTeam/FlipNote-User
Length of output: 5644
필수 파라미터로 인해 콜백 실패 케이스가 컨트롤러에 도달하기 전에 400으로 끝납니다.
현재 code 파라미터(48줄)와 verifier cookie(50줄)를 필수로 설정했으므로, OAuth 제공자의 error=access_denied 콜백이나 만료된 cookie 케이스에서 Spring이 요청을 거부하고 이 메서드는 실행되지 않습니다. 따라서 6379줄의 handleSocialLogin과 8298줄의 handleSocialLink에 구현된 실패 리다이렉트 로직이 우회되어 사용자가 적절한 실패 페이지로 이동하지 못합니다.
code와 codeVerifier를 모두 선택적으로 변경하고, 누락 시 로그인/연동 실패 URL로 직접 리다이렉트하는 분기를 추가하세요.
수정 예시
- `@RequestParam` String code,
+ `@RequestParam`(required = false) String code,
`@RequestParam`(required = false) String state,
- `@CookieValue`(HttpConstants.OAUTH_VERIFIER_COOKIE) String codeVerifier,
+ `@CookieValue`(value = HttpConstants.OAUTH_VERIFIER_COOKIE, required = false) String codeVerifier,
HttpServletRequest request,
HttpServletResponse response) {
+ if (!StringUtils.hasText(code) || !StringUtils.hasText(codeVerifier)) {
+ String failurePath = StringUtils.hasText(state)
+ ? clientProperties.getPaths().getSocialLinkFailure()
+ : clientProperties.getPaths().getSocialLoginFailure();
+ return ResponseEntity.status(HttpStatus.FOUND)
+ .location(URI.create(clientProperties.getUrl() + failurePath))
+ .build();
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/flipnote/user/interfaces/http/OAuthController.java` around
lines 45 - 60, The controller currently declares the code RequestParam and the
cookie (HttpConstants.OAUTH_VERIFIER_COOKIE) as required which causes Spring to
return 400 before handleCallback runs; change the signature of handleCallback so
code and codeVerifier are optional (make the `@RequestParam` and `@CookieValue`
non-required / nullable) and add an explicit branch at the top of handleCallback
that detects a missing code or missing codeVerifier and immediately redirects to
your login/link failure URL (reuse the same failure redirect logic used in
handleSocialLogin and handleSocialLink) before deleting the cookie and before
calling handleSocialLogin or handleSocialLink so the existing failure handling
is executed.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (7)
src/main/java/flipnote/user/infrastructure/oauth/OAuthApiClient.java (1)
71-76: 현재 Google만 지원됩니다 - 확장 시 주의 필요.
OAuthProperties에 다른 provider(예: kakao, naver)가 설정되어 있으면 런타임에INVALID_OAUTH_PROVIDER예외가 발생합니다. 의도된 설계라면 괜찮지만, 새 provider 추가 시 이 switch문도 함께 수정해야 함을 인지하고 계시면 됩니다.확장성을 위한 대안 (선택사항)
Provider별
OAuth2UserInfo구현체를 자동으로 찾는 전략 패턴 또는 팩토리 패턴을 고려할 수 있습니다:private final Map<String, Function<Map<String, Object>, OAuth2UserInfo>> userInfoFactories = Map.of( "google", GoogleUserInfo::new // 새 provider 추가 시 여기에 등록 ); public OAuth2UserInfo createUserInfo(String providerName, Map<String, Object> attributes) { var factory = userInfoFactories.get(providerName.toLowerCase()); if (factory == null) { throw new BizException(AuthErrorCode.INVALID_OAUTH_PROVIDER); } return factory.apply(attributes); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/flipnote/user/infrastructure/oauth/OAuthApiClient.java` around lines 71 - 76, The createUserInfo method in OAuthApiClient currently only handles "google" and will throw BizException for any other provider configured in OAuthProperties; replace the hardcoded switch with a lookup-based factory or strategy so new providers can be added without changing code. Implement a Map<String, Function<Map<String,Object>,OAuth2UserInfo>> (or similar registry) keyed by providerName.toLowerCase() and use it in createUserInfo to find and invoke the appropriate constructor (e.g., GoogleUserInfo::new); if no factory exists, still throw BizException(AuthErrorCode.INVALID_OAUTH_PROVIDER). This change centralizes provider registration and makes adding providers safer and extensible.src/main/java/flipnote/user/application/result/SocialLinkResult.java (1)
9-19: 불변 DTO에는record를 고려해 보세요.
UserRegisterResult와UserResult는record로 구현되어 있는 반면, 이 클래스는 Lombok을 사용하고 있습니다. 일관성과 불변성을 위해record로 변경하는 것을 고려해 보세요.♻️ record로 변환하는 예시
-import lombok.AllArgsConstructor; -import lombok.Getter; - -import java.time.LocalDateTime; - -@Getter -@AllArgsConstructor -public class SocialLinkResult { - - private Long socialLinkId; - private String provider; - private LocalDateTime linkedAt; +import java.time.LocalDateTime; + +public record SocialLinkResult(Long socialLinkId, String provider, LocalDateTime linkedAt) { public static SocialLinkResult from(OAuthLink link) { return new SocialLinkResult(link.getId(), link.getProvider(), link.getLinkedAt()); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/flipnote/user/application/result/SocialLinkResult.java` around lines 9 - 19, SocialLinkResult is an immutable DTO implemented with Lombok but should be converted to a Java record for consistency with UserRegisterResult/UserResult: replace the `@Getter/`@AllArgsConstructor class with a record declaration SocialLinkResult(Long socialLinkId, String provider, LocalDateTime linkedAt), remove Lombok annotations and fields, and update the static factory method from(OAuthLink link) to return new SocialLinkResult(link.getId(), link.getProvider(), link.getLinkedAt()); ensure imports (LocalDateTime, OAuthLink) remain correct.src/main/java/flipnote/user/application/result/SocialLinksResult.java (1)
9-18:SocialLinkResult와 동일하게record로 변환을 고려해 보세요.
SocialLinkResult와 함께record로 변환하면 다른 result 타입들과 일관성을 유지할 수 있습니다.♻️ record로 변환하는 예시
-import lombok.AllArgsConstructor; -import lombok.Getter; - -import java.util.List; - -@Getter -@AllArgsConstructor -public class SocialLinksResult { - - private List<SocialLinkResult> socialLinks; +import java.util.List; + +public record SocialLinksResult(List<SocialLinkResult> socialLinks) { public static SocialLinksResult from(List<OAuthLink> links) { return new SocialLinksResult(links.stream().map(SocialLinkResult::from).toList()); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/flipnote/user/application/result/SocialLinksResult.java` around lines 9 - 18, Convert the SocialLinksResult class to a record for consistency with SocialLinkResult: remove Lombok annotations and replace the class declaration with a record declaration SocialLinksResult(List<SocialLinkResult> socialLinks), keep or add the static factory method from(List<OAuthLink> links) that returns new SocialLinksResult(links.stream().map(SocialLinkResult::from).toList()), and ensure the field name socialLinks and the from method signature remain unchanged so callers keep working.src/main/java/flipnote/user/application/result/UserInfoResult.java (1)
7-18:imageRefId가 항상null로 설정됩니다.
from(User user)메서드에서imageRefId가 항상null로 설정되어 있습니다. 의도된 동작인지 확인이 필요합니다. 향후 구현 예정이라면 TODO 주석을 추가하는 것을 권장합니다.또한, 다른 result 타입(
UserResult,UserRegisterResult)과의 일관성을 위해record로 변환을 고려해 보세요.♻️ record로 변환하는 예시
-import lombok.AllArgsConstructor; -import lombok.Getter; - -@Getter -@AllArgsConstructor -public class UserInfoResult { - - private Long userId; - private String nickname; - private String profileImageUrl; - private Long imageRefId; +public record UserInfoResult(Long userId, String nickname, String profileImageUrl, Long imageRefId) { + // TODO: imageRefId 매핑 로직 추가 필요 public static UserInfoResult from(User user) { return new UserInfoResult(user.getId(), user.getNickname(), user.getProfileImageUrl(), null); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/flipnote/user/application/result/UserInfoResult.java` around lines 7 - 18, The from(User user) factory sets imageRefId to null causing missing image reference; update UserInfoResult.from to populate imageRefId from the User domain (e.g., use user.getImageRefId() or the correct getter) or, if this is intentionally unimplemented, add a TODO comment clarifying deferred implementation; while here also consider converting UserInfoResult to a record to match UserResult and UserRegisterResult for consistency.src/main/java/flipnote/user/application/UserService.java (3)
78-81: gRPC 예외 처리 시 원본 예외 정보 보존 권장모든 예외를
Exception으로 잡아서BizException으로 변환하면 디버깅에 필요한 정보가 손실될 수 있습니다. 로그에는 예외가 기록되지만, gRPC 관련 특정 예외(예:StatusRuntimeException)를 구분하여 처리하면 더 나은 에러 핸들링이 가능합니다.♻️ 예외 처리 개선 제안
- } catch (Exception ex) { - log.error("updateProfile", ex); + } catch (io.grpc.StatusRuntimeException ex) { + log.error("updateProfile - gRPC error: {}", ex.getStatus(), ex); + throw new BizException(ImageErrorCode.IMAGE_SERVICE_ERROR); + } catch (Exception ex) { + log.error("updateProfile - unexpected error", ex); throw new BizException(ImageErrorCode.IMAGE_SERVICE_ERROR); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/flipnote/user/application/UserService.java` around lines 78 - 81, The updateProfile catch block currently catches Exception and throws a new BizException losing gRPC-specific info; update the handling in UserService.updateProfile to catch gRPC-specific exceptions (e.g., io.grpc.StatusRuntimeException) first, log them with full details, and either rethrow a BizException that wraps the original exception as the cause or include the original exception message/code in the BizException (using ImageErrorCode.IMAGE_SERVICE_ERROR as appropriate); keep a fallback catch for generic Exception to log and wrap any other errors.
48-52: 중복 코드:findActiveUser()헬퍼 메서드 사용 권장
getUserInfo()메서드가findActiveUser()프라이빗 헬퍼와 동일한 로직을 중복하고 있습니다. 일관성과 유지보수를 위해 헬퍼 메서드를 사용하는 것이 좋습니다.♻️ 리팩토링 제안
public UserInfoResult getUserInfo(Long userId) { - User user = userRepository.findByIdAndStatus(userId, User.Status.ACTIVE) - .orElseThrow(() -> new BizException(UserErrorCode.USER_NOT_FOUND)); + User user = findActiveUser(userId); return UserInfoResult.from(user); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/flipnote/user/application/UserService.java` around lines 48 - 52, getUserInfo currently duplicates the logic of the private helper findActiveUser; replace the inline lookup in getUserInfo by calling findActiveUser(userId) to fetch the User and then convert it with UserInfoResult.from(user) so you keep a single source of truth for user retrieval (remove the direct userRepository.findByIdAndStatus(...) usage from getUserInfo and use the existing helper instead).
54-86: 트랜잭션 내 블로킹 gRPC 호출 주의
updateProfile()메서드가@Transactional내에서 블로킹 gRPC 호출을 수행합니다. 이미지 서비스의 응답이 지연되면 DB 커넥션이 오래 점유될 수 있습니다.현재 구조에서는 이미지 URL 업데이트가 같은 트랜잭션에 포함되어야 하므로 당장 변경이 어려울 수 있지만, 향후 이미지 서비스 호출을 트랜잭션 외부로 분리하거나 타임아웃을 설정하는 것을 고려해볼 수 있습니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/flipnote/user/application/UserService.java` around lines 54 - 86, The updateProfile method performs blocking gRPC calls to imageCommandServiceStub (activateImage / changeImage using ActivateImageRequest / ChangeImageRequest) while annotated `@Transactional`, risking long DB connection holds; fix by removing/blocking work from the transaction: extract the image activation/change logic out of updateProfile transaction (call imageCommandServiceStub.activateImage/changeImage before starting the DB transaction or after committing and apply a compensating update), or ensure the gRPC calls have bounded timeouts by configuring deadlines on the stub calls (e.g., set a per-call deadline or call options) and handle timeout exceptions, and then update user.updateProfile with the resolved profileImageUrl accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/java/flipnote/user/application/AuthService.java`:
- Around line 159-167: The current sendEmailVerificationCode method can race
between emailVerificationRepository.hasCode(email) and saveCode(email, code);
change logic to perform an atomic "save-if-absent" in the
EmailVerificationRepository (e.g., add saveCodeIfAbsent(email, code) that uses
Redis SETNX or equivalent) and have sendEmailVerificationCode call
verificationCodeGenerator.generate() then if
(!emailVerificationRepository.saveCodeIfAbsent(email, code)) throw new
BizException(AuthErrorCode.ALREADY_ISSUED_VERIFICATION_CODE); otherwise publish
the EmailVerificationSendEvent via eventPublisher; update repository
implementation and tests accordingly.
- Around line 169-181: The current verifyEmail method in AuthService uses
emailVerificationRepository.hasCode(email) then getCode(email) leading to a
TOCTOU race; change to a single atomic repository operation (e.g.,
emailVerificationRepository.verifyAndConsumeCode(email, code) or
getAndDeleteCodeIfMatches(email, code)) that returns success/failure (or throws)
and performs any deletion/consumption in one transaction, then call
emailVerificationRepository.markVerified(email) only after the atomic success;
replace uses of hasCode/getCode/deleteCode with this new atomic method inside
verifyEmail to eliminate the race.
In `@src/main/java/flipnote/user/infrastructure/oauth/OAuthApiClient.java`:
- Around line 66-68: 예외 발생 시 원인 정보가 손실되고 있으므로 OAuthApiClient 내 해당 catch 블록(같은
문제를 가진 requestAccessToken과 동일한 위치)에 있는 throw new
BizException(AuthErrorCode.OAUTH_COMMUNICATION_ERROR) 대신 원본 예외를 BizException에
전달하도록 수정하세요; 즉 BizException의 적절한 생성자(또는 initCause)를 사용해 e를 원인으로 포함시키고 로그/상위 호출에서
root cause를 확인할 수 있게 만드세요.
- Around line 48-53: Preserve the original exception and add null-safety for the
access token in OAuthApiClient: when catching exceptions around
objectMapper.readValue and extracting "access_token", include the original
exception as the cause (or log it) instead of swallowing it, and validate the
extracted value from responseMap (e.g., check String accessToken = (String)
responseMap.get("access_token"); if accessToken == null throw new
BizException(AuthErrorCode.OAUTH_COMMUNICATION_ERROR) ) before returning; update
the catch block to wrap the caught Exception into BizException (or log e) so
debugging retains the root cause.
---
Nitpick comments:
In `@src/main/java/flipnote/user/application/result/SocialLinkResult.java`:
- Around line 9-19: SocialLinkResult is an immutable DTO implemented with Lombok
but should be converted to a Java record for consistency with
UserRegisterResult/UserResult: replace the `@Getter/`@AllArgsConstructor class
with a record declaration SocialLinkResult(Long socialLinkId, String provider,
LocalDateTime linkedAt), remove Lombok annotations and fields, and update the
static factory method from(OAuthLink link) to return new
SocialLinkResult(link.getId(), link.getProvider(), link.getLinkedAt()); ensure
imports (LocalDateTime, OAuthLink) remain correct.
In `@src/main/java/flipnote/user/application/result/SocialLinksResult.java`:
- Around line 9-18: Convert the SocialLinksResult class to a record for
consistency with SocialLinkResult: remove Lombok annotations and replace the
class declaration with a record declaration
SocialLinksResult(List<SocialLinkResult> socialLinks), keep or add the static
factory method from(List<OAuthLink> links) that returns new
SocialLinksResult(links.stream().map(SocialLinkResult::from).toList()), and
ensure the field name socialLinks and the from method signature remain unchanged
so callers keep working.
In `@src/main/java/flipnote/user/application/result/UserInfoResult.java`:
- Around line 7-18: The from(User user) factory sets imageRefId to null causing
missing image reference; update UserInfoResult.from to populate imageRefId from
the User domain (e.g., use user.getImageRefId() or the correct getter) or, if
this is intentionally unimplemented, add a TODO comment clarifying deferred
implementation; while here also consider converting UserInfoResult to a record
to match UserResult and UserRegisterResult for consistency.
In `@src/main/java/flipnote/user/application/UserService.java`:
- Around line 78-81: The updateProfile catch block currently catches Exception
and throws a new BizException losing gRPC-specific info; update the handling in
UserService.updateProfile to catch gRPC-specific exceptions (e.g.,
io.grpc.StatusRuntimeException) first, log them with full details, and either
rethrow a BizException that wraps the original exception as the cause or include
the original exception message/code in the BizException (using
ImageErrorCode.IMAGE_SERVICE_ERROR as appropriate); keep a fallback catch for
generic Exception to log and wrap any other errors.
- Around line 48-52: getUserInfo currently duplicates the logic of the private
helper findActiveUser; replace the inline lookup in getUserInfo by calling
findActiveUser(userId) to fetch the User and then convert it with
UserInfoResult.from(user) so you keep a single source of truth for user
retrieval (remove the direct userRepository.findByIdAndStatus(...) usage from
getUserInfo and use the existing helper instead).
- Around line 54-86: The updateProfile method performs blocking gRPC calls to
imageCommandServiceStub (activateImage / changeImage using ActivateImageRequest
/ ChangeImageRequest) while annotated `@Transactional`, risking long DB connection
holds; fix by removing/blocking work from the transaction: extract the image
activation/change logic out of updateProfile transaction (call
imageCommandServiceStub.activateImage/changeImage before starting the DB
transaction or after committing and apply a compensating update), or ensure the
gRPC calls have bounded timeouts by configuring deadlines on the stub calls
(e.g., set a per-call deadline or call options) and handle timeout exceptions,
and then update user.updateProfile with the resolved profileImageUrl
accordingly.
In `@src/main/java/flipnote/user/infrastructure/oauth/OAuthApiClient.java`:
- Around line 71-76: The createUserInfo method in OAuthApiClient currently only
handles "google" and will throw BizException for any other provider configured
in OAuthProperties; replace the hardcoded switch with a lookup-based factory or
strategy so new providers can be added without changing code. Implement a
Map<String, Function<Map<String,Object>,OAuth2UserInfo>> (or similar registry)
keyed by providerName.toLowerCase() and use it in createUserInfo to find and
invoke the appropriate constructor (e.g., GoogleUserInfo::new); if no factory
exists, still throw BizException(AuthErrorCode.INVALID_OAUTH_PROVIDER). This
change centralizes provider registration and makes adding providers safer and
extensible.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6b833084-e96a-4174-b5be-3eb19279b4ee
📒 Files selected for processing (37)
build.gradle.ktssrc/main/java/flipnote/user/application/AuthService.javasrc/main/java/flipnote/user/application/OAuthService.javasrc/main/java/flipnote/user/application/UserService.javasrc/main/java/flipnote/user/application/command/ChangePasswordCommand.javasrc/main/java/flipnote/user/application/command/LoginCommand.javasrc/main/java/flipnote/user/application/command/SignupCommand.javasrc/main/java/flipnote/user/application/command/UpdateProfileCommand.javasrc/main/java/flipnote/user/application/result/MyInfoResult.javasrc/main/java/flipnote/user/application/result/SocialLinkResult.javasrc/main/java/flipnote/user/application/result/SocialLinksResult.javasrc/main/java/flipnote/user/application/result/TokenValidateResult.javasrc/main/java/flipnote/user/application/result/UserInfoResult.javasrc/main/java/flipnote/user/application/result/UserRegisterResult.javasrc/main/java/flipnote/user/application/result/UserResult.javasrc/main/java/flipnote/user/application/result/UserUpdateResult.javasrc/main/java/flipnote/user/domain/AuthErrorCode.javasrc/main/java/flipnote/user/domain/VerificationConstants.javasrc/main/java/flipnote/user/domain/repository/OAuthLinkRepository.javasrc/main/java/flipnote/user/infrastructure/jwt/JwtProvider.javasrc/main/java/flipnote/user/infrastructure/mail/PasswordResetTokenGenerator.javasrc/main/java/flipnote/user/infrastructure/mail/VerificationCodeGenerator.javasrc/main/java/flipnote/user/infrastructure/oauth/OAuthApiClient.javasrc/main/java/flipnote/user/infrastructure/oauth/OAuthProperties.javasrc/main/java/flipnote/user/infrastructure/redis/EmailVerificationRepository.javasrc/main/java/flipnote/user/infrastructure/redis/PasswordResetRepository.javasrc/main/java/flipnote/user/interfaces/grpc/GrpcExceptionHandlerImpl.javasrc/main/java/flipnote/user/interfaces/grpc/GrpcUserQueryService.javasrc/main/java/flipnote/user/interfaces/http/AuthController.javasrc/main/java/flipnote/user/interfaces/http/OAuthController.javasrc/main/java/flipnote/user/interfaces/http/UserController.javasrc/main/java/flipnote/user/interfaces/http/dto/request/ChangePasswordRequest.javasrc/main/java/flipnote/user/interfaces/http/dto/request/LoginRequest.javasrc/main/java/flipnote/user/interfaces/http/dto/request/SignupRequest.javasrc/main/java/flipnote/user/interfaces/http/dto/request/UpdateProfileRequest.javasrc/main/java/flipnote/user/interfaces/http/dto/response/.gitkeepsrc/main/resources/application.yml
💤 Files with no reviewable changes (1)
- build.gradle.kts
✅ Files skipped from review due to trivial changes (7)
- src/main/java/flipnote/user/application/command/LoginCommand.java
- src/main/resources/application.yml
- src/main/java/flipnote/user/infrastructure/oauth/OAuthProperties.java
- src/main/java/flipnote/user/application/command/ChangePasswordCommand.java
- src/main/java/flipnote/user/application/result/TokenValidateResult.java
- src/main/java/flipnote/user/application/command/SignupCommand.java
- src/main/java/flipnote/user/application/command/UpdateProfileCommand.java
🚧 Files skipped from review as they are similar to previous changes (11)
- src/main/java/flipnote/user/infrastructure/jwt/JwtProvider.java
- src/main/java/flipnote/user/interfaces/http/dto/request/ChangePasswordRequest.java
- src/main/java/flipnote/user/infrastructure/redis/EmailVerificationRepository.java
- src/main/java/flipnote/user/interfaces/http/dto/request/UpdateProfileRequest.java
- src/main/java/flipnote/user/infrastructure/redis/PasswordResetRepository.java
- src/main/java/flipnote/user/interfaces/http/dto/request/LoginRequest.java
- src/main/java/flipnote/user/domain/repository/OAuthLinkRepository.java
- src/main/java/flipnote/user/domain/AuthErrorCode.java
- src/main/java/flipnote/user/interfaces/http/dto/request/SignupRequest.java
- src/main/java/flipnote/user/interfaces/http/OAuthController.java
- src/main/java/flipnote/user/application/OAuthService.java
| public void sendEmailVerificationCode(String email) { | ||
| if (emailVerificationRepository.hasCode(email)) { | ||
| throw new BizException(AuthErrorCode.ALREADY_ISSUED_VERIFICATION_CODE); | ||
| } | ||
|
|
||
| String code = verificationCodeGenerator.generate(); | ||
| emailVerificationRepository.saveCode(email, code); | ||
| eventPublisher.publishEvent(new EmailVerificationSendEvent(email, code)); | ||
| } |
There was a problem hiding this comment.
이메일 인증 코드 발급 시 경쟁 상태(Race Condition) 가능성
hasCode() 확인 후 saveCode() 호출 사이에 동시 요청이 들어올 경우, 두 요청 모두 코드 발급에 성공할 수 있습니다. Redis의 원자적 연산(예: SETNX)을 사용하여 중복 발급을 방지하는 것이 더 안전합니다.
🛡️ 원자적 연산 사용 권장
EmailVerificationRepository에서 saveCodeIfAbsent() 같은 원자적 메서드를 제공하고, 실패 시 예외를 던지는 방식으로 개선할 수 있습니다:
public void sendEmailVerificationCode(String email) {
String code = verificationCodeGenerator.generate();
if (!emailVerificationRepository.saveCodeIfAbsent(email, code)) {
throw new BizException(AuthErrorCode.ALREADY_ISSUED_VERIFICATION_CODE);
}
eventPublisher.publishEvent(new EmailVerificationSendEvent(email, code));
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/flipnote/user/application/AuthService.java` around lines 159 -
167, The current sendEmailVerificationCode method can race between
emailVerificationRepository.hasCode(email) and saveCode(email, code); change
logic to perform an atomic "save-if-absent" in the EmailVerificationRepository
(e.g., add saveCodeIfAbsent(email, code) that uses Redis SETNX or equivalent)
and have sendEmailVerificationCode call verificationCodeGenerator.generate()
then if (!emailVerificationRepository.saveCodeIfAbsent(email, code)) throw new
BizException(AuthErrorCode.ALREADY_ISSUED_VERIFICATION_CODE); otherwise publish
the EmailVerificationSendEvent via eventPublisher; update repository
implementation and tests accordingly.
| public void verifyEmail(String email, String code) { | ||
| if (!emailVerificationRepository.hasCode(email)) { | ||
| throw new BizException(AuthErrorCode.NOT_ISSUED_VERIFICATION_CODE); | ||
| } | ||
|
|
||
| String savedCode = emailVerificationRepository.getCode(email); | ||
| if (!code.equals(savedCode)) { | ||
| throw new BizException(AuthErrorCode.INVALID_VERIFICATION_CODE); | ||
| } | ||
|
|
||
| emailVerificationRepository.deleteCode(email); | ||
| emailVerificationRepository.markVerified(email); | ||
| } |
There was a problem hiding this comment.
이메일 인증 검증 시 TOCTOU(Time-of-Check to Time-of-Use) 취약점
hasCode() 확인과 getCode() 호출 사이에 코드가 만료되거나 삭제될 수 있습니다. 단일 원자적 연산으로 코드를 조회하고 검증하는 것이 더 안전합니다.
🛡️ 원자적 조회 및 검증 제안
public void verifyEmail(String email, String code) {
- if (!emailVerificationRepository.hasCode(email)) {
- throw new BizException(AuthErrorCode.NOT_ISSUED_VERIFICATION_CODE);
- }
-
- String savedCode = emailVerificationRepository.getCode(email);
+ String savedCode = emailVerificationRepository.getCode(email);
+ if (savedCode == null) {
+ throw new BizException(AuthErrorCode.NOT_ISSUED_VERIFICATION_CODE);
+ }
+
if (!code.equals(savedCode)) {
throw new BizException(AuthErrorCode.INVALID_VERIFICATION_CODE);
}
emailVerificationRepository.deleteCode(email);
emailVerificationRepository.markVerified(email);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/flipnote/user/application/AuthService.java` around lines 169 -
181, The current verifyEmail method in AuthService uses
emailVerificationRepository.hasCode(email) then getCode(email) leading to a
TOCTOU race; change to a single atomic repository operation (e.g.,
emailVerificationRepository.verifyAndConsumeCode(email, code) or
getAndDeleteCodeIfMatches(email, code)) that returns success/failure (or throws)
and performs any deletion/consumption in one transaction, then call
emailVerificationRepository.markVerified(email) only after the atomic success;
replace uses of hasCode/getCode/deleteCode with this new atomic method inside
verifyEmail to eliminate the race.
| Map<String, Object> responseMap = objectMapper.readValue(responseBody, new TypeReference<>() { | ||
| }); | ||
| return (String)responseMap.get("access_token"); | ||
| } catch (Exception e) { | ||
| throw new BizException(AuthErrorCode.OAUTH_COMMUNICATION_ERROR); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
ast-grep --pattern $'class BizException {
$$$
}'Repository: FlipNoteTeam/FlipNote-User
Length of output: 52
🏁 Script executed:
#!/bin/bash
# Search for BizException class definition
rg "class BizException" -A 10 --type javaRepository: FlipNoteTeam/FlipNote-User
Length of output: 395
🏁 Script executed:
#!/bin/bash
# Also search for files containing BizException
fd -e java -x rg -l "class BizException" {} \;Repository: FlipNoteTeam/FlipNote-User
Length of output: 131
🏁 Script executed:
#!/bin/bash
# Get the full context of the OAuthApiClient requestAccessToken method
cat -n src/main/java/flipnote/user/infrastructure/oauth/OAuthApiClient.java | head -70Repository: FlipNoteTeam/FlipNote-User
Length of output: 2811
🏁 Script executed:
#!/bin/bash
# Read the complete BizException class file
cat -n src/main/java/flipnote/user/domain/common/BizException.javaRepository: FlipNoteTeam/FlipNote-User
Length of output: 408
예외 처리 시 원인 정보가 손실되고 null 안전성 문제가 있습니다.
catch (Exception e)에서 원본 예외가 버려지므로 디버깅이 어렵습니다. 또한 access_token이 응답에 없을 경우 null이 반환되어 호출부에서 NPE가 발생할 수 있습니다.
수정 제안
원본 예외 정보를 보존하려면 BizException 설계를 개선하거나, 로깅을 추가하세요:
} catch (Exception e) {
// 원본 예외 정보를 로그에 남기고 BizException 발생
// 또는 BizException이 cause를 지원하도록 수정
throw new BizException(AuthErrorCode.OAUTH_COMMUNICATION_ERROR);
}access_token null 체크는 필수입니다:
String accessToken = (String) responseMap.get("access_token");
if (accessToken == null) {
throw new BizException(AuthErrorCode.OAUTH_COMMUNICATION_ERROR);
}
return accessToken;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/flipnote/user/infrastructure/oauth/OAuthApiClient.java` around
lines 48 - 53, Preserve the original exception and add null-safety for the
access token in OAuthApiClient: when catching exceptions around
objectMapper.readValue and extracting "access_token", include the original
exception as the cause (or log it) instead of swallowing it, and validate the
extracted value from responseMap (e.g., check String accessToken = (String)
responseMap.get("access_token"); if accessToken == null throw new
BizException(AuthErrorCode.OAUTH_COMMUNICATION_ERROR) ) before returning; update
the catch block to wrap the caught Exception into BizException (or log e) so
debugging retains the root cause.
| } catch (Exception e) { | ||
| throw new BizException(AuthErrorCode.OAUTH_COMMUNICATION_ERROR); | ||
| } |
There was a problem hiding this comment.
동일하게 예외 원인 정보가 손실됩니다.
requestAccessToken과 동일한 문제입니다. 예외의 root cause를 BizException에 전달하는 것을 권장합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/flipnote/user/infrastructure/oauth/OAuthApiClient.java` around
lines 66 - 68, 예외 발생 시 원인 정보가 손실되고 있으므로 OAuthApiClient 내 해당 catch 블록(같은 문제를 가진
requestAccessToken과 동일한 위치)에 있는 throw new
BizException(AuthErrorCode.OAUTH_COMMUNICATION_ERROR) 대신 원본 예외를 BizException에
전달하도록 수정하세요; 즉 BizException의 적절한 생성자(또는 initCause)를 사용해 e를 원인으로 포함시키고 로그/상위 호출에서
root cause를 확인할 수 있게 만드세요.
Summary by CodeRabbit
New Features
개선 사항
Documentation
Chore