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

Merge pull request #2186 from w3c/tc-relatedorigins-tweaks #2201

Closed
wants to merge 1 commit into from

Conversation

agl
Copy link
Contributor

@agl agl commented Nov 13, 2024

Mozilla feedback: Related Origins

(This change landed on the level3 branch, but we need it on the main branch too because a) we don't want to lose it in level four and b) we're going to recut the level3 branch due to the number of changes that would otherwise need to be cherry-picked across.)


Preview | Diff

Mozilla feedback: Related Origins

(This change landed on the `level3` branch, but we need it on the main
branch too because a) we don't want to lose it in level four and b)
we're going to recut the `level3` branch due to the number of changes
that would otherwise need to be cherry-picked across.)
@agl agl requested a review from emlun November 13, 2024 23:23
@agl
Copy link
Contributor Author

agl commented Nov 13, 2024

@emlun this is the approved change that Tim landed on the level3 branch (d543bd3), ported to the main branch. If you approve this, I'll land it and then set the level3 branch to match the resulting main hash.

@timcappalli
Copy link
Member

I'm confused. Wasn't this addressed by: #2197

@emlun
Copy link
Member

emlun commented Nov 14, 2024

Diff from main to this PR:

$ git diff origin/main...origin/timschange
diff --git a/index.bs b/index.bs
index 82da64d7..ac2a5997 100644
--- a/index.bs
+++ b/index.bs
@@ -4529,7 +4529,7 @@ This can make deployment challenging for large environments where multiple count
 [=[WRPS]=] can opt in to allowing [=WebAuthn Clients=] to enable a credential to be created and used across a limited set of related [=origin|origins=].
 Such [=[RPS]=] MUST choose a common [=RP ID=] to use across all ceremonies from related origins.

-A JSON document MUST be hosted at the `webauthn` well-known URL [[!RFC8615]] for the [=RP ID=]. The JSON document MUST be returned as follows:
+A JSON document MUST be hosted at the `webauthn` well-known URL [[!RFC8615]] for the [=RP ID=], and MUST be served using HTTPS. The JSON document MUST be returned as follows:

     - The content type MUST be `application/json`.
     - The top-level JSON object MUST contain a key named `origins` whose value MUST be an array of one or more strings containing web origins.
@@ -4555,6 +4555,9 @@ For example, for the RP ID `example.com`:

 [=WebAuthn Clients=] supporting this feature MUST support at least five [=registrable origin labels=]. Client policy SHOULD define an upper limit to prevent abuse.

+Requests to this well-known endpoint by [=WebAuthn Clients=] MUST be made without [=request/credentials mode|credentials=], without a [=request/referrer policy|referrer=],
+and using the `https:` [=scheme=]. When following redirects, [=WebAuthn Clients=] MUST explicitly require all redirects to also use the `https:` [=scheme=].
+
 [=WebAuthn Clients=] supporting this feature SHOULD include {{ClientCapability/relatedOrigins}} in their response to [[#sctn-getClientCapabilities|getClientCapabilities()]].

 ### Validating Related Origins ### {#sctn-validating-relation-origin}
@@ -4562,7 +4565,7 @@ For example, for the RP ID `example.com`:
 The <dfn abstract-op>related origins validation procedure</dfn>, given arguments |callerOrigin| and |rpIdRequested|, is as follows:

 1. Let |maxLabels| be the maximum number of [=registrable origin labels=] allowed by client policy.
-1. Fetch the `webauthn` well-known URL [[!RFC8615]] for the RP ID |rpIdRequested| (i.e., <code>https://|rpIdRequested|/.well-known/webauthn</code>).
+1. Fetch the `webauthn` well-known URL [[!RFC8615]] for the RP ID |rpIdRequested| (i.e., <code>https://|rpIdRequested|/.well-known/webauthn</code>) without [=request/credentials mode|credentials=], without a [=request/referrer policy|referrer=] and using the `https:` [=scheme=].
     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}}.
     1. If the value of the |origins| property of the JSON object is missing, or is not an array of strings, then throw a "{{SecurityError}}" {{DOMException}}.

Diff from main to PR #2197:

$ git diff origin/main...origin/level3
diff --git a/index.bs b/index.bs
index 0db3e9d0..5f13a220 100644
--- a/index.bs
+++ b/index.bs
@@ -4537,7 +4537,7 @@ This can make deployment challenging for large environments where multiple count
 [=[WRPS]=] can opt in to allowing [=WebAuthn Clients=] to enable a credential to be created and used across a limited set of related [=origin|origins=].
 Such [=[RPS]=] MUST choose a common [=RP ID=] to use across all ceremonies from related origins.

-A JSON document MUST be hosted at the `webauthn` well-known URL [[!RFC8615]] for the [=RP ID=]. The JSON document MUST be returned as follows:
+A JSON document MUST be hosted at the `webauthn` well-known URL [[!RFC8615]] for the [=RP ID=], and MUST be served using HTTPS. The JSON document MUST be returned as follows:

     - The content type MUST be `application/json`.
     - The top-level JSON object MUST contain a key named `origins` whose value MUST be an array of one or more strings containing web origins.
@@ -4563,6 +4563,9 @@ For example, for the RP ID `example.com`:

 [=WebAuthn Clients=] supporting this feature MUST support at least five [=registrable origin labels=]. Client policy SHOULD define an upper limit to prevent abuse.

+Requests to this well-known endpoint by [=WebAuthn Clients=] MUST be made without [=request/credentials mode|credentials=], without a [=request/referrer policy|referrer=],
+and using the `https:` [=scheme=]. When following redirects, [=WebAuthn Clients=] MUST explicitly require all redirects to also use the `https:` [=scheme=].
+
 [=WebAuthn Clients=] supporting this feature SHOULD include {{ClientCapability/relatedOrigins}} in their response to [[#sctn-getClientCapabilities|getClientCapabilities()]].

 ### Validating Related Origins ### {#sctn-validating-relation-origin}
@@ -4570,7 +4573,7 @@ For example, for the RP ID `example.com`:
 The <dfn abstract-op>related origins validation procedure</dfn>, given arguments |callerOrigin| and |rpIdRequested|, is as follows:

 1. Let |maxLabels| be the maximum number of [=registrable origin labels=] allowed by client policy.
-1. Fetch the `webauthn` well-known URL [[!RFC8615]] for the RP ID |rpIdRequested| (i.e., <code>https://|rpIdRequested|/.well-known/webauthn</code>).
+1. Fetch the `webauthn` well-known URL [[!RFC8615]] for the RP ID |rpIdRequested| (i.e., <code>https://|rpIdRequested|/.well-known/webauthn</code>) without [=request/credentials mode|credentials=], without a [=request/referrer policy|referrer=] and using the `https:` [=scheme=].
     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}}.
     1. If the value of the |origins| property of the JSON object is missing, or is not an array of strings, then throw a "{{SecurityError}}" {{DOMException}}.

Diff between diffs:

$ git diff --no-index <(git diff origin/main...origin/timschange) <(git diff origin/main...origin/level3)
diff --git a/dev/fd/63 b/dev/fd/62
--- a/dev/fd/63
+++ b/dev/fd/62
@@ -1,8 +1,8 @@
 diff --git a/index.bs b/index.bs
-index 82da64d7..ac2a5997 100644
+index 0db3e9d0..5f13a220 100644
 --- a/index.bs
 +++ b/index.bs
-@@ -4529,7 +4529,7 @@ This can make deployment challenging for large environments where multiple count
+@@ -4537,7 +4537,7 @@ This can make deployment challenging for large environments where multiple count
  [=[WRPS]=] can opt in to allowing [=WebAuthn Clients=] to enable a credential to be created and used across a limited set of related [=origin|origins=].
  Such [=[RPS]=] MUST choose a common [=RP ID=] to use across all ceremonies from related origins.

@@ -11,7 +11,7 @@ index 82da64d7..ac2a5997 100644

      - The content type MUST be `application/json`.
      - The top-level JSON object MUST contain a key named `origins` whose value MUST be an array of one or more strings containing web origins.
-@@ -4555,6 +4555,9 @@ For example, for the RP ID `example.com`:
+@@ -4563,6 +4563,9 @@ For example, for the RP ID `example.com`:

  [=WebAuthn Clients=] supporting this feature MUST support at least five [=registrable origin labels=]. Client policy SHOULD define an upper limit to prevent abuse.

@@ -21,7 +21,7 @@ index 82da64d7..ac2a5997 100644
  [=WebAuthn Clients=] supporting this feature SHOULD include {{ClientCapability/relatedOrigins}} in their response to [[#sctn-getClientCapabilities|getClientCapabilities()]].

  ### Validating Related Origins ### {#sctn-validating-relation-origin}
-@@ -4562,7 +4565,7 @@ For example, for the RP ID `example.com`:
+@@ -4570,7 +4573,7 @@ For example, for the RP ID `example.com`:
  The <dfn abstract-op>related origins validation procedure</dfn>, given arguments |callerOrigin| and |rpIdRequested|, is as follows:

  1. Let |maxLabels| be the maximum number of [=registrable origin labels=] allowed by client policy.

So yes, the two PRs contain the same changes. PR #2197 has the slight advantage that it preserves @timcappalli's authorship tag on the commits:

$ git log --pretty=fuller --no-patch origin/main..origin/level3
commit b287006438e4522132b0b6419ace3818d914f984 (origin/level3)
Merge: efdf948e 241833d9
Author:     Tim Cappalli <[email protected]>
AuthorDate: Wed Nov 13 20:19:14 2024 +0000
Commit:     GitHub <[email protected]>
CommitDate: Wed Nov 13 20:19:14 2024 +0000

    Merge pull request #2186 from w3c/tc-relatedorigins-tweaks

    Mozilla feedback: Related Origins

commit 241833d9b964e4b4c5b1a82e04d23d9ae9038d77
Author:     Tim Cappalli <[email protected]>
AuthorDate: Wed Oct 23 13:39:33 2024 -0400
Commit:     Tim Cappalli <[email protected]>
CommitDate: Wed Oct 23 13:39:33 2024 -0400

    require HTTPS scheme for all well-known calls and redirects

commit 875486f36312ffe907c25ba8b9ad520aad94c59e
Author:     Tim Cappalli <[email protected]>
AuthorDate: Wed Oct 23 13:03:39 2024 -0400
Commit:     Tim Cappalli <[email protected]>
CommitDate: Wed Oct 23 13:03:39 2024 -0400

    No credentials or referrer for RoR well-known


$ git log --pretty=fuller --no-patch origin/main..origin/timschange
commit d543bd33ef13d336c2973bb83dfadf49e1a8cf2b (origin/timschange)
Author:     Adam Langley <[email protected]>
AuthorDate: Wed Nov 13 15:21:34 2024 -0800
Commit:     Adam Langley <[email protected]>
CommitDate: Wed Nov 13 15:21:34 2024 -0800

    Merge pull request #2186 from w3c/tc-relatedorigins-tweaks

    Mozilla feedback: Related Origins

    (This change landed on the `level3` branch, but we need it on the main
    branch too because a) we don't want to lose it in level four and b)
    we're going to recut the `level3` branch due to the number of changes
    that would otherwise need to be cherry-picked across.)

@nadalin nadalin added the process:meta-pr Pull requests into other pull requests rather than main label Nov 20, 2024
@agl agl closed this Nov 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
process:meta-pr Pull requests into other pull requests rather than main
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants