Okta + Duo: Allow passcodes from mobile app #1222
Conversation
Any Duo mobile app registered with Duo, usually for Duo Push, also has the capability to produce HOTP passcodes. In the case where Duo Push is administratively disabled and Yubikeys (etc) are not provided, HOTP might be the only way for a user to perform Duo MFA. This change adds Passcode to the list of MFA possibilities whenever a phone1 is registered with Duo to support that situation.
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #1222 +/- ##
==========================================
+ Coverage 40.44% 41.92% +1.48%
==========================================
Files 54 54
Lines 8276 6373 -1903
==========================================
- Hits 3347 2672 -675
+ Misses 4491 3265 -1226
+ Partials 438 436 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
any related unit test updates? |
There was no test coverage of the option selection/presentation today so I didn't update the tests, but happy to add coverage (I think... I don't work in Go often and the current tests in |
|
@wlonky yes, there seems to be coverage the modified code https://app.codecov.io/gh/Versent/saml2aws/blob/master/pkg%2Fprovider%2Fokta%2Fokta.go#L974 consider creating a copy of saml2aws/pkg/provider/okta/okta_test.go Line 268 in 14c45ab |
|
Got it, I'll see what I can do! Probably won't get to this until the weekend. |
|
Ah, something's not right. I'll comment when this is good to go, sorry. |
|
Sorted and ready to go! |
pkg/provider/okta/okta_test.go
Outdated
| verifyCounter := 0 | ||
| statusCounter := 0 | ||
| ts := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
|
|
||
| ts := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { |
There was a problem hiding this comment.
this seems to have wrong indent
|
@gliptak Is this ready to merge? |
|
Rats, I need to make one more change -- I just used my local build on my actual work Okta account, and I see duplicate "Passcode" options (because I have both |
OK - just let me know when its ready |
|
@mapkon Should be all set now. I switched around the logic to only add |
Any Duo mobile app registered with Duo, usually for Duo Push, also has the capability to produce HOTP passcodes:
In the case where Duo Push is administratively disabled and Yubikeys (etc) are not provided, HOTP might be the only way for a user to perform Duo MFA. This is the case for us, which is preventing our non-Yubikey users from using saml2aws at all.
This change adds Passcode to the list of MFA possibilities whenever a
phone1is registered with Duo, to support using HOTP.I debated between adding it like this, or adding another conditional to the
option[value="token"]case -- let me know if that would be preferable. I also checked the other Duo-enabled providers, but none of the others use this kind of logic to limit the MFA options presented to the user.