diff --git a/libs/brig-types/src/Brig/Types/Intra.hs b/libs/brig-types/src/Brig/Types/Intra.hs index 6a103642f86..1146c09c16b 100644 --- a/libs/brig-types/src/Brig/Types/Intra.hs +++ b/libs/brig-types/src/Brig/Types/Intra.hs @@ -26,6 +26,7 @@ module Brig.Types.Intra AccountStatusResp (..), ConnectionStatus (..), UserAccount (..), + NewUserScimInvitation (..), UserSet (..), ReAuthUser (..), ) @@ -35,7 +36,7 @@ import Brig.Types.Connection import Brig.Types.User import Data.Aeson import qualified Data.HashMap.Strict as M -import Data.Id (UserId) +import Data.Id (TeamId, UserId) import Data.Misc (PlainTextPassword (..)) import qualified Data.Text as Text import Imports @@ -48,6 +49,10 @@ data AccountStatus | Suspended | Deleted | Ephemeral + | -- | for most intents & purposes, this is another form of inactive. it is used for + -- allowing scim to find users that have not accepted their invitation yet after + -- creating via scim. + PendingInvitation deriving (Eq, Show, Generic) instance FromJSON AccountStatus where @@ -56,6 +61,7 @@ instance FromJSON AccountStatus where "suspended" -> pure Suspended "deleted" -> pure Deleted "ephemeral" -> pure Ephemeral + "pending-invitation" -> pure PendingInvitation _ -> fail $ "Invalid account status: " ++ Text.unpack s instance ToJSON AccountStatus where @@ -63,6 +69,7 @@ instance ToJSON AccountStatus where toJSON Suspended = String "suspended" toJSON Deleted = String "deleted" toJSON Ephemeral = String "ephemeral" + toJSON PendingInvitation = String "pending-invitation" data AccountStatusResp = AccountStatusResp {fromAccountStatusResp :: AccountStatus} @@ -135,6 +142,34 @@ instance ToJSON UserAccount where other -> error $ "toJSON UserAccount: not an object: " <> show (encode other) +------------------------------------------------------------------------------- +-- NewUserScimInvitation + +data NewUserScimInvitation = NewUserScimInvitation + { newUserScimInvTeamId :: TeamId, + newUserScimInvLocale :: Maybe Locale, + newUserScimInvName :: Name, + newUserScimInvEmail :: Email + } + deriving (Eq, Show, Generic) + +instance FromJSON NewUserScimInvitation where + parseJSON = withObject "NewUserScimInvitation" $ \o -> + NewUserScimInvitation + <$> o .: "team_id" + <*> o .:? "locale" + <*> o .: "name" + <*> o .: "email" + +instance ToJSON NewUserScimInvitation where + toJSON (NewUserScimInvitation tid loc name email) = + object + [ "team_id" .= tid, + "locale" .= loc, + "name" .= name, + "email" .= email + ] + ------------------------------------------------------------------------------- -- UserList diff --git a/libs/brig-types/src/Brig/Types/User.hs b/libs/brig-types/src/Brig/Types/User.hs index 036681fcc55..cf17f6e524a 100644 --- a/libs/brig-types/src/Brig/Types/User.hs +++ b/libs/brig-types/src/Brig/Types/User.hs @@ -21,6 +21,7 @@ module Brig.Types.User ( ManagedByUpdate (..), RichInfoUpdate (..), PasswordResetPair, + HavePendingInvitations (..), -- * re-exports Pict (..), @@ -80,6 +81,11 @@ import Wire.API.User.RichInfo newtype ManagedByUpdate = ManagedByUpdate {mbuManagedBy :: ManagedBy} deriving (Eq, Show, Generic) +data HavePendingInvitations + = WithPendingInvitations + | NoPendingInvitations + deriving (Eq, Show, Generic) + newtype RichInfoUpdate = RichInfoUpdate {riuRichInfo :: RichInfoAssocList} deriving (Eq, Show, Generic) instance FromJSON ManagedByUpdate where diff --git a/libs/brig-types/test/unit/Test/Brig/Types/User.hs b/libs/brig-types/test/unit/Test/Brig/Types/User.hs index 94ecc13d36c..89f712866eb 100644 --- a/libs/brig-types/test/unit/Test/Brig/Types/User.hs +++ b/libs/brig-types/test/unit/Test/Brig/Types/User.hs @@ -27,7 +27,7 @@ module Test.Brig.Types.User where -import Brig.Types.Intra (ReAuthUser (..)) +import Brig.Types.Intra (NewUserScimInvitation (..), ReAuthUser (..)) import Brig.Types.User (ManagedByUpdate (..), RichInfoUpdate (..)) import Imports import Test.Brig.Roundtrip (testRoundTrip) @@ -41,7 +41,8 @@ roundtripTests :: [TestTree] roundtripTests = [ testRoundTrip @ManagedByUpdate, testRoundTrip @ReAuthUser, - testRoundTrip @RichInfoUpdate + testRoundTrip @RichInfoUpdate, + testRoundTrip @NewUserScimInvitation ] instance Arbitrary ManagedByUpdate where @@ -52,3 +53,6 @@ instance Arbitrary RichInfoUpdate where instance Arbitrary ReAuthUser where arbitrary = ReAuthUser <$> arbitrary + +instance Arbitrary NewUserScimInvitation where + arbitrary = NewUserScimInvitation <$> arbitrary <*> arbitrary <*> arbitrary <*> arbitrary diff --git a/services/brig/src/Brig/API/Error.hs b/services/brig/src/Brig/API/Error.hs index e96ddf050ac..8e3003035ab 100644 --- a/services/brig/src/Brig/API/Error.hs +++ b/services/brig/src/Brig/API/Error.hs @@ -158,6 +158,7 @@ authError AuthInvalidUser = StdError badCredentials authError AuthInvalidCredentials = StdError badCredentials authError AuthSuspended = StdError accountSuspended authError AuthEphemeral = StdError accountEphemeral +authError AuthPendingInvitation = StdError accountPending reauthError :: ReAuthError -> Error reauthError ReAuthMissingPassword = StdError missingAuthError diff --git a/services/brig/src/Brig/API/Internal.hs b/services/brig/src/Brig/API/Internal.hs index 4c0ec07110e..437badb8c8f 100644 --- a/services/brig/src/Brig/API/Internal.hs +++ b/services/brig/src/Brig/API/Internal.hs @@ -35,6 +35,7 @@ import qualified Brig.Data.User as Data import Brig.Options hiding (internalEvents, sesQueue) import qualified Brig.Provider.API as Provider import qualified Brig.Team.API as Team +import Brig.Team.DB (lookupInvitationByEmail) import Brig.Types import Brig.Types.Intra import Brig.Types.Team.LegalHold (LegalHoldClientRequest (..)) @@ -57,6 +58,7 @@ import Network.Wai.Predicate hiding (result, setStatus) import Network.Wai.Routing import Network.Wai.Utilities as Utilities import Network.Wai.Utilities.ZAuth (zauthConnId, zauthUserId) +import qualified System.Logger.Class as Log import Wire.API.User import Wire.API.User.RichInfo @@ -117,10 +119,12 @@ sitemap = do get "/i/users" (continue listActivatedAccountsH) $ accept "application" "json" .&. (param "ids" ||| param "handles") + .&. def False (query "includePendingInvitations") get "/i/users" (continue listAccountsByIdentityH) $ accept "application" "json" .&. (param "email" ||| param "phone") + .&. def False (query "includePendingInvitations") put "/i/users/:uid/status" (continue changeAccountStatusH) $ capture "uid" @@ -334,26 +338,46 @@ changeSelfEmailMaybeSend u DoNotSendEmail email = do ChangeEmailIdempotent -> pure ChangeEmailResponseIdempotent ChangeEmailNeedsActivation _ -> pure ChangeEmailResponseNeedsActivation -listActivatedAccountsH :: JSON ::: Either (List UserId) (List Handle) -> Handler Response -listActivatedAccountsH (_ ::: qry) = do - json <$> lift (listActivatedAccounts qry) - -listActivatedAccounts :: Either (List UserId) (List Handle) -> AppIO [UserAccount] -listActivatedAccounts = \case - Left us -> byIds (fromList us) - Right hs -> do - us <- mapM (API.lookupHandle) (fromList hs) - byIds (catMaybes us) +listActivatedAccountsH :: JSON ::: Either (List UserId) (List Handle) ::: Bool -> Handler Response +listActivatedAccountsH (_ ::: qry ::: includePendingInvitations) = do + json <$> lift (listActivatedAccounts qry includePendingInvitations) + +listActivatedAccounts :: Either (List UserId) (List Handle) -> Bool -> AppIO [UserAccount] +listActivatedAccounts elh includePendingInvitations = do + Log.debug (Log.msg $ "listActivatedAccounts: " <> show (elh, includePendingInvitations)) + case elh of + Left us -> byIds (fromList us) + Right hs -> do + us <- mapM (API.lookupHandle) (fromList hs) + byIds (catMaybes us) where - byIds uids = - filter (isJust . userIdentity . accountUser) - <$> API.lookupAccounts uids - -listAccountsByIdentityH :: JSON ::: Either Email Phone -> Handler Response -listAccountsByIdentityH (_ ::: emailOrPhone) = + byIds :: [UserId] -> AppIO [UserAccount] + byIds uids = API.lookupAccounts uids >>= filterM accountValid + + accountValid :: UserAccount -> AppIO Bool + accountValid account = case userIdentity . accountUser $ account of + Nothing -> pure False + Just ident -> + case (accountStatus account, includePendingInvitations, emailIdentity ident) of + (PendingInvitation, False, _) -> pure False + (PendingInvitation, True, Just email) -> do + hasInvitation <- isJust <$> lookupInvitationByEmail email + unless hasInvitation $ do + -- user invited via scim should expire together with its invitation + API.deleteUserNoVerify (userId . accountUser $ account) + pure hasInvitation + (PendingInvitation, True, Nothing) -> + pure True -- cannot happen, user invited via scim always has an email + (Active, _, _) -> pure True + (Suspended, _, _) -> pure True + (Deleted, _, _) -> pure True + (Ephemeral, _, _) -> pure True + +listAccountsByIdentityH :: JSON ::: Either Email Phone ::: Bool -> Handler Response +listAccountsByIdentityH (_ ::: emailOrPhone ::: includePendingInvitations) = lift $ json - <$> API.lookupAccountsByIdentity emailOrPhone + <$> API.lookupAccountsByIdentity emailOrPhone includePendingInvitations getActivationCodeH :: JSON ::: Either Email Phone -> Handler Response getActivationCodeH (_ ::: emailOrPhone) = do @@ -511,7 +535,7 @@ updateUserName uid (NameUpdate nameUpd) = do uupAssets = Nothing, uupAccentId = Nothing } - lift (Data.lookupUser uid) >>= \case + lift (Data.lookupUser WithPendingInvitations uid) >>= \case Just _ -> lift $ API.updateUser uid Nothing uu Nothing -> throwStd invalidUser diff --git a/services/brig/src/Brig/API/Public.hs b/services/brig/src/Brig/API/Public.hs index 8fa022a4c94..71aa1c3c3e2 100644 --- a/services/brig/src/Brig/API/Public.hs +++ b/services/brig/src/Brig/API/Public.hs @@ -39,7 +39,9 @@ import Brig.Options hiding (internalEvents, sesQueue) import qualified Brig.Provider.API as Provider import qualified Brig.Team.API as Team import qualified Brig.Team.Email as Team +import Brig.Types.Activation (ActivationPair) import Brig.Types.Intra (AccountStatus (Ephemeral), UserAccount (UserAccount, accountUser)) +import Brig.Types.User (HavePendingInvitations (..)) import qualified Brig.User.API.Auth as Auth import qualified Brig.User.API.Search as Search import qualified Brig.User.Auth.Cookie as Auth @@ -74,6 +76,7 @@ import Network.Wai.Utilities as Utilities import Network.Wai.Utilities.Swagger (document, mkSwaggerApi) import qualified Network.Wai.Utilities.Swagger as Doc import Network.Wai.Utilities.ZAuth (zauthConnId, zauthUserId) +import qualified System.Logger.Class as Log import qualified Wire.API.Connection as Public import qualified Wire.API.Properties as Public import qualified Wire.API.Swagger as Public.Swagger (models) @@ -955,8 +958,8 @@ getRichInfo :: UserId -> UserId -> Handler Public.RichInfoAssocList getRichInfo self user = do -- Check that both users exist and the requesting user is allowed to see rich info of the -- other user - selfUser <- ifNothing userNotFound =<< lift (Data.lookupUser self) - otherUser <- ifNothing userNotFound =<< lift (Data.lookupUser user) + selfUser <- ifNothing userNotFound =<< lift (Data.lookupUser NoPendingInvitations self) + otherUser <- ifNothing userNotFound =<< lift (Data.lookupUser NoPendingInvitations user) case (Public.userTeam selfUser, Public.userTeam otherUser) of (Just t1, Just t2) | t1 == t2 -> pure () _ -> throwStd insufficientTeamPermissions @@ -987,6 +990,7 @@ createUser (Public.NewUserPublic new) = do for_ (Public.newUserPhone new) $ checkWhitelist . Right result <- API.createUser new !>> newUserError let acc = createdAccount result + lift $ Log.debug (Log.msg $ "createUser: acc: " <> show acc) let eac = createdEmailActivation result let pac = createdPhoneActivation result let epair = (,) <$> (activationKey <$> eac) <*> (activationCode <$> eac) @@ -1009,6 +1013,7 @@ createUser (Public.NewUserPublic new) = do UserAccount _ _ -> lift $ Auth.newCookie @ZAuth.User userId Public.PersistentCookie newUserLabel pure $ CreateUserResponse cok userId (Public.SelfProfile usr) where + sendActivationEmail :: Public.Email -> Public.Name -> ActivationPair -> Maybe Public.Locale -> Maybe Public.NewTeamUser -> AppIO () sendActivationEmail e u p l mTeamUser | Just teamUser <- mTeamUser, Public.NewTeamCreator creator <- teamUser, @@ -1016,6 +1021,7 @@ createUser (Public.NewUserPublic new) = do sendTeamActivationMail e u p l (fromRange $ team ^. Public.newTeamName) | otherwise = sendActivationMail e u p l Nothing + sendWelcomeEmail :: Public.Email -> CreateUserTeam -> Public.NewTeamUser -> Maybe Public.Locale -> AppIO () -- NOTE: Welcome e-mails for the team creator are not dealt by brig anymore sendWelcomeEmail e (CreateUserTeam t n) newUser l = case newUser of diff --git a/services/brig/src/Brig/API/User.hs b/services/brig/src/Brig/API/User.hs index 03bbea02914..fafda871c39 100644 --- a/services/brig/src/Brig/API/User.hs +++ b/services/brig/src/Brig/API/User.hs @@ -21,6 +21,7 @@ module Brig.API.User ( -- * User Accounts / Profiles createUser, + createUserInviteViaScim, checkRestrictedUserCreation, Brig.API.User.updateUser, changeLocale, @@ -145,6 +146,19 @@ import System.Logger.Message ------------------------------------------------------------------------------- -- Create User +verifyUniquenessAndCheckBlacklist :: UserKey -> ExceptT CreateUserError AppIO () +verifyUniquenessAndCheckBlacklist uk = do + checkKey Nothing uk + blacklisted <- lift $ Blacklist.exists uk + when blacklisted $ + throwE (BlacklistedUserKey uk) + where + checkKey u k = do + av <- lift $ Data.keyAvailable k u + unless av $ + throwE $ + DuplicateUserKey k + -- docs/reference/user/registration.md {#RefRegistration} createUser :: NewUser -> ExceptT CreateUserError AppIO CreateUserResult createUser new@NewUser {..} = do @@ -163,16 +177,23 @@ createUser new@NewUser {..} = do let ident = newIdentity email phone (newUserSSOId new) let emKey = userEmailKey <$> email let phKey = userPhoneKey <$> phone - -- Verify uniqueness and check the blacklist - for_ (catMaybes [emKey, phKey]) $ \uk -> do - checkKey Nothing uk - blacklisted <- lift $ Blacklist.exists uk - when blacklisted $ - throwE (BlacklistedUserKey uk) + for_ (catMaybes [emKey, phKey]) $ verifyUniquenessAndCheckBlacklist -- team user registration (newTeam, teamInvitation, tid) <- handleTeam (newUserTeam new) emKey + -- Create account - (account, pw) <- lift $ newAccount new {newUserIdentity = ident} (Team.inInvitation . fst <$> teamInvitation) tid + (account, pw) <- lift $ do + new' <- + case Team.inInvitation . fst <$> teamInvitation of + Just (Id uuid) -> do + mAcc <- Data.lookupAccount (Id uuid) + case mAcc of + Just existingAccount -> + pure (new {newUserManagedBy = Just . userManagedBy . accountUser $ existingAccount}) + Nothing -> pure new + Nothing -> pure new + newAccount new' {newUserIdentity = ident} (Team.inInvitation . fst <$> teamInvitation) tid + let uid = userId (accountUser account) Log.debug $ field "user" (toByteString uid) . field "action" (Log.val "User.createUser") Log.info $ field "user" (toByteString uid) . msg (val "Creating user") @@ -196,7 +217,7 @@ createUser new@NewUser {..} = do _ -> pure Nothing let joinedTeam :: Maybe CreateUserTeam joinedTeam = joinedTeamInvite <|> joinedTeamSSO - -- Handle e-mail activation + -- Handle e-mail activation (deprecated, see #RefRegistrationNoPreverification in /docs/reference/user/registration.md) edata <- if teamEmailInvited then return Nothing @@ -213,7 +234,7 @@ createUser new@NewUser {..} = do ak <- liftIO $ Data.mkActivationKey ek void $ activateWithCurrency (ActivateKey ak) c (Just uid) (join (bnuCurrency <$> newTeam)) !>> EmailActivationError return Nothing - -- Handle phone activation + -- Handle phone activation (deprecated, see #RefRegistrationNoPreverification in /docs/reference/user/registration.md) pdata <- fmap join . for phKey $ \pk -> case newUserPhoneCode of Nothing -> do timeout <- setActivationTimeout <$> view settings @@ -229,17 +250,13 @@ createUser new@NewUser {..} = do return Nothing return $! CreateUserResult account edata pdata (activatedTeam <|> joinedTeam) where - checkKey u k = do - av <- lift $ Data.keyAvailable k u - unless av $ - throwE $ - DuplicateUserKey k createTeam uid activating t tid = do created <- Intra.createTeam uid t tid return $ if activating then Just created else Nothing + handleTeam :: Maybe NewTeamUser -> Maybe UserKey -> @@ -258,6 +275,7 @@ createUser new@NewUser {..} = do handleTeam (Just (NewTeamCreator t)) _ = (Just t,Nothing,) <$> (Just . Id <$> liftIO nextRandom) handleTeam (Just (NewTeamMemberSSO tid)) _ = pure (Nothing, Nothing, Just tid) handleTeam Nothing _ = return (Nothing, Nothing, Nothing) + findTeamInvitation :: Maybe UserKey -> InvitationCode -> ExceptT CreateUserError AppIO (Maybe (Team.Invitation, Team.InvitationInfo, TeamId)) findTeamInvitation Nothing _ = throwE MissingIdentity findTeamInvitation (Just e) c = @@ -271,6 +289,7 @@ createUser new@NewUser {..} = do return $ Just (invite, ii, Team.iiTeam ii) _ -> throwE InvalidInvitationCode Nothing -> throwE InvalidInvitationCode + ensureMemberCanJoin :: TeamId -> ExceptT CreateUserError AppIO () ensureMemberCanJoin tid = do maxSize <- fromIntegral . setMaxTeamSize <$> view settings @@ -283,6 +302,14 @@ createUser new@NewUser {..} = do case canAdd of Just e -> throwE (ExternalPreconditionFailed e) Nothing -> pure () + + acceptTeamInvitation :: + UserAccount -> + Team.Invitation -> + Team.InvitationInfo -> + UserKey -> + UserIdentity -> + ExceptT CreateUserError (AppT IO) () acceptTeamInvitation account inv ii uk ident = do let uid = userId (accountUser account) ok <- lift $ Data.claimKey uk uid @@ -302,6 +329,7 @@ createUser new@NewUser {..} = do . field "team" (toByteString $ Team.iiTeam ii) . msg (val "Accepting invitation") Team.deleteInvitation (Team.inTeam inv) (Team.inInvitation inv) + addUserToTeamSSO :: UserAccount -> TeamId -> UserIdentity -> ExceptT CreateUserError AppIO CreateUserTeam addUserToTeamSSO account tid ident = do let uid = userId (accountUser account) @@ -318,6 +346,25 @@ createUser new@NewUser {..} = do Team.TeamName nm <- lift $ Intra.getTeamName tid pure $ CreateUserTeam tid nm +-- | 'createUser' is becoming hard to maintian, and instead of adding more case distinctions +-- all over the place there, we add a new function that handles just the one new flow where +-- users are invited to the team via scim. +createUserInviteViaScim :: UserId -> NewUserScimInvitation -> ExceptT Error.Error AppIO UserAccount +createUserInviteViaScim uid (NewUserScimInvitation tid loc name rawEmail) = (`catchE` (throwE . Error.newUserError)) $ do + email <- either (throwE . InvalidEmail rawEmail) pure (validateEmail rawEmail) + let emKey = userEmailKey email + verifyUniquenessAndCheckBlacklist emKey + account <- lift $ newAccountInviteViaScim uid tid loc name email + Log.debug $ field "user" (toByteString . userId . accountUser $ account) . field "action" (Log.val "User.createUserInviteViaScim") + let activated = + -- It would be nice to set this to 'False' to make sure we're not accidentally + -- treating 'PendingActivation' as 'Active', but then 'Brig.Data.User.toIdentity' + -- would not produce an identity, and so we won't have the email address to construct + -- the SCIM user. + True + lift $ Data.insertAccount account Nothing Nothing activated + return account + -- | docs/reference/user/registration.md {#RefRestrictRegistration}. checkRestrictedUserCreation :: NewUser -> ExceptT CreateUserError AppIO () checkRestrictedUserCreation new = do @@ -363,7 +410,7 @@ changeHandle :: UserId -> Maybe ConnId -> Handle -> ExceptT ChangeHandleError Ap changeHandle uid mconn hdl = do when (isBlacklistedHandle hdl) $ throwE ChangeHandleInvalid - usr <- lift $ Data.lookupUser uid + usr <- lift $ Data.lookupUser WithPendingInvitations uid case usr of Nothing -> throwE ChangeHandleNoIdentity Just u -> claim u @@ -458,7 +505,7 @@ changeEmail u email = do unless available $ throwE $ EmailExists email - usr <- maybe (throwM $ UserProfileNotFound u) return =<< lift (Data.lookupUser u) + usr <- maybe (throwM $ UserProfileNotFound u) return =<< lift (Data.lookupUser WithPendingInvitations u) case join (emailIdentity <$> userIdentity usr) of -- The user already has an email address and the new one is exactly the same Just current | current == em -> return ChangeEmailIdempotent @@ -561,6 +608,7 @@ changeAccountStatus usrs status = do Suspended -> liftIO $ mapConcurrently (runAppT e . revokeAllCookies) usrs >> return UserSuspended Deleted -> throwE InvalidAccountStatus Ephemeral -> throwE InvalidAccountStatus + PendingInvitation -> throwE InvalidAccountStatus liftIO $ mapConcurrently_ (runAppT e . (update ev)) usrs where update :: (UserId -> UserEvent) -> UserId -> AppIO () @@ -695,7 +743,9 @@ sendActivationCode emailOrPhone loc call = case emailOrPhone of lift $ sendVerificationMail em p loc sendActivationEmail ek uc uid = do - u <- maybe (notFound uid) return =<< lift (Data.lookupUser uid) + -- FUTUREWORK(fisx): we allow for 'PendingInvitations' here, but I'm not sure this + -- top-level function isn't another piece of a deprecated onboarding flow? + u <- maybe (notFound uid) return =<< lift (Data.lookupUser WithPendingInvitations uid) p <- mkPair ek (Just uc) (Just uid) let ident = userIdentity u name = userDisplayName u @@ -816,6 +866,7 @@ deleteUser uid pwd = do Suspended -> ensureNotOwner a >> go a Active -> ensureNotOwner a >> go a Ephemeral -> go a + PendingInvitation -> go a where ensureNotOwner :: UserAccount -> ExceptT DeleteUserError (AppT IO) () ensureNotOwner acc = do @@ -995,7 +1046,7 @@ lookupProfilesOfLocalUsers :: [UserId] -> AppIO [UserProfile] lookupProfilesOfLocalUsers self others = do - users <- Data.lookupUsers others >>= mapM userGC + users <- Data.lookupUsers NoPendingInvitations others >>= mapM userGC css <- toMap <$> Data.lookupConnectionStatus (map userId users) [self] emailVisibility' <- view (settings . emailVisibility) emailVisibility'' <- case emailVisibility' of @@ -1012,7 +1063,7 @@ lookupProfilesOfLocalUsers self others = do -- FUTUREWORK: it is an internal error for the two lookups (for 'User' and 'TeamMember') -- to return 'Nothing'. we could throw errors here if that happens, rather than just -- returning an empty profile list from 'lookupProfiles'. - mUser <- Data.lookupUser self + mUser <- Data.lookupUser NoPendingInvitations self case userTeam =<< mUser of Nothing -> pure Nothing Just tid -> (tid,) <$$> Intra.getTeamMember self tid @@ -1052,12 +1103,15 @@ getEmailForProfile _ EmailVisibleToSelf' = Nothing -- | Find user accounts for a given identity, both activated and those -- currently pending activation. -lookupAccountsByIdentity :: Either Email Phone -> AppIO [UserAccount] -lookupAccountsByIdentity emailOrPhone = do +lookupAccountsByIdentity :: Either Email Phone -> Bool -> AppIO [UserAccount] +lookupAccountsByIdentity emailOrPhone includePendingInvitations = do let uk = either userEmailKey userPhoneKey emailOrPhone activeUid <- Data.lookupKey uk uidFromKey <- (>>= fst) <$> Data.lookupActivationCode uk - Data.lookupAccounts (nub $ catMaybes [activeUid, uidFromKey]) + result <- Data.lookupAccounts (nub $ catMaybes [activeUid, uidFromKey]) + if includePendingInvitations + then pure result + else pure $ filter ((/= PendingInvitation) . accountStatus) result isBlacklisted :: Either Email Phone -> AppIO Bool isBlacklisted emailOrPhone = do diff --git a/services/brig/src/Brig/App.hs b/services/brig/src/Brig/App.hs index 198e2684bf5..c808312cba3 100644 --- a/services/brig/src/Brig/App.hs +++ b/services/brig/src/Brig/App.hs @@ -53,6 +53,7 @@ module Brig.App turnEnvV2, sftEnv, internalEvents, + emailSender, -- * App Monad AppT, @@ -126,6 +127,7 @@ import System.Logger.Class hiding (Settings, settings) import qualified System.Logger.Class as LC import qualified System.Logger.Extended as Log import Util.Options +import Wire.API.User.Identity (Email) schemaVersion :: Int32 schemaVersion = 61 @@ -139,6 +141,7 @@ data Env = Env _gundeck :: RPC.Request, _casClient :: Cas.ClientState, _smtpEnv :: Maybe SMTP.SMTP, + _emailSender :: Email, _awsEnv :: AWS.Env, _stompEnv :: Maybe Stomp.Env, _metrics :: Metrics, @@ -212,6 +215,7 @@ newEnv o = do _gundeck = mkEndpoint $ Opt.gundeck o, _casClient = cas, _smtpEnv = emailSMTP, + _emailSender = Opt.emailSender . Opt.general . Opt.emailSMS $ o, _awsEnv = aws, _stompEnv = stomp, _metrics = mtr, diff --git a/services/brig/src/Brig/Data/Activation.hs b/services/brig/src/Brig/Data/Activation.hs index 11ef153147e..55329cca716 100644 --- a/services/brig/src/Brig/Data/Activation.hs +++ b/services/brig/src/Brig/Data/Activation.hs @@ -84,7 +84,7 @@ activateKey k c u = verifyCode k c >>= pickUser >>= activate pickUser (uk, u') = maybe (throwE invalidUser) (return . (uk,)) (u <|> u') activate (key, uid) = do a <- lift (lookupAccount uid) >>= maybe (throwE invalidUser) return - unless (accountStatus a == Active) $ + unless (accountStatus a == Active) $ -- this is never 'PendingActivation' in the flow this function is used in. throwE invalidCode case userIdentity (accountUser a) of Nothing -> do diff --git a/services/brig/src/Brig/Data/Instances.hs b/services/brig/src/Brig/Data/Instances.hs index 19246eeb378..a103725d7fa 100644 --- a/services/brig/src/Brig/Data/Instances.hs +++ b/services/brig/src/Brig/Data/Instances.hs @@ -171,12 +171,14 @@ instance Cql AccountStatus where toCql Suspended = CqlInt 1 toCql Deleted = CqlInt 2 toCql Ephemeral = CqlInt 3 + toCql PendingInvitation = CqlInt 4 fromCql (CqlInt i) = case i of 0 -> return Active 1 -> return Suspended 2 -> return Deleted 3 -> return Ephemeral + 4 -> return PendingInvitation n -> Left $ "unexpected account status: " ++ show n fromCql _ = Left "account status: int expected" diff --git a/services/brig/src/Brig/Data/User.hs b/services/brig/src/Brig/Data/User.hs index 2eb4c9c1d69..95d20b7a2e1 100644 --- a/services/brig/src/Brig/Data/User.hs +++ b/services/brig/src/Brig/Data/User.hs @@ -25,6 +25,7 @@ module Brig.Data.User ( AuthError (..), ReAuthError (..), newAccount, + newAccountInviteViaScim, insertAccount, authenticate, reauthenticate, @@ -42,7 +43,6 @@ module Brig.Data.User lookupStatus, lookupRichInfo, lookupUserTeam, - lookupUsersTeam, lookupServiceUsers, lookupServiceUsersForTeam, @@ -95,6 +95,7 @@ data AuthError | AuthInvalidCredentials | AuthSuspended | AuthEphemeral + | AuthPendingInvitation -- | Re-authentication errors. data ReAuthError @@ -136,6 +137,27 @@ newAccount u inv tid = do managedBy = fromMaybe defaultManagedBy (newUserManagedBy u) user uid l e = User uid ident name pict assets colour False l Nothing Nothing e tid managedBy +newAccountInviteViaScim :: UserId -> TeamId -> Maybe Locale -> Name -> Email -> AppIO UserAccount +newAccountInviteViaScim uid tid locale name email = do + defLoc <- setDefaultLocale <$> view settings + return (UserAccount (user (fromMaybe defLoc locale)) PendingInvitation) + where + user loc = + User + uid + (Just $ EmailIdentity email) + name + (Pict []) + [] + defaultAccentId + False + loc + Nothing + Nothing + Nothing + (Just tid) + ManagedByScim + -- | Mandatory password authentication. authenticate :: UserId -> PlainTextPassword -> ExceptT AuthError AppIO () authenticate u pw = @@ -144,6 +166,7 @@ authenticate u pw = Just (_, Deleted) -> throwE AuthInvalidUser Just (_, Suspended) -> throwE AuthSuspended Just (_, Ephemeral) -> throwE AuthEphemeral + Just (_, PendingInvitation) -> throwE AuthPendingInvitation Just (Nothing, _) -> throwE AuthInvalidCredentials Just (Just pw', Active) -> unless (verifyPassword pw pw') $ @@ -158,6 +181,7 @@ reauthenticate u pw = Nothing -> throwE (ReAuthError AuthInvalidUser) Just (_, Deleted) -> throwE (ReAuthError AuthInvalidUser) Just (_, Suspended) -> throwE (ReAuthError AuthSuspended) + Just (_, PendingInvitation) -> throwE (ReAuthError AuthPendingInvitation) Just (Nothing, _) -> for_ pw $ const (throwE $ ReAuthError AuthInvalidCredentials) Just (Just pw', Active) -> maybeReAuth pw' Just (Just pw', Ephemeral) -> maybeReAuth pw' @@ -305,8 +329,8 @@ filterActive us = isActiveUser (_, True, Just Active) = True isActiveUser _ = False -lookupUser :: UserId -> AppIO (Maybe User) -lookupUser u = listToMaybe <$> lookupUsers [u] +lookupUser :: HavePendingInvitations -> UserId -> AppIO (Maybe User) +lookupUser hpi u = listToMaybe <$> lookupUsers hpi [u] activateUser :: UserId -> UserIdentity -> AppIO () activateUser u ident = do @@ -343,15 +367,14 @@ lookupRichInfo u = fmap runIdentity <$> retry x1 (query1 richInfoSelect (params Quorum (Identity u))) +-- | Lookup user (no matter what status) and return 'TeamId'. Safe to use for authorization: +-- suspended / deleted / ... users can't login, so no harm done if we authorize them *after* +-- successful login. lookupUserTeam :: UserId -> AppIO (Maybe TeamId) lookupUserTeam u = join . fmap runIdentity <$> retry x1 (query1 teamSelect (params Quorum (Identity u))) -lookupUsersTeam :: [UserId] -> AppIO [(UserId, Maybe TeamId)] -lookupUsersTeam us = - retry x1 (query usersTeamSelect (params Quorum (Identity us))) - lookupAuth :: (MonadClient m) => UserId -> m (Maybe (Maybe Password, AccountStatus)) lookupAuth u = fmap f <$> retry x1 (query1 authSelect (params Quorum (Identity u))) where @@ -360,10 +383,10 @@ lookupAuth u = fmap f <$> retry x1 (query1 authSelect (params Quorum (Identity u -- | Return users with given IDs. -- -- Skips nonexistent users. /Does not/ skip users who have been deleted. -lookupUsers :: [UserId] -> AppIO [User] -lookupUsers usrs = do +lookupUsers :: HavePendingInvitations -> [UserId] -> AppIO [User] +lookupUsers hpi usrs = do loc <- setDefaultLocale <$> view settings - toUsers loc <$> retry x1 (query usersSelect (params Quorum (Identity usrs))) + toUsers loc hpi <$> retry x1 (query usersSelect (params Quorum (Identity usrs))) lookupAccount :: UserId -> AppIO (Maybe UserAccount) lookupAccount u = listToMaybe <$> lookupAccounts [u] @@ -513,9 +536,6 @@ richInfoSelect = "SELECT json FROM rich_info WHERE user = ?" teamSelect :: PrepQuery R (Identity UserId) (Identity (Maybe TeamId)) teamSelect = "SELECT team FROM user WHERE id = ?" -usersTeamSelect :: PrepQuery R (Identity [UserId]) (UserId, Maybe TeamId) -usersTeamSelect = "SELECT id, team FROM user WHERE id IN ?" - accountsSelect :: PrepQuery R (Identity [UserId]) AccountRow accountsSelect = "SELECT id, name, picture, email, phone, sso_id, accent_id, assets, \ @@ -630,9 +650,36 @@ toUserAccount ) (fromMaybe Active status) -toUsers :: Locale -> [UserRow] -> [User] -toUsers defaultLocale = fmap mk +toUsers :: Locale -> HavePendingInvitations -> [UserRow] -> [User] +toUsers defaultLocale havePendingInvitations = fmap mk . filter fp where + fp :: UserRow -> Bool + fp = + case havePendingInvitations of + WithPendingInvitations -> const True + NoPendingInvitations -> + ( \( _uid, + _name, + _pict, + _email, + _phone, + _ssoid, + _accent, + _assets, + _activated, + status, + _expires, + _lan, + _con, + _pid, + _sid, + _handle, + _tid, + _managed_by + ) -> status /= Just PendingInvitation + ) + + mk :: UserRow -> User mk ( uid, name, diff --git a/services/brig/src/Brig/Provider/API.hs b/services/brig/src/Brig/Provider/API.hs index 9de1e49f6d4..f11d71080c8 100644 --- a/services/brig/src/Brig/Provider/API.hs +++ b/services/brig/src/Brig/Provider/API.hs @@ -49,7 +49,7 @@ import Brig.Types.Client (Client (..), ClientType (..), newClient, newClientPrek import Brig.Types.Intra (AccountStatus (..), UserAccount (..)) import Brig.Types.Provider (AddBot (..), DeleteProvider (..), DeleteService (..), NewService (..), PasswordChange (..), Provider (..), ProviderLogin (..), Service (..), ServiceProfile (..), ServiceToken (..), UpdateBotPrekeys (..), UpdateProvider (..), UpdateService (..), UpdateServiceConn (..), UpdateServiceWhitelist (..)) import qualified Brig.Types.Provider.External as Ext -import Brig.Types.User (ManagedBy (..), Name (..), Pict (..), User (..), defaultAccentId) +import Brig.Types.User (HavePendingInvitations (..), ManagedBy (..), Name (..), Pict (..), User (..), defaultAccentId) import qualified Brig.ZAuth as ZAuth import Control.Error (throwE) import Control.Exception.Enclosed (handleAny) @@ -799,7 +799,7 @@ addBotH (zuid ::: zcon ::: cid ::: req) = do addBot :: UserId -> ConnId -> ConvId -> Public.AddBot -> Handler Public.AddBotResponse addBot zuid zcon cid add = do - zusr <- lift (User.lookupUser zuid) >>= maybeInvalidUser + zusr <- lift (User.lookupUser NoPendingInvitations zuid) >>= maybeInvalidUser let pid = addBotProvider add let sid = addBotService add -- Get the conversation and check preconditions @@ -896,7 +896,7 @@ botGetSelfH bot = json <$> botGetSelf bot botGetSelf :: BotId -> Handler Public.UserProfile botGetSelf bot = do - p <- lift $ User.lookupUser (botUserId bot) + p <- lift $ User.lookupUser NoPendingInvitations (botUserId bot) maybe (throwStd userNotFound) (return . Public.publicProfile) p botGetClientH :: BotId -> Handler Response @@ -948,7 +948,7 @@ botListUserProfilesH uids = do botListUserProfiles :: List UserId -> Handler [Public.BotUserView] botListUserProfiles uids = do - us <- lift $ User.lookupUsers (fromList uids) + us <- lift $ User.lookupUsers NoPendingInvitations (fromList uids) return (map mkBotUserView us) botGetUserClientsH :: UserId -> Handler Response @@ -967,7 +967,7 @@ botDeleteSelfH (bid ::: cid) = do botDeleteSelf :: BotId -> ConvId -> Handler () botDeleteSelf bid cid = do - bot <- lift $ User.lookupUser (botUserId bid) + bot <- lift $ User.lookupUser NoPendingInvitations (botUserId bid) _ <- maybeInvalidBot (userService =<< bot) _ <- lift $ deleteBot (botUserId bid) Nothing bid cid return () @@ -992,7 +992,7 @@ deleteBot zusr zcon bid cid = do ev <- RPC.removeBotMember zusr zcon cid bid -- Delete the bot user and client let buid = botUserId bid - mbUser <- User.lookupUser buid + mbUser <- User.lookupUser NoPendingInvitations buid User.lookupClients buid >>= mapM_ (User.rmClient buid . clientId) for_ (userService =<< mbUser) $ \sref -> do let pid = sref ^. serviceRefProvider diff --git a/services/brig/src/Brig/Team/API.hs b/services/brig/src/Brig/Team/API.hs index 36ba98e281f..b9e7a024cd0 100644 --- a/services/brig/src/Brig/Team/API.hs +++ b/services/brig/src/Brig/Team/API.hs @@ -23,9 +23,9 @@ where import Brig.API.Error import Brig.API.Handler -import Brig.API.User (fetchUserIdentity) +import Brig.API.User (createUserInviteViaScim, fetchUserIdentity) import qualified Brig.API.User as API -import Brig.App (AppIO, currentTime, settings) +import Brig.App (currentTime, emailSender, settings) import qualified Brig.Data.Blacklist as Blacklist import Brig.Data.UserKey import qualified Brig.Data.UserKey as Data @@ -36,7 +36,7 @@ import qualified Brig.Phone as Phone import qualified Brig.Team.DB as DB import Brig.Team.Email import Brig.Team.Util (ensurePermissionToAddUser, ensurePermissions) -import Brig.Types.Intra (AccountStatus (..)) +import Brig.Types.Intra (AccountStatus (..), NewUserScimInvitation (..), UserAccount (..)) import Brig.Types.Team (TeamSize) import Brig.Types.Team.Invitation import Brig.Types.User (Email, InvitationCode, emailIdentity) @@ -59,11 +59,12 @@ import Network.Wai.Utilities hiding (code, message) import Network.Wai.Utilities.Swagger (document) import qualified Network.Wai.Utilities.Swagger as Doc import qualified Wire.API.Team.Invitation as Public +import qualified Wire.API.Team.Role as Public import qualified Wire.API.User as Public routesPublic :: Routes Doc.ApiBuilder Handler () routesPublic = do - post "/teams/:tid/invitations" (continue createInvitationH) $ + post "/teams/:tid/invitations" (continue createInvitationPublicH) $ accept "application" "json" .&. header "Z-User" .&. capture "tid" @@ -143,9 +144,10 @@ routesPublic = do Doc.errorResponse invalidInvitationCode -- FUTUREWORK: Add another endpoint to allow resending of invitation codes - head "/teams/invitations/by-email" (continue headInvitationsByEmailH) $ + head "/teams/invitations/by-email" (continue headInvitationByEmailH) $ accept "application" "json" .&. query "email" + document "HEAD" "headInvitationPending" $ do Doc.summary "Check if there is an invitation pending given an email address." Doc.parameter Doc.Query "email" Doc.bytes' $ @@ -156,6 +158,10 @@ routesPublic = do routesInternal :: Routes a Handler () routesInternal = do + get "/i/teams/invitations/by-email" (continue getInvitationByEmailH) $ + accept "application" "json" + .&. query "email" + get "/i/teams/invitation-code" (continue getInvitationCodeH) $ accept "application" "json" .&. param "team" @@ -173,6 +179,10 @@ routesInternal = do accept "application" "json" .&. capture "tid" + post "/i/teams/:tid/invitations" (continue createInvitationViaScimH) $ + accept "application" "json" + .&. jsonRequest @NewUserScimInvitation + teamSizeH :: JSON ::: TeamId -> Handler Response teamSizeH (_ ::: t) = json <$> teamSize t @@ -194,10 +204,10 @@ data FoundInvitationCode = FoundInvitationCode InvitationCode instance ToJSON FoundInvitationCode where toJSON (FoundInvitationCode c) = object ["code" .= c] -createInvitationH :: JSON ::: UserId ::: TeamId ::: JsonRequest Public.InvitationRequest -> Handler Response -createInvitationH (_ ::: uid ::: tid ::: req) = do +createInvitationPublicH :: JSON ::: UserId ::: TeamId ::: JsonRequest Public.InvitationRequest -> Handler Response +createInvitationPublicH (_ ::: uid ::: tid ::: req) = do body <- parseJsonBody req - newInv <- createInvitation uid tid body + newInv <- createInvitationPublic uid tid body pure . setStatus status201 . loc (inInvitation newInv) . json $ newInv where loc iid = @@ -210,30 +220,57 @@ data CreateInvitationInviter = CreateInvitationInviter } deriving (Eq, Show) -createInvitation :: UserId -> TeamId -> Public.InvitationRequest -> Handler Public.Invitation -createInvitation uid tid body = do - let inviteePerms = Team.rolePermissions inviteeRole - inviteeRole = fromMaybe Team.defaultRole . irRole $ body +createInvitationPublic :: UserId -> TeamId -> Public.InvitationRequest -> Handler Public.Invitation +createInvitationPublic uid tid body = do + let inviteeRole = fromMaybe Team.defaultRole . irRole $ body inviter <- do + let inviteePerms = Team.rolePermissions inviteeRole idt <- maybe (throwStd (noIdentity 7)) return =<< lift (fetchUserIdentity uid) from <- maybe (throwStd noEmail) return (emailIdentity idt) ensurePermissionToAddUser uid tid inviteePerms pure $ CreateInvitationInviter uid from + createInvitation' tid inviteeRole (Just (inviterUid inviter)) (inviterEmail inviter) body + +createInvitationViaScimH :: JSON ::: JsonRequest NewUserScimInvitation -> Handler Response +createInvitationViaScimH (_ ::: req) = do + body <- parseJsonBody req + setStatus status201 . json <$> createInvitationViaScim body + +createInvitationViaScim :: NewUserScimInvitation -> Handler UserAccount +createInvitationViaScim newUser@(NewUserScimInvitation tid loc name email) = do + env <- ask + let inviteeRole = Team.defaultRole + fromEmail = env ^. emailSender + invreq = + InvitationRequest + { irLocale = loc, + irRole = Nothing, -- (unused, it's in the type for 'createInvitationPublicH') + irInviteeName = Just name, + irInviteeEmail = email, + irInviteePhone = Nothing + } + inv <- createInvitation' tid inviteeRole Nothing fromEmail invreq + let uid = Id (toUUID (inInvitation inv)) + createUserInviteViaScim uid newUser + +createInvitation' :: TeamId -> Public.Role -> Maybe UserId -> Email -> Public.InvitationRequest -> Handler Public.Invitation +createInvitation' tid inviteeRole mbInviterUid fromEmail body = do -- FUTUREWORK: These validations are nearly copy+paste from accountCreation and -- sendActivationCode. Refactor this to a single place -- Validate e-mail - email <- either (const $ throwStd invalidEmail) return (Email.validateEmail (irInviteeEmail body)) - let uke = userEmailKey email + inviteeEmail <- either (const $ throwStd invalidEmail) return (Email.validateEmail (irInviteeEmail body)) + let uke = userEmailKey inviteeEmail blacklistedEm <- lift $ Blacklist.exists uke when blacklistedEm $ throwStd blacklistedEmail emailTaken <- lift $ isJust <$> Data.lookupKey uke when emailTaken $ throwStd emailExists + -- Validate phone - phone <- for (irInviteePhone body) $ \p -> do + inviteePhone <- for (irInviteePhone body) $ \p -> do validatedPhone <- maybe (throwStd invalidPhone) return =<< lift (Phone.validatePhone p) let ukp = userPhoneKey validatedPhone blacklistedPh <- lift $ Blacklist.exists ukp @@ -248,33 +285,25 @@ createInvitation uid tid body = do when (fromIntegral pending >= maxSize) $ throwStd tooManyTeamInvitations - iid <- liftIO DB.mkInvitationId - lift $ doInvite iid inviteeRole inviter (irLocale body) email (irInviteeName body) phone - where - doInvite :: - InvitationId -> - Team.Role -> - CreateInvitationInviter -> - Maybe Public.Locale -> - Email -> - Maybe Public.Name -> - Maybe Public.Phone -> - AppIO Invitation - doInvite iid role inviter lc toEmail toName toPhone = do - now <- liftIO =<< view currentTime - timeout <- setTeamInvitationTimeout <$> view settings - (newInv, code) <- - DB.insertInvitation - iid - tid - role - now - (Just $ inviterUid inviter) - toEmail - toName - toPhone - timeout - newInv <$ sendInvitationMail toEmail tid (inviterEmail inviter) code lc + let locale = irLocale body + let inviteeName = irInviteeName body + + lift $ do + iid <- liftIO DB.mkInvitationId + now <- liftIO =<< view currentTime + timeout <- setTeamInvitationTimeout <$> view settings + (newInv, code) <- + DB.insertInvitation + iid + tid + inviteeRole + now + mbInviterUid + inviteeEmail + inviteeName + inviteePhone + timeout + newInv <$ sendInvitationMail inviteeEmail tid fromEmail code locale deleteInvitationH :: JSON ::: UserId ::: TeamId ::: InvitationId -> Handler Response deleteInvitationH (_ ::: uid ::: tid ::: iid) = do @@ -316,14 +345,26 @@ getInvitationByCode c = do inv <- lift $ DB.lookupInvitationByCode c maybe (throwStd invalidInvitationCode) return inv -headInvitationsByEmailH :: JSON ::: Email -> Handler Response -headInvitationsByEmailH (_ ::: e) = do +headInvitationByEmailH :: JSON ::: Email -> Handler Response +headInvitationByEmailH (_ ::: e) = do inv <- lift $ DB.lookupInvitationInfoByEmail e return $ case inv of DB.InvitationByEmail _ -> setStatus status200 empty DB.InvitationByEmailNotFound -> setStatus status404 empty DB.InvitationByEmailMoreThanOne -> setStatus status409 empty +-- | FUTUREWORK: This should also respond with status 409 in case of +-- @DB.InvitationByEmailMoreThanOne@. Refactor so that 'headInvitationByEmailH' and +-- 'getInvitationByEmailH' are almost the same thing. +getInvitationByEmailH :: JSON ::: Email -> Handler Response +getInvitationByEmailH (_ ::: email) = + json <$> getInvitationByEmail email + +getInvitationByEmail :: Email -> Handler Public.Invitation +getInvitationByEmail email = do + inv <- lift $ DB.lookupInvitationByEmail email + maybe (throwStd (notFound "Invitation not found")) return inv + suspendTeamH :: JSON ::: TeamId -> Handler Response suspendTeamH (_ ::: tid) = do empty <$ suspendTeam tid diff --git a/services/brig/src/Brig/User/Auth.hs b/services/brig/src/Brig/User/Auth.hs index 650374da184..4b0eca679cb 100644 --- a/services/brig/src/Brig/User/Auth.hs +++ b/services/brig/src/Brig/User/Auth.hs @@ -114,10 +114,11 @@ login (PasswordLogin li pw label) typ = do Log.debug $ field "user" (toByteString uid) . field "action" (Log.val "User.login") checkRetryLimit uid Data.authenticate uid pw `catchE` \case + AuthInvalidUser -> loginFailed uid + AuthInvalidCredentials -> loginFailed uid AuthSuspended -> throwE LoginSuspended AuthEphemeral -> throwE LoginEphemeral - AuthInvalidCredentials -> loginFailed uid - AuthInvalidUser -> loginFailed uid + AuthPendingInvitation -> throwE LoginPendingActivation newAccess @ZAuth.User @ZAuth.Access uid typ label login (SmsLogin phone code label) typ = do uid <- resolveLoginId (LoginByPhone phone) @@ -245,9 +246,14 @@ isPendingActivation ident = case ident of Nothing -> return False Just u -> maybe False (checkAccount k) <$> Data.lookupAccount u checkAccount k a = - let s = accountStatus a - i = userIdentity (accountUser a) - in s == Active && case i of + let i = userIdentity (accountUser a) + statusAdmitsPending = case accountStatus a of + Active -> True + Suspended -> False + Deleted -> False + Ephemeral -> False + PendingInvitation -> True + in statusAdmitsPending && case i of Just (EmailIdentity e) -> userEmailKey e /= k Just (PhoneIdentity p) -> userPhoneKey p /= k Just (FullIdentity e p) -> userEmailKey e /= k && userPhoneKey p /= k @@ -296,10 +302,11 @@ ssoLogin (SsoLogin uid label) typ = do Data.reauthenticate uid Nothing `catchE` \case ReAuthMissingPassword -> pure () ReAuthError e -> case e of + AuthInvalidUser -> throwE LoginFailed AuthInvalidCredentials -> pure () AuthSuspended -> throwE LoginSuspended AuthEphemeral -> throwE LoginEphemeral - AuthInvalidUser -> throwE LoginFailed + AuthPendingInvitation -> throwE LoginPendingActivation newAccess @ZAuth.User @ZAuth.Access uid typ label -- | Log in as a LegalHold service, getting LegalHoldUser/Access Tokens. diff --git a/services/brig/src/Brig/User/Search/Index.hs b/services/brig/src/Brig/User/Search/Index.hs index d66267f3118..3318910779d 100644 --- a/services/brig/src/Brig/User/Search/Index.hs +++ b/services/brig/src/Brig/User/Search/Index.hs @@ -804,7 +804,8 @@ reindexRowToIndexUser (u, mteam, name, t0, status, t1, handle, t2, colour, t4, a Just Active -> True Just Suspended -> True Just Deleted -> False - Just Ephemeral -> False, + Just Ephemeral -> False + Just PendingInvitation -> False, activated, -- FUTUREWORK: how is this adding to the first case? isNothing service ] diff --git a/services/brig/test/integration/API/Team.hs b/services/brig/test/integration/API/Team.hs index 26644728549..aa338e22537 100644 --- a/services/brig/test/integration/API/Team.hs +++ b/services/brig/test/integration/API/Team.hs @@ -89,6 +89,8 @@ tests conf m n b c g aws = do test m "get /teams/:tid/invitations/info - 200" $ testInvitationInfo b, test m "get /teams/:tid/invitations/info - 400" $ testInvitationInfoBadCode b, test m "get /teams/:tid/invitations/info - 400 expired" $ testInvitationInfoExpired b it, + -- "get /i/teams/invitations/by-email?email=..." is tested in 'testCreateUserNoIdP', 'testCreateUserTimeout' + -- in spar's integration tests, module "Test.Spar.Scim.UserSpec" test m "post /i/teams/:tid/suspend - 200" $ testSuspendTeam b, test m "put /self - 200 update events" $ testUpdateEvents b c, test m "delete /self - 200 (ensure no orphan teams)" $ testDeleteTeamUser b g, diff --git a/services/brig/test/integration/API/User/Account.hs b/services/brig/test/integration/API/User/Account.hs index aba28d05f29..80b5599f510 100644 --- a/services/brig/test/integration/API/User/Account.hs +++ b/services/brig/test/integration/API/User/Account.hs @@ -103,6 +103,8 @@ tests _ at opts p b c ch g aws = test' aws p "post /i/users/phone-prefix" $ testInternalPhonePrefixes b, test' aws p "put /i/users/:uid/status (suspend)" $ testSuspendUser b, test' aws p "get /i/users?:(email|phone) - 200" $ testGetByIdentity b, + -- "get /i/users?:ids=...&includePendingInvitations=..." is tested in 'testCreateUserNoIdP', 'testCreateUserTimeout' + -- in spar's integration tests, module "Test.Spar.Scim.UserSpec" test' aws p "delete/phone-email" $ testEmailPhoneDelete b c, test' aws p "delete/by-password" $ testDeleteUserByPassword b c aws, test' aws p "delete/with-legalhold" $ testDeleteUserWithLegalHold b c aws, diff --git a/services/spar/spar.cabal b/services/spar/spar.cabal index 92b9befd6e0..60acb86e102 100644 --- a/services/spar/spar.cabal +++ b/services/spar/spar.cabal @@ -4,7 +4,7 @@ cabal-version: 1.12 -- -- see: https://github.com/sol/hpack -- --- hash: 8a7b77bbc7e9d990ac78dd068fccb94f15521732eac4d3a2cc9bc44aa38c7e7f +-- hash: 54710b4d4da7f1d6f621e0c28ded0702090f208db2f230715c129480502db771 name: spar version: 0.1 @@ -208,6 +208,7 @@ executable spar-integration Util Util.Core Util.Email + Util.Invitation Util.Scim Util.Types Paths_spar diff --git a/services/spar/src/Spar/App.hs b/services/spar/src/Spar/App.hs index 521830c0312..88a2baa9117 100644 --- a/services/spar/src/Spar/App.hs +++ b/services/spar/src/Spar/App.hs @@ -36,7 +36,8 @@ module Spar.App where import Bilge -import Brig.Types (ManagedBy (..)) +import Brig.Types (ManagedBy (..), userTeam) +import Brig.Types.Intra (AccountStatus (..), accountStatus, accountUser) import Cassandra import qualified Cassandra as Cas import Control.Exception (assert) @@ -170,27 +171,36 @@ wrapMonadClient action = do insertUser :: SAML.UserRef -> UserId -> Spar () insertUser uref uid = wrapMonadClient $ Data.insertSAMLUser uref uid --- | Look up user locally in table @spar.user@, then in brig, then return the 'UserId'. If --- either lookup fails, or user is not in a team, return 'Nothing'. See also: --- 'Spar.App.createUser'. +-- | Look up user locally in table @spar.user@ or @spar.scim_user@ (depending on the +-- argument), then in brig, then return the 'UserId'. If either lookup fails, or user is not +-- in a team, return 'Nothing'. -- --- It makes sense to require that users are required to be team members: the idp is created in --- the context of a team, and the only way for users to be created is as team members. If a --- user is not a team member, it cannot have been created using SAML. +-- It makes sense to require that users are required to be team members: both IdPs and SCIM +-- tokens are created in the context of teams, and the only way for users to be created is as +-- team members. If a user is not a team member, it cannot have been created using SAML or +-- SCIM. +-- +-- If a user has been created via scim invite (ie., no IdP present), and has status +-- 'PendingInvitation', its 'UserId' will be returned here, since for SCIM purposes it is an +-- existing (if inactive) user. If 'getUser' is called during SAML authentication, this may +-- cause an inactive user to log in, but that's ok: `PendingActivation` means that email and +-- password handshake have not been completed; it's still ok for the user to gain access to +-- the team with valid SAML credentials. -- -- ASSUMPTIONS: User creation on brig/galley is idempotent. Any incomplete creation (because of --- brig or galley crashing) will cause the lookup here to yield invalid user. +-- brig or galley crashing) will cause the lookup here to yield 'Nothing'. getUser :: ValidExternalId -> Spar (Maybe UserId) getUser veid = do muid <- wrapMonadClient $ runValidExternalId Data.getSAMLUser Data.lookupScimExternalId veid case muid of Nothing -> pure Nothing Just uid -> do - itis <- isJust <$> Intra.getBrigUserTeam uid + let withpending = Intra.WithPendingInvitations -- see haddocks above + itis <- isJust <$> Intra.getBrigUserTeam withpending uid pure $ if itis then Just uid else Nothing --- | Create a fresh 'Data.Id.UserId', store it on C* locally together with 'SAML.UserRef', then --- create user on brig. See also: 'Spar.App.getUser'. +-- | Create a fresh 'UserId', store it on C* locally together with 'SAML.UserRef', then +-- create user on brig. -- -- The manual for the team admin should say this: when deleting a user, delete it on the IdP first, -- then delete it on the team admin page in wire. If a user is deleted in wire but not in the IdP, @@ -209,7 +219,7 @@ createSamlUserWithId :: UserId -> SAML.UserRef -> ManagedBy -> Spar () createSamlUserWithId buid suid managedBy = do teamid <- (^. idpExtraInfo . wiTeam) <$> getIdPConfigByIssuer (suid ^. uidTenant) uname <- either (throwSpar . SparBadUserName . cs) pure $ Intra.mkUserName Nothing (UrefOnly suid) - buid' <- Intra.createBrigUser (UrefOnly suid) buid teamid uname managedBy + buid' <- Intra.createBrigUserSAML suid buid teamid uname managedBy assert (buid == buid') $ pure () insertUser suid buid @@ -232,44 +242,53 @@ autoprovisionSamlUserWithId buid suid managedBy = do if null scimtoks then do createSamlUserWithId buid suid managedBy - validateEmailIfExists buid (UrefOnly suid) + validateEmailIfExists buid suid else throwError . SAML.Forbidden $ "bad credentials (note that your team uses SCIM, " <> "which disables saml auto-provisioning)" --- | If (a) user's 'NameID' is an email address and the team has email validation for SSO --- enabled, or (b) user's SCIM externalId is an email address and there is no SAML involved: +-- | If user's 'NameID' is an email address and the team has email validation for SSO enabled, -- make brig initiate the email validate procedure. -validateEmailIfExists :: UserId -> ValidExternalId -> Spar () -validateEmailIfExists uid = - runValidExternalId - ( \case - (SAML.UserRef _ (view SAML.nameID -> UNameIDEmail email)) -> doValidate False email - _ -> pure () - ) - (doValidate True . Intra.emailToSAML) +validateEmailIfExists :: UserId -> SAML.UserRef -> Spar () +validateEmailIfExists uid = \case + (SAML.UserRef _ (view SAML.nameID -> UNameIDEmail email)) -> doValidate email + _ -> pure () where - doValidate :: Bool -> SAML.Email -> Spar () - doValidate always email = do + doValidate :: SAML.Email -> Spar () + doValidate email = do enabled <- do - tid <- Intra.getBrigUserTeam uid + tid <- Intra.getBrigUserTeam Intra.NoPendingInvitations uid maybe (pure False) Intra.isEmailValidationEnabledTeam tid - case enabled || always of - True -> Intra.updateEmail uid (Intra.emailFromSAML email) - False -> pure () + when enabled $ do + Intra.updateEmail uid (Intra.emailFromSAML email) --- | Check if 'UserId' is in the team that hosts the idp that owns the 'UserRef'. If so, write the --- 'UserRef' into the 'UserIdentity'. Otherwise, throw an error. +-- | Check if 'UserId' is in the team that hosts the idp that owns the 'UserRef'. If so, +-- register a the user under its SAML credentials and write the 'UserRef' into the +-- 'UserIdentity'. Otherwise, throw an error. +-- +-- Before returning, change account status or fail if account is nto active or pending an +-- invitation. bindUser :: UserId -> SAML.UserRef -> Spar UserId bindUser buid userref = do - do - teamid <- (^. idpExtraInfo . wiTeam) <$> getIdPConfigByIssuer (userref ^. uidTenant) - mteamid' <- Intra.getBrigUserTeam buid - unless (mteamid' == Just teamid) $ do - throwSpar . SparBindFromWrongOrNoTeam . cs . show $ buid + oldStatus <- do + let err :: Spar a + err = throwSpar . SparBindFromWrongOrNoTeam . cs . show $ buid + teamid :: TeamId <- (^. idpExtraInfo . wiTeam) <$> getIdPConfigByIssuer (userref ^. uidTenant) + acc <- Intra.getBrigUserAccount Intra.WithPendingInvitations buid >>= maybe err pure + teamid' :: TeamId <- userTeam (accountUser acc) & maybe err pure + unless (teamid' == teamid) err + pure (accountStatus acc) insertUser userref buid - buid <$ Intra.setBrigUserVeid buid (UrefOnly userref) + buid <$ do + Intra.setBrigUserVeid buid (UrefOnly userref) + let err = throwSpar . SparBindFromBadAccountStatus . cs . show + case oldStatus of + Active -> pure () + Suspended -> err oldStatus + Deleted -> err oldStatus + Ephemeral -> err oldStatus + PendingInvitation -> Intra.setStatus buid Active instance SPHandler SparError Spar where type NTCTX Spar = Env diff --git a/services/spar/src/Spar/Error.hs b/services/spar/src/Spar/Error.hs index 2074f29407b..6be5c1bda4b 100644 --- a/services/spar/src/Spar/Error.hs +++ b/services/spar/src/Spar/Error.hs @@ -75,6 +75,7 @@ data SparCustomError | SparCouldNotSubstituteFailureURI LT | SparBadInitiateLoginQueryParams LT | SparBindFromWrongOrNoTeam LT + | SparBindFromBadAccountStatus LT | SparBindUserRefTaken | SparBadUserName LT | SparCannotCreateUsersOnReplacedIdP LT @@ -136,6 +137,7 @@ renderSparError (SAML.CustomError (SparCouldNotSubstituteSuccessURI msg)) = Righ renderSparError (SAML.CustomError (SparCouldNotSubstituteFailureURI msg)) = Right $ Wai.Error status400 "bad-failure-redirect" ("re-parsing the substituted URI failed: " <> msg) renderSparError (SAML.CustomError (SparBadInitiateLoginQueryParams label)) = Right $ Wai.Error status400 label label renderSparError (SAML.CustomError (SparBindFromWrongOrNoTeam msg)) = Right $ Wai.Error status403 "bad-team" ("Forbidden: wrong user team " <> msg) +renderSparError (SAML.CustomError (SparBindFromBadAccountStatus msg)) = Right $ Wai.Error status403 "bad-account-status" ("Forbidden: user has account status " <> msg <> "; only Active, PendingInvitation are supported") renderSparError (SAML.CustomError SparBindUserRefTaken) = Right $ Wai.Error status403 "subject-id-taken" "Forbidden: SubjectID is used by another wire user. If you have an old user bound to this IdP, unbind or delete that user." renderSparError (SAML.CustomError (SparBadUserName msg)) = Right $ Wai.Error status400 "bad-username" ("Bad UserName in SAML response, except len [1, 128]: " <> msg) renderSparError (SAML.CustomError (SparCannotCreateUsersOnReplacedIdP replacingIdPId)) = Right $ Wai.Error status400 "cannont-provision-on-replaced-idp" ("This IdP has been replaced, users can only be auto-provisioned on the replacing IdP " <> replacingIdPId) diff --git a/services/spar/src/Spar/Intra/Brig.hs b/services/spar/src/Spar/Intra/Brig.hs index 46648b0c3f7..b17e56c69a4 100644 --- a/services/spar/src/Spar/Intra/Brig.hs +++ b/services/spar/src/Spar/Intra/Brig.hs @@ -31,8 +31,9 @@ module Spar.Intra.Brig emailToSAML, emailToSAMLNameID, emailFromSAMLNameID, - getBrigUser, getBrigUserAccount, + HavePendingInvitations (..), + getBrigUser, getBrigUserTeam, getBrigUserByHandle, getBrigUserByEmail, @@ -44,7 +45,8 @@ module Spar.Intra.Brig setBrigUserRichInfo, checkHandleAvailable, deleteBrigUser, - createBrigUser, + createBrigUserSAML, + createBrigUserNoSAML, updateEmail, getZUsrOwnedTeam, ensureReAuthorised, @@ -86,10 +88,10 @@ import Wire.API.User.RichInfo as RichInfo ---------------------------------------------------------------------- veidToUserSSOId :: ValidExternalId -> UserSSOId -veidToUserSSOId = - runValidExternalId - (\(SAML.UserRef t s) -> UserSSOId (cs $ SAML.encodeElem t) (cs $ SAML.encodeElem s)) - (UserScimExternalId . fromEmail) +veidToUserSSOId = runValidExternalId urefToUserSSOId (UserScimExternalId . fromEmail) + +urefToUserSSOId :: SAML.UserRef -> UserSSOId +urefToUserSSOId (SAML.UserRef t s) = UserSSOId (cs $ SAML.encodeElem t) (cs $ SAML.encodeElem s) veidFromUserSSOId :: MonadError String m => UserSSOId -> m ValidExternalId veidFromUserSSOId = \case @@ -197,11 +199,9 @@ class MonadError SparError m => MonadSparToBrig m where instance MonadSparToBrig m => MonadSparToBrig (ReaderT r m) where call = lift . call --- | Create a user on brig. -createBrigUser :: +createBrigUserSAML :: (HasCallStack, MonadSparToBrig m) => - -- | SSO identity - ValidExternalId -> + SAML.UserRef -> UserId -> TeamId -> -- | User name @@ -209,13 +209,13 @@ createBrigUser :: -- | Who should have control over the user ManagedBy -> m UserId -createBrigUser veid (Id buid) teamid uname managedBy = do +createBrigUserSAML uref (Id buid) teamid uname managedBy = do let newUser :: NewUser newUser = (emptyNewUser uname) { newUserUUID = Just buid, - newUserIdentity = Just $ SSOIdentity (veidToUserSSOId veid) Nothing Nothing, - newUserOrigin = Just . NewUserOriginTeamUser . NewTeamMemberSSO $ teamid, + newUserIdentity = Just (SSOIdentity (urefToUserSSOId uref) Nothing Nothing), + newUserOrigin = Just (NewUserOriginTeamUser . NewTeamMemberSSO $ teamid), newUserManagedBy = Just managedBy } resp :: Response (Maybe LBS) <- @@ -227,6 +227,25 @@ createBrigUser veid (Id buid) teamid uname managedBy = do then userId . selfUser <$> parseResponse @SelfProfile resp else rethrow resp +createBrigUserNoSAML :: + (HasCallStack, MonadSparToBrig m) => + Email -> + TeamId -> + -- | User name + Name -> + m UserId +createBrigUserNoSAML email teamid uname = do + let newUser = NewUserScimInvitation teamid Nothing uname email + resp :: Response (Maybe LBS) <- + call $ + method POST + . paths ["/i/teams", toByteString' teamid, "invitations"] + . json newUser + + if statusCode resp `elem` [200, 201] + then userId . accountUser <$> parseResponse @UserAccount resp + else rethrow resp + updateEmail :: (HasCallStack, MonadSparToBrig m) => UserId -> Email -> m () updateEmail buid email = do resp <- @@ -243,17 +262,26 @@ updateEmail buid email = do -- Wai.Error, it's ok to crash with a 500 here, so we use the unsafe parser. _ -> throwError . SAML.CustomServant . waiToServant . responseJsonUnsafe $ resp -getBrigUser :: (HasCallStack, MonadSparToBrig m) => UserId -> m (Maybe User) -getBrigUser = (accountUser <$$>) . getBrigUserAccount +getBrigUser :: (HasCallStack, MonadSparToBrig m) => HavePendingInvitations -> UserId -> m (Maybe User) +getBrigUser ifpend = (accountUser <$$>) . getBrigUserAccount ifpend -- | Get a user; returns 'Nothing' if the user was not found or has been deleted. -getBrigUserAccount :: (HasCallStack, MonadSparToBrig m) => UserId -> m (Maybe UserAccount) -getBrigUserAccount buid = do +getBrigUserAccount :: (HasCallStack, MonadSparToBrig m) => HavePendingInvitations -> UserId -> m (Maybe UserAccount) +getBrigUserAccount havePending buid = do resp :: ResponseLBS <- call $ method GET . paths ["/i/users"] - . query [("ids", Just $ toByteString' buid)] + . query + [ ("ids", Just $ toByteString' buid), + ( "includePendingInvitations", + Just . toByteString' $ + case havePending of + WithPendingInvitations -> True + NoPendingInvitations -> False + ) + ] + case statusCode resp of 200 -> do parseResponse @[UserAccount] resp >>= \case @@ -277,6 +305,7 @@ getBrigUserByHandle handle = do method GET . path "/i/users" . queryItem "handles" (toByteString' handle) + . queryItem "includePendingInvitations" "true" case statusCode resp of 200 -> listToMaybe <$> parseResponse @[UserAccount] resp _ -> rethrow resp @@ -288,6 +317,7 @@ getBrigUserByEmail email = do method GET . path "/i/users" . queryItem "email" (toByteString' email) + . queryItem "includePendingInvitations" "true" case statusCode resp of 200 -> do macc <- listToMaybe <$> parseResponse @[UserAccount] resp @@ -424,20 +454,20 @@ deleteBrigUser buid = do | otherwise -> throwSpar $ SparBrigError ("delete user failed with status " <> cs (show sCode)) --- | Check that a user id exists on brig and has a team id. -getBrigUserTeam :: (HasCallStack, MonadSparToBrig m) => UserId -> m (Maybe TeamId) -getBrigUserTeam = fmap (userTeam =<<) . getBrigUser +-- | Check that an id maps to an user on brig that is 'Active' (or optionally +-- 'PendingInvitation') and has a team id. +getBrigUserTeam :: (HasCallStack, MonadSparToBrig m) => HavePendingInvitations -> UserId -> m (Maybe TeamId) +getBrigUserTeam ifpend = fmap (userTeam =<<) . getBrigUser ifpend --- | Get the team that the user is an owner of. --- --- Called by post handler, and by 'authorizeIdP'. +-- | Get the team that the user is an owner of. This is used for authorization. It will fail +-- if the user is not in status 'Active'. getZUsrOwnedTeam :: (HasCallStack, SAML.SP m, MonadSparToBrig m, MonadSparToGalley m) => Maybe UserId -> m TeamId getZUsrOwnedTeam Nothing = throwSpar SparMissingZUsr getZUsrOwnedTeam (Just uid) = do - getBrigUserTeam uid + getBrigUserTeam NoPendingInvitations uid >>= maybe (throwSpar SparNotInTeam) (\teamid -> teamid <$ Galley.assertIsTeamOwner teamid uid) diff --git a/services/spar/src/Spar/Scim/Types.hs b/services/spar/src/Spar/Scim/Types.hs index 0f1e565286d..57ed84604a8 100644 --- a/services/spar/src/Spar/Scim/Types.hs +++ b/services/spar/src/Spar/Scim/Types.hs @@ -42,7 +42,7 @@ module Spar.Scim.Types where import Brig.Types.Common (Email) -import Brig.Types.Intra (AccountStatus (Active, Deleted, Ephemeral, Suspended)) +import Brig.Types.Intra (AccountStatus (..)) import qualified Brig.Types.User as BT import Control.Lens (Prism', makeLenses, prism') import Control.Monad.Except (throwError) @@ -244,6 +244,7 @@ scimActiveFlagFromAccountStatus = \case Suspended -> False Deleted -> False Ephemeral -> True -- do not treat ephemeral users any different from active ones. + PendingInvitation -> False -- | The second argument is constructed from a (potentially missing) json object field, hence -- @Nothing@ has the same meaning as @Just True@. This way, we stay consistent between the @@ -254,9 +255,23 @@ scimActiveFlagFromAccountStatus = \case -- should change the types so that the 'Ephemeral' case can be ruled out by the compiler. scimActiveFlagToAccountStatus :: AccountStatus -> Maybe Bool -> AccountStatus scimActiveFlagToAccountStatus oldstatus = \case - Nothing -> if oldstatus == Ephemeral then Ephemeral else Active - Just True -> if oldstatus == Ephemeral then Ephemeral else Active - Just False -> Suspended + Nothing -> activate + Just True -> activate + Just False -> deactivate + where + deactivate = case oldstatus of + Active -> Suspended + Suspended -> Suspended + Deleted -> Deleted -- this shouldn't happen, but it's harmless if it does. + Ephemeral -> Ephemeral -- never suspend ephemeral users via scim. doesn't really make sense anyway. + PendingInvitation -> PendingInvitation + + activate = case oldstatus of + Active -> Active + Suspended -> Active + Deleted -> Deleted -- this shouldn't happen, but it's harmless if it does. + Ephemeral -> Ephemeral + PendingInvitation -> PendingInvitation -- (do not activate: see 'scimActiveFlagFromAccountStatus') ---------------------------------------------------------------------------- -- Request and response types diff --git a/services/spar/src/Spar/Scim/User.hs b/services/spar/src/Spar/Scim/User.hs index 1ed04e002dd..069b455b1ad 100644 --- a/services/spar/src/Spar/Scim/User.hs +++ b/services/spar/src/Spar/Scim/User.hs @@ -46,13 +46,13 @@ import Brig.Types.Intra (AccountStatus, UserAccount (accountStatus, accountUser) import Brig.Types.User (ManagedBy (..), Name (..), User (..)) import qualified Brig.Types.User as BT import qualified Control.Applicative as Applicative (empty) -import Control.Lens (view, (^.), (^?)) +import Control.Lens (view, (^.)) import Control.Monad.Except (MonadError, throwError) import Control.Monad.Trans.Maybe (MaybeT (MaybeT), runMaybeT) import Crypto.Hash (Digest, SHA256, hashlazy) import qualified Data.Aeson as Aeson import Data.Handle (Handle (Handle), parseHandle) -import Data.Id (Id (Id), TeamId, UserId, idToText) +import Data.Id (Id (..), TeamId, UserId, idToText) import Data.Json.Util (UTCTimeMillis, fromUTCTimeMillis, toUTCTimeMillis) import Data.String.Conversions (cs) import qualified Data.Text as Text @@ -91,6 +91,7 @@ instance Scim.UserDB ST.SparTag Spar where getUsers _ Nothing = do throwError $ Scim.badRequest Scim.TooMany (Just "Please specify a filter when getting users.") getUsers ScimTokenInfo {stiTeam, stiIdP} (Just filter') = do + lift $ Log.debug (Log.msg $ "getUsers" <> show (stiTeam, stiIdP, filter')) mIdpConfig <- maybe (pure Nothing) (lift . wrapMonadClient . Data.getIdPConfig) stiIdP case filter' of Scim.FilterAttrCompare (Scim.AttrPath schema attrName _subAttr) Scim.OpEq (Scim.ValString val) @@ -110,7 +111,7 @@ instance Scim.UserDB ST.SparTag Spar where getUser ScimTokenInfo {stiTeam, stiIdP} uid = do mIdpConfig <- maybe (pure Nothing) (lift . wrapMonadClient . Data.getIdPConfig) stiIdP let notfound = Scim.notFound "User" (idToText uid) - brigUser <- lift (Brig.getBrigUserAccount uid) >>= maybe (throwError notfound) pure + brigUser <- lift (Brig.getBrigUserAccount Brig.WithPendingInvitations uid) >>= maybe (throwError notfound) pure unless (userTeam (accountUser brigUser) == Just stiTeam) (throwError notfound) case Brig.veidFromBrigUser (accountUser brigUser) ((^. SAML.idpMetadata . SAML.edIssuer) <$> mIdpConfig) of Right veid -> synthesizeStoredUser brigUser veid @@ -300,7 +301,7 @@ createValidScimUser :: ScimTokenInfo -> ST.ValidScimUser -> m (Scim.StoredUser ST.SparTag) -createValidScimUser ScimTokenInfo {stiTeam} vsu@(ST.ValidScimUser veid handl mbName richInfo active) = do +createValidScimUser ScimTokenInfo {stiTeam} (ST.ValidScimUser veid handl name richInfo _) = do -- ensure uniqueness constraints of all affected identifiers. -- {if we crash now, retry POST will just work} assertExternalIdUnused veid @@ -308,23 +309,46 @@ createValidScimUser ScimTokenInfo {stiTeam} vsu@(ST.ValidScimUser veid handl mbN -- {if we crash now, retry POST will just work, or user gets told the handle -- is already in use and stops POSTing} - buid <- lift $ do - -- Generate a UserId will be used both for scim user in spar and for brig. - buid <- Id <$> liftIO UUID.nextRandom - _ <- Brig.createBrigUser veid buid stiTeam mbName ManagedByScim - -- {If we crash now, we have an active user that cannot login. And can not - -- be bound this will be a zombie user that needs to be manually cleaned - -- up. We should consider making setUserHandle part of createUser and - -- making it transactional. If the user redoes the POST A new standalone - -- user will be created.} - Brig.setBrigUserHandle buid handl - Brig.setBrigUserRichInfo buid richInfo - pure buid + -- Generate a UserId will be used both for scim user in spar and for brig. + buid <- + lift $ do + buid <- + ST.runValidExternalId + ( \uref -> + do + uid <- liftIO $ Id <$> UUID.nextRandom + Brig.createBrigUserSAML uref uid stiTeam name ManagedByScim + ) + ( \email -> do + Brig.createBrigUserNoSAML email stiTeam name + ) + veid + + Log.debug (Log.msg $ "createValidScimUser: brig says " <> show buid) + + -- {If we crash now, we have an active user that cannot login. And can not + -- be bound this will be a zombie user that needs to be manually cleaned + -- up. We should consider making setUserHandle part of createUser and + -- making it transactional. If the user redoes the POST A new standalone + -- user will be created.} + Brig.setBrigUserHandle buid handl + Brig.setBrigUserRichInfo buid richInfo + pure buid + -- {If we crash now, a POST retry will fail with 409 user already exists. -- Azure at some point will retry with GET /Users?filter=userName eq handle -- and then issue a PATCH containing the rich info and the externalId.} - storedUser <- lift . toScimStoredUser buid $ synthesizeScimUser vsu + -- By now, vsu that was passed to 'createValidScimUser' may be outdated. Eg., if user is + -- invited via scim, we have @active == True@ above, but brig has stored the account in + -- @AccountStatus == PendingActivation@, which translates to @active == False@. So we need + -- to reload the Account from brig. + storedUser <- do + acc <- + lift (Brig.getBrigUserAccount Brig.WithPendingInvitations buid) + >>= maybe (throwError $ Scim.serverError "Server error: user vanished") pure + synthesizeStoredUser acc veid + lift $ Log.debug (Log.msg $ "createValidScimUser: spar says " <> show storedUser) -- {(arianvp): these two actions we probably want to make transactional.} lift . wrapMonadClient $ do @@ -336,12 +360,13 @@ createValidScimUser ScimTokenInfo {stiTeam} vsu@(ST.ValidScimUser veid handl mbN veid -- If applicable, trigger email validation procedure on brig. - lift $ validateEmailIfExists buid veid + lift $ ST.runValidExternalId (validateEmailIfExists buid) (\_ -> pure ()) veid -- {suspension via scim: if we don't reach the following line, the user will be active.} lift $ do old <- Brig.getStatus buid - let new = ST.scimActiveFlagToAccountStatus old (Just active) + let new = ST.scimActiveFlagToAccountStatus old active + active = Scim.active . Scim.value . Scim.thing $ storedUser when (new /= old) $ Brig.setStatus buid new pure storedUser @@ -401,8 +426,10 @@ updateVsuUref :: ST.ValidExternalId -> Spar () updateVsuUref uid old new = do - when (old ^? ST.veidEmail /= new ^? ST.veidEmail) $ do - validateEmailIfExists uid new + let geturef = ST.runValidExternalId Just (const Nothing) + case (geturef old, geturef new) of + (mo, mn@(Just newuref)) | mo /= mn -> validateEmailIfExists uid newuref + _ -> pure () wrapMonadClient $ do old & ST.runValidExternalId Data.deleteSAMLUser Data.deleteScimExternalId @@ -410,16 +437,6 @@ updateVsuUref uid old new = do Brig.setBrigUserVeid uid new -toScimStoredUser :: - UserId -> - Scim.User ST.SparTag -> - Spar (Scim.StoredUser ST.SparTag) -toScimStoredUser uid usr = do - SAML.Time (toUTCTimeMillis -> now) <- SAML.getNow - (createdAt, lastUpdatedAt) <- fromMaybe (now, now) <$> wrapMonadClient (Data.readScimUserTimes uid) - baseuri <- asks $ derivedOptsScimBaseURI . derivedOpts . sparCtxOpts - pure $ toScimStoredUser' createdAt lastUpdatedAt baseuri uid usr - toScimStoredUser' :: HasCallStack => UTCTimeMillis -> @@ -477,7 +494,7 @@ updScimStoredUser' now usr (Scim.WithMeta meta (Scim.WithId scimuid _)) = deleteScimUser :: ScimTokenInfo -> UserId -> Scim.ScimHandler Spar () deleteScimUser ScimTokenInfo {stiTeam} uid = do - mbBrigUser <- lift (Brig.getBrigUser uid) + mbBrigUser <- lift (Brig.getBrigUser Brig.WithPendingInvitations uid) case mbBrigUser of Nothing -> do -- double-deletion gets you a 404. @@ -562,7 +579,7 @@ assertHandleUnused' msg hndl = assertHandleNotUsedElsewhere :: UserId -> Handle -> Scim.ScimHandler Spar () assertHandleNotUsedElsewhere uid hndl = do - musr <- lift $ Brig.getBrigUser uid + musr <- lift $ Brig.getBrigUser Brig.WithPendingInvitations uid unless ((userHandle =<< musr) == Just hndl) $ assertHandleUnused' "userName does not match UserId" hndl @@ -666,7 +683,7 @@ scimFindUserByEmail :: Maybe IdP -> TeamId -> Text -> MaybeT (Scim.ScimHandler S scimFindUserByEmail mIdpConfig stiTeam email = do veid <- mkValidExternalId mIdpConfig (pure email) uid <- MaybeT . lift $ ST.runValidExternalId withUref withEmailOnly veid - brigUser <- MaybeT . lift . Brig.getBrigUserAccount $ uid + brigUser <- MaybeT . lift . Brig.getBrigUserAccount Brig.WithPendingInvitations $ uid guard $ userTeam (accountUser brigUser) == Just stiTeam lift $ synthesizeStoredUser brigUser veid where diff --git a/services/spar/test-integration/Test/Spar/Intra/BrigSpec.hs b/services/spar/test-integration/Test/Spar/Intra/BrigSpec.hs index ff729f7b427..e6ed4aac923 100644 --- a/services/spar/test-integration/Test/Spar/Intra/BrigSpec.hs +++ b/services/spar/test-integration/Test/Spar/Intra/BrigSpec.hs @@ -39,7 +39,7 @@ spec = do describe "getBrigUser" $ do it "return Nothing if n/a" $ do - musr <- runSpar $ Intra.getBrigUser (Id . fromJust $ UUID.fromText "29546d9e-ed5b-11ea-8228-c324b1ea1030") + musr <- runSpar $ Intra.getBrigUser Intra.WithPendingInvitations (Id . fromJust $ UUID.fromText "29546d9e-ed5b-11ea-8228-c324b1ea1030") liftIO $ musr `shouldSatisfy` isNothing it "return Just if /a" $ do @@ -52,5 +52,5 @@ spec = do scimUserId <$> createUser tok scimUser uid <- setup - musr <- runSpar $ Intra.getBrigUser uid + musr <- runSpar $ Intra.getBrigUser Intra.WithPendingInvitations uid liftIO $ musr `shouldSatisfy` isJust diff --git a/services/spar/test-integration/Test/Spar/Scim/UserSpec.hs b/services/spar/test-integration/Test/Spar/Scim/UserSpec.hs index b1d08c3e904..5a6bb7996c6 100644 --- a/services/spar/test-integration/Test/Spar/Scim/UserSpec.hs +++ b/services/spar/test-integration/Test/Spar/Scim/UserSpec.hs @@ -1,3 +1,4 @@ +{-# LANGUAGE NumericUnderscores #-} {-# LANGUAGE OverloadedStrings #-} {-# LANGUAGE QuasiQuotes #-} {-# LANGUAGE ScopedTypeVariables #-} @@ -29,8 +30,9 @@ where import Bilge import Bilge.Assert -import Brig.Types.Intra (AccountStatus (Active, Suspended)) +import Brig.Types.Intra (AccountStatus (Active, PendingInvitation, Suspended), accountStatus, accountUser) import Brig.Types.User as Brig +import qualified Control.Exception import Control.Lens import Control.Monad.Trans.Except import Control.Monad.Trans.Maybe @@ -43,6 +45,7 @@ import Data.Handle (Handle (Handle), fromHandle) import Data.Id (TeamId, UserId, randomId) import Data.Ix (inRange) import Data.String.Conversions (cs) +import Data.Text.Encoding (encodeUtf8) import Imports import qualified SAML2.WebSSO.Test.MockResponse as SAML import qualified SAML2.WebSSO.Types as SAML @@ -54,6 +57,7 @@ import Spar.Types (IdP) import qualified Spar.Types import qualified Text.XML.DSig as SAML import Util +import Util.Invitation (getInvitation, getInvitationCode, headInvitation404, registerInvitation) import qualified Web.Scim.Class.User as Scim.UserC import qualified Web.Scim.Filter as Filter import qualified Web.Scim.Schema.Common as Scim @@ -61,7 +65,10 @@ import qualified Web.Scim.Schema.Meta as Scim import qualified Web.Scim.Schema.PatchOp as PatchOp import qualified Web.Scim.Schema.User as Scim.User import qualified Wire.API.Team.Feature as Feature +import Wire.API.Team.Invitation (Invitation (..)) import Wire.API.User.RichInfo +import Wire.API.User.Search (SearchResult (..)) +import qualified Wire.API.User.Search as Search -- | Tests for @\/scim\/v2\/Users@. spec :: SpecWith TestEnv @@ -178,10 +185,12 @@ specCreateUser = describe "POST /Users" $ do it "rejects attempts at setting a password" $ do testCreateUserWithPass context "team has no SAML IdP" $ do - it "creates an active user without an email, and triggers email validation" $ do + it "creates a user with PendingInvitation, and user can follow usual invitation process" $ do testCreateUserNoIdP it "fails if no email can be extraced from externalId" $ do testCreateUserNoIdPNoEmail + it "doesn't list users that exceed their invivtation period, and allows recreating them" $ do + testCreateUserTimeout context "team has one SAML IdP" $ do it "creates a user in an existing team" $ do testCreateUserWithSamlIdP @@ -216,44 +225,103 @@ testCreateUserWithPass = do testCreateUserNoIdP :: TestSpar () testCreateUserNoIdP = do env <- ask + let brig = env ^. teBrig email <- randomEmail scimUser <- randomScimUser <&> \u -> u {Scim.User.externalId = Just $ fromEmail email} - (_, tid) <- call $ createUserWithTeam (env ^. teBrig) (env ^. teGalley) + (owner, tid) <- call $ createUserWithTeam (env ^. teBrig) (env ^. teGalley) tok <- registerScimToken tid Nothing scimStoredUser <- createUser tok scimUser + liftIO $ (Scim.User.active . Scim.value . Scim.thing $ scimStoredUser) `shouldBe` Just False let userid = scimUserId scimStoredUser - handle = Handle . Scim.User.userName . Scim.value . Scim.thing $ scimStoredUser - brigUser <- - aFewTimes (runSpar $ Intra.getBrigUser userid) isJust - >>= maybe (error "could not find user in brig") pure - brigUser `userShouldMatch` WrappedScimStoredUser scimStoredUser - liftIO $ userEmail brigUser `shouldBe` Nothing - accStatus <- runSpar $ Intra.getStatusMaybe userid - liftIO $ accStatus `shouldBe` Just Active - liftIO $ userManagedBy brigUser `shouldBe` ManagedByScim + handle = Handle $ Scim.User.userName scimUser + userName = Name . fromJust . Scim.User.displayName $ scimUser + + -- get account from brig, status should be PendingInvitation + do + aFewTimes (runSpar $ Intra.getBrigUserAccount Intra.NoPendingInvitations userid) isJust + >>= maybe (pure ()) (error "pending user in brig is visible, even though it should not be") + brigUserAccount <- + aFewTimes (runSpar $ Intra.getBrigUserAccount Intra.WithPendingInvitations userid) isJust + >>= maybe (error "could not find user in brig") pure + let brigUser = accountUser brigUserAccount + brigUser `userShouldMatch` WrappedScimStoredUser scimStoredUser + liftIO $ accountStatus brigUserAccount `shouldBe` PendingInvitation + liftIO $ userEmail brigUser `shouldBe` Just email + liftIO $ userManagedBy brigUser `shouldBe` ManagedByScim + + -- searching user in brig should fail + -- >>> searchUser brig owner userName False + -- Note: Cannot run this test here, because its delay would cause the invitation to + -- time out. + + -- scim-get should produce same stored user; stored user should be inactive and have an + -- email. + do + susr <- getUser tok userid + liftIO $ susr `shouldBe` scimStoredUser + let usr = Scim.value . Scim.thing $ susr + liftIO $ Scim.User.active usr `shouldBe` Just False + liftIO $ Scim.User.externalId usr `shouldBe` Just (fromEmail email) + + -- scim search should succeed + do + listUsers tok (Just (filterBy "userName" $ fromHandle handle)) >>= \users -> + liftIO $ users `shouldBe` [scimStoredUser] + listUsers tok (Just (filterBy "externalId" $ fromEmail email)) >>= \users -> + liftIO $ users `shouldBe` [scimStoredUser] + + -- user should be able to follow old team invitation flow + do + inv <- call $ getInvitation brig email + Just inviteeCode <- call $ getInvitationCode brig tid (inInvitation inv) + registerInvitation email userName inviteeCode True + call $ headInvitation404 brig email - let checkGet :: TestSpar () - checkGet = do - susr <- getUser tok userid - WrappedScimStoredUser susr `userShouldMatch` WrappedScimStoredUser scimStoredUser - let usr = Scim.value . Scim.thing $ susr - liftIO $ Scim.User.active usr `shouldBe` Just True - liftIO $ Scim.User.externalId usr `shouldBe` Just (fromEmail email) - - checkSearch :: TestSpar () - checkSearch = do - listUsers tok (Just (filterBy "userName" $ fromHandle handle)) >>= \users -> - liftIO $ users `shouldBe` [scimStoredUser] - listUsers tok (Just (filterBy "externalId" $ fromEmail email)) >>= \users -> - liftIO $ users `shouldBe` [scimStoredUser] - - checkGet - checkSearch - call $ do - activateEmail (env ^. teBrig) email - checkEmail (env ^. teBrig) userid email - checkGet - checkSearch + -- user should now be active + do + brigUser <- + aFewTimes (runSpar $ Intra.getBrigUserAccount Intra.NoPendingInvitations userid) isJust + >>= maybe (error "could not find user in brig") pure + liftIO $ accountStatus brigUser `shouldBe` Active + liftIO $ userManagedBy (accountUser brigUser) `shouldBe` ManagedByScim + + susr <- getUser tok userid + let usr = Scim.value . Scim.thing $ susr + liftIO $ Scim.User.active usr `shouldNotBe` Just False + + -- searching user in brig should succeed + searchUser brig owner userName True + where + -- cloned from brig's integration tests + + searchUser :: HasCallStack => BrigReq -> UserId -> Name -> Bool -> TestSpar () + searchUser brig searcherId searchTarget shouldSucceed = do + refreshIndex brig + aFewTimesAssert + ( do + let searchQuery = fromName searchTarget + resp <- call $ executeSearch brig searcherId searchQuery + pure $ searchFound resp + ) + (if shouldSucceed then (> 0) else (== 0)) + + refreshIndex :: BrigReq -> TestSpar () + refreshIndex brig = do + call $ void $ post (brig . path "/i/index/reindex" . expect2xx) + -- wait for async reindexing to complete (hopefully) + lift $ threadDelay 3_000_000 + + executeSearch :: BrigReq -> UserId -> Text -> Http (Search.SearchResult Search.Contact) + executeSearch brig self q = do + r <- + get + ( brig + . path "/search/contacts" + . zUser self + . queryItem "q" (encodeUtf8 q) + . expect2xx + ) + responseJsonError r testCreateUserNoIdPNoEmail :: TestSpar () testCreateUserNoIdPNoEmail = do @@ -555,6 +623,63 @@ testScimCreateVsUserRef = do (call . delete $ brig . paths ["i", "users", toByteString' uid]) !!! const 202 === statusCode +testCreateUserTimeout :: TestSpar () +testCreateUserTimeout = do + env <- ask + + (_owner, tid) <- call $ createUserWithTeam (env ^. teBrig) (env ^. teGalley) + tok <- registerScimToken tid Nothing + + email <- randomEmail + scimUser <- randomScimUser <&> \u -> u {Scim.User.externalId = Just $ fromEmail email} + let userName = Name . fromJust . Scim.User.displayName $ scimUser + + (scimStoredUser1, _inv, inviteeCode) <- createUser'step tok tid scimUser email + + waitUserExpiration + searchUser tok scimUser email False + registerInvitation email userName inviteeCode False + searchUser tok scimUser email False + + (scimStoredUser2, _inv, inviteeCode2) <- createUser'step tok tid scimUser email + + let id1 = (Scim.id . Scim.thing) scimStoredUser1 + let id2 = (Scim.id . Scim.thing) scimStoredUser2 + liftIO $ id1 `shouldNotBe` id2 + + registerInvitation email userName inviteeCode2 True + searchUser tok scimUser email True + waitUserExpiration + searchUser tok scimUser email True + where + createUser'step tok tid scimUser email = do + env <- ask + let brig = env ^. teBrig + + scimStoredUser <- aFewTimesRecover (createUser tok scimUser) + inv <- call $ getInvitation brig email + Just inviteeCode <- call $ getInvitationCode brig tid (inInvitation inv) + pure (scimStoredUser, inv, inviteeCode) + + searchUser :: HasCallStack => Spar.Types.ScimToken -> Scim.User.User tag -> Email -> Bool -> TestSpar () + searchUser tok scimUser email shouldSucceed = do + let handle = Handle . Scim.User.userName $ scimUser + tryquery qry = + aFewTimesAssert + (length <$> listUsers tok (Just qry)) + (if shouldSucceed then (> 0) else (== 0)) + + tryquery (filterBy "userName" $ fromHandle handle) + tryquery (filterBy "externalId" $ fromEmail email) + + waitUserExpiration = do + -- this should be something like @round . Brig.Options.setTeamInvitationTimeout . Brig.Options.optSettings . + -- view teBrigOpts $ env@, but if this goes out of sync with the brig config, we will only get benign false + -- negatives, and importing brig options into spar integration tests is just too awkward. + let setTeamInvitationTimeout = 5 + Control.Exception.assert (setTeamInvitationTimeout < 30) $ do + threadDelay $ (setTeamInvitationTimeout + 1) * 1_000_000 + ---------------------------------------------------------------------------- -- Listing users @@ -606,7 +731,7 @@ testFindSamlAutoProvisionedUserMigratedWithEmailInTeamWithSSO = do -- auto-provision user via saml memberWithSSO <- do uid <- loginSsoUserFirstTime idp privCreds - Just usr <- runSpar $ Intra.getBrigUser uid + Just usr <- runSpar $ Intra.getBrigUser Intra.NoPendingInvitations uid handle <- nextHandle runSpar $ Intra.setBrigUserHandle uid handle pure usr @@ -619,7 +744,7 @@ testFindSamlAutoProvisionedUserMigratedWithEmailInTeamWithSSO = do liftIO $ userManagedBy memberWithSSO `shouldBe` ManagedByWire users <- listUsers tok (Just (filterBy "externalId" externalId)) liftIO $ (scimUserId <$> users) `shouldContain` [memberIdWithSSO] - Just brigUser' <- runSpar $ Intra.getBrigUser memberIdWithSSO + Just brigUser' <- runSpar $ Intra.getBrigUser Intra.NoPendingInvitations memberIdWithSSO liftIO $ userManagedBy brigUser' `shouldBe` ManagedByScim testFindTeamSettingsInvitedUserMigratedWithEmailInTeamWithSSO :: TestSpar () @@ -633,7 +758,7 @@ testFindTeamSettingsInvitedUserMigratedWithEmailInTeamWithSSO = do users' <- listUsers tok (Just (filterBy "externalId" emailInvited)) liftIO $ (scimUserId <$> users') `shouldContain` [memberIdInvited] - Just brigUserInvited' <- runSpar $ Intra.getBrigUser (memberIdInvited) + Just brigUserInvited' <- runSpar $ Intra.getBrigUser Intra.NoPendingInvitations memberIdInvited liftIO $ userManagedBy brigUserInvited' `shouldBe` ManagedByScim testFindTeamSettingsInvitedUserMigratedWithEmailInTeamWithSSOViaUserId :: TestSpar () @@ -645,7 +770,7 @@ testFindTeamSettingsInvitedUserMigratedWithEmailInTeamWithSSOViaUserId = do let memberIdInvited = userId memberInvited _ <- getUser tok memberIdInvited - Just brigUserInvited' <- runSpar $ Intra.getBrigUser (memberIdInvited) + Just brigUserInvited' <- runSpar $ Intra.getBrigUser Intra.NoPendingInvitations (memberIdInvited) liftIO $ userManagedBy brigUserInvited' `shouldBe` ManagedByScim testFindProvisionedUserNoIdP :: TestSpar () @@ -665,7 +790,7 @@ testFindNonProvisionedUserNoIdP findBy = do uid <- userId <$> call (inviteAndRegisterUser (env ^. teBrig) owner teamid) handle <- nextHandle runSpar $ Intra.setBrigUserHandle uid handle - Just brigUser <- runSpar $ Intra.getBrigUser uid + Just brigUser <- runSpar $ Intra.getBrigUser Intra.NoPendingInvitations uid let Just email = userEmail brigUser do @@ -680,7 +805,7 @@ testFindNonProvisionedUserNoIdP findBy = do do liftIO $ users `shouldBe` [uid] - Just brigUser' <- runSpar $ Intra.getBrigUser uid + Just brigUser' <- runSpar $ Intra.getBrigUser Intra.NoPendingInvitations uid liftIO $ userManagedBy brigUser' `shouldBe` ManagedByScim liftIO $ brigUser' `shouldBe` brigUser {userManagedBy = ManagedByScim} @@ -765,7 +890,7 @@ testGetUser = do shouldBeManagedBy :: HasCallStack => UserId -> ManagedBy -> TestSpar () shouldBeManagedBy uid flag = do - managedBy <- maybe (error "user not found") userManagedBy <$> runSpar (Intra.getBrigUser uid) + managedBy <- maybe (error "user not found") userManagedBy <$> runSpar (Intra.getBrigUser Intra.WithPendingInvitations uid) liftIO $ managedBy `shouldBe` flag -- | This is (roughly) the behavior on develop as well as on the branch where this test was @@ -818,12 +943,12 @@ testGetUserWithNoHandle = do uid <- loginSsoUserFirstTime idp privcreds tok <- registerScimToken tid (Just (idp ^. SAML.idpId)) - mhandle :: Maybe Handle <- maybe (error "user not found") userHandle <$> runSpar (Intra.getBrigUser uid) + mhandle :: Maybe Handle <- maybe (error "user not found") userHandle <$> runSpar (Intra.getBrigUser Intra.WithPendingInvitations uid) liftIO $ mhandle `shouldSatisfy` isNothing storedUser <- getUser tok uid liftIO $ (Scim.User.displayName . Scim.value . Scim.thing) storedUser `shouldSatisfy` isJust - mhandle' :: Maybe Handle <- aFewTimes (maybe (error "user not found") userHandle <$> runSpar (Intra.getBrigUser uid)) isJust + mhandle' :: Maybe Handle <- aFewTimes (maybe (error "user not found") userHandle <$> runSpar (Intra.getBrigUser Intra.WithPendingInvitations uid)) isJust liftIO $ mhandle' `shouldSatisfy` isJust liftIO $ (fromHandle <$> mhandle') `shouldBe` (Just . Scim.User.userName . Scim.value . Scim.thing $ storedUser) @@ -1121,7 +1246,7 @@ testBrigSideIsUpdated = do validScimUser <- either (error . show) pure $ validateScimUser' (Just idp) 999999 user' - brigUser <- maybe (error "no brig user") pure =<< runSpar (Intra.getBrigUser userid) + brigUser <- maybe (error "no brig user") pure =<< runSpar (Intra.getBrigUser Intra.WithPendingInvitations userid) brigUser `userShouldMatch` validScimUser ---------------------------------------------------------------------------- @@ -1303,7 +1428,7 @@ specDeleteUser = do storedUser <- createUser tok user let uid :: UserId = scimUserId storedUser uref :: SAML.UserRef <- do - usr <- runSpar $ Intra.getBrigUser uid + usr <- runSpar $ Intra.getBrigUser Intra.WithPendingInvitations uid let err = error . ("brig user without UserRef: " <>) . show case (`Intra.veidFromBrigUser` Nothing) <$> usr of bad@(Just (Right veid)) -> runValidExternalId pure (const $ err bad) veid @@ -1312,7 +1437,7 @@ specDeleteUser = do deleteUser_ (Just tok) (Just uid) spar !!! const 204 === statusCode brigUser :: Maybe User <- - aFewTimes (runSpar $ Intra.getBrigUser uid) isNothing + aFewTimes (runSpar $ Intra.getBrigUser Intra.WithPendingInvitations uid) isNothing samlUser :: Maybe UserId <- aFewTimes (getUserIdViaRef' uref) isNothing scimUser <- diff --git a/services/spar/test-integration/Util/Core.hs b/services/spar/test-integration/Util/Core.hs index 93daf8dbfe0..3f471fe849f 100644 --- a/services/spar/test-integration/Util/Core.hs +++ b/services/spar/test-integration/Util/Core.hs @@ -39,6 +39,8 @@ module Util.Core shouldRespondWith, module Test.Hspec, aFewTimes, + aFewTimesAssert, + aFewTimesRecover, -- * HTTP call, @@ -276,7 +278,8 @@ pending = liftIO Test.Hspec.pending pendingWith :: (HasCallStack, MonadIO m) => String -> m () pendingWith = liftIO . Test.Hspec.pendingWith --- | Run a probe several times, until a "good" value materializes or until patience runs out. +-- | Run a probe several times, until a "good" value materializes or until patience runs out +-- (after ~2secs). -- If all retries were unsuccessful, 'aFewTimes' will return the last obtained value, even -- if it does not satisfy the predicate. aFewTimes :: TestSpar a -> (a -> Bool) -> TestSpar a @@ -284,10 +287,23 @@ aFewTimes action good = do env <- ask liftIO $ retrying - (exponentialBackoff 1000 <> limitRetries 10) + (exponentialBackoff 1000 <> limitRetries 11) (\_ -> pure . not . good) (\_ -> action `runReaderT` env) +aFewTimesAssert :: HasCallStack => TestSpar a -> (a -> Bool) -> TestSpar () +aFewTimesAssert action good = do + result <- aFewTimes action good + good result `assert` pure () + +aFewTimesRecover :: TestSpar a -> TestSpar a +aFewTimesRecover action = do + env <- ask + liftIO $ + recoverAll + (exponentialBackoff 1000 <> limitRetries 10) + (\_ -> action `runReaderT` env) + -- | Duplicate of 'Spar.Intra.Brig.getBrigUser'. getUserBrig :: HasCallStack => UserId -> TestSpar (Maybe User) getUserBrig uid = do @@ -1153,7 +1169,7 @@ getSsoidViaSelf uid = maybe (error "not found") pure =<< getSsoidViaSelf' uid getSsoidViaSelf' :: HasCallStack => UserId -> TestSpar (Maybe UserSSOId) getSsoidViaSelf' uid = do - musr <- aFewTimes (runSpar $ Intra.getBrigUser uid) isJust + musr <- aFewTimes (runSpar $ Intra.getBrigUser Intra.NoPendingInvitations uid) isJust pure $ case userIdentity =<< musr of Just (SSOIdentity ssoid _ _) -> Just ssoid Just (FullIdentity _ _) -> Nothing diff --git a/services/spar/test-integration/Util/Invitation.hs b/services/spar/test-integration/Util/Invitation.hs new file mode 100644 index 00000000000..4253fdd1324 --- /dev/null +++ b/services/spar/test-integration/Util/Invitation.hs @@ -0,0 +1,90 @@ +-- This file is part of the Wire Server implementation. +-- +-- Copyright (C) 2020 Wire Swiss GmbH +-- +-- This program is free software: you can redistribute it and/or modify it under +-- the terms of the GNU Affero General Public License as published by the Free +-- Software Foundation, either version 3 of the License, or (at your option) any +-- later version. +-- +-- This program is distributed in the hope that it will be useful, but WITHOUT +-- ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS +-- FOR A PARTICULAR PURPOSE. See the GNU Affero General Public License for more +-- details. +-- +-- You should have received a copy of the GNU Affero General Public License along +-- with this program. If not, see . + +module Util.Invitation + ( headInvitation404, + getInvitation, + getInvitationCode, + registerInvitation, + ) +where + +import Bilge +import Bilge.Assert ((!!!), ( Email -> Http () +headInvitation404 brig email = do + Bilge.head (brig . path "/teams/invitations/by-email" . contentJson . queryItem "email" (toByteString' email)) + !!! const 404 === statusCode + +getInvitation :: BrigReq -> Email -> Http Invitation +getInvitation brig email = + responseJsonUnsafe + <$> Bilge.get + ( brig . path "/i/teams/invitations/by-email" . contentJson + . queryItem "email" (toByteString' email) + . expect2xx + ) + +getInvitationCode :: + (MonadIO m, MonadHttp m, HasCallStack) => + BrigReq -> + TeamId -> + InvitationId -> + m (Maybe InvitationCode) +getInvitationCode brig t ref = do + r <- + get + ( brig + . path "/i/teams/invitation-code" + . queryItem "team" (toByteString' t) + . queryItem "invitation_id" (toByteString' ref) + ) + let lbs = fromMaybe "" $ responseBody r + return $ fromByteString . fromMaybe (error "No code?") $ encodeUtf8 <$> (lbs ^? key "code" . _String) + +registerInvitation :: Email -> Name -> InvitationCode -> Bool -> TestSpar () +registerInvitation email name inviteeCode shouldSucceed = do + env <- ask + let brig = env ^. teBrig + call $ + void $ + post + ( brig . path "/register" + . contentJson + . json (acceptWithName name email inviteeCode) + ) + Email -> InvitationCode -> Aeson.Value +acceptWithName name email code = + Aeson.object + [ "name" Aeson..= fromName name, + "email" Aeson..= fromEmail email, + "password" Aeson..= defPassword, + "team_code" Aeson..= code + ] diff --git a/services/spar/test-integration/Util/Scim.hs b/services/spar/test-integration/Util/Scim.hs index 313e4d0af0f..10c3c0476c9 100644 --- a/services/spar/test-integration/Util/Scim.hs +++ b/services/spar/test-integration/Util/Scim.hs @@ -134,7 +134,7 @@ randomScimUserWithSubjectAndRichInfo richInfo = do _ -> error "randomScimUserWithSubject: impossible" pure ( (Scim.User.empty userSchemas ("scimuser_" <> suffix) (ScimUserExtra richInfo)) - { Scim.User.displayName = Just ("Scim User #" <> suffix), + { Scim.User.displayName = Just ("ScimUser" <> suffix), Scim.User.externalId = Just externalId, Scim.User.emails = emails, Scim.User.phoneNumbers = phones @@ -149,7 +149,7 @@ randomScimUserWithEmail = do externalId = fromEmail email pure ( (Scim.User.empty userSchemas ("scimuser_" <> suffix) (ScimUserExtra mempty)) - { Scim.User.displayName = Just ("Scim User #" <> suffix), + { Scim.User.displayName = Just ("ScimUser" <> suffix), Scim.User.externalId = Just externalId }, email diff --git a/tools/db/find-undead/src/Work.hs b/tools/db/find-undead/src/Work.hs index 70d1675aa79..8a08096c335 100644 --- a/tools/db/find-undead/src/Work.hs +++ b/tools/db/find-undead/src/Work.hs @@ -133,11 +133,13 @@ instance Cql AccountStatus where toCql Suspended = CqlInt 1 toCql Deleted = CqlInt 2 toCql Ephemeral = CqlInt 3 + toCql PendingInvitation = CqlInt 4 fromCql (CqlInt i) = case i of 0 -> return Active 1 -> return Suspended 2 -> return Deleted 3 -> return Ephemeral + 4 -> return PendingInvitation n -> Left $ "unexpected account status: " ++ show n fromCql _ = Left "account status: int expected"