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

refactor: apply auth profile only if other auth profiles are not enabled #19645

Merged
merged 3 commits into from
Jun 24, 2024

Conversation

Ben-Sheppard
Copy link
Contributor

Description

I would like to start making progress on integrating the webapps (starting with Operate) into the new central auth layer. This PR is a small refactor to the logic around when the default auth profile is applied.

Essentially I wanted to centralise that logic and remove the dependency on the Operate specific classes.

@github-actions github-actions bot added component/zeebe Related to the Zeebe component/team component/operate Related to the Operate component/team component/tasklist Related to the Tasklist component/team labels Jun 21, 2024
Copy link
Contributor

github-actions bot commented Jun 21, 2024

Operate Opensearch ITs Results

182 tests  ±0   182 ✅ ±0   59s ⏱️ -2s
 19 suites ±0     0 💤 ±0 
 19 files   ±0     0 ❌ ±0 

Results for commit b7072d6. ± Comparison against base commit 5e372b6.

♻️ This comment has been updated with latest results.

Copy link
Contributor

github-actions bot commented Jun 21, 2024

Operate Unit Tests Results

308 tests  ±0   307 ✅ ±0   6m 16s ⏱️ -53s
 51 suites ±0     1 💤 ±0 
 51 files   ±0     0 ❌ ±0 

Results for commit b7072d6. ± Comparison against base commit 5e372b6.

♻️ This comment has been updated with latest results.

Copy link
Contributor

github-actions bot commented Jun 21, 2024

Operate Integration Tests Results

535 tests  ±0   533 ✅ ±0   7m 31s ⏱️ - 1m 59s
 89 suites ±0     2 💤 ±0 
 89 files   ±0     0 ❌ ±0 

Results for commit b7072d6. ± Comparison against base commit 5e372b6.

♻️ This comment has been updated with latest results.

Copy link
Collaborator

@houssain-barouni houssain-barouni left a comment

Choose a reason for hiding this comment

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

I had just one question about the difference between different auth profiles

@@ -34,7 +35,7 @@ public class WebappsConfigurationInitializer
private static final Set<String> WEBAPPS_PROFILES =
Set.of(OPERATE.getId(), TASKLIST.getId(), IDENTITY.getId());
private static final Set<String> LOGIN_DELEGATED_PROFILES =
Set.of(IDENTITY_AUTH.getId(), SSO_AUTH.getId());
Set.of(IDENTITY_AUTH.getId(), SSO_AUTH.getId(), AUTH_BASIC.getId());
Copy link
Collaborator

Choose a reason for hiding this comment

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

@Ben-Sheppard I am trying to understand the difference between AUTH , AUTH_BASIC and IDENTITY_AUTH
is AUTH_BASIC considered as authentication handled by (delegated to) identity with some basic functionalities?
How Identity webapp behaves when AUTH profile is used?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah good point, so in this case its I guess not delegated in that sense to Identity, the AUTH_BASIC profile relies currently on the Spring basic authentication login (the browser popup) so actually in that sense I don't think its considered delegated i.e. to the Identity service.

I will remove it from this set

@Ben-Sheppard Ben-Sheppard added this pull request to the merge queue Jun 24, 2024
Merged via the queue into main with commit 62148a8 Jun 24, 2024
60 checks passed
@Ben-Sheppard Ben-Sheppard deleted the auth/refactor-default-auth-logic branch June 24, 2024 12:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/operate Related to the Operate component/team component/tasklist Related to the Tasklist component/team component/zeebe Related to the Zeebe component/team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants