server: accept new config allowed_skew in JWT config to provide leeway in JWT expiry

fixes https://github.com/hasura/graphql-engine/issues/2109

This PR accepts a new config `allowed_skew` in the JWT config to provide for some leeway while comparing the JWT expiry time.

GitOrigin-RevId: ef50cf77d8e2780478685096ed13794b5c4c9de4

.circleci/test-server.sh

@@ -428,6 +428,21 @@ kill_hge_servers
 
 unset HASURA_GRAPHQL_JWT_SECRET
 
+echo -e "\n$(time_elapsed): <########## TEST GRAPHQL-ENGINE WITH ADMIN SECRET AND JWT (with JWT config allowing for leeway) #####################################>\n"
+TEST_TYPE="jwt-with-expiry-time-leeway"
+
+export HASURA_GRAPHQL_JWT_SECRET="$(jq -n --arg key "$(cat $OUTPUT_FOLDER/ssl/jwt_public.key)" '{ type: "RS512", key: $key , allowed_skew: 60}')"
+
+run_hge_with_args serve
+wait_for_port 8080
+
+pytest -n 1 -vv --hge-urls "$HGE_URL" --pg-urls "$HASURA_GRAPHQL_DATABASE_URL" --hge-key="$HASURA_GRAPHQL_ADMIN_SECRET" --hge-jwt-key-file="$OUTPUT_FOLDER/ssl/jwt_private.key" --hge-jwt-conf="$HASURA_GRAPHQL_JWT_SECRET" test_jwt.py::TestJWTExpirySkew
+
+kill_hge_servers
+
+unset HASURA_GRAPHQL_JWT_SECRET
+
+
 
 # test with CORS modes
 

CHANGELOG.md

@@ -89,6 +89,7 @@ and be accessible according to the permissions that were configured for the role
 - server: fix issue when the `relationships` field in `objects` field is passed `[]` in the `set_custom_types` API (fix #6357)
 - server: fix issue with event triggers defined on a table which is partitioned (fixes #6261)
 - server: fix issue with non-optional fields of the remote schema being added as optional in the graphql-engine (fix #6401)
+- server: accept new config `allowed_skew` in JWT config to provide leeway for JWT expiry (fixes #2109)
 - server: fix issue with query actions with relationship with permissions configured on the remote table (fix #6385)
 - console: allow user to cascade Postgres dependencies when dropping Postgres objects (close #5109) (#5248)
 - console: mark inconsistent remote schemas in the UI (close #5093) (#5181)

docs/graphql/core/auth/authentication/jwt.rst

@@ -134,7 +134,8 @@ JSON object:
      "claims_format": "json|stringified_json",
      "audience": <optional-string-or-list-of-strings-to-verify-audience>,
      "issuer": "<optional-string-to-verify-issuer>",
-     "claims_map": "<optional-object-of-session-variable-to-claim-jsonpath-or-literal-value>"
+     "claims_map": "<optional-object-of-session-variable-to-claim-jsonpath-or-literal-value>",
+     "allowed_skew": "<optional-number-of-seconds-in-integer>"
    }
 
 (``type``, ``key``) pair or ``jwk_url``, **one of them has to be present**.
@@ -494,6 +495,12 @@ The corresponding JWT config should be:
 In the above example, the ``x-hasura-allowed-roles`` and ``x-hasura-default-role`` values are set in the JWT
 config and the value of the ``x-hasura-user-id`` is a JSON path to the value in the JWT token.
 
+``allowed_skew``
+^^^^^^^^^^^^^^^^
+
+``allowed_skew`` is an optional field to provide some leeway (to account for clock skews) while comparing the JWT expiry time. This field
+expects an integer value which will be the number of seconds of the skew value.
+
 
 Examples
 ^^^^^^^^

server/src-lib/Hasura/Server/Auth.hs

@@ -156,7 +156,7 @@ setupAuthMode mAdminSecretHash mWebHook mJwtSecret mUnAuthRole httpManager logge
       jwkRef <- case jcKeyOrUrl of
         Left jwk  -> liftIO $ newIORef (JWKSet [jwk])
         Right url -> getJwkFromUrl url
-      return $ JWTCtx jwkRef jcAudience jcIssuer jcClaims
+      return $ JWTCtx jwkRef jcAudience jcIssuer jcClaims jcAllowedSkew
       where
         -- if we can't find any expiry time for the JWK (either in @Expires@ header or @Cache-Control@
         -- header), do not start a background thread for refreshing the JWK

server/src-lib/Hasura/Server/Auth/JWT.hs

@@ -153,7 +153,7 @@ instance J.FromJSON JWTCustomClaimsMap where
     let withNotFoundError sessionVariable =
           let errorMsg = T.unpack $
                 sessionVariableToText sessionVariable <> " is expected but not found"
-          in maybe (fail errorMsg) pure $ Map.lookup (sessionVariableToText sessionVariable) obj
+          in onNothing (Map.lookup (sessionVariableToText sessionVariable) obj) (fail errorMsg)
 
     allowedRoles <- withNotFoundError allowedRolesClaim  >>= J.parseJSON
     defaultRole <- withNotFoundError defaultRoleClaim  >>= J.parseJSON
@@ -182,10 +182,11 @@ data JWTClaims
 -- | The JWT configuration we got from the user.
 data JWTConfig
   = JWTConfig
-  { jcKeyOrUrl :: !(Either Jose.JWK URI)
+  { jcKeyOrUrl    :: !(Either Jose.JWK URI)
-  , jcAudience :: !(Maybe Jose.Audience)
+  , jcAudience    :: !(Maybe Jose.Audience)
-  , jcIssuer   :: !(Maybe Jose.StringOrURI)
+  , jcIssuer      :: !(Maybe Jose.StringOrURI)
-  , jcClaims   :: !JWTClaims
+  , jcClaims      :: !JWTClaims
+  , jcAllowedSkew :: !(Maybe NominalDiffTime)
   } deriving (Show, Eq)
 
 -- | The validated runtime JWT configuration returned by 'mkJwtCtx' in 'setupAuthMode'.
@@ -194,16 +195,17 @@ data JWTConfig
 -- expiration schedule could be determined.
 data JWTCtx
   = JWTCtx
-  { jcxKey      :: !(IORef Jose.JWKSet)
+  { jcxKey         :: !(IORef Jose.JWKSet)
     -- ^ This needs to be a mutable variable for 'updateJwkRef'.
-  , jcxAudience :: !(Maybe Jose.Audience)
+  , jcxAudience    :: !(Maybe Jose.Audience)
-  , jcxIssuer   :: !(Maybe Jose.StringOrURI)
+  , jcxIssuer      :: !(Maybe Jose.StringOrURI)
-  , jcxClaims   :: !JWTClaims
+  , jcxClaims      :: !JWTClaims
+  , jcxAllowedSkew :: !(Maybe NominalDiffTime)
   } deriving (Eq)
 
 instance Show JWTCtx where
-  show (JWTCtx _ audM iss claims) =
+  show (JWTCtx _ audM iss claims allowedSkew) =
-    show ["<IORef JWKSet>", show audM, show iss, show claims]
+    show ["<IORef JWKSet>", show audM, show iss, show claims, show allowedSkew]
 
 data HasuraClaims
   = HasuraClaims
@@ -249,7 +251,7 @@ updateJwkRef
   -> IORef Jose.JWKSet
   -> m (Maybe NominalDiffTime)
 updateJwkRef (Logger logger) manager url jwkRef = do
-  let urlT    = T.pack $ show url
+  let urlT    = tshow url
       infoMsg = "refreshing JWK from endpoint: " <> urlT
   liftIO $ logger $ JwkRefreshLog LevelInfo (Just infoMsg) Nothing
   res <- try $ do
@@ -415,7 +417,7 @@ processAuthZHeader jwtCtx authzHeader = do
       onLeft res (throwError . ef)
 
     invalidJWTError e =
-      err400 JWTInvalid $ "Could not verify JWT: " <> T.pack (show e)
+      err400 JWTInvalid $ "Could not verify JWT: " <> tshow e
 
     malformedAuthzHeader =
       throw400 InvalidHeaders "Malformed Authorization header"
@@ -429,9 +431,10 @@ parseClaimsMap
 parseClaimsMap unregisteredClaims jcxClaims =
   case jcxClaims of
     JCNamespace namespace claimsFormat -> do
-      claimsV <- maybe (claimsNotFound namespace) pure $ case namespace of
+      claimsV <- flip onNothing (claimsNotFound namespace) $
-            ClaimNs k -> Map.lookup k unregisteredClaims
+        case namespace of
-            ClaimNsPath path -> iResultToMaybe $ executeJSONPath path (J.toJSON unregisteredClaims)
+          ClaimNs k -> Map.lookup k unregisteredClaims
+          ClaimNsPath path -> iResultToMaybe $ executeJSONPath path (J.toJSON unregisteredClaims)
       -- get hasura claims value as an object. parse from string possibly
       claimsObject <- parseObjectFromString namespace claimsFormat claimsV
 
@@ -461,8 +464,8 @@ parseClaimsMap unregisteredClaims jcxClaims =
                             <> sessionVariableToText k <> " not found"
         case claimObj of
           JWTCustomClaimsMapJSONPath path defaultVal ->
-            maybe (onNothing (J.String <$> defaultVal) throwClaimErr) pure
+            onNothing (iResultToMaybe $ executeJSONPath path claimsObjValue) $
-              $ iResultToMaybe $ executeJSONPath path claimsObjValue
+            (onNothing (J.String <$> defaultVal) throwClaimErr)
           JWTCustomClaimsMapStatic claimStaticValue -> pure $ J.String claimStaticValue
 
       pure $  Map.fromList [
@@ -525,10 +528,15 @@ verifyJwt ctx (RawJWT rawJWT) = do
   t   <- liftIO getCurrentTime
   Jose.verifyClaimsAt config key t jwt
   where
+    validationSettingsWithSkew =
+      case jcxAllowedSkew ctx of
+        Just allowedSkew -> Jose.defaultJWTValidationSettings audCheck & set Jose.allowedSkew allowedSkew
+        -- In `Jose.defaultJWTValidationSettings`, the `allowedSkew` is 0
+        Nothing          -> Jose.defaultJWTValidationSettings audCheck
+
     config = case jcxIssuer ctx of
-      Nothing  -> Jose.defaultJWTValidationSettings audCheck
+      Nothing  -> validationSettingsWithSkew
-      Just iss -> Jose.defaultJWTValidationSettings audCheck
+      Just iss -> validationSettingsWithSkew & set Jose.issuerPredicate (== iss)
-                  & set Jose.issuerPredicate (== iss)
     audCheck audience =
       -- dont perform the check if there are no audiences in the conf
       case jcxAudience ctx of
@@ -537,7 +545,7 @@ verifyJwt ctx (RawJWT rawJWT) = do
 
 
 instance J.ToJSON JWTConfig where
-  toJSON (JWTConfig keyOrUrl aud iss claims) =
+  toJSON (JWTConfig keyOrUrl aud iss claims allowedSkew) =
     let keyOrUrlPairs = case keyOrUrl of
           Left _    -> [ "type" J..= J.String "<TYPE REDACTED>"
                        , "key" J..= J.String "<JWK REDACTED>"
@@ -554,7 +562,9 @@ instance J.ToJSON JWTConfig where
     in J.object $ keyOrUrlPairs <>
                   [ "audience" J..= aud
                   , "issuer" J..= iss
-                  ] <> claimsPairs
+                  ]
+                  <> claimsPairs
+                  <> (maybe [] (\skew -> ["allowed_skew" J..= skew]) allowedSkew)
 
 -- | Parse from a json string like:
 -- | `{"type": "RS256", "key": "<PEM-encoded-public-key-or-X509-cert>"}`
@@ -570,6 +580,7 @@ instance J.FromJSON JWTConfig where
     jwkUrl  <- o J..:? "jwk_url"
     claimsFormat <- o J..:? "claims_format" J..!= defaultClaimsFormat
     claimsMap <- o J..:? "claims_map"
+    allowedSkew <- o J..:? "allowed_skew"
 
     hasuraClaimsNs <-
       case (claimsNsPath,claimsNs) of
@@ -587,8 +598,9 @@ instance J.FromJSON JWTConfig where
         pure $ Left key
       (Nothing, Just url) -> pure $ Right url
 
-    pure $ JWTConfig keyOrUrl aud iss $
+    let jwtClaims = maybe (JCNamespace hasuraClaimsNs claimsFormat) JCMap claimsMap
-           maybe (JCNamespace hasuraClaimsNs claimsFormat) JCMap claimsMap
+
+    pure $ JWTConfig keyOrUrl aud iss jwtClaims allowedSkew
 
     where
       parseKey keyType rawKey =

server/src-test/Hasura/Server/AuthSpec.hs

@@ -557,6 +557,7 @@ fakeJWTConfig =
       jcAudience = Nothing
       jcIssuer = Nothing
       jcClaims = JCNamespace (ClaimNs "") JCFJson
+      jcAllowedSkew = Nothing
    in JWTConfig{..}
 
 fakeAuthHook :: AuthHook

server/tests-py/test_jwt.py

@@ -1,4 +1,4 @@
-from datetime import datetime, timedelta
+from datetime import datetime, timedelta, timezone
 import math
 import json
 import time
@@ -38,6 +38,69 @@ def mk_claims(conf, claims):
     else:
         return claims
 
+@pytest.mark.parametrize('endpoint', ['/v1/graphql', '/v1alpha1/graphql'])
+class TestJWTExpirySkew():
+
+    def test_jwt_expiry_leeway(self, hge_ctx, endpoint):
+        hasura_claims = mk_claims(hge_ctx.hge_jwt_conf, {
+            'x-hasura-user-id': '1',
+            'x-hasura-default-role': 'user',
+            'x-hasura-allowed-roles': ['user'],
+        })
+        claims_namespace_path = None
+        if 'claims_namespace_path' in hge_ctx.hge_jwt_conf_dict:
+            claims_namespace_path = hge_ctx.hge_jwt_conf_dict['claims_namespace_path']
+
+        if not 'allowed_skew' in hge_ctx.hge_jwt_conf_dict:
+            pytest.skip("This test expects 'allowed_skew' to be set in the JWT config" )
+
+        self.claims = mk_claims_with_namespace_path(self.claims,hasura_claims,claims_namespace_path)
+        exp = datetime.now(timezone.utc) - timedelta(seconds = 30)
+        self.claims['exp'] = round(exp.timestamp())
+        token = jwt.encode(self.claims, hge_ctx.hge_jwt_key, algorithm='RS512').decode('utf-8')
+        self.conf['headers']['Authorization'] = 'Bearer ' + token
+        self.conf['response'] = {
+            'data': {
+                'article': [{
+                    'id': 1,
+                    'title': 'Article 1',
+                    'content': 'Sample article content 1',
+                    'is_published': False,
+                    'author': {
+                        'id': 1,
+                        'name': 'Author 1'
+                    }
+                }]
+            }
+        }
+        self.conf['url'] = endpoint
+        self.conf['status'] = 200
+        print ("conf is ", self.conf)
+        check_query(hge_ctx, self.conf, add_auth=False,claims_namespace_path=claims_namespace_path)
+
+    @pytest.fixture(autouse=True)
+    def transact(self, setup):
+        self.dir = 'queries/graphql_query/permissions'
+        with open(self.dir + '/user_select_query_unpublished_articles.yaml') as c:
+            self.conf = yaml.safe_load(c)
+        curr_time = datetime.utcnow()
+        exp_time = curr_time + timedelta(hours=10)
+        self.claims = {
+            'sub': '1234567890',
+            'name': 'John Doe',
+            'iat': math.floor(curr_time.timestamp()),
+            'exp': math.floor(exp_time.timestamp())
+        }
+
+    @pytest.fixture(scope='class')
+    def setup(self, request, hge_ctx):
+        self.dir = 'queries/graphql_query/permissions'
+        st_code, resp = hge_ctx.v1q_f(self.dir + '/setup.yaml')
+        assert st_code == 200, resp
+        yield
+        st_code, resp = hge_ctx.v1q_f(self.dir + '/teardown.yaml')
+        assert st_code == 200, resp
+
 @pytest.mark.parametrize('endpoint', ['/v1/graphql', '/v1alpha1/graphql'])
 class TestJWTBasic():