Skip to content

Commit

Permalink
Merge branch 'wireapp:develop' into develop
Browse files Browse the repository at this point in the history
  • Loading branch information
offsoc authored Feb 15, 2025
2 parents ef4c270 + 03eb736 commit 07505c2
Show file tree
Hide file tree
Showing 9 changed files with 95 additions and 15 deletions.
1 change: 1 addition & 0 deletions changelog.d/2-features/WPB-16141
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Enforce no activation for email domains that are registered for another team or backend
75 changes: 75 additions & 0 deletions integration/test/Test/User.hs
Original file line number Diff line number Diff line change
Expand Up @@ -261,3 +261,78 @@ testUpdateEmailForEmailDomainForAnotherBackend = do

bindResponse (getSelf user) $ \resp -> do
resp.json %. "email" `shouldMatch` email

testActivateEmailForEmailDomainForAnotherBackend :: (HasCallStack) => App ()
testActivateEmailForEmailDomainForAnotherBackend = do
tid <- randomId
sso <- randomId
object
[ "domain_redirect" .= "backend",
"backend_url" .= "https://example.com",
"team_invite" .= "not-allowed"
]
& testActivateEmailShouldBeAllowed False
object
[ "domain_redirect" .= "none",
"team_invite" .= "allowed"
]
& testActivateEmailShouldBeAllowed True
object
[ "domain_redirect" .= "no-registration",
"team_invite" .= "team",
"team" .= tid
]
& testActivateEmailShouldBeAllowed False
object
[ "domain_redirect" .= "no-registration",
"team_invite" .= "not-allowed"
]
& testActivateEmailShouldBeAllowed False
object
[ "domain_redirect" .= "sso",
"sso_code" .= sso,
"team_invite" .= "not-allowed"
]
& testActivateEmailShouldBeAllowed False
object
[ "domain_redirect" .= "sso",
"sso_code" .= sso,
"team_invite" .= "team",
"team" .= tid
]
& testActivateEmailShouldBeAllowed False
where
testActivateEmailShouldBeAllowed :: (HasCallStack) => Bool -> Value -> App ()
testActivateEmailShouldBeAllowed activateAllowed update = do
registrationDomain <- randomDomain
user <- randomUser OwnDomain def
email <- user %. "email" & asString
(cookie, token) <- bindResponse (login user email defPassword) $ \resp -> do
resp.status `shouldMatchInt` 200
token <- resp.json %. "access_token" & asString
let cookie = fromJust $ getCookie "zuid" resp
pure ("zuid=" <> cookie, token)

let newEmail = "galadriel@" <> registrationDomain
updateEmail user newEmail cookie token >>= assertSuccess

(key, code) <- bindResponse (getActivationCode user newEmail) $ \resp -> do
resp.status `shouldMatchInt` 200
(,)
<$> (resp.json %. "key" & asString)
<*> (resp.json %. "code" & asString)

I.updateDomainRegistration OwnDomain registrationDomain update >>= assertSuccess

if activateAllowed
then do
API.Brig.activate user key code >>= assertSuccess
getSelf user `bindResponse` \resp -> do
resp.json %. "email" `shouldMatch` newEmail
else do
API.Brig.activate user key code `bindResponse` \resp -> do
resp.status `shouldMatchInt` 403
resp.json %. "label" `shouldMatch` "condition-failed"

getSelf user `bindResponse` \resp -> do
resp.json %. "email" `shouldMatch` email
2 changes: 1 addition & 1 deletion libs/wire-subsystems/src/Wire/UserSubsystem.hs
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ data UserSubsystem m a where
-- | Throw error if registered domain forbids user account creation under this email
-- address. (This may become internal to the interpreter once migration to wire-subsystems
-- has progressed enough.)
GuardRegisterUserEmailDomain :: EmailAddress -> UserSubsystem m ()
GuardRegisterActivateUserEmailDomain :: EmailAddress -> UserSubsystem m ()
GuardUpgradePersonalUserToTeamEmailDomain :: EmailAddress -> UserSubsystem m ()
-- | Check if an email is blocked.
IsBlocked :: EmailAddress -> UserSubsystem m Bool
Expand Down
8 changes: 4 additions & 4 deletions libs/wire-subsystems/src/Wire/UserSubsystem/Interpreter.hs
Original file line number Diff line number Diff line change
Expand Up @@ -131,8 +131,8 @@ runUserSubsystem authInterpreter = interpret $
updateHandleImpl uid mconn mb uhandle
LookupLocaleWithDefault luid ->
lookupLocaleOrDefaultImpl luid
GuardRegisterUserEmailDomain email ->
guardRegisterUserEmailDomainImpl email
GuardRegisterActivateUserEmailDomain email ->
guardRegisterActivateUserEmailDomainImpl email
GuardUpgradePersonalUserToTeamEmailDomain email ->
guardUpgradePersonalUserToTeamEmailDomainImpl email
IsBlocked email ->
Expand Down Expand Up @@ -220,15 +220,15 @@ internalFindTeamInvitationImpl (Just e) c =
mAddUserError <- checkUserCanJoinTeam tid
maybe (pure ()) (throw . UserSubsystemUserNotAllowedToJoinTeam) mAddUserError

guardRegisterUserEmailDomainImpl ::
guardRegisterActivateUserEmailDomainImpl ::
forall r.
( Member DRS.DomainRegistrationStore r,
Member (Error UserSubsystemError) r,
Member TinyLog r
) =>
EmailAddress ->
Sem r ()
guardRegisterUserEmailDomainImpl email = do
guardRegisterActivateUserEmailDomainImpl email = do
let throwGuardFailed = throw . UserSubsystemGuardFailed
mReg <-
emailDomain email
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ userSubsystemTestInterpreter initialUsers =
CheckHandles _ _ -> error "CheckHandles: implement on demand (userSubsystemInterpreter)"
UpdateHandle {} -> error "UpdateHandle: implement on demand (userSubsystemInterpreter)"
LookupLocaleWithDefault _ -> error "LookupLocaleWithDefault: implement on demand (userSubsystemInterpreter)"
GuardRegisterUserEmailDomain {} -> error "GuardRegisterUserEmailDomain: implemented on demand (userSubsystemInterpreter)"
GuardRegisterActivateUserEmailDomain {} -> error "GuardRegisterActivateUserEmailDomain: implemented on demand (userSubsystemInterpreter)"
GuardUpgradePersonalUserToTeamEmailDomain {} -> error "GuardUpgradePersonalUserToTeamEmailDomain: implemented on demand (userSubsystemInterpreter)"
IsBlocked _ -> pure False
BlockListDelete _ -> error "BlockListDelete: implement on demand (userSubsystemInterpreter)"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -937,7 +937,7 @@ spec = describe "UserSubsystem.Interpreter" do
_ -> Right ()
in outcome === expected

describe "GuardRegisterUserEmailDomain" $ do
describe "GuardRegisterActivateUserEmailDomain" $ do
prop "throws the appropriate errors" $
\(domreg :: DomainRegistration) (preEmail :: EmailAddress) config ->
let email :: EmailAddress
Expand All @@ -952,7 +952,7 @@ spec = describe "UserSubsystem.Interpreter" do
. runError
$ interpretNoFederationStack def Nothing def config do
DRS.upsert domreg
guardRegisterUserEmailDomain email
guardRegisterActivateUserEmailDomain email

expected = case domreg.domainRedirect of
None -> Right ()
Expand Down
3 changes: 2 additions & 1 deletion services/brig/src/Brig/API/Public.hs
Original file line number Diff line number Diff line change
Expand Up @@ -1448,7 +1448,8 @@ activateKey ::
(Handler r) ActivationRespWithStatus
activateKey (Public.Activate tgt code dryrun)
| dryrun = do
wrapClientE (API.preverify tgt code) !>> actError
(emailKey, _) <- wrapClientE (API.preverify tgt code) !>> actError
lift $ liftSem $ guardRegisterActivateUserEmailDomain (emailKeyOrig emailKey)
pure ActivationRespDryRun
| otherwise = do
result <- API.activate tgt code Nothing !>> actError
Expand Down
8 changes: 4 additions & 4 deletions services/brig/src/Brig/API/User.hs
Original file line number Diff line number Diff line change
Expand Up @@ -337,12 +337,12 @@ createUser new = do
inv <- lift $ liftSem $ internalFindTeamInvitation (mkEmailKey <$> email) i
pure (Nothing, Just inv, Just inv.teamId)
Just (NewTeamCreator t) -> do
for_ (emailIdentity =<< new.newUserIdentity) (lift . liftSem . guardRegisterUserEmailDomain)
for_ (emailIdentity =<< new.newUserIdentity) (lift . liftSem . guardRegisterActivateUserEmailDomain)
(Just t,Nothing,) <$> (Just . Id <$> liftIO nextRandom)
Just (NewTeamMemberSSO tid) ->
pure (Nothing, Nothing, Just tid)
Nothing -> do
for_ (emailIdentity =<< new.newUserIdentity) (lift . liftSem . guardRegisterUserEmailDomain)
for_ (emailIdentity =<< new.newUserIdentity) (lift . liftSem . guardRegisterActivateUserEmailDomain)
pure (Nothing, Nothing, Nothing)
let mbInv = (.invitationId) <$> teamInvitation
mbExistingAccount <-
Expand Down Expand Up @@ -683,10 +683,10 @@ preverify ::
) =>
ActivationTarget ->
ActivationCode ->
ExceptT ActivationError m ()
ExceptT ActivationError m (EmailKey, Maybe UserId)
preverify tgt code = do
key <- mkActivationKey tgt
void $ Data.verifyCode key code
Data.verifyCode key code

onActivated ::
( Member TinyLog r,
Expand Down
7 changes: 5 additions & 2 deletions services/brig/src/Brig/Data/Activation.hs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ import Wire.API.User.Password
import Wire.PasswordResetCodeStore (PasswordResetCodeStore)
import Wire.PasswordResetCodeStore qualified as Password
import Wire.UserKeyStore
import Wire.UserSubsystem (UserSubsystem)
import Wire.UserSubsystem
import Wire.UserSubsystem qualified as User

data ActivationError
Expand Down Expand Up @@ -77,7 +77,10 @@ activateKey ::
ActivationCode ->
Maybe UserId ->
ExceptT ActivationError (AppT r) (Maybe ActivationEvent)
activateKey k c u = wrapClientE (verifyCode k c) >>= pickUser >>= activate
activateKey k c u = do
(emailKey, mUser) <- wrapClientE (verifyCode k c)
lift $ liftSem $ guardRegisterActivateUserEmailDomain (emailKeyOrig emailKey)
pickUser (emailKey, mUser) >>= activate
where
pickUser :: (t, Maybe UserId) -> ExceptT ActivationError (AppT r) (t, UserId)
pickUser (uk, u') = maybe (throwE invalidUser) (pure . (uk,)) (u <|> u')
Expand Down

0 comments on commit 07505c2

Please sign in to comment.