feat(wds-mcp): refresh token 지원 및 Firestore 영속 저장#526
Merged
Conversation
Google OAuth refresh token을 활용하여 Firebase ID token 만료 시 자동 갱신 지원. refresh token은 Firestore에 저장하여 서버 재시작 시에도 유지. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughGoogle refresh 토큰을 Firestore에 저장·관리하도록 도입하고, authorization code 교환에서 MCP refresh token 생성·저장·반환 및 저장된 Google refresh token으로의 토큰 갱신 흐름(exchangeRefreshToken)을 구현했습니다. 또한 Express에 Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant Auth as auth.ts
participant Firestore as Firestore
participant Google as Google OAuth API
participant Firebase as Firebase Identity Toolkit
Client->>Auth: exchangeRefreshToken(mcp_refresh_token)
Auth->>Firestore: get(mcp_refresh_token)
Firestore-->>Auth: google_refresh_token (or not found)
Auth->>Google: POST /token (grant_type=refresh_token, refresh_token=google_refresh_token)
Google-->>Auth: access_token, id_token, (maybe refresh_token)
alt Google provides new refresh_token
Auth->>Firestore: set(updated google_refresh_token)
end
Auth->>Firebase: POST identitytoolkit.googleapis.com (id_token -> firebase token)
Firebase-->>Auth: firebase idToken
Auth->>Auth: 이메일 도메인 검증
Auth-->>Client: OAuthTokens (access_token, id_token, refresh_token = incoming MCP token)
예상 코드 리뷰 노력🎯 4 (Complex) | ⏱️ ~45 분 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Contributor
size-limit report 📦
|
Contributor
🚀 Preview
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/wds-mcp/src/auth.ts (2)
87-105: refreshTokenStore에서 데이터 안전성 개선 고려
- Line 99의 타입 단언(
as { googleRefreshToken: string })은 문서 구조가 예상과 다를 경우 런타임 에러를 일으킬 수 있습니다.- 저장된 refresh token에 대한 TTL/만료 정책이 없어 오래된 토큰이 Firestore에 누적될 수 있습니다.
♻️ 타입 안전성 개선 제안
async get(mcpToken: string): Promise<string | undefined> { const doc = await refreshTokensCollection.doc(mcpToken).get(); if (!doc.exists) return undefined; - return (doc.data() as { googleRefreshToken: string }).googleRefreshToken; + const data = doc.data(); + if (typeof data?.googleRefreshToken !== 'string') return undefined; + return data.googleRefreshToken; },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/wds-mcp/src/auth.ts` around lines 87 - 105, refreshTokenStore's get uses an unsafe type assertion and there is no TTL/expiry when storing tokens; update the methods on refreshTokenStore (set, get, delete) to validate doc.data() shape instead of using "as { googleRefreshToken: string }" (return undefined or throw a controlled error when googleRefreshToken is missing), and when setting a token include an expiresAt timestamp (e.g. Date.now() + TTL) so tokens have a clear expiry; also add handling in get to check expiresAt and treat expired entries as missing (and optionally delete them), and ensure refreshTokensCollection calls consistently handle missing or malformed documents.
304-346: Firebase 토큰 교환 로직 중복
exchangeAuthorizationCode와exchangeRefreshToken에서 Firebase 토큰 교환 및 도메인 검증 로직이 중복됩니다. 현재 상태로도 동작에 문제는 없지만, 향후 유지보수를 위해 공통 헬퍼 함수로 추출하는 것을 고려해 볼 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/wds-mcp/src/auth.ts` around lines 304 - 346, The Firebase token exchange and domain-checking logic duplicated in exchangeAuthorizationCode and exchangeRefreshToken should be extracted into a shared helper (e.g., create a function named exchangeFirebaseTokens or verifyAndExchangeFirebaseToken) that accepts googleTokens (or the id_token) and refreshToken and returns the OAuthTokens shape; move the fetch to the Identity Toolkit endpoint, the timeout/error handling, response parsing, email domain validation (using ALLOWED_DOMAIN), and mapping to the returned object into that helper, then call it from both exchangeAuthorizationCode and exchangeRefreshToken to remove duplication.packages/wds-mcp/src/http.ts (1)
17-17:trust proxy값 2는 AWS ALB 환경에 적절하지만, 보안 모범 사례에 따라 개선 권장현재
app.set('trust proxy', 2)는 AWS ECS 환경의 ALB(Application Load Balancer) + CDN 구조(총 2 홉)와 일치하는 것으로 보입니다. 다만, 숫자 기반 홉 개수보다는 IP/서브넷 목록이나 커스텀 함수를 사용하는 것이 2026 보안 가이드라인에서 권장되고 있습니다.
- 현재:
app.set('trust proxy', 2)- 권장:
app.set('trust proxy', ['loopback', 'your-proxy-ip/32'])또는 커스텀 검증 함수 사용이렇게 개선하면 신뢰 경계(trust boundary) 공격을 더욱 효과적으로 방어할 수 있으며, 설정의 의도를 더 명확하게 문서화할 수 있습니다. 또한 값이 왜 2인지 설명하는 주석을 추가하면 유지보수성이 향상됩니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/wds-mcp/src/http.ts` at line 17, The current call app.set('trust proxy', 2) should be replaced with an explicit trust policy to avoid relying on numeric hop counts: update the app.set('trust proxy', ...) usage to pass a list of trusted addresses/subnets (e.g., ['loopback', 'x.x.x.x/32']) or supply a custom validator function that checks req.ip or X-Forwarded-For origins, and add a brief comment explaining why those IPs/subnets or the function are trusted (refer to the existing app.set('trust proxy', 2) location to find the change). Ensure tests or startup docs reflect the new configuration and that any environment-specific proxy IPs are injected via config/ENV rather than hardcoding.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/wds-mcp/src/auth.ts`:
- Around line 263-296: exchangeRefreshToken currently has a race where
concurrent calls using the same refreshToken can race (one deletes it on
failure) and it ignores a new refresh_token returned by Google; fix by
serializing/locking refresh operations per refresh token (e.g., a per-token
mutex or inflight promise map keyed by the incoming refreshToken inside
exchangeRefreshToken) so only one network refresh runs for a given refresh token
at a time, and when the tokenResponse is successful parse the JSON
(tokenResponse.json()) and if a new refresh_token is returned update
refreshTokenStore atomically (replace old key with new one or set new mapping)
instead of dropping it; also ensure errors from the single inflight attempt are
propagated to waiters and only delete the stored refresh token after confirming
Google returned a revocation/failure.
---
Nitpick comments:
In `@packages/wds-mcp/src/auth.ts`:
- Around line 87-105: refreshTokenStore's get uses an unsafe type assertion and
there is no TTL/expiry when storing tokens; update the methods on
refreshTokenStore (set, get, delete) to validate doc.data() shape instead of
using "as { googleRefreshToken: string }" (return undefined or throw a
controlled error when googleRefreshToken is missing), and when setting a token
include an expiresAt timestamp (e.g. Date.now() + TTL) so tokens have a clear
expiry; also add handling in get to check expiresAt and treat expired entries as
missing (and optionally delete them), and ensure refreshTokensCollection calls
consistently handle missing or malformed documents.
- Around line 304-346: The Firebase token exchange and domain-checking logic
duplicated in exchangeAuthorizationCode and exchangeRefreshToken should be
extracted into a shared helper (e.g., create a function named
exchangeFirebaseTokens or verifyAndExchangeFirebaseToken) that accepts
googleTokens (or the id_token) and refreshToken and returns the OAuthTokens
shape; move the fetch to the Identity Toolkit endpoint, the timeout/error
handling, response parsing, email domain validation (using ALLOWED_DOMAIN), and
mapping to the returned object into that helper, then call it from both
exchangeAuthorizationCode and exchangeRefreshToken to remove duplication.
In `@packages/wds-mcp/src/http.ts`:
- Line 17: The current call app.set('trust proxy', 2) should be replaced with an
explicit trust policy to avoid relying on numeric hop counts: update the
app.set('trust proxy', ...) usage to pass a list of trusted addresses/subnets
(e.g., ['loopback', 'x.x.x.x/32']) or supply a custom validator function that
checks req.ip or X-Forwarded-For origins, and add a brief comment explaining why
those IPs/subnets or the function are trusted (refer to the existing
app.set('trust proxy', 2) location to find the change). Ensure tests or startup
docs reflect the new configuration and that any environment-specific proxy IPs
are injected via config/ENV rather than hardcoding.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0e8d453d-670a-4cfc-8d9d-f80619e28e08
📒 Files selected for processing (2)
packages/wds-mcp/src/auth.tspackages/wds-mcp/src/http.ts
google이 refresh 응답에서 새 refresh_token을 반환할 경우 Firestore 매핑을 업데이트 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
packages/wds-mcp/src/auth.ts (4)
87-105: Firestore에 저장된 refresh token의 만료/정리 전략 부재현재 구현에서는 Firestore에 저장된 refresh token에 대한 TTL이나 정리 메커니즘이 없습니다. Google refresh token이 취소되거나 사용자가 로그아웃해도 Firestore 문서는 계속 남아있게 됩니다.
고려 사항:
- Firestore TTL 정책 설정 또는
createdAt필드 추가 후 주기적 정리- 토큰 갱신 실패 시에만 삭제하는 현재 방식의 한계 인지
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/wds-mcp/src/auth.ts` around lines 87 - 105, The refreshTokenStore currently writes refresh tokens without any expiry metadata; update the set method on refreshTokenStore to write a createdAt (and optionally lastUsed) timestamp alongside googleRefreshToken to refreshTokensCollection, and enable/advise using Firestore's TTL policy on that timestamp or implement a periodic cleanup job that deletes documents older than your retention window; also ensure delete(mcpToken) is still used when token revocation is detected and consider updating get(mcpToken) to optionally check age and return undefined if the token is expired by your policy.
310-344: Firebase 토큰 교환 로직 중복
exchangeAuthorizationCode(lines 209-242)와exchangeRefreshToken(lines 310-344)의 Firebase 토큰 교환 로직이 거의 동일합니다. 공통 함수로 추출하면 유지보수성이 향상됩니다.♻️ 공통 함수 추출 제안
async function exchangeGoogleTokenForFirebase( googleIdToken: string, ): Promise<{ idToken: string; expiresIn: string; email?: string }> { const firebaseResponse = await fetch( `https://identitytoolkit.googleapis.com/v1/accounts:signInWithIdp?key=${FIREBASE_API_KEY}`, { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ postBody: `id_token=${googleIdToken}&providerId=google.com`, requestUri: getServerUrl(), returnIdToken: true, returnSecureToken: true, }), signal: AbortSignal.timeout(FETCH_TIMEOUT_MS), }, ).catch((error) => { if (error instanceof DOMException && error.name === 'TimeoutError') { throw new Error('Firebase sign-in timed out'); } throw error; }); if (!firebaseResponse.ok) { const error = await firebaseResponse.text(); throw new Error(`Firebase sign-in failed: ${error}`); } return firebaseResponse.json(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/wds-mcp/src/auth.ts` around lines 310 - 344, The Firebase token-exchange block is duplicated in exchangeAuthorizationCode and exchangeRefreshToken; extract it into a shared helper (e.g., exchangeGoogleTokenForFirebase) that accepts the Google ID token string and returns the parsed { idToken, expiresIn, email? } object, then replace the duplicated fetch/error-handling/ok-check logic in both exchangeAuthorizationCode and exchangeRefreshToken with a call to that helper and keep the subsequent email domain check (email?.endsWith) in the callers; ensure the helper preserves the timeout/error mapping (DOMException timeout -> throw new Error('Firebase sign-in timed out')) and rethrows other errors, and that callers use the returned JSON shape.
94-100: 타입 단언 없이 데이터 검증 추가 권장
doc.data()의 반환 타입이 예상과 다를 경우 런타임 오류가 발생할 수 있습니다. Firestore 문서 구조가 변경되거나 손상된 경우를 대비해 방어적으로 처리하는 것이 좋습니다.♻️ 데이터 검증 추가 제안
async get(mcpToken: string): Promise<string | undefined> { const doc = await refreshTokensCollection.doc(mcpToken).get(); if (!doc.exists) return undefined; - return (doc.data() as { googleRefreshToken: string }).googleRefreshToken; + const data = doc.data(); + if (!data || typeof data.googleRefreshToken !== 'string') { + return undefined; + } + return data.googleRefreshToken; },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/wds-mcp/src/auth.ts` around lines 94 - 100, In get(mcpToken) on refreshTokensCollection, guard against unexpected doc.data() shapes by validating the returned object before asserting its googleRefreshToken: check that doc.exists and that typeof doc.data() === 'object' and that ('googleRefreshToken' in data) and typeof data.googleRefreshToken === 'string'; if the checks fail, return undefined (or log/throw as appropriate) instead of directly casting to { googleRefreshToken: string } to avoid runtime errors when Firestore schema changes.
254-258: Firestore 저장 실패 시 에러 처리 고려
refreshTokenStore.set호출이 실패하면 전체 토큰 교환이 실패합니다. Firestore 저장 실패가 전체 인증 흐름을 막아야 하는지, 아니면 refresh token 없이 access token만 반환해도 되는지 검토가 필요합니다.현재 동작: Firestore 저장 실패 시 사용자는 로그인 자체가 불가능해집니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/wds-mcp/src/auth.ts` around lines 254 - 258, The Firestore write via refreshTokenStore.set can fail and currently blocks the whole token exchange; wrap the call in a try/catch around the googleTokens.refresh_token handling (the block using mcpRefreshToken, refreshTokenStore.set, and tokens.refresh_token) and decide behavior: on failure either (A) log the error (include error details) and skip setting tokens.refresh_token so the function returns only the access token, or (B) rethrow when a strict persistence flag is enabled; implement (A) as default and provide an optional config flag to switch to (B). Ensure you reference refreshTokenStore.set, googleTokens.refresh_token, mcpRefreshToken and tokens.refresh_token when adding the try/catch and logging so the code remains discoverable.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/wds-mcp/src/auth.ts`:
- Around line 291-296: The code currently unconditionally deletes the stored
refresh token when tokenResponse.ok is false; change this to inspect the error
payload (parse tokenResponse as JSON and check fields like error or
error_description) and only call refreshTokenStore.delete(refreshToken) when the
response indicates the token was revoked/invalid (e.g., error ===
"invalid_grant" or similar), otherwise preserve the token and log the transient
error (include the parsed error details) before throwing; update the block
around the failing Google refresh handling in auth.ts where
refreshTokenStore.delete(refreshToken) is invoked.
---
Nitpick comments:
In `@packages/wds-mcp/src/auth.ts`:
- Around line 87-105: The refreshTokenStore currently writes refresh tokens
without any expiry metadata; update the set method on refreshTokenStore to write
a createdAt (and optionally lastUsed) timestamp alongside googleRefreshToken to
refreshTokensCollection, and enable/advise using Firestore's TTL policy on that
timestamp or implement a periodic cleanup job that deletes documents older than
your retention window; also ensure delete(mcpToken) is still used when token
revocation is detected and consider updating get(mcpToken) to optionally check
age and return undefined if the token is expired by your policy.
- Around line 310-344: The Firebase token-exchange block is duplicated in
exchangeAuthorizationCode and exchangeRefreshToken; extract it into a shared
helper (e.g., exchangeGoogleTokenForFirebase) that accepts the Google ID token
string and returns the parsed { idToken, expiresIn, email? } object, then
replace the duplicated fetch/error-handling/ok-check logic in both
exchangeAuthorizationCode and exchangeRefreshToken with a call to that helper
and keep the subsequent email domain check (email?.endsWith) in the callers;
ensure the helper preserves the timeout/error mapping (DOMException timeout ->
throw new Error('Firebase sign-in timed out')) and rethrows other errors, and
that callers use the returned JSON shape.
- Around line 94-100: In get(mcpToken) on refreshTokensCollection, guard against
unexpected doc.data() shapes by validating the returned object before asserting
its googleRefreshToken: check that doc.exists and that typeof doc.data() ===
'object' and that ('googleRefreshToken' in data) and typeof
data.googleRefreshToken === 'string'; if the checks fail, return undefined (or
log/throw as appropriate) instead of directly casting to { googleRefreshToken:
string } to avoid runtime errors when Firestore schema changes.
- Around line 254-258: The Firestore write via refreshTokenStore.set can fail
and currently blocks the whole token exchange; wrap the call in a try/catch
around the googleTokens.refresh_token handling (the block using mcpRefreshToken,
refreshTokenStore.set, and tokens.refresh_token) and decide behavior: on failure
either (A) log the error (include error details) and skip setting
tokens.refresh_token so the function returns only the access token, or (B)
rethrow when a strict persistence flag is enabled; implement (A) as default and
provide an optional config flag to switch to (B). Ensure you reference
refreshTokenStore.set, googleTokens.refresh_token, mcpRefreshToken and
tokens.refresh_token when adding the try/catch and logging so the code remains
discoverable.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e0e38048-be10-4d6b-83b6-2dfd358b07c1
📒 Files selected for processing (1)
packages/wds-mcp/src/auth.ts
google 서버 장애(5xx) 등 일시적 오류에서는 refresh token을 유지하고, token revoke/invalid(400/401)인 경우에만 삭제 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
packages/wds-mcp/src/auth.ts (2)
267-312:⚠️ Potential issue | 🟠 Major동일 refresh token의 동시 갱신은 아직 원자적이지 않습니다.
Line 267에서 읽은 값을 기준으로 Line 295에서 삭제하거나 Line 311에서 덮어쓰는데, 그 사이를 보호하는 장치가 없습니다. 같은 token으로 두 요청이 겹치면 한 요청이 회전된 값을 저장한 직후 다른 요청이 문서를 삭제해 정상 토큰을 잃을 수 있습니다. 토큰 단위 직렬화나 Firestore transaction/CAS로
get → refresh → delete/set을 묶어주세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/wds-mcp/src/auth.ts` around lines 267 - 312, The refresh flow reads refreshToken via refreshTokenStore.get, then may delete or set it after calling the Google token endpoint, but these operations are not atomic and can race; wrap the read→refresh→(delete|set) sequence in an atomic operation (e.g., a per-token mutex/lock or a datastore transaction/CAS) so only one concurrent request for the same refreshToken performs the network refresh and mutation; specifically protect the sequence around refreshTokenStore.get(refreshToken), the fetch to oauth2.googleapis.com/token, and the subsequent refreshTokenStore.delete(refreshToken) / refreshTokenStore.set(refreshToken, ...) so rotation cannot be lost if two requests overlap.
291-299:⚠️ Potential issue | 🟠 Majorstatus code만으로는 영구 실패를 판별할 수 없습니다.
Line 295는 HTTP status만 보고 삭제하므로, revoke된 refresh token과 다른 client/request 오류를 구분하지 못합니다. 그 결과 설정 문제나 요청 이상도 사용자 재로그인으로 번질 수 있습니다. 응답 body의 오류 코드까지 확인해서 실제
invalid_grant같은 영구 실패일 때만 삭제하세요.Google OAuth 2.0 token endpoint refresh_token error response invalid_grant invalid_client HTTP 400 401🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/wds-mcp/src/auth.ts` around lines 291 - 299, The code currently deletes refreshToken based only on HTTP status in the token refresh block; instead parse the tokenResponse body (prefer JSON parse of the awaited text) and inspect the OAuth error field (e.g. error === "invalid_grant") before calling refreshTokenStore.delete(refreshToken), so only remove tokens for permanent OAuth errors like invalid_grant; ensure tokenResponse, refreshTokenStore.delete and refreshToken are referenced and that parsing is safe (fallback to not deleting on parse errors or unexpected error values).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/wds-mcp/src/auth.ts`:
- Around line 87-105: The refreshTokenStore currently only persists
googleRefreshToken which allows token reuse across different OAuth clients;
update refreshTokenStore.set to also store the issuing OAuth client id (save
client.client_id alongside googleRefreshToken), change refreshTokenStore.get (or
add a new verify/getForClient method) to require the current client id and
return undefined or throw if the stored client_id does not match the caller, and
ensure refreshTokenStore.delete still removes by mcpToken; then update the code
path that creates the MCP refresh token (the code that issues a new MCP refresh
token and references _client) to pass the issuing client.client_id into
refreshTokenStore.set and to verify client id via refreshTokenStore.get/verify
before allowing a refresh.
- Around line 87-91: The refresh token storage is insecure:
refreshTokenStore.set uses the raw mcpToken as the Firestore document ID
(refreshTokensCollection) and writes googleToken in plaintext; change this to
store only a non-reversible identifier derived from mcpToken (e.g., HMAC or
SHA-256 of mcpToken) as the document ID and encrypt googleToken at the app level
before writing (introduce/utilize encryptRefreshToken/decryptRefreshToken
helpers), and update corresponding lookup code that reads by mcpToken to hash
and lookup by that hash and to decrypt the stored googleRefreshToken on
retrieval. Ensure keys for HMAC/encryption come from secure config and reuse the
same helpers across refreshTokenStore.set and any get/remove methods.
---
Duplicate comments:
In `@packages/wds-mcp/src/auth.ts`:
- Around line 267-312: The refresh flow reads refreshToken via
refreshTokenStore.get, then may delete or set it after calling the Google token
endpoint, but these operations are not atomic and can race; wrap the
read→refresh→(delete|set) sequence in an atomic operation (e.g., a per-token
mutex/lock or a datastore transaction/CAS) so only one concurrent request for
the same refreshToken performs the network refresh and mutation; specifically
protect the sequence around refreshTokenStore.get(refreshToken), the fetch to
oauth2.googleapis.com/token, and the subsequent
refreshTokenStore.delete(refreshToken) / refreshTokenStore.set(refreshToken,
...) so rotation cannot be lost if two requests overlap.
- Around line 291-299: The code currently deletes refreshToken based only on
HTTP status in the token refresh block; instead parse the tokenResponse body
(prefer JSON parse of the awaited text) and inspect the OAuth error field (e.g.
error === "invalid_grant") before calling
refreshTokenStore.delete(refreshToken), so only remove tokens for permanent
OAuth errors like invalid_grant; ensure tokenResponse, refreshTokenStore.delete
and refreshToken are referenced and that parsing is safe (fallback to not
deleting on parse errors or unexpected error values).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 203d80a2-c6fa-485b-97c8-682990eb0154
📒 Files selected for processing (1)
packages/wds-mcp/src/auth.ts
Comment on lines
+87
to
+91
| const refreshTokenStore = { | ||
| async set(mcpToken: string, googleToken: string) { | ||
| await refreshTokensCollection.doc(mcpToken).set({ | ||
| googleRefreshToken: googleToken, | ||
| }); |
Contributor
There was a problem hiding this comment.
refresh token을 복구 가능한 형태로 저장하고 있습니다.
Line 255-257에서 발급해 클라이언트에 반환한 값을 Line 89의 문서 ID로 그대로 쓰고, Line 90은 Google refresh token을 평문으로 저장합니다. Firestore read 권한만 있어도 둘 다 즉시 재사용 가능한 bearer secret입니다. MCP token은 해시로 조회하고, Google refresh token은 앱 레벨에서 암호화한 뒤 저장하는 편이 안전합니다.
Also applies to: 255-257
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/wds-mcp/src/auth.ts` around lines 87 - 91, The refresh token storage
is insecure: refreshTokenStore.set uses the raw mcpToken as the Firestore
document ID (refreshTokensCollection) and writes googleToken in plaintext;
change this to store only a non-reversible identifier derived from mcpToken
(e.g., HMAC or SHA-256 of mcpToken) as the document ID and encrypt googleToken
at the app level before writing (introduce/utilize
encryptRefreshToken/decryptRefreshToken helpers), and update corresponding
lookup code that reads by mcpToken to hash and lookup by that hash and to
decrypt the stored googleRefreshToken on retrieval. Ensure keys for
HMAC/encryption come from secure config and reuse the same helpers across
refreshTokenStore.set and any get/remove methods.
다른 OAuth client가 refresh token을 재사용하지 못하도록 clientId를 함께 저장하고 갱신 시 검증 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Summary
montage-storage)에 영속 저장하여 서버 재시작 시에도 유지Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
릴리스 노트
새로운 기능
개선 사항