feat(saml2): Redirect to SLO URL on the frontend #77036
Conversation
github-actions
bot
added
the
Scope: Frontend
Bundle ReportChanges will increase total bundle size by 90.22kB (0.3%) ⬆️. This is within the configured threshold ✅ Detailed changes
|
❌ 1 Tests Failed:
View the top 1 failed tests by shortest run time
To view individual test run time comparison to the main branch, go to the Test Analytics Dashboard |
| await userEvent.click(screen.getByTestId('existing-member-link')); | ||
|
|
||
| expect(logout).toHaveBeenCalled(); | ||
| await waitFor(() => expect(window.location.replace).toHaveBeenCalled()); |
There was a problem hiding this comment.
Removing this line because redirect was moved into logout function, which was mocked.
| @@ -294,9 +291,7 @@ describe('AcceptOrganizationInvite', function () { | |||
|
|
|||
| expect(screen.getByTestId('existing-member')).toBeInTheDocument(); | |||
| await userEvent.click(screen.getByTestId('existing-member-link')); | |||
|
|
|||
| expect(logout).toHaveBeenCalled(); | |||
| await waitFor(() => expect(window.location.replace).toHaveBeenCalled()); | |||
There was a problem hiding this comment.
Removing this line because redirect was moved into logout function, which was mocked.
leedongwei
force-pushed
the
dlee/slo-redirect-fe
branch
from
a261f18 to
de33fe8
Compare
leedongwei
requested review from
evanpurkhiser and
a team
and removed request for
a team
There was a problem hiding this comment.
lgtm
just a nit for the onClick={() = {}} functions in handlers -- that will be a little harder to debug if there's an issue (errors will just have anonymous) and the browsers js engine won't be able to create a hot cache of the anon functions for the react re-render cycles.
| const data = await api.requestPromise('/auth/', {method: 'DELETE'}); | ||
|
|
||
| // If there's a URL for SAML Single-logout, redirect back to IdP | ||
| window.location.assign(data?.sloUrl || redirectUrl); |
There was a problem hiding this comment.
should we use react-router here for consistency?
import { redirect } from "react-router-dom";
redirect(data?.sloUrl || redirectUrl))| const redirectURL = `${trimEnd(superuserUrl, '/')}${authLoginPath}`; | ||
| window.location.assign(redirectURL); | ||
| return; | ||
| nextUrl = `${trimEnd(superuserUrl, '/')}${nextUrl}`; |
There was a problem hiding this comment.
nit: might be safer to use the URL constructor rather than build a string for the url. here's a little blog on how to use the URL constructor and the differences between the two approaches: https://www.builder.io/blog/new-url
Related to #77033 If a SLO URL is returned by the logout API, we will redirect the browser to that URL. This is needed for Single-Logout to work, because we need the browser to clear the cookies that are attached to the IdP's domain.
Related to #77033
If a SLO URL is returned by the logout API, we will redirect the browser to that URL. This is needed for Single-Logout to work, because we need the browser to clear the cookies that are attached to the IdP's domain.