[HOLD #22480] [$1000] Web - User details like email is being shown if user split bills with an account with no prior interaction

[kbecciv]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Action Performed:

1. Open a new account
2. Click on FAB > new chat
3. Input a new account and start a chat
4. Click on the chat header and notice that since there is no prior conversation account details are hidden
5. Click on FAB > Split bill
6. Enter amount and click next
7. Enter two other new accounts you didn't use before
8. Click on split bill
9. Click on the header and click on the users

Expected Result:

User details like email should not be visible since they are hidden for new accounts with no prior interaction

Actual Result:

User details get hidden if user starts a new chat with an account with no prior interaction but details get visible if user splits bill with an unknown account

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

• [ ] Android / native
• [ ] Android / Chrome
• [ ] iOS / native
• [ ] iOS / Safari
• [x] MacOS / Chrome / Safari
• [ ] MacOS / Desktop

Version Number: 1.3.35-5 Reproducible in staging?: y Reproducible in production?: y If this was caught during regression testing, add the test name, ID and link from TestRail: Email or phone of affected tester (no customers): Logs: https://stackoverflow.com/c/expensify/questions/4856 Notes/Photos/Videos: Any additional supporting documentation

https://github.com/Expensify/App/assets/93399543/f9c89e8a-a1c3-442d-914e-e5746fc62f96

https://github.com/Expensify/App/assets/93399543/072da4ac-4614-4eb4-a82b-03bfa562bbd0

Expensify/Expensify Issue URL: Issue reported by: @Nathan-Mulugeta Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1688212019267359

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~015a3ad10bcda1e234
• Upwork Job ID: 1676118022049849344
• Last Price Increase: 2023-07-18

[melvin-bot[bot]]

Triggered auto assignment to @kadiealexander (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[melvin-bot[bot]]

Bug0 Triage Checklist (Main S/O)

• [x] This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• [x] This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• [x] This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• [x] This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• [x] I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[kadiealexander]

Repro:

https://github.com/Expensify/App/assets/59587260/21e30ed4-3279-401e-a44b-3f78a3e5f405

[melvin-bot[bot]]

Job added to Upwork: https://www.upwork.com/jobs/~015a3ad10bcda1e234

[melvin-bot[bot]]

Current assignee @kadiealexander is eligible for the External assigner, not assigning anyone new.

[melvin-bot[bot]]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @thesahindia (External)

[daordonez11]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Private information of profile is shown after creating split bill request.

What is the root cause of that problem?

From my initial review email is shown if the personalDetail information stored in the personalDetailsList key contains the login of the user. Basically, MoneyRequestConfirmPage and NewChatPage create the optimistic data in different ways.

Specifically split bill uses createSplitsAndOnyxData

https://github.com/Expensify/App/blob/7e908b17565e7bf659309f4b4d95141e70094ab9/src/libs/actions/IOU.js#L642-L650

When the request fails login is still stored so the information is shown even if there is no interaction.

Whenever the request is executed correctly is the server the one answering with all the details of the accounts:

Probably because its a direct interaction but that would have to be checked, out of my knowledge

What changes do you think we should make in order to solve the problem?

https://github.com/Expensify/App/blob/7e908b17565e7bf659309f4b4d95141e70094ab9/src/libs/actions/IOU.js#L642-L650

Update createSplitsAndOnyxData and remove the login from the optimistic data to avoid showing the login in a failed request scenario. Change it to:

const oneOnOnePersonalDetailListAction = {
            [accountID]: {
                accountID,
                avatar: UserUtils.getDefaultAvatarURL(accountID),
                displayName: participant.displayName || email,
            },
        };

Same solution in openReport in Report.js

 const optimisticPersonalDetails = {};
        _.map(participantLoginList, (login, index) => {
            const accountID = newReportObject.participantAccountIDs[index];
            optimisticPersonalDetails[accountID] = allPersonalDetails[accountID] || {
                accountID,
                avatar: UserUtils.getDefaultAvatarURL(accountID),
                displayName: login,
            };
        });

Image of solution applied:

Regarding the successful scenario, backend needs to confirm if private data can be seen if the request is executed

In case data shouldn't be visible backend needs to update the OpenReport service when the split bills report is opened to avoid sending personal data of a user that had no interaction previously. Still, the upper solution is required to handle the case where the split fails. e.g.

What alternative solutions did you explore? (Optional)

This one is a bit hard, following the steps specified in the test you can see an initial account is created with a short id e.g. 14965227 Then when creating the split request a new id is assigned to the same email e.g.: 4771036978245898 maybe there is a point where we could use the method getAccountIDsByLogins same as in the new chat but couldn't find the place where the accounts are created in the split bill request to avoid the double accountID creation. Still when it is a new account included in MoneyRequestConfirmationPage it will not find a previous account in accountDetails

[mountiny]

cc @Beamanator would you be able to check this one out to see if the expected behaviour is correct?

[Beamanator]

Hmm this is an interesting situation because...

The person who send the "split bill" request doesn't technically "know" the other users (assuming they're on different private domains & no same private workspace rooms, etc) - BUT since the person initiating the request Knows the email addresses (they manually typed them in) - should we optimistically store them or not?

I can see the case that yes, might as well - but we shouldn't be returning them from the backend in this case

[daordonez11]

If I could enter the discussion! I would recommend unifying the experience, I agree with @Beamanator if you already entered the email we should show it optimistically, in that case we would have to update only the flow of creating a new chat. From my point of view, current storage and behavior of optimistic data in this flow is okay, I would prefer to edit new chat.

In terms of backend I would suggest avoiding sending Personal Identifiable Information. Yesterday I tested and I was even able to see user name of the person I requested money. Accessing PII only with the email is not GDPR compliant. We could include in the algorithm only sending emails if there is no additional interaction than the request.

[kadiealexander]

Not overdue. @Beamanator would you please summarise the expected behaviour here? I think it's going a bit over my head.

[Beamanator]

Interesting points @daordonez11 - one question:

Yesterday I tested and I was even able to see user name of the person I requested money.

Just to be clear, are you considering user name / display name PII? I would probably argue against that, since a display name can be absolutely anything you want it to be. A person's legal name cannot be retrieved in this way.

In one of the bug report videos, I see that we did show timezone (local time) for a person you just started a chat with - that should not happen and is a bug if it happens (I'm not exactly sure if that user was completely unknown or not)

So @kadiealexander @thesahindia is the main point of this issue about showing the email part on the participants/user page? Can we also verify if other info like timezone is always or only sometimes shown when following that flow with absolutely brand new users?

[daordonez11]

Oh I see where you are going! Maybe since it's a displayNameit isn't so bad but then there should be a disclaimer: any user can see this name if they contact you. The chances people use their legal name here are high since we ask for first and last name in the form. Se yeah since this can be their real name I do see it as PII. Definitely I could see Timezone from a user I hadn't interacted in that account. Let's wait for expected behavior of email but I thinks that's the least strange behavior in all of this interaction 😂

[melvin-bot[bot]]

@kadiealexander, @thesahindia Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[thesahindia]

In one of the bug report videos, I see that we did show timezone (local time) for a person you just started a chat with - that should not happen and is a bug if it happens (I'm not exactly sure if that user was completely unknown or not)

I was able to repro it for completely unknown users. After splitting the bill I waited a little and the timezone appeared.

[melvin-bot[bot]]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[kadiealexander]

@thesahindia should we bump the price, do you reckon? :)

[daordonez11]

So there are 2 things here @Beamanator @mountiny correct me if I'm wrong

1. Consistency between optimistic data and creation of user between SplitBill and NewReport, for this my proposal works fine and the idea is to unify the behavior between both, as @Beamanator said since we are using email we can just save it in optimistic data.
2. TZ can be seen between unknown users, this is a backend issue and needs to be fixed as well internally.

I don't think its a matter of increasing price, it's a matter of clarity!

If everyone agrees with 1 I can update the proposal and help with that part of the issue. In terms of the backend I'm not so sure about how to proceed. Also, I think OpenReport service needs to be checked because if it is a single report way less details are being shared: OpenReport single:

OpenReport of Request:

cc @thesahindia @kadiealexander

[daordonez11]

Regarding unifying the experience maybe we can take it to slack the two options are:

1. Without email in details:

2. With email in details (Ignore Local time)

[mountiny]

Sounds good to me and i agree this would be good to take so slack to confirm

[daordonez11]

Hey everyone posted in Slack to move on with proposal: https://expensify.slack.com/archives/C01GTK53T8Q/p1689280365532099. Also, how can we manage the back issue @Beamanator

[Beamanator]

@daordonez11 thanks for bringing up that slack convo 👍

As for the backend issue, can you explain the flows that resulted in different personal details coming back? What is "OpenReport single:" vs "OpenReport of Request:"? Sorry if I'm dense this morning :D

[daordonez11]

Oh yeah my bad "OpenReport of Request: = IOU, showing a video for clearer explanation 1. Open new empty chat 2. Open IOU report. Or more like U Owe Me .

Demo OpenReport diff.webm

PD: Since everyone responded option 2 on slack I will be updating proposal for optimistic data in front end, but also that means backend in option 1 shown in the video should respond the login attribute as well.

cc @mountiny @Beamanator

[mountiny]

I think that sounds good, @thesahindia @Beamanator should we assign @daordonez11 and figure out the details in the PR.

There was also the proposal to not allow users to open the details page, I think that makes sense to some extent but also it would be quite odd pattern in the app I think

[daordonez11]

Hey there so quick recap and conclusions! @Beamanator @mountiny I finished the analysis and the optimisticData in new chat DOES contain the login it's the server one that only responds displayname. In code it is created here:

https://github.com/Expensify/App/blob/272d921d4fbd3f6bf98e6b124c8cc53eefdafc3b/src/libs/actions/Report.js#L478-L484

As seen here:

Hence after all the discussions and definitions, this is only a backend issue (internal). Also if login is sent in openReport.personalDetailList this issue (that issue does have a workaround in front), will be fixed as well.

cc @thesahindia

[mountiny]

We gotta find some solution to this in general. I assume we wont be returning the email of the user until they are known. But that leads to an issue where certain flows such as money request flows which depend on debtorEmail wont work as we will not have that email available

[melvin-bot[bot]]

@kadiealexander @thesahindia this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[mountiny]

Alex is ooo today but should be back tomorrow to discuss

[melvin-bot[bot]]

@kadiealexander, @thesahindia Eep! 4 days overdue now. Issues have feelings too...

[mountiny]

Discussing in slack

[melvin-bot[bot]]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[kadiealexander]

@mountiny could you please link the discussion so we can all follow along?

[daordonez11]

Hey everyone FYI here is the link https://expensify.slack.com/archives/C01GTK53T8Q/p1689623386449679 @Beamanator has provided some solutions for @aldo-expensify to execute. From the front-end side, we can provide accountId for the fallback implementation. This is where a collaborator could help. PS: I can definitely provide a proposal for this. There is still some open discussion on how things will be managed on Back End and if the mail will still be hidden. @mountiny correct me if I missed something.

PSPS: @mountiny we should debate a bit more on the consistency of the response from the backend to unify the experience of this specific issue

cc @thesahindia @kadiealexander

[mountiny]

@daordonez11 thank you, I have been on mobile without the link before, sorry.

Yep I agree, I think @aldo-expensify is quite busy right now, but once we get this going, I think @daordonez11 should help us with the frontend implementation

[kadiealexander]

@mountiny in the meantime should I place this on hold for https://github.com/Expensify/App/issues/22480?

[mountiny]

yes

[kadiealexander]

Still on hold.

[kadiealexander]

Same!

[kadiealexander]

Same as above.

[kadiealexander]

Still on hold.

[kadiealexander]

Still on hold

[kadiealexander]

Still on hold

[kadiealexander]

@Nathan-Muguleta can you still reproduce this now that #22480 is resolved?

[Nathan-Mulugeta]

I will test this as soon as this DB is solved.

[Nathan-Mulugeta]

I just retested this now and this issue is no longer present.

[melvin-bot[bot]]

@kadiealexander, @thesahindia Whoops! This issue is 2 days overdue. Let's get this updated quick!

[kadiealexander]

Sweet! Thanks Nathan. Closing as no longer reproducible.