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

Fetching users returns users groups with all the groups members in them #71

Closed
SamMurphyDev opened this issue Nov 23, 2022 · 3 comments
Closed

Comments

@SamMurphyDev
Copy link
Contributor

When fetching user (/Users), the groups' attribute is returned containing a list of all the groups the user is part of. Then each of those groups within the users returns a list of all their members.
(Not representative of a complete response, here is an example of how the payload is currently structured)

{
  "id": "fbc56952-0c46-4c00-80b3-fc48fbef8eaa",
  "groups": [
    {
      "value": "383e9f70-a1af-4e26-b068-d2454aca0260",
      "members": [
        {
          "value": "fbc56952-0c46-4c00-80b3-fc48fbef8eaa"
        },
        {
          "value": "b6ccf7e6-ac9e-4db8-b81c-b0167d431a95"
        }
      ]
    }
  ]
}

This can create large responses when the user belongs to many groups with many users (in my case, 120MB for a single user).

I propose changing https://github.com/jelhub/scimgateway/blob/master/lib/scimgateway.js#L791 to remove the members.value. This could be breaking for people relying on it, or it could be hidden behind a feature flag.

Doing this is within the SCIM Schema specification. As outlined in section 4.1.2 https://www.rfc-editor.org/rfc/rfc7643#section-4.1.2, under the heading of groups,

SCIM service provider exposes a "Group" resource, the "value" sub-attribute MUST be the "id", and the "$ref" sub-attribute must be the URI of the corresponding "Group" resources to which the user belongs.

This, in my eyes, would mean that members is at least an optional attribute to return on the user response payload. This is further backed up by the full user representation listed here https://www.rfc-editor.org/rfc/rfc7643#section-8.2. While the specification refers to a non-normative example, it provides an insight into what the specification authors were expecting as part of the standard.

Let me know what you think. I've already implemented the change on my fork (without the feature flag). So happy to carry this forward to a PR if you agree with the above.

Cheers,
Sam

@jelhub
Copy link
Owner

jelhub commented Nov 23, 2022

Hi,

You may test using plugin-loki

/Users groups attribute should have following syntax (value is mandatory) and only include groups the actual user is member of

	"groups": [{
			"value": "Employees",
			"display": "Employees",
			"type": "direct"
		}
	],

If your plugin getUsers method do not return groups, scimgateway will automatically try /Groups?filter=members.value eq "userid"&attributes=members.value,id,displayName and include groups accordingly

I suspect your getUsers method includes groups and do not use correct format as shown above
(but you also mention https://github.com/jelhub/scimgateway/blob/master/lib/scimgateway.js#L791 having the auto-apply logic when groups not included...)

You may also exclude groups from the response by using: /Users?excludedAttributes=groups
or your getUsers could include empty groups "groups": []

Regards,
Jarle

@jelhub
Copy link
Owner

jelhub commented Nov 23, 2022

Your issue might refer to /Groups?filter=members.value eq "userid"&attributes=members.value,id,displayName returning what you mention (includes all members for the group user is member of)

Your are correct, skipping members.value in attributes is better attributes=id,displayName and I might look into this change, but I don't think general plugins care much of query attributes used. They probably return all supported without any query check.

The reason for including members.value was to double check that actual user is included in case plugin do not return /Groups as expected.

But anyhow these other members will not be included in the groups returned by /Users

Jarle

@jelhub
Copy link
Owner

jelhub commented Nov 23, 2022

v4.1.10 have been published addressing this problem

Jarle

@jelhub jelhub closed this as completed Nov 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants