haskell-servant / servant-auth

160 stars 73 forks source link

CSRF: document how double submit protection works #97

Open saurabhnanda opened 6 years ago

saurabhnanda commented 6 years ago

I was trying to get servant-auth + servant-generic + hoistServer setup for a side-project and ran into the following problem -- with the code given below, I am unable to ever get the Authenticated user_ branch of the case-match to work. The user should toggle between logged-in/logged-out state when visiting http://localhost:8000/login

login :: (CookieSettings, JWTSettings) -> AuthResult UserId -> AppM (Headers '[Header "Set-Cookie" SetCookie, Header "Set-Cookie" SetCookie] Text)
login (cookieCfg, jwtCfg) authResult = case authResult of
  Authenticated user_ -> do
    -- TODO: Log-out the user
    pure $ noHeader $ noHeader ([qc|Found auth user {user_}, who has now been logged out|])
  x -> liftIO $ acceptLogin cookieCfg jwtCfg (UserId 12 :: PrimaryKey UserT Identity) >>= \case
    Nothing -> pure $ noHeader $ noHeader ([qc|Authentication failed with {x}. Moreoever cookie creation failed, as well.|])
    Just applyCookies -> pure $ applyCookies ([qc|Authentication failed with {x}. UserId 12 has been signed in.|])

data Routes route = Routes
  { rTest :: route :- "test" :> Get '[PlainText] Text
  , rLogin :: route :- Auth '[Cookie] UserId :> "login" :> Get '[PlainText] (Headers '[Header "Set-Cookie" SetCookie, Header "Set-Cookie" SetCookie] Text)
  } deriving (Generic)

server :: (CookieSettings, JWTSettings) -> Routes (AsServerT (AppM))
server cookieConfig = Routes
  { rTest = pure "test works"
  , rLogin = login cookieConfig
  }

On a separate note, why does acceptLogin have TWO Set-Cookie headers? I didn't realise that the acceptLogin function existed, and I ended up using the makeSessionCookie earlier, and it seems to work with a single cookie header in the type-sig:

liftIO $ makeSessionCookie cookieCfg jwtCfg (UserId 12 :: PrimaryKey UserT Identity) >>= \case
    Nothing -> pure $ noHeader ([qc|Authentication failed with {x}. Moreoever cookie creation failed, as well.|])
    Just cookieHeader -> pure $ addHeader cookieHeader ([qc|Authentication failed with {x}. UserId 12 has been signed in.|])
domenkozar commented 6 years ago

You'll get authenticated user if everything works ok. That means you could be setting cookie to be https only, etc. Check if cookie is set when accessing rLogin.

About two cookies: it sets XSRF and Auth cookies separately.

saurabhnanda commented 6 years ago

You'll get authenticated user if everything works ok. That means you could be setting cookie to be https only, etc. Check if cookie is set when accessing rLogin.

Here's what my cookie config looks like:

cookieCfg = defaultCookieSettings{cookieIsSecure=Servant.Auth.Server.NotSecure}

And, yes, the cookie being set AND sent in request headers. How can I debug the cookie read/decrypt steps to figure out where things are going wrong?

domenkozar commented 6 years ago

@saurabhnanda I find it useful in such bugs to put together a http://sscce.org/

Could start with:

#!/usr/bin/env nix-shell
#!nix-shell -i runghc -p "haskellPackages.ghcWithPackages (pkgs: with pkgs; [servant-server servant-generic])"

{-# LANGUAGE DataKinds #-}
{-# LANGUAGE TypeOperators #-}

module ApiType where

import Data.Aeson.Types
import Data.Text
import Data.Time (UTCTime)
import Data.Time.Calendar
import GHC.Generics
import Servant.API
import Servant
import Servant.Generic
import Network.Wai
import Network.Wai.Handler.Warp

data Site route = Site
  { about :: route :-
      Capture "x" Text :> Get '[JSON] Text
  , faq :: route :-
      Capture "x" Int :> Get '[JSON] Int
  } deriving Generic

siteServer :: Site AsServer
siteServer = Site
  { about = return 
  , faq = return
  }

type UserApi = ToServant (Site AsApi)

userAPI :: Proxy UserAPI
userAPI = Proxy

server1 :: Server UserAPI
server1 = toServant siteServer

app1 :: Application
app1 = serve userAPI server1

main :: IO ()
main = do
    print "serving"
    run 8081 app1
saurabhnanda commented 6 years ago

@domenkozar here's an isolated piece of code: https://gist.github.com/saurabhnanda/032b80702f18a42badc62db3f7a1d401

Here's what happens on the web browser:

screenshot 2018-06-14 08 48 30

domenkozar commented 6 years ago

Made it compile in https://gist.github.com/domenkozar/120a36788706c5f49fd6bcc6fff2f3bc - I think there's something wrong with XSRF, I'd try upgrading to servant-auth master but ran out of time, will check over weekend.

jkarni commented 6 years ago

What happens if you set cookieXsrfSetting = Nothing (in CookieSettings)? The request headers seem like they were cut-off in your image - are you having your javascript send the X-XSRF-TOKEN as mentioned in the README?

Also I'm a little unclear on this logic:

 e -> do
    liftIO $ acceptLogin cookieCfg jwtCfg (UserId 12) >>= \case
      Nothing -> pure $ noHeader $ noHeader ([qc|Unable to create the session cookie. Something went wrong.|])
Just applyCookies -> pure $ applyCookies ([qc|No user faund due to {e}. User ID 12 should be automatically logged in now.|])

You are case-ing on a not-yet-logged-in request, then automatically authenticating the user as UserId 12, but also returning a response saying they're not logged in?

jkarni commented 6 years ago

Also, if you want the user to be logged out you'll have to explicitly do that. The browser will otherwise keep the cookie.

Domen Kožar notifications@github.com schrieb am Do., 14. Juni 2018, 11:29:

Made it compile in https://gist.github.com/domenkozar/120a36788706c5f49fd6bcc6fff2f3bc - I think there's something wrong with XSRF, I'd try upgrading to servant-auth master but ran out of time, will check over weekend.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/haskell-servant/servant-auth/issues/97#issuecomment-397232249, or mute the thread https://github.com/notifications/unsubscribe-auth/ABlKmk8CnBr8g3c6edj3sHfJLCwrrYnsks5t8iztgaJpZM4UhprJ .

saurabhnanda commented 6 years ago

What happens if you set cookieXsrfSetting = Nothing (in CookieSettings)?

I'm using https://hackage.haskell.org/package/servant-auth-server-0.3.2.0/docs/Servant-Auth-Server.html#t:CookieSettings which doesn't have this field. I've tried setting xsrfExcludeGet=False, but even that doesn't have any impact. Should I be using some dev branch?

The request headers seem like they were cut-off in your image - are you having your javascript send the X-XSRF-TOKEN as mentioned in the README?

Nope! I missed this part. Btw, there's some XSRF cookie as well which is being sent to the server. Is it possible to include another state for a missing/tampered XSRF in https://hackage.haskell.org/package/servant-auth-server-0.3.2.0/docs/Servant-Auth-Server.html#t:AuthResult ?

Also I'm a little unclear on this logic. You are case-ing on a not-yet-logged-in request, then automatically authenticating the user as UserId 12, but also returning a response saying they're not logged in?

This is just throw-away code to toggle the logged-in/logged-out state when the user visits /login

Also, if you want the user to be logged out you'll have to explicitly do that. The browser will otherwise keep the cookie.

Yes, that's a TODO at my end. It would be a good idea to add a logout function on lines of acceptLogin function that works off the same CookieSettings & JWTSettings

Just checking -- this is supposed to the future auth API that servant is moving towards, right?

saurabhnanda commented 6 years ago

I tried switching to master on LTS-11.12, but failed with the following error:

Error: While constructing the build plan, the following exceptions were encountered:

       In the dependencies for servant-auth-server-0.3.3.0:
           http-api-data-0.3.7.2 must match >=0.3.8 && <0.4 (latest applicable is 0.3.8.1)
       needed due to chaudhary-0.1.0.0 -> servant-auth-server-0.3.3.0

(proceeding with hard-coding http-api-data-0.3.8.1 -- hope nothing bad happens!)

saurabhnanda commented 6 years ago

@jkarni @domenkozar Got master to work, turned off XSRF and auth is working as expected now. Thanks for the help!

One feedback on the API -- does every authenticated endpoint now need to pattern-match on AuthResult and remember to redirect to /signin if the auth failed? In the older AuthProtect API, it was possible to put this logic in a central place.

domenkozar commented 6 years ago

I have confirmed currently implementation works with:

$ curl -v -b cookiejar -c cookiejar http://localhost:8000/login
*   Trying ::1...
* TCP_NODELAY set
* connect to ::1 port 8000 failed: Connection refused
*   Trying 127.0.0.1...
* TCP_NODELAY set
* Connected to localhost (127.0.0.1) port 8000 (#0)
> GET /login HTTP/1.1
> Host: localhost:8000
> User-Agent: curl/7.59.0
> Accept: */*
> Cookie: JWT-Cookie=eyJhbGciOiJIUzUxMiJ9.eyJkYXQiOjEyfQ.5Nou1YlygJIRucfpKW4-UAsJkLySLWMrp3QK2QgB-JHyREC8tHsZ8hcxmssvkz-2ch1QzXXDNYsrds7aLWwK5A; XSRF-TOKEN=iGVdA3dJJ4fJfAo1jLFCONKoEpFN1QmQYWvq9+JAfyI=
> 
< HTTP/1.1 200 OK
< Transfer-Encoding: chunked
< Date: Sun, 17 Jun 2018 14:24:39 GMT
< Server: Warp/3.2.22
< Content-Type: text/plain;charset=utf-8
* Replaced cookie XSRF-TOKEN="a0CdDJ9LC/RWNLZuDGp+nAsvmMnduY1RUr+N+xIWQcw=" for domain localhost, path /, expire 0
< Set-Cookie: XSRF-TOKEN=a0CdDJ9LC/RWNLZuDGp+nAsvmMnduY1RUr+N+xIWQcw=; Path=/
* Replaced cookie JWT-Cookie="eyJhbGciOiJIUzUxMiJ9.eyJkYXQiOjEyfQ.5Nou1YlygJIRucfpKW4-UAsJkLySLWMrp3QK2QgB-JHyREC8tHsZ8hcxmssvkz-2ch1QzXXDNYsrds7aLWwK5A" for domain localhost, path /, expire 0
< Set-Cookie: JWT-Cookie=eyJhbGciOiJIUzUxMiJ9.eyJkYXQiOjEyfQ.5Nou1YlygJIRucfpKW4-UAsJkLySLWMrp3QK2QgB-JHyREC8tHsZ8hcxmssvkz-2ch1QzXXDNYsrds7aLWwK5A; Path=/; HttpOnly
* Replaced cookie XSRF-TOKEN="808W0Z05dFpGyplSP/rBlnKrv4U0UzvmaOygPtjLI7s=" for domain localhost, path /, expire 0
< Set-Cookie: XSRF-TOKEN=808W0Z05dFpGyplSP/rBlnKrv4U0UzvmaOygPtjLI7s=; Path=/
< 
* Connection #0 to host localhost left intact
No user faund due to {e}. User ID 12 should be automatically logged in now.

$ curl -v -H "X-XSRF-TOKEN: 808W0Z05dFpGyplSP/rBlnKrv4U0UzvmaOygPtjLI7s=" -b cookiejar -c cookiejar http://localhost:8000/login
*   Trying ::1...
* TCP_NODELAY set
* connect to ::1 port 8000 failed: Connection refused
*   Trying 127.0.0.1...
* TCP_NODELAY set
* Connected to localhost (127.0.0.1) port 8000 (#0)
> GET /login HTTP/1.1
> Host: localhost:8000
> User-Agent: curl/7.59.0
> Accept: */*
> Cookie: JWT-Cookie=eyJhbGciOiJIUzUxMiJ9.eyJkYXQiOjEyfQ.5Nou1YlygJIRucfpKW4-UAsJkLySLWMrp3QK2QgB-JHyREC8tHsZ8hcxmssvkz-2ch1QzXXDNYsrds7aLWwK5A; XSRF-TOKEN=808W0Z05dFpGyplSP/rBlnKrv4U0UzvmaOygPtjLI7s=
> X-XSRF-TOKEN: 808W0Z05dFpGyplSP/rBlnKrv4U0UzvmaOygPtjLI7s=
> 
< HTTP/1.1 200 OK
< Transfer-Encoding: chunked
< Date: Sun, 17 Jun 2018 14:24:53 GMT
< Server: Warp/3.2.22
< Content-Type: text/plain;charset=utf-8
* Replaced cookie XSRF-TOKEN="yZM+Ut2NT3cbVj+srKJ8VlWQwEB5V15SSCcaHqV6z2E=" for domain localhost, path /, expire 0
< Set-Cookie: XSRF-TOKEN=yZM+Ut2NT3cbVj+srKJ8VlWQwEB5V15SSCcaHqV6z2E=; Path=/
* Replaced cookie JWT-Cookie="eyJhbGciOiJIUzUxMiJ9.eyJkYXQiOjEyfQ.5Nou1YlygJIRucfpKW4-UAsJkLySLWMrp3QK2QgB-JHyREC8tHsZ8hcxmssvkz-2ch1QzXXDNYsrds7aLWwK5A" for domain localhost, path /, expire 0
< Set-Cookie: JWT-Cookie=eyJhbGciOiJIUzUxMiJ9.eyJkYXQiOjEyfQ.5Nou1YlygJIRucfpKW4-UAsJkLySLWMrp3QK2QgB-JHyREC8tHsZ8hcxmssvkz-2ch1QzXXDNYsrds7aLWwK5A; Path=/; HttpOnly
< 
* Connection #0 to host localhost left intact
Found auth user UserId 12, who should now be logged out

However, I think we should improve docs.