Skip to content

fix: Update okta_classic.py to handle scenario when Okta MFA provider with factorType "push" may return no profiles - #497

Merged
epierce merged 3 commits into
Nike-Inc:masterfrom
its-mirus-lu:feat/okta-classic
Apr 12, 2026
Merged

fix: Update okta_classic.py to handle scenario when Okta MFA provider with factorType "push" may return no profiles#497
epierce merged 3 commits into
Nike-Inc:masterfrom
its-mirus-lu:feat/okta-classic

Conversation

@its-mirus-lu

@its-mirus-lu its-mirus-lu commented Apr 10, 2026

Copy link
Copy Markdown

Addressing possible case when Okta Verify App doesn't return a 'profile' attribute in the factor definition

Description

This PR addresses issue #496 by adding additional logic to test for the case when the Okta MFA provider of factorType 'push' is missing 'profile' attribute in factor.

It was discovered that the contents of the Okta 'push' factorType can be missing the 'profile' attribute. This causes the _build_factor_name method to error out with KeyError: 'profile'.

To fix this, this PR adds additional logic to test for the presence of the 'profile' attribute, in the factor array, and output accordingly.

Related Issue

This PR fixes #496

Motivation and Context

This fixes a break that affects users who choose to rely on force_classic to receive push notifications.

How Has This Been Tested?

This was tested by running gimme-aws-creds locally, which causes the application to run to completion. Running pytest also indicates that the changes do not introduce regression.

Screenshots (if appropriate):

Below is a screenshot of the dictionary of the Okta provider with a factorType of 'push' which lacks a "profile" attribute in its dict.

okta-redacted

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

…profile' attribute in the factor definition
@gblues

gblues commented Apr 10, 2026

Copy link
Copy Markdown

@its-mirus-lu you should add a test to test_okta_classic_client.py

@its-mirus-lu

Copy link
Copy Markdown
Author

@gblues thanks, just added one!

@gblues

gblues commented Apr 10, 2026

Copy link
Copy Markdown

@epierce please take a look at this PR

@call-me-nick call-me-nick left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

👍

@epierce epierce left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good - thanks!

@epierce

epierce commented Apr 12, 2026

Copy link
Copy Markdown
Member

Looks good - thanks for the submission. I'll be releasing v2.9.0 in a week or two

@epierce
epierce merged commit c3eab12 into Nike-Inc:master Apr 12, 2026
3 of 7 checks passed
@its-mirus-lu
its-mirus-lu deleted the feat/okta-classic branch April 13, 2026 16:15
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.

[Fix] KeyError: "profile" - _build_fact_name method in okta_classic.py needs to handle cases when Okta returns profile dict without 'profile' key

6 participants