Skip to content

validate identity provider form fields before save - #2816

Open
AndresTK89 wants to merge 3 commits into
release/v12.0.0from
backlog/identity-provider-form-field-validation
Open

AndresTK89 wants to merge 3 commits into
release/v12.0.0from
backlog/identity-provider-form-field-validation

Conversation

@AndresTK89

Copy link
Copy Markdown

feat(v12): validate identity provider form fields before save

The IdP create/edit form only checked presence and the backend only
checked format at login, so a mistyped URL, PEM or port reached a
signing user instead of the admin who typed it.

Add src/features/settings/lib/idp-form-validation.ts (pure, no React)
that returns one i18n key per bad field, and wire it into the
UpsertDialog: per-field red border + aria-invalid + message, and the
save button gated on it.

Rules per protocol:

  • name: required, <=64, no spaces (create only; the edit input is
    disabled, so its format must not lock the form out of saving
    anything else)
  • saml: metadataUrl + spAcsUrl are http(s); spEntityId is a URI
    (http/https/urn) on create; certificate and private key must carry
    PEM armour; secret required on create, optional on edit
  • oidc: issuer is https (discovery runs against it); redirectUrl is
    https or an http loopback per RFC 8252 section 7.3; client secret
    required on create
  • ldap: host is a hostname/IP without scheme; port is an integer
    1-65535 (0 is the 389 default, not a mistype); userFilter must
    contain %s and have balanced parentheses; bind password required
    on create

Fix the LDAP port input: it stored Number(e.target.value), so a float
like 389.5 400'd on unmarshal to int. Now Math.trunc(...).

Sources cross-checked: OpenID Connect Core 1.0, RFC 8252 section 7.3,
SAML vendor guides (Cisco IPR, OneStream, Vendasta), and the backend's
own service-bind -> search -> user-bind flow in modules/iam/usecase/idp.go.

41 unit tests in idp-form-validation.test.ts, one per rule.
i18n keys under idp.form.errors.* in en/es/fr/de/it/pt/ru.

@github-actions

github-actions Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

🛑 AI review — Sensitive area, extra care recommended

This PR touches critical paths or introduces changes the model cannot judge with sufficient confidence. Review carefully before merging.

🛑 architecture (silas-1.7-pro) — high/critical — please review

Summary: Tier 3: IdP auth/secret handling changes, including a removed LDAP bind-password assignment and new save-time format validation that can affect existing deployments.

  • high backend/modules/iam/usecase/idp.go:158 — Removes s.BindPassword = kept after keepOrEncrypt, so the LDAP bind password may be persisted as the raw input or blank instead of the encrypted/kept value. Restore the assignment or make the secret-retention helper mutate the settings struct.
  • high backend/modules/iam/usecase/idp.go:94 — New save-time format validation for SAML/OIDC/LDAP can reject edits to legacy IdP configurations that were previously accepted. Validate only newly supplied fields or preserve prior values to avoid breaking existing deployments.
  • medium backend/modules/iam/usecase/idp.go:250 — New IdP name restriction changes the public upsert contract for creates. If clients may send legacy names, document the change or defer URL-safety validation to SSO route construction.
  • low frontend/src/features/settings/lib/idp-form-validation.ts:1 — Duplicated URL/PEM/loopback validation rules with backend idp_validation.go. Consider a shared contract, generated tests, or cross-language golden tests to prevent drift.
  • low backend/modules/iam/usecase/idp_validation.go:19 — validateIDPSettingsFormat appears unused in this diff. Remove it or wire it into the validation path to avoid dead code.

🛑 bugs (silas-1.7-pro) — high/critical — please review

Summary: Tier 2: LDAP bind password is not persisted after keepOrEncrypt; additional validation gaps for IPv6, whitespace, and edit name changes plus i18n/doc inconsistencies.

  • high backend/modules/iam/usecase/idp.go:158 — The LDAP case removed s.BindPassword = kept before json.Marshal(s). Create stores the plaintext input and edit stores blank, so the kept/encrypted bind password is lost and LDAP login breaks.
  • medium backend/modules/iam/usecase/idp_validation.go:62 — Validators trim values only for checking but prepareSettings stores the original untrimmed strings. A host or URL with leading/trailing spaces passes validation, is persisted, and can fail at login.
  • medium backend/modules/iam/usecase/idp_validation.go:16 — idpHostRe allows only [A-Za-z0-9.-]+, so valid IPv6 LDAP hosts such as ::1 or 2001:db8::1 are rejected.
  • medium frontend/src/features/settings/lib/idp-form-validation.ts:27 — HOST_RE uses [A-Za-z0-9.-]+ and rejects IPv6 hosts, so the form blocks valid LDAP IPv6 addresses.
  • medium backend/modules/iam/usecase/idp.go:250 — Name format is enforced only when previous == nil. If the edit API accepts a changed name, a non-URL-safe name can be persisted because the regex is skipped for existing providers.
  • low frontend/src/features/settings/lib/idp-form-validation.ts:130 — Missing parentheses around the right-hand ?? 0 makes the condition parse as (openCount !== closeCount) ?? 0; a filter with zero parentheses is flagged unbalanced.
  • low frontend/src/shared/i18n/locales/en.json:1348 — Port error says between 1 and 65535, but validation and backend allow 0 as the default 389.
  • low frontend/src/shared/i18n/locales/de.json:1196 — Port error says between 1 and 65535, but validation and backend allow 0 as the default 389.
  • low frontend/src/shared/i18n/locales/es.json:1196 — Port error says between 1 and 65535, but validation and backend allow 0 as the default 389.
  • low frontend/src/shared/i18n/locales/fr.json:1196 — Port error says between 1 and 65535, but validation and backend allow 0 as the default 389.
  • low frontend/src/shared/i18n/locales/it.json:1196 — Port error says between 1 and 65535, but validation and backend allow 0 as the default 389.
  • low frontend/src/shared/i18n/locales/pt.json:1196 — Port error says between 1 and 65535, but validation and backend allow 0 as the default 389.
  • low frontend/src/shared/i18n/locales/ru.json:1208 — Port error says between 1 and 65535, but validation and backend allow 0 as the default 389.
  • low frontend/src/features/settings/lib/idp-form-validation.md:16 — Doc says backend only checks presence on save, but this PR adds format validation in prepareSettings.

🛑 security (silas-1.7-pro) — high/critical — please review

Summary: LDAP IDP bind password can be stored in plaintext because the encrypted kept value is not assigned before marshaling settings.

  • high backend/modules/iam/usecase/idp.go:158 — In the LDAP case, s.BindPassword is not set to the result of keepOrEncrypt before json.Marshal(s). If a new bind password is supplied, the plaintext request value is persisted instead of the encrypted secret. Restore s.BindPassword = kept after the kept == "" check, matching SAML/OIDC handling.

🔴 go-deps — pending updates

🔍 Discovered 30 Go projects

📦 Dependencies with updates available:

  📁 ./plugins/gcp:
     - github.com/threatwinds/go-sdk: v1.1.37-0.20261001164002-a596631a84c5 → v1.1.37

  📁 ./plugins/aws:
     - github.com/threatwinds/go-sdk: v1.1.37-0.20261001164002-a596631a84c5 → v1.1.37

  📁 ./plugins/alerts:
     - github.com/threatwinds/go-sdk: v1.1.37-0.20261001164002-a596631a84c5 → v1.1.37

  📁 ./plugins/events:
     - github.com/threatwinds/go-sdk: v1.1.37-0.20261001164002-a596631a84c5 → v1.1.37

  📁 ./plugins/stats:
     - github.com/threatwinds/go-sdk: v1.1.37-0.20261001164002-a596631a84c5 → v1.1.37

  📁 ./plugins/rule-flood-guard:
     - github.com/threatwinds/go-sdk: v1.1.37-0.20261001164002-a596631a84c5 → v1.1.37

  📁 ./plugins/o365:
     - github.com/threatwinds/go-sdk: v1.1.37-0.20261001164002-a596631a84c5 → v1.1.37

  📁 ./plugins/playground:
     - github.com/threatwinds/go-sdk: v1.1.37-0.20261001164002-a596631a84c5 → v1.1.37

  📁 ./plugins/soc-ai:
     - github.com/threatwinds/go-sdk: v1.1.37-0.20261001164002-a596631a84c5 → v1.1.37

  📁 ./plugins/sophos:
     - github.com/threatwinds/go-sdk: v1.1.37-0.20261001164002-a596631a84c5 → v1.1.37

  📁 ./plugins/azure:
     - github.com/threatwinds/go-sdk: v1.1.37-0.20261001164002-a596631a84c5 → v1.1.37

  📁 ./plugins/crowdstrike:
     - github.com/threatwinds/go-sdk: v1.1.37-0.20261001164002-a596631a84c5 → v1.1.37

  📁 ./plugins/bitdefender:
     - github.com/threatwinds/go-sdk: v1.1.37-0.20261001164002-a596631a84c5 → v1.1.37

  📁 ./plugins/feeds:
     - github.com/threatwinds/go-sdk: v1.1.37-0.20261001164002-a596631a84c5 → v1.1.37

  📁 ./plugins/geolocation:
     - github.com/threatwinds/go-sdk: v1.1.37-0.20261001164002-a596631a84c5 → v1.1.37

  📁 ./plugins/soar:
     - github.com/threatwinds/go-sdk: v1.1.37-0.20261001164002-a596631a84c5 → v1.1.37

  📁 ./backend:
     - github.com/threatwinds/go-sdk: v1.1.27-0.20260819160318-c56c250bc585 → v1.1.37

  📁 ./tools/rulecheck:
     - github.com/threatwinds/go-sdk: v1.1.31 → v1.1.37

  📁 ./agent-manager:
     - github.com/threatwinds/go-sdk: v1.1.31 → v1.1.37

  📁 ./log-input:
     - github.com/threatwinds/go-sdk: v1.1.31 → v1.1.37

  📁 ./agent:
     - github.com/threatwinds/go-sdk: v1.1.28 → v1.1.37

  📁 ./collectors/utmstack:
     - github.com/threatwinds/go-sdk: v1.1.31 → v1.1.37

  📁 ./collectors/forwarder:
     - github.com/threatwinds/go-sdk: v1.1.31 → v1.1.37

  📁 ./collectors/as400:
     - github.com/threatwinds/go-sdk: v1.1.31 → v1.1.37

❌ Please update dependencies before merging.

@AlexSanchez-bit AlexSanchez-bit linked an issue Oct 1, 2026 that may be closed by this pull request
2 of 3 tasks

This branch has not been deployed

No deployments
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.

identity providers form lacks of validation of fields

2 participants