hasura · 0x777 · Jun 11, 2020 · May 19, 2020 · May 28, 2020 · Jun 9, 2020
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -73,6 +73,7 @@ Read more about the session argument for computed fields in the [docs](https://h
 - server: compile with GHC 8.10.1, closing a space leak with subscriptions. (close #4517) (#3388)
 - server: fixes an issue where introspection queries with variables would fail because of caching (fix #4547)
 - server: avoid loss of precision when passing values in scientific notation (fix #4733)
+- server: raise error on startup when `--unauthorized-role` is ignored (#4736)
 - server: fix mishandling of GeoJSON inputs in subscriptions (fix #3239)
 - server: fix importing of allow list query from metadata (fix #4687)
 - server: flush log buffer during shutdown (#4800)

@@ -25,8 +25,8 @@ verified by the GraphQL engine, to authorize and get metadata about the request
    :alt: Authentication using JWT
 
 The JWT is decoded, the signature is verified, then it is asserted that the
-current role of the user (if specified in the request) is in the list of allowed roles.
-If the current role is not specified in the request, then the default role is applied.
+requested role of the user (if specified in the request) is in the list of allowed roles.
+If the desired role is not specified in the request, then the default role is applied.
 If the authorization passes, then all of the ``x-hasura-*`` values in the claim
 are used for the permissions system.
 
@@ -60,7 +60,7 @@ the following:
 1. A ``x-hasura-default-role`` field : indicating the default role of that user i.e. the role that will be
    used in case ``x-hasura-role`` header is not passed.
 2. A ``x-hasura-allowed-roles`` field : a list of allowed roles for the user i.e. acceptable values of the
-   ``x-hasura-role`` header.
+   ``x-hasura-role`` header. The ``x-hasura-default-role`` specified should be a member of this list.
 
 The claims in the JWT can have other ``x-hasura-*`` fields where their values
 can only be strings. You can use these ``x-hasura-*`` fields in your

@@ -244,6 +244,7 @@ library
                      -- Exposed for testing:
                      , Hasura.Server.Telemetry.Counters
                      , Data.Parser.JSONPath
+                     , Hasura.Server.Auth.JWT
 
                      , Hasura.RQL.Types
                      , Hasura.RQL.Types.Run
@@ -261,7 +262,6 @@ library
                      , Hasura.Incremental.Internal.Dependency
                      , Hasura.Incremental.Internal.Rule
 
-                     , Hasura.Server.Auth.JWT
                      , Hasura.Server.Auth.WebHook
                      , Hasura.Server.Middleware
                      , Hasura.Server.CheckUpdates
@@ -434,7 +434,9 @@ test-suite graphql-engine-tests
     , hspec-core >=2.6.1 && <3
     , hspec-expectations-lifted
     , http-client
+    , http-types
     , http-client-tls
+    , jose
     , lifted-base
     , monad-control
     , mtl
@@ -460,6 +462,7 @@ test-suite graphql-engine-tests
     Hasura.RQL.MetadataSpec
     Hasura.Server.MigrateSpec
     Hasura.Server.TelemetrySpec
+    Hasura.Server.AuthSpec
 
 -- Benchmarks related to caching (e.g. the plan cache).
 --

@@ -227,7 +227,7 @@ runHGEServer ServeOptions{..} InitCtx{..} initTime = do
   let sqlGenCtx = SQLGenCtx soStringifyNum
       Loggers loggerCtx logger _ = _icLoggers
 
-  authModeRes <- runExceptT $ mkAuthMode soAdminSecret soAuthHook soJwtSecret soUnAuthRole
+  authModeRes <- runExceptT $ setupAuthMode soAdminSecret soAuthHook soJwtSecret soUnAuthRole
                               _icHttpManager logger
 
   authMode <- either (printErrExit . T.unpack) return authModeRes

@@ -340,6 +340,8 @@ explainLiveQueryPlan :: (MonadTx m, MonadIO m) => LiveQueryPlan -> m LiveQueryPl
 explainLiveQueryPlan plan = do
   let parameterizedPlan = _lqpParameterizedPlan plan
       queryText = Q.getQueryText . unMultiplexedQuery $ _plqpQuery parameterizedPlan
+      -- CAREFUL!: an `EXPLAIN ANALYZE` here would actually *execute* this
+      -- query, maybe resulting in privilege escalation:
       explainQuery = Q.fromText $ "EXPLAIN (FORMAT TEXT) " <> queryText
   cohortId <- newCohortId
   explanationLines <- map runIdentity <$> executeQuery explainQuery [(cohortId, _lqpVariables plan)]

@@ -105,6 +105,8 @@ explainField userInfo gCtx sqlGenCtx actionExecuter fld =
       resolvedAST <- RS.traverseQueryRootFldAST (resolveVal userInfo) unresolvedAST
       let (query, remoteJoins) = RS.toPGQuery resolvedAST
           txtSQL = Q.getQueryText query
+          -- CAREFUL!: an `EXPLAIN ANALYZE` here would actually *execute* this
+          -- query, resulting in potential privilege escalation:
           withExplain = "EXPLAIN (FORMAT TEXT) " <> txtSQL
       -- Reject if query contains any remote joins
       when (remoteJoins /= mempty) $ throw400 NotSupported "Remote relationships are not allowed in explain query"
@@ -128,6 +130,7 @@ explainGQLQuery
   -> GQLExplain
   -> m EncJSON
 explainGQLQuery pgExecCtx sc sqlGenCtx enableAL actionExecuter (GQLExplain query userVarsRaw maybeIsRelay) = do
+  -- NOTE!: we will be executing what follows as though admin role. See e.g. notes in explainField:
   userInfo <- mkUserInfo (URBFromSessionVariablesFallback adminRoleName) UAdminSecretSent sessionVariables
   (execPlan, queryReusability) <- runReusabilityT $
     E.getExecPlanPartial userInfo sc queryType enableAL query