WICG / dbsc

Other
272 stars 19 forks source link

The scheme is a little bit redundant for a redirect-based auth #12

Open alextok opened 6 months ago

alextok commented 6 months ago

This scheme is a little bit redundant for a redirect-based auth. We need an extra roundtrip to initiate the binding, but we can save this roundtrip, if we will remember before auth that a web app wants to start session when request will go back on redirect URI. We can made up headers for this, that will initiate binding before the session.

kmonsen commented 6 months ago

Thanks for the suggestion! Can you make an example so I can understand better?

alextok commented 6 months ago

In a redirect based auth there is a key thing, "redirect_uri" a uri where an auth server sends a response, when the auth is completed.

Basic flow is following:

  1. User: GET https://web.app.com
  2. web.app.com: 302 HTTP Location: https://auth.server.com/...?redirect_uri=https://web.app.com/redirect_uri
  3. auth.server.com: performs auth, and issues redirect to back to redirect uri. 302 HTTP Location: https://web.app.com/redirect_uri?code=

We can adjust the scheme following:

  1. User: GET https://web.app.com
  2. web.app.com: 302 HTTP Sec-Session-Registration: registration=session/start;supported-alg=ES256,RS256;challenge=nonce; Location: https://auth.server.com/...?redirect_uri=https://web.app.com/session/start
  3. Because Sec-Session-Registration header is present, the web browser remembers that, when navigation goes to https://web.app.com/session/start it needs to supply the binding information.
  4. auth.server.com: performs auth, and issues redirect to back to redirect uri. 302 HTTP Location: https://web.app.com/session/start?code=...
  5. Web browsers observes a navigation to https://web.app.com/session/start, as a result it adds a new special header Sec-Session-Registration-Info to the request GET https://web.app.com/session/start?code=... Sec-Session-Registration-Info: \<base64-URL-encoded registration JWT>

Your exiting proposal can be fit in this scheme as well:

  1. User: GET https://web.app.com
  2. web.app.com: 302 HTTP Sec-Session-Registration: registration=/session/start;supported-alg=ES256,RS256;challenge=nonce; Location: https://web.app.com/session/start?authorization=...
  3. Because Sec-Session-Registration header is present, the web browser remembers that, when navigation goes to https://web.app.com/session/start it needs to supply binding information.
  4. The web browsers initializes navigation to https://web.app.com/session/start as it is 302, and adds the binding header Sec-Session-Registration-Info because binding is present. GET https://web.app.com/session/start?authorization= Sec-Session-Registration-Info: \<base64-URL-encoded registration JWT>
alextok commented 6 months ago

Idea is just work on headers. If browser sees header Sec-Session-Registration, it needs to supply Sec-Session-Registration-Info, when navigation goes to the path defined by registration parameter of Sec-Session-Registration. In this case, we do not add extra POST as the current protocol suggests.

It also will align with proposal we have sent you offline.

kmonsen commented 6 months ago

Let me try to explain how I understand to see if it matches what you propose. We have the following:

Currently what would happen in DBSC is:

  1. User: GET https://web.app.com/
  2. web.app.com: 302 HTTP Sec-Session-Registration: registration=/session/start;supported-alg=ES256,RS256;challenge=nonce; Location: https://web.app.com/session/start?authorization=...
  3. POST https://web.app.com/session/start Sec-Session-Registration-Info:
  4. web.app.com: 200 HTTP Registration JSON in body`
  5. GET https://web.app.com/session/start?authorization=... --> Following redirect

I think you would like to remove (5) in this special case? Would it be OK if (3) was a GET instead of a POST, or basically the same as (1)? What about the registration JSON in that case? We could put it in a header I guess?

So we would end up with something like this:

  1. User: GET https://web.app.com/
  2. web.app.com: 302 HTTP Sec-Session-Registration: registration=/session/start;supported-alg=ES256,RS256;challenge=nonce; Location: https://web.app.com/session/start?authorization=...
  3. GET https://web.app.com/session/start?authorization=... --> Following redirect Sec-Session-Registration-Info:
  4. web.app.com: 200 HTTP Sec-Session-Response: {} // Registration JSON Body is expected page content
alextok commented 6 months ago

I think you would like to remove (5) in this special case? Would it be OK if (3) was a GET instead of a POST, or basically the same as (1)? What about the registration JSON in that case? We could put it in a header I guess?

I actually wanted to remove 3 and 4 from your description (the special post).

However, your final scheme looks good for me:

  1. User: GET https://web.app.com/
  2. web.app.com: 302 HTTP Sec-Session-Registration: registration=/session/start;supported-alg=ES256,RS256;challenge=nonce; Location: https://web.app.com/session/start?authorization=...
  3. GET https://web.app.com/session/start?authorization=... --> Following redirect Sec-Session-Registration-Info:
  4. web.app.com: 200 HTTP Sec-Session-Response: {} // Registration JSON Body is expected page content

Only thing, it is a simple scheme, when the server redirects to itself. In reality, web.app.com will redirect to auth.server.com, and auth.server.com via multiple redirects eventually will redirect back to web.app.com with response. The web browser should remember that and attach the Sec-Session-Registration-Info, once final redirect to web.app.com happens.

mattjm commented 5 months ago

Do we need a separate registration endpoint in this case? Could the regular callback endpoint just do the registration if the Sec-Session-Regstration-Info header is present?

alextok commented 5 months ago

IMHO, yes, the regular callback can do registration. I don't have strong opinion on how important a separate registration endpoint is.

arnar commented 5 months ago

Thanks for this discussion!

The main issue with this imo is that it mixes the state management for the session with top-level requests. In the current dbsc proposal, we've specifically kept the session start and refresh as dedicated "background" requests to the session management endpoint on the server. This allows us to isolate the session state management in the browser in a single place, thus avoiding trying to maintain its invariants throughout the more complex network stack. It is also a big simplification that such requests are explicitly triggered and sent by this component, and we're not piggybacking on other requests that have their own semantics and side effects.

For example, remembering the fact we saw a Sec-Session-Registration header and applying the registration message on some later, and otherwise incidental request to that URL, is not trivial. Such requests could happen simultaneously in different threads, so I think this would at least need some locking strategy in code that is quite risky to change.

So maybe this is an optimization we can save for later? There is indeed an opportunity to get rid of one round-trip, but the benefits of that seem small compared to the added risk and complexity.

alextok commented 3 months ago

There is indeed an opportunity to get rid of one round-trip, but the benefits of that seem small compared to the added risk and complexity.

Extra roundtrip is an extra point of failure, extra memory, CPU, electricity, labor for supporting it and an extra delay. Some web applications will not be able to use it, because of follow up form post.

The cost of complexity is a onetime cost paid by one or two devs in every browser, but the cost of this extra roundtrip accrued daily by every web app and every login, who will use this protocol.

I want to hear others, but IMHO this onetime cost is not a comparable thing with a daily accrued cost in a world scale. We are doing this for many years for entire world.

mattjm commented 3 months ago

I've been staring at the diagram trying to come up with some comments, and I realized there's a discrepancy that might be relevant to the discussion here.

On the diagram I have the OAuth flow completing, and then negotiating the secure session. Per @kmonsen's post above, the (current) flow actually has the browser deferring the redirect, negotiating the secured session, and then finishing the redirect (similar to the refresh flow farther down). I don't think the text was explicit on this point so I'd like to confirm what's expected (if nothing else to make sure the diagram is accurate). That may or may not affect some of the complexity around threading.

To the immediate discussion: I'm inclined to agree with @alextok that "The cost of complexity is a onetime cost paid by one or two devs in every browser". At least it is my understanding this is mostly implemented in the browser. Concurrency issues are a thing (hello, rotating refresh tokens) but if anyone is qualified to sort them out it has to be the folks who wrote this browser that I regularly abuse with half a dozen windows and 50+ tabs, 10 of which are gmail 👀 .

alextok commented 3 months ago

Here an attempt to draw a diagram for OAuth protocols.

https://github.com/WICG/dbsc/issues/16#issuecomment-2016426763

mattjm commented 3 months ago

We're in alignment on signin--The existing diagram doesn't go into the details of the OAuth flow, but it shows the secure session is set up after the initial signin flow.

Back to the original discussion:

We only have the "extra" request on the initial sign-in, right? And for refresh the browser can tell if it has the right cookies and will defer requests internally, so we don't have to worry about concurrency there, if I'm understanding it right.

Thinking out loud here--not sure if these ideas make sense or are feasible to implement:

What if...you never did the secure registration during the initial sign-in? So no extra headers during the OAuth flow. And then if a bound session is required the internal deferral mechanism does the registration on the next request? Basically it would run the refresh flow like for expired bound cookies, but now the refresh flow is also able to do initial registrations if needed.

I guess that doesn't actually save you any requests, unless we think it will be a common pattern where some content would require a bound session and some would not (you would save the secure session negotiation if you never accessed content requiring a bound session). The diagram does include the situation where a request is made and a bound cookie is not required. 🤔

Could the internal deferral mechanism be used during the sign-in flow? But I think that's what @arnar is talking about where now you're not keeping the session state management in a single place. But a lot of that logic should be similar? Probably? Maybe? 👀 I've never contributed to a browser project so I will defer to those who have.

mattjm commented 3 months ago

Thought about my musings in the last post some more--the reason the internal deferral mechanism works (or at least, how it works as proposed) is because we establish during the secure session negotiation which paths require a bound cookie. So not doing the registration on initial sign-on opens a whole can of worms, and probably doesn't save many requests in the real world anyway.