Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Shouldn't PinPoint send Analytics with UserID as cognito:sub instead of IdentityID? #1779

Closed
mihaiblaga89 opened this issue Sep 28, 2018 · 21 comments
Assignees
Labels
Analytics Related to analytics Cognito Related to cognito issues

Comments

@mihaiblaga89
Copy link

mihaiblaga89 commented Sep 28, 2018

** Which Category is your question related to? **
Amplify PinPoint

** What AWS Services are you utilizing? **
Amplify, Cognito, PinPoint

** Provide additional details e.g. code snippets **
screenshot 2018-09-28 at 15 10 50

The issue goes like this:
I can't, for some reason, get phone numbers and emails from Cognito to PinPoint. Userpool analytics is on, sharing the data with Pinpoint, role is ok, etc, but nothing shows up in PinPoint.

Anyway, I'm now trying to do it manually (sms, email, push) by specifying the UserId as cognito:sub to match them. However, giving the fact that React Native Amplify Analytics remembers the EndpointID used, it triggers an automated updateEndpoint with the UserID as IdentityID and that basically overwrites my PushNotification manually sent update and giving the fact that I don't have a way of getting IdentityID myself on Cognito login or in the lambdas I can't switch to that one.

Why is it sending the IdentityID as UserID? Even in the documentation it says that UserID field should have the cognito:sub value.

@jordanranz jordanranz added Analytics Related to analytics Cognito Related to cognito issues labels Sep 28, 2018
@powerful23
Copy link
Contributor

@mihaiblaga89 Amplify has this automatic tracking feature which will update the endpoint automatically and for now the user Id by default would be the identity id from the aws credentials. We will talk to Pinpoint and Cognito team about the correct way to set the default user Id.

As for now you can try a work around which is to turn off that autotracking feature by setting the autoSessionRecord to false: https://aws-amplify.github.io/amplify-js/media/analytics_guide#manual-setup and then update your endpoint manually: https://aws-amplify.github.io/amplify-js/media/analytics_guide#update-user-attributes

@mihaiblaga89
Copy link
Author

mihaiblaga89 commented Sep 28, 2018

Apparently according to the docs Cognito should add the phone and email as endpoints in Pinpoint automatically but that does not happen.

Can't add a photo from mobile so I'll post a quote and a link.
"Tip
If your application uses Amazon Cognito user pools to handle user authentication, Amazon Cognito can add user IDs and attributes to your endpoints automatically. For the endpoint user ID value, Amazon Cognito assigns the sub value that's assigned to the user in the user pool. To add users with Amazon " https://docs.aws.amazon.com/pinpoint/latest/developerguide/audience-define-user.html

I'd rather not disable autoSession, I'll make a fork and call Auth.currentAuthenticatedUser() method in PinpointProvider._updateEndpoint() and use the user's sub value as Userid and if no user is logged in I'll default to your currently implemented solution while I wait for your fix.

P.S. On mobile, might not remember the method names correctly.

P.P.S. I've also discovered that on some calls the Demographics properties set manually in config like appVersion, etc are not taken into account. And if I remember correctly it happens only in the autoSession updates.

@ajhool
Copy link

ajhool commented Sep 30, 2018

I just ran into the same problem with the Storage module. It definitely doesn't seem correct to create user-level S3 folders per user-session. The folders should be created per user rather than per user-session; each user should only have one root folder for storage. Storage and Pinpoint are different, but the distinction between sub and identityId is the core issue.

#1787

If Amplify somehow manages identity tracking under the hood, I would point out that this makes manual backend configuration really challenging. I'd like to use Amplify as the client but use manually created backend services like S3 and Pinpoint. If the client is too complicated then that complicates backend development.

@mihaiblaga89
Copy link
Author

mihaiblaga89 commented Sep 30, 2018

@ajhool I've managed to fix my issue but I won't be making a PR because it's pretty hack-ish. I basically called Auth.currentAuthenticatedUser() and get my sub value from there, if not fallback to their original implementation. But giving the fact that Analytics package now depends on Auth, I had to make lerna build that first otherwise it would not build the project.

Waiting for a more elegant and clean way of doing it.

As for identityId, what I've debugged so far is that our issue is somewhere in Auth or Core/Credentials. Credentials is linked to amazon-cognito-identity-js and from what i've understood Amplify first gets some unauth credentials from Identity Pool in order to push to Pinpoint (in my case) and sets them with Credentials.set() in storage for later use and it updates them as soon as a user is logged in. But that only gets the Identity Pool identity, not the user from the User Pool. I just started working with Cognito and Amplify and for sure I don't have the "bigger picture" of how this all works so I stand to be corrected.

@powerful23 powerful23 added the investigating This issue is being investigated label Oct 1, 2018
@mihaiblaga89
Copy link
Author

@powerful23 any updates on this?

@mlabieniec
Copy link
Contributor

We have a story in our backlog to address this awaiting review

@ajhool
Copy link

ajhool commented Oct 14, 2018

@mlabieniec would you please consider attaching #1787 to that story? It's a similar conceptual issue but refers to Storage/S3 instead of Pinpoint. Storage also uses identityId rather than cognito:sub

@mihaiblaga89
Copy link
Author

@mlabieniec @powerful23 any updates on this? Thanks

@powerful23
Copy link
Contributor

powerful23 commented Nov 2, 2018

@mihaiblaga89 hey we recently merged a pr to allow users to define the default endpoint context in the configuration of Analytics. So if you could pass your specified userId when configuring the Analytics, then each time when Analytics send the updateEndpoint request, it will pick up that default userId instead of identityId. Does this solve your problem?

https://aws-amplify.github.io/docs/js/analytics#manual-setup

@mihaiblaga89
Copy link
Author

Sorry for the late reply, missed the email. In theory, it should, but I'll check it when I get back to that task again because at the moment it's on the back burner. Will keep you posted.

@lafiosca
Copy link
Contributor

@powerful23 Are you suggesting that we re-call Analytics.configure each time a user logs in or logs out?

@amidulanjana
Copy link

@powerful23 I need to send push notifications to specific user from my backend. How can I achieve this ?

@powerful23
Copy link
Contributor

@amilaDulanjana hi because Amplify is mainly focusing on the frontend so I am not the expert to answer this. Can you go to the AWS Forum: https://aws.amazon.com/premiumsupport/knowledge-center/send-feedback-aws/ and open an issue there which is a more efficient way.

@amidulanjana
Copy link

amidulanjana commented Mar 25, 2019

@powerful23 No I cannot registered the user with the userId I am using following code. is there any issue with the code ? And also how can I know whether the user is registered with the provided userId. Thanx
`
import React, {Component} from 'react';
import {Platform, StyleSheet, Text, View} from 'react-native';

import { Logger } from 'aws-amplify';
import Auth from '@aws-amplify/auth';
import Analytics from '@aws-amplify/analytics';
import PushNotification from '@aws-amplify/pushnotification';

import config from './aws-exports'

Auth.configure(config);
Analytics.configure(config);
PushNotification.configure(config);
Logger.LOG_LEVEL = 'DEBUG';

export default class App extends Component {

componentDidMount() {
// get the notification data when notification is received
PushNotification.onNotification((notification) => {
// Note that the notification object structure is different from Android and IOS
console.log('in app notification', notification);

  // required on iOS only (see fetchCompletionHandler docs: https://facebook.github.io/react-native/docs/pushnotificationios.html)
  //notification.finish(PushNotificationIOS.FetchResult.NoData);
});

// get the registration token
PushNotification.onRegister((token) => {
  console.log('in app registration', token);
  debugger;
  alert(token)
  Analytics.updateEndpoint({
    address: token,
    //channelType: "GCM",
    OptOut: 'NONE',
    userId: "absdcdfere123234"
  }).then(data => {
    console.log(data)
  }).catch(error => {
    console.log(error)
  });
  //this.setState({ token })

});

// get the notification data when notification is opened
PushNotification.onNotificationOpened((notification) => {
  console.log('the notification is opened', notification);
});

}

render() {
return (

Welcome to React Native!

);
}
}`

@madmed88
Copy link
Contributor

this comment helped me do it: #2477 (comment)

@jordanranz
Copy link
Contributor

@mihaiblaga89 any update on this?

@jordanranz jordanranz added pending-close-response-required and removed investigating This issue is being investigated labels Jun 3, 2019
@haverchuck
Copy link
Contributor

Closing this issue due to inactivity.

@udh3151
Copy link

udh3151 commented Nov 28, 2019

Any update?

@miguelocarvajal
Copy link

This problem is still occurring, using the default user id is not a workable solution because we'd have to call it early in the initialization process where the user id (Cognito sub) is not available.

Calling Amplify.configure multiple times is also not very workable as it was generating invalid requests to the server from my brief tests.

@kylekirkby
Copy link

This is still an issue... I've tried initializing the endpoints with:

 Auth.currentUserInfo()
            .then(({ attributes }) => {
              const userAttributes = {};
              Object.entries(attributes).forEach(([key, value]) => {
                userAttributes[key] = [`${value}`];
              });
              Analytics.updateEndpoint({
                address: attributes.email, // Or phone_number
                channelType: "EMAIL", // Or 'SMS'
                userId: attributes.sub,
                userAttributes,
              }).then((result) => {
                console.log(result);
              });
            })
            .then(() => console.log("Analytics endpoint updated"));
        })

This adds users attributes into Pinpoint but when I go to test the transactional email templates, the email fails to send when specifying the Cognito User Sub as the Endpoint ID.

@github-actions
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Analytics Related to analytics Cognito Related to cognito issues
Projects
None yet
Development

No branches or pull requests