auth: generalize credential field read-deny to better-auth plugin tables - #1019
Conversation
…n tables
Extends the ADR-0036 field-level read-deny from the four base Auth models
to plugin tables derived through the same registry (ADR-0034): the
mcp/oauth-provider plugin's oauthClient.clientSecret/oauthAccessToken.token/
oauthRefreshToken.token, and twoFactor()'s twoFactor.secret/backupCodes.
Adds authPlugin({ credentialFields }) so an app can mark credential fields
on a plugin the stack doesn't seed a set for — additive only, keyed by
better-auth's own model/field keys, and validated (throws on a field name
that doesn't exist on a derived model; no-op for an undeployed plugin).
Closes #1014
|
Deployment failed for project stack-docs with the following error: Learn More: https://vercel.com/open-saas?upgradeToPro=build-rate-limit |
🦋 Changeset detectedLatest commit: e7098ab The changes in this PR will be included in the next version bump. This PR includes changesets to release 9 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0d3dc51b65
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
An id-referencing field (references.field === 'id') derives to a relationship(), never a scalar column, so withCredentialAccess — only ever applied on the scalar-field path — could never actually deny it. Naming one in credentialFields previously validated (the field exists) and then silently had no effect. Throw instead, naming the model and field. Found in code review of #1019.
Review summaryRan a Finding (fixed): Fixed in e7098ab: the registry now throws, naming the model and field, when an entry resolves to an id-referencing relationship field, with a regression test ( No other findings. Test suite: 296 passed, 4 skipped (opt-in e2e guards). Generated by Claude Code |
|
Note: the Generated by Claude Code |
Coverage Report for Core Package Coverage (./packages/core)
File CoverageNo changed files found. |
Coverage Report for UI Package Coverage (./packages/ui)
File CoverageNo changed files found. |
Coverage Report for CLI Package Coverage (./packages/cli)
File CoverageNo changed files found. |
Coverage Report for Auth Package Coverage (./packages/auth)
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||||||||
Coverage Report for Storage Package Coverage (./packages/storage)
File CoverageNo changed files found. |
Coverage Report for RAG Package Coverage (./packages/rag)
File CoverageNo changed files found. |
Coverage Report for Storage S3 Package Coverage (./packages/storage-s3)
File CoverageNo changed files found. |
Coverage Report for Storage Vercel Package Coverage (./packages/storage-vercel)
File CoverageNo changed files found. |
2 participants
Summary
readdeny from the four base Auth models to better-auth plugin tables, since plugin tables derive through the same scalar-field-derivation pass (ADR-0034):oauthClient.clientSecret,oauthAccessToken.token,oauthRefreshToken.token(themcp/oauth-provider plugin), andtwoFactor.secret/twoFactor.backupCodes(twoFactor()).authPlugin({ credentialFields })so an app can mark a credential field on a better-auth plugin the stack doesn't seed a set for — keyed by better-auth's own model key, naming better-auth's own field keys, additive only (can add fields but can never unmark a stack-seeded one).storeTokens/storeClientSecret) or itsreturned: falseflag — neither is a reliable/complete signal (see the amended ADR-0036).packages/auth/CLAUDE.md, and the docs-site auth reference so the credential set is no longer presented as a closed list of six base fields.Test plan
packages/auth/tests/derive-auth-lists.test.tscover: realmcp/twoFactor()plugin instances denying the expected fields and leaving identifiers open (includingtwoFactor.userId, which carriesreturned: falseupstream but is a relationship field, not a scalar); a plugin-tablemodelName+ column remap surviving the deny;credentialFieldsmarking a field on a synthetic plugin;credentialFieldsfailing to unmark a seeded field; the throw-on-missing-field and no-op-on-undeployed-model paths (synthetic schemas).packages/auth/tests/plugin-derived-keys.test.tsproves an app that redeclares a plugin-derived list and its credential field key still gets the derived, read-denied field (extendList merge order), while its own access still applies.pnpm test(packages/auth): 295 passed, 4 skipped (opt-in e2e guards, unaffected by this change).pnpm lint/pnpm format/pnpm manypkg fixon the changed files.pnpm build(core, cli, auth) — no type errors.Closes #1014
Generated by Claude Code