ID-1113, ID-1115 Separate Logout Code and Redirect on Logout

[tlangs]

Jira Ticket: https://broadworkbench.atlassian.net/browse/ID-1114

Summary of changes:

Pulling logout code out of auth.ts and into its own file. This allows for much easier testing, as we don't have to manage all the subscriptions defined in auth.ts.

Splitting signOut into two functions is the start of a change where we hit B2C during sign out. Before we hit B2C, we might have a valid token. Afterwards, the token will be invalid.

signOut now also redirects to the B2C end_session_endpoint (well, actually Orch's new logout endpoint), which clears server-side HTTP Only cookies. The user is then redirected back to Terra UI at /#logout, where post-logout code is executed.

I was able to completely unit test this functionality!

What

• Split out signOut code and unit test it. Get code ready for calling out to B2C on logout.

Why

• We need to tell B2C when a session ends so that it can clear HTTP-only cookies that have caused users to be unable to log in.

Testing strategy

• Unit Tests!
• Testing locally
• Testing in a BEE

[nawatts]

jest.spyOn creates and returns a mock, so there's no need to mock it's implementation with a second mock.

Instead, it could be

jest.spyOn(console, 'error').mockImplementation(() => {});
...
expect(console.error).toHaveBeenCalledWith(...);

or

const consoleErrorFn = jest.spyOn(console, 'error').mockImplementation(() => {});
...
expect(consoleErrorFn).toHaveBeenCalledWith(...);

[tlangs]

Ok, I'll admit it. Copilot wrote this one 😅

[nawatts]

As long as we're here, since this has caused problems before, would it be worth adding a comment that sendSignOutMetrics should not be awaited, since it will fail if the sign out is due to session expiration?

[nawatts]

Nit: convention

Suggested change

	export const Disabled: React.FC = () => {
	export const Disabled = (): ReactNode => {

[nawatts]

Side effects should go in effect hooks. Also, since we're only interested in the side effect, this doesn't need to render an element.

Suggested change

	export const Logout = () => {
	try {
	userSignedOut();
	} catch (e) {
	console.error(e);
	}
	Nav.goToPath('root');
	return <div />;
	export const Logout = (): ReactNode => {
	useEffect(() => {
	try {
	userSignedOut();
	} catch (e) {
	console.error(e);
	}
	Nav.goToPath('root');
	}, []);
	return null;

[tlangs]

Now that Orch is deployed to dev, I pointed my local branch to Dev. It works!

[tlangs]

I'll wait for Orch to go to prod tomorrow before merging this.

[samanehsan]

Should we create a ticket for this?

[tlangs]

Ticket created! https://broadworkbench.atlassian.net/browse/ID-1172

[samanehsan]

I thought we needed to pass in the policy as a query param, like the token endpoint does:

terra-ui/src/auth/oidc-broker.ts

Line 82 in 95d7fab

extraTokenParams: { p: getConfig().b2cBillingPolicy },

. Is that not required for signing out? 🤔

[tlangs]

@samanehsan That policy is specific for billing. We need some additional scopes to get GCP Billing info, so we make them sign in again with a different policy. Its unused to normal sign-in/sign-out.

Orchestration inserts the policy when it returns a redirect to Terra-UI, so for sign-in/sign-out, the UI doesn't need to do anything.

[sonarcloud]

Quality Gate passed

Issues
4 New issues
0 Accepted issues

Measures
0 Security Hotspots
84.3% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud