Skip to content

fix(chat): close SSO auth bypass via checkSSOAccess body flag#4408

Merged
waleedlatif1 merged 3 commits intostagingfrom
waleedlatif1/security-review
May 2, 2026
Merged

fix(chat): close SSO auth bypass via checkSSOAccess body flag#4408
waleedlatif1 merged 3 commits intostagingfrom
waleedlatif1/security-review

Conversation

@waleedlatif1
Copy link
Copy Markdown
Collaborator

Summary

  • Remove checkSSOAccess short-circuit in validateChatAuth — SSO branch now always validates via getSession(), body-supplied email is ignored
  • Skip chat_auth cookie issuance and validation for SSO deployments to close the replay window
  • Split the eligibility pre-flight into a dedicated POST /api/chat/[identifier]/sso endpoint that returns { eligible } and never touches the executor
  • Drop .passthrough() and checkSSOAccess from deployedChatAuthBodySchema / deployedChatPostBodySchema
  • Add SSO branch test coverage in chat/utils.test.ts

Type of Change

  • Bug fix (security)

Testing

Tested manually; bun run check:api-validation passes.

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

- Remove checkSSOAccess short-circuit; SSO branch always validates via getSession()
- Skip chat_auth cookie issuance/validation for SSO deployments to prevent replay
- Split eligibility pre-flight into dedicated POST /api/chat/[identifier]/sso route
- Drop .passthrough() and checkSSOAccess from deployed chat contracts
- Add SSO branch test coverage in chat utils
@vercel
Copy link
Copy Markdown

vercel Bot commented May 2, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped May 2, 2026 6:06pm

Request Review

@cursor
Copy link
Copy Markdown

cursor Bot commented May 2, 2026

PR Summary

High Risk
Touches chat authentication/authorization paths (SSO, cookie issuance/validation, request schemas), so mistakes could lock users out or reintroduce an auth bypass. New /sso endpoint and session-based checks reduce risk but change runtime behavior for SSO deployments.

Overview
SSO chat authentication is hardened. validateChatAuth no longer supports the checkSSOAccess body flag; SSO now always authorizes via getSession() and checks the session email against the allowlist, while skipping chat_auth_* cookie validation for SSO.

SSO preflight is split into a separate endpoint. Adds POST /api/chat/[identifier]/sso (contracted as chatSSOContract) that rate-limits by IP and returns { eligible } without running the chat executor; the SSO UI now calls this endpoint before redirecting.

Request validation is tightened and tests added. Removes checkSSOAccess and .passthrough() from deployed chat auth/post body schemas, updates the route-count baseline, and adds targeted SSO unit tests (no session, ignore body email, allowlisted vs non-allowlisted).

Reviewed by Cursor Bugbot for commit 92ee9fe. Configure here.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 2, 2026

Greptile Summary

This PR closes an SSO authentication bypass by removing the checkSSOAccess body-flag short-circuit in validateChatAuth, ensuring all SSO requests are validated via getSession() rather than a client-supplied email. It also skips chat_auth cookie issuance and validation for SSO deployments, and splits the eligibility pre-flight into a dedicated POST /api/chat/[identifier]/sso endpoint with per-IP rate limiting.

Confidence Score: 5/5

Safe to merge — the core security bypass is correctly closed and all three attack surfaces are addressed.

Only P2 findings remain (rate-limiting no-op for unknown IPs). The critical bypass paths are all properly fixed with good test coverage.

apps/sim/app/api/chat/[identifier]/sso/route.ts — the unknown-IP rate-limit skip warrants a follow-up hardening.

Important Files Changed

Filename Overview
apps/sim/app/api/chat/[identifier]/route.ts POST handler now skips setChatAuthCookie for SSO; GET handler correctly excludes SSO from cookie-based validation. Both gaps from the original bypass are closed.
apps/sim/app/api/chat/[identifier]/sso/route.ts New SSO eligibility pre-flight endpoint with per-IP rate limiting. Rate limiter is silently skipped when IP resolves to unknown, which can be exploited to bypass the enumeration guard.
apps/sim/app/api/chat/utils.ts Removed checkSSOAccess short-circuit; SSO branch now always calls getSession() and ignores any body-supplied email for authorization.
apps/sim/ee/sso/components/sso-auth.tsx Updated to use new chatSSOContract; handles eligible: false before redirecting to SSO login, matching the refactored pre-flight design.
apps/sim/lib/api/contracts/chats.ts Removed checkSSOAccess and .passthrough() from auth/post schemas; added chatSSOContract and its body/response schemas.
scripts/check-api-validation-contracts.ts Baseline counts incremented from 717 to 718 to account for the new SSO route.

Reviews (2): Last reviewed commit: "fix(chat): close SSO GET cookie replay a..." | Re-trigger Greptile

Comment thread apps/sim/app/api/chat/[identifier]/sso/route.ts Outdated
Comment thread apps/sim/app/api/chat/utils.ts
Comment thread apps/sim/app/api/chat/[identifier]/sso/route.ts
- Skip chat_auth cookie validation for SSO in GET handler (replay vector for pre-fix cookies)
- Route SSO GET through getSession() instead of always returning auth_required_sso so post-IdP config fetch works
- Add per-IP rate limiting to /api/chat/[identifier]/sso to prevent allowlist enumeration
@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@cursor review

Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 92ee9fe. Configure here.

@waleedlatif1 waleedlatif1 merged commit 66bab93 into staging May 2, 2026
14 checks passed
@waleedlatif1 waleedlatif1 deleted the waleedlatif1/security-review branch May 2, 2026 18:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant