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

Is resultingClientId preserved over redirects? #1316

Closed
mattto opened this Issue May 30, 2018 · 13 comments

Comments

Projects
None yet
3 participants
@mattto
Member

mattto commented May 30, 2018

Splitting off of #1315

On my reading of the spec, I thought the reserved client gets set one time at the beginning of a navigation, and it never changes until navigation finishes.

In the Chrome we were just going to not give reserved clients URLs (or rather, we'd essentially ignore whatever URL it currently has until navigation settles). But we would preserve resultingClientId over cross-origin redirects. I believe a team here wanted this at least for same-origin redirects but probably don't care about cross-origin. Is there any reason we'd prefer to preserve or discard the id for cross-origin ones?

@annevk

This comment has been minimized.

Show comment
Hide comment
@annevk

annevk May 30, 2018

Member

Currently https://html.spec.whatwg.org/#process-a-navigate-fetch ends up resetting the reserved client for each redirect.

When and where is the reserved client exposed? It seems bad if different origins can get hold of the same unique identifier, so if you're proposing changing the above setup to not reset it for each redirect, we should make sure that's not possible.

Member

annevk commented May 30, 2018

Currently https://html.spec.whatwg.org/#process-a-navigate-fetch ends up resetting the reserved client for each redirect.

When and where is the reserved client exposed? It seems bad if different origins can get hold of the same unique identifier, so if you're proposing changing the above setup to not reset it for each redirect, we should make sure that's not possible.

@mattto

This comment has been minimized.

Show comment
Hide comment
@mattto

mattto May 30, 2018

Member

Reserved clients are never exposed. Clients.get(id) resolves once the navigation finishes or rejects once the navigation failed. It would also reject if the resulting client is cross-origin to the caller. So I was thinking just having the same unique identifier would be safe.

In #process-a-navigate-fetch, I thought the relevant part (navigation follows a redirect) is step 7:
"If response has a location URL and it is either failure or a URL whose scheme is an HTTP(S) scheme, then set response to the result of performing HTTP-redirect fetch using request and response and then run this step again."

I read that as running step 7 again, i.e., step 5 only happens once.

Member

mattto commented May 30, 2018

Reserved clients are never exposed. Clients.get(id) resolves once the navigation finishes or rejects once the navigation failed. It would also reject if the resulting client is cross-origin to the caller. So I was thinking just having the same unique identifier would be safe.

In #process-a-navigate-fetch, I thought the relevant part (navigation follows a redirect) is step 7:
"If response has a location URL and it is either failure or a URL whose scheme is an HTTP(S) scheme, then set response to the result of performing HTTP-redirect fetch using request and response and then run this step again."

I read that as running step 7 again, i.e., step 5 only happens once.

@annevk

This comment has been minimized.

Show comment
Hide comment
@annevk

annevk May 30, 2018

Member

You're reading that correctly, I was mistaken. That means the URL never gets updated though. It also means the CSP check only gets applied once. That's almost certainly a bug.

Member

annevk commented May 30, 2018

You're reading that correctly, I was mistaken. That means the URL never gets updated though. It also means the CSP check only gets applied once. That's almost certainly a bug.

@wanderview

This comment has been minimized.

Show comment
Hide comment
@wanderview

wanderview May 30, 2018

Member

I believe we agreed at a previous f2f to reset the resulting client when we perform a cross-origin redirect. Because clients are tied to their origin, something like this must happen. Otherwise we risk exposing the same client ID to two different origin service workers.

For example, a fetch event for origin A, then a redirect to origin B, and then a fetch event on origin B. The origin A and origin B should see different client IDs, IMO.

I'll look later today to try to find where we discussed that before.

Member

wanderview commented May 30, 2018

I believe we agreed at a previous f2f to reset the resulting client when we perform a cross-origin redirect. Because clients are tied to their origin, something like this must happen. Otherwise we risk exposing the same client ID to two different origin service workers.

For example, a fetch event for origin A, then a redirect to origin B, and then a fetch event on origin B. The origin A and origin B should see different client IDs, IMO.

I'll look later today to try to find where we discussed that before.

@wanderview

This comment has been minimized.

Show comment
Hide comment
@wanderview
Member

wanderview commented May 30, 2018

Please see #1031 (comment).

@wanderview

This comment has been minimized.

Show comment
Hide comment
@wanderview

wanderview May 30, 2018

Member

@jakearchibald In #1031 (comment), were you just bulk closing "reserved client" issues or were you saying you think we should reverse the previous f2f issue?

Member

wanderview commented May 30, 2018

@jakearchibald In #1031 (comment), were you just bulk closing "reserved client" issues or were you saying you think we should reverse the previous f2f issue?

@wanderview

This comment has been minimized.

Show comment
Hide comment
@wanderview

wanderview May 30, 2018

Member

Requiring a single client ID value to change origins would be quite difficult for me to implement in firefox. Our client architecture is designed to make it hard for clients to switch origins since normally that's a very undesirable thing.

Member

wanderview commented May 30, 2018

Requiring a single client ID value to change origins would be quite difficult for me to implement in firefox. Our client architecture is designed to make it hard for clients to switch origins since normally that's a very undesirable thing.

@mattto

This comment has been minimized.

Show comment
Hide comment
@mattto

mattto May 30, 2018

Member

The use case I'm hearing is if you load a page and it redirects to a different origin authentication server which does some passive authentication and then redirects back to the original origin (apparently this happens with Google). They'd like the resultingClientId to be preserved so the whole thing can be counted as one page load, and measure the page load time starting at the initial request.

Member

mattto commented May 30, 2018

The use case I'm hearing is if you load a page and it redirects to a different origin authentication server which does some passive authentication and then redirects back to the original origin (apparently this happens with Google). They'd like the resultingClientId to be preserved so the whole thing can be counted as one page load, and measure the page load time starting at the initial request.

@wanderview

This comment has been minimized.

Show comment
Hide comment
@wanderview

wanderview May 30, 2018

Member

Uh, that is a huge change and against the spirit of our previous f2f agreements in my opinion.

Also consider that sometimes a resulting client ID represents an initial about:blank and not a reserved client. In those cases a new client must be created.

I'm not saying we shouldn't try to meet this use case, but I'd like to discuss alternatives that don't require changing the origin of a given client ID.

To start, how does having a single resulting client ID give them timing information? I don't really understand that.

Member

wanderview commented May 30, 2018

Uh, that is a huge change and against the spirit of our previous f2f agreements in my opinion.

Also consider that sometimes a resulting client ID represents an initial about:blank and not a reserved client. In those cases a new client must be created.

I'm not saying we shouldn't try to meet this use case, but I'd like to discuss alternatives that don't require changing the origin of a given client ID.

To start, how does having a single resulting client ID give them timing information? I don't really understand that.

@mattto

This comment has been minimized.

Show comment
Hide comment
@mattto

mattto May 30, 2018

Member

I think having the same ID helps them connect the two fetch events together in their logging, so they know the events were part of the same navigation. They can probably do some manual work to accomplish the same but it'd be less convenient.

I'll loop back to them.

Sorry that this would amount to a huge change here and sorry I didn't acknowledge that earlier in the thread. I thought the earlier discussions were more complicated because we planned to expose reserved Clients to JavaScript. When we decided to not expose them, a lot of the problems went away, and I thought sharing an ID would be harmless if the cross-origin can't get a Client out of the ID (and I thought the spec was saying to preserve the IDs, but as I learned here that wasn't intended).

Member

mattto commented May 30, 2018

I think having the same ID helps them connect the two fetch events together in their logging, so they know the events were part of the same navigation. They can probably do some manual work to accomplish the same but it'd be less convenient.

I'll loop back to them.

Sorry that this would amount to a huge change here and sorry I didn't acknowledge that earlier in the thread. I thought the earlier discussions were more complicated because we planned to expose reserved Clients to JavaScript. When we decided to not expose them, a lot of the problems went away, and I thought sharing an ID would be harmless if the cross-origin can't get a Client out of the ID (and I thought the spec was saying to preserve the IDs, but as I learned here that wasn't intended).

@wanderview

This comment has been minimized.

Show comment
Hide comment
@wanderview

wanderview May 30, 2018

Member

I'm not sure I really want to do this either, but we could try to preserve the original client and use it again after the redirect back to the original origin. So the answer to the question in #1031 (comment) would be two clients instead of the previously agreed four clients.

My main concern here is that it could get complicated with long redirect chains. We could end up trying to preserve and track up to 19 different origin clients in case that last redirect goes back to one of them.

Member

wanderview commented May 30, 2018

I'm not sure I really want to do this either, but we could try to preserve the original client and use it again after the redirect back to the original origin. So the answer to the question in #1031 (comment) would be two clients instead of the previously agreed four clients.

My main concern here is that it could get complicated with long redirect chains. We could end up trying to preserve and track up to 19 different origin clients in case that last redirect goes back to one of them.

@mattto

This comment has been minimized.

Show comment
Hide comment
@mattto

mattto Jun 1, 2018

Member

Yes I think I'd rather not try to keep track of the reserved client per-origin. I'd rather keep the same ID for the whole navigation or just discard the ID completely on cross-origin redirect.

I'm sympathetic to the architecture that tries not to allow Clients to jump origins. I'll see if we can align with that.

Member

mattto commented Jun 1, 2018

Yes I think I'd rather not try to keep track of the reserved client per-origin. I'd rather keep the same ID for the whole navigation or just discard the ID completely on cross-origin redirect.

I'm sympathetic to the architecture that tries not to allow Clients to jump origins. I'll see if we can align with that.

mattto pushed a commit to mattto/html that referenced this issue Aug 8, 2018

Matt Falkenhagen
Create a new reserved environment for cross-origin redirects.
Previously, a navigation would create a reserved environment once,
and use it for all redirects. This commit changes that so
a new environment is created on a cross-origin redirect.

This also runs CSP for each redirect, which is probably more
correct.

Per w3c/ServiceWorker#1316

mattto pushed a commit to mattto/html that referenced this issue Aug 31, 2018

Matt Falkenhagen
Create a new reserved environment for cross-origin redirects.
Previously, a navigation would create a reserved environment once,
and use it for all redirects. This commit changes that so
a new environment is created on a cross-origin redirect.

This also runs CSP for each redirect, which is probably more
correct.

Per w3c/ServiceWorker#1316

mattto pushed a commit to mattto/html that referenced this issue Sep 28, 2018

Matt Falkenhagen
Create a new reserved environment for cross-origin redirects.
Previously, a navigation would create a reserved environment once,
and use it for all redirects. This commit changes that so
a new environment is created on a cross-origin redirect.

This also runs CSP for each redirect, which is probably more
correct.

Per w3c/ServiceWorker#1316

annevk added a commit to whatwg/html that referenced this issue Oct 8, 2018

Create a new reserved environment for cross-origin redirects
Previously, a navigation would create a reserved environment once and use it for all redirects. This commit changes that so a new environment is created on a cross-origin redirect.

We now also run CSP for each redirect.

See also w3c/ServiceWorker#1316.

@mattto mattto self-assigned this Oct 10, 2018

@mattto

This comment has been minimized.

Show comment
Hide comment
@mattto

mattto Oct 10, 2018

Member

Fixed by whatwg/html@45ddbd2

Preserved on same-origin redirects, changed on cross-origin redirects.

Member

mattto commented Oct 10, 2018

Fixed by whatwg/html@45ddbd2

Preserved on same-origin redirects, changed on cross-origin redirects.

@mattto mattto closed this Oct 10, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment