Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Datastore update rejected for composite key with authorization restriction #11089

Closed
3 tasks done
alex-breen opened this issue Mar 15, 2023 · 6 comments
Closed
3 tasks done
Assignees
Labels
bug Something isn't working

Comments

@alex-breen
Copy link

alex-breen commented Mar 15, 2023

Before opening, please confirm:

JavaScript Framework

React

Amplify APIs

DataStore

Amplify Categories

auth, api

Environment information

# Put output below this line

System:
    OS: Windows 10 10.0.22621
    CPU: (6) x64 Intel(R) Core(TM) i5-9600K CPU @ 3.70GHz
    Memory: 5.11 GB / 15.94 GB
  Binaries:
    Node: 14.17.0 - C:\Program Files\nodejs\node.EXE
    npm: 8.10.0 - C:\Program Files\nodejs\npm.CMD
  Browsers:
    Chrome: 111.0.5563.65
    Edge: Spartan (44.22621.1413.0), Chromium (111.0.1661.41)
    Internet Explorer: 11.0.22621.1
  npmPackages:
    @aws-amplify/pubsub: ^5.0.16 => 5.0.16 (5.0.13)
    @aws-amplify/ui-react: ^4.3.7 => 4.3.7
    @aws-amplify/ui-react-internal:  undefined ()
    @testing-library/jest-dom: ^5.16.5 => 5.16.5
    @testing-library/react: ^13.4.0 => 13.4.0
    @testing-library/user-event: ^13.5.0 => 13.5.0
    aws-amplify: ^5.0.13 => 5.0.13
    aws-sdk: ^2.1327.0 => 2.1327.0
    react: ^18.2.0 => 18.2.0
    react-dom: ^18.2.0 => 18.2.0
    react-scripts: 5.0.1 => 5.0.1
    uuid: ^9.0.0 => 9.0.0 (3.4.0, 8.3.2, 8.0.0)
    web-vitals: ^2.1.4 => 2.1.4
  npmGlobalPackages:
    @aws-amplify/cli: 10.4.0
    @forge/cli: 4.2.0
    npm-check-updates: 14.0.1
    npm: 8.10.0
    typescript: 4.8.4

Describe the bug

Under the following conditions, a DataStore update is rejected:

  • Model has a composite key
  • One of the keys has a write restriction
  • A DataStore update is made to one of the keys that are not restricted

Rejected update can be seen in network logs, under the GraphQL mutation, showing the rejected is due to a field restriction / lack of authorization. Even though the restricted field is not updated, its value is sent in the variables of the update mutation.

Expected behavior

Datastore update (saves) the change.

Reproduction steps

Deploy model.
Run DataStore update (save) query via javascript.

Code Snippet

// Put your code below this line.

Model (owner field is restricted to read only; composite key comprised of 'userId' and 'owner')

type UserProfiles @model @auth(rules: [{allow: owner}]) {
  id: ID!
  userId: String @index(sortKeyFields: ["owner"])
  name: String
  email: String
  owner: String @auth(rules: [{allow: owner, operations: [read, delete]}])
}

Javascript code:

const original = await DataStore.query(UserProfiles, u => u.and(
    u => [
        u.userId.eq('12345'),
        u.owner.eq('ABC')
    ])
);
await DataStore.save(
    UserProfiles.copyOf(original[0], updated => {
      updated.name = 'new name'
    })
  );
}

Log output

// Put your logs below this line


aws-exports.js

No response

Manual configuration

No response

Additional configuration

No response

Mobile Device

No response

Mobile Operating System

No response

Mobile Browser

No response

Mobile Browser Version

No response

Additional information and screenshots

No response

@alex-breen alex-breen added the pending-triage Issue is pending triage label Mar 15, 2023
@chrisbonifacio chrisbonifacio self-assigned this Mar 15, 2023
@guy-a
Copy link

guy-a commented Mar 22, 2023

I'm experiencing a similar issue.
Let's say I'm using this model:

type UserProfiles @model @auth(rules: [{allow: owner}]) {
  id: ID!
  owner: String @auth(rules: [{allow: owner, operations: [read, delete]}])
  email: String @auth(rules: [{allow: owner, operations: [read, delete]}])
  name: String
}

Up until @aws-amplify@5.0.15 when saving a new UserProfile({name: 'Smith'}) it would have sent only {name: 'Smith'} in the input variables.
Since at least 5.0.18 (haven't tested 16, 17 because of other issues), it'll also send read only properties, i.e. {name: 'Smith', email: null}
This is causing the AppSync resolver to fail with the error “Unauthorized on [email]”

@iartemiev
Copy link
Member

iartemiev commented Apr 6, 2023

We released a fix for this issue in aws-amplify@5.0.24.

@alex-breen There's another bug in the Amplify codegen package where using @index without a name param can result in unexpected behavior. I suggest modifying your @index directive like this to mitigate it:

userId: String @index(name: "byUserIdAndOwner", sortKeyFields: ["owner"])

Feel free to use any other string value for name.

@iartemiev iartemiev reopened this Apr 6, 2023
@iartemiev
Copy link
Member

Apologies, I spoke too soon. This patch will be released later today. We'll post another update once it goes out.

@guy-a
Copy link

guy-a commented Apr 7, 2023

5.0.25 works!
Thank you for the fix.

There is still a minor different which is causing minor issues in my client:
Although the null properties are filtered before sending the model to the server, they are still there when creating a new model. It wasn't there in pervious DataStore versions.

Since null behave different than undefined in some cases, it's causing some minor issues. These are generally easily fixable, but thought i should let you know anyway.

@svidgen
Copy link
Member

svidgen commented Apr 7, 2023

@guy-a Awesome! The fix did go out yesterday in 5.0.25.

The change to have models instantiated with null instead of undefined is an intentional fix. With the exception of internal metadata fields, we found that leaving fields undefined initially led to "inconsistent" views, where an optional field would be undefined until it synchronized with the cloud, after which it would become null (in subsequent queries).

Prior to that fix, you'd have had to account for the possibility that an "empty" field could be either null or undefined. This was particularly glaring and glitchy feeling if you attempted to query for an empty (or not-empty) field. E.g., we should expect this to select all records where alias fields are empty:

await DataStore.query(Profile, p => p.alias.eq(null));

Prior to the fix, this would only match "empty" fields that were either explicitly set to null or those was were organically normalized after the cloud sync.

Thanks for calling it out though! If something like that had been an unintended side effect of the fix in 5.0.25, we would have needed to jump on it!

@svidgen
Copy link
Member

svidgen commented Apr 7, 2023

@alex-breen The library side of this is resolved. Per @iartemiev's comment, to see correct behavior, you'd also need to update your indexes to work around amplify-codegen/issues/561 until it is resolved.

@svidgen svidgen closed this as completed Apr 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants