Skip to content

[codex] Fix GitHub OAuth in iframe#8

Merged
Che-Zhu merged 1 commit intomainfrom
codex/github-oauth-iframe-callback
May 7, 2026
Merged

[codex] Fix GitHub OAuth in iframe#8
Che-Zhu merged 1 commit intomainfrom
codex/github-oauth-iframe-callback

Conversation

@Che-Zhu
Copy link
Copy Markdown
Collaborator

@Che-Zhu Che-Zhu commented May 7, 2026

Summary

  • Add request-aware auth cookie policy for iframe-compatible session cookies, including localhost development hosts.
  • Make GitHub popup completion resilient to missed iframe messages by checking /api/auth/info.
  • Harden GitHub account merge handling and keep auth logs static.

Root Cause

GitHub OAuth was completing and writing users/sessions, but the app was embedded under a different top-level site. The session cookie was written as SameSite=Lax without Secure for localhost development, so iframe requests to /api/auth/info did not include the session cookie.

Validation

  • pnpm exec tsx --test lib/auth/iframe-oauth.test.ts
  • pnpm format
  • pnpm format:check
  • pnpm type-check
  • pnpm lint
  • Manual Chrome verification inside the Sealos iframe: GitHub avatar appears after sign-in.

Notes

The worktree still contains unrelated local changes that are not included in this PR.

@Che-Zhu Che-Zhu marked this pull request as ready for review May 7, 2026 09:05
@Che-Zhu Che-Zhu merged commit c6706c2 into main May 7, 2026
5 checks passed
@Che-Zhu Che-Zhu deleted the codex/github-oauth-iframe-callback branch May 7, 2026 09:09
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 936bd09500

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread lib/auth/cookie-policy.ts
Comment on lines +74 to +78
return options.nodeEnv === 'production' || options.isHttps || options.isLocalhost
}

export function getAuthCookieSameSite(input?: AuthCookiePolicyInput): AuthCookieSameSite {
return getAuthCookieSecure(input) ? 'none' : 'lax'
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Keep auth session cookies Lax outside iframe flows

getAuthCookieSameSite() now returns none whenever cookies are secure, and getAuthCookieSecure() returns true for all production requests, so every production session cookie becomes SameSite=None. That causes the auth cookie to be sent on cross-site requests, while many mutating endpoints (for example app/api/api-keys/route.ts POST/DELETE) authenticate only via getSessionFromReq and do not perform origin/CSRF checks, which enables cross-site request forgery from third-party sites. Limit SameSite=None to explicitly embedded iframe contexts and keep Lax as the default for normal sessions.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant