Versent / saml2aws

CLI tool which enables you to login and retrieve AWS temporary credentials using a SAML IDP
https://github.com/Versent/saml2aws
MIT License
2.09k stars 564 forks source link

Okta + Duo MFA broken due to DuoSecurity API change #527

Open himanshu-neema opened 4 years ago

himanshu-neema commented 4 years ago

Issue

Failures observed on release v2.14.0

On successful authentication with Okta & successful approval of Duo push notification, saml2aws fails with following error:

➜  ~ saml2aws login --duo-mfa-option="Duo Push"
Using IDP Account default to access Okta <redacted-IDP-url>
To use saved password just hit enter.
? Username <redacted-email>
? Password **************
Authenticating as <redacted-email> ...
Pushed a login request to your device...
Success. Logging you in...
Response did not contain a valid SAML assertion
Please check your username and password is correct

Failures observed on release v2.26.2

On latest released version, when user approve Duo Push notification saml2aws prompts again for Duo Push, and hence goes into infinite loop

➜  saml2aws git:(d947726) DBUS_SESSION_BUS_ADDRESS=/dev/null $GOPATH/bin/saml2aws login --force
Using IDP Account default to access Okta <redacted-IDP-url>
To use saved password just hit enter.
? Username <redacted-email>i
? Password *************

Authenticating as <redacted-email> ...
? Select a DUO MFA Option Duo Push
Pushed a login request to your device...
Success. Logging you in...
? Select a DUO MFA Option Duo Push
Pushed a login request to your device...
Success. Logging you in...
? Select a DUO MFA Option  [Use arrows to move, type to filter]
> Duo Push
  Passcode

Note about DBUG_SESSION_BUS_ADDRESS=/dev/null: without this env var, saml2aws appears to be hung.

Response from Duo Security support

They rolled out the change yesterday for our account, so this may not be affecting everyone. Following is the screenshot of response:

Screen Shot 2020-07-22 at 1 07 15 PM

Desired outcome for this issue:

  1. Need to fix latest code for the Duo Security API change. ~2. Backport fix to version 2.14.0 - since my team uses this version. Or should we fix the latest and upgrade to latest?~ we can update to latest.
himanshu-neema commented 4 years ago

Note related to DBUG_SESSION_BUS_ADDRESS=/dev/null there's an open issue https://github.com/Versent/saml2aws/issues/441

AaronAtDuo commented 4 years ago

Based on my review of the code, this could affect the Akamai and Shibboleth providers also, since they do basically the same thing as the Okta provider, when it comes to Duo integration.

nickatsegment commented 4 years ago

Was just about to post a notice that we found something similar in aws-okta. I wrote about it at https://github.com/aws-okta/aws-okta/issues/9

himanshu-neema commented 4 years ago

We do have a fix that's been working for us here https://github.com/himanshu-neema/saml2aws/pull/1

@AaronAtDuo If thats an acceptable fix: we can replicate the same for Akamai & Shibboleth

I'm waiting for internal approval to submit the PR upstream.

AaronAtDuo commented 4 years ago

I'm not the right person to ask, since this is not a Duo-created or supported integration flow. Speaking as a Duo engineer, I strongly suggest figuring out how to accomplish this using a supported, stable API such as our https://duo.com/docs/authapi instead.

nickatsegment commented 4 years ago

As (ex-) maintainer of aws-okta, I'd be all for using the official APIs. Haven't had the time to look into it. Considering there's several independent implementations of okta+duo out there and they all use roughly the same flow (including all these undocumented/unofficial /frame APIs), I'm led to believe it's not possible or has some drawback I'm not aware of. That, or they all actually derive from some original code base.

himanshu-neema commented 4 years ago

Using official APIs is good for long term - might require a major refactor. @nickatsegment has good point if unofficial APIs are used intentionally to make it work?

We saw the DUO reverted the API again today - so for short term made the fix backward compatible: https://github.com/Versent/saml2aws/pull/532/commits/6200cf3db26d7eb5c54e37098e43a0d884b9f292