diff --git a/changelog.d/2-features/blacklist-change-phone b/changelog.d/2-features/blacklist-change-phone new file mode 100644 index 0000000000..1a01f3cf73 --- /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 af50a8a7f1..c30b4fb5d7 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 8241149efb..61179203f4 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 544da73726..fe6bf6d931 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 f687f1b3de..a7a4de6b24 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