diff --git a/.changeset/quiet-tokens-wait.md b/.changeset/quiet-tokens-wait.md new file mode 100644 index 000000000..1abfed6a0 --- /dev/null +++ b/.changeset/quiet-tokens-wait.md @@ -0,0 +1,12 @@ +--- +'@wbce-d9/api': minor +--- + +Added the `AUTH__REQUIRE_REFRESH_TOKEN` option for the OpenID and OAuth2 drivers (default `true`, which keeps +the current behavior). Setting it to `false` lets users log in and renew their session when the identity provider issued +no refresh token (for example Google when the account has already granted consent), instead of rejecting them with +`INVALID_CREDENTIALS` / "No [OpenID] Session found". A stored refresh token is still checked against the provider. + +With the default, a login without a refresh token now redirects once to the provider with `prompt=consent` (as already +done for a revoked token) instead of failing right away, and a stored refresh token is no longer overwritten with `null` +when the provider does not issue a new one on login. diff --git a/api/src/auth/drivers/baseoauth.test.ts b/api/src/auth/drivers/baseoauth.test.ts new file mode 100644 index 000000000..8ccc16ff8 --- /dev/null +++ b/api/src/auth/drivers/baseoauth.test.ts @@ -0,0 +1,224 @@ +import * as oidc from 'openid-client'; +import type { Knex } from 'knex'; +import { beforeEach, describe, expect, test, vi } from 'vitest'; +import type { SchemaOverview } from '@wbce-d9/types'; +import { InvalidTokenException } from '../../exceptions/index.js'; +import logger from '../../logger.js'; +import type { User } from '../../types/index.js'; +import { OpenIDAuthDriver } from './openid.js'; + +vi.mock('../../env', () => { + const MOCK_ENV = { + SECRET: 'test', + PUBLIC_URL: 'https://app.example', + EXTENSIONS_PATH: './extensions', + EMAIL_TEMPLATES_PATH: './templates', + OAUTH2_TOKEN_TTL: 604800000, + }; + + return { + default: MOCK_ENV, + getEnv: () => MOCK_ENV, + }; +}); + +const { updateOne } = vi.hoisted(() => ({ updateOne: vi.fn() })); + +vi.mock('../../database/index.js', () => ({ + default: vi.fn(), +})); + +// The filter hooks return the payload unchanged +vi.mock('../../emitter.js', () => ({ + default: { emitFilter: vi.fn(async (_event: string, payload: any) => payload) }, +})); + +vi.mock('../../services/users.js', () => ({ + UsersService: class { + updateOne = updateOne; + }, +})); + +vi.mock('../../auth.js', () => ({ + getAuthProvider: vi.fn(), +})); + +vi.mock('../../logger.js', () => ({ + default: { error: vi.fn(), warn: vi.fn(), info: vi.fn(), debug: vi.fn(), trace: vi.fn() }, +})); + +// The constructor kicks off provider discovery; stub it so no network call happens. +vi.mock('openid-client', () => { + class ResponseBodyError extends Error { + error: string; + error_description?: string; + + constructor(error: string) { + super(error); + this.error = error; + } + } + + class ClientError extends Error {} + + return { + discovery: vi.fn(async () => ({ + serverMetadata: () => ({ response_types_supported: ['code'] }), + })), + refreshTokenGrant: vi.fn(), + ResponseBodyError, + ClientError, + }; +}); + +function createDriver(config: Record = {}) { + const knex = {} as Knex; + const schema = {} as SchemaOverview; + + return new OpenIDAuthDriver( + { knex, schema }, + { + issuerUrl: 'https://idp.example', + clientId: 'd9-client-id', + clientSecret: 'd9-client-secret', + provider: 'myidp', + ...config, + } + ); +} + +function createUser(authData: User['auth_data']): User { + return { id: 'user-1', provider: 'myidp', auth_data: authData } as User; +} + +const WITH_TOKEN = JSON.stringify({ refreshToken: 'stored-refresh-token' }); + +beforeEach(() => { + vi.clearAllMocks(); +}); + +describe('BaseOAuthDriver.refresh / login without a stored refresh token', () => { + test('rejects by default (option unset)', async () => { + const driver = createDriver(); + + await expect(driver.refresh(createUser(null))).rejects.toBeInstanceOf(InvalidTokenException); + await expect(driver.login(createUser(null))).rejects.toBeInstanceOf(InvalidTokenException); + expect(logger.error).toHaveBeenCalledWith('No [OpenID] Session found'); + }); + + test('rejects when requireRefreshToken is true', async () => { + const driver = createDriver({ requireRefreshToken: true }); + + await expect(driver.refresh(createUser(null))).rejects.toBeInstanceOf(InvalidTokenException); + }); + + test('resolves without contacting the provider when requireRefreshToken is false', async () => { + const driver = createDriver({ requireRefreshToken: false }); + + await expect(driver.refresh(createUser(null))).resolves.toBeUndefined(); + await expect(driver.login(createUser(null))).resolves.toBeUndefined(); + + expect(oidc.refreshTokenGrant).not.toHaveBeenCalled(); + expect(updateOne).not.toHaveBeenCalled(); + expect(logger.error).not.toHaveBeenCalled(); + }); + + test('also treats an auth_data object without refreshToken as "no token" when requireRefreshToken is false', async () => { + const driver = createDriver({ requireRefreshToken: false }); + + await expect(driver.refresh(createUser(JSON.stringify({})))).resolves.toBeUndefined(); + }); +}); + +describe('BaseOAuthDriver.refresh with a stored refresh token (requireRefreshToken: false)', () => { + test('exchanges the token at the provider and persists a rotated token', async () => { + vi.mocked(oidc.refreshTokenGrant).mockResolvedValue({ refresh_token: 'rotated-token' } as any); + + const driver = createDriver({ requireRefreshToken: false }); + + await expect(driver.refresh(createUser(WITH_TOKEN))).resolves.toBeUndefined(); + + expect(oidc.refreshTokenGrant).toHaveBeenCalledWith(expect.anything(), 'stored-refresh-token'); + + expect(updateOne).toHaveBeenCalledWith('user-1', { + auth_data: JSON.stringify({ refreshToken: 'rotated-token' }), + }); + }); + + test('still rejects when the provider reports invalid_grant (revoked)', async () => { + vi.mocked(oidc.refreshTokenGrant).mockRejectedValue(new (oidc.ResponseBodyError as any)('invalid_grant')); + + const driver = createDriver({ requireRefreshToken: false }); + + await expect(driver.refresh(createUser(WITH_TOKEN))).rejects.toBeInstanceOf(InvalidTokenException); + }); +}); + +describe('BaseOAuthDriver.refresh with a stored refresh token (default)', () => { + test('behaves as before: exchanges the token', async () => { + vi.mocked(oidc.refreshTokenGrant).mockResolvedValue({} as any); + + const driver = createDriver(); + + await expect(driver.refresh(createUser(WITH_TOKEN))).resolves.toBeUndefined(); + + expect(oidc.refreshTokenGrant).toHaveBeenCalledWith(expect.anything(), 'stored-refresh-token'); + expect(updateOne).not.toHaveBeenCalled(); + }); +}); + +describe('BaseOAuthDriver.getUserID for an existing user', () => { + const CALLBACK_PAYLOAD = { code: 'code', codeVerifier: 'verifier', state: 'state' }; + + function mockProviderResponse(driver: OpenIDAuthDriver, authData: string | undefined) { + vi.spyOn(driver, 'fetchUserId').mockResolvedValue('user-1'); + + vi.spyOn(driver, 'getTokenSetAndUserInfo').mockResolvedValue([ + { access_token: 'access-token' } as any, + {}, + { + provider: 'myidp', + first_name: 'Ada', + last_name: 'Lovelace', + email: 'ada@example.com', + external_identifier: 'ada@example.com', + role: 'role-1', + auth_data: authData, + }, + ]); + } + + test('stores the refresh token issued by the provider', async () => { + const driver = createDriver(); + mockProviderResponse(driver, JSON.stringify({ refreshToken: 'new-token' })); + + await expect(driver.getUserID(CALLBACK_PAYLOAD)).resolves.toBe('user-1'); + + expect(updateOne).toHaveBeenCalledWith('user-1', { + auth_data: JSON.stringify({ refreshToken: 'new-token' }), + }); + }); + + test('keeps the stored refresh token when the provider issued none', async () => { + const driver = createDriver(); + mockProviderResponse(driver, undefined); + + await expect(driver.getUserID(CALLBACK_PAYLOAD)).resolves.toBe('user-1'); + + // No update at all: auth_data is not overwritten with null, and an empty update would crash (#127) + expect(updateOne).not.toHaveBeenCalled(); + }); + + test('keeps the stored refresh token while updating the profile (updateUserOnLogin)', async () => { + const driver = createDriver({ updateUserOnLogin: true }); + mockProviderResponse(driver, undefined); + + await expect(driver.getUserID(CALLBACK_PAYLOAD)).resolves.toBe('user-1'); + + expect(updateOne).toHaveBeenCalledWith('user-1', { + first_name: 'Ada', + last_name: 'Lovelace', + email: 'ada@example.com', + }); + }); +}); diff --git a/api/src/auth/drivers/baseoauth.ts b/api/src/auth/drivers/baseoauth.ts index 59d455967..c926925b9 100644 --- a/api/src/auth/drivers/baseoauth.ts +++ b/api/src/auth/drivers/baseoauth.ts @@ -80,11 +80,17 @@ export abstract class BaseOAuthDriver extends LocalAuthDriver { { database: getDatabase(), schema: this.schema, accountability: null } ); - // Update user to update refresh_token and other properties that might have changed - await this.getUserService().updateOne(userId, { - ...updatedUserPayload, - auth_data: userPayload.auth_data ?? null, - }); + // Update user to update refresh_token and other properties that might have changed. + // Keep the stored refresh_token if the provider didn't issue a new one (e.g. Google when consent was + // already granted): refresh() still checks it against the provider + const updateUserPayload = userPayload.auth_data + ? { ...updatedUserPayload, auth_data: userPayload.auth_data } + : updatedUserPayload; + + // Skip empty updates, they crash (#127) + if (Object.keys(updateUserPayload).length > 0) { + await this.getUserService().updateOne(userId, updateUserPayload); + } return userId; } @@ -192,10 +198,15 @@ export abstract class BaseOAuthDriver extends LocalAuthDriver { } catch (e: any) { throw this.handleError(e); } + } else if (this.getConfig()['requireRefreshToken'] === false) { + // Opt-in: the provider issued no refresh token (e.g. Google when consent was already granted), + // so the Directus session is the only source of truth for this user + logger.debug(`[${this.getClientName()}] No refresh token stored, continuing (REQUIRE_REFRESH_TOKEN=false)`); } else { - // If no auth connexion, throw user out + // If no auth connexion, throw user out. On login, the callback retries once with prompt=consent + // so the provider can issue a refresh_token logger.error(`No [${this.getClientName()}] Session found`); - throw new InvalidCredentialsException(); + throw new InvalidTokenException(); } } diff --git a/api/src/env.ts b/api/src/env.ts index 0ef465546..25d3dd973 100644 --- a/api/src/env.ts +++ b/api/src/env.ts @@ -145,6 +145,7 @@ const allowedEnvironmentVars = [ 'AUTH_.+_FIRST_NAME_KEY', 'AUTH_.+_LAST_NAME_KEY', 'AUTH_.+_ALLOW_PUBLIC_REGISTRATION', + 'AUTH_.+_REQUIRE_REFRESH_TOKEN', 'AUTH_.+_DEFAULT_ROLE_ID', 'AUTH_.+_REGISTER_URL_ALLOW_LIST', 'AUTH_.+_ICON', diff --git a/docs/self-hosted/config-options.md b/docs/self-hosted/config-options.md index 4ae5be5ec..739e1fde3 100644 --- a/docs/self-hosted/config-options.md +++ b/docs/self-hosted/config-options.md @@ -800,6 +800,7 @@ These flows rely on the `PUBLIC_URL` variable for redirecting. Ensure the variab | `AUTH__FIRST_NAME_KEY` | User profile first name key. | -- | | `AUTH__LAST_NAME_KEY` | User profile last name key. | -- | | `AUTH__ALLOW_PUBLIC_REGISTRATION` | Automatically create accounts for authenticating users. | `false` | +| `AUTH__REQUIRE_REFRESH_TOKEN` | Reject logins and session renewals when the provider did not issue a refresh token [3]. | `true` | | `AUTH__DEFAULT_ROLE_ID` | A d9 role ID to assign created users. | -- | | `AUTH__ICON` | SVG icon to display with the login link. [See options here](/getting-started/glossary#icons). | `account_circle` | | `AUTH__LABEL` | Text to be presented on SSO button within App. | `` | @@ -822,6 +823,7 @@ OpenID is an authentication protocol built on OAuth 2.0, and should be preferred | `AUTH__IDENTIFIER_KEY` | User profile identifier key [1]. | `sub`[2] | | `AUTH__ALLOW_PUBLIC_REGISTRATION` | Automatically create accounts for authenticating users. | `false` | | `AUTH__REQUIRE_VERIFIED_EMAIL` | Require created users to have a verified email address. | `false` | +| `AUTH__REQUIRE_REFRESH_TOKEN` | Reject logins and session renewals when the provider did not issue a refresh token [3]. | `true` | | `AUTH__DEFAULT_ROLE_ID` | A d9 role ID to assign created users. | -- | | `AUTH__ICON` | SVG icon to display with the login link. [See options here](/getting-started/glossary#icons). | `account_circle` | | `AUTH__LABEL` | Text to be presented on SSO button within App. | `` | @@ -834,6 +836,15 @@ d9 users "External Identifier". [2] `sub` represents a unique user identifier defined by the OpenID provider. For users not relying on `PUBLIC_REGISTRATION` it is recommended to use a human-readable identifier, such as `email`. +[3] By default a user can only stay logged in through an external provider while d9 holds a refresh token +for them, which is checked against the provider on every login and session renewal. Some providers only issue a +refresh token in specific situations (for example Google, only when it shows the consent screen). When none is stored at +login, d9 sends the user back to the provider once with `prompt=consent` so it can issue one. Set this option to +`false` to let such users log in without one: the d9 session (`REFRESH_TOKEN_TTL`) is then the only thing that keeps them +logged in, so disabling or revoking the user at the provider is not noticed until that session expires. If a refresh +token _is_ stored, it is still checked against the provider and a revoked token still ends the session. Suspend the +user in d9 and/or lower `REFRESH_TOKEN_TTL` if you need faster revocation. + ### LDAP (`ldap`) LDAP allows Active Directory users to authenticate and use d9 without having to be manually configured. User diff --git a/docs/self-hosted/sso.md b/docs/self-hosted/sso.md index bea778114..4c940b4f7 100644 --- a/docs/self-hosted/sso.md +++ b/docs/self-hosted/sso.md @@ -75,6 +75,16 @@ AUTH_GOOGLE_DEFAULT_ROLE_ID="XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX" # Replace thi 8. Now you can see a nice functional `Login with Google` button on d9 login page. +::: tip Consent screen and refresh tokens + +Google only returns a refresh token when it shows the consent screen. If the Google account already granted access to +this OAuth client before and d9 has no refresh token stored for the user, d9 sends the user back to Google once with the +consent screen to get one. To skip that screen, accept logins without a refresh token with +`AUTH_GOOGLE_REQUIRE_REFRESH_TOKEN="false"` (see the trade-off in +[Configuration Options](/self-hosted/config-options#sso-oauth2-and-openid)). + +::: + ## Seamless SSO While sometimes you want your users to directly have access to the d9 Application, in other cases you may need to