diff --git a/changelog.d/3-bug-fixes/WPB-7093 b/changelog.d/3-bug-fixes/WPB-7093 new file mode 100644 index 00000000000..055101d0a9a --- /dev/null +++ b/changelog.d/3-bug-fixes/WPB-7093 @@ -0,0 +1 @@ +Invalidate 2nd factor after first use diff --git a/integration/integration.cabal b/integration/integration.cabal index bcafb9ff147..56bcb614de8 100644 --- a/integration/integration.cabal +++ b/integration/integration.cabal @@ -125,6 +125,7 @@ library Test.Federation Test.Federator Test.LegalHold + Test.Login Test.MessageTimer Test.MLS Test.MLS.KeyPackage diff --git a/integration/test/API/BrigInternal.hs b/integration/test/API/BrigInternal.hs index d538bb35561..71bde9877dd 100644 --- a/integration/test/API/BrigInternal.hs +++ b/integration/test/API/BrigInternal.hs @@ -253,3 +253,11 @@ getEJPDInfo dom handles mode = do "include_contacts" -> [("include_contacts", "true")] bad -> error $ show bad submit "POST" $ req & addJSONObject ["ejpd_request" .= handles] & addQueryParams query + +-- https://staging-nginz-https.zinfra.io/api-internal/swagger-ui/brig/#/brig/get_i_users__uid__verification_code__action_ +getVerificationCode :: (HasCallStack, MakesValue user) => user -> String -> App Response +getVerificationCode user action = do + uid <- objId user + domain <- objDomain user + req <- baseRequest domain Brig Unversioned $ joinHttpPath ["i", "users", uid, "verification-code", action] + submit "GET" req diff --git a/integration/test/API/GalleyInternal.hs b/integration/test/API/GalleyInternal.hs index 4b5ad4cc970..f3b2ef1a135 100644 --- a/integration/test/API/GalleyInternal.hs +++ b/integration/test/API/GalleyInternal.hs @@ -44,6 +44,13 @@ setTeamFeatureStatus domain team featureName status = do res <- submit "PATCH" $ req & addJSONObject ["status" .= status] res.status `shouldMatchInt` 200 +setTeamFeatureLockStatus :: (HasCallStack, MakesValue domain, MakesValue team) => domain -> team -> String -> String -> App () +setTeamFeatureLockStatus domain team featureName status = do + tid <- asString team + req <- baseRequest domain Galley Unversioned $ joinHttpPath ["i", "teams", tid, "features", featureName, status] + res <- submit "PUT" $ req + res.status `shouldMatchInt` 200 + getFederationStatus :: ( HasCallStack, MakesValue user @@ -79,3 +86,14 @@ legalholdIsEnabled tid uid = do tidStr <- asString tid baseRequest uid Galley Unversioned do joinHttpPath ["i", "teams", tidStr, "features", "legalhold"] >>= submit "GET" + +generateVerificationCode :: (HasCallStack, MakesValue domain, MakesValue email) => domain -> email -> App () +generateVerificationCode domain email = do + res <- generateVerificationCode' domain email + res.status `shouldMatchInt` 200 + +generateVerificationCode' :: (HasCallStack, MakesValue domain, MakesValue email) => domain -> email -> App Response +generateVerificationCode' domain email = do + req <- baseRequest domain Brig Versioned "/verification-code/send" + emailStr <- asString email + submit "POST" $ req & addJSONObject ["email" .= emailStr, "action" .= "login"] diff --git a/integration/test/API/Nginz.hs b/integration/test/API/Nginz.hs index 4c34ef639d3..b4c2f08db5b 100644 --- a/integration/test/API/Nginz.hs +++ b/integration/test/API/Nginz.hs @@ -14,6 +14,14 @@ login domain email pw = do pwStr <- make pw >>= asString submit "POST" (req & addJSONObject ["email" .= emailStr, "password" .= pwStr, "label" .= "auth"]) +loginWith2ndFactor :: (HasCallStack, MakesValue domain, MakesValue email, MakesValue password, MakesValue sndFactor) => domain -> email -> password -> sndFactor -> App Response +loginWith2ndFactor domain email pw sf = do + req <- rawBaseRequest domain Nginz Unversioned "/login" + emailStr <- make email >>= asString + pwStr <- make pw >>= asString + sfStr <- make sf >>= asString + submit "POST" (req & addJSONObject ["email" .= emailStr, "password" .= pwStr, "label" .= "auth", "verification_code" .= sfStr]) + access :: (HasCallStack, MakesValue domain, MakesValue cookie) => domain -> cookie -> App Response access domain cookie = do req <- rawBaseRequest domain Nginz Unversioned "/access" diff --git a/integration/test/Test/Login.hs b/integration/test/Test/Login.hs new file mode 100644 index 00000000000..ee0cabf0908 --- /dev/null +++ b/integration/test/Test/Login.hs @@ -0,0 +1,129 @@ +{-# OPTIONS_GHC -Wno-ambiguous-fields #-} + +module Test.Login where + +import API.BrigInternal (getVerificationCode) +import API.Common (defPassword) +import API.GalleyInternal +import API.Nginz (login, loginWith2ndFactor) +import Control.Concurrent (threadDelay) +import qualified Data.Aeson as Aeson +import SetupHelpers +import Testlib.Prelude + +testLoginVerify6DigitEmailCodeSuccess :: HasCallStack => App () +testLoginVerify6DigitEmailCodeSuccess = do + (owner, team, []) <- createTeam OwnDomain 0 + email <- owner %. "email" + setTeamFeatureLockStatus owner team "sndFactorPasswordChallenge" "unlocked" + setTeamFeatureStatus owner team "sndFactorPasswordChallenge" "enabled" + generateVerificationCode owner email + code <- getVerificationCode owner "login" >>= getJSON 200 >>= asString + bindResponse (loginWith2ndFactor owner email defPassword code) $ \resp -> do + resp.status `shouldMatchInt` 200 + +testLoginVerificationCodeCanOnlyBeUsedOnce :: HasCallStack => App () +testLoginVerificationCodeCanOnlyBeUsedOnce = do + (owner, team, []) <- createTeam OwnDomain 0 + email <- owner %. "email" + setTeamFeatureLockStatus owner team "sndFactorPasswordChallenge" "unlocked" + setTeamFeatureStatus owner team "sndFactorPasswordChallenge" "enabled" + generateVerificationCode owner email + code <- getVerificationCode owner "login" >>= getJSON 200 >>= asString + bindResponse (loginWith2ndFactor owner email defPassword code) $ \resp -> do + resp.status `shouldMatchInt` 200 + bindResponse (loginWith2ndFactor owner email defPassword code) $ \resp -> do + resp.status `shouldMatchInt` 403 + resp.json %. "label" `shouldMatch` "code-authentication-failed" + +-- @SF.Channel @TSFI.RESTfulAPI @S2 +-- +-- Test that login fails with wrong second factor email verification code +testLoginVerify6DigitWrongCodeFails :: HasCallStack => App () +testLoginVerify6DigitWrongCodeFails = do + (owner, team, []) <- createTeam OwnDomain 0 + email <- owner %. "email" + setTeamFeatureLockStatus owner team "sndFactorPasswordChallenge" "unlocked" + setTeamFeatureStatus owner team "sndFactorPasswordChallenge" "enabled" + bindResponse (loginWith2ndFactor owner email defPassword "123456") $ \resp -> do + resp.status `shouldMatchInt` 403 + resp.json %. "label" `shouldMatch` "code-authentication-failed" + +-- @END + +-- @SF.Channel @TSFI.RESTfulAPI @S2 +-- +-- Test that login without verification code fails if SndFactorPasswordChallenge feature is enabled in team +testLoginVerify6DigitMissingCodeFails :: HasCallStack => App () +testLoginVerify6DigitMissingCodeFails = do + (owner, team, []) <- createTeam OwnDomain 0 + email <- owner %. "email" + setTeamFeatureLockStatus owner team "sndFactorPasswordChallenge" "unlocked" + setTeamFeatureStatus owner team "sndFactorPasswordChallenge" "enabled" + bindResponse (login owner email defPassword) $ \resp -> do + resp.status `shouldMatchInt` 403 + resp.json %. "label" `shouldMatch` "code-authentication-required" + +-- @END + +-- @SF.Channel @TSFI.RESTfulAPI @S2 +-- +-- Test that login fails with expired second factor email verification code +testLoginVerify6DigitExpiredCodeFails :: HasCallStack => App () +testLoginVerify6DigitExpiredCodeFails = do + withModifiedBackend + (def {brigCfg = setField "optSettings.setVerificationTimeout" (Aeson.Number 2)}) + $ \domain -> do + (owner, team, []) <- createTeam domain 0 + email <- owner %. "email" + setTeamFeatureLockStatus owner team "sndFactorPasswordChallenge" "unlocked" + setTeamFeatureStatus owner team "sndFactorPasswordChallenge" "enabled" + generateVerificationCode owner email + code <- getVerificationCode owner "login" >>= getJSON 200 >>= asString + liftIO $ threadDelay 2_000_100 + bindResponse (loginWith2ndFactor owner email defPassword code) \resp -> do + resp.status `shouldMatchInt` 403 + resp.json %. "label" `shouldMatch` "code-authentication-failed" + +-- @END + +testLoginVerify6DigitResendCodeSuccessAndRateLimiting :: HasCallStack => App () +testLoginVerify6DigitResendCodeSuccessAndRateLimiting = do + (owner, team, []) <- createTeam OwnDomain 0 + email <- owner %. "email" + setTeamFeatureLockStatus owner team "sndFactorPasswordChallenge" "unlocked" + setTeamFeatureStatus owner team "sndFactorPasswordChallenge" "enabled" + generateVerificationCode owner email + fstCode <- getVerificationCode owner "login" >>= getJSON 200 >>= asString + bindResponse (generateVerificationCode' owner email) $ \resp -> do + resp.status `shouldMatchInt` 429 + mostRecentCode <- retryT $ do + resp <- generateVerificationCode' owner email + resp.status `shouldMatchInt` 200 + getVerificationCode owner "login" >>= getJSON 200 >>= asString + + bindResponse (loginWith2ndFactor owner email defPassword fstCode) \resp -> do + resp.status `shouldMatchInt` 403 + resp.json %. "label" `shouldMatch` "code-authentication-failed" + + bindResponse (loginWith2ndFactor owner email defPassword mostRecentCode) \resp -> do + resp.status `shouldMatchInt` 200 + +testLoginVerify6DigitLimitRetries :: HasCallStack => App () +testLoginVerify6DigitLimitRetries = do + (owner, team, []) <- createTeam OwnDomain 0 + email <- owner %. "email" + setTeamFeatureLockStatus owner team "sndFactorPasswordChallenge" "unlocked" + setTeamFeatureStatus owner team "sndFactorPasswordChallenge" "enabled" + generateVerificationCode owner email + correctCode <- getVerificationCode owner "login" >>= getJSON 200 >>= asString + let wrongCode = "123456" + -- try login with wrong code should fail 3 times + forM_ [1 .. 3] $ \(_ :: Int) -> do + bindResponse (loginWith2ndFactor owner email defPassword wrongCode) \resp -> do + resp.status `shouldMatchInt` 403 + resp.json %. "label" `shouldMatch` "code-authentication-failed" + -- after 3 failed attempts, login with correct code should fail as well + bindResponse (loginWith2ndFactor owner email defPassword correctCode) \resp -> do + resp.status `shouldMatchInt` 403 + resp.json %. "label" `shouldMatch` "code-authentication-failed" diff --git a/services/brig/src/Brig/Code.hs b/services/brig/src/Brig/Code.hs index 2d0fc50485c..2ca468fed49 100644 --- a/services/brig/src/Brig/Code.hs +++ b/services/brig/src/Brig/Code.hs @@ -332,10 +332,8 @@ verify :: MonadClient m => Key -> Scope -> Value -> m (Maybe Code) verify k s v = lookup k s >>= maybe (pure Nothing) continue where continue c - | codeValue c == v && codeRetries c > 0 = pure (Just c) - | codeRetries c > 0 = do - insertInternal (c {codeRetries = codeRetries c - 1}) - pure Nothing + | codeValue c == v && codeRetries c > 0 = delete k s >> pure (Just c) + | codeRetries c > 0 = insertInternal (c {codeRetries = codeRetries c - 1}) $> Nothing | otherwise = pure Nothing -- | Delete a code associated with the given key and scope. diff --git a/services/brig/test/integration/API/User/Auth.hs b/services/brig/test/integration/API/User/Auth.hs index 06a10761028..38264006369 100644 --- a/services/brig/test/integration/API/User/Auth.hs +++ b/services/brig/test/integration/API/User/Auth.hs @@ -25,7 +25,6 @@ module API.User.Auth where import API.Team.Util -import API.User.Util qualified as Util import Bilge hiding (body) import Bilge qualified as Http import Bilge.Assert hiding (assert) @@ -38,7 +37,6 @@ import Cassandra hiding (Value) import Cassandra qualified as DB import Control.Arrow ((&&&)) import Control.Lens (set, (^.)) -import Control.Monad.Catch (MonadCatch) import Control.Retry import Data.Aeson as Aeson hiding (json) import Data.ByteString qualified as BS @@ -49,9 +47,7 @@ import Data.Id import Data.Misc (PlainTextPassword6, plainTextPassword6, plainTextPassword6Unsafe) import Data.Proxy import Data.Qualified -import Data.Range (unsafeRange) import Data.Text qualified as Text -import Data.Text.Ascii (AsciiChars (validate)) import Data.Text.IO (hPutStrLn) import Data.Text.Lazy qualified as Lazy import Data.Time.Clock @@ -67,9 +63,7 @@ import UnliftIO.Async hiding (wait) import Util import Wire.API.Conversation (Conversation (..)) import Wire.API.Password (Password, mkSafePassword) -import Wire.API.Team.Feature qualified as Public import Wire.API.User -import Wire.API.User qualified as Public import Wire.API.User.Auth import Wire.API.User.Auth qualified as Auth import Wire.API.User.Auth.LegalHold @@ -132,15 +126,6 @@ tests conf m z db b g n = [ test m "nginz-login" (testNginz b n), test m "nginz-legalhold-login" (onlyIfLhWhitelisted (testNginzLegalHold b g n)), test m "nginz-login-multiple-cookies" (testNginzMultipleCookies conf b n) - ], - testGroup - "snd-factor-password-challenge" - [ test m "test-login-verify6-digit-email-code-success" $ testLoginVerify6DigitEmailCodeSuccess b g db, - test m "test-login-verify6-digit-wrong-code-fails" $ testLoginVerify6DigitWrongCodeFails b g, - test m "test-login-verify6-digit-missing-code-fails" $ testLoginVerify6DigitMissingCodeFails b g, - test m "test-login-verify6-digit-expired-code-fails" $ testLoginVerify6DigitExpiredCodeFails conf b g db, - test m "test-login-verify6-digit-resend-code-success-and-rate-limiting" $ testLoginVerify6DigitResendCodeSuccessAndRateLimiting b g conf db, - test m "test-login-verify6-digit-limit-retries" $ testLoginVerify6DigitLimitRetries b g conf db ] ], testGroup @@ -438,157 +423,6 @@ testSendLoginCode brig = do let _timeout = fromLoginCodeTimeout <$> responseJsonMaybe rsp2 liftIO $ assertEqual "timeout" (Just (Code.Timeout 600)) _timeout -testLoginVerify6DigitEmailCodeSuccess :: Brig -> Galley -> DB.ClientState -> Http () -testLoginVerify6DigitEmailCodeSuccess brig galley db = do - (u, tid) <- createUserWithTeam' brig - let Just email = userEmail u - let checkLoginSucceeds body = login brig body PersistentCookie !!! const 200 === statusCode - Util.setTeamFeatureLockStatus @Public.SndFactorPasswordChallengeConfig galley tid Public.LockStatusUnlocked - Util.setTeamSndFactorPasswordChallenge galley tid Public.FeatureStatusEnabled - Util.generateVerificationCode brig (Public.SendVerificationCode Public.Login email) - key <- Code.mkKey (Code.ForEmail email) - Just vcode <- Util.lookupCode db key Code.AccountLogin - checkLoginSucceeds $ - PasswordLogin $ - PasswordLoginData - (LoginByEmail email) - defPassword - (Just defCookieLabel) - (Just $ Code.codeValue vcode) - -testLoginVerify6DigitResendCodeSuccessAndRateLimiting :: Brig -> Galley -> Opts.Opts -> DB.ClientState -> Http () -testLoginVerify6DigitResendCodeSuccessAndRateLimiting brig galley _opts db = do - (u, tid) <- createUserWithTeam' brig - let Just email = userEmail u - let checkLoginSucceeds body = login brig body PersistentCookie !!! const 200 === statusCode - let getCodeFromDb = do - key <- Code.mkKey (Code.ForEmail email) - Just c <- Util.lookupCode db key Code.AccountLogin - pure c - - Util.setTeamFeatureLockStatus @Public.SndFactorPasswordChallengeConfig galley tid Public.LockStatusUnlocked - Util.setTeamSndFactorPasswordChallenge galley tid Public.FeatureStatusEnabled - - Util.generateVerificationCode brig (Public.SendVerificationCode Public.Login email) - fstCode <- getCodeFromDb - - let tooManyRequests = 429 - Util.generateVerificationCodeExpect tooManyRequests brig (Public.SendVerificationCode Public.Login email) - - void $ retryWhileN 10 ((==) 429 . statusCode) $ Util.generateVerificationCode' brig (Public.SendVerificationCode Public.Login email) - mostRecentCode <- getCodeFromDb - - checkLoginFails brig $ - PasswordLogin $ - PasswordLoginData - (LoginByEmail email) - defPassword - (Just defCookieLabel) - (Just $ Code.codeValue fstCode) - checkLoginSucceeds $ - PasswordLogin $ - PasswordLoginData - (LoginByEmail email) - defPassword - (Just defCookieLabel) - (Just $ Code.codeValue mostRecentCode) - -testLoginVerify6DigitLimitRetries :: Brig -> Galley -> Opts.Opts -> DB.ClientState -> Http () -testLoginVerify6DigitLimitRetries brig galley _opts db = do - (u, tid) <- createUserWithTeam' brig - let Just email = userEmail u - Util.setTeamFeatureLockStatus @Public.SndFactorPasswordChallengeConfig galley tid Public.LockStatusUnlocked - Util.setTeamSndFactorPasswordChallenge galley tid Public.FeatureStatusEnabled - Util.generateVerificationCode brig (Public.SendVerificationCode Public.Login email) - key <- Code.mkKey (Code.ForEmail email) - Just correctCode <- Util.lookupCode db key Code.AccountLogin - let wrongCode = Code.Value $ unsafeRange (fromRight undefined (validate "123456")) - -- login with wrong code should fail 3 times - forM_ [1 .. 3] $ \(_ :: Int) -> - checkLoginFails brig $ - PasswordLogin $ - PasswordLoginData - (LoginByEmail email) - defPassword - (Just defCookieLabel) - (Just wrongCode) - -- after 3 failed attempts, login with correct code should fail as well - checkLoginFails brig $ - PasswordLogin $ - PasswordLoginData - (LoginByEmail email) - defPassword - (Just defCookieLabel) - (Just (Code.codeValue correctCode)) - --- @SF.Channel @TSFI.RESTfulAPI @S2 --- --- Test that login fails with wrong second factor email verification code -testLoginVerify6DigitWrongCodeFails :: Brig -> Galley -> Http () -testLoginVerify6DigitWrongCodeFails brig galley = do - (u, tid) <- createUserWithTeam' brig - let Just email = userEmail u - Util.setTeamFeatureLockStatus @Public.SndFactorPasswordChallengeConfig galley tid Public.LockStatusUnlocked - Util.setTeamSndFactorPasswordChallenge galley tid Public.FeatureStatusEnabled - Util.generateVerificationCode brig (Public.SendVerificationCode Public.Login email) - let wrongCode = Code.Value $ unsafeRange (fromRight undefined (validate "123456")) - checkLoginFails brig $ - PasswordLogin $ - PasswordLoginData - (LoginByEmail email) - defPassword - (Just defCookieLabel) - (Just wrongCode) - --- @END - --- @SF.Channel @TSFI.RESTfulAPI @S2 --- --- Test that login without verification code fails if SndFactorPasswordChallenge feature is enabled in team -testLoginVerify6DigitMissingCodeFails :: Brig -> Galley -> Http () -testLoginVerify6DigitMissingCodeFails brig galley = do - (u, tid) <- createUserWithTeam' brig - let Just email = userEmail u - Util.setTeamFeatureLockStatus @Public.SndFactorPasswordChallengeConfig galley tid Public.LockStatusUnlocked - Util.setTeamSndFactorPasswordChallenge galley tid Public.FeatureStatusEnabled - Util.generateVerificationCode brig (Public.SendVerificationCode Public.Login email) - let body = - PasswordLogin $ - PasswordLoginData - (LoginByEmail email) - defPassword - (Just defCookieLabel) - Nothing - login brig body PersistentCookie !!! do - const 403 === statusCode - const (Just "code-authentication-required") === errorLabel - --- @END - --- @SF.Channel @TSFI.RESTfulAPI @S2 --- --- Test that login fails with expired second factor email verification code -testLoginVerify6DigitExpiredCodeFails :: Opts.Opts -> Brig -> Galley -> DB.ClientState -> Http () -testLoginVerify6DigitExpiredCodeFails opts brig galley db = do - (u, tid) <- createUserWithTeam' brig - let Just email = userEmail u - Util.setTeamFeatureLockStatus @Public.SndFactorPasswordChallengeConfig galley tid Public.LockStatusUnlocked - Util.setTeamSndFactorPasswordChallenge galley tid Public.FeatureStatusEnabled - Util.generateVerificationCode brig (Public.SendVerificationCode Public.Login email) - key <- Code.mkKey (Code.ForEmail email) - Just vcode <- Util.lookupCode db key Code.AccountLogin - let verificationTimeout = round (Opts.setVerificationTimeout (Opts.optSettings opts)) - threadDelay $ ((verificationTimeout + 1) * 1000_000) - checkLoginFails brig $ - PasswordLogin $ - PasswordLoginData - (LoginByEmail email) - defPassword - (Just defCookieLabel) - (Just $ Code.codeValue vcode) - --- @END - -- The testLoginFailure test conforms to the following testing standards: -- @SF.Provisioning @TSFI.RESTfulAPI @S2 -- @@ -1515,9 +1349,3 @@ remJson p l ids = wait :: MonadIO m => m () wait = liftIO $ threadDelay 1000000 - -checkLoginFails :: (MonadHttp m, MonadIO m, MonadCatch m) => Brig -> Login -> m () -checkLoginFails brig body = do - login brig body PersistentCookie !!! do - const 403 === statusCode - const (Just "code-authentication-failed") === errorLabel