Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/gentle-wombats-relax.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@opensaas/stack-auth': patch
---

Fix `deriveAuthLists` deriving a relation that collided with its own foreign-key column for a better-auth reference field whose name doesn't end in `Id`; it now falls back to a plain scalar column, same as a non-`id`-target reference.
8 changes: 7 additions & 1 deletion packages/auth/CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,13 @@ lists cannot silently drift from what better-auth itself declares (issue
A reference whose target field isn't the target's `id` (better-auth's own
oidc-provider schema does this — `oauthAccessToken.clientId` references
`oauthClient.clientId`, not its `id`) stays a plain scalar column,
since `relationship()` can only express an `id`-based FK. Plugin tables
since `relationship()` can only express an `id`-based FK. The same fallback
applies to an `id`-referencing field whose own name doesn't end in `Id`
(stripping the suffix to name the relation would be a no-op, so the
relation would need the exact name its own FK column physically maps to —
a self-collision the contract derivation refuses, #1236); better-auth's own
tables never hit this (every default reference name ends in `Id`), but a
plugin's could (issue #1222). Plugin tables
ship closed like the base models, with no `access` passthrough at all —
see ADR-0034 and "Access control on Auth lists" below

Expand Down
31 changes: 20 additions & 11 deletions packages/auth/src/config/derive-auth-lists.ts
Original file line number Diff line number Diff line change
Expand Up @@ -184,11 +184,15 @@ function buildCredentialFieldRegistry(
)
}
// 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') {
// `references.field === 'id'` branch below) unless stripping a
// trailing `Id` off its own key is a no-op, in which case it falls
// back to a scalar column instead (#1222) — the same fallback as a
// non-`id`-target reference, and the one case where
// withCredentialAccess actually applies. Every other id-referencing
// field stays a relationship, never a scalar field, 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' && relationshipFieldName(fieldKey) !== fieldKey) {
throw new Error(
`deriveAuthLists: credentialFields names "${modelKey}.${fieldKey}", but "${fieldKey}" is a ` +
`relationship field (references "${upstream.references.model}.id"), not a scalar credential column`,
Expand Down Expand Up @@ -644,8 +648,8 @@ export function deriveAuthLists(
)
}

if (upstream.references.field === 'id') {
const relationFieldKey = relationshipFieldName(fieldKey)
const relationFieldKey = relationshipFieldName(fieldKey)
if (upstream.references.field === 'id' && relationFieldKey !== fieldKey) {
const reverseName = reverseRelationName(modelKey)
if (reverseRelationFields[targetModelKey]?.[reverseName]) {
// The reverse name is derived from the *model* (pluralized), not
Expand All @@ -671,12 +675,17 @@ export function deriveAuthLists(
many: true,
})
} else {
// relationship() always references the target's `id` column —
// better-auth's own oidc-provider schema (the MCP plugin's OAuth
// Two distinct reasons land here. (1) `upstream.references.field !==
// 'id'`: relationship() always references the target's `id` column
// — better-auth's own oidc-provider schema (the MCP plugin's OAuth
// tables) references oauthApplication.clientId instead, which a
// relation can't express without pointing Prisma at the wrong
// column. Left as a plain scalar column, same as pre-consolidation
// behavior (issue #992).
// column (issue #992). (2) `relationFieldKey === fieldKey`: the
// upstream field name doesn't end in `Id`, so stripping it is a
// no-op — the relation would need the exact name its own FK column
// physically maps to (`db.foreignKey.map`), which the contract
// derivation refuses as a self-collision (#1236). Both fall back to
// a plain scalar column, same as pre-consolidation behavior.
;(scalarFields[modelKey] ??= {})[fieldKey] = withCredentialAccess(
credentialRegistry,
modelKey,
Expand Down
4 changes: 0 additions & 4 deletions packages/auth/tests/adapter-conformance.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,15 +46,11 @@ import type { AuthConfig, AuthModelConfig } from '../src/config/types.js'
* table-level, which `deriveAuthLists` does not yet emit (#986). A schema
* gap, not an adapter one — and a production one, stated as a known limit on
* `opensaasAuthAdapter` rather than only here.
* - The nullable foreign key: a `deriveAuthLists` gap tracked as
* [#1222](https://github.com/OpenSaasAU/stack/issues/1222), not an adapter
* one.
*/
const NOT_IMPLEMENTED: Record<string, boolean> = {
...Object.fromEntries(Object.keys(enableJoinTests).map((name) => [name, true])),
'create - should use generateId if provided': true,
'create - should enforce the issuer-scoped account identity key': true,
'create - should return null for nullable foreign keys': true,
}

/**
Expand Down
24 changes: 24 additions & 0 deletions packages/auth/tests/derive-auth-lists.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -701,6 +701,30 @@ describe('deriveAuthLists - credential fields on plugin tables (issue #1014)', (
/widget\.ownerId.*relationship field.*"user\.id"/,
)
})

it('accepts and applies credentialFields on an id-referencing field whose name does not end in "Id"', async () => {
// Unlike `ownerId` above, `owner` derives to a scalar column (#1222) —
// stripping `Id` would be a no-op, so it never reaches the
// relationship() path withCredentialAccess can't apply to.
const plugin = {
id: 'test-fk-credential-no-id-suffix',
schema: {
widget: {
fields: {
owner: {
type: 'string' as const,
required: false,
references: { model: 'user', field: 'id' },
},
},
},
},
}

const { lists } = deriveAuthLists(defaultModels, {}, {}, [plugin], { widget: ['owner'] })

expect(await lists.Widget.fields.owner.access!.read!({} as never)).toBe(false)
})
})

describe('deriveAuthLists - extendUserList', () => {
Expand Down
Loading