Include message body in 302 responses #399
Conversation
|
Someone is attempting to deploy a commit to the Auth0 Team on Vercel. A member of the Team first needs to authorize it. |
|
Thanks @michielvangendt - thanks for raising this, looks good Can we do what Next.js does and add It will keep this fix consistent with how the redirect works in |
|
Hi @adamjmcgrath, it appears that their implementation (with |
ok, np - can you reference this PR when you do? Thanks @michielvangendt |
|
@michielvangendt - when vercel/next.js#25257 is merged can you update this to match and I'll go ahead and approve this. |
### Description The redirect responses from the redirect function do not contain a message body. This is in conflict with the RFCs below and causes Traefik (a reverse proxy) to invalidate the responses. In this pull request, I add a response body to the redirect responses. ### References - https://datatracker.ietf.org/doc/html/rfc7230#section-3.3 > All 1xx (Informational), 204 (No Content), and 304 (Not Modified) responses must not include a message-body. All other responses do include a message-body, although the body may be of zero length. - https://datatracker.ietf.org/doc/html/rfc7231#section-6.4.3 > The server's response payload usually contains a short hypertext note with a hyperlink to the different URI(s). - traefik/traefik#4456 - auth0/nextjs-auth0#399
|
@adamjmcgrath the PR in next.js is merged! |
| @@ -54,6 +54,6 @@ export default function logoutHandlerFactory( | |||
| res.writeHead(302, { | |||
| Location: returnURL | |||
| }); | |||
| res.end(); | |||
| res.end(returnURL); | |||
There was a problem hiding this comment.
I just noticed that you need to update lines 31 and 43
| @@ -62,6 +62,6 @@ export default function callbackHandlerFactory( | |||
| res.writeHead(302, { | |||
| Location: openidState.returnTo || config.baseURL | |||
| }); | |||
| res.end(); | |||
| res.end(openidState.returnTo || config.baseURL); | |||
There was a problem hiding this comment.
Also - I think we should escape these (and the others)
It's probably overkill because modern browsers wont serve this content, but openidState.returnTo can contain reflected user input from /api/auth/login?returnTo=user-input
I also notice that express escapes the content in 302
There's a util method to escape html here https://github.com/auth0/nextjs-auth0/blob/main/src/utils/errors.ts#L26-L34
FYI @lzychowski
### Description The redirect responses from the redirect function do not contain a message body. This is in conflict with the RFCs below and causes Traefik (a reverse proxy) to invalidate the responses. In this pull request, I add a response body to the redirect responses. ### References - https://datatracker.ietf.org/doc/html/rfc7230#section-3.3 > All 1xx (Informational), 204 (No Content), and 304 (Not Modified) responses must not include a message-body. All other responses do include a message-body, although the body may be of zero length. - https://datatracker.ietf.org/doc/html/rfc7231#section-6.4.3 > The server's response payload usually contains a short hypertext note with a hyperlink to the different URI(s). - traefik/traefik#4456 - auth0/nextjs-auth0#399
|
Closing due to innactivity - happy to reopen if you want to finish this off |
Description
The 302 responses from the auth API do not contain a message body. This is in conflict with the RFCs below and causes Traefik (a reverse proxy) to invalidate the responses. In this pull request, I add a response body to the 302 responses.
References