Leave domain empty in cookies, but account for clients with old cookies.

deploy/services-demo/conf/brig.demo-docker.yaml

@@ -102,7 +102,6 @@ optSettings:
   setActivationTimeout: 1209600     # 1 day
   setTeamInvitationTimeout: 1814400 # 21 days
   setUserMaxConnections: 1000
-  setCookieDomain: brig
   setCookieInsecure: false
   setUserCookieRenewAge: 1209600 # 14 days
   setUserCookieLimit: 32

deploy/services-demo/conf/brig.demo.yaml

@@ -102,7 +102,6 @@ optSettings:
   setActivationTimeout: 1209600     # 1 day
   setTeamInvitationTimeout: 1814400 # 21 days
   setUserMaxConnections: 1000
-  setCookieDomain: localhost
   setCookieInsecure: false
   setUserCookieRenewAge: 1209600 # 14 days
   setUserCookieLimit: 32

deploy/services-demo/conf/nginz/nginx.conf

@@ -290,6 +290,11 @@ http {
       proxy_pass http://galley;
     }
 
+    location ~* ^/teams/([^/]*)/legalhold(.*) {
+      include common_response_with_zauth.conf;
+      proxy_pass http://galley;
+    }
+
     # Gundeck Endpoints
 
     rewrite ^/api-docs/push  /push/api-docs?base_url=http://127.0.0.1:8080/ break;

services/brig/brig.integration.yaml

@@ -135,7 +135,6 @@ optSettings:
   setNexmo: test/resources/nexmo-credentials.yaml
   # setStomp: test/resources/stomp-credentials.yaml
   setUserMaxConnections: 16
-  setCookieDomain: 127.0.0.1
   setCookieInsecure: true
   setUserCookieRenewAge: 2
   setUserCookieLimit: 5

services/brig/src/Brig/Options.hs

@@ -410,8 +410,6 @@ data Settings = Settings
     setUserMaxConnections :: !Int64,
     -- | Max. number of permanent clients per user
     setUserMaxPermClients :: !(Maybe Int),
-    -- | The domain to restrict cookies to
-    setCookieDomain :: !Text,
     -- | Whether to allow plain HTTP transmission
     --   of cookies (for testing purposes only)
     setCookieInsecure :: !Bool,

services/brig/src/Brig/Provider/API.hs

@@ -1045,7 +1045,6 @@ setProviderCookie t r = do
       Cookie.def
         { Cookie.setCookieName = "zprovider",
           Cookie.setCookieValue = toByteString' t,
-          Cookie.setCookieDomain = Just $ Text.encodeUtf8 . setCookieDomain $ s,
           Cookie.setCookiePath = Just "/provider",
           Cookie.setCookieExpires = Just (ZAuth.tokenExpiresUTC t),
           Cookie.setCookieSecure = not (setCookieInsecure s),

services/brig/src/Brig/User/API/Auth.hs

@@ -35,6 +35,8 @@ import qualified Data.ByteString as BS
 import Data.ByteString.Conversion
 import Data.Either.Combinators (leftToMaybe, rightToMaybe)
 import Data.Id
+import Data.List1 (List1)
+import qualified Data.List1 as List1
 import Data.Predicate
 import qualified Data.Swagger.Build.Api as Doc
 import qualified Data.ZAuth.Token as ZAuth
@@ -43,7 +45,7 @@ import Network.HTTP.Types.Status
 import Network.Wai (Response)
 import Network.Wai.Predicate
 import qualified Network.Wai.Predicate as P
-import Network.Wai.Predicate.Request
+import qualified Network.Wai.Predicate.Request as R
 import Network.Wai.Routing
 import Network.Wai.Utilities.Error ((!>>))
 import Network.Wai.Utilities.Request (JsonRequest, jsonRequest)
@@ -225,12 +227,12 @@ legalHoldLogin l = do
   let typ = PersistentCookie -- Session cookie isn't a supported use case here
   Auth.legalHoldLogin l typ !>> legalHoldLoginError
 
-logoutH :: JSON ::: Maybe (Either ZAuth.UserToken ZAuth.LegalHoldUserToken) ::: Maybe (Either ZAuth.AccessToken ZAuth.LegalHoldAccessToken) -> Handler Response
+logoutH :: JSON ::: Maybe (Either (List1 ZAuth.UserToken) (List1 ZAuth.LegalHoldUserToken)) ::: Maybe (Either ZAuth.AccessToken ZAuth.LegalHoldAccessToken) -> Handler Response
 logoutH (_ ::: ut ::: at) = empty <$ logout ut at
 
 -- TODO: add legalhold test checking cookies are revoked (/access/logout is called) when legalhold device is deleted.
 logout ::
-  Maybe (Either ZAuth.UserToken ZAuth.LegalHoldUserToken) ->
+  Maybe (Either (List1 ZAuth.UserToken) (List1 ZAuth.LegalHoldUserToken)) ->
   Maybe (Either ZAuth.AccessToken ZAuth.LegalHoldAccessToken) ->
   Handler ()
 logout Nothing Nothing = throwStd authMissingCookieAndToken
@@ -256,7 +258,7 @@ rmCookies :: UserId -> Public.RemoveCookies -> Handler ()
 rmCookies uid (Public.RemoveCookies pw lls ids) = do
   Auth.revokeAccess uid pw ids lls !>> authError
 
-renewH :: JSON ::: Maybe (Either ZAuth.UserToken ZAuth.LegalHoldUserToken) ::: Maybe (Either ZAuth.AccessToken ZAuth.LegalHoldAccessToken) -> Handler Response
+renewH :: JSON ::: Maybe (Either (List1 ZAuth.UserToken) (List1 ZAuth.LegalHoldUserToken)) ::: Maybe (Either ZAuth.AccessToken ZAuth.LegalHoldAccessToken) -> Handler Response
 renewH (_ ::: ut ::: at) = lift . either tokenResponse tokenResponse =<< renew ut at
 
 -- | renew access for either:
@@ -265,21 +267,21 @@ renewH (_ ::: ut ::: at) = lift . either tokenResponse tokenResponse =<< renew u
 --
 -- Other combinations of provided inputs will cause an error to be raised.
 renew ::
-  Maybe (Either ZAuth.UserToken ZAuth.LegalHoldUserToken) ->
+  Maybe (Either (List1 ZAuth.UserToken) (List1 ZAuth.LegalHoldUserToken)) ->
   Maybe (Either ZAuth.AccessToken ZAuth.LegalHoldAccessToken) ->
   Handler (Either (Auth.Access ZAuth.User) (Auth.Access ZAuth.LegalHoldUser))
 renew = \case
   Nothing ->
     const $ throwStd authMissingCookie
-  (Just (Left userToken)) ->
+  (Just (Left userTokens)) ->
     -- normal UserToken, so we want a normal AccessToken
-    fmap Left . renewAccess userToken <=< matchingOrNone leftToMaybe
-  (Just (Right legalholdUserToken)) ->
+    fmap Left . renewAccess userTokens <=< matchingOrNone leftToMaybe
+  (Just (Right legalholdUserTokens)) ->
     -- LegalholdUserToken, so we want a LegalholdAccessToken
-    fmap Right . renewAccess legalholdUserToken <=< matchingOrNone rightToMaybe
+    fmap Right . renewAccess legalholdUserTokens <=< matchingOrNone rightToMaybe
   where
-    renewAccess ut mat =
-      Auth.renewAccess ut mat !>> zauthError
+    renewAccess uts mat =
+      Auth.renewAccess uts mat !>> zauthError
     matchingOrNone :: (a -> Maybe b) -> Maybe a -> Handler (Maybe b)
     matchingOrNone matching = traverse $ \accessToken ->
       case matching accessToken of
@@ -292,24 +294,30 @@ renew = \case
 -- | A predicate that captures user and access tokens for a request handler.
 tokenRequest ::
   forall r.
-  (HasCookies r, HasHeaders r, HasQuery r) =>
+  (R.HasCookies r, R.HasHeaders r, R.HasQuery r) =>
   Predicate
     r
     P.Error
-    ( Maybe (Either ZAuth.UserToken ZAuth.LegalHoldUserToken)
+    ( Maybe (Either (List1 ZAuth.UserToken) (List1 ZAuth.LegalHoldUserToken))
         ::: Maybe (Either ZAuth.AccessToken ZAuth.LegalHoldAccessToken)
     )
 tokenRequest = opt (userToken ||| legalHoldUserToken) .&. opt (accessToken ||| legalHoldAccessToken)
   where
-    userToken = cookieErr @ZAuth.User <$> cookie "zuid"
-    legalHoldUserToken = cookieErr @ZAuth.LegalHoldUser <$> cookie "zuid"
+    userToken = cookieErr @ZAuth.User <$> cookies "zuid"
+    legalHoldUserToken = cookieErr @ZAuth.LegalHoldUser <$> cookies "zuid"
     accessToken = parse @ZAuth.Access <$> (tokenHeader .|. tokenQuery)
     legalHoldAccessToken = parse @ZAuth.LegalHoldAccess <$> (tokenHeader .|. tokenQuery)
+    --
+    tokenHeader :: r -> Result P.Error ByteString
     tokenHeader = bearer <$> header "authorization"
+    --
+    tokenQuery :: r -> Result P.Error ByteString
     tokenQuery = query "access_token"
-    cookieErr :: ZAuth.UserTokenLike u => Result P.Error (ZAuth.Token u) -> Result P.Error (ZAuth.Token u)
+    --
+    cookieErr :: ZAuth.UserTokenLike u => Result P.Error (List1 (ZAuth.Token u)) -> Result P.Error (List1 (ZAuth.Token u))
     cookieErr x@Okay {} = x
     cookieErr (Fail x) = Fail (setMessage "Invalid user token" (P.setStatus status403 x))
+    --
     -- Extract the access token from the Authorization header.
     bearer :: Result P.Error ByteString -> Result P.Error ByteString
     bearer (Fail x) = Fail x
@@ -323,6 +331,7 @@ tokenRequest = opt (userToken ||| legalHoldUserToken) .&. opt (accessToken ||| l
                     TypeError
                     (setMessage "Invalid authorization scheme" (err status403))
                 )
+    --
     -- Parse the access token
     parse :: ZAuth.AccessTokenLike a => Result P.Error ByteString -> Result P.Error (ZAuth.Token a)
     parse (Fail x) = Fail x
@@ -338,3 +347,26 @@ tokenRequest = opt (userToken ||| legalHoldUserToken) .&. opt (accessToken ||| l
 tokenResponse :: ZAuth.UserTokenLike u => Auth.Access u -> AppIO Response
 tokenResponse (Auth.Access t Nothing) = pure $ json t
 tokenResponse (Auth.Access t (Just c)) = Auth.setResponseCookie c (json t)
+
+-- | Internal utilities: These functions are nearly copies verbatim from the original
+-- project: https://gitlab.com/twittner/wai-predicates/-/blob/develop/src/Network/Wai/Predicate.hs#L106-112
+-- I will still make an upstream PR but would not like to block this PR because of
+-- it. Main difference: the original stops after finding the first valid cookie which
+-- is a problem if clients send more than 1 cookie and one of them happens to be invalid
+-- We should also be dropping this in favor of servant which will make this redundant
+cookies :: (R.HasCookies r, FromByteString a) => ByteString -> Predicate r P.Error (List1 a)
+cookies k r =
+  case R.lookupCookie k r of
+    [] -> Fail . addLabel "cookie" $ notAvailable k
+    cc ->
+      case mapMaybe fromByteString cc of
+        [] -> (Fail . addLabel "cookie" . typeError k $ "Failed to get zuid cookies")
+        (x : xs) -> return $ List1.list1 x xs
+
+notAvailable :: ByteString -> P.Error
+notAvailable k = e400 & setReason NotAvailable . setSource k
+{-# INLINE notAvailable #-}
+
+typeError :: ByteString -> ByteString -> P.Error
+typeError k m = e400 & setReason TypeError . setSource k . setMessage m
+{-# INLINE typeError #-}

services/brig/src/Brig/User/Auth.hs

@@ -60,7 +60,9 @@ import Control.Lens (to, view)
 import Data.ByteString.Conversion (toByteString)
 import Data.Handle (Handle)
 import Data.Id
-import Data.List1 (singleton)
+import qualified Data.List.NonEmpty as NE
+import Data.List1 (List1)
+import qualified Data.List1 as List1
 import Data.Misc (PlainTextPassword (..))
 import qualified Data.ZAuth.Token as ZAuth
 import Imports
@@ -152,18 +154,18 @@ withRetryLimit action uid = do
       BudgetExhausted ttl -> throwE . LoginBlocked . RetryAfter . floor $ ttl
       BudgetedValue () _ -> pure ()
 
-logout :: ZAuth.TokenPair u a => ZAuth.Token u -> ZAuth.Token a -> ExceptT ZAuth.Failure AppIO ()
-logout ut at = do
-  (u, ck) <- validateTokens ut (Just at)
+logout :: ZAuth.TokenPair u a => List1 (ZAuth.Token u) -> ZAuth.Token a -> ExceptT ZAuth.Failure AppIO ()
+logout uts at = do
+  (u, ck) <- validateTokens uts (Just at)
   lift $ revokeCookies u [cookieId ck] []
 
 renewAccess ::
   ZAuth.TokenPair u a =>
-  ZAuth.Token u ->
+  List1 (ZAuth.Token u) ->
   Maybe (ZAuth.Token a) ->
   ExceptT ZAuth.Failure AppIO (Access u)
-renewAccess ut at = do
-  (uid, ck) <- validateTokens ut at
+renewAccess uts at = do
+  (uid, ck) <- validateTokens uts at
   Log.debug $ field "user" (toByteString uid) . field "action" (Log.val "User.renewAccess")
   catchSuspendInactiveUser uid ZAuth.Expired
   ck' <- lift $ nextCookie ck
@@ -192,7 +194,7 @@ catchSuspendInactiveUser uid errval = do
       msg (val "Suspending user due to inactivity")
         ~~ field "user" (toByteString uid)
         ~~ field "action" ("user.suspend" :: String)
-    lift $ suspendAccount (singleton uid)
+    lift $ suspendAccount (List1.singleton uid)
     throwE errval
 
 newAccess :: forall u a. ZAuth.TokenPair u a => UserId -> CookieType -> Maybe CookieLabel -> ExceptT LoginError AppIO (Access u)
@@ -252,12 +254,32 @@ isPendingActivation ident = case ident of
             Just SSOIdentity {} -> False -- sso-created users are activated immediately.
             Nothing -> True
 
+-- | Validate a list of (User/LH) tokens potentially with an associated access token.
+--   If there are multiple valid cookies, we try all of them. When an access token is
+--   given, we perform the usual checks.
+--   If multiple cookies are given and several are valid, we return the first valid one.
 validateTokens ::
+  ZAuth.TokenPair u a =>
+  List1 (ZAuth.Token u) ->
+  Maybe (ZAuth.Token a) ->
+  ExceptT ZAuth.Failure AppIO (UserId, Cookie (ZAuth.Token u))
+validateTokens uts at = do
+  tokens <- forM uts $ \ut -> lift $ runExceptT (validateToken ut at)
+  getFirstSuccessOrFirstFail tokens
+  where
+    -- FUTUREWORK: There is surely a better way to do this
+    getFirstSuccessOrFirstFail :: List1 (Either ZAuth.Failure (UserId, Cookie (ZAuth.Token u))) -> ExceptT ZAuth.Failure AppIO (UserId, Cookie (ZAuth.Token u))
+    getFirstSuccessOrFirstFail tks = case (lefts $ NE.toList $ List1.toNonEmpty tks, rights $ NE.toList $ List1.toNonEmpty tks) of
+      (_, (suc : _)) -> return suc
+      ((e : _), _) -> throwE e
+      _ -> throwE ZAuth.Invalid -- Impossible
+
+validateToken ::
   ZAuth.TokenPair u a =>
   ZAuth.Token u ->
   Maybe (ZAuth.Token a) ->
   ExceptT ZAuth.Failure AppIO (UserId, Cookie (ZAuth.Token u))
-validateTokens ut at = do
+validateToken ut at = do
   unless (maybe True ((ZAuth.userTokenOf ut ==) . ZAuth.accessTokenOf) at) $
     throwE ZAuth.Invalid
   ExceptT (ZAuth.validateToken ut)

services/brig/src/Brig/User/Auth/Cookie.hs

@@ -52,7 +52,6 @@ import Data.Id
 import qualified Data.List as List
 import qualified Data.Metrics as Metrics
 import Data.Proxy
-import Data.Text.Encoding (encodeUtf8)
 import Data.Time.Clock
 import Imports
 import Network.Wai (Response)
@@ -235,7 +234,6 @@ setResponseCookie c r = do
       WebCookie.def
         { WebCookie.setCookieName = "zuid",
           WebCookie.setCookieValue = toByteString' (cookieValue c),
-          WebCookie.setCookieDomain = Just $ encodeUtf8 . setCookieDomain $ s,
           WebCookie.setCookiePath = Just "/access",
           WebCookie.setCookieExpires =
             if cookieType c == PersistentCookie