diff --git a/.claude/CLAUDE-KNOWLEDGE.md b/.claude/CLAUDE-KNOWLEDGE.md new file mode 100644 index 0000000000..9857ba1196 --- /dev/null +++ b/.claude/CLAUDE-KNOWLEDGE.md @@ -0,0 +1,125 @@ +# CLAUDE-KNOWLEDGE.md + +This file documents key learnings from implementing wildcard domain support in Stack Auth, organized in Q&A format. + +## OAuth Flow and Validation + +### Q: Where does OAuth redirect URL validation happen in the flow? +A: The validation happens in the callback endpoint (`/api/v1/auth/oauth/callback/[provider_id]/route.tsx`), not in the authorize endpoint. The authorize endpoint just stores the redirect URL and redirects to the OAuth provider. The actual validation occurs when the OAuth provider calls back, and the oauth2-server library validates the redirect URL. + +### Q: How do you test OAuth flows that should fail? +A: Use `Auth.OAuth.getMaybeFailingAuthorizationCode()` instead of `Auth.OAuth.getAuthorizationCode()`. The latter expects success (status 303), while the former allows you to test failure cases. The failure happens at the callback stage with a 400 status and specific error message. + +### Q: What error is thrown for invalid redirect URLs in OAuth? +A: The callback endpoint returns a 400 status with the message: "Invalid redirect URI. The URL you are trying to redirect to is not trusted. If it should be, add it to the list of trusted domains in the Stack Auth dashboard." + +## Wildcard Pattern Implementation + +### Q: How do you handle ** vs * precedence in regex patterns? +A: Use a placeholder approach to prevent ** from being corrupted when replacing *: +```typescript +const doubleWildcardPlaceholder = '\x00DOUBLE_WILDCARD\x00'; +regexPattern = regexPattern.replace(/\*\*/g, doubleWildcardPlaceholder); +regexPattern = regexPattern.replace(/\*/g, '[^.]*'); +regexPattern = regexPattern.replace(new RegExp(doubleWildcardPlaceholder, 'g'), '.*'); +``` + +### Q: Why can't you use `new URL()` with wildcard domains? +A: Wildcard characters (* and **) are not valid in URLs and will cause parsing errors. For wildcard domains, you need to manually parse the URL components instead of using the URL constructor. + +### Q: How do you validate URLs with wildcards? +A: Extract the hostname pattern manually and use `matchHostnamePattern()`: +```typescript +const protocolEnd = domain.baseUrl.indexOf('://'); +const protocol = domain.baseUrl.substring(0, protocolEnd + 3); +const afterProtocol = domain.baseUrl.substring(protocolEnd + 3); +const pathStart = afterProtocol.indexOf('/'); +const hostnamePattern = pathStart === -1 ? afterProtocol : afterProtocol.substring(0, pathStart); +``` + +## Testing Best Practices + +### Q: How should you run multiple independent test commands? +A: Use parallel execution by batching tool calls together: +```typescript +// Good - runs in parallel +const [result1, result2] = await Promise.all([ + niceBackendFetch("/endpoint1"), + niceBackendFetch("/endpoint2") +]); + +// In E2E tests, the framework handles this automatically when you +// batch multiple tool calls in a single response +``` + +### Q: What's the correct way to update project configuration in E2E tests? +A: Use the `/api/v1/internal/config/override` endpoint with PATCH method and admin access token: +```typescript +await niceBackendFetch("/api/v1/internal/config/override", { + method: "PATCH", + accessType: "admin", + headers: { + 'x-stack-admin-access-token': adminAccessToken, + }, + body: { + config_override_string: JSON.stringify({ + 'domains.trustedDomains.name': { baseUrl: '...', handlerPath: '...' } + }), + }, +}); +``` + +## Code Organization + +### Q: Where does domain validation logic belong? +A: Core validation functions (`isValidHostnameWithWildcards`, `matchHostnamePattern`) belong in the shared utils package (`packages/stack-shared/src/utils/urls.tsx`) so they can be used by both frontend and backend. + +### Q: How do you simplify validation logic with wildcards? +A: Replace wildcards with valid placeholders before validation: +```typescript +const normalizedDomain = domain.replace(/\*+/g, 'wildcard-placeholder'); +url = new URL(normalizedDomain); // Now this won't throw +``` + +## Debugging E2E Tests + +### Q: What does "ECONNREFUSED" mean in E2E tests? +A: The backend server isn't running. Make sure to start the backend with `pnpm dev` before running E2E tests. + +### Q: How do you debug which stage of OAuth flow is failing? +A: Check the error location: +- Authorize endpoint (307 redirect) - Initial request succeeded +- Callback endpoint (400 error) - Validation failed during callback +- Token endpoint (400 error) - Validation failed during token exchange + +## Git and Development Workflow + +### Q: How should you format git commit messages in this project? +A: Use a HEREDOC to ensure proper formatting: +```bash +git commit -m "$(cat <<'EOF' +Commit message here. + +🤖 Generated with [Claude Code](https://claude.ai/code) + +Co-Authored-By: Claude +EOF +)" +``` + +### Q: What commands should you run before considering a task complete? +A: Always run: +1. `pnpm test run ` - Run tests +2. `pnpm lint` - Check for linting errors +3. `pnpm typecheck` - Check for TypeScript errors + +## Common Pitfalls + +### Q: Why might imports get removed after running lint --fix? +A: ESLint may remove "unused" imports. Always verify your changes after auto-fixing, especially if you're using imports in a way ESLint doesn't recognize (like in test expectations). + +### Q: What's a common linting error in test files? +A: Missing newline at end of file. ESLint requires files to end with a newline character. + +### Q: How do you handle TypeScript errors about missing exports? +A: Double-check that you're only importing what's actually exported from a module. The error "Module declares 'X' locally, but it is not exported" means you're trying to import something that isn't exported. \ No newline at end of file diff --git a/.claude/settings.json b/.claude/settings.json new file mode 100644 index 0000000000..ee52ce43b2 --- /dev/null +++ b/.claude/settings.json @@ -0,0 +1,30 @@ +{ + "$schema": "https://json.schemastore.org/claude-code-settings.json", + "permissions": { + "allow": [ + "Bash(pnpm typecheck:*)", + "Bash(pnpm test:*)", + "Bash(pnpm lint:*)", + "Bash(find:*)", + "Bash(ls:*)", + "Bash(pnpm codegen)", + "Bash(pnpm vitest run:*)", + "Bash(pnpm eslint:*)" + ], + "deny": [] + }, + "includeCoAuthoredBy": false, + "hooks": { + "PostToolUse": [ + { + "matcher": "Edit|MultiEdit|Write", + "hooks": [ + { + "type": "command", + "command": "pnpm run lint --fix" + } + ] + } + ] + } +} diff --git a/.github/recurseml-rules/code_patterns.mdc b/.github/recurseml-rules/code_patterns.mdc index e09a8e1ee3..87f9e65591 100644 --- a/.github/recurseml-rules/code_patterns.mdc +++ b/.github/recurseml-rules/code_patterns.mdc @@ -8,7 +8,6 @@ The following conventions MUST be followed in new code. DON'T report code patterns outside of the examples explicitly listed below: - Never use `void asyncFunction()` or `asyncFunction().catch(console.error)` - use `runAsynchronously(asyncFunction)` instead -- Use `parseJson`/`stringifyJson` from `stack-shared/utils/json` instead of `JSON.parse`/`JSON.stringify` - Instead of Vercel `waitUntil`, use `runAsynchronously(promise, { promiseCallback: waitUntil })` - Don't concatenate URLs as strings - avoid patterns like `/users/${userId}` - Replace non-null assertions with `?? throwErr("message", { extraData })` pattern diff --git a/CLAUDE.md b/CLAUDE.md index 18b87fbb14..743c189775 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -6,18 +6,22 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co ### Essential Commands - **Install dependencies**: `pnpm install` +- **Run tests**: `pnpm test run` (uses Vitest). You can filter with `pnpm test run `. The `run` is important to not trigger watch mode +- **Lint code**: `pnpm lint`. `pnpm lint --fix` will fix some of the linting errors, prefer that over fixing them manually. +- **Type check**: `pnpm typecheck` + +#### Extra commands +These commands are usually already called by the user, but you can remind them to run it for you if they forgot to. - **Build packages**: `pnpm build:packages` - **Generate code**: `pnpm codegen` - **Start dependencies**: `pnpm restart-deps` (resets & restarts Docker containers for DB, Inbucket, etc. Usually already called by the user) - **Run development**: `pnpm dev` (starts all services on different ports. Usually already started by the user in the background) - **Run minimal dev**: `pnpm dev:basic` (only backend and dashboard for resource-limited systems) -- **Run tests**: `pnpm test --no-watch` (uses Vitest). You can filter with `pnpm test --no-watch ` -- **Lint code**: `pnpm lint` -- **Type check**: `pnpm typecheck` ### Testing -- **Run all tests**: `pnpm test --no-watch` -- **Run some tests**: `pnpm test --no-watch ` +You should ALWAYS add new E2E tests when you change the API or SDK interface. Generally, err on the side of creating too many tests; it is super important that our codebase is well-tested, due to the nature of the industry we're building in. +- **Run all tests**: `pnpm test run` +- **Run some tests**: `pnpm test run ` ### Database Commands - **Generate migration**: `pnpm db:migration-gen` @@ -62,15 +66,15 @@ The API follows a RESTful design with routes organized by resource type: - OAuth providers: `/api/latest/oauth-providers/*` ### Development Ports -- 8100: Dev launchpad -- 8101: Dashboard -- 8102: Backend API -- 8103: Demo app -- 8104: Documentation -- 8105: Inbucket (email testing) -- 8106: Prisma Studio +To see all development ports, refer to the index.html of `apps/dev-launchpad/public/index.html`. ## Important Notes - Environment variables are pre-configured in `.env.development` files -- Code generation (`pnpm codegen`) must be run after schema changes +- Always run typecheck, lint, and test to make sure your changes are working as expected. You can save time by only linting and testing the files you've changed (and/or related E2E tests). - The project uses a custom route handler system in the backend for consistent API responses +- Sometimes, the typecheck will give errors along the line of "Cannot assign Buffer to Uint8Array" or similar, on changes that are completely unrelated to your own changes. If that happens, tell the user to run `pnpm clean && pnpm i && pnpm run codegen && pnpm build:packages`, and restart the dev server (you cannot run this yourself). After that's done, the typecheck should pass. +- When writing tests, prefer .toMatchInlineSnapshot over other selectors, if possible. You can check (and modify) the snapshot-serializer.ts file to see how the snapshots are formatted and how non-deterministic values are handled. +- Whenever you learn something new, or at the latest right before you call the `Stop` tool, write whatever you learned into the ./claude/CLAUDE-KNOWLEDGE.md file, in the Q&A format in there. You will later be able to look up knowledge from there (based on the question you asked). + +### Code-related +- Use ES6 maps instead of records wherever you can. diff --git a/apps/backend/src/app/api/latest/auth/oauth/callback/[provider_id]/route.tsx b/apps/backend/src/app/api/latest/auth/oauth/callback/[provider_id]/route.tsx index 25aebe199b..b12479d3bc 100644 --- a/apps/backend/src/app/api/latest/auth/oauth/callback/[provider_id]/route.tsx +++ b/apps/backend/src/app/api/latest/auth/oauth/callback/[provider_id]/route.tsx @@ -398,6 +398,7 @@ const handler = createSmartRouteHandler({ } catch (error) { if (error instanceof InvalidClientError) { if (error.message.includes("redirect_uri") || error.message.includes("redirectUri")) { + console.log("User is trying to authorize OAuth with an invalid redirect URI", error, { redirectUri: oauthRequest.query?.redirect_uri, clientId: oauthRequest.query?.client_id }); throw new KnownErrors.RedirectUrlNotWhitelisted(); } } else if (error instanceof InvalidScopeError) { diff --git a/apps/backend/src/app/api/latest/auth/passkey/register/verification-code-handler.tsx b/apps/backend/src/app/api/latest/auth/passkey/register/verification-code-handler.tsx index a2de637c49..44ac869a17 100644 --- a/apps/backend/src/app/api/latest/auth/passkey/register/verification-code-handler.tsx +++ b/apps/backend/src/app/api/latest/auth/passkey/register/verification-code-handler.tsx @@ -1,3 +1,4 @@ +import { validateRedirectUrl } from "@/lib/redirect-urls"; import { getPrismaClientForTenancy, retryTransaction } from "@/prisma-client"; import { createVerificationCodeHandler } from "@/route-handlers/verification-code-handler"; import { VerificationCodeType } from "@prisma/client"; @@ -50,35 +51,16 @@ export const registerVerificationCodeHandler = createVerificationCodeHandler({ } // HACK: we validate origin and rpid outside of simpleauth, this should be replaced once we have a primary authentication domain - - let expectedRPID = ""; - let expectedOrigin = ""; const clientDataJSON = decodeClientDataJSON(credential.response.clientDataJSON); const { origin } = clientDataJSON; - const localhostAllowed = tenancy.config.domains.allowLocalhost; - const parsedOrigin = new URL(origin); - const isLocalhost = parsedOrigin.hostname === "localhost"; - - if (!localhostAllowed && isLocalhost) { - throw new KnownErrors.PasskeyAuthenticationFailed("Passkey registration failed because localhost is not allowed"); - } - if (localhostAllowed && isLocalhost) { - expectedRPID = parsedOrigin.hostname; - expectedOrigin = origin; + if (!validateRedirectUrl(origin, tenancy)) { + throw new KnownErrors.PasskeyRegistrationFailed("Passkey registration failed because the origin is not allowed"); } - if (!isLocalhost) { - if (!Object.values(tenancy.config.domains.trustedDomains) - .filter(e => e.baseUrl) - .map(e => e.baseUrl) - .includes(parsedOrigin.origin)) { - throw new KnownErrors.PasskeyAuthenticationFailed("Passkey registration failed because the origin is not allowed"); - } else { - expectedRPID = parsedOrigin.hostname; - expectedOrigin = origin; - } - } + const parsedOrigin = new URL(origin); + const expectedRPID = parsedOrigin.hostname; + const expectedOrigin = origin; let verification; diff --git a/apps/backend/src/app/api/latest/auth/passkey/sign-in/verification-code-handler.tsx b/apps/backend/src/app/api/latest/auth/passkey/sign-in/verification-code-handler.tsx index 690c0753a5..842bb3d4f7 100644 --- a/apps/backend/src/app/api/latest/auth/passkey/sign-in/verification-code-handler.tsx +++ b/apps/backend/src/app/api/latest/auth/passkey/sign-in/verification-code-handler.tsx @@ -1,3 +1,4 @@ +import { validateRedirectUrl } from "@/lib/redirect-urls"; import { createAuthTokens } from "@/lib/tokens"; import { getPrismaClientForTenancy } from "@/prisma-client"; import { createVerificationCodeHandler } from "@/route-handlers/verification-code-handler"; @@ -63,34 +64,16 @@ export const passkeySignInVerificationCodeHandler = createVerificationCodeHandle } // HACK: we validate origin and rpid outside of simpleauth, this should be replaced once we have a primary authentication domain - let expectedRPID = ""; - let expectedOrigin = ""; const clientDataJSON = decodeClientDataJSON(authentication_response.response.clientDataJSON); const { origin } = clientDataJSON; - const localhostAllowed = tenancy.config.domains.allowLocalhost; - const parsedOrigin = new URL(origin); - const isLocalhost = parsedOrigin.hostname === "localhost"; - - if (!localhostAllowed && isLocalhost) { - throw new KnownErrors.PasskeyAuthenticationFailed("Passkey authentication failed because localhost is not allowed"); - } - if (localhostAllowed && isLocalhost) { - expectedRPID = parsedOrigin.hostname; - expectedOrigin = origin; + if (!validateRedirectUrl(origin, tenancy)) { + throw new KnownErrors.PasskeyAuthenticationFailed("Passkey authentication failed because the origin is not allowed"); } - if (!isLocalhost) { - if (!Object.values(tenancy.config.domains.trustedDomains) - .filter(e => e.baseUrl) - .map(e => e.baseUrl) - .includes(parsedOrigin.origin)) { - throw new KnownErrors.PasskeyAuthenticationFailed("Passkey authentication failed because the origin is not allowed"); - } else { - expectedRPID = parsedOrigin.hostname; - expectedOrigin = origin; - } - } + const parsedOrigin = new URL(origin); + const expectedRPID = parsedOrigin.hostname; + const expectedOrigin = origin; let authVerify; authVerify = await verifyAuthenticationResponse({ diff --git a/apps/backend/src/lib/redirect-urls.test.tsx b/apps/backend/src/lib/redirect-urls.test.tsx new file mode 100644 index 0000000000..4e37b7cc06 --- /dev/null +++ b/apps/backend/src/lib/redirect-urls.test.tsx @@ -0,0 +1,476 @@ +import { describe, expect, it } from 'vitest'; +import { validateRedirectUrl } from './redirect-urls'; +import { Tenancy } from './tenancies'; + +describe('validateRedirectUrl', () => { + const createMockTenancy = (config: Partial): Tenancy => { + return { + config: { + domains: { + allowLocalhost: false, + trustedDomains: {}, + ...config.domains, + }, + ...config, + }, + } as Tenancy; + }; + + describe('exact domain matching', () => { + it('should validate exact domain matches', () => { + const tenancy = createMockTenancy({ + domains: { + allowLocalhost: false, + trustedDomains: { + '1': { baseUrl: 'https://example.com', handlerPath: '/handler' }, + }, + }, + }); + + expect(validateRedirectUrl('https://example.com/handler', tenancy)).toBe(true); + expect(validateRedirectUrl('https://example.com/handler/callback', tenancy)).toBe(true); + expect(validateRedirectUrl('https://example.com/other', tenancy)).toBe(true); // Any path on trusted domain is valid + expect(validateRedirectUrl('https://example.com/', tenancy)).toBe(true); // Root path is also valid + expect(validateRedirectUrl('https://other.com/handler', tenancy)).toBe(false); // Different domain is not trusted + expect(validateRedirectUrl('https://example.com.other.com/handler', tenancy)).toBe(false); // Similar different domain is also not trusted + }); + + it('should validate protocol matching', () => { + const tenancy = createMockTenancy({ + domains: { + allowLocalhost: false, + trustedDomains: { + '1': { baseUrl: 'https://example.com', handlerPath: '/handler' }, + }, + }, + }); + + expect(validateRedirectUrl('https://example.com/handler', tenancy)).toBe(true); + expect(validateRedirectUrl('https://example.com/any/path', tenancy)).toBe(true); // Any path is valid + expect(validateRedirectUrl('http://example.com/handler', tenancy)).toBe(false); // Wrong protocol + }); + }); + + describe('wildcard domain matching', () => { + it('should validate single wildcard subdomain patterns', () => { + const tenancy = createMockTenancy({ + domains: { + allowLocalhost: false, + trustedDomains: { + '1': { baseUrl: 'https://*.example.com', handlerPath: '/handler' }, + }, + }, + }); + + expect(validateRedirectUrl('https://api.example.com/handler', tenancy)).toBe(true); + expect(validateRedirectUrl('https://api.example.com/any/path', tenancy)).toBe(true); // Any path is valid + expect(validateRedirectUrl('https://www.example.com/', tenancy)).toBe(true); // Root path is valid + expect(validateRedirectUrl('https://staging.example.com/other', tenancy)).toBe(true); + expect(validateRedirectUrl('https://example.com/handler', tenancy)).toBe(false); // Not a subdomain + expect(validateRedirectUrl('https://api.v2.example.com/handler', tenancy)).toBe(false); // Too many subdomains for single * + }); + + it('should validate double wildcard patterns', () => { + const tenancy = createMockTenancy({ + domains: { + allowLocalhost: false, + trustedDomains: { + '1': { baseUrl: 'https://**.example.com', handlerPath: '/handler' }, + }, + }, + }); + + expect(validateRedirectUrl('https://api.example.com/handler', tenancy)).toBe(true); + expect(validateRedirectUrl('https://api.example.com/', tenancy)).toBe(true); // Root path is valid + expect(validateRedirectUrl('https://api.v2.example.com/other/path', tenancy)).toBe(true); // Any path is valid + expect(validateRedirectUrl('https://a.b.c.example.com/deep/nested/path', tenancy)).toBe(true); + expect(validateRedirectUrl('https://example.com/handler', tenancy)).toBe(false); // Not a subdomain + }); + + it('should validate wildcard patterns with prefixes', () => { + const tenancy = createMockTenancy({ + domains: { + allowLocalhost: false, + trustedDomains: { + '1': { baseUrl: 'https://api-*.example.com', handlerPath: '/handler' }, + }, + }, + }); + + expect(validateRedirectUrl('https://api-v1.example.com/handler', tenancy)).toBe(true); + expect(validateRedirectUrl('https://api-v2.example.com/any/path', tenancy)).toBe(true); // Any path is valid + expect(validateRedirectUrl('https://api-prod.example.com/', tenancy)).toBe(true); // Root path is valid + expect(validateRedirectUrl('https://api.example.com/handler', tenancy)).toBe(false); // Missing prefix + expect(validateRedirectUrl('https://v1-api.example.com/handler', tenancy)).toBe(false); // Wrong prefix position + }); + + it('should validate multiple wildcard patterns', () => { + const tenancy = createMockTenancy({ + domains: { + allowLocalhost: false, + trustedDomains: { + '1': { baseUrl: 'https://*.*.org', handlerPath: '/handler' }, + }, + }, + }); + + expect(validateRedirectUrl('https://mail.example.org/handler', tenancy)).toBe(true); + expect(validateRedirectUrl('https://mail.example.org/any/path', tenancy)).toBe(true); // Any path is valid + expect(validateRedirectUrl('https://api.company.org/', tenancy)).toBe(true); // Root path is valid + expect(validateRedirectUrl('https://example.org/handler', tenancy)).toBe(false); // Not enough subdomain levels + expect(validateRedirectUrl('https://a.b.c.org/handler', tenancy)).toBe(false); // Too many subdomain levels + }); + }); + + describe('localhost handling', () => { + it('should allow localhost when configured', () => { + const tenancy = createMockTenancy({ + domains: { + allowLocalhost: true, + trustedDomains: {}, + }, + }); + + expect(validateRedirectUrl('http://localhost/callback', tenancy)).toBe(true); + expect(validateRedirectUrl('http://localhost:3000/callback', tenancy)).toBe(true); + expect(validateRedirectUrl('http://127.0.0.1/callback', tenancy)).toBe(true); + expect(validateRedirectUrl('http://sub.localhost/callback', tenancy)).toBe(true); + }); + + it('should reject localhost when not configured', () => { + const tenancy = createMockTenancy({ + domains: { + allowLocalhost: false, + trustedDomains: {}, + }, + }); + + expect(validateRedirectUrl('http://localhost/callback', tenancy)).toBe(false); + expect(validateRedirectUrl('http://127.0.0.1/callback', tenancy)).toBe(false); + }); + }); + + describe('path validation', () => { + it('should allow any path on trusted domains (handlerPath is only a default)', () => { + const tenancy = createMockTenancy({ + domains: { + allowLocalhost: false, + trustedDomains: { + '1': { baseUrl: 'https://example.com', handlerPath: '/auth/handler' }, + }, + }, + }); + + // All paths on the trusted domain should be valid + expect(validateRedirectUrl('https://example.com/auth/handler', tenancy)).toBe(true); + expect(validateRedirectUrl('https://example.com/auth/handler/callback', tenancy)).toBe(true); + expect(validateRedirectUrl('https://example.com/auth', tenancy)).toBe(true); // Any path is valid + expect(validateRedirectUrl('https://example.com/other/handler', tenancy)).toBe(true); // Any path is valid + expect(validateRedirectUrl('https://example.com/', tenancy)).toBe(true); // Root is valid + }); + + it('should work with wildcard domains (any path is valid)', () => { + const tenancy = createMockTenancy({ + domains: { + allowLocalhost: false, + trustedDomains: { + '1': { baseUrl: 'https://*.example.com', handlerPath: '/api/auth' }, + }, + }, + }); + + // All paths on matched domains should be valid + expect(validateRedirectUrl('https://api.example.com/api/auth', tenancy)).toBe(true); + expect(validateRedirectUrl('https://app.example.com/api/auth/callback', tenancy)).toBe(true); + expect(validateRedirectUrl('https://api.example.com/api', tenancy)).toBe(true); // Any path is valid + expect(validateRedirectUrl('https://api.example.com/other/auth', tenancy)).toBe(true); // Any path is valid + expect(validateRedirectUrl('https://api.example.com/', tenancy)).toBe(true); // Root is valid + }); + }); + + describe('port number handling with wildcards', () => { + it('should handle exact domain without port (defaults to standard ports)', () => { + const tenancy = createMockTenancy({ + domains: { + allowLocalhost: false, + trustedDomains: { + '1': { baseUrl: 'https://localhost', handlerPath: '/' }, + }, + }, + }); + + // https://localhost should match https://localhost:443 (default HTTPS port) + expect(validateRedirectUrl('https://localhost/', tenancy)).toBe(true); + expect(validateRedirectUrl('https://localhost:443/', tenancy)).toBe(true); + + // Should NOT match other ports + expect(validateRedirectUrl('https://localhost:3000/', tenancy)).toBe(false); + expect(validateRedirectUrl('https://localhost:8080/', tenancy)).toBe(false); + }); + + it('should handle http domain without port (defaults to port 80)', () => { + const tenancy = createMockTenancy({ + domains: { + allowLocalhost: false, + trustedDomains: { + '1': { baseUrl: 'http://localhost', handlerPath: '/' }, + }, + }, + }); + + // http://localhost should match http://localhost:80 (default HTTP port) + expect(validateRedirectUrl('http://localhost/', tenancy)).toBe(true); + expect(validateRedirectUrl('http://localhost:80/', tenancy)).toBe(true); + + // Should NOT match other ports + expect(validateRedirectUrl('http://localhost:3000/', tenancy)).toBe(false); + expect(validateRedirectUrl('http://localhost:8080/', tenancy)).toBe(false); + }); + + it('should handle wildcard with port pattern to match any port', () => { + const tenancy = createMockTenancy({ + domains: { + allowLocalhost: false, + trustedDomains: { + '1': { baseUrl: 'https://localhost:*', handlerPath: '/' }, + }, + }, + }); + + // Should match localhost on any port + expect(validateRedirectUrl('https://localhost/', tenancy)).toBe(true); + expect(validateRedirectUrl('https://localhost:443/', tenancy)).toBe(true); + expect(validateRedirectUrl('https://localhost:3000/', tenancy)).toBe(true); + expect(validateRedirectUrl('https://localhost:8080/', tenancy)).toBe(true); + expect(validateRedirectUrl('https://localhost:12345/', tenancy)).toBe(true); + + // Should NOT match different hostnames + expect(validateRedirectUrl('https://example.com:3000/', tenancy)).toBe(false); + }); + + it('should handle subdomain wildcard without affecting port matching', () => { + const tenancy = createMockTenancy({ + domains: { + allowLocalhost: false, + trustedDomains: { + '1': { baseUrl: 'https://*.localhost', handlerPath: '/' }, + }, + }, + }); + + // Should match subdomains on default port only + expect(validateRedirectUrl('https://api.localhost/', tenancy)).toBe(true); + expect(validateRedirectUrl('https://api.localhost:443/', tenancy)).toBe(true); + expect(validateRedirectUrl('https://app.localhost/', tenancy)).toBe(true); + + // Should NOT match subdomains on other ports + expect(validateRedirectUrl('https://api.localhost:3000/', tenancy)).toBe(false); + expect(validateRedirectUrl('https://app.localhost:8080/', tenancy)).toBe(false); + + // Should NOT match the base domain (no subdomain) + expect(validateRedirectUrl('https://localhost/', tenancy)).toBe(false); + }); + + it('should handle subdomain wildcard WITH port wildcard', () => { + const tenancy = createMockTenancy({ + domains: { + allowLocalhost: false, + trustedDomains: { + '1': { baseUrl: 'https://*.localhost:*', handlerPath: '/' }, + }, + }, + }); + + // Should match subdomains on any port + expect(validateRedirectUrl('https://api.localhost/', tenancy)).toBe(true); + expect(validateRedirectUrl('https://api.localhost:3000/', tenancy)).toBe(true); + expect(validateRedirectUrl('https://app.localhost:8080/', tenancy)).toBe(true); + expect(validateRedirectUrl('https://staging.localhost:12345/', tenancy)).toBe(true); + + // Should NOT match the base domain (no subdomain) + expect(validateRedirectUrl('https://localhost:3000/', tenancy)).toBe(false); + }); + + it('should handle TLD wildcard without affecting port', () => { + const tenancy = createMockTenancy({ + domains: { + allowLocalhost: false, + trustedDomains: { + '1': { baseUrl: 'https://localhost.*', handlerPath: '/' }, + }, + }, + }); + + // Should match different TLDs on default port + expect(validateRedirectUrl('https://localhost.de/', tenancy)).toBe(true); + expect(validateRedirectUrl('https://localhost.com/', tenancy)).toBe(true); + expect(validateRedirectUrl('https://localhost.org/', tenancy)).toBe(true); + expect(validateRedirectUrl('https://localhost.de:443/', tenancy)).toBe(true); + + // Should NOT match different TLDs on other ports + expect(validateRedirectUrl('https://localhost.de:3000/', tenancy)).toBe(false); + expect(validateRedirectUrl('https://localhost.com:8080/', tenancy)).toBe(false); + + // Should NOT match without TLD + expect(validateRedirectUrl('https://localhost/', tenancy)).toBe(false); + }); + + it('should handle specific port in wildcard pattern', () => { + const tenancy = createMockTenancy({ + domains: { + allowLocalhost: false, + trustedDomains: { + '1': { baseUrl: 'https://*.example.com:8080', handlerPath: '/' }, + }, + }, + }); + + // Should match subdomains only on port 8080 + expect(validateRedirectUrl('https://api.example.com:8080/', tenancy)).toBe(true); + expect(validateRedirectUrl('https://app.example.com:8080/', tenancy)).toBe(true); + + // Should NOT match on other ports + expect(validateRedirectUrl('https://api.example.com/', tenancy)).toBe(false); + expect(validateRedirectUrl('https://api.example.com:443/', tenancy)).toBe(false); + expect(validateRedirectUrl('https://api.example.com:3000/', tenancy)).toBe(false); + }); + + it('should handle double wildcard with port patterns', () => { + const tenancy = createMockTenancy({ + domains: { + allowLocalhost: false, + trustedDomains: { + '1': { baseUrl: 'https://**.example.com:*', handlerPath: '/' }, + }, + }, + }); + + // Should match any subdomain depth on any port + expect(validateRedirectUrl('https://api.example.com:3000/', tenancy)).toBe(true); + expect(validateRedirectUrl('https://api.v2.example.com:8080/', tenancy)).toBe(true); + expect(validateRedirectUrl('https://staging.api.v2.example.com:12345/', tenancy)).toBe(true); + + // Should NOT match base domain + expect(validateRedirectUrl('https://example.com:3000/', tenancy)).toBe(false); + }); + + it('should handle single wildcard (*:*) pattern correctly', () => { + const tenancy = createMockTenancy({ + domains: { + allowLocalhost: false, + trustedDomains: { + '1': { baseUrl: 'http://*:*', handlerPath: '/' }, + }, + }, + }); + + // * matches single level (no dots), so should match simple hostnames on any port + expect(validateRedirectUrl('http://localhost:3000/', tenancy)).toBe(true); + expect(validateRedirectUrl('http://localhost:8080/', tenancy)).toBe(true); + expect(validateRedirectUrl('http://app:12345/', tenancy)).toBe(true); + + // Should NOT match hostnames with dots (need ** for that) + expect(validateRedirectUrl('http://example.com:8080/', tenancy)).toBe(false); + expect(validateRedirectUrl('http://api.test.com:12345/', tenancy)).toBe(false); + + // Should NOT match https (different protocol) + expect(validateRedirectUrl('https://localhost:3000/', tenancy)).toBe(false); + }); + + it('should handle double wildcard (**:*) pattern to match any hostname on any port', () => { + const tenancy = createMockTenancy({ + domains: { + allowLocalhost: false, + trustedDomains: { + '1': { baseUrl: 'http://**:*', handlerPath: '/' }, + }, + }, + }); + + // ** matches any characters including dots, so should match any hostname on any port + expect(validateRedirectUrl('http://localhost:3000/', tenancy)).toBe(true); + expect(validateRedirectUrl('http://example.com:8080/', tenancy)).toBe(true); + expect(validateRedirectUrl('http://api.test.com:12345/', tenancy)).toBe(true); + expect(validateRedirectUrl('http://192.168.1.1:80/', tenancy)).toBe(true); + expect(validateRedirectUrl('http://deeply.nested.subdomain.example.com:9999/', tenancy)).toBe(true); + + // Should NOT match https (different protocol) + expect(validateRedirectUrl('https://localhost:3000/', tenancy)).toBe(false); + }); + + it('should correctly distinguish between port wildcard and subdomain wildcard', () => { + const tenancy = createMockTenancy({ + domains: { + allowLocalhost: false, + trustedDomains: { + '1': { baseUrl: 'https://app-*.example.com', handlerPath: '/' }, + '2': { baseUrl: 'https://api.example.com:*', handlerPath: '/' }, + }, + }, + }); + + // First pattern should match app-* subdomains on default port + expect(validateRedirectUrl('https://app-v1.example.com/', tenancy)).toBe(true); + expect(validateRedirectUrl('https://app-staging.example.com/', tenancy)).toBe(true); + expect(validateRedirectUrl('https://app-v1.example.com:3000/', tenancy)).toBe(false); + + // Second pattern should match api.example.com on any port + expect(validateRedirectUrl('https://api.example.com/', tenancy)).toBe(true); + expect(validateRedirectUrl('https://api.example.com:3000/', tenancy)).toBe(true); + expect(validateRedirectUrl('https://api.example.com:8080/', tenancy)).toBe(true); + expect(validateRedirectUrl('https://api-v1.example.com:3000/', tenancy)).toBe(false); + }); + }); + + describe('edge cases', () => { + it('should handle invalid URLs', () => { + const tenancy = createMockTenancy({ + domains: { + allowLocalhost: false, + trustedDomains: { + '1': { baseUrl: 'https://example.com', handlerPath: '/handler' }, + }, + }, + }); + + expect(validateRedirectUrl('not-a-url', tenancy)).toBe(false); + expect(validateRedirectUrl('', tenancy)).toBe(false); + expect(validateRedirectUrl('javascript:alert(1)', tenancy)).toBe(false); + }); + + it('should handle missing baseUrl in domain config', () => { + const tenancy = createMockTenancy({ + domains: { + allowLocalhost: false, + trustedDomains: { + '1': { baseUrl: undefined as any, handlerPath: '/handler' }, + }, + }, + }); + + expect(validateRedirectUrl('https://example.com/handler', tenancy)).toBe(false); + }); + + it('should handle multiple trusted domains with wildcards', () => { + const tenancy = createMockTenancy({ + domains: { + allowLocalhost: false, + trustedDomains: { + '1': { baseUrl: 'https://example.com', handlerPath: '/handler' }, + '2': { baseUrl: 'https://*.staging.com', handlerPath: '/auth' }, + '3': { baseUrl: 'https://**.production.com', handlerPath: '/callback' }, + }, + }, + }); + + // Any path on trusted domains should be valid + expect(validateRedirectUrl('https://example.com/handler', tenancy)).toBe(true); + expect(validateRedirectUrl('https://example.com/any/path', tenancy)).toBe(true); + expect(validateRedirectUrl('https://api.staging.com/auth', tenancy)).toBe(true); + expect(validateRedirectUrl('https://api.staging.com/different/path', tenancy)).toBe(true); + expect(validateRedirectUrl('https://api.v2.production.com/callback', tenancy)).toBe(true); + expect(validateRedirectUrl('https://api.v2.production.com/', tenancy)).toBe(true); + expect(validateRedirectUrl('https://other.com/handler', tenancy)).toBe(false); + }); + }); +}); diff --git a/apps/backend/src/lib/redirect-urls.tsx b/apps/backend/src/lib/redirect-urls.tsx index 33acfa8135..a7d486c074 100644 --- a/apps/backend/src/lib/redirect-urls.tsx +++ b/apps/backend/src/lib/redirect-urls.tsx @@ -1,33 +1,85 @@ import { StackAssertionError, captureError } from "@stackframe/stack-shared/dist/utils/errors"; -import { createUrlIfValid, isLocalhost } from "@stackframe/stack-shared/dist/utils/urls"; +import { createUrlIfValid, isLocalhost, matchHostnamePattern } from "@stackframe/stack-shared/dist/utils/urls"; import { Tenancy } from "./tenancies"; +/** + * Normalizes a URL to include explicit default ports for comparison + */ +function normalizePort(url: URL): string { + const defaultPorts = new Map([['https:', '443'], ['http:', '80']]); + const port = url.port || defaultPorts.get(url.protocol) || ''; + return port ? `${url.hostname}:${port}` : url.hostname; +} + +/** + * Checks if a URL uses the default port for its protocol + */ +function isDefaultPort(url: URL): boolean { + return !url.port || + (url.protocol === 'https:' && url.port === '443') || + (url.protocol === 'http:' && url.port === '80'); +} + +/** + * Checks if two URLs have matching ports (considering default ports) + */ +function portsMatch(url1: URL, url2: URL): boolean { + return normalizePort(url1) === normalizePort(url2); +} + +/** + * Validates a URL against a domain pattern (with or without wildcards) + */ +function matchesDomain(testUrl: URL, pattern: string): boolean { + const baseUrl = createUrlIfValid(pattern); + + // If pattern is invalid as a URL, it might contain wildcards + if (!baseUrl || pattern.includes('*')) { + // Parse wildcard pattern manually + const match = pattern.match(/^([^:]+:\/\/)([^/]*)(.*)$/); + if (!match) { + captureError("invalid-redirect-domain", new StackAssertionError("Invalid domain pattern", { pattern })); + return false; + } + + const [, protocol, hostPattern] = match; + + // Check protocol + if (testUrl.protocol + '//' !== protocol) { + return false; + } + + // Check host with wildcard pattern + const hasPortInPattern = hostPattern.includes(':'); + if (hasPortInPattern) { + // Pattern includes port - match against normalized host:port + return matchHostnamePattern(hostPattern, normalizePort(testUrl)); + } else { + // Pattern doesn't include port - match hostname only, require default port + return matchHostnamePattern(hostPattern, testUrl.hostname) && isDefaultPort(testUrl); + } + } + + // For non-wildcard patterns, use URL comparison + return baseUrl.protocol === testUrl.protocol && + baseUrl.hostname === testUrl.hostname && + portsMatch(baseUrl, testUrl); +} + export function validateRedirectUrl( urlOrString: string | URL, tenancy: Tenancy, ): boolean { const url = createUrlIfValid(urlOrString); if (!url) return false; + + // Check localhost permission if (tenancy.config.domains.allowLocalhost && isLocalhost(url)) { return true; } - return Object.values(tenancy.config.domains.trustedDomains).some((domain) => { - if (!domain.baseUrl) { - return false; - } - - const testUrl = url; - const baseUrl = createUrlIfValid(domain.baseUrl); - if (!baseUrl) { - captureError("invalid-redirect-domain", new StackAssertionError("Invalid redirect domain; maybe this should be fixed in the database", { - domain: domain.baseUrl, - })); - return false; - } - - const sameOrigin = baseUrl.protocol === testUrl.protocol && baseUrl.hostname === testUrl.hostname; - const isSubPath = testUrl.pathname.startsWith(baseUrl.pathname); - return sameOrigin && isSubPath; - }); + // Check trusted domains + return Object.values(tenancy.config.domains.trustedDomains).some(domain => + domain.baseUrl && matchesDomain(url, domain.baseUrl) + ); } diff --git a/apps/backend/src/oauth/model.tsx b/apps/backend/src/oauth/model.tsx index fd049393fc..eff9bd497b 100644 --- a/apps/backend/src/oauth/model.tsx +++ b/apps/backend/src/oauth/model.tsx @@ -52,9 +52,13 @@ export class OAuthModel implements AuthorizationCodeModel { let redirectUris: string[] = []; try { - redirectUris = Object.entries(tenancy.config.domains.trustedDomains).map( - ([_, domain]) => new URL(domain.handlerPath, domain.baseUrl).toString() - ); + redirectUris = Object.entries(tenancy.config.domains.trustedDomains) + // note that this may include wildcard domains, which is fine because we correctly account for them in + // model.validateRedirectUri(...) + .filter(([_, domain]) => { + return domain.baseUrl; + }) + .map(([_, domain]) => new URL(domain.handlerPath, domain.baseUrl).toString()); } catch (e) { captureError("get-oauth-redirect-urls", { error: e, diff --git a/apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/domains/page-client.tsx b/apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/domains/page-client.tsx index 7cd55b2e82..99fdee72b4 100644 --- a/apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/domains/page-client.tsx +++ b/apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/domains/page-client.tsx @@ -5,7 +5,7 @@ import { SettingCard, SettingSwitch } from "@/components/settings"; import { AdminDomainConfig, AdminProject } from "@stackframe/stack"; import { yupString } from "@stackframe/stack-shared/dist/schema-fields"; import { StackAssertionError } from "@stackframe/stack-shared/dist/utils/errors"; -import { isValidHostname, isValidUrl } from "@stackframe/stack-shared/dist/utils/urls"; +import { isValidHostnameWithWildcards, isValidUrl } from "@stackframe/stack-shared/dist/utils/urls"; import { Accordion, AccordionContent, AccordionItem, AccordionTrigger, ActionCell, ActionDialog, Alert, Button, Table, TableBody, TableCell, TableHead, TableHeader, TableRow, Typography } from "@stackframe/stack-ui"; import React from "react"; import * as yup from "yup"; @@ -35,7 +35,7 @@ function EditDialog(props: { .test({ name: 'domain', message: (params) => `Invalid domain`, - test: (value) => value == null || isValidHostname(value) + test: (value) => value == null || isValidHostnameWithWildcards(value) }) .test({ name: 'unique-domain', @@ -77,6 +77,11 @@ function EditDialog(props: { return false; } + // Don't allow adding www. to wildcard domains + if (domain.includes('*')) { + return false; + } + const httpsUrl = 'https://' + domain; if (!isValidUrl(httpsUrl)) { return false; @@ -153,7 +158,16 @@ function EditDialog(props: { render={(form) => ( <> - Please ensure you own or have control over this domain. Also note that each subdomain (e.g. blog.example.com, app.example.com) is treated as a distinct domain. +
+

Please ensure you own or have control over this domain. Also note that each subdomain (e.g. blog.example.com, app.example.com) is treated as a distinct domain.

+

Wildcard domains: You can use wildcards to match multiple domains:

+
    +
  • *.example.com - matches any single subdomain (e.g., api.example.com, www.example.com)
    • +
  • **.example.com - matches any subdomain level (e.g., api.v2.example.com)
    • +
  • api-*.example.com - matches api-v1.example.com, api-prod.example.com, etc.
    • +
  • *.*.org - matches mail.example.org, but not example.org
    • +
+
diff --git a/apps/dashboard/src/lib/utils.tsx b/apps/dashboard/src/lib/utils.tsx index a6c50b572a..943d02e6b2 100644 --- a/apps/dashboard/src/lib/utils.tsx +++ b/apps/dashboard/src/lib/utils.tsx @@ -1,5 +1,4 @@ import { getPublicEnvVar } from "@/lib/env"; -import { parseJson } from "@stackframe/stack-shared/dist/utils/json"; import { clsx, type ClassValue } from "clsx"; import { redirect } from "next/navigation"; import { twMerge } from "tailwind-merge"; @@ -21,7 +20,7 @@ export function devFeaturesEnabledForProject(projectId: string) { if (projectId === "internal") { return true; } - const allowedProjectIds = parseJson(getPublicEnvVar("NEXT_PUBLIC_STACK_ENABLE_DEVELOPMENT_FEATURES_PROJECT_IDS") || "[]"); + const allowedProjectIds = JSON.parse(getPublicEnvVar("NEXT_PUBLIC_STACK_ENABLE_DEVELOPMENT_FEATURES_PROJECT_IDS") || "[]"); if (allowedProjectIds.status !== "ok" || !Array.isArray(allowedProjectIds.data)) { return false; } diff --git a/apps/e2e/tests/backend/endpoints/api/v1/auth/oauth/exact-domain-matching.test.ts b/apps/e2e/tests/backend/endpoints/api/v1/auth/oauth/exact-domain-matching.test.ts new file mode 100644 index 0000000000..91e5125d95 --- /dev/null +++ b/apps/e2e/tests/backend/endpoints/api/v1/auth/oauth/exact-domain-matching.test.ts @@ -0,0 +1,343 @@ +import { describe } from "vitest"; +import { it } from "../../../../../../helpers"; +import { Auth, InternalApiKey, Project, niceBackendFetch } from "../../../../../backend-helpers"; + +describe("OAuth with exact domain matching", () => { + it("should allow OAuth with exact matching domain", async ({ expect }) => { + const { adminAccessToken } = await Project.createAndSwitch({ + config: { + oauth_providers: [{ id: "spotify", type: "shared" }], + } + }); + await InternalApiKey.createAndSetProjectKeys(); + + // Add exact domain that matches our redirect URL + const configResponse = await niceBackendFetch("/api/v1/internal/config/override", { + method: "PATCH", + accessType: "admin", + headers: { + 'x-stack-admin-access-token': adminAccessToken, + }, + body: { + config_override_string: JSON.stringify({ + 'domains.trustedDomains.exact': { + baseUrl: 'http://stack-test.localhost', + handlerPath: '/some-callback-url', + }, + 'domains.allowLocalhost': false, + }), + }, + }); + expect(configResponse.status).toBe(200); + + // OAuth flow should succeed + const response = await Auth.OAuth.signIn(); + expect(response.tokenResponse.status).toBe(200); + await Auth.expectToBeSignedIn(); + }); + + it("should reject OAuth with non-matching exact domain", async ({ expect }) => { + const { adminAccessToken } = await Project.createAndSwitch({ + config: { + oauth_providers: [{ id: "spotify", type: "shared" }], + } + }); + await InternalApiKey.createAndSetProjectKeys(); + + // Add exact domain that does NOT match + const configResponse = await niceBackendFetch("/api/v1/internal/config/override", { + method: "PATCH", + accessType: "admin", + headers: { + 'x-stack-admin-access-token': adminAccessToken, + }, + body: { + config_override_string: JSON.stringify({ + 'domains.trustedDomains.production': { + baseUrl: 'https://app.production.com', + handlerPath: '/auth/handler', + }, + 'domains.allowLocalhost': false, // Ensure we only check exact domains + }), + }, + }); + expect(configResponse.status).toBe(200); + + // Try to complete the OAuth flow - it should fail at the callback stage + const { response } = await Auth.OAuth.getMaybeFailingAuthorizationCode(); + expect(response).toMatchInlineSnapshot(` + NiceResponse { + "status": 400, + "body": { + "code": "REDIRECT_URL_NOT_WHITELISTED", + "error": "Redirect URL not whitelisted. Did you forget to add this domain to the trusted domains list on the Stack Auth dashboard?", + }, + "headers": Headers { + "set-cookie": ' at path '/'>, + "x-stack-known-error": "REDIRECT_URL_NOT_WHITELISTED", +