aws-amplify / amplify-js

A declarative JavaScript library for application development using cloud services.
https://docs.amplify.aws/lib/q/platform/js
Apache License 2.0
9.43k stars 2.13k forks source link

AppSync throwing 401s at inappropriate times #10388

Closed jdanielsyeah closed 2 years ago

jdanielsyeah commented 2 years ago

Before opening, please confirm:

JavaScript Framework

Next.js

Amplify APIs

Authentication, DataStore

Amplify Categories

auth, api

Environment information

``` # Put output below this line System: OS: macOS 12.6 CPU: (8) x64 Intel(R) Core(TM) i5-1038NG7 CPU @ 2.00GHz Memory: 24.00 MB / 16.00 GB Shell: 5.8.1 - /bin/zsh Binaries: Node: 16.17.0 - /usr/local/bin/node Yarn: 1.22.19 - /usr/local/bin/yarn npm: 8.15.0 - /usr/local/bin/npm Browsers: Chrome: 105.0.5195.125 Safari: 16.0 npmPackages: @ampproject/toolbox-optimizer: undefined () @aws-amplify/pubsub: ^4.5.3 => 4.5.3 (4.4.8) @aws-amplify/ui-react: ^3.2.1 => 3.2.1 @aws-amplify/ui-react-internal: undefined () @aws-amplify/ui-react-legacy: undefined () @babel/core: ^7.19.0 => undefined (7.19.0, ) @babel/preset-env: ^7.19.0 => 7.19.0 @babel/runtime: 7.15.4 @date-io/date-fns: ^2.14.0 => 2.14.0 @edge-runtime/primitives: 1.1.0-beta.17 @emotion/react: ^11.10.0 => 11.10.0 @emotion/styled: ^11.10.0 => 11.10.0 @hapi/accept: undefined () @mui/icons-material: ^5.8.4 => 5.8.4 @mui/lab: ^5.0.0-alpha.96 => 5.0.0-alpha.96 @mui/material: ^5.9.2 => 5.9.2 @mui/x-date-pickers: ^5.0.0-beta.3 => 5.0.0-beta.3 @napi-rs/triples: undefined () @next/react-dev-overlay: undefined () @types/ajv: ^1.0.0 => 1.0.0 @types/email-validator: ^1.0.6 => 1.0.6 @types/jest: ^29.0.0 => 29.0.0 @types/lodash: ^4.14.182 => 4.14.182 @types/node: ^18.7.15 => 18.7.16 @types/react: ^17.0.15 => 17.0.48 @typescript-eslint/eslint-plugin: ^5.36.1 => 5.36.2 @typescript-eslint/parser: ^5.36.1 => 5.36.2 @vercel/nft: undefined () acorn: undefined () ajv: ^8.11.0 => 8.11.0 (6.12.6) ajv-draft-04: ^1.0.0 => 1.0.0 ajv-formats: ^2.1.1 => 2.1.1 amphtml-validator: undefined () arg: undefined () assert: undefined () async-retry: undefined () async-sema: undefined () aws-amplify: ^4.3.28 => 4.3.29 axios: ^0.27.2 => 0.27.2 (0.26.0) babel-jest: ^28.0.2 => 28.1.3 babel-packages: undefined () browserify-zlib: undefined () browserslist: undefined () buffer: undefined () bytes: undefined () chalk: undefined () ci-info: undefined () cli-select: undefined () comment-json: undefined () compression: undefined () conf: undefined () constants-browserify: undefined () content-disposition: undefined () content-type: undefined () cookie: undefined () cross-spawn: undefined () crypto-browserify: undefined () cssnano-simple: undefined () date-fns: ^2.29.1 => 2.29.1 debug: undefined () devalue: undefined () domain-browser: undefined () edge-runtime: undefined () eslint: 8.20.0 => 8.20.0 eslint-config-next: 12.2.3 => 12.2.3 events: undefined () find-cache-dir: undefined () find-up: undefined () fresh: undefined () get-orientation: undefined () glob: undefined () gzip-size: undefined () http-proxy: undefined () https-browserify: undefined () icss-utils: undefined () ignore-loader: undefined () image-size: undefined () is-animated: undefined () is-docker: undefined () is-wsl: undefined () jest-worker: undefined () json5: undefined () jsonwebtoken: undefined () loader-utils: undefined () lodash: ^4.17.21 => 4.17.21 lodash.curry: undefined () lru-cache: undefined () material-ui-phone-number: ^3.0.0 => 3.0.0 micromatch: undefined () mini-css-extract-plugin: undefined () moment: ^2.29.4 => 2.29.4 nanoid: undefined () native-url: undefined () neo-async: undefined () next: 12.2.3 => 12.2.3 node-fetch: undefined () node-html-parser: undefined () notistack: ^2.0.5 => 2.0.5 ora: undefined () os-browserify: undefined () p-limit: undefined () path-browserify: undefined () postcss-flexbugs-fixes: undefined () postcss-modules-extract-imports: undefined () postcss-modules-local-by-default: undefined () postcss-modules-scope: undefined () postcss-modules-values: undefined () postcss-preset-env: undefined () postcss-safe-parser: undefined () postcss-scss: undefined () postcss-value-parser: undefined () process: undefined () punycode: undefined () querystring-es3: undefined () raw-body: undefined () react: ^17.0.2 => 17.0.2 (18.0.0) react-dom: ^17.0.2 => 17.0.2 react-is: 17.0.2 react-refresh: 0.12.0 react-server-dom-webpack: undefined () regenerator-runtime: 0.13.4 sass-loader: undefined () schema-utils: undefined () semver: undefined () send: undefined () setimmediate: undefined () source-map: undefined () stream-browserify: undefined () stream-http: undefined () string-hash: undefined () string_decoder: undefined () strip-ansi: undefined () tar: undefined () terser: undefined () text-table: undefined () timers-browserify: undefined () ts-jest: ^28.0.8 => 28.0.8 ts-node: ^10.9.1 => 10.9.1 tty-browserify: undefined () typescript: ^4.8.2 => 4.8.2 ua-parser-js: undefined () unistore: undefined () util: undefined () vm-browserify: undefined () watchpack: undefined () web-vitals: undefined () webpack: undefined () webpack-sources: undefined () ws: undefined () npmGlobalPackages: @aws-amplify/cli: 9.1.0 @types/node: 18.7.2 corepack: 0.12.1 eslint: 8.21.0 n: 9.0.0 nodemon: 2.0.19 npm: 8.15.0 serverless: 3.22.0 ts-node: 10.9.1 typescript: 4.7.4 yarn: 1.22.19 ```

Describe the bug

AppSync is throwing 401 errors at confusing (and I suspect inappropriate times). I have a graphql schema:

type Card
  @model
  @auth(
    rules: [
      { allow: private, provider: iam }
      { allow: private, provider: userPools, operations: [create] }
      { allow: groups, groupsField: "canRead", operations: [read] }
      { allow: groups, groupsField: "canUpdate", operations: [update] }
    ]
  ) {
  approvedTime: AWSTimestamp
  canRead: [String]
  canUpdate: [String]
  id: ID!
...
}

When I try to update in the AppSync console with this:

mutation MyMutation($approvedTime: AWSTimestamp = 0) {
  updateCard(input: {id: "4ec6e3d5-f27e-454e-9221-1a1f4e96cd4c", approvedTime: $approvedTime, approvedUser: ""}) {
    id
    approvedTime
  }
}

The above works successfully, as expected.

When I replace 0 with undefined I get the following error: "Validation error of type BadValueForDefaultArg: Bad default value EnumValue{name='undefined'} for type AWSTimestamp". This makes sense to me.

When I try the above and replace 0 with null I get the following error: "Unauthorized on [approvedTime]". This is confusing.

What is extra confusing, is if I update the Card object in the front-end using DataStore as follows:

 await DataStore.save(
    Card.copyOf(initialCardDetails as Card, (updated) => {
        (updated.approvedTime = undefined),
    })
  );

This throws a 401 in my console. The network response reads:

{
  "errors" : [ {
    "errorType" : "UnauthorizedException",
    "message" : "Permission denied"
  } ]
}

Whereas if I set updated.approvedTime = null the update works fine. This is the opposite behaviour to what's happening in the AppSync console - which is confusing on it's own. What is additionally confusing is that this isn't an issue that can be fixed by updating authorization - this is an error with bad data inputs. I've just spent hours testing different @auth rules in my schema when the issue was with my js code all along.

Expected behavior

AWS suggests that a 401 should occur because:: "The request is denied by either AWS AppSync or the authorization mode because the credentials are missing or invalid. (...)"

It would be more useful if I had received the same error when I entered undefined in the AppSync console instead of any of the 401s that I received.

Reproduction steps

  1. Install basic NextJS app as per Amplify docs
  2. Set up Card object in model
  3. Set up iam and Cognito authentication
  4. Update GraphQL schema as above
  5. Update Amplify config as per Amplify docs concerning dynamic authentication with groupsField
  6. Execute the above DataStore call

Code Snippet

// Put your code below this line.

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

chrisbonifacio commented 2 years ago

Hi @jdanielsyeah 👋 thanks for raising this issue. This is definitely confusing behavior but one thing that stands out to me is that you are using dynamic group field auth rules. DataStore does not support dynamic auth rules, only static.

Here are the supported auth rules for DataStore: https://docs.amplify.aws/lib/datastore/setup-auth-rules/q/platform/js

This probably isn't the cause of the error but is worth pointing out so you can adjust the schema accordingly.

jdanielsyeah commented 2 years ago

Thanks for the prompt reply @chrisbonifacio . And thanks for pointing me to that documentation. I'd encountered it before but had a suspicion/hope that I might still be able to get some functionality from DataStore with the dynamic auth rules. The reason being that the Amplify Cli documentation: User group-based data access states "Known limitation: Real-time subscriptions are not supported for dynamic group authorization." (ie, suggesting that other features might work?) - but on attempting to use it I am finding that if I test deleting something (in a test intended to fail since I have set my @auth rules to prevent deletions), DataStore will delete items locally (contrary to my @auth rules) but then fail (401 - as per my rules) to delete them remotely. This suggests the rules are not taking effect on DataStore properly.

I would like to follow your suggestion to update the Schema, but I think it's too limited for the use-case we have. We want to retain one, single database table so that it's easier/simpler to retain records and review update history. In order to achieve this, I don't see any other way than having dynamic auth rules in place to prevent certain user groups from editing records. The only options Amplify seems to offer is having multiple tables with different auth rules, or the same table with static rules.

chrisbonifacio commented 2 years ago

No problem!

A couple things:

Regarding the behavior where DataStore will delete records locally but the same records persist on the server - DataStore cannot enforce auth rules or certain types of validation on the client so it will manipulate the local store as such. Mutation errors can be caught by the errorHandler if that information would be useful. Otherwise, the locally deleted record would be synced back down from the server again.

I agree that DataStore's supported auth rules do not fit your use case, but you could also leverage the API.graphql library directly which does support dynamic auth. Unless you also require offline capabilities, I would explore that route.

Lastly, I'm curious what authorization mode you might've been using when attempting the mutation in the AppSync console? Was it IAM or Cognito User Pools?

jdanielsyeah commented 2 years ago

I was using Cognito User Pools within the AppSync console. It seems to work well with the Dynamic Group Authorization rules.

I'm grateful to get your clarification. I thought I might ask for one more: I've been trying to find how to remove DataStore from my front-end (NextJS) and have only found the following instructions on this github issue:

_1. Type in terminal 'amplify update api'

  1. Select 'GraphQL'
  2. Select 'Disable DataStore from entire API'_

I just wanted to check this was the correct method before I ran it, because I couldn't find these commands detailed in any documentation?

chrisbonifacio commented 2 years ago

That seems to be correct.

amplify update api -> GraphQL -> Disable conflict detection -> amplify push

chrisbonifacio commented 2 years ago

Going to close this issue as resolved, but for anyone that might run into this error in the future - please note that nullable fields in the schema should be set to null instead of undefined from the client side, whether using DataStore or API.graphql.