Repository navigation
Conversation
Sean-mn
added
documentation:문서
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request removes the Swagger basic authentication middleware and introduces a comprehensive client integration guide. The review feedback highlights critical security concerns regarding the use of unencrypted HTTP for JWT transmission and the public exposure of Swagger in production. Additionally, it raises valid design improvements concerning potential race conditions in the Refresh Token Rotation (RTR) flow and usability issues with the player initialization API.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| | 항목 | 내용 | | ||
| |---|---| | ||
| | Base 경로 | 모든 엔드포인트는 `/v1` 접두사 사용 | | ||
| | 프로토콜 | **HTTP** (배포 환경 포트 제약으로 HTTPS 미사용) | |
There was a problem hiding this comment.
보안 위험 안내 (HTTP 사용 및 JWT 전송)
현재 배포 환경의 포트 제약으로 인해 HTTPS 대신 HTTP 프로토콜을 사용한다고 명시되어 있습니다. 하지만 HTTP 환경에서 JWT Bearer 토큰(Authorization 헤더)을 전송할 경우, 네트워크 상에서 토큰이 평문으로 노출되어 중간자 공격(MITM)을 통해 쉽게 탈취될 수 있습니다.
학교 포트 제약이 있더라도, Nginx 등의 역방향 프록시(Reverse Proxy)를 설정하거나 Cloudflare 등을 활용하여 비표준 포트에서도 HTTPS를 적용하는 것을 강력히 권장합니다.
There was a problem hiding this comment.
@gemini-code-assist[bot] HTTP 사용은 학교 서버 포트포워딩 제약(외부 443 확보 불가로 표준 ACME 검증 불가)으로 인지·수용한 의도된 결정입니다. 문서도 이 제약을 명시하고 있어 결함이 아닙니다. 비표준 포트 HTTPS(DuckDNS+Caddy DNS-01 등) 경로는 추후 별도 검토 예정입니다.
There was a problem hiding this comment.
이해했습니다. 학교 서버 환경의 물리적/네트워크적 제약으로 인해 HTTPS 적용이 어려운 상황임을 명확히 인지했습니다. 문서에 해당 제약을 명시하고 의도된 설계임을 밝혀주셨으므로, 해당 이슈는 해결된 것으로 간주하겠습니다. 추후 환경 개선 시 HTTPS 도입을 검토하시겠다는 계획도 확인했습니다.
| | 프로토콜 | **HTTP** (배포 환경 포트 제약으로 HTTPS 미사용) | | ||
| | 인증 | JWT Bearer — 보호 엔드포인트는 `Authorization: Bearer {accessToken}` 헤더 필요 | | ||
| | 본문 형식 | `application/json` | | ||
| | API 문서 | `/swagger` (현재 무인증 공개) | |
There was a problem hiding this comment.
보안 위험 안내 (Swagger 무인증 공개)
운영(Production) 환경에서 Swagger UI를 무인증으로 공개하는 것은 API 엔드포인트 구조와 스키마를 외부 공격자에게 노출시켜 공격 표면(Attack Surface)을 넓히는 보안 위험이 있습니다.
비록 클라이언트 연동 편의를 위해 공개하더라도, 최소한 특정 IP 대역만 접근할 수 있도록 방화벽 설정을 하거나, 개발/스테이징 환경에서만 Swagger를 활성화하고 운영 환경에서는 비활성화하는 방안을 검토해 주세요.
There was a problem hiding this comment.
@gemini-code-assist[bot] Swagger 무인증 공개는 클라이언트 연동용 스키마 참조 제공을 위해 이번 PR에서 의도적으로 결정한 정책입니다(HTTP 환경, 리스크 인지·수용). IP 허용목록 등 강화는 추후 별도 검토하겠습니다.
There was a problem hiding this comment.
의견 주셔서 감사합니다. 보안 위험에 대한 인지 및 수용 정책을 확인했습니다. 추후 환경 개선 시 고려해 주시기 바랍니다.
| - `refresh` 호출 시마다 **새 refreshToken이 발급되고 이전 토큰은 무효화**됩니다. | ||
| - 응답으로 받은 새 토큰을 반드시 저장하고 다음 갱신에 사용해야 합니다. | ||
| - 이미 사용한(이전) 토큰을 다시 보내면 **재사용 감지로 401** 처리됩니다 → 재로그인 필요. |
There was a problem hiding this comment.
Refresh Token Rotation (RTR) 적용 시 동시성(Race Condition) 고려
리프레시 토큰 회전(RTR) 방식을 사용할 때, 클라이언트(게임 앱)에서 여러 비동기 요청이 동시에 만료되어 각각 refresh API를 호출하는 상황이 발생할 수 있습니다.
이 경우 첫 번째 요청이 성공하여 토큰이 갱신되면, 거의 동시에 도달한 두 번째 요청은 이미 무효화된 이전 토큰을 사용하게 되므로 재사용 감지(401) 로직이 작동하여 사용자가 강제로 로그아웃되는 현상이 발생할 수 있습니다.
개선 제안:
- 서버 측 유예 기간(Grace Period) 도입: 이전 리프레시 토큰이 무효화된 후 아주 짧은 시간(예: 10~30초) 동안은 동시 요청을 감안하여 이전 토큰의 요청도 허용하거나 동일한 새 토큰 세트를 반환하도록 처리합니다.
- 클라이언트 측 갱신 락(Locking/Queueing): 클라이언트 연동 가이드에 '토큰 갱신 요청 중에는 다른 갱신 요청을 큐에 대기시키거나 단일 요청만 수행하도록 제한해야 한다'는 주의 사항을 명시해 주는 것이 좋습니다.
There was a problem hiding this comment.
@gemini-code-assist[bot] RTR 동시 갱신 레이스 컨디션 지적은 타당합니다. 클라이언트 측 갱신 직렬화/큐잉 안내는 곧 진행할 문서 정리에서 함께 검토하고, 서버 측 Grace Period 도입은 별도 서버 변경으로 분리하겠습니다.
There was a problem hiding this comment.
네, 알겠습니다. 클라이언트 측 갱신 직렬화/큐잉 안내는 문서에 반영하고, 서버 측 Grace Period 도입은 별도 서버 변경으로 분리하여 진행하는 방향에 동의합니다. 해당 이슈를 잘 인지하고 계시니, 이후 작업 시 참고하시기 바랍니다.
| | POST | `/v1/player/loadout` | `{ weaponId?, activeSkills[] }` | 장착 무기·액티브 스킬 저장 | | ||
| | POST | `/v1/player/skill/unlock` | `{ skillId }` | 스킬 해금 (SP 소모) | | ||
|
|
||
| - **init**: 플레이어가 없으면 `jobType`으로 신규 생성(201), 있으면 기존 데이터 로드(200). `jobType`은 **최초 생성 시에만 의미**가 있고, 이미 다른 직업으로 존재하면 **409**. 따라서 재접속 시에도 안전하게 호출 가능합니다. |
There was a problem hiding this comment.
플레이어 로드/생성 흐름 설계 검토 (init API)
설명에 따르면 POST /v1/player/init 호출 시 플레이어가 이미 존재하고 다른 직업일 경우 409 에러를 반환한다고 되어 있습니다.
이 경우, 새로운 기기에서 로그인한 클라이언트는 기존 플레이어의 존재 여부나 직업(JobType)을 알지 못하는 상태에서 init을 호출해야 합니다. 만약 잘못된 jobType으로 호출하여 409 에러를 받게 된다면, 클라이언트는 기존 플레이어 데이터를 어떻게 조회하고 로드해야 하는지 흐름이 모호해질 수 있습니다.
개선 제안:
jobType을 선택적으로 받기:init호출 시jobType을 누락하거나null로 보내면, 기존 플레이어가 있을 경우 그대로 로드(200)하고, 없을 경우에만 특정 에러(예: 404)를 반환하여 직업 선택 화면으로 유도하는 방식.- 조회 API 분리: 플레이어 존재 여부와 기본 정보를 조회하는
GET /v1/playerAPI를 추가하여, 플레이어가 있으면 바로 로드하고 없을 때만POST /v1/player/init을 통해 직업을 선택하여 생성하도록 분리하는 방식.
현재 가이드 문서상에 이 흐름에 대한 해결책(예: 409 응답 바디에 기존 플레이어 정보가 포함되는지 등)이 누락되어 있다면 보완이 필요해 보입니다.
There was a problem hiding this comment.
@gemini-code-assist[bot] 지적이 타당합니다. player/init의 생성·로드 겸용과 직업 불일치 409 모호함을 해소하기 위해 GET /v1/player(200/404, 로드) + POST /v1/player(201/409, 생성)로 분리하는 재설계를 진행합니다. 별도 스펙/브랜치로 처리합니다.
There was a problem hiding this comment.
제안해주신 GET /v1/player와 POST /v1/player로의 API 분리 설계는 클라이언트의 데이터 조회 및 생성 흐름을 훨씬 명확하게 만들어 줄 것입니다. 특히 기존 플레이어 존재 여부를 사전에 확인할 수 있게 되어, 409 Conflict 예외 처리에 의존하던 기존 방식보다 훨씬 견고한 구현이 가능할 것으로 보입니다. 진행하시는 재설계 방향에 전적으로 동의합니다.
There was a problem hiding this comment.
Code Review
This pull request removes the Swagger Basic Auth middleware, exposing the API documentation publicly in production, and introduces a comprehensive Client Integration Guide. The reviewer raised several valid security and design concerns: the risk of using unencrypted HTTP for transmitting credentials and JWTs, potential user experience issues with Refresh Token Rotation under unstable network conditions, the limitation of incrementing the maximum stage by only one level at a time, the lack of minimum elapsed time validation in gold runs which could lead to abuse, and the security implications of exposing the Swagger UI in production without any access restrictions.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
I am having trouble creating individual review comments. Click here to see my feedback.
docs/client-integration-guide.md (11)
현재 배포 환경의 포트 제약으로 인해 HTTPS 대신 HTTP 프로토콜을 사용한다고 명시되어 있습니다.
하지만 가이드에 따르면 /v1/account/signup, /v1/auth/login 등에서 사용자 비밀번호를 전송하고, 모든 보호된 API 호출 시 Authorization: Bearer {accessToken} 헤더를 통해 JWT 토큰을 전송합니다.
HTTP 환경에서는 네트워크 패킷 스니핑을 통해 비밀번호 및 JWT 토큰이 평문으로 노출되어 세션 하이재킹 및 계정 탈취 위험이 매우 높습니다. 학교 환경의 제약이 있더라도, Let's Encrypt 등을 활용하여 리버스 프록시(예: Nginx) 단에서 HTTPS(443 포트가 아니더라도 다른 포트를 통한 SSL/TLS 적용)를 적용하는 것을 강력히 권장합니다.
docs/client-integration-guide.md (95)
[네트워크 예외 처리 및 UX 검토]
회전식 리프레시 토큰(Refresh Token Rotation)을 사용할 때, 모바일 기기 등 네트워크가 불안정한 환경에서는 다음과 같은 문제가 발생할 수 있습니다:
- 클라이언트가
refreshAPI를 호출합니다. - 서버는 기존 토큰을 무효화하고 새 토큰을 발급하여 응답을 보냅니다.
- 응답이 클라이언트에 도달하기 전에 네트워크 연결이 끊어집니다.
- 클라이언트는 새 토큰을 받지 못했으므로, 재시도 시 기존(이미 무효화된) 토큰을 다시 전송하게 됩니다.
- 서버는 이를 재사용으로 감지하여 401 에러를 반환하고 사용자는 강제 로그아웃됩니다.
이러한 현상으로 인한 사용자 경험 저하를 방지하기 위해, 서버 측에서 **이전 리프레시 토큰에 대해 아주 짧은 유예 시간(Grace Period, 예: 10~30초)**을 두어 해당 시간 내의 중복 요청은 허용하거나 새 토큰을 재전송해 주는 완화책이 마련되어 있는지 검토해 보시는 것이 좋습니다.
docs/client-integration-guide.md (137)
[디자인/기획 검토]
현재 설명에 따르면 claim 호출 시 DPS가 현재 스테이지를 클리어할 수 있으면 maxStage를 단 1단계만(+1) 올린다고 되어 있습니다.
만약 플레이어가 무기 강화나 스킬 해금 등으로 전투력(DPS)이 급격히 상승하여 현재 스테이지보다 훨씬 높은 스테이지들을 한 번에 클리어할 수 있는 스펙이 되었을 때, maxStage가 한 번에 여러 단계 상승하지 않고 매번 claim을 호출할 때마다 1단계씩만 올라간다면 플레이어 경험(UX) 측면에서 답답함을 느끼거나 불필요한 API 호출을 반복해야 할 수 있습니다.
- 혹시 전투력에 따라 한 번에 여러 스테이지를 넘어가도록(예:
while루프 등을 통해 클리어 가능한 최대 스테이지까지 계산) 서버 로직이 구현되어 있는지 확인하시고, 가이드 문서의 설명을 보완하거나 서버 로직을 개선하는 것을 고려해 보세요.
docs/client-integration-guide.md (170)
[보안/어뷰징 방지 검토]
gold-runs는 30초 동안 진행되는 콘텐츠로 정의되어 있습니다. 클라이언트가 claim을 호출할 때, 서버에서 단순히 clicks > maxClicks나 경과 시간 대비 과다 클릭만 검사하고 **최소 진행 시간(예: 시작 후 최소 28~30초 경과)**을 검증하지 않는다면, 어뷰저가 런을 시작하자마자 5초 만에 최대 속도로 클릭한 후 즉시 claim을 호출하여 빠르게 보상을 챙기고 새로운 런을 반복하는 어뷰징이 가능할 수 있습니다.
- 서버 검증 로직에
현재 시간 - startedAt >= durationSeconds(또는 네트워크 마진을 고려한 최소 시간) 조건이 포함되어 있는지 확인하시고, 가이드나 서버 로직에 반영하는 것을 권장합니다.
Fantasy-server/Fantasy.Server/Program.cs (51-52)
Production 환경에서 Swagger Basic Auth 보호를 제거하면 API 스펙이 외부에 완전히 노출됩니다. 비록 클라이언트 연동 편의성을 위해 공개로 전환하더라도, 악의적인 사용자가 API 구조를 파악하여 취약점을 공격하거나 자동화된 스캐너에 노출될 위험이 있습니다.
만약 무인증 공개가 불가피하다면, 최소한 Swagger 페이지의 기본 경로(/swagger)를 추측하기 어려운 커스텀 경로(예: /swagger-secret-path)로 변경하거나, 특정 IP 대역에서만 접근할 수 있도록 제한하는 등의 최소한의 보안 조치를 추가하는 것을 검토해 보시기 바랍니다.
There was a problem hiding this comment.
Code Review
This pull request removes the SwaggerBasicAuthMiddleware and its configuration, leaving the Swagger UI unprotected in production. It also introduces a comprehensive client-integration-guide.md documentation file. The review feedback highlights a security concern regarding the use of HTTP instead of HTTPS, which exposes sensitive credentials and tokens to MITM attacks, and suggests clarifying the terminology around ticket charging in the documentation to avoid developer confusion.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| | 항목 | 내용 | | ||
| |---|---| | ||
| | Base 경로 | 모든 엔드포인트는 `/v1` 접두사 사용 | | ||
| | 프로토콜 | **HTTP** (배포 환경 포트 제약으로 HTTPS 미사용) | |
There was a problem hiding this comment.
프로토콜로 HTTPS 대신 HTTP를 사용하는 것은 로그인 비밀번호 및 JWT Access Token과 같은 민감한 정보가 평문으로 전송되어 중간자 공격(MITM)에 취약해지는 심각한 보안 위험이 있습니다. 학교 포트 제약 등의 환경적 한계가 있더라도, Caddy, Nginx 등의 역방향 프록시를 구성하거나 Cloudflare Tunnel 등을 활용하여 HTTPS를 적용하는 것을 강력히 권장합니다.
There was a problem hiding this comment.
@gemini-code-assist[bot] HTTP 사용은 동일 라인의 앞선 코멘트와 같이 학교 포트 제약으로 인지·수용한 의도된 결정입니다. 역방향 프록시/터널을 통한 HTTPS 경로는 추후 별도 검토 예정입니다.
There was a problem hiding this comment.
사용자의 상황과 의도를 충분히 이해했습니다. 학교 환경의 제약으로 인해 현재 단계에서 HTTP를 사용해야 한다는 점, 그리고 향후 역방향 프록시 등을 통해 HTTPS를 적용할 계획이 있다는 점을 명확히 인지했습니다. 보안 권고 사항을 수용해주셔서 감사합니다.
|
|
||
| 티켓 규칙: | ||
|
|
||
| - 매일(**KST 기준 날짜**) 첫 접근 시 자동으로 **3장 충전**됩니다(지연 지급). |
There was a problem hiding this comment.
'지연 지급'이라는 표현은 네트워크 지연이나 의도적인 지급 보류(delayed payment)로 오해하기 쉽습니다. 매일 첫 접근 시점에 필요한 계산을 수행하여 충전하는 방식(Lazy Loading)을 의미한다면, '최초 요청 시점에 충전' 또는 '첫 요청 시점에 실시간 충전'과 같이 표현을 명확히 수정하는 것이 클라이언트 개발자의 혼선을 줄일 수 있습니다.
| - 매일(**KST 기준 날짜**) 첫 접근 시 자동으로 **3장 충전**됩니다(지연 지급). | |
| - 매일(**KST 기준 날짜**) 첫 접근 시 자동으로 **3장 충전**됩니다(최초 요청 시점에 충전). |
There was a problem hiding this comment.
@gemini-code-assist[bot] '지연 지급' 표현이 모호하다는 지적 반영해 '최초 요청 시점에 충전'으로 수정했습니다. 이 문서는 곧 플레이어 흐름 재설계 작업에서 함께 정리됩니다.
There was a problem hiding this comment.
네, 확인했습니다. 해당 문서는 향후 플레이어 흐름 재설계와 함께 최신화될 예정이라는 점 인지했습니다. 추가로, 문서의 160번 라인에 있는 '지연 지급' 표현을 '최초 요청 시점에 충전'으로 수정해주셔서 감사합니다. 클라이언트 개발자 입장에서 훨씬 명확한 가이드가 될 것입니다.
|
PR #29로 대체되어 닫습니다. (Swagger Basic Auth 제거 + 클라이언트 연동 가이드 + 플레이어 로드/생성 재설계를 #29에 통합) 브랜치 update/swagger-open-client-guide는 보존합니다. |
1 participant
📚작업 내용
✅체크리스트