Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

This corrects an issue with the validation of the org url and app url… #375

Merged
merged 1 commit into from
Dec 21, 2022

Conversation

magni2000
Copy link
Contributor

… when using action-configure

This is correcting the logic that is used in the config.py file to validate if the url contains one of the valid okta domains.

Description

The updated code will not correctly validated that the url provided by the user for the okta org or the app url will contain one of the elements contained in the allowlist. The validation logic that is in the code will always return false.

Related Issue

bug fix discussed in #374

Motivation and Context

We currently utilize the --action-configure feature for our employees to properly configure gimme-aws-creds for the first time utilization. We can provide a work around, but would prefer to utilize the functionality provided by the project already.

How Has This Been Tested?

I was able to test with mutliple urls, both those that contain the one of the domains in the allow list and urls that do not contain those domains. The logic is now returning a proper true or false using the desired validation criteria.

Screenshots (if appropriate):

valdationscreenhot

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.

@magni2000
Copy link
Contributor Author

@epierce can you review this pull request? It is a fix to bug that was introduced with a recent merge in November.

@epierce
Copy link
Member

epierce commented Dec 21, 2022

The changes look good and are working for me. Thanks!

@epierce epierce merged commit 5034e62 into Nike-Inc:master Dec 21, 2022
@epierce epierce mentioned this pull request Jan 5, 2023
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.

2 participants