amplify-flutter icon indicating copy to clipboard operation
amplify-flutter copied to clipboard

Field level auth breaks all mutations

Open tiyberius opened this issue 1 year ago • 4 comments

Description

I've spent quite some time preparing this GitHub issue to make it as easy as possible to reproduce. I also believe it to be a pretty major issue given that it potentially breaks so many things.

Summary

From everything I can tell, implementing field level authorization as described by the Amplify commandline tool fundamentally breaks the API in that it causes every update mutation to respond with an "Unauthorized" error.

Environment

$ amplify --version
12.7.1
$ flutter --version
Flutter 3.13.9 • channel stable • https://github.com/flutter/flutter.git
Framework • revision d211f42860 (9 days ago) • 2023-10-25 13:42:25 -0700
Engine • revision 0545f8705d
Tools • Dart 3.1.5 • DevTools 2.25.0
$ flutter doctor
Doctor summary (to see all details, run flutter doctor -v):
[✓] Flutter (Channel stable, 3.13.9, on macOS 13.4.1 22F770820d darwin-arm64, locale en-US)
[✓] Android toolchain - develop for Android devices (Android SDK version 33.0.2)
[✓] Xcode - develop for iOS and macOS (Xcode 14.3.1)
[✓] Chrome - develop for the web
[✓] Android Studio (version 2022.1)
[✓] VS Code (version 1.83.1)
[✓] Connected device (5 available)
[✓] Network resources

• No issues found!

Categories

  • [ ] Analytics
  • [ ] API (REST)
  • [X] API (GraphQL)
  • [ ] Auth
  • [ ] Authenticator
  • [ ] DataStore
  • [ ] Notifications (Push)
  • [ ] Storage

Steps to Reproduce

  1. Clone the app I've put together to demonstrate this issue: git clone [email protected]:tiyberius/simple_thingy_app.git
  2. Change into the cloned directory cd simple_thingy_app/
  3. Initialize amplify root stack amplify init
  4. Create the Api, Auth, and PostConfirmation Lambda function amplify push -y
  5. Get all Flutter packages flutter pub get
  6. Start the app flutter run
  7. In the app, click on "Create Account" and register a new email and password, go through verification process, etc.
  8. After authentication, you should be presented with a counter value and a couple of buttons. On the backend, you should also have a new user in Cognito and a "Profile" record in Dynamo as a result of the Lambda PostConfirmation hook.
  9. At this point, you should be able to increment and decrement the counter by clicking the minus and plus buttons.

Now we are going to break things by implementing field level auth.

  1. Terminate the flutter app
  2. Check out the "break" branch git checkout break
  3. The "break" branch just adds field level auth to the owner field in the schema. Update the amplify stack amplify push -y
  4. Start the app again flutter run
  5. After starting the app, you should be presented with the counter again and the value of the counter should be whatever it was when you last updated it.
  6. Now try to increment or decrement the counter. You should see that the counter value doesn't change.

If you insert a print statement in the updateProfile() function of profile_api_service.dart, you should see that the response from the GraphQL mutation is

flutter: GraphQLResponse<Profile> error: [
flutter:   {
flutter:     "message": "Unauthorized on [owner]",
flutter:     "locations": [
flutter:       {
flutter:         "line": 1,
flutter:         "column": 96
flutter:       }
flutter:     ],
flutter:     "path": [
flutter:       "updateProfile"
flutter:     ],
flutter:     "errorType": "Unauthorized"
flutter:   }
flutter: ]

I was able to "fix" the issue by removing the owner from the variables in the request that gets sent as part of the updateProfile mutation.

  1. Checkout the "fix" branch git checkout fix
  2. Start the app flutter run
  3. Now increment or decrement the counter and it should work, and you'll see proof in the logs:
flutter: *** RESPONSE ***
flutter: GraphQLResponse<Profile> success: {
flutter:   "id": "b6a5862f-1555-4f4f-8c50-b3b321e7bc6e",
flutter:   "counter_value": 2,
flutter:   "email": "[email protected]",
flutter:   "owner": "f270f828-0621-4e8c-aacd-95943e235cd1",
flutter:   "createdAt": "2023-11-04T03:42:46.781000000Z",
flutter:   "updatedAt": "2023-11-04T04:15:11.106000000Z"
flutter: }

So as you can clearly see, by adding field level auth to the schema (as recommended by AWS) it breaks things in a very big way.

Looking forward to your thoughts!

Thanks.

PS

Both the official Amplify API documentation and the AWS Trip Planner Tutorial (see Step 6 under "Add the Amplify function to create the profile") say to set the "owner" field to "sub::userName", yet when we update the counter value for the first time, the "owner" field simply changes from "sub::userName" to just "userName", (sub and semicolons are removed). I think this is okay because the docs say "The API will authorize against the full value of <sub>::<username> or sub / username separately and return username." but it would be nice to get confirmation.

Screenshots

No response

Platforms

  • [X] iOS
  • [x] Android
  • [ ] Web
  • [ ] macOS
  • [ ] Windows
  • [ ] Linux

Flutter Version

3.13.9

Amplify Flutter Version

1.6.0

Deployment Method

Amplify CLI

Schema

No response

tiyberius avatar Nov 04 '23 04:11 tiyberius

Hello @tiyberius - Thanks for taking the time to open the issue and provide detailed reproduction steps. We will take a look and see if we can repro the issue.

Jordan-Nelson avatar Nov 10 '23 18:11 Jordan-Nelson

Hi @tiyberius, I was able to reproduce the issue by following your steps. I'm marking this as a bug now and will discuss with the team to move forward with this issue, thanks.

khatruong2009 avatar Dec 21 '23 22:12 khatruong2009

Hey, I have the same issue, when setting owner and using the flutter amplify codegen models feature a user cannot update anything they own, as it tries to update owner too, and results in unauthorized on [owner] error.

I think we need a way to exclude owner from the generated models or treat it slightly different (i.e. don't send it as part of a mutation if it hasn't changed)

  owner: String @auth(rules: [{ allow: owner, operations: [read, delete] }])

Currently on my app, users can take over other peoples profiles if they make a request manually...

Let me know if you'd like me to provide any more information!

adamtester avatar Jan 02 '24 09:01 adamtester

Hi @tiyberius, we've gone through the sample app and were able to find a solution to your problem. We updated the updateProfile call to include the authorizationMode argument in the ModelMutations.update call.

authorizationMode: APIAuthorizationType.iam

Here is the sample code:

    Future<void> updateProfile(Profile updatedProfile) async {
    try {
      final res = await Amplify.API
          .mutate(
            request: ModelMutations.update(updatedProfile,
                authorizationMode: APIAuthorizationType.iam
                ),
          )
          .response;
      print(res);
    } on Exception catch (error) {
      debugPrint('updateProfile failed: $error');
    }
  }

Please let us know if this fixes your issue. We will make this more clear in our documentation. @adamtester Could you also try this solution as well?

khatruong2009 avatar Feb 26 '24 22:02 khatruong2009

Closing this out as a solution was shared an we have not heard back. If you are still facing an issue, please let us know.

Jordan-Nelson avatar Apr 01 '24 15:04 Jordan-Nelson

Hi @tiyberius, we've gone through the sample app and were able to find a solution to your problem. We updated the updateProfile call to include the authorizationMode argument in the ModelMutations.update call.

authorizationMode: APIAuthorizationType.iam

Here is the sample code:

    Future<void> updateProfile(Profile updatedProfile) async {
    try {
      final res = await Amplify.API
          .mutate(
            request: ModelMutations.update(updatedProfile,
                authorizationMode: APIAuthorizationType.iam
                ),
          )
          .response;
      print(res);
    } on Exception catch (error) {
      debugPrint('updateProfile failed: $error');
    }
  }

Please let us know if this fixes your issue. We will make this more clear in our documentation. @adamtester Could you also try this solution as well?

I have the same problem, not on owner field but in all fields (i had to put field level authorization for all fields cause a bug in the schema). I dont think this solution is useful, the authorizationMode that i want to use is APIAuthorizationType.userPools,i can't give iam permission to all users in my app, they have to update just their own fields. Something else?

DanieleMoschiniMac avatar Jul 09 '24 11:07 DanieleMoschiniMac