Skip to content

fix: Allow config of WebOrigins & RedirectUris#4444

Merged
sambuc merged 3 commits intomasterfrom
fix-add-web-origins-and-redirect-uris-config
Apr 30, 2026
Merged

fix: Allow config of WebOrigins & RedirectUris#4444
sambuc merged 3 commits intomasterfrom
fix-add-web-origins-and-redirect-uris-config

Conversation

@sambuc
Copy link
Copy Markdown
Contributor

@sambuc sambuc commented Apr 27, 2026

/deploy extra-values=notebooks.oidc.extraRedirectUris=["https://domainA.example.org/*","https://domainB.example.org/*"],notebooks.oidc.extraWebOrigins=["https://domainC.example.org/*"]

@RenkuBot
Copy link
Copy Markdown
Collaborator

You can access the deployment of this PR at https://ci-renku-4444.dev.renku.ch

@sambuc sambuc marked this pull request as ready for review April 27, 2026 15:45
@sambuc sambuc requested review from a team as code owners April 27, 2026 15:45
Copy link
Copy Markdown
Member

@leafty leafty left a comment

Choose a reason for hiding this comment

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

Why is this not added as new parameters to the cluster object? This way we would keep security features tight and not need to weaken security.

@sambuc
Copy link
Copy Markdown
Contributor Author

sambuc commented Apr 28, 2026

This is set on the keycloak client, and used by keycloak to valid the authentication request.

The keycloak init-realm scripts (which reset the keycloak client in case of deviation with the expected parameters) are called every time there is a helm update/upgrade.

I went for the quickest and smallest fix as this is causing problems for users in prod right now every time we deploy a release. I also did not want to rewrite the whole process for the keyclaok configuration management on something which is more of a bug fix.

Adapting the current init script should be part of pitch about keycloak and its configuration management in Renku, IMHO. To make it retrieve the config from the PostgreSQL DB, introduces a dependency on the DB to be up and running before you can initialise keycloak. That new / adapted script would have to be self standing to not introduce chicken-and-egg problems.

I also believe we should be a bit more mindful of the runtime inter-service dependencies and make sure they stay an acyclic graph. We could even improve deployment speed by ensuring we have proper depends_on-like link between the services to avoid restarts with exponential backoffs.

@leafty
Copy link
Copy Markdown
Member

leafty commented Apr 28, 2026

I went for the quickest and smallest fix as this is causing problems for users in prod right now every time we deploy a release.

What is that issue? Do we have a GitHub issue? Or any other documented logs?

@sambuc
Copy link
Copy Markdown
Contributor Author

sambuc commented Apr 28, 2026

We had the issue on slack, literally yesterday. This is on a project channel, you may not have had access to it. @olevski is also aware of it.

Copy link
Copy Markdown
Member

@leafty leafty left a comment

Choose a reason for hiding this comment

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

Config change looks OK, though using these new values must be done with caution: all the URIs listed (especially if they contain wildcards) must be trusted.

@sambuc sambuc merged commit aee22c4 into master Apr 30, 2026
20 checks passed
@sambuc sambuc deleted the fix-add-web-origins-and-redirect-uris-config branch April 30, 2026 09:01
@RenkuBot
Copy link
Copy Markdown
Collaborator

Tearing down the temporary RenkuLab deployment for this PR.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants