Skip to content

Conversation

@dr-bizz
Copy link
Contributor

@dr-bizz dr-bizz commented Jan 9, 2025

Description

This PR fixes the error where users find themselves in an endless loop on the login screen. When you are logged out after a certain amount of time, you are redirected to the homepage with the redirect of /logout, which, once you log in, you will be automatically logged out.

This PR prevents the redirect for the logout page from being added to the login redirect.

Checklist:

  • I have given my PR a title with the format "MPDX-(JIRA#) (summary sentence max 80 chars)"
  • I have applied the appropriate labels. (Add the label "On Staging" to get the branch automatically merged into staging.)
  • I have requested a review from another person on the project

@dr-bizz dr-bizz changed the title Prevent MPDX from redirecting to the logout page and causing a endles… Fix login loop from logout redirect Jan 9, 2025
@dr-bizz dr-bizz requested a review from canac January 9, 2025 19:22
@dr-bizz dr-bizz added the On Staging Will be merged to the staging branch by Github Actions label Jan 9, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Jan 9, 2025

Bundle sizes [mpdx-react]

Compared against ff0441a

No significant changes found

Copy link
Contributor

@canac canac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code looks good to break the redirect loop, but when I click the sign out button, it takes me to an Okta login page. Is it doing that for you?

@canac
Copy link
Contributor

canac commented Jan 9, 2025

I think you also need to change all the signOut({ callbackUrl: 'signOut' }) calls to signOut(). When I do that it takes me to the login page with the page I was on in the redirect param, which is helpful (i.e. http://localhost:3000/login?redirect=%2FaccountLists%2F6bbc265f-ec81-4b23-b297-5bd298c4d619.

@dr-bizz dr-bizz added the Preview Environment Add this label to create an Amplify Preview label Jan 10, 2025
@github-actions
Copy link
Contributor

Preview branch generated at https://fix-logout-redirect.d3dytjb8adxkk5.amplifyapp.com

@dr-bizz
Copy link
Contributor Author

dr-bizz commented Jan 10, 2025

The code looks good to break the redirect loop, but when I click the sign-out button, it takes me to an Okta login page. Is it doing that for you?

It only takes you to the Okta sign-in page on localhost; on staging, it works as expected.
On local, mine has always taken me to the Okta page on signout.

@dr-bizz
Copy link
Contributor Author

dr-bizz commented Jan 10, 2025

I think you also need to change all the signOut({ callbackUrl: 'signOut' }) calls to signOut(). When I do that it takes me to the login page with the page I was on in the redirect param, which is helpful (i.e. http://localhost:3000/login?redirect=%2FaccountLists%2F6bbc265f-ec81-4b23-b297-5bd298c4d619.

We need to keep the {callbackUrl: 'signOut'} on the signOut() function as this is how we know we should redirect it to Okta to sign out of all sessions.

src/components/Reports/HealthIndicatorReport/HealthIndicatorReport.tsx Line 270 - 283

redirect({ url, baseUrl }) {
        if (url.startsWith(baseUrl)) {
          return url;
        }
        if (url === 'signOut' && AUTH_PROVIDER === 'OKTA') {
          return `https://signon.okta.com/login/signout?fromURI=${encodeURIComponent(
            process.env.OKTA_SIGNOUT_REDIRECT_URL ?? '',
          )}`;
        }
        if (url.startsWith('/')) {
          return new URL(url, baseUrl).toString();
        }
        return baseUrl;
      },
      ```

@dr-bizz dr-bizz requested a review from canac January 10, 2025 13:42
Copy link
Contributor

@canac canac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It only takes you to the Okta sign-in page on localhost; on staging, it works as expected.
On local, mine has always taken me to the Okta page on signout.

It would be nice to know why localhost behaves differently, but that's not urgent.

We need to keep the {callbackUrl: 'signOut'} on the signOut() function as this is how we know we should redirect it to Okta to sign out of all sessions.

Thanks, I missed that part of the code and thought it was just an invalid URL.

@dr-bizz
Copy link
Contributor Author

dr-bizz commented Jan 10, 2025

I believe the reason is that Okta doesn't know how to redirect back to localhost, but I could be wrong.

We run the same code, but Okta doesn't redirect us back to MPDX on local.

return `https://signon.okta.com/login/signout?fromURI=${encodeURIComponent(
   process.env.OKTA_SIGNOUT_REDIRECT_URL ?? '',
)}`;

@dr-bizz dr-bizz merged commit a4d5410 into main Jan 10, 2025
22 checks passed
@dr-bizz dr-bizz deleted the fix-logout-redirect branch January 10, 2025 17:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

On Staging Will be merged to the staging branch by Github Actions Preview Environment Add this label to create an Amplify Preview

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants