remove api token and addition access key #6960

DrJfrost · 2025-11-13T17:53:50Z

Ticket ENG-1916
https://ethyca.atlassian.net/browse/ENG-1916

Description Of Changes

Analyzed existing OAuth2 client-credentials patterns to guide the Okta worker migration and removed the remaining Okta API-token legacy usage so the connector now expects OAuth2-only secrets. Adjusted schemas, fixtures, configs, and tests to align with the new access-token fields while warning if a legacy token is still supplied.

Code Changes

reviewed existing OAuth2 client-credentials strategy implementations and documented reusable patterns (analysis only; no code changes required)
removed api_token handling from OktaConnector and introduced _get_oauth_access_token() erroring when OAuth2 secrets are missing
updated OktaSchema to deprecate/ignore api_token, surface access_token, and adjusted connection model defaults
refreshed SaaS YAML fixtures, integration tests, and Okta connection fixtures to rely on access_token
ensured Nox/test helpers reference OAuth2 environment variables instead of legacy API-token settings

Steps to Confirm

python -m pytest tests/ops/service/connectors/saas/test_zipfile_response_management.py
python -m pytest tests/ops/integration_tests/test_connection_configuration_integration.py -k okta

Pre-Merge Checklist

…word handling.

vercel · 2025-11-13T17:53:56Z

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

Project	Deployment	Preview	Comments	Updated (UTC)
fides-plus-nightly	Ready	Preview	Comment	Nov 14, 2025 11:02pm

1 Skipped Deployment

Project	Deployment	Preview	Comments	Updated (UTC)
fides-privacy-center	Ignored			Nov 14, 2025 11:02pm

greptile-apps · 2025-11-13T18:04:46Z

Greptile Overview

Greptile Summary

This PR migrates the Okta connector from legacy API token authentication to OAuth2 access token authentication. The changes deprecate the api_token field while introducing access_token, updating all related tests, fixtures, and configuration files to use the new field name.

Key changes:

Deprecated api_token in OktaSchema with a warning when detected
Added access_token field for OAuth2 authentication
Updated OktaConnector to validate and use access_token with improved error handling
Added backward-compatible model_dump() logic in OktaConfig to map access_token to token for the Okta client
Updated all test fixtures, YAML configs, and environment variable references from OKTA_API_TOKEN to OKTA_ACCESS_TOKEN

Issues identified:

The error message in _get_oauth_access_token() misleadingly suggests implementing an OAuth2 Client Credentials module, but this PR only adds static token support without automatic refresh
The access_token field should be in _required_components to provide early validation when creating connections, rather than failing at runtime

Confidence Score: 3/5

This PR has moderate risk due to validation gaps and misleading error messages that could confuse users
Score reflects two logical issues: (1) misleading error message suggesting OAuth2 auto-refresh when only static tokens are supported, and (2) missing schema validation for access_token which allows invalid connections to be created. The migration is otherwise thorough and includes proper backward compatibility, but these issues could lead to poor user experience and runtime errors.
Pay close attention to src/fides/api/schemas/connection_configuration/connection_secrets_okta.py (needs validation) and src/fides/api/service/connectors/okta_connector.py (misleading error message)

Important Files Changed

File Analysis

Filename	Score	Overview
src/fides/api/service/connectors/okta_connector.py	4/5	Migrated from API token to OAuth2 access token with deprecation warning; added validation logic
src/fides/api/schemas/connection_configuration/connection_secrets_okta.py	3/5	Deprecated api_token field, added access_token field; removed api_token from required components but validation may be insufficient
src/fides/connectors/models.py	4/5	Added access_token field and model_dump override to map access_token to token for OktaClient compatibility

greptile-apps

_{14 files reviewed, 3 comments}

_{Edit Code Review Agent Settings | Greptile}

src/fides/api/service/connectors/okta_connector.py

src/fides/api/schemas/connection_configuration/connection_secrets_okta.py

greptile-apps · 2025-11-13T18:04:45Z

tests/fixtures/saas/test_data/saas_async_callback_config.yml


  client_config:
-    protocol: http
+    protocol: https


style: protocol change from http to https appears unrelated to OAuth2 migration. Check that this doesn't break existing test expectations for the mock server.

Does this pass tests in CI, of so, keep

codecov · 2025-11-13T18:46:28Z

Codecov Report

❌ Patch coverage is 39.28571% with 17 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.06%. Comparing base (ce27594) to head (8250806).
⚠️ Report is 19 commits behind head on main.

Files with missing lines	Patch %	Lines
src/fides/api/service/connectors/okta_connector.py	14.28%	12 Missing ⚠️
src/fides/connectors/models.py	50.00%	5 Missing ⚠️

❌ Your project check has failed because the head coverage (82.06%) is below the target coverage (85.00%). You can increase the head coverage or adjust the target coverage.

❗ There is a different number of reports uploaded between BASE (ce27594) and HEAD (8250806). Click for more details.

HEAD has 19 uploads less than BASE

Flag BASE (ce27594) HEAD (8250806)

32 13

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6960      +/-   ##
==========================================
- Coverage   87.32%   82.06%   -5.26%     
==========================================
  Files         525      525              
  Lines       34418    34477      +59     
  Branches     3960     3970      +10     
==========================================
- Hits        30055    28295    -1760     
- Misses       3500     5361    +1861     
+ Partials      863      821      -42

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

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

stephenkiers · 2025-11-14T19:23:48Z

src/fides/api/schemas/connection_configuration/connection_secrets_okta.py

+    api_token: Optional[str] = Field(
+        default=None,
+        title="API Token (Deprecated)",
+        description="Legacy API token (ignored). Configure OAuth2 Client Credentials instead.",
+        json_schema_extra={"sensitive": True, "deprecated": True},
+    )


This will be removed in fast follow, correct?

The idea behind keeping this field is to ensure that existing integrations relying on it elsewhere don't inadvertently affect other parts of the functionality. For now, I've only left it referenced; I'd probably go ahead and remove this field only once I'm confident that the Okta access tokens are properly integrated and nothing breaks as a result.

stephenkiers · 2025-11-14T19:27:03Z

src/fides/api/service/connectors/okta_connector.py

+        if self.configuration.secrets.get("api_token"):
+            logger.warning(
+                "Deprecated Okta API token detected in secrets. This value is ignored; "
+                "configure OAuth2 Client Credentials instead."
+            )


Where is this secret set? Does this actually exist? ... this is just for bridge support right now, correct? Will be removed for GA?

Also, all other api_token references need to be cleaned up in follow up PR

https://github.com/ethyca/fides/search?q=api_token
https://github.com/ethyca/fides/search?q=OKTA_API_TOKEN

src/fides/connectors/models.py

stephenkiers · 2025-11-14T19:43:29Z

tests/integration/workflows/data/example_async_callback_config.yml

this is just for okta? just want to confirm

perhaps rename or confirm

stephenkiers · 2025-11-14T19:44:11Z

tests/integration/workflows/data/example_async_polling_config.yml

same here, is this ONLY idp/okta?

stephenkiers

Left comments. Also want you to confirm the changes to the tests are ONLY for okta oauth, they seem generic and overreaching

galvana · 2025-11-15T04:01:52Z

Just a drive by comment since Github auto-tagged me. I agree with @stephenkiers on this:

Also want you to confirm the changes to the tests are ONLY for okta oauth, they seem generic and overreaching

Too many SaaS-related files got caught up in this change. We should roll these back so we don't give the impression later that SaaS functionality changed in this PR.

DrJfrost added 3 commits November 12, 2025 02:28

remove api key okta and migrate to access token

76586d3

Follow-up on Okta unit test implementation, OAuth2 migration, and key…

437c68d

…word handling.

add testing to okta integration with oauth2

46b9482

DrJfrost requested review from a team as code owners November 13, 2025 17:53

DrJfrost requested review from galvana and removed request for a team November 13, 2025 17:53

greptile-apps bot reviewed Nov 13, 2025

View reviewed changes

Correction of import naming and suggested fixes.

cae10b3

DrJfrost requested a review from stephenkiers November 13, 2025 20:31

stephenkiers reviewed Nov 14, 2025

View reviewed changes

src/fides/connectors/models.py Outdated Show resolved Hide resolved

stephenkiers reviewed Nov 14, 2025

View reviewed changes

tests/integration/workflows/data/example_async_polling_config.yml

Copy link

Contributor

stephenkiers Nov 14, 2025

Choose a reason for hiding this comment

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

same here, is this ONLY idp/okta?

stephenkiers requested changes Nov 14, 2025

View reviewed changes

Refactoring and migrating remaining OAuth to OAuth2 Okta components.

8c5c6bd

DrJfrost requested a review from a team as a code owner November 14, 2025 22:39

DrJfrost requested review from jpople and removed request for a team November 14, 2025 22:39

vercel bot deployed to Preview – fides-plus-nightly November 14, 2025 22:45 View deployment

Re-adding Okta API token to admin-ui.

8250806

vercel bot deployed to Preview – fides-plus-nightly November 14, 2025 23:02 View deployment

galvana removed their request for review November 15, 2025 03:57

stephenkiers marked this pull request as draft November 25, 2025 17:05

remove api token and addition access key #6960

Are you sure you want to change the base?

remove api token and addition access key #6960

Uh oh!

Conversation

DrJfrost commented Nov 13, 2025

Description Of Changes

Code Changes

Steps to Confirm

Pre-Merge Checklist

Uh oh!

vercel bot commented Nov 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

greptile-apps bot commented Nov 13, 2025

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 Nov 13, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Nov 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stephenkiers left a comment

Choose a reason for hiding this comment

Uh oh!

galvana commented Nov 15, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

vercel bot commented Nov 13, 2025 •

edited

Loading

codecov bot commented Nov 13, 2025 •

edited

Loading