diff --git a/.github/workflows/test-build.yml b/.github/workflows/test-build.yml index 84a18b3fb2d..99e64b97f2b 100644 --- a/.github/workflows/test-build.yml +++ b/.github/workflows/test-build.yml @@ -34,7 +34,6 @@ jobs: DATABASE_URL: postgresql://postgres:postgres@127.0.0.1:5432/sim_auth_scim OAUTH_TOKEN_FAMILY_TEST_DATABASE_URL: postgresql://postgres:postgres@127.0.0.1:5432/sim_auth_scim BETTER_AUTH_SECRET: oauth-postgres-ci-secret-at-least-32-characters - OAUTH_PROVIDER_ENABLED: 'true' NEXT_PUBLIC_APP_URL: https://test.sim.ai ENCRYPTION_KEY: '0000000000000000000000000000000000000000000000000000000000000000' diff --git a/apps/docs/content/docs/cli/authentication.mdx b/apps/docs/content/docs/cli/authentication.mdx index bba047f69b5..c56eccfa94a 100644 --- a/apps/docs/content/docs/cli/authentication.mdx +++ b/apps/docs/content/docs/cli/authentication.mdx @@ -23,9 +23,9 @@ sim login --method oauth sim login --method api-key ``` -`--method oauth` requires OAuth support and never falls back to an API key. The -server must enable `OAUTH_PROVIDER_ENABLED=true`. Explicit OAuth selection also -overrides SSH/headless detection; your browser still needs to reach the CLI's +`--method oauth` requires a server with OAuth support and authentication enabled; +it never falls back to an API key. Explicit OAuth selection also overrides +SSH/headless detection; your browser still needs to reach the CLI's local callback. `--method api-key` uses pairing-code approval to create a new permanent API key. To supply an existing key, set `SIM_API_KEY` instead. @@ -257,13 +257,11 @@ Save it to avoid repeating the flag: sim configure --set-endpoint http://localhost:3000 --profile local ``` -A deployment offers OAuth sign-in when its global `oauth-provider` feature flag -is enabled. With AppConfig, enable it in the existing `feature-flags` document -using `"oauth-provider": { "enabled": true }`. When AppConfig is disabled or no -AppConfig document has been loaded, `OAUTH_PROVIDER_ENABLED=true` supplies the fallback. -With the provider off, the CLI uses the pairing-code handoff; `DISABLE_AUTH=true` -always forces OAuth off. Operators must apply the database migration and drain -older app instances before enabling it. See [Sign in with Sim](/platform/self-hosting/authentication#sign-in-with-sim). +OAuth sign-in is available by default when server authentication is enabled. +`DISABLE_AUTH=true` disables OAuth. Older servers without OAuth support use the +pairing-code handoff. Keep the database schema current and drain app instances +that predate the OAuth token-family lifecycle before accepting OAuth traffic. +See [Sign in with Sim](/platform/self-hosting/authentication#sign-in-with-sim). ## Where the login is stored diff --git a/apps/docs/content/docs/platform/enterprise/access-control.mdx b/apps/docs/content/docs/platform/enterprise/access-control.mdx index 8914a18519f..bfe5964cde6 100644 --- a/apps/docs/content/docs/platform/enterprise/access-control.mdx +++ b/apps/docs/content/docs/platform/enterprise/access-control.mdx @@ -173,13 +173,14 @@ The **Chat Deployment** row also carries an **auth-mode allowlist** — *Auth mo | API Keys | Revokes workspace API keys. Members cannot list, create, or revoke one. | | Personal API Keys | Prevents members from using a personal API key against this workspace. | | Personal Credentials | Prevents connecting personal credentials, leaving only workspace-shared ones. | -| CLI Access | Prevents approving a CLI login, which mints a key for the public API. | +| CLI Access | Prevents approving a CLI login or using Sim CLI OAuth tokens for the public API. Existing API keys retain their own restrictions. | +| OAuth App Access | Prevents OAuth apps from accessing the group's workspaces. The organization default group also governs authorization, token issuance and refresh, and account-level billing and audit reads. | ##### Rows read from the organization default group Two rows — **Workspace Creation** and **Member Directory** — are read only from the organization's **default group**, because the act they govern names no workspace. On any other group the editor renders them inert, tags them **Organization**, and skips them in **Select All**. Set them on the default group. -Five more rows — **Integrations**, **API Keys**, **Invitations**, **Personal API Keys**, and **CLI Access** — apply on the group in front of you for anything scoped to one of its workspaces. The account-level path of the same action falls back to the default group: minting a personal key, an organization-wide invitation, an account-level CLI login. To close one of these completely, set it on the default group as well. +Six more rows — **Integrations**, **API Keys**, **Invitations**, **Personal API Keys**, **CLI Access**, and **OAuth App Access** — apply on the group in front of you for anything scoped to one of its workspaces. Account-level actions use the organization's default group, including minting a personal key, sending an organization-wide invitation, approving a CLI login, and authorizing or refreshing an OAuth app. ### 4. Choose who it applies to diff --git a/apps/docs/content/docs/platform/enterprise/self-hosted.mdx b/apps/docs/content/docs/platform/enterprise/self-hosted.mdx index 1233f0cea70..f256a162b28 100644 --- a/apps/docs/content/docs/platform/enterprise/self-hosted.mdx +++ b/apps/docs/content/docs/platform/enterprise/self-hosted.mdx @@ -91,7 +91,7 @@ Persist that value as `CRON_SECRET` on the app **and** on whatever calls these e Both shipped deployments schedule the data-drain dispatcher and OAuth token cleanup, but **not** the three configurable data-retention endpoints. Setting `DATA_RETENTION_ENABLED=true` alone deletes no retained product data — those windows are evaluated only when one of the three endpoints is called. Add them to `cronjobs.jobs` yourself, or drive them from an external scheduler. - OAuth token cleanup continues when the global `oauth-provider` feature flag is off, so rows created while the provider was enabled do not become permanent. See [Sign in with Sim](/platform/self-hosting/authentication#sign-in-with-sim) for AppConfig and fallback configuration. + OAuth token cleanup runs independently of sign-in activity, removing expired and revoked credentials. See [Sign in with Sim](/platform/self-hosting/authentication#sign-in-with-sim) for provider configuration. ```bash diff --git a/apps/docs/content/docs/platform/self-hosting/authentication.mdx b/apps/docs/content/docs/platform/self-hosting/authentication.mdx index 4ed18f53fa7..c47590f6750 100644 --- a/apps/docs/content/docs/platform/self-hosting/authentication.mdx +++ b/apps/docs/content/docs/platform/self-hosting/authentication.mdx @@ -81,50 +81,32 @@ See the [SSO guide](/platform/enterprise/sso) for identity-provider setup and th ## Sign in with Sim -Your deployment can act as an OAuth 2.0 authorization server using authorization -code with PKCE and current OAuth security guidance. The Sim CLI uses it when -enabled; see [CLI authentication](/cli/authentication). +Your deployment acts as an OAuth 2.0 authorization server using authorization +code with PKCE. OAuth sign-in is available whenever authentication is enabled; +see [CLI authentication](/cli/authentication). -The global `oauth-provider` feature flag controls availability. Keep it off while -applying the database migration, then deploy and drain every older app instance -before enabling it. +Apply database migrations before deploying a new app version. When upgrading +from a version without the OAuth token-family lifecycle, drain older app +instances before accepting OAuth traffic so every instance enforces the same +refresh and revocation rules. -If your deployment uses AWS AppConfig, add this entry to the existing -`feature-flags` document and deploy that configuration: - -```json -{ - "oauth-provider": { "enabled": true } -} -``` - -Preserve the document's other entries. This flag is global: use `enabled`, not -workspace, organization, user, or admin targeting. Set `enabled` to `false` to -turn it off; changes take effect as instances refresh their AppConfig cache. - -When AppConfig is disabled or no AppConfig document has been loaded, the -fallback is: - -```bash -OAUTH_PROVIDER_ENABLED=true -``` - -In that fallback mode, unset or false keeps the provider off. An available -AppConfig document takes precedence over this variable, including when the -`oauth-provider` entry is missing or disabled. AppConfig fetch failures retain -the last successfully loaded document. - -When the provider is off, discovery at `/.well-known/oauth-authorization-server` -returns 404 and the CLI falls back to the pairing-code handoff. -`DISABLE_AUTH=true` also forces the provider off because the authorization flow -requires a real Better Auth user session. +`DISABLE_AUTH=true` disables OAuth sign-in and discovery because authorization +requires a real Better Auth user session. Older servers without OAuth support +return 404 from `/.well-known/oauth-authorization-server`, and the CLI falls +back to the pairing-code handoff. Access tokens are opaque and last an hour; refresh tokens rotate on every use. Each login has a fixed thirty-day lifetime that refreshing does not extend. Token validation checks current grants, so revoking a grant under -**Settings → General → Authorized apps** stops the app on its very next request. These -settings remain available for reviewing and revoking existing grants while the -provider is off, and scheduled OAuth token cleanup continues. +**Settings → General → Authorized apps** stops the app on its very next request. +Scheduled OAuth token cleanup runs independently of sign-in activity. + +Organization admins can restrict **OAuth App Access** under **Credentials & Access** +in [permission groups](/platform/enterprise/access-control). Workspace requests use +the group governing that workspace. Authorizing apps, issuing and refreshing tokens, +and account-level billing and audit reads use the organization's default group. +**CLI Access** also applies to the Sim CLI. Members can still review and revoke +existing grants when OAuth app access is restricted. ### Registering an app diff --git a/apps/docs/content/docs/platform/self-hosting/environment-variables.mdx b/apps/docs/content/docs/platform/self-hosting/environment-variables.mdx index 9fa71d93911..d424021946b 100644 --- a/apps/docs/content/docs/platform/self-hosting/environment-variables.mdx +++ b/apps/docs/content/docs/platform/self-hosting/environment-variables.mdx @@ -123,12 +123,6 @@ import { Callout } from 'fumadocs-ui/components/callout' Google, GitHub, and Microsoft sign-in, their callback URLs, and the `DISABLE_*_AUTH` switches are documented in [Authentication](/platform/self-hosting/authentication#social-login). -## Sign in with Sim - -| Variable | Description | -| --- | --- | -| `OAUTH_PROVIDER_ENABLED` | Fallback for the global `oauth-provider` feature flag when AppConfig is disabled or no AppConfig document has been loaded. Set to `true` only after the migration is applied and every older app instance is drained. With AppConfig, use `"oauth-provider": { "enabled": true }` in the existing `feature-flags` document instead. `DISABLE_AUTH=true` always forces it off. See [Authentication](/platform/self-hosting/authentication#sign-in-with-sim) | - ## Integration Credentials diff --git a/apps/sim/.env.example b/apps/sim/.env.example index cff17292594..8d46564462b 100644 --- a/apps/sim/.env.example +++ b/apps/sim/.env.example @@ -11,7 +11,6 @@ BETTER_AUTH_URL=http://localhost:3000 # Authentication Bypass (Optional - for self-hosted deployments behind private networks) # DISABLE_AUTH=true # Uncomment to bypass authentication entirely. Creates an anonymous session for all requests. -# OAUTH_PROVIDER_ENABLED=true # Enable Sim's OAuth authorization server after every app instance runs the matching migration/code. DISABLE_AUTH=true forces it off. # Private-network egress allowlist (Optional - self-hosted only; ignored on Sim Cloud) # EGRESS_ALLOWED_HOSTS=host.docker.internal,*.svc.cluster.local # Uncomment to let outbound requests reach these hosts on a private network. Widens the SSRF boundary; only use on a trusted private network. diff --git a/apps/sim/app/(auth)/oauth/consent/page.tsx b/apps/sim/app/(auth)/oauth/consent/page.tsx index 9c9bf878261..50956b2fc3d 100644 --- a/apps/sim/app/(auth)/oauth/consent/page.tsx +++ b/apps/sim/app/(auth)/oauth/consent/page.tsx @@ -2,7 +2,7 @@ import type { Metadata } from 'next' import { redirect } from 'next/navigation' import type { SearchParams } from 'nuqs/server' import { getSession } from '@/lib/auth' -import { isOAuthProviderEnabled } from '@/lib/auth/oauth-provider-feature' +import { isAuthDisabled } from '@/lib/core/config/env-flags' import { OAuthConsentView } from '@/app/(auth)/oauth/consent/consent-view' import { oauthConsentSearchParamsCache } from '@/app/(auth)/oauth/consent/search-params' @@ -22,7 +22,7 @@ export default async function OAuthConsentPage({ }: { searchParams: Promise }) { - if (!(await isOAuthProviderEnabled())) redirect('/') + if (isAuthDisabled) redirect('/') const [session, raw] = await Promise.all([getSession(), searchParams]) diff --git a/apps/sim/app/(auth)/oauth/sign-in/route.test.ts b/apps/sim/app/(auth)/oauth/sign-in/route.test.ts index ae2b7dea886..1ecbb6e398f 100644 --- a/apps/sim/app/(auth)/oauth/sign-in/route.test.ts +++ b/apps/sim/app/(auth)/oauth/sign-in/route.test.ts @@ -6,22 +6,21 @@ import { NextRequest } from 'next/server' import { beforeEach, describe, expect, it, vi } from 'vitest' const flags = vi.hoisted(() => ({ - enabled: true, + authDisabled: false, registrationDisabled: false, appUrl: 'https://sim.test', })) vi.mock('@/lib/core/config/env-flags', () => ({ ...envFlagsMock, + get isAuthDisabled() { + return flags.authDisabled + }, get isRegistrationDisabled() { return flags.registrationDisabled }, })) -vi.mock('@/lib/auth/oauth-provider-feature', () => ({ - isOAuthProviderEnabled: vi.fn(async () => flags.enabled), -})) - vi.mock('@/lib/core/config/env', () => { const mock = createEnvMock({ NEXT_PUBLIC_APP_URL: 'https://sim.test' }) return { @@ -48,15 +47,15 @@ function redirectParts(response: Response): { destination: URL; callback: URL } describe('OAuth login bridge', () => { beforeEach(() => { - flags.enabled = true + flags.authDisabled = false flags.registrationDisabled = false flags.appUrl = 'https://sim.test' }) it.each([true, false])( - 'keeps the configured auth origin when Next normalizes loopback hosts (enabled=%s)', - async (enabled) => { - flags.enabled = enabled + 'keeps the configured auth origin when Next normalizes loopback hosts (authDisabled=%s)', + async (authDisabled) => { + flags.authDisabled = authDisabled flags.appUrl = 'http://127.0.0.1:37488' const incoming = new NextRequest(`${flags.appUrl}/oauth/sign-in?client_id=sim-cli`) expect(incoming.nextUrl.origin).toBe('http://localhost:37488') @@ -64,8 +63,8 @@ describe('OAuth login bridge', () => { const response = await GET(incoming) const destination = new URL(response.headers.get('location')!) expect(destination.origin).toBe(flags.appUrl) - expect(destination.pathname).toBe(enabled ? '/signup' : '/') - if (enabled) expect(redirectParts(response).callback.origin).toBe(flags.appUrl) + expect(destination.pathname).toBe(authDisabled ? '/' : '/signup') + if (!authDisabled) expect(redirectParts(response).callback.origin).toBe(flags.appUrl) } ) @@ -94,12 +93,12 @@ describe('OAuth login bridge', () => { expect(callback.searchParams.has('prompt')).toBe(false) }) - it('uses login when registration is disabled and hides a disabled provider', async () => { + it('uses login when registration is disabled and hides OAuth when authentication is disabled', async () => { flags.registrationDisabled = true const enabled = await GET(request('client_id=sim-cli')) expect(redirectParts(enabled).destination.pathname).toBe('/login') - flags.enabled = false + flags.authDisabled = true const disabled = await GET(request('client_id=sim-cli')) expect(disabled.status).toBe(302) expect(new URL(disabled.headers.get('location') as string).pathname).toBe('/') @@ -155,7 +154,7 @@ describe('OAuth login bridge', () => { } }) - it('uses the same redirect precedence as the form and ignores a disabled OAuth provider', async () => { + it('uses the same redirect precedence as the form and requires authentication for OAuth', async () => { const destination = new URL('/login', 'https://sim.test') destination.searchParams.set('callbackUrl', '/api/auth/oauth2/authorize?client_id=sim-cli') destination.searchParams.set('redirect', '/workspace') @@ -163,7 +162,7 @@ describe('OAuth login bridge', () => { expect((await proxy(new NextRequest(destination, { headers }))).status).toBe(307) destination.searchParams.delete('redirect') - flags.enabled = false + flags.authDisabled = true expect((await proxy(new NextRequest(destination, { headers }))).status).toBe(307) }) }) diff --git a/apps/sim/app/(auth)/oauth/sign-in/route.ts b/apps/sim/app/(auth)/oauth/sign-in/route.ts index 2d5c3faa619..ff4b8b582ef 100644 --- a/apps/sim/app/(auth)/oauth/sign-in/route.ts +++ b/apps/sim/app/(auth)/oauth/sign-in/route.ts @@ -1,6 +1,5 @@ import { type NextRequest, NextResponse } from 'next/server' -import { isOAuthProviderEnabled } from '@/lib/auth/oauth-provider-feature' -import { isRegistrationDisabled } from '@/lib/core/config/env-flags' +import { isAuthDisabled, isRegistrationDisabled } from '@/lib/core/config/env-flags' import { getBaseUrl } from '@/lib/core/utils/urls' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' import { buildAuthCrossLink } from '@/app/(auth)/auth-redirect' @@ -40,7 +39,7 @@ function consumeInteractivePrompt(params: URLSearchParams): boolean { */ export const GET = withRouteHandler(async (request: NextRequest) => { /** Avoid sending a newly signed-in user to a disabled provider's JSON 404. */ - if (!(await isOAuthProviderEnabled())) { + if (isAuthDisabled) { return NextResponse.redirect(new URL('/', getBaseUrl()), 302) } diff --git a/apps/sim/app/api/auth/[...all]/route.test.ts b/apps/sim/app/api/auth/[...all]/route.test.ts index 532bf52bc4a..6257b7f4519 100644 --- a/apps/sim/app/api/auth/[...all]/route.test.ts +++ b/apps/sim/app/api/auth/[...all]/route.test.ts @@ -14,7 +14,6 @@ const handlerMocks = vi.hoisted(() => ({ user: { id: 'anon' }, session: { id: 'anon-session' }, })), - oauthEnabled: vi.fn(), })) vi.mock('better-auth/next-js', () => ({ @@ -27,9 +26,6 @@ vi.mock('better-auth/next-js', () => ({ vi.mock('@/lib/auth', () => ({ auth: { handler: {} }, })) -vi.mock('@/lib/auth/oauth-provider-feature', () => ({ - isOAuthProviderEnabled: handlerMocks.oauthEnabled, -})) vi.mock('@/lib/auth/anonymous', () => ({ ensureAnonymousUserExists: handlerMocks.ensureAnonymousUserExists, @@ -67,7 +63,7 @@ vi.mock('@/app/api/credential-groups/oauth-callback', () => ({ import { GET, POST } from '@/app/api/auth/[...all]/route' afterAll(resetEnvFlagsMock) -beforeEach(() => handlerMocks.oauthEnabled.mockResolvedValue(true)) +beforeEach(() => setEnvFlags({ isAuthDisabled: false })) describe('auth catch-all route managed OAuth callbacks', () => { beforeEach(() => { @@ -103,9 +99,9 @@ describe('auth catch-all route managed OAuth callbacks', () => { }) it.each([true, false])( - 'preserves connector callbacks when the provider is enabled=%s', - async (enabled) => { - handlerMocks.oauthEnabled.mockResolvedValue(enabled) + 'preserves connector callbacks with authentication disabled=%s', + async (authDisabled) => { + setEnvFlags({ isAuthDisabled: authDisabled }) handlerMocks.betterAuthGET.mockResolvedValueOnce(new Response(null, { status: 204 })) const request = createMockRequest( 'GET', @@ -119,7 +115,6 @@ describe('auth catch-all route managed OAuth callbacks', () => { expect(response.status).toBe(204) expect(handlerMocks.betterAuthGET).toHaveBeenCalledWith(request) expect(handlerMocks.credentialGroupCallback).not.toHaveBeenCalled() - expect(handlerMocks.oauthEnabled).not.toHaveBeenCalled() } ) @@ -372,29 +367,21 @@ describe('OAuth provider client endpoints', () => { }) it.each(['oauth2/consent', 'oauth2/continue', 'oauth2/public-client-prelogin'])( - 'stops serving %s after the runtime flag changes', + 'requires authentication for %s', async (path) => { - const request = () => - createMockRequest('POST', {}, {}, `http://localhost:3000/api/auth/${path}`) - expect((await POST(request())).status).toBe(200) - handlerMocks.betterAuthPOST.mockClear() - - handlerMocks.oauthEnabled.mockResolvedValue(false) - const disabled = await POST(request()) - expect(disabled.status).toBe(404) - expect(disabled.headers.get('cache-control')).toBe('no-store') + setEnvFlags({ isAuthDisabled: true }) + const request = createMockRequest('POST', {}, {}, `http://localhost:3000/api/auth/${path}`) + const response = await POST(request) + expect(response.status).toBe(404) + expect(response.headers.get('cache-control')).toBe('no-store') expect(handlerMocks.betterAuthPOST).not.toHaveBeenCalled() - - handlerMocks.oauthEnabled.mockResolvedValue(true) - expect((await POST(request())).status).toBe(200) - expect(handlerMocks.betterAuthPOST).toHaveBeenCalledOnce() } ) it.each([true, false])( - 'preserves connector POST callbacks when the provider is enabled=%s', - async (enabled) => { - handlerMocks.oauthEnabled.mockResolvedValue(enabled) + 'preserves connector POST callbacks with authentication disabled=%s', + async (authDisabled) => { + setEnvFlags({ isAuthDisabled: authDisabled }) const request = createMockRequest( 'POST', {}, @@ -403,14 +390,13 @@ describe('OAuth provider client endpoints', () => { ) expect((await POST(request)).status).toBe(200) expect(handlerMocks.betterAuthPOST).toHaveBeenCalledExactlyOnceWith(request) - expect(handlerMocks.oauthEnabled).not.toHaveBeenCalled() } ) it.each([true, false])( - 'preserves authenticated connector linking when the OAuth provider is enabled=%s', - async (enabled) => { - handlerMocks.oauthEnabled.mockResolvedValue(enabled) + 'preserves authenticated connector linking with authentication disabled=%s', + async (authDisabled) => { + setEnvFlags({ isAuthDisabled: authDisabled }) const request = createMockRequest( 'POST', { providerId: 'google-email', callbackURL: 'http://localhost:3000/workspace' }, @@ -422,7 +408,6 @@ describe('OAuth provider client endpoints', () => { expect(response.status).toBe(200) expect(handlerMocks.betterAuthPOST).toHaveBeenCalledExactlyOnceWith(request) - expect(handlerMocks.oauthEnabled).not.toHaveBeenCalled() } ) }) diff --git a/apps/sim/app/api/auth/[...all]/route.ts b/apps/sim/app/api/auth/[...all]/route.ts index f4f8b461fa3..32227242238 100644 --- a/apps/sim/app/api/auth/[...all]/route.ts +++ b/apps/sim/app/api/auth/[...all]/route.ts @@ -4,7 +4,6 @@ import { sharedCredentialGroupOAuthCallbackContract } from '@/lib/api/contracts/ import { parseRequest } from '@/lib/api/server' import { auth } from '@/lib/auth' import { createAnonymousSession, ensureAnonymousUserExists } from '@/lib/auth/anonymous' -import { isOAuthProviderEnabled } from '@/lib/auth/oauth-provider-feature' import { isAuthDisabled } from '@/lib/core/config/env-flags' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' import { isCredentialGroupOAuthState } from '@/lib/credential-groups/oauth-state' @@ -187,7 +186,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => { ) } - if (OAUTH_PROVIDER_PROTOCOL_POST_PATHS.has(path) && !(await isOAuthProviderEnabled())) { + if (OAUTH_PROVIDER_PROTOCOL_POST_PATHS.has(path) && isAuthDisabled) { return NextResponse.json( { error: 'OAuth provider is not enabled' }, { status: 404, headers: { 'Cache-Control': 'no-store', Pragma: 'no-cache' } } diff --git a/apps/sim/app/api/auth/oauth2/authorize/route.test.ts b/apps/sim/app/api/auth/oauth2/authorize/route.test.ts index 659ba385fd2..9c7c35370d7 100644 --- a/apps/sim/app/api/auth/oauth2/authorize/route.test.ts +++ b/apps/sim/app/api/auth/oauth2/authorize/route.test.ts @@ -1,8 +1,15 @@ /** * @vitest-environment node */ -import { createMockRequest, queueTableRows, resetDbChainMock, schemaMock } from '@sim/testing' -import { beforeEach, describe, expect, it, vi } from 'vitest' +import { + createMockRequest, + queueTableRows, + resetDbChainMock, + resetEnvFlagsMock, + schemaMock, + setEnvFlags, +} from '@sim/testing' +import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest' import { ForbiddenOperationError } from '@/lib/core/application/forbidden' import { InsufficientWorkspacePermissionsError } from '@/lib/core/application/workspace-authorization' import { OrchestrationError } from '@/lib/core/orchestration/types' @@ -20,7 +27,6 @@ const mocks = vi.hoisted(() => ({ decryptQuickBooksClientConfig: vi.fn(), createQuickBooksState: vi.fn(), getCanonicalScopes: vi.fn(), - oauthEnabled: vi.fn(), })) vi.mock('better-auth/next-js', () => ({ @@ -31,9 +37,6 @@ vi.mock('@/lib/auth/auth', () => ({ getSession: mocks.getSession, auth: { handler: {}, api: { oAuth2LinkAccount: mocks.linkAccount } }, })) -vi.mock('@/lib/auth/oauth-provider-feature', () => ({ - isOAuthProviderEnabled: mocks.oauthEnabled, -})) vi.mock('@/lib/core/utils/urls', () => ({ SITE_URL: 'https://www.sim.ai', getBaseUrl: mocks.getBaseUrl, @@ -88,11 +91,13 @@ function linkResponse(url = 'https://provider.example/authorize') { }) } +afterAll(resetEnvFlagsMock) + describe('OAuth2 authorize route', () => { beforeEach(() => { vi.clearAllMocks() resetDbChainMock() - mocks.oauthEnabled.mockResolvedValue(true) + setEnvFlags({ isAuthDisabled: false }) mocks.getBaseUrl.mockReturnValue(BASE_URL) mocks.getSession.mockResolvedValue({ user: { id: 'user-1' }, @@ -149,26 +154,113 @@ describe('OAuth2 authorize route', () => { expect(mocks.createConnection).not.toHaveBeenCalled() }) - it('applies a runtime flag change to the next authorization request', async () => { - const providerRequest = () => + it.each([ + ['https://client.example/callback', 'https://client.example/callback'], + ['http://127.0.0.1/callback', 'http://127.0.0.1:43123/callback'], + ])('returns permission denials to registered callback %s', async (registered, redirectUri) => { + queueTableRows(schemaMock.oauthClient, [{ disabled: false, redirectUris: [registered] }]) + mocks.betterAuthGET.mockResolvedValue( + Response.json( + { + error: 'access_denied', + error_description: 'OAuth apps are restricted for your account.', + }, + { status: 403 } + ) + ) + + const response = await GET( + request({ + client_id: 'client-1', + response_type: 'code', + redirect_uri: redirectUri, + state: 'state-1', + }) + ) + const location = new URL(response.headers.get('location') ?? '') + + expect(response.status).toBe(302) + expect(response.headers.get('cache-control')).toBe('no-store') + expect(`${location.origin}${location.pathname}`).toBe(redirectUri) + expect(location.searchParams.get('error')).toBe('access_denied') + expect(location.searchParams.get('error_description')).toBe( + 'OAuth apps are restricted for your account.' + ) + expect(location.searchParams.get('state')).toBe('state-1') + expect(location.searchParams.get('iss')).toBe(`${BASE_URL}/api/auth`) + expect(location.searchParams.has('code')).toBe(false) + expect(mocks.getSession).not.toHaveBeenCalled() + }) + + it.each([ + ['missing client', undefined], + ['disabled client', { disabled: true, redirectUris: ['https://client.example/callback'] }], + ['unregistered callback', { disabled: false, redirectUris: ['https://client.example/other'] }], + ])('does not redirect a permission denial for a %s', async (_case, client) => { + if (client) queueTableRows(schemaMock.oauthClient, [client]) + mocks.betterAuthGET.mockResolvedValue( + Response.json({ error: 'access_denied' }, { status: 403 }) + ) + + const response = await GET( request({ client_id: 'client-1', response_type: 'code', redirect_uri: 'https://client.example/callback', + state: 'state-1', }) - expect((await GET(providerRequest())).status).toBe(302) - mocks.betterAuthGET.mockClear() + ) + + expect(response.status).toBe(400) + expect(response.headers.has('location')).toBe(false) + await expect(response.json()).resolves.toEqual({ + error: 'access_denied', + error_description: 'Access denied.', + }) + }) + + it.each([ + [403, '{"error":"invalid_request"}'], + [403, 'Forbidden'], + [400, '{"error":"access_denied"}'], + ])('preserves delegated status %s and body %s', async (status, body) => { + mocks.betterAuthGET.mockResolvedValue(new Response(body, { status })) + + const response = await GET( + request({ + client_id: 'client-1', + response_type: 'code', + redirect_uri: 'https://client.example/callback', + }) + ) + + expect(response.status).toBe(status) + expect(response.headers.has('location')).toBe(false) + await expect(response.text()).resolves.toBe(body) + }) - mocks.oauthEnabled.mockResolvedValue(false) - const disabled = await GET(providerRequest()) - expect(disabled.status).toBe(404) - expect(disabled.headers.get('cache-control')).toBe('no-store') + it('requires authentication for provider authorization', async () => { + setEnvFlags({ isAuthDisabled: true }) + const response = await GET( + request({ + client_id: 'client-1', + response_type: 'code', + redirect_uri: 'https://client.example/callback', + }) + ) + expect(response.status).toBe(404) + expect(response.headers.get('cache-control')).toBe('no-store') expect(mocks.betterAuthGET).not.toHaveBeenCalled() expect(mocks.getSession).not.toHaveBeenCalled() + }) - mocks.oauthEnabled.mockResolvedValue(true) - expect((await GET(providerRequest())).status).toBe(302) - expect(mocks.betterAuthGET).toHaveBeenCalledOnce() + it('preserves connector authorization when user authentication is disabled', async () => { + setEnvFlags({ isAuthDisabled: true }) + const response = await GET(request({ draftId: 'draft-1' })) + expect(response.status).toBe(307) + expect(mocks.launchConnection).toHaveBeenCalled() + expect(mocks.linkAccount).toHaveBeenCalled() + expect(mocks.betterAuthGET).not.toHaveBeenCalled() }) it('keeps an OAuth request missing client_id out of the connector flow', async () => { @@ -318,9 +410,9 @@ describe('OAuth2 authorize route', () => { }) it.each([true, false])( - 'preserves legacy connector linking when the provider is enabled=%s', - async (enabled) => { - mocks.oauthEnabled.mockResolvedValue(enabled) + 'preserves legacy connector linking with authentication disabled=%s', + async (authDisabled) => { + setEnvFlags({ isAuthDisabled: authDisabled }) const response = await GET(request({ providerId: 'google-email', workspaceId: WORKSPACE_ID })) expect(response.headers.get('location')).toBe('https://provider.example/authorize') @@ -338,7 +430,6 @@ describe('OAuth2 authorize route', () => { }), }) ) - expect(mocks.oauthEnabled).not.toHaveBeenCalled() } ) diff --git a/apps/sim/app/api/auth/oauth2/authorize/route.ts b/apps/sim/app/api/auth/oauth2/authorize/route.ts index 8b9ab03f09c..546271edaba 100644 --- a/apps/sim/app/api/auth/oauth2/authorize/route.ts +++ b/apps/sim/app/api/auth/oauth2/authorize/route.ts @@ -6,9 +6,9 @@ import { parseRequest } from '@/lib/api/server' import { auth, getSession } from '@/lib/auth/auth' import { oauthAuthorizationErrorResponse } from '@/lib/auth/oauth-authorization-error' import { validateOAuthPkceAuthorizationRequest } from '@/lib/auth/oauth-protocol-request' -import { isOAuthProviderEnabled } from '@/lib/auth/oauth-provider-feature' import { ForbiddenOperationError } from '@/lib/core/application/forbidden' import { requireConfiguredOAuthClient } from '@/lib/core/config/env-capabilities.server' +import { isAuthDisabled } from '@/lib/core/config/env-flags' import { OrchestrationError } from '@/lib/core/orchestration/types' import { getBaseUrl } from '@/lib/core/utils/urls' import { isSameOrigin } from '@/lib/core/utils/validation' @@ -73,7 +73,7 @@ function isOAuthProviderAuthorize(request: NextRequest): boolean { */ export const GET = withRouteHandler(async (request: NextRequest) => { if (isOAuthProviderAuthorize(request)) { - if (!(await isOAuthProviderEnabled())) { + if (isAuthDisabled) { return NextResponse.json( { error: 'OAuth provider is not enabled' }, { status: 404, headers: { 'Cache-Control': 'no-store', Pragma: 'no-cache' } } @@ -135,7 +135,21 @@ export const GET = withRouteHandler(async (request: NextRequest) => { if (pkceError) { return oauthAuthorizationErrorResponse(request, 'invalid_request', pkceError) } - return betterAuthGET(request) + const response = await betterAuthGET(request) + if (response.status === 403) { + const body: unknown = await response + .clone() + .json() + .catch(() => null) + if (body && typeof body === 'object' && 'error' in body && body.error === 'access_denied') { + const description = + 'error_description' in body && typeof body.error_description === 'string' + ? body.error_description + : 'Access denied.' + return oauthAuthorizationErrorResponse(request, 'access_denied', description) + } + } + return response } const baseUrl = getBaseUrl() diff --git a/apps/sim/app/api/auth/oauth2/revoke/route.test.ts b/apps/sim/app/api/auth/oauth2/revoke/route.test.ts index c28088ca3df..32c5ff072ea 100644 --- a/apps/sim/app/api/auth/oauth2/revoke/route.test.ts +++ b/apps/sim/app/api/auth/oauth2/revoke/route.test.ts @@ -1,18 +1,15 @@ /** * @vitest-environment node */ +import { resetEnvFlagsMock, setEnvFlags } from '@sim/testing' import { NextRequest } from 'next/server' -import { beforeEach, describe, expect, it, vi } from 'vitest' +import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest' const mocks = vi.hoisted(() => ({ - oauthEnabled: vi.fn(), rateLimit: vi.fn(async () => null), revoke: vi.fn(), })) -vi.mock('@/lib/auth/oauth-provider-feature', () => ({ - isOAuthProviderEnabled: mocks.oauthEnabled, -})) vi.mock('@/lib/core/rate-limiter', () => ({ enforceIpRateLimit: mocks.rateLimit })) vi.mock('@/lib/auth/oauth-token-family', () => ({ revokeOAuthToken: mocks.revoke })) @@ -26,10 +23,12 @@ function revokeRequest(body: string) { }) } +afterAll(resetEnvFlagsMock) + describe('OAuth revocation route', () => { beforeEach(() => { vi.clearAllMocks() - mocks.oauthEnabled.mockResolvedValue(true) + setEnvFlags({ isAuthDisabled: false }) mocks.revoke.mockResolvedValue({ success: true, value: undefined }) }) @@ -45,22 +44,13 @@ describe('OAuth revocation route', () => { }) }) - it('applies the runtime flag before revocation admission or protected work', async () => { - const request = () => revokeRequest('client_id=sim-cli&token=sim_ort_current') - expect((await POST(request())).status).toBe(200) - mocks.revoke.mockClear() - mocks.rateLimit.mockClear() - - mocks.oauthEnabled.mockResolvedValue(false) - const disabled = await POST(request()) - expect(disabled.status).toBe(404) - expect(disabled.headers.get('cache-control')).toBe('no-store') + it('requires authentication before revocation admission or protected work', async () => { + setEnvFlags({ isAuthDisabled: true }) + const response = await POST(revokeRequest('client_id=sim-cli&token=sim_ort_current')) + expect(response.status).toBe(404) + expect(response.headers.get('cache-control')).toBe('no-store') expect(mocks.revoke).not.toHaveBeenCalled() expect(mocks.rateLimit).not.toHaveBeenCalled() - - mocks.oauthEnabled.mockResolvedValue(true) - expect((await POST(request())).status).toBe(200) - expect(mocks.revoke).toHaveBeenCalledOnce() }) it('returns a Basic challenge for Basic client-authentication failure', async () => { diff --git a/apps/sim/app/api/auth/oauth2/revoke/route.ts b/apps/sim/app/api/auth/oauth2/revoke/route.ts index 53d7f96c1df..dcb4653691a 100644 --- a/apps/sim/app/api/auth/oauth2/revoke/route.ts +++ b/apps/sim/app/api/auth/oauth2/revoke/route.ts @@ -7,8 +7,8 @@ import { oauthRevocationSuccessResponse, parseOAuthFormRequest, } from '@/lib/auth/oauth-protocol-request' -import { isOAuthProviderEnabled } from '@/lib/auth/oauth-provider-feature' import { revokeOAuthToken } from '@/lib/auth/oauth-token-family' +import { isAuthDisabled } from '@/lib/core/config/env-flags' import { enforceIpRateLimit, type TokenBucketConfig } from '@/lib/core/rate-limiter' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' @@ -24,7 +24,7 @@ const REVOKE_RATE_LIMIT: TokenBucketConfig = { /** Revokes one opaque access token or the complete family named by a refresh token. */ export const POST = withRouteHandler(async (request: NextRequest) => { - if (!(await isOAuthProviderEnabled())) { + if (isAuthDisabled) { return NextResponse.json( { error: 'OAuth provider is not enabled' }, { status: 404, headers: { 'Cache-Control': 'no-store', Pragma: 'no-cache' } } diff --git a/apps/sim/app/api/auth/oauth2/token/route.postgres.test.ts b/apps/sim/app/api/auth/oauth2/token/route.postgres.test.ts index 43ea9f679db..8a9ee8e0ea8 100644 --- a/apps/sim/app/api/auth/oauth2/token/route.postgres.test.ts +++ b/apps/sim/app/api/auth/oauth2/token/route.postgres.test.ts @@ -2,6 +2,7 @@ * @vitest-environment node */ import { randomBytes } from 'node:crypto' +import { envFlagsMock } from '@sim/testing/mocks/env-flags.mock' import { NextRequest } from 'next/server' import { describe, expect, it, vi } from 'vitest' @@ -9,6 +10,11 @@ vi.unmock('@sim/db') vi.unmock('@sim/db/schema') vi.unmock('drizzle-orm') vi.unmock('@/lib/auth') +vi.mock('@/lib/core/config/env-flags', () => ({ + ...envFlagsMock, + isHosted: true, + isBillingEnabled: true, +})) const databaseUrl = process.env.OAUTH_TOKEN_FAMILY_TEST_DATABASE_URL @@ -35,6 +41,7 @@ describe.skipIf(!databaseUrl)('OAuth token route in PostgreSQL', () => { tokenStore, provider, { requestUtilsMockFns }, + { isCapabilityWithheldForUser }, ] = await Promise.all([ import('@sim/db'), import('@sim/db/schema'), @@ -46,6 +53,7 @@ describe.skipIf(!databaseUrl)('OAuth token route in PostgreSQL', () => { import('@/lib/auth/oauth-access-token'), import('@/lib/auth/oauth-provider'), import('@sim/testing/mocks/request.mock'), + import('@/lib/permission-groups/user-scope.server'), ]) const testId = randomBytes(8).toString('hex') @@ -53,6 +61,8 @@ describe.skipIf(!databaseUrl)('OAuth token route in PostgreSQL', () => { const sessionId = `oauth-route-test-session-${testId}` const sessionToken = `oauth-route-test-session-token-${testId}` const consentId = `oauth-route-test-consent-${testId}` + const organizationId = `oauth-route-test-org-${testId}` + const groupId = `oauth-route-test-group-${testId}` const email = `oauth-route-${testId}@example.com` const clientIp = `192.0.2.${Number.parseInt(testId.slice(0, 2), 16) || 1}` const baseUrl = 'https://test.sim.ai' @@ -73,7 +83,7 @@ describe.skipIf(!databaseUrl)('OAuth token route in PostgreSQL', () => { }, }) - const issueAuthorizationCode = async (verifier: string): Promise => { + const createAuthorizeUrl = async (verifier: string) => { const challenge = await crypto.subtle.digest('SHA-256', new TextEncoder().encode(verifier)) const authorizeUrl = new URL('/api/auth/oauth2/authorize', baseUrl) authorizeUrl.searchParams.set('client_id', provider.SIM_CLI_CLIENT_ID) @@ -83,7 +93,11 @@ describe.skipIf(!databaseUrl)('OAuth token route in PostgreSQL', () => { authorizeUrl.searchParams.set('code_challenge', Buffer.from(challenge).toString('base64url')) authorizeUrl.searchParams.set('code_challenge_method', 'S256') authorizeUrl.searchParams.set('state', `state-${testId}`) + return authorizeUrl + } + const issueAuthorizationCode = async (verifier: string): Promise => { + const authorizeUrl = await createAuthorizeUrl(verifier) const response = await auth.handler( new Request(authorizeUrl, { headers: { cookie: sessionCookie } }) ) @@ -91,7 +105,7 @@ describe.skipIf(!databaseUrl)('OAuth token route in PostgreSQL', () => { const location = response.headers.get('location') expect(location).toBeTruthy() const code = new URL(location as string, baseUrl).searchParams.get('code') - expect(code, `Expected authorization code redirect, received ${location}`).toBeTruthy() + expect(code, 'Expected an authorization code redirect').toBeTruthy() issuedCodeHashes.push(tokenStore.hashOAuthToken(code as string)) const authorizationCodes = await db .select({ identifier: schema.verification.identifier }) @@ -148,6 +162,42 @@ describe.skipIf(!databaseUrl)('OAuth token route in PostgreSQL', () => { }) try { + await db.insert(schema.organization).values({ + id: organizationId, + name: 'OAuth route permission fixture', + slug: organizationId, + createdAt: now, + }) + await db + .insert(schema.member) + .values({ id: `oauth-route-member-${testId}`, userId, organizationId, role: 'owner' }) + await db + .insert(schema.userStats) + .values({ id: `oauth-route-stats-${testId}`, userId, billingBlocked: false }) + await db.insert(schema.subscription).values({ + id: `oauth-route-subscription-${testId}`, + plan: 'enterprise', + referenceId: organizationId, + status: 'active', + seats: 5, + periodStart: now, + periodEnd: sessionExpiresAt, + metadata: { + plan: 'enterprise', + referenceId: organizationId, + seats: 5, + monthlyPrice: 100, + }, + }) + await db.insert(schema.permissionGroup).values({ + id: groupId, + organizationId, + createdBy: userId, + name: 'Default', + isDefault: true, + config: {}, + }) + expect(await isCapabilityWithheldForUser(userId, 'oauth_apps.use')).toBe(false) const firstVerifier = `${testId}-first-verifier-with-more-than-forty-three-characters` const firstTokens = await exchangeAuthorizationCode( await issueAuthorizationCode(firstVerifier), @@ -349,6 +399,115 @@ describe.skipIf(!databaseUrl)('OAuth token route in PostgreSQL', () => { .from(schema.oauthTokenFamily) .where(eq(schema.oauthTokenFamily.userId, userId)) ).toHaveLength(1) + + const activeTokens = (await racingResponses + .find((response) => response.status === 200) + ?.json()) as TokenResponseBody + const withheldVerifier = `${testId}-withheld-verifier-with-more-than-forty-three-characters` + const withheldCode = await issueAuthorizationCode(withheldVerifier) + const consentUrl = await createAuthorizeUrl(withheldVerifier) + consentUrl.searchParams.set('prompt', 'consent') + const consentPage = await auth.handler( + new Request(consentUrl, { headers: { cookie: sessionCookie } }) + ) + expect(consentPage.status).toBe(302) + const signedQuery = new URL(consentPage.headers.get('location')!, baseUrl).search.slice(1) + expect(new URLSearchParams(signedQuery).has('sig')).toBe(true) + const submitConsent = (accept: boolean) => + auth.handler( + new Request(`${baseUrl}/api/auth/oauth2/consent`, { + method: 'POST', + headers: { cookie: sessionCookie, 'content-type': 'application/json', origin: baseUrl }, + body: JSON.stringify({ accept, oauth_query: signedQuery }), + }) + ) + + await db + .update(schema.permissionGroup) + .set({ config: { disableOAuthAppAccess: true } }) + .where(eq(schema.permissionGroup.id, groupId)) + expect(await isCapabilityWithheldForUser(userId, 'oauth_apps.use')).toBe(true) + const blockedCachedConsent = await auth.handler( + new Request(await createAuthorizeUrl(withheldVerifier), { + headers: { cookie: sessionCookie }, + }) + ) + expect(blockedCachedConsent.status).toBe(403) + await expect(blockedCachedConsent.json()).resolves.toMatchObject({ error: 'access_denied' }) + const blockedAccept = await submitConsent(true) + expect(blockedAccept.status).toBe(403) + await expect(blockedAccept.json()).resolves.toMatchObject({ error: 'access_denied' }) + + const denial = await submitConsent(false) + expect(denial.status).toBe(200) + const denialBody = (await denial.json()) as { redirect: boolean; url: string } + expect(denialBody.redirect).toBe(true) + const denialUrl = new URL(denialBody.url) + expect(`${denialUrl.origin}${denialUrl.pathname}`).toBe(redirectUri) + expect(denialUrl.searchParams.get('error')).toBe('access_denied') + expect(denialUrl.searchParams.get('state')).toBe(`state-${testId}`) + expect(denialUrl.searchParams.get('iss')).toBe(`${baseUrl}/api/auth`) + expect(denialUrl.searchParams.has('code')).toBe(false) + expect( + await db + .select({ scopes: schema.oauthConsent.scopes }) + .from(schema.oauthConsent) + .where(eq(schema.oauthConsent.id, consentId)) + ).toEqual([{ scopes: grantedScopes }]) + + const blockedCodeExchange = await exchangeToken( + createFormRequest( + '/api/auth/oauth2/token', + new URLSearchParams({ + grant_type: 'authorization_code', + client_id: provider.SIM_CLI_CLIENT_ID, + code: withheldCode, + code_verifier: withheldVerifier, + redirect_uri: redirectUri, + }) + ) + ) + expect(blockedCodeExchange.status).toBe(400) + const blockedCodeBody = await blockedCodeExchange.json() + expect(blockedCodeBody).toMatchObject({ error: 'invalid_grant' }) + expect(blockedCodeBody).not.toHaveProperty('access_token') + expect(blockedCodeBody).not.toHaveProperty('refresh_token') + + const blockedRefresh = await exchangeToken( + createFormRequest( + '/api/auth/oauth2/token', + new URLSearchParams({ + grant_type: 'refresh_token', + client_id: provider.SIM_CLI_CLIENT_ID, + refresh_token: activeTokens.refresh_token, + }) + ) + ) + expect(blockedRefresh.status).toBe(400) + await expect(blockedRefresh.json()).resolves.toMatchObject({ error: 'invalid_grant' }) + expect( + await db + .select({ generation: schema.oauthTokenFamily.currentGeneration }) + .from(schema.oauthTokenFamily) + .where(eq(schema.oauthTokenFamily.userId, userId)) + ).toEqual([{ generation: 0 }]) + const revokedWhileWithheld = await revokeToken( + createFormRequest( + '/api/auth/oauth2/revoke', + new URLSearchParams({ + client_id: provider.SIM_CLI_CLIENT_ID, + token: activeTokens.refresh_token, + }) + ) + ) + expect(revokedWhileWithheld.status).toBe(200) + expect(await revokedWhileWithheld.text()).toBe('') + expect( + await db + .select({ id: schema.oauthTokenFamily.id }) + .from(schema.oauthTokenFamily) + .where(eq(schema.oauthTokenFamily.userId, userId)) + ).toHaveLength(0) } finally { if (issuedCodeHashes.length) { await db @@ -357,6 +516,10 @@ describe.skipIf(!databaseUrl)('OAuth token route in PostgreSQL', () => { } await db.delete(schema.verification).where(like(schema.verification.value, `%${userId}%`)) await db.delete(schema.user).where(eq(schema.user.id, userId)) + await db + .delete(schema.subscription) + .where(eq(schema.subscription.referenceId, organizationId)) + await db.delete(schema.organization).where(eq(schema.organization.id, organizationId)) await db .delete(schema.rateLimitBucket) .where( diff --git a/apps/sim/app/api/auth/oauth2/token/route.test.ts b/apps/sim/app/api/auth/oauth2/token/route.test.ts index 3aeba421e37..2b72e219cf2 100644 --- a/apps/sim/app/api/auth/oauth2/token/route.test.ts +++ b/apps/sim/app/api/auth/oauth2/token/route.test.ts @@ -1,12 +1,12 @@ /** * @vitest-environment node */ +import { resetEnvFlagsMock, setEnvFlags } from '@sim/testing' import { NextRequest } from 'next/server' -import { beforeEach, describe, expect, it, vi } from 'vitest' +import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest' const mocks = vi.hoisted(() => ({ betterAuthPost: vi.fn(async () => new Response('delegated', { status: 201 })), - oauthEnabled: vi.fn(), rateLimit: vi.fn(async () => null), rotate: vi.fn(), validateClient: vi.fn(), @@ -19,9 +19,6 @@ vi.mock('@/lib/auth', () => ({ auth: { handler: vi.fn() } })) vi.mock('@/lib/auth/oauth-provider-adapter-guard', () => ({ withOAuthProviderIssuanceCompensation: (work: () => Promise) => work(), })) -vi.mock('@/lib/auth/oauth-provider-feature', () => ({ - isOAuthProviderEnabled: mocks.oauthEnabled, -})) vi.mock('@/lib/core/rate-limiter', () => ({ enforceIpRateLimit: mocks.rateLimit })) vi.mock('@/lib/auth/oauth-token-family', () => ({ rotateOAuthRefreshToken: mocks.rotate, @@ -38,10 +35,12 @@ function tokenRequest(body: string) { }) } +afterAll(resetEnvFlagsMock) + describe('OAuth token route', () => { beforeEach(() => { vi.clearAllMocks() - mocks.oauthEnabled.mockResolvedValue(true) + setEnvFlags({ isAuthDisabled: false }) mocks.rotate.mockResolvedValue({ success: true, value: { @@ -345,14 +344,8 @@ describe('OAuth token route', () => { }) }) - it('stops token issuance immediately after the runtime flag is disabled', async () => { - const request = () => - tokenRequest('grant_type=refresh_token&client_id=sim-cli&refresh_token=sim_ort_old') - expect((await POST(request())).status).toBe(200) - mocks.rotate.mockClear() - mocks.rateLimit.mockClear() - - mocks.oauthEnabled.mockResolvedValue(false) + it('requires authentication before token admission or issuance', async () => { + setEnvFlags({ isAuthDisabled: true }) const response = await POST( tokenRequest('grant_type=refresh_token&client_id=sim-cli&refresh_token=sim_ort_old') ) @@ -361,9 +354,5 @@ describe('OAuth token route', () => { expect(mocks.rotate).not.toHaveBeenCalled() expect(mocks.rateLimit).not.toHaveBeenCalled() expect(mocks.betterAuthPost).not.toHaveBeenCalled() - - mocks.oauthEnabled.mockResolvedValue(true) - expect((await POST(request())).status).toBe(200) - expect(mocks.rotate).toHaveBeenCalledOnce() }) }) diff --git a/apps/sim/app/api/auth/oauth2/token/route.ts b/apps/sim/app/api/auth/oauth2/token/route.ts index ad56ae01fc3..95fe53d9727 100644 --- a/apps/sim/app/api/auth/oauth2/token/route.ts +++ b/apps/sim/app/api/auth/oauth2/token/route.ts @@ -15,11 +15,11 @@ import { unsupportedGrantResponse, } from '@/lib/auth/oauth-protocol-request' import { withOAuthProviderIssuanceCompensation } from '@/lib/auth/oauth-provider-adapter-guard' -import { isOAuthProviderEnabled } from '@/lib/auth/oauth-provider-feature' import { rotateOAuthRefreshToken, validateOAuthClientCredentials, } from '@/lib/auth/oauth-token-family' +import { isAuthDisabled } from '@/lib/core/config/env-flags' import { enforceIpRateLimit, type TokenBucketConfig } from '@/lib/core/rate-limiter' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' @@ -40,7 +40,7 @@ const TOKEN_RATE_LIMIT: TokenBucketConfig = { * transaction that the provider does not expose as a configuration hook. */ export const POST = withRouteHandler(async (request: NextRequest) => { - if (!(await isOAuthProviderEnabled())) { + if (isAuthDisabled) { return NextResponse.json( { error: 'OAuth provider is not enabled' }, { status: 404, headers: { 'Cache-Control': 'no-store', Pragma: 'no-cache' } } diff --git a/apps/sim/app/api/organizations/[id]/permission-groups/[groupId]/route.test.ts b/apps/sim/app/api/organizations/[id]/permission-groups/[groupId]/route.test.ts new file mode 100644 index 00000000000..02948dc1ee5 --- /dev/null +++ b/apps/sim/app/api/organizations/[id]/permission-groups/[groupId]/route.test.ts @@ -0,0 +1,153 @@ +/** + * @vitest-environment node + */ +import { db } from '@sim/db' +import { permissionGroup } from '@sim/db/schema' +import { + authMockFns, + createMockRequest, + dbChainMockFns, + queueTableRows, + resetDbChainMock, +} from '@sim/testing' +import { beforeEach, describe, expect, it, vi } from 'vitest' +import type { UpdatePermissionGroupBody } from '@/lib/api/contracts/permission-groups' + +const mocks = vi.hoisted(() => ({ + acquireLock: vi.fn(), + authorize: vi.fn(), + loadGroup: vi.fn(), +})) + +vi.mock('@/lib/permission-groups/locks', () => ({ + acquirePermissionGroupOrgLock: mocks.acquireLock, +})) + +vi.mock('@/lib/permission-groups/application/group-membership', () => ({ + findAllMembersWorkspaceConflict: vi.fn(), + findScopeConflicts: vi.fn(), +})) + +vi.mock('@/app/api/organizations/[id]/permission-groups/utils', () => ({ + authorizeOrgAccessControl: mocks.authorize, + loadGroupInOrganization: mocks.loadGroup, + findWorkspacesNotInOrganization: vi.fn(), + formatAllMembersConflictError: vi.fn(), + formatScopeConflictError: vi.fn(), + getGroupWorkspaces: vi.fn(), +})) + +vi.mock('@sim/audit', () => ({ + recordAudit: vi.fn(), + AuditAction: { PERMISSION_GROUP_UPDATED: 'permission_group.updated' }, + AuditResourceType: { PERMISSION_GROUP: 'permission_group' }, +})) + +import { PUT } from '@/app/api/organizations/[id]/permission-groups/[groupId]/route' + +const ORGANIZATION_ID = 'org-1' +const GROUP_ID = 'group-1' +const GROUP = { + id: GROUP_ID, + organizationId: ORGANIZATION_ID, + name: 'Default', + description: null, + isDefault: true, + config: { disableOAuthAppAccess: false }, +} + +async function updateUnderLock(body: UpdatePermissionGroupBody) { + const lockEntered = Promise.withResolvers() + const lockReleased = Promise.withResolvers() + mocks.acquireLock.mockImplementationOnce(() => { + lockEntered.resolve(true) + return lockReleased.promise + }) + + const pendingResponse = PUT(createMockRequest('PUT', body), { + params: Promise.resolve({ id: ORGANIZATION_ID, groupId: GROUP_ID }), + }) + try { + expect(await Promise.race([lockEntered.promise, pendingResponse.then(() => false)])).toBe(true) + expect(mocks.acquireLock).toHaveBeenCalledExactlyOnceWith(db, ORGANIZATION_ID) + expect(dbChainMockFns.update).not.toHaveBeenCalled() + } finally { + lockReleased.resolve() + } + + const response = await pendingResponse + expect(response.status).toBe(200) + expect(dbChainMockFns.update).toHaveBeenCalledExactlyOnceWith(permissionGroup) + expect(mocks.loadGroup).toHaveBeenLastCalledWith(GROUP_ID, ORGANIZATION_ID, db) + return response +} + +describe('permission group PUT policy serialization', () => { + beforeEach(() => { + vi.clearAllMocks() + resetDbChainMock() + authMockFns.mockGetSession.mockResolvedValue({ user: { id: 'admin-1' } }) + mocks.authorize.mockResolvedValue(null) + mocks.loadGroup.mockResolvedValue(GROUP) + }) + + it('locks a config-only update and writes the requested OAuth restriction', async () => { + queueTableRows(permissionGroup, [{ ...GROUP, config: { disableOAuthAppAccess: true } }]) + + await updateUnderLock({ config: { disableOAuthAppAccess: true } }) + + expect(dbChainMockFns.set).toHaveBeenCalledExactlyOnceWith( + expect.objectContaining({ config: expect.objectContaining({ disableOAuthAppAccess: true }) }) + ) + }) + + it.each([{ name: 'Renamed group' }, { description: 'Updated description' }])( + 'locks metadata-only update %j without restoring stale policy', + async (metadata) => { + if ('name' in metadata) queueTableRows(permissionGroup, []) + queueTableRows(permissionGroup, [ + { ...GROUP, ...metadata, config: { disableOAuthAppAccess: true } }, + ]) + + const response = await updateUnderLock(metadata) + + expect(dbChainMockFns.set).toHaveBeenCalledExactlyOnceWith(expect.objectContaining(metadata)) + expect(dbChainMockFns.set.mock.calls[0][0]).not.toHaveProperty('config') + await expect(response.json()).resolves.toMatchObject({ + permissionGroup: { config: { disableOAuthAppAccess: true } }, + }) + } + ) + + it('merges a config patch with the policy reloaded under the lock', async () => { + mocks.loadGroup + .mockResolvedValueOnce(GROUP) + .mockResolvedValueOnce({ ...GROUP, config: { disableOAuthAppAccess: true } }) + queueTableRows(permissionGroup, [ + { ...GROUP, config: { disableOAuthAppAccess: true, disableCliAccess: true } }, + ]) + + await updateUnderLock({ config: { disableCliAccess: true } }) + + expect(dbChainMockFns.set).toHaveBeenCalledExactlyOnceWith( + expect.objectContaining({ + config: expect.objectContaining({ disableOAuthAppAccess: true, disableCliAccess: true }), + }) + ) + }) + + it('does not write when the group disappears before the locked reload', async () => { + mocks.loadGroup.mockResolvedValueOnce(GROUP).mockResolvedValueOnce(null) + mocks.acquireLock.mockResolvedValueOnce(undefined) + + const response = await PUT(createMockRequest('PUT', { description: 'Updated description' }), { + params: Promise.resolve({ id: ORGANIZATION_ID, groupId: GROUP_ID }), + }) + + expect(response.status).toBe(404) + await expect(response.json()).resolves.toEqual({ error: 'Permission group not found' }) + expect(mocks.acquireLock).toHaveBeenCalledExactlyOnceWith(db, ORGANIZATION_ID) + expect(mocks.loadGroup).toHaveBeenLastCalledWith(GROUP_ID, ORGANIZATION_ID, db) + expect(dbChainMockFns.update).not.toHaveBeenCalled() + }) +}) diff --git a/apps/sim/app/api/organizations/[id]/permission-groups/[groupId]/route.ts b/apps/sim/app/api/organizations/[id]/permission-groups/[groupId]/route.ts index 416c1353ee6..a245f86f430 100644 --- a/apps/sim/app/api/organizations/[id]/permission-groups/[groupId]/route.ts +++ b/apps/sim/app/api/organizations/[id]/permission-groups/[groupId]/route.ts @@ -112,11 +112,6 @@ export const PUT = withRouteHandler( } } - const currentConfig = parsePermissionGroupConfig(group.config) - const newConfig: PermissionGroupConfig = updates.config - ? { ...currentConfig, ...updates.config } - : currentConfig - // Demoting the org default with no new scope: it becomes a non-default // group with no workspaces (inert) until an admin re-scopes it. The client // sends only `isDefault: false`, so this never forwards a workspace list. @@ -169,19 +164,20 @@ export const PUT = withRouteHandler( const now = new Date() await db.transaction(async (tx) => { + await acquirePermissionGroupOrgLock(tx, organizationId) + const currentGroup = await loadGroupInOrganization(id, organizationId, tx) + if (!currentGroup) throw new Error('GROUP_NOT_FOUND') + const newConfig: PermissionGroupConfig | undefined = updates.config + ? { ...parsePermissionGroupConfig(currentGroup.config), ...updates.config } + : undefined + // For a specific-scope group the target workspaces are the request's // explicit ids, or — when omitted ("keep current") — the group's current // workspaces read under the lock so the conflict check and write share // one snapshot. let resolvedWorkspaceIds: string[] = [] - // When the scope changes, serialize against other permission-group writes - // for this org and re-check membership conflicts atomically with the - // write, so a concurrent member add (or scope change) can't slip a user - // into two groups that overlap on a workspace. if (scopeProvided) { - await acquirePermissionGroupOrgLock(tx, organizationId) - if (!effectiveIsDefault) { // May resolve to an empty list — a non-default group is allowed to // target zero workspaces (governs nothing). The write below deletes @@ -243,7 +239,7 @@ export const PUT = withRouteHandler( ...(updates.name !== undefined && { name: updates.name }), ...(updates.description !== undefined && { description: updates.description }), ...(updates.isDefault !== undefined && { isDefault: updates.isDefault }), - config: newConfig, + ...(newConfig !== undefined && { config: newConfig }), updatedAt: now, }) .where(eq(permissionGroup.id, id)) @@ -302,6 +298,9 @@ export const PUT = withRouteHandler( }, }) } catch (error) { + if (error instanceof Error && error.message === 'GROUP_NOT_FOUND') { + return NextResponse.json({ error: 'Permission group not found' }, { status: 404 }) + } if (error instanceof Error && error.message === 'SCOPE_CONFLICT') { return NextResponse.json( { error: formatScopeConflictError(scopeConflicts) }, diff --git a/apps/sim/lib/api/server/routes/v2-api-key-auth.test.ts b/apps/sim/lib/api/server/routes/v2-api-key-auth.test.ts index 1a7e0118441..cbf38f3cbbb 100644 --- a/apps/sim/lib/api/server/routes/v2-api-key-auth.test.ts +++ b/apps/sim/lib/api/server/routes/v2-api-key-auth.test.ts @@ -8,14 +8,10 @@ const mocks = vi.hoisted(() => ({ updateLastUsed: vi.fn(), resolveWorkspaceBillingPayer: vi.fn(), getHighestPrioritySubscription: vi.fn(), - isOAuthProviderEnabled: vi.fn(), envFlags: { isAuthDisabled: false }, })) vi.mock('@/lib/core/config/env-flags', () => mocks.envFlags) -vi.mock('@/lib/auth/oauth-provider-feature', () => ({ - isOAuthProviderEnabled: mocks.isOAuthProviderEnabled, -})) vi.mock('@/lib/api-key/crypto', () => ({ hashApiKey: (value: string) => `hash:${value}` })) vi.mock('@/lib/auth/oauth-provider', () => ({ OAUTH_ACCESS_TOKEN_PREFIX: 'sim_oat_' })) vi.mock('@sim/security/hash', () => ({ sha256Hex: (value: string) => `oauth-hash:${value}` })) @@ -40,7 +36,6 @@ describe('v2 API key authentication', () => { beforeEach(() => { vi.clearAllMocks() mocks.envFlags.isAuthDisabled = false - mocks.isOAuthProviderEnabled.mockResolvedValue(true) resetDbChainMock() mocks.updateLastUsed.mockResolvedValue(undefined) mocks.getHighestPrioritySubscription.mockResolvedValue(null) @@ -194,7 +189,6 @@ describe('v2 bearer token authentication', () => { beforeEach(() => { vi.clearAllMocks() mocks.envFlags.isAuthDisabled = false - mocks.isOAuthProviderEnabled.mockResolvedValue(true) resetDbChainMock() mocks.getHighestPrioritySubscription.mockResolvedValue({ plan: 'pro', @@ -273,8 +267,7 @@ describe('v2 bearer token authentication', () => { expect(mocks.updateLastUsed).not.toHaveBeenCalled() }) - it.each([true, false])('prefers the API key with OAuth enabled=%s', async (enabled) => { - mocks.isOAuthProviderEnabled.mockResolvedValue(enabled) + it('prefers the API key when an OAuth token is also present', async () => { queueTableRows(schemaMock.apiKey, [ { id: 'key-1', @@ -289,35 +282,9 @@ describe('v2 bearer token authentication', () => { const result = await authenticateV2ApiKey({ apiKey: 'secret', bearer: 'sim_oat_ignored' }) expect(result.keyType).toBe('personal') - expect(mocks.isOAuthProviderEnabled).not.toHaveBeenCalled() - }) - - it('applies runtime flag changes without reloading the authenticator', async () => { - mocks.isOAuthProviderEnabled - .mockResolvedValueOnce(true) - .mockResolvedValueOnce(false) - .mockResolvedValueOnce(true) - queueTableRows(schemaMock.oauthAccessToken, [tokenRow()]) - queueTableRows(schemaMock.oauthAccessToken, [tokenRow()]) - const credential = { apiKey: null, bearer: 'sim_oat_secret' } - - await expect(authenticateV2ApiKey(credential)).resolves.toMatchObject({ - keyType: 'oauth_access_token', - }) - await expect(authenticateV2ApiKey(credential)).rejects.toMatchObject({ - message: 'Bearer tokens are not accepted', - challenge: 'bearer', - }) - expect(dbChainMockFns.limit).toHaveBeenCalledTimes(1) - await expect(authenticateV2ApiKey(credential)).resolves.toMatchObject({ - keyType: 'oauth_access_token', - }) - expect(mocks.isOAuthProviderEnabled).toHaveBeenCalledTimes(3) - expect(mocks.isOAuthProviderEnabled).toHaveBeenCalledWith() - expect(dbChainMockFns.limit).toHaveBeenCalledTimes(2) }) - it('preserves auth-disabled deployment behavior without reading the OAuth flag', async () => { + it('preserves auth-disabled deployment behavior without verifying an OAuth token', async () => { mocks.envFlags.isAuthDisabled = true await expect( @@ -326,7 +293,6 @@ describe('v2 bearer token authentication', () => { principal: { kind: 'personal_api_key', keyId: 'auth-disabled' }, keyType: 'personal', }) - expect(mocks.isOAuthProviderEnabled).not.toHaveBeenCalled() expect(dbChainMockFns.limit).not.toHaveBeenCalled() }) diff --git a/apps/sim/lib/api/server/routes/v2-api-key-auth.ts b/apps/sim/lib/api/server/routes/v2-api-key-auth.ts index ca111042d92..75ee16986d5 100644 --- a/apps/sim/lib/api/server/routes/v2-api-key-auth.ts +++ b/apps/sim/lib/api/server/routes/v2-api-key-auth.ts @@ -12,7 +12,6 @@ import { hashApiKey } from '@/lib/api-key/crypto' import { updateApiKeyLastUsed } from '@/lib/api-key/service' import { ANONYMOUS_USER_ID } from '@/lib/auth/constants' import { InvalidOAuthAccessTokenError, verifyOAuthAccessToken } from '@/lib/auth/oauth-access-token' -import { isOAuthProviderEnabled } from '@/lib/auth/oauth-provider-feature' import { resolveWorkspaceBillingPayer } from '@/lib/billing/core/billing-attribution' import { getHighestPrioritySubscription } from '@/lib/billing/core/subscription' import { isAuthDisabled } from '@/lib/core/config/env-flags' @@ -158,9 +157,6 @@ async function authenticateApiKey(apiKeyHeader: string): Promise { - if (!(await isOAuthProviderEnabled())) { - throw new V2ApiKeyUnauthenticatedError('Bearer tokens are not accepted', 'bearer') - } let principal: OAuthAccessTokenPrincipal try { principal = await verifyOAuthAccessToken(token) diff --git a/apps/sim/lib/audit-logs/application/audit-log-use-cases.test.ts b/apps/sim/lib/audit-logs/application/audit-log-use-cases.test.ts index ed34984beb2..1a923ae5d88 100644 --- a/apps/sim/lib/audit-logs/application/audit-log-use-cases.test.ts +++ b/apps/sim/lib/audit-logs/application/audit-log-use-cases.test.ts @@ -14,6 +14,11 @@ const mocks = vi.hoisted(() => ({ decodeAuditLogCursor: vi.fn(), queryAuditLogs: vi.fn(), recordAudit: vi.fn(), + isCapabilityWithheldForUser: vi.fn(), +})) + +vi.mock('@/lib/permission-groups/user-scope.server', () => ({ + isCapabilityWithheldForUser: mocks.isCapabilityWithheldForUser, })) vi.mock('@/lib/audit-logs/authorization', () => ({ @@ -55,6 +60,7 @@ describe('audit-log application use cases', () => { beforeEach(() => { vi.clearAllMocks() resetDbChainMock() + mocks.isCapabilityWithheldForUser.mockResolvedValue(false) mocks.resolveDefaultOrganization.mockResolvedValue({ kind: 'resolved', organizationId: 'organization-1', @@ -115,6 +121,48 @@ describe('audit-log application use cases', () => { expect(mocks.recordAudit).not.toHaveBeenCalled() }) + it.each(['sim-cli', 'partner-app'])( + 'rechecks the organization OAuth restriction for an existing %s audit token', + async (clientId) => { + const principal = { + kind: 'oauth_access_token', + userId: 'admin-1', + clientId, + tokenId: 'token-1', + scopes: ['api:read'], + expiresAt: new Date('2099-01-01T00:00:00Z'), + } as const + await expect(listAuditLogs.execute({ principal, input: listInput })).resolves.toEqual({ + data: [], + nextCursor: undefined, + }) + mocks.queryAuditLogs.mockClear() + mocks.isCapabilityWithheldForUser.mockImplementation( + async (_userId: string, capability: string) => capability === 'oauth_apps.use' + ) + await expect( + listAuditLogs.execute({ principal, input: { ...listInput, organizationId: undefined } }) + ).rejects.toMatchObject({ capability: 'oauth_apps.use' }) + expect(mocks.isCapabilityWithheldForUser).toHaveBeenCalledWith('admin-1', 'oauth_apps.use') + expect(mocks.queryAuditLogs).not.toHaveBeenCalled() + } + ) + + it('keeps audit sessions and personal API keys independent of the OAuth app restriction', async () => { + mocks.isCapabilityWithheldForUser.mockImplementation( + async (_userId: string, capability: string) => capability === 'oauth_apps.use' + ) + await expect( + listAuditLogs.execute({ principal: sessionPrincipal, input: listInput }) + ).resolves.toBeDefined() + await expect( + listAuditLogs.execute({ + principal: { kind: 'personal_api_key', userId: 'admin-1', keyId: 'key-1' }, + input: listInput, + }) + ).resolves.toBeDefined() + }) + /** * Nothing an API key can reach publishes an organization id, so a required * `organizationId` made the whole resource unreachable from a key. It is diff --git a/apps/sim/lib/audit-logs/application/authorized-audit-log-use-case.ts b/apps/sim/lib/audit-logs/application/authorized-audit-log-use-case.ts index a823c622ff1..aaa3896c335 100644 --- a/apps/sim/lib/audit-logs/application/authorized-audit-log-use-case.ts +++ b/apps/sim/lib/audit-logs/application/authorized-audit-log-use-case.ts @@ -88,11 +88,17 @@ export function defineAuthorizedAuditLogUseCase { - /** Keep direct plugin calls behind the runtime gate without blocking connector OAuth. */ + /** Refuse provider calls when user authentication is disabled without blocking connector OAuth. */ if ( ((ctx.path.startsWith('/oauth2/') && ctx.path !== '/oauth2/link' && !ctx.path.startsWith('/oauth2/callback/')) || ctx.path === '/.well-known/oauth-authorization-server') && - !(await isOAuthProviderEnabled()) + isAuthDisabled ) { throw new APIError('NOT_FOUND', { message: 'OAuth provider is not enabled' }) } @@ -932,27 +931,34 @@ export const auth = betterAuth({ } /** - * A user consenting to the Sim CLI is the one moment a human is present - * in a CLI login, so `cli.use` is checked here to refuse the grant - * outright. `requireCliAccessAllowed` checks it again on every bearer - * request, because a consent already on file lets later authorizations - * skip this endpoint entirely — neither check makes the other redundant. - * - * The client id is read from the signed authorize query the consent page - * forwards. The gate fires if `sim-cli` appears anywhere in it, which is - * strictly more conservative than the plugin's own first-value read. - * - * permission-group-enforced: cli.use — gates OAuth consent for the - * first-party CLI client, which owns no workspace resource for the - * authorization funnel to authorize. + * permission-group-enforced: oauth_apps.use, cli.use — account-level + * authorization uses the default group; token issuance rechecks it later. + * Explicit denial remains available even when access has been withheld. */ - if (ctx.path === '/oauth2/consent') { - if (consentRequestNamesClient(ctx.body?.oauth_query, SIM_CLI_CLIENT_ID)) { - const session = await getSessionFromCtx(ctx) - const userId = session?.user?.id - if (userId && (await isCapabilityWithheldForUser(userId, 'cli.use'))) { - logger.warn('CLI OAuth consent blocked by permission group', { userId }) - throw new APIError('FORBIDDEN', { message: capabilityRefusal('cli.use') }) + if ( + ctx.path === '/oauth2/authorize' || + (ctx.path === '/oauth2/consent' && ctx.body?.accept === true) + ) { + const session = await getSessionFromCtx(ctx) + const userId = session?.user?.id + if (userId) { + if (await isCapabilityWithheldForUser(userId, 'oauth_apps.use')) { + throw new APIError('FORBIDDEN', { + message: capabilityRefusal('oauth_apps.use'), + error: 'access_denied', + error_description: capabilityRefusal('oauth_apps.use'), + }) + } + const isCli = + ctx.path === '/oauth2/authorize' + ? ctx.query?.client_id === SIM_CLI_CLIENT_ID + : consentRequestNamesClient(ctx.body?.oauth_query, SIM_CLI_CLIENT_ID) + if (isCli && (await isCapabilityWithheldForUser(userId, 'cli.use'))) { + throw new APIError('FORBIDDEN', { + message: capabilityRefusal('cli.use'), + error: 'access_denied', + error_description: capabilityRefusal('cli.use'), + }) } } } @@ -1270,9 +1276,6 @@ export const auth = betterAuth({ * ID-token semantics out of the advertised protocol. Clients are DB rows * only (the CLI is seeded by migration, the rest are admin-created), so * both registration paths stay closed. - * - * Register once; request-time gates let AppConfig change availability - * without a restart. */ ...(!isAuthDisabled ? [ diff --git a/apps/sim/lib/auth/oauth-authorization-error.ts b/apps/sim/lib/auth/oauth-authorization-error.ts index 22697374532..fa0c791b183 100644 --- a/apps/sim/lib/auth/oauth-authorization-error.ts +++ b/apps/sim/lib/auth/oauth-authorization-error.ts @@ -10,7 +10,10 @@ import { getBaseUrl } from '@/lib/core/utils/urls' const logger = createLogger('OAuthAuthorizationError') -export type OAuthAuthorizationErrorCode = 'invalid_request' | 'unsupported_response_type' +export type OAuthAuthorizationErrorCode = + | 'access_denied' + | 'invalid_request' + | 'unsupported_response_type' function isLoopbackIp(hostname: string): boolean { const address = hostname.startsWith('[') ? hostname.slice(1, -1) : hostname diff --git a/apps/sim/lib/auth/oauth-protocol-request.ts b/apps/sim/lib/auth/oauth-protocol-request.ts index 4c517a73d2c..361b83fe605 100644 --- a/apps/sim/lib/auth/oauth-protocol-request.ts +++ b/apps/sim/lib/auth/oauth-protocol-request.ts @@ -19,6 +19,7 @@ export type OAuthFormParseResult = export function oauthErrorResponse( error: | OAuthProtocolErrorCode + | 'access_denied' | 'invalid_request' | 'invalid_token' | 'insufficient_scope' diff --git a/apps/sim/lib/auth/oauth-provider-adapter-guard.test.ts b/apps/sim/lib/auth/oauth-provider-adapter-guard.test.ts index 445d43faa7e..4ddd9c77cf4 100644 --- a/apps/sim/lib/auth/oauth-provider-adapter-guard.test.ts +++ b/apps/sim/lib/auth/oauth-provider-adapter-guard.test.ts @@ -6,9 +6,13 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' const mocks = vi.hoisted(() => ({ delete: vi.fn(), insert: vi.fn(), + isCapabilityWithheldForUser: vi.fn(), })) vi.mock('@sim/db', () => ({ db: { delete: mocks.delete, insert: mocks.insert } })) +vi.mock('@/lib/permission-groups/user-scope.server', () => ({ + isCapabilityWithheldForUser: mocks.isCapabilityWithheldForUser, +})) import { guardOAuthProviderWrites, @@ -43,6 +47,7 @@ function mockUpsert(rows: Record[]) { describe('guardOAuthProviderWrites', () => { beforeEach(() => { vi.clearAllMocks() + mocks.isCapabilityWithheldForUser.mockResolvedValue(false) mocks.delete.mockReturnValue({ where: vi.fn(async () => undefined) }) }) @@ -94,12 +99,59 @@ describe('guardOAuthProviderWrites', () => { expect(mocks.insert).not.toHaveBeenCalled() }) + it.each(['oauthAccessToken', 'oauthRefreshToken'])( + 'refuses %s before persistence when the canonical owner is restricted', + async (model) => { + const base = adapter() + mocks.isCapabilityWithheldForUser.mockResolvedValue(true) + const guarded = guardOAuthProviderWrites(base) + await expect( + guarded.create({ model, data: { userId: 'token-owner', clientId: 'partner-app' } }) + ).rejects.toMatchObject({ body: { error: 'invalid_grant' } }) + expect(mocks.isCapabilityWithheldForUser).toHaveBeenCalledWith( + 'token-owner', + 'oauth_apps.use' + ) + expect(base.create).not.toHaveBeenCalled() + } + ) + + it.each(['oauthAccessToken', 'oauthRefreshToken'])( + 'requires a canonical user for %s', + async (model) => { + const base = adapter() + await expect( + guardOAuthProviderWrites(base).create({ model, data: { clientId: 'partner-app' } }) + ).rejects.toMatchObject({ body: { error: 'invalid_grant' } }) + expect(base.create).not.toHaveBeenCalled() + expect(mocks.isCapabilityWithheldForUser).not.toHaveBeenCalled() + } + ) + + it('rechecks policy between the refresh and access inserts and compensates the family', async () => { + const base = adapter() + const guarded = guardOAuthProviderWrites(base) + mocks.isCapabilityWithheldForUser.mockResolvedValueOnce(false).mockResolvedValueOnce(true) + await expect( + withOAuthProviderIssuanceCompensation(async () => { + await guarded.create({ model: 'oauthRefreshToken', data: { userId: 'user-1' } }) + await guarded.create({ model: 'oauthAccessToken', data: { userId: 'user-1' } }) + return new Response(null, { status: 200 }) + }) + ).rejects.toMatchObject({ body: { error: 'invalid_grant' } }) + expect(base.create).toHaveBeenCalledOnce() + expect(mocks.delete).toHaveBeenCalledOnce() + }) + it('deletes a refresh family when delegated token issuance fails', async () => { const base = adapter() const guarded = guardOAuthProviderWrites(base) const response = await withOAuthProviderIssuanceCompensation(async () => { - await guarded.create({ model: 'oauthRefreshToken', data: { token: 'hashed' } }) + await guarded.create({ + model: 'oauthRefreshToken', + data: { token: 'hashed', userId: 'user-1' }, + }) return new Response('failed', { status: 500 }) }) @@ -111,7 +163,10 @@ describe('guardOAuthProviderWrites', () => { const guarded = guardOAuthProviderWrites(adapter()) await withOAuthProviderIssuanceCompensation(async () => { - await guarded.create({ model: 'oauthRefreshToken', data: { token: 'hashed' } }) + await guarded.create({ + model: 'oauthRefreshToken', + data: { token: 'hashed', userId: 'user-1' }, + }) return new Response(null, { status: 200 }) }) diff --git a/apps/sim/lib/auth/oauth-provider-adapter-guard.ts b/apps/sim/lib/auth/oauth-provider-adapter-guard.ts index 32fcf16663d..c2eace39e03 100644 --- a/apps/sim/lib/auth/oauth-provider-adapter-guard.ts +++ b/apps/sim/lib/auth/oauth-provider-adapter-guard.ts @@ -3,7 +3,10 @@ import { db } from '@sim/db' import { oauthConsent, oauthTokenFamily } from '@sim/db/schema' import { generateId } from '@sim/utils/id' import type { drizzleAdapter } from 'better-auth/adapters/drizzle' +import { APIError } from 'better-auth/api' import { inArray } from 'drizzle-orm' +import { capabilityRefusal } from '@/lib/permission-groups/capabilities' +import { isCapabilityWithheldForUser } from '@/lib/permission-groups/user-scope.server' type BetterAuthAdapter = ReturnType> export type AuthDatabase = typeof db | Parameters[0]>[0] @@ -88,6 +91,26 @@ export function guardOAuthProviderWrites( return { ...adapter, create: async (input) => { + /** + * permission-group-enforced: oauth_apps.use — recheck the canonical token + * owner after code validation, including codes issued before a policy change. + */ + if (input.model === 'oauthAccessToken' || input.model === 'oauthRefreshToken') { + const userId = input.data.userId + if (typeof userId !== 'string' || !userId) { + throw new APIError('BAD_REQUEST', { + error: 'invalid_grant', + error_description: 'OAuth tokens require a user.', + }) + } + if (await isCapabilityWithheldForUser(userId, 'oauth_apps.use')) { + throw new APIError('BAD_REQUEST', { + error: 'invalid_grant', + error_description: capabilityRefusal('oauth_apps.use'), + }) + } + } + if (input.model !== 'oauthConsent') { const created = await adapter.create(input) if (input.model === 'oauthRefreshToken') { diff --git a/apps/sim/lib/auth/oauth-provider-feature.test.ts b/apps/sim/lib/auth/oauth-provider-feature.test.ts deleted file mode 100644 index 4ffb43a1e73..00000000000 --- a/apps/sim/lib/auth/oauth-provider-feature.test.ts +++ /dev/null @@ -1,41 +0,0 @@ -/** - * @vitest-environment node - */ -import { resetEnvFlagsMock, setEnvFlags } from '@sim/testing' -import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest' - -const { mockIsFeatureEnabled } = vi.hoisted(() => ({ mockIsFeatureEnabled: vi.fn() })) - -vi.mock('@/lib/core/config/feature-flags', () => ({ isFeatureEnabled: mockIsFeatureEnabled })) - -/** Isolates the real helper from route suites that mock it in the shared worker. */ -declare module '@/lib/auth/oauth-provider-feature?oauth-provider-feature-test' { - // biome-ignore lint/suspicious/noExportsInTest: ambient declaration for the isolated test import - export * from '@/lib/auth/oauth-provider-feature' -} - -import { isOAuthProviderEnabled } from '@/lib/auth/oauth-provider-feature?oauth-provider-feature-test' - -afterAll(resetEnvFlagsMock) - -describe('OAuth provider rollout', () => { - beforeEach(() => { - vi.clearAllMocks() - setEnvFlags({ isAuthDisabled: false }) - }) - - it('evaluates the global flag again on subsequent requests', async () => { - mockIsFeatureEnabled.mockResolvedValueOnce(false).mockResolvedValueOnce(true) - await expect(isOAuthProviderEnabled()).resolves.toBe(false) - await expect(isOAuthProviderEnabled()).resolves.toBe(true) - expect(mockIsFeatureEnabled).toHaveBeenNthCalledWith(1, 'oauth-provider') - expect(mockIsFeatureEnabled).toHaveBeenNthCalledWith(2, 'oauth-provider') - }) - - it('cannot enable OAuth without user authentication', async () => { - setEnvFlags({ isAuthDisabled: true }) - mockIsFeatureEnabled.mockResolvedValue(true) - await expect(isOAuthProviderEnabled()).resolves.toBe(false) - expect(mockIsFeatureEnabled).not.toHaveBeenCalled() - }) -}) diff --git a/apps/sim/lib/auth/oauth-provider-feature.ts b/apps/sim/lib/auth/oauth-provider-feature.ts deleted file mode 100644 index 52911163c20..00000000000 --- a/apps/sim/lib/auth/oauth-provider-feature.ts +++ /dev/null @@ -1,7 +0,0 @@ -import { isAuthDisabled } from '@/lib/core/config/env-flags' -import { isFeatureEnabled } from '@/lib/core/config/feature-flags' - -/** Global runtime rollout gate; OAuth requires real user sessions. */ -export async function isOAuthProviderEnabled(): Promise { - return !isAuthDisabled && (await isFeatureEnabled('oauth-provider')) -} diff --git a/apps/sim/lib/auth/oauth-provider-lifecycle.postgres.test.ts b/apps/sim/lib/auth/oauth-provider-lifecycle.postgres.test.ts index 9432ddc0848..6b4a777d230 100644 --- a/apps/sim/lib/auth/oauth-provider-lifecycle.postgres.test.ts +++ b/apps/sim/lib/auth/oauth-provider-lifecycle.postgres.test.ts @@ -2,6 +2,8 @@ * @vitest-environment node */ import { auditMock, auditMockFns } from '@sim/testing/mocks/audit.mock' +import { envFlagsMock } from '@sim/testing/mocks/env-flags.mock' +import { sleep } from '@sim/utils/helpers' import { generateId } from '@sim/utils/id' import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest' @@ -9,6 +11,11 @@ vi.unmock('@sim/db') vi.unmock('@sim/db/schema') vi.unmock('drizzle-orm') vi.mock('@sim/audit', () => auditMock) +vi.mock('@/lib/core/config/env-flags', () => ({ + ...envFlagsMock, + isHosted: true, + isBillingEnabled: true, +})) const databaseUrl = process.env.OAUTH_TOKEN_FAMILY_TEST_DATABASE_URL @@ -16,16 +23,19 @@ async function loadRuntime() { const [ { db }, schema, - { eq, inArray }, + { eq, inArray, sql: statement }, { oauthProvider }, { createSimAuthAdapter }, { withOAuthProviderIssuanceCompensation }, - { rotateOAuthRefreshToken }, + { rotateOAuthRefreshToken, revokeOAuthToken }, { listAuthorizedAppsUseCase, revokeAuthorizedAppUseCase }, { reconcileOAuthProviderLifecycle }, { default: postgres }, { hashOAuthToken }, { runCleanupOAuthTokens, OAUTH_TOKEN_RETENTION_DAYS }, + { isCapabilityWithheldForUser }, + { acquirePermissionGroupOrgLock }, + { acquireOrganizationMutationLock }, ] = await Promise.all([ import('@sim/db'), import('@sim/db/schema'), @@ -39,6 +49,9 @@ async function loadRuntime() { import('postgres'), import('@/lib/auth/oauth-access-token'), import('@/background/cleanup-oauth-tokens'), + import('@/lib/permission-groups/user-scope.server'), + import('@/lib/permission-groups/locks'), + import('@/lib/billing/organizations/membership'), ]) const adapter = createSimAuthAdapter({ plugins: [ @@ -56,16 +69,21 @@ async function loadRuntime() { schema, eq, inArray, + statement, adapter, sql, reconcileOAuthProviderLifecycle, withOAuthProviderIssuanceCompensation, rotateOAuthRefreshToken, + revokeOAuthToken, revokeAuthorizedAppUseCase, listAuthorizedAppsUseCase, hashOAuthToken, runCleanupOAuthTokens, OAUTH_TOKEN_RETENTION_DAYS, + isCapabilityWithheldForUser, + acquirePermissionGroupOrgLock, + acquireOrganizationMutationLock, } } @@ -75,6 +93,7 @@ describe.skipIf(!databaseUrl)('OAuth lifecycle on the provisioned PostgreSQL sch let clientId: string let createdClientIds: string[] let createdUserIds: string[] + let organizationId: string | undefined const scopes = ['offline_access', 'api:read', 'api:write'] beforeAll(async () => { @@ -88,6 +107,7 @@ describe.skipIf(!databaseUrl)('OAuth lifecycle on the provisioned PostgreSQL sch clientId = generateId() createdClientIds = [clientId] createdUserIds = [userId] + organizationId = undefined const now = new Date() await runtime.db.insert(runtime.schema.user).values({ id: userId, @@ -115,6 +135,12 @@ describe.skipIf(!databaseUrl)('OAuth lifecycle on the provisioned PostgreSQL sch .delete(schema.oauthClient) .where(inArray(schema.oauthClient.clientId, createdClientIds)) await db.delete(schema.user).where(inArray(schema.user.id, createdUserIds)) + if (organizationId) { + await db + .delete(schema.subscription) + .where(runtime.eq(schema.subscription.referenceId, organizationId)) + await db.delete(schema.organization).where(runtime.eq(schema.organization.id, organizationId)) + } }) afterAll(async () => { @@ -185,6 +211,248 @@ describe.skipIf(!databaseUrl)('OAuth lifecycle on the provisioned PostgreSQL sch ).toHaveLength(0) } + async function createDefaultGroup() { + const { db, schema } = runtime + organizationId = generateId() + const groupId = generateId() + await db.insert(schema.organization).values({ + id: organizationId, + name: 'OAuth permission fixture', + slug: organizationId, + createdAt: new Date(), + }) + await db + .insert(schema.member) + .values({ id: generateId(), userId, organizationId, role: 'owner' }) + await db.insert(schema.userStats).values({ id: generateId(), userId, billingBlocked: false }) + await db.insert(schema.subscription).values({ + id: generateId(), + plan: 'enterprise', + referenceId: organizationId, + status: 'active', + seats: 5, + periodStart: new Date(), + periodEnd: new Date(Date.now() + 86_400_000), + metadata: { plan: 'enterprise', referenceId: organizationId, seats: 5, monthlyPrice: 100 }, + }) + await db.insert(schema.permissionGroup).values({ + id: groupId, + organizationId, + createdBy: userId, + name: 'Default', + isDefault: true, + config: {}, + }) + return groupId + } + + it.each([false, true])( + 'withholds issuance and refresh while retaining revocation for skipConsent=%s', + async (skipConsent) => { + const { db, schema, eq } = runtime + const groupId = await createDefaultGroup() + if (skipConsent) + await db + .update(schema.oauthClient) + .set({ skipConsent: true }) + .where(eq(schema.oauthClient.clientId, clientId)) + else await grantConsent() + expect(await runtime.isCapabilityWithheldForUser(userId, 'oauth_apps.use')).toBe(false) + const family = await issueFamily() + + await db + .update(schema.permissionGroup) + .set({ config: { disableOAuthAppAccess: true } }) + .where(eq(schema.permissionGroup.id, groupId)) + expect(await runtime.isCapabilityWithheldForUser(userId, 'oauth_apps.use')).toBe(true) + await expect(issueFamily()).rejects.toMatchObject({ body: { error: 'invalid_grant' } }) + await expect( + runtime.rotateOAuthRefreshToken({ + credentials: { clientId, method: 'none' }, + refreshToken: family.refreshToken, + }) + ).resolves.toMatchObject({ success: false, error: 'invalid_grant' }) + const families = await db + .select({ + id: schema.oauthTokenFamily.id, + generation: schema.oauthTokenFamily.currentGeneration, + }) + .from(schema.oauthTokenFamily) + .where(eq(schema.oauthTokenFamily.clientId, clientId)) + expect(families).toEqual([{ id: family.id, generation: 0 }]) + + const principal = { kind: 'session' as const, userId, sessionId: generateId() } + const history = await runtime.listAuthorizedAppsUseCase.execute({ principal, input: {} }) + if (skipConsent) { + expect(history.apps).toHaveLength(0) + await expect( + runtime.revokeOAuthToken({ + credentials: { clientId, method: 'none' }, + token: family.refreshToken, + }) + ).resolves.toMatchObject({ success: true }) + } else { + expect(history.apps.map((app) => app.clientId)).toContain(clientId) + await runtime.revokeAuthorizedAppUseCase.execute({ principal, input: { clientId } }) + } + await expectNoTokens() + const after = await runtime.listAuthorizedAppsUseCase.execute({ principal, input: {} }) + expect(after.apps).toHaveLength(0) + } + ) + + it.each(['policy restriction', 'enterprise activation'] as const)( + 'serializes refresh against a pending %s without consuming the token', + async (change) => { + const { + db, + schema, + eq, + sql, + statement, + acquirePermissionGroupOrgLock, + acquireOrganizationMutationLock, + } = runtime + const groupId = await createDefaultGroup() + await grantConsent() + const family = await issueFamily() + if (change === 'enterprise activation') { + await db + .update(schema.subscription) + .set({ + plan: 'team', + metadata: { plan: 'team', referenceId: organizationId!, seats: 5, monthlyPrice: 100 }, + }) + .where(eq(schema.subscription.referenceId, organizationId!)) + await db + .update(schema.permissionGroup) + .set({ config: { disableOAuthAppAccess: true } }) + .where(eq(schema.permissionGroup.id, groupId)) + } + expect(await runtime.isCapabilityWithheldForUser(userId, 'oauth_apps.use')).toBe(false) + const credentials = { clientId, method: 'none' as const } + const writerReady = Promise.withResolvers() + const releaseWriter = Promise.withResolvers() + const writer = db.transaction(async (tx) => { + if (change === 'enterprise activation') { + await acquireOrganizationMutationLock(tx, organizationId!) + await tx + .update(schema.subscription) + .set({ + plan: 'enterprise', + metadata: { + plan: 'enterprise', + referenceId: organizationId!, + seats: 5, + monthlyPrice: 100, + }, + }) + .where(eq(schema.subscription.referenceId, organizationId!)) + } else { + await acquirePermissionGroupOrgLock(tx, organizationId!) + await tx + .update(schema.permissionGroup) + .set({ config: { disableOAuthAppAccess: true } }) + .where(eq(schema.permissionGroup.id, groupId)) + } + const [connection] = await tx.execute<{ pid: number }>( + statement`select pg_backend_pid() as pid` + ) + writerReady.resolve(connection.pid) + await releaseWriter.promise + }) + let refresh: ReturnType | undefined + + try { + const writerPid = await Promise.race([ + writerReady.promise, + writer.then(() => { + throw new Error('Policy writer finished before the concurrency check') + }), + ]) + let refreshSettled = false + refresh = runtime.rotateOAuthRefreshToken({ + credentials, + refreshToken: family.refreshToken, + }) + void refresh.then( + () => { + refreshSettled = true + }, + () => { + refreshSettled = true + } + ) + + let waitingForPolicy = false + const deadline = Date.now() + 2_000 + while (!refreshSettled && Date.now() < deadline) { + const [waiter] = await sql<{ waiting: boolean }[]>` + SELECT EXISTS ( + SELECT 1 FROM pg_stat_activity + WHERE ${writerPid} = ANY(pg_blocking_pids(pid)) + AND wait_event_type = 'Lock' + AND wait_event = 'advisory' + ) AS waiting + ` + if (waiter.waiting) { + waitingForPolicy = true + break + } + await sleep(1) + } + expect(waitingForPolicy, 'Refresh must wait for the organization policy writer').toBe(true) + expect(refreshSettled).toBe(false) + releaseWriter.resolve() + await writer + expect(await runtime.isCapabilityWithheldForUser(userId, 'oauth_apps.use')).toBe(true) + await expect(refresh).resolves.toMatchObject({ success: false, error: 'invalid_grant' }) + expect( + await db + .select({ generation: schema.oauthTokenFamily.currentGeneration }) + .from(schema.oauthTokenFamily) + .where(eq(schema.oauthTokenFamily.id, family.id)) + ).toEqual([{ generation: 0 }]) + expect( + await db + .select({ + generation: schema.oauthRefreshToken.generation, + revoked: schema.oauthRefreshToken.revoked, + }) + .from(schema.oauthRefreshToken) + .where(eq(schema.oauthRefreshToken.familyId, family.id)) + ).toEqual([{ generation: 0, revoked: null }]) + expect( + await db + .select({ id: schema.oauthAccessToken.id }) + .from(schema.oauthAccessToken) + .where(eq(schema.oauthAccessToken.clientId, clientId)) + ).toHaveLength(1) + + await db.transaction(async (tx) => { + await acquirePermissionGroupOrgLock(tx, organizationId!) + await tx + .update(schema.permissionGroup) + .set({ config: {} }) + .where(eq(schema.permissionGroup.id, groupId)) + }) + await expect( + runtime.rotateOAuthRefreshToken({ credentials, refreshToken: family.refreshToken }) + ).resolves.toMatchObject({ success: true }) + expect( + await db + .select({ generation: schema.oauthTokenFamily.currentGeneration }) + .from(schema.oauthTokenFamily) + .where(eq(schema.oauthTokenFamily.id, family.id)) + ).toEqual([{ generation: 1 }]) + } finally { + releaseWriter.resolve() + await Promise.allSettled([writer, ...(refresh ? [refresh] : [])]) + } + }, + 10_000 + ) + it('atomically converges concurrent consent submissions on one grant', async () => { const grants = await Promise.all([grantConsent(), grantConsent()]) expect(grants[0].id).toBe(grants[1].id) diff --git a/apps/sim/lib/auth/oauth-provider-metadata.test.ts b/apps/sim/lib/auth/oauth-provider-metadata.test.ts index 509a8dd440d..bc5ca5eb3a3 100644 --- a/apps/sim/lib/auth/oauth-provider-metadata.test.ts +++ b/apps/sim/lib/auth/oauth-provider-metadata.test.ts @@ -1,16 +1,12 @@ /** * @vitest-environment node */ +import { resetEnvFlagsMock, setEnvFlags } from '@sim/testing' import { NextRequest } from 'next/server' -import { beforeEach, describe, expect, it, vi } from 'vitest' +import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest' const mocks = vi.hoisted(() => ({ getOAuthServerConfig: vi.fn(), - isOAuthProviderEnabled: vi.fn(), -})) - -vi.mock('@/lib/auth/oauth-provider-feature', () => ({ - isOAuthProviderEnabled: mocks.isOAuthProviderEnabled, })) vi.mock('@/lib/auth/auth', () => ({ @@ -41,10 +37,12 @@ async function callRoute( return route(new NextRequest(`https://sim.test${path}`), { params: undefined }) } +afterAll(resetEnvFlagsMock) + describe('OAuth provider metadata', () => { beforeEach(() => { vi.clearAllMocks() - mocks.isOAuthProviderEnabled.mockResolvedValue(true) + setEnvFlags({ isAuthDisabled: false }) mocks.getOAuthServerConfig.mockResolvedValue({ issuer: 'https://sim.test/api/auth', authorization_endpoint: 'https://sim.test/api/auth/oauth2/authorize', @@ -84,9 +82,9 @@ describe('OAuth provider metadata', () => { }) it.each(routes)( - 'returns 404 from the %s alias when the provider is disabled', + 'returns 404 from the %s alias when authentication is disabled', async (_name, route, path) => { - mocks.isOAuthProviderEnabled.mockResolvedValue(false) + setEnvFlags({ isAuthDisabled: true }) const response = await callRoute(route, path) @@ -96,21 +94,4 @@ describe('OAuth provider metadata', () => { expect(mocks.getOAuthServerConfig).not.toHaveBeenCalled() } ) - - it.each(routes)( - 'rechecks the runtime flag for each request to the %s alias', - async (_name, route, path) => { - mocks.isOAuthProviderEnabled - .mockResolvedValueOnce(true) - .mockResolvedValueOnce(false) - .mockResolvedValueOnce(true) - - expect((await callRoute(route, path)).status).toBe(200) - expect((await callRoute(route, path)).status).toBe(404) - expect((await callRoute(route, path)).status).toBe(200) - expect(mocks.isOAuthProviderEnabled).toHaveBeenCalledTimes(3) - expect(mocks.isOAuthProviderEnabled).toHaveBeenCalledWith() - expect(mocks.getOAuthServerConfig).toHaveBeenCalledTimes(2) - } - ) }) diff --git a/apps/sim/lib/auth/oauth-provider-metadata.ts b/apps/sim/lib/auth/oauth-provider-metadata.ts index 949004642fd..851d1d54db5 100644 --- a/apps/sim/lib/auth/oauth-provider-metadata.ts +++ b/apps/sim/lib/auth/oauth-provider-metadata.ts @@ -1,6 +1,6 @@ import { NextResponse } from 'next/server' import { auth } from '@/lib/auth/auth' -import { isOAuthProviderEnabled } from '@/lib/auth/oauth-provider-feature' +import { isAuthDisabled } from '@/lib/core/config/env-flags' const DISCOVERY_CACHE_SECONDS = 300 @@ -41,7 +41,7 @@ export async function getOAuthProviderMetadata() { /** One response contract for every RFC 8414 discovery alias Sim exposes. */ export async function getOAuthProviderMetadataResponse(): Promise { - if (!(await isOAuthProviderEnabled())) { + if (isAuthDisabled) { return NextResponse.json( { error: 'OAuth provider is not enabled' }, { status: 404, headers: { ...DISCOVERY_HEADERS, 'Cache-Control': 'no-store' } } diff --git a/apps/sim/lib/auth/oauth-token-family.ts b/apps/sim/lib/auth/oauth-token-family.ts index 5d753a32977..9e4018778d7 100644 --- a/apps/sim/lib/auth/oauth-token-family.ts +++ b/apps/sim/lib/auth/oauth-token-family.ts @@ -22,7 +22,15 @@ import { OAUTH_REFRESH_TOKEN_PREFIX, OAUTH_TOKEN_FAMILY_MAX_GENERATION, } from '@/lib/auth/oauth-provider' +import { + acquireOrganizationUserMutationLocks, + getUserOrganization, +} from '@/lib/billing/organizations/membership' import { env } from '@/lib/core/config/env' +import { capabilityRefusal } from '@/lib/permission-groups/capabilities' +import { isEntitledOrganizationCapabilityWithheld } from '@/lib/permission-groups/capability-assertions' +import { acquirePermissionGroupOrgLock } from '@/lib/permission-groups/locks' +import { isOrganizationPermissionRegimeActive } from '@/lib/permission-groups/resolve.server' const logger = createLogger('OAuthTokenFamily') @@ -266,12 +274,28 @@ export async function rotateOAuthRefreshToken( return protocolError('invalid_grant', 'Refresh token is invalid.') } + const membership = await getUserOrganization(provisionalToken.userId, database) + const organizationId = membership?.organizationId ?? null const nextRefreshBody = generateSecureToken(32) const nextAccessBody = generateSecureToken(32) const nextRefreshId = generateId() const nextAccessId = generateId() return database.transaction(async (tx) => { + await acquireOrganizationUserMutationLocks(tx, { + userId: provisionalToken.userId, + organizationIds: organizationId ? [organizationId] : [], + }) + const currentMembership = await getUserOrganization(provisionalToken.userId, tx) + if ((currentMembership?.organizationId ?? null) !== organizationId) { + return protocolError( + 'invalid_grant', + 'Organization membership changed. Please sign in again.' + ) + } + const permissionRegimeActive = + organizationId !== null && (await isOrganizationPermissionRegimeActive(organizationId, tx)) + const [activeUser] = await tx .select({ id: user.id, @@ -383,6 +407,14 @@ export async function rotateOAuthRefreshToken( return protocolError('invalid_grant', 'Refresh token grant reached its rotation limit.') } + /** permission-group-enforced: oauth_apps.use — serialize the current policy with admin updates before consuming the token. */ + if (organizationId && permissionRegimeActive) { + await acquirePermissionGroupOrgLock(tx, organizationId, { lockTimeoutAlreadyBounded: true }) + if (await isEntitledOrganizationCapabilityWithheld(organizationId, 'oauth_apps.use', tx)) { + return protocolError('invalid_grant', capabilityRefusal('oauth_apps.use')) + } + } + const scopes = validateScopes(currentToken.scopes, lockedClient.scopes, input.requestedScopes) if (!scopes.success) return scopes diff --git a/apps/sim/lib/billing/application/authorized-billing-read-use-case.ts b/apps/sim/lib/billing/application/authorized-billing-read-use-case.ts index 40a00fd0ae2..8ca69fa2724 100644 --- a/apps/sim/lib/billing/application/authorized-billing-read-use-case.ts +++ b/apps/sim/lib/billing/application/authorized-billing-read-use-case.ts @@ -90,10 +90,16 @@ async function resolveBillingReadScope( throw new PersonalApiKeysDisabledError() } /** - * permission-group-enforced: cli.use — a CLI token reads the account's + * permission-group-enforced: cli.use, oauth_apps.use — an OAuth token reads the account's * plan, balance and usage here without naming a workspace, so the * workspace-scoped check in the funnel never sees it. */ + if ( + principal.kind === 'oauth_access_token' && + (await isCapabilityWithheldForUser(principal.userId, 'oauth_apps.use')) + ) { + refuseCapability('oauth_apps.use') + } if ( principal.kind === 'oauth_access_token' && principal.clientId === SIM_CLI_CLIENT_ID && diff --git a/apps/sim/lib/billing/application/billing-use-cases.test.ts b/apps/sim/lib/billing/application/billing-use-cases.test.ts index b299c078dc2..947df095156 100644 --- a/apps/sim/lib/billing/application/billing-use-cases.test.ts +++ b/apps/sim/lib/billing/application/billing-use-cases.test.ts @@ -109,6 +109,14 @@ const workspacePrincipal = { workspaceId: 'workspace-1', keyId: 'workspace-key-1', } +const oauthPrincipal = { + kind: 'oauth_access_token' as const, + userId: 'user-1', + clientId: 'partner-app', + tokenId: 'token-1', + scopes: ['api:read'], + expiresAt: new Date('2099-01-01T00:00:00Z'), +} describe('billing application use cases', () => { it('rejects an OAuth grant without API access before loading billing or workspace state', async () => { @@ -273,6 +281,55 @@ describe('billing application use cases', () => { ).resolves.toBeDefined() }) + it.each(['sim-cli', 'partner-app'])( + 'withholds account billing from the %s OAuth token through the default group', + async (clientId) => { + mocks.isCapabilityWithheldForUser.mockImplementation( + async (_userId: string, capability: string) => capability === 'oauth_apps.use' + ) + await expect( + getBillingStatus.execute({ principal: { ...oauthPrincipal, clientId }, input: {} }) + ).rejects.toMatchObject({ capability: 'oauth_apps.use' }) + expect(mocks.isCapabilityWithheldForUser).toHaveBeenCalledWith('user-1', 'oauth_apps.use') + expect(mocks.getSubscription).not.toHaveBeenCalled() + expect(mocks.getUsageLogs).not.toHaveBeenCalled() + } + ) + + it('rechecks an existing OAuth token against the billing workspace group', async () => { + await expect( + getBillingStatus.execute({ principal: oauthPrincipal, input: { workspaceId: 'workspace-1' } }) + ).resolves.toBeDefined() + permissionGroupScopeMockFns.mockResolvePermissionGroupConfig.mockResolvedValue({ + ...DEFAULT_PERMISSION_GROUP_CONFIG, + disableOAuthAppAccess: true, + }) + mocks.resolveAttribution.mockClear() + await expect( + getBillingStatus.execute({ principal: oauthPrincipal, input: { workspaceId: 'workspace-1' } }) + ).rejects.toMatchObject({ capability: 'oauth_apps.use' }) + expect(mocks.resolveAttribution).not.toHaveBeenCalled() + }) + + it('preserves API-key billing when only OAuth app access is withheld', async () => { + mocks.isCapabilityWithheldForUser.mockImplementation( + async (_userId: string, capability: string) => capability === 'oauth_apps.use' + ) + permissionGroupScopeMockFns.mockResolvePermissionGroupConfig.mockResolvedValue({ + ...DEFAULT_PERMISSION_GROUP_CONFIG, + disableOAuthAppAccess: true, + }) + await expect( + getBillingStatus.execute({ + principal: personalPrincipal, + input: { workspaceId: 'workspace-1' }, + }) + ).resolves.toBeDefined() + await expect( + getBillingStatus.execute({ principal: workspacePrincipal, input: {} }) + ).resolves.toBeDefined() + }) + it('never reads the payer storage pool it may not disclose', async () => { await getBillingStatus.execute({ principal: workspacePrincipal, input: {} }) await getBillingStatus.execute({ diff --git a/apps/sim/lib/core/application/operation.test.ts b/apps/sim/lib/core/application/operation.test.ts index ba609d93178..e7729059366 100644 --- a/apps/sim/lib/core/application/operation.test.ts +++ b/apps/sim/lib/core/application/operation.test.ts @@ -101,16 +101,17 @@ describe('assertOperationCapability', () => { ) }) - it('refuses the principal-wide capability the funnel applies to every operation', () => { - expect(() => - defineOperation({ - id: 'meta.principal_wide', - // @ts-expect-error personal_api_key.use is not an OperationDeclarableCapability - capability: 'personal_api_key.use', - principalKinds: ['session'], - }) - ).toThrow( - "Operation meta.principal_wide declares principal-wide capability personal_api_key.use; the authorization funnel's personal-key branch already applies it to every operation" - ) - }) + it.each(['personal_api_key.use', 'oauth_apps.use'] as const)( + 'refuses principal-wide capability %s as an operation capability', + (capability) => { + expect(() => + defineOperation({ + id: 'meta.principal_wide', + // @ts-expect-error principal-wide capabilities are not OperationDeclarableCapability + capability, + principalKinds: ['session'], + }) + ).toThrow(`Operation meta.principal_wide declares principal-wide capability ${capability}`) + } + ) }) diff --git a/apps/sim/lib/core/application/operation.ts b/apps/sim/lib/core/application/operation.ts index 22a819e90d8..1d16a183c6b 100644 --- a/apps/sim/lib/core/application/operation.ts +++ b/apps/sim/lib/core/application/operation.ts @@ -16,13 +16,15 @@ import { * authorizes in its own middleware — for every operation alike, ahead of and * independently of whatever module capability the operation names. * + * `oauth_apps.use` applies the same rule to OAuth credentials. + * * Excluded from {@link OperationDeclarableCapability} because an operation that * named it would be wrong either way: withheld, it would double-apply a refusal * the funnel has already made in the caller's own words; and a session caller * holding no API key at all would be refused an ordinary operation over a * setting about credentials they are not using. */ -export type PrincipalWideCapability = 'personal_api_key.use' +export type PrincipalWideCapability = 'personal_api_key.use' | 'oauth_apps.use' /** * The capabilities an operation may name — every static rule except the @@ -35,7 +37,10 @@ export type OperationDeclarableCapability = Exclude< > /** The runtime half of {@link PrincipalWideCapability}, for the builders' guard. */ -const PRINCIPAL_WIDE_CAPABILITIES: readonly PrincipalWideCapability[] = ['personal_api_key.use'] +const PRINCIPAL_WIDE_CAPABILITIES: readonly PrincipalWideCapability[] = [ + 'personal_api_key.use', + 'oauth_apps.use', +] export interface ApplicationOperation { readonly id: Id diff --git a/apps/sim/lib/core/application/workspace-authorization.test.ts b/apps/sim/lib/core/application/workspace-authorization.test.ts index 75efb107a13..86a65db1c48 100644 --- a/apps/sim/lib/core/application/workspace-authorization.test.ts +++ b/apps/sim/lib/core/application/workspace-authorization.test.ts @@ -741,6 +741,90 @@ describe('authorizeWorkspaceOperation OAuth access token policy', () => { ).resolves.toBeUndefined() }) + it.each(['sim-cli', 'partner-app'])( + 'rechecks OAuth app permission for an existing %s token', + async (clientId) => { + const existingToken = token({ clientId }) + await expect( + authorizeWorkspaceOperation(existingToken, readOperation, context) + ).resolves.toBeUndefined() + + resolveGroupConfigMock.mockResolvedValue({ + ...DEFAULT_PERMISSION_GROUP_CONFIG, + disableOAuthAppAccess: true, + }) + await expect( + authorizeWorkspaceOperation(existingToken, readOperation, context) + ).rejects.toMatchObject({ + capability: 'oauth_apps.use', + detailCode: 'PERMISSION_GROUP_CAPABILITY_BLOCKED', + }) + } + ) + + it('restricts the OAuth token only in the workspace whose group withholds apps', async () => { + resolveGroupConfigMock.mockImplementation(async (_userId: string, workspaceId: string) => ({ + ...DEFAULT_PERMISSION_GROUP_CONFIG, + disableOAuthAppAccess: workspaceId === context.workspaceId, + })) + const existingToken = token({ clientId: 'partner-app' }) + await expect( + authorizeWorkspaceOperation(existingToken, readOperation, context) + ).rejects.toMatchObject({ capability: 'oauth_apps.use' }) + await expect( + authorizeWorkspaceOperation(existingToken, readOperation, { + ...context, + workspaceId: 'workspace-allowed', + }) + ).resolves.toBeUndefined() + }) + + it('keeps session and API-key callers independent of the OAuth app restriction', async () => { + resolveGroupConfigMock.mockResolvedValue({ + ...DEFAULT_PERMISSION_GROUP_CONFIG, + disableOAuthAppAccess: true, + }) + await expect( + authorizeWorkspaceOperation(principal, readOperation, context) + ).resolves.toBeUndefined() + await expect( + authorizeWorkspaceOperation(personalKeyPrincipal, readOperation, context) + ).resolves.toBeUndefined() + await expect( + authorizeWorkspaceOperation( + { ...workspaceKeyPrincipal, workspaceId: context.workspaceId }, + workspaceKeyOperation, + context + ) + ).resolves.toBeUndefined() + }) + + it('does not apply an organization OAuth restriction to a personal workspace', async () => { + resolveGroupConfigMock.mockResolvedValue({ + ...DEFAULT_PERMISSION_GROUP_CONFIG, + disableOAuthAppAccess: true, + }) + await expect( + authorizeWorkspaceOperation(token(), readOperation, { + ...context, + workspaceOrganizationId: null, + }) + ).resolves.toBeUndefined() + expect(resolveGroupConfigMock).not.toHaveBeenCalled() + }) + + it('conceals missing membership before revealing an OAuth app restriction', async () => { + mocks.resolvePermission.mockResolvedValue(null) + resolveGroupConfigMock.mockResolvedValue({ + ...DEFAULT_PERMISSION_GROUP_CONFIG, + disableOAuthAppAccess: true, + }) + await expect( + authorizeWorkspaceOperation(token(), readOperation, context) + ).rejects.toBeInstanceOf(NoWorkspaceAccessError) + expect(resolveGroupConfigMock).not.toHaveBeenCalled() + }) + it('refuses a lapsed token as unauthorized rather than forbidden', async () => { const failure = await authorizeWorkspaceOperation( token({ expiresAt: new Date('2000-01-01T00:00:00.000Z') }), diff --git a/apps/sim/lib/core/application/workspace-authorization.ts b/apps/sim/lib/core/application/workspace-authorization.ts index d8cae5aeb7d..8a4f90b46dc 100644 --- a/apps/sim/lib/core/application/workspace-authorization.ts +++ b/apps/sim/lib/core/application/workspace-authorization.ts @@ -253,24 +253,8 @@ export async function requireCliAccessAllowed( } /** - * Refuses a personal API key the caller's permission group withholds. - * - * Separate from {@link requireCapability} because it is not a property of the - * operation: no operation opts into it, and every operation a personal key can - * reach is subject to it. - * - * Exported for the one authorization path that does not run through - * {@link authorizeWorkspaceOperation} — the billing reads, which resolve their - * own workspace scope. One copy, or the same key the funnel refuses keeps - * working somewhere. - */ -/** - * Both capability gates a user-held credential passes, in one call. - * - * The funnel runs these as part of its sequence, but three surfaces authorize - * themselves — billing reads, audit-log reads, and `/api/v2/meta` — and each - * has to repeat them. Repeating two separate calls is how one of them ends up - * with only the first: `cli.use` was missing from all three until this existed. + * Enforces credential-wide restrictions after current workspace membership is established. + * Shared by the authorization funnel and workspace billing/chat reads. */ export async function requireUserCredentialCapabilities( principal: PersonalApiKeyPrincipal | OAuthAccessTokenPrincipal, @@ -278,6 +262,15 @@ export async function requireUserCredentialCapabilities( ): Promise { await requirePersonalApiKeysAllowed(principal.userId, context) if (principal.kind === 'oauth_access_token') { + /** permission-group-enforced: oauth_apps.use — applies to every OAuth principal after the role check. */ + if (context.workspaceOrganizationId !== null) { + await assertWorkspaceCapability( + principal.userId, + context.workspaceId, + 'oauth_apps.use', + context.workspaceOrganizationId + ) + } await requireCliAccessAllowed(principal.clientId, principal.userId, context) } } diff --git a/apps/sim/lib/core/config/env.ts b/apps/sim/lib/core/config/env.ts index 999ae263b08..2d8acf55680 100644 --- a/apps/sim/lib/core/config/env.ts +++ b/apps/sim/lib/core/config/env.ts @@ -621,8 +621,6 @@ export const env = createEnv({ /** Comma-separated proxy IPs/CIDRs skipped while resolving the forwarded client chain. */ AUTH_TRUSTED_PROXIES: z.string().optional(), - /** Fallback for the global oauth-provider feature flag when AppConfig has no document. */ - OAUTH_PROVIDER_ENABLED: z.boolean().optional(), // SSO Configuration (for script-based registration) SSO_ENABLED: z.boolean().optional(), // Enable SSO functionality diff --git a/apps/sim/lib/core/config/feature-flags.test.ts b/apps/sim/lib/core/config/feature-flags.test.ts index 4aa4597245e..2d28ffa4cf4 100644 --- a/apps/sim/lib/core/config/feature-flags.test.ts +++ b/apps/sim/lib/core/config/feature-flags.test.ts @@ -15,7 +15,6 @@ const { mockFetch, mockIsPlatformAdmin, envRef } = vi.hoisted(() => ({ TABLE_ROW_TTL: undefined as boolean | undefined, CREDENTIAL_GROUPS: undefined as boolean | undefined, KNOWLEDGE_MEMBER_ACCESS: undefined as boolean | undefined, - OAUTH_PROVIDER_ENABLED: undefined as boolean | undefined, }, })) @@ -126,34 +125,6 @@ describe('isFeatureEnabled', () => { setEnvFlags({ isAppConfigEnabled: false }) envRef.CREDENTIAL_GROUPS = undefined envRef.KNOWLEDGE_MEMBER_ACCESS = undefined - envRef.OAUTH_PROVIDER_ENABLED = undefined - }) - - describe('oauth-provider flag', () => { - it('uses the global fallback only when AppConfig has no document', async () => { - expect(await isFeatureEnabled('oauth-provider')).toBe(false) - envRef.OAUTH_PROVIDER_ENABLED = true - expect(await isFeatureEnabled('oauth-provider')).toBe(true) - - setEnvFlags({ isAppConfigEnabled: true }) - mockFetch.mockResolvedValue(null) - expect(await isFeatureEnabled('oauth-provider')).toBe(true) - }) - - it('reads runtime changes from AppConfig without targeting or admin lookups', async () => { - envRef.OAUTH_PROVIDER_ENABLED = true - withAppConfig({ 'oauth-provider': { enabled: false } }) - expect(await isFeatureEnabled('oauth-provider')).toBe(false) - - withAppConfig({ 'oauth-provider': { enabled: true } }) - expect(await isFeatureEnabled('oauth-provider')).toBe(true) - - withAppConfig({ 'oauth-provider': { enabled: false } }) - expect(await isFeatureEnabled('oauth-provider')).toBe(false) - withAppConfig({}) - expect(await isFeatureEnabled('oauth-provider')).toBe(false) - expect(mockIsPlatformAdmin).not.toHaveBeenCalled() - }) }) describe('knowledge-member-access flag', () => { diff --git a/apps/sim/lib/core/config/feature-flags.ts b/apps/sim/lib/core/config/feature-flags.ts index fb9ce87bb17..bf6de543ba9 100644 --- a/apps/sim/lib/core/config/feature-flags.ts +++ b/apps/sim/lib/core/config/feature-flags.ts @@ -46,12 +46,6 @@ interface FeatureFlagDefinition { /** The single registry of known flags. To add a flag, add one entry here. */ const FEATURE_FLAGS = { - 'oauth-provider': { - description: - 'Enable OAuth authorization, discovery, and API bearer tokens. Global on/off only; ' + - 'enable after all app instances support the OAuth token-family lifecycle.', - fallback: 'OAUTH_PROVIDER_ENABLED', - }, 'trigger-eu-region': { description: 'Route Trigger.dev runs to eu-central-1 instead of the default us-east-1. Global on/off ' + diff --git a/apps/sim/lib/permission-groups/capabilities.ts b/apps/sim/lib/permission-groups/capabilities.ts index f6060c6590c..b11187c6583 100644 --- a/apps/sim/lib/permission-groups/capabilities.ts +++ b/apps/sim/lib/permission-groups/capabilities.ts @@ -53,6 +53,7 @@ export const CAPABILITY_IDS = [ 'workspace.create', 'organization.member_directory', 'cli.use', + 'oauth_apps.use', 'triggers.webhook', 'copilot.tool_auto_approval', 'sandboxes.use', @@ -386,6 +387,13 @@ export const CAPABILITY_RULES = { describe: 'The organization member directory', deniedBy: (config) => config.hideOrgMemberDirectory, }, + 'oauth_apps.use': { + kind: 'static', + configKeys: ['disableOAuthAppAccess'], + detailCode: 'PERMISSION_GROUP_CAPABILITY_BLOCKED', + describe: 'OAuth app access', + deniedBy: (config) => config.disableOAuthAppAccess, + }, 'cli.use': { kind: 'static', configKeys: ['disableCliAccess'], diff --git a/apps/sim/lib/permission-groups/features.test.ts b/apps/sim/lib/permission-groups/features.test.ts index 45211533c16..3d208ad7aa6 100644 --- a/apps/sim/lib/permission-groups/features.test.ts +++ b/apps/sim/lib/permission-groups/features.test.ts @@ -127,10 +127,11 @@ describe('platform feature scope', () => { ]) }) - it('reads exactly five keys from both a workspace group and the default group', () => { + it('reads exactly six keys from both a workspace group and the default group', () => { expect(keysWithScope('workspace-or-organization')).toEqual([ 'disableCliAccess', 'disableInvitations', + 'disableOAuthAppAccess', 'disablePersonalApiKeys', 'hideApiKeysTab', 'hideIntegrationsTab', diff --git a/apps/sim/lib/permission-groups/fields.test.ts b/apps/sim/lib/permission-groups/fields.test.ts index 5b9ad4cf981..8470d27bacc 100644 --- a/apps/sim/lib/permission-groups/fields.test.ts +++ b/apps/sim/lib/permission-groups/fields.test.ts @@ -164,6 +164,7 @@ const fixtures: readonly CoercionFixture[] = [ disableWebhookTriggers: true, disableToolAutoApproval: true, hideSandboxesTab: true, + disableOAuthAppAccess: true, }, expected: { allowedIntegrations: ['slack_v2'], @@ -206,6 +207,7 @@ const fixtures: readonly CoercionFixture[] = [ disableWebhookTriggers: true, disableToolAutoApproval: true, hideSandboxesTab: true, + disableOAuthAppAccess: true, }, }, ] diff --git a/apps/sim/lib/permission-groups/fields.ts b/apps/sim/lib/permission-groups/fields.ts index aec07f5de45..c73a62a1e83 100644 --- a/apps/sim/lib/permission-groups/fields.ts +++ b/apps/sim/lib/permission-groups/fields.ts @@ -464,7 +464,7 @@ export const PERMISSION_GROUP_FIELDS = { id: 'disable-cli-access', label: 'CLI Access', category: 'Credentials & Access', - hint: "Prevent approving a CLI login, which mints a key for the public API. A login naming one of this group's workspaces is refused; an account-level login names none, so it is read from the organization's default group.", + hint: "Prevent approving a CLI login or using Sim CLI OAuth tokens for the public API. A login naming one of this group's workspaces is refused; an account-level login names none, so it is read from the organization's default group.", }), disableWebhookTriggers: booleanRestriction('capability', { scope: 'workspace', @@ -487,6 +487,13 @@ export const PERMISSION_GROUP_FIELDS = { category: 'Modules', hint: 'Revoke the Sandboxes module. Members cannot view, create, or change a workspace sandbox.', }), + disableOAuthAppAccess: booleanRestriction('capability', { + scope: 'workspace-or-organization', + id: 'disable-oauth-app-access', + label: 'OAuth App Access', + category: 'Credentials & Access', + hint: "Prevent OAuth apps from accessing this group's workspaces. The organization's default group also governs authorizing apps and refreshing their access.", + }), } satisfies Record export type PermissionGroupFields = typeof PERMISSION_GROUP_FIELDS diff --git a/apps/sim/lib/permission-groups/locks.ts b/apps/sim/lib/permission-groups/locks.ts index a5016506d63..c1cb101f9f5 100644 --- a/apps/sim/lib/permission-groups/locks.ts +++ b/apps/sim/lib/permission-groups/locks.ts @@ -17,9 +17,9 @@ const PERMISSION_GROUP_LOCK_TIMEOUT_MS = 5_000 * correct than fine-grained per-user/per-group locks with acquire-ordering. * * Readers take it too, when the value they read decides whether a write in the - * same transaction may commit — workspace creation re-reads the default group's - * `workspace.create` capability under this lock, which is the only thing that - * makes the check-to-insert window closed rather than merely narrow. + * same transaction may commit. Workspace creation and OAuth refresh re-read + * the default group's capability under this lock and hold it through their + * protected writes, closing the check-to-write window. * * `pg_advisory_xact_lock` auto-releases at transaction end (safe on pooled * connections), and `lock_timeout` bounds the wait (raising SQLSTATE 55P03) @@ -35,10 +35,8 @@ const PERMISSION_GROUP_LOCK_TIMEOUT_MS = 5_000 * evaluation order is unspecified, so the bound might not be in force when the * lock is requested. * - * LOCK ORDER: this is a LEAF lock. Every transaction that holds it — the five - * `organizations/[id]/permission-groups` route transactions, and the workspace - * creation transaction — acquires no further advisory lock afterwards. That is - * what makes it safe for workspace creation to take it *last*, after + * LOCK ORDER: this is a LEAF lock. Transactions that hold it acquire no further + * advisory lock afterwards. Workspace creation and OAuth refresh take it last, after * `organization-mutation`, `user-billing-identity`, and the membership lock: a * deadlock needs a holder of this lock to wait on one of those, and no such * holder exists. Keep it a leaf. diff --git a/apps/sim/lib/permission-groups/resolve.server.test.ts b/apps/sim/lib/permission-groups/resolve.server.test.ts index 12e4176cb14..786bd552044 100644 --- a/apps/sim/lib/permission-groups/resolve.server.test.ts +++ b/apps/sim/lib/permission-groups/resolve.server.test.ts @@ -3,6 +3,7 @@ */ import { resetEnvFlagsMock, setEnvFlags } from '@sim/testing' import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest' +import type { DbOrTx } from '@/lib/db/types' const { mockIsOrganizationOnEnterprisePlan, mockGetWorkspaceWithOwner } = vi.hoisted(() => ({ mockIsOrganizationOnEnterprisePlan: vi.fn(), @@ -20,6 +21,7 @@ vi.mock('@/lib/workspaces/permissions/utils', () => ({ import { getUserPermissionConfig, getUserPermissionConfigForOrganization, + isOrganizationPermissionRegimeActive, resolveVerifiedUserAccessControlContext, } from '@/lib/permission-groups/resolve.server' @@ -101,4 +103,33 @@ describe('permission-group resolution under a failed entitlement read', () => { }) await expect(getUserPermissionConfigForOrganization(ORGANIZATION_ID)).resolves.toBeNull() }) + + it('rechecks entitlement on the caller transaction after an unentitled preflight', async () => { + const executor = {} as DbOrTx + mockIsOrganizationOnEnterprisePlan.mockResolvedValueOnce(false).mockResolvedValueOnce(true) + + await expect(isOrganizationPermissionRegimeActive(ORGANIZATION_ID)).resolves.toBe(false) + await expect(isOrganizationPermissionRegimeActive(ORGANIZATION_ID, executor)).resolves.toBe( + true + ) + expect(mockIsOrganizationOnEnterprisePlan).toHaveBeenLastCalledWith( + ORGANIZATION_ID, + 'throw', + executor + ) + }) + + it('propagates a transaction entitlement read failure instead of disabling restrictions', async () => { + const executor = {} as DbOrTx + entitlementReadFails() + + await expect(isOrganizationPermissionRegimeActive(ORGANIZATION_ID, executor)).rejects.toThrow( + 'billing database unavailable' + ) + expect(mockIsOrganizationOnEnterprisePlan).toHaveBeenCalledWith( + ORGANIZATION_ID, + 'throw', + executor + ) + }) }) diff --git a/apps/sim/lib/permission-groups/resolve.server.ts b/apps/sim/lib/permission-groups/resolve.server.ts index f0511724f07..c3844bf1c77 100644 --- a/apps/sim/lib/permission-groups/resolve.server.ts +++ b/apps/sim/lib/permission-groups/resolve.server.ts @@ -310,24 +310,22 @@ export async function getUserPermissionConfigForOrganization( * enables Access Control, and the organization holds the Enterprise entitlement * that turns the regime on. * - * Split out of {@link getUserPermissionConfigForOrganization} so a caller that - * must re-read the *group* under `acquirePermissionGroupOrgLock` can settle this - * half BEFORE opening its transaction. The entitlement read cannot move into a - * transaction: {@link isOrganizationOnEnterprisePlan} is `cache()`d on its - * argument list, so it admits no executor, and giving it one would both miss the - * memo on every call and — because an unentitled organization resolves to - * `config: null`, meaning every capability ALLOWED — turn a read failure into a - * fail-open. The lock never serialized this half either way: it guards - * permission-group writes, not subscription changes. + * Callers that serialize entitlement changes with an organization mutation + * lock must pass their transaction after acquiring that lock. The executor is + * part of the entitlement cache key, so this read cannot reuse a preflight + * result. A permission-group lock alone only serializes group writes. * * `'throw'` for the same reason as in * {@link resolveUserAccessControlContextForOrganization}. */ export async function isOrganizationPermissionRegimeActive( - organizationId: string + organizationId: string, + executor?: DbOrTx ): Promise { if (!isHosted && !isAccessControlEnabled) return false - return isOrganizationOnEnterprisePlan(organizationId, 'throw') + return executor + ? isOrganizationOnEnterprisePlan(organizationId, 'throw', executor) + : isOrganizationOnEnterprisePlan(organizationId, 'throw') } /** diff --git a/apps/sim/lib/workspaces/application/list-public-workspaces.test.ts b/apps/sim/lib/workspaces/application/list-public-workspaces.test.ts index b49cd6ea3b7..3bf396ae7df 100644 --- a/apps/sim/lib/workspaces/application/list-public-workspaces.test.ts +++ b/apps/sim/lib/workspaces/application/list-public-workspaces.test.ts @@ -210,6 +210,47 @@ describe('listPublicWorkspaces', () => { ).rejects.toThrow('Accessible workspace workspace-a disappeared during listing') }) + it.each(['sim-cli', 'partner-app'])( + 'filters restricted OAuth workspaces before paginating for %s', + async (clientId) => { + mocks.listAccessible.mockResolvedValue( + ['workspace-a', 'workspace-b'].map((id, index) => ({ + workspace: workspace(id, id, true, index + 1), + permissionType: 'read', + viaOrgAdmin: false, + })) + ) + permissionGroupScopeMockFns.mockResolvePermissionGroupConfig.mockImplementation( + async (_userId: string, workspaceId: string) => ({ + ...DEFAULT_PERMISSION_GROUP_CONFIG, + disableOAuthAppAccess: workspaceId === 'workspace-a', + }) + ) + const input = { sortBy: 'name', sortOrder: 'asc', limit: 1, offset: 0 } as const + const result = await listPublicWorkspaces.execute({ + principal: { + kind: 'oauth_access_token', + userId: 'user-1', + clientId, + tokenId: 'token-1', + scopes: ['api:read'], + expiresAt: new Date('2099-01-01T00:00:00Z'), + }, + input, + }) + expect(result.workspaces.map(({ id }) => id)).toEqual(['workspace-b']) + expect(result.hasMore).toBe(false) + expect(mocks.getDetails).toHaveBeenCalledWith(['workspace-b']) + + const apiKeyResult = await listPublicWorkspaces.execute({ + principal: { kind: 'personal_api_key', userId: 'user-1', keyId: 'key-1' }, + input, + }) + expect(apiKeyResult.workspaces.map(({ id }) => id)).toEqual(['workspace-a']) + expect(apiKeyResult.hasMore).toBe(true) + } + ) + it('limits a workspace key to its bound active workspace', async () => { mocks.loadContext.mockResolvedValue({ workspaceId: 'workspace-bound', diff --git a/apps/sim/lib/workspaces/application/list-public-workspaces.ts b/apps/sim/lib/workspaces/application/list-public-workspaces.ts index b8ecd7e2962..23635bf0256 100644 --- a/apps/sim/lib/workspaces/application/list-public-workspaces.ts +++ b/apps/sim/lib/workspaces/application/list-public-workspaces.ts @@ -93,6 +93,19 @@ export const listPublicWorkspaces: OperationUseCase< ) if (personalCredentialsWithheld) return null + /** permission-group-enforced: oauth_apps.use — omit restricted workspaces before pagination. */ + if ( + principal.kind === 'oauth_access_token' && + (await isWorkspaceCapabilityWithheld( + principal.userId, + workspace.id, + 'oauth_apps.use', + workspace.organizationId + )) + ) { + return null + } + if (principal.kind !== 'oauth_access_token' || principal.clientId !== SIM_CLI_CLIENT_ID) { return workspace } diff --git a/apps/sim/lib/workspaces/policy.ts b/apps/sim/lib/workspaces/policy.ts index 520f0e55e77..3b511b15945 100644 --- a/apps/sim/lib/workspaces/policy.ts +++ b/apps/sim/lib/workspaces/policy.ts @@ -144,19 +144,10 @@ export class WorkspaceCreationCapabilityWithheldError extends WorkspaceCreationC * refuses to commit unless live membership still equals that value, so a verdict * reached here can never be applied to a different organization. * - * Only the entitlement half of the decision is answered here, because it cannot - * be answered anywhere else — see {@link isOrganizationPermissionRegimeActive} - * for why that read admits no executor. Running it on the transaction executor - * would check out a second pooled connection while three advisory locks are - * held — what `packages/db/tx-tripwire.ts` fires on. - * - * Nothing is lost by settling it early. `permission_group:` serializes - * permission-group writes, not subscription changes, so holding it across this - * read never excluded anything. A concurrent entitlement LAPSE resolves to - * applying the group's config for one more request, which refuses rather than - * permits; a concurrent GRANT resolves to skipping the group for one more - * request, which is the same answer the route's own preflight gave microseconds - * earlier. + * This path settles entitlement during preflight. A concurrent entitlement + * lapse can keep the group's restrictions for this request; a concurrent grant + * can leave them inactive until the next request. The permission-group lock + * alone does not serialize subscription changes. * * The `forUpdate` subscription re-read below accepts Team *or* Enterprise; the * permission-group regime is Enterprise-only, so it cannot stand in for this. diff --git a/apps/sim/proxy.ts b/apps/sim/proxy.ts index 4ff5d0b4307..f8a833ed42d 100644 --- a/apps/sim/proxy.ts +++ b/apps/sim/proxy.ts @@ -1,7 +1,6 @@ import { createLogger } from '@sim/logger' import { getSessionCookie } from 'better-auth/cookies' import { type NextRequest, NextResponse } from 'next/server' -import { isOAuthProviderEnabled } from '@/lib/auth/oauth-provider-feature' import { isOAuthAuthorizationCallback, resolveAuthRedirect } from '@/app/(auth)/auth-redirect' import { getEnv } from './lib/core/config/env' import { isAuthDisabled, isDev, isHosted } from './lib/core/config/env-flags' @@ -309,7 +308,7 @@ function handleSecurityFiltering(request: NextRequest): NextResponse | null { return null } -export async function proxy(request: NextRequest) { +export function proxy(request: NextRequest) { const url = request.nextUrl if (url.pathname.startsWith('/api/')) { @@ -335,7 +334,7 @@ export async function proxy(request: NextRequest) { inviteFlow: url.searchParams.get('invite_flow'), }) const isOAuthSignIn = - isOAuthAuthorizationCallback(rawCallbackUrl, url.origin) && (await isOAuthProviderEnabled()) + isOAuthAuthorizationCallback(rawCallbackUrl, url.origin) && !isAuthDisabled if (hasActiveSession && !isOAuthSignIn) { return applyIndexingPolicy(request, NextResponse.redirect(new URL('/workspace', request.url))) }