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

Fix patch replace when path is omitted #29

Merged
merged 1 commit into from
Nov 3, 2022

Conversation

jasonopslevel
Copy link
Contributor

@jasonopslevel jasonopslevel commented Nov 3, 2022

This PR is to fix a particular request from Okta, but is likely more widely useful. We came across this when Okta attempts to deactivate a User. They use a PATCH request like so:

curl --location --request PATCH '$SCIM_URL/Users/$USER_ID' \
--header 'Content-Type:  application/scim+json' \
--header 'Authorization: Bearer $SCIM_TOKEN' \
--data-raw '   {
     "schemas":
       ["urn:ietf:params:scim:api:messages:2.0:PatchOp"],
     "Operations":[{
       "op":"replace",
       "value": {
         "active": false
       }
     }]
   }    '

This wasn't working as intended, however a request like this was working:

curl --location --request PATCH '$SCIM_URL/Users/$USER_ID' \
--header 'Content-Type:  application/scim+json' \
--header 'Authorization: Bearer $SCIM_TOKEN' \
--data-raw '   {
     "schemas":
       ["urn:ietf:params:scim:api:messages:2.0:PatchOp"],
       "Operations": [{
           "op": "Replace",
           "path": "active",
           "value": false
        }]
   }'    

Copy link
Member

@pond pond left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Strange payload but reading RFC 7644 3.5.2 it does say that path is optional for operation add or replace, though isn't too clear on what that would mean and proceeds to talk at length about what path might be but not talk much at all about value. The reference to RFC 6902 is unhelpful as this contains only examples with a path.

Your interpretation is certainly reasonable based on the Okta payload and the patch, with tests, looks good, so thanks for the contribution. It ought to work well enough for similar value-only cases with compatible nested structures under value too, but we'll have to wait and see what kind of things get sent in practice by various services "in the wild", I guess!

@pond pond merged commit cffcac8 into RIPAGlobal:main Nov 3, 2022
@pond pond mentioned this pull request Nov 3, 2022
@pond
Copy link
Member

pond commented Nov 3, 2022

This is now pushed as v1.3.1 for Rails 6 and v2.1.1 for Rails 7.

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

Successfully merging this pull request may close these issues.

None yet

2 participants