From 6a695f467d55c21b11840048587b5827304c3e75 Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Tue, 11 Aug 2026 12:59:47 +0200 Subject: [PATCH 1/9] Changelog. --- .../WPB-27953-make-scim-error-responses-comply-with-rfc7644 | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/1-api-changes/WPB-27953-make-scim-error-responses-comply-with-rfc7644 diff --git a/changelog.d/1-api-changes/WPB-27953-make-scim-error-responses-comply-with-rfc7644 b/changelog.d/1-api-changes/WPB-27953-make-scim-error-responses-comply-with-rfc7644 new file mode 100644 index 0000000000..f03aeea44f --- /dev/null +++ b/changelog.d/1-api-changes/WPB-27953-make-scim-error-responses-comply-with-rfc7644 @@ -0,0 +1 @@ +Make scim error responses comply with RFC7644. From a6ba0e45c0f19753f736d6482aaf0ed5cd1c1b29 Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Tue, 11 Aug 2026 13:02:53 +0200 Subject: [PATCH 2/9] Fix scim error response bodies. --- services/spar/spar.cabal | 1 + services/spar/src/Spar/Error.hs | 8 ++++- services/spar/test/Test/Spar/ErrorSpec.hs | 44 +++++++++++++++++++++++ 3 files changed, 52 insertions(+), 1 deletion(-) create mode 100644 services/spar/test/Test/Spar/ErrorSpec.hs diff --git a/services/spar/spar.cabal b/services/spar/spar.cabal index 8bd4d03aac..962388000e 100644 --- a/services/spar/spar.cabal +++ b/services/spar/spar.cabal @@ -530,6 +530,7 @@ test-suite spec Paths_spar Test.Spar.APISpec Test.Spar.DataSpec + Test.Spar.ErrorSpec Test.Spar.Intra.BrigSpec Test.Spar.Roundtrip.ByteString Test.Spar.Saml.IdPSpec diff --git a/services/spar/src/Spar/Error.hs b/services/spar/src/Spar/Error.hs index f15cb5dfad..a2cd61d9f7 100644 --- a/services/spar/src/Spar/Error.hs +++ b/services/spar/src/Spar/Error.hs @@ -135,7 +135,13 @@ sparToServerErrorWithLogging logger err = do pure errServant sparToServerError :: SparError -> ServerError -sparToServerError = httpErrorToServerError . renderSparError +-- SCIM errors have their own response format (RFC 7644, section 3.12): the body +-- must be the bare SCIM error object. Going through 'renderSparError' / +-- 'httpErrorToServerError' would instead nest it into a wire-server 'Wai.Error' +-- ('{"code":..,"label":"scim-error","message":}'), so we +-- render it directly here. +sparToServerError (SAML.CustomError (SparScimError err)) = Scim.scimToServerError err +sparToServerError err = httpErrorToServerError (renderSparError err) waiToServant :: Wai.Error -> ServerError waiToServant waierr = diff --git a/services/spar/test/Test/Spar/ErrorSpec.hs b/services/spar/test/Test/Spar/ErrorSpec.hs new file mode 100644 index 0000000000..750d223948 --- /dev/null +++ b/services/spar/test/Test/Spar/ErrorSpec.hs @@ -0,0 +1,44 @@ +{-# LANGUAGE OverloadedStrings #-} + +-- This file is part of the Wire Server implementation. +-- +-- Copyright (C) 2025 Wire Swiss GmbH +-- +-- This program is free software: you can redistribute it and/or modify it under +-- the terms of the GNU Affero General Public License as published by the Free +-- Software Foundation, either version 3 of the License, or (at your option) any +-- later version. +-- +-- This program is distributed in the hope that it will be useful, but WITHOUT +-- ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS +-- FOR A PARTICULAR PURPOSE. See the GNU Affero General Public License for more +-- details. +-- +-- You should have received a copy of the GNU Affero General Public License along +-- with this program. If not, see . + +module Test.Spar.ErrorSpec where + +import Data.Aeson (encode) +import Imports +import qualified SAML2.WebSSO as SAML +import Servant (ServerError (..)) +import Spar.Error +import Test.Hspec +import qualified Web.Scim.Schema.Error as Scim + +spec :: Spec +spec = describe "sparToServerError" $ do + -- RFC 7644 section 3.12 requires that the response body of a SCIM error *is* + -- the SCIM error object, not a wire-server 'Wai.Error' with the SCIM error + -- object nested (double-encoded) into its 'message' field. + it "renders a SCIM error as the bare RFC 7644 error object" $ do + let scimErr = + Scim.badRequest + Scim.InvalidValue + (Just "Could not process externalId.") + serverErr = sparToServerError (SAML.CustomError (SparScimError scimErr)) + errBody serverErr `shouldBe` encode scimErr + errHTTPCode serverErr `shouldBe` 400 + lookup "Content-Type" (errHeaders serverErr) + `shouldBe` Just "application/scim+json;charset=utf-8" From 4317e39e655a0edf28b2dd119453cbac930ab922 Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Tue, 11 Aug 2026 13:13:14 +0200 Subject: [PATCH 3/9] More changelog noise. --- ...e-scim-error-responses-comply-with-rfc7644 | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 changelog.d/0-release-notes/WPB-27953-make-scim-error-responses-comply-with-rfc7644 diff --git a/changelog.d/0-release-notes/WPB-27953-make-scim-error-responses-comply-with-rfc7644 b/changelog.d/0-release-notes/WPB-27953-make-scim-error-responses-comply-with-rfc7644 new file mode 100644 index 0000000000..d769a62ddf --- /dev/null +++ b/changelog.d/0-release-notes/WPB-27953-make-scim-error-responses-comply-with-rfc7644 @@ -0,0 +1,22 @@ +Make SCIM error responses comply with RFC7644. Any code that processes SCIM error responses must be changed to follow the standard, instead of the previous Wire implementation. + +Previous schema (incompatible with RFC): + +``` +{ + "code": 400, + "label": "scim-error", + "message": "{\"detail\":\"[...]\",\"schemas\":[\"urn:ietf:params:scim:api:messages:2.0:Error\"],\"scimType\":\"invalidValue\",\"status\":\"400\"}" +} +``` + +New schema (RFC-compliant): + +``` +{ + "schemas": ["urn:ietf:params:scim:api:messages:2.0:Error"], + "status": "400" + "scimType": "invalidValue", + "detail": "[...]", +} +``` From 7af0b9ab3ee92690042907ea0776689c07ec18ed Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Wed, 12 Aug 2026 09:28:28 +0200 Subject: [PATCH 4/9] Make unit test more interesting. --- services/spar/test/Test/Spar/ErrorSpec.hs | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/services/spar/test/Test/Spar/ErrorSpec.hs b/services/spar/test/Test/Spar/ErrorSpec.hs index 750d223948..1f0b697ab5 100644 --- a/services/spar/test/Test/Spar/ErrorSpec.hs +++ b/services/spar/test/Test/Spar/ErrorSpec.hs @@ -19,7 +19,8 @@ module Test.Spar.ErrorSpec where -import Data.Aeson (encode) +import Data.Aeson (eitherDecode') +import Data.Aeson.QQ (aesonQQ) import Imports import qualified SAML2.WebSSO as SAML import Servant (ServerError (..)) @@ -38,7 +39,17 @@ spec = describe "sparToServerError" $ do Scim.InvalidValue (Just "Could not process externalId.") serverErr = sparToServerError (SAML.CustomError (SparScimError scimErr)) - errBody serverErr `shouldBe` encode scimErr + eitherDecode' (errBody serverErr) + `shouldBe` Right + [aesonQQ| + { + "detail": "Could not process externalId.", + "schemas": [ + "urn:ietf:params:scim:api:messages:2.0:Error" + ], + "scimType": "invalidValue", + "status": "400" + }|] errHTTPCode serverErr `shouldBe` 400 lookup "Content-Type" (errHeaders serverErr) `shouldBe` Just "application/scim+json;charset=utf-8" From 265d3568cd04c0a9aaecf5e6232c06b94308b404 Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Thu, 13 Aug 2026 11:47:55 +0200 Subject: [PATCH 5/9] Update services/spar/test/Test/Spar/ErrorSpec.hs Co-authored-by: Sven Tennie --- services/spar/test/Test/Spar/ErrorSpec.hs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/spar/test/Test/Spar/ErrorSpec.hs b/services/spar/test/Test/Spar/ErrorSpec.hs index 1f0b697ab5..ebfba25b44 100644 --- a/services/spar/test/Test/Spar/ErrorSpec.hs +++ b/services/spar/test/Test/Spar/ErrorSpec.hs @@ -2,7 +2,7 @@ -- This file is part of the Wire Server implementation. -- --- Copyright (C) 2025 Wire Swiss GmbH +-- Copyright (C) 2026 Wire Swiss GmbH -- -- This program is free software: you can redistribute it and/or modify it under -- the terms of the GNU Affero General Public License as published by the Free From 749ca4e8b788a6ba2dbbc51e4d6efba27f0151fb Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Thu, 13 Aug 2026 13:40:30 +0200 Subject: [PATCH 6/9] Integration tests. --- .../Test/Spar/Scim/UserSpec.hs | 110 +++++++++++++++--- 1 file changed, 93 insertions(+), 17 deletions(-) diff --git a/services/spar/test-integration/Test/Spar/Scim/UserSpec.hs b/services/spar/test-integration/Test/Spar/Scim/UserSpec.hs index 545f1ff897..801421c1fb 100644 --- a/services/spar/test-integration/Test/Spar/Scim/UserSpec.hs +++ b/services/spar/test-integration/Test/Spar/Scim/UserSpec.hs @@ -36,7 +36,6 @@ import Control.Monad.Except (MonadError (throwError)) import Control.Monad.Random (randomRIO) import Control.Monad.Trans.Except import Control.Monad.Trans.Maybe -import qualified Data.Aeson import qualified Data.Aeson as Aeson import Data.Aeson.Lens (key, _String) import Data.Aeson.QQ (aesonQQ) @@ -56,7 +55,6 @@ import Data.Text.Encoding (decodeUtf8, encodeUtf8) import qualified Data.Vector as V import qualified Data.ZAuth.Token as ZAuth import Imports -import qualified Network.Wai.Utilities.Error as Wai import Polysemy import Polysemy.Error import qualified SAML2.WebSSO as SAML @@ -652,9 +650,17 @@ testCreateUserWithPass = do user <- randomScimUser <&> \u -> u {Scim.User.password = Just "geheim"} createUser_ (Just tok) user (env ^. teSpar) !!! do const 400 === statusCode - -- TODO: write a FAQ entry in wire-docs, reference it in the error description. - -- TODO: yes, we should just test for error labels consistently, i know... - const (Just "Setting user passwords is not supported for security reasons.") =~= responseBody + mkScimErrorResp + [aesonQQ| + { + "detail": "Setting user passwords is not supported for security reasons. (post)", + "schemas": [ + "urn:ietf:params:scim:api:messages:2.0:Error" + ], + "scimType": "invalidValue", + "status": "400" + }|] + === responseBody testCreateUserNoIdPInvalidRoles :: TestSpar () testCreateUserNoIdPInvalidRoles = do @@ -854,6 +860,17 @@ testCreateUserNoIdPNoEmail = do user <- randomScimUser <&> \u -> u {Scim.User.externalId = Just "notanemail"} createUser_ (Just tok) user (env ^. teSpar) !!! do const 400 === statusCode + mkScimErrorResp + [aesonQQ| + { + "detail": "Could not process externalId. Please check: (1) does the scim user contain a valid email address? (2) did you associate your scim token with a SAML IdP in wire?", + "schemas": [ + "urn:ietf:params:scim:api:messages:2.0:Error" + ], + "scimType": "invalidValue", + "status": "400" + }|] + === responseBody testCreateUserWithSamlIdP :: TestSpar () testCreateUserWithSamlIdP = do @@ -937,8 +954,19 @@ testExternalIdIsRequired = do user <- randomScimUser let user' = user {Scim.User.externalId = Nothing} (tok, _) <- registerIdPAndScimToken - createUser_ (Just tok) user' (env ^. teSpar) - !!! const 400 === statusCode + createUser_ (Just tok) user' (env ^. teSpar) !!! do + const 400 === statusCode + mkScimErrorResp + [aesonQQ| + { + "detail": "externalId is required", + "schemas": [ + "urn:ietf:params:scim:api:messages:2.0:Error" + ], + "scimType": "invalidValue", + "status": "400" + }|] + === responseBody -- The next line contains a mapping from this test to the following test standards: -- @SF.Provisioning @TSFI.RESTfulAPI @S2 @@ -967,11 +995,34 @@ testCreateRejectsTakenHandle = do -- Create and add a first user: success! _ <- createUser tokTeamA user1 -- Try to create different user with same handle in same team. - createUser_ (Just tokTeamA) (user2 {Scim.User.userName = Scim.User.userName user1}) (env ^. teSpar) - !!! const 409 === statusCode + createUser_ (Just tokTeamA) (user2 {Scim.User.userName = Scim.User.userName user1}) (env ^. teSpar) !!! do + const 409 === statusCode + mkScimErrorResp + [aesonQQ| + { + "detail": "userName is already taken", + "schemas": [ + "urn:ietf:params:scim:api:messages:2.0:Error" + ], + "scimType": "uniqueness", + "status": "409" + }|] + === responseBody + -- Try to create different user with same handle in different team. - createUser_ (Just tokTeamB) (user3 {Scim.User.userName = Scim.User.userName user1}) (env ^. teSpar) - !!! const 409 === statusCode + createUser_ (Just tokTeamB) (user3 {Scim.User.userName = Scim.User.userName user1}) (env ^. teSpar) !!! do + const 409 === statusCode + mkScimErrorResp + [aesonQQ| + { + "detail": "userName is already taken", + "schemas": [ + "urn:ietf:params:scim:api:messages:2.0:Error" + ], + "scimType": "uniqueness", + "status": "409" + }|] + === responseBody -- | Test that user creation fails if the @externalId@ is already in use for given IdP. testCreateRejectsTakenExternalId :: Bool -> TestSpar () @@ -1586,8 +1637,18 @@ testUserGetFailsWithNotFoundIfOutsideTeam = do (tokTeamB, _) <- registerIdPAndScimToken storedUser <- createUser tokTeamA user let userid = scimUserId storedUser - getUser_ (Just tokTeamB) userid (env ^. teSpar) - !!! const 404 === statusCode + getUser_ (Just tokTeamB) userid (env ^. teSpar) !!! do + const 404 === statusCode + mkScimErrorResp + [aesonQQ| + { + "detail": #{"User " <> idToText userid <> " not found"}, + "schemas": [ + "urn:ietf:params:scim:api:messages:2.0:Error" + ], + "status": "404" + }|] + === responseBody {- does not find a non-scim-provisioned user: @@ -2172,7 +2233,7 @@ testPatchIvalidInput patchOp = do PatchOp.Operation PatchOp.Replace (Just (PatchOp.NormalPath (Filter.topLevelAttrPath "roles"))) - (Just $ Data.Aeson.Array $ V.singleton $ Data.Aeson.String "invalid-role") + (Just $ Aeson.Array $ V.singleton $ Aeson.String "invalid-role") patchUser' tok uid (PatchOp.PatchOp [patchWithInvalidRole]) !!! do const 400 === statusCode const (Just "The role 'invalid-role' is not valid. Valid roles are owner, admin, member, partner.") =~= responseBody @@ -2494,27 +2555,39 @@ specSCIMManaged = do resp <- call $ post (brig . path "/access" . forceCookie cky) resp -> Maybe LByteString + expectedResponseBody updatedThing = + -- NB: this is for requests to brig, not scim requests, so + -- the code-label-message schema is legit! + mkScimErrorResp + [aesonQQ| + { + "code": 403, + "label": "managed-by-scim", + "message": #{"Updating " <> updatedThing <> " is not allowed, because it is managed by SCIM, or E2EId is enabled"} + }|] do newEmail <- randomEmail call $ changeEmailBrigCreds brig cky sessiontok newEmail !!! do - (fmap Wai.label . responseJsonEither @Wai.Error) === const (Right "managed-by-scim") statusCode === const 403 + expectedResponseBody "email" === responseBody do handleTxt <- randomAlphaNum call $ changeHandleBrig brig uid handleTxt !!! do - (fmap Wai.label . responseJsonEither @Wai.Error) === const (Right "managed-by-scim") statusCode === const 403 + expectedResponseBody "handle" === responseBody do displayName <- Name <$> randomAlphaNum let uupd = UserUpdate (Just displayName) Nothing Nothing Nothing Nothing call $ updateProfileBrig brig uid uupd !!! do - (fmap Wai.label . responseJsonEither @Wai.Error) === const (Right "managed-by-scim") statusCode === const 403 + expectedResponseBody "name" === responseBody + it "created_on should be filled in CSV export" $ do g <- view teGalley user <- randomScimUser @@ -2581,3 +2654,6 @@ executeTeamUserSearch brig teamid self mbSearchText = >= fmap Search.searchResults . responseJsonError + +mkScimErrorResp :: Aeson.Value -> resp -> Maybe LByteString +mkScimErrorResp val _ = Just (Aeson.encode val) From af00e70b31d70954222f9e1721cc88e242b354ee Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Thu, 13 Aug 2026 14:23:56 +0200 Subject: [PATCH 7/9] More integration test coverage. --- .../test-integration/Test/Spar/APISpec.hs | 24 ++- .../Test/Spar/Scim/UserSpec.hs | 203 ++++++++++-------- services/spar/test-integration/Util/Scim.hs | 35 +++ 3 files changed, 168 insertions(+), 94 deletions(-) diff --git a/services/spar/test-integration/Test/Spar/APISpec.hs b/services/spar/test-integration/Test/Spar/APISpec.hs index 315d798378..a7bc4deb44 100644 --- a/services/spar/test-integration/Test/Spar/APISpec.hs +++ b/services/spar/test-integration/Test/Spar/APISpec.hs @@ -1369,7 +1369,11 @@ specProvisionScimAndSAMLUserWithRole = do pure $ u {Scim.roles = ["member", "admin"]} ScimT.createUser' tok scimUser !!! do const 400 === statusCode - const (Just "A user cannot have more than one role.") =~= responseBody + ScimT.mkScimErrorResp + (Just "A user cannot have more than one role.") + (Just "invalidValue") + "400" + === responseBody it "create user - fail if role name cannot be parsed correctly" $ do (tok, _) <- ScimT.registerIdPAndScimTokenWithMeta scimUser <- do @@ -1377,7 +1381,11 @@ specProvisionScimAndSAMLUserWithRole = do pure $ u {Scim.roles = ["president"]} ScimT.createUser' tok scimUser !!! do const 400 === statusCode - const (Just "The role 'president' is not valid. Valid roles are owner, admin, member, partner.") =~= responseBody + ScimT.mkScimErrorResp + (Just "The role 'president' is not valid. Valid roles are owner, admin, member, partner.") + (Just "invalidValue") + "400" + === responseBody it "update user" $ do (tok, (owner, tid, _idp, (_, _privcreds))) <- ScimT.registerIdPAndScimTokenWithMeta scimUserWithDefaultRole <- ScimT.randomScimUser @@ -1404,14 +1412,22 @@ specProvisionScimAndSAMLUserWithRole = do uid <- ScimT.scimUserId <$> ScimT.createUser tok scimUser ScimT.updateUser' tok uid (scimUser {Scim.roles = ["admin", "member"]}) !!! do const 400 === statusCode - const (Just "A user cannot have more than one role.") =~= responseBody + ScimT.mkScimErrorResp + (Just "A user cannot have more than one role.") + (Just "invalidValue") + "400" + === responseBody it "updated user - fail if role name cannot be parsed correctly" $ do (tok, _) <- ScimT.registerIdPAndScimTokenWithMeta scimUser <- ScimT.randomScimUser uid <- ScimT.scimUserId <$> ScimT.createUser tok scimUser ScimT.updateUser' tok uid (scimUser {Scim.roles = ["hamlet"]}) !!! do const 400 === statusCode - const (Just "The role 'hamlet' is not valid. Valid roles are owner, admin, member, partner.") =~= responseBody + ScimT.mkScimErrorResp + (Just "The role 'hamlet' is not valid. Valid roles are owner, admin, member, partner.") + (Just "invalidValue") + "400" + === responseBody specAux :: SpecWith TestEnv specAux = do diff --git a/services/spar/test-integration/Test/Spar/Scim/UserSpec.hs b/services/spar/test-integration/Test/Spar/Scim/UserSpec.hs index 801421c1fb..5f94fd688f 100644 --- a/services/spar/test-integration/Test/Spar/Scim/UserSpec.hs +++ b/services/spar/test-integration/Test/Spar/Scim/UserSpec.hs @@ -651,15 +651,9 @@ testCreateUserWithPass = do createUser_ (Just tok) user (env ^. teSpar) !!! do const 400 === statusCode mkScimErrorResp - [aesonQQ| - { - "detail": "Setting user passwords is not supported for security reasons. (post)", - "schemas": [ - "urn:ietf:params:scim:api:messages:2.0:Error" - ], - "scimType": "invalidValue", - "status": "400" - }|] + (Just "Setting user passwords is not supported for security reasons. (post)") + (Just "invalidValue") + "400" === responseBody testCreateUserNoIdPInvalidRoles :: TestSpar () @@ -678,7 +672,11 @@ testCreateUserNoIdPInvalidRoles = do } createUser' tok scimUserTooManyRoles !!! do const 400 === statusCode - const (Just "A user cannot have more than one role.") =~= responseBody + mkScimErrorResp + (Just "A user cannot have more than one role.") + (Just "invalidValue") + "400" + === responseBody scimUserInvalidRole <- randomScimUser <&> \u -> u @@ -687,7 +685,11 @@ testCreateUserNoIdPInvalidRoles = do } createUser' tok scimUserInvalidRole !!! do const 400 === statusCode - const (Just "The role 'foobar' is not valid. Valid roles are owner, admin, member, partner.") =~= responseBody + mkScimErrorResp + (Just "The role 'foobar' is not valid. Valid roles are owner, admin, member, partner.") + (Just "invalidValue") + "400" + === responseBody testCreateUserNoIdPWithRoles :: TestSpar () testCreateUserNoIdPWithRoles = do @@ -861,15 +863,9 @@ testCreateUserNoIdPNoEmail = do createUser_ (Just tok) user (env ^. teSpar) !!! do const 400 === statusCode mkScimErrorResp - [aesonQQ| - { - "detail": "Could not process externalId. Please check: (1) does the scim user contain a valid email address? (2) did you associate your scim token with a SAML IdP in wire?", - "schemas": [ - "urn:ietf:params:scim:api:messages:2.0:Error" - ], - "scimType": "invalidValue", - "status": "400" - }|] + (Just "Could not process externalId. Please check: (1) does the scim user contain a valid email address? (2) did you associate your scim token with a SAML IdP in wire?") + (Just "invalidValue") + "400" === responseBody testCreateUserWithSamlIdP :: TestSpar () @@ -957,15 +953,9 @@ testExternalIdIsRequired = do createUser_ (Just tok) user' (env ^. teSpar) !!! do const 400 === statusCode mkScimErrorResp - [aesonQQ| - { - "detail": "externalId is required", - "schemas": [ - "urn:ietf:params:scim:api:messages:2.0:Error" - ], - "scimType": "invalidValue", - "status": "400" - }|] + (Just "externalId is required") + (Just "invalidValue") + "400" === responseBody -- The next line contains a mapping from this test to the following test standards: @@ -978,8 +968,10 @@ testCreateRejectsInvalidHandle = do -- Create a user via SCIM user <- randomScimUser (tok, _) <- registerIdPAndScimToken - createUser_ (Just tok) (user {Scim.User.userName = "#invalid name"}) (env ^. teSpar) - !!! const 400 === statusCode + createUser_ (Just tok) (user {Scim.User.userName = "#invalid name"}) (env ^. teSpar) !!! do + const 400 === statusCode + mkScimErrorResp Nothing (Just "invalidValue") "400" + === responseBody -- @END @@ -998,30 +990,18 @@ testCreateRejectsTakenHandle = do createUser_ (Just tokTeamA) (user2 {Scim.User.userName = Scim.User.userName user1}) (env ^. teSpar) !!! do const 409 === statusCode mkScimErrorResp - [aesonQQ| - { - "detail": "userName is already taken", - "schemas": [ - "urn:ietf:params:scim:api:messages:2.0:Error" - ], - "scimType": "uniqueness", - "status": "409" - }|] + (Just "userName is already taken") + (Just "uniqueness") + "409" === responseBody -- Try to create different user with same handle in different team. createUser_ (Just tokTeamB) (user3 {Scim.User.userName = Scim.User.userName user1}) (env ^. teSpar) !!! do const 409 === statusCode mkScimErrorResp - [aesonQQ| - { - "detail": "userName is already taken", - "schemas": [ - "urn:ietf:params:scim:api:messages:2.0:Error" - ], - "scimType": "uniqueness", - "status": "409" - }|] + (Just "userName is already taken") + (Just "uniqueness") + "409" === responseBody -- | Test that user creation fails if the @externalId@ is already in use for given IdP. @@ -1044,8 +1024,10 @@ testCreateRejectsTakenExternalId withidp = do _ <- createUser tok user1 -- Try to create different user with same @externalId@ in same team, and fail. user2 <- randomScimUser - createUser_ (Just tok) (user2 {Scim.User.externalId = Scim.User.externalId user1}) (env ^. teSpar) - !!! const 409 === statusCode + createUser_ (Just tok) (user2 {Scim.User.externalId = Scim.User.externalId user1}) (env ^. teSpar) !!! do + const 409 === statusCode + mkScimErrorResp Nothing (Just "uniqueness") "409" + === responseBody -- | Test that it's fine to have same @externalId@s for two users belonging to different IdPs. testCreateSameExternalIds :: TestSpar () @@ -1331,7 +1313,8 @@ testListProvisionedUsers = do (tok, _) <- registerIdPAndScimToken listUsers_ (Just tok) Nothing spar !!! do const 400 === statusCode - const (Just "tooMany") =~= responseBody + mkScimErrorResp Nothing (Just "tooMany") "400" + === responseBody testFindProvisionedUser :: TestSpar () testFindProvisionedUser = do @@ -1623,8 +1606,10 @@ testGetNoDeletedUsers = do -- Delete the user call $ deleteUserOnBrig (env ^. teBrig) userid -- Try to find the user - getUser_ (Just tok) userid (env ^. teSpar) - !!! const 404 === statusCode + getUser_ (Just tok) userid (env ^. teSpar) !!! do + const 404 === statusCode + mkScimErrorResp Nothing Nothing "404" + === responseBody -- TODO(arianvp): What does this mean; @fisx ?? pendingWith "TODO: delete via SCIM" @@ -1640,14 +1625,9 @@ testUserGetFailsWithNotFoundIfOutsideTeam = do getUser_ (Just tokTeamB) userid (env ^. teSpar) !!! do const 404 === statusCode mkScimErrorResp - [aesonQQ| - { - "detail": #{"User " <> idToText userid <> " not found"}, - "schemas": [ - "urn:ietf:params:scim:api:messages:2.0:Error" - ], - "status": "404" - }|] + (Just ("User " <> idToText userid <> " not found")) + Nothing + "404" === responseBody {- does not find a non-scim-provisioned user: @@ -1755,8 +1735,10 @@ testUserUpdateFailsWithNotFoundIfOutsideTeam = do let userid = scimUserId storedUser -- Overwrite the user with another randomly-generated user user' <- randomScimUser - updateUser_ (Just tokTeamB) (Just userid) user' (env ^. teSpar) - !!! const 404 === statusCode + updateUser_ (Just tokTeamB) (Just userid) user' (env ^. teSpar) !!! do + const 404 === statusCode + mkScimErrorResp Nothing Nothing "404" + === responseBody -- | Test that @PUT@-ting the user and then @GET@-ting it returns the right thing. testScimSideIsUpdated :: TestSpar () @@ -1808,8 +1790,10 @@ testUpdateToExistingExternalIdFails = do env <- ask -- Should fail with 409 to denote that the given externalId is in use by a -- different user. - updateUser_ (Just tok) (Just $ scimUserId storedNewUser) updatedNewUser (env ^. teSpar) - !!! const 409 === statusCode + updateUser_ (Just tok) (Just $ scimUserId storedNewUser) updatedNewUser (env ^. teSpar) !!! do + const 409 === statusCode + mkScimErrorResp Nothing (Just "uniqueness") "409" + === responseBody -- | Test that updating still works when name and handle are not changed. -- @@ -2107,7 +2091,10 @@ specPatchUser = do let userid = scimUserId storedUser _ <- createUser tok user' let patchOp = PatchOp.PatchOp [replaceAttrib "userName" (Scim.User.userName user')] - patchUser_ (Just tok) (Just userid) patchOp (env ^. teSpar) !!! const 409 === statusCode + patchUser_ (Just tok) (Just userid) patchOp (env ^. teSpar) !!! do + const 409 === statusCode + mkScimErrorResp Nothing (Just "uniqueness") "409" + === responseBody it "can't update to someone else's externalId" $ do env <- ask (tok, _) <- registerIdPAndScimToken @@ -2117,13 +2104,19 @@ specPatchUser = do let userid = scimUserId storedUser _ <- createUser tok user' let patchOp = PatchOp.PatchOp [replaceAttrib "externalId" (Scim.User.externalId user')] - patchUser_ (Just tok) (Just userid) patchOp (env ^. teSpar) !!! const 409 === statusCode + patchUser_ (Just tok) (Just userid) patchOp (env ^. teSpar) !!! do + const 409 === statusCode + mkScimErrorResp Nothing (Just "uniqueness") "409" + === responseBody it "can't update a non-existing user" $ do env <- ask (tok, _) <- registerIdPAndScimToken userid <- liftIO $ randomId let patchOp = PatchOp.PatchOp [replaceAttrib "externalId" ("blah" :: Text)] - patchUser_ (Just tok) (Just userid) patchOp (env ^. teSpar) !!! const 404 === statusCode + patchUser_ (Just tok) (Just userid) patchOp (env ^. teSpar) !!! do + const 404 === statusCode + mkScimErrorResp Nothing Nothing "404" + === responseBody it "can update displayName" $ do (tok, _) <- registerIdPAndScimToken user <- randomScimUser @@ -2181,8 +2174,10 @@ specPatchUser = do let userid = scimUserId storedUser env <- ask let patchOp = PatchOp.PatchOp [replaceAttrib "emails" ("hello" :: Text)] - patchUser_ (Just tok) (Just userid) patchOp (env ^. teSpar) - !!! const 400 === statusCode + patchUser_ (Just tok) (Just userid) patchOp (env ^. teSpar) !!! do + const 400 === statusCode + mkScimErrorResp Nothing (Just "invalidValue") "400" + === responseBody it "invalid attributes are quietly ignored for now" $ do (tok, _) <- registerIdPAndScimToken user <- randomScimUser @@ -2190,8 +2185,10 @@ specPatchUser = do let userid = scimUserId storedUser env <- ask let patchOp = PatchOp.PatchOp [replaceAttrib "totallyBogus" ("hello" :: Text)] - patchUser_ (Just tok) (Just userid) patchOp (env ^. teSpar) - !!! const 400 === statusCode + patchUser_ (Just tok) (Just userid) patchOp (env ^. teSpar) !!! do + const 400 === statusCode + mkScimErrorResp Nothing (Just "invalidValue") "400" + === responseBody -- NOTE: Remove at the moment actually never works! As all the fields -- we support are required in our book it "userName cannot be removed according to scim" $ do @@ -2201,7 +2198,10 @@ specPatchUser = do storedUser <- createUser tok user let userid = scimUserId storedUser let patchOp = PatchOp.PatchOp [removeAttrib "userName"] - patchUser_ (Just tok) (Just userid) patchOp (env ^. teSpar) !!! const 400 === statusCode + patchUser_ (Just tok) (Just userid) patchOp (env ^. teSpar) !!! do + const 400 === statusCode + mkScimErrorResp Nothing (Just "invalidValue") "400" + === responseBody it "displayName cannot be removed in spar (though possible in scim). Diplayname is required in Wire" $ do pendingWith "We default to the externalId when displayName is removed. lets keep that for now" @@ -2219,7 +2219,10 @@ specPatchUser = do storedUser <- createUser tok user let userid = scimUserId storedUser let patchOp = PatchOp.PatchOp [removeAttrib "externalId"] - patchUser_ (Just tok) (Just userid) patchOp (env ^. teSpar) !!! const 400 === statusCode + patchUser_ (Just tok) (Just userid) patchOp (env ^. teSpar) !!! do + const 400 === statusCode + mkScimErrorResp Nothing (Just "invalidValue") "400" + === responseBody testPatchIvalidInput :: (Text -> [Role] -> Operation) -> TestSpar () testPatchIvalidInput patchOp = do @@ -2236,11 +2239,19 @@ testPatchIvalidInput patchOp = do (Just $ Aeson.Array $ V.singleton $ Aeson.String "invalid-role") patchUser' tok uid (PatchOp.PatchOp [patchWithInvalidRole]) !!! do const 400 === statusCode - const (Just "The role 'invalid-role' is not valid. Valid roles are owner, admin, member, partner.") =~= responseBody + mkScimErrorResp + (Just "The role 'invalid-role' is not valid. Valid roles are owner, admin, member, partner.") + (Just "invalidValue") + "400" + === responseBody let patchWithTooManyRoles = patchOp "roles" [defaultRole, defaultRole] patchUser' tok uid (PatchOp.PatchOp [patchWithTooManyRoles]) !!! do const 400 === statusCode - const (Just "A user cannot have more than one role.") =~= responseBody + mkScimErrorResp + (Just "A user cannot have more than one role.") + (Just "invalidValue") + "400" + === responseBody testPatchRole :: (Text -> [Role] -> Operation) -> TestSpar () testPatchRole replaceOrAdd = do @@ -2294,8 +2305,10 @@ specDeleteUser = do it "responds with 405 (just making sure...)" $ do env <- ask (tok, _) <- registerIdPAndScimToken - deleteUser_ (Just tok) Nothing (env ^. teSpar) - !!! const 405 === statusCode + deleteUser_ (Just tok) Nothing (env ^. teSpar) !!! do + const 405 === statusCode + mkScimErrorResp Nothing Nothing "405" + === responseBody describe "DELETE /Users/:id" $ do it "should delete user from brig, spar.scim_user_times, spar.user" $ do (tok, (_, _, idp)) <- registerIdPAndScimToken @@ -2353,8 +2366,10 @@ specDeleteUser = do storedUser <- createUser tok user spar <- view teSpar let uid = scimUserId storedUser - deleteUser_ Nothing (Just uid) spar - !!! const 401 === statusCode + deleteUser_ Nothing (Just uid) spar !!! do + const 401 === statusCode + mkScimErrorResp Nothing Nothing "401" + === responseBody it "should always pretend to succeed, even if user exists in other team (does not leak information by diverging behavior)" $ do (tok, _) <- registerIdPAndScimToken user <- randomScimUser @@ -2374,8 +2389,10 @@ specDeleteUser = do let uid = scimUserId storedUser deleteUser_ (Just tok) (Just uid) spar !!! const 204 === statusCode - aFewTimes (getUser_ (Just tok) uid spar) ((== 404) . statusCode) - !!! const 404 === statusCode + aFewTimes (getUser_ (Just tok) uid spar) ((== 404) . statusCode) !!! do + const 404 === statusCode + mkScimErrorResp Nothing Nothing "404" + === responseBody deleteUser_ (Just tok) (Just uid) spar !!! const 204 === statusCode it "whether implemented or not, does *NOT EVER* respond with 5xx!" $ do @@ -2409,8 +2426,10 @@ specDeleteUser = do deleteUser_ (Just tok) (Just uid) spar !!! const 204 === statusCode - aFewTimes (getUser_ (Just tok) uid spar) ((== 404) . statusCode) - !!! const 404 === statusCode + aFewTimes (getUser_ (Just tok) uid spar) ((== 404) . statusCode) !!! do + const 404 === statusCode + mkScimErrorResp Nothing Nothing "404" + === responseBody context "user not touched via scim before" $ do it "works" $ do @@ -2429,8 +2448,10 @@ specDeleteUser = do !!! const 200 === statusCode deleteUser_ (Just tok) (Just uid) spar !!! const 204 === statusCode - aFewTimes (getUser_ (Just tok) uid spar) ((== 404) . statusCode) - !!! const 404 === statusCode + aFewTimes (getUser_ (Just tok) uid spar) ((== 404) . statusCode) !!! do + const 404 === statusCode + mkScimErrorResp Nothing Nothing "404" + === responseBody context "No IDP" $ do describe "Deleting a User" $ do @@ -2559,7 +2580,7 @@ specSCIMManaged = do expectedResponseBody updatedThing = -- NB: this is for requests to brig, not scim requests, so -- the code-label-message schema is legit! - mkScimErrorResp + mkBrigErrorResp [aesonQQ| { "code": 403, @@ -2655,5 +2676,7 @@ executeTeamUserSearch brig teamid self mbSearchText = === statusCode >>= fmap Search.searchResults . responseJsonError -mkScimErrorResp :: Aeson.Value -> resp -> Maybe LByteString -mkScimErrorResp val _ = Just (Aeson.encode val) +-- | Assert the exact body of an error response from an endpoint that is /not/ scim: brig has +-- its own code-label-message error schema. For scim errors use 'mkScimErrorResp'. +mkBrigErrorResp :: Aeson.Value -> resp -> Maybe LByteString +mkBrigErrorResp val _ = Just (Aeson.encode val) diff --git a/services/spar/test-integration/Util/Scim.hs b/services/spar/test-integration/Util/Scim.hs index 9f8d1b1c02..bd49b8236a 100644 --- a/services/spar/test-integration/Util/Scim.hs +++ b/services/spar/test-integration/Util/Scim.hs @@ -23,8 +23,11 @@ module Util.Scim where import Bilge import Bilge.Assert +import qualified Control.Exception import Control.Lens import Control.Monad.Random +import qualified Data.Aeson as Aeson +import Data.Aeson.Lens (key, _String) import Data.ByteString.Conversion import qualified Data.ByteString.Lazy as Lazy import Data.Handle (Handle, parseHandle) @@ -757,3 +760,35 @@ checkTeamMembersRole :: (HasCallStack) => TeamId -> UserId -> UserId -> Role -> checkTeamMembersRole tid owner uid role = do [member] <- filter ((== uid) . (^. Member.userId)) <$> getTeamMembers owner tid liftIO $ (member ^. Member.permissions . to Member.permissionsRole) `shouldBe` Just role + +-- | Assert the body of a scim error response (rfc7644, section 3.12). The arguments are the +-- @detail@, @scimType@ and @status@ fields; @schemas@ is the same for every scim error and is +-- filled in here. +-- +-- Both 'Maybe' arguments mean different things when they are 'Nothing': +-- +-- * @detail@: the message is /not tested/. Whatever the response contains is echoed into the +-- expected value, so this field can never make the comparison fail. +-- +-- * @scimType@: the key must be /absent/. That is what the server does when there is no error +-- type, as opposed to rendering it as @null@. +-- +-- > deleteUser_ Nothing (Just uid) spar !!! do +-- > const 401 === statusCode +-- > mkScimErrorResp Nothing Nothing "401" === responseBody +mkScimErrorResp :: + Maybe Text -> + Maybe Text -> + Text -> + Response (Maybe LByteString) -> + Maybe LByteString +mkScimErrorResp mDetail scimType status resp = + Just . Aeson.encode . Aeson.object . catMaybes $ + [ ("detail" Aeson..=) <$> (mDetail <|> (Control.Exception.assert (isJust actualDetail) actualDetail)), + Just ("schemas" Aeson..= ["urn:ietf:params:scim:api:messages:2.0:Error" :: Text]), + ("scimType" Aeson..=) <$> scimType, + Just ("status" Aeson..= status) + ] + where + actualDetail :: Maybe Text + actualDetail = responseBody resp >>= (^? key "detail" . _String) From b9a46f0fa51d975b8c41d319ba9c8f582dec982e Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Thu, 13 Aug 2026 14:53:19 +0200 Subject: [PATCH 8/9] Fixups --- .../test-integration/Test/Spar/APISpec.hs | 8 ++--- .../Test/Spar/Scim/UserSpec.hs | 32 +++++++++++++------ services/spar/test-integration/Util/Scim.hs | 3 +- 3 files changed, 28 insertions(+), 15 deletions(-) diff --git a/services/spar/test-integration/Test/Spar/APISpec.hs b/services/spar/test-integration/Test/Spar/APISpec.hs index a7bc4deb44..cfa22e96fc 100644 --- a/services/spar/test-integration/Test/Spar/APISpec.hs +++ b/services/spar/test-integration/Test/Spar/APISpec.hs @@ -1370,7 +1370,7 @@ specProvisionScimAndSAMLUserWithRole = do ScimT.createUser' tok scimUser !!! do const 400 === statusCode ScimT.mkScimErrorResp - (Just "A user cannot have more than one role.") + (Just "A user cannot have more than one role. (post)") (Just "invalidValue") "400" === responseBody @@ -1382,7 +1382,7 @@ specProvisionScimAndSAMLUserWithRole = do ScimT.createUser' tok scimUser !!! do const 400 === statusCode ScimT.mkScimErrorResp - (Just "The role 'president' is not valid. Valid roles are owner, admin, member, partner.") + (Just "The role 'president' is not valid. Valid roles are owner, admin, member, partner. (post)") (Just "invalidValue") "400" === responseBody @@ -1413,7 +1413,7 @@ specProvisionScimAndSAMLUserWithRole = do ScimT.updateUser' tok uid (scimUser {Scim.roles = ["admin", "member"]}) !!! do const 400 === statusCode ScimT.mkScimErrorResp - (Just "A user cannot have more than one role.") + (Just "A user cannot have more than one role. (put)") (Just "invalidValue") "400" === responseBody @@ -1424,7 +1424,7 @@ specProvisionScimAndSAMLUserWithRole = do ScimT.updateUser' tok uid (scimUser {Scim.roles = ["hamlet"]}) !!! do const 400 === statusCode ScimT.mkScimErrorResp - (Just "The role 'hamlet' is not valid. Valid roles are owner, admin, member, partner.") + (Just "The role 'hamlet' is not valid. Valid roles are owner, admin, member, partner. (put)") (Just "invalidValue") "400" === responseBody diff --git a/services/spar/test-integration/Test/Spar/Scim/UserSpec.hs b/services/spar/test-integration/Test/Spar/Scim/UserSpec.hs index 5f94fd688f..2f67d992b9 100644 --- a/services/spar/test-integration/Test/Spar/Scim/UserSpec.hs +++ b/services/spar/test-integration/Test/Spar/Scim/UserSpec.hs @@ -673,7 +673,7 @@ testCreateUserNoIdPInvalidRoles = do createUser' tok scimUserTooManyRoles !!! do const 400 === statusCode mkScimErrorResp - (Just "A user cannot have more than one role.") + (Just "A user cannot have more than one role. (post)") (Just "invalidValue") "400" === responseBody @@ -686,7 +686,7 @@ testCreateUserNoIdPInvalidRoles = do createUser' tok scimUserInvalidRole !!! do const 400 === statusCode mkScimErrorResp - (Just "The role 'foobar' is not valid. Valid roles are owner, admin, member, partner.") + (Just "The role 'foobar' is not valid. Valid roles are owner, admin, member, partner. (post)") (Just "invalidValue") "400" === responseBody @@ -2062,6 +2062,7 @@ specPatchUser = do PatchOp.Remove (Just (PatchOp.NormalPath (Filter.topLevelAttrPath name))) Nothing + it "doing nothing doesn't change the user" $ do (tok, _) <- registerIdPAndScimToken user <- randomScimUser @@ -2069,6 +2070,7 @@ specPatchUser = do let userid = scimUserId storedUser storedUser' <- patchUser tok userid (PatchOp.PatchOp []) liftIO $ storedUser `shouldBe` storedUser' + it "can update userName" $ do (tok, _) <- registerIdPAndScimToken user <- randomScimUser @@ -2082,6 +2084,7 @@ specPatchUser = do [replaceAttrib "userName" userName] let user'' = Scim.value (Scim.thing storedUser') liftIO $ Scim.User.userName user'' `shouldBe` userName + it "can't update to someone else's userName" $ do env <- ask (tok, _) <- registerIdPAndScimToken @@ -2095,6 +2098,7 @@ specPatchUser = do const 409 === statusCode mkScimErrorResp Nothing (Just "uniqueness") "409" === responseBody + it "can't update to someone else's externalId" $ do env <- ask (tok, _) <- registerIdPAndScimToken @@ -2108,6 +2112,7 @@ specPatchUser = do const 409 === statusCode mkScimErrorResp Nothing (Just "uniqueness") "409" === responseBody + it "can't update a non-existing user" $ do env <- ask (tok, _) <- registerIdPAndScimToken @@ -2117,6 +2122,7 @@ specPatchUser = do const 404 === statusCode mkScimErrorResp Nothing Nothing "404" === responseBody + it "can update displayName" $ do (tok, _) <- registerIdPAndScimToken user <- randomScimUser @@ -2130,6 +2136,7 @@ specPatchUser = do [replaceAttrib "displayName" displayName] let user'' = Scim.value (Scim.thing storedUser') liftIO $ Scim.User.displayName user'' `shouldBe` displayName + it "can update externalId" $ do (tok, _) <- registerIdPAndScimToken user <- randomScimUser @@ -2143,10 +2150,15 @@ specPatchUser = do [replaceAttrib "externalId" externalId] let user'' = Scim.value . Scim.thing $ storedUser' liftIO $ Scim.User.externalId user'' `shouldBe` externalId + it "replace role works" $ testPatchRole replaceAttrib + it "add role works" $ testPatchRole addAttrib + it "replace with invalid input should fail" $ testPatchIvalidInput replaceAttrib + it "add with invalid input should fail" $ testPatchIvalidInput addAttrib + it "replacing every supported atttribute at once works" $ do (tok, _) <- registerIdPAndScimToken user <- randomScimUser @@ -2167,6 +2179,7 @@ specPatchUser = do liftIO $ Scim.User.externalId user'' `shouldBe` externalId liftIO $ Scim.User.userName user'' `shouldBe` userName liftIO $ Scim.User.displayName user'' `shouldBe` displayName + it "other valid attributes that we do not explicit support throw an error" $ do (tok, _) <- registerIdPAndScimToken user <- randomScimUser @@ -2176,8 +2189,9 @@ specPatchUser = do let patchOp = PatchOp.PatchOp [replaceAttrib "emails" ("hello" :: Text)] patchUser_ (Just tok) (Just userid) patchOp (env ^. teSpar) !!! do const 400 === statusCode - mkScimErrorResp Nothing (Just "invalidValue") "400" + mkScimErrorResp Nothing (Just "invalidPath") "400" === responseBody + it "invalid attributes are quietly ignored for now" $ do (tok, _) <- registerIdPAndScimToken user <- randomScimUser @@ -2187,8 +2201,9 @@ specPatchUser = do let patchOp = PatchOp.PatchOp [replaceAttrib "totallyBogus" ("hello" :: Text)] patchUser_ (Just tok) (Just userid) patchOp (env ^. teSpar) !!! do const 400 === statusCode - mkScimErrorResp Nothing (Just "invalidValue") "400" + mkScimErrorResp Nothing (Just "invalidPath") "400" === responseBody + -- NOTE: Remove at the moment actually never works! As all the fields -- we support are required in our book it "userName cannot be removed according to scim" $ do @@ -2200,8 +2215,9 @@ specPatchUser = do let patchOp = PatchOp.PatchOp [removeAttrib "userName"] patchUser_ (Just tok) (Just userid) patchOp (env ^. teSpar) !!! do const 400 === statusCode - mkScimErrorResp Nothing (Just "invalidValue") "400" + mkScimErrorResp Nothing (Just "mutability") "400" === responseBody + it "displayName cannot be removed in spar (though possible in scim). Diplayname is required in Wire" $ do pendingWith "We default to the externalId when displayName is removed. lets keep that for now" @@ -2240,7 +2256,7 @@ testPatchIvalidInput patchOp = do patchUser' tok uid (PatchOp.PatchOp [patchWithInvalidRole]) !!! do const 400 === statusCode mkScimErrorResp - (Just "The role 'invalid-role' is not valid. Valid roles are owner, admin, member, partner.") + (Just "The role 'invalid-role' is not valid. Valid roles are owner, admin, member, partner. (put)") (Just "invalidValue") "400" === responseBody @@ -2248,7 +2264,7 @@ testPatchIvalidInput patchOp = do patchUser' tok uid (PatchOp.PatchOp [patchWithTooManyRoles]) !!! do const 400 === statusCode mkScimErrorResp - (Just "A user cannot have more than one role.") + (Just "A user cannot have more than one role. (put)") (Just "invalidValue") "400" === responseBody @@ -2307,8 +2323,6 @@ specDeleteUser = do (tok, _) <- registerIdPAndScimToken deleteUser_ (Just tok) Nothing (env ^. teSpar) !!! do const 405 === statusCode - mkScimErrorResp Nothing Nothing "405" - === responseBody describe "DELETE /Users/:id" $ do it "should delete user from brig, spar.scim_user_times, spar.user" $ do (tok, (_, _, idp)) <- registerIdPAndScimToken diff --git a/services/spar/test-integration/Util/Scim.hs b/services/spar/test-integration/Util/Scim.hs index bd49b8236a..6ee04f4886 100644 --- a/services/spar/test-integration/Util/Scim.hs +++ b/services/spar/test-integration/Util/Scim.hs @@ -23,7 +23,6 @@ module Util.Scim where import Bilge import Bilge.Assert -import qualified Control.Exception import Control.Lens import Control.Monad.Random import qualified Data.Aeson as Aeson @@ -784,7 +783,7 @@ mkScimErrorResp :: Maybe LByteString mkScimErrorResp mDetail scimType status resp = Just . Aeson.encode . Aeson.object . catMaybes $ - [ ("detail" Aeson..=) <$> (mDetail <|> (Control.Exception.assert (isJust actualDetail) actualDetail)), + [ ("detail" Aeson..=) <$> (mDetail <|> actualDetail), Just ("schemas" Aeson..= ["urn:ietf:params:scim:api:messages:2.0:Error" :: Text]), ("scimType" Aeson..=) <$> scimType, Just ("status" Aeson..= status) From 89addaffa2ac414e972c6e20af535537ace5a035 Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Thu, 13 Aug 2026 15:38:21 +0200 Subject: [PATCH 9/9] Update services/spar/test-integration/Util/Scim.hs Co-authored-by: Sven Tennie --- services/spar/test-integration/Util/Scim.hs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/spar/test-integration/Util/Scim.hs b/services/spar/test-integration/Util/Scim.hs index 6ee04f4886..bade67b50e 100644 --- a/services/spar/test-integration/Util/Scim.hs +++ b/services/spar/test-integration/Util/Scim.hs @@ -760,7 +760,7 @@ checkTeamMembersRole tid owner uid role = do [member] <- filter ((== uid) . (^. Member.userId)) <$> getTeamMembers owner tid liftIO $ (member ^. Member.permissions . to Member.permissionsRole) `shouldBe` Just role --- | Assert the body of a scim error response (rfc7644, section 3.12). The arguments are the +-- | Create the body of a scim error response (rfc7644, section 3.12). The arguments are the -- @detail@, @scimType@ and @status@ fields; @schemas@ is the same for every scim error and is -- filled in here. --