feat: Logging out by mofojed · Pull Request #1244 · deephaven/web-client-ui

mofojed · 2023-04-24T17:14:43Z

Write auth plugin components as wrappers
- Allows plugins to always be mounted and control the transition to loading the app
- Needed to ensure keycloak plugin can wipe token on logout
Refactor auth plugins to just use AuthPluginBase
- There was a lot of the same code, just refactored it into one common base
- Also added a useContextOrThrow hook
Added login screen for AuthPluginPsk
- Took the LoginAnimation from Enterprise, renamed to RandomAreaPlotAnimation
- Added a Login and LoginForm components to allow others to reuse if desired
- Also stores the pre-shared key as a cookie after successful login
Add broadcast channel for notifying other tabs about login/logout
- Notifies after successful login, or when logout button is pressed
Added permission for canLogout (since you can't logout when anonymous
Add context providers for overriding user info and permissions (used by auth plugins)

mofojed · 2023-04-27T15:58:40Z

Getting an error when trying to import from auth-keycloak:

@deephaven/js-plugin-auth-keycloak: build started...
@deephaven/js-plugin-auth-keycloak: ✓ 6190 modules transformed.
@deephaven/js-plugin-auth-keycloak: Could not resolve "./Login.css" from "../../node_modules/@deephaven/auth-plugins/dist/Login.js"
@deephaven/js-plugin-auth-keycloak: file: /home/bender/dev/deephaven/deephaven-js-plugins/node_modules/@deephaven/auth-plugins/dist/Login.js

codecov · 2023-04-27T16:16:14Z

Codecov Report

Merging #1244 (9d018fd) into main (0e286bd) will decrease coverage by 0.12%.
The diff coverage is 45.64%.

@@            Coverage Diff             @@
##             main    #1244      +/-   ##
==========================================
- Coverage   45.69%   45.57%   -0.12%     
==========================================
  Files         473      484      +11     
  Lines       33713    34077     +364     
  Branches     8457     8532      +75     
==========================================
+ Hits        15404    15531     +127     
- Misses      18258    18495     +237     
  Partials       51       51

Flag	Coverage Δ
unit	`45.57% <45.64%> (-0.12%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
packages/app-utils/src/components/useConnection.ts	`0.00% <0.00%> (ø)`
packages/app-utils/src/components/usePlugins.ts	`0.00% <0.00%> (ø)`
...s/app-utils/src/plugins/remote-component.config.ts	`100.00% <ø> (ø)`
packages/auth-plugins/src/AuthPlugin.ts	`0.00% <ø> (ø)`
packages/code-studio/src/main/AppInit.tsx	`0.00% <0.00%> (ø)`
packages/code-studio/src/settings/SettingsMenu.tsx	`7.93% <0.00%> (-3.18%)`	⬇️
packages/components/src/LoadingOverlay.tsx	`0.00% <0.00%> (ø)`
...ges/jsapi-components/src/RefreshTokenBootstrap.tsx	`0.00% <0.00%> (ø)`
packages/jsapi-components/src/RefreshTokenUtils.ts	`0.00% <0.00%> (ø)`
.../jsapi-components/src/useInitializeViewportData.ts	`88.88% <ø> (ø)`
... and 26 more

... and 1 file with indirect coverage changes

mattrunyon · 2023-04-28T23:46:25Z

+    'react-transition-group': path.join(
+      __dirname,
+      './__mocks__/react-transition-group.js'
+    ),


Hmm I'm not sure this should be needed based on the Jest docs. They say mocking node modules from __mocks__ adjacent to node_modules is an automatic mock here. This is different from mocking user modules

It might be the way we have jest set up. Honestly, I'm not sure the jest projects is all that helpful even though we're running from the root. I'd be interested to see how often we use it as developers for more than disabling the linter tests. I think that I set it up that way b/c some packages had different Jest setup requirements, but now I think it's basically code-studio and then every other package shares a config basically

Yea if we don't have this here the mock doesn't get applied and all the tests fail. Guess it's how we have jest set up.
The automatic mocking happens for __mocks__ folders next to the node_modules folders of our packages, but not at the root it would appear.
I'm not too invested in figuring it out beyond that, perhaps we switch to vitest at some point.

- Just added what was already part of app-utils

- Add tests for AuthPluginBase - Add mock for react-transition-gruop

- Just cleaning up the ternary op

- Probably not a problem, as we write the cookie, but on the outside chance someone modified their cookies should handle it safely - Also refactored deleting the refresh token a bit

- Seems to work fine, not sure what the previous comment was complaining about

- Is imported by the PSK plugin, now it appears correctly.

- Deleted package-lock.json and then did an `npm install`

- Needed to pin a couple versions

- Just copied the original from main and then ran an npm install instead of deleting and re-generating

- Was causing some tests to fail.

mattrunyon · 2023-05-01T20:56:48Z

@@ -1,5 +1,6 @@
 import React from 'react';
 import { RandomAreaPlotAnimation } from '@deephaven/components';
+import Logo from './logo.png';


I guess in DHC we won't allow changing the logo? Or we just recommend a custom auth plugin for that?

I know it's something we offer in DHE by just replacing the logo file.

It is something we do with DHE, which has some server side configuration: https://github.com/deephaven-ent/iris/blob/d6950c95398cbe9a400eca8ba90e11201dcdae7c/web/server-frontend/src/main/java/com/illumon/iris/web/server/AbstractIrisWebServer.java#L170
We don't have that same configuration on the DHC side.

That being said, I'll wire it up the same on the client. Just means the logo is in 3 public paths (ide, embed-grid, embed-chart) instead of just being packaged with auth plugins.

And yes, a custom auth plugin would be able to customize whatever.

- Add a prop so you can set it

mattrunyon

Looks good. Just confirming we intentionally remove the PSK from the URL after PSK auth is complete? Still refreshes fine, just making sure that's intentional since I thought that before this change the key stayed in the URL

mofojed · 2023-05-02T17:01:53Z

Correct. We actually remove it from the URL right away. It is stored as a cookie so refresh will work correctly.

mofojed commented Apr 27, 2023

View reviewed changes

Comment thread packages/components/src/RandomAreaPlotAnimation.tsx

mofojed requested a review from mattrunyon April 27, 2023 15:18

mofojed self-assigned this Apr 27, 2023

mofojed marked this pull request as ready for review April 27, 2023 15:19

mofojed force-pushed the 1239-logging-out branch from df14a9d to 8e15103 Compare April 27, 2023 17:19

mattrunyon requested changes May 1, 2023

View reviewed changes

mofojed added 13 commits May 1, 2023 08:56

Rebase off the latest

748f088

Add '@deephaven/*' packages to remote component resolution

997ded6

- Just added what was already part of app-utils

Add some more unit tests

1c3e9a0

- Add tests for AuthPluginBase - Add mock for react-transition-gruop

Update the react-transition-group mock

e254df0

- Just cleaning up the ternary op

Remove channel from RefreshTokenBootstrap

3856df9

Default to unknown in MessageUtils

24cddce

Fix up comments in useContextOrThrow

4e596be

Clean up getLoginOptions types to not require a promise

d167f82

Clean up how we delete the window token

58e2ffc

Remove TODO, filed a ticket for it

6a8fbec

Wrap parsing the refresh token with a try/catch

fc7dfe3

- Probably not a problem, as we write the cookie, but on the outside chance someone modified their cookies should handle it safely - Also refactored deleting the refresh token a bit

Use addEventListener in the BroadcastChannel

c2dcfe5

- Seems to work fine, not sure what the previous comment was complaining about

Add the logo to the auth plugins directly

466ffc9

- Is imported by the PSK plugin, now it appears correctly.

mofojed force-pushed the 1239-logging-out branch from 6330d6e to 466ffc9 Compare May 1, 2023 15:08

mofojed added 8 commits May 1, 2023 11:16

Fix up package-lock.json

d225cea

- Deleted package-lock.json and then did an `npm install`

Don't show user info for anonymous users

7d84daa

Fix up types after package-lock.json updates

952c2ae

- Needed to pin a couple versions

Show the logout button in Pre-shared key case still

a5bc3af

Actually fix the package-lock.json

585d109

- Just copied the original from main and then ran an npm install instead of deleting and re-generating

Move react-transition-group out from root __mocks__ folder

1f464da

- Was causing some tests to fail.

Move vite/client to vite-env.d.ts

7a92226

Add declarations.d.ts file to allow importing png

ae439be

mofojed requested a review from mattrunyon May 1, 2023 18:22

mattrunyon requested changes May 1, 2023

View reviewed changes

mofojed added 2 commits May 1, 2023 18:27

Fix has we're using requestParentResponse to only pass back unknown

352a1cf

Use the public URL logo instead

9d018fd

- Add a prop so you can set it

mofojed requested a review from mattrunyon May 1, 2023 22:44

mattrunyon approved these changes May 2, 2023

View reviewed changes

mofojed merged commit 769d753 into deephaven:main May 2, 2023

mofojed deleted the 1239-logging-out branch May 2, 2023 18:08

github-actions Bot locked and limited conversation to collaborators May 2, 2023

This was linked to issues May 4, 2023

Add support for logging out #1239

Closed

Permit restoring an existing session after browser refresh #1141

Closed

Prompt for pre-shared key if the one provided does not work #1240

Closed

Conversation

mofojed commented Apr 24, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

mofojed commented Apr 27, 2023

Uh oh!

codecov Bot commented Apr 27, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

mattrunyon Apr 28, 2023

Choose a reason for hiding this comment

Uh oh!

mofojed May 1, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mattrunyon May 1, 2023

Choose a reason for hiding this comment

Uh oh!

mofojed May 1, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mattrunyon left a comment

Choose a reason for hiding this comment

Uh oh!

mofojed commented May 2, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mofojed commented Apr 24, 2023 •

edited

Loading

codecov Bot commented Apr 27, 2023 •

edited

Loading