From 08f2bec188b0a26a036d3cc70d6b6c5a5a80d863 Mon Sep 17 00:00:00 2001 From: Anon Ray Date: Wed, 5 Feb 2020 07:07:31 +0000 Subject: [PATCH] fix parsing JWK expiry time from headers on startup (fix #3655) (#3779) --- .circleci/test-server.sh | 22 +++ .../ApiExplorer/Analyzer/QueryAnalyzer.js | 10 +- .../components/Services/ApiExplorer/utils.js | 4 +- .../PermissionsSummary/PermissionsSummary.js | 20 ++- .../components/Services/Data/RawSQL/RawSQL.js | 8 +- .../components/Services/Data/Schema/Schema.js | 12 +- .../Data/TableBrowseRows/FilterQuery.js | 8 +- .../Data/TableModify/ModifyActions.js | 28 +++- .../Data/TableRelationships/Relationships.js | 8 +- .../TableRelationships/RelationshipsView.js | 4 +- console/src/components/Services/Data/utils.js | 4 +- .../Services/EventTrigger/Add/AddTrigger.js | 32 +++- .../EventTrigger/Landing/EventTrigger.js | 4 +- .../RemoteSchema/Landing/RemoteSchema.js | 4 +- .../manual/auth/authentication/jwt.rst | 45 ++++- server/src-lib/Hasura/Server/Auth.hs | 39 +++-- server/src-lib/Hasura/Server/Auth/JWT.hs | 155 +++++++++--------- .../src-lib/Hasura/Server/Auth/JWT/Logging.hs | 71 ++++---- server/tests-py/jwk_server.py | 6 + 19 files changed, 311 insertions(+), 173 deletions(-) diff --git a/.circleci/test-server.sh b/.circleci/test-server.sh index e33a5f49d33421..eabd8cd1b4fcc3 100755 --- a/.circleci/test-server.sh +++ b/.circleci/test-server.sh @@ -561,6 +561,8 @@ wait_for_port 5001 cache_control_jwk_url='{"type": "RS256", "jwk_url": "http://localhost:5001/jwk-cache-control"}' expires_jwk_url='{"type": "RS256", "jwk_url": "http://localhost:5001/jwk-expires"}' +cc_nomaxage_jwk_url='{"type": "RS256", "jwk_url": "http://localhost:5001/jwk-cache-control?nomaxage"}' +cc_nocache_jwk_url='{"type": "RS256", "jwk_url": "http://localhost:5001/jwk-cache-control?nocache"}' # start HGE with cache control JWK URL export HASURA_GRAPHQL_JWT_SECRET=$cache_control_jwk_url @@ -596,6 +598,26 @@ pytest -n 1 -vv --hge-urls "$HGE_URL" --pg-urls "$HASURA_GRAPHQL_DATABASE_URL" - kill_hge_servers +# start HGE with nomaxage JWK URL +export HASURA_GRAPHQL_JWT_SECRET=$cc_nomaxage_jwk_url +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" --test-jwk-url test_jwk.py -k 'test_cache_control_header' + +kill_hge_servers +unset HASURA_GRAPHQL_JWT_SECRET + +# start HGE with nocache JWK URL +export HASURA_GRAPHQL_JWT_SECRET=$cc_nocache_jwk_url +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" --test-jwk-url test_jwk.py -k 'test_cache_control_header' + +kill_hge_servers +unset HASURA_GRAPHQL_JWT_SECRET + kill $JWKS_PID # end jwk url test diff --git a/console/src/components/Services/ApiExplorer/Analyzer/QueryAnalyzer.js b/console/src/components/Services/ApiExplorer/Analyzer/QueryAnalyzer.js index 46b06f07bfed50..7f0bf64707c030 100644 --- a/console/src/components/Services/ApiExplorer/Analyzer/QueryAnalyzer.js +++ b/console/src/components/Services/ApiExplorer/Analyzer/QueryAnalyzer.js @@ -144,8 +144,8 @@ export default class QueryAnalyser extends React.Component { {this.state.activeNode >= 0 && this.state.analyseData.length > 0 ? this.state.analyseData[ - this.state.activeNode - ].plan.join('\n') + this.state.activeNode + ].plan.join('\n') : ''} @@ -183,9 +183,9 @@ export default class QueryAnalyser extends React.Component { if (type === 'sql') { text = window.sqlFormatter ? window.sqlFormatter.format( - this.state.analyseData[this.state.activeNode].sql, - { language: 'sql' } - ) + this.state.analyseData[this.state.activeNode].sql, + { language: 'sql' } + ) : this.state.analyseData[this.state.activeNode].sql; } else { text = this.state.analyseData[this.state.activeNode].plan.join('\n'); diff --git a/console/src/components/Services/ApiExplorer/utils.js b/console/src/components/Services/ApiExplorer/utils.js index 24bcfa956ef615..3ed44f2e5f52ba 100644 --- a/console/src/components/Services/ApiExplorer/utils.js +++ b/console/src/components/Services/ApiExplorer/utils.js @@ -13,8 +13,6 @@ export const getHeadersAsJSON = (headers = []) => { export const isValidGraphQLOperation = operation => { return ( - operation.name && - operation.name.value && - operation.operation === 'query' + operation.name && operation.name.value && operation.operation === 'query' ); }; diff --git a/console/src/components/Services/Data/PermissionsSummary/PermissionsSummary.js b/console/src/components/Services/Data/PermissionsSummary/PermissionsSummary.js index 9ee8efe8b0c889..1769d6bfed3d74 100644 --- a/console/src/components/Services/Data/PermissionsSummary/PermissionsSummary.js +++ b/console/src/components/Services/Data/PermissionsSummary/PermissionsSummary.js @@ -496,7 +496,9 @@ class PermissionsSummary extends Component { const getTablesColumnTable = () => { return ( {getBackBtn('currTable')} @@ -626,7 +628,9 @@ class PermissionsSummary extends Component { return (
@@ -663,7 +667,9 @@ class PermissionsSummary extends Component { return (
{getRolesHeaderRow()}
@@ -694,7 +700,9 @@ class PermissionsSummary extends Component { return ( {getActionsHeaderRow()}{getRoleAllTablesAllActionsRows()} @@ -1037,7 +1045,9 @@ class PermissionsSummary extends Component { return (
diff --git a/console/src/components/Services/Data/RawSQL/RawSQL.js b/console/src/components/Services/Data/RawSQL/RawSQL.js index 93e57bdc4c03c9..210788aeef6362 100644 --- a/console/src/components/Services/Data/RawSQL/RawSQL.js +++ b/console/src/components/Services/Data/RawSQL/RawSQL.js @@ -284,7 +284,9 @@ const RawSQL = ({

SQL Result:

{getTableHeadings()} @@ -506,7 +508,9 @@ const RawSQL = ({
{getTrackThisSection()} {getMetadataCascadeSection()} diff --git a/console/src/components/Services/Data/Schema/Schema.js b/console/src/components/Services/Data/Schema/Schema.js index bff7e151b62d7c..75b98e4818a5cc 100644 --- a/console/src/components/Services/Data/Schema/Schema.js +++ b/console/src/components/Services/Data/Schema/Schema.js @@ -554,7 +554,9 @@ class Schema extends Component {
@@ -365,7 +367,9 @@ class Relationships extends Component { addedRelationshipsView = (
diff --git a/console/src/components/Services/Data/TableRelationships/RelationshipsView.js b/console/src/components/Services/Data/TableRelationships/RelationshipsView.js index 3078691d8c835f..073abba1bd513b 100644 --- a/console/src/components/Services/Data/TableRelationships/RelationshipsView.js +++ b/console/src/components/Services/Data/TableRelationships/RelationshipsView.js @@ -82,7 +82,9 @@ class RelationshipsView extends Component { addedRelationshipsView = (
diff --git a/console/src/components/Services/Data/utils.js b/console/src/components/Services/Data/utils.js index 99993f52f58c40..45c3a79ede123c 100644 --- a/console/src/components/Services/Data/utils.js +++ b/console/src/components/Services/Data/utils.js @@ -293,7 +293,9 @@ const generateWhereClause = options => { if (options.tables) { options.tables.forEach(tableInfo => { whereCondtions.push( - `(ist.table_schema='${tableInfo.table_schema}' and ist.table_name='${tableInfo.table_name}')` + `(ist.table_schema='${tableInfo.table_schema}' and ist.table_name='${ + tableInfo.table_name + }')` ); }); } diff --git a/console/src/components/Services/EventTrigger/Add/AddTrigger.js b/console/src/components/Services/EventTrigger/Add/AddTrigger.js index 4d557fcd79d105..b3047360aba760 100644 --- a/console/src/components/Services/EventTrigger/Add/AddTrigger.js +++ b/console/src/components/Services/EventTrigger/Add/AddTrigger.js @@ -357,7 +357,9 @@ class AddTrigger extends Component { return (
@@ -368,7 +370,9 @@ class AddTrigger extends Component {

Trigger Name     @@ -515,7 +519,9 @@ class AddTrigger extends Component {
@@ -526,7 +532,9 @@ class AddTrigger extends Component { dispatch(setRetryNum(e.target.value)); }} data-test="no-of-retries" - className={`${styles.display_inline} form-control ${styles.width300}`} + className={`${styles.display_inline} form-control ${ + styles.width300 + }`} type="text" placeholder="no of retries" /> @@ -535,7 +543,9 @@ class AddTrigger extends Component {
@@ -546,7 +556,9 @@ class AddTrigger extends Component { dispatch(setRetryInterval(e.target.value)); }} data-test="interval-seconds" - className={`${styles.display_inline} form-control ${styles.width300}`} + className={`${styles.display_inline} form-control ${ + styles.width300 + }`} type="text" placeholder="interval time in seconds" /> @@ -555,7 +567,9 @@ class AddTrigger extends Component {
@@ -566,7 +580,9 @@ class AddTrigger extends Component { dispatch(setRetryTimeout(e.target.value)); }} data-test="timeout-seconds" - className={`${styles.display_inline} form-control ${styles.width300}`} + className={`${styles.display_inline} form-control ${ + styles.width300 + }`} type="text" placeholder="timeout in seconds" /> diff --git a/console/src/components/Services/EventTrigger/Landing/EventTrigger.js b/console/src/components/Services/EventTrigger/Landing/EventTrigger.js index eff780eb8c248e..6eb3580ffb1c31 100644 --- a/console/src/components/Services/EventTrigger/Landing/EventTrigger.js +++ b/console/src/components/Services/EventTrigger/Landing/EventTrigger.js @@ -84,7 +84,9 @@ class EventTrigger extends Component { return (
diff --git a/console/src/components/Services/RemoteSchema/Landing/RemoteSchema.js b/console/src/components/Services/RemoteSchema/Landing/RemoteSchema.js index ac7ba4d6fa8bf7..f90dd3bc756564 100644 --- a/console/src/components/Services/RemoteSchema/Landing/RemoteSchema.js +++ b/console/src/components/Services/RemoteSchema/Landing/RemoteSchema.js @@ -57,7 +57,9 @@ class RemoteSchema extends React.Component { return (
diff --git a/docs/graphql/manual/auth/authentication/jwt.rst b/docs/graphql/manual/auth/authentication/jwt.rst index 6626fea804fc51..9760da1f8ab1f6 100644 --- a/docs/graphql/manual/auth/authentication/jwt.rst +++ b/docs/graphql/manual/auth/authentication/jwt.rst @@ -29,7 +29,7 @@ If the authorization passes, then all of the ``x-hasura-*`` values in the claim are used for the permissions system. .. admonition:: Prerequisite - + It is mandatory to first :doc:`secure your GraphQL endpoint <../../deployment/securing-graphql-endpoint>` for the JWT mode to take effect. @@ -164,14 +164,45 @@ https://tools.ietf.org/html/rfc7517. This is an optional field. You can also provide the key (certificate, PEM encoded public key) as a string - under the ``key`` field. -**Rotating JWKs**: +Rotating JWKs ++++++++++++++ + +Some providers rotate their JWKs (e.g. Firebase). If the provider sends + +1. ``max-age`` or ``s-maxage`` in ``Cache-Control`` header +2. or ``Expires`` header + +with the response of JWK, then the GraphQL engine will refresh the JWKs automatically. If the +provider does not send the above, the JWKs are not refreshed. + +Following is the behaviour in detail: + +**On startup**: + +1. GraphQL engine will fetch the JWK and will - + + 1. first, try to parse ``max-age`` or ``s-maxage`` directive in ``Cache-Control`` header. + 2. second, check if ``Expires`` header is present (if ``Cache-Control`` is not present), and try + to parse the value as a timestamp. + +2. If it is able to parse any of the above successfully, then it will use that parsed time to + refresh/refetch the JWKs again. If it is unable to parse, then it will not refresh the JWKs (it + assumes that if the above headers are not present, the provider doesn't rotate their JWKs). + +**While running**: + +1. While GraphQL engine is running with refreshing JWKs, in one of the refresh cycles it will - + + 1. first, try to parse ``max-age`` or ``s-maxage`` directive in ``Cache-Control`` header. + 2. second, check if ``Expires`` header is present (if ``Cache-Control`` is not present), and try + to parse the value as a timestamp. -Some providers rotate their JWKs (e.g. Firebase). If the provider sends an -``Expires`` header with the response of JWK, then the GraphQL engine will refresh -the JWKs automatically. If the provider does not send an ``Expires`` header, the -JWKs are not refreshed. +2. If it is able to parse any of the above successfully, then it will use that parsed time to + refresh/refetch the JWKs again. If it is unable to parse, then it will sleep for 1 minute and + will start another refresh cycle. -**Example**: +Example JWK URL ++++++++++++++++ - Auth0 publishes their JWK url at: ``https://.auth0.com``. But Auth0 has a bug. See known issues: :ref:`auth0-issues`. diff --git a/server/src-lib/Hasura/Server/Auth.hs b/server/src-lib/Hasura/Server/Auth.hs index 9c28a0b0550f4b..af04c6f7e08096 100644 --- a/server/src-lib/Hasura/Server/Auth.hs +++ b/server/src-lib/Hasura/Server/Auth.hs @@ -1,3 +1,4 @@ +{-# LANGUAGE RecordWildCards #-} module Hasura.Server.Auth ( getUserInfo , getUserInfoWithExpTime @@ -119,6 +120,7 @@ mkAuthMode mAdminSecret mWebHook mJwtSecret mUnAuthRole httpManager logger = "Fatal Error: --unauthorized-role (HASURA_GRAPHQL_UNAUTHORIZED_ROLE) is not allowed" <> " when --auth-hook (HASURA_GRAPHQL_AUTH_HOOK) is set" +-- | Given the 'JWTConfig' (the user input of JWT configuration), create the 'JWTCtx' (the runtime JWT config used) mkJwtCtx :: ( HasVersion , MonadIO m @@ -128,20 +130,37 @@ mkJwtCtx -> H.Manager -> Logger Hasura -> m JWTCtx -mkJwtCtx conf httpManager logger = do - jwkRef <- case jcKeyOrUrl conf of +mkJwtCtx JWTConfig{..} httpManager logger = do + jwkRef <- case jcKeyOrUrl of Left jwk -> liftIO $ newIORef (JWKSet [jwk]) - Right url -> do + Right url -> getJwkFromUrl url + let claimsFmt = fromMaybe JCFJson jcClaimsFormat + return $ JWTCtx jwkRef jcClaimNs jcAudience claimsFmt jcIssuer + 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 + getJwkFromUrl url = do ref <- liftIO $ newIORef $ JWKSet [] - mTime <- updateJwkRef logger httpManager url ref - case mTime of - Nothing -> return ref - Just t -> do - jwkRefreshCtrl logger httpManager url ref (fromUnits t) + maybeExpiry <- withJwkError $ updateJwkRef logger httpManager url ref + case maybeExpiry of + Nothing -> return ref + Just time -> do + jwkRefreshCtrl logger httpManager url ref (fromUnits time) return ref - let claimsFmt = fromMaybe JCFJson (jcClaimsFormat conf) - return $ JWTCtx jwkRef (jcClaimNs conf) (jcAudience conf) claimsFmt (jcIssuer conf) + withJwkError act = do + res <- runExceptT act + case res of + Right r -> return r + Left err -> case err of + -- when fetching JWK initially, except expiry parsing error, all errors are critical + JFEHttpException _ msg -> throwError msg + JFEHttpError _ _ _ e -> throwError e + JFEJwkParseError _ e -> throwError e + JFEExpiryParseError _ _ -> return Nothing + + +-- | Form the 'UserInfo' from the response from webhook mkUserInfoFromResp :: (MonadIO m, MonadError QErr m) => Logger Hasura diff --git a/server/src-lib/Hasura/Server/Auth/JWT.hs b/server/src-lib/Hasura/Server/Auth/JWT.hs index 09b1080a6a9f32..c63ce27ca82bf4 100644 --- a/server/src-lib/Hasura/Server/Auth/JWT.hs +++ b/server/src-lib/Hasura/Server/Auth/JWT.hs @@ -5,6 +5,7 @@ module Hasura.Server.Auth.JWT , JWTCtx (..) , Jose.JWKSet (..) , JWTClaimsFormat (..) + , JwkFetchError (..) , updateJwkRef , jwkRefreshCtrl , defaultClaimNs @@ -13,7 +14,7 @@ module Hasura.Server.Auth.JWT import Control.Exception (try) import Control.Lens import Control.Monad (when) -import Data.IORef (IORef, modifyIORef, readIORef) +import Data.IORef (IORef, readIORef, writeIORef) import Data.List (find) import Data.Parser.CacheControl (parseMaxAge) import Data.Time.Clock (NominalDiffTime, UTCTime, diffUTCTime, @@ -32,9 +33,9 @@ import Hasura.Server.Version (HasVersion) import qualified Control.Concurrent.Extended as C import qualified Crypto.JWT as Jose -import qualified Data.Aeson as A -import qualified Data.Aeson.Casing as A -import qualified Data.Aeson.TH as A +import qualified Data.Aeson as J +import qualified Data.Aeson.Casing as J +import qualified Data.Aeson.TH as J import qualified Data.ByteString.Lazy as BL import qualified Data.ByteString.Lazy.Char8 as BLC import qualified Data.CaseInsensitive as CI @@ -54,9 +55,8 @@ data JWTClaimsFormat | JCFStringifiedJson deriving (Show, Eq) -$(A.deriveJSON A.defaultOptions { A.sumEncoding = A.ObjectWithSingleField - , A.constructorTagModifier = A.snakeCase . drop 3 - } ''JWTClaimsFormat) +$(J.deriveJSON J.defaultOptions { J.sumEncoding = J.ObjectWithSingleField + , J.constructorTagModifier = J.snakeCase . drop 3 } ''JWTClaimsFormat) data JWTConfig = JWTConfig @@ -86,7 +86,7 @@ data HasuraClaims { _cmAllowedRoles :: ![RoleName] , _cmDefaultRole :: !RoleName } deriving (Show, Eq) -$(A.deriveJSON (A.aesonDrop 3 A.snakeCase) ''HasuraClaims) +$(J.deriveJSON (J.aesonDrop 3 J.snakeCase) ''HasuraClaims) allowedRolesClaim :: T.Text allowedRolesClaim = "x-hasura-allowed-roles" @@ -97,17 +97,6 @@ defaultRoleClaim = "x-hasura-default-role" defaultClaimNs :: T.Text defaultClaimNs = "https://hasura.io/jwt/claims" --- | if the time is greater than 100 seconds, should refresh the JWK 10 seonds --- before the expiry, else refresh at given interval. --- --- Apparently we want to make sure to refresh the JWKs preemptively so we're --- not verifying the signature of JWTs with a too old JWK, while the other case --- (interval <= 100 sec) is useful for development but someone who knows needs --- TODO a proper job documenting this. -computeDiffTime :: DiffTime -> DiffTime -computeDiffTime t = - if t > seconds 100 then t - seconds 10 else t - -- | create a background thread to refresh the JWK jwkRefreshCtrl :: (HasVersion, MonadIO m) @@ -124,19 +113,18 @@ jwkRefreshCtrl logger manager url ref time = res <- runExceptT $ updateJwkRef logger manager url ref mTime <- either (const $ logNotice >> return Nothing) return res -- if can't parse time from header, defaults to 1 min - let delay = maybe (minutes 1) (computeDiffTime . fromUnits) mTime + let delay = maybe (minutes 1) (fromUnits) mTime C.sleep delay where logNotice = do - let err = JwkRefreshLog LevelInfo (JLNInfo "retrying again in 60 secs") Nothing + let err = JwkRefreshLog LevelInfo (Just "retrying again in 60 secs") Nothing liftIO $ unLogger logger err - -- | Given a JWK url, fetch JWK from it and update the IORef updateJwkRef :: ( HasVersion , MonadIO m - , MonadError T.Text m + , MonadError JwkFetchError m ) => Logger Hasura -> HTTP.Manager @@ -146,22 +134,22 @@ updateJwkRef updateJwkRef (Logger logger) manager url jwkRef = do let options = wreqOptions manager [] urlT = T.pack $ show url - infoMsg = JLNInfo $ "refreshing JWK from endpoint: " <> urlT - liftIO $ logger $ JwkRefreshLog LevelInfo infoMsg Nothing + infoMsg = "refreshing JWK from endpoint: " <> urlT + liftIO $ logger $ JwkRefreshLog LevelInfo (Just infoMsg) Nothing res <- liftIO $ try $ Wreq.getWith options $ show url resp <- either logAndThrowHttp return res let status = resp ^. Wreq.responseStatus respBody = resp ^. Wreq.responseBody + statusCode = status ^. Wreq.statusCode - when (status ^. Wreq.statusCode /= 200) $ do - let respBodyT = Just $ CS.cs respBody - errMsg = "Non-200 response on fetching JWK from: " <> urlT - httpErr = Just (JwkRefreshHttpError (Just status) urlT Nothing respBodyT) - logAndThrow errMsg httpErr + unless (statusCode >= 200 && statusCode < 300) $ do + let errMsg = "Non-2xx response on fetching JWK from: " <> urlT + err = JFEHttpError url status respBody errMsg + logAndThrow err - let parseErr e = "Error parsing JWK from url (" <> urlT <> "): " <> T.pack e - jwkset <- either (\e -> logAndThrow (parseErr e) Nothing) return $ A.eitherDecode respBody - liftIO $ modifyIORef jwkRef (const jwkset) + let parseErr e = JFEJwkParseError (T.pack e) $ "Error parsing JWK from url: " <> urlT + jwkset <- either (logAndThrow . parseErr) return $ J.eitherDecode respBody + liftIO $ writeIORef jwkRef jwkset -- first check for Cache-Control header to get max-age, if not found, look for Expires header let cacheHeader = resp ^? Wreq.responseHeader "Cache-Control" @@ -172,42 +160,47 @@ updateJwkRef (Logger logger) manager url jwkRef = do where getTimeFromExpiresHeader header = do - let maybeExpires = parseTimeM True defaultTimeLocale timeFmt $ CS.cs header - expires <- maybe (logAndThrow parseTimeErr Nothing) return maybeExpires + let maybeExpiry = parseTimeM True defaultTimeLocale timeFmt (CS.cs header) + expires <- maybe (logAndThrowInfo parseTimeErr) return maybeExpiry currTime <- liftIO getCurrentTime return $ diffUTCTime expires currTime getTimeFromCacheControlHeader header = case parseCacheControlHeader (bsToTxt header) of - Left e -> logAndThrow e Nothing + Left e -> logAndThrowInfo e Right maxAge -> return $ Just $ fromInteger maxAge - parseCacheControlHeader = fmapL (const parseCacheControlErr) . parseMaxAge + parseCacheControlHeader = fmapL (parseCacheControlErr . T.pack) . parseMaxAge - parseCacheControlErr = + parseCacheControlErr e = + JFEExpiryParseError (Just e) "Failed parsing Cache-Control header from JWK response. Could not find max-age or s-maxage" parseTimeErr = + JFEExpiryParseError Nothing "Failed parsing Expires header from JWK response. Value of header is not a valid timestamp" - logAndThrow - :: (MonadIO m, MonadError T.Text m) - => T.Text -> Maybe JwkRefreshHttpError -> m a - logAndThrow err httpErr = do - liftIO $ logger $ JwkRefreshLog (LevelOther "critical") (JLNError err) httpErr + timeFmt = "%a, %d %b %Y %T GMT" + + logAndThrowInfo :: (MonadIO m, MonadError JwkFetchError m) => JwkFetchError -> m a + logAndThrowInfo err = do + liftIO $ logger $ JwkRefreshLog LevelInfo Nothing (Just err) throwError err - logAndThrowHttp :: (MonadIO m, MonadError T.Text m) => HTTP.HttpException -> m a - logAndThrowHttp err = do - let httpErr = JwkRefreshHttpError Nothing (T.pack $ show url) (Just $ HttpException err) Nothing - errMsg = "Error fetching JWK: " <> T.pack (getHttpExceptionMsg err) - logAndThrow errMsg (Just httpErr) + logAndThrow :: (MonadIO m, MonadError JwkFetchError m) => JwkFetchError -> m a + logAndThrow err = do + liftIO $ logger $ JwkRefreshLog (LevelOther "critical") Nothing (Just err) + throwError err + + logAndThrowHttp :: (MonadIO m, MonadError JwkFetchError m) => HTTP.HttpException -> m a + logAndThrowHttp httpEx = do + let errMsg = "Error fetching JWK: " <> T.pack (getHttpExceptionMsg httpEx) + err = JFEHttpException (HttpException httpEx) errMsg + logAndThrow err getHttpExceptionMsg = \case HTTP.HttpExceptionRequest _ reason -> show reason HTTP.InvalidUrlException _ reason -> show reason - timeFmt = "%a, %d %b %Y %T GMT" - -- | Process the request headers to verify the JWT and extract UserInfo from it processJwt @@ -271,7 +264,7 @@ processAuthZHeader jwtCtx headers authzHeader = do Map.delete defaultRoleClaim . Map.delete allowedRolesClaim $ claimsMap -- transform the map of text:aeson-value -> text:text - metadata <- decodeJSON $ A.Object finalClaims + metadata <- decodeJSON $ J.Object finalClaims return $ (, expTimeM) $ mkUserInfo role $ mkUserVars $ Map.toList metadata @@ -284,12 +277,12 @@ processAuthZHeader jwtCtx headers authzHeader = do parseObjectFromString claimsFmt jVal = case (claimsFmt, jVal) of - (JCFStringifiedJson, A.String v) -> + (JCFStringifiedJson, J.String v) -> either (const $ claimsErr $ strngfyErr v) return - $ A.eitherDecodeStrict $ T.encodeUtf8 v + $ J.eitherDecodeStrict $ T.encodeUtf8 v (JCFStringifiedJson, _) -> claimsErr "expecting a string when claims_format is stringified_json" - (JCFJson, A.Object o) -> return o + (JCFJson, J.Object o) -> return o (JCFJson, _) -> claimsErr "expecting a json object when claims_format is json" @@ -305,9 +298,9 @@ processAuthZHeader jwtCtx headers authzHeader = do mUserRole = snd <$> find (\h -> fst h == CI.mk userRoleHeaderB) headers in maybe defaultRole RoleName $ mUserRole >>= mkNonEmptyText . bsToTxt - decodeJSON val = case A.fromJSON val of - A.Error e -> throw400 JWTInvalidClaims ("x-hasura-* claims: " <> T.pack e) - A.Success a -> return a + decodeJSON val = case J.fromJSON val of + J.Error e -> throw400 JWTInvalidClaims ("x-hasura-* claims: " <> T.pack e) + J.Success a -> return a liftJWTError :: (MonadError e' m) => (e -> e') -> ExceptT e m a -> m a liftJWTError ef action = do @@ -329,15 +322,15 @@ processAuthZHeader jwtCtx headers authzHeader = do -- parse x-hasura-allowed-roles, x-hasura-default-role from JWT claims parseHasuraClaims :: (MonadError QErr m) - => A.Object -> m HasuraClaims + => J.Object -> m HasuraClaims parseHasuraClaims claimsMap = do let mAllowedRolesV = Map.lookup allowedRolesClaim claimsMap allowedRolesV <- maybe missingAllowedRolesClaim return mAllowedRolesV - allowedRoles <- parseJwtClaim (A.fromJSON allowedRolesV) errMsg + allowedRoles <- parseJwtClaim (J.fromJSON allowedRolesV) errMsg let mDefaultRoleV = Map.lookup defaultRoleClaim claimsMap defaultRoleV <- maybe missingDefaultRoleClaim return mDefaultRoleV - defaultRole <- parseJwtClaim (A.fromJSON defaultRoleV) errMsg + defaultRole <- parseJwtClaim (J.fromJSON defaultRoleV) errMsg return $ HasuraClaims allowedRoles defaultRole @@ -352,11 +345,11 @@ parseHasuraClaims claimsMap = do errMsg _ = "invalid " <> allowedRolesClaim <> "; should be a list of roles" - parseJwtClaim :: (MonadError QErr m) => A.Result a -> (String -> Text) -> m a + parseJwtClaim :: (MonadError QErr m) => J.Result a -> (String -> Text) -> m a parseJwtClaim res errFn = case res of - A.Success val -> return val - A.Error e -> throw400 JWTInvalidClaims $ errFn e + J.Success val -> return val + J.Error e -> throw400 JWTInvalidClaims $ errFn e -- | Verify the JWT against given JWK @@ -384,33 +377,33 @@ verifyJwt ctx (RawJWT rawJWT) = do Just (Jose.Audience audiences) -> audience `elem` audiences -instance A.ToJSON JWTConfig where +instance J.ToJSON JWTConfig where toJSON (JWTConfig ty keyOrUrl claimNs aud claimsFmt iss) = case keyOrUrl of - Left _ -> mkObj ("key" A..= A.String "") - Right url -> mkObj ("jwk_url" A..= url) + Left _ -> mkObj ("key" J..= J.String "") + Right url -> mkObj ("jwk_url" J..= url) where - mkObj item = A.object [ "type" A..= ty - , "claims_namespace" A..= claimNs - , "claims_format" A..= claimsFmt - , "audience" A..= aud - , "issuer" A..= iss + mkObj item = J.object [ "type" J..= ty + , "claims_namespace" J..= claimNs + , "claims_format" J..= claimsFmt + , "audience" J..= aud + , "issuer" J..= iss , item ] -- | Parse from a json string like: -- | `{"type": "RS256", "key": ""}` -- | to JWTConfig -instance A.FromJSON JWTConfig where - - parseJSON = A.withObject "JWTConfig" $ \o -> do - keyType <- o A..: "type" - mRawKey <- o A..:? "key" - claimNs <- o A..:? "claims_namespace" - aud <- o A..:? "audience" - iss <- o A..:? "issuer" - jwkUrl <- o A..:? "jwk_url" - isStrngfd <- o A..:? "claims_format" +instance J.FromJSON JWTConfig where + + parseJSON = J.withObject "JWTConfig" $ \o -> do + keyType <- o J..: "type" + mRawKey <- o J..:? "key" + claimNs <- o J..:? "claims_namespace" + aud <- o J..:? "audience" + iss <- o J..:? "issuer" + jwkUrl <- o J..:? "jwk_url" + isStrngfd <- o J..:? "claims_format" case (mRawKey, jwkUrl) of (Nothing, Nothing) -> fail "key and jwk_url both cannot be empty" diff --git a/server/src-lib/Hasura/Server/Auth/JWT/Logging.hs b/server/src-lib/Hasura/Server/Auth/JWT/Logging.hs index 9025f69d246a4f..bd2216a50ac1e1 100644 --- a/server/src-lib/Hasura/Server/Auth/JWT/Logging.hs +++ b/server/src-lib/Hasura/Server/Auth/JWT/Logging.hs @@ -1,60 +1,63 @@ module Hasura.Server.Auth.JWT.Logging ( JwkRefreshLog (..) - , JwkRefreshHttpError (..) - , JwkLogNotice (..) + , JwkFetchError (..) ) where import Data.Aeson +import Network.URI (URI) import Hasura.HTTP import Hasura.Logging (EngineLogType (..), Hasura, InternalLogTypes (..), LogLevel (..), ToEngineLog (..)) import Hasura.Prelude import Hasura.Server.Logging () -import Hasura.Server.Utils (httpExceptToJSON) +import qualified Data.ByteString.Lazy as BL import qualified Data.Text as T import qualified Network.HTTP.Types as HTTP - -data JwkLogNotice - = JLNInfo !Text - | JLNError !Text +-- | Possible errors during fetching and parsing JWK +-- (the 'Text' type at the end is a friendly error message) +data JwkFetchError + = JFEHttpException !HttpException !Text + -- ^ Exception while making the HTTP request + | JFEHttpError !URI !HTTP.Status !BL.ByteString !Text + -- ^ Non-2xx HTTP errors from the upstream server + | JFEJwkParseError !Text !Text + -- ^ Error parsing the JWK response itself + | JFEExpiryParseError !(Maybe Text) Text + -- ^ Error parsing the expiry of the JWK deriving (Show) +instance ToJSON JwkFetchError where + toJSON = \case + JFEHttpException e _ -> + object [ "http_exception" .= e ] + + JFEHttpError url status body _ -> + object [ "status_code" .= HTTP.statusCode status + , "url" .= T.pack (show url) + , "response" .= bsToTxt (BL.toStrict body) + ] + + JFEJwkParseError e msg -> + object [ "error" .= e, "message" .= msg ] + + JFEExpiryParseError e msg -> + object [ "error" .= e, "message" .= msg ] + data JwkRefreshLog = JwkRefreshLog - { jrlLogLevel :: !LogLevel - , jrlNotice :: !JwkLogNotice - , jrlHttpError :: !(Maybe JwkRefreshHttpError) - } deriving (Show) - -data JwkRefreshHttpError - = JwkRefreshHttpError - { jrheStatus :: !(Maybe HTTP.Status) - , jrheUrl :: !T.Text - , jrheHttpException :: !(Maybe HttpException) - , jrheResponse :: !(Maybe T.Text) + { jrlLogLevel :: !LogLevel + , jrlMessage :: !(Maybe Text) + , jrlError :: !(Maybe JwkFetchError) } deriving (Show) -instance ToJSON JwkRefreshHttpError where - toJSON jhe = - object [ "status_code" .= (HTTP.statusCode <$> jrheStatus jhe) - , "url" .= jrheUrl jhe - , "response" .= jrheResponse jhe - , "http_exception" .= (httpExceptToJSON . unHttpException <$> jrheHttpException jhe) - ] - instance ToJSON JwkRefreshLog where - toJSON jrl = case jrlNotice jrl of - JLNInfo info -> - object [ "message" .= info - , "http_error" .= (toJSON <$> jrlHttpError jrl) - ] - JLNError err -> - object [ "error" .= err - , "http_error" .= (toJSON <$> jrlHttpError jrl) + toJSON (JwkRefreshLog _ msg err) = + object [ "message" .= msg + , "error" .= err ] instance ToEngineLog JwkRefreshLog Hasura where diff --git a/server/tests-py/jwk_server.py b/server/tests-py/jwk_server.py index e780baac8a0ed9..7d9e1fbb09c1d0 100644 --- a/server/tests-py/jwk_server.py +++ b/server/tests-py/jwk_server.py @@ -52,8 +52,14 @@ def get(self, request): if request.qs: if 'error' in request.qs and 'true' in request.qs['error']: header_val = 'invalid-header-value=42' + elif 'nocache' in request.qs: + header_val = 'no-cache' + elif 'nomaxage' in request.qs: + header_val = 'public, must-revalidate=123, no-transform' elif 'field' in request.qs and 'smaxage' in request.qs['field']: header_val = 's-maxage=' + self.expires_in_secs + if 'field' in request.qs and 'smaxage' in request.qs['field']: + header_val = 's-maxage=' + self.expires_in_secs resp = mkJSONResp(res) resp.headers['Cache-Control'] = header_val # HGE should always prefer Cache-Control over Expires header