Skip to content

feat: Core authentication plugins#1180

Merged
mofojed merged 27 commits intodeephaven:mainfrom
mofojed:auth-plugins
Apr 20, 2023
Merged

feat: Core authentication plugins#1180
mofojed merged 27 commits intodeephaven:mainfrom
mofojed:auth-plugins

Conversation

@mofojed
Copy link
Copy Markdown
Member

@mofojed mofojed commented Mar 28, 2023

  • Add authentication plugins framework
  • Add core plugins (Pre-shared key, Parent, and anonymous authentication)
  • Fixes Support plugins for authentication #1058
  • Refactored app initialization so embed-grid and embed-chart share the same logic
    • Allows embed-grid/embed-chart to display a login screen if necessary
    • embed-grid/embed-chart now also authorize

@mofojed mofojed changed the title Core authentication plugins feat: Core authentication plugins Mar 28, 2023
@mofojed mofojed requested a review from niloc132 March 28, 2023 19:50
@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 28, 2023

Codecov Report

Merging #1180 (6882a2f) into main (2f9c020) will increase coverage by 0.52%.
The diff coverage is 53.11%.

@@            Coverage Diff             @@
##             main    #1180      +/-   ##
==========================================
+ Coverage   45.14%   45.67%   +0.52%     
==========================================
  Files         460      473      +13     
  Lines       33637    33703      +66     
  Branches     8462     8455       -7     
==========================================
+ Hits        15186    15394     +208     
+ Misses      18401    18258     -143     
- Partials       50       51       +1     
Flag Coverage Δ
unit 45.67% <53.11%> (+0.52%) ⬆️

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%> (ø)
packages/app-utils/src/plugins/loadRemoteModule.ts 100.00% <ø> (ø)
...s/app-utils/src/plugins/remote-component.config.ts 100.00% <ø> (ø)
packages/auth-plugins/src/AuthPlugin.ts 0.00% <0.00%> (ø)
packages/code-studio/src/AppRoot.tsx 0.00% <ø> (ø)
packages/code-studio/src/index.tsx 0.00% <0.00%> (ø)
packages/code-studio/src/main/AppInit.tsx 0.00% <0.00%> (ø)
packages/code-studio/src/main/AppMainContainer.tsx 34.84% <0.00%> (-0.13%) ⬇️
...ages/code-studio/src/styleguide/StyleGuideRoot.tsx 0.00% <ø> (ø)
... and 30 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Comment thread packages/code-studio/src/main/AppInit.tsx Outdated
Comment thread packages/code-studio/src/main/AppInit.tsx Outdated
@mofojed mofojed requested review from niloc132 and vbabich March 29, 2023 16:03
@mofojed mofojed self-assigned this Mar 29, 2023
@mofojed mofojed marked this pull request as ready for review March 29, 2023 16:03
mofojed added 6 commits March 29, 2023 14:53
- Load AuthPlugins from the server before logging in
- Find the appropriate auth plugin and login with it
- Add core plugins for Pre-shared Key, Parent, and Anonymous types of authentication
- Will create an example for authclock login in the deephaven-js-plugins repo
- Include an example in the auth-plugin/README
- Add tests for the core plugin types
- Cleanup some of the AppInit stuff
niloc132
niloc132 previously approved these changes Mar 31, 2023
Copy link
Copy Markdown
Member

@niloc132 niloc132 left a comment

Choose a reason for hiding this comment

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

Manual testing of PSK and anonymous login work great, also tested by creating my own auth wiring for keycloak and loading it through the plugins mechanism.

mofojed added 2 commits April 11, 2023 12:04
- Resolved conflicts in AppInit.tsx
- Moved requestParentLoginOptions call to AuthPluginParent
mofojed added 10 commits April 17, 2023 15:26
- Will be used by embed-grid and embed-chart as well
- Added types to @deephaven/plugin-utils
- Reference those types in @deephaven/redux
- Move SessionUtils into jsapi-utils package
- Move PluginUtils and some of the app loading stuff into app-utils package
- Will be needed for all the apps
- Now it should be easier for other apps to initialize (like the embedded apps)
- All unit tests pass
- Need to fix embed-grid and embed-chart apps
mofojed added 2 commits April 19, 2023 14:21
- Update readme
- Delete AppRootBootstrap (unused)
- Wasn't proxying the plugins URL
Comment thread packages/app-utils/src/plugins/RemoteComponent.ts
Comment thread packages/code-studio/src/serviceWorker.ts
- Rename 'utils.ts' to 'ConnectUtils.ts'
- Wrap styleguide in a FontBootstrap
@mofojed mofojed requested a review from mattrunyon April 19, 2023 19:16
Comment thread packages/app-utils/package.json
Comment thread packages/app-utils/src/components/useClient.ts
Comment thread packages/app-utils/src/components/usePlugins.ts Outdated
Comment thread packages/app-utils/src/components/AuthBootstrap.tsx
Comment thread packages/app-utils/src/components/AuthBootstrap.tsx
Comment thread packages/auth-core-plugins/src/AuthPluginPsk.tsx
Comment thread packages/auth-core-plugins/src/AuthPluginParent.tsx
Comment thread packages/auth-core-plugins/src/AuthPluginParent.test.tsx Outdated
Comment thread packages/code-studio/src/main/AppInit.tsx
Comment thread packages/jsapi-utils/src/SessionUtils.ts Outdated
Comment thread packages/app-utils/src/components/AppBootstrap.tsx
Comment thread packages/auth-plugins/src/AuthPlugin.ts Outdated
Comment thread packages/auth-plugins/src/AuthPlugin.ts Outdated
- Tests the full AppBootstrapping procedure
@mofojed mofojed requested a review from mattrunyon April 20, 2023 19:55
Copy link
Copy Markdown
Collaborator

@mattrunyon mattrunyon left a comment

Choose a reason for hiding this comment

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

Looking pretty good. There's a few unresolved conversations I commented on in this last round, so address those (just adding a comment or creating follow up tickets) and then this should be good to merge

Comment thread packages/app-utils/src/components/AppBootstrap.test.tsx
@mofojed mofojed requested a review from mattrunyon April 20, 2023 20:54
Copy link
Copy Markdown
Collaborator

@mattrunyon mattrunyon left a comment

Choose a reason for hiding this comment

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

Tested w/ psk and anonymous locally

We should maybe do a follow up to improve psk by adding a prompt if it's not in the URL. Just a little better user experience

@mofojed mofojed merged commit 1624309 into deephaven:main Apr 20, 2023
@mofojed mofojed deleted the auth-plugins branch April 20, 2023 21:19
@github-actions github-actions Bot locked and limited conversation to collaborators Apr 20, 2023
@mofojed
Copy link
Copy Markdown
Member Author

mofojed commented Apr 20, 2023

Yea I've had some discussions with @dsmmcken about that. I don't think we settled on exactly how we want to prompt for it. #1240

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support plugins for authentication

3 participants