fix: add field auth on aggregation queries (#8508)

packages/amplify-graphql-auth-transformer/src/__tests__/searchable-auth.test.ts

@@ -1,8 +1,29 @@
-import { AuthTransformer } from '../graphql-auth-transformer';
+import { AuthTransformer, SEARCHABLE_AGGREGATE_TYPES } from '../';
 import { ModelTransformer } from '@aws-amplify/graphql-model-transformer';
 import { SearchableModelTransformer } from '@aws-amplify/graphql-searchable-transformer';
 import { GraphQLTransform } from '@aws-amplify/graphql-transformer-core';
 import { AppSyncAuthConfiguration } from '@aws-amplify/graphql-transformer-interfaces';
+import { DocumentNode, ObjectTypeDefinitionNode, Kind, FieldDefinitionNode, parse, InputValueDefinitionNode } from 'graphql';
+
+const getObjectType = (doc: DocumentNode, type: string): ObjectTypeDefinitionNode | undefined => {
+  return doc.definitions.find(def => def.kind === Kind.OBJECT_TYPE_DEFINITION && def.name.value === type) as
+    | ObjectTypeDefinitionNode
+    | undefined;
+};
+const expectMultiple = (fieldOrType: ObjectTypeDefinitionNode | FieldDefinitionNode, directiveNames: string[]) => {
+  expect(directiveNames).toBeDefined();
+  expect(directiveNames).toHaveLength(directiveNames.length);
+  expect(fieldOrType.directives.length).toEqual(directiveNames.length);
+  directiveNames.forEach(directiveName => {
+    expect(fieldOrType.directives).toEqual(
+      expect.arrayContaining([
+        expect.objectContaining({
+          name: expect.objectContaining({ value: directiveName }),
+        }),
+      ]),
+    );
+  });
+};
 
 test('auth logic is enabled on owner/static rules in es request', () => {
   const validSchema = `
@@ -47,6 +68,7 @@ test('auth logic is enabled on owner/static rules in es request', () => {
 });
 
 test('auth logic is enabled for iam/apiKey auth rules', () => {
+  const expectedDirectives = ['aws_api_key', 'aws_iam'];
   const validSchema = `
         type Post @model
             @searchable
@@ -89,5 +111,14 @@ test('auth logic is enabled for iam/apiKey auth rules', () => {
   });
   const out = transformer.transform(validSchema);
   expect(out).toBeDefined();
-  expect(out.schema).toContain('SearchablePostConnection @aws_api_key @aws_iam');
+  expect(out.schema).toBeDefined();
+  const schemaDoc = parse(out.schema);
+  for (const aggregateType of SEARCHABLE_AGGREGATE_TYPES) {
+    expectMultiple(getObjectType(schemaDoc, aggregateType), expectedDirectives);
+  }
+  // expect the searchbable types to have the auth directives for total providers
+  // expect the allowed fields for agg to exclude secret
+  expect(out.pipelineFunctions['Query.searchPosts.auth.1.req.vtl']).toContain(
+    `#set( $allowedAggFields = ["createdAt","updatedAt","id","content"] )`,
+  );
 });

packages/amplify-graphql-auth-transformer/src/graphql-auth-transformer.ts

@@ -20,24 +20,25 @@ import {
   TransformerBeforeStepContextProvider,
 } from '@aws-amplify/graphql-transformer-interfaces';
 import {
+  AUTH_PROVIDER_DIRECTIVE_MAP,
+  DEFAULT_GROUP_CLAIM,
+  DEFAULT_IDENTITY_CLAIM,
+  DEFAULT_GROUPS_FIELD,
+  DEFAULT_OWNER_FIELD,
+  MODEL_OPERATIONS,
+  SEARCHABLE_AGGREGATE_TYPES,
   AuthRule,
   authDirectiveDefinition,
   ConfiguredAuthProviders,
   getConfiguredAuthProviders,
   AuthTransformerConfig,
   collectFieldNames,
-  DEFAULT_GROUP_CLAIM,
-  MODEL_OPERATIONS,
   ModelOperation,
   ensureAuthRuleDefaults,
-  DEFAULT_IDENTITY_CLAIM,
-  DEFAULT_GROUPS_FIELD,
-  DEFAULT_OWNER_FIELD,
   getModelConfig,
   validateFieldRules,
   validateRules,
   AuthProvider,
-  AUTH_PROVIDER_DIRECTIVE_MAP,
   extendTypeWithDirectives,
   RoleDefinition,
   addDirectivesToOperation,
@@ -229,6 +230,7 @@ Static group authorization should perform as expected.`,
   };
 
   transformSchema = (context: TransformerTransformSchemaStepContextProvider): void => {
+    const searchableAggregateServiceDirectives = new Set<AuthProvider>();
     const getOwnerFields = (acm: AccessControlMatrix) => {
       return acm.getRoles().reduce((prev: string[], role: string) => {
         if (this.roleMap.get(role)!.strategy === 'owner') prev.push(this.roleMap.get(role)!.entity!);
@@ -237,11 +239,15 @@ Static group authorization should perform as expected.`,
     };
     for (let [modelName, acm] of this.authModelConfig) {
       const def = context.output.getObject(modelName)!;
+      const modelHasSearchable = def.directives.some(dir => dir.name.value === 'searchable');
       // collect ownerFields and them in the model
       this.addFieldsToObject(context, modelName, getOwnerFields(acm));
       // Get the directives we need to add to the GraphQL nodes
       const providers = this.getAuthProviders(acm.getRoles());
       const directives = this.getServiceDirectives(providers, providers.length === 0 ? this.shouldAddDefaultServiceDirective() : false);
+      if (modelHasSearchable) {
+        providers.forEach(p => searchableAggregateServiceDirectives.add(p));
+      }
       if (directives.length > 0) {
         extendTypeWithDirectives(context, modelName, directives);
       }
@@ -257,6 +263,13 @@ Static group authorization should perform as expected.`,
         addDirectivesToField(context, typeName, fieldName, directives);
       }
     }
+    // add the service directives to the searchable aggregate types
+    if (searchableAggregateServiceDirectives.size > 0) {
+      const serviceDirectives = this.getServiceDirectives(Array.from(searchableAggregateServiceDirectives), false);
+      for (let aggType of SEARCHABLE_AGGREGATE_TYPES) {
+        extendTypeWithDirectives(context, aggType, serviceDirectives);
+      }
+    }
   };
 
   generateResolvers = (context: TransformerContextProvider): void => {
@@ -542,16 +555,48 @@ Static group authorization should perform as expected.`,
       );
     }
   };
+  /*
+  Searchable Auth
+  Protects
+    - Search Query
+    - Agg Query
+  */
   protectSearchResolver = (
     ctx: TransformerContextProvider,
     def: ObjectTypeDefinitionNode,
     typeName: string,
     fieldName: string,
     acm: AccessControlMatrix,
   ): void => {
+    const acmFields = acm.getResources();
+    const modelFields = def.fields ?? [];
+    // only add readonly fields if they exist
+    const allowedAggFields = modelFields.map(f => f.name.value).filter(f => !acmFields.includes(f));
+    let leastAllowedFields = acmFields;
     const resolver = ctx.resolvers.getResolver(typeName, fieldName) as TransformerResolverProvider;
-    const roleDefinitions = acm.getRolesPerOperation('read').map(r => this.roleMap.get(r)!);
-    const authExpression = generateAuthExpressionForSearchQueries(this.configuredAuthProviders, roleDefinitions, def.fields ?? []);
+    // to protect search and aggregation queries we need to collect all the roles which can query
+    // and the allowed fields to run field auth on aggregation queries
+    const readRoleDefinitions = acm.getRolesPerOperation('read').map(role => {
+      const allowedFields = acmFields.filter(resource => acm.isAllowed(role, resource, 'read'));
+      const roleDefinition = this.roleMap.get(role)!;
+      // we add the allowed fields if the role does not have full access
+      // or if the rule is a dynamic rule (ex. ownerField, groupField)
+      if (allowedFields.length !== acmFields.length || !roleDefinition.static) {
+        roleDefinition.allowedFields = allowedFields;
+        leastAllowedFields = leastAllowedFields.filter(f => allowedFields.includes(f));
+      } else {
+        roleDefinition.allowedFields = null;
+      }
+      return roleDefinition;
+    });
+    // add readonly fields with all the fields every role has access to
+    allowedAggFields.push(...leastAllowedFields);
+    const authExpression = generateAuthExpressionForSearchQueries(
+      this.configuredAuthProviders,
+      readRoleDefinitions,
+      modelFields,
+      allowedAggFields,
+    );
     resolver.addToSlot(
       'auth',
       MappingTemplate.s3MappingTemplateFromString(authExpression, `${typeName}.${fieldName}.{slotName}.{slotIndex}.req.vtl`),

packages/amplify-graphql-auth-transformer/src/resolvers/field.ts

@@ -20,6 +20,7 @@ import {
   qref,
   notEquals,
   obj,
+  list,
 } from 'graphql-mapping-template';
 import {
   RoleDefinition,
@@ -31,7 +32,7 @@ import {
   IS_AUTHORIZED_FLAG,
   API_KEY_AUTH_TYPE,
 } from '../utils';
-import { getOwnerClaim, generateStaticRoleExpression, apiKeyExpression, iamExpression, emptyPayload } from './helpers';
+import { getOwnerClaim, generateStaticRoleExpression, apiKeyExpression, iamExpression, emptyPayload, getIdentityClaimExp } from './helpers';
 
 // Field Read VTL Functions
 const generateDynamicAuthReadExpression = (roles: Array<RoleDefinition>, fields: ReadonlyArray<FieldDefinitionNode>) => {
@@ -66,15 +67,15 @@ const generateDynamicAuthReadExpression = (roles: Array<RoleDefinition>, fields:
           not(ref(IS_AUTHORIZED_FLAG)),
           compoundExpression([
             set(ref(`groupEntity${idx}`), methodCall(ref('util.defaultIfNull'), ref(`ctx.source.${role.entity!}`), nul())),
-            set(ref(`groupClaim${idx}`), getOwnerClaim(role.claim!)),
-            forEach(ref('userGroup'), ref('dynamicGroupClaim'), [
-              iff(
-                entityIsList
-                  ? methodCall(ref(`groupEntity${idx}.contains`), ref('userGroup'))
-                  : equals(ref(`groupEntity${idx}`), ref('userGroup')),
-                compoundExpression([set(ref(IS_AUTHORIZED_FLAG), bool(true)), raw('#break')]),
-              ),
-            ]),
+            set(ref(`groupClaim${idx}`), getIdentityClaimExp(str(role.claim), list([]))),
+            entityIsList
+              ? forEach(ref('userGroup'), ref(`groupClaim${idx}`), [
+                  iff(
+                    methodCall(ref(`groupEntity${idx}.contains`), ref('userGroup')),
+                    compoundExpression([set(ref(IS_AUTHORIZED_FLAG), bool(true)), raw('#break')]),
+                  ),
+                ])
+              : iff(ref(`groupClaim${idx}.contains($groupEntity${idx})`), set(ref(IS_AUTHORIZED_FLAG), bool(true))),
           ]),
         ),
       );

packages/amplify-graphql-auth-transformer/src/resolvers/index.ts

@@ -1,4 +1,5 @@
-export { generateAuthExpressionForQueries, generateAuthExpressionForSearchQueries, generateAuthExpressionForRelationQuery } from './query';
+export { generateAuthExpressionForQueries, generateAuthExpressionForRelationQuery } from './query';
+export { generateAuthExpressionForSearchQueries } from './search';
 export { generateAuthExpressionForCreate } from './mutation.create';
 export { generateAuthExpressionForUpdate } from './mutation.update';
 export { geneateAuthExpressionForDelete } from './mutation.delete';