feat(pocket-ios): Google OIDC sign-in via ASWebAuthenticationSession#1798
feat(pocket-ios): Google OIDC sign-in via ASWebAuthenticationSession#1798lulu0119 wants to merge 29 commits into
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aa94b91361
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| @@ -1,6 +1,7 @@ | |||
| import { Capacitor } from '@capacitor/core' | |||
There was a problem hiding this comment.
Declare Capacitor where stage-ui imports it
When building or typechecking @proj-airi/stage-ui (and when web/tamagotchi resolve this shared source under pnpm), this bare import is resolved from packages/stage-ui, but @capacitor/core is only declared by apps/stage-pocket and was not added to packages/stage-ui/package.json or the lockfile. With pnpm's strict package-local node_modules, Vite/vue-tsc cannot resolve the module before runtime, so shared auth consumers fail to build; please add the dependency to stage-ui or move the Capacitor-specific code behind an app-owned adapter.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Code Review
This pull request introduces native iOS authentication support using ASWebAuthenticationSession and adds an ADDITIONAL_TRUSTED_ORIGINS configuration to handle CORS and redirect URIs for development environments like LAN IPs. It also improves the ngrok development experience by automatically including the skip-warning header and refactors OIDC state persistence for native platforms. Feedback was provided regarding a potential bug in the OIDC redirect URI construction where /auth/callback might be double-appended if a full URL is supplied via environment variables.
| export const OIDC_REDIRECT_URI = import.meta.env.VITE_OIDC_REDIRECT_URI | ||
| ? `${origin}/auth/callback` | ||
| : capacitorPlatform === 'ios' | ||
| ? NATIVE_AUTH_REDIRECT_URI | ||
| : capacitorPlatform !== 'web' | ||
| ? CAPACITOR_WEBVIEW_REDIRECT_URI | ||
| : `${origin}/auth/callback` |
There was a problem hiding this comment.
The logic for OIDC_REDIRECT_URI appears to double-append /auth/callback if VITE_OIDC_REDIRECT_URI is set to a full URL. Since getRedirectOrigin returns VITE_OIDC_REDIRECT_URI directly when present, and this line appends /auth/callback to it, a value like https://example.com/auth/callback would result in https://example.com/auth/callback/auth/callback. If the environment variable is intended to be the full redirect URI, it should be used as-is. Additionally, ensure that all required environment variables are validated on startup to prevent runtime errors.
| export const OIDC_REDIRECT_URI = import.meta.env.VITE_OIDC_REDIRECT_URI | |
| ? `${origin}/auth/callback` | |
| : capacitorPlatform === 'ios' | |
| ? NATIVE_AUTH_REDIRECT_URI | |
| : capacitorPlatform !== 'web' | |
| ? CAPACITOR_WEBVIEW_REDIRECT_URI | |
| : `${origin}/auth/callback` | |
| export const OIDC_REDIRECT_URI = import.meta.env.VITE_OIDC_REDIRECT_URI | |
| ? import.meta.env.VITE_OIDC_REDIRECT_URI | |
| : capacitorPlatform === 'ios' | |
| ? NATIVE_AUTH_REDIRECT_URI | |
| : capacitorPlatform !== 'web' | |
| ? CAPACITOR_WEBVIEW_REDIRECT_URI | |
| : origin + '/auth/callback' |
References
- Validate environment variables on startup to ensure all required variables are present, preventing runtime errors from undefined values.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1e7f83638e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| // such as ngrok are not involved in request-time token verification. | ||
| cachedJWKS = createRemoteJWKSet( | ||
| new URL('/api/auth/jwks', env.API_SERVER_URL), | ||
| new URL('/api/auth/jwks', `http://127.0.0.1:${env.PORT}`), |
There was a problem hiding this comment.
Use the configured listener for JWKS lookups
When the server is started with HOST bound to a specific non-loopback interface (for example HOST=10.0.0.5 to expose the API on a LAN), this now fetches JWKS from 127.0.0.1:${env.PORT} even though the process is not listening on loopback. In that supported configuration createRemoteJWKSet cannot load keys, resolveJWTAccessToken falls through to null, and every OIDC Bearer-authenticated API request is rejected; please derive the local JWKS URL from the actual listener host or fall back to API_SERVER_URL when loopback is not bound.
Useful? React with 👍 / 👎.
1e7f836 to
5d952ea
Compare
⏳ Approval required for deploying to Cloudflare Workers (Preview) for stage-web.
Hey, maintainers, kindly take some time to review and approve this deployment when you are available. Thank you! 🙏 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5253229214
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const jwksHost = host === '0.0.0.0' || host === '127.0.0.1' || host === 'localhost' | ||
| ? '127.0.0.1' | ||
| : host | ||
| return new URL('/api/auth/jwks', `http://${jwksHost}:${env.PORT}`) |
There was a problem hiding this comment.
Bracket IPv6 listener hosts in JWKS URL builder
When HOST is configured as an IPv6 address (for example :: or ::1), interpolating it directly into http://${jwksHost}:${env.PORT} produces an invalid URL (e.g. http://:::3000), so createRemoteJWKSet throws and bearer-token auth silently falls back to null for every request. This is a regression from the previous API_SERVER_URL-based path and breaks JWT auth in valid IPv6 deployments; wrap IPv6 hosts in [] before constructing the base URL.
Useful? React with 👍 / 👎.
|
@codex review |
|
Codex Review: Didn't find any major issues. Breezy! ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
c2909cf to
921b8e1
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c2909cf576
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const lookupHost = host === '0.0.0.0' || host === '127.0.0.1' || host === 'localhost' | ||
| ? '127.0.0.1' |
There was a problem hiding this comment.
Keep localhost resolution family when deriving JWKS origin
Do not coerce HOST=localhost to 127.0.0.1 here. In deployments where Node binds localhost to IPv6 (::1), forcing the JWKS fetch to IPv4 makes createRemoteJWKSet hit a non-listening address, so JWT verification fails and bearer-authenticated requests are rejected. This regression is introduced by treating 'localhost' as equivalent to '127.0.0.1' instead of preserving the configured host.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 70705041d9
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| 'fetch(cfg.postUrl,{method:\'POST\',headers:{\'Content-Type\':\'application/json\'},body:JSON.stringify(cfg.body),credentials:\'include\'})', | ||
| '.then(function(r){return r.json().then(function(j){return{ok:r.ok,status:r.status,j:j};});})', |
There was a problem hiding this comment.
Handle redirect responses from social sign-in bridge
The new /auth/sign-in?provider=... bridge unconditionally parses /api/auth/sign-in/social as JSON, but this endpoint can validly return an HTTP redirect instead of a JSON body (the existing apps/ui-server-auth/src/modules/sign-in.ts flow already treats 302/opaqueredirect as success and uses redirect: 'manual'). In that redirect case, this fetch path either follows cross-origin and fails or throws on r.json(), so provider sign-in aborts with “Sign-in request failed” instead of navigating to the OAuth provider.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
@codex review
Thanks for the review on the OIDC social sign-in bridge.
Re: aligning with requestSocialSignInRedirect in ui-server-auth — we’ve updated the bridge inline fetch to use redirect: 'manual' and to handle 302 / opaqueredirect via Location, same as apps/ui-server-auth/src/modules/sign-in.ts.
On when the SPA auto-start runs: in the current flow, sign-in.vue’s watch(requestedProvider) is short-circuited for the common OIDC path. A full-page GET /auth/sign-in?provider=google|github&… (after an unauthenticated /api/auth/oauth2/authorize redirect) is handled by the dedicated Hono route in apps/server/src/routes/auth/index.ts, which returns renderOidcSocialPostBridgeHtml before renderServerAuthUiHtml. Vue never mounts, so that watch does not run.
The SPA watch only applies when the SPA shell is actually served — e.g. /auth/sign-in without a valid provider query (picker / email flow), or the edge case of client-side navigation within an already-loaded SPA that adds ?provider= without a full reload. That is not the Pocket / signInOIDC({ provider }) fast path, where the bridge POST is intentional (POST-only /api/auth/sign-in/social, avoid a GET redirect, and skip loading the full ui-server-auth bundle in the WebView).
Happy to hear if you see a scenario where we should drop the bridge and rely on the SPA watch only.
|
@codex review Updates on your inline comments (head
Bridge vs SPA Please re-review when you can. |
|
Re main Default Pocket sign-in still loads |
|
@codex review |
|
Codex Review: Didn't find any major issues. Chef's kiss. ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
…stage-web Made-with: Cursor
…itor WebView Made-with: Cursor
- isNgrokServerUrl / applyNgrokSkipRequestHeader: free ngrok interstitial; pass header on Hono client, token/refresh, better-auth client fetch - buildAuthorizationURL with full params (keep provider= on authorize URL) - Capacitor: use localStorage for PKCE state; full-page location.assign(authorize) so ensureDynamicFirstPartyRedirectUri runs before social (skip signIn.social in-app on native) Tighten or replace ngrok host sniffing when a stable config exists. Made-with: Cursor
When navigating to an ngrok host, reload with ngrok-skip-browser-warning if missing (WKWebView cannot set headers on top-level location). Tidy provisional navigation log; add -1200 hint for TLS on LAN (dev). DEBUG-only path. Made-with: Cursor
Use ASWebAuthenticationSession with an app-owned callback on iOS so Google OAuth no longer runs inside the Capacitor WKWebView. Made-with: Cursor
…ridge Drop WKNavigationDelegate branch that reloaded ngrok hosts with ngrok-skip-browser-warning; API requests still add the header in stage-ui when SERVER_URL is ngrok. Made-with: Cursor
Made-with: Cursor
Replace the reverted Caddy workflow with ngrok-oriented guidance in apps/server README. Update DevBridge TLS hint to match (no dev/caddy path). Made-with: Cursor
Made-with: Cursor Co-authored-by: Cursor <cursoragent@cursor.com>
…or Capacitor WebView" This reverts commit 2a5247f.
Restores parity with auth-config tests when window.Capacitor is unset but @capacitor/core is mocked (Vitest). Co-authored-by: Cursor <cursoragent@cursor.com>
Rolldown could not resolve @capacitor/core from stage-ui imports; stub Capacitor/registerPlugin for the browser-only auth bundle. Co-authored-by: Cursor <cursoragent@cursor.com>
- Auth store: replace router push to /auth/sign-in with dynamic triggerSignIn import (avoids cycle with libs/auth). - stage-pocket auth callback: retry uses triggerSignIn; use errorMessageFrom for exchange errors. Co-authored-by: Cursor <cursoragent@cursor.com>
Login uses triggerSignIn / server-hosted ui-server-auth; pocket no longer needs /auth/sign-in route. Co-authored-by: Cursor <cursoragent@cursor.com>
pnpm strict resolution requires Capacitor where stage-ui imports it for OIDC and native auth helpers. Co-authored-by: Cursor <cursoragent@cursor.com>
LAN dev with HOST set to a concrete interface failed token verification because JWKS was fetched from 127.0.0.1 while the API listened elsewhere. Co-authored-by: Cursor <cursoragent@cursor.com>
Avoid appending /auth/callback when the env already contains the complete callback URL. Co-authored-by: Cursor <cursoragent@cursor.com>
Block javascript:, file:, and other inputs that normalize to the opaque origin "null" so misconfiguration cannot trust Origin: null. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Bearer JWT verification failed when HOST was :: or ::1 because the JWKS fetch URL was not a valid HTTP authority. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
e4de0fc to
943d249
Compare
Co-authored-by: Cursor <cursoragent@cursor.com>
…u0119/airi into feat/pocket-oidc-server-auth
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c27c2b115b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| export const OIDC_REDIRECT_URI = import.meta.env.VITE_OIDC_REDIRECT_URI | ||
| ? import.meta.env.VITE_OIDC_REDIRECT_URI |
There was a problem hiding this comment.
Keep iOS redirect scheme aligned with native callback
When an iOS Pocket build sets VITE_OIDC_REDIRECT_URI to a non-airi-pocket URI, this override becomes the OAuth redirect_uri, but the native launcher still starts ASWebAuthenticationSession with the fixed airi-pocket callback scheme. In that configuration the provider redirects to the configured URI and the native promise never receives the callback, so sign-in hangs; either reject/ignore mismatched overrides on iOS or derive the native callback scheme from the selected redirect URI.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
On native Pocket, redirect_uri is platform-owned (airi-pocket://auth/callback + matching callbackScheme); VITE_OIDC_REDIRECT_URI is web-only and should not be set on iOS/Android builds.
Co-authored-by: Cursor <cursoragent@cursor.com>
Description
This PR is intentionally narrow: Stage Pocket on iOS, Google only, using the system
ASWebAuthenticationSessionto complete OIDC authorization code + PKCE, then returning to the app via theairi-pocket://callback scheme. It does not claim full parity for Android, GitHub-in-native-session, or all IdP edge cases yet.What problem this solves
Google sign-in in a captive
WKWebViewis the wrong shapeGoogle’s OAuth guidance pushes toward a trusted system browser context, not a generic in-app WebView for the consent screen.
ASWebAuthenticationSessionis the iOS-native way to present that flow and receive the redirect back on a custom URL scheme.Same OIDC shape as stage-web
The app still drives
/api/auth/oauth2/authorize+ PKCE and completes the callback with the same token/session helpers as web, but starts the user-visible Google portion in the native session on iOS (fix(pocket): open Google auth in system sessionand related wiring).Supporting server / UI work in this branch
Other commits (trusted origins, JWKS, ui-server-auth build shim,
needsLogin→triggerSignIn) support dev and hosted login so the iOS Google path can land onui-server-authand API calls behave behind https tunnels. They are not “all providers on all platforms done.”Why
ASWebAuthenticationSession(and custom scheme)airi-stage-pocketwithairi-pocket://auth/callbackso the session can hand off the authorization response outside the main WebView (see comment inapps/server/src/libs/auth.tsaround the Capacitor / Pocket OIDC client).Dev API tunnel: ngrok → frp
Early iOS testing used ngrok free tier (
https→ local:3000). That tier serves an interstitial unless requests sendngrok-skip-browser-warning: true;WKWebViewoften cannot dismiss that page like desktop Safari, so API/auth traffic can stall. That is why the branch briefly carried ngrok-specific client handling.We later moved dev to self-hosted frp (and similar reverse proxies) instead of ngrok. Frp does not need that header, so the final branch drops the ngrok-only workaround and documents generic https reverse proxy / frp in
apps/server/README.md.Linked Issues
Additional Context
airi-pocket://auth/callbackhandling, and that Info.plist / URL scheme registration matches serverredirect_uris.*.argotunnel.com, which is commonly blocked or poisoned.