fix(auth): support split OAuth and MCP resource URLs #259
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -54,6 +54,7 @@ const oauthConfigSchema = z.object({ | |
| accessTokenTtlSeconds: z.number().int().positive().default(60 * 60), | ||
| refreshTokenTtlSeconds: z.number().int().positive().default(30 * 24 * 60 * 60), | ||
| scopes: z.array(z.string().trim().min(1)).min(1).default(["devspace"]), | ||
| allowedResourceUrls: z.array(z.string().url()).default([]), | ||
|
There was a problem hiding this comment. 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -e
printf '%s\n' '--- applicable repository guidance ---'
find /tmp/coderabbit-repo-knowledge/waishnav-devspace-70958ecd -maxdepth 2 -type f -name '*.md' -print
printf '%s\n' '--- config schema context ---'
cat -n src/config-schema.ts | sed -n '35,75p'
printf '%s\n' '--- direct policy wiring ---'
rg -n -C 8 'allowedResourceUrls|OAuthResourcePolicy' src
printf '%s\n' '--- policy definition ---'
rg -n -C 12 'class OAuthResourcePolicy|interface OAuthResourcePolicy|function OAuthResourcePolicy|OAuthResourcePolicy' srcRepository: Waishnav/devspace Length of output: 28154 🏁 Script executed: #!/bin/bash
set -e
printf '%s\n' '--- scoped conventions and learning ---'
cat /tmp/coderabbit-repo-knowledge/waishnav-devspace-70958ecd/conventions/src.md
cat /tmp/coderabbit-repo-knowledge/waishnav-devspace-70958ecd/conventions/repo-wide.md
cat /tmp/coderabbit-repo-knowledge/waishnav-devspace-70958ecd/learnings/src.md
printf '%s\n' '--- policy behavior and URL normalization ---'
cat -n src/oauth-provider.ts | sed -n '1,125p'
rg -n -C 10 'normalizeOAuthResourceUrl|resourceUrlFromServerUrl|checkResourceAllowed' src
printf '%s\n' '--- package and lock references for MCP SDK ---'
rg -n -C 3 '`@modelcontextprotocol/sdk`' package.json package-lock.json pnpm-lock.yaml yarn.lock 2>/dev/null || trueRepository: Waishnav/devspace Length of output: 21802 🌐 Web query:
💡 Result: In the Citations:
🏁 Script executed: #!/bin/bash
set -e
printf '%s\n' '--- OAuth resource propagation and token validation ---'
cat -n src/oauth-provider.ts | sed -n '145,330p'
printf '%s\n' '--- tests for resource policy and token audience ---'
rg -n -C 12 'resourcePolicy|resource|audience|accessToken|AuthInfo|token' src/oauth-store.test.ts src/oauth-provider.test.ts src 2>/dev/null | head -n 260Repository: Waishnav/devspace Length of output: 24255 🌐 Web query:
💡 Result: In the context of the Model Context Protocol (MCP) and modern OAuth 2.1 implementations, bearer tokens are required to be sent via the HTTP Authorization header [1][2][3]. While security standards strictly mandate HTTPS for all production communication to protect bearer tokens, exceptions are made for development environments using local loopback addresses [2][4][5]. Key points regarding the use of bearer tokens with localhost and HTTP: 1. Security Requirement: OAuth 2.1 and MCP specifications require that all authorization server endpoints be served over HTTPS [2][3]. This is to prevent the interception of sensitive bearer tokens. 2. Loopback/Localhost Exception: Development environments are permitted to use plain HTTP when the resource URI or redirect URI is explicitly localhost (or another loopback host) [2][4][5]. This allows for local development and testing without requiring complex certificate management [4]. 3. Transmission Method: Bearer tokens must be sent using the Authorization header (Authorization: Bearer ) [1][3]. They must not be included in the URI query string, as query parameters can be logged or exposed in browser history [1][6][3]. 4. Implementation Enforcement: Software Development Kits (SDKs), such as the MCP Python or Ruby SDKs, enforce these security boundaries [4][5]. They will generally reject the use of plain HTTP for remote hosts, allowing it only when the host is identified as a local loopback address [4]. In summary, while you should prioritize HTTPS for all traffic, using plain HTTP for bearer token authentication is restricted to localhost/loopback scenarios during development to ensure security remains intact in production environments [4][5]. Citations:
Security Misconfiguration (CWE-319): Cleartext Transmission of Sensitive Information Reachability: Internal · Exploitability: Difficult Restrict additional resources to HTTPS endpoints. Allow 🤖 Prompt for AI Agents |
||
| allowedRedirectHosts: z.array(z.string().trim().min(1)).min(1).default([ | ||
| "chatgpt.com", | ||
| "localhost", | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Make the restart requirement explicit when changing
oauth.allowedResourceUrls.The configuration command only persists the file, while the running server loads the resource policy at startup. A successful update therefore does not affect the current process until restart. Print an explicit restart requirement and add the same guidance to
docs/configuration.mdanddocs/gotchas.md, or implement configuration reload.📍 Affects 2 files
src/cli.ts#L400-L400(this comment)docs/configuration.md#L93-L95🤖 Prompt for AI Agents