From cb76990d0a15904697c1cc8de38b0c043b704c48 Mon Sep 17 00:00:00 2001 From: Akshay Mankar Date: Mon, 13 Sep 2021 16:55:39 +0200 Subject: [PATCH] Allow blacklisting rules to changing phone numbers --- changelog.d/2-features/blacklist-change-phone | 1 + services/brig/src/Brig/API/Error.hs | 1 + services/brig/src/Brig/API/Types.hs | 1 + services/brig/src/Brig/API/User.hs | 13 ++++++++++--- .../brig/test/integration/API/User/Account.hs | 17 +++++++++++++++++ 5 files changed, 30 insertions(+), 3 deletions(-) create mode 100644 changelog.d/2-features/blacklist-change-phone diff --git a/changelog.d/2-features/blacklist-change-phone b/changelog.d/2-features/blacklist-change-phone new file mode 100644 index 00000000000..1a01f3cf738 --- /dev/null +++ b/changelog.d/2-features/blacklist-change-phone @@ -0,0 +1 @@ +Disallow changing phone number to a black listed phone number \ No newline at end of file diff --git a/services/brig/src/Brig/API/Error.hs b/services/brig/src/Brig/API/Error.hs index af50a8a7f19..c30b4fb5d77 100644 --- a/services/brig/src/Brig/API/Error.hs +++ b/services/brig/src/Brig/API/Error.hs @@ -144,6 +144,7 @@ changeEmailError EmailManagedByScim = StdError $ propertyManagedByScim "email" changePhoneError :: ChangePhoneError -> Error changePhoneError (InvalidNewPhone _) = StdError invalidPhone changePhoneError (PhoneExists _) = StdError userKeyExists +changePhoneError (BlacklistedNewPhone _) = StdError blacklistedPhone changePwError :: ChangePasswordError -> Error changePwError InvalidCurrentPassword = StdError badCredentials diff --git a/services/brig/src/Brig/API/Types.hs b/services/brig/src/Brig/API/Types.hs index 8241149efb8..61179203f4a 100644 --- a/services/brig/src/Brig/API/Types.hs +++ b/services/brig/src/Brig/API/Types.hs @@ -165,6 +165,7 @@ data ChangePasswordError data ChangePhoneError = PhoneExists !Phone | InvalidNewPhone !Phone + | BlacklistedNewPhone !Phone data ChangeEmailError = InvalidNewEmail !Email !String diff --git a/services/brig/src/Brig/API/User.hs b/services/brig/src/Brig/API/User.hs index 544da737262..fe6bf6d931f 100644 --- a/services/brig/src/Brig/API/User.hs +++ b/services/brig/src/Brig/API/User.hs @@ -604,19 +604,26 @@ changeEmail u email allowScim = do changePhone :: UserId -> Phone -> ExceptT ChangePhoneError AppIO (Activation, Phone) changePhone u phone = do - ph <- + canonical <- maybe (throwE $ InvalidNewPhone phone) return =<< lift (validatePhone phone) - let pk = userPhoneKey ph + let pk = userPhoneKey canonical available <- lift $ Data.keyAvailable pk (Just u) unless available $ throwE $ PhoneExists phone timeout <- setActivationTimeout <$> view settings + blacklisted <- lift $ Blacklist.exists pk + when blacklisted $ + throwE (BlacklistedNewPhone canonical) + -- check if any prefixes of this phone number are blocked + prefixExcluded <- lift $ Blacklist.existsAnyPrefix canonical + when prefixExcluded $ + throwE (BlacklistedNewPhone canonical) act <- lift $ Data.newActivation pk timeout (Just u) - return (act, ph) + return (act, canonical) ------------------------------------------------------------------------------- -- Remove Email diff --git a/services/brig/test/integration/API/User/Account.hs b/services/brig/test/integration/API/User/Account.hs index f687f1b3ded..a7a4de6b242 100644 --- a/services/brig/test/integration/API/User/Account.hs +++ b/services/brig/test/integration/API/User/Account.hs @@ -105,6 +105,7 @@ tests _ at opts p b c ch g aws = test' aws p "put /self - 200" $ testUserUpdate b c aws, test' aws p "put /access/self/email - 2xx" $ testEmailUpdate b aws, test' aws p "put /self/phone - 202" $ testPhoneUpdate b, + test' aws p "put /self/phone - 403" $ testPhoneUpdateBlacklisted b, test' aws p "head /self/password - 200/404" $ testPasswordSet b, test' aws p "put /self/password - 200" $ testPasswordChange b, test' aws p "put /self/locale - 200" $ testUserLocaleUpdate b aws, @@ -762,6 +763,22 @@ testPhoneUpdate brig = do const 200 === statusCode const (Just phn) === (userPhone <=< responseJsonMaybe) +testPhoneUpdateBlacklisted :: Brig -> Http () +testPhoneUpdateBlacklisted brig = do + uid <- userId <$> randomUser brig + phn <- randomPhone + let prefix = mkPrefix $ T.take 5 (fromPhone phn) + + insertPrefix brig prefix + let phoneUpdate = RequestBodyLBS . encode $ PhoneUpdate phn + put (brig . path "/self/phone" . contentJson . zUser uid . zConn "c" . body phoneUpdate) + !!! (const 403 === statusCode) + + -- check that phone is not updated + get (brig . path "/self" . zUser uid) !!! do + const 200 === statusCode + const (Right Nothing) === fmap userPhone . responseJsonEither + testCreateAccountPendingActivationKey :: Opt.Opts -> Brig -> Http () testCreateAccountPendingActivationKey (Opt.setRestrictUserCreation . Opt.optSettings -> Just True) _ = pure () testCreateAccountPendingActivationKey _ brig = do