From a6aa2a0815efbb4a67d87a6207a56fab6cd7473f Mon Sep 17 00:00:00 2001 From: Ross Reinhardt Date: Tue, 16 Jun 2020 11:12:17 -0400 Subject: [PATCH] Fix exception raised when patch endpoint receives invalid request fixes lessonly/scim_rails#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. --- .../scim_rails/scim_users_controller.rb | 20 ++++++-- .../scim_rails/scim_users_controller_spec.rb | 47 +++++++++++++++++++ 2 files changed, 64 insertions(+), 3 deletions(-) diff --git a/app/controllers/scim_rails/scim_users_controller.rb b/app/controllers/scim_rails/scim_users_controller.rb index e7f90f4a..a0eaef40 100644 --- a/app/controllers/scim_rails/scim_users_controller.rb +++ b/app/controllers/scim_rails/scim_users_controller.rb @@ -126,9 +126,23 @@ def put_active_param end def patch_active_param - active = params.dig("Operations", 0, "value", "active") - raise ScimRails::ExceptionHandler::UnsupportedPatchRequest if active.nil? - active + handle_invalid = lambda do + raise ScimRails::ExceptionHandler::UnsupportedPatchRequest + end + + operations = params["Operations"] || {} + + valid_operation = operations.find(handle_invalid) do |operation| + valid_patch_operation?(operation) + end + + valid_operation.dig("value", "active") + end + + def valid_patch_operation?(operation) + operation["op"] == "replace" && + operation["value"] && + operation["value"]["active"] end end end diff --git a/spec/controllers/scim_rails/scim_users_controller_spec.rb b/spec/controllers/scim_rails/scim_users_controller_spec.rb index 0aae482e..9c8f5f8b 100644 --- a/spec/controllers/scim_rails/scim_users_controller_spec.rb +++ b/spec/controllers/scim_rails/scim_users_controller_spec.rb @@ -569,6 +569,53 @@ response_body = JSON.parse(response.body) expect(response_body.dig("schemas", 0)).to eq "urn:ietf:params:scim:api:messages:2.0:Error" end + + it "returns 422 when value is " do + patch :patch_update, params: { + id: 1, + Operations: [ + { + op: "replace", + path: "displayName", + value: "Francis" + } + ] + } + + expect(response.status).to eq 422 + response_body = JSON.parse(response.body) + expect(response_body.dig("schemas", 0)).to eq "urn:ietf:params:scim:api:messages:2.0:Error" + end + + it "returns 422 when value is missing" do + patch :patch_update, params: { + id: 1, + Operations: [ + { + op: "replace" + } + ] + } + + expect(response.status).to eq 422 + response_body = JSON.parse(response.body) + expect(response_body.dig("schemas", 0)).to eq "urn:ietf:params:scim:api:messages:2.0:Error" + end + + it "returns 422 operations key is missing" do + patch :patch_update, params: { + id: 1, + Foobars: [ + { + op: "replace" + } + ] + } + + expect(response.status).to eq 422 + response_body = JSON.parse(response.body) + expect(response_body.dig("schemas", 0)).to eq "urn:ietf:params:scim:api:messages:2.0:Error" + end end end