Skip to content

Conversation

stephenmcgruer
Copy link
Collaborator

@stephenmcgruer stephenmcgruer commented Sep 30, 2021

These were previously incorrect specified as auth-time requirements, but they
are actually registration time arguments. This was overlooked as the build
action didn't fail on warnings by default. This commit also changes the action
to fail in such cases.

Fixes #129


Preview | Diff

@stephenmcgruer
Copy link
Collaborator Author

(Still a draft as the actual links aren't fixed yet; testing that the build change actually fails the build)

@stephenmcgruer stephenmcgruer force-pushed the smcgruer/fix-spec-errors branch from fde9362 to 2d6fa1a Compare September 30, 2021 17:20
@stephenmcgruer stephenmcgruer marked this pull request as ready for review September 30, 2021 17:20
Copy link
Collaborator

@ianbjacobs ianbjacobs left a comment

Choose a reason for hiding this comment

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

Hi @stephenmcgruer,
It seems you removed some text we had previously added about residentKey and authenticatorAttachment. Was that intentional?

@stephenmcgruer
Copy link
Collaborator Author

@rsolomakhin @ianbjacobs - residentKey and authenticatorAttachment are not auth-time arguments for WebAuthn. I suspect what you were trying to do was to record that Chrome's current implementation requires residentKey="required" and authenticatorAttachment="platform" at registration time - is that correct? Assuming so, that actually needs to go in the registration steps - I can do that in this CL too.

@ianbjacobs
Copy link
Collaborator

@stephenmcgruer,
+1 to doing any clarifications you deem helpful. Moving the info to another part of the spec seems fine. Thank you.

These were previously incorrect specified as auth-time requirements, but they
are actually registration time arguments. This was overlooked as the build
action didn't fail on warnings by default. This commit also changes the action
to fail in such cases.
@stephenmcgruer stephenmcgruer force-pushed the smcgruer/fix-spec-errors branch from 2d6fa1a to 9c9d0b7 Compare October 1, 2021 14:52
@stephenmcgruer stephenmcgruer changed the title [Editorial] Fixup invalid links in the spec [Spec] Correctly specify authenticatorSelection requirements Oct 1, 2021
@stephenmcgruer
Copy link
Collaborator Author

@rsolomakhin @ianbjacobs - properly specified the limitations, please take a look. Rouslan, please double-check I've listed these correctly.

Copy link
Collaborator

@rsolomakhin rsolomakhin left a comment

Choose a reason for hiding this comment

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

This is correct. Thank you!

@stephenmcgruer stephenmcgruer merged commit ba809db into main Oct 1, 2021
@stephenmcgruer stephenmcgruer deleted the smcgruer/fix-spec-errors branch October 1, 2021 17:36
github-actions bot added a commit that referenced this pull request Oct 1, 2021
SHA: ba809db
Reason: push, by @stephenmcgruer

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
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.

Require AuthenticatorSelection's residentKey for registration
3 participants