[$500] Asking sign-in again from public room after refresh

[izarutskaya]

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 any public room without any authentication
2. Login from the prompts shown in the public channel
3. Complete login and observe you're loggedin
4. Refresh page and it's asking login again.

Expected Result:

Once after login it shouldn't ask login again.

Actual Result:

Asking login again.

Workaround:

Unknown

Platforms:

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

• Android / native
• Android / Chrome
• iOS / native
• iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number: v1.3.67-3

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

Kapture.2023-09-06.at.14.15.52.mp4

Recording.1505.mp4

Expensify/Expensify Issue URL:

Issue reported by: @b4s36t4

Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1693989869553719

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~015c546ad4cf74de6e
• Upwork Job ID: 1701334953343184896
• Last Price Increase: 2023-10-02

[melvin-bot]

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

[melvin-bot]

Bug0 Triage Checklist (Main S/O)

• This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• 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
• 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.
• 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.
• I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[melvin-bot]

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

[melvin-bot]

Triggered auto assignment to @sophiepintoraetz (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

[melvin-bot]

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

[parasharrajat]

In the QA video, they were not able to sign in due to an invalid magic code. Is that the issue?

[parasharrajat]

Seems like a regression.

[saraelsa]

Proposal

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

If the user signs in while viewing an anonymous report, the authentication state is not persisted.

What is the root cause of that problem?

Once the user opens a public room without having been signed in, they are signed in as an anonymous user. This information is retained in Oryx in the session key with authTokenType being anonymousUser.

This should be cleared once the server returns a log in response with authTokenType merged to null. This is indeed what happens on the session.

However, Onyx fails to persist this to the disk. The authTokenType remains anonymousUser on the disk, and upon refresh, the session state also reverts to anonymousUser.

The application therefore believes that the current user is an anonymous user.

Onyx's failure arises as follows:

1. A modifiedData object is generated with the final intended value of the key in Onyx, Onyx.js:1100. Keys with the final value intended to be null are unset in this object.
2. This object is sent to Storage, Onyx.js:1120.
3. The IDB provider disregards the changes object and passes down the final intended value to its multiMerge method, IDBKeyVal.js:73.
4. The IDB provider treats the final intended value passed to it as a changes object, and merges it with the current value, IDBKeyVal.js:58. Because it is the final intended value, keys whose values were null have already been removed. The merge method retains therefore these keys from the first parameter supplied to it—the existing value. Therefore, the existing value is retained in storage.

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

Because the multiMerge method appears to be designed in such a way that a changes object should be passed to it (rather than a final intended value), the mergeItem method should be updated to disregard the final intended value and instead pass the changes object available to it.

What alternative solutions did you explore? (Optional)

Instead of using null for authTokenType, a string can be returned instead. This would override the existing value. However, as the root cause appears to be a bug in Oryx, it would be better to fix the bug at its source instead.

Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job.

[melvin-bot]

📣 @saraelsa! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

1. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
2. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
3. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
   
   Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

[melvin-bot]

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

[parasharrajat]

Thanks, @saraelsa for the proposal. As you mentioned, this is a regression from Recent Onyx changes. We are actively making changes to Onyx ATM. I think it will be better to let that team handle that issue.

If we find that some changes are required on app side, we can revisit the proposals here.

[parasharrajat]

https://expensify.slack.com/archives/C02NK2DQWUX/p1694531357695759 cc: @strepanier03

[strepanier03]

Thanks @parasharrajat - In that case do you recommend closing this or leaving it open but not accepting proposals?

[strepanier03]

@sophiepintoraetz - Looks like a double assignment so I'm going to remove you so you don't have to worry about this.

[parasharrajat]

@strepanier03 It would be better if someone from internal team confirms that same and plans the next action before we close it.

[strepanier03]

Gotcha, thanks @parasharrajat - I'm raising internally now.

[melvin-bot]

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

[parasharrajat]

Waiting on internal decision @strepanier03

[melvin-bot]

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

[strepanier03]

Bumped internally again.

[strepanier03]

@parasharrajat - Can you tell me who the people are that are working on these Onyx changes you mention? I've asked a few times in Slack and no one can point me in the right direction.

[parasharrajat]

Even I don't know. I will find them and tag here.

[parasharrajat]

@strepanier03 Tagged.

[strepanier03]

Thank you!

[melvin-bot]

@strepanier03 @parasharrajat 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!

[parasharrajat]

@izarutskaya @b4s36t4 is it still reproducible?

[b4s36t4]

@parasharrajat Yes it still re-produceable took a latest pull and tested.

[melvin-bot]

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

[marcochavezf]

Coming from here, assigning myself. @daordonez11 mentioned that we're still getting the value for authTokenType, so probably we're not clearing out the value from an API command.

[strepanier03]

Thank you @marcochavezf - I'll follow along but ping me if you need me to do something I'm not doing 🙌

[pradeepmdk]

I think this dube of this #26615 (comment)

@marcochavezf this onyx issue. api is giving null only but onyx not removing the key
Expensify/react-native-onyx#333

[marcochavezf]

Hi @pradeepmdk, I saw your comment here, do you know by chance if that also impacts this issue?

[pradeepmdk]

@marcochavezf yes, this is fixed now. you can verify this one staging now.

Untitled.mp4

[melvin-bot]

@marcochavezf @strepanier03 @parasharrajat this issue is now 3 weeks old. There is one more week left before this issue breaks WAQ and will need to go internal. What needs to happen to get a PR in review this week? Please create a thread in #expensify-open-source to discuss. Thanks!

[melvin-bot]

@marcochavezf, @strepanier03, @parasharrajat Whoops! This issue is 2 days overdue. Let's get this updated quick!

[melvin-bot]

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

[marcochavezf]

@marcochavezf yes, this is fixed now. you can verify this one staging now.

Untitled.mp4

Oh cool, @strepanier03 I think we can then close this one

[strepanier03]

Sounds good, I'll do that now.