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

auth/admin/realms/{realm-name}/groups?briefRepresentation=false returns empty subgroups. #27694

Open
1 of 2 tasks
KapilBillore opened this issue Mar 8, 2024 · 15 comments
Open
1 of 2 tasks

Comments

@KapilBillore
Copy link

KapilBillore commented Mar 8, 2024

Before reporting an issue

  • I have read and understood the above terms for submitting issues, and I understand that my issue may be closed without action if I do not follow them.

Area

core

Describe the bug

Hi Team,

We have been migrating to KC 23.5 and observed that the /groups functionality doesn't return subgroups anymore and the field is empty.
Can you suggest an alternative way/fix to handle the same?

Version

23.5

Regression

  • The issue is a regression

Expected behavior

The groups should be returned with subgroups populated.

Actual behavior

Subgroup field is empty

How to Reproduce?

https://{keycloak-host}:port/auth/admin/realms/{realm-name}/groups?briefRepresentation=false

Anything else?

No response

@KapilBillore KapilBillore added kind/bug Categorizes a PR related to a bug status/triage labels Mar 8, 2024
@KapilBillore KapilBillore changed the title auth/admin/realms/DPC/groups?briefRepresentation=false doesn't return empty subgroups. auth/admin/realms/{realm-name}/groups?briefRepresentation=false doesn't return empty subgroups. Mar 8, 2024
@RichieB2B
Copy link
Contributor

RichieB2B commented Mar 9, 2024

We are experiencing the same issue. I've read that with #22700 there now is a new /admin/realms/{realm}/groups/{group-id}/children paginated endpoint but nowhere is mentioned that /admin/realms/{realm}/groups will now always return empty subGroups (regardsless of what briefRepresentation and populateHierarchy is set to) .

This is a breaking change in the API that I wish was communicated more clearly.

@RichieB2B
Copy link
Contributor

RichieB2B commented Mar 10, 2024

I have just tested this issue with Keycloak 24.0.1
As reported above subGroups is always empty. The upgrade guide mentions:

subGroups list is now only populated on queries that request hierarchy data
This field is populated from the "bottom up" so can’t be relied on for getting all subgroups for a group

This confuses me since populateHierarchy still defaults to true in the groups API but it does work unless search is used. Without the search parameter subGroups stays empty. Even an empty search parameter is enough though.

This seems to be a result of using getTopLevelGroupsStream() in combination with GroupUtils.populateGroupHierarchyFromSubGroups() which now is a "bottom up" search. That is a bad combination to use as a default for the groups endpoint IMHO.

Output for /admin/realms/{realm}/groups

[
  {
    "id": "98d4be1e-16db-45d0-9888-c6f32c00d968",
    "name": "top-group",
    "path": "/top-group",
    "subGroupCount": 2,
    "subGroups": [],
    "access": {
      "view": true,
      "viewMembers": true,
      "manageMembership": true
    }
  }
]

Output for /admin/realms/myrealm/groups?search=

[
  {
    "id": "98d4be1e-16db-45d0-9888-c6f32c00d968",
    "name": "top-group",
    "path": "/top-group",
    "subGroupCount": 2,
    "subGroups": [
      {
        "id": "65da20e5-e8d8-4fe8-b105-b437a85a9880",
        "name": "child1",
        "path": "/top-group/child1",
        "parentId": "98d4be1e-16db-45d0-9888-c6f32c00d968",
        "subGroupCount": 1,
        "subGroups": [
          {
            "id": "d04ceddf-dff1-44b8-9e54-b821aa262fd0",
            "name": "grandchild1",
            "path": "/top-group/child1/grandchild1",
            "parentId": "65da20e5-e8d8-4fe8-b105-b437a85a9880",
            "subGroupCount": 0,
            "subGroups": [],
            "access": {
              "view": true,
              "viewMembers": true,
              "manageMembers": true,
              "manage": true,
              "manageMembership": true
            }
          }
        ],
        "access": {
          "view": true,
          "viewMembers": true,
          "manageMembers": true,
          "manage": true,
          "manageMembership": true
        }
      },
      {
        "id": "901e1e5e-2541-4909-a87c-8f61f82703a6",
        "name": "child2",
        "path": "/top-group/child2",
        "parentId": "98d4be1e-16db-45d0-9888-c6f32c00d968",
        "subGroupCount": 1,
        "subGroups": [
          {
            "id": "645dee83-0097-4866-a28f-c6a57d18ed9d",
            "name": "grandchild2",
            "path": "/top-group/child2/grandchild2",
            "parentId": "901e1e5e-2541-4909-a87c-8f61f82703a6",
            "subGroupCount": 0,
            "subGroups": [],
            "access": {
              "view": true,
              "viewMembers": true,
              "manageMembers": true,
              "manage": true,
              "manageMembership": true
            }
          }
        ],
        "access": {
          "view": true,
          "viewMembers": true,
          "manageMembers": true,
          "manage": true,
          "manageMembership": true
        }
      }
    ],
    "access": {
      "view": true,
      "viewMembers": true,
      "manageMembers": true,
      "manage": true,
      "manageMembership": true
    }
  }
]

@RichieB2B
Copy link
Contributor

RichieB2B commented Mar 10, 2024

@KapilBillore can you rename the title of this issue to:

/admin/realms/{realm-name}/groups returns empty subGroups when called without search parameter

@KapilBillore KapilBillore changed the title auth/admin/realms/{realm-name}/groups?briefRepresentation=false doesn't return empty subgroups. auth/admin/realms/{realm-name}/groups?briefRepresentation=false returns empty subgroups. Mar 11, 2024
@Skyppid
Copy link

Skyppid commented Mar 11, 2024

We also experience this issue since the upgrade. I'm using a .NET library and logged the JSON response I get from both the groups endpoint:

{"id":"7b05dd5f-1567-4c35-bdc8-b4a33eeb4d05","name":"Parent","path":"/Parent","subGroupCount":1,"subGroups":[],"attributes":{},"realmRoles":[],"clientRoles":{"***":["***"]},"access":{"view":true,"viewMembers":true,"manageMembers":true,"manage":true,"manageMembership":true}}

(I obfuscated some parts with *** though)
As you see it says subGroupCount: 1 but provides no groups. The group is not deeply nested, it's just one layer deep.

If I use the new endpoint for children I just get [] and that's it.

@pedroigor
Copy link
Contributor

As you mentioned/know we had some changes in this area due to some performance optimizations. AFAIK, the motivation behind not returning sub-groups when the search parameter is empty is that, in theory, you are filtering results and it should be fine to return sub-groups.

I'm not sure if we are going to revisit this behavior. If I understand correctly, the issues reported here are:

  • sub-group count returning a wrong value when no subgroups are returned
  • review how we are processing sub-groups when searching for top-level groups

@pedroigor pedroigor self-assigned this Mar 11, 2024
@RichieB2B
Copy link
Contributor

@Skyppid Are you sure you called the /children endpoint correctly? It should return a list of GroupRepresentation:

GET /admin/realms/{realm}/groups/7b05dd5f-1567-4c35-bdc8-b4a33eeb4d05/children

@RichieB2B
Copy link
Contributor

I'm not sure if we are going to revisit this behavior. If I understand correctly, the issues reported here are:

  • sub-group count returning a wrong value when no subgroups are returned
  • review how we are processing sub-groups when searching for top-level groups

My main issue is that a simple call to /admin/realms/{realm}/groups without any parameters is not populating subGroups. The description of this endpoint is:

Get group hierarchy. Only name and ids are returned.

Since 23.x it is not returning a hierarchy by default anymore. If that is by choice the description of the endpoint should reflect this. Maybe something like:

Search for groups. Without any parameters only the topmost groups are returned. The hierarchy in subGroups is not reliable, use /children instead.  

and set populateHierarchy to false by default.

@Skyppid
Copy link

Skyppid commented Mar 12, 2024

@Skyppid Are you sure you called the /children endpoint correctly? It should return a list of GroupRepresentation:

GET /admin/realms/{realm}/groups/7b05dd5f-1567-4c35-bdc8-b4a33eeb4d05/children

Never mind ... it was late and I overlooked that I passed max = 0 instead of null. My bad. Interesting that Keycloak accepts non-sense though.

If that is by choice the description of the endpoint should reflect this

At that point I might add that for me as a developer it's quite frustrating that for version 24 Keycloak's API documentation has more holes than a swiss cheese. It's really hard to figure out how to use the API since 90% of the parameters are not even described what they do. Most endpoints miss any description, it's just their name which often leaves you with speculation what it actually does as you just suggested for this specific one.

I know API documentation is a tough task to keep it maintained, but it's missing so much information that should be available for developers. Maybe the API is a bit niché but still. Keycloak is not a small tool. Would be highly appreciated if this would be improved sometime.

But yes, would really help if you change the description. I spent a whole day yesterday figuring out why a key feature broke. I'm not even sure if that change was mentioned in the Release Notes. At least I did not see it there. Luckily this discussion helped me find a way to solve it so we can manage our deadline.

@keycloak-github-bot
Copy link

Due to the amount of issues reported by the community we are not able to prioritise resolving this issue at the moment.

If you are affected by this issue, upvote it by adding a 👍 to the description. We would also welcome a contribution to fix the issue.

@RichieB2B
Copy link
Contributor

@pedroigor Can you confirm that requesting /admin/realms/{realm}/groups without any parameters not returning the group hierarchy is intended behaviour? If so, I'll make a PR to clearly mention this in the API docs.

@bamandineDoca
Copy link

Hello,
I agree with Skyppid and RichieB2B, if not returning the group hierarchy is an intended behavior, it should be added in the API documentation and also in the version 22 to 23 migration guide.

The migration guide currently explains that a new endpoint is available but it does not mention that the existing one has changed.

Endpoint GET {keycloak server}/realms/{realm}/groups/{group_id}/children added as a way to get subgroups of specific groups that support paging

It is important to highlight that a breaking change has been made and getting subgroups is no longer available on the GET {keycloak server}/realms/{realm}/groups endpoint. The migration guide should also explain that using the new endpoint GET {keycloak server}/realms/{realm}/groups/{group_id}/children is now required for getting subgroups.

@RichieB2B
Copy link
Contributor

I still think this is a bug that should be fixed. Breaking an API like this without documenting it cannot be intentional IMHO.

@mzndr
Copy link

mzndr commented Apr 29, 2024

Sorry I have to chime in. The keycloak admin API is probably one of the worst API's I have ever seen. Almost every field is optional and basically nothing is documented. In more cases than not it behaves differently than expected. Everything is trial and error, and it feels like you can not expect ANY consistency in responses between endpoints at all. This makes working with keycloak in strictly typed languages a true nightmare. I'm not writing this to just rant, but to press on that this is a serious issue.

@RichieB2B
Copy link
Contributor

I think it speaks volumes that after introducing a breaking change without any warning or documentation, the developers can't decide if this was intended behaviour or a bug.

@Skyppid
Copy link

Skyppid commented Apr 29, 2024

Sorry I have to chime in. The keycloak admin API is probably one of the worst API's I have ever seen. Almost every field is optional and basically nothing is documented. In more cases than not it behaves differently than expected. Everything is trial and error, and it feels like you can not expect ANY consistency in responses between endpoints at all. This makes working with keycloak in strictly typed languages a true nightmare. I'm not writing this to just rant, but to press on that this is a serious issue.

Sadly true. They really need to improve there by a lot. One of the main reasons why we currently look into alternatives. I think some of the most critical issues on production we had were related to Keycloaks API...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants