Fix exception raised when patch endpoint receives invalid request #30
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rreinhardt9
commented
Jun 16, 2020
| raise ScimRails::ExceptionHandler::UnsupportedPatchRequest | ||
| end | ||
|
|
||
| valid_operation = params["Operations"].find(handle_invalid) do |operation| |
There was a problem hiding this comment.
The argument supplied to "find" is called when no match is found. So if no valid operation is found, it calls the lambda raising the exception.
rreinhardt9
commented
Jun 16, 2020
| { | ||
| op: "replace", | ||
| path: "displayName", | ||
| value: "Francis" |
There was a problem hiding this comment.
This is the format of the bad operations we were seeing in production. They had paths with string values.
rreinhardt9
force-pushed
the
fix-malformed-patch-exception
branch
from
99c72ca to
058fe75
Compare
fixes lessonly#29 The patch endpoint isn't fully scim compliant yet and only allows for specific operations. When we received a specific format of request that wasn't valid, it was raising an exception instead of returning the expected 422 response. Add extra validation to prevent this situation when receiving an unexpected body for a patch request.
rreinhardt9
force-pushed
the
fix-malformed-patch-exception
branch
from
058fe75 to
a6aa2a0
Compare
rreinhardt9
commented
Jun 16, 2020
|
|
||
| operations = params["Operations"] || {} | ||
|
|
||
| valid_operation = operations.find(handle_invalid) do |operation| |
There was a problem hiding this comment.
The lambda passed to find is called when nothing is found... so in this case if a valid operation is not found it calls the handler that will raise the UnsupportedPatchRequest exception
wernull
approved these changes
Jun 17, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #29
Testing Notes
Is any special setup required to test this change? Non-obvious things that should be checked?
A list of things to test:
curl -X PATCH 'http://dev:api_key@localhost:3000/scim/v2/Users/819216' -d '{"schemas": ["urn:ietf:params:scim:api:messages:2.0:PatchOp"], "Operations": [{"op":"Add","path":"displayName","value":"Norris, Chuck"}]}' -H 'Content-Type: application/scim+json'curl -X PATCH 'http://dev:api_key@localhost:3000/scim/v2/Users/819216' -d '{"schemas": ["urn:ietf:params:scim:api:messages:2.0:PatchOp"], "Operations": [{"op":"replace,"value":{"active":true}]}' -H 'Content-Type: application/scim+json'To test this, I pointed a locally running lessonly app at the gem (locally) using the bundler path directive. Let me know and we can pair on testing because that requires some set up that I'd be glad to help with!
Merge Instructions
Please rebase my commits when merging