You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@directory.apache.org by "kjthorpe18 (via GitHub)" <gi...@apache.org> on 2023/12/10 22:05:51 UTC
[I] DefaultPatchHandler does not handle Group member remove operations correctly [directory-scimple]
kjthorpe18 opened a new issue, #436:
URL: https://github.com/apache/directory-scimple/issues/436
The DefaultPatchHandler does not treat ScimGroup `members` as a collection, and removes the `members` attribute instead of the specified value.
Example request:
`PATCH /api/scim/v2/Groups/035a3341-5cc0-4abd-983b-d4540064afc6`
```json
{
"schemas":[
"urn:ietf:params:scim:api:messages:2.0:PatchOp"
],
"Operations":[
{
"op":"remove",
"path":"members",
"value":[
{
"value":"aaebb169-b91d-481c-9957-097faeaf649a"
}
]
}
]
}
```
This request uses the DefaultPatchHandler's `RemoveOperationHandler`, and the method:
```java
@Override
public <T extends ScimResource> void applyMultiValue(T source, Map<String, Object> sourceAsMap, Schema schema, Attribute attribute, AttributeReference attributeReference, Object value) {
checkMutability(attribute, sourceAsMap.get(attribute.getName()));
// remove the collection
sourceAsMap.remove(attribute.getName());
}
```
Rather than removing the whole collection, only the specified value should be removed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@directory.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@directory.apache.org
For additional commands, e-mail: dev-help@directory.apache.org
Re: [I] DefaultPatchHandler does not handle Group member remove operations correctly [directory-scimple]
Posted by "bdemers (via GitHub)" <gi...@apache.org>.
bdemers commented on issue #436:
URL: https://github.com/apache/directory-scimple/issues/436#issuecomment-1850874468
Yup! PatchHandler is probably the best bet
That said, if we need to test this at a higher level, we can do that too!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@directory.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@directory.apache.org
For additional commands, e-mail: dev-help@directory.apache.org
Re: [I] DefaultPatchHandler does not handle Group member remove operations correctly [directory-scimple]
Posted by "kjthorpe18 (via GitHub)" <gi...@apache.org>.
kjthorpe18 closed issue #436: DefaultPatchHandler does not handle Group member remove operations correctly
URL: https://github.com/apache/directory-scimple/issues/436
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@directory.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@directory.apache.org
For additional commands, e-mail: dev-help@directory.apache.org
Re: [I] DefaultPatchHandler does not handle Group member remove operations correctly [directory-scimple]
Posted by "kjthorpe18 (via GitHub)" <gi...@apache.org>.
kjthorpe18 commented on issue #436:
URL: https://github.com/apache/directory-scimple/issues/436#issuecomment-1850433454
There are a few ways in which Azure doesn't conform to the spec, and I really don't expect them to fix these any time soon.
So, I think if we can catch this specific type of patch, we can handle it without messing with anything else. Would it be too hacky to check if a patch is this specific form?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@directory.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@directory.apache.org
For additional commands, e-mail: dev-help@directory.apache.org
Re: [I] DefaultPatchHandler does not handle Group member remove operations correctly [directory-scimple]
Posted by "bdemers (via GitHub)" <gi...@apache.org>.
bdemers commented on issue #436:
URL: https://github.com/apache/directory-scimple/issues/436#issuecomment-1883887278
@kjthorpe18 we can close this issue as fixed in #448 right?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@directory.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@directory.apache.org
For additional commands, e-mail: dev-help@directory.apache.org
Re: [I] DefaultPatchHandler does not handle Group member remove operations correctly [directory-scimple]
Posted by "bdemers (via GitHub)" <gi...@apache.org>.
bdemers commented on issue #436:
URL: https://github.com/apache/directory-scimple/issues/436#issuecomment-1850464887
Maybe we should create a test naming or tag for these. My guess is that Azure wont be the only one that will have quirks.
Do you want to take a pass creating tests for the azure cases? (Even if they don't pass currently)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@directory.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@directory.apache.org
For additional commands, e-mail: dev-help@directory.apache.org
Re: [I] DefaultPatchHandler does not handle Group member remove operations correctly [directory-scimple]
Posted by "kjthorpe18 (via GitHub)" <gi...@apache.org>.
kjthorpe18 commented on issue #436:
URL: https://github.com/apache/directory-scimple/issues/436#issuecomment-1852165423
@bdemers Started https://github.com/apache/directory-scimple/pull/448
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@directory.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@directory.apache.org
For additional commands, e-mail: dev-help@directory.apache.org
Re: [I] DefaultPatchHandler does not handle Group member remove operations correctly [directory-scimple]
Posted by "kjthorpe18 (via GitHub)" <gi...@apache.org>.
kjthorpe18 commented on issue #436:
URL: https://github.com/apache/directory-scimple/issues/436#issuecomment-1849105074
Also, this needs to handle removing members by _only_ their specified `value`, without specifying other information such as `display`. For example, in the `source` items we may have:
```json
{
"value": "aaebb169-b91d-481c-9957-097faeaf649a",
"display": "bjensen",
"type": "User"
}
```
and should be able to remove with the operation
```json
{
"schemas":[
"urn:ietf:params:scim:api:messages:2.0:PatchOp"
],
"Operations":[
{
"op":"remove",
"path":"members",
"value":[
{
"value":"aaebb169-b91d-481c-9957-097faeaf649a"
}
]
}
]
}
```
which corresponds to a `value` in the PatchHandler of just
```json
{
"value": "aaebb169-b91d-481c-9957-097faeaf649a"
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@directory.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@directory.apache.org
For additional commands, e-mail: dev-help@directory.apache.org
Re: [I] DefaultPatchHandler does not handle Group member remove operations correctly [directory-scimple]
Posted by "kjthorpe18 (via GitHub)" <gi...@apache.org>.
kjthorpe18 commented on issue #436:
URL: https://github.com/apache/directory-scimple/issues/436#issuecomment-1883977502
Yep! This can be closed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@directory.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@directory.apache.org
For additional commands, e-mail: dev-help@directory.apache.org
Re: [I] DefaultPatchHandler does not handle Group member remove operations correctly [directory-scimple]
Posted by "kjthorpe18 (via GitHub)" <gi...@apache.org>.
kjthorpe18 commented on issue #436:
URL: https://github.com/apache/directory-scimple/issues/436#issuecomment-1849107103
The SCIM RFC specifies the following as the way to remove a member from a group (and this is also how Okta formats their requests):
```
The following example shows how to remove a member from a group. As
with the previous example, the "display" sub-attribute is optional.
If the user was not a member of this group, no changes should be made
to the resource, and a success response should be returned.
Note that server responses have been omitted for the rest of the
PATCH examples.
Remove a single member from a group. Some text was removed for
readability (indicated by "..."):
PATCH /Groups/acbf3ae7-8463-...-9b4da3f908ce
Host: example.com
Accept: application/scim+json
Content-Type: application/scim+json
Authorization: Bearer h480djs93hd8
If-Match: W/"a330bc54f0671c9"
{
"schemas":
["urn:ietf:params:scim:api:messages:2.0:PatchOp"],
"Operations":[{
"op":"remove",
"path":"members[value eq \"2819c223-7f76-...413861904646\"]"
}]
}
```
So, if the example request from Azure is not compliant with the RFC, then SCIM Servers using SCIMple will have to add a case to support Azure-style removes
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@directory.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@directory.apache.org
For additional commands, e-mail: dev-help@directory.apache.org
Re: [I] DefaultPatchHandler does not handle Group member remove operations correctly [directory-scimple]
Posted by "kjthorpe18 (via GitHub)" <gi...@apache.org>.
kjthorpe18 commented on issue #436:
URL: https://github.com/apache/directory-scimple/issues/436#issuecomment-1850718076
@bdemers yep I can give it a shot. Specifically in the PatchHandler? Anywhere else?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@directory.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@directory.apache.org
For additional commands, e-mail: dev-help@directory.apache.org
Re: [I] DefaultPatchHandler does not handle Group member remove operations correctly [directory-scimple]
Posted by "bdemers (via GitHub)" <gi...@apache.org>.
bdemers commented on issue #436:
URL: https://github.com/apache/directory-scimple/issues/436#issuecomment-1850361386
Good find @kjthorpe18!
I agree that does look out of spec:
> If the target location is a multi-valued attribute and no filter is specified, the attribute and all values are removed, and the attribute SHALL be considered unassigned.
There is no filter in that request.
That said, I'm guessing this looks specific enough were we could support it out of the box:
e.g. if a `remove` request is formatted like this, remove that value (possibly normalize the request, anything else processing this operation sees it as `"path":"members[value eq \"<value>\"]"`
Worst case we can add some sort of flag (system property or something), `scimple.azure.workarounds=true`
But I do think the intention of this patch operation is clear.
Thoughts?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@directory.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@directory.apache.org
For additional commands, e-mail: dev-help@directory.apache.org