[HOLD for payment 2023-11-16] [$500] public room as anonymous user can able to edit profile

[m-natarajan]

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. Go to a public room as an anonymous user like https://staging.new.expensify.com/r/207591744844000
2. Click on the search icon.
3. Close the login menu.
4. enter url in chrome /settings/profile, /settings/profile/display-name

Expected Result:

we should show login modal

Actual Result:

able to edit the profile

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

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.78-1
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

Recording.480.mp4

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

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~017b639bc83b99b494
• Upwork Job ID: 1709944966242185216
• Last Price Increase: 2023-10-05
• Automatic offers:
• DylanDylann | Contributor | 27121862
• pradeepmdk | Reporter | 27121865

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~017b639bc83b99b494

[melvin-bot]

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

[DylanDylann]

Proposal

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

• public room as anonymous user can able to edit profile

What is the root cause of that problem?

• Currently, when users open the public room as anonymous users, the openReport API will set the ONYX.SESSION with the below value:

authToken: ****
authTokenType: "anonymousAccount"

• Then, if the user goes to /settings/profile/display-name, it will run the below logic:
  

  App/src/libs/actions/Report.js

  Lines 1889 to 1898 in 035ee44

  	InteractionManager.runAfterInteractions(() => {
  	Session.waitForUserSignIn().then(() => {
  	if (route === ROUTES.CONCIERGE) {
  	navigateToConciergeChat();
  	return;
  	}
  	Navigation.navigate(route, CONST.NAVIGATION.TYPE.PUSH);
  	});
  	});
  	}

• But in here, the Session.waitForUserSignIn() is always resolved because the sessionAuthToken is not null:
  

  App/src/libs/actions/Session/index.js

  Lines 29 to 40 in 567c32e

  	Onyx.connect({
  	key: ONYXKEYS.SESSION,
  	callback: (session) => {
  	sessionAuthTokenType = lodashGet(session, 'authTokenType');
  	sessionAuthToken = lodashGet(session, 'authToken');
  	if (sessionAuthToken && authPromiseResolver) {
  	authPromiseResolver(true);
  	authPromiseResolver = null;
  	}
  	},
  	});

• So it will navigate user to /settings/profile/display-name screen, which leads to the bug "public room as anonymous user can able to edit profile"

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

• We can update the handleStateChange from
  

  App/src/libs/Navigation/NavigationRoot.js

  Lines 121 to 131 in 567c32e

  	const handleStateChange = (state) => {
  	if (!state) {
  	return;
  	}
  	// Performance optimization to avoid context consumers to delay first render
  	setTimeout(() => {
  	updateCurrentReportID(state);
  	}, 0);
  	parseAndLogRoute(state);
  	animateStatusBarBackgroundColor();
  	};

  
  to

const handleStateChange = (state) => {
    ...
      const route = Navigation.getActiveRoute();
        if (Session.isAnonymousUser() && !canBeAccessByAnonymousUserListRoutes(route)) {
            Session.signOutAndRedirectToSignIn()

        }

 const canBeAccessByAnonymousUserListRoutes = (route) => {
        const reportID = getReportIDFromLink(route);
        if (reportID) {
            return true;
        }
        return false;
    };

What alternative solutions did you explore? (Optional)

• We can also create the function canBeAccessByAnonymousUserListRoutes like above.
• And update the logic
  

  App/src/libs/actions/Report.js

  Lines 1889 to 1898 in 035ee44

  	InteractionManager.runAfterInteractions(() => {
  	Session.waitForUserSignIn().then(() => {
  	if (route === ROUTES.CONCIERGE) {
  	navigateToConciergeChat();
  	return;
  	}
  	Navigation.navigate(route, CONST.NAVIGATION.TYPE.PUSH);
  	});
  	});
  	}

  
  to:

 InteractionManager.runAfterInteractions(() => {
        Session.waitForUserSignIn().then(() => {
            if (route === ROUTES.CONCIERGE) {
                navigateToConciergeChat();
                return;
            }
            if (Session.isAnonymousUser() && !canBeAccessByAnonymousUserListRoutes(route)) {
                Session.signOutAndRedirectToSignIn();
                return;
            }
            Navigation.navigate(route, CONST.NAVIGATION.TYPE.PUSH);
        });
    });

Result

Screencast.from.05-10-2023.23.52.27.webm

[redpanda-bit]

Proposal

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

Unauthorized access to screens after manual input of url.

What is the root cause of that problem?

When visiting a public chat room, user is being logged in as anonymous user and because of this the user is granted access to all AuthScreens as shown in this if condition (line # 10):

App/src/libs/Navigation/AppNavigator/index.js

Lines 10 to 17 in 389d7b0

	if (props.authenticated) {
	const AuthScreens = require('./AuthScreens').default;
	// These are the protected screens and only accessible when an authToken is present
	return <AuthScreens />;
	}
	const PublicScreens = require('./PublicScreens').default;
	return <PublicScreens />;

All AuthScreens are accessible by anonymous users until the function checkIfActionIsAllowed is called by onPress handlers in pressable items such as the search icon e.g. (line # 159):

App/src/pages/home/sidebar/SidebarLinks.js

Lines 155 to 160 in 389d7b0

	<PressableWithoutFeedback
	accessibilityLabel={translate('sidebarScreen.buttonSearch')}
	accessibilityRole={CONST.ACCESSIBILITY_ROLE.BUTTON}
	style={[styles.flexRow, styles.ph5]}
	onPress={Session.checkIfActionIsAllowed(showSearchPage)}
	>

The props.authenticated above comes all the way from Expensify.js and it is a boolean value based on the presence of props.session.authToken. – authToken is a valid string for anonymous users.

This checkIfActionIsAllowed pretty much checks if the user is allowed to execute a function based on anonymous or not.

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

Add a new function to Session/index.js that will redirect user to sign in if the user is anonymous e.g.,

// src/libs/actions/Session/index.js

function redirectToSignInIfAnonymous() {
    if (isAnonymousUser()) {
        signOutAndRedirectToSignIn();
    }
}

And make use of it within RightModalNavigator.js like so:

// src/libs/Navigation/AppNavigator/Navigators/RightModalNavigator.js
import * as Session from '../../../../libs/actions/Session';

function RightModalNavigator(props) {
    const {isSmallScreenWidth} = useWindowDimensions();
    useEffect(() => Session.redirectToSignInIfAnonymous(), []);
// ...

This will ensure that the user is redirected upon initial mounting of the screen.

What alternative solutions did you explore? (Optional)

1. Change the Stack.Screens in RightModalNavigator.js to return the sign in component instead of the expected screen when the user is an anonymous user:

<Stack.Screen
  name="Settings"
  component={isAnonymousUser ? ModalStackNavigators.SignInModalStackNavigator : ModalStackNavigators.SettingsModalStackNavigator}
/>

[burczu]

not overdue - I'll review proposals soon

[burczu]

I've just checked both proposals.

The one from @redpanda-bit is working only partially: when we type address like /settings/profile/display-name it indeed redirects to the login page inside the right sidebar, but when you click back button in the sidebar header, it redirects to /settings/profile page instead of staying to the login page.

In terms of the proposal form @DylanDylann, his main solution is working, so I think we can consider proceeding with it (just to note: his alternative solution has the same problem as described above, so shouldn't be considered).

🎀 👀 🎀 C+ reviewed

[melvin-bot]

Triggered auto assignment to @stitesExpensify, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[stitesExpensify]

So to confirm, is this only a front end problem? You can't actually change any settings right?

[melvin-bot]

📣 @DylanDylann 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[melvin-bot]

📣 @pradeepmdk 🎉 An offer has been automatically sent to your Upwork account for the Reporter role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

[DylanDylann]

@burczu PR #29248 is ready for review

[melvin-bot]

This issue has not been updated in over 15 days. @burczu, @stitesExpensify, @DylanDylann eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

[DylanDylann]

@stitesExpensify The PR is merged

[melvin-bot]

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

[mountiny]

This was deployed and should be ready for payment in two days, the automation misbehaved.

[alexpensify]

@mountiny - is there an agreed to payment amount here or is it the standard rate of $500? Thanks for the update!

[mountiny]

I think standard

[alexpensify]

Here is the payment summary:

• External issue reporter @pradeepmdk $50
• Contributor that fixed the issue @DylanDylann $500
• Contributor+ that helped on the issue and/or PR @burczu payment for contractor

Upwork job

*If applicable, the bonuses will be applied on the final payment

Extra Notes regarding payment: N/A

[alexpensify]

Closing - everyone has been paid via Upwork