Fix desktop CF Access cookie pairing#93
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3bfeeb8faa
ℹ️ 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".
| ...filteredHeaders(headers), | ||
| ...(cookieHeader | ||
| ? { | ||
| [CLOUDFLARE_ACCESS_COOKIE_HEADER_NAME]: cookieHeader, |
There was a problem hiding this comment.
Clear stale Access cookie rules before sign-in
When a saved desktop Cloudflare Access cookie has been installed for an origin, this persists that exact cookie in the session-wide webRequest rule. Later, if the user follows the existing re-authenticate path for the same origin, authenticateCloudflareAccess clears Electron cookies but does not delete this rule, and the auth window uses Electron.session.defaultSession, so onBeforeSendHeaders injects the stale CF_Authorization cookie right back into the sign-in navigation. In that expired/stale-cookie scenario the user cannot get a clean Access login; clear or suspend the rule for the origin while capturing a replacement cookie.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0484a9d4c9
ℹ️ 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".
| await clearAccessCookies(session, origin); | ||
| } | ||
| configureCloudflareAccessHeaders(session, origin, input.headers); | ||
| configureCloudflareAccessHeaders(session, origin, input.headers, cookieValue); |
There was a problem hiding this comment.
Roll back header rules when cookie install fails
When input.cookieValue is non-empty, this configures the webRequest rule before installAccessCookie runs. If session.cookies.set rejects, installCloudflareAccessCredentials reports failure but the in-memory rule remains and will still inject that rejected CF_Authorization value on later requests to the origin (and stale cookies may already have been cleared). Move the rule update after a successful cookie install or restore the previous rule in the catch path so a failed install does not leave active credentials behind.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a2dd90d784
ℹ️ 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".
| try { | ||
| if (!capturedReplacementCookie) { | ||
| await restoreAccessCookies(session, options.origin, previousAccessCookies); | ||
| restoreCloudflareAccessHeaders(suspendedAccessHeaders); |
There was a problem hiding this comment.
Restore headers even when cookie rollback fails
When a failed or cancelled reauth has suspended an existing Cloudflare Access header rule, this restore only runs after restoreAccessCookies completes. If Electron rejects any session.cookies.set while rolling back the previous cookies, the function exits before restoring the in-memory webRequest rule, leaving saved Access transport disabled for that origin even though the login failed. Restore the suspended headers in a finally around the cookie rollback so this rollback path is independent of cookie-store failures.
Useful? React with 👍 / 👎.
| cloudflareAccessHeaderRules.delete(requestOriginKey); | ||
| return { | ||
| requestOriginKey, | ||
| headers: previousHeaders, | ||
| version: bumpCloudflareAccessHeaderRuleVersion(requestOriginKey), |
There was a problem hiding this comment.
Preserve suspended headers across overlapping attempts
When two cookie installs or reauths for the same origin overlap, the first call records the existing rule, but the second call reaches this path after the map is already empty and bumps the version with headers: undefined. If both attempts then fail or are cancelled without installing a replacement, the second restore has nothing to restore and the first restore is skipped by the version mismatch, so the saved Cloudflare Access header rule is permanently lost for that origin. Track nested suspensions per origin, or avoid invalidating the original suspension when there was no rule to suspend.
Useful? React with 👍 / 👎.
Summary
Tests