-
Notifications
You must be signed in to change notification settings - Fork 0
Add direct CloudKit auth fallback #227
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -61,6 +61,13 @@ type CloudKitAuthErrorDetails = { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| uuidPresent?: boolean | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| type CloudKitAuthChallenge = { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| reason?: string | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| redirectURL?: string | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| serverErrorCode?: string | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| uuid?: string | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| type CloudKitContainer = { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| setUpAuth: (options?: { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| grabAuthToken?: boolean | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -82,8 +89,14 @@ type CloudKitGlobal = { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| apiTokenAuth: { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| apiToken: string | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| persist: boolean | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| signInButton: { id: string } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| signOutButton: { id: string } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| signInButton: { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| id: string | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| theme?: 'black' | 'white' | 'white-with-outline' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| signOutButton: { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| id: string | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| theme?: 'black' | 'white' | 'white-with-outline' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| services?: { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -107,6 +120,26 @@ function tokenDiagnostics(token: string | undefined): { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| function sanitizedURLDiagnostics(url: string | undefined): { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| present: boolean | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| origin?: string | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| pathname?: string | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!url) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return { present: false } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const parsed = new URL(url) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| present: true, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| origin: parsed.origin, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| pathname: parsed.pathname, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } catch { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return { present: true } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| function currentBrowserDiagnostics(): { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| origin: string | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| hostname: string | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -652,8 +685,8 @@ export async function initICloudAuth(): Promise<void> { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| apiTokenAuth: { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| apiToken: ICLOUD_API_TOKEN, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| persist: true, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| signInButton: { id: CLOUDKIT_SIGN_IN_BUTTON_ID }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| signOutButton: { id: CLOUDKIT_SIGN_OUT_BUTTON_ID }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| signInButton: { id: CLOUDKIT_SIGN_IN_BUTTON_ID, theme: 'black' }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| signOutButton: { id: CLOUDKIT_SIGN_OUT_BUTTON_ID, theme: 'black' }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -779,6 +812,129 @@ function requireStoredWebAuthToken( | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| : { accessToken: token } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 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 webAuthTokenFromMessageData(data: unknown): string | undefined { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (typeof data === 'string') { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return webAuthTokenFromMessageData(JSON.parse(data)) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } catch { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return undefined | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (data == undefined || typeof data !== 'object') { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return undefined | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const record = data as Record<string, unknown> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| for (const key of ['ckWebAuthToken', 'webAuthToken', 'authToken', 'token']) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const candidate = record[key] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (typeof candidate === 'string' && candidate.trim()) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return candidate.trim() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return undefined | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 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) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+874
to
+895
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift 🧩 Analysis chain🌐 Web query:
💡 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:
Open the popup synchronously before the awaits 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let settled = false | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let timeoutId: ReturnType<typeof setTimeout> | undefined = undefined | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const cleanup = () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| settled = true | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| window.removeEventListener('message', handleMessage) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (timeoutId !== undefined) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| clearTimeout(timeoutId) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 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) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+905
to
+923
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔒 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. 🧰 Tools🪛 ast-grep (0.44.1)[warning] 906-909: Avoid logging sensitive data (log-sensitive-data-typescript) 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| timeoutId = setTimeout(() => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (settled) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| cleanup() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| log.warn('CloudKit direct web auth fallback timed out', { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| timeoutMs, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| storage: webAuthTokenStorageDiagnostics(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| reject(cloudKitSignInTimeoutError()) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, timeoutMs) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+924
to
+935
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Popup window is left open after a timeout failure. The timeout handler calls 🧹 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
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| async function waitForCloudKitSignIn( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| container: CloudKitContainer, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| timeoutMs = ICLOUD_SIGN_IN_TIMEOUT_MS, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -792,6 +948,7 @@ async function waitForCloudKitSignIn( | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| control: cloudKitSignInControlDiagnostics(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const tokenPromise = waitForStoredWebAuthToken(timeoutMs) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let sawExpectedSignInFailure = false | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const signInPromise = container | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .whenUserSignsIn() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .then((userIdentity) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -805,6 +962,7 @@ async function waitForCloudKitSignIn( | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .catch((error: unknown) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (isExpectedSignInSetupFailure(error)) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| sawExpectedSignInFailure = true | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| log.info('CloudKit sign-in callback waiting for web auth token', { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| details: cloudKitAuthErrorDetails(error), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| hasSignInMount: hasCloudKitSignInControl(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -835,6 +993,13 @@ async function waitForCloudKitSignIn( | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return authSetupUserIdentity ?? {} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (sawExpectedSignInFailure) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| await requestDirectCloudKitWebAuthToken(timeoutMs) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| log.info('CloudKit sign-in succeeded through direct fallback', { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| token: tokenDiagnostics(readStoredWebAuthToken()), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return authSetupUserIdentity ?? {} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| await tokenPromise | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| log.info('CloudKit sign-in succeeded after token wait', { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| signedIn: Boolean(authSetupUserIdentity), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Challenge fetch has no timeout guard.
fetchCloudKitWebAuthChallengeawaitsfetch()with no timeout/abort. ThetimeoutMspassed intorequestDirectCloudKitWebAuthTokenonly guards the popup/message-listeningPromisecreated afterward (lines 895-935); if this initial fetch stalls (slow/broken network, hung connection), the whole fallback hangs indefinitely with no way forwaitForCloudKitSignIn's caller to recover via the configured timeout.🔧 Proposed fix using AbortController
📝 Committable suggestion
🤖 Prompt for AI Agents