Merge pull request #271 from begriffs/surprise-404

Allow continued auth access after db errors

CHANGELOG.md

@@ -13,6 +13,7 @@ This project adheres to [Semantic Versioning](http://semver.org/).
 - Allow filters by computed columns - @diogob
 
 ### Fixed
+- Reset user role on error
 - Compatible with Stack
 - Add materialized views to results in GET / - @diogob
 - Indicate insertable=true for views that are insertable through triggers - @diogob

src/PostgREST/Auth.hs

@@ -53,14 +53,11 @@ checkPass :: Text -> Text -> Bool
 checkPass = (. cs) . validatePassword . cs
 
 setRole :: Text -> H.Tx P.Postgres s ()
-setRole role = H.unitEx $ B.Stmt ("set role " <> cs (pgFmtLit role)) V.empty True
+setRole role = H.unitEx $ B.Stmt ("set local role " <> cs (pgFmtLit role)) V.empty True
-
-resetRole :: H.Tx P.Postgres s ()
-resetRole = H.unitEx [H.stmt|reset role|]
 
 setUserId :: Text -> H.Tx P.Postgres s ()
 setUserId uid = if uid /= "" then
-  H.unitEx $ B.Stmt ("set user_vars.user_id = " <> cs (pgFmtLit uid)) V.empty True
+  H.unitEx $ B.Stmt ("set local user_vars.user_id = " <> cs (pgFmtLit uid)) V.empty True
 else
   resetUserId
 
@@ -90,15 +87,15 @@ signInWithJWT secret input = case maybeRole of
       Just (Just (String uid)) -> LoginSuccess (cs role) (cs uid)
       _   -> LoginFailed
     _   -> LoginFailed
-  where 
+  where
     maybeRole = (Data.Map.lookup "role" <$> claims) ::Maybe (Maybe Value)
     maybeUserId = (Data.Map.lookup "id" <$> claims) ::Maybe (Maybe Value)
     claims = JWT.unregisteredClaims <$> JWT.claims <$> decoded
     decoded = JWT.decodeAndVerifySignature (JWT.secret secret) input
-    
+
 tokenJWT :: Text -> Text -> Text -> Text
 tokenJWT secret uid role = JWT.encodeSigned JWT.HS256 (JWT.secret secret) claimsSet
   where
     claimsSet = JWT.def {
       JWT.unregisteredClaims = Data.Map.fromList [("id", String uid), ("role", String role)]
-    }
+    }

src/PostgREST/Main.hs

@@ -73,7 +73,7 @@ main = do
 
   runSettings appSettings $ middle $ \req respond -> do
     body <- strictRequestBody req
-    resOrError <- liftIO $ H.session pool $ H.tx Nothing $
+    resOrError <- liftIO $ H.session pool $ H.tx (Just (H.ReadCommitted, Just True)) $
       authenticated conf (app conf body) req
     either (respond . errResponse) respond resOrError
 

src/PostgREST/Middleware.hs

@@ -23,7 +23,7 @@ import Network.Wai.Middleware.Static (staticPolicy, only)
 import Network.URI (URI(..), parseURI)
 
 import PostgREST.Config (AppConfig(..), corsPolicy)
-import PostgREST.Auth (LoginAttempt(..), signInRole, signInWithJWT, setRole, resetRole, setUserId, resetUserId)
+import PostgREST.Auth (LoginAttempt(..), signInRole, signInWithJWT, setRole, setUserId)
 import PostgREST.App (contentTypeForAccept)
 import Codec.Binary.Base64.String (decode)
 
@@ -62,10 +62,7 @@ authenticated conf app req = do
    runInRole r uid = do
      setUserId uid
      setRole r
-     res <- app req
+     app req
-     resetRole
-     resetUserId
-     return res
 
 
 redirectInsecure :: Application -> Application
@@ -93,7 +90,7 @@ unsupportedAccept :: Application -> Application
 unsupportedAccept app req respond = do
   let
     accept = lookup hAccept $ requestHeaders req
-  if isNothing $ contentTypeForAccept accept 
+  if isNothing $ contentTypeForAccept accept
   then respond $ responseLBS status415 [] "Unsupported Accept header, try: application/json"
   else app req respond
 

test/Feature/AuthSpec.hs

@@ -28,19 +28,26 @@ spec = beforeAll
     let auth = authHeaderBasic "jdoe" "1234"
     request methodGet "/authors_only" [auth] ""
       `shouldRespondWith` 200
-      
+
+  it "recovers after 400 error with logged in user" $ do
+    _ <- post "/postgrest/users" [json| { "id":"jdoe", "pass": "1234", "role": "postgrest_test_author" } |]
+    let auth = authHeaderBasic "jdoe" "1234"
+    _ <- request methodPost "/rpc/problem" [auth] ""
+    request methodGet "/authors_only" [auth] ""
+      `shouldRespondWith` 200
+
   it "allows users to login (JWT)" $ do
     _ <- post "/postgrest/users" [json| { "id":"jdoe", "pass": "1234", "role": "postgrest_test_author" } |]
-    post "/postgrest/tokens" [json| { "id":"jdoe", "pass": "1234" } |] 
+    post "/postgrest/tokens" [json| { "id":"jdoe", "pass": "1234" } |]
       `shouldRespondWith` ResponseMatcher {
           matchBody = Just [json| {"token":"eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJyb2xlIjoicG9zdGdyZXN0X3Rlc3RfYXV0aG9yIiwiaWQiOiJqZG9lIn0.y4vZuu1dDdwAl0-S00MCRWRYMlJ5YAMSir6Es6WtWx0"} |]
         , matchStatus = 201
         , matchHeaders = ["Content-Type" <:> "application/json"]
         }
-      
+
   it "indicates login failure (JWT)" $ do
     _ <- post "/postgrest/users" [json| { "id":"jdoe", "pass": "1234", "role": "postgrest_test_author" } |]
-    post "/postgrest/tokens" [json| { "id":"jdoe", "pass": "NOPE" } |] 
+    post "/postgrest/tokens" [json| { "id":"jdoe", "pass": "NOPE" } |]
       `shouldRespondWith` ResponseMatcher {
           matchBody = Just [json| {"message":"Failed authentication."} |]
         , matchStatus = 401
@@ -51,4 +58,4 @@ spec = beforeAll
     _ <- post "/postgrest/users" [json| { "id":"jdoe", "pass": "1234", "role": "postgrest_test_author" } |]
     let auth = authHeaderJWT "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJyb2xlIjoicG9zdGdyZXN0X3Rlc3RfYXV0aG9yIiwiaWQiOiJqZG9lIn0.y4vZuu1dDdwAl0-S00MCRWRYMlJ5YAMSir6Es6WtWx0"
     request methodGet "/authors_only" [auth] ""
-      `shouldRespondWith` 200
+      `shouldRespondWith` 200

test/SpecHelper.hs

@@ -54,7 +54,7 @@ withApp perform = do
 
   perform $ middle $ \req resp -> do
     body <- strictRequestBody req
-    result <- liftIO $ H.session pool $ H.tx Nothing
+    result <- liftIO $ H.session pool $ H.tx (Just (H.ReadCommitted, Just True))
       $ authenticated cfg (app cfg body) req
     either (resp . errResponse) resp result
 

test/fixtures/schema.sql

@@ -228,6 +228,14 @@ CREATE FUNCTION "1".sayhello(name text) RETURNS text AS $$
 $$ LANGUAGE SQL;
 
 
+CREATE FUNCTION "1".problem() RETURNS void LANGUAGE plpgsql AS
+$$
+BEGIN
+      RAISE 'bad thing';
+END;
+$$;
+
+
 CREATE TABLE menagerie (
     "integer" integer NOT NULL,
     double double precision NOT NULL,
@@ -544,6 +552,11 @@ GRANT EXECUTE ON FUNCTION sayhello(text) TO postgrest_test;
 GRANT EXECUTE ON FUNCTION sayhello(text) TO postgrest_anonymous;
 
 
+REVOKE ALL ON FUNCTION problem() FROM PUBLIC;
+REVOKE ALL ON FUNCTION problem() FROM postgrest_test_author;
+GRANT EXECUTE ON FUNCTION problem() TO postgrest_test_author;
+
+
 REVOKE ALL ON SEQUENCE items_id_seq FROM PUBLIC;
 REVOKE ALL ON SEQUENCE items_id_seq FROM postgrest_test;
 GRANT ALL ON SEQUENCE items_id_seq TO postgrest_test;