Skip to content

Conversation

@uberbrady
Copy link
Member

I'm not 100% on this yet, but this seems to basically do the trick - it takes any SCIM response and yanks out any elements that are from a 'core' schema - which shouldn't be put into a sub-block, but should actually be in the 'root' level of the returned JSON. This handles that for individual responses, as well as list-responses.

I've only really tested against grabbing a user listing or grabbing a single user, or doing a user-create.

@uberbrady
Copy link
Member Author

uberbrady commented Nov 7, 2022

I've tested all three of:

  • Without the new SCIM_STANDARDS_COMPLIANCE environment set
  • Or set to false
  • Or set to true

And it works as expected in all three cases.

@uberbrady uberbrady force-pushed the postprocessing_middleware branch from 5b4ad1a to 7754ef4 Compare November 8, 2022 19:14
@uberbrady uberbrady merged commit 8eba81d into master Nov 9, 2022
if(config('scim.standards_compliance')) {
$response_content = json_decode($response->content());

if (!$response_content->totalResults) {
Copy link

@dmyers dmyers Nov 17, 2022

Choose a reason for hiding this comment

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

@uberbrady This throws an undefined property error for me when viewing a user resource (ex: /scim/v2/Users/1).

ErrorException: Undefined property: stdClass::$totalResults

I think it should be this:

if (!property_exists($response_content, 'totalResults')) {


if (!$response_content->totalResults) {
$response->setContent(json_encode(undefault_schema($response_content)));
} else {
Copy link

Choose a reason for hiding this comment

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

I'd recommend this else be changed to make sure the Resources property was in the response in case other routes in this middleware might cause undefined property to be accessed.

} else if (property_exists($response_content, 'Resources')) {

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.

3 participants