Conversation
There was a problem hiding this comment.
Pull request overview
Adds a dedicated “review login” authentication path that can issue JWTs without Kakao OAuth, while also enhancing Kakao login to persist additional profile attributes and switching cookie handling to cookie-parser.
Changes:
- Introduces
POST /auth/review/loginflow (controller/service/DTOs) gated byREVIEW_LOGIN_*env settings, including user auto-create and refresh-token rotation. - Extends Kakao login upsert to store
birthdate,age, andsexwhen available from Kakao profile data. - Replaces custom cookie parsing middleware with
cookie-parserand adds related env schema/dependencies.
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/modules/auth/services/review-auth.service.ts | Implements review-login gating, user upsert, JWT issuance, and refresh-token rotation. |
| src/modules/auth/controllers/review-auth.controller.ts | Adds POST /auth/review/login endpoint and sets refresh token cookie. |
| src/modules/auth/dtos/review-login-request.dto.ts | Defines request DTO for review login (email/secret). |
| src/modules/auth/dtos/review-login-response.dto.ts | Defines response DTO for review login (access token + onboarding flags + user). |
| src/modules/auth/services/kakao-auth.service.ts | Persists additional Kakao profile fields (birth info, sex) during upsert. |
| src/modules/auth/auth.module.ts | Registers the new controller/service in the auth module. |
| src/main.ts | Uses cookie-parser middleware instead of a custom cookie header parser. |
| src/config/env.schema.ts | Adds REVIEW_LOGIN_* environment variables to the validated schema. |
| package.json | Adds cookie-related dependencies. |
| package-lock.json | Locks new dependencies and updates some pinned versions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "@prisma/client": "7.2.0", | ||
| "class-transformer": "^0.5.1", | ||
| "class-validator": "^0.14.3", | ||
| "cookie": "^1.1.1", |
There was a problem hiding this comment.
cookie is added as a direct dependency, but there are no direct imports/usages of the cookie package in the source (only cookie-parser). Unless something outside this PR relies on importing cookie directly, consider removing it to avoid carrying an unused dependency and duplicate cookie versions in the lockfile.
| "cookie": "^1.1.1", |
| this.logger.debug( | ||
| `process.env.REVIEW_LOGIN_ENABLED=${process.env.REVIEW_LOGIN_ENABLED}`, | ||
| ); | ||
| this.logger.debug( | ||
| `process.env.REVIEW_LOGIN_SECRET=${process.env.REVIEW_LOGIN_SECRET}`, | ||
| ); | ||
| console.log( | ||
| '[DEBUG] REVIEW_LOGIN_ENABLED', | ||
| process.env.REVIEW_LOGIN_ENABLED, | ||
| ); | ||
| console.log('[DEBUG] REVIEW_LOGIN_SECRET', process.env.REVIEW_LOGIN_SECRET); | ||
| console.log( | ||
| '[DEBUG] REVIEW_LOGIN_ALLOWED_EMAILS', | ||
| process.env.REVIEW_LOGIN_ALLOWED_EMAILS, | ||
| ); | ||
|
|
There was a problem hiding this comment.
assertReviewLoginAllowed logs REVIEW_LOGIN_SECRET (and other env values) via Logger.debug and console.log. This can leak the review-login secret and allowlist into application logs (including centralized log shipping), effectively weakening the protection on this endpoint. Remove these statements and, if diagnostics are needed, log only non-sensitive booleans (e.g., enabled flag / whether secret is configured) without printing secret values.
| this.logger.debug( | |
| `process.env.REVIEW_LOGIN_ENABLED=${process.env.REVIEW_LOGIN_ENABLED}`, | |
| ); | |
| this.logger.debug( | |
| `process.env.REVIEW_LOGIN_SECRET=${process.env.REVIEW_LOGIN_SECRET}`, | |
| ); | |
| console.log( | |
| '[DEBUG] REVIEW_LOGIN_ENABLED', | |
| process.env.REVIEW_LOGIN_ENABLED, | |
| ); | |
| console.log('[DEBUG] REVIEW_LOGIN_SECRET', process.env.REVIEW_LOGIN_SECRET); | |
| console.log( | |
| '[DEBUG] REVIEW_LOGIN_ALLOWED_EMAILS', | |
| process.env.REVIEW_LOGIN_ALLOWED_EMAILS, | |
| ); |
| private buildBirthInfo( | ||
| birthyear?: string, | ||
| birthday?: string, | ||
| ): { birthdate?: Date; age?: number } { | ||
| if (!birthyear || !birthday || birthday.length !== 4) { | ||
| return {}; | ||
| } | ||
|
|
||
| const year = Number(birthyear); | ||
| const month = Number(birthday.slice(0, 2)); | ||
| const day = Number(birthday.slice(2)); | ||
| if (!Number.isInteger(year) || !Number.isInteger(month)) { | ||
| return {}; | ||
| } | ||
| if (month < 1 || month > 12 || day < 1 || day > 31) { | ||
| return {}; | ||
| } | ||
|
|
||
| const birthdate = new Date(Date.UTC(year, month - 1, day)); | ||
| if (Number.isNaN(birthdate.getTime())) { | ||
| return {}; | ||
| } | ||
|
|
||
| const today = new Date(); | ||
| const currentYear = today.getUTCFullYear(); | ||
| const currentMonth = today.getUTCMonth() + 1; | ||
| const currentDay = today.getUTCDate(); | ||
| let age = currentYear - year; | ||
| if (currentMonth < month || (currentMonth === month && currentDay < day)) { | ||
| age -= 1; | ||
| } | ||
| if (age < 0) { | ||
| return {}; | ||
| } | ||
|
|
||
| return { birthdate, age }; | ||
| } |
There was a problem hiding this comment.
buildBirthInfo accepts any day 1-31 and then constructs a Date with Date.UTC(...), which will silently roll invalid dates (e.g., 02/31) into the next month, producing an incorrect birthdate and age. Since this data comes from an external provider, add a strict validity check (e.g., verify birthdate.getUTCMonth() === month - 1 and birthdate.getUTCDate() === day, and also validate day is an integer) before returning { birthdate, age }.
이슈 번호
close #97
주요 변경사항
REVIEW_LOGIN_ENABLED=true
요청 secret === REVIEW_LOGIN_SECRET
요청 email ∈ REVIEW_LOGIN_ALLOWED_EMAILS (comma-separated allowlist)
테스트 결과 (스크린샷)
참고 및 개선사항