Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: enable OIDC integration into preview #4440

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

DickPostma
Copy link
Contributor

What are the main changes you did:

  • Add OIDC integration

how to test:

  • Add user to Keycloak and login via OIDC on the preview

todo:

  • updated docs in case of new feature
  • added/updated tests
  • added/updated testplan to include a test for this fix, including ref to bug using # notation

@mswertz mswertz changed the title Feat/OIDC integration chore: enable OIDC integration into preview Nov 5, 2024
@mswertz
Copy link
Member

mswertz commented Nov 6, 2024

Ah, login tests moeten nu via andere route natuurlijk, want oidc login leidt tot ander login gedrag. 1 van de devs vragen?

@connoratrug
Copy link
Contributor

imo enabling oicd by default on the preview is not a good idea. Login for testing stuf stakes much longer , and lost of test setups become way more complicated. I would suggest a single ( or small set ) test to test the oidc integration.

@mswertz
Copy link
Member

mswertz commented Nov 6, 2024

imo enabling oicd by default on the preview is not a good idea. Login for testing stuf stakes much longer , and lost of test setups become way more complicated. I would suggest a single ( or small set ) test to test the oidc integration.

I propose we can set OidcEneabled to 'false' but keep all env variables. Than oidc can be easily switched on in the preview using changes Harm just made to switch oidc on/off via the settings.

Copy link
Member

@mswertz mswertz left a comment

Choose a reason for hiding this comment

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

nice
as discussed, can we default oidc enabled to 'false' but then in the UI we can re-enable?
I expect we depend on #4372 to make this work

Copy link
Member

@esthervanenckevort esthervanenckevort left a comment

Choose a reason for hiding this comment

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

This only set the settings on the preview server, but the preview server is not authorised in the IdP so it doesn't work yet. The IdP will return an unauthorised client message.

Copy link

sonarqubecloud bot commented Nov 8, 2024

@DickPostma DickPostma marked this pull request as draft November 8, 2024 08:54
@DickPostma
Copy link
Contributor Author

This only set the settings on the preview server, but the preview server is not authorised in the IdP so it doesn't work yet. The IdP will return an unauthorised client message.

Sorry, didn't know about the Draft function. I am currently still working on the PR.
In this case I needed to add the secret-key in the environment of circleCI. Secrets are stored in a secret-key variant in kubernetes.

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