CSRF failures due to duplicate XSRF-TOKEN values

[bitemyapp]

Presently, what you see in the screenshot will work because the XSRF-TOKEN values are the same, however if they differ, you'll get that CSRF token middleware error we all know and love.

I do not know how two different XSRF tokens were generated (no repro), but it's not clear to me why there isn't a single XSRF token for the / path which covers requests being made from other pages anyway.

I read this thread: #1016

It did not clear up for me why there should be multiple X-XSRF-TOKEN values in the cookies.

AFAICT, this code is responsible for the duplicates:

yesod/yesod-core/Yesod/Core/Handler.hs

Lines 858 to 859 in 8bbe91c

	addHeaderInternal :: MonadHandler m => Header -> m ()
	addHeaderInternal = tell . Endo . (:)

Some snippets:

addHeader :: MonadHandler m => Text -> Text -> m ()
addHeader a = addHeaderInternal . Header (encodeUtf8 a) . encodeUtf8

setCookie :: MonadHandler m => SetCookie -> m ()
setCookie = addHeaderInternal . AddCookie

-- | Internal use only, not to be confused with 'setHeader'.
addHeaderInternal :: MonadHandler m => Header -> m ()
addHeaderInternal = tell . Endo . (:)

tell :: MonadHandler m => Endo [Header] -> m ()
tell hs = modify $ \g -> g { ghsHeaders = ghsHeaders g `mappend` hs }

data GHState = GHState
    { ghsSession :: SessionMap
    , ghsRBC     :: Maybe RequestBodyContents
    , ghsIdent   :: Int
    , ghsCache   :: TypeMap
    , ghsCacheBy :: KeyedTypeMap
    , ghsHeaders :: Endo [Header]
    }

[bitemyapp]

This prevents duplicates but the paths do change as you shuffle about. I don't know how acceptable a solution it is but it should at least prevent the CSRF failures we had.

https://github.com/bitemyapp/yesod/commit/5e4cefc9ad3880258d9685849dea1f9ea0daf768

[MaxGabriel]

Oh, I didn't think about specifying the path when I implemented this. I haven't researched it yet, but

a single XSRF token for the / path which covers requests being made from other pages...

sounds good to me

[bitemyapp]

@MaxGabriel my solution doesn't currently do that, but I could modify it. I don't really understand what place duplicate cookie values or mostly duplicate ones with different paths are even supposed to mean here. I'd like to understand the problem a bit more first before doing more.

[MaxGabriel]

Here's my guess of what's going on:

1. When the cookie is set by the middleware, its path defaults to the current path of the request.

2. Based on a StackOverflow answer, cookies are uniqued by (domain, name, path), so there can be multiple cookies with the same name I guess. This is the relevant part of RFC 2965:

   3.3.3 Cookie Management If a user agent receives a Set-Cookie2
   response header whose NAME is the same as that of a cookie it has
   previously stored, the new cookie supersedes the old when: the old
   and new Domain attribute values compare equal, using a case-
   insensitive string-compare; and, the old and new Path attribute
   values string-compare equal (case-sensitive).

3. When the Javascript looks up the cookie by name, it's probably getting the older cookie with the incorrect CSRF token value. It then sends the wrong CSRF token to the server, which the server rejects.

[MaxGabriel]

Hm, I'm having trouble reproing getting the CSRF token for a path like "/signup", not sure why.

[bitemyapp]

@MaxGabriel We have GHCJS code talking to the backend, did you try performing a CSRF secured AJAX call?

[MaxGabriel]

Yes, using the comment AJAX text field that comes with the scaffolding. I can see the XSRF token in the headers of that POST request, too.

[MaxGabriel]

Ok got it, wasn't thinking that the path needed to be something like:

localhost:3000/foo/bar

for the cookie path to be foo. Working on a fix