From 614bd91e6ddda104e5e3b1b998e4b7514b61e423 Mon Sep 17 00:00:00 2001 From: Andres Aguilera Date: Thu, 1 Oct 2026 18:38:11 -0300 Subject: [PATCH 1/2] validate identity provider form fields before save --- .../settings/lib/idp-form-validation.md | 124 +++++++++ .../settings/lib/idp-form-validation.test.ts | 249 ++++++++++++++++++ .../settings/lib/idp-form-validation.ts | 186 +++++++++++++ .../settings/pages/IdentityProvidersPage.tsx | 162 +++++++++--- frontend/src/shared/i18n/locales/de.json | 16 +- frontend/src/shared/i18n/locales/en.json | 16 +- frontend/src/shared/i18n/locales/es.json | 16 +- frontend/src/shared/i18n/locales/fr.json | 16 +- frontend/src/shared/i18n/locales/it.json | 16 +- frontend/src/shared/i18n/locales/pt.json | 16 +- frontend/src/shared/i18n/locales/ru.json | 16 +- 11 files changed, 794 insertions(+), 39 deletions(-) create mode 100644 frontend/src/features/settings/lib/idp-form-validation.md create mode 100644 frontend/src/features/settings/lib/idp-form-validation.test.ts create mode 100644 frontend/src/features/settings/lib/idp-form-validation.ts diff --git a/frontend/src/features/settings/lib/idp-form-validation.md b/frontend/src/features/settings/lib/idp-form-validation.md new file mode 100644 index 000000000..ca4b05e00 --- /dev/null +++ b/frontend/src/features/settings/lib/idp-form-validation.md @@ -0,0 +1,124 @@ +# Reglas de validación — Formulario de Identity Provider + +Documento de referencia de las validaciones del frontend que se aplican sobre el +formulario **Add / Edit identity provider** (`/settings/identity-providers`). + +- **Dónde corre:** `src/features/settings/lib/idp-form-validation.ts` + (`validateIdpForm`), consumido por `UpsertDialog` en + `src/features/settings/pages/IdentityProvidersPage.tsx`. +- **Cuándo:** en vivo, en cada render del formulario (los errores se pintan bajo + cada campo y bloquean el botón de guardar). No es asincrónico: no hace peticiones. +- **Qué devuelve:** un objeto `campo → clave i18n` (namespace `idp.form.errors.*`). + Objeto vacío = formulario válido. El componente traduce la clave al texto. + +## Por qué existe + +El backend solo comprueba **presencia** al guardar y **formato** al hacer login +(`modules/iam/usecase/idp.go` → `prepareSettings` y `buildSP`/`oidcConfig`/`ldapBind`). +Sin validación en el frontend, un admin puede guardar una URL mal escrita, un PEM +roto o un puerto fuera de rango y el error le llega a un usuario cuando intenta +iniciar sesión, no al admin cuando configura. Este módulo intercede ese gap. + +> **La validación de formato aquí es de primera línea, no única.** El backend +> sigue siendo la última defensa y hace el parseo real (X.509 / PKCS8, discovery +> OIDC, dial LDAP) en el momento del login. + +## Semántica del secreto (aplica a los tres protocolos) + +Cada protocolo lleva exactamente **un campo secreto write-only** que **no** vuelve +del backend: + +| Protocolo | Campo secreto | +|---|---| +| saml | `spPrivateKeyPem` | +| oidc | `clientSecret` | +| ldap | `bindPassword` | + +Regla: + +- **Create** → el secreto es **obligatorio** (sin él no hay nada que guardar). +- **Edit** → vacío significa **"mantener el guardado"**; no se valida formato. +- Si tiene valor en create, **sí** se valida su formato (armadura PEM, en SAML). + +## Reglas comunes + +| Campo | Tipo | Qué maneja | Validación | Error key | +|---|---|---|---|---| +| `name` | `string` | Nombre del provider; va en la URL de SSO (`/api/v1/sso//login`) | Obligatorio. Solo en **create**: ≤ 64 chars y `^[A-Za-z0-9][A-Za-z0-9._-]*$` (sin espacios). En **edit** el input está deshabilitado, así que su formato no bloquea guardar | `idp.form.errors.required` / `idp.form.errors.nameFormat` | + +En edit, `providerType` también es inmutable (no validable por el usuario). + +## SAML + +| Campo | Tipo | Qué maneja | Validación | Error key | +|---|---|---|---|---| +| `metadataUrl` | `string` | URL de metadatos del IdP | Obligatorio. URL `http(s)` válida (parseo con `URL`) | `required` / `idp.form.errors.url` | +| `spEntityId` | `string` | Entity ID del Service Provider | Obligatorio. Solo en **create**: URI válida (http, https o `urn:`) — coge espacios y esquemas raros que el IdP rechazaría en login. En **edit** solo presencia (un valor heredado no bloquea el save) | `required` / `idp.form.errors.entityId` | +| `spAcsUrl` | `string` | Endpoint ACS que recibe la respuesta SAML | Obligatorio. URL `http(s)` válida | `required` / `idp.form.errors.url` | +| `spCertificatePem` | `string` | Certificado del SP | Obligatorio. Contiene armadura `-----BEGIN/END CERTIFICATE-----` | `required` / `idp.form.errors.pemCertificate` | +| `spPrivateKeyPem` | `string` | Clave privada del SP (secreto) | Ver sección de secreto | `required` / `idp.form.errors.pemKey` | + +## OIDC + +| Campo | Tipo | Qué maneja | Validación | Error key | +|---|---|---|---|---| +| `issuer` | `string` | Issuer contra el que corre la discovery | Obligatorio. URL **`https`** válida (https estricto: la discovery expone el secreto) | `required` / `idp.form.errors.httpsUrl` | +| `clientId` | `string` | Client ID de la app | Obligatorio (solo presencia) | `idp.form.errors.required` | +| `redirectUrl` | `string` | Callback registrado en el provider | Obligatorio. URL **https** válida; excepción: loopback `http` (`localhost`, `127.0.0.1`, `[::1]`) para clientes nativos, según RFC 8252 §7.3 — el token exchange no sale de la máquina | `required` / `idp.form.errors.redirectUrl` | +| `clientSecret` | `string` | Client secret (secreto) | Ver sección de secreto | `idp.form.errors.required` | + +## LDAP + +| Campo | Tipo | Qué maneja | Validación | Error key | +|---|---|---|---|---| +| `host` | `string` | Host del servidor LDAP | Obligatorio. Hostname/IP **sin esquema** ni ruta: `^(?!.*[\/\s])[A-Za-z0-9.-]+$` | `required` / `idp.form.errors.hostname` | +| `port` | `number` | Puerto de conexión | Opcional en formato. Si está, entero sin ceros a la izquierda y ≤ 65535. `0` es válido = default 389 (el backend diala `0` como `389`) | `idp.form.errors.port` | +| `bindDn` | `string` | Servicio que busca en el directorio | Obligatorio (solo presencia; no se valida la sintaxis DN, sería brittle) | `idp.form.errors.required` | +| `baseDn` | `string` | Raíz de la búsqueda | Obligatorio (solo presencia) | `idp.form.errors.required` | +| `userFilter` | `string` | Filtro LDAP donde entra la dirección de login | Obligatorio. Debe contener `%s` (placeholder). Paréntesis balanceados | `required` / `idp.form.errors.userFilterPlaceholder` / `idp.form.errors.userFilterUnbalanced` | +| `bindPassword` | `string` | Password del servicio (secreto) | Ver sección de secreto | `idp.form.errors.required` | + +Los atributos LDAP `emailAttribute`, `nameAttribute`, `groupAttribute` y el +toggle `startTls` **no** se validan: el backend los tolera vacíos / usa defaults. + +## Decisiones de diseño + +- **PEM por armadura, no por parseo.** Se verifica la presencia de las líneas + `-----BEGIN/END …-----`. Un admin que pega una clave/cert real siempre trae los + marcadores, y su ausencia es justo el error de dedo que esto debe atrapar. No se + usa `crypto.subtle` para parsear porque no hay API de "parsear sin usar la clave" + y solo engordaría la lib. El parseo real (X.509 / PKCS8) lo hace el backend al + login. +- **`issuer` exige `https`**, mientras que `metadataUrl`/`redirectUrl` aceptan + `http`: la discovery OIDC viaja al issuer con las credenciales, así que el + canal debe ser cifrado. +- **`name` estricto solo en create.** En edit el input está deshabilitado; validar + su formato ahí bloquearía guardar cualquier otro cambio sobre un nombre heredado. +- **`port = 0` es válido.** Borrar el campo produce `0`, que es el default, no un + typo; no debe atrancar el form. + +## Fuentes de las reglas + +Las reglas de formato (URL, `https` estricto para `issuer`, loopback para +`redirectUrl`, URI para `spEntityId`, placeholder `%s` y paréntesis para +`userFilter`) se cruzaron contra las specs de cada protocolo: + +- **OIDC** — [OpenID Connect Core 1.0 §3.1.2.1](https://openid.net/specs/openid-connect-core-1_0.html) + (`client_id`, `redirect_uri` required en la auth request) y + [RFC 8252 §7.3](https://datatracker.ietf.org/doc/html/rfc8252#section-7.3) + (loopback redirect URIs sobre http para clientes nativos). +- **SAML** — guías de proveedores (Cisco IPR, OneStream, Vendasta): el entity ID + del SP puede ser `http(s)` o `urn:`; ACS y metadata siempre son URLs + navegables, y la cert/keys son X.509 / PEM. +- **LDAP** — flujo `service bind → search → user bind` del propio backend + (`modules/iam/usecase/idp.go`): por eso `bindDn` y `bindPassword` son + obligatorios aquí aunque algunas fuentes los marquen opcionales (simple bind). + +El set de *required* por protocolo (presencia) no lo define esta lib, sino el +backend en `prepareSettings`. Esta lib añade el *formato* por encima, de forma +aditiva. + +## Cubertura + +`src/features/settings/lib/idp-form-validation.test.ts` — 41 casos, uno por regla, +corriendo con `npx vitest run`. diff --git a/frontend/src/features/settings/lib/idp-form-validation.test.ts b/frontend/src/features/settings/lib/idp-form-validation.test.ts new file mode 100644 index 000000000..cf50586e4 --- /dev/null +++ b/frontend/src/features/settings/lib/idp-form-validation.test.ts @@ -0,0 +1,249 @@ +import { describe, expect, it } from 'vitest' +import { validateIdpForm, type IdpFormInput } from './idp-form-validation' +import type { LdapSettings, OidcSettings, SamlSettings } from '../types/idp.types' + +const CERT = '-----BEGIN CERTIFICATE-----\nMIIB\n-----END CERTIFICATE-----' +const KEY = '-----BEGIN PRIVATE KEY-----\nMIIB\n-----END PRIVATE KEY-----' + +const samlSettings: SamlSettings = { + metadataUrl: 'https://idp.example.com/metadata', + spEntityId: 'https://utmstack.example.com/sp', + spAcsUrl: 'https://utmstack.example.com/api/v1/sso/saml/acme-entra/login', + spCertificatePem: CERT, +} +const oidcSettings: OidcSettings = { + issuer: 'https://login.microsoftonline.com/tenant/v2.0', + clientId: 'client-123', + redirectUrl: 'https://utmstack.example.com/api/v1/sso/oidc/acme/login', +} +const ldapSettings: LdapSettings = { + host: 'ldap.example.com', + port: 389, + startTls: true, + bindDn: 'cn=svc,cn=users,dc=example,dc=com', + baseDn: 'dc=example,dc=com', + userFilter: '(mail=%s)', + emailAttribute: 'mail', + nameAttribute: 'displayName', + groupAttribute: 'memberOf', +} + +interface CommonOverrides { + name?: string + editing?: boolean + secret?: string +} + +function samlBase(o: CommonOverrides = {}, s: Partial = {}): IdpFormInput { + return { + name: o.name ?? 'acme-entra', + editing: o.editing ?? false, + providerType: 'saml', + secret: o.secret ?? KEY, + settings: { ...samlSettings, ...s }, + } +} +function oidcBase(o: CommonOverrides = {}, s: Partial = {}): IdpFormInput { + return { + name: o.name ?? 'acme-entra', + editing: o.editing ?? false, + providerType: 'oidc', + secret: o.secret ?? 'client-secret', + settings: { ...oidcSettings, ...s }, + } +} +function ldapBase(o: CommonOverrides = {}, s: Partial = {}): IdpFormInput { + return { + name: o.name ?? 'acme-ad', + editing: o.editing ?? false, + providerType: 'ldap', + secret: o.secret ?? 'bind-password', + settings: { ...ldapSettings, ...s }, + } +} + +describe('validateIdpForm — SAML', () => { + it('accepts a fully valid create form', () => { + expect(validateIdpForm(samlBase())).toEqual({}) + }) + + it('accepts an edit form with the secret left blank', () => { + expect(validateIdpForm(samlBase({ editing: true, secret: '' }))).toEqual({}) + }) + + it('flags a metadataUrl that is not a URL', () => { + expect(validateIdpForm(samlBase({}, { metadataUrl: 'not a url' })).metadataUrl).toBe('idp.form.errors.url') + }) + + it('flags an spAcsUrl that is not a URL', () => { + expect(validateIdpForm(samlBase({}, { spAcsUrl: 'ftp://x' })).spAcsUrl).toBe('idp.form.errors.url') + }) + + it('flags a certificate that has no PEM armour', () => { + expect(validateIdpForm(samlBase({}, { spCertificatePem: 'MIIBnotarmoured' })).spCertificatePem).toBe( + 'idp.form.errors.pemCertificate', + ) + }) + + it('requires the private key on create', () => { + expect(validateIdpForm(samlBase({ secret: '' })).spPrivateKeyPem).toBe('idp.form.errors.required') + }) + + it('flags a private key that has no PEM armour on create', () => { + expect(validateIdpForm(samlBase({ secret: 'just some text' })).spPrivateKeyPem).toBe('idp.form.errors.pemKey') + }) + + it('does not flag a blank key on edit (keep the stored one)', () => { + expect(validateIdpForm(samlBase({ editing: true, secret: '' })).spPrivateKeyPem).toBeUndefined() + }) + + it('flags a blank required setting', () => { + expect(validateIdpForm(samlBase({}, { spEntityId: '' })).spEntityId).toBe('idp.form.errors.required') + }) + + it('accepts an https entity ID on create', () => { + expect(validateIdpForm(samlBase({}, { spEntityId: 'https://utmstack.example.com/sp' })).spEntityId).toBeUndefined() + }) + + it('accepts a urn: entity ID on create', () => { + expect(validateIdpForm(samlBase({}, { spEntityId: 'urn:utmstack:sp' })).spEntityId).toBeUndefined() + }) + + it('flags an entity ID that is not a URI (has a space) on create', () => { + expect(validateIdpForm(samlBase({}, { spEntityId: 'https://foo com/sp' })).spEntityId).toBe( + 'idp.form.errors.entityId', + ) + }) + + it('does not flag a non-URI legacy entity ID on edit', () => { + // The field is not disabled on edit, so a value inherited before this rule + // must not lock the form out of saving any other change. + expect(validateIdpForm(samlBase({ editing: true }, { spEntityId: 'not a uri' })).spEntityId).toBeUndefined() + }) +}) + +describe('validateIdpForm — OIDC', () => { + it('accepts a fully valid create form', () => { + expect(validateIdpForm(oidcBase())).toEqual({}) + }) + + it('flags an http issuer (must be https)', () => { + expect(validateIdpForm(oidcBase({}, { issuer: 'http://login.example.com' })).issuer).toBe( + 'idp.form.errors.httpsUrl', + ) + }) + + it('flags a redirectUrl that is not a URL', () => { + expect(validateIdpForm(oidcBase({}, { redirectUrl: 'not a url' })).redirectUrl).toBe( + 'idp.form.errors.redirectUrl', + ) + }) + + it('accepts an https redirectUrl', () => { + expect(validateIdpForm(oidcBase({}, { redirectUrl: 'https://utmstack.example.com/cb' })).redirectUrl).toBeUndefined() + }) + + it.each(['http://localhost:3000/cb', 'http://127.0.0.1/cb', 'http://[::1]:8080/cb'])( + 'accepts a loopback redirectUrl over http (RFC 8252): %s', + (url) => { + expect(validateIdpForm(oidcBase({}, { redirectUrl: url })).redirectUrl).toBeUndefined() + }, + ) + + it('rejects a private-range redirect over http (loopback is the only carve-out)', () => { + expect(validateIdpForm(oidcBase({}, { redirectUrl: 'http://10.0.0.5/cb' })).redirectUrl).toBe( + 'idp.form.errors.redirectUrl', + ) + }) + + it('requires the client secret on create', () => { + expect(validateIdpForm(oidcBase({ secret: '' })).clientSecret).toBe('idp.form.errors.required') + }) + + it('does not require the client secret on edit', () => { + expect(validateIdpForm(oidcBase({ editing: true, secret: '' })).clientSecret).toBeUndefined() + }) + + it('flags a blank client id', () => { + expect(validateIdpForm(oidcBase({}, { clientId: '' })).clientId).toBe('idp.form.errors.required') + }) +}) + +describe('validateIdpForm — LDAP', () => { + it('accepts a fully valid create form', () => { + expect(validateIdpForm(ldapBase())).toEqual({}) + }) + + it('flags a host that carries a scheme', () => { + expect(validateIdpForm(ldapBase({}, { host: 'ldap://ldap.example.com' })).host).toBe('idp.form.errors.hostname') + }) + + it('flags a host with a path', () => { + expect(validateIdpForm(ldapBase({}, { host: 'ldap.example.com/dc=foo' })).host).toBe('idp.form.errors.hostname') + }) + + it('accepts a plain IP host', () => { + expect(validateIdpForm(ldapBase({}, { host: '10.0.0.5' })).host).toBeUndefined() + }) + + it('flags a port above 65535', () => { + expect(validateIdpForm(ldapBase({}, { port: 99999 })).port).toBe('idp.form.errors.port') + }) + + it('accepts a zero port (the backend dials it as 389)', () => { + // Clearing the field yields 0; that is the default, not a mistype, so it must + // not lock the form out of saving. + expect(validateIdpForm(ldapBase({}, { port: 0 })).port).toBeUndefined() + }) + + it('flags a filter missing the %s placeholder', () => { + expect(validateIdpForm(ldapBase({}, { userFilter: '(mail=jane@example.com)' })).userFilter).toBe( + 'idp.form.errors.userFilterPlaceholder', + ) + }) + + it('flags a filter with unbalanced parentheses', () => { + expect(validateIdpForm(ldapBase({}, { userFilter: '(&|(mail=%s)(userPrincipalName=%s)' })).userFilter).toBe( + 'idp.form.errors.userFilterUnbalanced', + ) + }) + + it('accepts a balanced filter with the placeholder', () => { + expect(validateIdpForm(ldapBase({}, { userFilter: '(&(|(mail=%s)(upn=%s)))' })).userFilter).toBeUndefined() + }) + + it('requires the bind password on create', () => { + expect(validateIdpForm(ldapBase({ secret: '' })).bindPassword).toBe('idp.form.errors.required') + }) + + it('does not require the bind password on edit', () => { + expect(validateIdpForm(ldapBase({ editing: true, secret: '' })).bindPassword).toBeUndefined() + }) + + it('flags a blank baseDn', () => { + expect(validateIdpForm(ldapBase({}, { baseDn: '' })).baseDn).toBe('idp.form.errors.required') + }) +}) + +describe('validateIdpForm — name', () => { + it('requires a name', () => { + expect(validateIdpForm(samlBase({ name: ' ' })).name).toBe('idp.form.errors.required') + }) + + it('rejects a name with a space on create', () => { + expect(validateIdpForm(samlBase({ name: 'acme entra' })).name).toBe('idp.form.errors.nameFormat') + }) + + it('rejects a name longer than 64 on create', () => { + expect(validateIdpForm(samlBase({ name: 'a'.repeat(65) })).name).toBe('idp.form.errors.nameFormat') + }) + + it('allows a dot in the name', () => { + expect(validateIdpForm(samlBase({ name: 'acme.entra' })).name).toBeUndefined() + }) + + it('does not block saving an edit over a legacy name with a space', () => { + // The name input is disabled on edit, so its format must not lock the form out. + expect(validateIdpForm(samlBase({ editing: true, name: 'legacy idp name' })).name).toBeUndefined() + }) +}) diff --git a/frontend/src/features/settings/lib/idp-form-validation.ts b/frontend/src/features/settings/lib/idp-form-validation.ts new file mode 100644 index 000000000..69ba0d982 --- /dev/null +++ b/frontend/src/features/settings/lib/idp-form-validation.ts @@ -0,0 +1,186 @@ +import type { ProviderSettings, ProviderType } from '../types/idp.types' + +/** + * Pure, dependency-free validation for the identity provider form. Returns an + * i18n key per field that is wrong (an empty object means the form is good), so + * the component owns the strings and this file owns the rules. The backend only + * checks presence on save and format at login, so a mistyped URL or key is what + * this catches before it is written to the directory. + */ + +export interface IdpFormInput { + name: string + providerType: ProviderType + settings: ProviderSettings + /** The protocol secret, blank on edit to mean "keep it". */ + secret: string + editing: boolean +} + +export type IdpFieldErrors = Partial> + +const NAME_MAX = 64 +// The name goes into the SSO login URL, so a space would break it. Dots are +// allowed (acme.entra) — they are legal path segments. +const NAME_RE = /^[A-Za-z0-9][A-Za-z0-9._-]*$/ +// Host or hostname the LDAP dial builds into ldap://host:port — no scheme. +const HOST_RE = /^(?!.*[\/\s])[A-Za-z0-9.-]+$/ +// Port 1..65535 as a string, no leading zeros. +const PORT_RE = /^(0|[1-9][0-9]*)$/ + +function isHttpUrl(value: string): boolean { + try { + const u = new URL(value) + return u.protocol === 'http:' || u.protocol === 'https:' + } catch { + return false + } +} + +function isHttpsUrl(value: string): boolean { + try { + return new URL(value).protocol === 'https:' + } catch { + return false + } +} + +// RFC 8252 §7.3: native clients may register loopback redirect URIs over http. +// The token exchange from the loopback host is what keeps them off the wire, so +// http on 127.0.0.1/::1/localhost is the standard's own carve-out and a form +// that rejects it blocks a legitimate mobile app configuration. +const LOOPBACK_HOSTS = new Set(['localhost', '127.0.0.1', '::1']) + +function isLoopbackRedirect(value: string): boolean { + try { + const u = new URL(value) + // URL keeps the IPv6 literal bracketed; drop it so [::1] matches ::1. + const host = u.hostname.replace(/^\[|\]$/g, '') + return u.protocol === 'http:' && LOOPBACK_HOSTS.has(host) + } catch { + return false + } +} + +// A SAML entity ID is a URI, not necessarily an http(s) one — urn: is legal. +// We catch "https://foo com/sp" (the space) and "foo bar" here, which is the +// typo an IdP would reject at login; a pure presence check would not. +function isEntityIdUri(value: string): boolean { + try { + const proto = new URL(value).protocol + return proto === 'http:' || proto === 'https:' || proto === 'urn:' + } catch { + return false + } +} + +// A PEM document is its armour lines; the base64 body is never checked because +// an operator pasting a real key always has the markers, and a missing marker +// is exactly the typo we are here to catch. +const CERT_PEM_RE = /-----BEGIN CERTIFICATE-----[\s\S]*?-----END CERTIFICATE-----/ +const KEY_PEM_RE = + /-----BEGIN (RSA |EC |ENCRYPTED |DSA )?PRIVATE KEY-----[\s\S]*?-----END (RSA |EC |ENCRYPTED |DSA )?PRIVATE KEY-----/ + +function samlErrors(input: IdpFormInput, errors: IdpFieldErrors): void { + const s = input.settings + if (s && 'metadataUrl' in s) { + const metadataUrl = s.metadataUrl.trim() + if (metadataUrl && !isHttpUrl(metadataUrl)) errors.metadataUrl = 'idp.form.errors.url' + // Only checked on create: on edit the form carries the stored value, and a + // legacy non-URI entity ID would otherwise block saving any other field. + if (!input.editing) { + const spEntityId = s.spEntityId.trim() + if (spEntityId && !isEntityIdUri(spEntityId)) errors.spEntityId = 'idp.form.errors.entityId' + } + const spAcsUrl = s.spAcsUrl.trim() + if (spAcsUrl && !isHttpUrl(spAcsUrl)) errors.spAcsUrl = 'idp.form.errors.url' + if (s.spCertificatePem.trim() && !CERT_PEM_RE.test(s.spCertificatePem)) { + errors.spCertificatePem = 'idp.form.errors.pemCertificate' + } + } +} + +function oidcErrors(input: IdpFormInput, errors: IdpFieldErrors): void { + const s = input.settings + if (s && 'issuer' in s) { + // Discovery fetches the issuer, so it must be https to keep the secret safe. + const issuer = s.issuer.trim() + if (issuer && !isHttpsUrl(issuer)) errors.issuer = 'idp.form.errors.httpsUrl' + const redirectUrl = s.redirectUrl.trim() + // The one carve-out from the https rule is a loopback redirect for a native + // client, where the token exchange never leaves the machine. + if (redirectUrl && !isLoopbackRedirect(redirectUrl) && !isHttpsUrl(redirectUrl)) { + errors.redirectUrl = 'idp.form.errors.redirectUrl' + } + } +} + +function ldapErrors(input: IdpFormInput, errors: IdpFieldErrors): void { + const s = input.settings + if (!s || !('host' in s)) return + const host = s.host.trim() + if (host && !HOST_RE.test(host)) errors.host = 'idp.form.errors.hostname' + const port = String(s.port ?? '').trim() + if (port !== '' && (!PORT_RE.test(port) || Number(port) > 65535)) errors.port = 'idp.form.errors.port' + const userFilter = s.userFilter.trim() + if (userFilter) { + // A filter is a chain of (attr=value); it needs the placeholder and balanced + // parens. Both are the common ways a search silently returns nothing. + if (!userFilter.includes('%s')) errors.userFilter = 'idp.form.errors.userFilterPlaceholder' + else if ((userFilter.match(/\(/g)?.length ?? 0) !== (userFilter.match(/\)/g)?.length ?? 0)) { + errors.userFilter = 'idp.form.errors.userFilterUnbalanced' + } + } +} + +function commonErrors(input: IdpFormInput, errors: IdpFieldErrors): void { + const name = input.name.trim() + if (!name) { + errors.name = 'idp.form.errors.required' + return + } + // On edit the name input is disabled, so a name with a space or a dot in it + // (legal at save time, but odd in a URL) would otherwise lock the whole form + // out of saving any other field. + if (!input.editing && (name.length > NAME_MAX || !NAME_RE.test(name))) { + errors.name = 'idp.form.errors.nameFormat' + } +} + +export function validateIdpForm(input: IdpFormInput): IdpFieldErrors { + const errors: IdpFieldErrors = {} + commonErrors(input, errors) + + // Presence of the protocol's required settings is the form's existing + // job; the format checks above only fire on values that are present. + const s = input.settings + if (input.providerType === 'saml' && s && 'metadataUrl' in s) { + const missing = ['metadataUrl', 'spEntityId', 'spAcsUrl', 'spCertificatePem'].filter( + (k) => String(s[k as keyof typeof s] ?? '').trim() === '', + ) + for (const k of missing) errors[k] = errors[k] ?? 'idp.form.errors.required' + // The secret is required on create (the backend keeps nothing else) and + // optional on edit (blank means keep it). + if (!input.editing && !input.secret.trim()) { + errors.spPrivateKeyPem = 'idp.form.errors.required' + } else if (input.secret.trim() && !KEY_PEM_RE.test(input.secret)) { + errors.spPrivateKeyPem = 'idp.form.errors.pemKey' + } + } else if (input.providerType === 'oidc' && s && 'issuer' in s) { + for (const k of ['issuer', 'clientId', 'redirectUrl']) { + if (String(s[k as keyof typeof s] ?? '').trim() === '') errors[k] = errors[k] ?? 'idp.form.errors.required' + } + if (!input.editing && !input.secret.trim()) errors.clientSecret = 'idp.form.errors.required' + } else if (input.providerType === 'ldap' && s && 'host' in s) { + for (const k of ['host', 'bindDn', 'baseDn', 'userFilter']) { + if (String(s[k as keyof typeof s] ?? '').trim() === '') errors[k] = errors[k] ?? 'idp.form.errors.required' + } + if (!input.editing && !input.secret.trim()) errors.bindPassword = 'idp.form.errors.required' + } + + if (input.providerType === 'saml') samlErrors(input, errors) + else if (input.providerType === 'oidc') oidcErrors(input, errors) + else ldapErrors(input, errors) + + return errors +} diff --git a/frontend/src/features/settings/pages/IdentityProvidersPage.tsx b/frontend/src/features/settings/pages/IdentityProvidersPage.tsx index 7d3aff029..240b6d909 100644 --- a/frontend/src/features/settings/pages/IdentityProvidersPage.tsx +++ b/frontend/src/features/settings/pages/IdentityProvidersPage.tsx @@ -20,7 +20,8 @@ import { useBilling } from '@/features/billing' import { EnterpriseGate } from '@/shared/components/EnterpriseGate' import { PlatformBroadcastButton, broadcast, BULK_PATHS } from '@/features/platform-broadcast' import { IdpHttpError, idpHttpService } from '../services/idp-http.service' -import type { GroupMapping, IdentityProvider, IdentityProviderRequest, ProviderType } from '../types/idp.types' +import { validateIdpForm } from '../lib/idp-form-validation' +import type { GroupMapping, IdentityProvider, IdentityProviderRequest, ProviderSettings, ProviderType } from '../types/idp.types' import { EMPTY_SETTINGS, PROVIDER_TYPES, REDIRECTING_PROVIDER_TYPES } from '../types/idp.types' import { rolesHttpService, type RoleOption } from '../services/roles-http.service' @@ -383,12 +384,24 @@ function UpsertDialog({ const set = (key: string, value: unknown) => setSettings((s) => ({ ...s, [key]: value })) const str = (key: string) => String(settings[key] ?? '') + // Per-field format errors (URL, PEM, port, %s …). Pure, in the lib; the + // component only renders the i18n key it returns. + const fieldErrors = useMemo( + () => validateIdpForm({ name, providerType, settings: settings as unknown as ProviderSettings, secret, editing }), + [name, providerType, settings, secret, editing], + ) + const fieldErr = (key: string): { invalid: boolean; err: string | undefined } => { + const k = fieldErrors[key] + return { invalid: Boolean(k), err: k ? t(k) : undefined } + } + const required = REQUIRED_FIELDS[providerType] const valid = !!name.trim() && required.every((k) => String(settings[k] ?? '').trim() !== '') && (editing || !!secret.trim()) && - (providerType !== 'ldap' || str('userFilter').includes('%s')) + (providerType !== 'ldap' || str('userFilter').includes('%s')) && + Object.keys(fieldErrors).length === 0 const submit = async () => { if (!valid || busy) return @@ -425,13 +438,14 @@ function UpsertDialog({
- + setName(e.target.value)} placeholder="acme-entra" disabled={editing} - className={editing ? 'opacity-70' : ''} + aria-invalid={fieldErr('name').invalid} + className={cn(fieldErr('name').invalid && 'border-red-500', editing ? 'opacity-70' : '')} /> @@ -451,34 +465,71 @@ function UpsertDialog({ {providerType === 'saml' && ( <> - - set('metadataUrl', e.target.value)} className="font-mono text-xs" /> + + set('metadataUrl', e.target.value)} + aria-invalid={fieldErr('metadataUrl').invalid} + className={cn('font-mono text-xs', fieldErr('metadataUrl').invalid && 'border-red-500')} + />
- - set('spEntityId', e.target.value)} className="font-mono text-xs" /> + + set('spEntityId', e.target.value)} + aria-invalid={fieldErr('spEntityId').invalid} + className={cn('font-mono text-xs', fieldErr('spEntityId').invalid && 'border-red-500')} + /> - - set('spAcsUrl', e.target.value)} className="font-mono text-xs" /> + + set('spAcsUrl', e.target.value)} + aria-invalid={fieldErr('spAcsUrl').invalid} + className={cn('font-mono text-xs', fieldErr('spAcsUrl').invalid && 'border-red-500')} + />
- -