Skip to content

Allowed Domains Part 2 secrets validation#7423

Merged
Linker44 merged 34 commits intomainfrom
ENG-2569-part2-secrets-validation
Mar 2, 2026
Merged

Allowed Domains Part 2 secrets validation#7423
Linker44 merged 34 commits intomainfrom
ENG-2569-part2-secrets-validation

Conversation

@Linker44
Copy link
Contributor

@Linker44 Linker44 commented Feb 19, 2026

Ticket ENG-2569

Description Of Changes

Second of a 4-part series. This PR validates endpoint values against allowed_values at the time connection secrets are set, and fixes a pre-existing Pydantic V2 issue with get_connector_param.

The old get_connector_param used PrivateAttr + __private_attributes__ to store connector param metadata on dynamically created models. This was brittle in Pydantic V2 — the code had multiple defensive guards and a TODO acknowledging the fragility. This PR replaces that approach by storing metadata in json_schema_extra on each field (the same mechanism already used for sensitive), making it accessible via the stable cls.model_fields API.

Code Changes

  • Replaced _connector_params PrivateAttr with per-field json_schema_extra containing sensitive, options, multiselect, param_type, and allowed_values in connection_secrets_saas.py
  • Rewrote SaaSSchema.get_connector_param to read from cls.model_fields[name].json_schema_extra
  • Added value validation in required_components_supplied that checks values against allowed_values when param_type == "endpoint" and secrets are set
  • Updated test_connection_template_endpoints.py hubspot schema test to include the new json_schema_extra keys in the expected response

Steps to Confirm

  1. Verify that setting a connector secret with a value matching allowed_values succeeds
  2. Verify that setting a value not in allowed_values raises a validation error
  3. Verify that validation is skipped when disable_domain_validation is true or allowed_values is None
  4. Verify that existing options/multiselect validation still works (e.g., test_value_not_in_options)

Pre-Merge Checklist

  • Issue requirements met
  • All CI pipelines succeeded
  • CHANGELOG.md updated
    • Add a db-migration This indicates that a change includes a database migration label to the entry if your change includes a DB migration
    • Add a high-risk This issue suggests changes that have a high-probability of breaking existing code label to the entry if your change includes a high-risk change (i.e. potential for performance impact or unexpected regression) that should be flagged
    • Updates unreleased work already in Changelog, no new entry necessary
  • UX feedback:
    • All UX related changes have been reviewed by a designer
    • No UX review needed
  • Followup issues:
    • Followup issues created
    • No followup issues
  • Database migrations:
    • Ensure that your downrev is up to date with the latest revision on main
    • Ensure that your downgrade() migration is correct and works
      • If a downgrade migration is not possible for this change, please call this out in the PR description!
    • No migrations
  • Documentation:
    • Documentation complete, PR opened in fidesdocs
    • Documentation issue created in fidesdocs
    • If there are any new client scopes created as part of the pull request, remember to update public-facing documentation that references our scope registry
    • No documentation updates required

@Linker44 Linker44 self-assigned this Feb 19, 2026
@vercel
Copy link
Contributor

vercel bot commented Feb 19, 2026

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

Project Deployment Actions Updated (UTC)
fides-plus-nightly Ready Ready Preview, Comment Mar 2, 2026 11:21pm
1 Skipped Deployment
Project Deployment Actions Updated (UTC)
fides-privacy-center Ignored Ignored Mar 2, 2026 11:21pm

Request Review

@Linker44 Linker44 changed the base branch from main to ENG-2569-part1-foundation February 19, 2026 14:26
@Linker44 Linker44 marked this pull request as ready for review February 19, 2026 18:33
@Linker44 Linker44 requested a review from a team as a code owner February 19, 2026 18:33
@Linker44 Linker44 requested review from erosselli and removed request for a team February 19, 2026 18:33
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 19, 2026

Greptile Summary

This PR implements domain validation for SaaS connector endpoint parameters and refactors connector param metadata storage from Pydantic V2's fragile PrivateAttr mechanism to the stable json_schema_extra field API.

Key changes:

  • Replaced _connector_params PrivateAttr with per-field metadata in json_schema_extra containing sensitive, options, multiselect, param_type, and allowed_values
  • Rewrote SaaSSchema.get_connector_param() to read from cls.model_fields[name].json_schema_extra instead of __private_attributes__
  • Added domain validation in required_components_supplied() that checks endpoint values against allowed_values patterns (supporting wildcards like *.salesforce.com)
  • Validation is skipped when disable_domain_validation config flag is true or allowed_values is None/empty
  • Test coverage includes allowed/disallowed domains, validation toggles, wildcard patterns, and edge cases

Confidence Score: 5/5

  • This PR is safe to merge with no concerns
  • The implementation is clean and well-tested. The refactoring from PrivateAttr to json_schema_extra removes technical debt and uses Pydantic's stable API. Domain validation logic properly handles edge cases (None, empty lists, wildcards) with appropriate guard clauses. Comprehensive test coverage validates all paths including validation toggles and pattern matching.
  • No files require special attention

Important Files Changed

Filename Overview
src/fides/api/schemas/connection_configuration/connection_secrets_saas.py Refactored metadata storage from fragile PrivateAttr to stable json_schema_extra and added domain validation for endpoint params. Clean implementation with proper guard clauses.
tests/ops/api/v1/endpoints/test_connection_template_endpoints.py Updated expected schema to include new json_schema_extra fields (options, multiselect, param_type, allowed_values). Straightforward test maintenance.
tests/ops/schemas/connection_configuration/test_connection_secrets_saas.py Added comprehensive domain validation tests covering allowed/disallowed domains, validation toggle, empty/None allowed_values, and wildcard patterns. Excellent edge case coverage.

Last reviewed commit: c1c3ce1

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

3 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

Copy link
Contributor

@daveqnet daveqnet left a comment

Choose a reason for hiding this comment

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

Commenting just to let you know I've reviewed this and have no issues with it @Linker44

@Linker44 Linker44 requested a review from galvana February 26, 2026 15:30
@Linker44
Copy link
Contributor Author

@greptileai

Base automatically changed from ENG-2569-part1-foundation to main March 2, 2026 23:11
@github-actions github-actions bot requested a review from a team as a code owner March 2, 2026 23:11
@github-actions github-actions bot requested review from RobertKeyser and removed request for a team March 2, 2026 23:11
@Linker44 Linker44 enabled auto-merge March 2, 2026 23:20
@Linker44 Linker44 added this pull request to the merge queue Mar 2, 2026
Merged via the queue into main with commit 4be05c7 Mar 2, 2026
56 of 57 checks passed
@Linker44 Linker44 deleted the ENG-2569-part2-secrets-validation branch March 2, 2026 23:52
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.

3 participants