fix: allow duplicate auth rules when creating the join type (#8680)

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

@@ -129,4 +129,32 @@ test('that adding a role again without a resource is not allowed', () => {
     expect(acm.isAllowed(blogOwnerRole, field, 'delete')).toBe(true);
   }
   expect(() => acm.setRole({ role: blogOwnerRole, operations: ['read'] })).toThrow(`@auth ${blogOwnerRole} already exists for Blog`);
+  // field overwrites should still be allowed
+  acm.setRole({ role: blogOwnerRole, operations: ['read'], resource: 'name' });
+  acm.setRole({ role: blogOwnerRole, operations: ['read'], resource: 'id' });
+  expect(acm.isAllowed(blogOwnerRole, 'id', 'read')).toBe(true);
+});
+
+test('that adding a role again without a resource is allowed with overwrite flag enabled', () => {
+  const blogOwnerRole = 'userPools:owner';
+  const blogFields = ['id', 'owner', 'name', 'content'];
+  const acm = new AccessControlMatrix({
+    name: 'Blog',
+    resources: blogFields,
+    operations: MODEL_OPERATIONS,
+  });
+  acm.setRole({ role: blogOwnerRole, operations: MODEL_OPERATIONS });
+  for (let field of blogFields) {
+    expect(acm.isAllowed(blogOwnerRole, field, 'create')).toBe(true);
+    expect(acm.isAllowed(blogOwnerRole, field, 'read')).toBe(true);
+    expect(acm.isAllowed(blogOwnerRole, field, 'update')).toBe(true);
+    expect(acm.isAllowed(blogOwnerRole, field, 'delete')).toBe(true);
+  }
+  acm.setRole({ role: blogOwnerRole, operations: ['read'], allowRoleOverwrite: true });
+  for (let field of blogFields) {
+    expect(acm.isAllowed(blogOwnerRole, field, 'create')).toBe(false);
+    expect(acm.isAllowed(blogOwnerRole, field, 'read')).toBe(true);
+    expect(acm.isAllowed(blogOwnerRole, field, 'update')).toBe(false);
+    expect(acm.isAllowed(blogOwnerRole, field, 'delete')).toBe(false);
+  }
 });

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

@@ -11,6 +11,7 @@ type SetRoleInput = {
   role: string;
   operations: Array<string>;
   resource?: string;
+  allowRoleOverwrite?: boolean;
 };
 
 type ValidateInput = {
@@ -48,15 +49,15 @@ export class AccessControlMatrix {
   }
 
   public setRole(input: SetRoleInput): void {
-    const { role, resource, operations } = input;
+    const { role, resource, operations, allowRoleOverwrite = false } = input;
     this.validate({ resource, operations });
     let allowedVector: Array<Array<boolean>>;
     if (!this.roles.includes(role)) {
       allowedVector = this.getResourceOperationMatrix({ operations, resource });
       this.roles.push(input.role);
       this.matrix.push(allowedVector);
       assert(this.roles.length === this.matrix.length, 'Roles are not aligned with Roles added in Matrix');
-    } else if (this.roles.includes(role) && resource) {
+    } else if (this.roles.includes(role) && (resource || allowRoleOverwrite)) {
       allowedVector = this.getResourceOperationMatrix({ operations, resource, role });
       const roleIndex = this.roles.indexOf(role);
       this.matrix[roleIndex] = allowedVector;

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

@@ -149,6 +149,11 @@ export class AuthTransformer extends TransformerAuthBase implements TransformerA
       throw new TransformerContractError('Types annotated with @auth must also be annotated with @model.');
     }
     const typeName = def.name.value;
+    let isJoinType = false;
+    // check if type is a joinedType
+    if (context.metadata.has('joinTypeList')) {
+      isJoinType = context.metadata.get<Array<string>>('joinTypeList')!.includes(typeName);
+    }
     const authDir = new DirectiveWrapper(directive);
     const rules: AuthRule[] = authDir.getArguments<{ rules: Array<AuthRule> }>({ rules: [] }).rules;
     ensureAuthRuleDefaults(rules);
@@ -166,7 +171,7 @@ export class AuthTransformer extends TransformerAuthBase implements TransformerA
     // add object into policy
     this.addTypeToResourceReferences(def.name.value, rules);
     // turn rules into roles and add into acm and roleMap
-    this.convertRulesToRoles(acm, rules);
+    this.convertRulesToRoles(acm, rules, isJoinType);
     this.modelDirectiveConfig.set(typeName, getModelConfig(modelDirective, typeName, context.isProjectUsingDataStore()));
     this.authModelConfig.set(typeName, acm);
   };
@@ -226,7 +231,7 @@ Static group authorization should perform as expected.`,
         acm = this.authModelConfig.get(typeName) as AccessControlMatrix;
         acm.resetAccessForResource(fieldName);
       }
-      this.convertRulesToRoles(acm, rules, fieldName);
+      this.convertRulesToRoles(acm, rules, false, fieldName);
       this.authModelConfig.set(typeName, acm);
     } else {
       // if @auth is used without @model only generate static group rules in the resolver
@@ -239,7 +244,7 @@ Static group authorization should perform as expected.`,
         operations: ['read'],
         resources: [typeFieldName],
       });
-      this.convertRulesToRoles(acm, staticRules, typeFieldName, ['read']);
+      this.convertRulesToRoles(acm, staticRules, false, typeFieldName, ['read']);
       this.authNonModelConfig.set(typeFieldName, acm);
     }
   };
@@ -761,7 +766,13 @@ Static group authorization should perform as expected.`,
   /*
   Role Helpers
   */
-  private convertRulesToRoles(acm: AccessControlMatrix, authRules: AuthRule[], field?: string, overideOperations?: ModelOperation[]) {
+  private convertRulesToRoles(
+    acm: AccessControlMatrix,
+    authRules: AuthRule[],
+    allowRoleOverwrite: boolean,
+    field?: string,
+    overideOperations?: ModelOperation[],
+  ) {
     for (let rule of authRules) {
       let operations: ModelOperation[] = overideOperations ? overideOperations : rule.operations || MODEL_OPERATIONS;
       if (rule.groups && !rule.groupsField) {
@@ -777,7 +788,7 @@ Static group authorization should perform as expected.`,
               entity: group,
             });
           }
-          acm.setRole({ role: roleName, resource: field, operations });
+          acm.setRole({ role: roleName, resource: field, operations, allowRoleOverwrite });
         });
       } else {
         let roleName: string;
@@ -841,7 +852,7 @@ Static group authorization should perform as expected.`,
         if (!(roleName in this.roleMap)) {
           this.roleMap.set(roleName, roleDefinition);
         }
-        acm.setRole({ role: roleName, resource: field, operations });
+        acm.setRole({ role: roleName, resource: field, operations, allowRoleOverwrite });
       }
     }
   }