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

Cognito users can see each others creadit card information via subscriptions #7065

Closed pr1ze closed 3 years ago

pr1ze commented 3 years ago

Describe the bug Users are able too see others data, when they have an active subscription and someone else created a entity via AppSync.

To Reproduce

Graphql:

type CreditCard
@model
@auth(rules: [
    { allow: owner },
    { allow: groups, groups: ["administrators"] },
    { allow: private, provider: iam }
])
@key(name: "byUser", fields: ["userID"]) {
    id: ID!
    creditCardNumer: String!
    expiryData: String!
    cVC: String!
    userID: String!
    user: User @connection(fields: ["userID"])
    owner: String!
}

Having two different cognito users logged in on each of their device (Seen on iphone), and do:

        DataStore.query(CreditCard).then((result) => {
            //Some somewhere
        });

        // Subscribe!
        return DataStore.observe(CreditCard).subscribe(() => {
            DataStore.query(type).then((result) => {
                //Some somewhere -> here the other users credit card information comes in.,
            });
        })

When one user do DataStore.save({the credit card}) it pops into the DataStore of the other user (With a different username / sub / cognito id).

Closing the app, and do a fresh start removes the entry. So it does not seems like the "sync" has the error, but the subscription does.

Expected behavior The other users should not get the sensible data as the @auth mode is owner.

Code Snippet I do not know if it's related to this: https://github.com/aws-amplify/amplify-js/issues/6990 But i think the subscription is setup with IAM

The generated subscriptions does however have the owner input field:

export const onCreateCreditCard = /* GraphQL */ `
  subscription onCreateCreditCard ($owner: String) {
    onCreateCreditCard (owner: $owner) {
      id
....

What is Configured?

React native app - package.json:

{
    "main": "node_modules/expo/AppEntry.js",
    "scripts": {
        "start": "expo start",
        "android": "expo start --android",
        "ios": "expo start --ios",
        "web": "expo start --web",
        "eject": "expo eject",
        "amplify-modelgen": "node amplify\\scripts\\amplify-modelgen.js",
        "amplify-push": "node amplify\\scripts\\amplify-push.js"
    },
    "dependencies": {
        "@aws-amplify/api": "^3.2.7",
        "@aws-amplify/datastore": "^2.6.1",
        "@aws-amplify/pubsub": "^3.2.5",
        "@react-native-community/masked-view": "0.1.10",
        "@react-native-community/netinfo": "5.9.6",
        "@react-native-community/slider": "3.0.3",
        "@react-navigation/drawer": "^5.5.1",
        "@react-navigation/material-bottom-tabs": "^5.2.10",
        "@react-navigation/native": "^5.5.1",
        "@react-navigation/stack": "^5.5.1",
        "@unimodules/core": "~5.5.0",
        "aws-amplify": "^3.3.4",
        "aws-amplify-react-native": "^4.2.7",
        "expo": "^39.0.0",
        "expo-av": "~8.6.0",
        "expo-camera": "~9.0.0",
        "expo-constants": "~9.2.0",
        "expo-device": "~2.3.0",
        "expo-font": "~8.3.0",
        "expo-image-manipulator": "~8.3.0",
        "expo-keep-awake": "~8.3.0",
        "expo-linking": "^1.0.3",
        "expo-notifications": "~0.7.2",
        "expo-permissions": "~9.3.0",
        "expo-screen-orientation": "~2.0.0",
        "expo-splash-screen": "~0.6.1",
        "expo-updates": "~0.3.3",
        "expo-video-player": "^1.5.8",
        "expo-video-thumbnails": "~4.3.0",
        "expo-web-browser": "~8.5.0",
        "inquirer": "^7.2.0",
        "react": "16.13.1",
        "react-dom": "16.13.1",
        "react-native": "https://github.com/expo/react-native/archive/sdk-39.0.3.tar.gz",
        "react-native-aws3": "^0.0.9",
        "react-native-fs": "^2.16.6",
        "react-native-gesture-handler": "~1.7.0",
        "react-native-keyboard-aware-scroll-view": "^0.9.1",
        "react-native-paper": "~3.10.1",
        "react-native-reanimated": "~1.13.0",
        "react-native-safe-area-context": "3.1.4",
        "react-native-screens": "~2.10.1",
        "react-native-search-header": "^0.3.5",
        "react-native-svg": "12.1.0",
        "react-native-svg-transformer": "^0.14.3",
        "react-native-tab-view": "^2.14.4",
        "react-native-uuid": "^1.4.9",
        "react-native-vector-icons": "^6.6.0",
        "react-native-web": "~0.13.7",
        "react-navigation": "^4.3.9",
        "react-navigation-material-bottom-tabs": "^2.2.12"
    },
    "devDependencies": {
        "@babel/core": "^7.8.6",
        "@types/react": "~16.9.35",
        "@types/react-native": "~0.63.2",
        "babel-preset-expo": "^8.3.0",
        "ini": "^1.3.5",
        "typescript": "~3.9.2"
    },
    "private": true
}

aws-exports.js

const awsmobile = {
    "aws_project_region": "eu-central-1",
    "aws_cognito_identity_pool_id": "eu-central-1:b6caf1e1-a91e-4099-9249-ea99a9d95132",
    "aws_cognito_region": "eu-central-1",
    "aws_user_pools_id": "eu-central-1_TpCHLAtpD",
    "aws_user_pools_web_client_id": "7uim50j5pgha43fvjekd34fv48",
    "oauth": {},
    "aws_cloud_logic_custom": [
        {
            "name": "AdminQueries",
            "endpoint": "https://nrng6ss6sl.execute-api.eu-central-1.amazonaws.com/test",
            "region": "eu-central-1"
        },
        {
            "name": "LandingLinkAPI",
            "endpoint": "https://y9o25l4f42.execute-api.eu-central-1.amazonaws.com/test",
            "region": "eu-central-1"
        }
    ],
    "aws_appsync_graphqlEndpoint": "https://ubgjraf4zbcbxndz3pcqc3xvda.appsync-api.eu-central-1.amazonaws.com/graphql",
    "aws_appsync_region": "eu-central-1",
    "aws_appsync_authenticationType": "AMAZON_COGNITO_USER_POOLS",
    "aws_user_files_s3_bucket": "diialoggstatic211857-test",
    "aws_user_files_s3_bucket_region": "eu-central-1"
};

How amplify is configured in App.tsx

Amplify.configure({
    ...config,
    Analytics: {
        disabled: true
    }
})

Smartphone (please complete the following information):

amhinson commented 3 years ago

@pr1ze You are correct that the subscription does seem to be created with IAM instead of User Pools as the authorization type. However, DataStore doesn't currently support using multiple auth type on a model, so the use case you have here won't work as you are expecting. If you remove the { allow: private, provider: iam } rule, it should work. We are working to improve this soon, but that should be the resolution for now.

pr1ze commented 3 years ago

@amhinson Does the Documentation state that the DataStore does not support that? Cause reading about the auth directive, one really gets the impression that this is an acceptable model.

I cant remove IAM - then i cannot call the API from my lambda functions.

Can you point me to where it's documented, that the DataStore only supports a single auth type? But if it is not documented, then this is not just something that needs improvement. Then it's a critical issue. It concerns me how many production systems that might be running now, where people are capable of seeing each others sensible data.

iartemiev commented 3 years ago

Hey @pr1ze, we're currently in the process of improving our docs around this use case.

In the meantime, please try the following:

This will prevent DataStore from attempting to authenticate the subscriptions with IAM in your app, but you will still be able to authenticate with IAM from your Lambda function.

Please note that you will need to make this manual change each time after running amplify codegen models.

You can also apply more fine-grained control over which operations are permitted for each of the auth rules you have configured. For example:

@auth(rules: [
    { allow: owner, operations: [create, read, update, delete] },
    { allow: groups, groups: ["administrators"], operations: [update, delete] },
    { allow: private, provider: iam, operations: [create, update, delete] }
])

This will prevent the iam users from subscribing to any mutations on this model.

stale[bot] commented 3 years ago

This issue has been automatically closed because of inactivity. Please open a new issue if are still encountering problems.

github-actions[bot] commented 2 years ago

This issue has been automatically locked since there hasn't been any recent activity after it was closed. Please open a new issue for related bugs.

Looking for a help forum? We recommend joining the Amplify Community Discord server *-help channels or Discussions for those types of questions.