Revert "feat: add handling of colon-delimited identity claims to query (

aws-amplify#10189)" (aws-amplify#10213) This reverts commit d7983f4.
danielleadams · Apr 14, 2022 · 9f13064 · 9f13064
1 parent 9cb1be0
commit 9f13064
Show file tree

Hide file tree

Showing 8 changed files with 27 additions and 288 deletions.
diff --git a/...ify-graphql-auth-transformer/src/__tests__/__snapshots__/conflict-resolution.test.ts.snap b/...ify-graphql-auth-transformer/src/__tests__/__snapshots__/conflict-resolution.test.ts.snap
@@ -15,9 +15,9 @@ $util.qr($ctx.stash.put(\\"hasAuth\\", true))
 #if( $util.authType() == \\"User Pool Authorization\\" )
   #if( !$isAuthorized )
     #set( $authFilter = [] )
-    #set( $role0_1 = $util.defaultIfNull($ctx.identity.claims.get(\\"username\\"), $util.defaultIfNull($ctx.identity.claims.get(\\"cognito:username\\"), \\"___xamznone____\\")) )
-    #if( $role0_1 != \\"___xamznone____\\" )
-      $util.qr($authFilter.add({\\"owner\\": { \\"eq\\": $role0_1 }}))
+    #set( $role0 = $util.defaultIfNull($ctx.identity.claims.get(\\"username\\"), $util.defaultIfNull($ctx.identity.claims.get(\\"cognito:username\\"), \\"___xamznone____\\")) )
+    #if( $role0 != \\"___xamznone____\\" )
+      $util.qr($authFilter.add({\\"owner\\": { \\"eq\\": $role0 }}))
     #end
     #if( !$authFilter.isEmpty() )
       $util.qr($ctx.stash.put(\\"authFilter\\", { \\"or\\": $authFilter }))
@@ -79,9 +79,9 @@ $util.qr($ctx.stash.put(\\"hasAuth\\", true))
 #if( $util.authType() == \\"User Pool Authorization\\" )
   #if( !$isAuthorized )
     #set( $authFilter = [] )
-    #set( $role0_1 = $util.defaultIfNull($ctx.identity.claims.get(\\"username\\"), $util.defaultIfNull($ctx.identity.claims.get(\\"cognito:username\\"), \\"___xamznone____\\")) )
-    #if( $role0_1 != \\"___xamznone____\\" )
-      $util.qr($authFilter.add({\\"owner\\": { \\"eq\\": $role0_1 }}))
+    #set( $role0 = $util.defaultIfNull($ctx.identity.claims.get(\\"username\\"), $util.defaultIfNull($ctx.identity.claims.get(\\"cognito:username\\"), \\"___xamznone____\\")) )
+    #if( $role0 != \\"___xamznone____\\" )
+      $util.qr($authFilter.add({\\"owner\\": { \\"eq\\": $role0 }}))
     #end
     #if( !$authFilter.isEmpty() )
       $util.qr($ctx.stash.put(\\"authFilter\\", { \\"or\\": $authFilter }))

diff --git a/...ages/amplify-graphql-auth-transformer/src/__tests__/__snapshots__/owner-auth.test.ts.snap b/...ages/amplify-graphql-auth-transformer/src/__tests__/__snapshots__/owner-auth.test.ts.snap
@@ -89,9 +89,9 @@ $util.qr($ctx.stash.put(\\"hasAuth\\", true))
 #if( $util.authType() == \\"User Pool Authorization\\" )
   #if( !$isAuthorized )
     #set( $authFilter = [] )
-    #set( $role0_1 = $util.defaultIfNull($ctx.identity.claims.get(\\"username\\"), $util.defaultIfNull($ctx.identity.claims.get(\\"cognito:username\\"), \\"___xamznone____\\")) )
-    #if( $role0_1 != \\"___xamznone____\\" )
-      $util.qr($authFilter.add({\\"editors\\": { \\"contains\\": $role0_1 }}))
+    #set( $role0 = $util.defaultIfNull($ctx.identity.claims.get(\\"username\\"), $util.defaultIfNull($ctx.identity.claims.get(\\"cognito:username\\"), \\"___xamznone____\\")) )
+    #if( $role0 != \\"___xamznone____\\" )
+      $util.qr($authFilter.add({\\"editors\\": { \\"contains\\": $role0 }}))
     #end
     #if( !$authFilter.isEmpty() )
       $util.qr($ctx.stash.put(\\"authFilter\\", { \\"or\\": $authFilter }))
@@ -113,9 +113,9 @@ $util.qr($ctx.stash.put(\\"hasAuth\\", true))
 #if( $util.authType() == \\"User Pool Authorization\\" )
   #if( !$isAuthorized )
     #set( $authFilter = [] )
-    #set( $role0_1 = $util.defaultIfNull($ctx.identity.claims.get(\\"username\\"), $util.defaultIfNull($ctx.identity.claims.get(\\"cognito:username\\"), \\"___xamznone____\\")) )
-    #if( $role0_1 != \\"___xamznone____\\" )
-      $util.qr($authFilter.add({\\"editors\\": { \\"contains\\": $role0_1 }}))
+    #set( $role0 = $util.defaultIfNull($ctx.identity.claims.get(\\"username\\"), $util.defaultIfNull($ctx.identity.claims.get(\\"cognito:username\\"), \\"___xamznone____\\")) )
+    #if( $role0 != \\"___xamznone____\\" )
+      $util.qr($authFilter.add({\\"editors\\": { \\"contains\\": $role0 }}))
     #end
     #if( !$authFilter.isEmpty() )
       $util.qr($ctx.stash.put(\\"authFilter\\", { \\"or\\": $authFilter }))
@@ -196,149 +196,3 @@ $util.unauthorized()
 $util.toJson({\\"version\\":\\"2018-05-29\\",\\"payload\\":{}})
 ## [End] Authorization Steps. **"
 `;
-
-exports[`owner where field is ":" delimited string 1`] = `
-"## [Start] Authorization Steps. **
-$util.qr($ctx.stash.put(\\"hasAuth\\", true))
-#set( $inputFields = $util.parseJson($util.toJson($ctx.args.input.keySet())) )
-#set( $isAuthorized = false )
-#set( $allowedFields = [] )
-#if( $util.authType() == \\"User Pool Authorization\\" )
-  #if( !$isAuthorized )
-    #set( $ownerEntity0 = $util.defaultIfNull($ctx.args.input.owner, null) )
-    #set( $ownerClaim0 = $util.defaultIfNull($ctx.identity.claims.get(\\"sub:username\\"), \\"___xamznone____\\") )
-    #set( $ownerAllowedFields0 = [\\"id\\",\\"title\\",\\"createdAt\\",\\"updatedAt\\"] )
-    #set( $isAuthorizedOnAllFields0 = true )
-    #if( $ownerClaim0 == $ownerEntity0 )
-      #if( $isAuthorizedOnAllFields0 )
-        #set( $isAuthorized = true )
-      #else
-        $util.qr($allowedFields.addAll($ownerAllowedFields0))
-      #end
-    #end
-    #if( $util.isNull($ownerEntity0) && !$ctx.args.input.containsKey(\\"owner\\") )
-      $util.qr($ctx.args.input.put(\\"owner\\", $ownerClaim0))
-      #if( $isAuthorizedOnAllFields0 )
-        #set( $isAuthorized = true )
-      #else
-        $util.qr($allowedFields.addAll($ownerAllowedFields0))
-      #end
-    #end
-  #end
-#end
-#if( !$isAuthorized && $allowedFields.isEmpty() )
-$util.unauthorized()
-#end
-#if( !$isAuthorized )
-  #set( $deniedFields = $util.list.copyAndRemoveAll($inputFields, $allowedFields) )
-  #if( $deniedFields.size() > 0 )
-    $util.error(\\"Unauthorized on \${deniedFields}\\", \\"Unauthorized\\")
-  #end
-#end
-$util.toJson({\\"version\\":\\"2018-05-29\\",\\"payload\\":{}})
-## [End] Authorization Steps. **"
-`;
-
-exports[`owner where field is ":" delimited string 2`] = `
-"## [Start] Get Request template. **
-#set( $GetRequest = {
-  \\"version\\": \\"2018-05-29\\",
-  \\"operation\\": \\"GetItem\\"
-} )
-#if( $ctx.stash.metadata.modelObjectKey )
-  #set( $key = $ctx.stash.metadata.modelObjectKey )
-#else
-  #set( $key = {
-  \\"id\\":   $util.dynamodb.toDynamoDB($ctx.args.input.id)
-} )
-#end
-$util.qr($GetRequest.put(\\"key\\", $key))
-$util.toJson($GetRequest)
-## [End] Get Request template. **"
-`;
-
-exports[`owner where field is ":" delimited string 3`] = `
-"## [Start] Get Request template. **
-#set( $GetRequest = {
-  \\"version\\": \\"2018-05-29\\",
-  \\"operation\\": \\"GetItem\\"
-} )
-#if( $ctx.stash.metadata.modelObjectKey )
-  #set( $key = $ctx.stash.metadata.modelObjectKey )
-#else
-  #set( $key = {
-  \\"id\\":   $util.dynamodb.toDynamoDB($ctx.args.input.id)
-} )
-#end
-$util.qr($GetRequest.put(\\"key\\", $key))
-$util.toJson($GetRequest)
-## [End] Get Request template. **"
-`;
-
-exports[`owner where field is ":" delimited string 4`] = `
-"## [Start] Authorization Steps. **
-$util.qr($ctx.stash.put(\\"hasAuth\\", true))
-#set( $isAuthorized = false )
-#set( $primaryFieldMap = {} )
-#if( $util.authType() == \\"User Pool Authorization\\" )
-  #if( !$isAuthorized )
-    #set( $authFilter = [] )
-    #set( $role0_0 = $util.defaultIfNull($ctx.identity.claims.get(\\"sub\\"), \\"___xamznone____\\") )
-    #set( $ownerPrefix0_0 = \\"$role0_0:\\" )
-    #if( $role0_0 != \\"___xamznone____\\" )
-      $util.qr($authFilter.add({\\"owner\\": { \\"beginsWith\\": $ownerPrefix0_0 }}))
-    #end
-    #set( $role0_1 = $util.defaultIfNull($ctx.identity.claims.get(\\"username\\"), $util.defaultIfNull($ctx.identity.claims.get(\\"cognito:username\\"), \\"___xamznone____\\")) )
-    #set( $ownerPrefix0_1 = \\"$role0_1:\\" )
-    #if( $role0_1 != \\"___xamznone____\\" )
-      $util.qr($authFilter.add({\\"owner\\": { \\"beginsWith\\": $ownerPrefix0_1 }}))
-    #end
-    #set( $role0_2 = $util.defaultIfNull($ctx.identity.claims.get(\\"sub:username\\"), \\"___xamznone____\\") )
-    #if( $role0_2 != \\"___xamznone____\\" )
-      $util.qr($authFilter.add({\\"owner\\": { \\"eq\\": $role0_2 }}))
-    #end
-    #if( !$authFilter.isEmpty() )
-      $util.qr($ctx.stash.put(\\"authFilter\\", { \\"or\\": $authFilter }))
-    #end
-  #end
-#end
-#if( !$isAuthorized && $util.isNull($ctx.stash.authFilter) )
-$util.unauthorized()
-#end
-$util.toJson({\\"version\\":\\"2018-05-29\\",\\"payload\\":{}})
-## [End] Authorization Steps. **"
-`;
-
-exports[`owner where field is ":" delimited string 5`] = `
-"## [Start] Authorization Steps. **
-$util.qr($ctx.stash.put(\\"hasAuth\\", true))
-#set( $isAuthorized = false )
-#set( $primaryFieldMap = {} )
-#if( $util.authType() == \\"User Pool Authorization\\" )
-  #if( !$isAuthorized )
-    #set( $authFilter = [] )
-    #set( $role0_0 = $util.defaultIfNull($ctx.identity.claims.get(\\"sub\\"), \\"___xamznone____\\") )
-    #set( $ownerPrefix0_0 = \\"$role0_0:\\" )
-    #if( $role0_0 != \\"___xamznone____\\" )
-      $util.qr($authFilter.add({\\"owner\\": { \\"beginsWith\\": $ownerPrefix0_0 }}))
-    #end
-    #set( $role0_1 = $util.defaultIfNull($ctx.identity.claims.get(\\"username\\"), $util.defaultIfNull($ctx.identity.claims.get(\\"cognito:username\\"), \\"___xamznone____\\")) )
-    #set( $ownerPrefix0_1 = \\"$role0_1:\\" )
-    #if( $role0_1 != \\"___xamznone____\\" )
-      $util.qr($authFilter.add({\\"owner\\": { \\"beginsWith\\": $ownerPrefix0_1 }}))
-    #end
-    #set( $role0_2 = $util.defaultIfNull($ctx.identity.claims.get(\\"sub:username\\"), \\"___xamznone____\\") )
-    #if( $role0_2 != \\"___xamznone____\\" )
-      $util.qr($authFilter.add({\\"owner\\": { \\"eq\\": $role0_2 }}))
-    #end
-    #if( !$authFilter.isEmpty() )
-      $util.qr($ctx.stash.put(\\"authFilter\\", { \\"or\\": $authFilter }))
-    #end
-  #end
-#end
-#if( !$isAuthorized && $util.isNull($ctx.stash.authFilter) )
-$util.unauthorized()
-#end
-$util.toJson({\\"version\\":\\"2018-05-29\\",\\"payload\\":{}})
-## [End] Authorization Steps. **"
-`;
diff --git a/packages/amplify-graphql-auth-transformer/src/__tests__/owner-auth.test.ts b/packages/amplify-graphql-auth-transformer/src/__tests__/owner-auth.test.ts
@@ -64,36 +64,6 @@ test('owner field where the field is a list', () => {
   expect(out.resolvers['Query.listPosts.auth.1.req.vtl']).toMatchSnapshot();
 });
 
-test('owner where field is ":" delimited string', () => {
-  const authConfig: AppSyncAuthConfiguration = {
-    defaultAuthentication: {
-      authenticationType: 'AMAZON_COGNITO_USER_POOLS',
-    },
-    additionalAuthenticationProviders: [],
-  };
-  const validSchema = `
-    type Post @model @auth(rules: [{allow: owner, identityClaim: "sub:username" }]) {
-      id: ID!
-      title: String!
-      createdAt: String
-      updatedAt: String
-    }`;
-  const transformer = new GraphQLTransform({
-    authConfig,
-    transformers: [new ModelTransformer(), new AuthTransformer()],
-  });
-  const out = transformer.transform(validSchema);
-  expect(out).toBeDefined();
-  expect(out.rootStack.Resources[ResourceConstants.RESOURCES.GraphQLAPILogicalID].Properties.AuthenticationType).toEqual(
-    'AMAZON_COGNITO_USER_POOLS',
-  );
-  expect(out.resolvers['Mutation.createPost.auth.1.req.vtl']).toMatchSnapshot();
-  expect(out.resolvers['Mutation.updatePost.auth.1.req.vtl']).toMatchSnapshot();
-  expect(out.resolvers['Mutation.deletePost.auth.1.req.vtl']).toMatchSnapshot();
-  expect(out.resolvers['Query.getPost.auth.1.req.vtl']).toMatchSnapshot();
-  expect(out.resolvers['Query.listPosts.auth.1.req.vtl']).toMatchSnapshot();
-});
-
 test('owner field with subscriptions', () => {
   const authConfig: AppSyncAuthConfiguration = {
     defaultAuthentication: {

diff --git a/packages/amplify-graphql-auth-transformer/src/graphql-auth-transformer.ts b/packages/amplify-graphql-auth-transformer/src/graphql-auth-transformer.ts
@@ -58,7 +58,6 @@ import {
   setDeniedFieldFlag,
   generateAuthExpressionForRelationQuery,
   generateSandboxExpressionForField,
-  generateFieldResolverForOwner,
 } from './resolvers';
 import { AccessControlMatrix } from './accesscontrol';
 import {
@@ -305,9 +304,7 @@ export class AuthTransformer extends TransformerAuthBase implements TransformerA
       const def = context.output.getObject(modelName)!;
       const modelNameConfig = this.modelDirectiveConfig.get(modelName);
       const searchableDirective = def.directives.find(dir => dir.name.value === 'searchable');
-      const readRoles = acm.getRolesPerOperation('read');
-      const roleDefinitions = readRoles.map(role => this.roleMap.get(role)!);
-
+      // queries
       const queryFields = getQueryFieldNames(this.modelDirectiveConfig.get(modelName)!);
       queryFields.forEach(query => {
         switch (query.type) {
@@ -340,6 +337,7 @@ export class AuthTransformer extends TransformerAuthBase implements TransformerA
       // get fields specified in the schema
       // if there is a role that does not have read access on the field then we create a field resolver
       // or there is a relational directive on the field then we should protect that as well
+      const readRoles = acm.getRolesPerOperation('read');
       const modelFields = def.fields?.filter(f => acm.hasResource(f.name.value)) ?? [];
       const errorFields = new Array<string>();
       modelFields.forEach(field => {
@@ -381,18 +379,14 @@ export class AuthTransformer extends TransformerAuthBase implements TransformerA
       });
 
       const subscriptionFieldNames = getSubscriptionFieldNames(this.modelDirectiveConfig.get(modelName)!);
-      const subscriptionRoles = roleDefinitions
+      const subscriptionRoles = acm
+        .getRolesPerOperation('read')
+        .map(role => this.roleMap.get(role)!)
         // for subscriptions we only use static rules or owner rule where the field is not a list
         .filter(roleDef => (roleDef.strategy === 'owner' && !fieldIsList(def.fields ?? [], roleDef.entity!)) || roleDef.static);
       subscriptionFieldNames.forEach(subscription => {
         this.protectSubscriptionResolver(context, subscription.typeName, subscription.fieldName, subscriptionRoles);
       });
-
-      roleDefinitions.forEach(role => {
-        if (role.strategy === 'owner') {
-          this.addFieldResolverForDynamicAuth(context, def, modelName, role.entity);
-        }
-      });
     });
 
     this.authNonModelConfig.forEach((acm, typeFieldName) => {
@@ -403,45 +397,6 @@ export class AuthTransformer extends TransformerAuthBase implements TransformerA
     });
   };
 
-  addFieldResolverForDynamicAuth = (
-    ctx: TransformerContextProvider,
-    def: ObjectTypeDefinitionNode,
-    typeName: string,
-    fieldName: string,
-  ): void => {
-    let resolver = ctx.resolvers.getResolver(typeName, fieldName);
-
-    if (resolver) {
-      resolver.addToSlot(
-        'finish',
-        undefined,
-        MappingTemplate.s3MappingTemplateFromString(
-          generateFieldResolverForOwner(fieldName),
-          `${typeName}.${fieldName}.{slotName}.{slotIndex}.res.vtl`,
-        ),
-      );
-    } else {
-      const hasModelDirective = def.directives.some(dir => dir.name.value === 'model');
-      const stack = getStackForField(ctx, def, fieldName, hasModelDirective);
-
-      resolver = ctx.resolvers.addResolver(
-        typeName,
-        fieldName,
-        new TransformerResolver(
-          typeName,
-          fieldName,
-          ResolverResourceIDs.ResolverResourceID(typeName, fieldName),
-          MappingTemplate.s3MappingTemplateFromString('$util.toJson({"version":"2018-05-29","payload":{}})', `${typeName}.${fieldName}.req.vtl`),
-          MappingTemplate.s3MappingTemplateFromString(generateFieldResolverForOwner(fieldName), `${typeName}.${fieldName}.res.vtl`),
-          ['init'],
-          ['finish'],
-        ),
-      );
-
-      resolver.mapToStack(stack);
-    }
-  };
-
   protectSchemaOperations = (
     ctx: TransformerTransformSchemaStepContextProvider,
     def: ObjectTypeDefinitionNode,

diff --git a/packages/amplify-graphql-auth-transformer/src/resolvers/field.ts b/packages/amplify-graphql-auth-transformer/src/resolvers/field.ts
@@ -191,18 +191,3 @@ export const generateSandboxExpressionForField = (sandboxEnabled: boolean): stri
   else exp = methodCall(ref('util.unauthorized'));
   return printBlock(`Sandbox Mode ${sandboxEnabled ? 'Enabled' : 'Disabled'}`)(compoundExpression([exp, toJson(obj({}))]));
 };
-
-/**
- * Creates field resolver for owner
- */
-export const generateFieldResolverForOwner = (entity: string): string => {
-  const expressions: Expression[] = [
-    set(ref('ownerEntities'), ref(`ctx.source.${entity}.split(":")`)),
-    set(ref('ownerEntitiesLastIdx'), raw('$ownerEntities.size() - 1')),
-    set(ref('ownerEntitiesLast'), ref('ownerEntities.get($ownerEntitiesLastIdx)')),
-    qref(methodCall(ref('ctx.source.put'), str(entity), ref('ownerEntitiesLast'))),
-    toJson(ref(`ctx.source.${entity}`)),
-  ];
-
-  return printBlock('Parse owner field auth for Get')(compoundExpression(expressions));
-};
diff --git a/packages/amplify-graphql-auth-transformer/src/resolvers/index.ts b/packages/amplify-graphql-auth-transformer/src/resolvers/index.ts
@@ -1,7 +1,4 @@
-export {
-  generateAuthExpressionForQueries,
-  generateAuthExpressionForRelationQuery,
-} from './query';
+export { generateAuthExpressionForQueries, generateAuthExpressionForRelationQuery } from './query';
 export { generateAuthExpressionForSearchQueries } from './search';
 export { generateAuthExpressionForCreate } from './mutation.create';
 export { generateAuthExpressionForUpdate } from './mutation.update';
@@ -11,7 +8,6 @@ export {
   generateFieldAuthResponse,
   setDeniedFieldFlag,
   generateSandboxExpressionForField,
-  generateFieldResolverForOwner,
 } from './field';
 export { generateAuthExpressionForSubscriptions } from './subscriptions';
 export { generateAuthRequestExpression } from './helpers';