fix: remove caching empty JWT in Auth header

CHANGELOG.md

@@ -28,6 +28,7 @@ This project adheres to [Semantic Versioning](http://semver.org/).
  - #3727, Clarify "listening" logs - @steve-chavez
  - #3795, Clarify `Accept: vnd.pgrst.object` error message - @steve-chavez
  - #3697, #3602, Handle queries on non-existing table gracefully - @taimoorzaeem
+ - #3926, Remove caching empty JWT in Auth header - @taimoorzaeem
 
 ### Changed
 

postgrest.cabal

@@ -207,6 +207,7 @@ test-suite spec
                       Feature.Auth.AudienceJwtSecretSpec
                       Feature.Auth.AuthSpec
                       Feature.Auth.BinaryJwtSecretSpec
+                      Feature.Auth.JwtCacheSpec
                       Feature.Auth.NoAnonSpec
                       Feature.Auth.NoJwtSpec
                       Feature.ConcurrentSpec

src/PostgREST/Auth.hs

@@ -54,9 +54,9 @@ import Protolude
 
 -- | Receives the JWT secret and audience (from config) and a JWT and returns a
 -- JSON object of JWT claims.
-parseToken :: AppConfig -> ByteString -> UTCTime -> ExceptT Error IO JSON.Value
-parseToken _ "" _ = return JSON.emptyObject
-parseToken AppConfig{..} token time = do
+parseToken :: AppConfig -> Maybe ByteString -> UTCTime -> ExceptT Error IO JSON.Value
+parseToken _ Nothing _ = return JSON.emptyObject
+parseToken AppConfig{..} (Just token) time = do
   secret <- liftEither . maybeToRight JwtTokenMissing $ configJWKS
   eitherContent <- liftIO $ JWT.decode (JWT.keys secret) Nothing token
   content <- liftEither . mapLeft jwtDecodeError $ eitherContent
@@ -148,7 +148,7 @@ middleware appState app req respond = do
   conf <- getConfig appState
   time <- getTime appState
 
-  let token  = fromMaybe "" $ Wai.extractBearerAuth =<< lookup HTTP.hAuthorization (Wai.requestHeaders req)
+  let token = Wai.extractBearerAuth =<< lookup HTTP.hAuthorization (Wai.requestHeaders req)
       parseJwt = runExceptT $ parseToken conf token time >>= parseClaims conf
       jwtCacheState = getJwtCacheState appState
 
@@ -160,15 +160,19 @@ middleware appState app req respond = do
           return $ req { Wai.vault = Wai.vault req & Vault.insert authResultKey authResult & Vault.insert jwtDurKey dur }
 
     (True, maxLifetime)  -> do
-          (dur, authResult) <- timeItT $ lookupJwtCache jwtCacheState token maxLifetime parseJwt time
+          (dur, authResult) <- timeItT $ case token of
+              Nothing -> parseJwt
+              Just tok -> lookupJwtCache jwtCacheState tok maxLifetime parseJwt time
           return $ req { Wai.vault = Wai.vault req & Vault.insert authResultKey authResult & Vault.insert jwtDurKey dur }
 
     (False, 0)           -> do
           authResult <- parseJwt
           return $ req { Wai.vault = Wai.vault req & Vault.insert authResultKey authResult }
 
     (False, maxLifetime) -> do
-          authResult <- lookupJwtCache jwtCacheState token maxLifetime parseJwt time
+          authResult <- case token of
+            Nothing -> parseJwt
+            Just tok -> lookupJwtCache jwtCacheState tok maxLifetime parseJwt time
           return $ req { Wai.vault = Wai.vault req & Vault.insert authResultKey authResult }
 
   app req' respond

test/spec/Feature/Auth/AuthSpec.hs

@@ -159,3 +159,14 @@ spec = describe "authorization" $ do
       `shouldRespondWith` 200
     request methodGet "/authors_only" [authHeader "bearer" token] ""
       `shouldRespondWith` 200
+
+  describe "when JWT is empty string" $ do
+
+    it "raises error on empty token in Auth header" $ do
+      let auth = authHeaderJWT ""
+      request methodGet "/authors_only"
+        [auth]
+        ""
+        `shouldRespondWith`
+        [json| {"code":"PGRST301","details":null,"hint":null,"message":"JWSError (CompactDecodeError Invalid number of parts: Expected 3 parts; got 2)"} |]
+        { matchStatus = 401 }

test/spec/Feature/Auth/JwtCacheSpec.hs

@@ -0,0 +1,22 @@
+module Feature.Auth.JwtCacheSpec where
+
+import Network.Wai (Application)
+
+import Test.Hspec
+import Test.Hspec.Wai
+import Test.Hspec.Wai.JSON
+
+import Protolude hiding (get)
+
+spec :: SpecWith ((), Application)
+spec = describe "jwt cache enabled" $ do
+  it "denies access to tables that anonymous does not own" $
+    get "/authors_only" `shouldRespondWith`
+      [json| {
+        "hint":null,
+        "details":null,
+        "code":"42501",
+        "message":"permission denied for table authors_only"} |]
+      { matchStatus = 401
+      , matchHeaders = ["WWW-Authenticate" <:> "Bearer"]
+      }

test/spec/Main.hs

@@ -24,6 +24,7 @@ import qualified Feature.Auth.AsymmetricJwtSpec
 import qualified Feature.Auth.AudienceJwtSecretSpec
 import qualified Feature.Auth.AuthSpec
 import qualified Feature.Auth.BinaryJwtSecretSpec
+import qualified Feature.Auth.JwtCacheSpec
 import qualified Feature.Auth.NoAnonSpec
 import qualified Feature.Auth.NoJwtSpec
 import qualified Feature.ConcurrentSpec
@@ -124,6 +125,8 @@ main = do
       obsApp               = app testObservabilityCfg
       serverTiming         = app testCfgServerTiming
       aggregatesEnabled    = app testCfgAggregatesEnabled
+      jwtCacheWithServerTiming = app testCfgJwtCacheServerTiming
+      jwtCacheWithoutServerTiming = app testCfgJwtCacheNoServerTiming
 
       extraSearchPathApp   = appDbs testCfgExtraSearchPath
       unicodeApp           = appDbs testUnicodeCfg
@@ -221,6 +224,14 @@ main = do
     parallel $ before asymJwkSetApp $
       describe "Feature.Auth.AsymmetricJwtSpec" Feature.Auth.AsymmetricJwtSpec.spec
 
+    -- this test jwt cache with server timing enabled
+    parallel $ before jwtCacheWithServerTiming $
+      describe "Feature.Auth.JwtCacheSpec" Feature.Auth.JwtCacheSpec.spec
+
+    -- this test jwt cache with server timing disabled
+    parallel $ before jwtCacheWithoutServerTiming $
+      describe "Feature.Auth.JwtCacheSpec" Feature.Auth.JwtCacheSpec.spec
+
     -- this test runs with an extra search path
     parallel $ before extraSearchPathApp $ do
       describe "Feature.ExtraSearchPathSpec" Feature.ExtraSearchPathSpec.spec

test/spec/SpecHelper.hs

@@ -252,6 +252,12 @@ testCfgServerTiming = baseCfg { configDbPlanEnabled = True }
 testCfgAggregatesEnabled :: AppConfig
 testCfgAggregatesEnabled = baseCfg { configDbAggregates = True }
 
+testCfgJwtCacheServerTiming :: AppConfig
+testCfgJwtCacheServerTiming = baseCfg { configServerTimingEnabled = True, configJwtCacheMaxLifetime = 86400 }
+
+testCfgJwtCacheNoServerTiming :: AppConfig
+testCfgJwtCacheNoServerTiming = baseCfg { configServerTimingEnabled = False, configJwtCacheMaxLifetime = 86400 }
+
 analyzeTable :: Text -> IO ()
 analyzeTable tableName =
   void $ readProcess "psql" ["-U", "postgres", "--set", "ON_ERROR_STOP=1", "-a", "-c", toS $ "ANALYZE test.\"" <> tableName <> "\""] []