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

Amplify PubSub keeps previous user's authorization policy #9882

Open PanGian97 opened 2 years ago

PanGian97 commented 2 years ago

Before opening, please confirm:

JavaScript Framework

React

Amplify APIs

PubSub

Amplify Categories

auth

Environment information

``` # Put output below this line System: OS: Windows 10 10.0.19044 CPU: (4) x64 Intel(R) Core(TM) i3-4160 CPU @ 3.60GHz Memory: 3.35 GB / 7.94 GB Binaries: Node: 16.9.1 - C:\Program Files\nodejs\node.EXE Yarn: 1.22.11 - ~\AppData\Roaming\npm\yarn.CMD npm: 7.21.1 - C:\Program Files\nodejs\npm.CMD Browsers: Chrome: 101.0.4951.54 Edge: Spartan (44.19041.1266.0), Chromium (101.0.1210.39) Internet Explorer: 11.0.19041.1566 npmPackages: @aws-amplify/pubsub: ^4.2.9 => 4.2.9 @aws-amplify/ui-react: ^2.7.0 => 2.7.0 @aws-amplify/ui-react-internal: undefined () @aws-amplify/ui-react-legacy: undefined () @emotion/react: ^11.9.0 => 11.9.0 @emotion/styled: ^11.8.1 => 11.8.1 @material-ui/icons: ^4.11.3 => 4.11.3 @mui/icons-material: ^5.6.2 => 5.6.2 @mui/material: ^5.6.4 => 5.6.4 @testing-library/jest-dom: ^5.16.2 => 5.16.2 @testing-library/react: ^12.1.3 => 12.1.3 @testing-library/user-event: ^13.5.0 => 13.5.0 amazon-cognito-identity-js: ^5.2.7 => 5.2.7 assert: ^2.0.0 => 2.0.0 aws-amplify: ^4.3.15 => 4.3.15 aws-sdk: ^2.1089.0 => 2.1089.0 axios: ^0.27.0 => 0.27.0 (0.21.4) bootstrap: ^5.1.3 => 5.1.3 buffer: ^6.0.3 => 6.0.3 (4.9.2) crypto-browserify: ^3.12.0 => 3.12.0 https-browserify: ^1.0.0 => 1.0.0 immer: ^9.0.12 => 9.0.12 (9.0.6) os-browserify: ^0.3.0 => 0.3.0 process: ^0.11.10 => 0.11.10 react: ^17.0.2 => 17.0.2 react-apexcharts: ^1.4.0 => 1.4.0 react-bootstrap: ^2.3.1 => 2.3.1 react-bootstrap/AbstractModalHeader: undefined () react-bootstrap/Accordion: undefined () react-bootstrap/AccordionBody: undefined () react-bootstrap/AccordionButton: undefined () react-bootstrap/AccordionCollapse: undefined () react-bootstrap/AccordionContext: undefined () react-bootstrap/AccordionHeader: undefined () react-bootstrap/AccordionItem: undefined () react-bootstrap/AccordionItemContext: undefined () react-bootstrap/Alert: undefined () react-bootstrap/Anchor: undefined () react-bootstrap/Badge: undefined () react-bootstrap/BootstrapModalManager: undefined () react-bootstrap/Breadcrumb: undefined () react-bootstrap/BreadcrumbItem: undefined () react-bootstrap/Button: undefined () react-bootstrap/ButtonGroup: undefined () react-bootstrap/ButtonToolbar: undefined () react-bootstrap/Card: undefined () react-bootstrap/CardGroup: undefined () react-bootstrap/CardHeader: undefined () react-bootstrap/CardHeaderContext: undefined () react-bootstrap/CardImg: undefined () react-bootstrap/Carousel: undefined () react-bootstrap/CarouselCaption: undefined () react-bootstrap/CarouselItem: undefined () react-bootstrap/CloseButton: undefined () react-bootstrap/Col: undefined () react-bootstrap/Collapse: undefined () react-bootstrap/Container: undefined () react-bootstrap/Dropdown: undefined () react-bootstrap/DropdownButton: undefined () react-bootstrap/DropdownContext: undefined () react-bootstrap/DropdownItem: undefined () react-bootstrap/DropdownMenu: undefined () react-bootstrap/DropdownToggle: undefined () react-bootstrap/ElementChildren: undefined () react-bootstrap/Fade: undefined () react-bootstrap/Feedback: undefined () react-bootstrap/Figure: undefined () react-bootstrap/FigureCaption: undefined () react-bootstrap/FigureImage: undefined () react-bootstrap/FloatingLabel: undefined () react-bootstrap/Form: undefined () react-bootstrap/FormCheck: undefined () react-bootstrap/FormCheckInput: undefined () react-bootstrap/FormCheckLabel: undefined () react-bootstrap/FormContext: undefined () react-bootstrap/FormControl: undefined () react-bootstrap/FormFloating: undefined () react-bootstrap/FormGroup: undefined () react-bootstrap/FormLabel: undefined () react-bootstrap/FormRange: undefined () react-bootstrap/FormSelect: undefined () react-bootstrap/FormText: undefined () react-bootstrap/Image: undefined () react-bootstrap/InputGroup: undefined () react-bootstrap/InputGroupContext: undefined () react-bootstrap/ListGroup: undefined () react-bootstrap/ListGroupItem: undefined () react-bootstrap/Modal: undefined () react-bootstrap/ModalBody: undefined () react-bootstrap/ModalContext: undefined () react-bootstrap/ModalDialog: undefined () react-bootstrap/ModalFooter: undefined () react-bootstrap/ModalHeader: undefined () react-bootstrap/ModalTitle: undefined () react-bootstrap/Nav: undefined () react-bootstrap/NavContext: undefined () react-bootstrap/NavDropdown: undefined () react-bootstrap/NavItem: undefined () react-bootstrap/NavLink: undefined () react-bootstrap/Navbar: undefined () react-bootstrap/NavbarBrand: undefined () react-bootstrap/NavbarCollapse: undefined () react-bootstrap/NavbarContext: undefined () react-bootstrap/NavbarOffcanvas: undefined () react-bootstrap/NavbarToggle: undefined () react-bootstrap/Offcanvas: undefined () react-bootstrap/OffcanvasBody: undefined () react-bootstrap/OffcanvasHeader: undefined () react-bootstrap/OffcanvasTitle: undefined () react-bootstrap/OffcanvasToggling: undefined () react-bootstrap/Overlay: undefined () react-bootstrap/OverlayTrigger: undefined () react-bootstrap/PageItem: undefined () react-bootstrap/Pagination: undefined () react-bootstrap/Placeholder: undefined () react-bootstrap/PlaceholderButton: undefined () react-bootstrap/Popover: undefined () react-bootstrap/PopoverBody: undefined () react-bootstrap/PopoverHeader: undefined () react-bootstrap/ProgressBar: undefined () react-bootstrap/Ratio: undefined () react-bootstrap/Row: undefined () react-bootstrap/SSRProvider: undefined () react-bootstrap/Spinner: undefined () react-bootstrap/SplitButton: undefined () react-bootstrap/Stack: undefined () react-bootstrap/Switch: undefined () react-bootstrap/Tab: undefined () react-bootstrap/TabContainer: undefined () react-bootstrap/TabContent: undefined () react-bootstrap/TabPane: undefined () react-bootstrap/Table: undefined () react-bootstrap/Tabs: undefined () react-bootstrap/ThemeProvider: undefined () react-bootstrap/Toast: undefined () react-bootstrap/ToastBody: undefined () react-bootstrap/ToastContainer: undefined () react-bootstrap/ToastContext: undefined () react-bootstrap/ToastFade: undefined () react-bootstrap/ToastHeader: undefined () react-bootstrap/ToggleButton: undefined () react-bootstrap/ToggleButtonGroup: undefined () react-bootstrap/Tooltip: undefined () react-bootstrap/TransitionWrapper: undefined () react-bootstrap/createChainedFunction: undefined () react-bootstrap/createUtilityClasses: undefined () react-bootstrap/createWithBsPrefix: undefined () react-bootstrap/divWithClassName: undefined () react-bootstrap/getTabTransitionComponent: undefined () react-bootstrap/helpers: undefined () react-bootstrap/safeFindDOMNode: undefined () react-bootstrap/transitionEndListener: undefined () react-bootstrap/triggerBrowserReflow: undefined () react-bootstrap/types: undefined () react-bootstrap/useOverlayOffset: undefined () react-bootstrap/usePlaceholder: undefined () react-bootstrap/useWrappedRefWithWarning: undefined () react-dom: ^17.0.2 => 17.0.2 react-redux: ^8.0.1 => 8.0.1 react-router-bootstrap: ^0.25.0 => 0.25.0 react-router-dom: ^5.3.1 => 5.3.1 react-scripts: 5.0.0 => 5.0.0 redux: ^4.2.0 => 4.2.0 redux-devtools-extension: ^2.13.9 => 2.13.9 redux-persist: ^6.0.0 => 6.0.0 redux-persist/integration/react: undefined () redux-thunk: ^2.4.1 => 2.4.1 sass: ^1.51.0 => 1.51.0 stream-browserify: ^3.0.0 => 3.0.0 stream-http: ^3.2.0 => 3.2.0 url: ^0.11.0 => 0.11.0 (0.10.3) util: ^0.12.4 => 0.12.4 web-vitals: ^2.1.4 => 2.1.4 npmGlobalPackages: @aws-amplify/cli: 7.6.22 amplify-app: 3.0.16 corepack: 0.9.0 npm: 7.21.1 serverless: 2.70.0 ```

Describe the bug

I'm using PubSub to subscribe to mqtt topics. I will describe you a scenario that it's behavior is faulty.

Let's say we have a react web app with amplify authenticator to handle sign-in/out. We have 2 users (userA userB)

1st faulty scenario: userA sign-in -> attempt to subscribe(let's say pressing a sub button) -> subscribes successfully to aws iot -> decides to sign-out -> userB sing-in from the same machine ->attempt to subscribe(let's say pressing a sub button) -> subscribes succesfully even by NOT having the authority for this action (because previously signed user had the authority) !

userB stops receiving messages or isn't able to subscribe successfully ONLY if he reloads the page after sign-in.

The above can be fixed by automatically calling unsubscribe method on user sign-out (but it isn't a good and safe practice)

2nd faulty scenario (and even worse than the above): userB sign-in -> attempt to subscribe(let's say pressing a sub button) -> cannot subscribe successfully because he doesn't has the authority based on his cognito identity id -> decides to sign-out -> userA sing-in from the same machine ->attempt to subscribe(let's say pressing a sub button) -> cannot subscribe successfully even by HAVING the authority for this action based on his cognito identity id (because previously signed user had NOT the authority) !

userA is able to subscribe successfully ONLY if he reloads the page after sign-in.

Please reply me cause i rely all my web application on Amplify, and as you can see a behavior like this isn't acceptable

Expected behavior

Accoring to the bug's description scenarios...

1st scenario (should work as):>userA sign-in -> attempt to subscribe(let's say pressing a sub button) -> subscribes successfully to aws iot -> decides to sign-out -> userB sing-in from the same machine ->attempt to subscribe(let's say pressing a sub button) -> cannot subscribe succesfuly because he has NOT the authority for this action according to his cognito identity id. (without the need for a page reload)

2nd scenario (should work as):>userB sign-in -> attempt to subscribe(let's say pressing a sub button) -> cannot subscribe successfully because he doesn't has the authority based on his cognito identity id -> decides to sign-out -> userA sing-in from the same machine ->attempt to subscribe(let's say pressing a sub button) -> subscribes successfully because he has the authority for this action based on his cognito identity id . (without the need for a page reload )

Reproduction steps

I explained on details on bug description above by mentioning 2 complete scenarios (use cases)

Code Snippet

// Put your code below this line.

Log output

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

aws-exports.js

const awsmobile = {

    "aws_project_region": "us-east-2",
    "aws_cognito_identity_pool_id": "us-east-2:4f3b168f-384d-4477-97fb-a61a0f6c6c7c",
    "aws_cognito_region": "us-east-2",
    "aws_user_pools_id": "us-east-2_gXvYmTlCU",
    "aws_user_pools_web_client_id": "4jvprq4u11cv1kdafvcoqm6720",
    "oauth": {},
    "aws_cognito_username_attributes": [
        "EMAIL"
    ],
    "aws_cognito_social_providers": [],
    "aws_cognito_signup_attributes": [
        "EMAIL"
    ],
    "aws_cognito_mfa_configuration": "OFF",
    "aws_cognito_mfa_types": [
        "SMS"
    ],
    "aws_cognito_password_protection_settings": {
        "passwordPolicyMinLength": 8,
        "passwordPolicyCharacters": []
    },
    "aws_cognito_verification_mechanisms": [
        "EMAIL"
    ]
};

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

stocaaro commented 1 year ago

Hello @PanGian97,

What your describing sounds like the behavior I would expect if your application doesn't clean up subscriptions when one user logs out and another logs in. Is this the case?

These subscriptions are managed by an open WebSocket, where auth is managed when the websocket is opened. When the last subscription is unsubscribed, then the socket is closed. If any subscription is open while user A logs out and user B logs in, then when user B opens their subscriptions it will continue with the connection authenticated for user A.

I've seen two approaches to ensuring this happens.

  1. Component cleanup:

    function MyComponent () {
    useEffect(() => {
    const sub1 = PubSub.subscribe('myTopicA').subscribe({
        next: data => console.log('Message received', data),
        error: error => console.error(error),
        complete: () => console.log('Done'),
    });
    
    return () => sub1.unsubscribe();
    }
    }

When the user logs out, all session components are unloaded, calling the cleanup functions on these useEffects to prompt subscriptions to close and the connection to close.

  1. Keeping all open subscriptions in a list (maybe in a useState([]) in React) so that the Auth close behavior can loop through and unsubscribe when the user is logging out, which will close the websocket connection.

For this purpose, it might be useful to offer something like PubSub.closeAllConnections() that can be called on logout to ensure the websockets don't outlive the user session.

Even if one of the above recommendations works for your use-case, I would be inclined to leave this issue open as a feature request to offer something like this close all behavior.

Hope this helps. I appreciate the detail you put into this issue.

Thanks, Aaron