feat(api): 인증 기본 흐름 구현 - #63
Merged
Merged
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:
Walkthrough초기 Prisma ERD 마이그레이션(11개 테이블·인덱스·외래키)과 시드 데이터를 추가하고, JWT 기반 액세스 토큰 및 리프레시 토큰 유틸을 구현한다. 환경변수 및 공유 스키마에 토큰과 에러 코드를 확장한다. HttpError 클래스, errorHandler, validate 미들웨어를 통해 일관된 에러 처리 파이프라인을 수립한다. signup, login, logout, refresh, getMe 서비스 로직과 Express 핸들러를 연결하고, 리프레시 토큰 쿠키 유틸로 보안 저장소를 구현한다. 기존 require-auth·require-organization 미들웨어를 HttpError 기반으로 업그레이드하고 동일 출처 검증 미들웨어를 추가한다. 앱에 미들웨어와 라우팅을 등록하며, 패키지 의존성을 추가하고 API 문서 및 스키마를 갱신하여 API 계약을 반영한다. ChangesJWT 인증 API 구현 및 초기 DB 스키마
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60분 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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: 10
🤖 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 `@apps/api/src/app.ts`:
- Line 29: The errorHandler middleware registered at the app.use(errorHandler)
call is returning error responses in an incorrect format that doesn't match the
API contract. The handler is currently returning responses with a nested
structure like { error: { code, message } }, but the API contract requires {
statusCode, errorCode, message } format. Modify the errorHandler function to
restructure the error response to match the required format by mapping the
response fields appropriately so that statusCode, errorCode, and message are at
the top level of the response object instead of nested under an error property.
- Around line 19-23: The cors() middleware at the beginning of the app
configuration is using default settings which do not allow credentials (cookies)
in cross-origin requests. Since the application uses HTTP-only cookie-based
refresh token authentication with separate deployments for Web (Vercel) and API
(Railway), you need to configure the cors() call to explicitly enable
credentials and whitelist the production web origin. Replace the cors() call
with a configuration object that sets credentials to true and specifies the
allowed origin from your production Vercel deployment environment variable or
configuration.
In `@apps/api/src/middlewares/error-handler.ts`:
- Around line 8-24: The error response format in the error-handler middleware is
inconsistent with the project API contract standard. In the isHttpError
conditional block where res.status() is called and in the generic 500 error
response block, change the JSON response structure from the nested format {
error: { code, message } } to the standard format { statusCode, errorCode,
message }. Specifically, map error.statusCode to the statusCode field,
error.code to the errorCode field, and keep error.message as the message field.
Apply this same restructuring to both the HttpError case and the generic server
error case to ensure all error responses follow the required contract format.
In `@apps/api/src/modules/auth/auth.cookies.ts`:
- Around line 5-10: The refreshTokenCookieOptions object currently uses a fixed
sameSite value of "lax" which can cause refresh token cookies to not be sent on
cross-site requests in production when the frontend (Vercel) makes requests to
the API (Railway). Modify the sameSite property to conditionally vary based on
NODE_ENV: set it to "none" for production environments, and keep it as "lax" for
development. Additionally, ensure that when sameSite is "none", the secure flag
is explicitly set to true to comply with browser requirements for SameSite=None
cookies.
In `@apps/api/src/modules/auth/auth.service.ts`:
- Line 69: The error codes used in the auth.service.ts file such as
EMAIL_ALREADY_EXISTS, INVALID_CREDENTIALS, and REFRESH_TOKEN_INVALID do not
conform to the project's standard error code contract. Replace these custom
error codes with the standardized constants defined in
common/constants/error-codes.ts. Import the error code constants from the
common/constants/error-codes.ts file and update all instances where non-standard
error codes are used (the HttpError throw statements at lines 69, 91-92, 97-98,
105-106, and 114-115) to use the appropriate standard error code constants
instead of hardcoded string values.
- Around line 64-80: The email uniqueness check has a race condition where
concurrent requests can bypass the findUnique check and hit a database unique
constraint error that propagates as 500 instead of 409. Wrap the
prisma.user.create call in a try-catch block to handle Prisma unique constraint
violations on the email field, and throw an HttpError with 409 status code and
DUPLICATE_EMAIL error code when the email unique constraint is violated,
ensuring consistent error handling at the API layer regardless of timing.
In `@apps/api/src/modules/auth/auth.tokens.ts`:
- Around line 54-60: The getRefreshTokenExpiresAt function does not validate the
REFRESH_TOKEN_EXPIRES_DAYS value after converting it to a number, which can
result in NaN or negative values that break the expiration date calculation. Add
validation after the Number conversion to ensure the days variable is a positive
integer, and throw an error or return a default value if validation fails. This
prevents invalid expiration dates from being silently calculated with corrupted
values.
- Line 6: The DEFAULT_REFRESH_TOKEN_EXPIRES_DAYS constant in the auth.tokens.ts
file is set to 14 days, but according to the JWT token policy guidelines, the
refresh token expiration should be 7 days. Change the value of
DEFAULT_REFRESH_TOKEN_EXPIRES_DAYS from 14 to 7 to align with the established
security policy for refresh token validity.
In `@apps/api/src/routes/auth.routes.ts`:
- Around line 35-36: The POST /auth/refresh endpoint is missing from the auth
router despite being documented in docs/openapi.yaml and docs/API.md, causing a
contract mismatch. Add a new route handler for POST /auth/refresh in the
authRoutes definition (following the same pattern as logoutHandler and
meHandler) that accepts the refresh token from the request cookie and returns
renewed access and refresh tokens. Ensure this route is added at the same level
as the existing logout and me routes to maintain consistency.
In `@packages/database/prisma/seed.ts`:
- Line 174: The seed completion log in the console.info call is outputting the
plaintext password from SEED_USER.password, which exposes credentials to log
repositories and CI artifacts. Remove the password interpolation from the log
message and only log the email address or other non-sensitive information to
complete the seed operation safely.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: 0a16609b-0037-4d57-8365-58e79958193f
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (18)
.env.exampleapps/api/package.jsonapps/api/src/app.tsapps/api/src/errors/http-error.tsapps/api/src/middlewares/error-handler.tsapps/api/src/middlewares/validate.tsapps/api/src/modules/auth/auth.cookies.tsapps/api/src/modules/auth/auth.handlers.tsapps/api/src/modules/auth/auth.service.tsapps/api/src/modules/auth/auth.tokens.tsapps/api/src/routes/auth.routes.tsdocs/API.mddocs/openapi.yamlpackages/database/package.jsonpackages/database/prisma/migrations/20260622134729_init_erd/migration.sqlpackages/database/prisma/migrations/migration_lock.tomlpackages/database/prisma/seed.tspackages/shared/src/schemas/auth.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@packages/shared/src/schemas/common.ts`:
- Line 45: The ErrorResponse enum in docs/openapi.yaml is missing the
INTERNAL_SERVER_ERROR value that is defined in the errorCodeSchema in
packages/shared/src/schemas/common.ts and returned by the error handler
middleware. Update the ErrorResponse enum in docs/openapi.yaml (around lines
261-275) to include INTERNAL_SERVER_ERROR so that the OpenAPI documentation
accurately reflects all error codes that can be returned by the API
implementation.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: a224e435-5d35-40aa-96b1-d5105d65af5f
📒 Files selected for processing (9)
.env.exampleapps/api/src/app.tsapps/api/src/middlewares/error-handler.tsapps/api/src/modules/auth/auth.cookies.tsapps/api/src/modules/auth/auth.service.tsapps/api/src/modules/auth/auth.tokens.tsdocs/API.mdpackages/database/prisma/seed.tspackages/shared/src/schemas/common.ts
Contributor
|
A+B 통합 정리로 develop의 guard/utils 작업을 auth 브랜치에 병합했고, 에러 처리는 HttpError + ERROR_CODES 기준으로 통일했습니다. 추가로 CodeRabbit이 언급한 POST /auth/refresh는 현재 PR의 기존 구현 범위(signup/login/logout/me)를 넘어서는 refresh token rotation 정책 결정이 필요하므로, 팀 내 합의 후 이번 PR에 포함할지 별도 작업으로 분리할지 결정하겠습니다. |
Contributor
|
추가 마무리 작업 반영했습니다.
검증:
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@apps/api/src/modules/auth/auth.service.ts`:
- Around line 41-58: The refresh token rotation has a race condition where
concurrent requests with the same token can both pass validation and commit,
causing the later request to revoke the earlier request's newly created token.
Refactor the transaction at lines 41-58 to atomically validate and consume the
token in a single operation instead of reading validity separately beforehand.
Modify the updateMany call for refreshToken to include conditions for revokedAt:
null and expiresAt greater than current time, then capture the count of updated
records. Only proceed with creating the new refresh token within the same
transaction if the update count equals 1, ensuring that exactly one matching
token was found and revoked.
In `@apps/api/src/routes/auth.routes.ts`:
- Line 40: The `/refresh` POST route at line 40 with the refreshHandler is
vulnerable to CSRF attacks because it uses cookies for session rotation with
SameSite=None and CORS credentials enabled. Add CSRF protection by implementing
Origin/Referer validation in the common middleware before the refreshHandler
middleware chain. The validation should check that the incoming Origin or
Referer header matches the WEB_APP_ORIGIN environment variable, or alternatively
implement double-submit CSRF token validation by checking a CSRF token from
request headers against a token stored in cookies. Apply this validation
middleware to the refreshHandler to prevent cross-site requests from triggering
token rotation.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: 60cb5019-f221-4b72-b6c5-b1d23a9277b7
📒 Files selected for processing (5)
apps/api/src/app.tsapps/api/src/modules/auth/auth.handlers.tsapps/api/src/modules/auth/auth.service.tsapps/api/src/routes/auth.routes.tsapps/api/src/routes/index.ts
Contributor
|
CodeRabbit 리뷰 대응 추가 반영했습니다.
검증:
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@apps/api/src/middlewares/require-same-origin.ts`:
- Around line 31-38: The issue is that the current code treats the parsing
failure of a malicious header like "Origin: null" the same as a completely
missing header. You need to differentiate between the two cases in the
require-same-origin middleware. Modify the logic so that if the Origin or
Referer header is actually present as a string in req.headers, you attempt to
parse it with getHeaderOrigin, and if parsing fails or the result doesn't match
the normalizedAllowedOrigin, you reject the request (call next with an error or
deny it). Only when both Origin and Referer headers are completely absent should
you allow the request to pass through (for Postman/curl clients). The key change
is checking for header existence first before allowing any pass-through.
In `@apps/api/src/modules/auth/auth.service.ts`:
- Around line 160-175: The logout function is not idempotent because
consumeRefreshToken throws an error when the token is already expired or
revoked, preventing the cookie cleanup from completing. To fix this, modify the
logout function to handle the case where the refresh token has already been
consumed or is expired gracefully. Either wrap the consumeRefreshToken call in a
try-catch block and allow the logout to continue even if the token is already
consumed, or check the token's expiration and consumption state before calling
consumeRefreshToken, ensuring the function completes successfully and allows the
calling handler to clear the cookie regardless of the token's current state.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: 48c57b4a-f2ea-4631-b495-54aaf9a0eb39
📒 Files selected for processing (3)
apps/api/src/middlewares/require-same-origin.tsapps/api/src/modules/auth/auth.service.tsapps/api/src/routes/auth.routes.ts
Comment on lines
+160
to
+175
| export async function logout(prisma: PrismaClient, refreshToken?: string) { | ||
| if (!refreshToken) { | ||
| throw createInvalidRefreshTokenError(); | ||
| } | ||
|
|
||
| const tokenHash = hashRefreshToken(refreshToken); | ||
| const storedToken = await prisma.refreshToken.findUnique({ | ||
| where: { tokenHash }, | ||
| }); | ||
|
|
||
| if (!storedToken) { | ||
| throw createInvalidRefreshTokenError(); | ||
| } | ||
|
|
||
| await consumeRefreshToken(prisma, storedToken.id, new Date()); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
만료·폐기된 refresh 토큰으로 로그아웃하면 쿠키가 정리되지 않습니다.
consumeRefreshToken은 count !== 1이면 401을 던집니다. 토큰이 이미 만료(7일 경과)되었거나 회전으로 폐기된 상태에서 로그아웃을 호출하면, logoutHandler가 clearRefreshTokenCookie에 도달하기 전에 401로 빠져 무효 쿠키가 그대로 남습니다. 로그아웃은 멱등하게 처리해 항상 쿠키가 비워지도록 하는 편이 안전합니다.
보완 예시
const tokenHash = hashRefreshToken(refreshToken);
const storedToken = await prisma.refreshToken.findUnique({
where: { tokenHash },
});
- if (!storedToken) {
- throw createInvalidRefreshTokenError();
- }
-
- await consumeRefreshToken(prisma, storedToken.id, new Date());
+ if (!storedToken) {
+ return;
+ }
+
+ await prisma.refreshToken.updateMany({
+ where: { id: storedToken.id, revokedAt: null },
+ data: { revokedAt: new Date() },
+ });🤖 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 `@apps/api/src/modules/auth/auth.service.ts` around lines 160 - 175, The logout
function is not idempotent because consumeRefreshToken throws an error when the
token is already expired or revoked, preventing the cookie cleanup from
completing. To fix this, modify the logout function to handle the case where the
refresh token has already been consumed or is expired gracefully. Either wrap
the consumeRefreshToken call in a try-catch block and allow the logout to
continue even if the token is already consumed, or check the token's expiration
and consumption state before calling consumeRefreshToken, ensuring the function
completes successfully and allows the calling handler to clear the cookie
regardless of the token's current state.
Contributor
|
추가 리뷰 보완 반영했습니다.
검증:
|
ehlung
approved these changes
Jun 23, 2026
18 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
작업 내용
검증
pnpm --filter @fragment/shared buildpnpm --filter @fragment/database buildpnpm --filter @fragment/api buildpnpm --filter @fragment/api lintCloses #62
Summary by CodeRabbit
릴리스 노트
accessToken포함INTERNAL_SERVER_ERROR) 갱신