Conversation
WalkthroughIntroduces OAuth launch functionality by adding a new OAuthLaunchComponent that orchestrates OAuth login flows, supporting both native and web builds with popup-based authentication, error handling, and configurable redirect behavior. Exports the component from the auth module and integrates it into a new /oauth/launch route. Changes
Sequence DiagramsequenceDiagram
participant Mount as OAuthLaunchComponent<br/>(Mount)
participant Auth as AuthService
participant Server as ServerService
participant UI as Popup Window
participant Callback as onAuthenticated<br/>Callback
Mount->>Auth: Subscribe to authentication<br/>status via useLiveData
Mount->>Server: Compute effective<br/>redirect URL
activate Mount
Note over Mount: Initiate OAuth flow<br/>(onContinue)
alt Native Build
Mount->>UI: Preflight check &<br/>scheme-based popup
else Web Build
Mount->>UI: Construct oauth/login URL<br/>with provider & redirect_uri<br/>+ open popup
end
deactivate Mount
UI->>Auth: User authenticates<br/>& sign-in event triggered
Auth-->>Mount: Authentication status<br/>updated
alt Authentication Successful
Mount->>UI: Display success toast
Mount->>Callback: Invoke callback with<br/>AuthSessionStatus
Callback->>Callback: Handle redirect or<br/>close popup
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–25 minutes Areas requiring extra attention:
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 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: 0
🧹 Nitpick comments (2)
packages/frontend/core/src/desktop/pages/auth/oauth-launch.tsx (1)
15-25: Consider forwardingredirectUrlintoOAuthLaunchComponentThe page computes a
redirectUrl(from props orredirect_uriquery) and uses it inhandleAuthenticated, butOAuthLaunchComponentalso exposes aredirectUrlprop that currently isn’t used here:<OAuthLaunchComponent onAuthenticated={handleAuthenticated} />If the intent is for the same
redirectUrlto control both:
- the final in-app navigation/close behavior (handled here), and
- the
redirect_uripassed into the OAuth login URL (handled insideOAuthLaunchComponent),then you likely want to pass it through:
- <OAuthLaunchComponent onAuthenticated={handleAuthenticated} /> + <OAuthLaunchComponent + redirectUrl={redirectUrl ?? undefined} + onAuthenticated={handleAuthenticated} + />Otherwise you end up always using the default
/oauth/callbackas the OAuthredirect_uri, regardless of theredirect_uri/redirectUrlpassed into this page.Also applies to: 43-66
packages/frontend/core/src/components/affine/auth/oauth-launch-component.tsx (1)
71-81: Tighten success toast &onAuthenticatedsemantics to avoid duplicatesThe effect:
useEffect(() => { if (loginStatus === 'authenticated') { notify.success(...); } onAuthenticated?.(loginStatus); }, [loginStatus, onAuthenticated, t]);fires on every
loginStatusemission, so repeated'authenticated'emissions will show multiple success toasts and invokeonAuthenticatedmultiple times.If you only care about the transition to authenticated once, consider tracking the previous status in a ref and only:
- showing the toast, and
- calling
onAuthenticatedwhen
prevStatus !== 'authenticated' && loginStatus === 'authenticated'.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
packages/frontend/core/src/components/affine/auth/index.ts(1 hunks)packages/frontend/core/src/components/affine/auth/oauth-launch-component.tsx(1 hunks)packages/frontend/core/src/desktop/pages/auth/oauth-launch.tsx(1 hunks)packages/frontend/core/src/desktop/router.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-27T03:23:11.880Z
Learnt from: CR
Repo: toeverything/AFFiNE PR: 0
File: packages/frontend/apps/ios/AGENTS.md:0-0
Timestamp: 2025-11-27T03:23:11.880Z
Learning: Applies to packages/frontend/apps/ios/**/*.{ts,tsx} : Expose JavaScript APIs to native iOS code through window object for Capacitor integration
Applied to files:
packages/frontend/core/src/components/affine/auth/index.ts
🧬 Code graph analysis (2)
packages/frontend/core/src/desktop/pages/auth/oauth-launch.tsx (3)
packages/frontend/core/src/components/hooks/use-navigate-helper.ts (1)
useNavigateHelper(24-268)packages/frontend/core/src/modules/cloud/entities/session.ts (1)
AuthSessionStatus(39-42)packages/frontend/core/src/components/affine/auth/oauth-launch-component.tsx (1)
OAuthLaunchComponent(13-88)
packages/frontend/core/src/components/affine/auth/oauth-launch-component.tsx (5)
packages/frontend/core/src/modules/cloud/entities/session.ts (1)
AuthSessionStatus(39-42)packages/common/infra/src/framework/react/index.tsx (1)
useService(15-17)packages/frontend/core/src/modules/url/services/url.ts (1)
UrlService(6-58)packages/frontend/core/src/components/hooks/affine-async-hooks.ts (1)
useAsyncCallback(18-30)packages/backend/server/src/base/error/def.ts (1)
UserFriendlyError(63-166)
🔇 Additional comments (3)
packages/frontend/core/src/components/affine/auth/index.ts (1)
1-1: OAuthLaunchComponent re-export looks goodRe-exporting
OAuthLaunchComponentfrom the auth barrel file cleanly exposes the new component without changing behavior elsewhere.packages/frontend/core/src/desktop/router.tsx (1)
153-157: New/oauth/launchroute wiring is consistentThe new
/oauth/launchroute follows the existing auth routing pattern (lazy import under the"auth"chunk and placement alongside other OAuth routes). No issues from a router perspective.packages/frontend/core/src/components/affine/auth/oauth-launch-component.tsx (1)
26-60: Auto-launching OAuth popup from an effect may be blocked in browsers
onContinueultimately callsurlService.openPopupWindow(...), and it's invoked from auseEffecton mount without a direct user gesture. TheUrlService.openPopupWindowdocumentation warns that popup windows called from async callbacks/effects can be blocked by browsers, which may break the OAuth launch flow in web environments.Consider one of the following:
- For web builds, switch to
urlService.openExternal(oauthUrl)so the tab navigates instead of opening a popup, or- Gate
onContinuebehind an explicit user action (e.g. a "Continue" button) when running in a browser, keeping auto-launch only for native/electron where popup blocking is not an issue, or- Introduce a dedicated UrlService method tailored for this launch route that internally chooses the safest behavior per platform.
Also, please double-check that
UserFriendlyError.fromAny(e)(in the native branch) returns an object shape thatnotify.errorcan render correctly.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## canary #14071 +/- ##
==========================================
- Coverage 57.10% 56.53% -0.58%
==========================================
Files 2757 2757
Lines 138059 138059
Branches 21156 21048 -108
==========================================
- Hits 78843 78056 -787
- Misses 56944 57738 +794
+ Partials 2272 2265 -7
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Implements feature outlined here
OIDC Launch URL
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.