From b864788d806afa163f57b2f9721ef75cdfe4e5bb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Marko=20Dimja=C5=A1evi=C4=87?= <marko.dimjasevic@wire.com>
Date: Thu, 16 Dec 2021 11:42:11 +0100
Subject: [PATCH 01/24] Tag an existing test case for an invalid client
 certificate

---
 services/federator/test/unit/Test/Federator/Options.hs | 1 +
 1 file changed, 1 insertion(+)

diff --git a/services/federator/test/unit/Test/Federator/Options.hs b/services/federator/test/unit/Test/Federator/Options.hs
index 3bf5930a2d..f3ec7c8484 100644
--- a/services/federator/test/unit/Test/Federator/Options.hs
+++ b/services/federator/test/unit/Test/Federator/Options.hs
@@ -171,6 +171,7 @@ testSettings =
             assertFailure $
               "expected failure for non-existing client certificate, got: "
                 <> show (tlsSettings ^. creds),
+      -- @SF.FEDERATION @TSFI.RESTfulAPI @S2 @S3 @S7
       testCase "fail on invalid certificate" $ do
         let settings =
               defRunSettings

From 7b354aae7b99452e636943ee67824b99b4445522 Mon Sep 17 00:00:00 2001
From: Leif Battermann <leif.battermann@wire.com>
Date: Thu, 16 Dec 2021 13:06:32 +0000
Subject: [PATCH 02/24] test for invalid certificate

---
 .../integration/Test/Federator/IngressSpec.hs | 34 +++++++++++++++++--
 1 file changed, 32 insertions(+), 2 deletions(-)

diff --git a/services/federator/test/integration/Test/Federator/IngressSpec.hs b/services/federator/test/integration/Test/Federator/IngressSpec.hs
index 08d989b647..297791fa90 100644
--- a/services/federator/test/integration/Test/Federator/IngressSpec.hs
+++ b/services/federator/test/integration/Test/Federator/IngressSpec.hs
@@ -32,6 +32,7 @@ import Federator.Env
 import Federator.Remote
 import Imports
 import qualified Network.HTTP.Types as HTTP
+import qualified Network.TLS as TLS
 import Polysemy
 import Polysemy.Embed
 import Polysemy.Error
@@ -61,7 +62,7 @@ spec env = do
           runTestSem
             . assertNoError @RemoteError
             $ inwardBrigCallViaIngress "get-user-by-handle" $
-              (Aeson.fromEncoding (Aeson.toEncoding hdl))
+              Aeson.fromEncoding (Aeson.toEncoding hdl)
         liftIO $ do
           bdy <- streamingResponseStrictBody resp
           let actualProfile = Aeson.decode (toLazyByteString bdy)
@@ -86,12 +87,41 @@ spec env = do
         runTestSem
           . runError @RemoteError
           $ inwardBrigCallViaIngressWithSettings tlsSettings "get-user-by-handle" $
-            (Aeson.fromEncoding (Aeson.toEncoding hdl))
+            Aeson.fromEncoding (Aeson.toEncoding hdl)
       liftIO $ case r of
         Right _ -> expectationFailure "Expected client certificate error, got response"
         Left (RemoteError _ _) ->
           expectationFailure "Expected client certificate error, got remote error"
         Left (RemoteErrorResponse _ status _) -> status `shouldBe` HTTP.status400
+  -- @SF.FEDERATION @TSFI.RESTfulAPI @S2 @S3 @S7
+  it "should not be accessible with an invalid client certificate" $
+    runTestFederator env $ do
+      brig <- view teBrig <$> ask
+      user <- randomUser brig
+      hdl <- randomHandle
+      _ <- putHandle brig (userId user) hdl
+
+      tlsSettings0 <- view teTLSSettings
+      certOrError <- liftIO $ TLS.credentialLoadX509 "todo" "todo"
+      let tlsSettings = case certOrError of
+            Left _ -> error "this test should fail if we cannot successfully create an invalid certificate"
+            Right (cred, _) ->
+              tlsSettings0
+                { _creds = case _creds tlsSettings0 of
+                    (_, privkey) -> (cred, privkey)
+                }
+      r <-
+        runTestSem
+          . runError @RemoteError
+          $ inwardBrigCallViaIngressWithSettings tlsSettings "get-user-by-handle" $
+            Aeson.fromEncoding (Aeson.toEncoding hdl)
+      liftIO $ case r of
+        Right _ -> expectationFailure "Expected client certificate error, got response"
+        Left (RemoteError _ _) ->
+          expectationFailure "Expected client certificate error, got remote error"
+        Left (RemoteErrorResponse _ status _) -> status `shouldBe` HTTP.status400
+
+-- @END
 
 runTestSem :: Sem '[Input TestEnv, Embed IO] a -> TestFederator IO a
 runTestSem action = do

From 12952b7249e34a81d4a89a9e0a54928d26c33da5 Mon Sep 17 00:00:00 2001
From: Leif Battermann <leif.battermann@wire.com>
Date: Thu, 16 Dec 2021 14:32:28 +0000
Subject: [PATCH 03/24] Revert "test for invalid certificate"

This reverts commit c902bb18f0200ed0744f90e6cb7d61dead139c87.
---
 .../integration/Test/Federator/IngressSpec.hs | 34 ++-----------------
 1 file changed, 2 insertions(+), 32 deletions(-)

diff --git a/services/federator/test/integration/Test/Federator/IngressSpec.hs b/services/federator/test/integration/Test/Federator/IngressSpec.hs
index 297791fa90..08d989b647 100644
--- a/services/federator/test/integration/Test/Federator/IngressSpec.hs
+++ b/services/federator/test/integration/Test/Federator/IngressSpec.hs
@@ -32,7 +32,6 @@ import Federator.Env
 import Federator.Remote
 import Imports
 import qualified Network.HTTP.Types as HTTP
-import qualified Network.TLS as TLS
 import Polysemy
 import Polysemy.Embed
 import Polysemy.Error
@@ -62,7 +61,7 @@ spec env = do
           runTestSem
             . assertNoError @RemoteError
             $ inwardBrigCallViaIngress "get-user-by-handle" $
-              Aeson.fromEncoding (Aeson.toEncoding hdl)
+              (Aeson.fromEncoding (Aeson.toEncoding hdl))
         liftIO $ do
           bdy <- streamingResponseStrictBody resp
           let actualProfile = Aeson.decode (toLazyByteString bdy)
@@ -87,41 +86,12 @@ spec env = do
         runTestSem
           . runError @RemoteError
           $ inwardBrigCallViaIngressWithSettings tlsSettings "get-user-by-handle" $
-            Aeson.fromEncoding (Aeson.toEncoding hdl)
+            (Aeson.fromEncoding (Aeson.toEncoding hdl))
       liftIO $ case r of
         Right _ -> expectationFailure "Expected client certificate error, got response"
         Left (RemoteError _ _) ->
           expectationFailure "Expected client certificate error, got remote error"
         Left (RemoteErrorResponse _ status _) -> status `shouldBe` HTTP.status400
-  -- @SF.FEDERATION @TSFI.RESTfulAPI @S2 @S3 @S7
-  it "should not be accessible with an invalid client certificate" $
-    runTestFederator env $ do
-      brig <- view teBrig <$> ask
-      user <- randomUser brig
-      hdl <- randomHandle
-      _ <- putHandle brig (userId user) hdl
-
-      tlsSettings0 <- view teTLSSettings
-      certOrError <- liftIO $ TLS.credentialLoadX509 "todo" "todo"
-      let tlsSettings = case certOrError of
-            Left _ -> error "this test should fail if we cannot successfully create an invalid certificate"
-            Right (cred, _) ->
-              tlsSettings0
-                { _creds = case _creds tlsSettings0 of
-                    (_, privkey) -> (cred, privkey)
-                }
-      r <-
-        runTestSem
-          . runError @RemoteError
-          $ inwardBrigCallViaIngressWithSettings tlsSettings "get-user-by-handle" $
-            Aeson.fromEncoding (Aeson.toEncoding hdl)
-      liftIO $ case r of
-        Right _ -> expectationFailure "Expected client certificate error, got response"
-        Left (RemoteError _ _) ->
-          expectationFailure "Expected client certificate error, got remote error"
-        Left (RemoteErrorResponse _ status _) -> status `shouldBe` HTTP.status400
-
--- @END
 
 runTestSem :: Sem '[Input TestEnv, Embed IO] a -> TestFederator IO a
 runTestSem action = do

From c0bdcfc703c3ebbccee3b29c78297099236f38a0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Marko=20Dimja=C5=A1evi=C4=87?= <marko.dimjasevic@wire.com>
Date: Thu, 16 Dec 2021 15:56:11 +0100
Subject: [PATCH 04/24] Mark test cases for an invalid certificate

---
 services/federator/test/unit/Test/Federator/Options.hs    | 3 ++-
 services/federator/test/unit/Test/Federator/Validation.hs | 1 +
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/services/federator/test/unit/Test/Federator/Options.hs b/services/federator/test/unit/Test/Federator/Options.hs
index f3ec7c8484..ac5edb8d0d 100644
--- a/services/federator/test/unit/Test/Federator/Options.hs
+++ b/services/federator/test/unit/Test/Federator/Options.hs
@@ -171,7 +171,7 @@ testSettings =
             assertFailure $
               "expected failure for non-existing client certificate, got: "
                 <> show (tlsSettings ^. creds),
-      -- @SF.FEDERATION @TSFI.RESTfulAPI @S2 @S3 @S7
+      -- @SF.Federation @S3 @S7
       testCase "fail on invalid certificate" $ do
         let settings =
               defRunSettings
@@ -194,6 +194,7 @@ testSettings =
             assertFailure $
               "expected failure for invalid client certificate, got: "
                 <> show (tlsSettings ^. creds),
+      -- @END
       testCase "fail on invalid private key" $ do
         let settings =
               defRunSettings
diff --git a/services/federator/test/unit/Test/Federator/Validation.hs b/services/federator/test/unit/Test/Federator/Validation.hs
index 4c4597a6c5..29363de627 100644
--- a/services/federator/test/unit/Test/Federator/Validation.hs
+++ b/services/federator/test/unit/Test/Federator/Validation.hs
@@ -164,6 +164,7 @@ validateDomainCertInvalid =
         $ validateDomain (Just "not a certificate") "foo.example.com"
     res @?= Left (CertificateParseError "no certificate found")
 
+-- @SF.Federation @S3 @S7
 validateDomainCertWrongDomain :: TestTree
 validateDomainCertWrongDomain =
   testCase "should fail if the client certificate has a wrong domain" $ do

From 8dc52147472d6291f6531cc8eb3f242962d53b6e Mon Sep 17 00:00:00 2001
From: Leif Battermann <leif.battermann@wire.com>
Date: Thu, 16 Dec 2021 15:32:46 +0000
Subject: [PATCH 05/24] added test

---
 .../integration/Test/Federator/InwardSpec.hs  | 32 +++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/services/federator/test/integration/Test/Federator/InwardSpec.hs b/services/federator/test/integration/Test/Federator/InwardSpec.hs
index da02ec6f45..16aa9b6bb8 100644
--- a/services/federator/test/integration/Test/Federator/InwardSpec.hs
+++ b/services/federator/test/integration/Test/Federator/InwardSpec.hs
@@ -72,6 +72,19 @@ spec env =
             <!! const 200 === statusCode
         liftIO $ bdy `shouldBe` expectedProfile
 
+    -- @SF.Federation @TSFI.RESTfulAPI @S2 @S3 @S7
+    -- This maybe case 2 from https://wearezeta.atlassian.net/browse/SQSERVICES-1128
+    it "shouldRejectMissMatchingOriginDomain" $
+      runTestFederator env $
+        do
+          brig <- view teBrig <$> ask
+          user <- randomUser brig
+          hdl <- randomHandle
+          _ <- putHandle brig (userId user) hdl
+          inwardCallWithOriginDomain "unknown-domain.com" "/federation/brig/get-user-by-handle" (encode hdl)
+            !!! const 500 === statusCode
+    -- @END
+
     it "should be able to call cargohold" $
       runTestFederator env $ do
         inwardCall "/federation/cargohold/get-asset" (encode ())
@@ -144,3 +157,22 @@ inwardCall requestPath payload = do
         . header originDomainHeaderName (toByteString' originDomain)
         . bytes (toByteString' payload)
     )
+
+inwardCallWithOriginDomain ::
+  (MonadIO m, MonadHttp m, MonadReader TestEnv m, HasCallStack) =>
+  ByteString ->
+  ByteString ->
+  LBS.ByteString ->
+  m (Response (Maybe LByteString))
+inwardCallWithOriginDomain originDomain requestPath payload = do
+  Endpoint fedHost fedPort <- cfgFederatorExternal <$> view teTstOpts
+  clientCertFilename <- clientCertificate . optSettings . view teOpts <$> ask
+  clientCert <- liftIO $ BS.readFile clientCertFilename
+  post
+    ( host (encodeUtf8 fedHost)
+        . port fedPort
+        . path requestPath
+        . header "X-SSL-Certificate" (HTTP.urlEncode True clientCert)
+        . header originDomainHeaderName originDomain
+        . bytes (toByteString' payload)
+    )

From 8a877a1e6294de6d0f12a138722515ddfaa31c20 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Marko=20Dimja=C5=A1evi=C4=87?= <marko.dimjasevic@wire.com>
Date: Thu, 16 Dec 2021 17:11:20 +0100
Subject: [PATCH 06/24] WIP: Some test cases in Validation.hs

---
 .../test/unit/Test/Federator/Validation.hs         | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/services/federator/test/unit/Test/Federator/Validation.hs b/services/federator/test/unit/Test/Federator/Validation.hs
index 29363de627..a7ab13525d 100644
--- a/services/federator/test/unit/Test/Federator/Validation.hs
+++ b/services/federator/test/unit/Test/Federator/Validation.hs
@@ -113,6 +113,12 @@ validateDomainAllowListFailSemantic =
         $ validateDomain (Just exampleCert) "invalid//.><-semantic-&@-domain"
     res @?= Left (DomainParseError "invalid//.><-semantic-&@-domain")
 
+-- @SF.Federation @TSFI.RESTfulAPI @S2 @S3 @S7
+--
+-- 4. Outgoing request to non-included domain when allowlist is configured ->
+-- authorization error expected
+--
+-- Does it make sense to tag this one with @TSFI.RESTfulAPI ?
 validateDomainAllowListFail :: TestTree
 validateDomainAllowListFail =
   testCase "allow list validation" $ do
@@ -127,6 +133,12 @@ validateDomainAllowListFail =
         $ validateDomain (Just exampleCert) "localhost.example.com"
     res @?= Left (FederationDenied (Domain "localhost.example.com"))
 
+-- @SF.Federation @TSFI.RESTfulAPI @S2 @S3 @S7
+--
+validateDomainAllowListFailSendingSide :: TestTree
+validateDomainAllowListFailSendingSide = do
+  undefined
+
 validateDomainAllowListSuccess :: TestTree
 validateDomainAllowListSuccess =
   testCase "should give parsed domain if in the allow list" $ do
@@ -165,6 +177,8 @@ validateDomainCertInvalid =
     res @?= Left (CertificateParseError "no certificate found")
 
 -- @SF.Federation @S3 @S7
+--
+-- Is this test case #2?
 validateDomainCertWrongDomain :: TestTree
 validateDomainCertWrongDomain =
   testCase "should fail if the client certificate has a wrong domain" $ do

From 98fcc0d7ae654a79132bd30fcced838b9604e422 Mon Sep 17 00:00:00 2001
From: Matthias Fischmann <mf@zerobuzz.net>
Date: Fri, 17 Dec 2021 09:30:29 +0100
Subject: [PATCH 07/24] ...

---
 .../test/integration/Test/Federator/IngressSpec.hs        | 8 ++++++++
 .../test/integration/Test/Federator/InwardSpec.hs         | 5 +++++
 services/federator/test/unit/Test/Federator/Validation.hs | 8 ++++++++
 3 files changed, 21 insertions(+)

diff --git a/services/federator/test/integration/Test/Federator/IngressSpec.hs b/services/federator/test/integration/Test/Federator/IngressSpec.hs
index 08d989b647..0f6d76e175 100644
--- a/services/federator/test/integration/Test/Federator/IngressSpec.hs
+++ b/services/federator/test/integration/Test/Federator/IngressSpec.hs
@@ -68,6 +68,9 @@ spec env = do
           responseStatusCode resp `shouldBe` HTTP.status200
           actualProfile `shouldBe` (Just expectedProfile)
 
+  -- @SF.Federation @TSFI.RESTfulAPI @S2 @S3 @S7
+  -- 1 - Receiving backend fails to authenticate the client certificate when sending
+  -- connection request to infrastructure domain -> Authentication Error expected
   it "should not be accessible without a client certificate" $
     runTestFederator env $ do
       brig <- view teBrig <$> ask
@@ -93,6 +96,11 @@ spec env = do
           expectationFailure "Expected client certificate error, got remote error"
         Left (RemoteErrorResponse _ status _) -> status `shouldBe` HTTP.status400
 
+{-# DISABLE_ORMOLU #-}
+-- TODO: how does ormolu do this again?
+-- @END
+{-# ENSABLE_ORMOLU #-}
+
 runTestSem :: Sem '[Input TestEnv, Embed IO] a -> TestFederator IO a
 runTestSem action = do
   e <- ask
diff --git a/services/federator/test/integration/Test/Federator/InwardSpec.hs b/services/federator/test/integration/Test/Federator/InwardSpec.hs
index 16aa9b6bb8..d481eb5bad 100644
--- a/services/federator/test/integration/Test/Federator/InwardSpec.hs
+++ b/services/federator/test/integration/Test/Federator/InwardSpec.hs
@@ -111,6 +111,9 @@ spec env =
         inwardCall "/i/users" (encode o)
           !!! const 403 === statusCode
 
+    -- @SF.Federation @TSFI.RESTfulAPI @S2 @S3 @S7
+    -- 1 - Receiving backend fails to authenticate the client certificate when sending
+    -- connection request to infrastructure domain -> Authentication Error expected
     -- Matching client certificates against domain names is better tested in
     -- unit tests.
     it "should reject requests without a client certificate" $
@@ -123,6 +126,8 @@ spec env =
           (encode hdl)
           !!! const 403 === statusCode
 
+-- @END
+
 inwardCallWithHeaders ::
   (MonadIO m, MonadHttp m, MonadReader TestEnv m, HasCallStack) =>
   ByteString ->
diff --git a/services/federator/test/unit/Test/Federator/Validation.hs b/services/federator/test/unit/Test/Federator/Validation.hs
index a7ab13525d..412b773c71 100644
--- a/services/federator/test/unit/Test/Federator/Validation.hs
+++ b/services/federator/test/unit/Test/Federator/Validation.hs
@@ -165,6 +165,9 @@ validateDomainCertMissing =
         $ validateDomain Nothing "foo.example.com"
     res @?= Left NoClientCertificate
 
+-- @SF.Federation @TSFI.RESTfulAPI @S2 @S3 @S7
+-- 1 - Receiving backend fails to authenticate the client certificate when sending connection
+-- request to infrastructure domain -> Authentication Error expected
 validateDomainCertInvalid :: TestTree
 validateDomainCertInvalid =
   testCase "should fail if the client certificate is invalid" $ do
@@ -179,6 +182,9 @@ validateDomainCertInvalid =
 -- @SF.Federation @S3 @S7
 --
 -- Is this test case #2?
+--
+-- 2 - Sending backend provided infrastructure domain + mismatching backend domain (wrong
+-- Wire-origin-domain header) -> Authorization error expected
 validateDomainCertWrongDomain :: TestTree
 validateDomainCertWrongDomain =
   testCase "should fail if the client certificate has a wrong domain" $ do
@@ -226,6 +232,8 @@ validateDomainMultipleFederators =
         validateDomain (Just secondExampleCert) (toByteString' domain)
     resSecond @?= domain
 
+-- TODO: what does this test do?
+--
 -- FUTUREWORK: is this test really necessary?
 validateDomainDiscoveryFailed :: TestTree
 validateDomainDiscoveryFailed =

From 9091c8731084863e099a7e208ad9b318f9518d5d Mon Sep 17 00:00:00 2001
From: Matthias Fischmann <mf@zerobuzz.net>
Date: Fri, 17 Dec 2021 10:45:12 +0100
Subject: [PATCH 08/24] ...

---
 services/federator/src/Federator/Discovery.hs            | 2 +-
 services/federator/src/Federator/Validation.hs           | 3 ++-
 .../test/integration/Test/Federator/IngressSpec.hs       | 8 ++++++--
 .../test/integration/Test/Federator/InwardSpec.hs        | 8 ++++++--
 .../federator/test/unit/Test/Federator/InternalServer.hs | 1 +
 .../federator/test/unit/Test/Federator/Validation.hs     | 9 +++++++--
 6 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/services/federator/src/Federator/Discovery.hs b/services/federator/src/Federator/Discovery.hs
index 65844e33d8..9e0cd93ff1 100644
--- a/services/federator/src/Federator/Discovery.hs
+++ b/services/federator/src/Federator/Discovery.hs
@@ -49,7 +49,7 @@ instance AsWai DiscoveryFailure where
     where
       (status, label) = case e of
         DiscoveryFailureSrvNotAvailable _ -> (HTTP.status422, "invalid-domain")
-        DiscoveryFailureDNSError _ -> (HTTP.status500, "discovery-failure")
+        DiscoveryFailureDNSError _ -> (HTTP.status4xx {- which one? -}, "discovery-failure")
   waiErrorDescription :: DiscoveryFailure -> Text
   waiErrorDescription (DiscoveryFailureSrvNotAvailable msg) =
     "srv record not found: " <> Text.decodeUtf8 msg
diff --git a/services/federator/src/Federator/Validation.hs b/services/federator/src/Federator/Validation.hs
index 4d7f35867d..6dc9e21ebd 100644
--- a/services/federator/src/Federator/Validation.hs
+++ b/services/federator/src/Federator/Validation.hs
@@ -150,6 +150,7 @@ validateDomain ::
 validateDomain Nothing _ = throw NoClientCertificate
 validateDomain (Just encodedCertificate) unparsedDomain = do
   targetDomain <- parseDomain unparsedDomain
+  ensureCanFederateWith targetDomain
 
   -- run discovery to find the hostname of the client federator
   certificate <-
@@ -160,7 +161,7 @@ validateDomain (Just encodedCertificate) unparsedDomain = do
   unless (any null validationErrors) $
     throw $ AuthenticationFailure validationErrors
 
-  ensureCanFederateWith targetDomain $> targetDomain
+  pure targetDomain
 
 -- | Match a hostname against the domain names of a certificate.
 --
diff --git a/services/federator/test/integration/Test/Federator/IngressSpec.hs b/services/federator/test/integration/Test/Federator/IngressSpec.hs
index 0f6d76e175..b1db9851b1 100644
--- a/services/federator/test/integration/Test/Federator/IngressSpec.hs
+++ b/services/federator/test/integration/Test/Federator/IngressSpec.hs
@@ -71,6 +71,10 @@ spec env = do
   -- @SF.Federation @TSFI.RESTfulAPI @S2 @S3 @S7
   -- 1 - Receiving backend fails to authenticate the client certificate when sending
   -- connection request to infrastructure domain -> Authentication Error expected
+  --
+  -- we test interface between federator and ingress here, nginz mocks ingress.  primarily
+  -- intended to test that federator is using the right header name.  but it's also testing
+  -- prop 1 as a side-effec.
   it "should not be accessible without a client certificate" $
     runTestFederator env $ do
       brig <- view teBrig <$> ask
@@ -96,10 +100,10 @@ spec env = do
           expectationFailure "Expected client certificate error, got remote error"
         Left (RemoteErrorResponse _ status _) -> status `shouldBe` HTTP.status400
 
-{-# DISABLE_ORMOLU #-}
+-- {-# DISABLE_ORMOLU #-}
 -- TODO: how does ormolu do this again?
 -- @END
-{-# ENSABLE_ORMOLU #-}
+--- {-# ENSABLE_ORMOLU #-}
 
 runTestSem :: Sem '[Input TestEnv, Embed IO] a -> TestFederator IO a
 runTestSem action = do
diff --git a/services/federator/test/integration/Test/Federator/InwardSpec.hs b/services/federator/test/integration/Test/Federator/InwardSpec.hs
index d481eb5bad..a97233f814 100644
--- a/services/federator/test/integration/Test/Federator/InwardSpec.hs
+++ b/services/federator/test/integration/Test/Federator/InwardSpec.hs
@@ -73,7 +73,7 @@ spec env =
         liftIO $ bdy `shouldBe` expectedProfile
 
     -- @SF.Federation @TSFI.RESTfulAPI @S2 @S3 @S7
-    -- This maybe case 2 from https://wearezeta.atlassian.net/browse/SQSERVICES-1128
+    -- This maybe case 3 from https://wearezeta.atlassian.net/browse/SQCORE-1198
     it "shouldRejectMissMatchingOriginDomain" $
       runTestFederator env $
         do
@@ -82,7 +82,7 @@ spec env =
           hdl <- randomHandle
           _ <- putHandle brig (userId user) hdl
           inwardCallWithOriginDomain "unknown-domain.com" "/federation/brig/get-user-by-handle" (encode hdl)
-            !!! const 500 === statusCode
+            !!! const 4 xx === statusCode "discovery-error"
     -- @END
 
     it "should be able to call cargohold" $
@@ -114,6 +114,7 @@ spec env =
     -- @SF.Federation @TSFI.RESTfulAPI @S2 @S3 @S7
     -- 1 - Receiving backend fails to authenticate the client certificate when sending
     -- connection request to infrastructure domain -> Authentication Error expected
+    --
     -- Matching client certificates against domain names is better tested in
     -- unit tests.
     it "should reject requests without a client certificate" $
@@ -128,6 +129,8 @@ spec env =
 
 -- @END
 
+-- what's the difference between "ingress" and "inward"?  ingress is nginz, inward is call galley directly.
+
 inwardCallWithHeaders ::
   (MonadIO m, MonadHttp m, MonadReader TestEnv m, HasCallStack) =>
   ByteString ->
@@ -150,6 +153,7 @@ inwardCall ::
   LBS.ByteString ->
   m (Response (Maybe LByteString))
 inwardCall requestPath payload = do
+  -- TODO: implement in terms of 'inwardCallwithOriginDomain'
   Endpoint fedHost fedPort <- cfgFederatorExternal <$> view teTstOpts
   originDomain <- cfgOriginDomain <$> view teTstOpts
   clientCertFilename <- clientCertificate . optSettings . view teOpts <$> ask
diff --git a/services/federator/test/unit/Test/Federator/InternalServer.hs b/services/federator/test/unit/Test/Federator/InternalServer.hs
index db3393c529..a00169e5ee 100644
--- a/services/federator/test/unit/Test/Federator/InternalServer.hs
+++ b/services/federator/test/unit/Test/Federator/InternalServer.hs
@@ -99,6 +99,7 @@ federatedRequestSuccess =
     body <- Wai.lazyResponseBody res
     body @?= "\"bar\""
 
+-- 3.  yeay
 federatedRequestFailureAllowList :: TestTree
 federatedRequestFailureAllowList =
   testCase "should not make a call when target domain not in the allowList" $ do
diff --git a/services/federator/test/unit/Test/Federator/Validation.hs b/services/federator/test/unit/Test/Federator/Validation.hs
index 412b773c71..a08e47f65c 100644
--- a/services/federator/test/unit/Test/Federator/Validation.hs
+++ b/services/federator/test/unit/Test/Federator/Validation.hs
@@ -133,12 +133,17 @@ validateDomainAllowListFail =
         $ validateDomain (Just exampleCert) "localhost.example.com"
     res @?= Left (FederationDenied (Domain "localhost.example.com"))
 
+-- @END
+
 -- @SF.Federation @TSFI.RESTfulAPI @S2 @S3 @S7
 --
+-- 3.
 validateDomainAllowListFailSendingSide :: TestTree
 validateDomainAllowListFailSendingSide = do
   undefined
 
+-- @END
+
 validateDomainAllowListSuccess :: TestTree
 validateDomainAllowListSuccess =
   testCase "should give parsed domain if in the allow list" $ do
@@ -168,6 +173,8 @@ validateDomainCertMissing =
 -- @SF.Federation @TSFI.RESTfulAPI @S2 @S3 @S7
 -- 1 - Receiving backend fails to authenticate the client certificate when sending connection
 -- request to infrastructure domain -> Authentication Error expected
+--
+-- leave a comment that actual tls termination happens elsewhere, can't be tested in this repo.
 validateDomainCertInvalid :: TestTree
 validateDomainCertInvalid =
   testCase "should fail if the client certificate is invalid" $ do
@@ -181,8 +188,6 @@ validateDomainCertInvalid =
 
 -- @SF.Federation @S3 @S7
 --
--- Is this test case #2?
---
 -- 2 - Sending backend provided infrastructure domain + mismatching backend domain (wrong
 -- Wire-origin-domain header) -> Authorization error expected
 validateDomainCertWrongDomain :: TestTree

From 002ea71de716233883fc69478fe84ce56bb4df41 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Marko=20Dimja=C5=A1evi=C4=87?= <marko.dimjasevic@wire.com>
Date: Fri, 17 Dec 2021 11:46:25 +0100
Subject: [PATCH 09/24] Perform a cleanup after identifying the 5 test cases

---
 services/federator/src/Federator/Discovery.hs |  2 +-
 .../integration/Test/Federator/IngressSpec.hs |  7 +++---
 .../integration/Test/Federator/InwardSpec.hs  | 25 ++++++++-----------
 .../unit/Test/Federator/InternalServer.hs     |  7 +++++-
 .../test/unit/Test/Federator/Response.hs      |  2 +-
 .../test/unit/Test/Federator/Validation.hs    | 15 +++--------
 6 files changed, 25 insertions(+), 33 deletions(-)

diff --git a/services/federator/src/Federator/Discovery.hs b/services/federator/src/Federator/Discovery.hs
index 9e0cd93ff1..d3de035181 100644
--- a/services/federator/src/Federator/Discovery.hs
+++ b/services/federator/src/Federator/Discovery.hs
@@ -49,7 +49,7 @@ instance AsWai DiscoveryFailure where
     where
       (status, label) = case e of
         DiscoveryFailureSrvNotAvailable _ -> (HTTP.status422, "invalid-domain")
-        DiscoveryFailureDNSError _ -> (HTTP.status4xx {- which one? -}, "discovery-failure")
+        DiscoveryFailureDNSError _ -> (HTTP.status409, "discovery-failure")
   waiErrorDescription :: DiscoveryFailure -> Text
   waiErrorDescription (DiscoveryFailureSrvNotAvailable msg) =
     "srv record not found: " <> Text.decodeUtf8 msg
diff --git a/services/federator/test/integration/Test/Federator/IngressSpec.hs b/services/federator/test/integration/Test/Federator/IngressSpec.hs
index b1db9851b1..4d02a284ec 100644
--- a/services/federator/test/integration/Test/Federator/IngressSpec.hs
+++ b/services/federator/test/integration/Test/Federator/IngressSpec.hs
@@ -72,9 +72,9 @@ spec env = do
   -- 1 - Receiving backend fails to authenticate the client certificate when sending
   -- connection request to infrastructure domain -> Authentication Error expected
   --
-  -- we test interface between federator and ingress here, nginz mocks ingress.  primarily
-  -- intended to test that federator is using the right header name.  but it's also testing
-  -- prop 1 as a side-effec.
+  -- We test the interface between federator and ingress here, nginz mocks ingress.  Primarily
+  -- intended to test that federator is using the right header name, but it's also testing
+  -- prop 1 as a side-effect.
   it "should not be accessible without a client certificate" $
     runTestFederator env $ do
       brig <- view teBrig <$> ask
@@ -101,7 +101,6 @@ spec env = do
         Left (RemoteErrorResponse _ status _) -> status `shouldBe` HTTP.status400
 
 -- {-# DISABLE_ORMOLU #-}
--- TODO: how does ormolu do this again?
 -- @END
 --- {-# ENSABLE_ORMOLU #-}
 
diff --git a/services/federator/test/integration/Test/Federator/InwardSpec.hs b/services/federator/test/integration/Test/Federator/InwardSpec.hs
index a97233f814..ffbb9a43fb 100644
--- a/services/federator/test/integration/Test/Federator/InwardSpec.hs
+++ b/services/federator/test/integration/Test/Federator/InwardSpec.hs
@@ -31,6 +31,7 @@ import Data.Text.Encoding
 import Federator.Options
 import Imports
 import qualified Network.HTTP.Types as HTTP
+import Network.Wai.Utilities.Error
 import qualified Network.Wai.Utilities.Error as E
 import Test.Federator.Util
 import Test.Hspec
@@ -73,7 +74,9 @@ spec env =
         liftIO $ bdy `shouldBe` expectedProfile
 
     -- @SF.Federation @TSFI.RESTfulAPI @S2 @S3 @S7
-    -- This maybe case 3 from https://wearezeta.atlassian.net/browse/SQCORE-1198
+    --
+    -- 4 - Test incoming request from non-included domain when allowlist is
+    -- configured -> authorization error expected
     it "shouldRejectMissMatchingOriginDomain" $
       runTestFederator env $
         do
@@ -82,7 +85,9 @@ spec env =
           hdl <- randomHandle
           _ <- putHandle brig (userId user) hdl
           inwardCallWithOriginDomain "unknown-domain.com" "/federation/brig/get-user-by-handle" (encode hdl)
-            !!! const 4 xx === statusCode "discovery-error"
+            !!! do
+              const 409 === statusCode
+              const (Just "discovery-failure") === fmap label . responseJsonMaybe
     -- @END
 
     it "should be able to call cargohold" $
@@ -129,7 +134,8 @@ spec env =
 
 -- @END
 
--- what's the difference between "ingress" and "inward"?  ingress is nginz, inward is call galley directly.
+-- The difference between "ingress" and "inward": ingress is nginz, inward is
+-- calling the federator directly.
 
 inwardCallWithHeaders ::
   (MonadIO m, MonadHttp m, MonadReader TestEnv m, HasCallStack) =>
@@ -153,19 +159,8 @@ inwardCall ::
   LBS.ByteString ->
   m (Response (Maybe LByteString))
 inwardCall requestPath payload = do
-  -- TODO: implement in terms of 'inwardCallwithOriginDomain'
-  Endpoint fedHost fedPort <- cfgFederatorExternal <$> view teTstOpts
   originDomain <- cfgOriginDomain <$> view teTstOpts
-  clientCertFilename <- clientCertificate . optSettings . view teOpts <$> ask
-  clientCert <- liftIO $ BS.readFile clientCertFilename
-  post
-    ( host (encodeUtf8 fedHost)
-        . port fedPort
-        . path requestPath
-        . header "X-SSL-Certificate" (HTTP.urlEncode True clientCert)
-        . header originDomainHeaderName (toByteString' originDomain)
-        . bytes (toByteString' payload)
-    )
+  inwardCallWithOriginDomain (toByteString' originDomain) requestPath payload
 
 inwardCallWithOriginDomain ::
   (MonadIO m, MonadHttp m, MonadReader TestEnv m, HasCallStack) =>
diff --git a/services/federator/test/unit/Test/Federator/InternalServer.hs b/services/federator/test/unit/Test/Federator/InternalServer.hs
index a00169e5ee..f5688c62c2 100644
--- a/services/federator/test/unit/Test/Federator/InternalServer.hs
+++ b/services/federator/test/unit/Test/Federator/InternalServer.hs
@@ -99,7 +99,10 @@ federatedRequestSuccess =
     body <- Wai.lazyResponseBody res
     body @?= "\"bar\""
 
--- 3.  yeay
+-- @SF.Federation @TSFI.RESTfulAPI @S2 @S3 @S7
+--
+-- 3. Outgoing request to non-included domain when allowlist is configured ->
+-- authorization error expected
 federatedRequestFailureAllowList :: TestTree
 federatedRequestFailureAllowList =
   testCase "should not make a call when target domain not in the allowList" $ do
@@ -135,3 +138,5 @@ federatedRequestFailureAllowList =
         . runInputConst settings
         $ callOutward request
     eith @?= Left (FederationDenied targetDomain)
+
+-- @END
diff --git a/services/federator/test/unit/Test/Federator/Response.hs b/services/federator/test/unit/Test/Federator/Response.hs
index 8413b9b95a..b02d62da82 100644
--- a/services/federator/test/unit/Test/Federator/Response.hs
+++ b/services/federator/test/unit/Test/Federator/Response.hs
@@ -72,7 +72,7 @@ testDiscoveryFailure =
         throw (DiscoveryFailureDNSError "mock error")
     body <- Wai.lazyResponseBody resp
     let merr = Aeson.decode body
-    Wai.responseStatus resp @?= HTTP.status500
+    Wai.responseStatus resp @?= HTTP.status409
     fmap Wai.label merr @?= Just "discovery-failure"
 
 testRemoteError :: TestTree
diff --git a/services/federator/test/unit/Test/Federator/Validation.hs b/services/federator/test/unit/Test/Federator/Validation.hs
index a08e47f65c..248fbbb3bd 100644
--- a/services/federator/test/unit/Test/Federator/Validation.hs
+++ b/services/federator/test/unit/Test/Federator/Validation.hs
@@ -135,15 +135,6 @@ validateDomainAllowListFail =
 
 -- @END
 
--- @SF.Federation @TSFI.RESTfulAPI @S2 @S3 @S7
---
--- 3.
-validateDomainAllowListFailSendingSide :: TestTree
-validateDomainAllowListFailSendingSide = do
-  undefined
-
--- @END
-
 validateDomainAllowListSuccess :: TestTree
 validateDomainAllowListSuccess =
   testCase "should give parsed domain if in the allow list" $ do
@@ -186,6 +177,8 @@ validateDomainCertInvalid =
         $ validateDomain (Just "not a certificate") "foo.example.com"
     res @?= Left (CertificateParseError "no certificate found")
 
+-- @END
+
 -- @SF.Federation @S3 @S7
 --
 -- 2 - Sending backend provided infrastructure domain + mismatching backend domain (wrong
@@ -202,6 +195,8 @@ validateDomainCertWrongDomain =
         $ validateDomain (Just exampleCert) "foo.example.com"
     res @?= Left (AuthenticationFailure (pure [X509.NameMismatch "foo.example.com"]))
 
+-- @END
+
 validateDomainCertCN :: TestTree
 validateDomainCertCN =
   testCase "should succeed if the certificate has subject CN but no SAN" $ do
@@ -237,8 +232,6 @@ validateDomainMultipleFederators =
         validateDomain (Just secondExampleCert) (toByteString' domain)
     resSecond @?= domain
 
--- TODO: what does this test do?
---
 -- FUTUREWORK: is this test really necessary?
 validateDomainDiscoveryFailed :: TestTree
 validateDomainDiscoveryFailed =

From 332f11c577cfb70beddd23bccf8b6a41420131dc Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Marko=20Dimja=C5=A1evi=C4=87?= <marko.dimjasevic@wire.com>
Date: Fri, 17 Dec 2021 11:52:32 +0100
Subject: [PATCH 10/24] Add a description to a test case

---
 services/federator/test/unit/Test/Federator/Options.hs | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/services/federator/test/unit/Test/Federator/Options.hs b/services/federator/test/unit/Test/Federator/Options.hs
index ac5edb8d0d..24ebcb3c94 100644
--- a/services/federator/test/unit/Test/Federator/Options.hs
+++ b/services/federator/test/unit/Test/Federator/Options.hs
@@ -172,6 +172,10 @@ testSettings =
               "expected failure for non-existing client certificate, got: "
                 <> show (tlsSettings ^. creds),
       -- @SF.Federation @S3 @S7
+      --
+      -- 1 - Receiving backend fails to authenticate the client certificate when
+      -- sending connection request to infrastructure domain -> Authentication
+      -- Error expected
       testCase "fail on invalid certificate" $ do
         let settings =
               defRunSettings

From c6e087ee814c13c4ed79fdd862314095124f3588 Mon Sep 17 00:00:00 2001
From: Matthias Fischmann <mf@zerobuzz.net>
Date: Fri, 17 Dec 2021 12:01:41 +0100
Subject: [PATCH 11/24] Changelog.

---
 changelog.d/5-internal/map-federation-tests | 1 +
 1 file changed, 1 insertion(+)
 create mode 100644 changelog.d/5-internal/map-federation-tests

diff --git a/changelog.d/5-internal/map-federation-tests b/changelog.d/5-internal/map-federation-tests
new file mode 100644
index 0000000000..b53c1449dd
--- /dev/null
+++ b/changelog.d/5-internal/map-federation-tests
@@ -0,0 +1 @@
+Tag integration tests for security audit.
\ No newline at end of file

From e5ad3ec3b328a3cb8c52ba8b6c69d836cec8d87b Mon Sep 17 00:00:00 2001
From: Matthias Fischmann <mf@zerobuzz.net>
Date: Fri, 17 Dec 2021 12:08:55 +0100
Subject: [PATCH 12/24] Haddocks.

---
 .../federator/test/integration/Test/Federator/IngressSpec.hs | 2 ++
 .../federator/test/integration/Test/Federator/InwardSpec.hs  | 5 ++++-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/services/federator/test/integration/Test/Federator/IngressSpec.hs b/services/federator/test/integration/Test/Federator/IngressSpec.hs
index 4d02a284ec..8c26162cb9 100644
--- a/services/federator/test/integration/Test/Federator/IngressSpec.hs
+++ b/services/federator/test/integration/Test/Federator/IngressSpec.hs
@@ -46,6 +46,8 @@ import Wire.API.Federation.Domain
 import Wire.API.User
 import Wire.Network.DNS.SRV
 
+-- | This module contains tests for the interface between federator and ingress.  Ingress is
+-- mocked with nginz.
 spec :: TestEnv -> Spec
 spec env = do
   describe "Ingress" $ do
diff --git a/services/federator/test/integration/Test/Federator/InwardSpec.hs b/services/federator/test/integration/Test/Federator/InwardSpec.hs
index ffbb9a43fb..2995f618a8 100644
--- a/services/federator/test/integration/Test/Federator/InwardSpec.hs
+++ b/services/federator/test/integration/Test/Federator/InwardSpec.hs
@@ -40,8 +40,11 @@ import Wire.API.Federation.Domain
 import Wire.API.User
 
 -- FUTUREWORK(federation): move these tests to brig-integration (benefit: avoid duplicating all of the brig helper code)
+-- FUTUREWORK(fisx): better yet, reorganize integration tests (or at least the helpers) so
+-- they don't spread out over the different sevices.
 
--- | Path covered by this test
+-- | This module contains tests for the interface between federator and brig.  The thests call
+-- federator directly, circumnventing ingress:
 --
 --  +----------+
 --  |federator-|          +------+--+

From 7142265535245b8a98ce893f5653a8265ed20605 Mon Sep 17 00:00:00 2001
From: Matthias Fischmann <mf@zerobuzz.net>
Date: Fri, 17 Dec 2021 12:15:50 +0100
Subject: [PATCH 13/24] ormolu

---
 .../test/integration/Test/Federator/IngressSpec.hs          | 6 +++---
 .../federator/test/integration/Test/Federator/InwardSpec.hs | 2 ++
 services/spar/test-integration/Test/Spar/APISpec.hs         | 2 ++
 3 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/services/federator/test/integration/Test/Federator/IngressSpec.hs b/services/federator/test/integration/Test/Federator/IngressSpec.hs
index 8c26162cb9..940fa6ce0e 100644
--- a/services/federator/test/integration/Test/Federator/IngressSpec.hs
+++ b/services/federator/test/integration/Test/Federator/IngressSpec.hs
@@ -102,9 +102,9 @@ spec env = do
           expectationFailure "Expected client certificate error, got remote error"
         Left (RemoteErrorResponse _ status _) -> status `shouldBe` HTTP.status400
 
--- {-# DISABLE_ORMOLU #-}
--- @END
---- {-# ENSABLE_ORMOLU #-}
+{- ORMOLU_DISABLE -}
+  -- @END
+{- ORMOLU_ENSABLE -}
 
 runTestSem :: Sem '[Input TestEnv, Embed IO] a -> TestFederator IO a
 runTestSem action = do
diff --git a/services/federator/test/integration/Test/Federator/InwardSpec.hs b/services/federator/test/integration/Test/Federator/InwardSpec.hs
index 2995f618a8..67aee5b999 100644
--- a/services/federator/test/integration/Test/Federator/InwardSpec.hs
+++ b/services/federator/test/integration/Test/Federator/InwardSpec.hs
@@ -135,7 +135,9 @@ spec env =
           (encode hdl)
           !!! const 403 === statusCode
 
+{- ORMOLU_DISABLE -} -- FUTUREWORK: try a newer release of ormolu?
 -- @END
+{- ORMOLU_ENABLE -}
 
 -- The difference between "ingress" and "inward": ingress is nginz, inward is
 -- calling the federator directly.
diff --git a/services/spar/test-integration/Test/Spar/APISpec.hs b/services/spar/test-integration/Test/Spar/APISpec.hs
index af466b1650..7d73042cf0 100644
--- a/services/spar/test-integration/Test/Spar/APISpec.hs
+++ b/services/spar/test-integration/Test/Spar/APISpec.hs
@@ -482,7 +482,9 @@ specFinalizeLogin = do
               (cs . fromJust . responseBody $ sparresp) `shouldContain` "<title>wire:sso:error:forbidden</title>"
         check mkareq mkaresp submitaresp checkresp
 
+    -- {- ORMOLU_DISABLE -} -- FUTUREWORK: try a newer release of ormolu?
     -- @END
+    -- {- ORMOLU_ENABLE -}
 
     context "IdP changes response format" $ do
       it "treats NameId case-insensitively" $ do

From aac8ea0ce082b05a21836ac5df66a79da11aa0c5 Mon Sep 17 00:00:00 2001
From: Matthias Fischmann <mf@zerobuzz.net>
Date: Fri, 17 Dec 2021 12:22:44 +0100
Subject: [PATCH 14/24] Haddocks, tags, test identifiers.

---
 .../integration/Test/Federator/IngressSpec.hs  | 14 ++++++++------
 .../integration/Test/Federator/InwardSpec.hs   | 18 ++++++------------
 .../test/unit/Test/Federator/InternalServer.hs |  3 +--
 .../test/unit/Test/Federator/Options.hs        |  8 ++------
 .../test/unit/Test/Federator/Validation.hs     | 15 ++++-----------
 5 files changed, 21 insertions(+), 37 deletions(-)

diff --git a/services/federator/test/integration/Test/Federator/IngressSpec.hs b/services/federator/test/integration/Test/Federator/IngressSpec.hs
index 940fa6ce0e..a6f580e313 100644
--- a/services/federator/test/integration/Test/Federator/IngressSpec.hs
+++ b/services/federator/test/integration/Test/Federator/IngressSpec.hs
@@ -71,13 +71,15 @@ spec env = do
           actualProfile `shouldBe` (Just expectedProfile)
 
   -- @SF.Federation @TSFI.RESTfulAPI @S2 @S3 @S7
-  -- 1 - Receiving backend fails to authenticate the client certificate when sending
-  -- connection request to infrastructure domain -> Authentication Error expected
   --
-  -- We test the interface between federator and ingress here, nginz mocks ingress.  Primarily
-  -- intended to test that federator is using the right header name, but it's also testing
-  -- prop 1 as a side-effect.
-  it "should not be accessible without a client certificate" $
+  -- This test was primarily intended to test that federator is using the API right (header
+  -- name etc.), but it is also effectively testing that federator rejects clients without
+  -- certificates that have been validated by ingress.
+  --
+  -- We can't test end-to-end here: the TLS termination happens in k8s, and would have to be
+  -- tested there (and with a good emulation of the concrete configuration of the prod
+  -- system).
+  it "rejectRequestsWithoutClientCertIngress" $
     runTestFederator env $ do
       brig <- view teBrig <$> ask
       user <- randomUser brig
diff --git a/services/federator/test/integration/Test/Federator/InwardSpec.hs b/services/federator/test/integration/Test/Federator/InwardSpec.hs
index 67aee5b999..42ba000b80 100644
--- a/services/federator/test/integration/Test/Federator/InwardSpec.hs
+++ b/services/federator/test/integration/Test/Federator/InwardSpec.hs
@@ -78,9 +78,8 @@ spec env =
 
     -- @SF.Federation @TSFI.RESTfulAPI @S2 @S3 @S7
     --
-    -- 4 - Test incoming request from non-included domain when allowlist is
-    -- configured -> authorization error expected
-    it "shouldRejectMissMatchingOriginDomain" $
+    -- (This is also tested in unit tests; search for 'validateDomainCertInvalid'.)
+    it "shouldRejectMissMatchingOriginDomainInward" $
       runTestFederator env $
         do
           brig <- view teBrig <$> ask
@@ -120,12 +119,10 @@ spec env =
           !!! const 403 === statusCode
 
     -- @SF.Federation @TSFI.RESTfulAPI @S2 @S3 @S7
-    -- 1 - Receiving backend fails to authenticate the client certificate when sending
-    -- connection request to infrastructure domain -> Authentication Error expected
     --
-    -- Matching client certificates against domain names is better tested in
-    -- unit tests.
-    it "should reject requests without a client certificate" $
+    -- See related tests in unit tests (for matching client certificates against domain names)
+    -- and "IngressSpec".
+    it "rejectRequestsWithoutClientCertInward" $
       runTestFederator env $ do
         originDomain <- cfgOriginDomain <$> view teTstOpts
         hdl <- randomHandle
@@ -139,9 +136,6 @@ spec env =
 -- @END
 {- ORMOLU_ENABLE -}
 
--- The difference between "ingress" and "inward": ingress is nginz, inward is
--- calling the federator directly.
-
 inwardCallWithHeaders ::
   (MonadIO m, MonadHttp m, MonadReader TestEnv m, HasCallStack) =>
   ByteString ->
@@ -164,7 +158,7 @@ inwardCall ::
   LBS.ByteString ->
   m (Response (Maybe LByteString))
 inwardCall requestPath payload = do
-  originDomain <- cfgOriginDomain <$> view teTstOpts
+  originDomain :: Text <- cfgOriginDomain <$> view teTstOpts
   inwardCallWithOriginDomain (toByteString' originDomain) requestPath payload
 
 inwardCallWithOriginDomain ::
diff --git a/services/federator/test/unit/Test/Federator/InternalServer.hs b/services/federator/test/unit/Test/Federator/InternalServer.hs
index f5688c62c2..b384d0b88f 100644
--- a/services/federator/test/unit/Test/Federator/InternalServer.hs
+++ b/services/federator/test/unit/Test/Federator/InternalServer.hs
@@ -101,8 +101,7 @@ federatedRequestSuccess =
 
 -- @SF.Federation @TSFI.RESTfulAPI @S2 @S3 @S7
 --
--- 3. Outgoing request to non-included domain when allowlist is configured ->
--- authorization error expected
+-- Refuse to send outgoing request to non-included domain when allowlist is configured.
 federatedRequestFailureAllowList :: TestTree
 federatedRequestFailureAllowList =
   testCase "should not make a call when target domain not in the allowList" $ do
diff --git a/services/federator/test/unit/Test/Federator/Options.hs b/services/federator/test/unit/Test/Federator/Options.hs
index 24ebcb3c94..9034586273 100644
--- a/services/federator/test/unit/Test/Federator/Options.hs
+++ b/services/federator/test/unit/Test/Federator/Options.hs
@@ -171,12 +171,8 @@ testSettings =
             assertFailure $
               "expected failure for non-existing client certificate, got: "
                 <> show (tlsSettings ^. creds),
-      -- @SF.Federation @S3 @S7
-      --
-      -- 1 - Receiving backend fails to authenticate the client certificate when
-      -- sending connection request to infrastructure domain -> Authentication
-      -- Error expected
-      testCase "fail on invalid certificate" $ do
+      -- @SF.Federation @TSFI.RESTfulAPI @S3 @S7
+      testCase "failToStartWithInvalidServerCredentials" $ do
         let settings =
               defRunSettings
                 "test/resources/unit/invalid.pem"
diff --git a/services/federator/test/unit/Test/Federator/Validation.hs b/services/federator/test/unit/Test/Federator/Validation.hs
index 248fbbb3bd..785757927f 100644
--- a/services/federator/test/unit/Test/Federator/Validation.hs
+++ b/services/federator/test/unit/Test/Federator/Validation.hs
@@ -115,10 +115,7 @@ validateDomainAllowListFailSemantic =
 
 -- @SF.Federation @TSFI.RESTfulAPI @S2 @S3 @S7
 --
--- 4. Outgoing request to non-included domain when allowlist is configured ->
--- authorization error expected
---
--- Does it make sense to tag this one with @TSFI.RESTfulAPI ?
+-- Refuse to send outgoing request to non-included domain when allowlist is configured.
 validateDomainAllowListFail :: TestTree
 validateDomainAllowListFail =
   testCase "allow list validation" $ do
@@ -162,10 +159,6 @@ validateDomainCertMissing =
     res @?= Left NoClientCertificate
 
 -- @SF.Federation @TSFI.RESTfulAPI @S2 @S3 @S7
--- 1 - Receiving backend fails to authenticate the client certificate when sending connection
--- request to infrastructure domain -> Authentication Error expected
---
--- leave a comment that actual tls termination happens elsewhere, can't be tested in this repo.
 validateDomainCertInvalid :: TestTree
 validateDomainCertInvalid =
   testCase "should fail if the client certificate is invalid" $ do
@@ -179,10 +172,10 @@ validateDomainCertInvalid =
 
 -- @END
 
--- @SF.Federation @S3 @S7
+-- @SF.Federation @TSFI.RESTfulAPI @S3 @S7
 --
--- 2 - Sending backend provided infrastructure domain + mismatching backend domain (wrong
--- Wire-origin-domain header) -> Authorization error expected
+-- Reject request if the infrastructure domain in the client cert does not match the backend
+-- domain in the `Wire-origin-domain` header.
 validateDomainCertWrongDomain :: TestTree
 validateDomainCertWrongDomain =
   testCase "should fail if the client certificate has a wrong domain" $ do

From 7cfd78a716036971e4d6cc0faf23ed6c95b12fca Mon Sep 17 00:00:00 2001
From: Matthias Fischmann <mf@zerobuzz.net>
Date: Fri, 17 Dec 2021 12:45:50 +0100
Subject: [PATCH 15/24] Typo.

---
 .../federator/test/integration/Test/Federator/InwardSpec.hs     | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/services/federator/test/integration/Test/Federator/InwardSpec.hs b/services/federator/test/integration/Test/Federator/InwardSpec.hs
index 42ba000b80..b89e7920b2 100644
--- a/services/federator/test/integration/Test/Federator/InwardSpec.hs
+++ b/services/federator/test/integration/Test/Federator/InwardSpec.hs
@@ -79,7 +79,7 @@ spec env =
     -- @SF.Federation @TSFI.RESTfulAPI @S2 @S3 @S7
     --
     -- (This is also tested in unit tests; search for 'validateDomainCertInvalid'.)
-    it "shouldRejectMissMatchingOriginDomainInward" $
+    it "shouldRejectMissmatchingOriginDomainInward" $
       runTestFederator env $
         do
           brig <- view teBrig <$> ask

From 5553dd5e40bb77d8d559f86a99e59b7a5aba2805 Mon Sep 17 00:00:00 2001
From: Matthias Fischmann <mf@zerobuzz.net>
Date: Fri, 17 Dec 2021 12:45:59 +0100
Subject: [PATCH 16/24] Http status code.

---
 services/federator/src/Federator/Discovery.hs                   | 2 +-
 .../federator/test/integration/Test/Federator/InwardSpec.hs     | 2 +-
 services/federator/test/unit/Test/Federator/Response.hs         | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/services/federator/src/Federator/Discovery.hs b/services/federator/src/Federator/Discovery.hs
index d3de035181..4148e67c13 100644
--- a/services/federator/src/Federator/Discovery.hs
+++ b/services/federator/src/Federator/Discovery.hs
@@ -49,7 +49,7 @@ instance AsWai DiscoveryFailure where
     where
       (status, label) = case e of
         DiscoveryFailureSrvNotAvailable _ -> (HTTP.status422, "invalid-domain")
-        DiscoveryFailureDNSError _ -> (HTTP.status409, "discovery-failure")
+        DiscoveryFailureDNSError _ -> (HTTP.status400, "discovery-failure")
   waiErrorDescription :: DiscoveryFailure -> Text
   waiErrorDescription (DiscoveryFailureSrvNotAvailable msg) =
     "srv record not found: " <> Text.decodeUtf8 msg
diff --git a/services/federator/test/integration/Test/Federator/InwardSpec.hs b/services/federator/test/integration/Test/Federator/InwardSpec.hs
index b89e7920b2..f8977a54ae 100644
--- a/services/federator/test/integration/Test/Federator/InwardSpec.hs
+++ b/services/federator/test/integration/Test/Federator/InwardSpec.hs
@@ -88,7 +88,7 @@ spec env =
           _ <- putHandle brig (userId user) hdl
           inwardCallWithOriginDomain "unknown-domain.com" "/federation/brig/get-user-by-handle" (encode hdl)
             !!! do
-              const 409 === statusCode
+              const 400 === statusCode
               const (Just "discovery-failure") === fmap label . responseJsonMaybe
     -- @END
 
diff --git a/services/federator/test/unit/Test/Federator/Response.hs b/services/federator/test/unit/Test/Federator/Response.hs
index b02d62da82..b8addefb8d 100644
--- a/services/federator/test/unit/Test/Federator/Response.hs
+++ b/services/federator/test/unit/Test/Federator/Response.hs
@@ -72,7 +72,7 @@ testDiscoveryFailure =
         throw (DiscoveryFailureDNSError "mock error")
     body <- Wai.lazyResponseBody resp
     let merr = Aeson.decode body
-    Wai.responseStatus resp @?= HTTP.status409
+    Wai.responseStatus resp @?= HTTP.status400
     fmap Wai.label merr @?= Just "discovery-failure"
 
 testRemoteError :: TestTree

From d3a58b6dd829ff5547968a455beef89d49bc5642 Mon Sep 17 00:00:00 2001
From: Matthias Fischmann <mf@zerobuzz.net>
Date: Fri, 17 Dec 2021 12:50:37 +0100
Subject: [PATCH 17/24] Ormolu

---
 .../test/integration/Test/Federator/IngressSpec.hs          | 6 +++---
 .../federator/test/integration/Test/Federator/InwardSpec.hs | 4 ++--
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/services/federator/test/integration/Test/Federator/IngressSpec.hs b/services/federator/test/integration/Test/Federator/IngressSpec.hs
index a6f580e313..9a2bdd0c5f 100644
--- a/services/federator/test/integration/Test/Federator/IngressSpec.hs
+++ b/services/federator/test/integration/Test/Federator/IngressSpec.hs
@@ -104,9 +104,9 @@ spec env = do
           expectationFailure "Expected client certificate error, got remote error"
         Left (RemoteErrorResponse _ status _) -> status `shouldBe` HTTP.status400
 
-{- ORMOLU_DISABLE -}
-  -- @END
-{- ORMOLU_ENSABLE -}
+-- TODO: ORMOLU_DISABLE
+-- @END
+-- ORMOLU_ENSABLE
 
 runTestSem :: Sem '[Input TestEnv, Embed IO] a -> TestFederator IO a
 runTestSem action = do
diff --git a/services/federator/test/integration/Test/Federator/InwardSpec.hs b/services/federator/test/integration/Test/Federator/InwardSpec.hs
index f8977a54ae..d5f223536d 100644
--- a/services/federator/test/integration/Test/Federator/InwardSpec.hs
+++ b/services/federator/test/integration/Test/Federator/InwardSpec.hs
@@ -132,9 +132,9 @@ spec env =
           (encode hdl)
           !!! const 403 === statusCode
 
-{- ORMOLU_DISABLE -} -- FUTUREWORK: try a newer release of ormolu?
+-- TODO: ORMOLU_DISABLE
 -- @END
-{- ORMOLU_ENABLE -}
+-- ORMOLU_ENABLE
 
 inwardCallWithHeaders ::
   (MonadIO m, MonadHttp m, MonadReader TestEnv m, HasCallStack) =>

From 1b765ac59f511698ac1e3c07443800bab59441ab Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Marko=20Dimja=C5=A1evi=C4=87?= <marko.dimjasevic@wire.com>
Date: Fri, 17 Dec 2021 14:43:34 +0100
Subject: [PATCH 18/24] Fix a couple of typos

---
 .../federator/test/integration/Test/Federator/IngressSpec.hs  | 4 ++--
 .../federator/test/integration/Test/Federator/InwardSpec.hs   | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/services/federator/test/integration/Test/Federator/IngressSpec.hs b/services/federator/test/integration/Test/Federator/IngressSpec.hs
index 9a2bdd0c5f..34b6af1d32 100644
--- a/services/federator/test/integration/Test/Federator/IngressSpec.hs
+++ b/services/federator/test/integration/Test/Federator/IngressSpec.hs
@@ -104,9 +104,9 @@ spec env = do
           expectationFailure "Expected client certificate error, got remote error"
         Left (RemoteErrorResponse _ status _) -> status `shouldBe` HTTP.status400
 
--- TODO: ORMOLU_DISABLE
+-- FUTUREWORK: ORMOLU_DISABLE
 -- @END
--- ORMOLU_ENSABLE
+-- ORMOLU_ENABLE
 
 runTestSem :: Sem '[Input TestEnv, Embed IO] a -> TestFederator IO a
 runTestSem action = do
diff --git a/services/federator/test/integration/Test/Federator/InwardSpec.hs b/services/federator/test/integration/Test/Federator/InwardSpec.hs
index d5f223536d..7e3978ab7d 100644
--- a/services/federator/test/integration/Test/Federator/InwardSpec.hs
+++ b/services/federator/test/integration/Test/Federator/InwardSpec.hs
@@ -43,7 +43,7 @@ import Wire.API.User
 -- FUTUREWORK(fisx): better yet, reorganize integration tests (or at least the helpers) so
 -- they don't spread out over the different sevices.
 
--- | This module contains tests for the interface between federator and brig.  The thests call
+-- | This module contains tests for the interface between federator and brig.  The tests call
 -- federator directly, circumnventing ingress:
 --
 --  +----------+

From acfd89611f204fe9d5c398823fff77fb2dbc2e55 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Marko=20Dimja=C5=A1evi=C4=87?= <marko.dimjasevic@wire.com>
Date: Fri, 17 Dec 2021 15:18:51 +0100
Subject: [PATCH 19/24] Point to a non-existing domain in a DNS test

---
 .../federator/test/integration/Test/Federator/InwardSpec.hs | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/services/federator/test/integration/Test/Federator/InwardSpec.hs b/services/federator/test/integration/Test/Federator/InwardSpec.hs
index 7e3978ab7d..2f76e470b6 100644
--- a/services/federator/test/integration/Test/Federator/InwardSpec.hs
+++ b/services/federator/test/integration/Test/Federator/InwardSpec.hs
@@ -86,10 +86,10 @@ spec env =
           user <- randomUser brig
           hdl <- randomHandle
           _ <- putHandle brig (userId user) hdl
-          inwardCallWithOriginDomain "unknown-domain.com" "/federation/brig/get-user-by-handle" (encode hdl)
+          inwardCallWithOriginDomain "too.notadomain" "/federation/brig/get-user-by-handle" (encode hdl)
             !!! do
-              const 400 === statusCode
-              const (Just "discovery-failure") === fmap label . responseJsonMaybe
+              const 422 === statusCode
+              const (Just "invalid-domain") === fmap label . responseJsonMaybe
     -- @END
 
     it "should be able to call cargohold" $

From f205a21b1dd0d5de571f288ae0c5c6a6f5d389ab Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Marko=20Dimja=C5=A1evi=C4=87?= <marko.dimjasevic@wire.com>
Date: Fri, 17 Dec 2021 15:20:51 +0100
Subject: [PATCH 20/24] Update the Swagger documentation for a federation error

---
 services/brig/docs/swagger.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/services/brig/docs/swagger.md b/services/brig/docs/swagger.md
index 63ec978afb..59e6c1c3b8 100644
--- a/services/brig/docs/swagger.md
+++ b/services/brig/docs/swagger.md
@@ -54,7 +54,7 @@ An error in this category likely indicates an issue with the configuration of fe
 
  - **Federation unavailable** (status: 500, label: `federation-not-available`): Federation is configured for this backend, but the local federator cannot be reached. This can be transient, so clients should retry the request.
  - **Federation not implemented** (status: 500, label: `federation-not-implemented`): Federated behaviour for a certain endpoint is not yet implemented.
- - **Federator discovery failed** (status: 500, label: `discovery-failure`): A DNS error occurred during discovery of a remote backend. This can be transient, so clients should retry the request.
+ - **Federator discovery failed** (status: 400, label: `discovery-failure`): A DNS error occurred during discovery of a remote backend. This can be transient, so clients should retry the request.
  - **Local federation error** (status: 500, label: `federation-local-error`): An error occurred in the communication between this backend and its local federator. These errors are most likely caused by bugs in the backend, and should be reported as such.
 
 ### Remote federation errors

From 1e528dae904b9ff04e6c7162ef0cc4c9d68b06d7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Marko=20Dimja=C5=A1evi=C4=87?= <marko.dimjasevic@wire.com>
Date: Mon, 20 Dec 2021 13:50:26 +0100
Subject: [PATCH 21/24] Redirect from an integration test to an existing unit
 test

---
 .../test/integration/Test/Federator/InwardSpec.hs | 15 +++------------
 1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/services/federator/test/integration/Test/Federator/InwardSpec.hs b/services/federator/test/integration/Test/Federator/InwardSpec.hs
index 2f76e470b6..14000d693b 100644
--- a/services/federator/test/integration/Test/Federator/InwardSpec.hs
+++ b/services/federator/test/integration/Test/Federator/InwardSpec.hs
@@ -31,7 +31,6 @@ import Data.Text.Encoding
 import Federator.Options
 import Imports
 import qualified Network.HTTP.Types as HTTP
-import Network.Wai.Utilities.Error
 import qualified Network.Wai.Utilities.Error as E
 import Test.Federator.Util
 import Test.Hspec
@@ -78,18 +77,10 @@ spec env =
 
     -- @SF.Federation @TSFI.RESTfulAPI @S2 @S3 @S7
     --
-    -- (This is also tested in unit tests; search for 'validateDomainCertInvalid'.)
+    -- (This is also tested in unit tests; search for
+    -- 'validateDomainCertInvalid' and 'testDiscoveryFailure'.)
     it "shouldRejectMissmatchingOriginDomainInward" $
-      runTestFederator env $
-        do
-          brig <- view teBrig <$> ask
-          user <- randomUser brig
-          hdl <- randomHandle
-          _ <- putHandle brig (userId user) hdl
-          inwardCallWithOriginDomain "too.notadomain" "/federation/brig/get-user-by-handle" (encode hdl)
-            !!! do
-              const 422 === statusCode
-              const (Just "invalid-domain") === fmap label . responseJsonMaybe
+      runTestFederator env $ pure ()
     -- @END
 
     it "should be able to call cargohold" $

From df8e1756b464ce3a43f9cf7ab7f7fbafa3900cf7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Marko=20Dimja=C5=A1evi=C4=87?= <marko.dimjasevic@wire.com>
Date: Mon, 20 Dec 2021 15:07:48 +0100
Subject: [PATCH 22/24] Feedback by Matthias: Update
 services/federator/test/integration/Test/Federator/InwardSpec.hs

Co-authored-by: fisx <mf@zerobuzz.net>
---
 .../federator/test/integration/Test/Federator/InwardSpec.hs     | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/services/federator/test/integration/Test/Federator/InwardSpec.hs b/services/federator/test/integration/Test/Federator/InwardSpec.hs
index 14000d693b..00b9d2022f 100644
--- a/services/federator/test/integration/Test/Federator/InwardSpec.hs
+++ b/services/federator/test/integration/Test/Federator/InwardSpec.hs
@@ -77,7 +77,7 @@ spec env =
 
     -- @SF.Federation @TSFI.RESTfulAPI @S2 @S3 @S7
     --
-    -- (This is also tested in unit tests; search for
+    -- (This is also tested in tests; search for
     -- 'validateDomainCertInvalid' and 'testDiscoveryFailure'.)
     it "shouldRejectMissmatchingOriginDomainInward" $
       runTestFederator env $ pure ()

From bf29e7aa861d3550fbdf848b920ef75066bf2213 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Marko=20Dimja=C5=A1evi=C4=87?= <marko.dimjasevic@wire.com>
Date: Mon, 20 Dec 2021 16:20:10 +0100
Subject: [PATCH 23/24] Hi CI


From 654695172cfbe0717b3e1cc66777b2581415dfad Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Marko=20Dimja=C5=A1evi=C4=87?= <marko.dimjasevic@wire.com>
Date: Mon, 20 Dec 2021 16:21:41 +0100
Subject: [PATCH 24/24] Improve a test comment

---
 .../federator/test/integration/Test/Federator/InwardSpec.hs     | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/services/federator/test/integration/Test/Federator/InwardSpec.hs b/services/federator/test/integration/Test/Federator/InwardSpec.hs
index 00b9d2022f..3ef28b069d 100644
--- a/services/federator/test/integration/Test/Federator/InwardSpec.hs
+++ b/services/federator/test/integration/Test/Federator/InwardSpec.hs
@@ -77,7 +77,7 @@ spec env =
 
     -- @SF.Federation @TSFI.RESTfulAPI @S2 @S3 @S7
     --
-    -- (This is also tested in tests; search for
+    -- (This is tested in unit tests; search for
     -- 'validateDomainCertInvalid' and 'testDiscoveryFailure'.)
     it "shouldRejectMissmatchingOriginDomainInward" $
       runTestFederator env $ pure ()