diff --git a/.changeset/plugin-table-credential-fields.md b/.changeset/plugin-table-credential-fields.md new file mode 100644 index 00000000..1ccb6d6d --- /dev/null +++ b/.changeset/plugin-table-credential-fields.md @@ -0,0 +1,23 @@ +--- +'@opensaas/stack-auth': minor +--- + +Extend the ADR-0036 credential field read-deny from the four base Auth models to better-auth plugin tables, and add a `credentialFields` config option for plugins the stack doesn't seed a set for. + +The following fields now ship field-level `read`-denied, on top of the existing `Session.token`/`Verification.value`/`Account.password`/`accessToken`/`refreshToken`/`idToken`: + +- `oauthClient.clientSecret`, `oauthAccessToken.token`, `oauthRefreshToken.token` (the `mcp`/oauth-provider plugin) +- `twoFactor.secret`, `twoFactor.backupCodes` (`twoFactor()`) + +An application opening one of these lists (e.g. declaring `OauthClient` under its own `lists` to grant access) no longer also exposes the credential column — same behavior as the existing base-model deny, `sudo()` still reads it. + +For a plugin the stack has no seeded credential set for, mark a field yourself: + +```typescript +authPlugin({ + betterAuthPlugins: [passkey()], + credentialFields: { passkey: ['publicKey'] }, +}) +``` + +`credentialFields` is additive only — it can add fields to any model (including a seeded one) but can never unmark a seeded field. An entry naming a field missing from a model your app actually derives throws at config time; an entry for a model your app doesn't derive is a no-op. diff --git a/docs/adr/0034-plugin-tables-derive-through-the-same-registry-as-the-base-auth-models.md b/docs/adr/0034-plugin-tables-derive-through-the-same-registry-as-the-base-auth-models.md index b20ed46f..4dcd5cbb 100644 --- a/docs/adr/0034-plugin-tables-derive-through-the-same-registry-as-the-base-auth-models.md +++ b/docs/adr/0034-plugin-tables-derive-through-the-same-registry-as-the-base-auth-models.md @@ -27,3 +27,5 @@ Concretely: - This absorbs #994 (which had specified the same FK fix against list-only refs, before ADR-0033 shipped the base models' bidirectional-ref convention) — the FK column name is controllable on a bidirectional ref already, so #994's premise (list-only refs as the only viable shape) no longer applies, and its fix is superseded by this consolidation rather than landed separately. See issue #992 for the full triage history, including the confirmed orphaning and the correction that bidirectional refs were viable all along. + +Because plugin tables derive through this same registry and scalar-field-derivation pass, they are also in scope for ADR-0036's credential field read-deny — see that ADR (as amended by issue #1014) for the plugin-table credential set and the `credentialFields` seam. diff --git a/docs/adr/0036-plugin-derived-credential-fields-ship-read-denied.md b/docs/adr/0036-plugin-derived-credential-fields-ship-read-denied.md index bf81db4e..1ee4699c 100644 --- a/docs/adr/0036-plugin-derived-credential-fields-ship-read-denied.md +++ b/docs/adr/0036-plugin-derived-credential-fields-ship-read-denied.md @@ -2,7 +2,7 @@ Status: accepted -A field on a plugin-derived list that holds a **live, presentable credential** — one where reading the value is equivalent to holding it — ships with a field-level `read` deny that the plugin sets when it creates the list. Granting operation-level access to such a list does not grant access to those fields. For the auth plugin this covers `Session.token`, `Verification.value`, `Account.password`, and the OAuth token columns (`accessToken`, `refreshToken`, `idToken`). +A field on a plugin-derived list that holds a **live, presentable credential** — one where reading the value is equivalent to holding it — ships with a field-level `read` deny that the plugin sets when it creates the list. Granting operation-level access to such a list does not grant access to those fields. For the auth plugin this covers `Session.token`, `Verification.value`, `Account.password`, and the OAuth token columns (`accessToken`, `refreshToken`, `idToken`) on the four **base** models, and — as of issue #1014 — the same rule over **better-auth plugin tables** derived through the same registry (ADR-0034): `oauthClient.clientSecret`, `oauthAccessToken.token`, `oauthRefreshToken.token` (the `mcp`/oauth-provider plugin) and `twoFactor.secret`/`twoFactor.backupCodes` (`twoFactor()`). An application can mark further plugin fields as credentials via `authPlugin({ credentialFields })`, additive only over the stack's seeded set. ## Context @@ -29,3 +29,4 @@ Two properties make the deny cheap. `sudo()` skips field-level access checks, so - **A denied field is stripped, not an error.** It follows the field-level access path's existing behaviour; a list read still succeeds and returns every other field. The application sees a missing column, not a failure. - **`sudo()` still reads these fields.** That is the supported path for an application with a legitimate need until a narrower seam exists, and it is what the auth package's own helpers already use. - **The rule is about the credential, not the list.** A derived field that merely identifies (`Session.ipAddress`, `Account.providerId`) stays open. The test is whether reading the value confers the ability to act as someone — that is what earns a deny. +- **The rule applies uniformly to plugin tables, keyed by better-auth's own model/field keys — not by whatever store mode the value happens to sit in.** `oauth-provider`'s token/secret columns default to hashed storage (encrypted when `disableJwtPlugin`) and `twoFactor`'s `secret`/`backupCodes` are stored encrypted; the deny does not condition on `storeTokens`/`storeClientSecret` or any other at-rest mode, and it is not derived from better-auth's own `returned: false` flag — that flag is both noisy (it also marks plain identifiers, e.g. `twoFactor.userId`) and incomplete (`oauth-provider` never sets it). The stack seeds the set for the plugins it has first-class support for; `authPlugin({ credentialFields })` lets an application extend it to a plugin the stack has never seen, additive-only over the seeded set (issue #1014). diff --git a/docs/content/reference/auth.md b/docs/content/reference/auth.md index 4b44c3bd..a89f057d 100644 --- a/docs/content/reference/auth.md +++ b/docs/content/reference/auth.md @@ -310,6 +310,19 @@ authPlugin({ The auth plugin automatically converts Better Auth plugin schemas to OpenSaaS lists. +### `credentialFields` + +Mark additional better-auth model fields as credentials, so they ship field-level read-denied +alongside the stack's own seeded set. See [Credential fields are read-denied](#credential-fields-are-read-denied-adr-0036) +below for the full contract and the seeded set. + +```typescript +authPlugin({ + betterAuthPlugins: [passkey()], + credentialFields: { passkey: ['publicKey'] }, +}) +``` + ### `betterAuthOptions` Escape hatch for any better-auth option the stack doesn't model as its own config field. Deep-merged into the options `createAuth()` builds, applied **last** — a plain-object value at a given key merges recursively with what the stack already set there (so a nested addition like `session.cookieCache` adds alongside the stack's own `session.expiresIn`/`updateAge` rather than replacing them), and on a genuine key collision `betterAuthOptions` wins. Arrays and any other value type replace the stack's value outright. @@ -439,6 +452,35 @@ derives the list, so granting operation-level access to a list (e.g. `access: { column — the field is silently stripped from a returned row, the same as any other field-level read denial, and the rest of the row is returned normally. +The same deny covers **better-auth plugin table** credential fields for the plugins the stack has +first-class support for — plugin tables derive through the identical field-derivation pass as the +base models (ADR-0034): + +| Model (better-auth key) | Field(s) | Plugin | +| ----------------------- | ----------------------- | ---------------------- | +| `oauthClient` | `clientSecret` | `mcp` / oauth-provider | +| `oauthAccessToken` | `token` | `mcp` / oauth-provider | +| `oauthRefreshToken` | `token` | `mcp` / oauth-provider | +| `twoFactor` | `secret`, `backupCodes` | `twoFactor()` | + +For any other plugin, mark a field as a credential yourself via `credentialFields` — keyed by +better-auth's own **model key** (not the derived list key) and naming better-auth's own **field +keys** (not mapped column names). It is strictly additive: it can mark further fields, but can never +unmark one of the fields above. + +```typescript +authPlugin({ + betterAuthPlugins: [passkey()], + credentialFields: { + passkey: ['publicKey'], + }, +}) +``` + +An entry naming a field that doesn't exist on a model your app actually derives (the plugin is +registered) throws at config time, naming the model and field; an entry for a model your app doesn't +derive at all (the plugin isn't registered) is a silent no-op. + Naming a denied field in `findMany`'s (or `count`'s) `where`/`orderBy` is different: that's rejected up front with a `ValidationError` rather than silently stripped, the same as any other field-level `read` deny. A `findUnique` lookup is not — its `where` only unique-selects the row, so diff --git a/packages/auth/CLAUDE.md b/packages/auth/CLAUDE.md index 223255ff..c4dce4c9 100644 --- a/packages/auth/CLAUDE.md +++ b/packages/auth/CLAUDE.md @@ -195,13 +195,11 @@ then stands, same as any other `extendList`). ### Credential fields are read-denied independent of operation access (ADR-0036) Operation-level access is all-or-nothing at the list, not the column — so -the deny above (list-level) isn't the whole story for the six -credential-bearing fields (`Session.token`, `Verification.value`, -`Account.password`/`accessToken`/`refreshToken`/`idToken`): `deriveAuthLists` -sets a field-level `access: { read: () => false }` on each of them -unconditionally, independent of whatever `accessConfig` an app supplies. -Opening `query` on `Session` for a "your active sessions" screen no longer -also exposes `token` — the field is stripped from a returned row (the +the deny above (list-level) isn't the whole story for a credential-bearing +field: `deriveAuthLists` sets a field-level `access: { read: () => false }` +on each one unconditionally, independent of whatever `accessConfig` an app +supplies. Opening `query` on `Session` for a "your active sessions" screen no +longer also exposes `token` — the field is stripped from a returned row (the ordinary field-access-denial behavior), the row itself still returns. This holds even for a `findUnique` lookup that selects the row BY the denied field (`context.db.session.findUnique({ where: { token } })` still finds @@ -214,17 +212,51 @@ predicate-time path (`validateQueryFieldReadAccess` in required for that shape too, not only for reading the column back off a row fetched another way. +This is not a closed list of six base-model fields — it also covers **plugin +table** credential fields the stack has first-class support for (ADR-0034), +since a plugin table derives through the same scalar-field derivation pass: + +| Model (better-auth key) | Field(s) | Source | +| ----------------------- | ---------------------------------------------------- | ---------------------- | +| `session` | `token` | base | +| `verification` | `value` | base | +| `account` | `password`, `accessToken`, `refreshToken`, `idToken` | base | +| `oauthClient` | `clientSecret` | `mcp` / oauth-provider | +| `oauthAccessToken` | `token` | `mcp` / oauth-provider | +| `oauthRefreshToken` | `token` | `mcp` / oauth-provider | +| `twoFactor` | `secret`, `backupCodes` | `twoFactor()` | + The deny is applied in the scalar-field derivation loop -(`withCredentialAccess` in `derive-auth-lists.ts`), keyed by better-auth's -own model/field key (`CREDENTIAL_FIELDS`) — not the app's list key or -column `db.map` — so it survives a `modelName` remap or a `fields` column -override. `sudo()` still reads these fields either way, unaffected: this is -the supported path for a genuine need (an admin tool, or an app's own auth +(`withCredentialAccess` in `derive-auth-lists.ts`), against a registry built +by `buildCredentialFieldRegistry` — the stack-seeded `CREDENTIAL_FIELDS` table +above merged with an app's `authPlugin({ credentialFields })` — keyed by +better-auth's own model/field key, not the app's list key or column `db.map`, +so it survives a `modelName` remap or a `fields` column override either way. +`credentialFields` (`Record`) is how an +app marks a credential field on a better-auth plugin the stack doesn't seed a +set for; it is **additive only** — it can add fields to any model, including a +seeded one, but can never unmark a seeded field. An entry naming a field +absent from a model the app actually derives (that plugin is registered) +throws, naming the model and field; an entry for a model the app doesn't +derive at all is a silent no-op: + +```typescript +authPlugin({ + betterAuthPlugins: [passkey()], + credentialFields: { passkey: ['publicKey'] }, +}) +``` + +`sudo()` still reads every one of these fields either way, unaffected: this +is the supported path for a genuine need (an admin tool, or an app's own auth code verifying a password hash via `HashedPassword.compare()`). See `packages/core/CLAUDE.md`'s "Access Control Execution Flow" for how a -field-level `read` denial is enforced, and ADR-0036 for why this list is -exactly these six fields and not, say, `Account.providerId` or -`Session.ipAddress` (identifying, not authenticating — left open). +field-level `read` denial is enforced, and ADR-0036 for why the rule is about +whether reading the value confers a live credential, not, say, +`Account.providerId` or `Session.ipAddress` (identifying, not authenticating +— left open) — and it does not condition on better-auth's own storage mode +(`storeTokens`/`storeClientSecret`) or `returned: false` flag, which is +neither a reliable nor a complete signal (see ADR-0036). ### Schema placement (relocatable Auth lists) diff --git a/packages/auth/src/config/derive-auth-lists.ts b/packages/auth/src/config/derive-auth-lists.ts index f2e7a89a..9b53676d 100644 --- a/packages/auth/src/config/derive-auth-lists.ts +++ b/packages/auth/src/config/derive-auth-lists.ts @@ -107,17 +107,94 @@ const TIMESTAMP_FIELDS = new Set(['createdAt', 'updatedAt']) * OAuth token) — rather than merely identifying a row. Keyed by better-auth's * own model/field keys, not the app's list/column names, so the deny holds * under `modelName` and column `fields` remapping alike (ADR-0036). + * + * Covers the four base models plus every better-auth plugin table the stack + * has first-class support for (ADR-0034's registry of known plugins) — the + * `mcp`/oauth-provider plugin's client secret and token columns, and + * `twoFactor()`'s encrypted secret/backup codes (issue #1014). An app can + * mark further fields via `authPlugin({ credentialFields })`, merged in by + * {@link buildCredentialFieldRegistry} — this constant is never mutated. */ -const CREDENTIAL_FIELDS: Partial> = { +const CREDENTIAL_FIELDS: Record = { session: ['token'], verification: ['value'], account: ['password', 'accessToken', 'refreshToken', 'idToken'], + oauthClient: ['clientSecret'], + oauthAccessToken: ['token'], + oauthRefreshToken: ['token'], + twoFactor: ['secret', 'backupCodes'], } const DENY_READ: FieldAccess = { read: () => false } -function withCredentialAccess(modelKey: string, fieldKey: string, field: FieldConfig): FieldConfig { - if (!CREDENTIAL_FIELDS[modelKey as BaseModelKey]?.includes(fieldKey)) return field +/** Every better-auth model/field key marked as a credential — the stack's own {@link CREDENTIAL_FIELDS} plus an app's `credentialFields`. */ +type CredentialFieldRegistry = Record> + +/** + * Merges the stack-seeded {@link CREDENTIAL_FIELDS} with an app's + * `authPlugin({ credentialFields })`, then validates every entry against the + * models actually being derived (`tables`, keyed by better-auth model key). + * + * Additive only: an app entry can add fields to a model — including a + * stack-seeded one — but nothing can remove a seeded field, so a config that + * omits or empties a seeded model's list leaves that model's seeded deny + * standing. + * + * A field named on a model that isn't in `tables` at all (a plugin the app + * doesn't use) is a silent no-op — the entry simply never matches anything. + * A field named on a model that IS in `tables` but doesn't declare that field + * throws, naming the model and field, since that is a config mistake the app + * would otherwise never learn about (the deny would just never fire). + */ +function buildCredentialFieldRegistry( + tables: Record, + appConfig: Record, +): CredentialFieldRegistry { + const merged = new Map>() + for (const [modelKey, fields] of Object.entries(CREDENTIAL_FIELDS)) { + merged.set(modelKey, new Set(fields)) + } + for (const [modelKey, fields] of Object.entries(appConfig)) { + const set = merged.get(modelKey) ?? new Set() + for (const fieldKey of fields) set.add(fieldKey) + merged.set(modelKey, set) + } + + const registry: CredentialFieldRegistry = {} + for (const [modelKey, fields] of merged) { + const table = tables[modelKey] + if (!table) continue // model not derived (plugin unused) -> silent no-op + for (const fieldKey of fields) { + const upstream = table.fields[fieldKey] + if (!upstream) { + throw new Error( + `deriveAuthLists: credentialFields names "${modelKey}.${fieldKey}", but "${modelKey}" has no field "${fieldKey}"`, + ) + } + // An id-referencing field derives to a relationship() (see the + // `references.field === 'id'` branch below), never a scalar field — + // withCredentialAccess is only ever applied on the scalar-field path, + // so a deny registered against one would silently never apply. Fail + // loudly instead of accepting a config that has no effect. + if (upstream.references?.field === 'id') { + throw new Error( + `deriveAuthLists: credentialFields names "${modelKey}.${fieldKey}", but "${fieldKey}" is a ` + + `relationship field (references "${upstream.references.model}.id"), not a scalar credential column`, + ) + } + } + registry[modelKey] = fields + } + return registry +} + +function withCredentialAccess( + registry: CredentialFieldRegistry, + modelKey: string, + fieldKey: string, + field: FieldConfig, +): FieldConfig { + if (!registry[modelKey]?.has(fieldKey)) return field return { ...field, access: DENY_READ } } @@ -493,6 +570,9 @@ function buildModelRegistry( * @param accessConfig - App-authored access for each base Auth list, keyed by better-auth model name * @param plugins - The app's better-auth plugins (`authPlugin({ betterAuthPlugins })`), whose own * `schema` (base-model extensions and standalone plugin tables) is derived alongside the base models + * @param credentialFieldsConfig - App-authored additions to the credential-field read-deny + * (`authPlugin({ credentialFields })`), keyed by better-auth model key. Additive only — see + * {@link buildCredentialFieldRegistry}. * @returns The derived base-model list keys and every derived list config (base models and plugin tables) */ export function deriveAuthLists( @@ -500,12 +580,14 @@ export function deriveAuthLists( userConfig: ExtendUserListConfig = {}, accessConfig: AuthAccessConfig = {}, plugins: BetterAuthPlugin[] = [], + credentialFieldsConfig: Record = {}, ): DerivedAuthLists { const tables = getAuthTables(buildBetterAuthTableOptions(models, plugins)) as Record< string, ResolvedTable > const { keys, registry } = buildModelRegistry(tables, models) + const credentialRegistry = buildCredentialFieldRegistry(tables, credentialFieldsConfig) // Only the five base models carry an app-authored `db.indexes` passthrough // (`AuthModelConfig.indexes`) — plugin tables have no per-model config @@ -577,6 +659,7 @@ export function deriveAuthLists( // column. Left as a plain scalar column, same as pre-consolidation // behavior (issue #992). ;(scalarFields[modelKey] ??= {})[fieldKey] = withCredentialAccess( + credentialRegistry, modelKey, fieldKey, buildScalarField( @@ -588,6 +671,7 @@ export function deriveAuthLists( } } else { ;(scalarFields[modelKey] ??= {})[fieldKey] = withCredentialAccess( + credentialRegistry, modelKey, fieldKey, buildScalarField( diff --git a/packages/auth/src/config/index.ts b/packages/auth/src/config/index.ts index b1231547..4214df8e 100644 --- a/packages/auth/src/config/index.ts +++ b/packages/auth/src/config/index.ts @@ -138,6 +138,7 @@ export function normalizeAuthConfig(config: AuthConfig): NormalizedAuthConfig { sessionFields, extendUserList: config.extendUserList || {}, access: config.access || {}, + credentialFields: config.credentialFields || {}, betterAuthPlugins: config.betterAuthPlugins || [], rateLimit: config.rateLimit, betterAuthOptions: config.betterAuthOptions || {}, diff --git a/packages/auth/src/config/plugin.ts b/packages/auth/src/config/plugin.ts index 7019a08f..caac0cb9 100644 --- a/packages/auth/src/config/plugin.ts +++ b/packages/auth/src/config/plugin.ts @@ -51,6 +51,7 @@ export function authPlugin(config: AuthConfig): Plugin { normalized.models, normalized.access, normalized.betterAuthPlugins, + normalized.credentialFields, ) // Base models are always the first entries in `authLists` (see diff --git a/packages/auth/src/config/types.ts b/packages/auth/src/config/types.ts index b153693a..4e163976 100644 --- a/packages/auth/src/config/types.ts +++ b/packages/auth/src/config/types.ts @@ -393,6 +393,39 @@ export type AuthConfig = { */ access?: AuthAccessConfig + /** + * Mark additional better-auth model fields as credentials, so they ship + * field-level read-denied like the stack's own seeded set (ADR-0036) — the + * six base-model fields plus, as of issue #1014, the `mcp`/oauth-provider + * plugin's `oauthClient.clientSecret`/`oauthAccessToken.token`/ + * `oauthRefreshToken.token` and `twoFactor()`'s `twoFactor.secret`/ + * `backupCodes`. Use this for a better-auth plugin the stack doesn't seed a + * credential set for. + * + * Keyed by better-auth's own **model key** (e.g. `'twoFactor'`, + * `'oauthClient'` — not the derived list key, so it stays remap-proof), each + * naming better-auth **field keys** (not mapped column names). + * + * Strictly additive: an entry can mark further fields as credentials, but + * can never unmark one of the stack's seeded fields — omitting or emptying + * a seeded model's list here leaves that model's seeded deny standing. An + * entry naming a field that doesn't exist on a model the app actually + * derives (has that plugin registered) throws, naming the model and field; + * an entry for a model the app doesn't derive at all is a silent no-op. + * An entry naming an id-referencing relationship field (e.g. an FK ending + * `Id`) also throws — that field derives to a `relationship()`, never a + * scalar column, so a deny registered against it could never apply. + * + * @example + * ```typescript + * authPlugin({ + * betterAuthPlugins: [passkey()], + * credentialFields: { passkey: ['publicKey'] }, + * }) + * ``` + */ + credentialFields?: Record + /** * Additional Better Auth plugins to enable * Allows integrating any Better Auth plugin (MCP, 2FA, etc.) diff --git a/packages/auth/src/lists/index.ts b/packages/auth/src/lists/index.ts index 7207f4a7..638302cd 100644 --- a/packages/auth/src/lists/index.ts +++ b/packages/auth/src/lists/index.ts @@ -90,13 +90,22 @@ export function createVerificationList(): ListConfig { * @param models - Resolved better-auth model config; defaults to the better-auth defaults * @param accessConfig - App-authored access for each base Auth list, keyed by better-auth model name * @param plugins - The app's better-auth plugins (`authPlugin({ betterAuthPlugins })`) + * @param credentialFieldsConfig - App-authored additions to the credential-field read-deny + * (`authPlugin({ credentialFields })`), keyed by better-auth model key */ export function getAuthLists( userConfig?: ExtendUserListConfig, models: NormalizedAuthModels = DEFAULT_MODELS, accessConfig?: AuthAccessConfig, plugins?: BetterAuthPlugin[], + credentialFieldsConfig?: Record, // eslint-disable-next-line @typescript-eslint/no-explicit-any -- ListConfig must accept any TypeInfo ): Record> { - return deriveAuthLists(models, userConfig || {}, accessConfig || {}, plugins || []).lists + return deriveAuthLists( + models, + userConfig || {}, + accessConfig || {}, + plugins || [], + credentialFieldsConfig || {}, + ).lists } diff --git a/packages/auth/tests/derive-auth-lists.test.ts b/packages/auth/tests/derive-auth-lists.test.ts index 83db1f02..5ac88aff 100644 --- a/packages/auth/tests/derive-auth-lists.test.ts +++ b/packages/auth/tests/derive-auth-lists.test.ts @@ -1,4 +1,6 @@ import { describe, it, expect } from 'vitest' +import { mcp } from '@better-auth/mcp' +import { twoFactor } from 'better-auth/plugins' import { deriveAuthLists } from '../src/config/derive-auth-lists.js' import type { NormalizedAuthModels } from '../src/config/types.js' @@ -534,6 +536,152 @@ describe('deriveAuthLists - credential fields ship read-denied (ADR-0036, issue }) }) +describe('deriveAuthLists - credential fields on plugin tables (issue #1014)', () => { + const mcpPlugin = mcp({ + loginPage: '/sign-in', + consentPage: '/consent', + resource: 'https://example.com/mcp', + }) + + it('denies read on the mcp/oauth-provider credential fields', async () => { + const { lists } = deriveAuthLists(defaultModels, {}, {}, [mcpPlugin]) + + const denied: Array<[string, string]> = [ + ['OauthClient', 'clientSecret'], + ['OauthAccessToken', 'token'], + ['OauthRefreshToken', 'token'], + ] + + for (const [listKey, fieldKey] of denied) { + const field = lists[listKey].fields[fieldKey] + expect(field.access?.read).toBeTypeOf('function') + // eslint-disable-next-line @typescript-eslint/no-explicit-any -- minimal read-access call fixture + expect(await field.access!.read!({} as any)).toBe(false) + } + }) + + it('leaves identifying oauth-provider fields open', () => { + const { lists } = deriveAuthLists(defaultModels, {}, {}, [mcpPlugin]) + + expect(lists.OauthClient.fields.name.access).toBeUndefined() + expect(lists.OauthClient.fields.uri.access).toBeUndefined() + expect(lists.OauthClient.fields.clientId.access).toBeUndefined() + expect(lists.OauthAccessToken.fields.scopes.access).toBeUndefined() + expect(lists.OauthAccessToken.fields.expiresAt.access).toBeUndefined() + }) + + it('denies read on twoFactor.secret and twoFactor.backupCodes', async () => { + const { lists } = deriveAuthLists(defaultModels, {}, {}, [twoFactor()]) + + for (const fieldKey of ['secret', 'backupCodes']) { + const field = lists.TwoFactor.fields[fieldKey] + expect(field.access?.read).toBeTypeOf('function') + // eslint-disable-next-line @typescript-eslint/no-explicit-any -- minimal read-access call fixture + expect(await field.access!.read!({} as any)).toBe(false) + } + }) + + it('leaves twoFactor.verified open, and twoFactor.userId open despite carrying returned: false upstream', () => { + const { lists } = deriveAuthLists(defaultModels, {}, {}, [twoFactor()]) + + expect(lists.TwoFactor.fields.verified.access).toBeUndefined() + // userId is a relationship field (references user.id), not a scalar — + // it's never routed through the credential-deny path at all. + expect(lists.TwoFactor.fields.user.access).toBeUndefined() + }) + + it('survives a plugin-table modelName + column remap', () => { + const plugin = { + id: 'test-remap', + schema: { + widget: { + modelName: 'Gadget', + fields: { + apiKey: { type: 'string' as const, required: true, fieldName: 'api_key' }, + }, + }, + }, + } + + const { lists } = deriveAuthLists(defaultModels, {}, {}, [plugin], { + widget: ['apiKey'], + }) + + expect(lists.Gadget.fields.apiKey.access?.read).toBeTypeOf('function') + expect(lists.Gadget.fields.apiKey.db?.map).toBe('api_key') + }) + + it('authPlugin({ credentialFields }) marks an additional field on a synthetic plugin table', async () => { + const plugin = { + id: 'test-passkey', + schema: { + passkey: { + fields: { + publicKey: { type: 'string' as const, required: true }, + deviceType: { type: 'string' as const, required: true }, + }, + }, + }, + } + + const { lists } = deriveAuthLists(defaultModels, {}, {}, [plugin], { + passkey: ['publicKey'], + }) + + expect(await lists.Passkey.fields.publicKey.access!.read!({} as never)).toBe(false) + expect(lists.Passkey.fields.deviceType.access).toBeUndefined() + }) + + it('cannot unmark a stack-seeded credential field', async () => { + // An empty (or omitted) field list for a seeded model must not remove its + // seeded deny — credentialFields is additive-only. + const { lists } = deriveAuthLists(defaultModels, {}, {}, [], { session: [] }) + + expect(await lists.Session.fields.token.access!.read!({} as never)).toBe(false) + }) + + it('throws, naming the model and field, when credentialFields names a field missing from a derived model', () => { + const plugin = { + id: 'test-typo', + schema: { + widget: { fields: { apiKey: { type: 'string' as const } } }, + }, + } + + expect(() => deriveAuthLists(defaultModels, {}, {}, [plugin], { widget: ['apiKye'] })).toThrow( + /widget\.apiKye.*no field "apiKye"/, + ) + }) + + it('is a silent no-op when credentialFields names a model that is not derived at all', () => { + // No plugin registers `passkey` here, so `tables` never contains it. + expect(() => + deriveAuthLists(defaultModels, {}, {}, [], { passkey: ['publicKey'] }), + ).not.toThrow() + }) + + it('throws, naming the model and field, when credentialFields names an id-referencing relationship field', () => { + // An id-referencing field (references.field === 'id') derives to a + // relationship(), never a scalar — withCredentialAccess only ever runs on + // the scalar-field path, so a deny registered against one would silently + // never apply. Reject the config instead of accepting a no-op. + const plugin = { + id: 'test-fk-credential', + schema: { + widget: { + fields: { + ownerId: { type: 'string' as const, references: { model: 'user', field: 'id' } }, + }, + }, + }, + } + + expect(() => deriveAuthLists(defaultModels, {}, {}, [plugin], { widget: ['ownerId'] })).toThrow( + /widget\.ownerId.*relationship field.*"user\.id"/, + ) + }) +}) + describe('deriveAuthLists - extendUserList', () => { it('adds custom fields to the derived user list', () => { const { lists } = deriveAuthLists( diff --git a/packages/auth/tests/plugin-derived-keys.test.ts b/packages/auth/tests/plugin-derived-keys.test.ts index c06454a5..fe9030f0 100644 --- a/packages/auth/tests/plugin-derived-keys.test.ts +++ b/packages/auth/tests/plugin-derived-keys.test.ts @@ -209,6 +209,45 @@ describe('authPlugin - add-vs-extend with derived keys', () => { // ...or its access config. expect(authUser.access?.operation?.query).toBe(userQuery) }) + + it('keeps a plugin-table credential field read-denied even when the app redeclares that list and field (issue #1014)', async () => { + // An app opening a plugin-derived list the documented way (declaring the + // list itself under the derived key) merges in via extendList — field + // additions win over the app's own field of the same key (core's + // extendList spreads `extension.fields` last), so the derived, + // read-denied clientSecret must survive even though the app also + // declares a `clientSecret` field on its own `OauthClient` list. + const mcpBetterAuthPlugin = { + id: 'test-mcp', + schema: { + oauthClient: { + fields: { + clientSecret: { type: 'string' as const, required: false }, + name: { type: 'string' as const, required: false }, + }, + }, + }, + } + + const result = await config({ + db: { provider: 'sqlite' }, + plugins: [authPlugin({ betterAuthPlugins: [mcpBetterAuthPlugin] })], + lists: { + OauthClient: list({ + fields: { clientSecret: text() }, + access: { operation: { query: () => true } }, + }), + }, + }) + + const oauthClient = result.lists.OauthClient + expect(oauthClient.fields.clientSecret.access?.read).toBeTypeOf('function') + // eslint-disable-next-line @typescript-eslint/no-explicit-any -- minimal read-access call fixture + expect(await oauthClient.fields.clientSecret.access!.read!({} as any)).toBe(false) + // The app's own access survives (extendList never forwards access). + // eslint-disable-next-line @typescript-eslint/no-explicit-any -- access control parameters are runtime values + expect(oauthClient.access?.operation?.query?.({} as any)).toBe(true) + }) }) describe('authPlugin - runtime user-key resolution', () => {