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

catching other hid device errors and prompt for FIDO auth selections #668

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

hinling-sonder
Copy link

Try to fix the following problems:

Screen Shot 2021-05-19 at 1 33 24 AM

  • yubikey usb device throwing some strange errors while waiting for user response. From our experience, they are harmless. Just catching them and wait for the timeouts to kick in.

hinling-sonder and others added 7 commits May 16, 2021 22:24
sync from base saml2aws
print out devices info

deference the device pointer and print

print out MFA also

add more logging and catch more errors?

fix logging

still fixing logging

more logging

not an address

add more logging

add more logging around mfaoptions

always prompt for which MFA

getting all the matching mfas back

add more logging
@shedimon
Copy link

shedimon commented Feb 9, 2022

Hi @wolfeidau recently we've faced an issue with the problem this PR solves. Okta returns the full list of FIDO MFAs and without the ability to choose the factor (let's say Yubikey) saml2aws tries to use the first option in the FIDO MFAs and if it's not a said Yubikey it can use fails by getting the u2fhost.BadKeyHandleError which stops the process before trying other options.

Using the code from this PR resolves this by adding an ability to specifically choose which FIDO device to use for each authorization.

Is there anything we can do/help with to move forward with this approach?

@@ -85,6 +85,12 @@ type mfaChallengeContext struct {
challengeResponseBody string
}

// mfaOption store the mfa position in response and mfa description
type mfaOption struct {

Choose a reason for hiding this comment

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

@hinling-sonder WDYT about extracting the profile.authenticatorName from the json and add it to this struct to have a human-readable MFA option in the prompt? I found it really helpful to have this instead of the full profile field.

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