From c5be66978cd1ffae8f5e45aa0bf517a4e11b877a Mon Sep 17 00:00:00 2001 From: Arie Timmerman Date: Tue, 18 Nov 2025 21:26:28 +0000 Subject: [PATCH] Fix location header handling and add tests for successful resource creation #148 --- src/Helper.php | 34 +---------- src/Http/Controllers/ResourceController.php | 36 +++++++---- tests/HeaderTest.php | 66 +++++++++++++++++++++ 3 files changed, 94 insertions(+), 42 deletions(-) diff --git a/src/Helper.php b/src/Helper.php index 8fcb9fd..19bc130 100644 --- a/src/Helper.php +++ b/src/Helper.php @@ -59,7 +59,7 @@ public static function prepareReturn(Arrayable $object, ResourceType $resourceTy public static function objectToSCIMArray($object, ResourceType $resourceType = null, array $attributes = [], array $excludedAttributes = []) { - if($resourceType == null){ + if ($resourceType == null) { $result = $object instanceof Arrayable ? $object->toArray() : $object; if (is_array($result) && !empty($excludedAttributes)) { @@ -81,7 +81,7 @@ public static function objectToSCIMArray($object, ResourceType $resourceType = n // Move main schema to the top. It may not be defined, for example when only specific attributes are requested. $main = $result[$defaultSchema] ?? []; - + unset($result[$defaultSchema]); $result = array_merge($result, $main); @@ -106,8 +106,7 @@ public static function getResourceObjectVersion($object) } /** - * - * @param unknown $object + * @param Model $object * @param ResourceType $resourceType */ public static function objectToSCIMResponse(Model $object, ResourceType $resourceType = null, array $attributes = [], array $excludedAttributes = []) @@ -115,33 +114,6 @@ public static function objectToSCIMResponse(Model $object, ResourceType $resourc $response = response(self::objectToSCIMArray($object, $resourceType, $attributes, $excludedAttributes)) ->header('ETag', self::getResourceObjectVersion($object)); - if ($resourceType !== null) { - $resourceTypeName = $resourceType->getName(); - - if ($resourceTypeName === null) { - $routeResourceType = request()?->route('resourceType'); - - if ($routeResourceType instanceof ResourceType) { - $resourceTypeName = $routeResourceType->getName(); - } elseif (is_string($routeResourceType)) { - $resourceTypeName = $routeResourceType; - } - } - - if ($resourceTypeName !== null) { - $response->header( - 'Location', - route( - 'scim.resource', - [ - 'resourceType' => $resourceTypeName, - 'resourceObject' => $object->getKey(), - ] - ) - ); - } - } - return $response; } diff --git a/src/Http/Controllers/ResourceController.php b/src/Http/Controllers/ResourceController.php index a494a63..522df4f 100644 --- a/src/Http/Controllers/ResourceController.php +++ b/src/Http/Controllers/ResourceController.php @@ -103,7 +103,16 @@ public function create(Request $request, PolicyDecisionPoint $pdp, ResourceType { $resourceObject = $this->createObject($request, $pdp, $resourceType, $isMe); - return $this->respondWithResource($request, $resourceType, $resourceObject, 201); + return $this->respondWithResource($request, $resourceType, $resourceObject, 201)->header( + 'Location', + route( + 'scim.resource', + [ + 'resourceType' => $resourceType->getName(), + 'resourceObject' => $resourceObject->getKey(), + ] + ) + ); } public function show(Request $request, PolicyDecisionPoint $pdp, ResourceType $resourceType, Model $resourceObject) @@ -260,33 +269,33 @@ function (Builder $query) use ($filter, $resourceType) { if (!$cursorPaginationEnabled) { throw (new SCIMException('Cursor pagination is disabled.'))->setCode(400)->setScimType('invalidCursor'); } - if($sortBy == null){ + if ($sortBy == null) { $resourceObjects = $resourceObjects->orderBy('id'); } - if($request->input('cursor')){ + if ($request->input('cursor')) { $cursor = @Cursor::fromEncoded($request->input('cursor')); - if($cursor == null){ + if ($cursor == null) { throw (new SCIMException('Invalid Cursor'))->setCode(400)->setScimType('invalidCursor'); } } $countRaw = $request->input('count'); - if($countRaw < 1 || $countRaw > config('scim.pagination.maxPageSize')){ + if ($countRaw < 1 || $countRaw > config('scim.pagination.maxPageSize')) { throw (new SCIMException( sprintf('Count value is invalid. Count value must be between 1 - and maxPageSize (%s) (when using cursor pagination)', config('scim.pagination.maxPageSize')) ))->setCode(400)->setScimType('invalidCount'); } - + $resourceObjects = $resourceObjects->cursorPaginate( $count, cursor: $request->input('cursor') ); $resources = collect($resourceObjects->items()); - + } else { // The 1-based index of the first query result. A value less than 1 SHALL be interpreted as 1. $startIndex = max(1, intVal($request->input('startIndex', 0))); @@ -327,7 +336,8 @@ public function crossResourceIndex(Request $request, PolicyDecisionPoint $pdp, S return $this->runCrossResourceQuery($request, $config); } - public function search(Request $request, PolicyDecisionPoint $pdp, ResourceType $resourceType){ + public function search(Request $request, PolicyDecisionPoint $pdp, ResourceType $resourceType) + { $input = $request->json()->all(); @@ -466,8 +476,12 @@ protected function respondWithResource(Request $request, ResourceType $resourceT { [$attributes, $excludedAttributes] = $this->resolveAttributeParameters($request); - return Helper::objectToSCIMResponse($resourceObject, $resourceType, $attributes, $excludedAttributes) - ->setStatusCode($status); + return Helper::objectToSCIMResponse( + $resourceObject, + $resourceType, + $attributes, + $excludedAttributes + )->setStatusCode($status); } protected function resolveAttributeParameters(Request $request): array @@ -520,7 +534,7 @@ private function normalizeAttributeList($value): array if (is_string($value)) { $parts = preg_split('/\s*,\s*/', $value); - $normalized = array_filter(array_map('trim', $parts), fn ($item) => $item !== ''); + $normalized = array_filter(array_map('trim', $parts), fn($item) => $item !== ''); return array_values(array_unique($normalized)); } diff --git a/tests/HeaderTest.php b/tests/HeaderTest.php index 0765112..2af1695 100644 --- a/tests/HeaderTest.php +++ b/tests/HeaderTest.php @@ -55,4 +55,70 @@ public function testPut(){ ], ['If-Match' => $etag]); $response->assertStatus(200); } + + public function testLocationHeaderOnlyReturnedOnSuccessfulCreate() + { + $createResponse = $this->post('/scim/v2/Users', $this->validUserPayload()); + $createResponse->assertStatus(201); + $createResponse->assertHeader('Location'); + + $locationHeader = $createResponse->baseResponse->headers->get('Location'); + $this->assertMatchesRegularExpression('#^http://localhost/scim/v2/Users/\d+$#', $locationHeader); + + $userId = $createResponse->json('id'); + $this->assertNotEmpty($userId, 'Created SCIM resource is missing an id'); + + $getResponse = $this->get("/scim/v2/Users/{$userId}"); + $getResponse->assertStatus(200); + $getResponse->assertHeaderMissing('Location'); + $etag = $getResponse->baseResponse->headers->get('ETag'); + $this->assertNotEmpty($etag, 'GET response should expose an ETag for optimistic locking'); + + $putResponse = $this->put( + "/scim/v2/Users/{$userId}", + [ + 'schemas' => ['urn:ietf:params:scim:schemas:core:2.0:User'], + 'userName' => 'location-put-' . uniqid(), + 'id' => (string) $userId, + ], + ['If-Match' => $etag] + ); + $putResponse->assertStatus(200); + $putResponse->assertHeaderMissing('Location'); + + $patchResponse = $this->patch("/scim/v2/Users/{$userId}", [ + 'schemas' => ['urn:ietf:params:scim:api:messages:2.0:PatchOp'], + 'Operations' => [[ + 'op' => 'add', + 'path' => 'userName', + 'value' => 'location-patch-' . uniqid(), + ]], + ]); + $patchResponse->assertStatus(200); + $patchResponse->assertHeaderMissing('Location'); + + $deleteResponse = $this->delete("/scim/v2/Users/{$userId}"); + $deleteResponse->assertStatus(204); + $deleteResponse->assertHeaderMissing('Location'); + } + + protected function validUserPayload(?string $email = null): array + { + $email = $email ?? sprintf('location.%s@example.test', uniqid()); + + return [ + 'schemas' => [ + 'urn:ietf:params:scim:schemas:core:2.0:User', + ], + 'urn:ietf:params:scim:schemas:core:2.0:User' => [ + 'userName' => 'location-user-' . uniqid(), + 'password' => 'Password123', + 'emails' => [[ + 'value' => $email, + 'type' => 'work', + 'primary' => true, + ]], + ], + ]; + } }