haskell-servant / servant-auth

160 stars 73 forks source link

Set-Cookie JWT - happens if there is cookie in request #167

Open popara opened 4 years ago

popara commented 4 years ago

Hello,

I am sorry to open this issue, but I don't know how to debug it, and I have spent now almost a week on this issue.

Namely if user already has JWT-Cookie on every request I see Set-Cookie.

Problem is during login request I get 2 Set-Cookies which makes a mess and user can't authenticate with new credentials, even though the auth process it self works out.

First image shows login action, so you can see Set-Cookie twice

Screenshot 2020-07-23 at 13 59 48

the other one is indication that other non-Set-Cookies requests actually append Set-Cookie

Screenshot 2020-07-23 at 14 00 06

I can build SCEE if you want, but I am mostly looking for clues what could be wrong here. I have stripped whole app to barebones and it behaves the same.

I have tried to clearSession before applying cookies but avail.

Thank you for any clue that you could give me,

and THANK you for whole Servant effort <3

domenkozar commented 4 years ago

Do you have CSRF enabled?

domenkozar commented 4 years ago

See #166

popara commented 4 years ago

Here is my code for setting up cookie settings

     cookieSettings =
        defaultCookieSettings
          { cookieIsSecure = ifDev' NotSecure Secure, -- this doesn't affect behavior it happens both on dev and prod env
            cookieXsrfSetting = Nothing,
            cookieSameSite = SameSiteStrict
          }

Problem is that browser doesn't keep cookie sent from code. This happens sporadically, hence it is even more annoying. :)

here are relevant bits, I may be missing something:


-- type def part 
      "login"
      :> Auth auths Session
      :> ReqBody '[JSON] SI.Login
        :> Post '[JSON] (SetCookieHeader ())

type SetCookieHeader a = 
               Headers '[ Header "Set-Cookie" SetCookie
                              , Header "Set-Cookie" SetCookie
                               ]  a

-- handler 
  :<|> (\cs sess lf -> do
           res <- liftIO $ ZA.login pool lf -- logic of authentication
           case res of
             Left _       ->
               throwError err401
             Right (userId, isAdmin) -> do
               sid <- liftIO $
                 case sess of
                   Authenticated ses -> return $ sessionId ses
                   _                 -> newSessionId
               toExpire <- liftIO $ (addUTCTime (nominalDay * 365)) <$> getCurrentTime -- very loose cookies 
               setSession (cs { cookieExpires = Just toExpire }) jwt  (SignedIn sid userId)
       )

setSession :: CookieSettings -> JWTSettings -> S.Session
           -> Handler (Headers '[ Header "Set-Cookie" SetCookie
                                , Header "Set-Cookie" SetCookie]
                               ())
setSession = setSessionA ()

setSessionA :: a -> CookieSettings -> JWTSettings -> S.Session
           -> Handler (Headers '[ Header "Set-Cookie" SetCookie
                                , Header "Set-Cookie" SetCookie]
                               a)
setSessionA a cookieSettings jwtSettings sess = liftIO $ do
  mApplyCookies <- acceptLogin cookieSettings jwtSettings $ pTraceShowId sess
  case mApplyCookies of
    Nothing           -> fail "Can't Set Cookie?"
    Just applyCookies ->
      return $ applyCookies a

🤷 I really cant figure it out what I am doing wrong to have two set-cookie headers.

In above screenshots, /mem is a route that is a ping to a server and it has a signature of


type Mem auths =
  Auth auths Session :> RemoteHost
  :> ReqBody '[JSON] MemState
  :> Post '[JSON] ()

hence, no Set-Cookie there, but still as you can see in screen shot it happens.

Thanks for any ideas

domenkozar commented 4 years ago

There are two headers because statically XSRF cookie header is still present, but contains an empty value.

popara commented 4 years ago

Sorry, Point is that of Set-Cookie happens twice:

Screenshot 2020-09-14 at 19 27 30
HTTP/1.1 200 OK
Transfer-Encoding: chunked
Date: Mon, 14 Sep 2020 18:27:07 GMT
Server: Warp/3.2.28
Content-Type: application/json;charset=utf-8
Set-Cookie: NO-XSRF-TOKEN=; Path=/; Max-Age=69120000
Set-Cookie: JWT-Cookie=eyJhbGciOiJIUzM4NCJ9.eyJkYXQiOnsidGFnIjoiU2lnbmVkT3V0IiwiY29udGVudHMiOlt7InVuU2Vzc2lvbiI6ImUwOTBlNDQzLWY1MGMtNGFlMi05MjNiLWFlZWNjZGYwMzFiZSJ9LHsidXVpZCI6ImEzMWJiMDVlLWNjNDEtMTFlNi1iOTgzLWRiMDZmYTI0NjEyNSJ9LDEwXX19.Ejq8uwQOvluTPiiYpc6c9L-kdnZGIOpN2RQ6LpqKcQFyLJluGveo_ZEv4gvwYBsK; Path=/; Max-Age=69120000; HttpOnly; SameSite=Strict
Set-Cookie: JWT-Cookie=eyJhbGciOiJIUzM4NCJ9.eyJleHAiOjEuNjMxNjQ0MDI3NTQ2MzYyZTksImRhdCI6eyJ0YWciOiJTaWduZWRJbiIsImNvbnRlbnRzIjpbeyJ1blNlc3Npb24iOiJlMDkwZTQ0My1mNTBjLTRhZTItOTIzYi1hZWVjY2RmMDMxYmUifSx7InV1aWQiOiJhMzFiYjA1ZS1jYzQxLTExZTYtYjk4My1kYjA2ZmEyNDYxMjUifSwiQWRtaW4iXX19.R9Gw3FHPmefoAa0t0m2xSYDMuevM1Tjt-_7kiwEItowakzaVH7bomwcv_GkcuJ49; Path=/; Expires=Tue, 14-Sep-2021 18:27:07 GMT; Max-Age=69120000; HttpOnly; SameSite=Strict
Set-Cookie: NO-XSRF-TOKEN=; Path=/; Expires=Tue, 14-Sep-2021 18:27:07 GMT; Max-Age=69120000
Access-Control-Allow-Origin: *
popara commented 4 years ago

Sorry, I am not sure that I get it:

Documentation for acceptLogin says:

For a JWT-serializable session, returns a function that decorates a provided response object with XSRF and session cookies. This should be used when a user successfully authenticates with credentials.

And if I don't call applyCookies in my last line of code in above, but just

return a

I get following error message for compiler:

    • Occurs check: cannot construct the infinite type:
        a
        ~
        Headers
          '[Header "Set-Cookie" SetCookie, Header "Set-Cookie" SetCookie] a
      Expected type: Handler
                       (Headers
                          '[Header "Set-Cookie" SetCookie, Header
"Set-Cookie" SetCookie] a)
        Actual type: Handler a
    • In the expression:
        liftIO
          $ do mApplyCookies <- acceptLogin cookieSettings jwtSettings sess
               case mApplyCookies of
                 Nothing -> fail "Can't Set Cookie?"
                 Just _ -> return a
      In an equation for ‘setSessionA’:
          setSessionA a cookieSettings jwtSettings sess
            = liftIO
                $ do mApplyCookies <- acceptLogin cookieSettings
jwtSettings sess
                     case mApplyCookies of
                       Nothing -> fail "Can't Set Cookie?"
                       Just _ -> return a
    • Relevant bindings include
        a :: a (bound at server/src/Server/Common.hs:134:13)
        setSessionA :: a
                       -> CookieSettings
                       -> JWTSettings
                       -> S.Session
                       -> Handler
                            (Headers
                               '[Header "Set-Cookie" SetCookie, Header
"Set-Cookie" SetCookie] a)
          (bound at server/src/Server/Common.hs:134:1)
    |
134 | setSessionA a cookieSettings jwtSettings sess = liftIO $ do
    |                                                 ^^^^^^^^^^^...

So again, I am missing something ^^

Thanks for patience with me :)

domenkozar commented 4 years ago

I think I would need a SCEE to really help here :/

popara commented 4 years ago

On it! On 18 Sep 2020, 16:21 +0100, Domen Kožar notifications@github.com, wrote:

I think I would need a SCEE to really help here :/ — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe.

and-pete commented 3 years ago

🤷 I really cant figure it out what I am doing wrong to have two set-cookie headers.

If you still care about this, there are some clues you can use to work out what's going on.

Big caveat first though that I'm still learning also and if anyone spots any understanding errors or code errors I'm happy to edit in the fixes into my Wall Of Text below.

So... if you compare the two JWT-Token-containing Set-Cookie headers you're receiving, you'll notice that one of them has an expiry date and the other doesn't. That is:

Set-Cookie: JWT-Cookie=....; Path=/; Max-Age=69120000; HttpOnly; SameSite=Strict Set-Cookie: JWT-Cookie=....; Path=/; Expires=Tue, 14-Sep-2021 18:27:07 GMT; Max-Age=69120000; HttpOnly; SameSite=Strict

The only time an expiry comes into play in your code is in your login handler:

setSession (cs { cookieExpires = Just toExpire }) jwt  (SignedIn sid userId)

...which feeds into the CookieSettings that you pass into Servant.Auth.Server's acceptLogin function.

But the other mysterious JWT-Cookie appearing in your code must be receiving a value of type CookieSettings where the cookieExpires field hasn't been modified from the default Nothing. Can you find anywhere else in your code that you have created and used a value of type CookieSettings other than in this setSession/acceptLogin login handler?

I'll bet somewhere else in your code you have :) For example, do you have a serveWithContext function that you have used when creating the WAI Application type? I bet you have! And I bet you passed an argument to that function something like:

defaultCookieSettings :. defaultJWTSettings jwk :. EmptyContext

Or... at the very least... you passed in some modified version of the defaultCookieSettings when creating that context. And that's what's creating that first JWT-Cookie. From what I can tell, you modified Servant.Auth.Server's default behaviour of having cookieMaxAge set to Nothing, which is why both cookies have a max age set.

Ahh... this is a big clue for me is this below:

In above screenshots, /mem is a route that is a ping to a server and it has a signature of

type Mem auths =
  Auth auths Session :> RemoteHost
  :> ReqBody '[JSON] MemState
  :> Post '[JSON] ()

hence, no Set-Cookie there, but still as you can see in screen shot it happens.

"hence, no Set-Cookie there"? But there IS a Set-Cookie there! That's what the Auth combinator is doing! The Auth combinator is using the CookieSettings and the JWTSettings that were passed into the Context that you created on the server initialization and it is making new cookies with it! :)

Any time you hit an endpoint protected by that Auth combinator, if your request contains the required valid cookies you will notice the response that you get back also has new Set-Cookie headers attached. This is the expected behaviour! With a cookieMaxAge field set to a Just value in your Context's CookieSettings, an active user can keep sending requests to the routes protected by the Auth combinator and keep receiving their auth token back in a new cookie with each response. And the Max Age value will mean that the browser will extend the effective expiry to Time Response is Received + Max Age.

So it's only after a period of time of inactivity (of using Auth-protected routes) longer than Max Age that the cookie will expire and you'll have to log in again. But if you the client keeps hitting those routes, they'll keep getting new cookies extending their session.

This is pretty easy to test actually! Set up a protected endpoint (like your Mem) and make sure only the cookieMaxAge field is set for the CookieSettings you pass into the server's Context (...and not the cookieExpiry). If you hit that protected endpoint every few seconds with your browser (let's roll with Chrome), you'll see the JWT-Cookie stored in the DevTools/Application/Cookies pane has an expiry that increases by those few seconds. Because as I mentioned, every response you receive sets a new Cookie whose Max Age is treated as a new thing that the browser compares to the current time. And it overwrites the previous cookie that was there with the old expiry time based on the Time of Response to Your Earlier Request + Max Age.

So the Servant.Auth.Server model is not one where you receive a JWT-Cookie once and keep reusing that same cookie until it expires, which could be in the middle of an active user session.

As for why you've got 2 sets of JWT-Cookie headers... well if we take a look at your login endpoint's type:

-- type def part 
      "login"
      :> Auth auths Session
      :> ReqBody '[JSON] SI.Login
        :> Post '[JSON] (SetCookieHeader ())

...you can see that you've protected that route also using an Auth combinator! That is, according to that type-level route, in order to login, you must send a valid header with it. That could be.... kind of tricky... to say the least.

Though maybe that's what you were attempting? As when I decoded the JWTs you posted via https://jwt.io I saw that the one without an expiry contained a field "tag": "SignedOut" in its payload, whereas the JWT with an expiry contained a field "tag": "SignedIn"). And if we look at your first screenshot, you can see that you sent a JWT-Cookie with your login request, which (along with valid credentials/etc.) is why the request wasn't rejected with a 401. So perhaps you're trying to make having a SignedOut session cookie a requirement to sign in with your SI.Login credentials?

Let's say we have the following example code:

type LoginAPI content = "login" :> ReqBody '[JSON] SI.Login :> Verb POST 204 '[JSON] content

type Mem = RemoteHost :> ReqBody '[JSON] MemState :> Post '[JSON] NoContent

type SomethingPublic = "whatever" :> Get '[JSON] Text

Then your current code is roughly equivalent to something like:

-- | Anything in the Protected branch receives a Set-Cookie from the Auth combinator
type API = "api" :> (SAS.Auth '[Cookie,JWT] User :> Protected) :<|> Unprotected

type Unprotected = "unprotected" :> SomethingPublic 

type Protected = "protected" :> ( LoginAPI (SetCookieHeader NoContent) :<|> Mem )

So again... because the Login route is within the protected route, it receives 1 set of headers from the Auth combinator protecting that route (...using the CookieSettings in your Servant server's Context) AND another set from the SetCookieHeader response to the login request itself (where you modified that CookieSettings to add an expiry JUST for the very first cookie that gets sent in response to the login request, but no others after it). Whereas the Mem endpoint is also in the Auth-protected route and so also receives the new cookie from the Context on each valid request. But you don't see the doubling there as we're not dressing it up with the extra cookies from acceptLogin like we are in the LoginAPI route.

Assuming that you wanted a more conventional auth flow, it would be something instead like this:

type API = "api" :> (SAS.Auth '[Cookie,JWT] User :> Protected) :<|> Unprotected    -- same as before

-- LoginAPI has been moved from Protected to Unprotected
type Unprotected = "unprotected" :> (LoginAPI (SetCookieHeader NoContent) :<|> SomethingPublic)

type Protected = "protected" :> Mem

So under that new scheme the following should be true. As a response, a request to:

Also it's worth emphasizing again.... If you set a cookie expiry time in the CookieSettings you pass into the acceptLogin function, that expiry time will only apply to the cookies that are sent in response to that login request. As soon as you hit any other Auth combinator-protected route you are going to get a new cookie that will overwrite it based on whatever CookieSettings are in your context. And based on your current settings, that will have the default cookieExpires = Nothing field set. Of course, it's probably not a good idea to try to add a short expiry to the CookieSettings you pass into the Server's Context either. Because that same expiry will be on every new Set-Cookie that you receive in your valid responses under the Auth-protected routes (...though at least to my understanding from MDN you should see browsers defaulting to choosing Max Age over Expires if both are present).

Hopefully this helps you (...and anyone else, in particular those who go searching the lib's Issues for anything about Expiry times). If anyone spots any errors or comes up with any tests that violates what I said above, please let me know! I'm still learning too and have had to revise my assumptions about how this all works a few times already :)

Servant is challenging! For many reasons, but here in particular because type-level combinators that protect routes like Auth can be hiding a lot :) And it's definitely not clear on first glance that you're going to be getting a new Set-Cookie for every valid request you send to a protected route.

popara commented 3 years ago

This is amazing writing! Thank you very much!

I have read servant-auth but it never clicked to me that Auth x y combinator might be the source of the issue.

I think this is mostly linguistical confusion: I was using Auth to get information about the session, not necessarily to protect the route. Also "protecting" sounds like somebody reads and check is you have valid cookie on request. I wasn't expecting that it would put stamps in my passport whenever I pass trough Auth :) Maybe some note about in docs would be good idea.

I wanted to carry session information in the cookie even before user logs in, hence even my home page is "protected", I wanted to keep their work, so after they authenicate I have a way to reference their work. I will try tomorrow morning to verify by removing Auth from login end point.

Thank you for this!