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

Add support for Okta question MFA #151

Merged
merged 4 commits into from
Dec 18, 2023

Conversation

jonsolakis
Copy link
Contributor

@jonsolakis jonsolakis commented Dec 9, 2023

Description

Adding support for Okta question MFA.

Related Issue

Motivation and Context

I wasn't sure if this would be a feature or a fix since this did work on older releases (I think the breaking change came in 2.1).

my 2FA in Okta has recently been switched to security questions and as a result the latest version doesn't work anymore (The MFA isn't supported).

I wasn't sure how you would want this implemented so please let me know if you want me to change anything. One issue with this solution is the answer does appear in plain text on the console.

How Has This Been Tested?

Prod Okta Instance
example run:

☁  ~  tokendito --profile ro
Password:

Select your preferred MFA method and press Enter:
[0]  OKTA  question xxxxxxxxxxxxxx Id: xxxxxxxxxxxxxxxx
-> 0
Enter your verification code: xxxxxxxxxxxxxx
2023-12-08 17:42:47,796 INFO |tokendito.okta local_authenticate():845| User has been successfully authenticated to https://xxxxxxxxxxxxxx.okta.com.
2023-12-08 17:42:48,211 INFO |tokendito.aws authenticate_to_roles():63| Discovering roles in 1 tile.
Enter a profile name or leave blank to use 'xxxxxxxxxxxxxxxxx':

Generated profile 'xxxxxxxxxxxxxxxxx' in /Users/xxxxxxx/.aws/credentials.

Use profile to authenticate to AWS:
	aws --profile 'xxxxxxxxxxxx' sts get-caller-identity
OR
	export AWS_PROFILE='xxxxxxxxxxxxxx'

Credentials are valid until 2023-12-08 23:42:52+00:00 (2023-12-08 18:42:52 EST).

Types of changes

  • New feature (non-breaking change which adds functionality)

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

@ruhulio
Copy link
Contributor

ruhulio commented Dec 14, 2023

💭 With regards to the answers showing up in plain text, you could add the following to user.py (it follows the pattern of get_input()):

def get_secure_input(prompt="-> "):
    """Collect secure user input

    :param prompt: optional string with prompt.
    :return user_input: raw from user.
    """
    tty_assertion()

    secure_input = getpass(prompt)
    logger.debug(f"Secure input: {secure_input}")

    return secure_input

Then modify totp_approval() to add an optional parameter that would control whether to use get_input() or the new get_secure_input() here.

There is the existing get_password() function that is similar but it does not currently support prompting and it might be better to keep these concerns separate.

Note: I'm not a maintainer; just another consumer of this app.

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.

LGTM

@sevignyj sevignyj merged commit dd74bde into dowjones:main Dec 18, 2023
7 of 8 checks passed
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.

4 participants