Update Domains page with ability to add/remove "organization" domains, view "administrator" domains set via security settings, and improve various UX bugs and copy

[adamsachs]

Addresses a number of issues:

• closes PROD-1419, i.e. addresses bug identified in this comment. for tracking purposes, this limitation had been called out in original PR description, but we hadn't done due diligence to ensure that the limitation was acceptable -- turns out, it wasn't!
• closes PROD-1404 by updating copy, navigation, and various labels/messages to "Domains"
• closes PROD-1468 by improving the FE validation tests and descriptive error message

Description Of Changes

BE updates (@adamsachs)

in previous uses of the config proxy, we haven't had a requirement to merge (or reconcile) both API-set and config-set (i.e. via .toml or env var) values for a given app config property. the requirement here, i.e. to overcome the limitation mentioned above, requires this type of resolution.

rather than try to special-case the logic too heavily on the security.cors_origins config property and its uses, i've built what i hope is a relatively generic mechanism for config resolution into the underlying config proxy. in order to leverage the merge functionality, though, a developer must specifically annotate a given config proxy property as a merge_property with a class decorator. this is probably a bit easier to see in action, in the code diff, than it is to explain in words 😅

FE updates (@NevilleS)

This PR overhauls the "CORS Configuration" page and rebrands it as "Domains" with a number of improvements:

• updated to use the improved config API (see above!) to get/set CORS origins from the UI that are merged into any config-set values on the server
• query the config API to retrieve any config-set values (both security.cors_origins and security.cors_origin_regex) and display them as read-only values in the UI
• improve the FE validations to catch common gotchas for origins: require HTTP(s) URLs, don't allow wildcards, don't allow paths
• update the copy to be a bit friendlier, with some tooltips for more details
• add Cypress E2E tests to ensure the FE behaviour is tested

Code Changes

BE updates

• provide generic merge_values behavior for the config proxy's property resolution, for merge_properties that are specifically annotated to have their values merged
  • only Iterable property values can be merged. it'd be presumptuous to try and merge non-iterable elements in a generic way, so better not pretend or intend to support it at all!
• apply the merge_values logic to the security.cors_origins config property by specifying it in the merge_properties of the SecuritySettingsProxy constructor

FE updates

• Update nav-config and tests with revised labels for Domain pages
• Update Redux reducers to fetch both api_set=true and api_set=false config for CORS security settings
• Add additional Yum validations for isValidURL, containsNoWildcard, containsNoPath and use in form
• Render the BE "config-set" values for security.cors_origins and security.cors_origin_regex as read-only input fields on the Domains page
• Add Cypress tests to cover basic view/edit/remove/validate cases

Steps to Confirm

• i was able to adjust automated unit and "integration" tests to provide good coverage here
• i tested locally with the test setup that's in the original PR's loom, and ensured that the limitation is no longer present:
  https://www.loom.com/share/ee68c8f83df1445dac0d7578532ffd38
• Test that config-set domains are allowlisted for CORS (e.g. http://localhost)
• Test adding a new, valid domain (https://example.com) and confirm CORS is allowed
• Test removing a valid domain (https://example.com) and confirm CORS is blocked
• Test the various FE validations for consistency

Pre-Merge Checklist

• All CI Pipelines Succeeded
• Documentation:
  • documentation complete, PR opened in fidesdocs
  • documentation issue created in fidesdocs
• Issue Requirements are Met
• Relevant Follow-Up Issues Created
• Update CHANGELOG.md
• For API changes, the Postman collection has been updated

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
fides-plus-nightly	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Feb 1, 2024 11:37pm

[adamsachs]

some light context:

[cypress]

Passing run #6166 ↗︎

0

4

0

0

0

Details:

Merge 4373cba into 67a515a...
Project: fides	Commit: d1fc94a893 ℹ️
Status: Passed	Duration: 00:33 💡
Started: Feb 1, 2024 11:45 PM	Ended: Feb 1, 2024 11:45 PM

Review all test suite changes for PR #4584 ↗︎

[codecov]

Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

Comparison is base (65af596) 78.06% compared to head (54a0cba) 86.80%.
Report is 2 commits behind head on main.

Files	Patch %	Lines
src/fides/api/models/application_config.py	73.33%	2 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4584      +/-   ##
==========================================
+ Coverage   78.06%   86.80%   +8.74%     
==========================================
  Files         330      330              
  Lines       19839    19858      +19     
  Branches     2545     2550       +5     
==========================================
+ Hits        15487    17238    +1751     
+ Misses       3888     2152    -1736     
- Partials      464      468       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[NevilleS]

I think this feels pretty good... a couple nitpicks in here that will feel entirely onerous I'm sure.

I'm going to pull this locally and try it out with the UI. Assuming I can do that successfully, I can run UAT pretty quickly and wrap this one up 👍

[NevilleS]

OK, I did some manual UAT on this.

My setup was to point my fidesplus test environment at this fides branch, then run the test environment. Our default test environment pre-configures a couple CORS origins already by setting security.cors_origins with these:

      "http://localhost",
      "http://localhost:8080",
      "http://localhost:3000",
      "http://localhost:3001"

My test steps were:

1. Confirm that http://localhost:3000 is still permitted by default
2. Confirm that https://example.com is not permitted by default
3. Use the Admin UI to add https://example.com as a domain, and confirm that both https://example.com and http://localhost:3000 are permitted
4. Use the Admin UI to remove https://example.com, confirm it is no longer supported

This looks pretty good to me...

[galvana]

Verified functionality locally 👍 I just made some small recommendations for the domain page layout

[NevilleS]

Added an empty state:

[galvana]

Beautiful 🙌