aws-amplify / amplify-flutter

A declarative library with an easy-to-use interface for building Flutter applications on AWS.
https://docs.amplify.aws
Apache License 2.0
1.3k stars 238 forks source link

User keeps receiving Pinpoint push notifications after `Amplify.Auth.signOut()` #3179

Open dJani97 opened 1 year ago

dJani97 commented 1 year ago

Description

We use Pinpoint to send push notifications, and to help with that we use the Amplify.Notifications.Push.identifyUser() method.

But it seems like after a user logs out, the endpoint associated with their current device does not get deleted and they keep receiving push notifications. This is not desired and could be a security concern.

I looked at the docs but couldn't find any information on how and when a pinpoint endpoint gets unregistered. Could you help me with that?

Expected behaviour

Additional details

Here is how the client identifies the user with Pinpoint:

Future<void> registerUserWithPinpoint(Profile profile) async {
  final user = await Amplify.Auth.getCurrentUser();
  await Amplify.Notifications.Push.identifyUser(
    userId: user.userId,
    userProfile: AWSPinpointUserProfile(email: profile.email, name: profile.name),
  );
}

And here is how the server targets the client:

async function sendNotification(payload) {
  const sendNotificationInput = {
    ApplicationId: process.env.ANALYTICS_APP_NOTIFICATIONS_ID,
    SendUsersMessageRequest: {
      MessageConfiguration: {
        GCMMessage: {
          Title: payload.title,
          Body: payload.message,
          Data: { data: JSON.stringify(payload) },
          Priority: 'high',
        },
        APNSMessage: {
          Title: payload.title,
          Body: payload.message,
          Data: { data: JSON.stringify(payload) },
          Priority: '10',
        },
      },
      Users: {
        [payload.userId]: {},
      },
    },
  }
  return pinpointClient.send(
    new SendUsersMessagesCommand(sendNotificationInput),
  )
}

Categories

Steps to Reproduce

Screenshots

No response

Platforms

Flutter Version

3.10.4

Amplify Flutter Version

1.1.1

Deployment Method

Amplify CLI

Schema

No response

dJani97 commented 12 months ago

@dnys1 is there any update on this? Can I assist you with more information?

abdallahshaban557 commented 12 months ago

Hi @dJani97 - It is recommended that after you can signOut, to then call the identifyUser API, and then pass the userID as an empty string. That will disassociate the endpoint from that user ID. We have not built in the wiring so that the signOut automatically handles dissociating the endpoint for your signed in user, we left that responsibility to the developer building out the features for their app.

dJani97 commented 11 months ago

@abdallahshaban557 thank you for the info! I'm going to try this now:

await Amplify.Notifications.Push.identifyUser(
  userId: '',
  userProfile: AWSPinpointUserProfile(),
);

As a feedback, this is really obscure for me, and it's not documented. Since both userId and userProfile are required, I would have never thought that passing empty instances will result in the user being unregistered. Please update the docs, or add an unregisterUser() call that wraps this call, to make it more obvious for developers.

I would go as far as to suggest that whenever a user logs out, this unsubscription should be handled automatically, by default. Not doing so is a security risk that developers won't expect. Alternatively, you can warn developers about this in the docs.

abdallahshaban557 commented 11 months ago

Hi @dJani97 - you are right, we can definitely make that experience better. We will at least cover this in our documentation as a bare minimum, and investigate adding the wrapper API you mentioned. Thank you so much for the feedback!

abdallahshaban557 commented 10 months ago

Hi @dJani97 - I have just received new information that the best way to make sure you de-associate a user with an endpoint is to pass null to the user_id, rather than passing an empty string. Just wanted to make sure I send this your way.

kouz75 commented 9 months ago

Hi @dJani97 - I have just received new information that the best way to make sure you de-associate a user with an endpoint is to pass null to the user_id, rather than passing an empty string. Just wanted to make sure I send this your way.

hi, the user_id is a string and can't be null. setting an empty string works.

dJani97 commented 9 months ago

Yes, it cannot be null. This is my current solution to this problem, and it is working:

await Amplify.Notifications.Push.identifyUser(
  userId: '',
  userProfile: AWSPinpointUserProfile(userAttributes: {}),
);

Is is a lot of boilerplate though, and it's very obscure. Notice how I must specify userAttributes: {} or else it will produce a runtime error.

abdallahshaban557 commented 9 months ago

We are currently working on improving the API to enable this behavior. We will provide an update here when we make more progress.

@dJani97 - just to confirm, setting the userId and userProfile as you did stopped the user from receiving new Notifications?

dJani97 commented 9 months ago

@abdallahshaban557 yes.

fjnoyp commented 9 months ago

Hey @dJani97 thanks for your insights here. Please note that if you save an attribute you must set its value to '' if you send notifications to a segment created with that attribute.

Ex:

identifyUser( userAttributes: { 'key' : 'value' } )

Pinpoint notifications to Segment with key : value will continue to send even if you called: identifyUser( userAttributes: {})

Instead you must call: identifyUser(userAttributes: {'key': ''})

That won't actually delete 'key' though.

We are aware of this behavior and have a separate issue for it here: https://github.com/aws-amplify/amplify-flutter/issues/3721

victoravr commented 2 months ago

How can I unsubscribe user from APNs push notifications from pinpoint upon sign-out with Amplify for react-native v6?

Jordan-Nelson commented 1 month ago

@victoravr since this question appears to be about react native, can you open an issue here: https://github.com/aws-amplify/amplify-js? Thank you

loe-lobo commented 3 weeks ago

Hi @fjnoyp

Is there a better solution than clearing all the properties? Maybe updating the endpoint with OptOut = ALL?

Thanks

NikaHsn commented 2 weeks ago

thanks @loe-lobo. there is an open feature request #3721 to improve this behaviour.