Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
A new endpoint is added to Brig (`put /users/:uid/email`) that allows a team owner to initiate changing/setting a user email by (re-)sending an activation email.
2 changes: 2 additions & 0 deletions libs/galley-types/src/Galley/Types/Teams.hs
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,7 @@ data HiddenPerm
-- efficient this end-point is. better not let all team members
-- play with it unless we have to.
DownloadTeamMembersCsv
| ChangeTeamMemberProfiles
deriving (Eq, Ord, Show)

-- | See Note [hidden team roles]
Expand Down Expand Up @@ -367,6 +368,7 @@ roleHiddenPermissions role = HiddenPermissions p p
ChangeTeamFeature TeamFeatureFileSharing,
ChangeTeamFeature TeamFeatureClassifiedDomains {- the features not listed here can only be changed in stern -},
ChangeTeamFeature TeamFeatureSelfDeletingMessages,
ChangeTeamMemberProfiles,
ReadIdp,
CreateUpdateDeleteIdp,
CreateReadDeleteScimToken,
Expand Down
10 changes: 10 additions & 0 deletions libs/wire-api/src/Wire/API/Routes/Public/Brig.hs
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,16 @@ data Api routes = Api
:> "self"
:> ReqBody '[JSON] DeleteUser
:> MultiVerb 'DELETE '[JSON] DeleteSelfResponses (Maybe Timeout),
updateUserEmail ::
routes
:- Summary "Resend email address validation email."
:> Description "If the user has a pending email validation, the validation email will be resent."
:> ZUser
:> "users"
:> CaptureUserId "uid"
:> "email"
:> ReqBody '[JSON] EmailUpdate
:> Put '[JSON] (),
getHandleInfoUnqualified ::
routes
:- Summary "(deprecated, use /search/contacts) Get information on a user handle"
Expand Down
7 changes: 7 additions & 0 deletions libs/wire-api/src/Wire/API/User.hs
Original file line number Diff line number Diff line change
Expand Up @@ -905,6 +905,13 @@ instance FromJSON LocaleUpdate where
newtype EmailUpdate = EmailUpdate {euEmail :: Email}
deriving stock (Eq, Show, Generic)
deriving newtype (Arbitrary)
deriving (S.ToSchema) via (Schema EmailUpdate)

instance ToSchema EmailUpdate where
schema =
object "EmailUpdate" $
EmailUpdate
<$> euEmail .= field "email" schema

modelEmailUpdate :: Doc.Model
modelEmailUpdate = Doc.defineModel "EmailUpdate" $ do
Expand Down
24 changes: 24 additions & 0 deletions services/brig/src/Brig/API/Public.hs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import Brig.App
import qualified Brig.Calling.API as Calling
import qualified Brig.Data.Connection as Data
import qualified Brig.Data.User as Data
import qualified Brig.IO.Intra as Intra
import Brig.Options hiding (internalEvents, sesQueue)
import qualified Brig.Provider.API as Provider
import qualified Brig.Team.API as Team
Expand Down Expand Up @@ -80,6 +81,7 @@ import qualified Data.Text.Ascii as Ascii
import Data.Text.Encoding (decodeLatin1)
import Data.Text.Lazy (pack)
import qualified Data.ZAuth.Token as ZAuth
import Galley.Types.Teams (HiddenPerm (..), hasPermission)
import Imports hiding (head)
import Network.HTTP.Types.Status
import Network.Wai (Response, lazyRequestBody)
Expand Down Expand Up @@ -251,6 +253,7 @@ servantSitemap =
BrigAPI.getUserQualified = getUser,
BrigAPI.getSelf = getSelf,
BrigAPI.deleteSelf = deleteUser,
BrigAPI.updateUserEmail = updateUserEmail,
BrigAPI.getHandleInfoUnqualified = getHandleInfoUnqualifiedH,
BrigAPI.getUserByHandleQualified = Handle.getHandleInfo,
BrigAPI.listUsersByUnqualifiedIdsOrHandles = listUsersByUnqualifiedIdsOrHandles,
Expand Down Expand Up @@ -1193,6 +1196,27 @@ verifyDeleteUserH (r ::: _) = do
API.verifyDeleteUser body !>> deleteUserError
return (setStatus status200 empty)

updateUserEmail :: UserId -> UserId -> Public.EmailUpdate -> Handler ()
updateUserEmail zuserId emailOwnerId (Public.EmailUpdate email) = do
maybeZuserTeamId <- lift $ Data.lookupUserTeam zuserId
whenM (not <$> assertHasPerm maybeZuserTeamId) $ throwStd insufficientTeamPermissions
maybeEmailOwnerTeamId <- lift $ Data.lookupUserTeam emailOwnerId
checkSameTeam maybeZuserTeamId maybeEmailOwnerTeamId
void $ API.changeSelfEmail emailOwnerId email API.AllowSCIMUpdates
where
checkSameTeam :: Maybe TeamId -> Maybe TeamId -> Handler ()
checkSameTeam (Just zuserTeamId) maybeEmailOwnerTeamId =
when (Just zuserTeamId /= maybeEmailOwnerTeamId) $ throwStd $ notFound "user not found"
checkSameTeam Nothing _ = throwStd insufficientTeamPermissions

assertHasPerm :: Maybe TeamId -> Handler Bool
assertHasPerm maybeTeamId = fromMaybe False <$> check
where
check = runMaybeT $ do
teamId <- hoistMaybe maybeTeamId
teamMember <- MaybeT $ lift $ Intra.getTeamMember zuserId teamId
pure $ teamMember `hasPermission` ChangeTeamMemberProfiles

-- activation

data ActivationRespWithStatus
Expand Down
5 changes: 2 additions & 3 deletions services/brig/src/Brig/API/User.hs
Original file line number Diff line number Diff line change
Expand Up @@ -1232,9 +1232,8 @@ getEmailForProfile profileOwner EmailVisibleIfOnTeam' =
then userEmail profileOwner
else Nothing
getEmailForProfile profileOwner (EmailVisibleIfOnSameTeam' (Just (viewerTeamId, viewerTeamMember))) =
if ( Just viewerTeamId == userTeam profileOwner
&& Team.hasPermission viewerTeamMember Team.ViewSameTeamEmails
)
if Just viewerTeamId == userTeam profileOwner
&& Team.hasPermission viewerTeamMember Team.ViewSameTeamEmails
then userEmail profileOwner
else Nothing
getEmailForProfile _ (EmailVisibleIfOnSameTeam' Nothing) = Nothing
Expand Down
4 changes: 2 additions & 2 deletions services/brig/src/Brig/Data/User.hs
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,7 @@ lookupRichInfoMultiUsers users = do
-- successful login.
lookupUserTeam :: UserId -> AppIO (Maybe TeamId)
lookupUserTeam u =
join . fmap runIdentity
(runIdentity =<<)
<$> retry x1 (query1 teamSelect (params LocalQuorum (Identity u)))

lookupAuth :: (MonadClient m) => UserId -> m (Maybe (Maybe Password, AccountStatus))
Expand Down Expand Up @@ -471,7 +471,7 @@ lookupFeatureConferenceCalling uid = do
pure $ ApiFt.TeamFeatureStatusNoConfig <$> mStatusValue
where
select :: PrepQuery R (Identity UserId) (Identity (Maybe ApiFt.TeamFeatureStatusValue))
select = fromString $ "select feature_conference_calling from user where id = ?"
select = fromString "select feature_conference_calling from user where id = ?"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally, these changes would go into a separate PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know, but this makes the barrier for improving the code on the fly much higher.


-------------------------------------------------------------------------------
-- Queries
Expand Down
12 changes: 12 additions & 0 deletions services/brig/test/integration/API/Team/Util.hs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ import Util
import Web.Cookie (parseSetCookie, setCookieName)
import Wire.API.Team.Feature (TeamFeatureStatusValue (..))
import qualified Wire.API.Team.Feature as Public
import qualified Wire.API.User as Public

-- | FUTUREWORK: Remove 'createPopulatedBindingTeam', 'createPopulatedBindingTeamWithNames',
-- and rename 'createPopulatedBindingTeamWithNamesAndHandles' to 'createPopulatedBindingTeam'.
Expand Down Expand Up @@ -476,3 +477,14 @@ setTeamSearchVisibility galley tid typ =
)
!!! do
const 204 === statusCode

setUserEmail :: Brig -> UserId -> UserId -> Email -> Http ResponseLBS
setUserEmail brig from uid email = do
put
( brig
. paths ["users", toByteString' uid, "email"]
. zUser from
. zConn "conn"
. contentJson
. body (RequestBodyLBS . encode $ Public.EmailUpdate email)
)
49 changes: 49 additions & 0 deletions services/brig/test/integration/API/User/Account.hs
Original file line number Diff line number Diff line change
Expand Up @@ -141,9 +141,58 @@ tests _ at opts p b c ch g aws =
testGroup
"temporary customer extensions"
[ test' aws p "domains blocked for registration" $ testDomainsBlockedForRegistration opts b
],
testGroup
"update user email by team owner"
[ test' aws p "put /users/:uid/email" $ testUpdateUserEmailByTeamOwner b
]
]

testUpdateUserEmailByTeamOwner :: Brig -> Http ()
testUpdateUserEmailByTeamOwner brig = do
(_, teamOwner, emailOwner : otherTeamMember : _) <- createPopulatedBindingTeamWithNamesAndHandles brig 2
(teamOwnerDifferentTeam, _) <- createUserWithTeam' brig
newEmail <- randomEmail
initiateEmailUpdateNoSend brig newEmail (userId emailOwner) !!! (const 202 === statusCode)
checkActivationCode newEmail True
checkLetActivationExpire newEmail
checkActivationCode newEmail False
checkSetUserEmail teamOwner emailOwner newEmail 200
checkActivationCode newEmail True
checkUnauthorizedRequests emailOwner otherTeamMember teamOwnerDifferentTeam newEmail
activateEmail brig newEmail
-- apparently activating the email does not invalidate the activation code
-- therefore we let the activation code expire again
checkLetActivationExpire newEmail
checkSetUserEmail teamOwner emailOwner newEmail 200
checkActivationCode newEmail False
checkUnauthorizedRequests emailOwner otherTeamMember teamOwnerDifferentTeam newEmail
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we remove this line? ff not, can you add a comment helping me to find out what this is testing?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line 168: Ideally if the email has been verified the team owner can call the endpoint again and nothing will happen (no new activation code will be generated). In order to test this specifically we would have to wait until the activation code expires. This is too costly for the test, but at least we can test that the request still responds with status code 200 in this case.

Line: 169: Similar, if the request is made with insufficient permissions, the responses should be the same as before (regardless of the state of the activation)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe these are not the most useful tests, but they don't hurt.

Copy link
Contributor Author

@battermann battermann Dec 2, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed in another conversation, I added the expiration of the token again, so this conversation is obsolete, I think.

checkActivationCode newEmail False
where
checkLetActivationExpire :: Email -> Http ()
checkLetActivationExpire email = do
-- assumption: `optSettings.setActivationTimeout = 5` in `brig.yaml`
threadDelay (5100 * 1000)
checkActivationCode email False

checkActivationCode :: Email -> Bool -> Http ()
checkActivationCode email shouldExist = do
maybeActivationCode <- Util.getActivationCode brig (Left email)
void $
lift $
if shouldExist
then assertBool "activation code should exists" (isJust maybeActivationCode)
else assertBool "activation code should not exists" (isNothing maybeActivationCode)

checkSetUserEmail :: User -> User -> Email -> Int -> Http ()
checkSetUserEmail teamOwner emailOwner email expectedStatusCode =
setUserEmail brig (userId teamOwner) (userId emailOwner) email !!! (const expectedStatusCode === statusCode)

checkUnauthorizedRequests :: User -> User -> User -> Email -> Http ()
checkUnauthorizedRequests emailOwner otherTeamMember teamOwnerDifferentTeam email = do
setUserEmail brig (userId teamOwnerDifferentTeam) (userId emailOwner) email !!! (const 404 === statusCode)
setUserEmail brig (userId otherTeamMember) (userId emailOwner) email !!! (const 403 === statusCode)

testCreateUserWithPreverified :: Opt.Opts -> Brig -> AWS.Env -> Http ()
testCreateUserWithPreverified opts brig aws = do
-- Register (pre verified) user with phone
Expand Down