feat(auth): add createDcrProvider for RFC 7591 dynamic client registration#31
Merged
Conversation
doistbot
reviewed
May 17, 2026
Member
doistbot
left a comment
doistbot
left a comment
There was a problem hiding this comment.
This PR introduces the createDcrProvider for RFC 7591 dynamic client registration and cleanly refactors shared OAuth logic into a reusable helper module. The implementation successfully sets up the new dynamic flow while keeping the API surface consistent and significantly reducing downstream code size. There are a few remaining refinements to ensure protocol correctness—such as using the server-provided token_endpoint_auth_method, URL-encoding Basic Auth credentials, and caching client credentials to prevent redundant registrations—along with some opportunities to generalize type names, further deduplicate fetch and authorization logic, and expand test coverage for error hints and fallback behaviors.
…ation Ships the missing OAuth provider factory for flows that mint a per-install `client_id` / `client_secret` via [RFC 7591](https://datatracker.ietf.org/doc/html/rfc7591). `prepare()` POSTs the client metadata, threads the issued credentials through the handshake, and `exchangeCode()` authenticates the token POST per `tokenEndpointAuthMethod` (`client_secret_basic` default, `_post`, or `none` for public-client). Mirrors `createPkceProvider`'s ergonomics — caller supplies `validate`; same `PkceLazyString` resolvers for URLs. Also: - Extract a private `_oauth.ts` with `postTokenEndpoint`, `buildPkceAuthorizeUrl`, `buildAuthError`, `resolve`, `safeReadText`. `pkce.ts` is now thin (≈115 LOC). The helper is grant-agnostic so a future refresh-token feature reuses it unchanged — `body: new URLSearchParams({grant_type: 'refresh_token', …})`. - New `errorHints?: string[]` option on both factories. Prepended to every `CliError` they throw; server-returned body text (on non-2xx) is appended after so the actionable hint stays at the top. - New `AUTH_DCR_FAILED` error code on the auth-error union. No external API change for existing `createPkceProvider` consumers. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Drop tests that duplicate behaviour already exercised elsewhere or that test obvious helpers in isolation: - `_oauth.test.ts`: remove standalone describes for `resolve`, `safeReadText`, and `buildPkceAuthorizeUrl` — already covered by the provider integration tests. Consolidate the four token-endpoint error cases into a single parametrised case and merge the duplicate errorHints assertion into the non-2xx test. - `dcr.test.ts`: collapse the three DCR error tests (missing client_id + non-JSON kept as a single parametrised case; non-2xx absorbed the errorHints assertion) and drop the standalone "token endpoint non-2xx" test that overlaps with `_oauth.test.ts`. Net –231 LOC across the two test files. 374 tests still pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…code Basic, dedupe fetch+parse - DCR now persists the registration response's `token_endpoint_auth_method` on the handshake and lets it override the configured one at `exchangeCode()`, per RFC 7591 §3.2.1. Validate the value against the known set so an unknown method silently falls back to the configured one. - URL-encode `client_id` and `client_secret` before joining with a colon for HTTP Basic auth per RFC 6749 §2.3.1. A literal colon (or any reserved character) in either value would otherwise corrupt the credential. - Extract `postAndParseJson` in `_oauth.ts`. Both `postTokenEndpoint` and DCR `prepare()` now share the network / non-2xx / parse error wrapping while still validating their own success shape. - Add `OAuthLazyString` as a grant-agnostic alias of `PkceLazyString` (non-breaking; existing name still exported). - Export `DcrTokenEndpointAuthMethod` from the auth subpath so callers can refer to the union directly. - Tests: cover the "no `client_secret` but `client_secret_post` requested → falls back to public-client POST" branch, the "server-issued `token_endpoint_auth_method` wins" branch, and the PKCE-level `errorHints` plumbing (both the token-endpoint path and the handshake-lost guard). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Route DCR registration and token exchange through oauth4webapi (dynamicClientRegistrationRequest / authorizationCodeGrantRequest with ClientSecretBasic/Post/None) instead of hand-rolled fetch + Basic encoding. Share the lazy oauth4webapi loader and expiry helper from _oauth.ts; PKCE refresh reuses the same loader. oauth4webapi stays an optional peer dep — a missing install surfaces as AUTH_DCR_FAILED with an install hint. This adopts oauth4webapi's RFC-conformant contract: registration must return 201 (+ client_secret_expires_at when a secret is issued), token responses must carry token_type, endpoints must be HTTPS, and Basic credentials are form-url-encoded per RFC 6749 §2.3.1. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
scottlovegrove
force-pushed
the
scottl/dcr-provider
branch
from
c1d9c85 to
36812e8
Compare
Collaborator
Author
|
@doistbot /review |
doistbot
reviewed
May 21, 2026
Member
doistbot
left a comment
doistbot
left a comment
There was a problem hiding this comment.
This PR introduces the new createDcrProvider factory for RFC 7591 dynamic client registration and integrates the oauth4webapi library to handle registration and token exchange. Relying on an established library for these flows improves the maintainability and correctness of our OAuth implementation by enforcing strict RFC compliance and removing hand-rolled encoding. A few minor adjustments are needed before merging, including failing fast on unsupported token endpoint auth methods, preserving caller-supplied error hints during dependency loading, restoring concurrent resolution for client credentials, and cleaning up some test mocks, types, and unused exports.
…, keep errorHints on load failure
- prepare() rejects with AUTH_DCR_FAILED when the registration server
selects a token_endpoint_auth_method outside the supported set
(basic/post/none) instead of silently falling back to the configured one.
- loadOauth4webapi accepts userHints, prepended on both failure branches so
createDcrProvider({ errorHints }) survives a missing/broken peer dep.
- Restore concurrent Promise.all resolution of clientId + authorizeUrl in
the PKCE authorize path.
- Use the grant-agnostic OAuthLazyString alias in _oauth.ts and dcr.ts.
- Drop export from internal-only _oauth.ts helper types.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The underscore signalled "internal module", but cli-core re-exports symbols individually (no `export *`), so the shared provider helpers can't leak into the public surface by accident — the prefix is noise. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
doist-release-bot Bot
added a commit
that referenced
this pull request
May 21, 2026
## [0.20.0](v0.19.0...v0.20.0) (2026-05-21) ### Features * **auth:** add createDcrProvider for RFC 7591 dynamic client registration ([#31](#31)) ([dd34551](dd34551))
Contributor
|
🎉 This PR is included in version 0.20.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
createDcrProviderfactory for the RFC 7591 Dynamic Client Registration flow.prepare()registers the client;authorize()runs the standard PKCE S256 dance;exchangeCode()redeems the code, authenticating the token request per the server-issuedtoken_endpoint_auth_method(client_secret_basicdefault /_post/none/ public-client fallback when DCR returned no secret).oauth4webapi— registration viadynamicClientRegistrationRequest/processDynamicClientRegistrationResponse, token exchange viaauthorizationCodeGrantRequest+ClientSecretBasic/ClientSecretPost/None. This replaces hand-rolled fetch + Basic-auth encoding with the library's RFC-conformant implementation (correct RFC 6749 §2.3.1 form-url-encoding of credentials, structured OAuth error parsing).oauth4webapiloader and theexpires_in → expiresAthelper are shared from_oauth.ts;createPkceProvider's refresh path reuses the same loader.oauth4webapistays an optional peer dep — a missing install surfaces asAUTH_DCR_FAILEDwith an install hint._oauth.ts-backed authorize / public-client token exchange (unchanged); only the DCR-specific Basic-auth helper (encodeBasicAuth) is removed, since oauth4webapi now owns that.errorHints?: string[]on both factories, prepended to everyCliError; server-returned error detail is appended after.AUTH_DCR_FAILEDcode on theAuthErrorCodeunion.Adopted RFC-conformant contract (via oauth4webapi)
Stricter than the previous lenient parser — DCR servers must comply:
client_secret_expires_atwhen aclient_secretis issued).token_type.The Twist and Todoist registration endpoints have been updated to return
201on successful creation, in line with this.No external API change for existing
createPkceProviderconsumers.Validation
Verified end-to-end against
twist.comvianpm linkinto a downstream PR that migratestwist-cli's bespokeAuthProviderontocreateDcrProvider. Twist PR follows once this releases.Test plan
npm run check— cleannpm run type-check— cleannpm test— 435 passnpm run build— cleantwist.com— full DCR registration + PKCE + token exchange + persist🤖 Generated with Claude Code