fix: Validate aud is a valid URI even if jwt-aud is not configured

* Introduced StringOrURI newtype and its validating FromJSON implementation in Auth.JWT
* Changed JwtError constructor AudClaimNotStringOrArray to AudClaimNotStringOrURIOrArray and modified error message for it
* Added test in AuthSpec
* Modified tests to verify new error message

Left validation of URI in jwt-aud as is.

Author: mkleczek

src/PostgREST/Auth/Jwt.hs

@@ -37,12 +37,13 @@ import PostgREST.Auth.Types (AuthResult (..))
 import PostgREST.Config     (AppConfig (..), FilterExp (..), JSPath,
                              JSPathExp (..), audMatchesCfg)
 import PostgREST.Error      (Error (..),
-                             JwtClaimsError (AudClaimNotStringOrArray, ExpClaimNotNumber, IatClaimNotNumber, JWTExpired, JWTIssuedAtFuture, JWTNotInAudience, JWTNotYetValid, NbfClaimNotNumber, ParsingClaimsFailed),
+                             JwtClaimsError (AudClaimNotStringOrURIOrArray, ExpClaimNotNumber, IatClaimNotNumber, JWTExpired, JWTIssuedAtFuture, JWTNotInAudience, JWTNotYetValid, NbfClaimNotNumber, ParsingClaimsFailed),
                              JwtDecodeError (..), JwtError (..))
 
 import Data.Aeson       ((.:?))
 import Data.Aeson.Types (parseMaybe)
 import Jose.Jwk         (JwkSet)
+import Network.URI      (isURI)
 import Protolude        hiding (first)
 
 parseAndDecodeClaims :: (MonadError Error m, MonadIO m) => JwkSet -> ByteString -> m JSON.Object
@@ -55,7 +56,13 @@ decodeClaims _ = throwError $ JwtErr $ JwtDecodeErr UnsupportedTokenType
 validateClaims :: MonadError Error m => UTCTime -> (Text -> Bool) -> JSON.Object -> m ()
 validateClaims time audMatches claims = liftEither $ maybeToLeft () (fmap JwtErr . getAlt $ JwtClaimsErr <$> checkForErrors time audMatches claims)
 
-data ValidAud = VAString Text | VAArray [Text] deriving Generic
+newtype StringOrURI = StringOrURI { unStringOrURI :: Text }
+instance JSON.FromJSON StringOrURI where
+  parseJSON = fmap StringOrURI . mfilter isValidURI . JSON.parseJSON
+    where
+      isValidURI = (||) <$> not . T.isInfixOf ":" <*> isURI . T.unpack
+
+data ValidAud = VAString StringOrURI | VAArray [StringOrURI] deriving Generic
 instance JSON.FromJSON ValidAud where
   parseJSON = JSON.genericParseJSON JSON.defaultOptions { JSON.sumEncoding = JSON.UntaggedValue }
 
@@ -65,7 +72,7 @@ checkForErrors time audMatches = mconcat
     claim "exp" ExpClaimNotNumber $ inThePast JWTExpired
   , claim "nbf" NbfClaimNotNumber $ inTheFuture JWTNotYetValid
   , claim "iat" IatClaimNotNumber $ inTheFuture JWTIssuedAtFuture
-  , claim "aud" AudClaimNotStringOrArray $ checkValue (not . validAud) JWTNotInAudience
+  , claim "aud" AudClaimNotStringOrURIOrArray $ checkValue (not . validAud) JWTNotInAudience
   ]
   where
       allowedSkewSeconds = 30 :: Int64
@@ -79,8 +86,9 @@ checkForErrors time audMatches = mconcat
       checkTime cond = checkValue (cond. sciToInt)
 
       validAud = \case
-        (VAString aud) -> audMatches aud
-        (VAArray auds) -> null auds || any audMatches auds
+        (VAString aud) -> validAudString aud
+        (VAArray auds) -> null auds || any validAudString auds
+      validAudString = audMatches . unStringOrURI
 
       checkValue invalid msg val =
         if invalid val then

src/PostgREST/Error.hs

@@ -675,7 +675,7 @@ data JwtClaimsError
   | ExpClaimNotNumber
   | NbfClaimNotNumber
   | IatClaimNotNumber
-  | AudClaimNotStringOrArray
+  | AudClaimNotStringOrURIOrArray
   deriving Show
 
 instance PgrstError Error where
@@ -752,15 +752,15 @@ instance ErrorBody JwtError where
     UnreachableDecodeError -> "JWT couldn't be decoded"
   message JwtTokenRequired = "Anonymous access is disabled"
   message (JwtClaimsErr e) = case e of
-    JWTExpired               -> "JWT expired"
-    JWTNotYetValid           -> "JWT not yet valid"
-    JWTIssuedAtFuture        -> "JWT issued at future"
-    JWTNotInAudience         -> "JWT not in audience"
-    ParsingClaimsFailed      -> "Parsing claims failed"
-    ExpClaimNotNumber        -> "The JWT 'exp' claim must be a number"
-    NbfClaimNotNumber        -> "The JWT 'nbf' claim must be a number"
-    IatClaimNotNumber        -> "The JWT 'iat' claim must be a number"
-    AudClaimNotStringOrArray -> "The JWT 'aud' claim must be a string or an array of strings"
+    JWTExpired                    -> "JWT expired"
+    JWTNotYetValid                -> "JWT not yet valid"
+    JWTIssuedAtFuture             -> "JWT issued at future"
+    JWTNotInAudience              -> "JWT not in audience"
+    ParsingClaimsFailed           -> "Parsing claims failed"
+    ExpClaimNotNumber             -> "The JWT 'exp' claim must be a number"
+    NbfClaimNotNumber             -> "The JWT 'nbf' claim must be a number"
+    IatClaimNotNumber             -> "The JWT 'iat' claim must be a number"
+    AudClaimNotStringOrURIOrArray -> "The JWT 'aud' claim must be a string, URI or an array of mixed strings or URIs"
 
   details (JwtDecodeErr jde) = case jde of
     KeyError dets     -> Just $ JSON.String dets

test/spec/Feature/Auth/AudienceJwtSecretSpec.hs

@@ -175,6 +175,21 @@ disabledSpec = describe "test handling of aud claims in JWT when the jwt-aud con
       request methodGet "/authors_only" [auth] ""
         `shouldRespondWith` 200
 
+    it "fails when the audience claim is invalid URI" $ do
+      let jwtPayload = [json|
+            {
+              "exp": 9999999999,
+              "role": "postgrest_test_author",
+              "id": "jdoe",
+              "aud": "http://%%"
+            }|]
+          auth = authHeaderJWT $ generateJWT jwtPayload
+      request methodGet "/authors_only" [auth] ""
+        `shouldRespondWith`
+          [json|{"code":"PGRST303","details":null,"hint":null,"message":"The JWT 'aud' claim must be a string, URI or an array of mixed strings or URIs"}|]
+          { matchStatus = 401 }
+
+
   context "when the audience is an array of strings" $ do
     it "ignores the audience claim and suceeds when it has 1 element" $ do
       let jwtPayload = [json|

test/spec/Feature/Auth/AuthSpec.hs

@@ -173,7 +173,7 @@ spec = describe "authorization" $ do
         [json|{"code":"PGRST303","details":null,"hint":null,"message":"The JWT 'iat' claim must be a number"}|]
         { matchStatus = 401 }
 
-  it "fails when the aud claim has a single value and it's not a string" $ do
+  it "fails when the aud claim has a single value and it's an object" $ do
     let jwtPayload = [json|
           {
             "aud": {"invalid": "value"},
@@ -182,7 +182,7 @@ spec = describe "authorization" $ do
         auth = authHeaderJWT $ generateJWT jwtPayload
     request methodGet "/authors_only" [auth] ""
       `shouldRespondWith`
-        [json|{"code":"PGRST303","details":null,"hint":null,"message":"The JWT 'aud' claim must be a string or an array of strings"}|]
+        [json|{"code":"PGRST303","details":null,"hint":null,"message":"The JWT 'aud' claim must be a string, URI or an array of mixed strings or URIs"}|]
         { matchStatus = 401 }
 
   it "fails when the aud claim is an array but it has non-string elements" $ do
@@ -194,7 +194,7 @@ spec = describe "authorization" $ do
         auth = authHeaderJWT $ generateJWT jwtPayload
     request methodGet "/authors_only" [auth] ""
       `shouldRespondWith`
-        [json|{"code":"PGRST303","details":null,"hint":null,"message":"The JWT 'aud' claim must be a string or an array of strings"}|]
+        [json|{"code":"PGRST303","details":null,"hint":null,"message":"The JWT 'aud' claim must be a string, URI or an array of mixed strings or URIs"}|]
         { matchStatus = 401 }
 
   describe "custom pre-request proc acting on id claim" $ do