fix(sso): re-grant provider trust when an already-verified domain is re-submitted - #6320
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryMedium Risk Overview Provider trust on verify uses a shared SSO verified-domains UI wires Reviewed by Cursor Bugbot for commit f5a3de6. Configure here. |
Greptile SummaryThe PR distinguishes missing DNS records from resolver failures and re-grants provider trust when verification encounters an already-verified domain.
Confidence Score: 3/5The PR is not yet safe to merge because concurrent domain deletion can still be followed by stale restoration of provider trust. The fallback path reads a verified proof and grants provider trust in separate unlocked operations, so deletion can remove the proof and revoke trust between them before the stale update re-enables SSO account linking. Files Needing Attention: apps/sim/app/api/organizations/[id]/domains/[domainId]/verify/route.ts
|
| Filename | Overview |
|---|---|
| apps/sim/app/api/organizations/[id]/domains/[domainId]/verify/route.ts | Adds resolver-failure handling and restores provider trust for an already-verified domain. |
| apps/sim/lib/auth/sso/domain-verification.ts | Replaces the boolean TXT lookup result with explicit present, absent, and unavailable outcomes. |
| apps/sim/app/api/organizations/[id]/domains/[domainId]/verify/route.test.ts | Covers resolver unavailability, verified-domain trust recovery, and genuinely stale challenges. |
| apps/sim/lib/auth/sso/domain-verification.test.ts | Updates helper tests for the new three-state DNS lookup contract. |
| apps/sim/ee/sso/components/verified-domains-section.tsx | Connects domain-related labels to their input elements with stable IDs. |
Reviews (3): Last reviewed commit: "chore(sso): tighten the re-grant rationa..." | Re-trigger Greptile
…label the domain fields
|
@cursor review |
|
@cursor review |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit f5a3de6. Configure here.
| await db | ||
| .update(ssoProvider) | ||
| .set({ domainVerified: true }) | ||
| .where(providersOnDomain(current.domain)) |
There was a problem hiding this comment.
Re-grant skips proof row lock
Medium Severity
The idempotent re-grant runs a standalone sso_provider update after a re-read, without wrapping the grant in a transaction or locking the matching verified sso_domain row. If the domain proof is deleted between that read and the update, providers can end up with domainVerified: true while no verified proof row remains.
Reviewed by Cursor Bugbot for commit f5a3de6. Configure here.


Summary
domainVerifiedto the providers that proof covers, instead of returning success having done nothingprovidersOnDomainhelper shared by both grant paths, so it can't drift from the revocation on deleteFollow-up to #6311. A provider can hold a verified domain while its own trust flag is off: an SSO update whose trust grant is refused reverts the config and clears the flag. The verify route's idempotent branch previously short-circuited to a 200 without touching providers, so re-running verification — the obvious recovery — converged nothing. The proof is present in that branch, which is exactly what authorizes the grant.
Scope note: this is an API-level correctness fix, not a UX change. The Verify control only renders for pending domains, so this branch is not reachable from the settings UI; the user-facing recovery for a stuck provider remains re-saving the SSO form. No UI is touched by this PR.
Type of Change
Testing
bunx vitest run app/api/organizations— 67 passed. The new test was verified to fail without the fix (removing the re-grant turns it red). Typecheck, lint, andcheck:api-validationall clean.Checklist