feat(core): implement getProviderTokens for stateful sessions#231
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
📝 WalkthroughWalkthroughAdds provider-token retrieval to stateful and stateless session strategies, centralizes OAuth refresh handling, wires OAuth configuration through strategy creation, and delegates the API flow to the active session strategy. ChangesProvider token retrieval
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant getProviderTokens
participant SessionStrategy
participant OAuthProvider
Client->>getProviderTokens: Request provider tokens
getProviderTokens->>SessionStrategy: getProviderTokens(oauth, request)
SessionStrategy->>OAuthProvider: Refresh when required
OAuthProvider-->>SessionStrategy: Return refreshed tokens
SessionStrategy-->>getProviderTokens: Return tokens, headers, or error
getProviderTokens-->>Client: Return JSON response
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/core/src/`@types/session.ts:
- Around line 239-245: Restore the optional skipCSRFCheck?: boolean parameter in
the refreshSession interface declaration, matching the third argument used by
the stateful.ts and stateless.ts implementations and preserving its
default-false behavior.
In `@packages/core/src/session/stateful.ts`:
- Line 664: Update all response branches in
packages/core/src/session/stateful.ts around the session response handling to
stop passing request.headers and instead construct fresh response-only headers,
merging only intentional Set-Cookie values. In
packages/core/src/session/stateless.ts around lines 211-215, remove
request-header merging, use the secure cookie builder after refresh, and use
fresh secureApiHeaders in other branches.
- Around line 689-704: Update the OAuth account flow in the stateful
provider-token handler to scope getOAuthAccount to both oauthId and
sessionByToken.userId, or verify the returned account belongs to that user
before reading tokens. Ensure the same ownership constraint is applied to
updateOAuthTokens so another user’s account cannot be disclosed or overwritten,
updating the adapter contract if needed.
- Around line 667-680: Update the session validation in the provider-token
retrieval flow around getSessionByToken to reject sessions whose status is
inactive or whose expiresAt is in the past, in addition to missing sessions or
users. Reuse the same validity criteria and invalid-session response already
established by getSession, while preserving the existing provider-token success
path for active, unexpired sessions.
In `@packages/core/src/shared/utils/refresh-tokens.ts`:
- Around line 27-34: Update the refresh-token request construction to omit
client-secret authentication entirely when provider.clientSecret is undefined,
preventing both createBasicAuthHeader and URLSearchParams from receiving an
undefined secret. In the same logic, replace the typeof provider.refreshToken
=== "object" checks with the existing isRefreshTokenObject guard for headers and
params access, preserving support for object-form refresh-token configuration.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 05149e4e-a060-4eae-ba9f-2061e9b1090f
📒 Files selected for processing (9)
packages/core/src/@types/session.tspackages/core/src/api/getProviderTokens.tspackages/core/src/router/context.tspackages/core/src/session/stateful.tspackages/core/src/session/stateless.tspackages/core/src/session/strategy.tspackages/core/src/shared/logger.tspackages/core/src/shared/utils/refresh-tokens.tspackages/core/test/api/getProviderTokens.test.ts
143e617 to
07593e8
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/core/src/api/signInCredentials.ts`:
- Line 66: Remove the unconditional console.error call from the catch block in
signInCredentials; rely on the existing injected logger handling, including the
AUTH_CREDENTIALS_INVALID warning path, and keep behavior consistent with
getProviderTokens without emitting raw credential-related errors to stdout.
In `@packages/core/src/shared/utils.ts`:
- Around line 152-167: Update database-mode verifySession() to perform the
stateful session record and status validation instead of calling
verifyPresentSessionValue(), which only checks cookie presence. Reuse the
stateful strategy’s getSession() validation on the authorization and call paths,
preserving existing error handling and session-token behavior.
In `@packages/core/test/actions/providers/tokens/tokens/stateful.test.ts`:
- Around line 175-209: Rename the test case around the OAuth account absence
scenario to state that it returns 401, matching the existing response.status
assertion and the sibling session-not-found test. Do not change the test
behavior or assertions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f153da0f-ede7-440e-85b0-168b5e5f8a24
📒 Files selected for processing (20)
packages/core/src/@types/config.tspackages/core/src/@types/session.tspackages/core/src/api/getProviderTokens.tspackages/core/src/api/signInCredentials.tspackages/core/src/router/context.tspackages/core/src/session/stateful.tspackages/core/src/session/stateless.tspackages/core/src/session/strategy.tspackages/core/src/shared/logger.tspackages/core/src/shared/utils.tspackages/core/src/shared/utils/api.tspackages/core/src/shared/utils/refresh-tokens.tspackages/core/test/actions/providers/tokens/tokens/stateful.test.tspackages/core/test/actions/signIn/signInCredentials/stateful.test.tspackages/core/test/actions/signUp/stateful.test.tspackages/core/test/api/getProviderTokens.test.tspackages/core/test/api/stateful/getProviderTokens.test.tspackages/core/test/api/stateful/signInCredentials.test.tspackages/core/test/api/stateful/signUp.test.tspackages/core/test/presets.ts
🚧 Files skipped from review as they are similar to previous changes (8)
- packages/core/src/session/strategy.ts
- packages/core/test/api/getProviderTokens.test.ts
- packages/core/src/session/stateless.ts
- packages/core/src/session/stateful.ts
- packages/core/src/shared/utils/refresh-tokens.ts
- packages/core/src/shared/logger.ts
- packages/core/src/api/getProviderTokens.ts
- packages/core/src/@types/session.ts
Description
This pull request adds Stateful session support for the
getProviderTokens()APIs.The implementation extends both the server-side
api.getProviderTokens()API and theGET /providers/:provider/tokensendpoint to support the experimental Stateful session strategy. The new functionality has been thoroughly tested to ensure consistent behavior across both Stateless and Stateful session strategies.For more information about these APIs, see the official documentation:
api.getProviderTokens()client.getProviderTokens()Key Changes
api.getProviderTokens().GET /providers/:provider/tokensendpoint.Note
Expanding support for
getProviderTokens()uncovered several inconsistencies between the existing Stateless implementation and the new Stateful implementation. As part of this work, the authentication flow was refactored to provide a more consistent behavior across both strategies. These improvements also affected related flows such assignUp(),signInCredentials(), andgetProviderTokens().Note
This PR is part of the ongoing effort to implement the Stateful session strategy. To keep reviews focused and manageable, the implementation has been split into a series of smaller pull requests, each covering a specific aspect of the feature.
Related PRs
@coderabbitai ignore