allow onyxia to inject OIDC token and refreshtoken in helm chart only on personal project #430
Conversation
Co-authored-by: Joseph Garrone <joseph.garrone.gj@gmail.com>
…/alexisdondon/onyxia-web into alexisdondon-specific_configuration_injection
Bumps [deep-object-diff](https://github.com/mattphillips/deep-object-diff) from 1.1.7 to 1.1.9. - [Release notes](https://github.com/mattphillips/deep-object-diff/releases) - [Commits](https://github.com/mattphillips/deep-object-diff/commits) --- updated-dependencies: - dependency-name: deep-object-diff dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com>
…yarn/deep-object-diff-1.1.9 Bump deep-object-diff from 1.1.7 to 1.1.9
|
Discuted with Frederic. We could auhtoriez the ui to inject jwt and refreshtoken to be used in the helm chart but only on personal project to mitigate security issue in group namespaces. |
alexisdondon
changed the title
alexisdondon
changed the title
|
what's needed on this to be accepted? |
| "oidc": { | ||
| "enabled": project.group ? false : true, | ||
| "accessToken": project.group ? undefined : oidcClient.accessToken, | ||
| "refreshToken": project.group ? undefined : oidcClient.refreshToken |
There was a problem hiding this comment.
The type system should be more transparent here.
project.group should be of type string | undefined.
Also we introduce the notion of group for only using it to tell if it's an empty string or not.
We should instead have a project.isPersonal: boolean.
Also, until now we assumed that the first project was the personal project. So we should update the part of the code where we made this assemption (serach for projects[0])
src/core/adapters/phonyOidcClient.ts
Outdated
| @@ -51,7 +51,7 @@ export function createPhonyOidcClient(params: { | |||
| "xxx" | |||
| ); | |||
|
|
|||
| return { accessToken }; | |||
| return { accessToken, refreshToken: "phonyToken" }; | |||
There was a problem hiding this comment.
"refreshToken": "phonyToken"
| @@ -701,6 +701,7 @@ export function createOfficialOnyxiaApiClient(params: { | |||
| projects: { | |||
| id: string; | |||
| name: string; | |||
| group: string; | |||
There was a problem hiding this comment.
Are we sure of that?
Is it not group?: string ?
Seems more likely.
| @@ -72,6 +72,7 @@ export async function createKeycloakOidcClient(params: { | |||
| const oidcClient = id<OidcClient.LoggedIn>({ | |||
| "isUserLoggedIn": true, | |||
| "accessToken": keycloakInstance.token!, | |||
| "refreshToken": keycloakInstance.refreshToken!, | |||
There was a problem hiding this comment.
I'm pretty sure that when the accessToken is refreshed, the refreshToken changes as well.
Also, you should pull, this have been updated. It's now getToken()
|
Hey @alexisdondon, Thank you for your work. Here is my review, sorry for the delay. |
|
UP? |
That could be used to propagate identity of user in helm charts
should fix #410