Skip to content

Conversation

@GulSauce
Copy link
Member

@GulSauce GulSauce commented Jan 23, 2026

📢 설명

해당 Pull Request에 대해 간략하게 설명해주세요!

1. problem_set의 title 제거

2. token rotate는 프론트가 명시적 호출할때만 수행하게 변경

기존에는 로그인하면 알아서 token rotate가 수행되었는데, 프론트엔드가 명시적으로 호출하는 경우에만 일어나게한다. 암묵적으로 token rotate가 수행되게 하지않는다

액세스토큰 응답을 헤더 응답에서 JSON응답으로

요청-응답 구조에서는 JSON응답이 명확한 방식이므로 그렇게 변경하였다.

ex)
POST /auth/refresh로 리프레시 토큰 포함하여 요청

응답

{
    "accessToken": "eyJhbGciOiJIUzUxMiIsInR5c..."
}

3. 리프레시 토큰 저장소를 Redis -> MySQL로

@Entity
@Getter
@NoArgsConstructor
public class RefreshToken extends CreatedAt {

    // 사용자 ID
    @Id
    private String userId;
    // 리프레시 토큰 값
    private String rtHash;
    // 만료 시간
    private Instant expiresAt;

4. userId 리졸버 추가

  @PostMapping
    @Override
    public ResponseEntity<GenerationResponse> postProblemSetId(
        @UserId
        String userId,
        @Valid @RequestBody FeGenerationRequest feGenerationRequest) {
        return ResponseEntity.ok(
            generationService.processGenerationRequest(feGenerationRequest, userId));
    }

와 같이 어노테이션을 붙이면 스프링 시큐리타가 만들어준 principal에서 userId를 추출하여 반환한다

코드 설명: 코멘트 확인

✅ 체크 리스트 | 프론트엔드 develop 브랜치로 테스트 진행!!

  • 프론트엔드 develop 브랜치 pull & npm install & npm run dev
  • 로그인 수행
  • 퀴즈 생성 - problem_set 테이블에 userId값이 들어갔는지 확인
  • 로그아웃 수행
  • 퀴즈 생성 - problem_set 테이블에 null값이 들어갔는지 확인
  • 코드 리뷰

Summary by CodeRabbit

릴리스 노트

  • 새 기능

    • OAuth2 기반 소셜 로그인 지원 추가 (Google, Kakao)
    • 향상된 토큰 로테이션 메커니즘 구현
    • 사용자 ID 기반 요청 처리 기능 추가
  • 개선 사항

    • 인증 및 보안 설정 최적화
    • 토큰 저장소 개선으로 안정성 증대
    • 오류 메시지 명확화
  • 제거됨

    • 기존 일반 회원가입/로그인 엔드포인트

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Jan 23, 2026

Walkthrough

OAuth2 인증을 지원하도록 인증 모듈을 리팩터링했습니다. Redis 기반 토큰 저장소를 JPA 데이터베이스로 전환하고, 일반 로그인/회원가입 기능을 제거했으며, 전역 설정 모듈을 추가했습니다.

Changes

코호트 / 파일(s) 변경 요약
의존성 및 SQL 마이그레이션
app/build.gradle, app/src/main/resources/db/refresh_token.sql
글로벌 모듈 의존성 추가 및 RefreshToken 테이블 생성 마이그레이션 추가
인증 API 삭제
modules/auth/api/src/main/java/com/icc/qasker/auth/NormalJoinService.java, modules/auth/api/src/main/java/com/icc/qasker/auth/NormalLoginService.java
일반 회원가입/로그인 서비스 인터페이스 제거
토큰 관련 API 변경
modules/auth/api/src/main/java/com/icc/qasker/auth/TokenRotationService.java, modules/auth/api/src/main/java/com/icc/qasker/auth/dto/response/RotateTokenResponse.java
TokenRotationService 반환 타입을 void/String에서 RotateTokenResponse로 변경, RotateTokenResponse DTO 신규 추가
모듈 빌드 설정 변경
modules/auth/impl/build.gradle
Redis 의존성 제거 및 주석 변경
Redis 기반 설정 클래스 삭제
modules/auth/impl/src/main/java/com/icc/qasker/auth/config/RedisConfig.java, modules/auth/impl/src/main/java/com/icc/qasker/auth/security/PasswordEncoderConfig.java, modules/auth/impl/src/main/java/com/icc/qasker/auth/properties/RedisProperties.java
RedisConfig, PasswordEncoderConfig, RedisProperties 클래스 완전 삭제
OAuth2 보안 설정
modules/auth/impl/src/main/java/com/icc/qasker/auth/config/SecurityConfig.java
기존 보안 설정 클래스를 config 패키지로 이전하여 OAuth2 로그인, JWT 필터, 권한 관리 기능 추가
OAuth2 관련 신규 클래스
modules/auth/impl/src/main/java/com/icc/qasker/auth/config/security/handler/OAuth2LoginSuccessHandler.java, modules/auth/impl/src/main/java/com/icc/qasker/auth/config/security/service/PrincipalOAuth2UserService.java, modules/auth/impl/src/main/java/com/icc/qasker/auth/principal/UserPrincipal.java
OAuth2 로그인 핸들러, OAuth2 사용자 서비스, UserPrincipal 구현체 신규 추가
OAuth2 제공자 정보
modules/auth/impl/src/main/java/com/icc/qasker/auth/config/security/provider/GoogleUserInfo.java, modules/auth/impl/src/main/java/com/icc/qasker/auth/config/security/provider/KakaoUserInfo.java, modules/auth/impl/src/main/java/com/icc/qasker/auth/config/security/provider/OAuth2UserInfo.java
패키지 경로를 security.provider에서 config.security.provider로 이전
필터 및 JwtProperties 이전
modules/auth/impl/src/main/java/com/icc/qasker/auth/config/security/filter/JwtTokenAuthenticationFilter.java, modules/auth/impl/src/main/java/com/icc/qasker/auth/component/AccessTokenHandler.java
JwtProperties 임포트를 auth.properties에서 global.properties로 변경
RefreshToken 엔티티 재설계
modules/auth/impl/src/main/java/com/icc/qasker/auth/entity/RefreshToken.java, modules/auth/impl/src/main/java/com/icc/qasker/auth/repository/RefreshTokenRepository.java
Redis 기반에서 JPA @Entity로 변환, rotate/isExpired 메서드 추가, JPA 리포지토리 신규 생성
User 엔티티 변경
modules/auth/impl/src/main/java/com/icc/qasker/auth/entity/User.java
password 필드 제거, 생성자를 private에서 public으로 변경
일반 인증 서비스 구현 삭제
modules/auth/impl/src/main/java/com/icc/qasker/auth/service/NormalJoinServiceImpl.java, modules/auth/impl/src/main/java/com/icc/qasker/auth/service/NormalLoginServiceImpl.java
일반 회원가입/로그인 서비스 구현체 완전 삭제
토큰 로테이션 서비스 업데이트
modules/auth/impl/src/main/java/com/icc/qasker/auth/service/TokenRotationServiceImpl.java, modules/auth/impl/src/main/java/com/icc/qasker/auth/util/RefreshTokenUtil.java
issueTokens/rotateTokens 반환 타입을 RotateTokenResponse로 변경, RefreshTokenUtil을 Redis에서 데이터베이스 기반으로 리팩터링
컨트롤러 정리
modules/auth/impl/src/main/java/com/icc/qasker/auth/controller/AuthController.java, modules/auth/impl/src/main/java/com/icc/qasker/auth/controller/TestController.java
NormalJoinService/NormalLoginService 의존성 제거, /test 엔드포인트 추가, refresh 응답 타입 변경, TestController 삭제
기존 필터 및 Principal 삭제
modules/auth/impl/src/main/java/com/icc/qasker/auth/security/filter/RefreshRotationFilter.java, modules/auth/impl/src/main/java/com/icc/qasker/auth/security/principal/PrincipalDetails.java, modules/auth/impl/src/main/java/com/icc/qasker/auth/security/SecurityConfig.java
RefreshRotationFilter, PrincipalDetails, 기존 SecurityConfig 완전 삭제
WebMvcConfig 이전 및 UserIdArgumentResolver
modules/auth/impl/src/main/java/com/icc/qasker/auth/config/WebMvcConfig.java, modules/auth/impl/src/main/java/com/icc/qasker/auth/resolver/UserIdArgumentResolver.java
WebMvcConfig를 global.config에서 auth.config로 이전, UserIdArgumentResolver 신규 추가
글로벌 모듈 설정 변경
modules/global/src/main/java/com/icc/qasker/global/annotation/UserId.java, modules/global/src/main/java/com/icc/qasker/global/properties/JwtProperties.java, modules/global/src/main/java/com/icc/qasker/global/properties/HashIdProperties.java, modules/global/src/main/java/com/icc/qasker/global/properties/QAskerProperties.java, modules/global/src/main/java/com/icc/qasker/global/properties/SlackProperties.java
@UserId 어노테이션 신규 추가, JwtProperties를 auth에서 global로 이전, HashProperties를 HashIdProperties로 이름 변경, 설정 프로퍼티 바인딩 경로 일원화
기타 글로벌 모듈 변경
modules/global/src/main/java/com/icc/qasker/global/config/HashIdConfig.java, modules/global/src/main/java/com/icc/qasker/global/error/ExceptionMessage.java
HashIdProperties 참조로 업데이트, TOKEN_GENERATION_FAILED 메시지 변경
Quiz 모듈 userId 추가
modules/quiz/api/src/main/java/com/icc/qasker/quiz/GenerationService.java, modules/quiz/impl/src/main/java/com/icc/qasker/quiz/controller/GenerationController.java, modules/quiz/impl/src/main/java/com/icc/qasker/quiz/service/GenerationServiceImpl.java, modules/quiz/impl/src/main/java/com/icc/qasker/quiz/entity/ProblemSet.java
GenerationService API에 userId 파라미터 추가, ProblemSet에 userId 필드 추가 및 팩토리 메서드 오버로드
문서 및 기타
modules/quiz/impl/src/main/java/com/icc/qasker/quiz/controller/doc/GenerationApiDoc.java
GenerationApiDoc에 userId 파라미터 추가

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant SecurityFilter
    participant OAuth2Service
    participant UserRepository
    participant TokenService
    participant Database
    participant Frontend

    Client->>SecurityFilter: OAuth2 로그인 요청
    SecurityFilter->>OAuth2Service: OAuth2 인증 처리
    OAuth2Service->>OAuth2Service: Google/Kakao로부터 사용자 정보 조회
    OAuth2Service->>UserRepository: 사용자 존재 여부 확인
    alt 사용자 없음
        UserRepository-->>OAuth2Service: 없음
        OAuth2Service->>UserRepository: 새 User 엔티티 생성 및 저장
        UserRepository->>Database: INSERT User
    else 사용자 있음
        UserRepository-->>OAuth2Service: User 반환
    end
    
    OAuth2Service-->>SecurityFilter: UserPrincipal 반환
    SecurityFilter->>OAuth2Service: OAuth2LoginSuccessHandler 실행
    OAuth2Service->>TokenService: issueTokens(userId) 호출
    TokenService->>Database: RefreshToken 생성 및 저장
    Database-->>TokenService: RotateTokenResponse 반환
    TokenService-->>OAuth2Service: accessToken 포함 응답
    OAuth2Service->>Frontend: frontendDeployUrl로 리다이렉트
Loading
sequenceDiagram
    participant Client
    participant AuthController
    participant UserIdResolver
    participant SecurityContext
    participant TokenService
    participant RefreshTokenRepository
    participant Database
    participant Client2

    Client->>AuthController: POST /refresh (refresh_token 쿠키)
    AuthController->>UserIdResolver: `@UserId` 파라미터 해석
    UserIdResolver->>SecurityContext: 현재 인증 정보 조회
    SecurityContext-->>UserIdResolver: Authentication 반환
    UserIdResolver-->>AuthController: userId 주입
    AuthController->>TokenService: rotateTokens(refreshToken)
    TokenService->>RefreshTokenRepository: findByRtHash(rtHash)
    RefreshTokenRepository->>Database: SELECT refresh_token
    Database-->>RefreshTokenRepository: RefreshToken 엔티티 반환
    RefreshTokenRepository-->>TokenService: Optional<RefreshToken> 반환
    
    alt 토큰 만료
        TokenService->>Database: DELETE refresh_token
        TokenService-->>AuthController: UNAUTHORIZED 예외
    else 토큰 유효
        TokenService->>TokenService: 새 accessToken 생성
        TokenService->>Database: UPDATE refresh_token (rotate)
        TokenService-->>AuthController: RotateTokenResponse(newAccessToken)
    end
    
    AuthController-->>Client2: ResponseEntity<RotateTokenResponse>
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Poem

🐰 레디스 대신 데이터베이스 선택했네,
OAuth2 춤을 추며 사용자 맞이하고,
토큰 회전은 우아하게, 타입 안전하게,
글로벌과 모듈의 경계 정했구나,
코드가 더 깔끔해지니 마음도 맑아져!

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed PR 제목은 ICC-210 로그인 기능 구현을 명시하며, 변경사항의 주요 목표인 OAuth2 기반 로그인 기능 추가를 반영합니다.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@GulSauce
Copy link
Member Author

@coderabbitai full review

@coderabbitai
Copy link

coderabbitai bot commented Jan 23, 2026

✅ Actions performed

Full review triggered.

@coderabbitai
Copy link

coderabbitai bot commented Jan 23, 2026

Note

Docstrings generation - SUCCESS
Generated docstrings for this pull request at #123

coderabbitai bot added a commit that referenced this pull request Jan 23, 2026
Docstrings generation was requested by @GulSauce.

* #122 (comment)

The following files were modified:

* `modules/auth/api/src/main/java/com/icc/qasker/auth/TokenRotationService.java`
* `modules/auth/impl/src/main/java/com/icc/qasker/auth/config/SecurityConfig.java`
* `modules/auth/impl/src/main/java/com/icc/qasker/auth/config/WebMvcConfig.java`
* `modules/auth/impl/src/main/java/com/icc/qasker/auth/config/security/handler/OAuth2LoginSuccessHandler.java`
* `modules/auth/impl/src/main/java/com/icc/qasker/auth/config/security/service/PrincipalOAuth2UserService.java`
* `modules/auth/impl/src/main/java/com/icc/qasker/auth/controller/AuthController.java`
* `modules/auth/impl/src/main/java/com/icc/qasker/auth/entity/RefreshToken.java`
* `modules/auth/impl/src/main/java/com/icc/qasker/auth/entity/User.java`
* `modules/auth/impl/src/main/java/com/icc/qasker/auth/principal/UserPrincipal.java`
* `modules/auth/impl/src/main/java/com/icc/qasker/auth/repository/RefreshTokenRepository.java`
* `modules/auth/impl/src/main/java/com/icc/qasker/auth/resolver/UserIdArgumentResolver.java`
* `modules/auth/impl/src/main/java/com/icc/qasker/auth/service/TokenRotationServiceImpl.java`
* `modules/auth/impl/src/main/java/com/icc/qasker/auth/util/RefreshTokenUtil.java`
* `modules/global/src/main/java/com/icc/qasker/global/config/HashIdConfig.java`
* `modules/quiz/api/src/main/java/com/icc/qasker/quiz/GenerationService.java`
* `modules/quiz/impl/src/main/java/com/icc/qasker/quiz/controller/GenerationController.java`
* `modules/quiz/impl/src/main/java/com/icc/qasker/quiz/controller/doc/GenerationApiDoc.java`
* `modules/quiz/impl/src/main/java/com/icc/qasker/quiz/entity/ProblemSet.java`
* `modules/quiz/impl/src/main/java/com/icc/qasker/quiz/service/GenerationServiceImpl.java`
GulSauce pushed a commit that referenced this pull request Jan 23, 2026
Docstrings generation was requested by @GulSauce.

* #122 (comment)

The following files were modified:

* `modules/auth/api/src/main/java/com/icc/qasker/auth/TokenRotationService.java`
* `modules/auth/impl/src/main/java/com/icc/qasker/auth/config/SecurityConfig.java`
* `modules/auth/impl/src/main/java/com/icc/qasker/auth/config/WebMvcConfig.java`
* `modules/auth/impl/src/main/java/com/icc/qasker/auth/config/security/handler/OAuth2LoginSuccessHandler.java`
* `modules/auth/impl/src/main/java/com/icc/qasker/auth/config/security/service/PrincipalOAuth2UserService.java`
* `modules/auth/impl/src/main/java/com/icc/qasker/auth/controller/AuthController.java`
* `modules/auth/impl/src/main/java/com/icc/qasker/auth/entity/RefreshToken.java`
* `modules/auth/impl/src/main/java/com/icc/qasker/auth/entity/User.java`
* `modules/auth/impl/src/main/java/com/icc/qasker/auth/principal/UserPrincipal.java`
* `modules/auth/impl/src/main/java/com/icc/qasker/auth/repository/RefreshTokenRepository.java`
* `modules/auth/impl/src/main/java/com/icc/qasker/auth/resolver/UserIdArgumentResolver.java`
* `modules/auth/impl/src/main/java/com/icc/qasker/auth/service/TokenRotationServiceImpl.java`
* `modules/auth/impl/src/main/java/com/icc/qasker/auth/util/RefreshTokenUtil.java`
* `modules/global/src/main/java/com/icc/qasker/global/config/HashIdConfig.java`
* `modules/quiz/api/src/main/java/com/icc/qasker/quiz/GenerationService.java`
* `modules/quiz/impl/src/main/java/com/icc/qasker/quiz/controller/GenerationController.java`
* `modules/quiz/impl/src/main/java/com/icc/qasker/quiz/controller/doc/GenerationApiDoc.java`
* `modules/quiz/impl/src/main/java/com/icc/qasker/quiz/entity/ProblemSet.java`
* `modules/quiz/impl/src/main/java/com/icc/qasker/quiz/service/GenerationServiceImpl.java`

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 5

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
modules/global/src/main/java/com/icc/qasker/global/properties/JwtProperties.java (1)

10-12: 정적 가변 필드 사용은 동시성 문제와 테스트 어려움을 야기합니다.

SECRET, ACCESS_EXPIRATION_TIME, REFRESH_EXPIRATION_TIME을 정적 가변 필드로 사용하면 다음과 같은 문제가 발생할 수 있습니다:

  • 스레드 안전성: 여러 스레드에서 동시에 setter가 호출되면 race condition 발생 가능
  • 테스트 격리: 정적 상태가 테스트 간에 유지되어 테스트 격리가 어려움
  • DI 원칙 위반: Spring의 의존성 주입 원칙과 맞지 않음

가능하다면 JwtProperties 빈을 직접 주입받아 인스턴스 필드를 사용하는 방식으로 리팩터링을 권장합니다.

modules/auth/impl/src/main/java/com/icc/qasker/auth/util/RefreshTokenUtil.java (1)

30-33: System.out.println 대신 로거 사용 권장

프로덕션 코드에서 System.out.println은 적절하지 않습니다. SLF4J 로거를 사용하여 적절한 로그 레벨로 기록하세요.

♻️ 제안 코드

클래스에 로거 추가:

+import lombok.extern.slf4j.Slf4j;
+
 `@Component`
+@Slf4j
 `@RequiredArgsConstructor`
 public class RefreshTokenUtil {

예외 로깅 수정:

         } catch (Exception e) {
-            System.out.println(e.getMessage());
+            log.error("토큰 발급 실패: {}", e.getMessage(), e);
             throw new CustomException(ExceptionMessage.TOKEN_GENERATION_FAILED);
         }
🤖 Fix all issues with AI agents
In
`@modules/auth/impl/src/main/java/com/icc/qasker/auth/config/security/service/PrincipalOAuth2UserService.java`:
- Around line 27-35: PrincipalOAuth2UserService currently assumes registrationId
is "google" or "kakao" and may dereference a null oAuth2UserInfo causing NPE;
update the logic in the method that constructs oAuth2UserInfo (the block using
userRequest.getClientRegistration().getRegistrationId()) to explicitly handle
unsupported providers by throwing a clear exception (e.g.,
IllegalArgumentException or OAuth2AuthenticationException) or returning an
appropriate error response instead of proceeding, and include the registrationId
in the error message so callers can diagnose which provider was unsupported.

In
`@modules/auth/impl/src/main/java/com/icc/qasker/auth/controller/AuthController.java`:
- Around line 24-28: The public test endpoint exposed by AuthController (method
test() with `@GetMapping`("/test")) should be removed or protected for production;
either delete the test() method entirely or restrict it to non-production
profiles or authenticated/internal callers (e.g., guard with a profile check or
security annotation like `@PreAuthorize`("hasRole('ADMIN')") or
`@Profile`("!prod")), or replace it with a proper health/check endpoint integrated
with your monitoring (Spring Actuator). Ensure any retained endpoint is not
anonymously accessible by referencing AuthController and the test() handler and
applying the chosen protection.
- Around line 31-35: The refresh method in AuthController calls
rtCookie.getValue() without checking for null, causing an NPE when the
refresh_token cookie is missing; update AuthController.refresh to check the
Optional (or the rtCookie variable) before calling getValue() and when absent
return a 401 Unauthorized response (e.g.,
ResponseEntity.status(HttpStatus.UNAUTHORIZED).build()) instead of invoking
tokenRotationService.rotateTokens; keep TokenRotationService.rotateTokens
unchanged and only call it when rtCookie is present.

In
`@modules/auth/impl/src/main/java/com/icc/qasker/auth/util/RefreshTokenUtil.java`:
- Around line 36-53: Add transactional boundaries to prevent race conditions
during token rotate: annotate the validateAndRotate method in RefreshTokenUtil
with `@Transactional` (org.springframework.transaction.annotation.Transactional)
so the sequence of findByRtHash, isExpired check, refreshToken.rotate(...), and
refreshTokenRepository.save(...) runs in a single transaction (use propagation =
REQUIRED and rollbackFor = Exception.class). Ensure the save call remains inside
that method and, if not already present, consider adding optimistic locking (a
version field) on the RefreshToken entity to further guard against concurrent
rotates.

In
`@modules/global/src/main/java/com/icc/qasker/global/properties/HashIdProperties.java`:
- Around line 9-10: HashIdProperties 클래스의
`@ConfigurationProperties`("q-asker.hashid")에 맞춰 application.yml에 q-asker.hashid
설정을 추가하세요; 구체적으로 HashIdProperties에서 사용하는 필수 프로퍼티 salt와 minLength를 yml에 추가하여
애플리케이션이 시작될 때 해당 값들을 주입받을 수 있도록 설정합니다 (예: q-asker -> hashid 아래에 salt와 min-length
키를 추가해 적절한 값을 넣기).
🧹 Nitpick comments (11)
modules/auth/impl/src/main/java/com/icc/qasker/auth/config/security/filter/JwtTokenAuthenticationFilter.java (1)

78-80: JWT 예외 처리 범위 확장 검토

현재 TokenExpiredException만 catch하고 있습니다. auth0/java-jwt 라이브러리는 SignatureVerificationException, InvalidClaimException, JWTDecodeException 등 다른 예외도 발생시킬 수 있으며, 이 경우 500 에러가 클라이언트에 노출될 수 있습니다.

♻️ 제안하는 수정
-        } catch (TokenExpiredException ex) {
+        } catch (TokenExpiredException | com.auth0.jwt.exceptions.JWTVerificationException ex) {
             chain.doFilter(request, response);
         }
modules/auth/impl/src/main/java/com/icc/qasker/auth/component/AccessTokenHandler.java (1)

21-27: subject와 userId claim 중복 검토

withSubject(user.getUserId())withClaim("userId", user.getUserId())가 동일한 값을 담고 있습니다. JWT 표준에서 sub claim이 사용자 식별자 역할을 하므로, 별도의 userId claim이 필요한지 검토해 주세요. 의도된 설계라면 무시하셔도 됩니다.

modules/auth/impl/src/main/java/com/icc/qasker/auth/config/security/provider/KakaoUserInfo.java (1)

18-21: NullPointerException 방어 코드 검토

attributes.get("id")null을 반환할 경우 toString() 호출 시 NPE가 발생합니다. Kakao OAuth2 응답에서 "id" 필드가 항상 존재한다고 보장되지만, 방어적 코딩을 고려해 보세요.

♻️ 제안하는 수정
     `@Override`
     public String getProviderId() {
-        return attributes.get("id").toString();
+        Object id = attributes.get("id");
+        if (id == null) {
+            throw new IllegalStateException("Kakao OAuth2 response missing 'id' attribute");
+        }
+        return id.toString();
     }

또는 Objects.requireNonNull을 사용:

+import java.util.Objects;
+
     `@Override`
     public String getProviderId() {
-        return attributes.get("id").toString();
+        return Objects.requireNonNull(attributes.get("id"), "Kakao OAuth2 response missing 'id' attribute").toString();
     }
modules/auth/impl/src/main/java/com/icc/qasker/auth/config/security/provider/GoogleUserInfo.java (1)

7-11: attributes 필드를 final로 선언하는 것을 권장합니다.

불변성을 보장하고 실수로 재할당되는 것을 방지하기 위해 final 키워드 추가를 고려해 주세요.

♻️ 제안된 수정
-    private Map<String, Object> attributes;
+    private final Map<String, Object> attributes;
modules/global/src/main/java/com/icc/qasker/global/properties/JwtProperties.java (1)

7-7: spring.security.* prefix는 Spring Security의 예약된 네임스페이스입니다.

spring.security.jwt는 Spring Security의 공식 설정과 충돌할 수 있습니다. 커스텀 프로퍼티는 q-asker.security.jwt와 같은 애플리케이션 고유의 prefix 사용을 권장합니다.

♻️ 제안된 수정
-@ConfigurationProperties(prefix = "spring.security.jwt")
+@ConfigurationProperties(prefix = "q-asker.security.jwt")
modules/auth/impl/src/main/java/com/icc/qasker/auth/util/CookieUtil.java (1)

11-12: 불필요한 @RequiredArgsConstructor 어노테이션.

CookieUtil 클래스는 모든 메서드가 static이며 인스턴스 필드가 없습니다. @RequiredArgsConstructor는 불필요하며, 유틸리티 클래스 패턴(private 생성자 + final 클래스)을 적용하는 것이 좋습니다.

♻️ 제안된 수정
-import lombok.RequiredArgsConstructor;
 import org.springframework.http.ResponseCookie;

-@RequiredArgsConstructor
-public class CookieUtil {
+public final class CookieUtil {
+
+    private CookieUtil() {
+        // Utility class
+    }
app/src/main/resources/db/refresh_token.sql (1)

1-8: expires_at 컬럼에 인덱스 추가를 고려해 주세요.

만료된 토큰을 정리하는 배치 작업이나 쿼리에서 expires_at 컬럼을 조건으로 사용할 가능성이 높습니다. 해당 쿼리의 성능을 위해 인덱스 추가를 권장합니다.

♻️ 제안된 수정
 CREATE TABLE IF NOT EXISTS refresh_token (
     user_id VARCHAR(255) NOT NULL,
     rt_hash VARCHAR(255) NOT NULL,
     expires_at TIMESTAMP(6) NULL,
     created_at TIMESTAMP(6) NULL,
     PRIMARY KEY (user_id),
-    UNIQUE KEY uk_refresh_token_rt_hash (rt_hash)
+    UNIQUE KEY uk_refresh_token_rt_hash (rt_hash),
+    INDEX idx_refresh_token_expires_at (expires_at)
 );
modules/auth/impl/src/main/java/com/icc/qasker/auth/entity/User.java (1)

21-26: 사용되지 않는 password 파라미터 제거 권장
필드가 제거됐는데 생성자 파라미터로 남아 있어 전달값이 조용히 무시됩니다. 혼동 방지를 위해 시그니처를 정리하는 편이 안전합니다.

♻️ 제안 수정
-    public User(String userId, String password, String role, String provider, String nickname) {
+    public User(String userId, String role, String provider, String nickname) {
         this.userId = userId;
         this.role = role;
         this.provider = provider;
         this.nickname = nickname;
     }
modules/auth/impl/src/main/java/com/icc/qasker/auth/entity/RefreshToken.java (1)

15-18: rtHash 필드에 유니크 제약 조건 및 인덱스 추가 권장

RefreshTokenRepository.findByRtHash()로 조회하므로, rtHash 필드에 유니크 제약 조건과 인덱스를 추가하면 조회 성능과 데이터 무결성이 향상됩니다.

♻️ 제안 코드
 `@Id`
 private String userId;
+@Column(unique = true, nullable = false)
 private String rtHash;
 private Instant expiresAt;

jakarta.persistence.Column import도 추가 필요:

 import jakarta.persistence.Entity;
 import jakarta.persistence.Id;
+import jakarta.persistence.Column;
modules/quiz/impl/src/main/java/com/icc/qasker/quiz/controller/doc/GenerationApiDoc.java (1)

16-18: userId 파라미터에 Swagger 어노테이션 누락

API 문서화를 위해 userId 파라미터에 @Parameter 어노테이션을 추가하세요. 또한 이 파라미터가 @UserId로 주입되는 경우 hidden = true로 설정하여 Swagger UI에서 숨기는 것이 좋습니다.

♻️ 제안 코드
+import io.swagger.v3.oas.annotations.Parameter;
     ResponseEntity<GenerationResponse> postProblemSetId(
+        `@Parameter`(hidden = true)
         String userId,
         `@RequestBody` FeGenerationRequest feGenerationRequest);
modules/quiz/impl/src/main/java/com/icc/qasker/quiz/entity/ProblemSet.java (1)

31-47: 사용자별 조회가 예상되면 userId 인덱스도 고려하세요.

ProblemSet을 사용자 단위로 조회/집계할 계획이라면, userId 컬럼 인덱스와 마이그레이션을 함께 준비해두는 게 좋습니다.

Comment on lines 27 to 35
OAuth2UserInfo oAuth2UserInfo = null;
if (userRequest.getClientRegistration().getRegistrationId().equals("google")) {
oAuth2UserInfo = new GoogleUserInfo(oAuth2User.getAttributes());
} else if (userRequest.getClientRegistration().getRegistrationId()
.equals("kakao")) {
oAuth2UserInfo = new KakaoUserInfo(oAuth2User.getAttributes());
}
String userId = oAuth2UserInfo.getProvider() + "_" + oAuth2UserInfo.getProviderId();
String provider = oAuth2UserInfo.getProvider();

This comment was marked as resolved.

Comment on lines +24 to 28
@GetMapping("/test")
public ResponseEntity<?> test() {
System.out.println("test 성공");
return ResponseEntity.ok().build();
}

This comment was marked as off-topic.

Comment on lines 31 to 35
public ResponseEntity<RotateTokenResponse> refresh(HttpServletRequest request,
HttpServletResponse response) {
var rtCookie = CookieUtil.getCookie(request, "refresh_token").orElse(null);
tokenRotationService.rotateTokens(rtCookie.getValue(), response);
return ResponseEntity.ok().build();
return ResponseEntity.ok(tokenRotationService.rotateTokens(rtCookie.getValue(), response));
}

This comment was marked as resolved.

Comment on lines 36 to 53
public RotateResult validateAndRotate(String oldRtPlain) {
String oldRtHash = TokenUtils.sha256Hex(oldRtPlain);

String userId = (String) redis.opsForHash().get(oldRtHash, "userId");
RefreshToken refreshToken = refreshTokenRepository.findByRtHash(oldRtHash)
.orElseThrow(() -> new CustomException(ExceptionMessage.UNAUTHORIZED));

if (userId == null) {
if (refreshToken.isExpired(Instant.now())) {
refreshTokenRepository.delete(refreshToken);
throw new CustomException(ExceptionMessage.UNAUTHORIZED);
}
redis.delete(oldRtHash);
redis.opsForSet().remove(rtKeys.userSet(userId), oldRtHash);

String newRtPlain = issue(userId);
return new RotateResult(userId, newRtPlain);
String newRtPlain = TokenUtils.randomUrlSafe(64);
String newRtHash = TokenUtils.sha256Hex(newRtPlain);
refreshToken.rotate(newRtHash, nextExpiry());
refreshTokenRepository.save(refreshToken);

return new RotateResult(refreshToken.getUserId(), newRtPlain);
}

This comment was marked as resolved.

Comment on lines +9 to +10
@ConfigurationProperties("q-asker.hashid")
public class HashIdProperties {

This comment was marked as off-topic.

Copy link
Member Author

Choose a reason for hiding this comment

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

헤더말고 액세스토큰을 JSON 응답으로 변경

@Getter
@AllArgsConstructor
@ConfigurationProperties(prefix = "spring.security.jwt")
public class JwtProperties {
Copy link
Member Author

Choose a reason for hiding this comment

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

환경변수를 의존성 주입 방식으로 변경


// JWT
TOKEN_GENERATION_FAILED(HttpStatus.INTERNAL_SERVER_ERROR, "토큰 생성 중 서버 오류가 발생했습니다."),
TOKEN_GENERATION_FAILED(HttpStatus.INTERNAL_SERVER_ERROR, "로그인에 실패했습니다."),
Copy link
Member Author

Choose a reason for hiding this comment

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

사용자가 알기 쉬운 메시지로 변경


@Target(ElementType.PARAMETER)
@Retention(RetentionPolicy.RUNTIME)
public @interface UserId {
Copy link
Member Author

Choose a reason for hiding this comment

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

리졸버에게 힌트를 제공하기 위한 어노테이션 @userid 도입

@RequiredArgsConstructor
public class AccessTokenHandler {

private final JwtProperties jwtProperties;
Copy link
Member Author

Choose a reason for hiding this comment

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

STATIC 변수 말고 의존성 주입해서 쓰는 것으로 변경

UserPrincipal principal = (UserPrincipal) authentication.getPrincipal();
User user = principal.getUser();
tokenRotationService.issueRefreshToken(user.getUserId(), response);
response.sendRedirect(qAskerProperties.getFrontendDeployUrl() + "/login/redirect");
Copy link
Member Author

Choose a reason for hiding this comment

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

login/redirect로 이동시켜서 프론트엔드가 추가처리 하도록 함

.allowedOrigins(qAskerProperties.getFrontendDevUrl(),
qAskerProperties.getFrontendDeployUrl())
.allowedMethods("GET", "POST", "PUT", "DELETE")
.allowCredentials(true)
Copy link
Member Author

Choose a reason for hiding this comment

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

클라이언트에게 쿠키를 전송할 자격 부여

import org.springframework.web.method.support.ModelAndViewContainer;

@Component
public class UserIdArgumentResolver implements HandlerMethodArgumentResolver {
Copy link
Member Author

Choose a reason for hiding this comment

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

userPricipal에서 userId를 추출하여 반환하는 리졸버

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants