From 28555660354c6ef0db5201dd6d0d3591739a3774 Mon Sep 17 00:00:00 2001 From: Anon Ray Date: Thu, 31 Jan 2019 13:47:44 +0530 Subject: [PATCH 1/3] add config for stringified hasura claims in JWT (fix #1176) this is required, as some providers like AWS Cognito allows only strings in custom claims. --- .circleci/test-server.sh | 16 +++++++- docs/graphql/manual/auth/jwt.rst | 51 +++++++++++++++++++++++- server/src-lib/Hasura/Server/Auth.hs | 3 +- server/src-lib/Hasura/Server/Auth/JWT.hs | 43 +++++++++++++------- server/tests-py/conftest.py | 14 ++++++- server/tests-py/context.py | 3 +- server/tests-py/test_jwt.py | 46 +++++++++++++++++++-- 7 files changed, 152 insertions(+), 24 deletions(-) diff --git a/.circleci/test-server.sh b/.circleci/test-server.sh index b74a60c27074d..465b1c8d13a4b 100755 --- a/.circleci/test-server.sh +++ b/.circleci/test-server.sh @@ -155,7 +155,21 @@ export HASURA_GRAPHQL_JWT_SECRET="$(jq -n --arg key "$(cat $OUTPUT_FOLDER/ssl/jw "$GRAPHQL_ENGINE" serve >> "$OUTPUT_FOLDER/graphql-engine.log" & PID=$! -pytest -vv --hge-url="$HGE_URL" --pg-url="$HASURA_GRAPHQL_DATABASE_URL" --hge-key="$HASURA_GRAPHQL_ACCESS_KEY" --hge-jwt-key-file="$OUTPUT_FOLDER/ssl/jwt_private.key" +pytest -vv --hge-url="$HGE_URL" --pg-url="$HASURA_GRAPHQL_DATABASE_URL" --hge-key="$HASURA_GRAPHQL_ACCESS_KEY" --hge-jwt-key-file="$OUTPUT_FOLDER/ssl/jwt_private.key" --hge-jwt-conf="$HASURA_GRAPHQL_JWT_SECRET" + +kill -INT $PID +sleep 4 +combine_hpc_reports + +unset HASURA_GRAPHQL_JWT_SECRET + +echo -e "\n<########## TEST GRAPHQL-ENGINE WITH ACCESS KEY AND JWT (in stringified mode) #####################################>\n" + +export HASURA_GRAPHQL_JWT_SECRET="$(jq -n --arg key "$(cat $OUTPUT_FOLDER/ssl/jwt_public.key)" '{ type: "RS512", key: $key , is_stringified: true}')" + +"$GRAPHQL_ENGINE" serve >> "$OUTPUT_FOLDER/graphql-engine.log" & PID=$! + +pytest -vv --hge-url="$HGE_URL" --pg-url="$HASURA_GRAPHQL_DATABASE_URL" --hge-key="$HASURA_GRAPHQL_ACCESS_KEY" --hge-jwt-key-file="$OUTPUT_FOLDER/ssl/jwt_private.key" --hge-jwt-conf="$HASURA_GRAPHQL_JWT_SECRET" test_jwt.py kill -INT $PID sleep 4 diff --git a/docs/graphql/manual/auth/jwt.rst b/docs/graphql/manual/auth/jwt.rst index 95fa801d66438..338e6e8bd4c60 100644 --- a/docs/graphql/manual/auth/jwt.rst +++ b/docs/graphql/manual/auth/jwt.rst @@ -119,10 +119,11 @@ JSON object: "type": "", "key": "", "jwk_url": "", - "claims_namespace": "" + "claims_namespace": "", + "is_stringified": } -``key`` or ``jwk_url``, either of them has to be present. +``key`` or ``jwk_url``, **one of them has to be present**. ``type`` ^^^^^^^^ @@ -174,6 +175,52 @@ inside which the Hasura specific claims will be present. E.g. - ``https://mydoma **Default value** is: ``https://hasura.io/jwt/claims``. + +``is_stringified`` +^^^^^^^^^^^^^^^^^^ +This is an optional boolean field. Default is ``false``. + +This is to indicate that if the hasura specific claims are stringified JSON or +not. Even in the stringified case, the value of the string should be decodable +as JSON. + +This is required because providers like AWS Cognito only allows strings in the +JWT claims. `See #1176 `_. + +Example:- + +If ``is_stringified`` is ``false`` then JWT claims should look like: + +.. code-block:: json + + { + "sub": "1234567890", + "name": "John Doe", + "admin": true, + "iat": 1516239022, + "https://hasura.io/jwt/claims": { + "x-hasura-allowed-roles": ["editor","user", "mod"], + "x-hasura-default-role": "user", + "x-hasura-user-id": "1234567890", + "x-hasura-org-id": "123", + "x-hasura-custom": "custom-value" + } + } + + +If ``is_stringified`` is ``true`` then JWT claims should look like: + +.. code-block:: json + + { + "sub": "1234567890", + "name": "John Doe", + "admin": true, + "iat": 1516239022, + "https://hasura.io/jwt/claims": "{\"x-hasura-allowed-roles\":[\"editor\",\"user\",\"mod\"],\"x-hasura-default-role\":\"user\",\"x-hasura-user-id\":\"1234567890\",\"x-hasura-org-id\":\"123\",\"x-hasura-custom\":\"custom-value\"}" + } + + Examples ^^^^^^^^ diff --git a/server/src-lib/Hasura/Server/Auth.hs b/server/src-lib/Hasura/Server/Auth.hs index 05b186bb21d7c..33b65af576c8e 100644 --- a/server/src-lib/Hasura/Server/Auth.hs +++ b/server/src-lib/Hasura/Server/Auth.hs @@ -138,7 +138,8 @@ mkJwtCtx jwtConf httpManager loggerCtx = do Just t -> do jwkRefreshCtrl logger httpManager url ref t return ref - return $ JWTCtx jwkRef (jcClaimNs conf) (jcAudience conf) + let strngfd = fromMaybe False (jcIsStringified conf) + return $ JWTCtx jwkRef (jcClaimNs conf) (jcAudience conf) strngfd where decodeErr e = throwError . T.pack $ "Fatal Error: JWT conf: " <> e diff --git a/server/src-lib/Hasura/Server/Auth/JWT.hs b/server/src-lib/Hasura/Server/Auth/JWT.hs index 240afc162991e..8d79a61d684f2 100644 --- a/server/src-lib/Hasura/Server/Auth/JWT.hs +++ b/server/src-lib/Hasura/Server/Auth/JWT.hs @@ -39,6 +39,7 @@ import qualified Data.CaseInsensitive as CI import qualified Data.HashMap.Strict as Map import qualified Data.String.Conversions as CS import qualified Data.Text as T +import qualified Data.Text.Encoding as T import qualified Network.HTTP.Client as HTTP import qualified Network.HTTP.Types as HTTP import qualified Network.Wreq as Wreq @@ -48,23 +49,25 @@ newtype RawJWT = RawJWT BL.ByteString data JWTConfig = JWTConfig - { jcType :: !T.Text - , jcKeyOrUrl :: !(Either JWK URI) - , jcClaimNs :: !(Maybe T.Text) - , jcAudience :: !(Maybe T.Text) + { jcType :: !T.Text + , jcKeyOrUrl :: !(Either JWK URI) + , jcClaimNs :: !(Maybe T.Text) + , jcAudience :: !(Maybe T.Text) + , jcIsStringified :: !(Maybe Bool) -- , jcIssuer :: !(Maybe T.Text) } deriving (Show, Eq) data JWTCtx = JWTCtx - { jcxKey :: !(IORef JWKSet) - , jcxClaimNs :: !(Maybe T.Text) - , jcxAudience :: !(Maybe T.Text) + { jcxKey :: !(IORef JWKSet) + , jcxClaimNs :: !(Maybe T.Text) + , jcxAudience :: !(Maybe T.Text) + , jcxIsStringified :: !Bool } deriving (Eq) instance Show JWTCtx where - show (JWTCtx _ nsM audM) = - show ["", show nsM, show audM] + show (JWTCtx _ nsM audM strfd) = + show ["", show nsM, show audM, show strfd] data HasuraClaims = HasuraClaims @@ -189,12 +192,14 @@ processAuthZHeader jwtCtx headers authzHeader = do claims <- liftJWTError invalidJWTError $ verifyJwt jwtCtx $ RawJWT jwt let claimsNs = fromMaybe defaultClaimNs $ jcxClaimNs jwtCtx + isStrngfd = jcxIsStringified jwtCtx -- see if the hasura claims key exist in the claims map let mHasuraClaims = Map.lookup claimsNs $ claims ^. unregisteredClaims hasuraClaimsV <- maybe claimsNotFound return mHasuraClaims - -- the value of hasura claims key has to be an object - hasuraClaims <- validateIsObject hasuraClaimsV + + -- get hasura claims value as an object. parse from string possibly + hasuraClaims <- parseObjectFromString isStrngfd hasuraClaimsV -- filter only x-hasura claims and convert to lower-case let claimsMap = Map.filterWithKey (\k _ -> T.isPrefixOf "x-hasura-" k) @@ -220,11 +225,20 @@ processAuthZHeader jwtCtx headers authzHeader = do ["Bearer", jwt] -> return jwt _ -> malformedAuthzHeader - validateIsObject jVal = + parseObjectFromString isStrngfd jVal = case jVal of A.Object x -> return x + A.String x + | isStrngfd -> either (const $ throw400 JWTInvalidClaims $ strngfdErr <> x) return + $ A.eitherDecodeStrict $ T.encodeUtf8 x + | otherwise -> throw400 JWTInvalidClaims "hasura claims should be an object" + _ -> throw400 JWTInvalidClaims "hasura claims should be an object" + strngfdErr = "Could not parse JSON string under: '" + <> fromMaybe defaultClaimNs (jcxClaimNs jwtCtx) + <> "'. When stringified, the claims inside should be a JSON object, but found: " + -- see if there is a x-hasura-role header, or else pick the default role getCurrentRole defaultRole = let userRoleHeaderB = CS.cs userRoleHeader @@ -314,15 +328,16 @@ instance A.FromJSON JWTConfig where claimNs <- o A..:? "claims_namespace" aud <- o A..:? "audience" jwkUrl <- o A..:? "jwk_url" + isStrngfd <- o A..:? "is_stringified" case (mRawKey, jwkUrl) of (Nothing, Nothing) -> fail "key and jwk_url both cannot be empty" (Just _, Just _) -> fail "key, jwk_url both cannot be present" (Just rawKey, Nothing) -> do key <- parseKey keyType rawKey - return $ JWTConfig keyType (Left key) claimNs aud + return $ JWTConfig keyType (Left key) claimNs aud isStrngfd (Nothing, Just url) -> - return $ JWTConfig keyType (Right url) claimNs aud + return $ JWTConfig keyType (Right url) claimNs aud isStrngfd where parseKey keyType rawKey = diff --git a/server/tests-py/conftest.py b/server/tests-py/conftest.py index e6737adcd68f1..cee9240af952c 100644 --- a/server/tests-py/conftest.py +++ b/server/tests-py/conftest.py @@ -23,6 +23,9 @@ def pytest_addoption(parser): parser.addoption( "--hge-jwt-key-file", metavar="HGE_JWT_KEY_FILE", help="File containting the private key used to encode jwt tokens using RS512 algorithm", required=False ) + parser.addoption( + "--hge-jwt-conf", metavar="HGE_JWT_CONF", help="The JWT conf", required=False + ) @pytest.fixture(scope='session') @@ -34,8 +37,17 @@ def hge_ctx(request): hge_webhook = request.config.getoption('--hge-webhook') webhook_insecure = request.config.getoption('--test-webhook-insecure') hge_jwt_key_file = request.config.getoption('--hge-jwt-key-file') + hge_jwt_conf = request.config.getoption('--hge-jwt-conf') try: - hge_ctx = HGECtx(hge_url=hge_url, pg_url=pg_url, hge_key=hge_key, hge_webhook=hge_webhook, hge_jwt_key_file=hge_jwt_key_file, webhook_insecure = webhook_insecure ) + hge_ctx = HGECtx( + hge_url=hge_url, + pg_url=pg_url, + hge_key=hge_key, + hge_webhook=hge_webhook, + webhook_insecure=webhook_insecure, + hge_jwt_key_file=hge_jwt_key_file, + hge_jwt_conf=hge_jwt_conf + ) except HGECtxError as e: pytest.exit(str(e)) yield hge_ctx # provide the fixture value diff --git a/server/tests-py/context.py b/server/tests-py/context.py index f91f364091202..8669dc60c5f7e 100644 --- a/server/tests-py/context.py +++ b/server/tests-py/context.py @@ -60,7 +60,7 @@ def server_bind(self): class HGECtx: - def __init__(self, hge_url, pg_url, hge_key, hge_webhook, hge_jwt_key_file, webhook_insecure): + def __init__(self, hge_url, pg_url, hge_key, hge_webhook, webhook_insecure, hge_jwt_key_file, hge_jwt_conf): server_address = ('0.0.0.0', 5592) self.resp_queue = queue.Queue(maxsize=1) @@ -83,6 +83,7 @@ def __init__(self, hge_url, pg_url, hge_key, hge_webhook, hge_jwt_key_file, webh else: with open(hge_jwt_key_file) as f: self.hge_jwt_key = f.read() + self.hge_jwt_conf = hge_jwt_conf self.webhook_insecure = webhook_insecure self.may_skip_test_teardown = False diff --git a/server/tests-py/test_jwt.py b/server/tests-py/test_jwt.py index 66358b4e778e9..969602d601774 100644 --- a/server/tests-py/test_jwt.py +++ b/server/tests-py/test_jwt.py @@ -1,17 +1,37 @@ - from datetime import datetime, timedelta import math +import json + import yaml import pytest import jwt +from cryptography.hazmat.backends import default_backend +from cryptography.hazmat.primitives.asymmetric import rsa +from cryptography.hazmat.primitives import serialization + from validate import check_query + if not pytest.config.getoption('--hge-jwt-key-file'): pytest.skip('--hge-jwt-key-file is missing, skipping JWT basic tests', allow_module_level=True) +if not pytest.config.getoption('--hge-jwt-conf'): + pytest.skip('--hge-jwt-key-conf is missing, skipping JWT basic tests', allow_module_level=True) + class TestJWTBasic: + def test_jwt_valid_claims_success(self, hge_ctx): + self.claims['https://hasura.io/jwt/claims'] = { + 'x-hasura-user-id': '1', + 'x-hasura-allowed-roles': ['user', 'editor'], + 'x-hasura-default-role': 'user' + } + token = jwt.encode(self.claims, hge_ctx.hge_jwt_key, algorithm='RS512').decode('utf-8') + self.conf['headers']['Authorization'] = 'Bearer ' + token + self.conf['status'] = 200 + check_query(hge_ctx, self.conf, add_auth=False) + def test_jwt_invalid_role_in_request_header(self, hge_ctx): self.claims['https://hasura.io/jwt/claims'] = { 'x-hasura-user-id': '1', @@ -92,6 +112,7 @@ def test_jwt_no_default_role(self, hge_ctx): def test_jwt_expired(self, hge_ctx): self.claims['https://hasura.io/jwt/claims'] = { + 'x-hasura-user-id': '1', 'x-hasura-default-role': 'user', 'x-hasura-allowed-roles': ['user'], } @@ -114,6 +135,7 @@ def test_jwt_expired(self, hge_ctx): def test_jwt_invalid_signature(self, hge_ctx): self.claims['https://hasura.io/jwt/claims'] = { + 'x-hasura-user-id': '1', 'x-hasura-default-role': 'user', 'x-hasura-allowed-roles': ['user'], } @@ -133,6 +155,25 @@ def test_jwt_invalid_signature(self, hge_ctx): self.conf['status'] = 400 check_query(hge_ctx, self.conf, add_auth=False) + def test_jwt_stringified_claims(self, hge_ctx): + print(hge_ctx.hge_jwt_conf) + conf = json.loads(hge_ctx.hge_jwt_conf) + if 'is_stringified' not in conf: + pytest.skip('is_stringified is false, skipping testing stringified claims.') + if 'is_stringified' in conf and not conf['is_stringified']: + pytest.skip('is_stringified is false, skipping testing stringified claims.') + + self.claims['https://hasura.io/jwt/claims'] = json.dumps({ + 'x-hasura-user-id': '1', + 'x-hasura-default-role': 'user', + 'x-hasura-allowed-roles': ['user'], + }) + + token = jwt.encode(self.claims, hge_ctx.hge_jwt_key, algorithm='RS512').decode('utf-8') + self.conf['headers']['Authorization'] = 'Bearer ' + token + self.conf['status'] = 200 + check_query(hge_ctx, self.conf, add_auth=False) + @pytest.fixture(autouse=True) def transact(self, request, hge_ctx): self.dir = 'queries/graphql_query/permissions' @@ -154,9 +195,6 @@ def transact(self, request, hge_ctx): def gen_rsa_key(): - from cryptography.hazmat.backends import default_backend - from cryptography.hazmat.primitives.asymmetric import rsa - from cryptography.hazmat.primitives import serialization private_key = rsa.generate_private_key( public_exponent=65537, key_size=2048, From 5815e10491400f69c54f26f9e4ed2da1dfded4ea Mon Sep 17 00:00:00 2001 From: Anon Ray Date: Tue, 5 Feb 2019 12:58:32 +0530 Subject: [PATCH 2/3] change `is_stringified` to `claims_format` in JWT conf --- .circleci/test-server.sh | 2 +- docs/graphql/manual/auth/jwt.rst | 19 +++--- server/src-lib/Hasura/Server/Auth.hs | 4 +- server/src-lib/Hasura/Server/Auth/JWT.hs | 55 +++++++++------- server/tests-py/test_jwt.py | 80 ++++++++++++++---------- 5 files changed, 95 insertions(+), 65 deletions(-) diff --git a/.circleci/test-server.sh b/.circleci/test-server.sh index 465b1c8d13a4b..b9c5473ac501b 100755 --- a/.circleci/test-server.sh +++ b/.circleci/test-server.sh @@ -165,7 +165,7 @@ unset HASURA_GRAPHQL_JWT_SECRET echo -e "\n<########## TEST GRAPHQL-ENGINE WITH ACCESS KEY AND JWT (in stringified mode) #####################################>\n" -export HASURA_GRAPHQL_JWT_SECRET="$(jq -n --arg key "$(cat $OUTPUT_FOLDER/ssl/jwt_public.key)" '{ type: "RS512", key: $key , is_stringified: true}')" +export HASURA_GRAPHQL_JWT_SECRET="$(jq -n --arg key "$(cat $OUTPUT_FOLDER/ssl/jwt_public.key)" '{ type: "RS512", key: $key , claims_format: "stringified_json"}')" "$GRAPHQL_ENGINE" serve >> "$OUTPUT_FOLDER/graphql-engine.log" & PID=$! diff --git a/docs/graphql/manual/auth/jwt.rst b/docs/graphql/manual/auth/jwt.rst index 338e6e8bd4c60..1d8c115ab97a7 100644 --- a/docs/graphql/manual/auth/jwt.rst +++ b/docs/graphql/manual/auth/jwt.rst @@ -120,7 +120,7 @@ JSON object: "key": "", "jwk_url": "", "claims_namespace": "", - "is_stringified": + "claims_format": "json|stringified_json" } ``key`` or ``jwk_url``, **one of them has to be present**. @@ -176,20 +176,23 @@ inside which the Hasura specific claims will be present. E.g. - ``https://mydoma **Default value** is: ``https://hasura.io/jwt/claims``. -``is_stringified`` +``claims_format`` ^^^^^^^^^^^^^^^^^^ -This is an optional boolean field. Default is ``false``. +This is an optional field, with only the following possible values: +- ``json`` +- ``stringified_json`` -This is to indicate that if the hasura specific claims are stringified JSON or -not. Even in the stringified case, the value of the string should be decodable -as JSON. +Default is ``json``. + +This is to indicate that if the hasura specific claims are a regular JSON object +or stringified JSON This is required because providers like AWS Cognito only allows strings in the JWT claims. `See #1176 `_. Example:- -If ``is_stringified`` is ``false`` then JWT claims should look like: +If ``claims_format`` is ``json`` then JWT claims should look like: .. code-block:: json @@ -208,7 +211,7 @@ If ``is_stringified`` is ``false`` then JWT claims should look like: } -If ``is_stringified`` is ``true`` then JWT claims should look like: +If ``claims_format`` is ``stringified_json`` then JWT claims should look like: .. code-block:: json diff --git a/server/src-lib/Hasura/Server/Auth.hs b/server/src-lib/Hasura/Server/Auth.hs index 33b65af576c8e..1cb95248e6834 100644 --- a/server/src-lib/Hasura/Server/Auth.hs +++ b/server/src-lib/Hasura/Server/Auth.hs @@ -138,8 +138,8 @@ mkJwtCtx jwtConf httpManager loggerCtx = do Just t -> do jwkRefreshCtrl logger httpManager url ref t return ref - let strngfd = fromMaybe False (jcIsStringified conf) - return $ JWTCtx jwkRef (jcClaimNs conf) (jcAudience conf) strngfd + let claimsFmt = fromMaybe JCFJson (jcClaimsFormat conf) + return $ JWTCtx jwkRef (jcClaimNs conf) (jcAudience conf) claimsFmt where decodeErr e = throwError . T.pack $ "Fatal Error: JWT conf: " <> e diff --git a/server/src-lib/Hasura/Server/Auth/JWT.hs b/server/src-lib/Hasura/Server/Auth/JWT.hs index 8d79a61d684f2..bcb0986ca1a22 100644 --- a/server/src-lib/Hasura/Server/Auth/JWT.hs +++ b/server/src-lib/Hasura/Server/Auth/JWT.hs @@ -4,6 +4,7 @@ module Hasura.Server.Auth.JWT , JWTConfig (..) , JWTCtx (..) , JWKSet (..) + , JWTClaimsFormat (..) , updateJwkRef , jwkRefreshCtrl ) where @@ -47,22 +48,31 @@ import qualified Network.Wreq as Wreq newtype RawJWT = RawJWT BL.ByteString +data JWTClaimsFormat + = JCFJson + | JCFStringifiedJson + deriving (Show, Eq) + +$(A.deriveJSON A.defaultOptions { A.sumEncoding = A.ObjectWithSingleField + , A.constructorTagModifier = A.snakeCase . drop 3 + } ''JWTClaimsFormat) + data JWTConfig = JWTConfig - { jcType :: !T.Text - , jcKeyOrUrl :: !(Either JWK URI) - , jcClaimNs :: !(Maybe T.Text) - , jcAudience :: !(Maybe T.Text) - , jcIsStringified :: !(Maybe Bool) + { jcType :: !T.Text + , jcKeyOrUrl :: !(Either JWK URI) + , jcClaimNs :: !(Maybe T.Text) + , jcAudience :: !(Maybe T.Text) + , jcClaimsFormat :: !(Maybe JWTClaimsFormat) -- , jcIssuer :: !(Maybe T.Text) } deriving (Show, Eq) data JWTCtx = JWTCtx - { jcxKey :: !(IORef JWKSet) - , jcxClaimNs :: !(Maybe T.Text) - , jcxAudience :: !(Maybe T.Text) - , jcxIsStringified :: !Bool + { jcxKey :: !(IORef JWKSet) + , jcxClaimNs :: !(Maybe T.Text) + , jcxAudience :: !(Maybe T.Text) + , jcxClaimsFormat :: !JWTClaimsFormat } deriving (Eq) instance Show JWTCtx where @@ -191,15 +201,15 @@ processAuthZHeader jwtCtx headers authzHeader = do -- verify the JWT claims <- liftJWTError invalidJWTError $ verifyJwt jwtCtx $ RawJWT jwt - let claimsNs = fromMaybe defaultClaimNs $ jcxClaimNs jwtCtx - isStrngfd = jcxIsStringified jwtCtx + let claimsNs = fromMaybe defaultClaimNs $ jcxClaimNs jwtCtx + claimsFmt = jcxClaimsFormat jwtCtx -- see if the hasura claims key exist in the claims map let mHasuraClaims = Map.lookup claimsNs $ claims ^. unregisteredClaims hasuraClaimsV <- maybe claimsNotFound return mHasuraClaims -- get hasura claims value as an object. parse from string possibly - hasuraClaims <- parseObjectFromString isStrngfd hasuraClaimsV + hasuraClaims <- parseObjectFromString claimsFmt hasuraClaimsV -- filter only x-hasura claims and convert to lower-case let claimsMap = Map.filterWithKey (\k _ -> T.isPrefixOf "x-hasura-" k) @@ -225,15 +235,16 @@ processAuthZHeader jwtCtx headers authzHeader = do ["Bearer", jwt] -> return jwt _ -> malformedAuthzHeader - parseObjectFromString isStrngfd jVal = - case jVal of - A.Object x -> return x - A.String x - | isStrngfd -> either (const $ throw400 JWTInvalidClaims $ strngfdErr <> x) return - $ A.eitherDecodeStrict $ T.encodeUtf8 x - | otherwise -> throw400 JWTInvalidClaims "hasura claims should be an object" - - _ -> throw400 JWTInvalidClaims "hasura claims should be an object" + parseObjectFromString claimsFmt jVal = + case (claimsFmt, jVal) of + (JCFStringifiedJson, A.String v) -> + either (const $ throw400 JWTInvalidClaims $ strngfdErr <> v) return + $ A.eitherDecodeStrict $ T.encodeUtf8 v + (JCFStringifiedJson, _) -> + throw400 JWTInvalidClaims "expecting a string when claims_format is stringified_json" + (JCFJson, A.Object o) -> return o + (JCFJson, _) -> + throw400 JWTInvalidClaims "expecting a json object when claims_format is json" strngfdErr = "Could not parse JSON string under: '" <> fromMaybe defaultClaimNs (jcxClaimNs jwtCtx) @@ -328,7 +339,7 @@ instance A.FromJSON JWTConfig where claimNs <- o A..:? "claims_namespace" aud <- o A..:? "audience" jwkUrl <- o A..:? "jwk_url" - isStrngfd <- o A..:? "is_stringified" + isStrngfd <- o A..:? "claims_format" case (mRawKey, jwkUrl) of (Nothing, Nothing) -> fail "key and jwk_url both cannot be empty" diff --git a/server/tests-py/test_jwt.py b/server/tests-py/test_jwt.py index 969602d601774..62359b758b893 100644 --- a/server/tests-py/test_jwt.py +++ b/server/tests-py/test_jwt.py @@ -18,26 +18,42 @@ if not pytest.config.getoption('--hge-jwt-conf'): pytest.skip('--hge-jwt-key-conf is missing, skipping JWT basic tests', allow_module_level=True) +def get_claims_fmt(raw_conf): + conf = json.loads(raw_conf) + try: + claims_fmt = conf['claims_format'] + except KeyError: + claims_fmt = 'json' + return claims_fmt + +def mk_claims(conf, claims): + claims_fmt = get_claims_fmt(conf) + if claims_fmt == 'json': + return claims + elif claims_fmt == 'stringified_json': + return json.dumps(claims) + else: + return claims class TestJWTBasic: def test_jwt_valid_claims_success(self, hge_ctx): - self.claims['https://hasura.io/jwt/claims'] = { + self.claims['https://hasura.io/jwt/claims'] = mk_claims(hge_ctx.hge_jwt_conf, { 'x-hasura-user-id': '1', 'x-hasura-allowed-roles': ['user', 'editor'], 'x-hasura-default-role': 'user' - } + }) token = jwt.encode(self.claims, hge_ctx.hge_jwt_key, algorithm='RS512').decode('utf-8') self.conf['headers']['Authorization'] = 'Bearer ' + token self.conf['status'] = 200 check_query(hge_ctx, self.conf, add_auth=False) def test_jwt_invalid_role_in_request_header(self, hge_ctx): - self.claims['https://hasura.io/jwt/claims'] = { + self.claims['https://hasura.io/jwt/claims'] = mk_claims(hge_ctx.hge_jwt_conf, { 'x-hasura-user-id': '1', 'x-hasura-allowed-roles': ['contractor', 'editor'], 'x-hasura-default-role': 'contractor' - } + }) token = jwt.encode(self.claims, hge_ctx.hge_jwt_key, algorithm='RS512').decode('utf-8') self.conf['headers']['Authorization'] = 'Bearer ' + token self.conf['response'] = { @@ -53,10 +69,10 @@ def test_jwt_invalid_role_in_request_header(self, hge_ctx): check_query(hge_ctx, self.conf, add_auth=False) def test_jwt_no_allowed_roles_in_claim(self, hge_ctx): - self.claims['https://hasura.io/jwt/claims'] = { + self.claims['https://hasura.io/jwt/claims'] = mk_claims(hge_ctx.hge_jwt_conf, { 'x-hasura-user-id': '1', 'x-hasura-default-role': 'user' - } + }) token = jwt.encode(self.claims, hge_ctx.hge_jwt_key, algorithm='RS512').decode('utf-8') self.conf['headers']['Authorization'] = 'Bearer ' + token self.conf['response'] = { @@ -72,11 +88,11 @@ def test_jwt_no_allowed_roles_in_claim(self, hge_ctx): check_query(hge_ctx, self.conf, add_auth=False) def test_jwt_invalid_allowed_roles_in_claim(self, hge_ctx): - self.claims['https://hasura.io/jwt/claims'] = { + self.claims['https://hasura.io/jwt/claims'] = mk_claims(hge_ctx.hge_jwt_conf, { 'x-hasura-user-id': '1', 'x-hasura-allowed-roles': 'user', 'x-hasura-default-role': 'user' - } + }) token = jwt.encode(self.claims, hge_ctx.hge_jwt_key, algorithm='RS512').decode('utf-8') self.conf['headers']['Authorization'] = 'Bearer ' + token self.conf['response'] = { @@ -92,10 +108,10 @@ def test_jwt_invalid_allowed_roles_in_claim(self, hge_ctx): check_query(hge_ctx, self.conf, add_auth=False) def test_jwt_no_default_role(self, hge_ctx): - self.claims['https://hasura.io/jwt/claims'] = { + self.claims['https://hasura.io/jwt/claims'] = mk_claims(hge_ctx.hge_jwt_conf, { 'x-hasura-user-id': '1', 'x-hasura-allowed-roles': ['user'], - } + }) token = jwt.encode(self.claims, hge_ctx.hge_jwt_key, algorithm='RS512').decode('utf-8') self.conf['headers']['Authorization'] = 'Bearer ' + token self.conf['response'] = { @@ -111,11 +127,11 @@ def test_jwt_no_default_role(self, hge_ctx): check_query(hge_ctx, self.conf, add_auth=False) def test_jwt_expired(self, hge_ctx): - self.claims['https://hasura.io/jwt/claims'] = { + self.claims['https://hasura.io/jwt/claims'] = mk_claims(hge_ctx.hge_jwt_conf, { 'x-hasura-user-id': '1', 'x-hasura-default-role': 'user', 'x-hasura-allowed-roles': ['user'], - } + }) exp = datetime.now() - timedelta(minutes=1) self.claims['exp'] = round(exp.timestamp()) @@ -134,11 +150,11 @@ def test_jwt_expired(self, hge_ctx): check_query(hge_ctx, self.conf, add_auth=False) def test_jwt_invalid_signature(self, hge_ctx): - self.claims['https://hasura.io/jwt/claims'] = { + self.claims['https://hasura.io/jwt/claims'] = mk_claims(hge_ctx.hge_jwt_conf, { 'x-hasura-user-id': '1', 'x-hasura-default-role': 'user', 'x-hasura-allowed-roles': ['user'], - } + }) wrong_key = gen_rsa_key() token = jwt.encode(self.claims, wrong_key, algorithm='HS256').decode('utf-8') @@ -155,24 +171,24 @@ def test_jwt_invalid_signature(self, hge_ctx): self.conf['status'] = 400 check_query(hge_ctx, self.conf, add_auth=False) - def test_jwt_stringified_claims(self, hge_ctx): - print(hge_ctx.hge_jwt_conf) - conf = json.loads(hge_ctx.hge_jwt_conf) - if 'is_stringified' not in conf: - pytest.skip('is_stringified is false, skipping testing stringified claims.') - if 'is_stringified' in conf and not conf['is_stringified']: - pytest.skip('is_stringified is false, skipping testing stringified claims.') - - self.claims['https://hasura.io/jwt/claims'] = json.dumps({ - 'x-hasura-user-id': '1', - 'x-hasura-default-role': 'user', - 'x-hasura-allowed-roles': ['user'], - }) - - token = jwt.encode(self.claims, hge_ctx.hge_jwt_key, algorithm='RS512').decode('utf-8') - self.conf['headers']['Authorization'] = 'Bearer ' + token - self.conf['status'] = 200 - check_query(hge_ctx, self.conf, add_auth=False) + # def test_jwt_stringified_claims(self, hge_ctx): + # print(hge_ctx.hge_jwt_conf) + # conf = json.loads(hge_ctx.hge_jwt_conf) + # if 'claims_format' not in conf: + # pytest.skip('claims_format is not stringified_json, skipping testing stringified claims.') + # if 'claims_format' in conf and conf['claims_format'] != 'stringified_json': + # pytest.skip('claims_format is not stringified_json, skipping testing stringified claims.') + + # self.claims['https://hasura.io/jwt/claims'] = mk_claims(hge_ctx.hge_jwt_conf, { + # 'x-hasura-user-id': '1', + # 'x-hasura-default-role': 'user', + # 'x-hasura-allowed-roles': ['user'], + # }) + + # token = jwt.encode(self.claims, hge_ctx.hge_jwt_key, algorithm='RS512').decode('utf-8') + # self.conf['headers']['Authorization'] = 'Bearer ' + token + # self.conf['status'] = 200 + # check_query(hge_ctx, self.conf, add_auth=False) @pytest.fixture(autouse=True) def transact(self, request, hge_ctx): From a6b0a1dfc7110477ebbfb0c1d144facf741a03a3 Mon Sep 17 00:00:00 2001 From: Anon Ray Date: Tue, 5 Feb 2019 16:15:53 +0530 Subject: [PATCH 3/3] remove references of stringify and better error message --- server/src-lib/Hasura/Server/Auth/JWT.hs | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/server/src-lib/Hasura/Server/Auth/JWT.hs b/server/src-lib/Hasura/Server/Auth/JWT.hs index bcb0986ca1a22..674b7b4fa533b 100644 --- a/server/src-lib/Hasura/Server/Auth/JWT.hs +++ b/server/src-lib/Hasura/Server/Auth/JWT.hs @@ -76,8 +76,8 @@ data JWTCtx } deriving (Eq) instance Show JWTCtx where - show (JWTCtx _ nsM audM strfd) = - show ["", show nsM, show audM, show strfd] + show (JWTCtx _ nsM audM cf) = + show ["", show nsM, show audM, show cf] data HasuraClaims = HasuraClaims @@ -238,17 +238,19 @@ processAuthZHeader jwtCtx headers authzHeader = do parseObjectFromString claimsFmt jVal = case (claimsFmt, jVal) of (JCFStringifiedJson, A.String v) -> - either (const $ throw400 JWTInvalidClaims $ strngfdErr <> v) return + either (const $ claimsErr $ strngfyErr v) return $ A.eitherDecodeStrict $ T.encodeUtf8 v (JCFStringifiedJson, _) -> - throw400 JWTInvalidClaims "expecting a string when claims_format is stringified_json" + claimsErr "expecting a string when claims_format is stringified_json" (JCFJson, A.Object o) -> return o (JCFJson, _) -> - throw400 JWTInvalidClaims "expecting a json object when claims_format is json" + claimsErr "expecting a json object when claims_format is json" - strngfdErr = "Could not parse JSON string under: '" - <> fromMaybe defaultClaimNs (jcxClaimNs jwtCtx) - <> "'. When stringified, the claims inside should be a JSON object, but found: " + strngfyErr v = "expecting stringified json at: '" + <> fromMaybe defaultClaimNs (jcxClaimNs jwtCtx) + <> "', but found: " <> v + + claimsErr = throw400 JWTInvalidClaims -- see if there is a x-hasura-role header, or else pick the default role getCurrentRole defaultRole =