Intergrate Authorisation into embedded ui #311

richard-cox · 2023-08-18T15:35:30Z

Summary

Fixes: #358

Occurred changes and/or fixed issues

Previously the username & password were hardcoded for demo purposes
Now the user is shown a login modal when going to a cluster
The user can choose to log in via the local user (username / password) or auth provider (aka dex)
Requests made from within that cluster now use the credentials / tokens from that login process
If the user wants to log out they can choose to via the epinio cluster list and the cluster's actions
- There should be an option to log out from within the epinio context (RC to create issue)
If the user has logged in via local user, they will need to do so after every refresh / visit
- We have an idea on how to resolve this, which would also resolve CORS issues
- See [ui] Investigate, if applicable, persist local user credentials in embedded ui #360
If the user has logged in via dex, they will NOT need to log in again until they either log out or their token expires
- There is a way to refresh the token silently in the background (RC to create issue)
Discussion Point - The login modal doesn't really look / behave too great. Split out to [ui] Improve embedded ui log in UX #359

Technical notes summary

For this to work we also need to update the Dex config (see dependent PR epinio/helm-charts#504)

web: <-- this needs adding
  allowedOrigins: ['*'] <--- needs to be the rancher url

staticClients
- id: epinio-apo
  trustedPeers: 
    - rancher-dashboard <-- needs adding

- id: rancher-dashboard <-- this whole section needs adding
  name: 'Rancher Dashboard'
  public: true
  redirectURIs:
  - "https://<rancher url>/epinio/auth/verify/" <--- needs to be the rancher url

Depends on

Areas or cases that should be tested

Refresh on an epinio cluster when logged in via local user. User should be redirect to epinio cluster list
Refresh on an epinio cluster when logged in via a dex user. User should see the same page
Functionality when there are two clusters that contain epinio. In theory user should be able to switch between them both fine

- can login via dex. - need to wire in auth to

- requires new backend

torchiaf

I've done some tests as first round of the review.

The login for local users works fine but there is something wrong with RedirectToError class from shell which is causing a crash during the logout.
Minor issue:
The link in the 'URL' column to get the Epinio UI certificate in the instances table is broken.

it redirects to https://localhost:8005/api/v1/info

torchiaf · 2023-11-03T09:15:56Z

dashboard/pkg/epinio/store/epinio-store/actions.ts

 import {
  EpinioInfo, EpinioVersion, EPINIO_MGMT_STORE, EPINIO_PRODUCT_NAME, EPINIO_STANDALONE_CLUSTER_NAME, EPINIO_TYPES
 } from '../../types';
+import EpinioCluster from '../../models/cluster';
+import { RedirectToError } from '@shell/utils/error';


RedirectToError is not exported by @shell/utils/error. Do we need to update the shell dependency maybe ?

This is causing an error during logout:

Loading error Error: _shell_utils_error__WEBPACK_IMPORTED_MODULE_30__.RedirectToError is not a constructor at Store.setError (index.js:654:1) at wrappedMutationHandler (vuex.esm.js:844:1) at commitIterator (vuex.esm.js:466:1) at Array.forEach (<anonymous>) at vuex.esm.js:465:1 at Store._withCommit (vuex.esm.js:624:1) at Store.commit (vuex.esm.js:464:1) at Store.boundCommit [as commit] (vuex.esm.js:409:1) at _callee$ (authenticated.js:509:1) at tryCatch (regeneratorRuntime.js:44:1)

That was rancher/dashboard#9941, which was blocked on branching. If you yarn link to master the error should resolve itself.

Now that we've forked it has merged, so we can do another shell release

i've now published a new shell and referenced that from the extension

richard-cox · 2023-11-03T10:09:46Z

Thanks @torchiaf . I'll do the bump to shell and fix the url

torchiaf

Tested the Dex login as well + a couple of comments.

torchiaf · 2023-11-03T15:21:59Z

dashboard/pkg/epinio/dialog/LoginDialog.vue

+  },
+
+  computed: {
+    cluster() {


Is there a specific reason to define cluster as computed value ?

Just for readability. This makes it clearer that it's a cluster, instead of an odd first entry in an array

torchiaf · 2023-11-03T15:22:27Z

dashboard/pkg/epinio/pages/auth/verify.vue

+
+  async fetch({ store, route }: { store: any, route: any}) {
+    const {
+      error, error_description: errorDescription, errorCode, errorMsg


error_description should be removed:

const { error, errorDescription, errorCode, errorMsg } = route.query;

I've removed errorCode and errorMsg, as these weren't used by dex or the dep to deal with oidc. error_description is a valid param though

- can be removed once rancher/dashboard#10035 is merged and published

richard-cox · 2023-11-13T15:39:54Z

@torchiaf all comments should be addressed now. i've published a new verison of shell and references that as well. So no need now to yarn link @rancher/shell. I added a workaround for a dep missing from there, which can be removed in the future.

torchiaf · 2023-11-14T09:05:22Z

@torchiaf all comments should be addressed now. i've published a new verison of shell and references that as well. So no need now to yarn link @rancher/shell. I added a workaround for a dep missing from there, which can be removed in the future.

Working on the review, thanks!

torchiaf

LGTM

richard-cox added this to the v1.11.0 milestone Aug 18, 2023

richard-cox self-assigned this Aug 18, 2023

richard-cox added 10 commits October 16, 2023 13:54

WIP

f250bf0

- can login via dex. - need to wire in auth to

use user in auth requests to epinio, start tidying up flow

6e78815

fix a number of things

99396b1

support local auth as well

c7bf1c4

started removing todos

d61d725

more todos

672f753

tidying up

c88b714

apply new info / me changes

ae82067

Fix build epinio pkg

a6bc810

Fix initialisation of cluster model & re-enter once locally logged in

80f37fb

richard-cox force-pushed the embedded-auth-dex branch from 7cf16ed to 80f37fb Compare October 17, 2023 14:50

richard-cox added 2 commits October 17, 2023 15:59

Fix lint, changes after self review

2d6100a

Fix redirect, remove useless pagination stuff

c3bef11

- requires new backend

richard-cox mentioned this pull request Oct 18, 2023

Optionally supplement Dex configuration with information required for embedded ui auth epinio/helm-charts#504

Merged

richard-cox marked this pull request as ready for review October 18, 2023 13:36

richard-cox requested a review from torchiaf October 18, 2023 14:46

torchiaf requested changes Nov 3, 2023

View reviewed changes

richard-cox added 4 commits November 13, 2023 13:47

Fix epinio#361

1bea25a

Remove unused query params from verify

2473e74

bump shell

d5b3da1

Temporarily add typing for is-url

a3c3937

- can be removed once rancher/dashboard#10035 is merged and published

richard-cox requested a review from torchiaf November 13, 2023 15:38

torchiaf approved these changes Nov 14, 2023

View reviewed changes

richard-cox merged commit 91f6d0b into epinio:main Nov 14, 2023
5 checks passed

richard-cox deleted the embedded-auth-dex branch November 14, 2023 10:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Intergrate Authorisation into embedded ui #311

Intergrate Authorisation into embedded ui #311

richard-cox commented Aug 18, 2023 •

edited

Loading

torchiaf left a comment •

edited

Loading

torchiaf Nov 3, 2023

richard-cox Nov 3, 2023

richard-cox Nov 13, 2023

richard-cox commented Nov 3, 2023

torchiaf left a comment •

edited

Loading

torchiaf Nov 3, 2023

richard-cox Nov 13, 2023

torchiaf Nov 3, 2023 •

edited

Loading

richard-cox Nov 13, 2023

richard-cox commented Nov 13, 2023

torchiaf commented Nov 14, 2023

torchiaf left a comment

Intergrate Authorisation into embedded ui #311

Intergrate Authorisation into embedded ui #311

Conversation

richard-cox commented Aug 18, 2023 • edited Loading

Summary

Occurred changes and/or fixed issues

Technical notes summary

Areas or cases that should be tested

torchiaf left a comment • edited Loading

Choose a reason for hiding this comment

torchiaf Nov 3, 2023

Choose a reason for hiding this comment

richard-cox Nov 3, 2023

Choose a reason for hiding this comment

richard-cox Nov 13, 2023

Choose a reason for hiding this comment

richard-cox commented Nov 3, 2023

torchiaf left a comment • edited Loading

Choose a reason for hiding this comment

torchiaf Nov 3, 2023

Choose a reason for hiding this comment

richard-cox Nov 13, 2023

Choose a reason for hiding this comment

torchiaf Nov 3, 2023 • edited Loading

Choose a reason for hiding this comment

richard-cox Nov 13, 2023

Choose a reason for hiding this comment

richard-cox commented Nov 13, 2023

torchiaf commented Nov 14, 2023

torchiaf left a comment

Choose a reason for hiding this comment

richard-cox commented Aug 18, 2023 •

edited

Loading

torchiaf left a comment •

edited

Loading

torchiaf left a comment •

edited

Loading

torchiaf Nov 3, 2023 •

edited

Loading