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

Note what kind of library is needed for displaying a QR code. #329

Closed
wants to merge 1 commit into from

Conversation

mooreds
Copy link
Contributor

@mooreds mooreds commented Nov 30, 2020

See https://fusionauth.io/community/forum/topic/599/unable-to-get-successful-enabletwofactor-using-dotnet-client for more info about a user's experience with different QR code generators.

@robotdan
Copy link
Member

Not sure I understand how the QR code generator has anything to do with this? I doubt the QR code type has anything to do with this, the most likely answer is some sort of user error, or buggy QR code generator.

@mooreds
Copy link
Contributor Author

mooreds commented Nov 30, 2020

@robotdan I was basing this change on this comment:

I've also determined that FusionAuth bases their two factor authentication on TOTP as defined in RFC 6238, so I can only assume that the package I chose is using a different algorithm. I've looked at a the documentation for the NuGet package and it does not mention what algorithm it is using.

I'm going to switch to another QR code generator that does adhere to RFC 6238 and see if that fixes things.

Not sure how much it is worth digging into the library using a different algorithm given the impending overhaul, but thought this would be a quick win. I'm ok with leaving this unmerged.

@robotdan
Copy link
Member

He is correct, we do use the algorithm specified in that RFC. However, the QR code is simply a way to display the secret, so it is possible the library he was using is not compliant, but I don't believe this has anything to do with the QR code generator. If the QR code is incorrect, then the value generated by the library is invalid.

@mooreds
Copy link
Contributor Author

mooreds commented Nov 30, 2020

Hmm. Maybe I misread the forum post. It sounds to me like he generated the secret using FusionAuth, and then built the QR image using some library:

The first thing I do is GenerateTwoFactorSecret, which I use to create the QR code (using the GoogleAuthenticator NuGet Package by Brandon Potter). The QR code is brought into the authenticator app and I get a code. I validate the code with the QR code package's check method and get a successful validation of the code.

Later:

I've determined that the NuGet package mentioned above is giving me a different QR code than what FusionAuth would give me if I manually enabled two factor authentication using the same secret.

When he changed the library which he used to build the QR image, he saw the correct behavior (and was able to login).

So all this PR is doing is saying, hey, use whatever library you want to build the QR code, but it must be RFC 6238 compliant.

@robotdan
Copy link
Member

Yeah, I think I understand, but I think it is more likely that there is some user error here as I don't believe there is such a thing as an QR code generator that is RFC 6238 compliant.

It is possible that different QR code generators behave differently, but sounds like a QR code generate bug to me. A QR code generate simply encodes a value.

If the QR code library doesn't support the correct modes, or has selected an incorrect mode it would be possible that it would not be encoded correctly. https://en.wikipedia.org/wiki/QR_code#Encoding

@mooreds
Copy link
Contributor Author

mooreds commented Dec 8, 2020

OK, closing.

@mooreds mooreds closed this Dec 8, 2020
@mooreds mooreds deleted the add-two-factor-rfc-6238-note branch May 11, 2021 12:50
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.

None yet

2 participants