Skip to content
Permalink
Browse files Browse the repository at this point in the history
Merge pull request from GHSA-2j98-fw5g-j43v
* fix bug in audience check while verifying JWT

  - previously the check was converting the audience type into a string
  and then comparing with the conf value. all audience types (as it is a
  string or URI) will convert to plain strings
  - use the Audience type from the jose library for comparing

* add docs for audience

* add issuer check as well

* docs minor syntax fix

* skip audience check if not given in conf

* minor docs update

* qualify import jose library
  • Loading branch information
ecthiender authored and shahidhk committed Jul 11, 2019
1 parent 92c4cff commit f2f14e7
Show file tree
Hide file tree
Showing 5 changed files with 252 additions and 29 deletions.
44 changes: 44 additions & 0 deletions .circleci/test-server.sh
Expand Up @@ -265,6 +265,50 @@ kill_hge_servers

unset HASURA_GRAPHQL_JWT_SECRET

echo -e "\n$(time_elapsed): <########## TEST GRAPHQL-ENGINE WITH ADMIN SECRET AND JWT (with audience check - string) #####################################>\n"
TEST_TYPE="jwt-audience-check-single-string"


export HASURA_GRAPHQL_JWT_SECRET="$(jq -n --arg key "$(cat $OUTPUT_FOLDER/ssl/jwt_public.key)" '{ type: "RS512", key: $key , audience: "myapp-1234"}')"

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

kill_hge_servers

unset HASURA_GRAPHQL_JWT_SECRET

echo -e "\n$(time_elapsed): <########## TEST GRAPHQL-ENGINE WITH ADMIN SECRET AND JWT (with audience check - list of strings) #################################>\n"
TEST_TYPE="jwt-audience-check-list-string"

export HASURA_GRAPHQL_JWT_SECRET="$(jq -n --arg key "$(cat $OUTPUT_FOLDER/ssl/jwt_public.key)" '{ type: "RS512", key: $key , audience: ["myapp-1234", "myapp-9876"]}')"

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

kill_hge_servers

unset HASURA_GRAPHQL_JWT_SECRET

echo -e "\n$(time_elapsed): <########## TEST GRAPHQL-ENGINE WITH ADMIN SECRET AND JWT (with issuer check) #####################################>\n"
TEST_TYPE="jwt-issuer-check"

export HASURA_GRAPHQL_JWT_SECRET="$(jq -n --arg key "$(cat $OUTPUT_FOLDER/ssl/jwt_public.key)" '{ type: "RS512", key: $key , issuer: "https://hasura.com"}')"

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

kill_hge_servers

unset HASURA_GRAPHQL_JWT_SECRET


# test with CORS modes

echo -e "\n$(time_elapsed): <########## TEST GRAPHQL-ENGINE WITH CORS DOMAINS ########>\n"
Expand Down
5 changes: 1 addition & 4 deletions docs/graphql/manual/auth/authentication/jwt.rst
Expand Up @@ -230,7 +230,6 @@ If ``claims_format`` is ``stringified_json`` then JWT claims should look like:
"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\"}"
}
``audience``
^^^^^^^^^^^^
This is an optional field. Certain providers might set a claim which indicates
Expand All @@ -252,7 +251,6 @@ Examples:
"type": "RS512",
"jwk_url": "https://......",
"audience": "myapp-1234"
...
}
or
Expand All @@ -263,7 +261,6 @@ or
"type": "RS512",
"jwk_url": "https://......",
"audience": ["myapp-1234", "myapp-6789"]
...
}
Expand All @@ -285,10 +282,10 @@ Examples:
"type": "RS512",
"jwk_url": "https://......",
"issuer": "https://my-auth-server.com"
...
}
Examples
^^^^^^^^

Expand Down
2 changes: 1 addition & 1 deletion server/src-lib/Hasura/Server/Auth.hs
Expand Up @@ -129,7 +129,7 @@ mkJwtCtx conf httpManager loggerCtx = do
jwkRefreshCtrl logger httpManager url ref t
return ref
let claimsFmt = fromMaybe JCFJson (jcClaimsFormat conf)
return $ JWTCtx jwkRef (jcClaimNs conf) (jcAudience conf) claimsFmt
return $ JWTCtx jwkRef (jcClaimNs conf) (jcAudience conf) claimsFmt (jcIssuer conf)

mkUserInfoFromResp
:: (MonadIO m, MonadError QErr m)
Expand Down
54 changes: 32 additions & 22 deletions server/src-lib/Hasura/Server/Auth/JWT.hs
Expand Up @@ -3,7 +3,7 @@ module Hasura.Server.Auth.JWT
, RawJWT
, JWTConfig (..)
, JWTCtx (..)
, JWKSet (..)
, Jose.JWKSet (..)
, JWTClaimsFormat (..)
, updateJwkRef
, jwkRefreshCtrl
Expand All @@ -14,7 +14,6 @@ import Control.Arrow (first)
import Control.Exception (try)
import Control.Lens
import Control.Monad (when)
import Crypto.JWT
import Data.IORef (IORef, modifyIORef, readIORef)

import Data.List (find)
Expand All @@ -33,6 +32,7 @@ import Hasura.Server.Utils (diffTimeToMicro,
userRoleHeader)

import qualified Control.Concurrent 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
Expand Down Expand Up @@ -62,24 +62,25 @@ $(A.deriveJSON A.defaultOptions { A.sumEncoding = A.ObjectWithSingleField
data JWTConfig
= JWTConfig
{ jcType :: !T.Text
, jcKeyOrUrl :: !(Either JWK URI)
, jcKeyOrUrl :: !(Either Jose.JWK URI)
, jcClaimNs :: !(Maybe T.Text)
, jcAudience :: !(Maybe T.Text)
, jcAudience :: !(Maybe Jose.Audience)
, jcClaimsFormat :: !(Maybe JWTClaimsFormat)
-- , jcIssuer :: !(Maybe T.Text)
, jcIssuer :: !(Maybe Jose.StringOrURI)
} deriving (Show, Eq)

data JWTCtx
= JWTCtx
{ jcxKey :: !(IORef JWKSet)
{ jcxKey :: !(IORef Jose.JWKSet)
, jcxClaimNs :: !(Maybe T.Text)
, jcxAudience :: !(Maybe T.Text)
, jcxAudience :: !(Maybe Jose.Audience)
, jcxClaimsFormat :: !JWTClaimsFormat
, jcxIssuer :: !(Maybe Jose.StringOrURI)
} deriving (Eq)

instance Show JWTCtx where
show (JWTCtx _ nsM audM cf) =
show ["<IORef JWKSet>", show nsM, show audM, show cf]
show (JWTCtx _ nsM audM cf iss) =
show ["<IORef JWKSet>", show nsM, show audM, show cf, show iss]

data HasuraClaims
= HasuraClaims
Expand All @@ -103,7 +104,7 @@ jwkRefreshCtrl
=> Logger
-> HTTP.Manager
-> URI
-> IORef JWKSet
-> IORef Jose.JWKSet
-> NominalDiffTime
-> m ()
jwkRefreshCtrl lggr mngr url ref time =
Expand All @@ -124,7 +125,7 @@ updateJwkRef
=> Logger
-> HTTP.Manager
-> URI
-> IORef JWKSet
-> IORef Jose.JWKSet
-> m (Maybe NominalDiffTime)
updateJwkRef (Logger logger) manager url jwkRef = do
let options = wreqOptions manager []
Expand Down Expand Up @@ -210,10 +211,10 @@ processAuthZHeader jwtCtx headers authzHeader = do

let claimsNs = fromMaybe defaultClaimNs $ jcxClaimNs jwtCtx
claimsFmt = jcxClaimsFormat jwtCtx
expTimeM = fmap (\(NumericDate t) -> t) $ claims ^. claimExp
expTimeM = fmap (\(Jose.NumericDate t) -> t) $ claims ^. Jose.claimExp

-- see if the hasura claims key exist in the claims map
let mHasuraClaims = Map.lookup claimsNs $ claims ^. unregisteredClaims
let mHasuraClaims = Map.lookup claimsNs $ claims ^. Jose.unregisteredClaims
hasuraClaimsV <- maybe claimsNotFound return mHasuraClaims

-- get hasura claims value as an object. parse from string possibly
Expand Down Expand Up @@ -322,24 +323,31 @@ parseHasuraClaims claimsMap = do

-- | Verify the JWT against given JWK
verifyJwt
:: ( MonadError JWTError m
:: ( MonadError Jose.JWTError m
, MonadIO m
)
=> JWTCtx
-> RawJWT
-> m ClaimsSet
-> m Jose.ClaimsSet
verifyJwt ctx (RawJWT rawJWT) = do
key <- liftIO $ readIORef $ jcxKey ctx
jwt <- decodeCompact rawJWT
jwt <- Jose.decodeCompact rawJWT
t <- liftIO getCurrentTime
verifyClaimsAt config key t jwt
Jose.verifyClaimsAt config key t jwt
where
audCheck aud = maybe True (== (T.pack . show) aud) $ jcxAudience ctx
config = defaultJWTValidationSettings audCheck
config = case jcxIssuer ctx of
Nothing -> Jose.defaultJWTValidationSettings audCheck
Just iss -> Jose.defaultJWTValidationSettings audCheck
& set Jose.issuerPredicate (== iss)
audCheck audience =
-- dont perform the check if there are no audiences in the conf
case jcxAudience ctx of
Nothing -> True
Just (Jose.Audience audiences) -> audience `elem` audiences


instance A.ToJSON JWTConfig where
toJSON (JWTConfig ty keyOrUrl claimNs aud claimsFmt) =
toJSON (JWTConfig ty keyOrUrl claimNs aud claimsFmt iss) =
case keyOrUrl of
Left _ -> mkObj ("key" A..= A.String "<JWK REDACTED>")
Right url -> mkObj ("jwk_url" A..= url)
Expand All @@ -348,6 +356,7 @@ instance A.ToJSON JWTConfig where
, "claims_namespace" A..= claimNs
, "claims_format" A..= claimsFmt
, "audience" A..= aud
, "issuer" A..= iss
, item
]

Expand All @@ -361,6 +370,7 @@ instance A.FromJSON JWTConfig where
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"

Expand All @@ -369,9 +379,9 @@ instance A.FromJSON JWTConfig where
(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 isStrngfd
return $ JWTConfig keyType (Left key) claimNs aud isStrngfd iss
(Nothing, Just url) ->
return $ JWTConfig keyType (Right url) claimNs aud isStrngfd
return $ JWTConfig keyType (Right url) claimNs aud isStrngfd iss

where
parseKey keyType rawKey =
Expand Down

0 comments on commit f2f14e7

Please sign in to comment.