feat: Add support for step up authentication #140

ruhulio · 2023-10-27T12:12:13Z

Description

When "Extra Verification" is required in authenticate_to_roles, make an additional request to /api/v1/authn using the current state token.

Use /login/sessionCookieRedirect in authenticate_to_roles to ensure the HTTP_client cookies have the values needed for step up authentication and also the device token.

Add optional support for device token across session so that step up authentication does not require multiple MFA requests.

Add device token options to config.py.

Add getting and setting the device token to http_client.py.

Add storing the device token to the ini file to user.py.

Add the device token options to docs/README.md.

Related Issue

N/A

Motivation and Context

I work for Realtor.com which is peer organization to DowJones.com and we are wanting to switch a tech group to using tokendito for generating AWS credentials. To fully support this group, some of the tiles require step up authentication which tokendito does not currently support. So, I used my insomnia time to make these changes.

How Has This Been Tested?

Testing included:

Running in environment with unknown IP address
Running in environment with known corporate IP address
Running in both of the above environments with corporate VPN enabled, and with third party VPN enabled
Running in all of the above with and without device token enabled

Example run (with user and company specific info redacted):

❯ tokendito --use-device-token --profile test_profile --aws-profile test_profile --config-file ./test-tokendito.ini
Organization username. E.g. [email protected]: xxxx.xxxx
Password:
2023-10-27 06:17:50,536 WARNING |tokendito.tool cli():46| Device token unavailable for config profile test-profile. May see multiple MFA requests this time.

Select your preferred MFA method and press Enter:
[0]  OKTA    push                xxxx Id: aaaa
[1]  GOOGLE  token:software:totp xxxx Id: bbbb
[2]  FIDO    webauthn            xxxx Id: cccc
[3]  OKTA    token:software:totp xxxx Id: dddd
-> 0
Waiting for an approval from the device...
2023-10-27 06:18:04,403 INFO |tokendito.okta local_auth():341| User has been successfully authenticated to https://xxxx.
2023-10-27 06:18:04,839 INFO |tokendito.aws authenticate_to_roles():67| Discovering roles in 1 tile.
2023-10-27 06:18:06,075 INFO |tokendito.aws authenticate_to_roles():85| Step-Up authentication required for https://xxxx.

Select your preferred MFA method and press Enter:
[0]  OKTA    push                xxxx Id: aaaa
[1]  GOOGLE  token:software:totp xxxx Id: bbbb
[2]  FIDO    webauthn            xxxx Id: cccc
[3]  OKTA    token:software:totp xxxx Id: dddd
-> 0
Waiting for an approval from the device...
2023-10-27 06:18:19,870 INFO |tokendito.aws authenticate_to_roles():67| Discovering roles in 1 tile.
2023-10-27 06:18:24,406 INFO |tokendito.tool cli():82| Saving device token to config profile test-profile
2023-10-27 06:18:24,407 INFO |tokendito.user update_device_token():939| Updated /xxxx/test-tokendito.ini with profile test-profile

Generated profile 'test-profile' in /Users/xxxx/.aws/credentials.

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

Credentials are valid until 2023-10-27 19:18:24+00:00 (2023-10-27 14:18:24 CDT).

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

When "Extra Verification" is required in authenticate_to_roles, make an additional request to '/api/v1/authn' using the current state token. Use '/login/sessionCookieRedirect' in authenticate_to_roles to ensure the the 'HTTP_client' cookies have the values needed for step up authentication and also the device token. Add optional support for device token across session so that step up authentication does not require multiple MFA requests. Add device token options to 'config.py'. Add getting and setting the device token to 'http_client.py'. Add storing the device token to the ini file to 'user.py'. Add the device token options to 'docs/README.md'.

tokendito/__init__.py

ruhulio · 2023-10-27T12:15:49Z

tokendito/user.py

    cookies = requests.cookies.RequestsCookieJar()
-    cookies.set("sid", sess_id, domain=urlparse(url).netloc, path="/")
+    cookies.set("sid", sess_id, domain=domain, path="/")
+    cookies.set("sessionToken", session_token, domain=domain, path="/")


ℹ️ This is necessary as the session token is needed for the later call to /login/sessionCookieRedirect.

ruhulio · 2023-10-27T12:17:30Z

tokendito/okta.py

@@ -429,7 +458,7 @@ def extract_state_token(html):
    state_token = None
    pattern = re.compile(r"var stateToken = '(?P<stateToken>.*)';", re.MULTILINE)

-    script = soup.find("script", text=pattern)
+    script = soup.find("script", string=pattern)


⚠️ This change removes warnings that were appearing when running tests due to a deprecation.

/Volumes/xxxxx/tokendito/tokendito/okta.py:461: DeprecationWarning: The 'text' argument to find()-type methods is deprecated. Use 'string' instead. script = soup.find("script", text=pattern)

Before:

191 passed, 18 deselected, 5 warnings in 0.85s

After:

191 passed, 18 deselected in 0.66s

ruhulio · 2023-10-27T12:34:52Z

tokendito/okta.py

+    :return: True if step up authentication was successful; False otherwise
+    """
+    auth_properties = get_auth_properties(userid=config.okta["username"], url=config.okta["org"])
+    if "type" not in auth_properties or not is_local_auth(auth_properties):


ℹ️ I only added support for local_auth since I do not have access to a saml2 environment to test appropriately.

pcmxgti · 2023-10-27T18:39:03Z

hi @ruhulio this looks amazing - let us do a bit of internal testing and come back with feedback.

ruhulio · 2023-10-27T18:48:45Z

It looks like a check failed due to an error unrelated to the code changes.

During Lint and Test / Tox Tests (ubuntu-latest, 3.11) (pull_request) in Upload coverage in Parallel:

Error: Bad response: 422 {"message":"Can't add a job to a build that is already closed. Build 6668060962 is closed. See docs.coveralls.io/parallel-builds","error":true}

pcmxgti · 2023-10-27T19:45:28Z

It looks like a check failed due to an error unrelated to the code changes.

That was me, sorry. I was hoping to re-run tests through the auth pipeline. These new errors can be ignored.

pcmxgti · 2023-11-10T01:39:05Z

Hey @ruhulio, I just tested your branch and ran into a small problem. If the MFA response is passed in the command line, then the second round of authorization fails. My guess is that we need to clear config['okta']['mfa_response'] so that the next round of auth prompts the user for its value.

DUO seemed to be a bit problematic as well, but that's secondary at this point. It's likely that once we fix that issue with the logic flow I mentioned above we will get that working as well. Happy to work with you out of band to get it to work. The code looks awesome otherwise.

 ./tokendito/tokendito.py --config-file tokendito.ini --profile test --okta-mfa-response=123456

Select your preferred MFA method and press Enter:
[0]  GOOGLE  token:software:totp [email protected] Id: uftxxxxxxxxxxxxxxxh7
[1]  DUO     web                 DUO                  Id: dsfxxxxxxxxxxxxxxxh8
-> 0
2023-11-09 20:26:45,059 INFO |tokendito.okta local_auth():341| User has been successfully authenticated to https://myorg.com.
2023-11-09 20:26:45,501 INFO |tokendito.aws authenticate_to_roles():67| Discovering roles in 1 tile.
2023-11-09 20:26:46,081 INFO |tokendito.aws authenticate_to_roles():82| Step-Up authentication required for https://myorg.com/home/amazon_aws/0xxxxxxxxxxxxxxxxxh7/137.

Select your preferred MFA method and press Enter:
[0]  GOOGLE  token:software:totp [email protected] Id: uftxxxxxxxxxxxxxxxh7
[1]  DUO     web                 DUO                                            Id: dsfxxxxxxxxxxxxxxxh8
-> 0
2023-11-09 20:26:48,621 ERROR |tokendito.http_client post():66| Error during POST request to https://myorg.com/api/v1/authn/factors/uftxxxxxxxxxxxxxxxh7/verify. Error: 403 Client Error: Forbidden for url: https://myorg.com/api/v1/authn/factors/uftxxxxxxxxxxxxxxxh7/verify

ruhulio · 2023-11-10T02:52:50Z

Hey @ruhulio, I just tested your branch and ran into a small problem. If the MFA response is passed in the command line, then the second round of authorization fails. My guess is that we need to clear config['okta']['mfa_response'] so that the next round of auth prompts the user for its value.

DUO seemed to be a bit problematic as well, but that's secondary at this point. It's likely that once we fix that issue with the logic flow I mentioned above we will get that working as well. Happy to work with you out of band to get it to work. The code looks awesome otherwise.

Thanks @pcmxgti for the details on the failure you're seeing. I'll take a look at addressing the issue and get this PR updated by tomorrow (hopefully).

pcmxgti · 2023-11-10T03:04:10Z

Hey @ruhulio, I just tested your branch and ran into a small problem. If the MFA response is passed in the command line, then the second round of authorization fails. My guess is that we need to clear config['okta']['mfa_response'] so that the next round of auth prompts the user for its value.
DUO seemed to be a bit problematic as well, but that's secondary at this point. It's likely that once we fix that issue with the logic flow I mentioned above we will get that working as well. Happy to work with you out of band to get it to work. The code looks awesome otherwise.

Thanks @pcmxgti for the details on the failure you're seeing. I'll take a look at addressing the issue and get this PR updated by tomorrow (hopefully).

Anytime. Since you are going to make changes, would you mind removing the version change in init.py? We are prepping a release and can fit this together with it.

ruhulio · 2023-11-10T16:02:15Z

tokendito/okta.py

@@ -605,6 +634,9 @@ def totp_approval(config, selected_mfa_option, headers, mfa_challenge_url, paylo
        user.add_sensitive_value_to_be_masked(mfa_verify["sessionToken"])
    logger.debug(f"mfa_verify [{json.dumps(mfa_verify)}]")

+    # Clear out any MFA response since it is no longer valid
+    config.okta["mfa_response"] = None


ℹ️ This should address the re-use of MFA response during step up authentication. By putting it here, I'm hoping this will catch any future™ downstream re-use issues as well.

ruhulio · 2023-11-10T16:04:12Z

@pcmxgti The version change has been reverted and the MFA response re-use should be addressed as well. I don't have access to a Duo based account so I am unable to validate that path. If you can provide access to a test account/credentials, I am glad to follow up on that.

pcmxgti · 2023-11-10T16:57:54Z

@ruhulio I tested locally with Step-Up (TOTP, Duo, and Push) and everything looks great. We'll add Step-up tests soon so they are part of the end-to-end automated tests.

pcmxgti

@ruhulio @sevignyj this LGTM, it's ready to be squashed and merged.

ruhulio commented Oct 27, 2023

View reviewed changes

tokendito/__init__.py Outdated Show resolved Hide resolved

ruhulio commented Oct 27, 2023

View reviewed changes

ruhulio marked this pull request as ready for review October 27, 2023 12:17

ruhulio commented Oct 27, 2023

View reviewed changes

fix: Address linting issues

b02ce81

ruhulio added 2 commits November 10, 2023 09:26

chore: Revert version change

53a4027

fix: Clear out MFA response after using it

0c74252

ruhulio commented Nov 10, 2023

View reviewed changes

pcmxgti assigned pcmxgti and sevignyj Nov 10, 2023

pcmxgti approved these changes Nov 10, 2023

View reviewed changes

sevignyj merged commit fa7b00b into dowjones:main Nov 14, 2023
23 checks passed

pcmxgti mentioned this pull request Nov 15, 2023

Feature Request: Friendly device name #144

Closed

ruhulio deleted the feature/step-up-authentication branch November 16, 2023 17:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add support for step up authentication #140

feat: Add support for step up authentication #140

ruhulio commented Oct 27, 2023

ruhulio Oct 27, 2023 •

edited

Loading

ruhulio Oct 27, 2023

ruhulio Oct 27, 2023 •

edited

Loading

pcmxgti commented Oct 27, 2023

ruhulio commented Oct 27, 2023

pcmxgti commented Oct 27, 2023

pcmxgti commented Nov 10, 2023

ruhulio commented Nov 10, 2023

pcmxgti commented Nov 10, 2023

ruhulio Nov 10, 2023 •

edited

Loading

ruhulio commented Nov 10, 2023

pcmxgti commented Nov 10, 2023

pcmxgti left a comment

feat: Add support for step up authentication #140

feat: Add support for step up authentication #140

Conversation

ruhulio commented Oct 27, 2023

Description

Related Issue

Motivation and Context

How Has This Been Tested?

Types of changes

Checklist:

ruhulio Oct 27, 2023 • edited Loading

Choose a reason for hiding this comment

ruhulio Oct 27, 2023

Choose a reason for hiding this comment

ruhulio Oct 27, 2023 • edited Loading

Choose a reason for hiding this comment

pcmxgti commented Oct 27, 2023

ruhulio commented Oct 27, 2023

pcmxgti commented Oct 27, 2023

pcmxgti commented Nov 10, 2023

ruhulio commented Nov 10, 2023

pcmxgti commented Nov 10, 2023

ruhulio Nov 10, 2023 • edited Loading

Choose a reason for hiding this comment

ruhulio commented Nov 10, 2023

pcmxgti commented Nov 10, 2023

pcmxgti left a comment

Choose a reason for hiding this comment

ruhulio Oct 27, 2023 •

edited

Loading

ruhulio Oct 27, 2023 •

edited

Loading

ruhulio Nov 10, 2023 •

edited

Loading