Refactor Vue Router and Authentication infrastructure #41

jujaga · 2023-02-28T23:28:51Z

Description

This PR rewrites major chunks of the codebase to support using oidc-client-ts as our core authentication library instead of keycloak-js. It also contains a rewrite of the Vue Router layer so that it can properly support dynamic route entrypoints as a properly functioning SPA. Other numerous fixes have been included to help shore up the application, including a security audit and dependency update pass.

8b3af09 Routine npm dependency and RedHat image updates
fd4c9ba Change configmap environment values to support OIDC instead of KC
dd34c47 Fix remaining linting, testing and compilation issues
1511830 Implement componetized progress loader bar and integration to router
1fabe1e Add bcgov styling for header and navbar
8dfff8c Implement explicit oidc flow routes and views for soft redirection
e08a009 Attempt to perform vue-router navigation after /oidc/callback
2f11a23 Change vue-router to depend on authStore instead of authService
0a5df11 Update components to use new authStore methods
eaec877 Update base views and userStore to use new store getters
c9dd25f Switch axios service interceptor to leverage new store getter functions
a174e77 Drop generateGetters higher order function support
46699b5 Update authService to use a static userManager instance
6235b87 Remove suspense support from App.vue
4d2c896 Refactor main initializeApp to force a pinia configStore init
d92961c Rename ObjectFileDetailsView to DetailObjectsView
8614698 Consolidate vue router to handle oidc redirection and logouts
3d7ebc7 Update authStore and configStore to support state masking
b85f38f Implement higher order generateGetters function and supporting types
8ff6f68 Add npm debug script to frontend package
6fbb089 Add generic createProps transform function to router
6b6f6c4 Refactor core routing logic to utilize new authService
ba98368 Refactor main startup code to use new blocking initialization flows
6eca2e5 Implement new AuthService singleton service
bfbe322 Implement new ConfigService singleton service
9e9141a Add oidc-client-ts library dependency
471f278 Add dist folder to local ts-node-dev watchlist

Without these changes, we would not be able to proceed with public sharing and other general frontend application behavior.

SHOWCASE-3002

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)

Breaking change (fix or feature that would cause existing functionality to change)

Checklist

I have read the CONTRIBUTING doc
I have checked that unit tests pass locally with my changes
I have added tests that prove my fix is effective or that my feature works
I have added necessary documentation (if appropriate)

Further comments

Things will break. You will need to realign your local.json for things to work again. Do npm ci as nearly all dependencies have been updated.

Signed-off-by: Jeremy Ho <jujaga@gmail.com>

We are planning on swapping out authentication library systems from keycloak-js. Signed-off-by: Jeremy Ho <jujaga@gmail.com>

In order to ensure there are hooks to acquiring the configuration data, we wrap the action in a singleton service that can be explicitly initialized. This will be used upon application bootup and ensure that we do not call the /config endpoint more than necessary. Signed-off-by: Jeremy Ho <jujaga@gmail.com>

In order to provide a stable interface for general OIDC authentication operations, we wrap the common pathways of the oidc-client-ts library into a singleton service so that the rest of the application can invoke and interact with the new authentication system without worrying about the order of operations needed to bootstrap a user. Signed-off-by: Jeremy Ho <jujaga@gmail.com>

Part of the reason we ended up refactoring the config and auth layers is because we had no way to synchronously wait on the acquisition of config and user auth state. This change ensures that both config and auth are loaded before we mount the application. Signed-off-by: Jeremy Ho <jujaga@gmail.com>

This change focuses on two things: Using the new authentication system and ensuring that static redirection still occurs in production mode code. To achieve this, a full rewrite of the router interceptor lifecycles had to be done. Note that this implementation allows any route to handle the OIDC callback flow. This may or may not be best practices, as other examples frequently direct the OIDC provider to redirect back to a known hard path instead. We went this generic approach as we want to reasonably preserve the potential differences in path and redirections that will also already happen in tandem with on-load behavior. Signed-off-by: Jeremy Ho <jujaga@gmail.com>

This transformer function's sole purpose is to handle aggregation of any query params or path params and have them be exposed to the view for consumption. Signed-off-by: Jeremy Ho <jujaga@gmail.com>

This will be useful for development purposes where we need to flip on verbose debug logs. Signed-off-by: Jeremy Ho <jujaga@gmail.com>

Since we want to expose most pinia state as a gated read-only computed getter, we add in this utility function so that it can autowire all the functions needed without having to explicitly write a function for each state object. Signed-off-by: Jeremy Ho <jujaga@gmail.com>

Also completes implementation of authStore so that the store data is updated based on userManager events. Signed-off-by: Jeremy Ho <jujaga@gmail.com>

The changes to the vue-router are needed in order to make sure redirection and order of operations on login and mounting are done in the right order. Constants are also redone so that they are in all caps. Signed-off-by: Jeremy Ho <jujaga@gmail.com>

This rename was done in order to make the view more accurately reflect its purpose and where it is mounted in the application. Signed-off-by: Jeremy Ho <jujaga@gmail.com>

While the service singletons are up and running at this point, we want the stores to also be aware of the data changes and be able to react to async update events that show up in userManager. For this to happen, the init function must explicitly add event handlers to userManager before the app starts up. Signed-off-by: Jeremy Ho <jujaga@gmail.com>

While we want to have a way to support a loading visual, the logic that needs to wait is now done before the application is mounted. We will need to revisit this later with a different approach. Signed-off-by: Jeremy Ho <jujaga@gmail.com>

This change reduces instance duplication and makes sure the event handlers mounted in the store stay mounted to the same userManager. This change also adds debug support to conditionally expose the direct state elements in the setup function. Lastly, InitApp is removed as we now initialize earlier in the lifecycle before vue even gets mounted to the DOM. Signed-off-by: Jeremy Ho <jujaga@gmail.com>

While this HoF does work and autowires our state as intended, it is unable to pass typescript compilation checks because typescript expects the methods to be defined explicitly instead of being generated at runtime. Signed-off-by: Jeremy Ho <jujaga@gmail.com>

Signed-off-by: Jeremy Ho <jujaga@gmail.com>

Also bugfixes the requiredAuth meta flag not being picked up correctly Signed-off-by: Jeremy Ho <jujaga@gmail.com>

Signed-off-by: Jeremy Ho <jujaga@gmail.com>

There are fringe cases where an access token has expired and is not able to be refreshed. This is addressed by adding an accessToken expiration check to isAuthenticated state checker. Signed-off-by: Jeremy Ho <jujaga@gmail.com>

Unfortunately this doesn't appear to fully navigate the application. Left some commented breadcrumbs to revisit later. Also added some various fixes to ensure the application is nominally working on quick inspection. Will need to revisit how and where userStore actually needs to be loaded, but leaving it in top level of application to maintain functionality. Signed-off-by: Jeremy Ho <jujaga@gmail.com>

By shifting the redirection and storage logic from service layer up to component/view layer, we are able to take advantage of the vue context and navigate cleanly between pages for the login flow. Signed-off-by: Jeremy Ho <jujaga@gmail.com>

Values still may need to be tweaked, but were grafted from vue-scaffold. Also fixed the config version footer being broken. Signed-off-by: Jeremy Ho <jujaga@gmail.com>

This change wraps up the progress loader so that it is its own component. As well, general file location changes are applied so that layout related components are indexed and grouped together. Signed-off-by: Jeremy Ho <jujaga@gmail.com>

Signed-off-by: Jeremy Ho <jujaga@gmail.com>

Also removes keycloak-js dependency from frontend. Signed-off-by: Jeremy Ho <jujaga@gmail.com>

github-actions · 2023-02-28T23:47:50Z

Coverage Report (Frontend)

Totals
Statements:	5.45% ( 36 / 660 )
Methods:	0.7% ( 1 / 142 )
Lines:	7.67% ( 31 / 404 )
Branches:	3.51% ( 4 / 114 )

github-actions · 2023-02-28T23:48:30Z

Coverage Report (Application)

Totals
Statements:	75% ( 51 / 68 )
Methods:	62.5% ( 5 / 8 )
Lines:	82.61% ( 38 / 46 )
Branches:	57.14% ( 8 / 14 )

We discovered later on that this flag was actually preventing proper handling of the r query parameter. Since we are doing explicit oidc paths for the auth flow, we can safely remove this flag. Signed-off-by: Jeremy Ho <jujaga@gmail.com>

Typescript 4.9.x is not yet supported by a dependency so we are rolling back in the short term. To avoid circular dependency issues, the store index needs to be exporting default aliases for tree shaking purposes. Signed-off-by: Jeremy Ho <jujaga@gmail.com>

frontend/src/components/bucket/BucketsTable.vue

frontend/src/components/layout/ProgressLoader.vue

frontend/src/utils/constants.ts

TimCsaky · 2023-03-01T20:07:07Z

frontend/src/App.vue

+
+onBeforeMount(async () => {
+  appStore.beginDeterminateLoading();
+  await useAuthStore().init();


i thought useAuthStore().init(); was already called when you initializeApp() in main.ts

We might not necessarily need it here as in theory this should have either already been done in main.ts or done in children components; App.vue itself has no requirements to know about the authentication state; but the children components do.

Also removes unused types, changes store state types to be *StoreState, and changes the progressloader to use hyphenated case for show-value. Signed-off-by: Jeremy Ho <jujaga@gmail.com>

jujaga added 27 commits February 24, 2023 11:19

Add dist folder to local ts-node-dev watchlist

471f278

Signed-off-by: Jeremy Ho <jujaga@gmail.com>

Add oidc-client-ts library dependency

9e9141a

We are planning on swapping out authentication library systems from keycloak-js. Signed-off-by: Jeremy Ho <jujaga@gmail.com>

Add generic createProps transform function to router

6fbb089

This transformer function's sole purpose is to handle aggregation of any query params or path params and have them be exposed to the view for consumption. Signed-off-by: Jeremy Ho <jujaga@gmail.com>

Add npm debug script to frontend package

8ff6f68

This will be useful for development purposes where we need to flip on verbose debug logs. Signed-off-by: Jeremy Ho <jujaga@gmail.com>

Update authStore and configStore to support state masking

3d7ebc7

Also completes implementation of authStore so that the store data is updated based on userManager events. Signed-off-by: Jeremy Ho <jujaga@gmail.com>

Rename ObjectFileDetailsView to DetailObjectsView

d92961c

This rename was done in order to make the view more accurately reflect its purpose and where it is mounted in the application. Signed-off-by: Jeremy Ho <jujaga@gmail.com>

Remove suspense support from App.vue

6235b87

While we want to have a way to support a loading visual, the logic that needs to wait is now done before the application is mounted. We will need to revisit this later with a different approach. Signed-off-by: Jeremy Ho <jujaga@gmail.com>

Switch axios service interceptor to leverage new store getter functions

c9dd25f

Signed-off-by: Jeremy Ho <jujaga@gmail.com>

Update base views and userStore to use new store getters

eaec877

Also bugfixes the requiredAuth meta flag not being picked up correctly Signed-off-by: Jeremy Ho <jujaga@gmail.com>

Update components to use new authStore methods

0a5df11

Signed-off-by: Jeremy Ho <jujaga@gmail.com>

Add bcgov styling for header and navbar

1fabe1e

Values still may need to be tweaked, but were grafted from vue-scaffold. Also fixed the config version footer being broken. Signed-off-by: Jeremy Ho <jujaga@gmail.com>

Fix remaining linting, testing and compilation issues

dd34c47

Signed-off-by: Jeremy Ho <jujaga@gmail.com>

Change configmap environment values to support OIDC instead of KC

fd4c9ba

Signed-off-by: Jeremy Ho <jujaga@gmail.com>

Routine npm dependency and RedHat image updates

8b3af09

Also removes keycloak-js dependency from frontend. Signed-off-by: Jeremy Ho <jujaga@gmail.com>

jujaga added bug Something isn't working documentation Improvements or additions to documentation enhancement New feature or request labels Feb 28, 2023

jujaga requested a review from loneil February 28, 2023 23:28

jujaga self-assigned this Feb 28, 2023

jujaga requested review from kamorel, TimCsaky and bcgv123 as code owners February 28, 2023 23:28

jujaga added 2 commits February 28, 2023 17:06

jujaga mentioned this pull request Mar 1, 2023

File Details load object #40

Merged

4 tasks

kamorel reviewed Mar 1, 2023

View reviewed changes

frontend/src/components/bucket/BucketsTable.vue Outdated Show resolved Hide resolved

kamorel reviewed Mar 1, 2023

View reviewed changes

frontend/src/components/layout/ProgressLoader.vue Outdated Show resolved Hide resolved

kamorel reviewed Mar 1, 2023

View reviewed changes

frontend/src/utils/constants.ts Outdated Show resolved Hide resolved

kamorel approved these changes Mar 1, 2023

View reviewed changes

TimCsaky approved these changes Mar 1, 2023

View reviewed changes

TimCsaky reviewed Mar 1, 2023

View reviewed changes

Standardize constants to use capital snake_case format

d808948

Also removes unused types, changes store state types to be *StoreState, and changes the progressloader to use hyphenated case for show-value. Signed-off-by: Jeremy Ho <jujaga@gmail.com>

TimCsaky merged commit d4a8293 into master Mar 1, 2023

jujaga deleted the feature/oidcauth branch March 1, 2023 20:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor Vue Router and Authentication infrastructure #41

Refactor Vue Router and Authentication infrastructure #41

jujaga commented Feb 28, 2023

github-actions bot commented Feb 28, 2023

github-actions bot commented Feb 28, 2023

TimCsaky Mar 1, 2023

jujaga Mar 1, 2023

Refactor Vue Router and Authentication infrastructure #41

Refactor Vue Router and Authentication infrastructure #41

Conversation

jujaga commented Feb 28, 2023

Description

Types of changes

Checklist

Further comments

github-actions bot commented Feb 28, 2023

Coverage Report (Frontend)

github-actions bot commented Feb 28, 2023

Coverage Report (Application)

TimCsaky Mar 1, 2023

Choose a reason for hiding this comment

jujaga Mar 1, 2023

Choose a reason for hiding this comment