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.42k stars 2.12k forks source link

DataStore JS Web Authorization Configuration #11870

Open jared-ookla opened 1 year ago

jared-ookla commented 1 year ago

Before opening, please confirm:

JavaScript Framework

React

Amplify APIs

DataStore

Amplify Categories

api

Environment information

``` System: OS: macOS 13.4.1 CPU: (12) arm64 Apple M2 Pro Memory: 74.45 MB / 32.00 GB Shell: 5.9 - /bin/zsh Binaries: Node: 16.5.0 - ~/.nvm/versions/node/v16.5.0/bin/node npm: 7.19.1 - ~/.nvm/versions/node/v16.5.0/bin/npm Browsers: Chrome: 116.0.5845.110 Firefox: 116.0.2 Safari: 16.5.2 npmPackages: @aws-amplify/cache: ^5.0.12 => 5.1.5 (4.0.66) @aws-amplify/core: ^4.7.8 => 4.7.15 (5.7.0) @aws-amplify/core/internals/aws-client-utils: undefined () @aws-amplify/core/internals/aws-client-utils/composers: undefined () @aws-amplify/core/internals/aws-clients/pinpoint: undefined () @aws-amplify/datastore: ^3.14.0 => 3.14.7 @babel/cli: ^7.19.3 => 7.22.9 @babel/core: ^7.19.3 => 7.22.9 @babel/plugin-proposal-class-properties: ^7.8.3 => 7.18.6 @babel/plugin-proposal-object-rest-spread: ^7.8.3 => 7.20.7 @babel/plugin-syntax-dynamic-import: ^7.8.3 => 7.8.3 @babel/plugin-syntax-jsx: ^7.8.3 => 7.22.5 @babel/plugin-transform-arrow-functions: ^7.13.0 => 7.22.5 @babel/plugin-transform-block-scoping: ^7.14.4 => 7.22.5 @babel/plugin-transform-modules-commonjs: ^7.8.3 => 7.22.5 @babel/plugin-transform-template-literals: ^7.13.0 => 7.22.5 @babel/preset-env: ^7.19.4 => 7.22.9 @babel/preset-react: ^7.8.3 => 7.22.5 @babel/register: ^7.8.3 => 7.22.5 @maxmind/geoip2-node: ^3.3.0 => 3.3.0 @mui/material: ^5.14.1 => 5.14.1 @ookla/ad-manager: ^0.72.0 => 0.72.0 @ookla/common-payment-client: ^0.0.9 => 0.0.9 @ookla/common-payment-node: 0.0.15 => 0.0.15 @ookla/costanza: ~2.1.0 => 2.1.0 @ookla/gulp-hash-filename: ~1.1.1 => 1.1.1 @ookla/gulp-spawn-mocha: ~3.1.2 => 3.1.2 @ookla/mock-socket: ^4.0.1 => 4.0.1 @ookla/pure: ^0.3.0-2 => 0.3.0-2 @ookla/speedtest-js-engine: ^2.8.16 => 2.8.17 @ookla/thorax: 2.2.1-ookla.3 => 2.2.1-ookla.3 @sailshq/lodash: ^3.10.4 => 3.10.4 @testing-library/react: ^12.1.5 => 12.1.5 accept-language-parser: ^1.5.0 => 1.5.0 acorn: ^7.4.1 => 7.4.1 (6.4.2, 1.2.2, 8.2.4, 8.10.0) animation-frame: ^0.3.0 => 0.3.0 assert: ^2.0.0 => 2.0.0 axios: ^0.21.1 => 0.21.4 (0.26.0, 0.27.2) axios-retry: ^3.2.4 => 3.2.4 babel-eslint: ^10.0.3 => 10.0.3 babel-loader: ^8.0.6 => 8.0.6 backbone: ^1.4.0 => 1.4.0 backbone-query-parameters: ^0.4.0 => 0.4.0 basic-auth-connect: ^1.0.0 => 1.0.0 blanket: ^1.2.3 => 1.2.3 bluebird: ^3.5.5 => 3.5.5 bluebird-retry: ^0.11.0 => 0.11.0 body-parser: ^1.19.0 => 1.19.0 bowser: ^2.9.0 => 2.11.0 browserify-zlib: ^0.2.0 => 0.2.0 bunyan: ^1.8.12 => 1.8.12 bunyan-format: ^0.2.1 => 0.2.1 bunyan-logstash: ^0.3.4 => 0.3.4 chai: ^4.2.0 => 4.2.0 chai-as-promised: ^7.1.1 => 7.1.1 chai-jq: 0.0.9 => 0.0.9 compression: ^1.7.4 => 1.7.4 config: ^3.2.6 => 3.2.6 (1.31.0) cookie-parser: ^1.4.4 => 1.4.4 core-js: ^3.6.4 => 3.6.4 (2.6.12) cors: ^2.8.5 => 2.8.5 cross-spawn: ^7.0.1 => 7.0.1 (6.0.5, 7.0.3) d3: ^5.15.0 => 5.15.0 del: ^5.1.0 => 5.1.0 (4.1.1) errorhandler: ^1.5.1 => 1.5.1 eslint: ^6.8.0 => 6.8.0 eslint-plugin-filenames: ^1.3.2 => 1.3.2 eslint-plugin-import: ^2.20.1 => 2.20.1 eslint-plugin-mocha: ^6.3.0 => 6.3.0 eslint-plugin-react: ^7.18.3 => 7.18.3 eslint-utils: ^2.0.0 => 2.0.0 (1.4.3) exports-loader: ^0.7.0 => 0.7.0 express: ^4.17.1 => 4.17.1 express-bunyan-logger: ^1.3.3 => 1.3.3 express-promise-router: ^3.0.3 => 3.0.3 express-session: ^1.17.0 => 1.17.0 express-sitemap: ^1.8.0 => 1.8.0 express-subdomain: ^1.0.5 => 1.0.5 flat: ^5.0.2 => 5.0.2 form-autofill: ^0.2.0 => 0.2.0 forwarded: ^0.1.2 => 0.1.2 git-repo-info: ^2.1.1 => 2.1.1 gulp: ^4.0.2 => 4.0.2 gulp-babel: ^8.0.0 => 8.0.0 gulp-clean-css: ^4.2.0 => 4.2.0 gulp-cli: ^2.3.0 => 2.3.0 gulp-eslint: ^6.0.0 => 6.0.0 gulp-filter: ^6.0.0 => 6.0.0 gulp-gzip: ^1.4.2 => 1.4.2 gulp-if: ^3.0.0 => 3.0.0 gulp-ignore: ^3.0.0 => 3.0.0 gulp-install: ^1.1.0 => 1.1.0 gulp-json-transform: ^0.4.7 => 0.4.7 gulp-less: ^4.0.1 => 4.0.1 gulp-mocha: ^8.0.0 => 8.0.0 gulp-newer: ^1.4.0 => 1.4.0 gulp-nodemon: ^2.5.0 => 2.5.0 gulp-plumber: ^1.2.1 => 1.2.1 gulp-rename: ^2.0.0 => 2.0.0 gulp-rtlcss: ^1.4.2 => 1.4.2 gulp-sourcemaps: ^3.0.0 => 3.0.0 gulp-tar: ^3.1.0 => 3.1.0 gulp-transform: ^3.0.5 => 3.0.5 gulp-util: ^3.0.8 => 3.0.8 handlebars: ^4.7.7 => 4.7.7 handlebars-template-loader: ^1.0.0 => 1.0.0 hbs: ^4.1.2 => 4.2.0 heapdump: ^0.3.15 => 0.3.15 highcharts: ^11.1.0 => 11.1.0 highcharts-react-official: ^3.2.0 => 3.2.0 http-proxy: ^1.18.1 => 1.18.1 http-terminator: ^3.0.0 => 3.0.0 https-browserify: ^1.0.0 => 1.0.0 i18n: ^0.8.5 => 0.8.5 i18n-for-browser: ^1.2.2 => 1.2.2 imports-loader: ^0.8.0 => 0.8.0 ipaddr.js: ^1.9.1 => 1.9.1 (1.9.0) is-running: ^2.1.0 => 2.1.0 istanbul: ^0.4.5 => 0.4.5 jquery: ^3.6.0 => 3.6.0 js-combinatorics: ^0.5.5 => 0.5.5 js-cookie: ^2.2.1 => 2.2.1 js-yaml: ^3.13.1 => 3.13.1 (4.0.0) jsdom: ^16.5.3 => 16.5.3 jsdom-global: ^3.0.2 => 3.0.2 json-loader: ^0.5.7 => 0.5.7 less: ^3.11.1 => 3.13.1 lodash: ^4.17.21 => 4.17.21 logrotate-stream: ^0.2.8 => 0.2.8 memo-parser: 0.2.1 merge-stream: ^2.0.0 => 2.0.0 minimist: ^1.2.6 => 1.2.6 mktemp: ^1.0.0 => 1.0.0 mobile-detect: ^1.4.4 => 1.4.4 mocha: ^8.4.0 => 8.4.0 mocha-jenkins-reporter: ^0.4.5 => 0.4.5 moment: ^2.29.1 => 2.29.3 moxios: ^0.4.0 => 0.4.0 net: ^1.0.2 => 1.0.2 node-cache: ^5.1.2 => 5.1.2 node-yaml: ^4.0.1 => 4.0.1 oo-eventtarget: 0.0.3 => 0.0.3 os-browserify: ^0.3.0 => 0.3.0 os-tmpdir: ^2.0.0 => 2.0.0 (1.0.2) parsleyjs: ^2.9.2 => 2.9.2 path-browserify: ^1.0.1 => 1.0.1 phantomjs-prebuilt: ^2.1.16 => 2.1.16 pikaday: ^1.8.0 => 1.8.0 polyfill-crypto.getrandomvalues: ^1.0.0 => 1.0.0 process: ^0.11.10 => 0.11.10 promised-handlebars: ^2.0.1 => 2.0.1 prop-types: ^15.7.2 => 15.8.1 ps-tree: ^1.2.0 => 1.2.0 purecss: ^2.0.6 => 2.0.6 qs: ^6.9.1 => 6.9.1 (6.7.0, 6.5.2) react: ^17.0.2 => 17.0.2 (18.2.0) react-dom: ^17.0.0 => 17.0.2 react-redux: ^7.0.3 => 7.0.3 recaptcha2: ^1.3.3 => 1.3.3 recluster: 0.4.5 => 0.4.5 redux: ^4.0.5 => 4.0.5 redux-actions: ^2.6.5 => 2.6.5 request: ^2.88.2 => 2.88.2 (2.88.0) request-promise: ^4.2.5 => 4.2.6 require-dir: ^1.2.0 => 1.2.0 rtlcss: ^2.5.0 => 2.6.2 scriptjs: ^2.5.9 => 2.5.9 seedrandom: ^3.0.5 => 3.0.5 selenium-standalone: ^6.17.0 => 6.17.0 selenium-webdriver: ^3.6.0 => 3.6.0 serialize-javascript: ^5.0.1 => 5.0.1 sinon: ^9.0.0 => 9.0.0 sinon-chai: ^3.5.0 => 3.5.0 source-map-loader: ^0.2.4 => 0.2.4 source-map-support: ^0.5.16 => 0.5.21 sprintf-js: ^1.1.2 => 1.1.2 (1.0.3) stacktrace-js: ^2.0.2 => 2.0.2 statsd-client: ^0.4.4 => 0.4.4 stream-browserify: ^3.0.0 => 3.0.0 stream-http: ^3.2.0 => 3.2.0 through2-concurrent: ^2.0.0 => 2.0.0 tmp: ^0.1.0 => 0.1.0 (0.0.33, 0.0.30) tty-browserify: ^0.0.1 => 0.0.1 url-search-params-polyfill: ^8.0.0 => 8.0.0 util-is: ^0.1.0 => 0.1.0 uuid: ^7.0.0 => 7.0.0 (3.4.0, 3.3.2, 7.0.3) v8flags: ^3.1.3 => 3.1.3 (3.2.0) velocity-animate: ^1.5.2 => 1.5.2 webpack: ^5.37.1 => 5.37.1 webpack-babel-env-deps: ^1.6.4 => 1.6.4 webpack-cli: ^4.7.0 => 4.7.0 webpack-dev-server: ^3.11.2 => 3.11.3 webpack-stream: ^7.0.0 => 7.0.0 yaml-loader: ^0.5.0 => 0.5.0 zen-observable: ^0.10.0 => 0.10.0 (0.8.15, 0.7.1) npmGlobalPackages: @aws-amplify/cli: 12.1.1 npm: 7.19.1 ```

Describe the bug

The javascript docs don't appear to indicate what to do within the client JS code to support owner-based authorization with an OIDC provider. The Android docs do. They show using the following code:

Amplify.addPlugin(AWSApiPlugin(ApiAuthProviders.builder()
    .oidcAuthProvider { "token-from-your-oidc-provider" }
    .build()))

I need to understand how to do the same thing in JS. From looking through the API documentation (https://aws-amplify.github.io/amplify-js/api/globals.html#authproviders ), I have tried to understand how to set it up but it isn't working.

Here is my current code:

    DataStore.configure({
      syncExpressions: [
        syncExpression(DataStoreModelType, () => {
          return (user) => {
            user.userId('eq', this.userId);
          };
        }),
      ],
      authProviders: {
        functionAuthProvider: () => {
          return {
            token: this.token,
          };
        },
      },
    }

When I run this, I get the following error:

AuthError - 
            Error: Amplify has not been configured correctly. 
            The configuration object is missing required auth properties.

It suggests I need to run amplify add auth and then amplify push, but I don't believe it make sense to do this in my case because this same DataStore is already being used in prod by our Android and iOS applications. Is that correct?

As a workaround, we have been using the cache instead, but I'm concerned that this is not the intended approach and could lead to undesired behavior and potential bugs. Can you advise whether that is a concern I should have? Here is that code:

Cache.setItem('federatedInfo', {token: this.token});

Thank you

Expected behavior

I expected that adding the code as described would not have an error and would result in the browser's data being populated with the appropriate user's data.

Reproduction steps

  1. Setup DataStore from within a JS web application
  2. Configure DataStore to expect custom OIDC authorization with a JWT token
  3. Try to access data from the web application that is authorized on a per user basis as described above.

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 1 year ago

Hi @jared-ookla 👋 thanks for raising this issue.

Please correct me if I'm wrong, but it sounds like you already have an Amplify backend that you are using for an Android and iOS app. You are just trying to use DataStore with an OIDC token to authorize qraphql queries, correct?

Can you provide us with some more details about your configuration? Since you are not configuring DataStore to use multi auth, can you confirm what the default authorization mode on your API resource is? DataStore will try to use that by default. There is no other way to tell DataStore which authMode to try, unlike the API.graphql method which accepts the authMode as an option.

Lastly, do you have an aws-exports file? If not, please let us know how you might be calling Amplify.configure.

jared-ookla commented 1 year ago

Oh, I see, that's helpful to understand, thanks!

Our default authorization is OIDC but this is only configured on the api module (appsync). We aren’t using the auth module (i.e. we already have logic in place that captures this bearer token without the use of the auth module).

Here are the contents of our aws-exports.js file:

/* eslint-disable */
// WARNING: DO NOT EDIT. This file is automatically generated by AWS Amplify. It will be overwritten.

const awsmobile = {
    "aws_project_region": "us-west-2",
    "aws_appsync_graphqlEndpoint": "<our-custom-oidc-provider-endpoint>",
    "aws_appsync_region": "us-west-2",
    "aws_appsync_authenticationType": "OPENID_CONNECT"
};

export default awsmobile;
jared-ookla commented 1 year ago

@chrisbonifacio The client will need some way of communicating which user is currently signed in. I would assume the user.userId('eq', this.userId) sync expression is not sufficient since it isn't explicitly about authorization. This is what I would have thought adding my customer authProvider function would offer. But while it isn't working, Cache.setItem is.

Is that the appropriate way for the web client to indicate the current user or is there a better way?

jared-ookla commented 1 year ago

For context, this is the code that led us to the Cache.setItem solution.

I was hoping that I could search the codebase for Cache.setItem to find some wrapper function that does this setItem action that we should be using instead, but the search comes up empty.

Alternatively, I would have hoped that there was support for Authorization info passed through the headers (as is true with the AWS_LAMBDA case), but that doesn't appear to be the case. If that was added, I wonder if our authProvider would work.

jared-ookla commented 1 year ago

@chrisbonifacio Can you offer any insight? Is there any additional context that'd be helpful for me to provide?

chrisbonifacio commented 1 year ago

Hi @jared-ookla. Sorry for the delay. Thanks for referencing the code. It appears that the library tries to get the token from either Auth or Cache. So, in your case, I think it's alright to use Cache.setItem for now. This is simply an abstraction of localStorage on the browser.

If you do run into issues, the alternative would be to use Auth.federatedSignIn like the documentation suggests here:

https://docs.amplify.aws/lib/auth/advanced/q/platform/js/#identity-pool-federation

This would be using the token you receive from your OIDC provider and exchanging it for Cognito credentials and Amplify will manage the refresh.

jared-ookla commented 1 year ago

Thank you for your response.

It sounds like the Cache solution is not ill-advised but also doesn't come with much confidence that it will work seamlessly. We are looking into this alternate solution you suggested to understand whether we can easily add identity pools to our current DataStore setup, so that we can implement this in a way that will give us higher confidence of no surprise issues when we launch on web. If it's ok, we'll update here with what we decide to do and any question that arise.

cwomack commented 1 year ago

@jared-ookla, definitely report back on whether the alternative solution works for you!

jared-ookla commented 1 year ago

@cwomack and @chrisbonifacio, I just want to confirm, the reason you are suggesting that the alternative to our Cache.setItem workaround is to use Auth Identity Pool federation is because there is no way for us to specify a custom oidc auth provider on the DataStore configuration the way we are able to do with Android and iOS, is that correct?

Is that an intentional gap in support or something you plan to add?

Thanks!

chrisbonifacio commented 1 year ago

Hi @jared-ookla , unfortunately that authProviders configuration option you found only supports Lambda

This seems to be a feature disparity between the JS and Android implementations of DataStore. I'm going to mark this issue as a feature request. The way you are currently setting the oidc token in the Cache is fine for now. It is the only way to provide Amplify with it outside of using the Auth library so it should be okay to continue doing so until we add OIDC support to the authProviders config option.

jared-ookla commented 1 year ago

Thank you for providing that clarification. I'm glad to see this go in as a feature ticket!