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

feat(oauth2): add sso logout support #4604

Merged
merged 5 commits into from
Sep 19, 2024
Merged

Conversation

danielkelemen
Copy link
Member

@danielkelemen danielkelemen added ci:run Runs the integration tests for the Run distribution. ci:spring-boot Runs the integration tests for the Spring Boot starter. labels Sep 13, 2024
@danielkelemen danielkelemen self-assigned this Sep 13, 2024
@danielkelemen
Copy link
Member Author

I added some details in the corresponding issue: #4455
@venetrius, I'm adding you too, to review the plugin.js frontend file.

Copy link
Member

@venetrius venetrius left a comment

Choose a reason for hiding this comment

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

Added a question and a suggestion to plugin.js

Copy link
Member

@yanavasileva yanavasileva left a comment

Choose a reason for hiding this comment

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

👍 Looks good. I have only improvement suggestion for the logging.

Tested Camunda Run + okta with enabled and disabled sso logout.

/**
* Enable SSO Logout. Default {@code {baseUrl}}.
*/
private String postLogoutRedirectUri = "{baseUrl}";
Copy link
Member

Choose a reason for hiding this comment

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

✍️ In docs, we need to add that this must be provided if sso-logout is enabled. At least for Okta, this property is mandatory.

/**
* Enable SSO Logout. Default {@code false}.
*/
private boolean enabled = false;
Copy link
Member

Choose a reason for hiding this comment

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

✍️ We need to document this and default value.


var validateTokenFilter = new AuthorizeTokenFilter(clientManager);
Copy link
Member Author

Choose a reason for hiding this comment

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

🔧 Refactored this so it uses the same @Bean structure as other classes.

Copy link
Member

@venetrius venetrius left a comment

Choose a reason for hiding this comment

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

👍

@danielkelemen danielkelemen merged commit 8d843b4 into master Sep 19, 2024
2 of 3 checks passed
@danielkelemen danielkelemen deleted the 4455-oauth2-sso-logout branch September 19, 2024 14:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci:run Runs the integration tests for the Run distribution. ci:spring-boot Runs the integration tests for the Spring Boot starter.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants