Add direct CloudKit auth fallback#227
Conversation
📝 WalkthroughWalkthroughAdds a direct CloudKit web-auth fallback to the iCloud OAuth flow: fetches an auth challenge, opens a popup for Apple sign-in, listens for a postMessage token, and stores it. Updates related types, button theming, documentation, and adds unit tests for success and failure paths. ChangesDirect CloudKit Auth Fallback
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Poem A rabbit taps the popup pane, 🚥 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 |
Preview deployed |
nook-core + nook-auth coveragePASS: line coverage passed: 92.23% (floor 90.0%)
Artifact: nook-core-coverage |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@nook-app/nook-web/src/lib/icloud-oauth.ts`:
- Around line 924-935: The timeout branch in the CloudKit direct web auth flow
leaves the Apple sign-in popup open; update the timeout handler in the auth flow
around the cleanup/reject logic to also close the popup window just like the
success path does. Use the same auth window handle used by the sign-in promise,
and make sure it is closed before rejecting after timeout so the UI is cleaned
up consistently.
- Around line 815-851: The initial CloudKit auth challenge request can hang
indefinitely because fetchCloudKitWebAuthChallenge() performs a bare fetch with
no abort path. Add timeout handling with AbortController in
fetchCloudKitWebAuthChallenge, and thread the timeoutMs from
requestDirectCloudKitWebAuthToken into that call so the direct-web fallback
fails fast when the network stalls. Make sure the timeout is enforced before the
popup/message-listening Promise path continues, and keep the existing error
handling/logging around the CloudKit challenge response.
- Around line 874-895: Open the auth popup synchronously before any awaits in
requestDirectCloudKitWebAuthToken, because the current await
fetchCloudKitWebAuthChallenge() can clear user activation and cause
Safari/Chrome to block window.open(). Adjust the flow so a blank placeholder
window is opened immediately, then after the challenge is fetched set that
window’s location to challenge.redirectURL, and keep the existing popup-blocked
handling using currentBrowserDiagnostics/sanitizedURLDiagnostics.
- Around line 905-923: The CloudKit auth message handler in handleMessage
currently accepts any window message that parses as a token, so add an origin
check before token handling and ignore messages not from the expected Apple auth
origin. Use the existing event.origin in the message listener around
webAuthTokenFromMessageData and only proceed with cleanup,
storeCloudKitWebAuthToken, authWindow.close, and resolve when the origin matches
https://idmsa.apple.com.
🪄 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: 244a7e6d-5204-4c2e-aa24-6218986b9e14
📒 Files selected for processing (3)
.cortex/design-docs/auth-providers.mdnook-app/nook-web/src/lib/icloud-oauth.tsnook-app/nook-web/tests/unit/lib/icloud-oauth.test.ts
| function cloudKitCurrentUserURL(): string { | ||
| const container = encodeURIComponent(ICLOUD_CONTAINER_ID) | ||
| const environment = encodeURIComponent(ICLOUD_ENVIRONMENT) | ||
| const apiToken = encodeURIComponent(ICLOUD_API_TOKEN) | ||
| return `https://api.apple-cloudkit.com/database/1/${container}/${environment}/public/users/current?ckAPIToken=${apiToken}` | ||
| } | ||
|
|
||
| async function fetchCloudKitWebAuthChallenge(): Promise<CloudKitAuthChallenge> { | ||
| const response = await fetch(cloudKitCurrentUserURL(), { | ||
| method: 'GET', | ||
| headers: { Accept: 'application/json' }, | ||
| }) | ||
| const body = (await response | ||
| .json() | ||
| .catch(() => ({}))) as CloudKitAuthChallenge | ||
| log.info('CloudKit direct web auth challenge received', { | ||
| status: response.status, | ||
| ok: response.ok, | ||
| serverErrorCode: body.serverErrorCode, | ||
| reason: body.reason, | ||
| redirectURL: sanitizedURLDiagnostics(body.redirectURL), | ||
| uuidPresent: Boolean(body.uuid), | ||
| }) | ||
| if (body.serverErrorCode === 'AUTHENTICATION_REQUIRED' && body.redirectURL) { | ||
| return body | ||
| } | ||
| if (body.serverErrorCode === 'AUTHENTICATION_FAILED') { | ||
| throw new Error( | ||
| 'Apple rejected the iCloud API token for this container. Check the CloudKit production API token and allowed origin https://nokey.sh.', | ||
| ) | ||
| } | ||
| throw new Error( | ||
| body.reason ?? | ||
| body.serverErrorCode ?? | ||
| `Apple CloudKit auth challenge failed with HTTP ${response.status}.`, | ||
| ) | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Challenge fetch has no timeout guard.
fetchCloudKitWebAuthChallenge awaits fetch() with no timeout/abort. The timeoutMs passed into requestDirectCloudKitWebAuthToken only guards the popup/message-listening Promise created afterward (lines 895-935); if this initial fetch stalls (slow/broken network, hung connection), the whole fallback hangs indefinitely with no way for waitForCloudKitSignIn's caller to recover via the configured timeout.
🔧 Proposed fix using AbortController
-async function fetchCloudKitWebAuthChallenge(): Promise<CloudKitAuthChallenge> {
- const response = await fetch(cloudKitCurrentUserURL(), {
- method: 'GET',
- headers: { Accept: 'application/json' },
- })
+async function fetchCloudKitWebAuthChallenge(
+ timeoutMs: number,
+): Promise<CloudKitAuthChallenge> {
+ const controller = new AbortController()
+ const timer = setTimeout(() => controller.abort(), timeoutMs)
+ const response = await fetch(cloudKitCurrentUserURL(), {
+ method: 'GET',
+ headers: { Accept: 'application/json' },
+ signal: controller.signal,
+ }).finally(() => clearTimeout(timer))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function cloudKitCurrentUserURL(): string { | |
| const container = encodeURIComponent(ICLOUD_CONTAINER_ID) | |
| const environment = encodeURIComponent(ICLOUD_ENVIRONMENT) | |
| const apiToken = encodeURIComponent(ICLOUD_API_TOKEN) | |
| return `https://api.apple-cloudkit.com/database/1/${container}/${environment}/public/users/current?ckAPIToken=${apiToken}` | |
| } | |
| async function fetchCloudKitWebAuthChallenge(): Promise<CloudKitAuthChallenge> { | |
| const response = await fetch(cloudKitCurrentUserURL(), { | |
| method: 'GET', | |
| headers: { Accept: 'application/json' }, | |
| }) | |
| const body = (await response | |
| .json() | |
| .catch(() => ({}))) as CloudKitAuthChallenge | |
| log.info('CloudKit direct web auth challenge received', { | |
| status: response.status, | |
| ok: response.ok, | |
| serverErrorCode: body.serverErrorCode, | |
| reason: body.reason, | |
| redirectURL: sanitizedURLDiagnostics(body.redirectURL), | |
| uuidPresent: Boolean(body.uuid), | |
| }) | |
| if (body.serverErrorCode === 'AUTHENTICATION_REQUIRED' && body.redirectURL) { | |
| return body | |
| } | |
| if (body.serverErrorCode === 'AUTHENTICATION_FAILED') { | |
| throw new Error( | |
| 'Apple rejected the iCloud API token for this container. Check the CloudKit production API token and allowed origin https://nokey.sh.', | |
| ) | |
| } | |
| throw new Error( | |
| body.reason ?? | |
| body.serverErrorCode ?? | |
| `Apple CloudKit auth challenge failed with HTTP ${response.status}.`, | |
| ) | |
| } | |
| function cloudKitCurrentUserURL(): string { | |
| const container = encodeURIComponent(ICLOUD_CONTAINER_ID) | |
| const environment = encodeURIComponent(ICLOUD_ENVIRONMENT) | |
| const apiToken = encodeURIComponent(ICLOUD_API_TOKEN) | |
| return `https://api.apple-cloudkit.com/database/1/${container}/${environment}/public/users/current?ckAPIToken=${apiToken}` | |
| } | |
| async function fetchCloudKitWebAuthChallenge( | |
| timeoutMs: number, | |
| ): Promise<CloudKitAuthChallenge> { | |
| const controller = new AbortController() | |
| const timer = setTimeout(() => controller.abort(), timeoutMs) | |
| const response = await fetch(cloudKitCurrentUserURL(), { | |
| method: 'GET', | |
| headers: { Accept: 'application/json' }, | |
| signal: controller.signal, | |
| }).finally(() => clearTimeout(timer)) | |
| const body = (await response | |
| .json() | |
| .catch(() => ({}))) as CloudKitAuthChallenge | |
| log.info('CloudKit direct web auth challenge received', { | |
| status: response.status, | |
| ok: response.ok, | |
| serverErrorCode: body.serverErrorCode, | |
| reason: body.reason, | |
| redirectURL: sanitizedURLDiagnostics(body.redirectURL), | |
| uuidPresent: Boolean(body.uuid), | |
| }) | |
| if (body.serverErrorCode === 'AUTHENTICATION_REQUIRED' && body.redirectURL) { | |
| return body | |
| } | |
| if (body.serverErrorCode === 'AUTHENTICATION_FAILED') { | |
| throw new Error( | |
| 'Apple rejected the iCloud API token for this container. Check the CloudKit production API token and allowed origin https://nokey.sh.', | |
| ) | |
| } | |
| throw new Error( | |
| body.reason ?? | |
| body.serverErrorCode ?? | |
| `Apple CloudKit auth challenge failed with HTTP ${response.status}.`, | |
| ) | |
| } |
🤖 Prompt for 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.
In `@nook-app/nook-web/src/lib/icloud-oauth.ts` around lines 815 - 851, The
initial CloudKit auth challenge request can hang indefinitely because
fetchCloudKitWebAuthChallenge() performs a bare fetch with no abort path. Add
timeout handling with AbortController in fetchCloudKitWebAuthChallenge, and
thread the timeoutMs from requestDirectCloudKitWebAuthToken into that call so
the direct-web fallback fails fast when the network stalls. Make sure the
timeout is enforced before the popup/message-listening Promise path continues,
and keep the existing error handling/logging around the CloudKit challenge
response.
| async function requestDirectCloudKitWebAuthToken( | ||
| timeoutMs = ICLOUD_SIGN_IN_TIMEOUT_MS, | ||
| ): Promise<string> { | ||
| log.info('CloudKit direct web auth fallback started', { | ||
| timeoutMs, | ||
| browser: currentBrowserDiagnostics(), | ||
| }) | ||
| const challenge = await fetchCloudKitWebAuthChallenge() | ||
| const authWindow = window.open( | ||
| challenge.redirectURL, | ||
| 'nook-icloud-auth', | ||
| 'popup,width=520,height=720', | ||
| ) | ||
| if (!authWindow) { | ||
| log.warn('CloudKit direct web auth popup blocked', { | ||
| redirectURL: sanitizedURLDiagnostics(challenge.redirectURL), | ||
| }) | ||
| throw new Error( | ||
| 'Apple sign-in popup was blocked. Allow popups and try again.', | ||
| ) | ||
| } | ||
| return new Promise((resolve, reject) => { |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🌐 Web query:
Does calling window.open() after multiple chained awaits get blocked in Safari and Chrome even with recent user activation?
💡 Result:
Yes, calling window.open after multiple chained awaits is frequently blocked in both Safari and Chrome [1][2][3]. While modern browser "User Activation" models (like Chrome's User Activation v2) aim to propagate activation states through some asynchronous operations, they are not universally reliable for window.open [2][4]. Browser vendors maintain strict security policies to prevent spam, and they often require window.open to be called synchronously within the same execution context as the user gesture (e.g., a click event) to reliably bypass popup blockers [2][5]. The common technical constraints and workarounds include: 1. The "Await" Problem: When you await an asynchronous operation (like a network request), the JavaScript execution is paused and resumed outside the original synchronous call stack of the user gesture [2]. Even if the browser's activation token is technically still active, many implementations will treat the subsequent window.open call as untrusted if it is not directly linked to the user's action [2][3]. 2. The Standard Workaround: The widely accepted solution for this behavior is to open a blank window synchronously at the moment of the user gesture, and then update its location asynchronously once your data is ready [1][5]. Example of the safe pattern: const newWindow = window.open('', '_blank'); // Open synchronously during click try { const data = await fetch('/some-api'); // Perform async work const url = await data.json; if (newWindow) { newWindow.location.href = url; // Redirect the existing window } } catch (e) { newWindow?.close; // Clean up if operation fails window.location.href = errorUrl; // Fallback to current tab } This approach ensures the initial window creation is tied to the user gesture, which browsers permit, while the subsequent navigation is allowed because the window context is already established [1][5]. Using this pattern prevents the silent failures that occur when simply calling window.open after an await [1][2].
Citations:
- 1: fix(frontend): open customer portal in popup synchronously to bypass Safari blocker windmill-labs/windmill#9242
- 2: https://powernerdinsights.com/2025/05/01/web-resource-scripts-chromium-pop-up-blocking/
- 3: fix: render URI buttons as anchors to fix mobile popup blocker FrigadeHQ/javascript#489
- 4: Algorithms triggered by user activation and window.postMessage / MessageChannel / BroadcastChannel whatwg/html#4741
- 5: https://www.ryanthomson.net/articles/you-shouldnt-call-window-open-asynchronously/
Open the popup synchronously before the awaits
window.open() runs after await fetchCloudKitWebAuthChallenge(), so the user activation is no longer guaranteed and Safari/Chrome can block the popup. Open a blank placeholder window from the original click handler, then set location.href once the challenge URL is ready.
🤖 Prompt for 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.
In `@nook-app/nook-web/src/lib/icloud-oauth.ts` around lines 874 - 895, Open the
auth popup synchronously before any awaits in requestDirectCloudKitWebAuthToken,
because the current await fetchCloudKitWebAuthChallenge() can clear user
activation and cause Safari/Chrome to block window.open(). Adjust the flow so a
blank placeholder window is opened immediately, then after the challenge is
fetched set that window’s location to challenge.redirectURL, and keep the
existing popup-blocked handling using
currentBrowserDiagnostics/sanitizedURLDiagnostics.
| const handleMessage = (event: MessageEvent<unknown>) => { | ||
| const token = webAuthTokenFromMessageData(event.data) | ||
| log.info('CloudKit direct web auth message received', { | ||
| origin: event.origin, | ||
| token: tokenDiagnostics(token), | ||
| }) | ||
| if (!token || settled) { | ||
| return | ||
| } | ||
| cleanup() | ||
| storeCloudKitWebAuthToken(ICLOUD_CONTAINER_ID, token) | ||
| try { | ||
| authWindow.close() | ||
| } catch { | ||
| // Ignore browser-specific popup close failures. | ||
| } | ||
| resolve(token) | ||
| } | ||
| window.addEventListener('message', handleMessage) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate relevant symbols and files.
rg -n "webAuthTokenFromMessageData|storeCloudKitWebAuthToken|handleMessage|addEventListener\\('message'|event.origin|redirectURL|CloudKit direct web auth" nook-app/nook-web/src/lib/icloud-oauth.ts nook-app/nook-web/src/lib -S
# Show the relevant section around the listener.
sed -n '840,980p' nook-app/nook-web/src/lib/icloud-oauth.ts
# Show surrounding declarations for redirect/origin computation if present.
sed -n '1,220p' nook-app/nook-web/src/lib/icloud-oauth.tsRepository: meta-secret/nook
Length of output: 17379
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the challenge-fetching flow and any origin-related handling.
sed -n '800,900p' nook-app/nook-web/src/lib/icloud-oauth.ts
sed -n '520,590p' nook-app/nook-web/src/lib/icloud-oauth.ts
# Search for CloudKit redirect/origin assumptions elsewhere in the repo.
rg -n "allowed origin|redirectURLOrigin|redirectURLPathname|CloudKit direct web auth|postMessage\\(|message listener|ckWebAuthToken|webAuthToken" nook-app -SRepository: meta-secret/nook
Length of output: 11540
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the iCloud config docs and the direct web auth tests.
sed -n '1,120p' nook-app/nook-web/src/lib/icloud-oauth-config.ts
sed -n '320,390p' nook-app/nook-web/tests/unit/lib/icloud-oauth.test.ts
# Show the direct-web-auth related test block with surrounding context.
sed -n '300,380p' nook-app/nook-web/tests/unit/lib/icloud-oauth.test.tsRepository: meta-secret/nook
Length of output: 5731
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the iCloud config docs and the direct web auth tests.
sed -n '1,120p' nook-app/nook-web/src/lib/icloud-oauth-config.ts
sed -n '300,390p' nook-app/nook-web/tests/unit/lib/icloud-oauth.test.tsRepository: meta-secret/nook
Length of output: 3673
Validate the CloudKit popup origin before accepting auth messages.
handleMessage trusts any payload that looks like a token; a window with a reference to this page can inject a forged auth token that gets stored and treated as a successful sign-in. Restrict this handler to the expected Apple auth origin used by the popup (https://idmsa.apple.com in the current flow).
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 906-909: Avoid logging sensitive data
Context: log.info('CloudKit direct web auth message received', {
origin: event.origin,
token: tokenDiagnostics(token),
})
Note: [CWE-532] Insertion of Sensitive Information into Log File.
(log-sensitive-data-typescript)
🤖 Prompt for 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.
In `@nook-app/nook-web/src/lib/icloud-oauth.ts` around lines 905 - 923, The
CloudKit auth message handler in handleMessage currently accepts any window
message that parses as a token, so add an origin check before token handling and
ignore messages not from the expected Apple auth origin. Use the existing
event.origin in the message listener around webAuthTokenFromMessageData and only
proceed with cleanup, storeCloudKitWebAuthToken, authWindow.close, and resolve
when the origin matches https://idmsa.apple.com.
| timeoutId = setTimeout(() => { | ||
| if (settled) { | ||
| return | ||
| } | ||
| cleanup() | ||
| log.warn('CloudKit direct web auth fallback timed out', { | ||
| timeoutMs, | ||
| storage: webAuthTokenStorageDiagnostics(), | ||
| }) | ||
| reject(cloudKitSignInTimeoutError()) | ||
| }, timeoutMs) | ||
| }) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Popup window is left open after a timeout failure.
The timeout handler calls cleanup() (removes listener, clears timer) but never calls authWindow.close(), unlike the success path (line 917). On timeout the Apple sign-in popup stays open indefinitely even though the promise has already rejected, leaving a stray/confusing window for the user.
🧹 Proposed fix
timeoutId = setTimeout(() => {
if (settled) {
return
}
cleanup()
+ try {
+ authWindow.close()
+ } catch {
+ // Ignore browser-specific popup close failures.
+ }
log.warn('CloudKit direct web auth fallback timed out', {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| timeoutId = setTimeout(() => { | |
| if (settled) { | |
| return | |
| } | |
| cleanup() | |
| log.warn('CloudKit direct web auth fallback timed out', { | |
| timeoutMs, | |
| storage: webAuthTokenStorageDiagnostics(), | |
| }) | |
| reject(cloudKitSignInTimeoutError()) | |
| }, timeoutMs) | |
| }) | |
| timeoutId = setTimeout(() => { | |
| if (settled) { | |
| return | |
| } | |
| cleanup() | |
| try { | |
| authWindow.close() | |
| } catch { | |
| // Ignore browser-specific popup close failures. | |
| } | |
| log.warn('CloudKit direct web auth fallback timed out', { | |
| timeoutMs, | |
| storage: webAuthTokenStorageDiagnostics(), | |
| }) | |
| reject(cloudKitSignInTimeoutError()) | |
| }, timeoutMs) | |
| }) |
🤖 Prompt for 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.
In `@nook-app/nook-web/src/lib/icloud-oauth.ts` around lines 924 - 935, The
timeout branch in the CloudKit direct web auth flow leaves the Apple sign-in
popup open; update the timeout handler in the auth flow around the
cleanup/reject logic to also close the popup window just like the success path
does. Use the same auth window handle used by the sign-in promise, and make sure
it is closed before rejecting after timeout so the UI is cleaned up
consistently.
1 participant
Summary
UNKNOWN_ERRORAUTHENTICATION_REQUIREDchallenge, open the returned Apple sign-in URL, and store theckWebAuthTokenreceived by postMessageOrigin: https://nokey.shcurl probe needed to reproduce browser auth behaviorWhy
The latest production logs prove the deployed trusted-click fix is live:
CloudKit native sign-in click observedCloudKit native sign-in deferred wait startedclickPreparedControl:falseCloudKit JS still immediately reports
UNKNOWN_ERRORand no token enters the custom token store/session storage. A direct CloudKit Web Services probe with the registered browser origin returns the real challenge:That returns
AUTHENTICATION_REQUIREDwith aredirectURL, so Nook can recover without waiting for CloudKit JS to expose the challenge correctly.Verification
curl -H 'Origin: https://nokey.sh' .../public/users/current?ckAPIToken=...returnedAUTHENTICATION_REQUIREDwithredirectURLcd nook-app/nook-web && bun run test -- tests/unit/lib/icloud-oauth.test.tscd nook-app/nook-web && bun run testcd nook-app/nook-web && bun run checkcd nook-app/nook-web && bun run lintcd nook-app/nook-web && bun run buildSummary by CodeRabbit
New Features
Bug Fixes