Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
🧪 Test Selection✅ Tests that will run
⏭️ Tests skipped (no relevant file changes detected)
|
| * from a maintained list (e.g. Kickbox `free-email-domains`) if needed — keep | ||
| * it a vendored static set, never a runtime fetch. | ||
| */ | ||
| export const PUBLIC_EMAIL_PROVIDER_DOMAINS = new Set([ |
There was a problem hiding this comment.
This was already rejecting public emails, this is not the full list, and it is not scalable, we should relay on a different approach, like an npm package with a more comprehensive list
|
Your preview environment pr-23559 has been deployed with errors. |
Preview Environment🌐 URL: https://lightdash-preview-pr-23559.lightdash.okteto.dev 📋 Logs: View in GCP Console 🔧 SSH: |
rephus
force-pushed
the
05-27-feat_add_organization_domain_verification_with_otp_email_flow_and_sso_routing_integration
branch
from
b1f224e to
d06a81a
Compare
…O routing integration
rephus
force-pushed
the
05-27-feat_add_organization_domain_verification_with_otp_email_flow_and_sso_routing_integration
branch
from
d06a81a to
6114456
Compare
| * Domain verification gates the per-organization SSO settings, so it shares | ||
| * the same feature flag. | ||
| */ | ||
| private async assertFeatureEnabled( |
There was a problem hiding this comment.
currently, this is enabled using hte SsoOrganizationSettings which is the only valid use of verified domains, and a dependency for adding sso providers
rephus
deleted the
05-27-feat_add_organization_domain_verification_with_otp_email_flow_and_sso_routing_integration
branch
|
Docs PR opened: lightdash/mintlify-docs#715 Added a new Verified domains reference page explaining how admins verify organization domains for SSO routing. |
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
| async requestVerification( | ||
| @Request() req: express.Request, | ||
| @Body() body: RequestDomainVerification, | ||
| ): Promise<ApiDomainVerificationStatusResponse> { | ||
| assertRegisteredAccount(req.account); | ||
| const results = await this.services | ||
| .getOrganizationDomainVerificationService() | ||
| .requestVerification(req.account, body); | ||
| this.setStatus(200); | ||
| return { status: 'ok', results }; | ||
| } |
There was a problem hiding this comment.
The backend rules require 'proper request validation using zod' in controllers. The requestVerification endpoint accepts body: RequestDomainVerification directly from tsoa without any zod validation of the incoming fields (e.g. domain format, email format). Add a zod schema to validate the body before passing it to the service.
| async requestVerification( | |
| @Request() req: express.Request, | |
| @Body() body: RequestDomainVerification, | |
| ): Promise<ApiDomainVerificationStatusResponse> { | |
| assertRegisteredAccount(req.account); | |
| const results = await this.services | |
| .getOrganizationDomainVerificationService() | |
| .requestVerification(req.account, body); | |
| this.setStatus(200); | |
| return { status: 'ok', results }; | |
| } | |
| async requestVerification( | |
| @Request() req: express.Request, | |
| @Body() body: RequestDomainVerification, | |
| ): Promise<ApiDomainVerificationStatusResponse> { | |
| assertRegisteredAccount(req.account); | |
| const validatedBody = requestDomainVerificationSchema.parse(body); | |
| const results = await this.services | |
| .getOrganizationDomainVerificationService() | |
| .requestVerification(req.account, validatedBody); | |
| this.setStatus(200); | |
| return { status: 'ok', results }; | |
| } | |
Spotted by Graphite (based on custom rule: packages/backend rules)
Is this helpful? React 👍 or 👎 to let us know.
| try { | ||
| const challenge = | ||
| await this.organizationDomainVerificationModel.verifyChallengePasscode( | ||
| { | ||
| organizationUuid, | ||
| domain: normalizedDomain, | ||
| passcode, | ||
| }, | ||
| ); | ||
| if ( | ||
| !isOtpExpired(challenge.createdAt) && | ||
| !isOtpMaxAttempts(challenge.numberOfAttempts) | ||
| ) { | ||
| try { | ||
| await this.organizationDomainVerificationModel.markChallengeVerified( | ||
| { | ||
| organizationUuid, | ||
| domain: normalizedDomain, | ||
| verifiedByUserUuid: account.user.userUuid, | ||
| }, | ||
| ); | ||
| } catch (error) { | ||
| if (isUniqueViolation(error)) { | ||
| throw new ParameterError( | ||
| `${normalizedDomain} has already been verified by another organization.`, | ||
| ); | ||
| } | ||
| throw error; | ||
| } | ||
| } | ||
| } catch (error) { | ||
| // Wrong/expired passcode — count the attempt, mirroring email OTP. | ||
| if (error instanceof NotFoundError) { | ||
| await this.organizationDomainVerificationModel.incrementChallengeAttempts( | ||
| { | ||
| organizationUuid, | ||
| domain: normalizedDomain, | ||
| }, | ||
| ); | ||
| } else { | ||
| throw error; | ||
| } | ||
| } |
There was a problem hiding this comment.
Security concern: when verifyChallengePasscode throws a NotFoundError (wrong passcode), the code increments the attempt counter but then falls through to findByOrgAndDomain and returns the current status — effectively giving the caller no indication that the passcode was wrong beyond inspecting isVerified. This is a subtle information-leakage / brute-force risk: a caller can distinguish a wrong code (status returned, isVerified=false) from a missing challenge (NotFoundError re-thrown). The response should be consistent regardless of whether the passcode was wrong or no challenge exists, to avoid leaking state. Consider returning a uniform error or status for both cases.
| try { | |
| const challenge = | |
| await this.organizationDomainVerificationModel.verifyChallengePasscode( | |
| { | |
| organizationUuid, | |
| domain: normalizedDomain, | |
| passcode, | |
| }, | |
| ); | |
| if ( | |
| !isOtpExpired(challenge.createdAt) && | |
| !isOtpMaxAttempts(challenge.numberOfAttempts) | |
| ) { | |
| try { | |
| await this.organizationDomainVerificationModel.markChallengeVerified( | |
| { | |
| organizationUuid, | |
| domain: normalizedDomain, | |
| verifiedByUserUuid: account.user.userUuid, | |
| }, | |
| ); | |
| } catch (error) { | |
| if (isUniqueViolation(error)) { | |
| throw new ParameterError( | |
| `${normalizedDomain} has already been verified by another organization.`, | |
| ); | |
| } | |
| throw error; | |
| } | |
| } | |
| } catch (error) { | |
| // Wrong/expired passcode — count the attempt, mirroring email OTP. | |
| if (error instanceof NotFoundError) { | |
| await this.organizationDomainVerificationModel.incrementChallengeAttempts( | |
| { | |
| organizationUuid, | |
| domain: normalizedDomain, | |
| }, | |
| ); | |
| } else { | |
| throw error; | |
| } | |
| } | |
| try { | |
| const challenge = | |
| await this.organizationDomainVerificationModel.verifyChallengePasscode( | |
| { | |
| organizationUuid, | |
| domain: normalizedDomain, | |
| passcode, | |
| }, | |
| ); | |
| if (isOtpExpired(challenge.createdAt)) { | |
| throw new ParameterError( | |
| `Verification code has expired. Please request a new one.`, | |
| ); | |
| } | |
| if (isOtpMaxAttempts(challenge.numberOfAttempts)) { | |
| throw new ParameterError( | |
| `Maximum verification attempts exceeded. Please request a new code.`, | |
| ); | |
| } | |
| try { | |
| await this.organizationDomainVerificationModel.markChallengeVerified( | |
| { | |
| organizationUuid, | |
| domain: normalizedDomain, | |
| verifiedByUserUuid: account.user.userUuid, | |
| }, | |
| ); | |
| } catch (error) { | |
| if (isUniqueViolation(error)) { | |
| throw new ParameterError( | |
| `${normalizedDomain} has already been verified by another organization.`, | |
| ); | |
| } | |
| throw error; | |
| } | |
| } catch (error) { | |
| if (error instanceof ParameterError) { | |
| throw error; | |
| } | |
| // Wrong passcode — count the attempt and return a uniform error. | |
| if (error instanceof NotFoundError) { | |
| await this.organizationDomainVerificationModel.incrementChallengeAttempts( | |
| { | |
| organizationUuid, | |
| domain: normalizedDomain, | |
| }, | |
| ); | |
| throw new ParameterError( | |
| `Invalid verification code. Please check the code and try again.`, | |
| ); | |
| } | |
| throw error; | |
| } | |
Spotted by Graphite (based on custom rule: Security rules)
Is this helpful? React 👍 or 👎 to let us know.
| @@ -0,0 +1,228 @@ | |||
| import { type DomainVerificationStatus } from '@lightdash/common'; | |||
| import { Button, PinInput, Stack, Text, TextInput } from '@mantine-8/core'; | |||
| import { useForm } from '@mantine/form'; | |||
There was a problem hiding this comment.
The frontend rules require using @mantine/form with Zod for validation ('Use Mantine useForm with Zod for validation'). The form here uses useForm from @mantine/form but the validate object uses a plain function instead of a Zod schema. Replace the inline validate functions with a Zod schema passed to zodResolver.
Spotted by Graphite (based on custom rule: packages/frontend rules)
Is this helpful? React 👍 or 👎 to let us know.
# [0.3029.0](0.3028.1...0.3029.0) (2026-05-28) ### Features * add organization domain verification with OTP email flow and SSO routing integration ([#23559](#23559)) ([9e5abc5](9e5abc5))
|
🎉 This PR is included in version 0.3029.0 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
2 participants
Closes: https://linear.app/lightdash/issue/PROD-7516/domain-verification
Closes: https://linear.app/lightdash/issue/PROD-7216/add-organization-level-domain-allowlist-settings
Description:
Introduces organization domain verification — a flow that lets organization admins prove they own a domain by receiving a one-time passcode at an address on that domain. Verified domains become the single source of truth for SSO routing, replacing the previous approach of routing based on the organization's allowed email domains list.
Backend:
organization_domain_verificationstable with a partial unique index on(domain) WHERE verified_at IS NOT NULL, enforcing a global first-verified-wins rule so only one organization can hold a verified domain at a time.OrganizationDomainVerificationModelhandles challenge upserts (bcrypt-hashed passcodes), attempt tracking, passcode comparison, and marking domains as verified.OrganizationDomainVerificationServiceorchestrates the full lifecycle: requesting a challenge (sends a branded email via a newdomainVerificationemail template), confirming a passcode, listing verified domains, and deleting a domain. Rejects public email providers and domains already claimed by another org.OrganizationSsoModelnow joins againstorganization_domain_verificationsinstead oforganization_allowed_email_domainswhen resolving which SSO methods to offer for a given email domain. Methods withoverride_email_domains = falseroute all of the org's verified domains; methods withoverride_email_domains = trueroute only the configured subset.OrganizationSsoServicevalidates that any domains added to an override subset are already verified by the organization.generateOneTimePasscode,isOtpExpired,isOtpMaxAttempts,otpExpirationDate) extracted into a reusable utility with unit tests.isPublicEmailProviderDomainconsolidated into@lightdash/common, merging the previousEMAIL_PROVIDER_LISTwith the separate blocklist that lived inOrganizationSsoService.GET/POST/DELETE /api/v1/org/domainswith full OpenAPI/tsoa definitions.Frontend:
/generalSettings/verifiedDomains) where admins can add a domain (entering a challenge email), enter the 6-digit code with a live countdown timer, and remove verified domains.SsoMethodDomainsFieldcomponent. The override toggle switches from a free-text tags input to aMultiSelectrestricted to the organization's verified domains, with a link to the verified domains page when none exist.