Skip to content

fix(sso): re-grant provider trust when an already-verified domain is re-submitted - #6320

Merged
waleedlatif1 merged 4 commits into
stagingfrom
sso-verify-regrant
Aug 6, 2026
Merged

fix(sso): re-grant provider trust when an already-verified domain is re-submitted#6320
waleedlatif1 merged 4 commits into
stagingfrom
sso-verify-regrant

Conversation

@waleedlatif1

Copy link
Copy Markdown
Collaborator

Summary

  • Re-submitting an already-verified domain now re-grants domainVerified to the providers that proof covers, instead of returning success having done nothing
  • Extracted the provider-match expression into one providersOnDomain helper shared by both grant paths, so it can't drift from the revocation on delete

Follow-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

  • Bug fix

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, and check:api-validation all clean.

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)

@vercel

vercel Bot commented Aug 6, 2026

Copy link
Copy Markdown

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

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Aug 6, 2026 8:27am

Request Review

@cursor

cursor Bot commented Aug 6, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Changes SSO domain verification responses and domainVerified grant logic on a security-sensitive path; UI changes are low risk accessibility only.

Overview
DNS TXT verification now returns present / absent / unavailable instead of a boolean. The verify API maps infrastructure lookup failures to 503 with an “on our side” message, and missing records stay 422, so admins aren’t told to fix DNS when the resolver failed.

Provider trust on verify uses a shared providersOnDomain matcher (aligned with delete/revoke). When the conditional flip matches nothing but the row is already verified, the route re-grants domainVerified on matching SSO providers instead of succeeding without updating trust—covering stuck providers after a refused SSO update cleared the flag.

SSO verified-domains UI wires htmlFor / id on the add-domain field and pending challenge host/value copy inputs for proper label focus.

Reviewed by Cursor Bugbot for commit f5a3de6. Configure here.

@greptile-apps

greptile-apps Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR distinguishes missing DNS records from resolver failures and re-grants provider trust when verification encounters an already-verified domain.

  • Introduces explicit present, absent, and unavailable TXT lookup outcomes.
  • Shares normalized provider-domain matching across both trust-grant paths.
  • Adds verification-route and helper coverage for recovery and DNS failure cases.
  • Associates settings labels with their corresponding domain inputs.

Confidence Score: 3/5

The 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

Important Files Changed

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

Comment thread apps/sim/app/api/organizations/[id]/domains/[domainId]/verify/route.ts Outdated
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cursor review

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cursor review

@waleedlatif1
waleedlatif1 merged commit 01f4b43 into staging Aug 6, 2026
22 checks passed
@waleedlatif1
waleedlatif1 deleted the sso-verify-regrant branch August 6, 2026 08:29

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

❌ 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))

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit f5a3de6. Configure here.

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