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

Tn/fix authn bug #800

Merged
merged 4 commits into from May 6, 2023
Merged

Tn/fix authn bug #800

merged 4 commits into from May 6, 2023

Conversation

TylerNoblett
Copy link
Collaborator

Within login.go (of the webauthn library), there was a check that was failing:

userHandle := parsedResponse.Response.UserHandle
	if len(userHandle) > 0 {
		a := user.WebAuthnID()
		if !bytes.Equal(userHandle, user.WebAuthnID()) {
			return nil, protocol.ErrBadRequest.WithDetails("userHandle and User ID do not match")
		}
	}

When I looked into it more, userHandle is a uuid generated when a new webauthn user is created that was saved in the session data (but wasn't saved in the database), and the WebAuthnID was the ID in the database (ie 26). I took me a while to figure out exactly what was going, but saving the uuid and getting user.WebAuthnID() to return that solved the issue.

I'm uncertain though of what caused this issue, as the library version we were using didn't change, and when I rolled the commit history back to when webauthn was first added, I still got the same error that John encountered 🤷🏼‍♂️

I confirm that this contribution is made under the terms of the license found in the root directory of this repository's source tree and that I have the authority necessary to make this contribution on behalf of its copyright owner.

jkennedyvz
jkennedyvz previously approved these changes Apr 20, 2023
@jkennedyvz jkennedyvz self-requested a review April 20, 2023 16:46
@jrozner
Copy link
Member

jrozner commented May 6, 2023

Changes here look good. I'm able to register and login. Did discover that AUTH_WEBAUTHN_AUTHENTICATOR_RESIDENT_KEY does not seem to be respected but we can create a new issue for that. We also probably want to rework the UI a bit and make some changes to webauthn in the future, specifically around supporting level 3 however the go library doesn't support it yet.

@jrozner jrozner merged commit 9638f1b into main May 6, 2023
9 checks passed
@jrozner jrozner deleted the tn/fix-authn-bug branch May 6, 2023 23:15
@jrozner jrozner mentioned this pull request May 7, 2023
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

3 participants