[FEAT] social_auth, usage_limit 테이블 엔티티 및 에러 선언 (#8) - #14
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthrough소셜 인증 엔티티에 토큰 해시와 검증·갱신·만료 로직을 추가하고, 소셜 로그인 유형과 오류 상태를 정의했습니다. 사용자별 일일 사용량 제한 엔티티 및 관련 오류 상태와 로컬 실행용 예시 설정도 추가했습니다. Changes인증 및 사용량 제한 도메인
Estimated code review effort: 3 (Moderate) | ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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: 3
🧹 Nitpick comments (3)
src/main/java/com/mr/domain/user/entity/enums/UserUsageErrorStatus.java (1)
13-13: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win403보단 429가 더 정확한 의미를 전달할 것 같아요.
USAGE_LIMIT_EXCEEDED를HttpStatus.FORBIDDEN(403)으로 매핑했는데, 403은 보통 "권한 없음"을 뜻합니다. 여기서 다루는 상황은 인가 실패가 아니라 일일 사용 할당량 소진이므로, RFC 6585에서 정의한429 Too Many Requests가 클라이언트에 더 정확한 시맨틱을 전달합니다. 클라이언트가 "권한이 없다"와 "오늘 할당량을 다 썼다"를 구분해서 처리할 수 있게 됩니다.🔧 제안하는 수정
- USAGE_LIMIT_EXCEEDED(HttpStatus.FORBIDDEN, "USAGE_403_01", "일일 잔여 분석 횟수를 모두 소진하였습니다."), + USAGE_LIMIT_EXCEEDED(HttpStatus.TOO_MANY_REQUESTS, "USAGE_429_01", "일일 잔여 분석 횟수를 모두 소진하였습니다."),코드값(
USAGE_403_01)도 상태 코드와 맞춰 변경이 필요하며, 이미 이 코드값을 참조하는 곳이 있다면 함께 확인해주세요.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/mr/domain/user/entity/enums/UserUsageErrorStatus.java` at line 13, Update the USAGE_LIMIT_EXCEEDED mapping to HttpStatus.TOO_MANY_REQUESTS and change its code value from USAGE_403_01 to the corresponding 429-based value. Search for references to the existing code and update them consistently.src/main/java/com/mr/domain/auth/entity/SocialAuth.java (1)
19-24: 📐 Maintainability & Code Quality | 🔵 TrivialTODO에 유니크 제약도 함께 챙겨주시면 좋을 것 같아요.
지금은
refresh_token에만 인덱스가 있고,(socialType, socialId)조합에 대한 유니크 제약은 없습니다. 이미 19행 TODO로 User 도메인 완성 시 인덱스를 추가할 계획이라고 밝혀두셨으니, 그 작업에 소셜 계정 중복 연동을 막는 유니크 제약도 함께 포함해주시면 좋겠습니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/mr/domain/auth/entity/SocialAuth.java` around lines 19 - 24, Update the `@Table` definition in SocialAuth to add a unique constraint for the (socialType, socialId) combination alongside the planned User-domain indexes, preventing duplicate social-account linkages while preserving the existing refresh_token index.src/main/java/com/mr/domain/user/entity/UsageLimit.java (1)
65-75: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
fieldName매개변수, 받아놓고 아무 일도 안 시키고 있어요.
validateRequired와validatePositiveOrZero모두fieldName파라미터를 받지만 예외 메시지에 전혀 활용하지 않아, 어떤 필드가 문제인지 로그/메시지로 구분할 수 없습니다. 또한validateRequired가public으로 열려 있어 다른 도메인(SocialAuth)에서도 정적 import로 가져다 쓰는 결합을 유발하고 있습니다(해당 파일 리뷰 참고). SocialAuth 쪽 의존을 제거한 뒤에는 이 메서드들을private으로 좁히고, 필요하다면fieldName을 메시지에 반영하는 것을 권장합니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/mr/domain/user/entity/UsageLimit.java` around lines 65 - 75, Update validateRequired and validatePositiveOrZero in UsageLimit to use fieldName in the validation error message when reporting the invalid field. After removing the SocialAuth dependency on validateRequired, narrow validateRequired from public to private, keeping both helpers encapsulated within UsageLimit.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/main/java/com/mr/domain/auth/entity/SocialAuth.java`:
- Line 15: Remove the static import and usage of user-domain
UsageLimit.validateRequired in SocialAuth, and update the constructor validation
to call the class’s local validateRequiredField method so missing userId,
socialType, or socialId raises AuthErrorStatus.INVALID_AUTH_REQUEST. Remove or
otherwise replace only the unused duplicate validation path, keeping auth
validation independent of the user domain.
- Around line 44-45: Update the SocialAuth.refreshToken persistence so the raw
refresh token is never stored; use encrypted storage when retrieval is required,
or store a deterministic HMAC-SHA256 value in a separate lookup column when only
comparison is needed. Remove idx_social_auth_refresh_token and any indexing of
plaintext tokens, and update related authentication lookup logic to use the
protected value.
In `@src/main/java/com/mr/domain/user/entity/UsageLimit.java`:
- Around line 15-19: Update the UsageLimit table mapping so the user_id and
limit_date column combination is enforced as unique at the database level,
either by making the existing idx_usage_limit_user_date index unique or by
adding an equivalent table-level unique constraint. Preserve the existing index
columns and naming where applicable.
---
Nitpick comments:
In `@src/main/java/com/mr/domain/auth/entity/SocialAuth.java`:
- Around line 19-24: Update the `@Table` definition in SocialAuth to add a unique
constraint for the (socialType, socialId) combination alongside the planned
User-domain indexes, preventing duplicate social-account linkages while
preserving the existing refresh_token index.
In `@src/main/java/com/mr/domain/user/entity/enums/UserUsageErrorStatus.java`:
- Line 13: Update the USAGE_LIMIT_EXCEEDED mapping to
HttpStatus.TOO_MANY_REQUESTS and change its code value from USAGE_403_01 to the
corresponding 429-based value. Search for references to the existing code and
update them consistently.
In `@src/main/java/com/mr/domain/user/entity/UsageLimit.java`:
- Around line 65-75: Update validateRequired and validatePositiveOrZero in
UsageLimit to use fieldName in the validation error message when reporting the
invalid field. After removing the SocialAuth dependency on validateRequired,
narrow validateRequired from public to private, keeping both helpers
encapsulated within UsageLimit.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 200181cb-4e23-4813-9fd0-e42cc3bbeb3a
📒 Files selected for processing (5)
src/main/java/com/mr/domain/auth/entity/SocialAuth.javasrc/main/java/com/mr/domain/auth/entity/enums/AuthErrorStatus.javasrc/main/java/com/mr/domain/auth/entity/enums/SocialType.javasrc/main/java/com/mr/domain/user/entity/UsageLimit.javasrc/main/java/com/mr/domain/user/entity/enums/UserUsageErrorStatus.java
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/main/java/com/mr/domain/auth/entity/SocialAuth.java (2)
100-103: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
expireToken()이refreshTokenHash를 남겨둬 만료가 반쪽짜리가 됩니다.
refreshToken과expiredAt은null로 지웠지만refreshTokenHash는 그대로 남습니다. 조회 인덱스가refresh_token_hash기준(Line 21)이라, 만료 처리된 레코드도 해시 기반 조회에는 여전히 매칭됩니다. 또한refresh_token_hash는unique(Line 45)이므로 낡은 해시가 유니크 슬롯을 계속 점유해, 동일 해시의 재발급/재사용 시 제약 충돌 가능성도 생깁니다. 만료 상태 표현이라면 해시도 함께 무효화하는 것이 일관적입니다.🔧 제안하는 수정
public void expireToken() { this.refreshToken = null; + this.refreshTokenHash = null; this.expiredAt = null; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/mr/domain/auth/entity/SocialAuth.java` around lines 100 - 103, Update SocialAuth.expireToken() to also clear refreshTokenHash along with refreshToken and expiredAt, ensuring expired records no longer match hash-based lookups or retain the unique hash value.
70-80: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
create(...)가 토큰 해시를 채우지 않아updateRefreshToken(...)과 비대칭이에요.
updateRefreshToken은encryptedToken과tokenHash를 함께 받아refreshToken/refreshTokenHash를 세팅하고 만료 시간까지 검증하는데,create는 원본refreshToken을 그대로 저장하고refreshTokenHash는 손대지 않습니다. 결과적으로:
- 조회 인덱스인
idx_social_auth_token_hash(Line 21)가refresh_token_hash를 대상으로 하는데, 생성 직후 레코드는 해시가null이라 해시 기반 조회로 찾을 수 없습니다.- 원본 토큰이 평문으로 저장되어, 이전 리뷰에서 지적된 리프레시 토큰 보호(HMAC/암호화) 문제가 생성 경로에서는 여전히 남아 있습니다.
create에는newExpiredAt같은 만료 시간 유효성 검증이 없어 과거 시각도 그대로 통과됩니다.
create도updateRefreshToken과 동일하게encryptedToken+tokenHash(+ 만료 검증)를 받도록 시그니처를 맞추는 것을 권합니다. 참고: OAuth 2.0 Security BCP, OWASP Cheat Sheet Series(토큰 저장).🔧 제안하는 방향
- public static SocialAuth create(Long userId, SocialType socialType, String socialId, - String refreshToken, LocalDateTime expiredAt, String deviceInfo) { + public static SocialAuth create(Long userId, SocialType socialType, String socialId, + String encryptedToken, String tokenHash, + LocalDateTime expiredAt, String deviceInfo) { + // encryptedToken / tokenHash 필수 및 expiredAt 유효성 검증을 updateRefreshToken과 동일하게 적용 return SocialAuth.builder() .userId(userId) .socialType(socialType) .socialId(socialId) - .refreshToken(refreshToken) + .refreshToken(encryptedToken) + // refreshTokenHash 세팅을 위한 빌더 필드 추가 필요 .expiredAt(expiredAt) .deviceInfo(deviceInfo) .build(); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/mr/domain/auth/entity/SocialAuth.java` around lines 70 - 80, Update SocialAuth.create to accept encryptedToken and tokenHash like updateRefreshToken, store them in refreshToken and refreshTokenHash, and validate expiredAt using the same expiration check as updateRefreshToken before building the entity. Preserve the existing user, social type, social ID, and device information assignments.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/main/java/com/mr/domain/auth/entity/SocialAuth.java`:
- Around line 100-103: Update SocialAuth.expireToken() to also clear
refreshTokenHash along with refreshToken and expiredAt, ensuring expired records
no longer match hash-based lookups or retain the unique hash value.
- Around line 70-80: Update SocialAuth.create to accept encryptedToken and
tokenHash like updateRefreshToken, store them in refreshToken and
refreshTokenHash, and validate expiredAt using the same expiration check as
updateRefreshToken before building the entity. Preserve the existing user,
social type, social ID, and device information assignments.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 56e6aa7f-e066-453b-9623-19677166079e
📒 Files selected for processing (2)
src/main/java/com/mr/domain/auth/entity/SocialAuth.javasrc/main/java/com/mr/domain/user/entity/UsageLimit.java
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/com/mr/domain/user/entity/UsageLimit.java
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/main/java/com/mr/domain/auth/entity/SocialAuth.java (2)
69-84: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value필드 이름
refreshToken이 실제로는 암호화된 토큰을 담고 있어 오해의 소지가 있어요.
create(...)는encryptedToken을 받아refreshToken필드에 넣고 있습니다. 파라미터명은 "암호화됨"을 말하는데 필드명은 "평문 리프레시 토큰"처럼 읽혀, 나중에 이 필드를 그대로 비교/전송하는 실수를 유발하기 쉽습니다. 필드명을encryptedRefreshToken(컬럼명은 유지)으로 바꾸면 저장 값의 성격이 코드에서도 분명해집니다.♻️ 제안
- `@Column`(name = "refresh_token", length = 1000) - private String refreshToken; + `@Column`(name = "refresh_token", length = 1000) + private String encryptedRefreshToken;이름으로 의도를 드러내는 것은 클린 코드의 "의도를 드러내는 이름" 원칙과도 맞닿아 있습니다.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/mr/domain/auth/entity/SocialAuth.java` around lines 69 - 84, Rename the SocialAuth entity field and its accessors from refreshToken to encryptedRefreshToken to reflect that create(...) stores encryptedToken, while preserving the existing database column name. Update all references, mappings, builders, and usages consistently without changing the stored value or behavior.
90-94: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value만료 검증이 시스템 기본 타임존과
now()에 직접 묶여 있어요.
LocalDateTime.now()는 JVM 기본 타임존에 의존하고 테스트에서 시간을 고정하기 어렵습니다.Clock을 주입하거나 UTC 기준(LocalDateTime.now(ZoneOffset.UTC))으로 통일하면 서버 타임존이 바뀌어도 만료 판정이 일관되고, 단위 테스트에서 경계값을 재현하기 쉬워집니다.expiredAt필드가 서비스 계층에서 어떤 타임존 기준으로 채워지는지도 함께 맞춰주세요.
java.time.Clock문서를 참고하시면 시간 의존성 주입 패턴을 잡는 데 도움이 됩니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/mr/domain/auth/entity/SocialAuth.java` around lines 90 - 94, Update validateExpiryTime in SocialAuth to avoid the system-default LocalDateTime.now() dependency by using an injected Clock or the application’s established UTC time standard. Ensure expiredAt is populated and compared using the same timezone basis, while preserving null and already-expired token rejection and enabling deterministic boundary-time tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/main/java/com/mr/domain/auth/entity/SocialAuth.java`:
- Around line 69-84: Rename the SocialAuth entity field and its accessors from
refreshToken to encryptedRefreshToken to reflect that create(...) stores
encryptedToken, while preserving the existing database column name. Update all
references, mappings, builders, and usages consistently without changing the
stored value or behavior.
- Around line 90-94: Update validateExpiryTime in SocialAuth to avoid the
system-default LocalDateTime.now() dependency by using an injected Clock or the
application’s established UTC time standard. Ensure expiredAt is populated and
compared using the same timezone basis, while preserving null and
already-expired token rejection and enabling deterministic boundary-time tests.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 154e32ff-17ef-430c-9441-06f2c299edde
📒 Files selected for processing (1)
src/main/java/com/mr/domain/auth/entity/SocialAuth.java
…-auth-entities # Please enter a commit message to explain why this merge is necessary, # especially if it merges an updated upstream into a topic branch. # # Lines starting with '#' will be ignored, and an empty message aborts # the commit.
ownue
left a comment
ownue
left a comment
There was a problem hiding this comment.
코멘트들 확인 부탁드립니다!
이건 예원님이 수정하실 사항은 아니나, 올려주신 PR 보고 제 파트에서 수정할 것들이 생각나 기록해둡니다...
(1) 제 docker-compose.local.yml 파일에 PostgreSQL 버전이 달랐던 것 같아서 수정하도록 해야겟네요...
(2) AI 멘토 채팅 횟수도 제한을 둬야 하는데 이걸 테이블로 관리할지 고민을 해야겠어요...ㅇ.ㅇ!!
kimyw1018
changed the title
| @Column(name = "social_id", nullable = false, length = 100) | ||
| private String socialId; | ||
|
|
||
| @Column(name = "refresh_token", length = 1000) |
There was a problem hiding this comment.
길이 제한이 1000으로 되어 있는데, 혹시 TEXT 타입을 쓰는 방법도 가능한가요? 제가 토큰의 길이가 어느 정도 인지 잘 몰라서 여쭈어봅니다!
There was a problem hiding this comment.
PostgreSQL에서는 TEXT 타입을 사용해도 괜찮습니다! TEXT와 VARCHAR(n) 사이에 성능상 큰 차이는 없어서, refresh token의 최대 길이가 명확하지 않다면 TEXT로 두는 것도 안전한 선택일 것 같습니다. 반대로 현재 사용하는 토큰이 1,000자를 넘지 않는 것이 보장되고, DB 차원에서 길이 제한을 명시하고 싶다면 지금처럼 VARCHAR(1000)을 유지해도 괜찮을 것 같습니다!
사용 중인 OAuth 제공자의 안내는 다음과 같습니다.
- 카카오는 access token과 refresh token의 길이가 상황에 따라 달라질 수 있으므로 length 100 이상을 권장하고 있습니다.
- Google은 refresh token의 최대 크기를 512바이트로 안내하고 있으며, 해당 한도 내에서 토큰 크기가 달라질 수 있다고 명시하고 있습니다.
현재 카카오와 Google 토큰을 저장하는 용도라면 VARCHAR(1000)으로도 충분해 보이지만, 특별히 길이를 제한해야 하는 요구사항이 없다면 TEXT로 두어 향후 토큰 형식 변경에 유연하게 대응하는 것도 괜찮겠네용
[참고한 공식 답변/문서]
| @NoArgsConstructor(access = AccessLevel.PROTECTED) | ||
| public class UsageLimit extends BaseCreatedEntity { | ||
|
|
||
| private static final int DEFAULT_MAX_FREE_COUNT = 3; |
There was a problem hiding this comment.
추후에 횟수가 변경될 가능성도 고려하면 좋을 것 같습니다!
4 participants
📍 개요
⛓️💥 관련 이슈
🛠️ 작업 내용
SocialAuth엔티티 생성 (SocialType Enum 매핑 적용 및 기기 정보/만료 시간 반영)UsageLimit엔티티 생성 (기본 3회 제한 값 설정)updateRefreshToken,expireToken,consume,updateRemainingCount)를 통한 상태 변경 보장create,createDefault) 및 외부 생성 차단(AccessLevel.PROTECTED/PRIVATE빌더) 적용GeneralException과BaseCode규격을 완벽하게 따르는AuthErrorStatus및UserUsageErrorStatus구현🔥 리뷰 요청 사항
IllegalArgumentException등)를 던지거나 공통 에러에 뭉뚱그리지 않고, 우리가 만든 공통GeneralException구조에 각 도메인 전용 구체적 에러 코드(예:AuthErrorStatus.TOKEN_MISSING,UserUsageErrorStatus.USAGE_LIMIT_EXCEEDED)를 바인딩하여 처리하도록 구현했습니다. 이 검증 흐름이 도메인 관점에서 어색하지 않은지 봐주세요!✅ 체크리스트
📎 참고 사항
Summary by CodeRabbit