Skip to content

feat: add OAuth 2.1 authentication via Scalekit for HTTP transport#1

Open
hashcott wants to merge 2 commits intomasterfrom
feat/oauth
Open

feat: add OAuth 2.1 authentication via Scalekit for HTTP transport#1
hashcott wants to merge 2 commits intomasterfrom
feat/oauth

Conversation

@hashcott
Copy link
Owner

@hashcott hashcott commented Mar 6, 2026

  • Add src/auth/scalekit.ts with JWT validation middleware, resource metadata builder, and scope-checking helper
  • Mount authMiddleware on HTTP server when OAUTH_ENABLED=true (opt-in, stdio unaffected)
  • Expose GET /.well-known/oauth-protected-resource for MCP client OAuth discovery (RFC 9728)
  • Install @scalekit-sdk/node dependency
  • Add .env.example with full Scalekit OAuth configuration template
  • Update README with OAuth setup guide, env var table, and flow diagram

Note

Medium Risk
Introduces new HTTP auth middleware and token validation logic; incorrect configuration or route handling (e.g., duplicated /health handlers) could inadvertently allow unauthenticated access or break clients. Changes are opt-in via OAUTH_ENABLED, limiting impact to deployments that enable it.

Overview
Adds opt-in OAuth 2.1 authentication for the HTTP transport using Scalekit: a new authMiddleware validates Bearer JWTs (with optional audience and per-scope checks) and returns RFC-compliant WWW-Authenticate challenges.

Exposes GET /.well-known/oauth-protected-resource (RFC 9728) for MCP client discovery, expands GET /health to report OAuth/scopes, and wires the middleware into the HTTP server when OAUTH_ENABLED=true.

Updates packaging/docs by adding @scalekit-sdk/node, a full .env.example template, and README setup guidance for Scalekit-based deployments.

Written by Cursor Bugbot for commit 98f5642. This will update automatically on new commits. Configure here.

- Add src/auth/scalekit.ts with JWT validation middleware, resource metadata builder, and scope-checking helper
- Mount authMiddleware on HTTP server when OAUTH_ENABLED=true (opt-in, stdio unaffected)
- Expose GET /.well-known/oauth-protected-resource for MCP client OAuth discovery (RFC 9728)
- Install @scalekit-sdk/node dependency
- Add .env.example with full Scalekit OAuth configuration template
- Update README with OAuth setup guide, env var table, and flow diagram

return {
resource: resourceId,
authorization_servers: [`${envUrl}/resources/${scaleKitResourceId}`],
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wrong authorization_servers URL format in OAuth metadata

High Severity

The authorization_servers URL is constructed as ${envUrl}/resources/${scaleKitResourceId}, but per Scalekit's own documentation, it simply needs to be the environment URL (e.g., ["https://your-org.scalekit.com"]). Additionally, when SCALEKIT_RESOURCE_ID is not set, scaleKitResourceId falls back to resourceId (the full MCP_SERVER_URL), producing a malformed URL like https://env.scalekit.cloud/resources/https://mcp.yourapp.com. MCP clients consuming this discovery metadata will fail to locate the correct authorization server.

Additional Locations (1)

Fix in Cursor Fix in Web

});
return false;
}
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exported requireScope function is never used anywhere

Low Severity

requireScope is exported but never imported or called anywhere in the codebase — it only appears in its own definition and JSDoc comment. This is dead code that adds maintenance burden without providing any functionality.

Fix in Cursor Fix in Web

// Allow public discovery + health check without auth
if (req.path.includes(".well-known") || req.path === "/health") {
return next();
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overly broad .well-known path check bypasses auth

Low Severity

The auth bypass check req.path.includes(".well-known") matches any request path containing .well-known as a substring anywhere, not just the specific well-known endpoint prefix. A more precise check like req.path.startsWith("/.well-known") would be safer and match the intended behavior.

Fix in Cursor Fix in Web

next();
} catch {
res.status(401).set(WWWHeader.key, WWWHeader.value()).end();
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Configuration errors silently swallowed as 401 responses

Medium Severity

The catch block in authMiddleware catches all errors — including the configuration error thrown by getScalekitClient() when Scalekit credentials are missing — and silently returns a 401 with no logging. If OAUTH_ENABLED=true but the credentials aren't configured, every request fails with a generic 401 and the operator has zero diagnostic output to understand why. Configuration errors and token-validation errors are fundamentally different but handled identically here.

Fix in Cursor Fix in Web

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

res.json({
status: "ok",
server: "meta-ads-mcp-server",
version: "1.1.0",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Health endpoint hardcodes stale version string

Medium Severity

The new /health endpoint hardcodes version: "1.1.0" instead of using the dynamic version variable imported from package.json (currently "1.2.2"). The original health endpoint at line 131 correctly referenced the version variable but is now unreachable dead code because this new route is registered first with the same path.

Fix in Cursor Fix in Web

"OAuth disabled — set OAUTH_ENABLED=true to require Scalekit JWT validation"
);
}

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicate unreachable /health route is dead code

Low Severity

A second app.get("/health", ...) handler is registered after the new one at line 100. Since Express matches the first registered route for a given path, this second handler is never reached. It appears to be a leftover from the pre-PR code that wasn't removed when the new health endpoint was added.

Fix in Cursor Fix in Web

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant