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.44k stars 2.13k forks source link

Custom claims not working any longer after upgrading to v6 #12643

Closed hanna-becker closed 1 year ago

hanna-becker commented 1 year ago

Before opening, please confirm:

JavaScript Framework

Angular

Amplify APIs

Authentication, GraphQL API, Storage

Amplify Categories

auth, storage, function, api

Environment information

``` # Put output below this line ➜ npx envinfo --system --binaries --browsers --npmPackages --duplicates --npmGlobalPackages Need to install the following packages: envinfo@7.11.0 Ok to proceed? (y) System: OS: Linux 5.15 Ubuntu 22.04.3 LTS 22.04.3 LTS (Jammy Jellyfish) CPU: (24) x64 AMD Ryzen 9 5900X 12-Core Processor Memory: 23.30 GB / 31.31 GB Container: Yes Shell: 5.1.16 - /bin/bash Binaries: Node: 18.18.2 - ~/.nvm/versions/node/v18.18.2/bin/node Yarn: 1.22.21 - ~/.nvm/versions/node/v18.18.2/bin/yarn npm: 9.8.1 - ~/.nvm/versions/node/v18.18.2/bin/npm Browsers: Chrome: 119.0.6045.159 Chromium: 119.0.6045.159 npmPackages: @amplitude/analytics-browser: ^2.2.3 => 2.3.5 @angular-builders/jest: ^17.0.0 => 17.0.0 @angular-devkit/build-angular: ^17.0.3 => 17.0.3 @angular-eslint/builder: ^17.1.0 => 17.1.0 @angular-eslint/eslint-plugin: ^17.1.0 => 17.1.0 @angular-eslint/eslint-plugin-template: ^17.1.0 => 17.1.0 @angular-eslint/schematics: ^17.1.0 => 17.1.0 @angular-eslint/template-parser: ^17.1.0 => 17.1.0 @angular/animations: ^17.0.4 => 17.0.4 @angular/cdk: ^17.0.1 => 17.0.1 @angular/cli: ^17.0.3 => 17.0.3 @angular/common: ^17.0.4 => 17.0.4 @angular/compiler: ^17.0.4 => 17.0.4 @angular/compiler-cli: ^17.0.4 => 17.0.4 @angular/core: ^17.0.4 => 17.0.4 @angular/forms: ^17.0.4 => 17.0.4 @angular/language-service: ^17.0.4 => 17.0.4 @angular/material: ^17.0.1 => 17.0.1 @angular/platform-browser: ^17.0.4 => 17.0.4 @angular/platform-browser-dynamic: ^17.0.4 => 17.0.4 @angular/router: ^17.0.4 => 17.0.4 @angular/youtube-player: ^17.0.1 => 17.0.1 @aws-amplify/ui-angular: ^5.0.2 => 5.0.2 @babel/core: ^7.20.2 => 7.23.3 (7.23.2) @babel/plugin-proposal-decorators: ^7.22.3 => 7.23.3 @babel/preset-env: ^7.22.4 => 7.23.3 (7.23.2) @babel/preset-typescript: ^7.21.5 => 7.23.3 @bahmutov/cypress-esbuild-preprocessor: ^2.2.0 => 2.2.0 @commitlint/cli: ^17.6.6 => 17.8.1 @commitlint/config-conventional: ^17.6.6 => 17.8.1 @cypress/angular: 0.0.0-development @cypress/mount-utils: 0.0.0-development @cypress/react: 0.0.0-development @cypress/react18: 0.0.0-development @cypress/schematic: ^2.5.1 => 2.5.1 @cypress/svelte: 0.0.0-development @cypress/vue: 0.0.0-development @cypress/vue2: 0.0.0-development @esbuild-plugins/node-globals-polyfill: ^0.2.3 => 0.2.3 @esbuild-plugins/node-modules-polyfill: ^0.2.2 => 0.2.2 @frsource/cypress-plugin-visual-regression-diff: ^3.3.10 => 3.3.10 @jspm/core: ^2.0.1 => 2.0.1 @juggle/resize-observer: ^3.3.1 => 3.4.0 @ngrx/component: ^17.0.0 => 17.0.1 @ngrx/effects: ^17.0.0 => 17.0.1 @ngrx/entity: ^17.0.0 => 17.0.1 @ngrx/router-store: ^17.0.0 => 17.0.1 @ngrx/store: ^17.0.0 => 17.0.1 @ngrx/store-devtools: ^17.0.0 => 17.0.1 @types/jest: ^29.5.3 => 29.5.10 @types/node: ^20.2.5 => 20.10.0 (20.5.1, 18.18.13, 14.18.63) @types/spotify-web-playback-sdk: ^0.1.15 => 0.1.19 @typescript-eslint/eslint-plugin: ^5.59.1 => 5.62.0 @typescript-eslint/parser: ^5.59.1 => 5.62.0 angular-audio-context: ^35.0.7 => 35.0.7 autoprefixer: ^10.4.7 => 10.4.16 aws-amplify: ^6.0.5 => 6.0.5 aws-amplify/adapter-core: undefined () aws-amplify/analytics: undefined () aws-amplify/analytics/kinesis: undefined () aws-amplify/analytics/kinesis-firehose: undefined () aws-amplify/analytics/personalize: undefined () aws-amplify/analytics/pinpoint: undefined () aws-amplify/api: undefined () aws-amplify/api/server: undefined () aws-amplify/auth: undefined () aws-amplify/auth/cognito: undefined () aws-amplify/auth/cognito/server: undefined () aws-amplify/auth/server: undefined () aws-amplify/datastore: undefined () aws-amplify/in-app-messaging: undefined () aws-amplify/in-app-messaging/pinpoint: undefined () aws-amplify/push-notifications: undefined () aws-amplify/push-notifications/pinpoint: undefined () aws-amplify/storage: undefined () aws-amplify/storage/s3: undefined () aws-amplify/storage/s3/server: undefined () aws-amplify/storage/server: undefined () aws-amplify/utils: undefined () aws-sdk: ^2.1360.0 => 2.1505.0 babel-loader: ^9.1.2 => 9.1.3 babel-plugin-dynamic-import-node: ^2.3.3 => 2.3.3 chokidar: ^3.5.2 => 3.5.3 commitizen: ^4.3.0 => 4.3.0 cross-spawn: ^7.0.3 => 7.0.3 cypress: 12.14.0 => 12.14.0 cypress-duration-metrics: ^1.1.2 => 1.1.2 cypress-social-logins: ^1.14.1 => 1.14.1 cz-conventional-changelog: ^3.3.0 => 3.3.0 echarts: ^5.4.3 => 5.4.3 esbuild: ^0.19.8 => 0.19.8 (0.19.5, 0.18.20) eslint: ^8.38.0 => 8.54.0 eslint-config-prettier: ^8.5.0 => 8.10.0 eslint-plugin-cypress: ^2.15.1 => 2.15.1 eslint-plugin-import: 2.27.5 => 2.27.5 eslint-plugin-jsdoc: 43.0.0 => 43.0.0 eslint-plugin-ngrx: ^2.1.4 => 2.1.4 eslint-plugin-no-skipped-tests: file:./eslint-plugin-no-skipped-tests => 1.7.0 eslint-plugin-prefer-arrow: 1.2.3 => 1.2.3 eslint-plugin-prettier: ^5.0.0 => 5.0.1 eslint-plugin-rxjs: ^5.0.2 => 5.0.3 eslint-plugin-rxjs-angular: ^2.0.1 => 2.0.1 everpolate: 0.0.3 => 0.0.3 express: ^4.17.1 => 4.18.2 fabric: 6.0.0-beta15 => 6.0.0-beta15 finalhandler: ^1.1.2 => 1.2.0 (1.1.0) http: 0.0.1-security => 0.0.1-security husky: ^8.0.3 => 8.0.3 jest: ^29.6.2 => 29.7.0 jest-junit: ^16.0.0 => 16.0.0 jest-preset-angular: ^13.1.4 => 13.1.4 ngrx-store-freeze: ^0.2.4 => 0.2.4 ngx-echarts: ^17.1.0 => 17.1.0 ngx-file-drop: ^15.0.0 => 15.0.0 ngx-image-cropper: ^6.3.3 => 6.3.4 ngx-scrollbar: ^13.0.3 => 13.0.3 node-fetch: ^3.2.4 => 3.3.2 (2.7.0) pako: ^2.1.0 => 2.1.0 (1.0.11) spotify-web-api-js: ^1.5.2 => 1.5.2 standard-version: ^9.5.0 => 9.5.0 start-server-and-test: ^1.14.0 => 1.15.5 supports-webp: ^3.0.0 => 3.0.0 swiper: ^8.4.5 => 8.4.7 swiper_angular: 0.0.1 tailwindcss: ^3.1.4 => 3.3.5 ts-jest: ^29.1.1 => 29.1.1 ts-node: ^10.9.1 => 10.9.1 typescript: ~5.2.2 => 5.2.2 (5.3.2) uuid: ^9.0.0 => 9.0.1 (8.3.2, 8.0.0) wavesurfer.js: ^6.6.4 => 6.6.4 webpack: ^5.84.1 => 5.89.0 webpack-bundle-analyzer: ^4.8.0 => 4.10.1 zen-observable-ts: ^1.1.0 => 1.1.0 zone.js: ~0.14.2 => 0.14.2 npmGlobalPackages: @angular/cli: 17.0.2 @aws-amplify/cli: 12.8.2 @expo/ngrok: 4.1.3 corepack: 0.19.0 eas-cli: 5.9.1 npm: 9.8.1 yarn: 1.22.21 ```

Describe the bug

This bug is related to the one opened here, but slightly different, as it affects custom claims, not group claims. While GROUP claims still work for queries and mutations in our case, CUSTOM claims stopped working altogether in amplify v6.

This is our model of a Project. There are members that have read-write permissions and there are invitees that have read-only permissions:

type Project
  @model(
    queries: { get: "getProject" }
    mutations: { create: "createProject", update: "updateProject" }
    subscriptions: null
  )
  @auth(
    rules: [
      { allow: owner }
      { allow: groups, groupsField: "group", operations: [read, update] } # We add these claims in alter-claim.js - called by PreToken generator Lambda
      { allow: groups, groupsField: "group", groupClaim: "project:invitee", operations: [read] }
    ]
  ) {
  id: ID!
  group: String! # Duplicate of id -> workaround until https://github.com/aws-amplify/amplify-cli/issues/9788 is fixed
  name: String!
  description: String
  ...
}

In the pre token generation lambda, we add those permissions as custom/group claims, respectively:

// in alter-claims.js
const projectIDsReadWrite = await queryMemberProjectIDs(userID);
const projectIDsReadOnly = await queryInvitedProjectIDs(userID);

event.response = {
  claimsOverrideDetails: {
    claimsToAddOrOverride: {
      'project:invitee': JSON.stringify(projectIDsReadOnly), // this custom claim stopped working altogether in amplify v6
    },
    groupOverrideDetails: {
      groupsToOverride: projectIDsReadWrite, // this group claim works for queries & mutations
    },
  },
};

Looking at the idToken in the browser (In Chrome dev tools: Application -> Local storage -> localhost/app url -> CognitoIdentityServiceProvider.{{sth.sth}}.idToken -> decode value e.g. here: https://jwt.io/), I can see the claims are still being set correctly from the pre token lambda function. The payload json looks something like this (ids are altered for this post). This is for my test user who is a full member of 2 projects (read-write access) and invited to another one (read-only access). :

{
  "sub": "12345678-9101-1121-3141-5161171819202"
  "cognito:groups": [
    "f411ed1b-3de8-41cf-b1ac-6fd21ec001d8",
    "8a57b40c-270c-45f0-94d6-68b3ac666d42"
  ],
  "project:invitee": "[\"5332e068-0aab-4738-bd97-897f1a7bace4\"]",
  ...
}

However, when querying read-only projects for this user, I get an unauthorized error from the grapqhql API call, indicating the read-only claim is potentially not being applied correctly:

"errorType":"Unauthorized" "message":"Not Authorized to access getMusicProject on type Query"

This is working with amplify v5.

As opposed to the subscription issue I referenced above, we know the idToken is being passed down correctly for the query. Otherwise we would also see authorization errors for for the 2 projects our test user is a member of. But there seems to have been some parsing logic on the custom claim in v5 that is not there any more or different in v6.

Expected behavior

We can use custom claims as in v5 without getting unauthorized errors.

Reproduction steps

Reproduction steps:

  1. Create an amplify app with Cognito user pool, no datastore, and this schema:
    type Project
    @model(
    queries: { get: "getProject" }
    mutations: { create: "createProject", update: "updateProject" }
    subscriptions: null
    )
    @auth(
    rules: [
      { allow: owner }
      { allow: groups, groupsField: "group", operations: [read, update] } # We add these claims in alter-claim.js - called by PreToken generator Lambda
      { allow: groups, groupsField: "group", groupClaim: "project:invitee", operations: [read] }
    ]
    ) {
    id: ID!
    group: String! # Duplicate of id -> workaround until https://github.com/aws-amplify/amplify-cli/issues/9788 is fixed
    name: String!
    description: String
    ...
    }
  2. Create a Cognito user that can log into the application, e.g. via the authenticator component from one of the amplify-ui packages.
  3. Create 2 projects in DDB that each have the owner set to something other than the user's id.
  4. Use amplify update auth command with the amplify cli to create a pre token generation lambda and override claims like shown above. Instead of querying the project ids, just hard-code them to the ids of the 2 projects you created in DDB, one for the 'project:invitee' claim, the other one for the groupsToOverride claim (this is a normal JS array).
  5. Query the projects with grapqhl from the application. You should see that it works for the group claim, but not for the 'project:invitee'.
  6. Notice that it works in both cases with amplify v5.

Code Snippet

// Put your code below this line.

Log output

``` // Put your logs below this line ```

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

hanna-becker commented 1 year ago

Upon further investigation I can see that the groups are being passed on in the Graphql request, but the custom claim isn't. Meaning, the idToken that I see in dev tools network tab, when decoded, has a "cognito:groups" array, but is missing my "project:invitee" claim, even though both are present in my local storage's idToken.

hanna-becker commented 1 year ago

Also, overriding the api config like mentioned in the other issue makes no difference for me:

// V6
import { Amplify } from "aws-amplify";
import { fetchAuthSession } from "aws-amplify/auth";
import config from "./amplifyconfiguration.json";

Amplify.configure(config, {
  API: {
    GraphQL: {
      headers: async () => ({
        Authorization: (
          await fetchAuthSession()
        ).tokens?.idToken?.toString() as string,
      }),
    },
  },
});

Whether I have this override or not, in both cases the idToken of the request has "cognito:groups", but not "project:invitee".

hanna-becker commented 1 year ago

So in summary it seems like the idToken that my pre token generator lambda produces with correct values is being decoded, the non-group custom claim stripped away, and then reencoded before making the API call to AppSync somewhere in the client.

hanna-becker commented 1 year ago

New update: The override above does work to patch the "project:invitee" claim, I just got the syntax wrong. Still wondering why without this override group claims are passed on automatically, while others are stripped away from the id token?

hanna-becker commented 1 year ago

If it is intended behavior that non-group custom claims get stripped from the id token, while group custom claims aren't, I'm ready to close this issue. Otherwise it'd be worth investigating/fixing this behvior.

chrisbonifacio commented 1 year ago

Hi @hanna-becker 👋 thank you for raising this issue and providing us the context of your troubleshooting.

I will discuss this with the team to confirm if it is expected behavior.

Also, out of curiosity, when you said:

The override above does work to patch the "project:invitee" claim, I just got the syntax wrong

Does this mean you are unblocked and able to use custom claims again? Lastly, what was the change you had to make for it to work?

hanna-becker commented 1 year ago

Hi @chrisbonifacio, thank you for getting back to me so quickly!

Yes, I'm unblocked. After reading a bit more, I believe what I'm observing is expected: Normally, the header contains the access token, which includes the Cognito groups (in my case the ones added in the pre token generator lambda). But any additional custom claims (in my case "project:invitee") are only included in the id token, so to use them we need to configure amplify to send the id token, instead of the access token when making api calls:

import { Amplify } from "aws-amplify";
import { fetchAuthSession } from "aws-amplify/auth";
import config from "./amplifyconfiguration.json";

Amplify.configure(config, {
  API: {
    GraphQL: {
      headers: async () => ({
        Authorization: (
          await fetchAuthSession()
        ).tokens?.idToken?.toString() as string,
      }),
    },
  },
});

The reason it didn't work initially is that I got confused with the syntax (the header now gets passed in a second argument to Amplify.configure, while in v5 it got passed as part of the first arg). It works now!

The whole topic of adding/overriding group and custom claims needs way better documentation, though. There is already an open ticket for better docs around custom claims, and I will add a suggestion there and close this ticket.