Fixes redirect on login (#6900)

[Om-Thorat]

Proposed Changes

• Fixes Redirect not working upon login #6900
• Added a test for the same

@coronasafe/care-fe-code-reviewers @coronasafe/code-reviewers

Merge Checklist

• Add specs that demonstrate bug / test a new feature.
• Update product documentation.
• Ensure that UI text is kept in I18n files.
• Prep screenshot or demo video for changelog entry, and attach it to issue.
• Request for Peer Reviews
• Completion of QA

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
care-storybook	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jan 16, 2024 8:18pm

[netlify]

✅ Deploy Preview for care-egov-staging ready!

Name	Link
🔨 Latest commit	cb7cc94
🔍 Latest deploy log	https://app.netlify.com/sites/care-egov-staging/deploys/65a6e47d1b966e0008c73317
😎 Deploy Preview	https://deploy-preview-6911--care-egov-staging.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[rithviknishad]

Now that you've removed the support for redirect param completely, the "Session Expired" workflow wouldn't work this way right, since that still relies on the redirect param on the login page route?

[Om-Thorat]

not sure why run failed since it passed on local, looking into it.

[rithviknishad]

Very likely because of the hardcoded UUID itself. Such an entity does not exist in cypress environments database.

[Om-Thorat]

Now that you've removed the support for redirect param completely, the "Session Expired" workflow wouldn't work this way right, since that still relies on the redirect param on the login page route?

Hmm, How do we plan on dealing with that? Should we just redirect them to the same page and bring up the login screen?

[rithviknishad]

You could do the following:

1. If a redirect query param is present, use that. (Same as original implementation).
2. If redirect query param is not present, simply call the signIn method from useAuth when the login button is clicked without doing a navigate. (Since it's very likely that the we need to land in the current route itself, just that we need to cause a re-render the AuthProvider component.

[rithviknishad]

So that means, the only code change required to fix this would be:

[Om-Thorat]

So that means, the only code change required to fix this would be:

The new redirector function doesn't check for a redirect param anymore so, There'll be that to fix as well

[Om-Thorat]

You could do the following:

1. If a redirect query param is present, use that. (Same as original implementation).

2. If redirect query param is not present, simply call the `signIn` method from `useAuth` when the login button is clicked without doing a navigate. (Since it's very likely that the we need to land in the current route itself, just that we need to cause a re-render the AuthProvider component.

as required clicking on the button signs out and redirects to the page which then loads a login screen since the user is signed out, In the other case that a query param isn't present the url will just be /session-expired which should redirect to /.

[Om-Thorat]

Sorry for the oversight before added a check to ensure that the redirect param redirects to a site that has the same origin as the current site, Not using this could have let malformed urls to be able to redirect to any sites they please ,potentially malicious.

[Om-Thorat]

@rithviknishad This new function works with both the redirect param and the direct urls , When an invalid redirect param is provided with a valid base url, The app chooses to discard the param and simply navigate to the valid base url Instead. The session expired redirects to the redirect param if available.

It seems like there should be a test for the redirect param as well?

[rithviknishad]

@Om-Thorat

Didn't this one line change (#6911 (comment)) alone solve the original issue? Was there some other issues?

[Om-Thorat]

@Om-Thorat

Didn't this one line change (#6911 (comment)) alone solve the original issue? Was there some other issues?

I did change the line to that but there had to be the reserved URLs change because of that rest much isn't changed.

[Om-Thorat]

@rithviknishad The tests should work now.

[nihal467]

1. In the test, navigate to a patient page and store the current URL of the page in a variable.
2. Log out and then log back in using the specific URL stored in the variable.

[nihal467]

the page is going into an infinite loop on the expired session, to replicate the bug do the following steps

1. click on the asset tab on the left sidebar
2. Now, open the inspect page of your browser and edit the access token in the application section by adding '12' to it. After that, click on the facility tab.
3. then you will see the application go into an infinite loop

@Om-Thorat @rithviknishad @Ashesh3

[Om-Thorat]

the page is going into an infinite loop on the expired session, to replicate the bug do the following steps

1. click on the asset tab on the left sidebar

2. Now, open the inspect page of your browser and edit the access token in the application section by adding '12' to it. After that, click on the facility tab.

3. then you will see the application go into an infinite loop

@Om-Thorat @rithviknishad @Ashesh3

Can replicate will try to figure it out.

[Om-Thorat]

export default function SessionExpired() {
  const { signOut, user } = useAuthContext();
  const isAuthenticated = Boolean(user);
  const navigate = useNavigate();
  const { t } = useTranslation();

  useEffect(() => {
    Notification.closeAllNotifications();
  }, []);

  if (isAuthenticated) {
    navigate("/");
  }

The bug seems to be due to the navigate("/") and i cannot understand why we need it? If the session has expired do we not wish to log the user out on the same page and show session expired with a return to login?

[Om-Thorat]

@rithviknishad @nihal467 @Ashesh3 The issue being session expired page navigates to "/" and then since the session has expired it loads the session expired page again anyway which again navigates to "/", If the token is invalid i don't understand the need for checking the IsAuthenthicated and returning a navigate.

[github-actions]

Hi, This pr has been automatically marked as stale because it has not had any recent activity. It will be automatically closed if no further activity occurs for 7 more days. Thank you for your contributions.

[Om-Thorat]

The bug also happens on the live version of care. https://care.ohc.network hence wasn't introduced by this pr.

@nihal467 @rithviknishad

[github-actions]

Hi, This pr has been automatically marked as stale because it has not had any recent activity. It will be automatically closed if no further activity occurs for 7 more days. Thank you for your contributions.

[github-actions]

Hi, @gigincg, @nihal467, @khavinshankar, @mathew-alex, This pr has been automatically closed because it has not had any recent activity. Thank you for your contributions. Feel free to repopen the pr.