amplify-cli: Field @auth transformer combinaisons can lead to unexpected authorized mutation

@SteffeyDev it is harder for owner and dynamic group auth because AppSync can’t determine if owner is authorised for update operations, instead it needs to write DynamoDB condition expressions. These are not easy to generate in complex cases. Not impossible, but not a small amount of work. Dynamo also complains if you do things such as wrap an expression in and extra set of parenthesis, have unused variables, and so on which requires extra attention.

@SwaySway Can you explicitly state that my interpretation of your object level documentation is correct? Your quote from me was referencing the documentation where there is object level authorisation. You have posted the documentation example using field level authorisation as a solution. I want to know if the intention of the field-level auth author is as I described, that an object level auth which blocks updates can nevertheless have any of its fields updated, as long as a single field in the update grants access?

type Todo
  @model @auth(rules: [{allow: groups, groups: ["Admin"], operations:[update] }]
{
  id: ID! 
  updatedAt: AWSDateTime! 
  content: String! @auth(rules: [{ allow: owner, operations: [update] }])
}

In this example from the documentation, who is allowed to update the “updatedAt” field? I’m very carefully reading the documentation, and it implies that an owner is allowed to update this field, even though they are blocked from updates at an object level, and there is no field level auth on the ‘updatedAt’ field. The only catch is that they need to update the ‘content’ field at the same time.

I’m trying to point out that the field level authorisation is simply too difficult to get right. If it is hard to read a model and understand if its secure, the design is lacking. I want to assume best intentions, but it almost feels like there is post hoc rationalisation of the design.

@kaustavghosh06 A second feedback on testing the documentation. e2e tests have the following comment

  # Fields with ownership conditions take precendence to the Object @auth.
  # That means that both the @auth on Object AND the @auth on the field must
  # be satisfied.

the documentation has this schema and comment:

type Todo
  @model @auth(rules: [{allow: groups, groups: ["Admin"], operations:[update] }]
{
  id: ID! 
  updatedAt: AWSDateTime! 
  content: String! @auth(rules: [{ allow: owner, operations: [update] }])
}

In addition an owner can perform an update operation on the Todo object, only when the content field is present as a part of the input

The e2e test comment and documentation are in conflict. According to e2e tests, Todo content field can only be updated by the owner if the owner is also an Admin, where the comment suggests being an owner is sufficient to update the content field.

I’m now running into the same issue, thanks @RossWilliams for explaining it so well because I was just starting to understand the problem. The variable re-use is still a problem but one that seems like it should have an easy fix by using custom variable names for each section. For example, you could use $<fieldName>_isOwnerAuthorized for field-level auth checks and $<objectName>_isOwnerAuthorized for object level, instead of using $isOwnerAuthorized for both types. It looks like you are already using this strategy for static group level authorization (as I discovered in an inspection of the generated transformer) so I was surprised you weren’t doing the same for owner auth.

@kaustavghosh06 You updated the documentation last week to add the following in response to this issue,

An owner of an object, has the authorization to create Todo types and read all the objects of type Todo. In addition an owner can perform an update operation on the Todo object, only when the content field is present as a part of the input.

https://github.com/aws-amplify/docs/blame/master/cli-toolchain/graphql.md#L1276

The critical part for me: is present as a part of the input. This reads to say that if an owner includes content as a field in a mutation, then they are authorised to update any other field, even if the object-level auth forbids the update, and potentially even if the other field is protected by auth rules.

In your above comment you state

since we don’t deny access to the field by default on the operation if the operation is not included in the top level auth rule.

Taking the inverse, you suggest the intention is to deny access to the field by default on the operation if the operation is included in the top level auth rule.

In addition, the Employee salary example in the document clearly states

here is a model where owners and admins can read employee salaries but only admins may create or update them

Can you advise on the apparent conflict in these statements. Does the language in your documentation update last week need to be changed?

@dz0l038 Thanks for the detailed report. It helped us diagnose an issue and root-cause it. The issue you’ve mentioned in the “part 2” of your description was a bug and the authorization of the fields shouldn’t be dependent on the order of the fields. We fixed this with the latest version of the CLI - v3.8.0. We adhere to the authorization policy for both the fields you’re updating - independent of the order in the schema and provide unauthorized/authorized response for the same.

Regarding “part 1” of the issue, you were going in the right direction by explicitly providing authorization rules to your fields - by adding the forbidden groups/owner rules - since we don’t deny access to the field by default on the operation if the operation is not included in the top level auth rule. To add more clarity we’ve updated our docs describing this behavior - https://aws-amplify.github.io/docs/cli-toolchain/graphql#owner-authorization (top level @auth operations) and per field level @auth operations - https://aws-amplify.github.io/docs/cli-toolchain/graphql#field-level-authorization . Please try out the latest version of the CLI and let us know if you’ve any questions around this behavior.

Hiya, this does sound strange. I can give an overview of how authentication works for field level updates that can explain why you are seeing this behaviour. I does seem like a problem in Amplify 1.12, I have not checked if this still exists in the latest versions.

There are two main phases of authorisation evaluation for updates, field-level and object level. Inside each phase, there are static group checks, dynamic group checks, and owner checks. Static group checks happen inside AppSync, and the two others happen inside DynamoDB, as they rely on fields where AppSync does not have information.

If a field has its static group check pass (allowed operation), the field will not have dynamic group or owner permissions checked. Otherwise the dynamic groups and owner fields are checked. How they are checked is problematic, because each field over-writes the previous field’s checks for that auth type. This is a bug. If two fields are both in the update object, and both fields are protected by owner auth, only the second field’s rules will be evaluated. If two fields are both in the update object, and one field is protected by owner auth, and the other field is protected by dynamic group auth, both field’s rules will be evaluated.

If a static group check fails for a field or object and there is no dynamic group rule or owner rule previously set for any previous rule, AppSync returns as unauthorised. This is likely also a bug.

If the object has a rule to enforce updates using dynamic group or owner rules, then these rules will override any field level controls. Unless any field has its static rule check pass, or the object’s field level static group rule check passes, in which case dynamic groups and owner rules are skipped.

Using the above shorthand, here is why you are seeing each behaviour:

1a. Object level static group rule for update fails, server-side ruleset is empty, AppSync returns as unauthorised. 1b. Object level static group rule for update fails, but owner auth rule has already been set, evaluate server-side field rules for updatedAt, rule passes, update works. 2a. Field level rule for user1Sub static group fails, server-side ruleset is empty, AppSync returns as unauthorised. 2b. Field level rule for user1Sub static group fails, but updatedAt rule has been processed and rule set, rule runs server-side and passses. 3a. Field level rule for user1Sub runs server-side, rule fails. 3b. Field level rule for user1Sub evaluated after updatedAt, over-writes updatedAt conditions, rule fails.

When you modified the field order, fields over-writing gets swapped around:

2b. Field level rule for user1Sub runs first, static group check fails, no dynamic group or owner rule yet set, AppSync returns as unauthorised. 3b. Field level rule for user1Sub runs first, is server-side rule. Field level rule for updatedAt runs, over-writes user1Sub auth checks, rule runs server-side and passes.

What is interesting is if you test by setting a groupsField rule on user1Sub (and adding that field to your model). Then the rules should not overwrite each other as long as your schema does not change.

At a code level, these issues mainly occur because field-level authorisation re-uses methods for object-level authorisation, but doesn’t account for the fact that variables declared in the velocity template are being re-used or over-written. Reducing the amount of work done at the field level, changing variable declarations to use defaultIfNull, re-setting isStaticGroupAuthorised for each field evaluation, and some property naming changes could tighten this all up.

I’m also confused about the auth design that allows updating field level properties when there is no permission to update at the object level. It should be documented how object-level and field-level rules interact, currently docs give the impression that field level rules can only further lock down access.