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

Allow caller to pick between strict and eTLD+1 matching #162

Closed
wants to merge 6 commits into from
Closed

Allow caller to pick between strict and eTLD+1 matching #162

wants to merge 6 commits into from

Conversation

vijaybh
Copy link
Contributor

@vijaybh vijaybh commented Aug 10, 2016

Per suggestion from @bifurcation on this morning's WG call.

@vijaybh
Allow caller to pick between strict and eTLD+1 matching
98fb91e 
Per suggestion from @bifurcation on this morning's WG call.
@jcjones
Copy link
Contributor

jcjones commented Aug 31, 2016

This LGTM; it does require a rebase/merge.

@balfanz
Copy link
Contributor

balfanz commented Aug 31, 2016

How about we do something akin to domain lowering:

dictionary CredentialOptions {
  unsigned long                       timeoutSeconds;
  DOMString                           rpId;
  sequence < CredentialDescription >  excludeList;
  WebAuthnExtensions                  extensions;
};

rpId would be optional, and if left unspecified, the caller's origin shall be used. If it is specified, then valid values are postfixes of the caller's origin, up to - but no further than - eTLD+1.

We'd have to define what happens to the scheme and port of the caller's origin, but I assume domain lowering already deals with this in some way (i.e., both http://foo.example.com:1234 and https://foo.example.com:9876 are allowed to lower to "example.com").

@equalsJeffH equalsJeffH added this to the WD-02 milestone Aug 31, 2016
@vijaybh
Copy link
Contributor Author

vijaybh commented Sep 2, 2016

I do think @balfanz proposes a more elegant solution than the Boolean. Let me write it up in a new PR and we can pick one of the two.

@equalsJeffH
Copy link
Contributor

Ok, I can live with either this "boolean switch" approach, or Dirk's as-yet-not-in-a-PR-it-seems approach. There are detail-level issues in this "boolean switch" approach, tho I'll wait to see which approach we go with before working on detail polishing. thanks for writing this up.

yaronf
yaronf reviewed Sep 7, 2016
View reviewed changes
index.bs
@@ -343,7 +344,8 @@ When this method is invoked, the user agent MUST execute the following algorithm
2. Let |promise| be a new <a data-lt="Promises">Promise</a>. Return |promise| and start a timer for |adjustedTimeout| seconds.
Then asynchronously continue executing the following steps.

3. Set |callerOrigin| to the <a link-for='web'>origin</a> of the caller. Derive the RP ID from |callerOrigin| by computing the
3. Set |callerOrigin| to the <a link-for='web'>origin</a> of the caller. If {{CredentialOptions/rpStrict}} was set to true or
not specified, then set the RP ID to |callerOrigin|. Otherwise, erive the RP ID from |callerOrigin| by computing the
Copy link

Choose a reason for hiding this comment

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

typo: erive

@yaronf
Copy link

yaronf commented Sep 7, 2016

For either of these solutions, please note that RFC 7719 basically says that "eTLD+1" is not well defined. Quoting: "Note that the term "public suffix" is controversial in the DNS community for many reasons, and may be significantly changed in the future." Can we clarify that the Mozilla-maintained list of public suffixes is authoritative on this term?

@vijaybh vijaybh self-assigned this Sep 7, 2016
vijaybh added a commit that referenced this pull request Sep 14, 2016
@vijaybh
Allow caller to explicitly specify its claimed RP ID
443482e 
Alternative to #162 as suggested by @balfanz. This change also gets rid
of references to PSL and eTLD+1, replacing them with references to the
HTML specification instead.
@vijaybh vijaybh mentioned this pull request Sep 14, 2016
Merged
@equalsJeffH
Copy link
Contributor

Let's go with #198 rather than this one.

@vijaybh
Copy link
Contributor Author

vijaybh commented Sep 16, 2016

Closing this out since the consensus on the previous call was similar, that this is inferior to #198.

@vijaybh vijaybh closed this Sep 16, 2016
@vijaybh vijaybh deleted the vgb-etld-minus-one branch September 16, 2016 22:05
vijaybh added a commit that referenced this pull request Sep 28, 2016
@vijaybh
Allow callers to explicitly specify RP ID (#198)
ca1f0ee 
* Allow caller to explicitly specify its claimed RP ID

Alternative to #162 as suggested by @balfanz. This change also gets rid
of references to PSL and eTLD+1, replacing them with references to the
HTML specification instead.

* Remove rpId from ClientData

Since rpId is now equal to origin except when explicitly specified by
the caller, and it is folded into the authenticatorData, having it in
ClientData adds no value.

Fixes #189

* Incorporate feedback from @equalsJeffH

* More feedback from @equalsJeffH

* Incorporate feedback from @equalsJeffH

Note this PR also fixes #178 by removing all use of the term lowercase
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@vijaybh vijaybh

Labels
None yet
Projects
None yet
Milestone
WD-02
Development

Successfully merging this pull request may close these issues.
5 participants
@vijaybh @jcjones @balfanz @equalsJeffH @yaronf