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 Keycloak Authorization dynamic tenant config resolution #39643

Conversation

michalvavrik
Copy link
Member

@michalvavrik michalvavrik commented Mar 22, 2024

This comment has been minimized.

This comment has been minimized.

Copy link

github-actions bot commented Mar 22, 2024

🙈 The PR is closed and the preview is expired.

@michalvavrik
Copy link
Member Author

hey @pedroigor please review if you would like, otherwise in terms of what is changed, changes are not specific to Keycloak (it's a CDI feature...), so @sberyozkin is right reviewer for this.

@michalvavrik michalvavrik force-pushed the feature/kc-authz-dynamic-policy-enforcer-resolver branch from af3c7d6 to 56072ca Compare March 24, 2024 10:39

This comment has been minimized.

This comment has been minimized.

@michalvavrik michalvavrik force-pushed the feature/kc-authz-dynamic-policy-enforcer-resolver branch 2 times, most recently from 051b04b to c998c60 Compare March 24, 2024 11:12

This comment has been minimized.

This comment has been minimized.

@michalvavrik michalvavrik force-pushed the feature/kc-authz-dynamic-policy-enforcer-resolver branch from c998c60 to 1789d1a Compare March 28, 2024 01:15
@michalvavrik michalvavrik marked this pull request as draft March 28, 2024 01:15
@michalvavrik
Copy link
Member Author

@sberyozkin I pushed it by accident as I need to fix some tests, docs and write more tests, but as it's already there, you can look at the builder and provide early feedback. Or not. Anyway, it's as compact as I could make it. Cheers.

This comment has been minimized.

@michalvavrik michalvavrik force-pushed the feature/kc-authz-dynamic-policy-enforcer-resolver branch from 1789d1a to 96f0b5c Compare March 28, 2024 13:27
@michalvavrik michalvavrik marked this pull request as ready for review March 28, 2024 13:29
@michalvavrik
Copy link
Member Author

Done, the DynamicTenantConfigPolicyEnforcerTest tests new builder comprehensively.

@michalvavrik michalvavrik force-pushed the feature/kc-authz-dynamic-policy-enforcer-resolver branch from 96f0b5c to a4a1aa6 Compare March 28, 2024 13:38

This comment has been minimized.

This comment has been minimized.

@sberyozkin
Copy link
Member

Hi @michalvavrik, can you please type a builder sequence showing all the supported properties being set, I was not sure about setGet("write")...

@michalvavrik michalvavrik force-pushed the feature/kc-authz-dynamic-policy-enforcer-resolver branch from a4a1aa6 to 9a6ad0e Compare April 2, 2024 14:25

This comment has been minimized.

This comment has been minimized.

@michalvavrik michalvavrik force-pushed the feature/kc-authz-dynamic-policy-enforcer-resolver branch from 9a6ad0e to cb558a3 Compare April 8, 2024 21:58

This comment has been minimized.

@michalvavrik michalvavrik force-pushed the feature/kc-authz-dynamic-policy-enforcer-resolver branch from cb558a3 to 80f2fbc Compare April 8, 2024 22:08

This comment has been minimized.

This comment has been minimized.

@michalvavrik
Copy link
Member Author

ping @sberyozkin

@michalvavrik michalvavrik force-pushed the feature/kc-authz-dynamic-policy-enforcer-resolver branch from 80f2fbc to 268049b Compare May 30, 2024 11:10
@michalvavrik
Copy link
Member Author

I've rebased on the current main and resolved merge conflicts just in case @sberyozkin find a time to review.

This comment has been minimized.

@michalvavrik
Copy link
Member Author

@sberyozkin please have a look; I won't resolve merge conflicts again as I'm bit worried it's wasting of CI resources. Will do it when you find a time for this

@michalvavrik michalvavrik force-pushed the feature/kc-authz-dynamic-policy-enforcer-resolver branch from 268049b to 24aef29 Compare July 11, 2024 12:49
@michalvavrik
Copy link
Member Author

Merge conflicts were about TLS registry, the rest of this PR still applies. Resolved.

Copy link

quarkus-bot bot commented Jul 11, 2024

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 24aef29.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

Warning

There are other workflow runs running, you probably need to wait for their status before merging.

You can consult the Develocity build scans.

Copy link

quarkus-bot bot commented Jul 11, 2024

Status for workflow Quarkus Documentation CI

This is the status report for running Quarkus Documentation CI on commit 24aef29.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

Copy link
Member

@sberyozkin sberyozkin left a comment

Choose a reason for hiding this comment

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

Thanks @michalvavrik , let's do it and start planning doing something similar for OIDC 🙂
Cheers Sergey

@sberyozkin sberyozkin merged commit c466d16 into quarkusio:main Jul 20, 2024
23 checks passed
@quarkus-bot quarkus-bot bot added the kind/enhancement New feature or request label Jul 20, 2024
@quarkus-bot quarkus-bot bot added this to the 3.14 - main milestone Jul 20, 2024
@michalvavrik michalvavrik deleted the feature/kc-authz-dynamic-policy-enforcer-resolver branch July 20, 2024 22:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
2 participants