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

Clean up extensions section #130

Merged
merged 3 commits into from
Jun 22, 2016
Merged

Clean up extensions section #130

merged 3 commits into from
Jun 22, 2016

Conversation

vijaybh
Copy link
Contributor

@vijaybh vijaybh commented Jun 15, 2016

- Resolve inconsistent wording in different parts of the section: fix
places that incorrectly suggested extensions cannot be used with
makeCredential, and resolve conflict between wording that said client
arguments are required vs. not.
- Treat implementations that choose to pass through extensions as a
first-class citizen by requiring that extensions be defined to allow for
this. Fixes #97.
- Fix all the predefined extensions to conform to the updated
guidelines.
- Minor issues: AAGUID should be binary not string (see #61, fixes #51)
and encode true as Boolean in CBOR instead of forcing it to integer
value 1.
processing.
An extension defines up to two request arguments. The <dfn>client argument</dfn> is passed from the <a>[RP]</a> to the client
in the {{getAssertion()}} or {{makeCredential()}} call, while the <dfn>authenticator argument</dfn> is passed from the client
to the authenticator during the processing of these calls.
Copy link
Contributor

@equalsJeffH equalsJeffH Jun 15, 2016

Choose a reason for hiding this comment

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

this parag has always been ambiguous (to me, anyway) as to whether it is possible for an RP to pass an authenticator argument that the client then simply passes on to the authnr. If that was an original intention of the definition of extensions, AND we intend to continue that, then lines 1453-1455 ought not be deleted. Otherwise, carry on :) tho clarification is perhaps appropriate.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Lines 1453-1455 are not deleted, they are just moved down (see 1462-1464 in the green, below). Lines 1474+ explicitly say that what you ask is true, except for transcoding (the RP passes a client argument in JSON, it gets sent as an authenticator argument in the corresponding CBOR). Perhaps we can do a round of editorial improvements later?

Copy link
Contributor

Choose a reason for hiding this comment

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

thanks for clarification, sounds fine.

- Rename "signature extension" to "authentication extension" (will be
added to glossary in #131)
- Pre-defined extensions recommended (but not required) for
implementation by clients
- Require extensions to update either ClientData or authenticatorData,
update authenticator selection extension to obey this rule
- Clarify that authenticator arguments are sent to the authenticator as
a map, not as bare values
@leshi
Copy link
Contributor

leshi commented Jun 22, 2016

LGTM

@leshi leshi merged commit d2cff07 into master Jun 22, 2016
@vijaybh vijaybh deleted the vgb-extension-opt branch June 22, 2016 23:26
@equalsJeffH equalsJeffH restored the vgb-extension-opt branch September 12, 2016 23:50
@vijaybh vijaybh deleted the vgb-extension-opt branch September 13, 2016 06:06
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.

Clarify when an extension may be ignored by user agent AAGUID extension underspecified
3 participants