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

HTTP 401 MUST return a WWW-Authenticate header #65

Open
danmarg opened this issue Jul 26, 2024 · 9 comments
Open

HTTP 401 MUST return a WWW-Authenticate header #65

danmarg opened this issue Jul 26, 2024 · 9 comments

Comments

@danmarg
Copy link
Contributor

danmarg commented Jul 26, 2024

We initially wrote here that the HTTP request that triggers a challenge/response should serve an HTTP 401 response code and a Sec-Session-Challenge header.

But per https://datatracker.ietf.org/doc/html/rfc2616#section-10.4.2, a 401 MUST return a WWW-Authenticate header.

Without having put any thought into this, I can see a few obvious options:

  1. Add a new HTTP authentication method, and replace the Sec-Session-Challenge header with a WWW-Authenticate header.
  2. Just return a different response code.

I see the point of doing (1) just to avoid violating the spec--(2) seems like a better choice for that. And I could see some value in reusing HTTP authentication if we think that the DBSC-style challenge/response authn will be useful on its own (and thus is worth fleshing out "standalone", without the whole refresh/session-management stuff).

But I am not sure the value there is worth trying to make this fit the existing HTTP authentication paradigm; option (2) thus seems better to me.

Curious to hear what others think! Should we just return a 403 instead?

@MattMenke2
Copy link

Worth noting that while RFC 2616 is of course obsolete, the current RFC has similar language (https://datatracker.ietf.org/doc/html/rfc7235).

@mattjm
Copy link
Contributor

mattjm commented Jul 26, 2024

@MattMenke2 thanks for the reminder about updated RFCs--based on the obsolete one I was going to say 403 might not be appropriate, but per updated RFCs it should be OK:

"The client SHOULD NOT automatically repeat the request with the same credentials. The client MAY repeat the request with new or different credentials. " citation

I agree it seems cumbersome to define a new authentication scheme just for this.

I feel like I've seen (don't ask me for examples) a few cases where a service returned 403 when a 401 might have made more sense. Now I'm wondering how often that was because the engineer didn't want to bother returning the WWW-authenticate header but needed to comply with the spec. 😂

@MattMenke2
Copy link

I think a 403 would be reasonable, though I'm no expert on how 403 are typically used (or intended to be used). Another option would be to return a 200 with a no-cache, no-store header. Main advantage of the 4xx status code is that it informs any intermediaries that this is an error and should not be cached, and potentially shows up in any applicable logs as an error.

@MattMenke2
Copy link

MattMenke2 commented Jul 26, 2024

Separating it out from an error code would also allow redirecting browsers that don't support to, e.g., an alternative login flow or whatnot, rather than having to sniff browser version to determine compatibility. So return a redirect + challenge. If browser knows how to handle the request, it send credentials. Otherwise, it follows the redirect. That's how Sec-Session-Storage (?) works.

I guess as specified, the 401s aren't actually normal loads, but rather browser-initiated loads in response to other requests, so I that capability may not be useful here.

@danmarg
Copy link
Contributor Author

danmarg commented Jul 26, 2024

Uh oh. As I read this, I think I've stepped into a can of worms.

Three observations:

  1. The explainer also contains this: The server can also serve challenges ahead of time attached to any response as an optimization, for example...1
  2. The refresh endpoint can choose to not require a new challenge/response--it can set the required cookies right away if, for example, the IP address hasn't changed (or whatever). It can also choose to not set the required cookies, of course (like, if the signature is broken).
  3. We are only talking about requests to the refresh endpoint, so browsers that don't understand DBSC should never even make such requests.

So this leaves me a bit confused! On the one hand, I think point 3 above matches your point, @MattMenke2, that these aren't normal page loads, so the advantage of serving existing error codes is limited.

On the other hand, point 2 suggests to me that it's important that unexpected errors from refresh trigger DBSC session termination: it's important that the browser stop throttling requests that require some DBSC-managed cookie if it tried and failed to refresh the session; it's those throttled requests--which are "normal" page loads--that will then be treated as unauthenticated and result in the user being redirected to a sign-in or error page.

But, point (1) makes me think the server can choose to merely return a Sec-Session-Challenge header on refresh and the right thing will happen, regardless of error code.

Whew!

Now, I sort of think the state machine for client-side behavior is something like:

  • If a response has a Sec-Session-Challenge, store that challenge as "latest challenge"
  • If a pending request matches a DBSC session and is missing a required cookie, call the refresh endpoint
  • If calling the refresh endpoint and there's an existing "latest challenge", sign it and add the signature to the request

All of that suggests to me that we can just get rid of specifying HTTP response codes entirely on calls to refresh and the above algorithm just does The Right Thing.

Sorry for the very long response. Does that reasoning make sense?

Footnotes

  1. I think this is unclear. Reading the text, it seems like it's saying that that should immediately trigger a refresh ("The browser replies to that response with a Sec-Session-Response header, containing a signed JWT..."), but I believe the intent was that the browser should pre-cache that challenge and use it in the future whenever it decides to do a refresh.

@danmarg
Copy link
Contributor Author

danmarg commented Jul 26, 2024

Oh, and to my comment

On the other hand, point 2 suggests to me that it's important that unexpected errors from refresh trigger DBSC session termination

I think this means that if there is an error code from refresh, it should trigger session termination. (Though we need to specify which codes are "fatal" and which are "transient" and require a client retry.)

@MattMenke2
Copy link

Alternatively, could just consider the session refreshed until it's refresh time again, or the server tells us otherwise, but since I don't think we tell the server in normal requests about the live session, unlike cookies, killing the session on error does seem reasonable to me.

@kmonsen
Copy link
Collaborator

kmonsen commented Jul 31, 2024

I think there are a few options:

  1. Update 401 to mandate WWW-Authenticate OR Sec-Session-Challenge, would be broken against browsers that doesn't support DBSC, but the server should only send this where it expects there to be a session. I guess one questions is what would the client do if the session has just been delete locally? We could resend the request with a "No such session" or something.
  2. Use some other 4xx/invent a new one. This sort of has the same issues as (1) I think.
  3. Use 302 as we once intended. This has a few issues, first 401 forbidden makes a lot of sense in that the content is forbidden until the client authenticates, it has not been moved. Second all the current standard compliant browsers know how to resend a request when receiving 401 after updating the authentication.

I don't feel strongly about this (I argued for 302 for a long time, primarily for backwards compatibility). Happy to update as needed to what others want. I think right now, I would propose we try (1) unless we meet large resistance.

I should say currently there is an option where the refresh requests doesn't happen, but instead the browser start signing every request and the server can reply with 401. So the client need to expect 401 on different requests, but all requests that can have a 401 response have "Sec-Session-Response: JWT" header in the request.

@wparad
Copy link

wparad commented Aug 1, 2024

302 feels right, but may cause the wrong things to happen in browsers/clients that handle this differently.

Since the DBSC process starts authentication, wouldn't it make sense that the www-authenticate header is still returned with the matching information? Is somehow using this incompatible with DBSC?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants