-
Notifications
You must be signed in to change notification settings - Fork 172
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
Replace AuthenticationExtensionsAuthenticatorInputs with CDDL #1440
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM thx @emlun, one modest suggestion...
on 2020-06-17 call: @selfissued will review this week. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm OK with the change to CDDL but I think it would be more helpful to people comparing level 1 and level 2 to leave the type names the same. Therefore please change extensionInputs
back to AuthenticationExtensionsAuthenticatorInputs
and change extensionOutputs
back to AuthenticationExtensionsClientOutputs
.
After that change, I'll approve the PR.
Thanks @selfissued - I've renamed the I also took the opportunity to add some more cross-references and explicitly spell out to use the extension ID as the entry key for the group plugs. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks even better to me :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good now - thanks
This replaces the unused WebIDL type AuthenticationExtensionsAuthenticatorInputs with formally defined CDDL extensions point for authenticator extension in/outputs. It doesn't precisely encode that entry keys must be extension identifiers, but the client counterparts also don't, and the prose definition does state this requirement and illustrate it with an example.
Fixes #1435.
Preview | Diff