From 62ed9e2c4dd640d63764f71f99424f910408f655 Mon Sep 17 00:00:00 2001 From: Joe Nelson Date: Sun, 4 Sep 2016 01:38:01 -0700 Subject: [PATCH] Do not require jwt secret, but die on auth without it --- main/Main.hs | 4 ++-- postgrest.cabal | 1 + src/PostgREST/App.hs | 21 +++++++++++++-------- src/PostgREST/Auth.hs | 16 ++++++++++------ src/PostgREST/Config.hs | 4 ++-- src/PostgREST/Middleware.hs | 4 +++- test/Feature/NoJwtSpec.hs | 24 ++++++++++++++++++++++++ test/Main.hs | 6 ++++++ test/SpecHelper.hs | 12 ++++++++---- 9 files changed, 69 insertions(+), 23 deletions(-) create mode 100644 test/Feature/NoJwtSpec.hs diff --git a/main/Main.hs b/main/Main.hs index 904a10d99..938b6d5ab 100644 --- a/main/Main.hs +++ b/main/Main.hs @@ -88,7 +88,7 @@ main = do loadSecretFile :: AppConfig -> IO AppConfig loadSecretFile conf = do let s = configJwtSecret conf - real <- case stripPrefix "@" s of + real <- case join (stripPrefix "@" <$> s) of Nothing -> return s -- the string is the secret, not a filename - Just filename -> readFile (toS filename) + Just filename -> sequence . Just $ readFile (toS filename) return conf { configJwtSecret = real } diff --git a/postgrest.cabal b/postgrest.cabal index 6b2774b9c..8619ee999 100644 --- a/postgrest.cabal +++ b/postgrest.cabal @@ -143,6 +143,7 @@ Test-Suite spec , Feature.CorsSpec , Feature.DeleteSpec , Feature.InsertSpec + , Feature.NoJwtSpec , Feature.QuerySpec , Feature.QueryLimitedSpec , Feature.RangeSpec diff --git a/src/PostgREST/App.hs b/src/PostgREST/App.hs index 63244e885..e258f2f88 100644 --- a/src/PostgREST/App.hs +++ b/src/PostgREST/App.hs @@ -80,7 +80,8 @@ postgrest conf refDbStructure pool = let schema = toS $ configSchema conf apiRequest = userApiRequest schema req body - eClaims = jwtClaims (secret $ configJwtSecret conf) (iJWT apiRequest) time + eClaims = jwtClaims + (secret <$> configJwtSecret conf) (iJWT apiRequest) time authed = containsRole eClaims handleReq = runWithClaims conf eClaims (app dbStructure conf) apiRequest txMode = transactionMode $ iAction apiRequest @@ -212,7 +213,7 @@ app dbStructure conf apiRequest = Just (PayloadJSON (UniformObjects payload))) -> do let p = V.head payload singular = iPreferSingular apiRequest - jwtSecret = secret $ configJwtSecret conf + jwtSecret = secret <$> configJwtSecret conf returnType = lookup (qiName qi) $ dbProcs dbStructure returnsJWT = fromMaybe False $ isInfixOf "jwt_claims" . pdReturnType <$> returnType @@ -220,13 +221,17 @@ app dbStructure conf apiRequest = Left e -> return $ responseLBS status400 [jsonH] $ toS e Right (q,cq) -> respondToRange $ do row <- H.query () (callProc qi p q cq topLevelRange shouldCount singular) - let (tableTotal, queryTotal, body) = fromMaybe (Just 0, 0, emptyArray) row + let (tableTotal, queryTotal, body) = + fromMaybe (Just 0, 0, emptyArray) row (status, contentRange) = rangeHeader queryTotal tableTotal - in - return $ responseLBS status [jsonH, contentRange] - (if returnsJWT - then "{\"token\":\"" <> toS (tokenJWT jwtSecret body) <> "\"}" - else toS $ encode body) + return $ case (returnsJWT, jwtSecret) of + (True, Nothing) -> + errResponse status500 "Server lacks JWT secret" + (True, Just s) -> + responseLBS status [jsonH, contentRange] $ + "{\"token\":\"" <> toS (tokenJWT s body) <> "\"}" + (False, _) -> + responseLBS status [jsonH, contentRange] (toS . encode $ body) (ActionRead, TargetRoot, Nothing) -> do let host = configHost conf diff --git a/src/PostgREST/Auth.hs b/src/PostgREST/Auth.hs index 44ca75d1c..8ea9a3f9b 100644 --- a/src/PostgREST/Auth.hs +++ b/src/PostgREST/Auth.hs @@ -52,6 +52,7 @@ claimsToSQL claims = roleStmts <> varStmts -} data JWTAttempt = JWTExpired | JWTInvalid + | JWTMissingSecret | JWTClaims (M.HashMap Text Value) deriving Eq @@ -59,18 +60,21 @@ data JWTAttempt = JWTExpired Receives the JWT secret (from config) and a JWT and returns a map of JWT claims. -} -jwtClaims :: JWT.Secret -> Text -> NominalDiffTime -> JWTAttempt +jwtClaims :: Maybe JWT.Secret -> Text -> NominalDiffTime -> JWTAttempt jwtClaims _ "" _ = JWTClaims M.empty jwtClaims secret jwt time = - case isExpired <$> mClaims of - Just True -> JWTExpired - Nothing -> JWTInvalid - Just False -> JWTClaims $ value2map $ fromJust mClaims + case secret of + Nothing -> JWTMissingSecret + Just s -> + let mClaims = toJSON . JWT.claims <$> JWT.decodeAndVerifySignature s jwt in + case isExpired <$> mClaims of + Just True -> JWTExpired + Nothing -> JWTInvalid + Just False -> JWTClaims $ value2map $ fromJust mClaims where isExpired claims = let mExp = claims ^? key "exp" . _Integer in fromMaybe False $ (<= time) . fromInteger <$> mExp - mClaims = toJSON . JWT.claims <$> JWT.decodeAndVerifySignature secret jwt value2map (Object o) = o value2map _ = M.empty diff --git a/src/PostgREST/Config.hs b/src/PostgREST/Config.hs index 9e681fcdc..9e07d8c41 100644 --- a/src/PostgREST/Config.hs +++ b/src/PostgREST/Config.hs @@ -40,7 +40,7 @@ data AppConfig = AppConfig { , configSchema :: Text , configHost :: Text , configPort :: Int - , configJwtSecret :: Text + , configJwtSecret :: Maybe Text , configPool :: Int , configMaxRows :: Maybe Integer , configQuiet :: Bool @@ -54,7 +54,7 @@ argParser = AppConfig <*> (toS <$> strOption (long "schema" <> short 's' <> help "schema to use for API routes" <> metavar "NAME" <> value "public" <> showDefault)) <*> (toS <$> strOption (long "host" <> short 'l' <> help "hostname or ip on which to run HTTP server" <> metavar "HOST" <> value "*4" <> showDefault)) <*> option auto (long "port" <> short 'p' <> help "port number on which to run HTTP server" <> metavar "PORT" <> value 3000 <> showDefault) - <*> (toS <$> strOption (long "jwt-secret" <> short 'j' <> help "secret used to encrypt and decrypt JWT tokens" <> metavar "SECRET")) + <*> (optional . map toS <$> strOption) (long "jwt-secret" <> short 'j' <> help "secret used to encrypt and decrypt JWT tokens" <> metavar "SECRET") <*> option auto (long "pool" <> short 'o' <> help "max connections in database pool" <> metavar "COUNT" <> value 10 <> showDefault) <*> (readMay <$> strOption (long "max-rows" <> short 'm' <> help "max rows in response" <> metavar "COUNT" <> value "infinity" <> showDefault)) <*> pure False diff --git a/src/PostgREST/Middleware.hs b/src/PostgREST/Middleware.hs index 801991040..9ed6b1227 100644 --- a/src/PostgREST/Middleware.hs +++ b/src/PostgREST/Middleware.hs @@ -7,7 +7,7 @@ import Data.Aeson (Value (..)) import qualified Data.HashMap.Strict as M import qualified Hasql.Transaction as H -import Network.HTTP.Types.Status (unauthorized401) +import Network.HTTP.Types.Status (unauthorized401, status500) import Network.Wai (Application, Response, responseLBS) import Network.Wai.Middleware.Cors (cors) @@ -18,6 +18,7 @@ import PostgREST.ApiRequest (ApiRequest(..), ContentType(..), ctToHeader) import PostgREST.Auth (claimsToSQL, JWTAttempt(..)) import PostgREST.Config (AppConfig (..), corsPolicy) +import PostgREST.Error (errResponse) import Protolude hiding (concat, null) @@ -28,6 +29,7 @@ runWithClaims conf eClaims app req = case eClaims of JWTExpired -> return $ unauthed "JWT expired" JWTInvalid -> return $ unauthed "JWT invalid" + JWTMissingSecret -> return $ errResponse status500 "Server lacks JWT secret" JWTClaims claims -> do -- role claim defaults to anon if not specified in jwt H.sql . mconcat . claimsToSQL $ M.union claims (M.singleton "role" anon) diff --git a/test/Feature/NoJwtSpec.hs b/test/Feature/NoJwtSpec.hs new file mode 100644 index 000000000..3cc09b643 --- /dev/null +++ b/test/Feature/NoJwtSpec.hs @@ -0,0 +1,24 @@ +module Feature.NoJwtSpec where + +-- {{{ Imports +import Test.Hspec +import Test.Hspec.Wai +import Test.Hspec.Wai.JSON +import Network.HTTP.Types + +import SpecHelper +import Network.Wai (Application) +-- }}} + +spec :: SpecWith Application +spec = describe "server started without JWT secret" $ do + + -- this test will stop working 9999999999s after the UNIX EPOCH + it "responds with error on attempted auth" $ do + let auth = authHeaderJWT "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJleHAiOjk5OTk5OTk5OTksInJvbGUiOiJwb3N0Z3Jlc3RfdGVzdF9hdXRob3IiLCJpZCI6Impkb2UifQ.QaPPLWTuyydMu_q7H4noMT7Lk6P4muet1OpJXF6ofhc" + request methodGet "/authors_only" [auth] "" + `shouldRespondWith` 500 + + it "responds with error when attempting to generate JWT token" $ + post "/rpc/login" [json| { "id": "jdoe", "pass": "1234" } |] + `shouldRespondWith` 500 diff --git a/test/Main.hs b/test/Main.hs index 7be28d525..6334ac88a 100644 --- a/test/Main.hs +++ b/test/Main.hs @@ -15,6 +15,7 @@ import qualified Feature.ConcurrentSpec import qualified Feature.CorsSpec import qualified Feature.DeleteSpec import qualified Feature.InsertSpec +import qualified Feature.NoJwtSpec import qualified Feature.QueryLimitedSpec import qualified Feature.QuerySpec import qualified Feature.RangeSpec @@ -34,6 +35,7 @@ main = do ltdApp = return $ postgrest testLtdRowsCfg refDbStructure pool unicodeApp = return $ postgrest testUnicodeCfg refDbStructure pool proxyApp = return $ postgrest testProxyCfg refDbStructure pool + noJwtApp = return $ postgrest testCfgNoJWT refDbStructure pool hspec $ do mapM_ (beforeAll_ resetDb . before withApp) specs @@ -50,6 +52,10 @@ main = do beforeAll_ resetDb . before proxyApp $ describe "Feature.ProxySpec" Feature.ProxySpec.spec + -- this test runs without a JWT secret + beforeAll_ resetDb . before noJwtApp $ + describe "Feature.NoJwtSpec" Feature.NoJwtSpec.spec + where specs = map (uncurry describe) [ ("Feature.AuthSpec" , Feature.AuthSpec.spec) diff --git a/test/SpecHelper.hs b/test/SpecHelper.hs index ffedb8d05..ed79a9bd8 100644 --- a/test/SpecHelper.hs +++ b/test/SpecHelper.hs @@ -51,19 +51,23 @@ testDbConn = "postgres://postgrest_test_authenticator@localhost:5432/postgrest_t testCfg :: AppConfig testCfg = - AppConfig testDbConn "postgrest_test_anonymous" Nothing "test" "localhost" 3000 "safe" 10 Nothing True + AppConfig testDbConn "postgrest_test_anonymous" Nothing "test" "localhost" 3000 (Just "safe") 10 Nothing True + +testCfgNoJWT :: AppConfig +testCfgNoJWT = + AppConfig testDbConn "postgrest_test_anonymous" Nothing "test" "localhost" 3000 Nothing 10 Nothing True testUnicodeCfg :: AppConfig testUnicodeCfg = - AppConfig testDbConn "postgrest_test_anonymous" Nothing "تست" "localhost" 3000 "safe" 10 Nothing True + AppConfig testDbConn "postgrest_test_anonymous" Nothing "تست" "localhost" 3000 (Just "safe") 10 Nothing True testLtdRowsCfg :: AppConfig testLtdRowsCfg = - AppConfig testDbConn "postgrest_test_anonymous" Nothing "test" "localhost" 3000 "safe" 10 (Just 2) True + AppConfig testDbConn "postgrest_test_anonymous" Nothing "test" "localhost" 3000 (Just "safe") 10 (Just 2) True testProxyCfg :: AppConfig testProxyCfg = - AppConfig testDbConn "postgrest_test_anonymous" (Just "https://postgrest.com/openapi.json") "test" "localhost" 3000 "safe" 10 Nothing True + AppConfig testDbConn "postgrest_test_anonymous" (Just "https://postgrest.com/openapi.json") "test" "localhost" 3000 (Just "safe") 10 Nothing True setupDb :: IO () setupDb = do