Feat: 로그인 기능 추가 #115
Feat: 로그인 기능 추가 #115
Conversation
mjy926
commented
Nov 9, 2025
mjy926
commented
- 가입할 때 username, password 필드를 추가했습니다.
- 토큰 방식의 인증을 추가했습니다.
- 현재 유저를 얻을 때 토큰을 사용하도록 했습니다.
WalkthroughJWT 기반 인증 추가: 로그인/리프레시 토큰 발급·갱신·차단, Argon2 비밀번호 해싱 도입, User 모델에 username/password 추가, SSO 헤더에서 Bearer 토큰 방식으로 전환, 관련 라우터·마이그레이션·테스트 업데이트 및 의존성 추가. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant LoginAPI as POST /v3/auth/login
participant AuthService
participant AuthRepo
participant DB
participant PasswordSvc as PasswordService
participant JWT
User->>LoginAPI: LoginRequest(username, password)
LoginAPI->>AuthService: login(username, password)
AuthService->>AuthRepo: get_user_by_username(username)
AuthRepo->>DB: SELECT user
DB-->>AuthRepo: User | None
alt 사용자 없음 또는 비밀번호 불일치
AuthService-->>LoginAPI: UserNotFoundException (404)
LoginAPI-->>User: 404
else 인증 성공
AuthService->>PasswordSvc: verify_password(password, user.password)
PasswordSvc-->>AuthService: true
AuthService->>JWT: issue_token(user_id, 24h, "access")
JWT-->>AuthService: access_token
AuthService->>JWT: issue_token(user_id, 168h, "refresh")
JWT-->>AuthService: refresh_token
AuthService-->>LoginAPI: TokenResponse(access_token, refresh_token)
LoginAPI-->>User: TokenResponse
end
sequenceDiagram
participant User
participant RefreshAPI as POST /v3/auth/refresh
participant AuthService
participant JWT
participant AuthRepo
participant DB
User->>RefreshAPI: Authorization: Bearer {refresh_token}
RefreshAPI->>AuthService: refresh_token(refresh_token)
AuthService->>JWT: decode_token(refresh_token)
alt 토큰 유효성 실패
JWT-->>AuthService: raise InvalidTokenException (401)
AuthService-->>RefreshAPI: 401
RefreshAPI-->>User: 401
else 토큰 유효
AuthService->>AuthRepo: is_blocked_token(refresh_token)
AuthRepo->>DB: SELECT blocked_token
DB-->>AuthRepo: exists?
alt 이미 차단됨
AuthService-->>RefreshAPI: InvalidTokenException (401)
RefreshAPI-->>User: 401
else 차단되지 않음
AuthService->>AuthRepo: block_token(refresh_token)
AuthRepo->>DB: INSERT blocked_token
AuthService->>JWT: issue_token(user_id, 24h, "access")
AuthService->>JWT: issue_token(user_id, 168h, "refresh")
AuthService-->>RefreshAPI: TokenResponse(new_access, new_refresh)
RefreshAPI-->>User: TokenResponse
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~70 minutes
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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: 5
🧹 Nitpick comments (2)
wacruit/src/apps/common/security.py (1)
13-17: 추가 예외를 처리해 주세요.
PasswordHasher.verify는VerifyMismatchError말고도InvalidHash등의 예외를 던질 수 있어 데이터가 손상되면 401 대신 500 을 내보내게 됩니다. 로그인 흐름이 흔들리지 않도록 해당 예외들도 잡아 False 를 반환하도록 보완하는 편이 안전합니다.예:
-from argon2.exceptions import VerifyMismatchError +from argon2.exceptions import InvalidHash, VerifyMismatchError ... - except VerifyMismatchError: + except (VerifyMismatchError, InvalidHash): return Falsewacruit/src/apps/user/services.py (1)
47-48: 주입한 PasswordService를 실제로 활용해주세요DI로 password_service를 주입했는데,
create_user에서는 여전히 클래스 메서드를 직접 호출하고 있어 의존성 교체·모킹이 불가능합니다. 주입한 인스턴스를 사용하도록 바꿔 주세요.- password=PasswordService.hash_password(request.password), + password=self.password_service.hash_password(request.password),
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (18)
.env.test(1 hunks)pyproject.toml(1 hunks)wacruit/src/apps/auth/__init__.py(1 hunks)wacruit/src/apps/auth/exceptions.py(1 hunks)wacruit/src/apps/auth/models.py(1 hunks)wacruit/src/apps/auth/repositories.py(1 hunks)wacruit/src/apps/auth/schemas.py(1 hunks)wacruit/src/apps/auth/services.py(1 hunks)wacruit/src/apps/auth/views.py(1 hunks)wacruit/src/apps/common/security.py(1 hunks)wacruit/src/apps/router.py(2 hunks)wacruit/src/apps/user/dependencies.py(1 hunks)wacruit/src/apps/user/models.py(1 hunks)wacruit/src/apps/user/schemas.py(1 hunks)wacruit/src/apps/user/services.py(3 hunks)wacruit/src/apps/user/views.py(1 hunks)wacruit/src/database/migrations/versions/2025_11_03_0010-13ccaabc3ec3_add_auth_info.py(1 hunks)wacruit/src/settings.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (8)
wacruit/src/apps/auth/views.py (2)
wacruit/src/apps/auth/schemas.py (2)
LoginRequest(4-6)TokenResponse(9-11)wacruit/src/apps/auth/services.py (3)
AuthService(19-82)login(35-45)refresh_token(47-59)
wacruit/src/apps/auth/repositories.py (4)
wacruit/src/apps/auth/models.py (1)
BlockedToken(8-12)wacruit/src/apps/user/models.py (1)
User(20-59)wacruit/src/database/connection.py (2)
get_db_session(38-45)Transaction(48-69)wacruit/src/apps/auth/services.py (2)
get_user_by_id(32-33)block_token(61-65)
wacruit/src/apps/auth/exceptions.py (1)
wacruit/src/apps/common/exceptions.py (1)
WacruitException(6-24)
wacruit/src/apps/auth/schemas.py (2)
wacruit/src/apps/auth/services.py (1)
refresh_token(47-59)wacruit/src/apps/auth/views.py (1)
refresh_token(28-34)
wacruit/src/apps/user/views.py (4)
wacruit/src/tests/recruiting/conftest.py (1)
user_service(113-114)wacruit/src/tests/resume/conftest.py (1)
user_service(125-126)wacruit/src/tests/user/conftest.py (1)
user_service(29-30)wacruit/src/apps/user/services.py (1)
create_user(34-53)
wacruit/src/apps/user/services.py (3)
wacruit/src/apps/common/security.py (2)
PasswordService(5-17)hash_password(9-10)wacruit/src/apps/user/views.py (1)
create_user(47-51)wacruit/src/apps/user/schemas.py (2)
UserCreateRequest(12-24)UserCreateUpdateResponse(39-50)
wacruit/src/apps/auth/services.py (5)
wacruit/src/apps/auth/exceptions.py (2)
InvalidTokenException(9-11)UserNotFoundException(4-6)wacruit/src/apps/auth/repositories.py (5)
AuthRepository(12-39)get_user_by_id(24-25)get_user_by_username(21-22)is_blocked_token(27-33)block_token(35-39)wacruit/src/apps/common/security.py (2)
PasswordService(5-17)verify_password(13-17)wacruit/src/apps/user/models.py (1)
User(20-59)wacruit/src/secrets.py (3)
AWSSecretManager(12-37)is_available(21-30)get_secret(32-37)
wacruit/src/apps/user/dependencies.py (2)
wacruit/src/apps/auth/services.py (2)
AuthService(19-82)decode_token(67-73)wacruit/src/apps/user/repositories.py (1)
UserRepository(10-50)
🪛 Ruff (0.14.3)
wacruit/src/apps/user/services.py
24-24: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
wacruit/src/settings.py
12-12: Possible hardcoded password assigned to: "TOKEN_SECRET"
(S105)
wacruit/src/apps/auth/services.py
71-71: Consider moving this statement to an else block
(TRY300)
🔇 Additional comments (8)
.env.test (1)
7-7: 테스트 환경용 시크릿 설정 확인테스트 환경에서
TOKEN_SECRET=test를 사용하는 것은 괜찮습니다. 다만 프로덕션 환경에서는 반드시 강력한 시크릿을 사용해야 합니다.wacruit/src/apps/auth/__init__.py (1)
1-1: LGTM!인증 라우터를 패키지 레벨에서 재내보내기하는 표준적인 패턴입니다.
wacruit/src/apps/router.py (2)
4-4: LGTM!인증 모듈 임포트가 적절하게 추가되었습니다.
41-41: LGTM!인증 라우터가 API 라우터에 올바르게 포함되었습니다.
wacruit/src/apps/auth/schemas.py (2)
4-6: LGTM!로그인 요청 스키마가 적절하게 정의되었습니다.
9-11: LGTM!토큰 응답 스키마가 표준 JWT 패턴을 따르고 있습니다.
pyproject.toml (1)
31-32: 의존성 버전이 최신이며 보안 취약점이 모두 해결됨검증 결과:
argon2-cffi25.1.0: 최신 버전이며 알려진 보안 취약점 없음authlib1.6.5: 최신 버전이며, 이 버전에서 모든 알려진 취약점(JWE zip=DEF DoS, Oversized JOSE DoS, JWS/JWT crit headers, 알고리즘 혼동)이 해결됨현재 지정된 버전(
^25.1.0,^1.6.5)은 보안상 안전하며 추가 조치가 필요하지 않습니다.wacruit/src/apps/user/models.py (1)
40-42: 응답 스키마에서 password 필드 제외 확인 완료검증 결과,
password필드는 응답 스키마에서 올바르게 제외되어 있습니다:
UserCreateUpdateResponse(line 39-50): password 필드 없음UserDetailResponse(line 59-75): password 필드 없음Response 클래스들이 명시적으로 필드를 정의하여 ORM 객체에서 필요한 필드만 선택적으로 노출하고 있으므로, 민감한 인증 정보가 API 응답에서 누출되지 않습니다.
| waffle_credentials: Annotated[HTTPAuthorizationCredentials, Security(security)], | ||
| auth_service: Annotated[AuthService, Depends()], | ||
| ) -> User: | ||
| user = user_repository.get_user_by_sso_id(waffle_user_id) | ||
| waffle_token = waffle_credentials.credentials | ||
| user_id = auth_service.decode_token(waffle_token)["sub"] | ||
| user = auth_service.get_user_by_id(user_id) | ||
| if user is None: | ||
| raise UserPermissionDeniedException |
There was a problem hiding this comment.
refresh 토큰이 보호 API에 그대로 통과합니다
get_current_user는 단순히 sub만 꺼내기 때문에 refresh 토큰(심지어 이미 차단된 토큰)도 Authorization 헤더에 넣으면 그대로 인증이 됩니다. refresh 토큰 탈취만으로 모든 보호 API가 열려 버려서 치명적인 취약점입니다. issue_token에서 토큰 유형을 구분해 넣고, 여기서는 token_type == "access"인지와 블록 리스트 여부를 검증하도록 바꿔 주세요(예: AuthService에 is_blocked_token 헬퍼 추가).
- user_id = auth_service.decode_token(waffle_token)["sub"]
+ claims = auth_service.decode_token(waffle_token)
+ if claims.get("token_type") != "access":
+ raise UserPermissionDeniedException
+ if auth_service.is_blocked_token(waffle_token):
+ raise UserPermissionDeniedException
+ user_id = claims["sub"]Committable suggestion skipped: line range outside the PR's diff.
| username: str = Field(max_length=50) | ||
| password: str = Field(max_length=50) |
There was a problem hiding this comment.
비밀번호 검증 로직 누락
비밀번호 필드에 최소 길이나 복잡도 요구사항이 없어 보안이 취약합니다. 약한 비밀번호 사용을 방지하기 위한 검증 로직이 필요합니다.
다음과 같이 비밀번호 검증을 추가하는 것을 권장합니다:
+from pydantic import validator
+
class UserCreateRequest(BaseModel):
first_name: str = Field(..., max_length=30)
last_name: str = Field(..., max_length=30)
phone_number: str = Field(max_length=30)
email: EmailStr
department: str | None = Field(default=None, max_length=50)
college: str | None = Field(default=None, max_length=50)
university: str | None = Field(default=None, max_length=50)
- username: str = Field(max_length=50)
- password: str = Field(max_length=50)
+ username: str = Field(min_length=3, max_length=50)
+ password: str = Field(min_length=8, max_length=50)
+
+ @validator('password')
+ def validate_password(cls, v):
+ if len(v) < 8:
+ raise ValueError('비밀번호는 최소 8자 이상이어야 합니다')
+ if not any(c.isupper() for c in v):
+ raise ValueError('비밀번호는 최소 1개의 대문자를 포함해야 합니다')
+ if not any(c.islower() for c in v):
+ raise ValueError('비밀번호는 최소 1개의 소문자를 포함해야 합니다')
+ if not any(c.isdigit() for c in v):
+ raise ValueError('비밀번호는 최소 1개의 숫자를 포함해야 합니다')
+ return v📝 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.
| username: str = Field(max_length=50) | |
| password: str = Field(max_length=50) | |
| from pydantic import validator, EmailStr, BaseModel, Field | |
| class UserCreateRequest(BaseModel): | |
| first_name: str = Field(..., max_length=30) | |
| last_name: str = Field(..., max_length=30) | |
| phone_number: str = Field(max_length=30) | |
| email: EmailStr | |
| department: str | None = Field(default=None, max_length=50) | |
| college: str | None = Field(default=None, max_length=50) | |
| university: str | None = Field(default=None, max_length=50) | |
| username: str = Field(min_length=3, max_length=50) | |
| password: str = Field(min_length=8, max_length=50) | |
| @validator('password') | |
| def validate_password(cls, v): | |
| if len(v) < 8: | |
| raise ValueError('비밀번호는 최소 8자 이상이어야 합니다') | |
| if not any(c.isupper() for c in v): | |
| raise ValueError('비밀번호는 최소 1개의 대문자를 포함해야 합니다') | |
| if not any(c.islower() for c in v): | |
| raise ValueError('비밀번호는 최소 1개의 소문자를 포함해야 합니다') | |
| if not any(c.isdigit() for c in v): | |
| raise ValueError('비밀번호는 최소 1개의 숫자를 포함해야 합니다') | |
| return v |
🤖 Prompt for AI Agents
wacruit/src/apps/user/schemas.py around lines 23-24: the password field
currently has no strength/length validation; change the Field to enforce a
minimum length and complexity (e.g., use pydantic.constr with min_length=8 and a
regex for required character classes) or add a @validator for password that
checks min length and complexity (uppercase, lowercase, digit, special) and
raises a ValueError with a clear message on failure; update or add unit tests to
cover valid and invalid passwords.
| op.add_column("user", sa.Column("username", sa.String(length=50), nullable=False)) | ||
| op.add_column("user", sa.Column("password", sa.String(length=255), nullable=False)) |
There was a problem hiding this comment.
기존 레코드 때문에 마이그레이션이 실패합니다.
이미 user 테이블에 데이터가 존재하면, 값이 없는 상태에서 nullable=False 컬럼을 바로 추가하는 순간 마이그레이션이 즉시 실패합니다. 기존 계정의 username/password 값을 채우는 단계가 없으니, 컬럼을 우선 nullable 로 추가한 뒤 데이터를 채우고 마지막에 nullable=False 로 바꾸거나, server_default 를 둔 뒤 후속 스텝에서 기본값을 제거하는 식으로 안전장치를 넣어 주세요.
가능한 수정 예시는 다음과 같습니다.
- op.add_column("user", sa.Column("username", sa.String(length=50), nullable=False))
- op.add_column("user", sa.Column("password", sa.String(length=255), nullable=False))
+ op.add_column("user", sa.Column("username", sa.String(length=50), nullable=True))
+ op.add_column("user", sa.Column("password", sa.String(length=255), nullable=True))
+ # TODO: existing rows 에 대해 username/password 를 채우는 데이터 마이그레이션 추가
+ op.alter_column("user", "username", nullable=False)
+ op.alter_column("user", "password", nullable=False)🤖 Prompt for AI Agents
In
wacruit/src/database/migrations/versions/2025_11_03_0010-13ccaabc3ec3_add_auth_info.py
around lines 26-27, the migration adds non-nullable username/password columns
which will fail if user rows exist; modify the migration to (a) add the two
columns as nullable first (or add them with a safe server_default), (b) run an
UPDATE to populate a valid value for existing rows (e.g. generate unique
usernames or set a placeholder and hash a generated password) within the same
migration, and then (c) alter the columns to set nullable=False (or remove the
server_default) as a final step so existing records are populated before
enforcing NOT NULL.
| class Settings(BaseSettings): | ||
| env: Literal["dev", "prod", "local", "test"] = "local" | ||
| sql_echo: bool = False | ||
| TOKEN_SECRET: str = "secret" |
There was a problem hiding this comment.
취약한 기본 시크릿 값 사용 - 보안 취약점
TOKEN_SECRET의 기본값이 "secret"으로 설정되어 있어 심각한 보안 취약점이 됩니다. JWT 토큰 서명에 사용되는 이 값이 약하면 공격자가 토큰을 위조할 수 있습니다.
다음과 같이 수정하여 프로덕션 환경에서 반드시 환경 변수를 설정하도록 강제하세요:
- TOKEN_SECRET: str = "secret"
+ TOKEN_SECRET: str또는 개발 환경에서만 기본값을 허용하고 프로덕션에서는 검증을 추가하세요:
TOKEN_SECRET: str = "secret"
def __init__(self, **kwargs):
super().__init__(**kwargs)
if self.is_prod and self.TOKEN_SECRET == "secret":
raise ValueError("TOKEN_SECRET must be set in production environment")🧰 Tools
🪛 Ruff (0.14.3)
12-12: Possible hardcoded password assigned to: "TOKEN_SECRET"
(S105)
🤖 Prompt for AI Agents
wacruit/src/settings.py around lines 12 to 12: TOKEN_SECRET is set to the
insecure literal "secret"; change it to read from an environment variable and
require a non-default value in production. Update the setting to load
TOKEN_SECRET from os.environ (or your config loader) with no insecure hardcoded
default, and add a runtime check that if running in production (e.g., is_prod
flag or ENV == "production") and TOKEN_SECRET is empty or still the unsafe
default, raise an exception so the app fails fast until a proper secret is
provided.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
wacruit/src/tests/announcement/conftest.py (1)
18-30: 중복 sso_id로 인한 잠재적 충돌user와 admin_user 픽스처 모두 동일한
sso_id="abcdef123"를 사용하고 있습니다. User 모델에서 sso_id에 대한 unique 제약이 있을 경우, 두 픽스처를 동시에 사용하는 테스트에서 IntegrityError가 발생할 수 있습니다.다음과 같이 수정하는 것을 권장합니다:
@pytest.fixture def admin_user(db_session: Session) -> User: user = User( - sso_id="abcdef123", + sso_id="admin_sso_id", first_name="Test", last_name="User", phone_number="010-0000-0000", email="example@email.com", is_admin=True, username="admin", password=PasswordService.hash_password("password123"), )Also applies to: 34-47
wacruit/src/apps/user/services.py (1)
32-46: SSO 식별자를 저장하지 않아 기존 플로우가 깨집니다. 여전히check_signup등 여러 경로에서sso_id로 가입 여부를 판별하고 있는데,create_user가 더 이상sso_id를 받아 저장하지 않아 새로 가입한 사용자도 SSO 기준으로는 존재하지 않는 것으로 취급됩니다. 이 상태로 배포하면 기존 SSO 체크가 전부 실패합니다.create_user시그니처에sso_id인자를 유지하고User(sso_id=...)로 저장하거나, 요청 스키마에sso_id를 포함하도록 전체 플로우를 맞춰 주세요.
♻️ Duplicate comments (2)
wacruit/src/apps/user/dependencies.py (1)
20-26: 차단된 토큰 검증이 여전히 누락되어 있습니다.decode_token으로 타입은 검사하지만, 블록리스트를 확인하지 않아 refresh 토큰을 차단한 뒤에도 그대로 Authorization 헤더에 넣으면 인증이 성공합니다. 지난 코멘트에서도 동일하게 지적된 부분으로,auth_service.is_blocked_token(waffle_token)같은 검사를 추가한 뒤 차단된 토큰이면UserPermissionDeniedException을 던지도록 보완해 주세요.wacruit/src/apps/auth/services.py (1)
47-63: 잘못된 token_type 검증 시 예외 타입을 수정하세요Line 50에서
token_type이"refresh"가 아닐 때UserNotFoundException을 발생시키고 있습니다. 이는 사용자 부재가 아닌 토큰 오류이므로InvalidTokenException을 발생시켜야 의미상 정확합니다. 현재 구현은 공격자에게 혼란스러운 정보를 제공할 수 있습니다.다음 diff를 적용하세요:
def refresh_token(self, refresh_token: str) -> tuple[str, str]: decoded_token = self.decode_token(refresh_token) if decoded_token["token_type"] != "refresh": - raise UserNotFoundException() + raise InvalidTokenException() user_id = decoded_token["sub"] user = self.auth_repository.get_user_by_id(user_id)
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
wacruit/src/apps/auth/services.py(1 hunks)wacruit/src/apps/user/dependencies.py(1 hunks)wacruit/src/apps/user/models.py(1 hunks)wacruit/src/apps/user/services.py(3 hunks)wacruit/src/database/migrations/versions/2025_11_03_0010-13ccaabc3ec3_add_auth_info.py(1 hunks)wacruit/src/tests/announcement/conftest.py(3 hunks)wacruit/src/tests/announcement/test_announcement_views.py(1 hunks)wacruit/src/tests/models/conftest.py(2 hunks)wacruit/src/tests/models/test_user.py(2 hunks)wacruit/src/tests/portfolio/file/conftest.py(2 hunks)wacruit/src/tests/portfolio/url/conftest.py(3 hunks)wacruit/src/tests/recruiting/conftest.py(2 hunks)wacruit/src/tests/resume/conftest.py(2 hunks)wacruit/src/tests/user/conftest.py(2 hunks)wacruit/src/tests/user/test_user_service.py(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- wacruit/src/apps/user/models.py
🧰 Additional context used
🧬 Code graph analysis (13)
wacruit/src/apps/auth/services.py (6)
wacruit/src/apps/auth/exceptions.py (2)
InvalidTokenException(9-11)UserNotFoundException(4-6)wacruit/src/apps/auth/repositories.py (5)
AuthRepository(12-39)get_user_by_id(24-25)get_user_by_username(21-22)is_blocked_token(27-33)block_token(35-39)wacruit/src/apps/common/security.py (2)
PasswordService(5-17)verify_password(13-17)wacruit/src/apps/user/models.py (1)
User(20-59)wacruit/src/secrets.py (3)
AWSSecretManager(12-37)is_available(21-30)get_secret(32-37)wacruit/src/apps/auth/views.py (2)
login(19-24)refresh_token(28-34)
wacruit/src/tests/resume/conftest.py (1)
wacruit/src/apps/common/security.py (2)
PasswordService(5-17)hash_password(9-10)
wacruit/src/tests/models/test_user.py (1)
wacruit/src/apps/common/security.py (2)
PasswordService(5-17)hash_password(9-10)
wacruit/src/tests/user/conftest.py (1)
wacruit/src/apps/common/security.py (2)
PasswordService(5-17)hash_password(9-10)
wacruit/src/apps/user/dependencies.py (2)
wacruit/src/apps/auth/services.py (3)
AuthService(19-87)decode_token(71-77)get_user_by_id(32-33)wacruit/src/apps/user/repositories.py (1)
UserRepository(10-50)
wacruit/src/tests/portfolio/url/conftest.py (1)
wacruit/src/apps/common/security.py (2)
PasswordService(5-17)hash_password(9-10)
wacruit/src/tests/portfolio/file/conftest.py (1)
wacruit/src/apps/common/security.py (2)
PasswordService(5-17)hash_password(9-10)
wacruit/src/tests/announcement/test_announcement_views.py (3)
wacruit/src/tests/conftest.py (1)
test_client(55-64)wacruit/src/tests/announcement/conftest.py (1)
admin_user(34-47)wacruit/src/apps/user/models.py (1)
User(20-59)
wacruit/src/tests/user/test_user_service.py (6)
wacruit/src/apps/common/security.py (1)
PasswordService(5-17)wacruit/src/tests/user/conftest.py (1)
user_service(32-33)wacruit/src/apps/user/services.py (2)
create_user(32-51)UserService(20-97)wacruit/src/apps/user/views.py (1)
create_user(47-51)wacruit/src/apps/user/repositories.py (1)
create_user(34-37)wacruit/src/apps/user/schemas.py (1)
UserCreateRequest(12-24)
wacruit/src/tests/announcement/conftest.py (1)
wacruit/src/apps/common/security.py (2)
PasswordService(5-17)hash_password(9-10)
wacruit/src/tests/recruiting/conftest.py (1)
wacruit/src/apps/common/security.py (2)
PasswordService(5-17)hash_password(9-10)
wacruit/src/apps/user/services.py (4)
wacruit/src/apps/common/security.py (2)
PasswordService(5-17)hash_password(9-10)wacruit/src/apps/user/views.py (1)
create_user(47-51)wacruit/src/apps/user/repositories.py (1)
create_user(34-37)wacruit/src/apps/user/schemas.py (2)
UserCreateRequest(12-24)UserCreateUpdateResponse(39-50)
wacruit/src/tests/models/conftest.py (1)
wacruit/src/apps/common/security.py (2)
PasswordService(5-17)hash_password(9-10)
🪛 Ruff (0.14.3)
wacruit/src/apps/auth/services.py
49-49: Possible hardcoded password assigned to: "token_type"
(S105)
75-75: Consider moving this statement to an else block
(TRY300)
wacruit/src/apps/user/dependencies.py
22-22: Possible hardcoded password assigned to: "token_type"
(S105)
wacruit/src/tests/user/test_user_service.py
24-24: Possible hardcoded password assigned to argument: "password"
(S106)
37-37: Possible hardcoded password assigned to argument: "password"
(S106)
96-96: Possible hardcoded password assigned to argument: "password"
(S106)
113-113: Possible hardcoded password assigned to argument: "password"
(S106)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: pytest
🔇 Additional comments (13)
wacruit/src/tests/recruiting/conftest.py (1)
6-6: 테스트 픽스처 인증 필드 추가 확인인증 기능 추가에 따라 테스트 픽스처가 올바르게 업데이트되었습니다. PasswordService를 사용한 비밀번호 해싱이 적절하게 적용되었습니다.
Also applies to: 73-74
wacruit/src/tests/user/conftest.py (1)
4-4: 테스트 픽스처 인증 필드 추가 확인User 픽스처가 새로운 인증 필드(username, password)를 포함하도록 올바르게 업데이트되었습니다.
Also applies to: 21-22
wacruit/src/tests/models/test_user.py (1)
5-5: 테스트 케이스 인증 필드 추가 확인중복 사용자 생성 테스트에 인증 필드가 적절하게 추가되었습니다.
Also applies to: 33-34
wacruit/src/tests/portfolio/url/conftest.py (1)
7-7: 테스트 픽스처 인증 필드 추가 확인user1과 user2 픽스처가 고유한 username과 해시된 password로 올바르게 업데이트되었습니다.
Also applies to: 27-28, 44-45
wacruit/src/tests/models/conftest.py (1)
3-3: 테스트 픽스처 인증 필드 추가 확인User 픽스처가 인증 필드를 포함하도록 올바르게 업데이트되었습니다.
Also applies to: 16-17
wacruit/src/tests/announcement/test_announcement_views.py (2)
8-10: Bearer 토큰 기반 인증으로 전환 확인테스트가 로그인 API를 통해 액세스 토큰을 획득하고 Authorization 헤더에 Bearer 토큰을 사용하도록 올바르게 업데이트되었습니다.
Also applies to: 17-17
24-26: 관리자 테스트 인증 플로우 확인관리자 사용자 테스트도 동일한 토큰 기반 인증 패턴을 올바르게 적용했습니다.
Also applies to: 33-33
wacruit/src/tests/portfolio/file/conftest.py (1)
9-9: 테스트 픽스처 인증 필드 추가 확인user1 픽스처가 인증 필드를 포함하도록 올바르게 업데이트되었습니다.
Also applies to: 29-30
wacruit/src/tests/announcement/conftest.py (1)
10-10: 테스트 픽스처 인증 필드 추가 확인user와 admin_user 픽스처가 각각 고유한 username("name", "admin")과 해시된 password로 올바르게 업데이트되었습니다.
Also applies to: 25-26, 42-43
wacruit/src/apps/auth/services.py (4)
32-33: LGTM!리포지토리로의 단순 위임이 적절합니다.
35-45: 이전 보안 이슈가 해결되었습니다
token_type파라미터를 추가하여 access 토큰과 refresh 토큰을 구분하도록 수정되었습니다. 이전 리뷰의 critical 이슈가 적절히 반영되었습니다.
65-69: LGTM!중복 블록킹을 방지하는 로직이 적절합니다.
71-77: LGTM!JWT 디코딩과 검증 로직이 올바르게 구현되었습니다.
JoseError를InvalidTokenException으로 변환하는 것이 적절합니다.참고: Line 75의 static analysis 힌트(TRY300)는 false positive입니다 - return 문이 있어 else 블록이 불필요합니다.
wacruit/src/apps/auth/services.py
Outdated
| def issue_token(self, user_id: int, expiration_hour: int, token_type: str) -> str: | ||
| header = {"alg": "HS256"} | ||
| payload = { | ||
| "sub": user_id, | ||
| "exp": int((datetime.now() + timedelta(hours=expiration_hour)).timestamp()), | ||
| "token_type": token_type, | ||
| } | ||
|
|
||
| return jwt.encode(header, payload, key=self.secret_token).decode("utf-8") |
There was a problem hiding this comment.
🧩 Analysis chain
타임존 명시적 처리로 분산 환경 버그를 방지하세요
Line 83에서 datetime.now()를 사용하면 서버의 로컬 타임존을 사용하게 되어, 분산 환경이나 타임존이 다른 서버에서 토큰 만료 시점이 일관되지 않을 수 있습니다. JWT exp 클레임은 UTC 기준 Unix timestamp를 사용하는 것이 표준입니다.
다음 diff를 적용하여 UTC를 명시적으로 사용하세요:
+from datetime import timezone
+
def issue_token(self, user_id: int, expiration_hour: int, token_type: str) -> str:
header = {"alg": "HS256"}
payload = {
"sub": user_id,
- "exp": int((datetime.now() + timedelta(hours=expiration_hour)).timestamp()),
+ "exp": int((datetime.now(timezone.utc) + timedelta(hours=expiration_hour)).timestamp()),
"token_type": token_type,
}
return jwt.encode(header, payload, key=self.secret_token).decode("utf-8")추가로, authlib 1.6.5의 jwt.encode()가 이미 문자열을 반환할 수 있으므로 Line 87의 .decode("utf-8")이 필요한지 확인하세요.
🌐 Web query:
Does authlib 1.6.5 jwt.encode return bytes or string?
💡 Result:
Authlib 1.6.5 jwt.encode returns a str (a JWT string), not bytes. [1][2]
If you need bytes, encode it yourself: token_bytes = jwt.encode(...).encode("utf-8"). [1][2]
Sources:
[1] Authlib docs — JSON Web Token (JWT) (jwt.encode described as creating a "JSON Web Token string").
[2] joserfc guide/API — examples show jwt.encode returning a string.
Line 87에서 .decode("utf-8") 제거 및 Line 83 타임존 명시적 처리
두 가지 주요 문제를 수정해야 합니다:
-
Line 83 - 타임존 명시:
datetime.now()는 서버 로컬 타임존을 사용하여 분산 환경에서 토큰 만료 시점이 일관되지 않을 수 있습니다. JWT의exp클레임은 UTC 기준 Unix timestamp를 사용해야 합니다. -
Line 87 - .decode() 제거: authlib 1.6.5의 jwt.encode()는 문자열을 반환하므로
.decode("utf-8")을 호출하면 AttributeError가 발생합니다.
다음 diff를 적용하세요:
+from datetime import timezone
+
def issue_token(self, user_id: int, expiration_hour: int, token_type: str) -> str:
header = {"alg": "HS256"}
payload = {
"sub": user_id,
- "exp": int((datetime.now() + timedelta(hours=expiration_hour)).timestamp()),
+ "exp": int((datetime.now(timezone.utc) + timedelta(hours=expiration_hour)).timestamp()),
"token_type": token_type,
}
- return jwt.encode(header, payload, key=self.secret_token).decode("utf-8")
+ return jwt.encode(header, payload, key=self.secret_token)Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In wacruit/src/apps/auth/services.py around lines 79 to 87, the token issuance
uses datetime.now() (local timezone) for the exp claim and calls
.decode("utf-8") on jwt.encode() which returns a str; change the timestamp
generation to use UTC (e.g., datetime.now(timezone.utc) or datetime.utcnow()
with timezone-aware conversion) so exp is int((datetime.now(timezone.utc) +
timedelta(hours=expiration_hour)).timestamp()), and remove the trailing
.decode("utf-8") so the function simply returns the string returned by
jwt.encode(...).
There was a problem hiding this comment.
이거 byte 문자열로 반환되길래 decode 한건데 다시 바꾸라고 하네요..
There was a problem hiding this comment.
🧩 Analysis chain
I see some test code shows .decode('utf-8') being called. Let me search more specifically for authlib jwt.encode bytes vs string version differences.
🏁 Script executed:
#!/bin/bash
# Check the actual source code of authlib's jwt.encode to see what it returns
cat > /tmp/test_jwt.py << 'EOF'
try:
from authlib.jose import jwt
# Simple test to check return type
header = {"alg": "HS256"}
payload = {"sub": "123", "exp": 1234567890}
token = jwt.encode(header, payload, "secret-key")
print(f"Type: {type(token)}")
print(f"Is bytes: {isinstance(token, bytes)}")
print(f"Is str: {isinstance(token, str)}")
print(f"Token: {token[:50] if len(str(token)) > 50 else token}")
except Exception as e:
print(f"Error: {e}")
import traceback
traceback.print_exc()
EOF
python /tmp/test_jwt.py
Length of output: 194
🏁 Script executed:
#!/bin/bash
# Install authlib and test jwt.encode return type
pip install authlib==1.6.5 -q
cat > /tmp/test_jwt.py << 'EOF'
try:
from authlib.jose import jwt
# Simple test to check return type
header = {"alg": "HS256"}
payload = {"sub": "123", "exp": 1234567890}
token = jwt.encode(header, payload, "secret-key")
print(f"Type: {type(token)}")
print(f"Is bytes: {isinstance(token, bytes)}")
print(f"Is str: {isinstance(token, str)}")
print(f"Token value: {token[:50] if len(str(token)) > 50 else token}")
# Try decoding if it's bytes
if isinstance(token, bytes):
print(f"Decoded: {token.decode('utf-8')[:50]}")
except Exception as e:
print(f"Error: {e}")
import traceback
traceback.print_exc()
EOF
python /tmp/test_jwt.py
Length of output: 177
🏁 Script executed:
#!/bin/bash
# Double check with the actual pyproject.toml version and test more thoroughly
pip show authlib | grep Version
cat > /tmp/test_jwt_detailed.py << 'EOF'
from authlib.jose import jwt
# Test with the exact pattern used in the code
header = {"alg": "HS256"}
payload = {
"sub": 123,
"exp": 1234567890,
"token_type": "access",
}
token = jwt.encode(header, payload, "test-secret")
print("=== Return Type Analysis ===")
print(f"Type: {type(token)}")
print(f"Type name: {type(token).__name__}")
print(f"Is bytes: {isinstance(token, bytes)}")
print(f"Is str: {isinstance(token, str)}")
print("\n=== Token Value ===")
print(f"Raw: {token}")
print("\n=== Decoding Test ===")
try:
decoded = token.decode('utf-8')
print(f"Decoded successfully: {decoded[:80]}")
print(f"Decoded type: {type(decoded).__name__}")
except AttributeError as e:
print(f"AttributeError when calling .decode(): {e}")
except Exception as e:
print(f"Error: {e}")
EOF
python /tmp/test_jwt_detailed.py
Length of output: 433
@MunJaeyoung 죄송합니다! 제가 확인한 결과 당신이 맞습니다. authlib 1.6.5의 jwt.encode()는 실제로 bytes를 반환하므로 .decode('utf-8')가 필요합니다.
제가 테스트한 결과:
token = jwt.encode(header, payload, "secret-key")
# Type: <class 'bytes'>
# 따라서 .decode('utf-8')로 str로 변환 필요.decode('utf-8')는 그대로 유지하시면 됩니다. 다만 타임존 이슈는 여전히 유효하니 Line 83의 datetime.now()를 datetime.now(timezone.utc)로 변경하는 것을 권장드립니다:
from datetime import timezone
def issue_token(self, user_id: int, expiration_hour: int, token_type: str) -> str:
header = {"alg": "HS256"}
payload = {
"sub": user_id,
"exp": int((datetime.now(timezone.utc) + timedelta(hours=expiration_hour)).timestamp()),
"token_type": token_type,
}
return jwt.encode(header, payload, key=self.secret_token).decode("utf-8")JWT의 exp 클레임은 UTC 기준 Unix timestamp를 사용하는 것이 표준이므로, 서버의 로컬 타임존에 의존하지 않도록 명시적으로 UTC를 사용하는 것이 안전합니다.
| op.add_column("user", sa.Column("username", sa.String(length=50), nullable=True)) | ||
| op.add_column("user", sa.Column("password", sa.String(length=255), nullable=True)) |
There was a problem hiding this comment.
기존 사용자 계정이 새 로그인 방식에서 잠기게 됩니다. 기존 user 레코드에는 username/password 값이 없어 이 마이그레이션 이후에도 NULL 그대로 남고, AuthService.login은 username으로만 사용자를 찾습니다. 결과적으로 기존 계정은 로그인 자체가 불가능해집니다. 업그레이드 단계에서 최소한 임시 username/password를 생성해 채우고 해시한 뒤(또는 별도 백필 스크립트를 제공) 마지막에 nullable=False로 전환하는 등 기존 데이터를 안전하게 마이그레이션해 주세요.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
wacruit/src/apps/auth/services.py (1)
76-84: 타임존을 UTC로 명시하여 분산 환경 버그를 방지하세요.Line 80에서
datetime.now()를 사용하면 서버의 로컬 타임존을 사용하게 되어, 분산 환경이나 타임존이 다른 서버에서 토큰 만료 시점이 일관되지 않을 수 있습니다. JWT의exp클레임은 UTC 기준 Unix timestamp를 사용하는 것이 표준입니다.다음과 같이 수정하세요:
+from datetime import timezone + def issue_token(self, user_id: int, expiration_hour: int, token_type: str) -> str: header = {"alg": "HS256"} payload = { "sub": user_id, - "exp": int((datetime.now() + timedelta(hours=expiration_hour)).timestamp()), + "exp": int((datetime.now(timezone.utc) + timedelta(hours=expiration_hour)).timestamp()), "token_type": token_type, } return jwt.encode(header, payload, key=self.token_secret).decode("utf-8")참고: Line 84의
.decode("utf-8")는 authlib 1.6.5에서jwt.encode()가 bytes를 반환하므로 올바릅니다. 정적 분석 힌트(S105)는 "token_type"을 비밀번호로 오인한 false positive이므로 무시하셔도 됩니다.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
wacruit/src/apps/auth/services.py(1 hunks)wacruit/src/apps/common/security.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
wacruit/src/apps/auth/services.py (5)
wacruit/src/apps/auth/exceptions.py (2)
InvalidTokenException(9-11)UserNotFoundException(4-6)wacruit/src/apps/auth/repositories.py (5)
AuthRepository(12-39)get_user_by_id(24-25)get_user_by_username(21-22)is_blocked_token(27-33)block_token(35-39)wacruit/src/apps/common/security.py (3)
get_token_secret(11-17)PasswordService(20-32)verify_password(28-32)wacruit/src/apps/user/models.py (1)
User(20-59)wacruit/src/apps/auth/views.py (2)
login(19-24)refresh_token(28-34)
wacruit/src/apps/common/security.py (1)
wacruit/src/secrets.py (3)
AWSSecretManager(12-37)is_available(21-30)get_secret(32-37)
🪛 Ruff (0.14.3)
wacruit/src/apps/auth/services.py
46-46: Possible hardcoded password assigned to: "token_type"
(S105)
72-72: Consider moving this statement to an else block
(TRY300)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: pytest
🔇 Additional comments (7)
wacruit/src/apps/common/security.py (2)
10-17: LGTM! 토큰 시크릿 조회 로직이 적절합니다.
@cache데코레이터를 사용하여 함수 결과를 메모이제이션하므로 AWS Secrets Manager API 호출이 한 번만 발생합니다. 로컬/테스트 환경에서는settings.TOKEN_SECRET으로 폴백하는 로직도 적절합니다.
20-32: LGTM! Argon2 기반 비밀번호 해싱이 안전하게 구현되었습니다.Argon2는 업계 표준 비밀번호 해싱 알고리즘으로 적절한 선택입니다.
VerifyMismatchError예외 처리도 정확하게 구현되어 있습니다.wacruit/src/apps/auth/services.py (5)
19-25: LGTM! 의존성 주입이 올바르게 구현되었습니다.
AuthRepository와token_secret을 FastAPI의Depends를 통해 주입받는 구조가 적절합니다.get_token_secret이 캐시되므로 성능 문제도 없습니다.
27-28: LGTM! 간단하고 명확한 위임 패턴입니다.
30-42: LGTM! 로그인 로직이 안전하게 구현되었습니다.사용자 검증, 비밀번호 확인, 토큰 발급 흐름이 올바르며, access/refresh 토큰을
token_type으로 구분하여 발급하는 점이 좋습니다.
62-66: LGTM! 토큰 차단 로직이 올바릅니다.이미 차단된 토큰에 대한 검증과 예외 처리가 적절합니다.
68-74: LGTM! JWT 디코딩 및 검증이 올바르게 구현되었습니다.예외 체이닝(
from e)을 사용하여 디버깅에 도움이 되도록 한 점이 좋습니다. 정적 분석 힌트(TRY300)는 무시하셔도 됩니다 - 현재 패턴이 더 가독성이 좋습니다.
| def refresh_token(self, refresh_token: str) -> tuple[str, str]: | ||
| decoded_token = self.decode_token(refresh_token) | ||
| if decoded_token["token_type"] != "refresh": | ||
| raise UserNotFoundException() | ||
|
|
||
| user_id = decoded_token["sub"] | ||
| user = self.auth_repository.get_user_by_id(user_id) | ||
|
|
||
| if self.auth_repository.is_blocked_token(refresh_token): | ||
| raise UserNotFoundException() | ||
| if user is None: | ||
| raise UserNotFoundException() | ||
|
|
||
| self.block_token(refresh_token) | ||
| access_token = self.issue_token(user.id, 24, "access") | ||
| new_refresh_token = self.issue_token(user.id, 24 * 7, "refresh") | ||
| return (access_token, new_refresh_token) |
There was a problem hiding this comment.
예외 타입을 의미에 맞게 수정하세요.
현재 토큰 검증 실패 시 UserNotFoundException을 발생시키고 있는데, 이는 의미적으로 부적절합니다:
- Line 46-47: 잘못된
token_type은 사용자가 없는 것이 아니라 유효하지 않은 토큰입니다. - Line 52-53: 차단된 토큰도 마찬가지로 유효하지 않은 토큰입니다.
API 소비자가 에러를 명확히 구분할 수 있도록 InvalidTokenException을 사용하는 것이 좋습니다.
다음과 같이 수정하세요:
def refresh_token(self, refresh_token: str) -> tuple[str, str]:
decoded_token = self.decode_token(refresh_token)
if decoded_token["token_type"] != "refresh":
- raise UserNotFoundException()
+ raise InvalidTokenException()
user_id = decoded_token["sub"]
user = self.auth_repository.get_user_by_id(user_id)
if self.auth_repository.is_blocked_token(refresh_token):
- raise UserNotFoundException()
+ raise InvalidTokenException()
if user is None:
raise UserNotFoundException()📝 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.
| def refresh_token(self, refresh_token: str) -> tuple[str, str]: | |
| decoded_token = self.decode_token(refresh_token) | |
| if decoded_token["token_type"] != "refresh": | |
| raise UserNotFoundException() | |
| user_id = decoded_token["sub"] | |
| user = self.auth_repository.get_user_by_id(user_id) | |
| if self.auth_repository.is_blocked_token(refresh_token): | |
| raise UserNotFoundException() | |
| if user is None: | |
| raise UserNotFoundException() | |
| self.block_token(refresh_token) | |
| access_token = self.issue_token(user.id, 24, "access") | |
| new_refresh_token = self.issue_token(user.id, 24 * 7, "refresh") | |
| return (access_token, new_refresh_token) | |
| def refresh_token(self, refresh_token: str) -> tuple[str, str]: | |
| decoded_token = self.decode_token(refresh_token) | |
| if decoded_token["token_type"] != "refresh": | |
| raise InvalidTokenException() | |
| user_id = decoded_token["sub"] | |
| user = self.auth_repository.get_user_by_id(user_id) | |
| if self.auth_repository.is_blocked_token(refresh_token): | |
| raise InvalidTokenException() | |
| if user is None: | |
| raise UserNotFoundException() | |
| self.block_token(refresh_token) | |
| access_token = self.issue_token(user.id, 24, "access") | |
| new_refresh_token = self.issue_token(user.id, 24 * 7, "refresh") | |
| return (access_token, new_refresh_token) |
🧰 Tools
🪛 Ruff (0.14.3)
46-46: Possible hardcoded password assigned to: "token_type"
(S105)
🤖 Prompt for AI Agents
wacruit/src/apps/auth/services.py around lines 44 to 60: the code raises
UserNotFoundException for token validation failures which is misleading; replace
the exceptions raised when token_type != "refresh" (lines ~46-47) and when a
token is blocked (lines ~52-53) with InvalidTokenException (or another
semantically correct invalid-token exception type used in the project), ensure
imports are added/updated for that exception, and keep raising
UserNotFoundException only when the user lookup returns None.
