ENG-2519: Additional SSO Configuration by tvandort · Pull Request #7351 · ethyca/fides

tvandort · 2026-02-09T23:14:57Z

Ticket ENG-2519

Description Of Changes

Allows further SSO configuration including scopes, fields, verification toggle.

Steps to Confirm

Good question! I don't know since I don't have a good example SSO to use this against.

Pre-Merge Checklist

vercel · 2026-02-09T23:15:03Z

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

2 Skipped Deployments

Project	Deployment	Actions	Updated (UTC)
fides-plus-nightly	Ignored	Preview	Feb 12, 2026 8:56pm
fides-privacy-center	Ignored		Feb 12, 2026 8:56pm

greptile-apps · 2026-02-10T21:09:40Z

Greptile Overview

Greptile Summary

This PR adds additional SSO configuration options for custom OpenID providers, including scopes, email field mappings, and email verification settings.

Key Changes:

Added four new columns to openid_provider table: scopes, email_field, verify_email, and verify_email_field
Frontend form now supports configuring these fields for custom SSO providers
verify_email defaults to true to require verified emails by default

Issues Found:

Backend model missing server_default="t" for verify_email column (should match migration)
TypeScript types incorrectly mark verify_email as nullable when backend has nullable=False

Confidence Score: 3/5

This PR has minor issues that should be fixed before merging to avoid runtime errors
The migration and frontend form implementation are solid, but there's a mismatch between the model definition and migration (server_default missing), and the auto-generated TypeScript types incorrectly mark verify_email as nullable when the database column is non-nullable. These issues could cause runtime errors.
Pay close attention to src/fides/api/models/openid_provider.py and the API schema generation that produces the TypeScript types

Important Files Changed

Filename	Overview
src/fides/api/models/openid_provider.py	Added new SSO configuration fields (`scopes`, `email_field`, `verify_email`, `verify_email_field`), but model is missing `server_default="t"` for `verify_email` column to match the migration
src/fides/api/alembic/migrations/versions/xx_2026_02_10_1850_4d64174f422e_additional_sso_configurations.py	Migration correctly adds four new columns with proper nullable settings and `server_default="t"` for boolean column
clients/admin-ui/src/types/api/models/OpenIDProvider.ts	Auto-generated TypeScript types add new fields but incorrectly define `verify_email` as nullable when backend model has `nullable=False`
clients/admin-ui/src/features/openid-authentication/SSOProviderForm.tsx	Added new form fields for scopes, email configuration, and email verification with proper conditional rendering. `verify_email` defaults to `true` correctly

greptile-apps

_{3 files reviewed, 3 comments}

_{Edit Code Review Agent Settings | Greptile}

...alembic/migrations/versions/xx_2026_02_10_1850_4d64174f422e_additional_sso_configurations.py

...lembic/migrations/versions/xx_2026_02_10_1850_4d64174f422e_additional_cors_configurations.py

greptile-apps · 2026-02-10T21:09:51Z

Additional Comments (1)

clients/admin-ui/src/features/openid-authentication/SSOProviderForm.tsx
TypeScript types for OpenIDProviderCreate and OpenIDProvider are missing the new fields added to the backend model (scopes, email_field, verify_email, verify_email_field). Need to regenerate types by running npm run openapi:generate after updating backend Pydantic schemas, or manually add these fields to the type definitions.

tvandort · 2026-02-10T22:27:31Z

@greptileai pls rereview

greptile-apps

_{7 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

clients/admin-ui/src/features/openid-authentication/SSOProviderForm.tsx

tvandort · 2026-02-10T22:37:24Z

@greptileai pls rereview

greptile-apps

_{7 files reviewed, 2 comments}

_{Edit Code Review Agent Settings | Greptile}

src/fides/api/models/openid_provider.py

greptile-apps · 2026-02-10T22:41:22Z

clients/admin-ui/src/types/api/models/OpenIDProvider.ts

  domain?: string | null;
+  scopes?: Array<string> | null;
+  email_field?: string | null;
+  verify_email?: boolean | null;


Type mismatch: verify_email is defined as nullable here but the backend model has nullable=False. This could cause runtime errors if the frontend sends null. Since these are auto-generated files, check the backend API schema generation to ensure it correctly reflects nullable=False.

I'll try it but this should be fine given that the database has a default value.

lucanovera

Code changes look good. Form generally looks good, but I was very confused when looking at this state:

I get now that this is meant to support an array of multiple string values, but I don't think that is communicated correctly. Could we change that something that looks like this?

lucanovera

Tested UI with the new array interface, add/edit/save works correctly. approved

lucanovera · 2026-02-11T20:01:43Z

@tvandort I noticed when you edit a provider, the client id and secret fields are empty but marked as required so you can't save until you enter those again. I'm not sure if the api supports partial updates or not, in any case it can be a followup improvement

thabofletcher · 2026-02-11T21:02:39Z

Migration/BE model looks good - I have not tested.

tvandort · 2026-02-12T21:34:45Z

@tvandort I noticed when you edit a provider, the client id and secret fields are empty but marked as required so you can't save until you enter those again. I'm not sure if the api supports partial updates or not, in any case it can be a followup improvement

@lucanovera I didn't see this before but I believe this is by design. If you were able to change the authorization endpoint without re-entering the client id and client secret you a malicious administrator could redirect the auth flow to a malicious server and receive the id and secret.

Could be wrong but it reads to me like a feature not a bug.

vercel bot deployed to Preview – fides-plus-nightly February 9, 2026 23:17 View deployment

tvandort force-pushed the ENG-2519-additional-sso-configuration branch from d6706cf to 8d912a1 Compare February 9, 2026 23:18

vercel bot deployed to Preview – fides-plus-nightly February 9, 2026 23:22 View deployment

tvandort force-pushed the ENG-2519-additional-sso-configuration branch 3 times, most recently from 35e741e to 9e38fbc Compare February 10, 2026 20:08

tvandort marked this pull request as ready for review February 10, 2026 20:59

tvandort requested review from a team as code owners February 10, 2026 20:59

tvandort requested review from lucanovera and thabofletcher and removed request for a team February 10, 2026 20:59

greptile-apps bot reviewed Feb 10, 2026

View reviewed changes

...alembic/migrations/versions/xx_2026_02_10_1850_4d64174f422e_additional_sso_configurations.py Outdated Show resolved Hide resolved

...lembic/migrations/versions/xx_2026_02_10_1850_4d64174f422e_additional_cors_configurations.py Outdated Show resolved Hide resolved

vercel bot had a problem deploying to Preview – fides-plus-nightly February 10, 2026 21:37 Failure

vercel bot deployed to Preview – fides-plus-nightly February 10, 2026 21:49 View deployment

greptile-apps bot reviewed Feb 10, 2026

View reviewed changes

clients/admin-ui/src/features/openid-authentication/SSOProviderForm.tsx Outdated Show resolved Hide resolved

vercel bot deployed to Preview – fides-plus-nightly February 10, 2026 22:36 View deployment

greptile-apps bot reviewed Feb 10, 2026

View reviewed changes

lucanovera requested changes Feb 11, 2026

View reviewed changes

vercel bot deployed to Preview – fides-plus-nightly February 11, 2026 19:53 View deployment

lucanovera approved these changes Feb 11, 2026

View reviewed changes

thabofletcher approved these changes Feb 11, 2026

View reviewed changes

vercel bot deployed to Preview – fides-plus-nightly February 12, 2026 19:37 View deployment

tvandort added 20 commits February 12, 2026 15:14

Extended SSO implementation.

a73782f

Explicitely state default.

d71be78

Changelog.

39af9be

Move migration.

e7b40e5

Fix migration.

ffde50b

New migration.

63bd7e0

Robo theory.

221c4ee

Try another migration again.

0b72371

Different migration?

5557e54

Align model.

8231799

Update data categorization.

52e2fd6

Robot feedback.

23ae831

TLAs

1e171e2

Generated API updates.

572a8f3

Make types happier.

2bb9c9c

Form validation.

c018b20

Email field.

8ba3d0c

Update scopes UI.

0f819b2

Nullable.

b5e5d90

Update down-revision & add tag to changelog.

fc23a8b

tvandort force-pushed the ENG-2519-additional-sso-configuration branch from b8c1605 to fc23a8b Compare February 12, 2026 20:25

tvandort added 2 commits February 12, 2026 15:53

Manualy update changelog.

7640626

db-migration tag

697a2c6

tvandort added this pull request to the merge queue Feb 12, 2026

Merged via the queue into main with commit f9855d8 Feb 12, 2026
59 of 60 checks passed

tvandort deleted the ENG-2519-additional-sso-configuration branch February 12, 2026 22:10

tvandort added a commit that referenced this pull request Feb 12, 2026

ENG-2519: Additional SSO Configuration (#7351)

91c9f02

Conversation

tvandort commented Feb 9, 2026 • edited by atlassian bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description Of Changes

Steps to Confirm

Pre-Merge Checklist

Uh oh!

vercel bot commented Feb 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

greptile-apps bot commented Feb 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Overview

Greptile Summary

Confidence Score: 3/5

Important Files Changed

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

greptile-apps bot commented Feb 10, 2026

Uh oh!

tvandort commented Feb 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tvandort commented Feb 10, 2026

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

greptile-apps bot Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

tvandort Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

lucanovera left a comment

Choose a reason for hiding this comment

Uh oh!

lucanovera left a comment

Choose a reason for hiding this comment

Uh oh!

lucanovera commented Feb 11, 2026

Uh oh!

thabofletcher commented Feb 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tvandort commented Feb 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

tvandort commented Feb 9, 2026 •

edited by atlassian bot

Loading

vercel bot commented Feb 9, 2026 •

edited

Loading

greptile-apps bot commented Feb 10, 2026 •

edited

Loading

tvandort commented Feb 10, 2026 •

edited

Loading

thabofletcher commented Feb 11, 2026 •

edited

Loading

tvandort commented Feb 12, 2026 •

edited

Loading