feat!: OIDC device flow 인증으로 API key 인증 대체#1
Conversation
장기 API 키 대신 외부 OIDC IdP (Authentik/Auth0/Keycloak 등) 에서 PKCE 로 발급받은 짧은 액세스 토큰으로 Redash API 를 호출하도록 전환. 설계 결정: - MCP 서버는 Claude Desktop 의 stdio 서브프로세스로 동작하므로 직접 브라우저를 열 수 없음. 대신 `redash-mcp login` 서브커맨드를 새로 만들고, 그 결과로 생성된 토큰 캐시를 MCP 서버가 읽어 사용 (gcloud auth login / gh auth login 패턴). - HS256 등 대칭 알고리즘은 redash-custom 측에서 이미 거부하므로 클라이언트는 토큰 형식을 신경 쓰지 않음 — Bearer 헤더만 붙임. - 만료 임박 시 silent refresh, 401 응답 시 한 번에 한해 강제 refresh 후 재시도. 변경: - 신규 src/auth.ts: OIDC 디스커버리, PKCE 코드 verifier/challenge, loopback HTTP 서버, 브라우저 자동 오픈 (외부 dep 없이 platform 별 spawn), 토큰 캐시 (0600, $XDG_STATE_HOME/redash-mcp/tokens.json), refresh 로직, AuthError. - src/cli.ts: 서브커맨드 dispatcher (serve / login / logout / status / help). REDASH_API_KEY 환경변수 체크 제거, REDASH_OIDC_ISSUER + CLIENT_ID 필수화. serve 시작 전에 getValidTokens() 로 토큰 존재 확인 → 없으면 명확한 안내 후 종료. - src/redashClient.ts: 생성자에서 Authorization 헤더 베이크 제거. axios request interceptor 가 매 요청마다 getValidTokens().accessToken 으로 Bearer 헤더 주입. response interceptor 가 401 1회 재시도 (forceRefresh). - src/__tests__/auth.test.ts (신규): loadOidcConfig, tokenCachePath, getValidTokens (캐시 적중/만료 갱신/issuer 불일치/refresh_token 부재), forceRefresh, performLogout (idempotent), readStatus. - src/__tests__/redashClient.test.ts: API key 관련 케이스 제거 → URL 만 필수, request/response interceptor 동작 검증 (Bearer 주입, 401 재시도). - src/__tests__/integration.test.ts: env 요구사항 갱신. - playwright.config.ts: e2e 가 API key 대신 OIDC env / 캐시 경로 사용하도록. - README.md: 인증 모델·환경 변수·서브커맨드·Claude Desktop 설정·마이그레이션 가이드 재작성. - package.json: 0.0.13 → 0.1.0 (Breaking). 마이그레이션: - REDASH_API_KEY 제거 - REDASH_OIDC_ISSUER, REDASH_OIDC_CLIENT_ID 추가 - `redash-mcp login` 한 번 실행 77/77 테스트 통과, tsc --noEmit clean. Refs: Buzzvil/redash-custom#18, Buzzvil/buzz-k8s-resources#1802, DO-2616 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Walkthrough이 PR은 Redash MCP 서버의 인증 메커니즘을 API key 정적 기반에서 OIDC Device Authorization Grant 기반 단기 토큰으로 완전히 전환합니다. 새로운 Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes 🚥 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 docstrings
🧪 Generate unit tests (beta)
Comment |
README 예제의 REDASH_URL=redash-dev.buzzvil.com 은 오타. 실제는 .dev. silent refresh 동작은 Authentik blueprint 측 offline_access scope 추가 후 정상화됨 (Buzzvil/buzz-k8s-resources#1802 의 fix 커밋 e5585c2e). Refs: DO-2616, Buzzvil/buzz-k8s-resources#1802 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`redash-mcp serve` no longer requires a separate `redash-mcp login` step. On cache miss / unrecoverable AuthError (no refresh_token, expired refresh_token, etc.) the server transparently launches the OIDC PKCE browser flow on the first tool call and blocks until the user completes it. - new `ensureValidTokens()` wraps `getValidTokens` and triggers `performLogin` on AuthError; concurrent callers share one in-flight login so the browser is only opened once. - `runServer()` uses `ensureValidTokens` and exits only on non-auth fatal errors. - 401 retry path in `redashClient.ts`: when `forceRefresh` fails with AuthError (revoked / no refresh_token), clear the stale cache via `performLogout` and call `ensureValidTokens` so the user is prompted to log in again instead of getting a generic 401 error. This keeps the "no explicit login step" UX for plugin users at the cost of a one-time browser popup on first use. Manual `redash-mcp login` is still supported for headless / pre-warmed setups. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The MCP server runs as a stdio subprocess of Claude Desktop / IDE, so any browser popup launched during boot is invisible — the user has no way to know what's happening, and the host UI just sits there spinning while PKCE waits 5 minutes for a callback. Drop the pre-flight `ensureValidTokens()` from runServer entirely. Auth is already deferred to the request interceptor in redashClient.ts, which runs in response to an explicit tool call — that's when the user is already attending to the action and can sensibly interpret a browser popup. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…rowser The MCP server runs as a stdio subprocess of Claude Desktop / Claude Code and is frequently deployed in containers (no xdg-open / no DISPLAY). The previous "auto-launch browser on auth failure" path is invisible to the user and broken in container environments. New flow: - `ensureValidTokens()` no longer spawns a browser. On cache miss it calls `startPendingLogin()` (loopback callback server only) and throws an AuthError whose message embeds the auth URL. - The MCP CallTool dispatcher in index.ts now gates every tool call on `ensureValidTokens()`. When it throws AuthError, the dispatcher returns a tool response (isError: true) carrying the URL. The host UI renders this text, the user clicks the URL, completes PKCE in their browser, and the loopback server picks up the callback. The next tool call succeeds with the freshly cached token. - `performLogin()` (CLI `redash-mcp login`) still auto-launches the browser for terminal users — factored through the shared `beginLoginFlow()` helper. - redashClient interceptors simplified: request interceptor uses `getValidTokens` (cache-only), 401 retry path no longer tries to trigger interactive login. Concurrent tool calls during a pending login share the same URL via a module-level `pendingLogin` slot with a 5-minute TTL. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The PKCE+loopback flow assumes the user's browser can reach http://127.0.0.1:<port>/callback on this process. That works on regular desktops but breaks in containers (different network namespace, no port forward) and headless Linux (no DISPLAY). Add OAuth 2.0 Device Authorization Grant as an alternative flow that needs only outbound HTTPS to the IdP — the user's browser can be on any device. Both flows now coexist; selection rule: 1. REDASH_OIDC_FLOW env var override (`pkce` | `device`) 2. Auto: device flow if running in a container or headless Linux (KUBERNETES_SERVICE_HOST, `container`, /.dockerenv, /run/.containerenv, /proc/1/cgroup heuristic, or linux without DISPLAY/WAYLAND_DISPLAY). PKCE otherwise. Implementation: - `selectAuthFlow()` + `detectContainerLike()` decide per-process - `beginDeviceFlow()` calls device_authorization_endpoint, returns verification_uri_complete + user_code + poll-driven completion promise - `beginPkceFlow()` (renamed from `beginLoginFlow`) unchanged - Unified `LoginFlowHandle` shape; `beginLoginFlow()` dispatches - `performLogin` and `startPendingLogin` work with either flow - `ensureValidTokens` includes the user_code in its tool-response message when the device flow is used (verification_uri_complete pre-fills it, but the fallback line covers IdPs that don't honor pre-fill) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
When `selectAuthFlow()` picks PKCE (i.e. running on a non-container desktop where the user's own machine has a reachable browser), startPendingLogin now also spawns `open` / `start` / `xdg-open` so the browser pops up in response to the tool call without forcing the user to click the URL in the chat surface. - Device flow path unchanged — the IdP page is meant to be opened on a separate device, and the server may not have a usable browser at all. - Browser is launched only on the first attempt of a pending login; subsequent tool calls during the same in-flight flow reuse the existing pending state without spawning more browser windows. - Tool response still includes the URL as a clickable fallback in case the auto-launch silently failed (e.g. xdg-open missing on Linux). - Wording adjusts to "your browser should have opened automatically" vs "open this URL" depending on whether launch was attempted. - Opt out via `REDASH_OIDC_AUTO_LAUNCH_BROWSER=false`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add `canLaunchBrowser()` pre-flight check that synchronously verifies the platform-specific browser-opening command is reachable (`open` on macOS, `cmd /c start` on Windows, `xdg-open` + display server on Linux/*BSD). Wire it into `selectAuthFlow`: even on a non-container desktop, if we can't actually launch a browser we switch to device flow instead of serving the user a PKCE loopback URL their browser would have no path to reach. This covers the previously broken case: PKCE selected on desktop → browser launch fails (xdg-open missing / DISPLAY torn down / etc.) → user given a useless 127.0.0.1 redirect URL. After this change, that case is handled by surfacing the device flow's verification_uri_complete + user_code, both of which are reachable from any browser on any device. DISPLAY/WAYLAND_DISPLAY presence moved from detectContainerLike to canLaunchBrowser — they're a browser-availability signal, not a container signal — so the two functions express orthogonal concerns. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
After iterating through PKCE+loopback, URL-via-tool-response, container detection, and PKCE-with-browser-fallback, settle on the simpler model: always use OAuth 2.0 Device Authorization Grant (RFC 8628). It works uniformly across containers, headless setups, and regular desktops, and the verification_uri_complete + user_code UX is short enough that the "single click" advantage of PKCE doesn't justify the extra code path or runtime branching. Removed: - PKCE primitives: generatePkcePair, base64UrlEncode, exchangeCodeForTokens - Loopback callback server: awaitCallback, CallbackResult - Flow selection: selectAuthFlow, detectContainerLike, canLaunchBrowser, AuthFlow type, REDASH_OIDC_FLOW env var, REDASH_OIDC_AUTO_LAUNCH_BROWSER - Per-flow branches in performLogin / startPendingLogin / ensureValidTokens Kept: - Device flow: requestDeviceCode, pollForDeviceToken, beginDeviceFlow - Token cache I/O, discovery, refresh_token grant - openBrowser is still called from `redash-mcp login` CLI as a convenience (silently no-ops in containers — the user already has the URL printed) - ensureValidTokens surfaces URL + user_code via tool response for the MCP serve path Net: auth.ts goes from ~800 to ~450 lines, single flow, no env-var behavior matrix to reason about. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds a new MCP tool the assistant can call right after surfacing the
device flow URL to the user. The tool blocks on the in-flight pending
login's polling promise and returns once tokens are cached, so the
assistant can immediately retry the original Redash tool without
prompting the user a second time ("you authorized, now ask me again").
Changes:
- auth.ts: export `waitForPendingLogin()` — checks cache first, starts
or attaches to a pending login, races on `pending.completion` with the
device code's own expiry. Also `getPendingLogin()` for read-only
introspection.
- index.ts: register `wait_for_oidc_login` as the first tool. Tool
dispatcher bypasses `ensureValidTokens` for this tool (it IS the auth
step). Returns the authenticated email on success.
- auth.ts: `ensureValidTokens` error message now contains explicit
"## For the user" / "## For the assistant" sections instructing the
assistant to surface the URL AND call `wait_for_oidc_login` in the
same turn, then retry the original tool.
End-to-end UX:
1. User: "list redash queries"
2. Assistant calls `list_queries` → gets auth-required error with URL
3. Assistant prints URL to user + calls `wait_for_oidc_login` (blocks)
4. User clicks URL, completes device flow in browser
5. Device-flow polling resolves, `wait_for_oidc_login` returns success
6. Assistant retries `list_queries`, returns results
All in a single turn from the user's POV.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Previously, a Redash 401 followed by a failed `forceRefresh` (revoked /
rotated refresh_token, no refresh_token at all) raised an AuthError
inside the axios response interceptor, but every RedashClient method's
catch block wrapped it as `throw new Error('Failed to fetch queries
from Redash')` — discarding the auth context. The assistant saw a
generic failure and didn't know to prompt the user to re-authorize.
Two fixes:
1. Extract `makeLoginRequiredError(reason, opts)` in auth.ts that starts
a pending device flow and returns an AuthError with the standard
For-the-user / For-the-assistant message pointing at
`wait_for_oidc_login`. Used by both `ensureValidTokens` (cold cache)
and the 401 retry path (warm-but-revoked).
2. In redashClient.ts: when `forceRefresh` fails with AuthError on the
401 retry path, wipe the stale cache via `performLogout` and throw
`makeLoginRequiredError(...)` so the failure surfaces the same way
as a cold cache miss.
3. Add `if (error instanceof AuthError) throw error` at the top of every
RedashClient method's catch block (57 sites) so the auth-context
error is preserved verbatim through to the MCP tool response instead
of being replaced with a generic message.
End-to-end effect: when the session-cached token gets rejected by
Redash mid-conversation, the assistant gets back the same URL-and-
wait_for_oidc_login instructions it would on a cold start, and the
"resume after auth" UX (pr.9) continues to work transparently.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/__tests__/integration.test.ts (1)
37-65:⚠️ Potential issue | 🟠 Major | ⚡ Quick win싱글턴(redashClient) 생성 전에 axios.create mockReturnValue를 주입하세요.
src/redashClient.ts는export const redashClient = new RedashClient();로 eager singleton을 만들고, 생성자에서 곧바로this.client = axios.create(axiosConfig)를 저장합니다. 그런데src/__tests__/integration.test.ts는redashClient를 먼저 import한 뒤(라인 37~38) 테스트 본문에서mockedAxios.create.mockReturnValue(...)(라인 64)를 설정하고 있어, 싱글턴 내부this.client가 의도한mockInstance로 고정되지 않을 수 있습니다(가드 로직은 주로 interceptors 주입 누락만 다룹니다). 게다가 현재 테스트는logger.error호출 여부만 확인하고 있어(인자/에러 원인 검증 없음) 잘못된 실패 경로로도 통과할 위험이 큽니다.
mockReturnValue설정을redashClientimport 이전으로 옮기거나,jest.mock('axios')팩토리에서axios.create의 구현을 제공하세요.🤖 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 `@src/__tests__/integration.test.ts` around lines 37 - 65, The test imports the eager singleton redashClient (constructed by new RedashClient()) before stubbing axios.create, so the internal this.client may be created with the real axios instead of your mock; move the axios.create mockReturnValue setup so it runs before importing redashClient (or replace the import with a jest.mock('axios', () => { create: ... }) factory that returns the mockInstance), ensure the factory/mock provides create and the interceptors/defaults shape used by RedashClient, and update the test assertion to verify logger.error was called with the expected error/arguments rather than only checking that it was called.src/redashClient.ts (1)
277-282:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAuthorization 차단이 대소문자 변형을 놓칩니다.
여기서는
Authorization과authorization만 지우는데,AUTHORIZATION같은 키는 그대로 남습니다. HTTP 헤더명은 대소문자를 구분하지 않아서REDASH_EXTRA_HEADERS로 들어온 값이 같은 헤더로 정규화될 때 Bearer 토큰 주입을 덮어쓸 수 있습니다. 키를 normalize해서 비교하거나, request interceptor에서 authorization 계열 키를 전부 제거한 뒤 설정해 주세요.🤖 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 `@src/redashClient.ts` around lines 277 - 282, The current check only deletes 'Authorization' and 'authorization' but misses other casings (e.g., 'AUTHORIZATION'); update the logic around extraHeaders in src/redashClient.ts (the extraHeaders handling block and any request interceptor that applies REDASH_EXTRA_HEADERS) to normalize header names before comparing or to iterate all keys and delete any whose lowercased form === 'authorization' so all case variants are removed prior to injecting the OIDC Authorization header.
🧹 Nitpick comments (3)
src/__tests__/redashClient.test.ts (2)
129-141: ⚡ Quick win401 단일 재시도 보장을 음수 케이스로도 고정하세요.
Line 129~141은 “첫 401에서 재시도 성공”만 검증합니다.
_retried가 이미true인 401에서 즉시 중단되는 경로를 추가로 고정해야 무한 재시도 회귀를 막을 수 있습니다.테스트 추가 예시
+ it('should not retry when request is already marked as retried', async () => { + const originalConfig: any = { _retried: true, headers: {}, url: '/api/queries', method: 'get' }; + const err: any = { response: { status: 401 }, config: originalConfig }; + + await expect(responseErrorInterceptor!(err)).rejects.toBe(err); + expect(mockAxiosInstance.request).not.toHaveBeenCalled(); + });🤖 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 `@src/__tests__/redashClient.test.ts` around lines 129 - 141, Add a negative test verifying that responseErrorInterceptor does not retry when a 401 arrives and the request already has _retried === true: invoke responseErrorInterceptor with an error object whose response.status is 401 and config._retried is true, assert that mockAxiosInstance.request is not called, originalConfig.headers.Authorization is not changed to the refreshed token, and the interceptor rethrows or returns the original rejection (e.g., rejects with the same error). This targets the responseErrorInterceptor behavior to prevent infinite retry loops.
182-191: ⚡ Quick winAuthorization 우회 방지 테스트를 헤더 대소문자 변형까지 확장하세요.
Line 182~191은
Authorization키만 확인합니다.authorization/AUTHORIZATION변형을 추가해도 기본 헤더/요청 헤더에 반영되지 않는지 검증해야 보안 계약을 더 견고하게 고정할 수 있습니다.테스트 확장 예시
+ it('should refuse lowercase authorization header from extra headers', () => { + process.env.REDASH_EXTRA_HEADERS = '{"authorization":"malicious-key"}'; + + mockedAxios.create.mockClear(); + new RedashClient(); + + const callArgs = mockedAxios.create.mock.calls[0]?.[0]; + expect((callArgs?.headers as any)?.Authorization).toBeUndefined(); + expect((callArgs?.headers as any)?.authorization).toBeUndefined(); + });🤖 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 `@src/__tests__/redashClient.test.ts` around lines 182 - 191, Update the test 'should refuse to let extra headers override Authorization' to assert that extra headers with different casing cannot inject an Authorization header: when setting process.env.REDASH_EXTRA_HEADERS include keys like "authorization" and "AUTHORIZATION" (in addition to "Authorization"), instantiate new RedashClient(), read the axios.create call config (mockedAxios.create.mock.calls[0]?.[0]) and assert that none of callArgs.headers contain Authorization in any casing (e.g., check that callArgs.headers.Authorization, callArgs.headers.authorization, and callArgs.headers.AUTHORIZATION are all undefined), ensuring the interceptor remains the sole place that sets the Authorization header.README.md (1)
103-109: ⚡ Quick win코드 블록에 언어 식별자 추가
Fenced code block에 언어 식별자가 누락되어 markdownlint 경고가 발생합니다.
수정 제안
-``` +```text redash-mcp [serve] Start the MCP server (default). Requires a prior `login`. redash-mcp login Run the OIDC PKCE browser flow and cache tokens. redash-mcp logout Clear the cached tokens. redash-mcp status Show cached token info (email, expiry). redash-mcp help Show help.</details> As per coding guidelines: markdownlint-cli2 규칙 MD040은 모든 fenced code block에 언어 식별자를 요구합니다. <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@README.mdaround lines 103 - 109, Add a language identifier to the fenced
code block that lists the CLI commands (the block starting with the three
backticks followed by lines beginning "redash-mcp [serve]" through "redash-mcp
help"); change the opening fence fromtotext so markdownlint MD040 is
satisfied and the block is treated as plain text.</details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>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@playwright.config.ts:
- Around line 37-44: The comment clarifies that DANGEROUSLY_OMIT_AUTH used in
the command array is only disabling Inspector proxy auth and does not bypass
OIDC in server code (see ensureValidTokens and src/index.ts), and that
e2e/inspector-basic.spec.ts relies on a valid pre-populated
REDASH_OIDC_TOKEN_CACHE_PATH; update the playground docs or the playbook comment
to state this explicitly and ensure CI injects a valid
REDASH_OIDC_TOKEN_CACHE_PATH token cache (or modify the test to run
wait_for_oidc_login device flow), referencing the symbols DANGEROUSLY_OMIT_AUTH,
REDASH_OIDC_TOKEN_CACHE_PATH, ensureValidTokens, src/index.ts, and
e2e/inspector-basic.spec.ts so readers know where the behavior originates and
what to change.In
@README.md:
- Line 105: Update the "redash-mcp login" subcommand description to accurately
reflect the implemented auth flow: if the code implements the OIDC Device Flow,
replace "OIDC PKCE browser flow" with "OIDC Device Flow (device code flow) and
token caching"; otherwise remove or generalize the PKCE wording to avoid
claiming PKCE when not implemented. Locate the descriptive text for the
subcommand named "redash-mcp login" in the README and edit that single-line
description to match the actual implementation.- Line 11: The README currently describes an OIDC/PKCE browser flow but the code
in src/auth.ts implements the RFC 8628 Device Authorization Grant
(device_code/user_code, verification_uri) and does not use redirect URIs or
PKCE; update the README text that mentions "OIDC + PKCE browser login", any
"PKCE required" / loopback redirect URI language, and the REDASH_OIDC_AUDIENCE
description to state that the implementation sends audience as a parameter in
the device code request body (not a PKCE audience parameter), add a language
identifier to the code fence that lacks one, and reword the "v1.0.0: Initial
release (legacy numbering)" line to something clearer; search the README for the
phrases "OIDC + PKCE browser login", "PKCE required",
"http://127.0.0.1:*/callback", "REDASH_OIDC_AUDIENCE", the unnamed code fence,
and "v1.0.0: Initial release (legacy numbering)" to make these edits so wording
matches src/auth.ts behavior.In
@src/auth.ts:
- Around line 345-357: startPendingLogin creates a race: multiple concurrent
callers can each call beginDeviceFlow and overwrite the global pendingLogin; to
fix, serialize by introducing an in-flight promise (e.g., pendingLoginPromise)
that is checked/returned before calling beginDeviceFlow, set pendingLogin only
once when that promise resolves, and ensure you clear the in-flight promise on
error or after resolution; update startPendingLogin to return the resolved
pendingLogin (with url, userCode, expiresAt) for all concurrent callers and keep
the existing expiresAt calculation and pendingLogin shape.- Around line 411-468: Both getValidTokens and forceRefresh perform read →
refresh → write concurrently; serialize refreshes by adding a shared in-memory
inflight lock so only one refresh for a given cachePath runs at a time.
Implement a module-level Map<string, Promise> inflightRefreshes
keyed by cache (tokenCachePath) and factor the refresh logic into a helper
(e.g., performRefreshWithLock(cache, cfg, doRefresh)) that: 1) checks
readTokenCache(cache) and returns cached if still valid (REFRESH_LEEWAY_MS)
before starting a refresh, 2) if a refresh is needed uses an existing inflight
promise from the map or creates one that runs discover(...) → refreshTokens(...)
→ tokenResponseToCached(...) → writeTokenCache(...), 3) awaits that promise,
removes it from the map on completion, and returns the updated tokens. Call this
helper from both getValidTokens (when cached expired) and forceRefresh so
parallel callers share the same inflight refresh and avoid simultaneous /token
calls.In
@src/index.ts:
- Around line 2514-2533: The auth pre-check is gating public-dashboard calls;
update the logic around ensureValidTokens so it only runs for tools that require
authentication (e.g., skip when the tool name is "get_public_dashboard" or when
a tool metadata flag like requiresAuth === false). Concretely, modify the block
that imports and calls ensureValidTokens (and still handles AuthError) to first
check the tool identity or a per-tool requiresAuth property and only invoke
ensureValidTokens for authenticated tools; keep the same AuthError handling for
those cases so behavior and messages remain unchanged for protected tools.
Outside diff comments:
In@src/__tests__/integration.test.ts:
- Around line 37-65: The test imports the eager singleton redashClient
(constructed by new RedashClient()) before stubbing axios.create, so the
internal this.client may be created with the real axios instead of your mock;
move the axios.create mockReturnValue setup so it runs before importing
redashClient (or replace the import with a jest.mock('axios', () => { create:
... }) factory that returns the mockInstance), ensure the factory/mock provides
create and the interceptors/defaults shape used by RedashClient, and update the
test assertion to verify logger.error was called with the expected
error/arguments rather than only checking that it was called.In
@src/redashClient.ts:
- Around line 277-282: The current check only deletes 'Authorization' and
'authorization' but misses other casings (e.g., 'AUTHORIZATION'); update the
logic around extraHeaders in src/redashClient.ts (the extraHeaders handling
block and any request interceptor that applies REDASH_EXTRA_HEADERS) to
normalize header names before comparing or to iterate all keys and delete any
whose lowercased form === 'authorization' so all case variants are removed prior
to injecting the OIDC Authorization header.
Nitpick comments:
In@README.md:
- Around line 103-109: Add a language identifier to the fenced code block that
lists the CLI commands (the block starting with the three backticks followed by
lines beginning "redash-mcp [serve]" through "redash-mcp help"); change the
opening fence fromtotext so markdownlint MD040 is satisfied and the
block is treated as plain text.In
@src/__tests__/redashClient.test.ts:
- Around line 129-141: Add a negative test verifying that
responseErrorInterceptor does not retry when a 401 arrives and the request
already has _retried === true: invoke responseErrorInterceptor with an error
object whose response.status is 401 and config._retried is true, assert that
mockAxiosInstance.request is not called, originalConfig.headers.Authorization is
not changed to the refreshed token, and the interceptor rethrows or returns the
original rejection (e.g., rejects with the same error). This targets the
responseErrorInterceptor behavior to prevent infinite retry loops.- Around line 182-191: Update the test 'should refuse to let extra headers
override Authorization' to assert that extra headers with different casing
cannot inject an Authorization header: when setting
process.env.REDASH_EXTRA_HEADERS include keys like "authorization" and
"AUTHORIZATION" (in addition to "Authorization"), instantiate new
RedashClient(), read the axios.create call config
(mockedAxios.create.mock.calls[0]?.[0]) and assert that none of callArgs.headers
contain Authorization in any casing (e.g., check that
callArgs.headers.Authorization, callArgs.headers.authorization, and
callArgs.headers.AUTHORIZATION are all undefined), ensuring the interceptor
remains the sole place that sets the Authorization header.</details> <details> <summary>🪄 Autofix (Beta)</summary> Fix all unresolved CodeRabbit comments on this PR: - [ ] <!-- {"checkboxId": "4b0d0e0a-96d7-4f10-b296-3a18ea78f0b9"} --> Push a commit to this branch (recommended) - [ ] <!-- {"checkboxId": "ff5b1114-7d8c-49e6-8ac1-43f82af23a33"} --> Create a new PR with the fixes </details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: Organization UI **Review profile**: CHILL **Plan**: Pro **Run ID**: `c0418360-f962-4a76-b6bb-547813cbbfe0` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between f73f39f896239bdd0bcb096592a17ac3717d384c and ebc429346981143b7c3549a3e1fb34f85f610054. </details> <details> <summary>⛔ Files ignored due to path filters (1)</summary> * `package-lock.json` is excluded by `!**/package-lock.json` </details> <details> <summary>📒 Files selected for processing (10)</summary> * `README.md` * `package.json` * `playwright.config.ts` * `src/__tests__/auth.test.ts` * `src/__tests__/integration.test.ts` * `src/__tests__/redashClient.test.ts` * `src/auth.ts` * `src/cli.ts` * `src/index.ts` * `src/redashClient.ts` </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
| command: [ | ||
| `REDASH_URL=${TEST_REDASH_URL}`, | ||
| `REDASH_OIDC_ISSUER=${TEST_OIDC_ISSUER}`, | ||
| `REDASH_OIDC_CLIENT_ID=${TEST_OIDC_CLIENT_ID}`, | ||
| TEST_OIDC_TOKEN_CACHE_PATH ? `REDASH_OIDC_TOKEN_CACHE_PATH=${TEST_OIDC_TOKEN_CACHE_PATH}` : '', | ||
| 'DANGEROUSLY_OMIT_AUTH=true', | ||
| 'npm run inspector', | ||
| ].filter(Boolean).join(' '), |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: DANGEROUSLY_OMIT_AUTH 플래그가 어떻게 사용되는지 확인
# auth.ts나 index.ts에서 이 플래그가 어떻게 처리되는지 검색
rg -nC5 'DANGEROUSLY_OMIT_AUTH' src/
# E2E 테스트에서 실제 인증이 수행되는지 확인
rg -nC3 'login|getValidTokens|performLogin' e2e/Repository: Buzzvil/redash-mcp
Length of output: 44
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) playwright.config.ts 해당 구간 확인
if [ -f playwright.config.ts ]; then
echo "=== playwright.config.ts (lines around 37-44) ==="
nl -ba playwright.config.ts | sed -n '25,60p'
else
echo "playwright.config.ts not found at repo root"
fi
# 2) DANGEROUSLY_OMIT_AUTH 사용처를 repo 전체에서 탐색
echo
echo "=== occurrences: DANGEROUSLY_OMIT_AUTH (repo-wide) ==="
rg -n --hidden --no-ignore -S 'DANGEROUSLY_OMIT_AUTH' .
# 3) OIDC 관련 env가 어디서 참조되는지 확인
echo
echo "=== occurrences: REDASH_OIDC_* (repo-wide) ==="
rg -n --hidden --no-ignore -S 'REDASH_OIDC_' .
# 4) e2e에서 로그인/토큰 관련 호출이 있는지 확인 (폴더 존재 여부와 함께)
echo
echo "=== e2e folder scan for login/token keywords ==="
ls -la | head
if [ -d e2e ]; then
rg -n -S --hidden --no-ignore 'login|getValidTokens|performLogin|token|OIDC' e2e || true
else
echo "No e2e directory at repo root"
fi
# 5) Playwright setup/테스트 러너가 어떤 환경을 사용해 호출하는지(있다면) 확인
echo
echo "=== search for playwright config usage / auth setup in tests ==="
rg -n -S --hidden --no-ignore 'playwright|globalSetup|globalTeardown|storageState' . || trueRepository: Buzzvil/redash-mcp
Length of output: 173
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) playwright.config.ts 해당 구간 확인
if [ -f playwright.config.ts ]; then
echo "=== playwright.config.ts (lines around 25-60) ==="
awk 'NR>=25 && NR<=60 {printf "%d:%s\n", NR, $0}' playwright.config.ts
else
echo "playwright.config.ts not found at repo root"
ls -la | head
fi
# 2) DANGEROUSLY_OMIT_AUTH 사용처를 repo 전체에서 탐색
echo
echo "=== occurrences: DANGEROUSLY_OMIT_AUTH (repo-wide) ==="
rg -n --hidden --no-ignore -S 'DANGEROUSLY_OMIT_AUTH' . || true
# 3) OIDC 관련 env가 어디서 참조되는지 확인
echo
echo "=== occurrences: REDASH_OIDC_* (repo-wide) ==="
rg -n --hidden --no-ignore -S 'REDASH_OIDC_' . || true
# 4) e2e에서 로그인/토큰 관련 호출이 있는지 확인 (폴더 존재 여부와 함께)
echo
echo "=== e2e folder scan for login/token keywords ==="
if [ -d e2e ]; then
rg -n -S --hidden --no-ignore 'login|getValidTokens|performLogin|token|OIDC' e2e || true
else
echo "No e2e directory at repo root"
fi
# 5) Playwright 관련 setup/teardown 및 storageState 사용 여부 확인
echo
echo "=== search for playwright setup/teardown usage ==="
rg -n -S --hidden --no-ignore 'globalSetup|globalTeardown|storageState|playwright\.config' . || trueRepository: Buzzvil/redash-mcp
Length of output: 6327
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) playwright.config.ts 전체 상단/관련 주석 확인
echo "=== playwright.config.ts (head) ==="
head -n 120 playwright.config.ts | nl -ba | sed -n '1,120p'
# 2) package.json의 inspector 스크립트 확인
if [ -f package.json ]; then
echo
echo "=== package.json scripts ==="
cat package.json | sed -n '1,200p' | rg -n '"scripts"' -n -C 2 || true
# Print full scripts block more directly
node -e "const p=require('./package.json'); console.log(JSON.stringify(p.scripts,null,2))"
else
echo "package.json not found"
fi
# 3) inspector 엔트리/실행 파일(예: dist/src) 확인
echo
echo "=== search for 'inspector' implementation ==="
rg -n --hidden --no-ignore -S "inspector" . || true
# 4) README에 DANGEROUSLY_OMIT_AUTH 언급이 있는지 확인
echo
echo "=== README.md: DANGEROUSLY_OMIT_AUTH mentions ==="
rg -n --hidden --no-ignore -S "DANGEROUSLY_OMIT_AUTH" README.md || true
# 5) Playwright 테스트/폴더 존재 여부 확인
echo
echo "=== search for playwright usage ==="
rg -n --hidden --no-ignore -S "playwright" . || trueRepository: Buzzvil/redash-mcp
Length of output: 159
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) playwright.config.ts 상단/관련 주석 확인 (nl 미사용)
echo "=== playwright.config.ts (head 1-120) ==="
awk 'NR<=120 {printf "%d:%s\n", NR, $0}' playwright.config.ts
# 2) package.json의 scripts 확인
if [ -f package.json ]; then
echo
echo "=== package.json scripts ==="
node -e "const p=require('./package.json'); console.log(p.scripts||{})"
else
echo "package.json not found"
fi
# 3) 'npm run inspector'가 가리키는 실제 구현 확인
echo
echo "=== inspector-related references (repo-wide) ==="
rg -n --hidden --no-ignore -S 'inspector' . || true
# 4) README에 DANGEROUSLY_OMIT_AUTH 언급 있는지 확인
echo
echo "=== README.md: DANGEROUSLY_OMIT_AUTH mentions ==="
rg -n --hidden --no-ignore -S "DANGEROUSLY_OMIT_AUTH" README.md || true
# 5) Playwright/E2E 폴더/테스트 존재 여부 확인
echo
echo "=== directories check ==="
ls -la | head -n 50
echo
echo "=== search for playwright test usage (repo-wide) ==="
rg -n --hidden --no-ignore -S "test\(|`@playwright/test`|playwright/test" . || trueRepository: Buzzvil/redash-mcp
Length of output: 5821
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) DANGEROUSLY/OMIT_AUTH 관련 키워드 추가 탐색 (repo-wide)
echo "=== occurrences: OMIT_AUTH / omitAuth / DANGEROUSLY ==="
rg -n --hidden --no-ignore -S '(OMIT_AUTH|omitAuth|DANGEROUSLY_OMIT_AUTH)' . || true
# 2) dist가 있으면 문자열이 컴파일 결과에도 존재하는지 확인
echo
echo "=== dist directory check ==="
ls -la dist 2>/dev/null || echo "no dist/ directory"
echo
echo "=== occurrences in dist (if any) ==="
if [ -d dist ]; then
rg -n --hidden --no-ignore -S '(OMIT_AUTH|DANGEROUSLY_OMIT_AUTH)' dist || true
fi
# 3) src/auth.ts에서 환경변수 기반 auth 우회 로직 있는지 확인(파일 일부 출력)
echo
echo "=== src/auth.ts: search for OIDC/auth env handling ==="
if [ -f src/auth.ts ]; then
rg -n --hidden --no-ignore -S 'process\.env|env\.|REDASH_OIDC|tokenCachePath|cache' src/auth.ts
echo
awk 'NR>=1 && NR<=220 {print}' src/auth.ts | sed -n '1,220p'
fi
# 4) e2e 테스트에서 실제로 로그인/인증 플로우를 타는지 코드 확인
echo
echo "=== e2e/inspector-basic.spec.ts (head to 220) ==="
if [ -f e2e/inspector-basic.spec.ts ]; then
awk 'NR>=1 && NR<=220 {printf "%d:%s\n", NR, $0}' e2e/inspector-basic.spec.ts
fi
# 5) e2e 테스트가 무엇을 검증하는지 확인: 폴더 내용/다른 spec 유무
echo
echo "=== e2e directory listing ==="
ls -ლა e2eRepository: Buzzvil/redash-mcp
Length of output: 19591
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# src 트리 대략 확인
echo "=== src/ tree (depth 2) ==="
find src -maxdepth 2 -type f | sed -n '1,200p'
# OIDC/auth 로직 호출 지점 탐색
echo
echo "=== occurrences in src of auth/token-related functions ==="
rg -n --hidden --no-ignore -S 'ensureValidTokens|getValidTokens|performLogin|startPendingLogin|waitForPendingLogin|performLogout|loadOidcConfig|tokenCachePath|readTokenCache|clearTokenCache' src || true
echo
echo "=== occurrences in src of REDASH_OIDC / OIDC env usage (beyond auth.ts/cli.ts) ==="
rg -n --hidden --no-ignore -S 'process\.env\.REDASH_OIDC_|REDASH_OIDC_' src || true
echo
echo "=== occurrences in src of token cache / headers / auth middleware keywords ==="
rg -n --hidden --no-ignore -S '(Authorization|Bearer|access_token|id_token|refresh_token|auth(?!enticated)|oidc)' src || trueRepository: Buzzvil/redash-mcp
Length of output: 10089
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# src/index.ts에서 ensureValidTokens 관련 라인 및 주변 문맥
echo "=== src/index.ts: ensureValidTokens / waitForPendingLogin occurrences ==="
rg -n --hidden --no-ignore -S 'ensureValidTokens|waitForPendingLogin' src/index.ts || true
echo
echo "=== src/index.ts: print around ensureValidTokens matches ==="
# Print small context blocks by line number
python3 - <<'PY'
import re, subprocess, json, os, sys, textwrap
path="src/index.ts"
with open(path,'r',encoding='utf-8') as f:
lines=f.readlines()
text=''.join(lines)
# find line numbers for key phrases
keywords=["ensureValidTokens","waitForPendingLogin"]
line_nums=set()
for i,l in enumerate(lines, start=1):
if any(k in l for k in keywords):
line_nums.add(i)
for ln in sorted(line_nums):
start=max(1, ln-20)
end=min(len(lines), ln+20)
print(f"\n--- {path}:{ln} ---")
for j in range(start,end+1):
print(f"{j}:{lines[j-1].rstrip()}")
PY
# src/index.ts에 DANGEROUSLY_OMIT_AUTH가 아예 없는지 재확인(정확 문자열)
echo
echo "=== src/index.ts: DANGEROUSLY_OMIT_AUTH exact string check ==="
rg -n --hidden --no-ignore -S 'DANGEROUSLY_OMIT_AUTH' src/index.ts || true
# auth 우회에 쓰일 수 있는 다른 env 키워드 탐색
echo
echo "=== src/: possible auth-skip env keywords ==="
rg -n --hidden --no-ignore -S '(OMIT_AUTH|SKIP_AUTH|SKIP_AUTHORIZATION|BYPASS_AUTH|DANGEROUSLY)' src || trueRepository: Buzzvil/redash-mcp
Length of output: 8128
🌐 Web query:
DANGEROUSLY_OMIT_AUTH @modelcontextprotocol/inspector env var
💡 Result:
The DANGEROUSLY_OMIT_AUTH environment variable is used in the @modelcontextprotocol/inspector to disable the default authentication requirement [1][2]. By default, the MCP Inspector proxy server generates a random session token and requires it to be included as a Bearer token in the Authorization header for all requests to ensure secure communication [2][3]. Setting DANGEROUSLY_OMIT_AUTH=true bypasses this security measure [1][4]. Security Warnings: - This practice is strongly discouraged and officially labeled as incredibly dangerous [1]. - Disabling authentication exposes your machine to potential attacks not only from the public internet but also via your web browser [1]. Visiting a malicious website or viewing a malicious advertisement while the inspector is running could allow an attacker to remotely compromise your computer [1]. - It should not be used in enterprise or production environments [5]. The primary intended use case for this variable is strictly for local development, where a user may need to frequently restart the server and wants to avoid manually re-entering or pre-filling authentication tokens [4]. Top results: [1], [5], [2], [3]
Citations:
- 1: https://github.com/modelcontextprotocol/inspector
- 2: https://github.com/modelcontextprotocol/inspector/tree/87bb2425a3780bef3fce1bb8145c4c7c69eb59be
- 3: https://npmx.dev/package/@modelcontextprotocol/inspector
- 4: https://medium.com/@martin.hodges/creating-an-mcp-server-to-use-in-your-agentic-solution-f32190dbaa6e
- 5: https://www.celigo.com/blog/mcp-inspector/
DANGEROUSLY_OMIT_AUTH는 OIDC 우회 플래그가 아니라 Inspector 프록시 인증 비활성화 용도
DANGEROUSLY_OMIT_AUTH=true는playwright.config.ts의npm run inspector실행에만 존재하고,src/에서는 OIDC 처리(예:ensureValidTokens)를 우회하는 용도로 사용되지 않습니다(서버 OIDC 게이트는src/index.ts에서 여전히 걸립니다).- 다만 E2E(
e2e/inspector-basic.spec.ts)에서wait_for_oidc_login디바이스 플로우 완료를 직접 수행하지 않으므로, 현재 구성은 주석대로 사전 토큰 캐시(REDASH_OIDC_TOKEN_CACHE_PATH)가 유효함을 전제로 동작할 가능성이 큽니다. CI에서 해당 값이 실제로 주입되는지(유효 토큰인지) 문서/주석으로 명확히 해주세요.
🤖 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 `@playwright.config.ts` around lines 37 - 44, The comment clarifies that
DANGEROUSLY_OMIT_AUTH used in the command array is only disabling Inspector
proxy auth and does not bypass OIDC in server code (see ensureValidTokens and
src/index.ts), and that e2e/inspector-basic.spec.ts relies on a valid
pre-populated REDASH_OIDC_TOKEN_CACHE_PATH; update the playground docs or the
playbook comment to state this explicitly and ensure CI injects a valid
REDASH_OIDC_TOKEN_CACHE_PATH token cache (or modify the test to run
wait_for_oidc_login device flow), referencing the symbols DANGEROUSLY_OMIT_AUTH,
REDASH_OIDC_TOKEN_CACHE_PATH, ensureValidTokens, src/index.ts, and
e2e/inspector-basic.spec.ts so readers know where the behavior originates and
what to change.
| ## Features | ||
|
|
||
| - Connect to Redash instances via the Redash API | ||
| - Connect to Redash instances using **OIDC + PKCE browser login** (no long-lived API keys) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: auth.ts에서 실제 사용된 OAuth 플로우 확인
# Device Flow 관련 키워드 검색 (device_code, user_code 등)
rg -nC3 'device.*code|user.*code|device.*authorization' src/auth.ts
# PKCE/Authorization Code 관련 키워드 검색 (code_challenge, redirect_uri 등)
rg -nC3 'code_challenge|code_verifier|redirect_uri' src/auth.tsRepository: Buzzvil/redash-mcp
Length of output: 6176
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== README.md (head to ~140) with line numbers ==="
nl -ba README.md | sed -n '1,160p'
echo "=== README.md (around line ~220-260) with line numbers ==="
nl -ba README.md | sed -n '200,260p'
echo "=== README.md OAuth flow keyword check ==="
rg -n --context 2 "PKCE|device flow|Device Flow|RFC 8628|device_authorization|verification_uri|redirect URI|127\.0\.0\.1" README.md || true
echo "=== auth.ts OAuth flow keyword check ==="
# Locate auth.ts
AUTH_FILE="$(fd -tf auth.ts . | head -n 1 || true)"
if [[ -z "${AUTH_FILE}" ]]; then
AUTH_FILE="$(fd -tf '*auth.ts' . | head -n 1 || true)"
fi
echo "Found auth file: ${AUTH_FILE:-<none>}"
rg -n --context 2 "RFC 8628|device_authorization_endpoint|urn:ietf:params:oauth:grant-type:device_code|verification_uri|user_code|code_challenge|redirect_uri" "${AUTH_FILE:-README.md}" || trueRepository: Buzzvil/redash-mcp
Length of output: 174
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== README.md (1-200) ==="
awk 'NR<=200{printf "%6d\t%s\n", NR, $0}' README.md
echo "=== README.md (200-280) ==="
awk 'NR>=200 && NR<=280{printf "%6d\t%s\n", NR, $0}' README.md
echo "=== README.md OAuth flow keyword check ==="
rg -n --context 1 "PKCE|device flow|Device Flow|RFC 8628|device_authorization|verification_uri|redirect URI|127\.0\.0\.1|loopback|audience.*PKCE|Authorization Code \\+ PKCE" README.md || true
echo "=== auth.ts path lookup ==="
AUTH_FILE="$(fd -tf auth.ts . | head -n 5 || true)"
if [[ -z "${AUTH_FILE}" ]]; then
AUTH_FILE="$(fd -tf '*auth.ts' . | head -n 5 || true)"
fi
echo "Found auth.ts candidates:"
echo "${AUTH_FILE:-<none>}"
# Use first candidate if multiple
AUTH_FILE_ONE="$(echo "${AUTH_FILE}" | head -n 1)"
if [[ -n "${AUTH_FILE_ONE}" ]]; then
echo "=== auth.ts (flow keyword check) ==="
rg -n --context 2 "RFC 8628|device_authorization_endpoint|urn:ietf:params:oauth:grant-type:device_code|device_code|user_code|verification_uri|code_challenge|code_verifier|redirect_uri|loopback|PKCE|authorization_code|grant_type" "${AUTH_FILE_ONE}"
fiRepository: Buzzvil/redash-mcp
Length of output: 16453
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Search for PKCE/Auth Code usage in src ==="
rg -n "code_challenge|code_verifier|redirect_uri|authorization_code|grant_type['\"]?\s*:\s*['\"]authorization_code|urn:ietf:params:oauth:grant-type:authorization_code" src || true
echo "=== Search for Device Flow usage in src ==="
rg -n "RFC 8628|device_authorization_endpoint|urn:ietf:params:oauth:grant-type:device_code|device_code|user_code|verification_uri" src || trueRepository: Buzzvil/redash-mcp
Length of output: 1675
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== auth.ts: find audience usage ==="
rg -n "audience" src/auth.ts || true
echo "=== auth.ts: show audience-related block (around matches) ==="
rg -n --context 5 "audience" src/auth.ts || trueRepository: Buzzvil/redash-mcp
Length of output: 3071
README.md OAuth 플로우 문구가 실제 구현(RFC 8628 Device Flow)과 불일치
- README.md 11, 26-27: 구현은
src/auth.ts에서 RFC 8628 Device Authorization Grant(device_code/user_code,urn:ietf:params:oauth:grant-type:device_code, verification_uri 기반)인데, README는 Authorization Code + PKCE / OIDC + PKCE browser login을 안내합니다. - README.md 49-50: 구현은 redirect URI/PKCE 챌린지에 의존하지 않는데, README는 PKCE required 및
http://127.0.0.1:*/callback(loopback redirect URI)을 요구사항으로 적고 있습니다. - README.md 59:
REDASH_OIDC_AUDIENCE설명이 “audience PKCE parameter”라고 되어 있으나, 실제 구현에서는 device code 요청 바디의 일반audience로만 사용됩니다. - README.md 103: 코드펜스에 언어 식별자 누락(`````, markdownlint 이슈).
- README.md 235:
v1.0.0: Initial release (legacy numbering)문구는 오해 소지가 있어 더 명확히 정리 필요(소폭).
문구 교체 제안
Line 11:
-- Connect to Redash instances using **OIDC + PKCE browser login** (no long-lived API keys)
+- Connect to Redash instances using **OIDC Device Flow login** (no long-lived API keys)Lines 26-27:
-authenticated with a short-lived OIDC access token obtained from your IdP
-(Authentik, Auth0, Keycloak, Okta, …) using the **Authorization Code + PKCE**
-flow.
+authenticated with a short-lived OIDC access token obtained from your IdP
+(Authentik, Auth0, Keycloak, Okta, …) using the **Device Authorization Grant**
+flow (RFC 8628).Line 50:
-- An OIDC client registered as **public** with **PKCE required** and a
- loopback redirect URI like `http://127.0.0.1:*/callback`
+- An OIDC client registered as **public** with **Device Flow enabled**Line 59:
-| `REDASH_OIDC_AUDIENCE` | no | Override the `audience` PKCE parameter. Defaults to `REDASH_OIDC_CLIENT_ID`. |
+| `REDASH_OIDC_AUDIENCE` | no | Override the `audience` parameter. Defaults to `REDASH_OIDC_CLIENT_ID`. |🤖 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 `@README.md` at line 11, The README currently describes an OIDC/PKCE browser
flow but the code in src/auth.ts implements the RFC 8628 Device Authorization
Grant (device_code/user_code, verification_uri) and does not use redirect URIs
or PKCE; update the README text that mentions "OIDC + PKCE browser login", any
"PKCE required" / loopback redirect URI language, and the REDASH_OIDC_AUDIENCE
description to state that the implementation sends audience as a parameter in
the device code request body (not a PKCE audience parameter), add a language
identifier to the code fence that lacks one, and reword the "v1.0.0: Initial
release (legacy numbering)" line to something clearer; search the README for the
phrases "OIDC + PKCE browser login", "PKCE required",
"http://127.0.0.1:*/callback", "REDASH_OIDC_AUDIENCE", the unnamed code fence,
and "v1.0.0: Initial release (legacy numbering)" to make these edits so wording
matches src/auth.ts behavior.
|
|
||
| ``` | ||
| redash-mcp [serve] Start the MCP server (default). Requires a prior `login`. | ||
| redash-mcp login Run the OIDC PKCE browser flow and cache tokens. |
There was a problem hiding this comment.
Subcommand 설명의 PKCE 언급 제거 또는 수정 필요
Line 105의 login 서브커맨드 설명에서 "OIDC PKCE browser flow"라고 명시되어 있는데, 실제 구현이 Device Flow라면 이 부분도 수정이 필요합니다.
Device Flow인 경우 수정 제안
-redash-mcp login Run the OIDC PKCE browser flow and cache tokens.
+redash-mcp login Run the OIDC Device Flow and cache tokens.📝 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.
| redash-mcp login Run the OIDC PKCE browser flow and cache tokens. | |
| redash-mcp login Run the OIDC Device Flow and cache tokens. |
🤖 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 `@README.md` at line 105, Update the "redash-mcp login" subcommand description
to accurately reflect the implemented auth flow: if the code implements the OIDC
Device Flow, replace "OIDC PKCE browser flow" with "OIDC Device Flow (device
code flow) and token caching"; otherwise remove or generalize the PKCE wording
to avoid claiming PKCE when not implemented. Locate the descriptive text for the
subcommand named "redash-mcp login" in the README and edit that single-line
description to match the actual implementation.
| export async function startPendingLogin(opts: { cfg?: OidcConfig; cachePath?: string } = {}): Promise<{ url: string; userCode: string; expiresAt: number }> { | ||
| if (pendingLogin && pendingLogin.expiresAt > Date.now()) { | ||
| return { url: pendingLogin.url, userCode: pendingLogin.userCode, expiresAt: pendingLogin.expiresAt }; | ||
| } | ||
|
|
||
| const handle = await beginDeviceFlow(opts); | ||
| const entry: PendingLogin = { | ||
| url: handle.url, | ||
| userCode: handle.userCode, | ||
| completion: handle.completion, | ||
| expiresAt: Date.now() + handle.expiresInSec * 1000, | ||
| }; | ||
| pendingLogin = entry; |
There was a problem hiding this comment.
startPendingLogin()가 동시 호출에서 서로 다른 device flow를 만들 수 있습니다.
Line 346에서 상태를 확인한 뒤 Line 350의 await beginDeviceFlow()로 넘어가는 동안 다른 호출도 같은 경로를 탈 수 있습니다. 그러면 각 호출이 서로 다른 URL/user_code를 반환하지만 전역 pendingLogin에는 마지막 flow만 남아서, 먼저 받은 URL로 인증한 사용자는 wait_for_oidc_login이 다른 flow를 기다리게 됩니다. cold start도 하나의 in-flight promise로 먼저 등록해서 직렬화해야 합니다.
🤖 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 `@src/auth.ts` around lines 345 - 357, startPendingLogin creates a race:
multiple concurrent callers can each call beginDeviceFlow and overwrite the
global pendingLogin; to fix, serialize by introducing an in-flight promise
(e.g., pendingLoginPromise) that is checked/returned before calling
beginDeviceFlow, set pendingLogin only once when that promise resolves, and
ensure you clear the in-flight promise on error or after resolution; update
startPendingLogin to return the resolved pendingLogin (with url, userCode,
expiresAt) for all concurrent callers and keep the existing expiresAt
calculation and pendingLogin shape.
| export async function getValidTokens(opts: { cfg?: OidcConfig; cachePath?: string; now?: () => number } = {}): Promise<CachedTokens> { | ||
| const cfg = opts.cfg ?? loadOidcConfig(); | ||
| const cache = opts.cachePath ?? tokenCachePath(); | ||
| const now = opts.now ?? Date.now; | ||
|
|
||
| const cached = await readTokenCache(cache); | ||
| if (!cached) { | ||
| throw new AuthError( | ||
| `No cached OIDC tokens at ${cache}. Run \`redash-mcp login\` once in your terminal.`, | ||
| ); | ||
| } | ||
| if (cached.issuer !== cfg.issuer || cached.clientId !== cfg.clientId) { | ||
| throw new AuthError( | ||
| `Cached tokens belong to a different OIDC client (issuer=${cached.issuer}, client_id=${cached.clientId}). Run \`redash-mcp login\` again.`, | ||
| ); | ||
| } | ||
| if (cached.expiresAt - REFRESH_LEEWAY_MS > now()) { | ||
| return cached; | ||
| } | ||
| if (!cached.refreshToken) { | ||
| throw new AuthError( | ||
| 'Cached access token is expired and no refresh_token is available. Run `redash-mcp login` again.', | ||
| ); | ||
| } | ||
|
|
||
| const discovery = await discover(cfg.issuer); | ||
| const refreshed = await refreshTokens(discovery.token_endpoint, { | ||
| refreshToken: cached.refreshToken, | ||
| clientId: cfg.clientId, | ||
| scopes: cfg.scopes, | ||
| }); | ||
| const updated = tokenResponseToCached(refreshed, cfg, cached.refreshToken); | ||
| await writeTokenCache(cache, updated); | ||
| return updated; | ||
| } | ||
|
|
||
| /** | ||
| * Force a refresh and persist new tokens. Used by the API client when a | ||
| * request fails with 401 — the cached token may have been revoked or rotated | ||
| * by the IdP earlier than its stated expiry. | ||
| */ | ||
| export async function forceRefresh(opts: { cfg?: OidcConfig; cachePath?: string } = {}): Promise<CachedTokens> { | ||
| const cfg = opts.cfg ?? loadOidcConfig(); | ||
| const cache = opts.cachePath ?? tokenCachePath(); | ||
|
|
||
| const cached = await readTokenCache(cache); | ||
| if (!cached?.refreshToken) { | ||
| throw new AuthError('No refresh_token available; run `redash-mcp login` again.'); | ||
| } | ||
| const discovery = await discover(cfg.issuer); | ||
| const refreshed = await refreshTokens(discovery.token_endpoint, { | ||
| refreshToken: cached.refreshToken, | ||
| clientId: cfg.clientId, | ||
| scopes: cfg.scopes, | ||
| }); | ||
| const updated = tokenResponseToCached(refreshed, cfg, cached.refreshToken); | ||
| await writeTokenCache(cache, updated); | ||
| return updated; |
There was a problem hiding this comment.
refresh 토큰 갱신을 단일 비행으로 묶어야 합니다.
getValidTokens()와 forceRefresh()가 같은 캐시를 독립적으로 갱신해서, 만료 직전 병렬 요청이 동일한 refresh token으로 동시에 /token을 칠 수 있습니다. refresh token rotation을 쓰는 IdP면 한 요청이 새 refresh token을 받은 직후 다른 요청이 invalid_grant로 실패하고, 정상 갱신이 끝났어도 일부 요청이 재로그인 경로로 떨어집니다. cache read → refresh → write 전체를 공유 promise/lock으로 직렬화해 주세요.
🤖 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 `@src/auth.ts` around lines 411 - 468, Both getValidTokens and forceRefresh
perform read → refresh → write concurrently; serialize refreshes by adding a
shared in-memory inflight lock so only one refresh for a given cachePath runs at
a time. Implement a module-level Map<string, Promise<CachedTokens>>
inflightRefreshes keyed by cache (tokenCachePath) and factor the refresh logic
into a helper (e.g., performRefreshWithLock(cache, cfg, doRefresh)) that: 1)
checks readTokenCache(cache) and returns cached if still valid
(REFRESH_LEEWAY_MS) before starting a refresh, 2) if a refresh is needed uses an
existing inflight promise from the map or creates one that runs discover(...) →
refreshTokens(...) → tokenResponseToCached(...) → writeTokenCache(...), 3)
awaits that promise, removes it from the map on completion, and returns the
updated tokens. Call this helper from both getValidTokens (when cached expired)
and forceRefresh so parallel callers share the same inflight refresh and avoid
simultaneous /token calls.
| // Gate every other tool call on a valid OIDC token. On cache miss | ||
| // ensureValidTokens starts a device flow in the background and throws an | ||
| // AuthError whose message tells both the user and the assistant what to | ||
| // do next (open URL + call `wait_for_oidc_login`). | ||
| try { | ||
| const { ensureValidTokens } = await import('./auth.js'); | ||
| await ensureValidTokens(); | ||
| } catch (authErr) { | ||
| if (authErr instanceof AuthError) { | ||
| logger.info(`Auth required for ${name}; surfacing login URL via tool response`); | ||
| return { | ||
| isError: true, | ||
| content: [{ | ||
| type: "text", | ||
| text: authErr.message, | ||
| }], | ||
| }; | ||
| } | ||
| throw authErr; | ||
| } |
There was a problem hiding this comment.
공개 대시보드 조회까지 인증 게이트에 걸립니다.
이 pre-check는 모든 도구에 적용돼서 get_public_dashboard도 먼저 ensureValidTokens()를 통과해야 합니다. 하지만 그 도구는 공유 토큰으로 호출하는 공개 엔드포인트(/api/dashboards/public/${token})를 사용하므로, 지금 상태면 공개 링크 조회 시나리오가 로그인 요구로 깨집니다. 인증이 필요한 도구만 게이트하거나 public endpoint는 예외 처리해야 합니다.
🤖 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 `@src/index.ts` around lines 2514 - 2533, The auth pre-check is gating
public-dashboard calls; update the logic around ensureValidTokens so it only
runs for tools that require authentication (e.g., skip when the tool name is
"get_public_dashboard" or when a tool metadata flag like requiresAuth ===
false). Concretely, modify the block that imports and calls ensureValidTokens
(and still handles AuthError) to first check the tool identity or a per-tool
requiresAuth property and only invoke ensureValidTokens for authenticated tools;
keep the same AuthError handling for those cases so behavior and messages remain
unchanged for protected tools.
luis-hong
left a comment
There was a problem hiding this comment.
🤖 github-review
verdict: CHANGES_REQUESTED · confidence: LOW
Summary
API key 인증을 RFC 8628 device flow OIDC 인증으로 전환하는 대규모 auth refactor. 전반적인 구조 (0600 perms, atomic write, issuer 바인딩 캐시, 401 single-retry, AuthError 보존 가드) 는 견고하나 동시성/문서 일관성/테스트 커버리지 면에서 사람 확인이 필요한 항목들이 있음. Auth 코드 + breaking change 라 LOW 로 분류.
Findings
-
[Warning]
src/auth.ts:402(getValidTokens의 refresh 분기) 와redashClient.ts:316(401 interceptor) 양쪽에서 refresh 동시성 보호가 없음. MCP 도구 dispatcher 가 여러 Redash 호출을 동시에 fan-out 하면 (예: dashboard widget 들 병렬 조회) 토큰이 만료 직전일 때 N 개의 동시 refresh 가 IdP 를 때리고, refresh_token rotation 을 켠 IdP (Authentik 포함 가능) 에서는 첫 번째만 성공하고 나머지는invalid_grant로 실패해 사용자가 갑자기 재로그인 요구를 받게 됨. in-flight refresh promise 를 module-level 변수로 캐시 (singleflight) 하는 게 안전. -
[Warning]
src/auth.ts:355startPendingLogin도 같은 race 가능성. 첫 캐시 미스 시 두 도구 호출이 거의 동시에 들어오면if (pendingLogin && ...)둘 다 통과 →beginDeviceFlow두 번 호출 → IdP 에 device code 2 개 발급, 그중 하나만pendingLogin에 기록되고 다른 하나는 고아. 사용자는 어쨌든 한 번의 verification 만 하므로 결과적으론 한쪽만 성공. 영향은 작지만 PR 본문의 "단일 flight (pendingLoginslot)" 보장이 깨짐.pendingLogin을Promise<PendingLogin>으로 두고 동시 호출이 같은 promise 를 await 하게 바꿔야 진짜 single-flight. -
[Warning]
README.md가 device flow 로 전환됐는데도 "OIDC + PKCE browser login" (line 11), "PKCE required" prerequisites,login서브커맨드 도움말의 "Run the OIDC PKCE browser flow", migration 가이드의 "PKCE-required" 등 PKCE 용어를 계속 사용. PR description 에는 v0.1.0-pr.8 에서 device-only 로 단순화했다고 명시되어 있으나 README 가 따라가지 않음. 사용자가 IdP 에 PKCE-only client 를 설정하면 device flow grant_type 이 거부될 수 있음 — 호환성 혼동 가능. -
[Warning]
src/__tests__/redashClient.test.ts가 -901/+103 으로 대폭 축소. API key 인증 케이스 제거는 정당하나getQueries(... q='test'),executeQueryasync polling,executeAdhocQuery,updateQueryfield-pruning, dashboard/alert/widget/snippet/destination 의 method-level 테스트들이 모두 사라졌음. 이들은 auth 와 직교한 endpoint 동작 검증이라 같이 지울 이유가 없음. interceptor 검증을 추가한 가치는 인정하나 endpoint 커버리지 회귀를 동반. -
[Suggestion]
src/auth.ts:18import * as fs from 'fs';가 미사용 (fspfromfs/promises만 사용). 제거해도 무방. -
[Suggestion]
src/redashClient.ts:319,328의(config.headers as any).set?.(...) ?? ((config.headers as any).Authorization = ...)패턴은 의도 (AxiosHeaders 신규 API 와 plain object 양쪽 지원) 는 알겠으나 가독성이 낮음.if (typeof (h as any).set === 'function') (h as any).set('Authorization', token) else (h as any).Authorization = token;같이 풀어 쓰는 게 리뷰어 친화적. -
[Suggestion]
src/auth.ts:484의pl.completion.finally(...).catch(() => {})가 에러를 조용히 삼킴.pendingLogin슬롯을 비우는 목적 자체는 맞지만 디버깅 시 device flow 실패 원인을 로그에서 추적하기 어려움 —.catch((err) => logger.debug(...))정도면 충분.
generated by github-review · elapsed: 137.3s
Summary
기존
REDASH_API_KEY기반 인증을 제거하고, 외부 OIDC IdP가 RFC 8628 device flow로 발급한 짧은 액세스 토큰으로 Redash API에 접근하도록 변경합니다.REDASH_API_KEY환경변수 제거verification_uri_complete+ user_code를 발급, 사용자가 본인 브라우저 한 번 클릭으로 인증Authorization: Bearer <jwt>헤더 자동 주입wait_for_oidc_loginMCP tool — 어시스턴트가 device flow 완료를 await 후 원래 tool을 자동 재시도 (resume-after-auth UX)서버 측 토큰 검증은
Buzzvil/redash-custom#18, IdP 측 OAuth2 client + device code flow는Buzzvil/buzz-k8s-resources#1802. datavil 플러그인 연동은Buzzvil/buzzvil-plugins#101.설계 결정 — 왜 device flow인가
MCP 서버는 Claude Desktop / Claude Code의 stdio 서브프로세스로 동작하고 종종 컨테이너 안에서 돕니다.
127.0.0.1과 사용자 브라우저의127.0.0.1이 다른 network namespace, 별도 port-forward 없이는 도달 불가Device flow는 outbound HTTPS만 필요하고, 사용자의 브라우저는 어떤 장비에 있어도 됩니다. Authentik의
verification_uri_complete이 user_code를 URL에 자동 임베드해주므로 single-click UX이고, PKCE 대비 UX 비용 차이도 거의 없습니다. 초기엔 PKCE/device 환경 분기를 시도했지만 (v0.1.0-pr.5~7), 복잡도 대비 이득이 작아 v0.1.0-pr.8에서 device-only로 단순화.런타임 흐름
list_queries) 호출ensureValidTokens게이트 → 캐시 미스 → device flow 시작 →verification_uri_complete+user_code담은AuthErrorthrow## For the user/## For the assistant두 섹션 명시)wait_for_oidc_loginMCP tool 호출 (블로킹)wait_for_oidc_loginresolve → 어시스턴트가 원래 tool 자동 재시도 → 결과 반환사용자는 한 번 prompt하고 URL을 한 번 클릭하면 됩니다 — 별도 재요청 불필요.
CLI
redash-mcp login서브커맨드도 동일한 device flow를 사용하며, 추가로open/start/xdg-open로 브라우저 자동 spawn 시도 (실패 시 stderr에 URL + user_code 출력).변경 사항
src/auth.tsrequestDeviceCode+pollForDeviceToken+beginDeviceFlow), 토큰 캐시 atomic write + 0600, refresh_token grant,AuthError,makeLoginRequiredError공유 헬퍼 (cold cache miss와 warm 401 retry 양쪽이 동일한 user/assistant 지시 메시지 사용)src/cli.tsserve/login/logout/status/help).REDASH_OIDC_ISSUER+CLIENT_ID필수.serve는 부팅 시 인증 체크 없음src/redashClient.tsKey <apiKey>헤더 제거. axios request interceptor가 매 요청마다getValidTokens()→Bearer주입. response interceptor가 401 1회 재시도 (forceRefresh), refresh 실패 시 fresh device flow URL 담은AuthErrorthrow. 모든 메서드 catch에AuthError instancepreserve 가드 (57 sites) — 이전엔 generic error로 wrap되어 URL 메시지 손실src/index.tsensureValidTokens게이트. 신규wait_for_oidc_loginMCP tool — 어시스턴트가 device flow URL을 사용자에게 노출한 직후 호출하면 polling resolve까지 블로킹 → 동일 turn에서 원래 tool 자동 재시도 가능src/__tests__/auth.test.tsloadOidcConfig,tokenCachePath,getValidTokens(캐시 적중/만료 갱신/issuer 불일치/refresh_token 부재),forceRefresh,performLogout(idempotent),readStatus— 13 케이스src/__tests__/redashClient.test.tssrc/__tests__/integration.test.tsplaywright.config.tsREDASH_OIDC_*env / 캐시 경로 사용README.md.dev수정package.json새 환경 변수
REDASH_URLREDASH_OIDC_ISSUERhttps://authentik.buzzvil.com/application/o/redash-cli/)REDASH_OIDC_CLIENT_IDredash-cli)REDASH_OIDC_AUDIENCECLIENT_IDREDASH_OIDC_SCOPESopenid email offline_accessREDASH_OIDC_TOKEN_CACHE_PATHREDASH_API_KEY보안 모델
REDASH_EXTRA_HEADERS가Authorization을 덮어쓰지 못하도록 차단forceRefresh1회 시도, 실패 시 stale 캐시 clear + 새 device flow URL surfacing (무한 루프 방지)pendingLoginslot) — 동시 호출이 같은 URL 공유토큰 만료 처리
getValidTokens이 refresh_token grant로 자동 갱신 (투명)ensureValidTokens→makeLoginRequiredError→ device flow URL surfacingforceRefresh→ 새 토큰으로 1회 재시도performLogout+makeLoginRequiredError→ 동일 URL surfacingTest plan
npx tsc --noEmitcleannpm test77/77 통과redash-mcp login→ 토큰 캐시 +refreshToken발급 확인 (offline_access scope 부여 후)list_queriesE2E (dev Redash에서 5개 쿼리 반환 성공)wait_for_oidc_loginresume-after-auth 흐름 동작 확인npm run e2e:test) — 유효 토큰 캐시 준비 후device_flow.token_validity늘린 후 60초 timeout 회피 확인 (현재 default 60초)GitHub Release prerelease 시퀀스 (anonymous npx 설치 가능)
GitHub Packages npm은 visibility public이어도 anonymous read 차단 → GitHub Release tarball URL을 npx 인자로 직접 박는 방식으로 우회. v0.1.0-pr.1 ~ v0.1.0-pr.10 단계별 UX 폴리싱, 최신 v0.1.0-pr.10이 production-ready 상태:
마이그레이션 가이드
기존 사용자 (.env / Claude Desktop config):
CLI에서 미리 인증해두려면 (선택):
MCP 서버 안에서 첫 tool 호출 시 자동으로 device flow가 시작되므로, 별도 명시 호출은 필수 아닙니다.
관련 PR
🤖 Generated with Claude Code