[REPORTING PAYMENT][$500] Web - Opening the #teachers-unite room directly through the link shows sign in option even when signed in

[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. Logout if already logged in
2. Go to https://staging.new.expensify.com/r/207591744844000
3. Click on the Sign in button
4. Enter the email and click Continue
5. Enter magic code
6. Next, click on the #teachers-unite room header
7. Click on Leave room
8. Open a new tab and go to https://staging.new.expensify.com/r/207591744844000

Expected Result:

The sign-in option should not be displayed

Actual Result:

The sign-in option is displayed. Also, the sign-in modal still has the previously entered magic code (Note: this may not be reproducible on the newest version - #26615 (comment))

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: 1.3.62.0
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

Screen.Recording.2023-08-30.at.12.51.28.PM.mov

Recording.4216.mp4

Expensify/Expensify Issue URL:
Issue reported by: @adeel0202
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1693382434318589

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0164d1ccb8bef91f4d
• Upwork Job ID: 1698473014682202112
• Last Price Increase: 2023-09-10

[melvin-bot]

Triggered auto assignment to @jliexpensify (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

[jliexpensify]

@adeel0202 @kbecciv on v1.3.62-3, I can reproduce part of this issue - basically, the part I cannot reproduce is showing the Magic Link details prefilled. On the newest staging version, I see this:

However, leaving the room and opening a new tab and using the URL still shows a "Sign In" button - I think this should be fixed, since you shouldn't need to sign in again as all you've done is left the room.

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~0164d1ccb8bef91f4d

[melvin-bot]

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

[melvin-bot]

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

[AmjedNazzal]

Proposal

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

Web - Opening the #teachers-unite room directly through the link shows sign in option even when signed in

What is the root cause of that problem?

The method in which we check to hide the composer and show sign in button is unreliable, there is no doubt here that we are facing a backend issue where leaving a room in some cases will result in session.authTokenType being returned by the backend as "anonymousAccount" and I believe that should be addressed on an internal level, however I think we should also address this on frontend level to avoid this kind of behaviour in the future as relying on authTokenType isn't the best idea when we have access to two more unique items that will be available when the user is signed in which are CREDENTIALS.login and CREDENTIALS.validateCode.

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

We should refrain from using authTokenType and start using CREDENTIALS when we are checking for credentials as is the case with checking if isAnonymousUser, to do this we can make a change to session.isAnonymousUser to use these credentials for the check.

function isAnonymousUser() {
    return !lodashGet(credentials, 'login') && !lodashGet(credentials, 'validateCode');
}

Another thing we can change is ReportUtils.shouldDisableWriteActions which uses isAnonymousUser that it calls itself from Onyx which is not good for keeping the code DRY as we are already connecting to the exact same part of Onyx in Session component so we can simply change the check for shouldDisableWriteActions to use the same Session.isAnonymousUser function.

function shouldDisableWriteActions(report) {
    const reportErrors = getAddWorkspaceRoomOrChatReportErrors(report);
    return isArchivedRoom .. || Session.isAnonymousUser(); // Changing from local variable to calling Session component
}

Result

Screen.Recording.2023-09-04.at.3.16.19.AM.mov

[kaushiktd]

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

Opening #teachers-unite room directly through the link shows sign in option even when signed in

What is the root cause of that problem?

The root cause of the problem is the unreliability of the method used to decide whether to show the signup modal in the frontend of the application. The issue is exacerbated by inconsistent data returned from the backend, specifically the session.authTokenType. This inconsistency occurs when leaving a room, resulting in session.authTokenType being sometimes incorrectly set to 'anonymousAccount'.

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

To address this issue effectively, we propose a solution that involves both frontend and potentially backend adjustments:

Frontend Solution:

Enhance Authentication Checks: Instead of relying solely on session.authTokenType, which has proven to be unreliable due to backend inconsistencies, prioritize the use of more dependable indicators of user authentication, such as CREDENTIALS.login and CREDENTIALS.validateCode.

Modify the isAnonymousUser Function: Refactor the isAnonymousUser function to check the user's anonymity based on the presence of CREDENTIALS.authTokenType rather than session.authTokenType. This adjustment ensures more accurate identification of anonymous users.
Below is the path for file index
https://github.com/Expensify/App/blob/main/src/libs/actions/Session/index.js

function isAnonymousUser(userCredentials) {
    return (
        userCredentials &&
        userCredentials.authTokenType === 'anonymousAccount'
    );
}

Update the shouldDisableWriteActions Function: In the shouldDisableWriteActions function, prioritize checking user authentication based on the existence of CREDENTIALS.login and CREDENTIALS.validateCode. This change reduces reliance on session.authTokenType.
Below is the path for file and code
https://github.com/Expensify/App/blob/main/src/libs/ReportUtils.js

function shouldDisableWriteActions(report, userCredentials) {
    const reportErrors = getAddWorkspaceRoomOrChatReportErrors(report);

    // Check if the user is authenticated using CREDENTIALS.login and CREDENTIALS.validateCode
    const isAuthenticated = !!userCredentials && !!userCredentials.login && !!userCredentials.validateCode;

    // Additional conditions and logic for disabling write actions
    return (
        isArchivedRoom(report) ||
        !_.isEmpty(reportErrors) ||
        !isAllowedToComment(report) ||
        !isAuthenticated
    );
}

Backend Solution (if necessary):

Address Backend Inconsistencies: Investigate and address the root cause of the backend inconsistency where leaving a room results in an incorrect session.authTokenType. Implement measures to ensure that user authentication data is consistently and accurately updated.
By implementing these frontend adjustments and, if necessary, addressing backend inconsistencies, you can create a more reliable and resilient authentication and user experience in your application. These changes will help prevent the recurrence of the issue where the signup modal is displayed incorrectly.

Video:
https://drive.google.com/file/d/1_JvY6K3QzcFRzkldx0z8uxuOnaT7DI5I/view?usp=sharing

[jliexpensify]

Not overdue, but @sobitneupane we have some proposals rolling in!

[pradeepmdk]

Proposal

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

Opening the #teachers-unite room directly through the link shows sign in option even when signed

What is the root cause of that problem?

authTokenType = 'anonymousAccount' not removing from onyx state

when we set Onyx.merge('session', {'authTokenType': null}); means that key should be removed from database but not happing when set this callback triggered without null but database not updating.

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

we should be update the backend passing the empty string for update the onyx
frontend means:
signinapi success data we should update this empty string;

//successData
{
            onyxMethod: Onyx.METHOD.MERGE,
            key: ONYXKEYS.SESSION,
            value: {
                authTokenType:'',
            },
        },

we need update all signin call SigninUser, apple signin, googlesignin
Note: after login and refresh the page means it will ask again signin .

this ONYX BUG:
Onyx.merge('session', {'authTokenType': null}); when you set this null means database is not updated.
but sub level is accepted here
Onyx.merge('session', {'authTokenType':{data: null}}); when you set this null means database will allow to insert

@sobitneupane if you want i will fix this on ONYX repo

[melvin-bot]

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

[melvin-bot]

@sobitneupane, @jliexpensify Huh... This is 4 days overdue. Who can take care of this?

[sobitneupane]

Thanks for the proposal everyone.

@pradeepmdk Would you mind explaining your proposal. I am trying to understand your proposal. But I am not sure if I am getting your point.

when we set Onyx.merge('session', {'authTokenType': null}); means that key should be removed from database but not happing when set this callback triggered without null but database not updating.

Where are we calling Onyx.merge('session', {'authTokenType': null})? Is the issue existent on Onyx? What is the issue?

[pradeepmdk]

@sobitneupane yes this is bug Onyx.

we are not calling anywhere Onyx. merge('session', {'authTokenType': null}) just ref I added. you can run this on local env if authTokenType already has the value means it should not removed or updated as null in indexdb.

as mentioned in my screenshot after signing successfully API sends sending Onyx merge command with authTokenType: null but indexDB is not updated.

Screen.Recording.2023-09-11.at.1.52.08.PM.mov

this is the ref of the merge command in Onyx. because API is sending the merge command right .so i am trying to explain here merge command has this issue.
Onyx. merge('session', {'authTokenType': null})

[pradeepmdk]

@sobitneupane already we have PR for this fix Expensify/react-native-onyx#333 we should wait for merging after the upgrade of the onyx lib so this will work.

[sobitneupane]

Thanks for the update @pradeepmdk. Let's hold this issue for now. We can come back and check after Expensify/react-native-onyx#333 gets merged.

cc: @jliexpensify

[jliexpensify]

Sure thing @sobitneupane and @pradeepmdk - bump me when you want this taken off hold!

[jliexpensify]

Still on hold

[pradeepmdk]

@sobitneupane this is fixed on this version 1.0.89. can I raise the pr for the upgraded version?

[sobitneupane]

@pradeepmdk I don't think we need to raise PR here. I will be raised in the associated issue for which PR was raised.

[pradeepmdk]

@sobitneupane this issue was solved only when we upgraded React native Onyx version 1.0.89.

[sobitneupane]

We will wait for it to update from the associated issue.

[pradeepmdk]

@sobitneupane Actually I only commented on that. based on my comment they raised the another PR.
Expensify/react-native-onyx#333
Expensify/react-native-onyx#333 (comment)
Expensify/react-native-onyx#359

[pradeepmdk]

@sobitneupane this is fixed now.

[jliexpensify]

@pradeepmdk thanks for letting us know. Just tried on v1.3.78-4 and can verify it's fixed.

Going to pay @adeel0202 for reporting and then close this out.

[jliexpensify]

Payment Summary

• Reporter: @adeel0202 $50

Upworks Job

[jliexpensify]

Paid, job closed!