-
Notifications
You must be signed in to change notification settings - Fork 324
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 client authz policies #221
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow, this is a lot of new resources! Thanks for the contribution!
Before this gets merged, I'd like to have at least one acceptance test for each of these new resources that tests that some basic configuration works as expected. I'd also appreciate it if you could add some example HCL for these in examples/main.tf
.
I won't force you to write docs for these, but that would be awesome if you wanted to take the time to do that. If not, I can try and get them backfilled at some point later.
I'll take over the tests! |
@mrparkers I just finished the tests for the new providers, and they all pass locally. |
@mrparkers @Amad27 I was able to get more info from the server response. Here is the error:
This is because JS policies are in feature preview and thus need to be enabled when keycloak runs. We do run that way in production, however, the image in the test pipeline does not (as that is the default). Would you prefer that we run with preview features on, or comment out the tests for the JS policy? |
… scope and resource permission types (keycloak#220)
f62a37a
to
a159296
Compare
@yspotts @mrparkers |
Examples are up too! @mrparkers could you please take another look at this pr when you get a chance. |
Thanks for adding the tests! I don't have a problem with passing that flag to keycloak during tests, nice work. I'm trying to look at the diff that GitHub is showing me, but a lot of the changes that are showing are already in master, and it's making this difficult to review. Would you be able to fix the git history for this branch, or make a new branch and cherry-pick your commits to it so the diff only displays what files were changed? Thanks! |
@mrparkers i'm making a new pr to remove the merged code to make it easier to read |
@mrparkers just moved it to a new pr hopefully that will make it easier to read. |
Closed via #246 |
This PR adds all the client authorization policies keycloak provides (except for rule based which is deprecated).