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

Update finish login example #192

Merged
merged 1 commit into from Dec 1, 2023
Merged

Conversation

mitar
Copy link
Contributor

@mitar mitar commented Dec 1, 2023

I think it is important to note that credential.Authenticator.CloneWarning should be checked (or willfully ignored). And also credential should be stored, otherwise counter checking does not really work?

Is it safe to just overwrite the credential or are there values in registration credential which are missing from the logic credential object?

Is there anything else which should be validated in business logic and is missing from the example?

@mitar mitar requested a review from a team as a code owner December 1, 2023 22:50
Copy link
Member

@james-d-elliott james-d-elliott left a comment

Choose a reason for hiding this comment

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

The only other potential check that may be missing is checking the Backup Eligible flag. This flag can never change per spec semantics. But yes this is a good spot and it was on my list of things to address specifically with a helper function or breaking change.

Regardless the doc adjustment is a welcome one. Let me know if you want me to just commit as is, or if you'd like to handle the BE check too. I'll have to double check the other elements of the examples as they have not been updated in a while.

@mitar
Copy link
Contributor Author

mitar commented Dec 1, 2023

I think BE checks should just be handled internally and error out in my opinion. So I think it is fine to merge as-is.

@james-d-elliott james-d-elliott merged commit f23bfd6 into go-webauthn:master Dec 1, 2023
4 checks passed
@mitar mitar changed the title Update finish loging example Update finish login example Dec 2, 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

2 participants