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

OIE implementation, with authorization code flow . #132

Merged
merged 1 commit into from
Nov 15, 2023
Merged

Conversation

sevignyj
Copy link
Contributor

@sevignyj sevignyj commented Aug 16, 2023

Description

Related Issue

Motivation and Context

How Has This Been Tested?

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)
  • Documentation update

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@sevignyj sevignyj force-pushed the feature/oie branch 3 times, most recently from 3ba6eae to 9de3d7c Compare August 23, 2023 02:21
@sevignyj sevignyj changed the title oie implementation draft 1 oie implementation draft Aug 23, 2023
@tadcrazio
Copy link

This is great. Looking forward to this implementation.

@sevignyj sevignyj changed the title oie implementation draft oie implementation (WIP) Aug 27, 2023
@sevignyj sevignyj force-pushed the feature/oie branch 7 times, most recently from e38ffbe to 590ad2d Compare August 30, 2023 19:20
@sevignyj sevignyj force-pushed the feature/oie branch 9 times, most recently from 2a97bce to ffcd43c Compare September 14, 2023 15:00
@sevignyj sevignyj force-pushed the feature/oie branch 4 times, most recently from bfdb4a3 to 332b6df Compare September 19, 2023 17:38
@sevignyj sevignyj force-pushed the feature/oie branch 2 times, most recently from bba74dc to f6e9c4f Compare September 25, 2023 00:58
@sevignyj sevignyj marked this pull request as ready for review September 25, 2023 18:52
@sevignyj sevignyj force-pushed the feature/oie branch 3 times, most recently from 2329cf8 to 4df1952 Compare October 18, 2023 15:43
@sevignyj sevignyj force-pushed the feature/oie branch 6 times, most recently from dc3b22c to b6c8dd7 Compare October 26, 2023 00:22
tokendito/okta.py Fixed Show fixed Hide fixed
Copy link
Contributor

@pcmxgti pcmxgti left a comment

Choose a reason for hiding this comment

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

The code flow looks good. I left some inline comments for you to review what makes sense to keep, and what to remove or clean up.

Some important notes to keep in mind:

  • put back the version dependency for certifi
  • clean up the duplicate arguments
  • rebase and squash (if not already done)
  • update README to reference Okta OIE support forcing Classic mode

requirements.txt Outdated Show resolved Hide resolved
tokendito/__init__.py Outdated Show resolved Hide resolved
tokendito/http_client.py Outdated Show resolved Hide resolved
tokendito/http_client.py Outdated Show resolved Hide resolved
tokendito/user.py Show resolved Hide resolved
tokendito/okta.py Outdated Show resolved Hide resolved
tokendito/okta.py Outdated Show resolved Hide resolved
tokendito/okta.py Outdated Show resolved Hide resolved
tokendito/okta.py Outdated Show resolved Hide resolved
tokendito/okta.py Outdated Show resolved Hide resolved
@sevignyj sevignyj changed the title oie implementation (WIP) OIE implementation, authorization code flow for authorization. Nov 14, 2023
@sevignyj sevignyj force-pushed the feature/oie branch 3 times, most recently from d0be3c3 to d3e8b0d Compare November 14, 2023 20:49
tokendito/okta.py Dismissed
payload = {"username": config.okta["username"], "password": config.okta["password"]}

logger.debug(f"Authenticate user to {config.okta['org']}/api/v1/authn")
logger.debug(f"Sending {headers}, {payload} to {config.okta['org']}/api/vi/authn")

Check failure

Code scanning / CodeQL

Clear-text logging of sensitive information High

This expression logs
sensitive data (password)
as clear text.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

config.okta["password"] is masked.

@sevignyj sevignyj force-pushed the feature/oie branch 2 times, most recently from a9eb3f3 to a3ea3f8 Compare November 15, 2023 01:29
@sevignyj sevignyj force-pushed the feature/oie branch 2 times, most recently from c3b794c to c045e89 Compare November 15, 2023 02:56
@sevignyj sevignyj changed the title OIE implementation, authorization code flow for authorization. OIE implementation, plus authorization code flow . Nov 15, 2023
@sevignyj sevignyj changed the title OIE implementation, plus authorization code flow . OIE implementation, with authorization code flow . Nov 15, 2023
@pcmxgti pcmxgti merged commit eba165e into main Nov 15, 2023
23 checks passed
@pcmxgti pcmxgti deleted the feature/oie branch November 15, 2023 11:56
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.

3 participants