From e49ec82e77ff56d495eb0f46867b4a53ea6a8428 Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Fri, 30 Jul 2021 15:27:47 +0200 Subject: [PATCH 1/5] Change http response code for `missing-legalhold-consent`. 412 is used for "need to add some clients" in message posting; 403 makes client implementations more straight-forward. --- services/brig/src/Brig/API/Error.hs | 2 +- services/galley/src/Galley/API/Error.hs | 2 +- .../test/integration/API/Teams/LegalHold.hs | 16 ++++++++-------- .../API/Teams/LegalHold/DisabledByDefault.hs | 4 ++-- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/services/brig/src/Brig/API/Error.hs b/services/brig/src/Brig/API/Error.hs index f6c40d54830..9d48aa894d8 100644 --- a/services/brig/src/Brig/API/Error.hs +++ b/services/brig/src/Brig/API/Error.hs @@ -457,7 +457,7 @@ sameBindingTeamUsers :: Wai.Error sameBindingTeamUsers = Wai.mkError status403 "same-binding-team-users" "Operation not allowed to binding team users." missingLegalholdConsent :: Wai.Error -missingLegalholdConsent = Wai.mkError status412 "missing-legalhold-consent" "Failed to connect to a user or to invite a user to a group because somebody is under legalhold and somebody else has not granted consent." +missingLegalholdConsent = Wai.mkError status403 "missing-legalhold-consent" "Failed to connect to a user or to invite a user to a group because somebody is under legalhold and somebody else has not granted consent." ownerDeletingSelf :: Wai.Error ownerDeletingSelf = diff --git a/services/galley/src/Galley/API/Error.hs b/services/galley/src/Galley/API/Error.hs index 8d9c4f1b004..976cea6e0a6 100644 --- a/services/galley/src/Galley/API/Error.hs +++ b/services/galley/src/Galley/API/Error.hs @@ -210,7 +210,7 @@ legalHoldCouldNotBlockConnections :: Error legalHoldCouldNotBlockConnections = mkError status500 "legalhold-internal" "legal hold service: could not block connections when resolving policy conflicts." missingLegalholdConsent :: Error -missingLegalholdConsent = mkError status412 "missing-legalhold-consent" "Failed to connect to a user or to invite a user to a group because somebody is under legalhold and somebody else has not granted consent." +missingLegalholdConsent = mkError status403 "missing-legalhold-consent" "Failed to connect to a user or to invite a user to a group because somebody is under legalhold and somebody else has not granted consent." disableSsoNotImplemented :: Error disableSsoNotImplemented = diff --git a/services/galley/test/integration/API/Teams/LegalHold.hs b/services/galley/test/integration/API/Teams/LegalHold.hs index 5287d88392e..f58a5740d3a 100644 --- a/services/galley/test/integration/API/Teams/LegalHold.hs +++ b/services/galley/test/integration/API/Teams/LegalHold.hs @@ -806,7 +806,7 @@ testOldClientsBlockDeviceHandshake = do -- If user has a client without the ClientSupportsLegalholdImplicitConsent -- capability then message sending is prevented to legalhold devices. peerClient <- randomClient peer (someLastPrekeys !! 2) - runit peer peerClient >>= errWith 412 (\err -> Error.label err == "missing-legalhold-consent") + runit peer peerClient >>= errWith 403 (\err -> Error.label err == "missing-legalhold-consent") upgradeClientToLH peer peerClient runit peer peerClient >>= errWith 412 (\(_ :: Msg.ClientMismatch) -> True) @@ -928,8 +928,8 @@ testNoConsentBlockOne2OneConv connectFirst teamPeer approveLH testPendingConnect const 201 === statusCode else do void doEnableLH - postConnection legalholder peer !!! do testResponse 412 (Just "missing-legalhold-consent") - postConnection peer legalholder !!! do testResponse 412 (Just "missing-legalhold-consent") + postConnection legalholder peer !!! do testResponse 403 (Just "missing-legalhold-consent") + postConnection peer legalholder !!! do testResponse 403 (Just "missing-legalhold-consent") data GroupConvAdmin = LegalholderIsAdmin @@ -1044,7 +1044,7 @@ testGroupConvInvitationHandlesLHConflicts inviteCase = do assertNotConvMember peer convId InviteAlsoNonConsenters -> do API.Util.postMembers userWithConsent (List1.list1 legalholder [peer2]) convId - >>= errWith 412 (\err -> Error.label err == "missing-legalhold-consent") + >>= errWith 403 (\err -> Error.label err == "missing-legalhold-consent") testNoConsentCannotBeInvited :: HasCallStack => TestM () testNoConsentCannotBeInvited = do @@ -1081,11 +1081,11 @@ testNoConsentCannotBeInvited = do liftIO $ assertEqual "approving should change status" UserLegalHoldEnabled userStatus API.Util.postMembers userLHNotActivated (List1.list1 peer2 []) convId - >>= errWith 412 (\err -> Error.label err == "missing-legalhold-consent") + >>= errWith 403 (\err -> Error.label err == "missing-legalhold-consent") localdomain <- viewFederationDomain API.Util.postQualifiedMembers userLHNotActivated ((Qualified peer2 localdomain) :| []) convId - >>= errWith 412 (\err -> Error.label err == "missing-legalhold-consent") + >>= errWith 403 (\err -> Error.label err == "missing-legalhold-consent") testCannotCreateGroupWithUsersInConflict :: HasCallStack => TestM () testCannotCreateGroupWithUsersInConflict = do @@ -1120,7 +1120,7 @@ testCannotCreateGroupWithUsersInConflict = do liftIO $ assertEqual "approving should change status" UserLegalHoldEnabled userStatus createTeamConvAccessRaw userLHNotActivated tid [peer2, legalholder] (Just "corp + us") Nothing Nothing Nothing (Just roleNameWireMember) - >>= errWith 412 (\err -> Error.label err == "missing-legalhold-consent") + >>= errWith 403 (\err -> Error.label err == "missing-legalhold-consent") data TestClaimKeys = TCKConsentMissing @@ -1169,7 +1169,7 @@ testClaimKeys testcase = do TCKConsentAndNewClients -> good where good = testResponse 200 Nothing - bad = testResponse 412 (Just "missing-legalhold-consent") + bad = testResponse 403 (Just "missing-legalhold-consent") let fetchKeys :: ClientId -> TestM () fetchKeys legalholderLHDevice = do diff --git a/services/galley/test/integration/API/Teams/LegalHold/DisabledByDefault.hs b/services/galley/test/integration/API/Teams/LegalHold/DisabledByDefault.hs index e9b75dfa805..b023b3abf56 100644 --- a/services/galley/test/integration/API/Teams/LegalHold/DisabledByDefault.hs +++ b/services/galley/test/integration/API/Teams/LegalHold/DisabledByDefault.hs @@ -752,7 +752,7 @@ testOldClientsBlockDeviceHandshake = do -- If user has a client without the ClientSupportsLegalholdImplicitConsent -- capability then message sending is prevented to legalhold devices. peerClient <- randomClient peer (someLastPrekeys !! 2) - runit peer peerClient >>= errWith 412 (\err -> Error.label err == "missing-legalhold-consent") + runit peer peerClient >>= errWith 403 (\err -> Error.label err == "missing-legalhold-consent") upgradeClientToLH peer peerClient runit peer peerClient >>= errWith 412 (\(_ :: Msg.ClientMismatch) -> True) @@ -803,7 +803,7 @@ testClaimKeys testcase = do TCKConsentAndNewClients -> good where good = testResponse 200 Nothing - bad = testResponse 412 (Just "missing-legalhold-consent") + bad = testResponse 403 (Just "missing-legalhold-consent") let fetchKeys :: ClientId -> TestM () fetchKeys legalholderLHDevice = do From 1cc46f5c6e8afb50a659e522d1706fd094ae96d4 Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Fri, 30 Jul 2021 15:31:35 +0200 Subject: [PATCH 2/5] CHANGELOG --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c4205ef9d5c..0f56aed297c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -53,6 +53,7 @@ Upgrade nginz (#1658) * New, hardened end-point for changing email * Fix: CSV export is missing SCIM external id when SAML is also used (#1608) * Fix: sso_id field in user record (brig) was not always filled correctly in cassandra (#1334) +* Change http response code for `missing-legalhold-consent` from 412 to 403 (#1688) ## Documentation From eaf7945a7f0bf8cd9e6282545449f6bafaf2dacf Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Mon, 2 Aug 2021 21:00:21 +0200 Subject: [PATCH 3/5] Missed a spot... --- services/brig/src/Brig/IO/Intra.hs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/brig/src/Brig/IO/Intra.hs b/services/brig/src/Brig/IO/Intra.hs index f0bcc91927e..536e7c9a859 100644 --- a/services/brig/src/Brig/IO/Intra.hs +++ b/services/brig/src/Brig/IO/Intra.hs @@ -898,7 +898,7 @@ guardLegalhold protectee userClients = do res <- lift $ galleyRequest PUT req case Bilge.statusCode res of 200 -> pure () - 412 -> throwE ClientMissingLegalholdConsent + 403 -> throwE ClientMissingLegalholdConsent 404 -> pure () -- allow for galley not to be ready, so the set of valid deployment orders is non-empty. _ -> throwM internalServerError where From 83abb9cae85112193dac8b9e976a7bc8a70612ff Mon Sep 17 00:00:00 2001 From: Paolo Capriotti Date: Tue, 3 Aug 2021 11:49:53 +0200 Subject: [PATCH 4/5] Mark error idioms as deprecated. --- services/brig/src/Brig/API/Error.hs | 1 + services/galley/src/Galley/API/Error.hs | 9 +++++++++ 2 files changed, 10 insertions(+) diff --git a/services/brig/src/Brig/API/Error.hs b/services/brig/src/Brig/API/Error.hs index 9d48aa894d8..ad510639216 100644 --- a/services/brig/src/Brig/API/Error.hs +++ b/services/brig/src/Brig/API/Error.hs @@ -456,6 +456,7 @@ propertyManagedByScim prop = Wai.mkError status403 "managed-by-scim" $ "Updating sameBindingTeamUsers :: Wai.Error sameBindingTeamUsers = Wai.mkError status403 "same-binding-team-users" "Operation not allowed to binding team users." +-- | See 'Galley.API.Error.missingLegalholdConsent'. missingLegalholdConsent :: Wai.Error missingLegalholdConsent = Wai.mkError status403 "missing-legalhold-consent" "Failed to connect to a user or to invite a user to a group because somebody is under legalhold and somebody else has not granted consent." diff --git a/services/galley/src/Galley/API/Error.hs b/services/galley/src/Galley/API/Error.hs index 976cea6e0a6..a82e43b68aa 100644 --- a/services/galley/src/Galley/API/Error.hs +++ b/services/galley/src/Galley/API/Error.hs @@ -209,6 +209,15 @@ noLegalHoldDeviceAllocated = mkError status404 "legalhold-no-device-allocated" " legalHoldCouldNotBlockConnections :: Error legalHoldCouldNotBlockConnections = mkError status500 "legalhold-internal" "legal hold service: could not block connections when resolving policy conflicts." +-- | See 'Brig.API.Error.missingLegalholdConsent'. +-- +-- FUTUREWORK: To avoid this duplication, you could turn this into an `ErrorDescription` and +-- move it to the `ErrorDescription` module in wire-api, where it can then be used from both +-- brig and galley. Also, it makes it easier to add it to swagger, if desired, or even later +-- turn it into a statically checked response with `MultiVerb`. +-- +-- For examples, see https://github.com/wireapp/wire-server/pull/1657 or +-- https://github.com/wireapp/wire-server/pull/1657/commits/8e73769d7d4a657fef5fbe9210f885f9ccbcaab6. missingLegalholdConsent :: Error missingLegalholdConsent = mkError status403 "missing-legalhold-consent" "Failed to connect to a user or to invite a user to a group because somebody is under legalhold and somebody else has not granted consent." From 1dade31717be27c8e4c33bb68a59fcc714111f96 Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Tue, 3 Aug 2021 12:51:34 +0200 Subject: [PATCH 5/5] hi ci