Skip to content

adds Related Origin Requests #2040

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

Merged
merged 32 commits into from
Jul 17, 2024
Merged

adds Related Origin Requests #2040

Changes from 1 commit
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
0522807
initial text
timcappalli Mar 12, 2024
d48a18c
remaining draft text
timcappalli Mar 12, 2024
2a18351
s/webauthn-origins/webauthn
timcappalli Mar 13, 2024
689647c
MUST support 5 origin labels
timcappalli Mar 13, 2024
3aad054
s/member/property, s/list/array
timcappalli Mar 13, 2024
5187e72
use [=continue=]
timcappalli Mar 26, 2024
881ac8f
collapse multiple steps into one
timcappalli Mar 26, 2024
9062a15
s/vanity/alternative
timcappalli Mar 26, 2024
2315db8
capitalization
timcappalli Mar 26, 2024
8ea0303
s/relatedOriginRequests/relatedOrigins
timcappalli Mar 26, 2024
28a2b0f
attempt at using a procedure
timcappalli Mar 26, 2024
b51bd16
s/must/MUST
timcappalli Mar 26, 2024
a21babf
remove break
timcappalli Apr 29, 2024
7466f32
remove break for create
timcappalli Apr 29, 2024
e7d0c7c
s/originLabelsSeen/labelsSeen and set def
timcappalli Apr 29, 2024
b3bf34c
word order
timcappalli Apr 29, 2024
bdba742
Validation step optimization
timcappalli Apr 29, 2024
de25c37
use macros for true and false
timcappalli Apr 29, 2024
71e4e80
add period
timcappalli Jun 12, 2024
26d350c
text tweak
timcappalli Jun 12, 2024
a2ac319
clean up well-known JSON definition
timcappalli Jun 14, 2024
b120220
s/origin labels/labels
timcappalli Jun 14, 2024
bcec7fa
s/rpIdRequested/callerOrigin
timcappalli Jun 14, 2024
3ba2ff3
move set contains label check after origin match check
timcappalli Jun 14, 2024
a108a2c
add RP guidance around adding all origins with the same origin label …
timcappalli Jun 14, 2024
c0c40d1
remove check for label in labelsSeen, not necessary
timcappalli Jun 14, 2024
ebf03eb
fix indents
timcappalli Jun 14, 2024
e4f24d9
update RWO algo to support additional origins with the same label
timcappalli Jun 26, 2024
2e9bdcf
remove consideration text for RPs about RWO well-known ordering
timcappalli Jun 26, 2024
2462fd8
Emil's feedback
timcappalli Jul 17, 2024
9da2c4b
procedure dfn
timcappalli Jul 17, 2024
017a5e3
Editorial tweaks
timcappalli Jul 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
word order
  • Loading branch information
@timcappalli
timcappalli committed Apr 29, 2024
commit b3bf34c961435c15b3f55f0dbbe4e539d831d1d5
2 changes: 1 addition & 1 deletion index.bs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4110,7 +4110,7 @@ For example, for the RP ID `example.com`:

To validate the calling origin is an authorized related origin for a given ceremony:
Copy link
Member

Choose a reason for hiding this comment

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

This should explicitly define callerOrigin and rpIdRequested as parameters of the procedure, and the references in the client algorithms should explicitly specify the arguments.

I think we should also put a <dfn> here and reference that rather than simply linking to the section with a different link text. Right now the link text "related origins validation procedure" appears nowhere else in the document, so readers have to rely solely on the hyperlink in order to discover what is actually referenced (this is not the only case of that throughout the spec, but IMO it's preferable to minimize them).

Copy link
Member Author

Choose a reason for hiding this comment

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

This should explicitly define callerOrigin and rpIdRequested as parameters of the procedure, and the references in the client algorithms should explicitly specify the arguments.

Do you have an example of what that should look like in Bikeshed?

I think we should also put a here and reference that rather than simply linking to the section with a different link text. Right now the link text "related origins validation procedure" appears nowhere else in the document, so readers have to rely solely on the hyperlink in order to discover what is actually referenced (this is not the only case of that throughout the spec, but IMO it's preferable to minimize them).

Not sure I understand. What would a definition look like for a procedure? Isn't that just the section that defines the procedue?

Copy link
Member

Choose a reason for hiding this comment

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

We have a few examples in the spec:

So for example, this section could start with:

### Validating Related Origins ### {#sctn-validating-relation-origin}

The <dfn abstract-op>related origins validation procedure</dfn>, given arguments |callerOrigin| and |rpIdRequested|,
is as follows:

and then the references in create() and get() could look like this:

                            1. Run the [$related origins validation procedure$] with arguments |callerOrigin| and |rpIdRequested|.
                                If the result is [FALSE], throw a "{{SecurityError}}" {{DOMException}}.

This has a few advantages: most importantly it makes the connection clearer and less reliant on hidden metadata, so it's more likely to survive format conversions such as into PDF or plain text (or even actual print, if someone were to actually do that), or quotations. Less importantly it also shows Bikeshed's list of references when you click the definition, which doesn't happen for links to sections.

Alternatively, the link to the section should be a plain section reference without replacing the link text, so that it renders the whole section heading, for the reasons above.

Like I said, we don't always respect these concerns throughout the spec, and I don't think we always should, but I think this is one of the cases where there's little or nothing to gain from not doing so.

Copy link
Member

Choose a reason for hiding this comment

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

But to be fair, there is also at least this counter-example where we do link to a section with a (slightly) different link text:

Copy link
Member Author

Choose a reason for hiding this comment

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

Makes total sense. Addressed in 9da2c4b. Thanks for explaining @emlun!


1. Let |maxLabels| be the number of maximum [=origin labels=] allowed by client policy.
1. Let |maxLabels| be the maximum number of [=origin labels=] allowed by client policy.
1. Fetch the well-known URL for the RP ID (<code>https://|rpIdRequested|/.well-known/webauthn</code>).
Copy link
Contributor

Choose a reason for hiding this comment

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

I have a question about the fetching of the well-known URL. Could this be abused given that the origin of the page that contains the WebAuthn call is necessarily a different origin from the site where the well-known document will be retrieved? This seems to constitute a cross origin request.

Copy link
Member Author

Choose a reason for hiding this comment

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

It is the WebAuthn client software fetching the document, not anything in the web session. What types of abuse were you thinking?

Copy link
Member Author

Choose a reason for hiding this comment

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

@nsatragno can you comment based on the discussion on the call?

1. If the fetch fails, the response does not have a content type of `application/json`, or does not have a status code (after following redirects) of 200, then throw a "{{SecurityError}}" {{DOMException}}.
1. If the body of the resource is not a valid JSON object, then throw a "{{SecurityError}}" {{DOMException}}.
Expand Down