CCCHH/keycloak-attribute-endpoints-provider
0
0
Fork 0
Code Issues 2 Pull requests 2 Projects Releases Packages Activity Actions

Add exporting individual attributes and a map endpoint #26

Merged
stb merged 2 commits from add-map-endpoint into main 2026-07-21 21:47:25 +02:00
Conversation 13 Commits 2 Files changed 6 +117 -140
stb commented 2026-07-12 19:22:06 +02:00
Owner
Copy link

Based off #32

Also:

  • rework README.md
  • improve configuration error messages

Closes #22

Based off #32 Also: * rework README.md * improve configuration error messages Closes #22
Fix matchRole lookup to include group-inherited and composite roles
All checks were successful
/ Verify (push) Successful in 1m8s
Details
/ Verify (pull_request) Successful in 46s
Details
2c3c0a3dab 
getRoleMembersStream only returns users with the role assigned
directly, so exported attributes silently excluded users who have
matchRole via group membership. Filter all realm users with
hasRole() instead, matching the resolution already used for authRole.
Add returning attributes split by name
All checks were successful
/ Verify (push) Successful in 44s
Details
/ Verify (pull_request) Successful in 48s
Details
464c02dfb0 
Adds an endpoint that returns the list of attribute values as a map of attribute names in the requested attribute group.

This is useful for requesting the mailing list addresses, where the individual attributes apply to different mailing lists.
stb requested reviews from june, kritzl, lilly 2026-07-12 19:22:22 +02:00
kritzl requested changes 2026-07-14 14:57:51 +02:00
Dismissed
kritzl left a comment
Copy link

Haven't tested the PR yet, but it looks reasonable so far.

Haven't tested the PR yet, but it looks reasonable so far.
attribute-endpoints-provider/src/main/java/de/ccc/hamburg/keycloak/attribute_endpoints/AttributeEndpointsResourceProvider.java Outdated
@ -146,0 +83,4 @@
* @return
*/
@GET
@Path("export/{attributeGroupName}/map")
kritzl commented 2026-07-14 14:55:47 +02:00
Member
Copy link

why is the slug named attributeGroupName in this path? As far as I can see, it has no connection to the actual attribute group name and can still be chosen freely.

why is the `slug` named `attributeGroupName` in this path? As far as I can see, it has no connection to the actual attribute group name and can still be chosen freely.
stb marked this conversation as resolved
kritzl commented 2026-07-14 15:02:26 +02:00
Member
Copy link

@stb

Since @june and I have put a lot of time and energy into this project and I’d like to continue standing behind the codebase, I’d like to ask you to clarify whether the code was modified by an AI Agent or if you refactored it by hand.

@stb Since @june and I have put a lot of time and energy into this project and I’d like to continue standing behind the codebase, I’d like to ask you to clarify whether the code was modified by an AI Agent or if you refactored it by hand.
stb commented 2026-07-14 15:22:22 +02:00
Author
Owner
Copy link

It's done by hand, but it's not correct. Give me some time to wrap my head around everything.

It's done by hand, but it's not correct. Give me some time to wrap my head around everything.
stb changed title from Add an endpoint producing the individual attribute values per name to WIP: Add an endpoint producing the individual attribute values per name 2026-07-14 15:22:48 +02:00
Merge branch 'main' into add-map-endpoint
Some checks failed
/ Verify (pull_request) Failing after 39s
Details
/ Verify (push) Failing after 40s
Details
9073ef086a 
* simplify AttributeEndpointsResourceProvider
* improve explanation of properties in AdminUiPage
* add example for mailing lists and map endpoint
* better explain configuration and operation of provider in main readme
* adjust the Keycloak realm
* Add unit test for endpoint class
Add running tests
All checks were successful
/ Test (push) Successful in 52s
Details
/ Test (pull_request) Successful in 54s
Details
1c2adb1a92
stb changed title from WIP: Add an endpoint producing the individual attribute values per name to Refactor and expand 2026-07-18 18:28:43 +02:00
stb requested review from kritzl 2026-07-18 18:30:29 +02:00
stb removed review request for lilly 2026-07-18 21:15:01 +02:00
kritzl reviewed 2026-07-18 21:33:52 +02:00
README.md Outdated
@ -20,79 +42,8 @@ Once all dependencies are met, simply call `make` to build the provider, which s
There's also `make clean` available for removing the output directory.
To add the provider to your Keycloak install, copy `attribute-endpoints-provider/target/attribute-endpoints-provider-1.0-SNAPSHOT.jar` to the Keycloak Provider directory (`/opt/keycloak/providers/).
kritzl commented 2026-07-18 21:33:52 +02:00
Member
Copy link

missing backtick:

- (`/opt/keycloak/providers/)
+ (`/opt/keycloak/providers/`)
missing backtick: ```diff - (`/opt/keycloak/providers/) + (`/opt/keycloak/providers/`) ```
stb marked this conversation as resolved
Typo
All checks were successful
/ Test (push) Successful in 52s
Details
/ Test (pull_request) Successful in 51s
Details
7452879ef5
Merge branch 'main' into add-map-endpoint
All checks were successful
/ Test (pull_request) Successful in 52s
Details
6ef6d7adeb
kritzl requested changes 2026-07-18 21:46:45 +02:00
kritzl left a comment
Copy link

Is it possible to split this PR into seperate PRs? It's too confusing for me to compare the large diff and a review commit by commit is also not possible, as some thing are reverted by a following commit.

I propose:

  • First PR
    • refactor code without adding new features
    • improve explanation of properties in AdminUiPage
    • better explain configuration and operation of provider in main readme
    • add example for mailing lists and map endpoint
    • adjust the Keycloak realm
  • Second PR
    • extend AttributeEndpointsResourceProvider to add a mapped result (may not be needed immediately)
  • Third PR
    • Add unit test for endpoint class
    • Add running tests

Does this work for you @stb?
I can also try wrapping my head around this and split the PR, but I don't think I will have time for that in the neear future :/

Is it possible to split this PR into seperate PRs? It's too confusing for me to compare the large diff and a review commit by commit is also not possible, as some thing are reverted by a following commit. I propose: - First PR - refactor code _without adding new features_ - improve explanation of properties in AdminUiPage - better explain configuration and operation of provider in main readme - add example for mailing lists and map endpoint - adjust the Keycloak realm - Second PR - extend AttributeEndpointsResourceProvider to add a mapped result (may not be needed immediately) - Third PR - Add unit test for endpoint class - Add running tests Does this work for you @stb? I can also try wrapping my head around this and split the PR, but I don't think I will have time for that in the neear future :/
stb commented 2026-07-18 22:13:10 +02:00
Author
Owner
Copy link

Hhm, splitting this up is a lot of additional work, and it risks introducing additional unwanted changes in the process. Let me think about what it would take to split it up like that.

Hhm, splitting this up is a lot of additional work, and it risks introducing additional unwanted changes in the process. Let me think about what it would take to split it up like that.
stb commented 2026-07-18 22:14:45 +02:00
Author
Owner
Copy link

CAn you at least try and run it as is and tell me whether the tests and the docker compose integration testing are working for you?

CAn you at least try and run it as is and tell me whether the tests and the docker compose integration testing are working for you?
kritzl commented 2026-07-18 23:02:34 +02:00
Member
Copy link

@stb wrote in #26 (comment):

CAn you at least try and run it as is and tell me whether the tests and the docker compose integration testing are working for you?

Yes, will try to test the PR tomorrow

@stb wrote in https://git.hamburg.ccc.de/CCCHH/keycloak-attribute-endpoints-provider/pulls/26#issuecomment-5641: > CAn you at least try and run it as is and tell me whether the tests and the docker compose integration testing are working for you? Yes, will try to test the PR tomorrow
stb referenced this pull request from a commit 2026-07-19 10:28:28 +02:00
Add just the testing setup from #26.
stb referenced this pull request from a commit 2026-07-19 10:45:11 +02:00
Add just the refactoring from #26.
stb commented 2026-07-19 10:47:02 +02:00
Author
Owner
Copy link

I've created #31 for the testing infrastructure, and #32 for the refactoring. After merging those, this can be rebased for an easier review. I can split out the doc changes, but I think it's easy enough to do those as part of this.

I've created #31 for the testing infrastructure, and #32 for the refactoring. After merging those, this can be rebased for an easier review. I can split out the doc changes, but I think it's easy enough to do those as part of this.
stb referenced this pull request from a commit 2026-07-19 13:22:06 +02:00
Add just the refactoring from #26.
stb force-pushed add-map-endpoint from 6ef6d7adeb
All checks were successful
/ Test (pull_request) Successful in 52s
Details
to c9f36f7100
All checks were successful
/ Verify (pull_request) Successful in 41s
Details
2026-07-19 13:38:27 +02:00 Compare
stb changed title from Refactor and expand to Add exporting individual attributes 2026-07-19 13:39:16 +02:00
stb changed title from Add exporting individual attributes to Add exporting individual attributes and a map endpoint 2026-07-19 13:39:30 +02:00
stb commented 2026-07-19 13:40:06 +02:00
Author
Owner
Copy link

I've rebased this PR off #32. I hope the three PRs together make the changes clearer.

I've rebased this PR off #32. I hope the three PRs together make the changes clearer.
stb commented 2026-07-19 13:40:55 +02:00
Author
Owner
Copy link

If people are happy with everything, merging #31 and #32 can be skipped, since this includes all changes.

If people are happy with everything, merging #31 and #32 can be skipped, since this includes all changes.
stb force-pushed add-map-endpoint from c9f36f7100
All checks were successful
/ Verify (pull_request) Successful in 41s
Details
to af1a869de9
All checks were successful
/ Verify (pull_request) Successful in 52s
Details
2026-07-21 21:24:55 +02:00 Compare
june requested changes 2026-07-21 21:45:08 +02:00
Dismissed
README.md Outdated
@ -4,2 +3,2 @@
For this it will provide API endpoints for every configured attribute-group.
The configuration of the provider is possible via an admin page.
This is a Keycloak Provider that exports user profile attribute values.
The selection of attributes and authorization is manage through an endpoint configuration.
june commented 2026-07-21 21:26:46 +02:00
Owner
Copy link

"The selection of attributes and authorization is managed through an endpoint configuration via the admin UI."

"The selection of attributes and authorization is managed through an endpoint configuration via the admin UI."
stb marked this conversation as resolved
attribute-endpoints-provider/src/main/java/de/ccc/hamburg/keycloak/attribute_endpoints/AdminUiPage.java Outdated
@ -122,2 +128,4 @@
.add()
.property()
.name("attribute-group")
june commented 2026-07-21 21:35:42 +02:00
Owner
Copy link

Let's have a comment here to note that we leave this name for backwards compatibility.

Let's have a comment here to note that we leave this name for backwards compatibility.
stb marked this conversation as resolved
Address junes comments
All checks were successful
/ Verify (pull_request) Successful in 52s
Details
91c32295d5
june approved these changes 2026-07-21 21:46:50 +02:00
june left a comment
Copy link

Looks good!

Looks good!
stb merged commit c6f7d2f335 into main 2026-07-21 21:47:25 +02:00
stb deleted branch add-map-endpoint 2026-07-21 21:47:25 +02:00
Sign in to join this conversation.
Reviewers
No reviewers
kritzl
june
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Depends on
#25 Fix matchRole lookup to include group-inherited and composite roles
CCCHH/keycloak-attribute-endpoints-provider
Reference
CCCHH/keycloak-attribute-endpoints-provider!26
No description provided.