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.

Sign up for GitHub

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

Jump to bottom

Processing model for extensions is very underdefined #270

Closed
bzbarsky opened this issue Nov 4, 2016 · 6 comments
Closed

Processing model for extensions is very underdefined #270

bzbarsky opened this issue Nov 4, 2016 · 6 comments
Labels
priority:implementation subtype:CTAP subtype:extensions type:technical
Milestone
WD-05

Comments

@bzbarsky
Copy link

bzbarsky commented Nov 4, 2016

I'm not sure how well this can really be specified, but...

https://w3c.github.io/webauthn/#dom-webauthentication-makecredential step 7 says:

If an error is encountered while processing an extension, skip that extension and do not produce any extension data for it.

It's not clear what that means. Can "an error" include ES exceptions? I see no reason it can't, but then should those be caught or reported?
@bzbarsky
Copy link
Author

bzbarsky commented Nov 4, 2016

This applies to https://w3c.github.io/webauthn/#getAssertion step 4 too.

@equalsJeffH equalsJeffH added this to the WD-04 milestone Nov 7, 2016
@equalsJeffH
Copy link
Contributor

ES is "ecmaScript" I believe. also, #290 #295 are related to this one in that they address "extensions".

@jyasskin jyasskin mentioned this issue Feb 15, 2017
Merged
jyasskin added a commit to jyasskin/webauthn that referenced this issue Feb 16, 2017
@jyasskin
Make makeCredential() more precise.
f16e959 
I've linked a lot more terms, reordered explanations to be clearer, and
specified some missing behavior.

This fixes w3c#273 and improves w3c#270.
@vijaybh vijaybh modified the milestones: WD-04, WD-05 Feb 16, 2017
jyasskin added a commit to jyasskin/webauthn that referenced this issue Feb 22, 2017
@jyasskin
Make makeCredential() more precise.
0eaef1d 
I've linked a lot more terms, reordered explanations to be clearer, and
specified some missing behavior.

This fixes w3c#273 and improves w3c#270.
jyasskin added a commit to jyasskin/webauthn that referenced this issue Feb 24, 2017
@jyasskin
Make makeCredential() more precise.
b69aefa 
I've linked a lot more terms, reordered explanations to be clearer, and
specified some missing behavior.

This fixes w3c#273 and improves w3c#270.
vijaybh pushed a commit that referenced this issue Mar 1, 2017
@jyasskin @vijaybh
Make makeCredential() more precise. (#347)
546f82f 
* Make makeCredential() more precise.

I've linked a lot more terms, reordered explanations to be clearer, and
specified some missing behavior.

This fixes #273 and improves #270.

* Treat rpId as an origin.

* Go parallel later in makeCredential().

This fixes #263 and fixes half of #254.

* Fix #265.

* Fix #266.

* Fix annevk's and equalsJeffH's comments.

* Refer to #362.

* Improve processing of unsupported extensions.
@selfissued selfissued mentioned this issue Mar 21, 2017
Merged
@jyasskin
Copy link
Member

Recording another couple missing bits in the extension processing model:

This is not a complete list of places where the input, output, and important utility operations are underdefined for extension algorithms. Someone who cares about extensions should re-read their use and definitions and file bugs for problems similar to the above.

@selfissued
Copy link
Contributor

selfissued commented Apr 2, 2017 •
edited by equalsJeffH
Loading

PR #389 Addresses the error handling aspects of this and defines a default extension result value so that callers can know that the extension was processed.

selfissued added a commit to selfissued/webauthn that referenced this issue Apr 7, 2017
@selfissued
Addressed comments by @jyasskin in issue w3c#270
1a41514
@selfissued
Copy link
Contributor

PR #389 has been updated to address the ambiguities pointed out by @jyasskin . In particular, all extensions now have a "Client data" field and none depend upon the undefined "default forwarding" language.

equalsJeffH pushed a commit that referenced this issue Apr 21, 2017
@selfissued @equalsJeffH
Separated proposed changes to extension semantics from PR #386 and us…
8eb7b5c 
…e TypeError, per @jyasskin (#389)

Major polishing of definition and exposition of extensions by selfissued - yay, thx!  includes:

* Separated proposed changes to extension semantics from PR #386 and use TypeError, per @jyasskin

* Added client data descriptions to all extensions. Accepted suggestions by @jyasskin and @vijaybh.

* Addressed comments by @jyasskin in issue #270

* Gave distinct names to extension inputs and outputs to make descriptions more precise.

* Corrected indexing errors

* Addressed additional comments by Jeff Hodges and Jeffrey Yasskin
@selfissued
Copy link
Contributor

The extension processing model is now longer very undefined, now that PR #389 has been merged in. We will still find and fix nits (for instance, the JSON string vs. DOMString issue that @jyasskin just pointed out), but we should file other issues for these as we find them and fix them in different PRs.

@selfissued selfissued mentioned this issue Feb 16, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
priority:implementation subtype:CTAP subtype:extensions type:technical
Projects
None yet
Milestone
WD-05
Development

No branches or pull requests
5 participants
@jyasskin @equalsJeffH @bzbarsky @selfissued @vijaybh