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

Add note about actual implementations of appid. #1032

Merged
merged 1 commit into from Aug 22, 2018
Merged

Conversation

agl
Copy link
Contributor

@agl agl commented Aug 9, 2018

This change adds a note to the appid extension remarking that, in
practice, I don't think anyone actually implements the FIDO FacetID spec
fully and instead checks whether the claimed AppID is same host with the
origin.

Fixes #972.


Preview | Diff

This change adds a note to the `appid` extension remarking that, in
practice, I don't think anyone actually implements the FIDO FacetID spec
fully and instead checks whether the claimed AppID is same host with the
origin.

Fixes w3c#972.
@agl agl added this to the PropRec milestone Aug 9, 2018
@@ -4257,6 +4264,11 @@ JavaScript APIs.
<var ignore>allowCredentialDescriptorList</var> and set |output| to [TRUE]. The value of |appId| then replaces the `rpId`
parameter of [=authenticatorGetAssertion=].

Note: In practice, several implementations do not implement steps four and onward of the
algorithm for [=determining if a caller's FacetID is authorized for an AppID=].
Instead, in step three, the comparison on the host is relaxed to accept hosts on the
Copy link
Contributor

Choose a reason for hiding this comment

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

What does "hosts on the same site" mean? Do you mean subdomains, such as "onmicrosoft.com" also matching "mytenant.onmicrosoft.com"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The words same site should be linked in the generated HTML to the WHATWG specification which defines this.

Copy link
Contributor

@selfissued selfissued left a comment

Choose a reason for hiding this comment

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

Looks good, other than the ambiguous terminology usage called out in the individual comment.

Copy link
Contributor

@selfissued selfissued left a comment

Choose a reason for hiding this comment

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

Thanks for the whatwg "same site" link

@nadalin
Copy link
Contributor

nadalin commented Aug 15, 2018

@aql Please merge

Copy link
Contributor

@equalsJeffH equalsJeffH left a comment

Choose a reason for hiding this comment

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

LGTM, thx @agl!

@equalsJeffH equalsJeffH merged commit bd9bc3e into w3c:master Aug 22, 2018
WebAuthnBot pushed a commit that referenced this pull request Aug 22, 2018
WebAuthnBot pushed a commit that referenced this pull request Aug 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants