Fix inherited roles leaking supposedly-hidden data via aggregation predicates for Postgres

[GDC-1292]: https://hasurahq.atlassian.net/browse/GDC-1292?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ

PR-URL: https://github.com/hasura/graphql-engine-mono/pull/9961
GitOrigin-RevId: cffe2fda2f4dbc7929fc1aaac3887b7d12c4b467
This commit is contained in:
Daniel Chambers 2023-07-26 22:05:41 +10:00 committed by hasura-bot
parent d1cdacca60
commit bae14c70d4
13 changed files with 198 additions and 33 deletions

View File

@ -80,7 +80,7 @@ employee =
Schema.column "first_name" Schema.TStr,
Schema.column "last_name" Schema.TStr,
Schema.column "nationality" Schema.TStr,
Schema.column "monthly_salary" Schema.TInt,
Schema.column "monthly_salary" Schema.TDouble,
Schema.column "engineering_manager_id" Schema.TInt,
Schema.column "hr_manager_id" Schema.TInt
],
@ -90,10 +90,10 @@ employee =
Schema.reference "hr_manager_id" "manager" "id"
],
tableData =
[ [Schema.VInt 1, Schema.VStr "David", Schema.VStr "Holden", Schema.VStr "Australian", Schema.VInt 5000, Schema.VInt 1, Schema.VInt 3],
[Schema.VInt 2, Schema.VStr "Grant", Schema.VStr "Smith", Schema.VStr "Australian", Schema.VInt 6000, Schema.VInt 1, Schema.VInt 4],
[Schema.VInt 3, Schema.VStr "Xin", Schema.VStr "Cheng", Schema.VStr "Chinese", Schema.VInt 5500, Schema.VInt 2, Schema.VInt 3],
[Schema.VInt 4, Schema.VStr "Sarah", Schema.VStr "Smith", Schema.VStr "British", Schema.VInt 4000, Schema.VInt 2, Schema.VInt 4]
[ [Schema.VInt 1, Schema.VStr "David", Schema.VStr "Holden", Schema.VStr "Australian", Schema.VDouble 5000, Schema.VInt 1, Schema.VInt 3],
[Schema.VInt 2, Schema.VStr "Grant", Schema.VStr "Smith", Schema.VStr "Australian", Schema.VDouble 6000, Schema.VInt 1, Schema.VInt 4],
[Schema.VInt 3, Schema.VStr "Xin", Schema.VStr "Cheng", Schema.VStr "Chinese", Schema.VDouble 5500, Schema.VInt 2, Schema.VInt 3],
[Schema.VInt 4, Schema.VStr "Sarah", Schema.VStr "Smith", Schema.VStr "British", Schema.VDouble 4000, Schema.VInt 2, Schema.VInt 4]
]
}
@ -829,3 +829,148 @@ tests = do
|]
shouldReturnYaml testEnvironment actual expected
describe "Redaction in aggregation predicates" $ do
it "aggregation functions are applied over redacted input columns" \testEnvironment -> do
let schemaName = Schema.getSchemaName testEnvironment
actual :: IO Value
actual =
GraphqlEngine.postGraphqlWithHeaders
testEnvironment
[ ("X-Hasura-Role", "hr_manager"),
("X-Hasura-Manager-Id", "3")
]
[graphql|
query {
#{schemaName}_manager(where: {
employees_by_id_to_hr_manager_id_aggregate: {
max: {
arguments: monthly_salary,
predicate: {_gte: 5500}
}
}
}) {
id
first_name
last_name
}
}
|]
-- Althea Weiss can only see the salaries of the employees she is HR manager for.
-- This is because the 'manager_employee_private_info' role provides access to the salary
-- for the current manager's HR-managed employees, but the rest of the employees
-- are accessed via 'all_managers', which does not expose 'monthly_salary'.
-- So when Althea applies an aggregation predicate over 'monthly_salary', the aggregation
-- should only be computed over monthly salaries she has access to.
-- max(monthly_salary) >= 5500 applies to both Althea and Bec's employees, but
-- since Althea can only see her employee's salaries, Bec should not be returned
-- from this query.
expected :: Value
expected =
[interpolateYaml|
data:
#{schemaName}_manager:
- id: 3
first_name: Althea
last_name: Weiss
|]
shouldReturnYaml testEnvironment actual expected
it "column filters in aggregation predicates are applied over the redacted column" \testEnvironment -> do
let schemaName = Schema.getSchemaName testEnvironment
actual :: IO Value
actual =
GraphqlEngine.postGraphqlWithHeaders
testEnvironment
[ ("X-Hasura-Role", "hr_manager"),
("X-Hasura-Manager-Id", "3")
]
[graphql|
query {
#{schemaName}_manager(where: {
employees_by_id_to_hr_manager_id_aggregate: {
count: {
predicate: {_eq: 1}
filter: { monthly_salary: { _gte: 5500 } }
}
}
}) {
id
first_name
last_name
}
}
|]
-- Althea Weiss can only see the salaries of the employees she is HR manager for.
-- This is because the 'manager_employee_private_info' role provides access to the salary
-- for the current manager's HR-managed employees, but the rest of the employees
-- are accessed via 'all_managers', which does not expose 'monthly_salary'.
-- So when Althea applies an aggregation predicate over 'monthly_salary', the aggregation
-- should only be computed over monthly salaries she has access to.
-- Both Althea and Bec have one employee with a monthly salary >= 5500, but ALthea
-- should only be able to see her employees' monthly salary, not Bec's
-- so Bec should get filtered out
expected :: Value
expected =
[interpolateYaml|
data:
#{schemaName}_manager:
- id: 3
first_name: Althea
last_name: Weiss
|]
shouldReturnYaml testEnvironment actual expected
it "computed field filters in aggregation predicates are applied over the redacted computed field" \testEnvironment -> do
when ((Fixture.backendType <$> getBackendTypeConfig testEnvironment) `elem` [Just Fixture.Citus, Just Fixture.Cockroach])
$ pendingWith "Backend does not support computed fields"
let schemaName = Schema.getSchemaName testEnvironment
actual :: IO Value
actual =
GraphqlEngine.postGraphqlWithHeaders
testEnvironment
[ ("X-Hasura-Role", "hr_manager"),
("X-Hasura-Manager-Id", "3")
]
[graphql|
query {
#{schemaName}_manager(where: {
employees_by_id_to_hr_manager_id_aggregate: {
count: {
predicate: {_eq: 1}
filter: { yearly_salary: { _gte: 66000 } }
}
}
}) {
id
first_name
last_name
}
}
|]
-- Althea Weiss can only see the salaries of the employees she is HR manager for.
-- This is because the 'manager_employee_private_info' role provides access to the salary
-- for the current manager's HR-managed employees, but the rest of the employees
-- are accessed via 'all_managers', which does not expose 'yearly_salary'.
-- So when Althea applies an aggregation predicate over 'yearly_salary', the aggregation
-- should only be computed over yearly salaries she has access to.
-- Both Althea and Bec have one employee with a yearly salary >= 66000, but ALthea
-- should only be able to see her employees' yearly salary, not Bec's
-- so Bec should get filtered out
expected :: Value
expected =
[interpolateYaml|
data:
#{schemaName}_manager:
- id: 3
first_name: Althea
last_name: Weiss
|]
shouldReturnYaml testEnvironment actual expected

View File

@ -149,6 +149,7 @@ tableInsertions (Schema.Table {tableColumns, tableData}) =
scalarType :: (HasCallStack) => Schema.ScalarType -> Text
scalarType = \case
Schema.TInt -> "INT64"
Schema.TDouble -> "FLOAT64"
Schema.TStr -> "STRING"
Schema.TUTCTime -> "DATETIME"
Schema.TBool -> "BOOL"
@ -168,6 +169,7 @@ mkColumn Schema.Column {columnName, columnType} =
serialize :: ScalarValue -> Text
serialize = \case
VInt i -> tshow i
VDouble d -> tshow d
VStr s -> "'" <> T.replace "'" "\'" s <> "'"
VUTCTime t -> T.pack $ formatTime defaultTimeLocale "DATETIME '%F %T'" t
VBool b -> tshow b

View File

@ -169,6 +169,7 @@ createTable testEnv Schema.Table {tableName, tableColumns, tablePrimaryKey = pk,
scalarType :: (HasCallStack) => Schema.ScalarType -> Text
scalarType = \case
Schema.TInt -> "integer"
Schema.TDouble -> "double precision"
Schema.TStr -> "text"
Schema.TUTCTime -> "timestamp"
Schema.TBool -> "boolean"
@ -201,6 +202,7 @@ insertTable testEnv Schema.Table {tableName, tableColumns, tableData}
serialize :: ScalarValue -> Text
serialize = \case
VInt n -> tshow n
VDouble d -> tshow d
VStr s -> "'" <> T.replace "'" "\'" s <> "'"
VUTCTime t -> T.pack $ formatTime defaultTimeLocale "'%F %T'" t
VBool b -> if b then "TRUE" else "FALSE"

View File

@ -169,6 +169,7 @@ createTable testEnv Schema.Table {tableName, tableColumns, tablePrimaryKey = pk,
scalarType :: (HasCallStack) => Schema.ScalarType -> Text
scalarType = \case
Schema.TInt -> "integer"
Schema.TDouble -> "double precision"
Schema.TStr -> "text"
Schema.TUTCTime -> "timestamp"
Schema.TBool -> "boolean"
@ -213,6 +214,7 @@ wrapIdentifier identifier = "\"" <> identifier <> "\""
serialize :: ScalarValue -> Text
serialize = \case
VInt n -> tshow n
VDouble d -> tshow d
VStr s -> "'" <> T.replace "'" "\'" s <> "'"
VUTCTime t -> T.pack $ formatTime defaultTimeLocale "'%F %T'" t
VBool b -> if b then "TRUE" else "FALSE"

View File

@ -264,6 +264,7 @@ mkReference _schemaName Schema.Reference {referenceLocalColumn, referenceTargetT
scalarType :: Schema.ScalarType -> Text
scalarType = \case
Schema.TInt -> "INTEGER"
Schema.TDouble -> "REAL"
Schema.TStr -> "TEXT"
Schema.TUTCTime -> "TIMESTAMP"
Schema.TBool -> "BOOLEAN"
@ -274,6 +275,7 @@ scalarType = \case
serialize :: Schema.ScalarValue -> Text
serialize = \case
Schema.VInt i -> tshow i
Schema.VDouble d -> tshow d
Schema.VStr s -> "'" <> Text.replace "'" "\'" s <> "'"
Schema.VUTCTime t -> Text.pack $ Time.formatTime Time.defaultTimeLocale "'%F %T'" t
Schema.VBool b -> if b then "1" else "0"

View File

@ -279,6 +279,7 @@ createUniqueIndexSql (SchemaName schemaName) tableName = \case
scalarType :: (HasCallStack) => Schema.ScalarType -> Text
scalarType = \case
Schema.TInt -> "integer"
Schema.TDouble -> "double precision"
Schema.TStr -> "text"
Schema.TUTCTime -> "timestamp"
Schema.TBool -> "boolean"
@ -347,6 +348,7 @@ wrapIdentifier identifier = "\"" <> identifier <> "\""
serialize :: ScalarValue -> Text
serialize = \case
VInt n -> tshow n
VDouble d -> tshow d
VStr s -> "'" <> T.replace "'" "\'" s <> "'"
VUTCTime t -> T.pack $ formatTime defaultTimeLocale "'%F %T'" t
VBool b -> if b then "TRUE" else "FALSE"

View File

@ -159,6 +159,7 @@ createTable testEnvironment Schema.Table {tableName, tableColumns, tablePrimaryK
scalarType :: (HasCallStack) => Schema.ScalarType -> Text
scalarType = \case
Schema.TInt -> "int"
Schema.TDouble -> "float(53)"
Schema.TStr -> "nvarchar(127)"
Schema.TUTCTime -> "time"
Schema.TBool -> "bit"
@ -243,6 +244,7 @@ wrapIdentifier identifier = "[" <> T.replace "]" "]]" identifier <> "]"
serialize :: ScalarValue -> Text
serialize = \case
VInt int -> tshow int
VDouble d -> tshow d
VStr s -> "'" <> T.replace "'" "\'" s <> "'"
VUTCTime t -> T.pack $ formatTime defaultTimeLocale "'%F %T'" t
VBool b -> tshow @Int $ if b then 1 else 0

View File

@ -262,6 +262,7 @@ wrapIdentifier identifier = "\"" <> identifier <> "\""
scalarType :: (HasCallStack) => Schema.ScalarType -> Text
scalarType = \case
Schema.TInt -> "integer"
Schema.TDouble -> "double precision"
Schema.TStr -> "varchar"
Schema.TUTCTime -> "timestamp"
Schema.TBool -> "boolean"
@ -292,6 +293,7 @@ insertTable testEnv table@(Schema.Table {tableColumns, tableData}) = unless (nul
serialize :: ScalarValue -> Text
serialize = \case
VInt n -> tshow n
VDouble d -> tshow d
VStr s -> "'" <> T.replace "'" "\'" s <> "'"
VUTCTime t -> T.pack $ formatTime defaultTimeLocale "'%F %T'" t
VBool b -> if b then "TRUE" else "FALSE"

View File

@ -127,6 +127,7 @@ defaultBackendScalarValue =
-- a future iteration.
data ScalarType
= TInt
| TDouble
| TStr
| TUTCTime
| TBool
@ -138,6 +139,7 @@ data ScalarType
-- to 'ScalarType'
data ScalarValue
= VInt Int
| VDouble Double
| VStr Text
| VUTCTime UTCTime
| VBool Bool

View File

@ -250,13 +250,12 @@ translateAggPredBoolExp
subselectIdentifier
(AggregationPredicate {aggPredFunctionName, aggPredPredicate}) = do
BoolExpCtx {rootReference} <- ask
-- TODO(redactionExp): Use of NoRedaction below is a placeholder. Investigate what is necessary here
let (Identifier aggAlias) = identifierWithSuffix relTableName aggPredFunctionName
boolExps =
map
(mkFieldCompExp rootReference (S.QualifiedIdentifier subselectIdentifier Nothing) NoRedaction $ LColumn (FieldName aggAlias))
aggPredPredicate
pure $ sqlAnd boolExps
pure $ S.simplifyBoolExp $ sqlAnd boolExps
translateAggPredsSubselect ::
forall pgKind.
@ -298,15 +297,16 @@ translateAggPredsSubselect
S.mkSelect
{ S.selExtr = [extractorsExp],
S.selFrom = Just $ S.FromExp fromExp,
S.selWhere = Just $ S.WhereFrag whereExp
S.selWhere = Just $ S.WhereFrag $ S.simplifyBoolExp whereExp
}
subselectAlias
translateAggPredExtractor ::
forall pgKind field.
forall pgKind.
(Backend ('Postgres pgKind)) =>
TableIdentifier ->
TableName ('Postgres pgKind) ->
AggregationPredicate ('Postgres pgKind) field ->
AggregationPredicate ('Postgres pgKind) S.SQLExp ->
S.Extractor
translateAggPredExtractor relTableNameIdentifier relTableName (AggregationPredicate {aggPredFunctionName, aggPredArguments}) =
let predArgsExp = toList $ translateAggPredArguments aggPredArguments relTableNameIdentifier
@ -315,14 +315,19 @@ translateAggPredExtractor relTableNameIdentifier relTableName (AggregationPredic
translateAggPredArguments ::
forall pgKind.
AggregationPredicateArguments ('Postgres pgKind) ->
(Backend ('Postgres pgKind)) =>
AggregationPredicateArguments ('Postgres pgKind) S.SQLExp ->
TableIdentifier ->
NonEmpty S.SQLExp
translateAggPredArguments predArgs relTableNameIdentifier =
case predArgs of
AggregationPredicateArgumentsStar -> pure $ S.SEStar Nothing
(AggregationPredicateArguments cols) ->
S.SEQIdentifier . S.mkQIdentifier relTableNameIdentifier <$> cols
cols
<&> ( \(column, redactionExp) ->
withRedactionExp (S.QualifiedIdentifier relTableNameIdentifier Nothing) redactionExp
$ S.mkQIdenExp relTableNameIdentifier column
)
translateTableRelationship :: HashMap PGCol PGCol -> TableIdentifier -> BoolExpM S.BoolExp
translateTableRelationship colMapping relTableNameIdentifier = do

View File

@ -102,8 +102,7 @@ defaultAggregationPredicatesParser aggFns ti = runMaybeT do
ArgumentsStar ->
maybe AggregationPredicateArgumentsStar AggregationPredicateArguments
. nonEmpty
-- TODO(redactionExp): Probably need to deal with the redaction expressions from tableSelectColumnsEnum here
<$> fuse (fieldOptionalDefault Name._arguments Nothing [] . P.list . fmap fst <$> fails (tableSelectColumnsEnum relTable))
<$> fuse (fieldOptionalDefault Name._arguments Nothing [] . P.list <$> fails (tableSelectColumnsEnum relTable))
SingleArgument typ ->
AggregationPredicateArguments
. (NE.:| [])

View File

@ -17,7 +17,7 @@ module Hasura.GraphQL.Schema.Table
)
where
import Control.Lens ((^?), _1)
import Control.Lens ((^?))
import Data.Has
import Data.HashMap.Strict qualified as HashMap
import Data.HashSet qualified as Set
@ -142,14 +142,14 @@ tableSelectColumnsPredEnum ::
(ColumnType b -> Bool) ->
GQLNameIdentifier ->
TableInfo b ->
SchemaT r m (Maybe (Parser 'Both n (Column b)))
SchemaT r m (Maybe (Parser 'Both n (Column b, AnnRedactionExpUnpreparedValue b)))
tableSelectColumnsPredEnum columnPredicate predName tableInfo = do
customization <- retrieve $ _siCustomization @b
let tCase = _rscNamingConvention customization
mkTypename = runMkTypename $ _rscTypeNames customization
predName' = applyFieldNameCaseIdentifier tCase predName
tableGQLName <- getTableIdentifierName @b tableInfo
columns <- filter (columnPredicate . ciType) . mapMaybe (^? _1 . _SCIScalarColumn) <$> tableSelectColumns tableInfo
columns <- filter (columnPredicate . ciType . fst) . mapMaybe (\(column, redactionExp) -> (,redactionExp) <$> (column ^? _SCIScalarColumn)) <$> tableSelectColumns tableInfo
let enumName = mkTypename $ applyTypeNameCaseIdentifier tCase $ mkSelectColumnPredTypeName tableGQLName predName
description =
Just
@ -162,9 +162,9 @@ tableSelectColumnsPredEnum columnPredicate predName tableInfo = do
$ P.enum enumName description
<$> nonEmpty
[ ( define $ ciName column,
ciColumn column
(ciColumn column, redactionExp)
)
| column <- columns
| (column, redactionExp) <- columns
]
where
define name =

View File

@ -13,7 +13,7 @@ import Data.Aeson
import Data.Aeson.Extended (ToJSONKeyValue (..))
import Data.Aeson.Key (fromText)
import Hasura.Prelude
import Hasura.RQL.IR.BoolExp (AnnBoolExp, OpExpG)
import Hasura.RQL.IR.BoolExp (AnnBoolExp, AnnRedactionExp, OpExpG)
import Hasura.RQL.Types.Backend (Backend (Column))
import Hasura.RQL.Types.Backend qualified as B
import Hasura.RQL.Types.BackendType (BackendType)
@ -101,7 +101,7 @@ data AggregationPredicate (b :: BackendType) field = AggregationPredicate
{ aggPredFunctionName :: Text,
aggPredDistinct :: Bool,
aggPredFilter :: Maybe (AnnBoolExp b field),
aggPredArguments :: AggregationPredicateArguments b,
aggPredArguments :: AggregationPredicateArguments b field,
aggPredPredicate :: [OpExpG b field]
}
deriving stock (Foldable, Traversable, Functor, Generic)
@ -110,7 +110,7 @@ deriving instance
( B.Backend b,
Eq (AnnBoolExp b field),
Eq (OpExpG b field),
Eq (AggregationPredicateArguments b)
Eq (AggregationPredicateArguments b field)
) =>
Eq (AggregationPredicate b field)
@ -118,7 +118,7 @@ deriving instance
( B.Backend b,
Show (AnnBoolExp b field),
Show (OpExpG b field),
Show (AggregationPredicateArguments b)
Show (AggregationPredicateArguments b field)
) =>
Show (AggregationPredicate b field)
@ -132,14 +132,14 @@ instance
instance
( B.Backend b,
NFData (AggregationPredicateArguments b),
NFData (AggregationPredicateArguments b field),
NFData (AnnBoolExp b field),
NFData (OpExpG b field)
) =>
NFData (AggregationPredicate b field)
instance
( ToJSON (AggregationPredicateArguments b),
( ToJSON (AggregationPredicateArguments b field),
ToJSON (AnnBoolExp b field),
ToJSONKeyValue (OpExpG b field)
) =>
@ -155,17 +155,17 @@ instance
]
)
data AggregationPredicateArguments (b :: BackendType)
data AggregationPredicateArguments (b :: BackendType) field
= AggregationPredicateArgumentsStar
| AggregationPredicateArguments (NonEmpty (Column b))
deriving stock (Generic)
| AggregationPredicateArguments (NonEmpty (Column b, AnnRedactionExp b field))
deriving stock (Generic, Foldable, Traversable, Functor)
deriving instance (B.Backend b) => Eq (AggregationPredicateArguments b)
deriving instance (Backend b, Eq (AnnRedactionExp b field)) => Eq (AggregationPredicateArguments b field)
deriving instance (B.Backend b) => Show (AggregationPredicateArguments b)
deriving instance (Backend b, Show (AnnRedactionExp b field)) => Show (AggregationPredicateArguments b field)
instance (Backend b) => Hashable (AggregationPredicateArguments b)
instance (Backend b, Hashable (AnnRedactionExp b field)) => Hashable (AggregationPredicateArguments b field)
instance (Backend b) => NFData (AggregationPredicateArguments b)
instance (Backend b, NFData (AnnRedactionExp b field)) => NFData (AggregationPredicateArguments b field)
instance (Backend b) => ToJSON (AggregationPredicateArguments b)
instance (Backend b, ToJSON (AnnRedactionExp b field)) => ToJSON (AggregationPredicateArguments b field)