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

Add OAUTH2 integration usage and documentation #42

Merged
merged 2 commits into from
Sep 12, 2024

Conversation

arbulu89
Copy link
Contributor

Add the OAUTH2 configuration parameters for trento-web project in rpm and docker installations

@arbulu89 arbulu89 force-pushed the add-oauth2-integration branch from 143dead to 3bd8cf2 Compare September 10, 2024 14:18
Copy link
Member

@EMaksy EMaksy left a comment

Choose a reason for hiding this comment

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

Hey man lgtm, just left a question about naming :)

| enable_oauth2 | Enable OAUTH2 integration, this disables the username/password authentication method (self exclusive SSO type) | false |
| oauth2_client_id | OAUTH2 client id, required when enable_oauth2 is true | |
| oauth2_client_secret | OAUTH2 client secret, required when enable_oauth2 is true | |
| oauth2_server_base_url | OAUTH2 identity provider base url, required when enable_oauth2 is true | |
Copy link
Member

Choose a reason for hiding this comment

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

It would not harm to align the naming between helm chart and ansible vars what do you think ?
https://github.com/trento-project/helm-charts/pull/105/files#diff-4fe47bf048b209721fcc089ff2be62ad2649ba771dec4767cdc690fcc1b02cb5R23

So why not just oauth2_base_url ?

Copy link
Contributor Author

@arbulu89 arbulu89 Sep 12, 2024

Choose a reason for hiding this comment

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

We do the same for oidc, I just followed what we have.
I think it is fine, as it is documented here.
What we don't have actually documented is the helm chart options, which we will need to do at some point

roles/app/defaults/main.yml Show resolved Hide resolved
@arbulu89 arbulu89 merged commit d8f91e0 into main Sep 12, 2024
9 checks passed
@arbulu89 arbulu89 deleted the add-oauth2-integration branch September 12, 2024 08:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants