http4s/http4s-session

The problem of SessionMiddleware always overwriting cookies.

Opened this issue ยท 0 comments

Hi there ๐Ÿ‘‹

If another cookie is set using methods such as addCookie while SessionMiddleware is in use, it will be overwritten by SessionMiddleware.

When tried with VaultSessionExample.

def app[F[_]: Sync](key: Key[PageViews]): HttpRoutes[F] = {
  val dsl = new Http4sDsl[F] {}; import dsl._
  HttpRoutes.of {
    ...
    case GET -> Root / "other" / "cookie" =>
      Ok("Burn Other Cookies")
        .map(_.addCookie("OtherCookie", "hogehoge"))
        .map(res => {
          println(res.headers.headers) // List(Content-Type: text/plain; charset=UTF-8, Content-Length: 18, Set-Cookie: OtherCookie=hogehoge)
          res
        })
    ...
  }
}

Check the value of the header inside the SessionMiddleware.

...
  sessionCookie(id).map(response.response.putHeaders(_)).map(res => {
    println(response.response.headers.headers) // List(Content-Type: text/plain; charset=UTF-8, Content-Length: 18, Set-Cookie: OtherCookie=hogehoge)
    println(res.headers.headers) // List(Content-Type: text/plain; charset=UTF-8, Content-Length: 18, Set-Cookie: id=QiTmr/STABek+iyx+Z4SB7j98GJygDupnTokqr1ioDs=; SameSite=Lax; HttpOnly)
    res
  })
...

OtherCookie is never burned into the browser's cookie, only the id generated by SessionMiddleware is burned into the browser's cookie.

This is probably due to the fact that a new Set-Cookie has been generated inside SessionMiddleware using the putHeaders method.

As a test, instead of generating a Set-Cookie and updating the header, I was able to update the ResponseCookie value inside SessionMiddleware using the addCookie method to process the update without overwriting other cookie information.

example

    def sessionCookie(id: SessionIdentifier): F[ResponseCookie] = {
      expiration match {
        case ExpirationManagement.Static(maxAge, expires) =>
          ResponseCookie(sessionIdentifierName,
                         id.value,
                         domain = domain,
                         httpOnly = httpOnly,
                         secure = secure,
                         path = path,
                         sameSite = sameSite.some,
                         maxAge = maxAge,
                         expires = expires
          ).pure[F]
        case e @ ExpirationManagement.Dynamic(fromNow) =>
          HttpDate.current[F](Functor[F], e.C).flatMap { now =>
            fromNow(now).map { case ExpirationManagement.Static(maxAge, expires) =>
                ResponseCookie(sessionIdentifierName,
                               id.value,
                               domain = domain,
                               httpOnly = httpOnly,
                               secure = secure,
                               path = path,
                               sameSite = sameSite.some,
                               maxAge = maxAge,
                               expires = expires
                )
            }
          }
      }
    }

...

  sessionCookie(id).map(response.response.addCookie(_)).map(res => {
    println(response.response.headers.headers) // List(Content-Type: text/plain; charset=UTF-8, Content-Length: 18, Set-Cookie: OtherCookie=hogehoge)
    println(res.headers.headers) // List(Content-Type: text/plain; charset=UTF-8, Content-Length: 18, Set-Cookie: OtherCookie=hogehoge, Set-Cookie: id=Z15hkKuTeO7xSPACYOYn2kq4Krqf0QYDWgYJ7fPKliE=; SameSite=Lax; HttpOnly)
    res
  })
...

If this solution is acceptable, I will create a pull request.
If I am wrong, I would appreciate it if you could let me know.

Thanks!