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.

Sign up for GitHub

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

Jump to bottom

Consider using HTTP 307 instead of HTP 302 #82

Open
swantzter opened this issue Jan 11, 2024 · 1 comment · May be fixed by #83
Open

Consider using HTTP 307 instead of HTP 302 #82

swantzter opened this issue Jan 11, 2024 · 1 comment · May be fixed by #83

Comments

@swantzter
Copy link

What happened:

Our tokens are being refreshed because of a background POST request that is then invalidly retried as a GET request by the browser because of the semantic ambiguity of 301 and 302.

For example:

  1. accessToken expires
  2. user-agent makes a background fetch request to POST /api/whatever
  3. Lambda@Edge refreshes the tokens using the refresh token
  4. Lambda@Edge responds with a 302 Location /api/whatever and Set-Cookie headers
  5. user-agent retries the background fetch request to /api/whatever as a GET request which fails

What did you expect to have happen:

  1. Should retry the request as a POST request again - a 307 status code rather than 302 would enforce this

How to reproduce this (as precisely and succinctly as possible):

This server behind cognito at edge should be able to reproduce the error (visible in the console/network panel)

const http = require('http')

const server = http.createServer((req, res) => {
  const url = new URL(req.url, 'http://localhost:8000')

  if (url.pathname === '/' && req.method === 'GET') {
    res.writeHead(200, { 'Content-type': 'text/html' })
    res.end(siteHtml)
    return
  } else if (url.pathname === '/api' && req.method === 'POST') {
    res.writeHead(200, { 'content-type': 'text/plain', 'access-control-allow-origin': '*', 'access-control-allow-credentials': '*' })
    res.end('Success')
    return
  }

  res.writeHead(404, { 'content-type': 'text/plain' })
  res.end('Not found')
})

server.listen(8000, () => {
  console.log('listening on http://localhost:8000')
})

const siteHtml = `
<html>
  <head><title>Test Cognito At Edge Background Redirect</title></head>
  <body>
    <script>
      setInterval(() => {
        fetch('/api', { method: 'POST', mode: 'cors', credentials: 'include' })
          .then(async res => { console.log(res.status, await res.text()) })
          .catch(err => { console.error(err) })
      }, 10_000)
    </script>
  </body>
</html>
`

Anything else you think we should know?

Environment:

  • version of cognito-at-edge being used: 1.5.0
  • node version of code base which uses cognito-at-edge: lambda runtime node20.x
@swantzter
Copy link
Author

We've tested using this module patched to use 307 in our setup - it works

swantzter added a commit to swantzter/cognito-at-edge that referenced this issue Jan 12, 2024
@swantzter
Use 307 redirect when refreshing tokens in background to keep HTTP me…
422e210 
…thod

fixes awslabs#82
@swantzter swantzter linked a pull request Jan 12, 2024 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Use 307 redirect when refreshing tokens in background to keep HTTP method swantzter/cognito-at-edge
1 participant
@swantzter