return to options#898
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughAdds an optional options parameter with returnTo to signInWithOAuth. The implementation now passes options.returnTo to the OAuth flow when provided, otherwise defaults to the existing oauthCallback. The public interface is updated accordingly; no other behavior changes. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant C as StackClientApp
participant O as OAuth Service
U->>C: signInWithOAuth(provider, { returnTo? })
alt returnTo provided
C->>O: initiateOAuth(provider, redirect=returnTo)
else no returnTo
C->>O: initiateOAuth(provider, redirect=oauthCallback)
end
Note over O,C: OAuth provider redirects to chosen target
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
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 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Greptile Summary
This PR adds an optional returnTo parameter to the signInWithOAuth method in the Stack Auth client library. The change allows developers to specify a custom redirect URL where users should be sent after completing OAuth authentication, rather than always using the default OAuth callback URL.
The implementation involves two key changes:
- Interface Update: The
ClientAppInterfacenow definessignInWithOAuthwith an optionaloptionsparameter containing areturnTofield - Implementation Update: The
ClientAppImplclass now accepts this options parameter and usesoptions?.returnTo ?? this.urls.oauthCallbackas the redirect URL, maintaining backward compatibility by falling back to the default callback URL when no custom return URL is provided
This enhancement integrates seamlessly with the existing OAuth flow infrastructure, which already supports custom redirect URLs through the redirectUrl parameter in the underlying signInWithOAuth helper function. The change maintains the existing error handling and state management while simply allowing the redirect destination to be customized.
Confidence score: 4/5
- This PR is safe to merge with low risk as it's an additive change that maintains backward compatibility
- Score reflects straightforward implementation with proper fallback handling, though limited test coverage visibility
- Pay attention to the interface and implementation files to ensure the API contract is properly maintained
2 files reviewed, no comments
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
| { | ||
| provider, | ||
| redirectUrl: this.urls.oauthCallback, | ||
| redirectUrl: options?.returnTo ?? this.urls.oauthCallback, |
There was a problem hiding this comment.
Consider validating the 'returnTo' URL to ensure it is a trusted, relative URL. Unchecked, an arbitrary URL could lead to an open redirect vulnerability.
| redirectUrl: options?.returnTo ?? this.urls.oauthCallback, | |
| redirectUrl: isRelative(options?.returnTo) ? options.returnTo : this.urls.oauthCallback, |
Documentation Changes RequiredFiles to Update:
Changes Needed:
Note:The documentation in
Please ensure these changes are reflected in the relevant documentation files, except for |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/template/src/lib/stack-app/apps/implementations/client-app-impl.ts (1)
1600-1617: Bug + open-redirect risk: don’t replace OAuth callback with returnTo
redirectUrlmust point to the OAuth callback handler. Passingoptions.returnTodirectly asredirectUrlbreaks the callback flow and can enable open redirects if arbitrary URLs are allowed. Instead, keepredirectUrl = this.urls.oauthCallbackand propagatereturnTovia a safe, relative query param (or use an explicitafterCallbackRedirectUrloption if supported by the backend).Apply this diff:
- async signInWithOAuth(provider: ProviderType, options?: { returnTo?: string }) { + async signInWithOAuth(provider: ProviderType, options?: { returnTo?: string }) { if (typeof window === "undefined") { throw new Error("signInWithOAuth can currently only be called in a browser environment"); } this._ensurePersistentTokenStore(); const session = await this._getSession(); - await signInWithOAuth( + // Always use the OAuth callback as redirect target + let redirectUrl = this.urls.oauthCallback; + if (options?.returnTo) { + // enforce relative returnTo to avoid open-redirects + if (!(await this._isTrusted(options.returnTo))) { + throw new Error("returnTo must be a relative URL"); + } + const base = new URL(window.location.href); + const cb = new URL(this.urls.oauthCallback, base); + cb.searchParams.set("after_auth_return_to", getRelativePart(new URL(options.returnTo, base))); + redirectUrl = getRelativePart(cb); + } + + await signInWithOAuth( this._interface, { provider, - redirectUrl: options?.returnTo ?? this.urls.oauthCallback, + redirectUrl, errorRedirectUrl: this.urls.error, providerScope: this._oauthScopesOnSignIn[provider]?.join(" "), }, session, ); }
🧹 Nitpick comments (3)
packages/template/src/lib/stack-app/apps/interfaces/client-app.ts (1)
41-41: Public API: add options.returnTo — document and align types
- Please document that
options.returnTomust be a relative path (security).- Consider using the stronger
ProviderTypehere for parity with the implementation.Apply this diff to align the type and add brief docs:
@@ - signInWithOAuth(provider: string, options?: { returnTo?: string }): Promise<void>, + /** + * Starts an OAuth sign-in. If `options.returnTo` is set, the app will return to that relative path after the OAuth callback completes. + * Note: `returnTo` must be a relative URL (e.g., "/dashboard"). + */ + signInWithOAuth(provider: ProviderType, options?: { returnTo?: string }): Promise<void>,And add at the top:
+import { ProviderType } from "@stackframe/stack-shared/dist/utils/oauth";packages/template/src/lib/stack-app/apps/implementations/client-app-impl.ts (2)
1600-1600: Optional: extract and reuse a typed options objectDefine a shared
SignInWithOAuthOptionstype and reuse it here and in the interface for consistency.- async signInWithOAuth(provider: ProviderType, options?: { returnTo?: string }) { + type SignInWithOAuthOptions = { returnTo?: string }; + async signInWithOAuth(provider: ProviderType, options?: SignInWithOAuthOptions) {
1600-1617: Send explicit afterCallbackRedirectUrl from signInWithOAuth (backend already supports it)Backend / client-interface already accept afterCallbackRedirectUrl and addNewOAuthProviderOrScope already sends it, but signInWithOAuth does not — it currently embeds return paths into the callback URL. Change signInWithOAuth to pass afterCallbackRedirectUrl to iface.getOAuthUrl instead of encoding after_auth_return_to into redirectUrl.
Files to check: packages/template/src/lib/auth.ts (signInWithOAuth / addNewOAuthProviderOrScope), packages/stack-shared/src/interface/client-interface.ts, packages/template/src/lib/stack-app/apps/implementations/client-app-impl.ts.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/template/src/lib/stack-app/apps/implementations/client-app-impl.ts(2 hunks)packages/template/src/lib/stack-app/apps/interfaces/client-app.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
packages/template/**
📄 CodeRabbit inference engine (AGENTS.md)
When modifying the SDK copies, make changes in packages/template (source of truth)
Files:
packages/template/src/lib/stack-app/apps/implementations/client-app-impl.tspackages/template/src/lib/stack-app/apps/interfaces/client-app.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Prefer ES6 Map over Record when representing key–value collections
Files:
packages/template/src/lib/stack-app/apps/implementations/client-app-impl.tspackages/template/src/lib/stack-app/apps/interfaces/client-app.ts
🧬 Code graph analysis (1)
packages/template/src/lib/stack-app/apps/implementations/client-app-impl.ts (1)
packages/stack-shared/src/utils/oauth.tsx (1)
ProviderType(6-6)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: docker
- GitHub Check: build (22.x)
- GitHub Check: restart-dev-and-test
- GitHub Check: setup-tests
- GitHub Check: lint_and_build (latest)
- GitHub Check: build (22.x)
- GitHub Check: docker
- GitHub Check: all-good
- GitHub Check: Security Check
2 participants
High-level PR Summary
This PR adds an optional
returnToparameter to thesignInWithOAuthfunction in the Stack Auth SDK. This allows developers to specify a custom redirect URL after OAuth authentication instead of always using the default OAuth callback URL. The change updates both the interface definition and the implementation to support this new optional parameter.⏱️ Estimated Review Time: 0h 15m
💡 Review Order Suggestion
packages/template/src/lib/stack-app/apps/interfaces/client-app.tspackages/template/src/lib/stack-app/apps/implementations/client-app-impl.tsImportant
Add optional
returnToparameter tosignInWithOAuthfor custom redirect URLs inclient-app-impl.tsand update interface inclient-app.ts.signInWithOAuthinclient-app-impl.tsnow accepts an optionaloptionsparameter withreturnTofor custom redirect URLs.this.urls.oauthCallbackifreturnTois not provided.signInWithOAuthsignature inclient-app.tsto include optionaloptionsparameter withreturnTo.This description was created by
for eb748cb. You can customize this summary. It will automatically update as commits are pushed.
Review by RecurseML
🔍 Review performed on 2d2a6d7..eb748cb
✨ No bugs found, your code is sparkling clean
✅ Files analyzed, no issues (2)
•
packages/template/src/lib/stack-app/apps/implementations/client-app-impl.ts•
packages/template/src/lib/stack-app/apps/interfaces/client-app.tsSummary by CodeRabbit