refactor(server): update auth, user, and recruit member domains with …#277
refactor(server): update auth, user, and recruit member domains with …#277CSE-Shaco merged 1 commit intoGDGoCINHA:developfrom
Conversation
Walkthrough이 PR은 JWT 토큰 생성에 sessionId를 도입하고 Redis 기반 리프레시 토큰 관리를 추가하며, Google ID 토큰 검증 워크플로우를 구현합니다. 또한 사용자 관리 도메인을 재구성하고, 쿠키 기반 인증으로 전환하며, 회원 가입 및 이메일/전화번호 중복 확인 엔드포인트를 추가합니다. Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant AuthCtrl as AuthController
participant AuthSvc as AuthService
participant GoogleAPI as Google API
participant UserRepo as UserRepository
participant Redis as Redis
participant TokenProv as TokenProvider
Client->>AuthCtrl: POST /login (idTokenString)
AuthCtrl->>AuthSvc: authenticate(idTokenString)
AuthSvc->>GoogleAPI: verifyGoogleToken(idTokenString)
GoogleAPI-->>AuthSvc: GoogleUserInfo payload
AuthSvc->>UserRepo: findByOAuthSubject()
alt User exists
UserRepo-->>AuthSvc: User
AuthSvc->>TokenProv: generateTokens(user)
TokenProv->>Redis: storeRefreshSession(refreshToken, sessionId)
TokenProv-->>AuthSvc: TokenDto (accessToken, refreshToken)
AuthSvc-->>AuthCtrl: LoginSuccessResponse
AuthCtrl-->>Client: Set-Cookie + Response
else User not found
UserRepo-->>AuthSvc: null
AuthSvc-->>AuthCtrl: SignupNeededResponse
AuthCtrl-->>Client: Response (requires signup)
end
sequenceDiagram
actor Client
participant AuthCtrl as AuthController
participant AuthSvc as AuthService
participant Redis as Redis
participant TokenProv as TokenProvider
participant UserRepo as UserRepository
Client->>AuthCtrl: POST /refresh (refreshToken from cookie)
AuthCtrl->>AuthSvc: refresh(refreshToken)
AuthSvc->>Redis: resolveRefreshSession(refreshToken)
alt Session found
Redis-->>AuthSvc: RefreshSession (sessionId, userId)
AuthSvc->>UserRepo: findById(userId)
UserRepo-->>AuthSvc: User
AuthSvc->>TokenProv: createAccessToken(user, sessionId)
TokenProv-->>AuthSvc: new accessToken
AuthSvc-->>AuthCtrl: RefreshResult (accessToken, user)
AuthCtrl-->>Client: Set-Cookie (accessToken) + AccessTokenResponse
else Session not found or invalid
Redis-->>AuthSvc: null
AuthSvc-->>AuthCtrl: Exception
AuthCtrl-->>Client: 401 Unauthorized
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/main/java/inha/gdgoc/global/security/TokenAuthenticationFilter.java (1)
56-67:⚠️ Potential issue | 🟠 Major
sanitizeToken에서 발생하는 예외가 catch되지 않아 500 에러가 발생합니다.
getAccessToken(Line 56)은try블록 바깥에서 호출되므로,sanitizeToken이IllegalArgumentException을 던지면 필터에서 처리되지 않고 500 응답이 반환됩니다. 토큰 취득과 인증을 모두try-catch안에서 처리하거나,sanitizeToken이 예외 대신null을 반환하도록 수정해야 합니다.🐛 수정 제안 (Option 1: try-catch 범위 확장)
- String token = getAccessToken(request); - log.info("요청 URI: {}, access token 존재 여부: {}", request.getRequestURI(), token != null); - - if (token != null) { - try { - Authentication authentication = tokenProvider.getAuthentication(token); + try { + String token = getAccessToken(request); + log.info("요청 URI: {}, access token 존재 여부: {}", request.getRequestURI(), token != null); + + if (token != null) { + Authentication authentication = tokenProvider.getAuthentication(token); SecurityContextHolder.getContext().setAuthentication(authentication); log.info("JWT 인증 성공: {}", authentication.getName()); - } catch (Exception e) { - SecurityContextHolder.clearContext(); - log.warn("JWT 인증 실패: {}", e.getMessage()); + } else { + log.info("access token 없음 → 인증 시도 안함"); } - } else { - log.info("access token 없음 → 인증 시도 안함"); + } catch (Exception e) { + SecurityContextHolder.clearContext(); + log.warn("JWT 인증 실패: {}", e.getMessage()); }src/main/java/inha/gdgoc/domain/user/entity/User.java (1)
34-39:⚠️ Potential issue | 🔴 Critical클래스와 생성자에
@Builder가 중복 선언되어@Default가 무시될 수 있습니다.Line 34의 클래스 레벨
@Builder와 Line 102의 생성자 레벨@Builder가 동시에 존재합니다. Lombok은 생성자 레벨@Builder를 우선하므로, 필드에 선언된@Default(Line 70, 79)가 무시됩니다. 그 결과:
membershipStatus: 생성자(Line 102-122)에 해당 파라미터가 없어 빌더로 생성 시null이 되며, DB의NOT NULL제약 조건 위반으로 런타임 에러가 발생합니다.userRole: 생성자에서 파라미터 값을 그대로 할당(Line 116)하므로null이 전달되면GUEST기본값이 적용되지 않습니다.클래스 레벨
@Builder를 제거하고 생성자에서 기본값을 직접 처리하거나, 반대로 생성자 레벨@Builder를 제거하고 클래스 레벨만 유지하세요.🐛 수정 제안 (생성자 레벨 `@Builder` 유지 시)
-@Builder `@Getter` `@Entity` `@Table`(name = "users") `@NoArgsConstructor`(access = AccessLevel.PROTECTED) `@AllArgsConstructor` public class User extends BaseEntity {그리고 생성자에서 기본값을 직접 설정:
`@Builder` public User( String name, String oauthSubject, String major, String studentId, String phoneNumber, - String email, UserRole userRole, + String email, UserRole userRole, MembershipStatus membershipStatus, TeamType team, String image, SocialUrls social, Careers careers ) { this.oauthSubject = oauthSubject; this.name = name; this.major = major; this.studentId = studentId; this.phoneNumber = phoneNumber; this.email = email; - this.userRole = userRole; + this.userRole = (userRole != null ? userRole : UserRole.GUEST); + this.membershipStatus = (membershipStatus != null ? membershipStatus : MembershipStatus.PENDING); this.team = team; this.image = image; this.social = (social != null ? social : new SocialUrls()); this.careers = (careers != null ? careers : new Careers()); }Also applies to: 70-71, 78-80, 102-122
src/main/java/inha/gdgoc/domain/admin/user/service/UserAdminService.java (1)
146-183:⚠️ Potential issue | 🟠 Major
updateUserRoleWithRules에서me(CustomUserDetails)의 role/team을 직접 사용하지만,updateRoleAndTeam에서는 DB에서 다시 조회합니다.
updateUserRoleWithRules는 토큰의me.getRole()/me.getTeam()을 신뢰하고,updateRoleAndTeam은getEditor(editor)로 DB에서 최신 상태를 조회합니다. 토큰 발급 후 역할/팀이 변경되었을 경우,updateUserRoleWithRules는 stale 데이터로 권한을 판단할 수 있습니다. 두 메서드 모두 일관되게 DB 조회를 사용하는 것이 더 안전합니다.src/main/java/inha/gdgoc/domain/auth/controller/AuthController.java (1)
131-151:⚠️ Potential issue | 🟡 MinorRefresh 엔드포인트에서
Exception전체를 catch하고 있습니다.Line 147에서 모든
Exception을 잡아INVALID_REFRESH_TOKEN으로 변환하면, 실제 서버 오류(DB 장애, NullPointerException 등)도 "유효하지 않은 토큰" 오류로 노출됩니다. 이는 디버깅을 어렵게 만들고 실제 문제를 숨길 수 있습니다. 특정 예외 타입만 catch하는 것을 권장합니다.
🤖 Fix all issues with AI agents
In @.github/workflows/deploy-dev.yml:
- Around line 42-45: Add the missing ACCESS_COOKIE_* environment variables to
the deploy workflows: ensure ACCESS_COOKIE_SECURE, ACCESS_COOKIE_SAME_SITE, and
ACCESS_COOKIE_DOMAIN are exported into the environment block (same style as
REFRESH_COOKIE_SECURE, REFRESH_COOKIE_SAME_SITE, REFRESH_COOKIE_DOMAIN) in both
deploy-dev.yml and deploy-prod.yml so that application-prod.yml can read
explicit values; use the corresponding secrets (e.g.,
secrets.ACCESS_COOKIE_SECURE, secrets.ACCESS_COOKIE_SAME_SITE,
secrets.ACCESS_COOKIE_DOMAIN) and fallbacks consistent with REFRESH_COOKIE_*
conventions.
In @.github/workflows/deploy-prod.yml:
- Line 46: The REFRESH_COOKIE_SECURE environment variable currently defaults to
'false' in the deploy workflow; change its default to 'true' so production
deployments set the refresh-token cookie Secure flag by default, while
preserving the ability for secrets to override it (i.e., use
REFRESH_COOKIE_SECURE=${{ secrets.REFRESH_COOKIE_SECURE || 'true' }}). Update
the workflow entry that references REFRESH_COOKIE_SECURE to use the new default
and ensure any downstream logic that reads REFRESH_COOKIE_SECURE treats the
value as a boolean/string accordingly.
In `@src/main/java/inha/gdgoc/domain/auth/controller/AuthController.java`:
- Around line 196-199: The closing brace for the method checkRoleOrTeam is
mis-indented (it appears at class-level indentation); move the `}` so it aligns
with the method's opening brace and ensure braces are balanced for
checkRoleOrTeam (verify the method body ends right after the
GlobalErrorCode.FORBIDDEN_USER block), then run the project's Java formatter to
confirm consistent indentation.
- Around line 109-129: The controller-level bean validation is not active
because the AuthController class lacks the `@Validated` annotation; add `@Validated`
to the AuthController class declaration (next to existing annotations like
`@RestController/`@RequestMapping/@RequiredArgsConstructor/@Slf4j) so method
parameter constraints on duplicatedStudentIdDetails and
duplicatedPhoneNumberDetails (`@NotBlank`, `@Pattern`) are enforced at runtime.
In `@src/main/java/inha/gdgoc/domain/auth/service/AuthService.java`:
- Line 63: The if (user == null) block in AuthService.java is mis-indented
compared to the surrounding 2-space style; reformat the block (inside the
AuthService class/method where the if (user == null) appears) to use consistent
2-space indentation and run the project formatter so all surrounding lines match
the repository style.
- Around line 260-281: The deriveNameParts method currently splits space-free
names by taking substring(0,1) as family and substring(1) as given, which
assumes a single-character family name (common in Korean) and is
locale-dependent; add a concise comment above deriveNameParts documenting this
assumption, noting that Google-provided given_name/family_name are preferred
when available, that the substring fallback may mis-handle compound surnames
(e.g., "남궁", "선우") and foreign names, and suggest using a locale-aware parser or
configurable policy in future if stricter correctness is required.
- Around line 193-214: In resolveRefreshSession, remove or disable the automatic
legacy-format migration (the block that calls userRepository.findByOauthSubject,
creates a new RefreshSession, and calls storeRefreshSession) because it allows
arbitrary Redis values to be upgraded into valid sessions; instead, treat
storedValue without SESSION_VALUE_DELIMITER as invalid and throw an exception
(or require an explicit migration flag/mechanism), ensuring only
decodeSessionValue(...) handles valid session strings; reference
resolveRefreshSession, refreshTokenKey, decodeSessionValue, storeRefreshSession,
userRepository.findByOauthSubject, and RefreshSession when making the change.
- Around line 48-77: The login method currently returns Object; introduce a
sealed interface LoginResponse that permits SignupNeededResponse and
LoginSuccessResponse, have those two response classes implement/extend
LoginResponse, change AuthService.login(String idToken) to return LoginResponse,
and update any callers (e.g., controller) to accept LoginResponse instead of
Object so callers no longer need instanceof checks and gain compile-time type
safety; ensure SignupNeededResponse.builder() and LoginSuccessResponse.of(...)
produce types that implement LoginResponse.
- Around line 79-107: The signup() flow currently only checks studentId
uniqueness; add service-level checks for phone and email before saving: after
computing cleanPhone use userRepository.existsByPhoneNumber(cleanPhone) and
throw IllegalArgumentException("이미 존재하는 전화번호입니다.") if true, and use
userRepository.existsByEmail(request.getEmail()) and throw
IllegalArgumentException("이미 존재하는 이메일입니다.") if true; keep the rest of the flow
(User.builder(...), userRepository.save(newUser), generateTokens(newUser), and
returning LoginSuccessResponse.of(tokens, AuthUserResponse.from(newUser)))
unchanged.
In
`@src/main/java/inha/gdgoc/domain/recruit/member/controller/RecruitMemberController.java`:
- Around line 75-83: The multipart endpoint recruitMemberAddMultipart accepts a
MultipartFile file but currently ignores it; either pass the file into the
service (e.g., update recruitMemberService.addRecruitMember(applicationRequest,
file) and implement file handling in the service), or remove the unused
`@RequestPart`("file") MultipartFile file parameter; if file handling is planned
later, add a clear TODO comment in recruitMemberAddMultipart referencing
MultipartFile file and/or create an overloaded service method
(recruitMemberService.addRecruitMember) that will accept and process the file
when implemented.
- Line 114: The email regex in the `@Pattern` annotation inside
RecruitMemberController (the string
"^[a-zA-Z0-9+-\\_.]+@[a-zA-Z0-9-]+\\.[a-zA-Z0-9-.]+$") incorrectly allows a
character range because the hyphen in the local-part character class is not
escaped; update the regex in the `@Pattern` on RecruitMemberController to treat
'-' literally by either escaping it (e.g., '\-') or placing it at the start or
end of the character class so the local-part class only permits the intended
characters.
In `@src/main/java/inha/gdgoc/global/config/jwt/TokenProvider.java`:
- Around line 40-43: initSigningKey currently calls
buildSigningKey(jwtProperties.getSecretKey()) without validating the secret; add
a null/empty check for jwtProperties.getSecretKey() in initSigningKey and fail
fast with a clear exception (e.g., throw IllegalStateException with a
descriptive message) or log error and avoid calling buildSigningKey to prevent
NPE/invalid key creation; reference initSigningKey, buildSigningKey,
jwtProperties.getSecretKey(), and cachedSigningKey when making the change so the
validation wraps the call that sets cachedSigningKey.
- Around line 115-132: The buildSigningKey method in TokenProvider currently
replaces short keys by hashing them (candidateKey.length < 32) which hides
configuration errors; instead, change TokenProvider.buildSigningKey to validate
the decoded/bytes candidateKey length and throw a clear exception (e.g.,
IllegalArgumentException or IllegalStateException) when it's shorter than the
required 32 bytes, removing the SHA-256 fallback so misconfigured/too-short
secrets fail fast and surface the error.
In `@src/main/java/inha/gdgoc/global/security/SecurityConfig.java`:
- Around line 50-51: SecurityConfig permitAll() paths were changed to
"/api/v1/recruit/member/apply/**" and "/api/v1/recruit/member/check/**" but
TokenAuthenticationFilter.shouldNotFilter() still checks the old
"/api/v1/apply/**" and "/api/v1/check/**"; update the path patterns inside
TokenAuthenticationFilter.shouldNotFilter() to match the new patterns (e.g.,
"/api/v1/recruit/member/apply/**" and "/api/v1/recruit/member/check/**") so the
filter excludes the same public endpoints that SecurityConfig permits, keeping
behavior consistent between SecurityConfig and TokenAuthenticationFilter.
In `@src/main/resources/application-dev.yml`:
- Around line 60-70: Update the production cookie defaults so they are secure
even if env vars are missing: in the production config change the
auth.refresh-cookie and auth.access-cookie entries (properties named
refresh-cookie, access-cookie under auth) to set secure: true and same-site:
Strict (or same-site: None if you will enforce HTTPS and need cross-site use);
ensure these are hard-coded defaults in application-prod.yml rather than relying
solely on environment variable fallbacks.
In `@src/main/resources/application-prod.yml`:
- Around line 60-70: The production YAML sets REFRESH_COOKIE_SECURE and
ACCESS_COOKIE_SECURE defaulting to false; change the defaults so cookies are
secure by default in production by updating the auth.refresh-cookie.secure and
auth.access-cookie.secure entries to default to true (i.e., change
${REFRESH_COOKIE_SECURE:false} and ${ACCESS_COOKIE_SECURE:false} to default to
true) so that when those environment variables are missing the cookies will be
sent only over HTTPS.
- Around line 79-80: Add support for configuring JWT_ACCESS_TOKEN_VALIDITY via
CI secrets so accessTokenValidity in application-prod.yml can be set per
environment: update the deploy-prod.yml and deploy-dev.yml workflows to export
JWT_ACCESS_TOKEN_VALIDITY from secrets (e.g., add an env entry mapping
JWT_ACCESS_TOKEN_VALIDITY: ${{ secrets.JWT_ACCESS_TOKEN_VALIDITY }} alongside
JWT_AUDIENCE/GOOGLE_ISSUER/SELF_ISSUER), ensure the corresponding secret is
created in the target repo/environment, and keep the existing
${JWT_ACCESS_TOKEN_VALIDITY:3600000} fallback in application-prod.yml unchanged
so deployments without the secret still work.
🧹 Nitpick comments (23)
src/main/java/inha/gdgoc/domain/auth/dto/response/CheckPhoneNumberResponse.java (1)
1-4:recruit.member패키지의 동일 DTO와 중복됩니다.
inha.gdgoc.domain.recruit.member.dto.response.CheckPhoneNumberResponse와 완전히 동일한 레코드가 존재합니다. 두 도메인 모두에서 필요하다면 공통 패키지로 추출하거나, 한쪽에서 다른 쪽을 재사용하는 것을 검토해 주세요.#!/bin/bash # 두 CheckPhoneNumberResponse의 사용처 확인 rg -rn "CheckPhoneNumberResponse" --type javasrc/main/java/inha/gdgoc/domain/auth/dto/response/CheckStudentIdResponse.java (1)
1-4:recruit.member패키지의 동일 DTO와 중복됩니다.
CheckPhoneNumberResponse와 마찬가지로inha.gdgoc.domain.recruit.member.dto.response.CheckStudentIdResponse와 동일한 레코드입니다. 위 리뷰 코멘트와 동일하게 공통화를 검토해 주세요.src/main/java/inha/gdgoc/domain/admin/user/dto/request/UpdateUserRoleTeamRequest.java (1)
6-9: null 의미를 설명하는 주석이 누락되었습니다.기존 코드에는
role과team필드에// null 이면 변경 안 함주석이 있었습니다. 이 DTO의 null 허용 계약은UserAdminService.updateRoleAndTeam()의 핵심 동작이므로, 주석을 유지하는 것이 좋습니다.📝 주석 복원 제안
public record UpdateUserRoleTeamRequest( - UserRole role, - TeamType team + UserRole role, // null 이면 변경 안 함 + TeamType team // null 이면 변경 안 함 ) {}src/main/java/inha/gdgoc/global/security/TokenAuthenticationFilter.java (1)
75-85:HEADER_AUTHORIZATION와TOKEN_PREFIX가 미사용 상태입니다.쿠키 기반으로 전환하면서 Line 76-77의 상수가 더 이상 사용되지 않습니다. 제거하세요.
♻️ 수정 제안
private String getAccessToken(HttpServletRequest request) { - final String HEADER_AUTHORIZATION = "Authorization"; - final String TOKEN_PREFIX = "Bearer "; - String cookieToken = readCookieToken(request, "access_token"); if (cookieToken != null) { return sanitizeToken(cookieToken.trim()); } return null; }src/main/java/inha/gdgoc/domain/user/entity/User.java (1)
151-151:MembershipStatusenum을 별도 파일로 분리하는 것을 권장합니다.엔티티 내부에 중첩된 enum은 다른 패키지에서 참조 시
User.MembershipStatus로 접근해야 하므로 불필요한 결합이 생깁니다. DTO, 서비스, 마이그레이션 등 여러 곳에서 사용되므로inha.gdgoc.domain.user.enums패키지에 독립 enum으로 분리하면UserRole,TeamType과 일관성도 유지됩니다.src/main/java/inha/gdgoc/domain/admin/user/service/UserAdminService.java (2)
206-207: 빈ADMINcase 블록에 의도를 명시하는 주석 추가를 권장합니다.빈 블록은 "ADMIN은 rank 체크를 이미 통과했으므로 추가 제한 없음"이라는 의도이지만, 유지보수 시 실수로 누락된 것으로 오인될 수 있습니다.
♻️ 제안
- case ADMIN -> { - } + case ADMIN -> { + // rank 검증만으로 충분 — 추가 제한 없음 + }
232-235:getEditor헬퍼 메서드 추출은 좋은 리팩토링입니다.
deleteUserWithRules(라인 187-188)도 동일 패턴을 사용하고 있으므로, 이 메서드를 재사용하면 중복을 줄일 수 있습니다.♻️ 제안
public void deleteUserWithRules(CustomUserDetails me, Long targetUserId) { - User editor = userRepository.findById(me.getUserId()) - .orElseThrow(() -> new BusinessException(GlobalErrorCode.UNAUTHORIZED_USER)); + User editor = getEditor(me); User target = userRepository.findById(targetUserId)src/main/java/inha/gdgoc/domain/auth/dto/response/AuthUserResponse.java (1)
20-23:from(null)이null을 반환하면 downstream에서 NPE 가능성이 있습니다.
LoginSuccessResponse.of(tokens, AuthUserResponse.from(user))에서user가 null이면 응답의user필드가 null이 되어 클라이언트 측에서 예기치 않은 오류를 발생시킬 수 있습니다. null user는 호출 전에 검증하고, 여기서는 예외를 던지는 것이 더 안전할 수 있습니다.src/main/java/inha/gdgoc/domain/admin/user/controller/UserAdminController.java (1)
54-57:size파라미터에 상한이 없어 과도한 메모리 사용이 가능합니다.악의적인 요청으로
size=100000같은 값을 전달하면 대량의 데이터를 한 번에 조회할 수 있습니다. 상한값 제한을 권장합니다.♻️ 제안
`@RequestParam`(defaultValue = "0") int page, `@RequestParam`(defaultValue = "20") int size, ... - Pageable pageable = PageRequest.of(page, size, Sort.by(direction, sort)); + int safeSize = Math.min(size, 100); + Pageable pageable = PageRequest.of(page, safeSize, Sort.by(direction, sort));src/main/java/inha/gdgoc/global/config/jwt/JwtProperties.java (1)
17-17:audience속성에 대한 유효성 검증이 없습니다.JWT audience는 토큰 검증에 중요한 값입니다.
@NotBlank등의 어노테이션을 추가하면 애플리케이션 시작 시 누락된 설정을 빠르게 감지할 수 있습니다. 다른 필드(secretKey,selfIssuer등)에도 동일하게 적용하면 좋지만, 기존 패턴과의 일관성을 고려하여 선택적으로 적용하시면 됩니다.src/main/java/inha/gdgoc/domain/auth/dto/response/AccessTokenResponse.java (2)
7-7: DTO가BaseEntity를 상속하고 있습니다.
AccessTokenResponse는 API 응답 DTO인데 JPA 엔티티용BaseEntity를 상속하고 있습니다. DTO에createdAt/updatedAt같은 영속성 관련 필드가 JSON 응답에 불필요하게 포함될 수 있습니다. 이 상속을 제거하는 것을 권장합니다.
8-8: 필드명이 Java 네이밍 컨벤션을 따르지 않습니다.
access_token은 snake_case로, Java에서는accessToken으로 선언하고@JsonProperty("access_token")을 사용하는 것이 관례입니다. 같은 파일 내LoginSuccessResponse에서는accessToken(camelCase)을 사용하고 있어 일관성이 없습니다.src/main/java/inha/gdgoc/domain/auth/controller/AuthController.java (3)
46-69: 8개의@Value필드를@ConfigurationProperties클래스로 통합하는 것을 권장합니다.Refresh 쿠키 4개 + Access 쿠키 4개의
@Value필드가 컨트롤러에 직접 주입되어 있습니다.@ConfigurationProperties로 바인딩하면 타입 안전성, 유효성 검증, 테스트 용이성이 향상되고 컨트롤러가 간결해집니다.♻️ 제안: CookieProperties 클래스 분리
`@ConfigurationProperties`(prefix = "app.auth") `@Validated` public record AuthCookieProperties( CookieConfig refreshCookie, CookieConfig accessCookie ) { public record CookieConfig( boolean secure, String sameSite, String domain, String path ) {} }컨트롤러에서는
AuthCookieProperties를 주입받아 사용:- `@Value`("${app.auth.refresh-cookie.domain:}") - private String refreshCookieDomain; - // ... 7개 더 + private final AuthCookieProperties cookieProperties;
73-88:login과signup의 쿠키 설정 로직이 중복됩니다.Lines 76-82와 95-101이 거의 동일합니다. 공통 헬퍼 메서드로 추출하면 유지보수성이 향상됩니다.
또한,
Object response타입 사용으로 인해 타입 안전성이 없습니다.authService.login()이LoginSuccessResponse또는SignupNeededResponse를 반환하는 것이라면, sealed interface나 공통 인터페이스를 도입하여instanceof체크를 대체하는 것을 고려해 주세요.
201-227: 쿠키 생성/삭제 로직이 컨트롤러에 과도하게 포함되어 있습니다.약 60줄의 쿠키 헬퍼 메서드가 컨트롤러에 있습니다. 별도 유틸리티/서비스 클래스로 추출하면 컨트롤러의 단일 책임 원칙을 지킬 수 있고, 다른 컨트롤러에서도 재사용 가능합니다.
src/main/java/inha/gdgoc/global/config/jwt/TokenProvider.java (3)
46-63:subject와uid클레임이 중복됩니다.Line 55에서
subject를user.getId()로, Line 57에서uid클레임도 동일하게user.getId()로 설정하고 있습니다. 그런데getAuthentication에서는uid클레임만 사용합니다.subject에 userId를 넣을 필요가 있는지 확인해 주세요. 불필요하다면 제거하여 토큰 페이로드를 간결하게 유지하는 것이 좋습니다.
73-100: 매 요청마다 DB 조회가 발생하여 성능 부담이 될 수 있습니다.
getAuthentication이 호출될 때마다userRepository.findById(userId)로 DB를 조회합니다. 인증 필터에서 모든 요청에 대해 이 메서드가 호출되면, DB 부하가 상당할 수 있습니다. 짧은 TTL의 로컬 캐시(예: Caffeine)를 도입하거나, JWT 클레임에 role/team 정보를 포함하여 매번 DB를 조회하지 않도록 개선을 고려해 주세요.
163-185:CustomUserDetails에 빈 문자열 패스워드 사용 — 의도된 설계인지 확인 필요.Line 179에서
super(username, "", authorities)로 빈 패스워드를 전달하고 있습니다. JWT 기반 인증이므로 패스워드가 필요 없는 것은 이해하지만, Spring Security의User클래스를 상속하는 대신 직접UserDetails를 구현하면 패스워드 필드 자체를 피할 수 있어 더 명확합니다.src/main/java/inha/gdgoc/domain/auth/service/AuthService.java (5)
166-185:GoogleIdTokenVerifier를 매 로그인 호출마다 새로 생성하고 있습니다.
NetHttpTransport와GsonFactory는 thread-safe한 객체로,GoogleIdTokenVerifier를 필드로 캐싱하거나@Bean으로 등록하면 불필요한 객체 생성을 줄일 수 있습니다.♻️ 제안: 필드로 초기화
+ private GoogleIdTokenVerifier googleIdTokenVerifier; + + `@jakarta.annotation.PostConstruct` + void initGoogleVerifier() { + this.googleIdTokenVerifier = new GoogleIdTokenVerifier.Builder( + new NetHttpTransport(), GsonFactory.getDefaultInstance()) + .setAudience(Collections.singletonList(googleClientId)) + .build(); + } + private GoogleUserInfo verifyGoogleToken(String idTokenString) { try { - GoogleIdTokenVerifier verifier = - new GoogleIdTokenVerifier.Builder(new NetHttpTransport(), new GsonFactory()) - .setAudience(Collections.singletonList(googleClientId)) - .build(); - - GoogleIdToken idToken = verifier.verify(idTokenString); + GoogleIdToken idToken = googleIdTokenVerifier.verify(idTokenString);
55-57: 도메인 검증에IllegalArgumentException을 사용하고 있습니다.프로젝트 전반에
BusinessException과ErrorCode패턴이 있음에도 불구하고, 여기서는IllegalArgumentException을 사용합니다. 일관된 예외 처리를 위해 전용ErrorCode를 정의하고BusinessException을 사용하는 것이 좋습니다. 이 파일 내 다른IllegalArgumentException사용처(Line 84, 128, 147, 179, 183, 198, 209, 219, 225)에도 동일하게 적용되어야 합니다.
37-39:REFRESH_TOKEN_TTL이 하드코딩되어 있습니다.14일 TTL이 코드에 직접 하드코딩되어 있습니다.
JwtProperties나 별도 설정에서 관리하면 재배포 없이 변경할 수 있어 운영 유연성이 높아집니다.
122-133:refresh메서드에서 Refresh Token rotation이 적용되지 않습니다.현재 구현에서는 Access Token만 재발급하고 Refresh Token은 그대로 유지합니다. Refresh Token이 탈취된 경우 14일간 계속 사용될 수 있습니다. 보안 강화를 위해 refresh 시 Refresh Token도 함께 재발급(rotation)하고, 기존 토큰을 무효화하는 패턴을 고려해 주세요.
135-140: 로그아웃 시 삭제 결과를 확인하지 않습니다.
redisTemplate.delete()는 삭제된 키 수를 반환하지만, 현재 코드에서는 이를 무시합니다. 이미 로그아웃된 토큰이나 존재하지 않는 토큰에 대해서도 성공으로 처리됩니다. 멱등성 관점에서는 문제없지만, 로깅이라도 추가하면 디버깅에 도움이 됩니다.
| JWT_AUDIENCE=${{ secrets.JWT_AUDIENCE }} | ||
| REFRESH_COOKIE_SECURE=${{ secrets.REFRESH_COOKIE_SECURE || 'false' }} | ||
| REFRESH_COOKIE_SAME_SITE=${{ secrets.REFRESH_COOKIE_SAME_SITE || 'Lax' }} | ||
| REFRESH_COOKIE_DOMAIN=${{ secrets.REFRESH_COOKIE_DOMAIN }} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, search for ACCESS_COOKIE references across workflows
rg -n "ACCESS_COOKIE" .github/workflows/Repository: GDGoCINHA/24-2_GDGoC_Server
Length of output: 53
🏁 Script executed:
# Check the full deploy-dev.yml file to see all env vars defined
cat -n .github/workflows/deploy-dev.ymlRepository: GDGoCINHA/24-2_GDGoC_Server
Length of output: 3598
🏁 Script executed:
# Check deploy-prod.yml for comparison
cat -n .github/workflows/deploy-prod.ymlRepository: GDGoCINHA/24-2_GDGoC_Server
Length of output: 3669
🏁 Script executed:
# Look for application-prod.yml to see what variables it references
fd "application-prod.yml" --type fRepository: GDGoCINHA/24-2_GDGoC_Server
Length of output: 110
🏁 Script executed:
# Check if application-prod.yml references ACCESS_COOKIE_* variables
cat src/main/resources/application-prod.ymlRepository: GDGoCINHA/24-2_GDGoC_Server
Length of output: 2010
ACCESS_COOKIE_* 환경 변수가 두 배포 워크플로우에서 누락되었습니다.
application-prod.yml에서 ACCESS_COOKIE_SECURE, ACCESS_COOKIE_SAME_SITE, ACCESS_COOKIE_DOMAIN을 참조하고 있으나, deploy-dev.yml과 deploy-prod.yml 모두에서 .env에 포함되지 않았습니다. REFRESH_COOKIE_* 환경 변수는 명시적으로 추가되었지만, 동일한 수준의 쿠키 설정인 ACCESS_COOKIE_*는 누락되어 있어 설정 관리의 일관성이 부족합니다. 애플리케이션은 기본값(secure: false, same-site: Lax)으로 정상 작동하겠지만, 명시적 관리를 위해 두 워크플로우에 모두 ACCESS_COOKIE_SECURE, ACCESS_COOKIE_SAME_SITE, ACCESS_COOKIE_DOMAIN을 추가해 주세요.
🤖 Prompt for AI Agents
In @.github/workflows/deploy-dev.yml around lines 42 - 45, Add the missing
ACCESS_COOKIE_* environment variables to the deploy workflows: ensure
ACCESS_COOKIE_SECURE, ACCESS_COOKIE_SAME_SITE, and ACCESS_COOKIE_DOMAIN are
exported into the environment block (same style as REFRESH_COOKIE_SECURE,
REFRESH_COOKIE_SAME_SITE, REFRESH_COOKIE_DOMAIN) in both deploy-dev.yml and
deploy-prod.yml so that application-prod.yml can read explicit values; use the
corresponding secrets (e.g., secrets.ACCESS_COOKIE_SECURE,
secrets.ACCESS_COOKIE_SAME_SITE, secrets.ACCESS_COOKIE_DOMAIN) and fallbacks
consistent with REFRESH_COOKIE_* conventions.
| GOOGLE_ISSUER=${{ secrets.GOOGLE_ISSUER }} | ||
| SELF_ISSUER=${{ secrets.SELF_ISSUER }} | ||
| JWT_AUDIENCE=${{ secrets.JWT_AUDIENCE }} | ||
| REFRESH_COOKIE_SECURE=${{ secrets.REFRESH_COOKIE_SECURE || 'false' }} |
There was a problem hiding this comment.
프로덕션 환경에서 REFRESH_COOKIE_SECURE의 기본값이 false입니다 — 보안 위험.
리프레시 토큰 쿠키가 Secure 플래그 없이 전송되면 HTTP를 통해 탈취될 수 있습니다. 프로덕션 배포에서는 기본값을 true로 설정해야 합니다.
🔒 제안
- REFRESH_COOKIE_SECURE=${{ secrets.REFRESH_COOKIE_SECURE || 'false' }}
+ REFRESH_COOKIE_SECURE=${{ secrets.REFRESH_COOKIE_SECURE || 'true' }}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| REFRESH_COOKIE_SECURE=${{ secrets.REFRESH_COOKIE_SECURE || 'false' }} | |
| REFRESH_COOKIE_SECURE=${{ secrets.REFRESH_COOKIE_SECURE || 'true' }} |
🤖 Prompt for AI Agents
In @.github/workflows/deploy-prod.yml at line 46, The REFRESH_COOKIE_SECURE
environment variable currently defaults to 'false' in the deploy workflow;
change its default to 'true' so production deployments set the refresh-token
cookie Secure flag by default, while preserving the ability for secrets to
override it (i.e., use REFRESH_COOKIE_SECURE=${{ secrets.REFRESH_COOKIE_SECURE
|| 'true' }}). Update the workflow entry that references REFRESH_COOKIE_SECURE
to use the new default and ensure any downstream logic that reads
REFRESH_COOKIE_SECURE treats the value as a boolean/string accordingly.
| @GetMapping("/check/student-id") | ||
| public ResponseEntity<ApiResponse<CheckStudentIdResponse, Void>> duplicatedStudentIdDetails( | ||
| @RequestParam | ||
| @NotBlank(message = "학번은 필수 입력 값입니다.") | ||
| @Pattern(regexp = "^12[0-9]{6}$", message = "유효하지 않은 학번 값입니다.") | ||
| String studentId | ||
| ) { | ||
| CheckStudentIdResponse response = authService.isRegisteredStudentId(studentId); | ||
| return ResponseEntity.ok(ApiResponse.ok(STUDENT_ID_DUPLICATION_CHECK_SUCCESS, response)); | ||
| } | ||
|
|
||
| @GetMapping("/check/phone-number") | ||
| public ResponseEntity<ApiResponse<CheckPhoneNumberResponse, Void>> duplicatedPhoneNumberDetails( | ||
| @RequestParam | ||
| @NotBlank(message = "전화번호는 필수 입력 값입니다.") | ||
| @Pattern(regexp = "^010-\\d{4}-\\d{4}$", message = "전화번호 형식은 010-XXXX-XXXX 이어야 합니다.") | ||
| String phoneNumber | ||
| ) { | ||
| CheckPhoneNumberResponse response = authService.isRegisteredPhoneNumber(phoneNumber); | ||
| return ResponseEntity.ok(ApiResponse.ok(PHONE_NUMBER_DUPLICATION_CHECK_SUCCESS, response)); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n "@Validated" src/main/java/inha/gdgoc/domain/auth/controller/AuthController.javaRepository: GDGoCINHA/24-2_GDGoC_Server
Length of output: 53
🏁 Script executed:
head -30 src/main/java/inha/gdgoc/domain/auth/controller/AuthController.javaRepository: GDGoCINHA/24-2_GDGoC_Server
Length of output: 1580
🏁 Script executed:
rg -n "class AuthController" src/main/java/inha/gdgoc/domain/auth/controller/AuthController.java -A 5Repository: GDGoCINHA/24-2_GDGoC_Server
Length of output: 301
🏁 Script executed:
rg -B 5 "class AuthController" src/main/java/inha/gdgoc/domain/auth/controller/AuthController.javaRepository: GDGoCINHA/24-2_GDGoC_Server
Length of output: 181
메서드 파라미터 검증을 활성화하려면 @Validated 어노테이션이 필요합니다.
@NotBlank와 @Pattern 유효성 검증이 메서드 파라미터에 적용되어 있으나, 이들이 실제로 작동하려면 클래스 레벨에 @Validated 어노테이션을 추가해야 합니다. 현재 클래스에는 이 어노테이션이 없어 검증이 수행되지 않습니다.
`@Validated`
`@Slf4j`
`@RequestMapping`("/api/v1/auth")
`@RestController`
`@RequiredArgsConstructor`
public class AuthController {
🤖 Prompt for AI Agents
In `@src/main/java/inha/gdgoc/domain/auth/controller/AuthController.java` around
lines 109 - 129, The controller-level bean validation is not active because the
AuthController class lacks the `@Validated` annotation; add `@Validated` to the
AuthController class declaration (next to existing annotations like
`@RestController/`@RequestMapping/@RequiredArgsConstructor/@Slf4j) so method
parameter constraints on duplicatedStudentIdDetails and
duplicatedPhoneNumberDetails (`@NotBlank`, `@Pattern`) are enforced at runtime.
| GlobalErrorCode.FORBIDDEN_USER.getMessage(), | ||
| null | ||
| )); | ||
| } |
There was a problem hiding this comment.
checkRoleOrTeam 메서드의 닫는 괄호 들여쓰기가 어긋나 있습니다.
Line 199의 }가 메서드 본문과 같은 레벨에 있어야 하는데, 클래스 레벨 들여쓰기로 보입니다.
🔧 수정 제안
-}
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| GlobalErrorCode.FORBIDDEN_USER.getMessage(), | |
| null | |
| )); | |
| } | |
| GlobalErrorCode.FORBIDDEN_USER.getMessage(), | |
| null | |
| )); | |
| } |
🤖 Prompt for AI Agents
In `@src/main/java/inha/gdgoc/domain/auth/controller/AuthController.java` around
lines 196 - 199, The closing brace for the method checkRoleOrTeam is
mis-indented (it appears at class-level indentation); move the `}` so it aligns
with the method's opening brace and ensure braces are balanced for
checkRoleOrTeam (verify the method body ends right after the
GlobalErrorCode.FORBIDDEN_USER block), then run the project's Java formatter to
confirm consistent indentation.
| // 로그인 | ||
| @Transactional | ||
| public Object login(String idToken) { | ||
| // Google ID Token 검증 | ||
| GoogleUserInfo googleUser = verifyGoogleToken(idToken); | ||
|
|
||
| // 도메인 검증 (인하대 메일만 허용) | ||
| if (!googleUser.getEmail().endsWith("@inha.edu")) { | ||
| throw new IllegalArgumentException("인하대학교(@inha.edu) 계정만 이용 가능합니다."); | ||
| } | ||
| // 도메인 검증 (인하대 메일만 허용) | ||
| if (!googleUser.getEmail().endsWith("@inha.edu")) { | ||
| throw new IllegalArgumentException("인하대학교(@inha.edu) 계정만 이용 가능합니다."); | ||
| } | ||
|
|
||
| // DB에서 유저 조회 (OAuth Subject 기준) | ||
| User user = userRepository.findByOauthSubject(googleUser.getSub()).orElse(null); | ||
| // DB에서 유저 조회 (OAuth Subject 기준) | ||
| User user = userRepository.findByOauthSubject(googleUser.getSub()).orElse(null); | ||
|
|
||
| // 신규 유저 -> 회원가입 필요 응답 (202 or 200 with isNewUser=true) | ||
| // 신규 유저 -> 회원가입 필요 응답 (202 or 200 with isNewUser=true) | ||
| if (user == null) { | ||
| return SignupNeededResponse.builder() | ||
| .isNewUser(true) | ||
| .oauthSubject(googleUser.getSub()) | ||
| .email(googleUser.getEmail()) | ||
| .name(googleUser.getName()) | ||
| .build(); | ||
| } | ||
|
|
||
| // 기존 유저 -> 토큰 발급 및 로그인 성공 응답 | ||
| TokenDto tokens = generateTokens(user); | ||
| return LoginSuccessResponse.of(user, tokens); | ||
| String preferredName = | ||
| hasText(googleUser.getFamilyName()) ? googleUser.getFamilyName() : googleUser.getName(); | ||
| return SignupNeededResponse.builder() | ||
| .isNewUser(true) | ||
| .oauthSubject(googleUser.getSub()) | ||
| .email(googleUser.getEmail()) | ||
| .name(preferredName) | ||
| .build(); | ||
| } | ||
|
|
||
| //회원가입 | ||
| @Transactional | ||
| public LoginSuccessResponse signup(SignupRequest request) { | ||
| // 학번 중복 체크 | ||
| if (userRepository.existsByStudentId(request.getStudentId())) { | ||
| throw new IllegalArgumentException("이미 존재하는 학번입니다."); | ||
| } | ||
|
|
||
| // 전화번호 정규화 (숫자만 남김) | ||
| String cleanPhone = request.getPhoneNumber().replaceAll("[^0-9]", ""); | ||
|
|
||
| // 유저 엔티티 생성 및 저장 | ||
| User newUser = User.builder() | ||
| .oauthSubject(request.getOauthSubject()) // 구글 sub | ||
| .email(request.getEmail()) | ||
| .name(request.getName()) | ||
| .studentId(request.getStudentId()) | ||
| .major(request.getMajor()) | ||
| .phoneNumber(cleanPhone) | ||
| // Role(GUEST), Status(PENDING) 등은 User 엔티티 생성자에서 기본값 처리됨 | ||
| .build(); | ||
|
|
||
| userRepository.save(newUser); | ||
|
|
||
| // 토큰 발급 | ||
| TokenDto tokens = generateTokens(newUser); | ||
| return LoginSuccessResponse.of(newUser, tokens); | ||
| // 기존 유저 -> 토큰 발급 및 로그인 성공 응답 | ||
| TokenDto tokens = generateTokens(user); | ||
| return LoginSuccessResponse.of(tokens, AuthUserResponse.from(user)); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
login 메서드의 반환 타입이 Object로 선언되어 타입 안전성이 결여됩니다.
SignupNeededResponse 또는 LoginSuccessResponse를 반환하는데, 반환 타입이 Object이면 호출자(Controller)에서 instanceof 체크가 필요하고 컴파일 타임에 오류를 잡을 수 없습니다. sealed interface 또는 공통 상위 타입을 도입하여 타입 안전성을 확보하는 것을 권장합니다.
♻️ 예시: sealed interface 활용
public sealed interface LoginResponse permits SignupNeededResponse, LoginSuccessResponse {}그 후 반환 타입을 LoginResponse로 변경:
- public Object login(String idToken) {
+ public LoginResponse login(String idToken) {🤖 Prompt for AI Agents
In `@src/main/java/inha/gdgoc/domain/auth/service/AuthService.java` around lines
48 - 77, The login method currently returns Object; introduce a sealed interface
LoginResponse that permits SignupNeededResponse and LoginSuccessResponse, have
those two response classes implement/extend LoginResponse, change
AuthService.login(String idToken) to return LoginResponse, and update any
callers (e.g., controller) to accept LoginResponse instead of Object so callers
no longer need instanceof checks and gain compile-time type safety; ensure
SignupNeededResponse.builder() and LoginSuccessResponse.of(...) produce types
that implement LoginResponse.
| private SecretKey buildSigningKey(String rawSecret) { | ||
| byte[] candidateKey; | ||
| try { | ||
| candidateKey = Decoders.BASE64.decode(rawSecret); | ||
| } catch (IllegalArgumentException ignore) { | ||
| candidateKey = rawSecret.getBytes(StandardCharsets.UTF_8); | ||
| } | ||
|
|
||
| if (candidateKey.length < 32) { | ||
| try { | ||
| candidateKey = MessageDigest.getInstance("SHA-256").digest(candidateKey); | ||
| } catch (NoSuchAlgorithmException e) { | ||
| throw new IllegalStateException("SHA-256 algorithm not available", e); | ||
| } | ||
| } | ||
|
|
||
| return Keys.hmacShaKeyFor(candidateKey); | ||
| } |
There was a problem hiding this comment.
짧은 시크릿 키를 SHA-256으로 자동 변환하면 설정 오류가 은폐됩니다.
candidateKey.length < 32인 경우 SHA-256 해시로 대체하는 로직은, 잘못되거나 너무 짧은 시크릿이 설정되었을 때 경고 없이 동작하게 만듭니다. 프로덕션에서 약한 키가 사용되는 것을 방지하려면, 최소 길이 미달 시 명시적으로 실패(예외)시키는 것이 더 안전합니다.
🛡️ 제안: 짧은 키에 대해 예외를 던지도록 변경
if (candidateKey.length < 32) {
- try {
- candidateKey = MessageDigest.getInstance("SHA-256").digest(candidateKey);
- } catch (NoSuchAlgorithmException e) {
- throw new IllegalStateException("SHA-256 algorithm not available", e);
- }
+ throw new IllegalStateException(
+ "JWT signing key must be at least 256 bits (32 bytes). " +
+ "Provided key is only " + candidateKey.length + " bytes.");
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private SecretKey buildSigningKey(String rawSecret) { | |
| byte[] candidateKey; | |
| try { | |
| candidateKey = Decoders.BASE64.decode(rawSecret); | |
| } catch (IllegalArgumentException ignore) { | |
| candidateKey = rawSecret.getBytes(StandardCharsets.UTF_8); | |
| } | |
| if (candidateKey.length < 32) { | |
| try { | |
| candidateKey = MessageDigest.getInstance("SHA-256").digest(candidateKey); | |
| } catch (NoSuchAlgorithmException e) { | |
| throw new IllegalStateException("SHA-256 algorithm not available", e); | |
| } | |
| } | |
| return Keys.hmacShaKeyFor(candidateKey); | |
| } | |
| private SecretKey buildSigningKey(String rawSecret) { | |
| byte[] candidateKey; | |
| try { | |
| candidateKey = Decoders.BASE64.decode(rawSecret); | |
| } catch (IllegalArgumentException ignore) { | |
| candidateKey = rawSecret.getBytes(StandardCharsets.UTF_8); | |
| } | |
| if (candidateKey.length < 32) { | |
| throw new IllegalStateException( | |
| "JWT signing key must be at least 256 bits (32 bytes). " + | |
| "Provided key is only " + candidateKey.length + " bytes."); | |
| } | |
| return Keys.hmacShaKeyFor(candidateKey); | |
| } |
🤖 Prompt for AI Agents
In `@src/main/java/inha/gdgoc/global/config/jwt/TokenProvider.java` around lines
115 - 132, The buildSigningKey method in TokenProvider currently replaces short
keys by hashing them (candidateKey.length < 32) which hides configuration
errors; instead, change TokenProvider.buildSigningKey to validate the
decoded/bytes candidateKey length and throw a clear exception (e.g.,
IllegalArgumentException or IllegalStateException) when it's shorter than the
required 32 bytes, removing the SHA-256 fallback so misconfigured/too-short
secrets fail fast and surface the error.
| "/api/v1/recruit/member/apply/**", | ||
| "/api/v1/recruit/member/check/**", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd "TokenAuthenticationFilter.java" --exec cat {} \;Repository: GDGoCINHA/24-2_GDGoC_Server
Length of output: 3949
TokenAuthenticationFilter.shouldNotFilter()의 엔드포인트 경로도 함께 업데이트되어야 합니다.
이 프로젝트의 보안 아키텍처는 SecurityConfig의 .permitAll()과 TokenAuthenticationFilter.shouldNotFilter()에서 공개 엔드포인트를 이중으로 관리합니다. 그러나 현재 SecurityConfig는 /api/v1/apply/**, /api/v1/check/**에서 /api/v1/recruit/member/apply/**, /api/v1/recruit/member/check/**로 변경되었으나, TokenAuthenticationFilter의 shouldNotFilter() 메서드(40-41줄)는 여전히 기존 경로 패턴을 사용하고 있습니다.
결과적으로 새로운 엔드포인트 경로(/api/v1/recruit/member/apply/**, /api/v1/recruit/member/check/**)는 Spring Security에서는 인증 불필요로 설정되었지만, 토큰 필터링 단계에서 제외되지 않아 일관성이 깨집니다. 건축상 일관성을 유지하고 불필요한 필터 실행을 방지하려면 TokenAuthenticationFilter의 shouldNotFilter() 메서드도 동일하게 업데이트하세요.
🤖 Prompt for AI Agents
In `@src/main/java/inha/gdgoc/global/security/SecurityConfig.java` around lines 50
- 51, SecurityConfig permitAll() paths were changed to
"/api/v1/recruit/member/apply/**" and "/api/v1/recruit/member/check/**" but
TokenAuthenticationFilter.shouldNotFilter() still checks the old
"/api/v1/apply/**" and "/api/v1/check/**"; update the path patterns inside
TokenAuthenticationFilter.shouldNotFilter() to match the new patterns (e.g.,
"/api/v1/recruit/member/apply/**" and "/api/v1/recruit/member/check/**") so the
filter excludes the same public endpoints that SecurityConfig permits, keeping
behavior consistent between SecurityConfig and TokenAuthenticationFilter.
| auth: | ||
| refresh-cookie: | ||
| secure: ${REFRESH_COOKIE_SECURE:false} | ||
| same-site: ${REFRESH_COOKIE_SAME_SITE:Lax} | ||
| domain: ${REFRESH_COOKIE_DOMAIN:} | ||
| path: / | ||
| access-cookie: | ||
| secure: ${ACCESS_COOKIE_SECURE:false} | ||
| same-site: ${ACCESS_COOKIE_SAME_SITE:Lax} | ||
| domain: ${ACCESS_COOKIE_DOMAIN:} | ||
| path: / |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd "application-prod" --extension yml --exec cat {}Repository: GDGoCINHA/24-2_GDGoC_Server
Length of output: 2010
프로덕션 환경에서 쿠키 보안 설정이 취약합니다.
application-prod.yml에서 쿠키 설정이 개발 환경과 동일하게 secure: false와 same-site: Lax의 기본값을 사용하고 있습니다. 환경 변수 설정에 의존하는 것은 설정 누락 시 보안 위험을 초래할 수 있습니다.
프로덕션 환경에서는 다음을 수정해 주세요:
secure: true(기본값으로 설정)same-site: Strict또는None(HTTPS 필수)
프로덕션에서 안전한 기본값을 보장해야 합니다.
🤖 Prompt for AI Agents
In `@src/main/resources/application-dev.yml` around lines 60 - 70, Update the
production cookie defaults so they are secure even if env vars are missing: in
the production config change the auth.refresh-cookie and auth.access-cookie
entries (properties named refresh-cookie, access-cookie under auth) to set
secure: true and same-site: Strict (or same-site: None if you will enforce HTTPS
and need cross-site use); ensure these are hard-coded defaults in
application-prod.yml rather than relying solely on environment variable
fallbacks.
| auth: | ||
| refresh-cookie: | ||
| secure: ${REFRESH_COOKIE_SECURE:false} | ||
| same-site: ${REFRESH_COOKIE_SAME_SITE:Lax} | ||
| domain: ${REFRESH_COOKIE_DOMAIN:} | ||
| path: / | ||
| access-cookie: | ||
| secure: ${ACCESS_COOKIE_SECURE:false} | ||
| same-site: ${ACCESS_COOKIE_SAME_SITE:Lax} | ||
| domain: ${ACCESS_COOKIE_DOMAIN:} | ||
| path: / |
There was a problem hiding this comment.
프로덕션 환경에서 쿠키 secure 기본값이 false입니다.
프로덕션 설정 파일에서 REFRESH_COOKIE_SECURE와 ACCESS_COOKIE_SECURE의 기본값이 false로 설정되어 있습니다. 환경 변수가 누락되거나 설정되지 않으면 쿠키가 HTTP를 통해 전송되어 중간자 공격(MITM)에 의한 토큰 탈취 위험이 있습니다. application-local.yml이나 application-dev.yml에서는 false가 적절하지만, 프로덕션 설정에서는 기본값을 true로 설정하는 것이 안전합니다.
🔒 제안된 수정
auth:
refresh-cookie:
- secure: ${REFRESH_COOKIE_SECURE:false}
+ secure: ${REFRESH_COOKIE_SECURE:true}
same-site: ${REFRESH_COOKIE_SAME_SITE:Lax}
domain: ${REFRESH_COOKIE_DOMAIN:}
path: /
access-cookie:
- secure: ${ACCESS_COOKIE_SECURE:false}
+ secure: ${ACCESS_COOKIE_SECURE:true}
same-site: ${ACCESS_COOKIE_SAME_SITE:Lax}
domain: ${ACCESS_COOKIE_DOMAIN:}
path: /🤖 Prompt for AI Agents
In `@src/main/resources/application-prod.yml` around lines 60 - 70, The production
YAML sets REFRESH_COOKIE_SECURE and ACCESS_COOKIE_SECURE defaulting to false;
change the defaults so cookies are secure by default in production by updating
the auth.refresh-cookie.secure and auth.access-cookie.secure entries to default
to true (i.e., change ${REFRESH_COOKIE_SECURE:false} and
${ACCESS_COOKIE_SECURE:false} to default to true) so that when those environment
variables are missing the cookies will be sent only over HTTPS.
| audience: ${JWT_AUDIENCE} | ||
| accessTokenValidity: ${JWT_ACCESS_TOKEN_VALIDITY:3600000} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's check if JWT_ACCESS_TOKEN_VALIDITY is referenced in GitHub workflows
rg -n "ACCESS_TOKEN_VALIDITY" .github/workflows/Repository: GDGoCINHA/24-2_GDGoC_Server
Length of output: 53
🏁 Script executed:
# Also search for deploy-dev and deploy-prod files to understand .env creation
fd -e yml -e yaml | grep -E "deploy-(dev|prod)" | head -20Repository: GDGoCINHA/24-2_GDGoC_Server
Length of output: 53
🏁 Script executed:
# Check how other JWT environment variables are configured in workflows
rg -n "JWT_" .github/workflows/ -A 2 -B 2Repository: GDGoCINHA/24-2_GDGoC_Server
Length of output: 1640
🏁 Script executed:
# Look at the application-prod.yml file to see what JWT configs are there
cat -n src/main/resources/application-prod.yml | grep -A 10 -B 10 "JWT\|jwt" | head -50Repository: GDGoCINHA/24-2_GDGoC_Server
Length of output: 696
배포 환경별로 JWT_ACCESS_TOKEN_VALIDITY를 설정할 수 있도록 구성 필요합니다.
application-prod.yml에서 accessTokenValidity는 기본값(3600000ms = 1시간)으로 설정되어 있으나, .github/workflows/deploy-prod.yml과 .github/workflows/deploy-dev.yml에서 JWT_ACCESS_TOKEN_VALIDITY 환경 변수가 secrets를 통해 전달되지 않고 있습니다. 다른 JWT 설정들(JWT_AUDIENCE, GOOGLE_ISSUER, SELF_ISSUER)은 이미 secrets로 관리되고 있는데, JWT_ACCESS_TOKEN_VALIDITY도 같은 방식으로 관리하면 배포 환경별로 토큰 유효성 시간을 유연하게 조정할 수 있습니다. 현재는 기본값만으로 동작하므로 필요시 워크플로우 파일에 이 환경 변수를 추가해 주시기 바랍니다.
🤖 Prompt for AI Agents
In `@src/main/resources/application-prod.yml` around lines 79 - 80, Add support
for configuring JWT_ACCESS_TOKEN_VALIDITY via CI secrets so accessTokenValidity
in application-prod.yml can be set per environment: update the deploy-prod.yml
and deploy-dev.yml workflows to export JWT_ACCESS_TOKEN_VALIDITY from secrets
(e.g., add an env entry mapping JWT_ACCESS_TOKEN_VALIDITY: ${{
secrets.JWT_ACCESS_TOKEN_VALIDITY }} alongside
JWT_AUDIENCE/GOOGLE_ISSUER/SELF_ISSUER), ensure the corresponding secret is
created in the target repo/environment, and keep the existing
${JWT_ACCESS_TOKEN_VALIDITY:3600000} fallback in application-prod.yml unchanged
so deployments without the secret still work.
1 participant
…new requirements
📌 연관된 이슈
✨ 작업 내용
💬 리뷰 요구사항(선택)
Summary by CodeRabbit
릴리스 노트
새로운 기능
개선 사항
설정 변경