Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SCIM onboarding flow again #1213

Merged
merged 132 commits into from
Oct 27, 2020
Merged
Show file tree
Hide file tree
Changes from 127 commits
Commits
Show all changes
132 commits
Select commit Hold shift + click to select a range
5d28a92
Add new AccountStatus constructor, rename all others.
fisx Sep 25, 2020
16bc18d
Revert the renaming (squash this after PR review).
fisx Sep 25, 2020
2511593
Resolve TODOs, fixups.
fisx Sep 25, 2020
151d261
When creating users via SCIM, use invitation logic.
fisx Sep 25, 2020
474627e
...
fisx Sep 25, 2020
e894bc9
Add CreateUserNoVerifyViaScim param newAccount and handler
smatting Oct 5, 2020
3c6a759
maybe of interest, but does not compile.
fisx Oct 6, 2020
58f4f94
Update integration test.
fisx Oct 6, 2020
8c8330d
...
fisx Oct 7, 2020
be8b56a
...
fisx Oct 7, 2020
29a25a8
Remove viaScim param; Add NewTeamMemberScimInvitation
smatting Oct 8, 2020
1ff06f8
... from discussion Mo afternoon, details below
smatting Oct 12, 2020
aabf6f2
Cleanup
fisx Oct 13, 2020
34cacb9
Undo change noise.
fisx Oct 13, 2020
ed6dce5
ormolu.
fisx Oct 13, 2020
1f718d6
Suggestions: no welcome mail + comment
smatting Oct 13, 2020
f29a2bd
Haddocks.
fisx Oct 13, 2020
e992bca
Make spar and brig compile (not sure if correct)
smatting Oct 13, 2020
973109b
Dont require password for while creating users via scim
smatting Oct 13, 2020
70a1153
Add TODO for test
smatting Oct 13, 2020
9fed1c4
Haddocks.
fisx Oct 13, 2020
9d283f5
Spar.Intra.Brig.getUser w/, & w/o PendingInvitations (call side).
fisx Oct 13, 2020
255c312
Fix compilation errors in integration test.
fisx Oct 13, 2020
df4ad39
Merge remote-tracking branch 'refs/remotes/origin/fisx/scim-onboardin…
fisx Oct 13, 2020
9967164
Fixup
fisx Oct 13, 2020
441d203
createUserNoVerify: dont activate if created via scim, send email
smatting Oct 14, 2020
f216892
Add param includePendingInvitations
smatting Oct 14, 2020
63e6af1
Revert LANGUAGE pragmas
smatting Oct 14, 2020
45ac8b8
Fix incorrect paramter passing
smatting Oct 14, 2020
6c6ebc3
Apply suggestions from @fisx
smatting Oct 14, 2020
8749a21
Add a copy of createUser dedicated to the new flow only.
fisx Oct 15, 2020
457f402
Add comment and TODO
jschaul Oct 15, 2020
0ef5ce3
Add createInvitationByBackend, refactor createInvitation
smatting Oct 15, 2020
2d815c2
Refactor createBrigUser -> createBrigUserSAML/SCIM
smatting Oct 15, 2020
5100c0f
Nit-pick.
fisx Oct 15, 2020
5ddacb3
Remove unused location url header in internal response of end-point.
fisx Oct 15, 2020
5fd60fb
Fix compiler warning.
fisx Oct 15, 2020
b43d937
Adhere more to (undocumented, sadly) naming conventions.
fisx Oct 15, 2020
dd2a97f
Simplify.
fisx Oct 15, 2020
325d4c2
Renames.
fisx Oct 15, 2020
b02dadb
Do not export helper we may end up not using.
fisx Oct 15, 2020
5d19b87
Refactor: reduce code duplication.
fisx Oct 16, 2020
777db9f
createUserInviteViaScim
fisx Oct 16, 2020
3f13301
createUserInviteViaScim
fisx Oct 16, 2020
3b3a5ca
Add integration test for users that exeed their invitation period
smatting Oct 16, 2020
9365794
createUserInviteViaScim
fisx Oct 16, 2020
0b35958
createUserInviteViaScim (argument type)
fisx Oct 16, 2020
a738265
Finish initernal brig end-point for invite-via-scim flow.
fisx Oct 16, 2020
1135b30
Renames.
fisx Oct 16, 2020
47a086b
Merge branch 'fisx/scim-onboarding-flow-again' of ssh://github.com/wi…
fisx Oct 16, 2020
04923f9
createBrigUserNoSAML: Call right endpoint
smatting Oct 16, 2020
dcb21fa
Make brig service options available in spar integration tests.
fisx Oct 16, 2020
835137d
Fix compiler warning.
fisx Oct 16, 2020
f6fe9f5
Remove NewTeamMemberScimInvitation and call sites
smatting Oct 16, 2020
c6fbc04
Add TTL for users created with scim, remove TTL on register
smatting Oct 16, 2020
fab55d9
Fix compiler warnings
smatting Oct 16, 2020
c3699c8
Fix broken arguments
smatting Oct 19, 2020
220a5b1
round @Timeout @Int returns seconds
smatting Oct 19, 2020
7396c39
createUserInviteVIScim: Insert user "activated", otherwise no id
smatting Oct 19, 2020
3755428
Fix hole.
fisx Oct 19, 2020
b1b3fea
Revert "createUserInviteVIScim: Insert user "activated", otherwise no…
fisx Oct 19, 2020
869a6ab
Revert "Add TTL for users created with scim, remove TTL on register"
fisx Oct 19, 2020
a01e372
Users with status PendingActivation must be activated.
fisx Oct 20, 2020
afc3ebd
Make test more strict.
fisx Oct 20, 2020
a3eef9f
Fix scim active user flag.
fisx Oct 20, 2020
a535413
Crank up debugging.
fisx Oct 20, 2020
f773e43
getBrigUser: Remove HavePendingInvitations and clean up scim users
smatting Oct 20, 2020
2af134b
Fix: honour brig's change of user data when creating via scim.
fisx Oct 20, 2020
e48bc9b
Merge branch 'fisx/scim-onboarding-flow-again' of ssh://github.com/wi…
fisx Oct 20, 2020
d51af36
Revert "remove HavePendingInvitations", update listAcitvatedAccounts
smatting Oct 20, 2020
ea8c95a
Add public /teams/invitations/by-email and refactor
smatting Oct 20, 2020
3dccb23
Update services/brig/src/Brig/API/Internal.hs
smatting Oct 20, 2020
7d9d40a
Update services/brig/src/Brig/API/Internal.hs
smatting Oct 20, 2020
9e573df
Replace guard with if
smatting Oct 20, 2020
3486fe9
Flesh out filterM refactor
smatting Oct 20, 2020
59da561
Recover lost (and correct) comment.
fisx Oct 21, 2020
eee28b4
Fix handler name.
fisx Oct 21, 2020
66fb589
Work on testCreateUserTimeout + lots of debugging
smatting Oct 21, 2020
87db55d
Shuffle around helper functions, eliminate redundancy.
fisx Oct 21, 2020
4bf4675
Fix name.
fisx Oct 21, 2020
aa2c5a3
Fix!
fisx Oct 21, 2020
35a9b39
Logging.
fisx Oct 21, 2020
714c025
...
fisx Oct 21, 2020
eb89185
brig: Remove user id from NewUserScimInvitation
smatting Oct 21, 2020
8c6f2ec
Remove createBrigUser and refactor
smatting Oct 21, 2020
e507dd0
Fixup
fisx Oct 21, 2020
6926a21
focus.
fisx Oct 21, 2020
582bfb9
Fix.
fisx Oct 21, 2020
bc3670a
Remove outdated comment.
fisx Oct 21, 2020
0f1e57c
accept invites with correct display name.
fisx Oct 21, 2020
ee59108
Fix search query.
fisx Oct 21, 2020
025c923
Make user display names more search query-friendly.
fisx Oct 21, 2020
456e81b
Leave a TODO with last failing test.
fisx Oct 21, 2020
ce6de94
Fix swagger; leave FUTUREWORK note.
fisx Oct 21, 2020
a529a0e
Test that pending users are invisible in `get /i/users/ids=...`.
fisx Oct 21, 2020
ddc36c6
Re-enabled disabled test fragment.
fisx Oct 21, 2020
f5d839b
might as well...
fisx Oct 21, 2020
3e01d2c
Cleanup
fisx Oct 22, 2020
75f7fe5
Fix email validation for saml credentials.
fisx Oct 22, 2020
1fdf2d4
Cleanup
fisx Oct 22, 2020
a218e02
Merge branch 'develop' into fisx/scim-onboarding-flow-again
fisx Oct 22, 2020
779e148
ormolu
fisx Oct 22, 2020
cdf4df6
Wait after reindexing -> Fixes search problem
smatting Oct 22, 2020
bce1a4b
make add-license.
fisx Oct 22, 2020
bc5f6e3
Merge branch 'fisx/scim-onboarding-flow-again' of ssh://github.com/wi…
fisx Oct 22, 2020
f3d2db0
Make /teams/invitations/by-email internal
smatting Oct 23, 2020
1f0e1e3
Remove debug logs, was too verbose
smatting Oct 23, 2020
a78843d
Remove brig cfg from spar-integration (and dependency on brig)
smatting Oct 23, 2020
d94591a
Refactor nested ifs to a pattern match
smatting Oct 23, 2020
fe81046
Make error codes consistent.
fisx Oct 23, 2020
de131bb
Update services/brig/src/Brig/API/Internal.hs
smatting Oct 23, 2020
82deaf7
Update services/spar/test-integration/Test/Spar/Scim/UserSpec.hs
smatting Oct 23, 2020
ca6e7df
Increase timeouts for CI
smatting Oct 23, 2020
e97343e
testCreateUserTimeout: replace threadDelay by retry
smatting Oct 23, 2020
90be1c7
testCreateUserNoIdp: wrap aFewTimesAssert around search
smatting Oct 23, 2020
e7337ef
ormulo
smatting Oct 23, 2020
6539e46
Fix test: call refreshIndex before *every* search.
fisx Oct 23, 2020
59d1cba
Tweak wait-times.
fisx Oct 23, 2020
fbc60ee
Cleanup.
fisx Oct 23, 2020
6b60d7f
Leave some bread crumbs in integration tests.
fisx Oct 23, 2020
6113986
Disable search step, because its delay causes other steps to fail
smatting Oct 26, 2020
452e8d3
remove dead code.
fisx Oct 26, 2020
7f95497
Test + Fix bug: User is not ManagedByScim after registration
smatting Oct 26, 2020
9e918d6
Haddocks.
fisx Oct 26, 2020
3101a8c
Merge branch 'fisx/scim-onboarding-flow-again' of ssh://github.com/wi…
fisx Oct 26, 2020
21b5045
Remove unused usersTeamSelect
smatting Oct 26, 2020
04fb07b
isPendingActivation: handle PendingInvitation
smatting Oct 26, 2020
8c62996
nit-pick.
fisx Oct 26, 2020
a1a1a15
hint.
fisx Oct 26, 2020
9cdc902
Make one more internal end-point not change behavior unless told to.
fisx Oct 26, 2020
dc33811
lookupUser, lookupUsers.
fisx Oct 26, 2020
dc6761a
hi ci
fisx Oct 27, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 36 additions & 1 deletion libs/brig-types/src/Brig/Types/Intra.hs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ module Brig.Types.Intra
AccountStatusResp (..),
ConnectionStatus (..),
UserAccount (..),
NewUserScimInvitation (..),
UserSet (..),
ReAuthUser (..),
)
Expand All @@ -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
Expand All @@ -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
Expand All @@ -56,13 +61,15 @@ 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
toJSON Active = String "active"
toJSON Suspended = String "suspended"
toJSON Deleted = String "deleted"
toJSON Ephemeral = String "ephemeral"
toJSON PendingInvitation = String "pending-invitation"

data AccountStatusResp = AccountStatusResp {fromAccountStatusResp :: AccountStatus}

Expand Down Expand Up @@ -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

Expand Down
8 changes: 6 additions & 2 deletions libs/brig-types/test/unit/Test/Brig/Types/User.hs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -41,7 +41,8 @@ roundtripTests :: [TestTree]
roundtripTests =
[ testRoundTrip @ManagedByUpdate,
testRoundTrip @ReAuthUser,
testRoundTrip @RichInfoUpdate
testRoundTrip @RichInfoUpdate,
testRoundTrip @NewUserScimInvitation
]

instance Arbitrary ManagedByUpdate where
Expand All @@ -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
1 change: 1 addition & 0 deletions services/brig/src/Brig/API/Error.hs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
49 changes: 36 additions & 13 deletions services/brig/src/Brig/API/Internal.hs
Original file line number Diff line number Diff line change
Expand Up @@ -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 (..))
Expand All @@ -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

Expand Down Expand Up @@ -117,6 +119,7 @@ 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"
Expand Down Expand Up @@ -334,20 +337,40 @@ 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
byIds :: [UserId] -> AppIO [UserAccount]
byIds uids = API.lookupAccounts uids >>= filterM accountValid
fisx marked this conversation as resolved.
Show resolved Hide resolved

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 -> Handler Response
listAccountsByIdentityH (_ ::: emailOrPhone) =
Expand Down
5 changes: 5 additions & 0 deletions services/brig/src/Brig/API/Public.hs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ 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 qualified Brig.User.API.Auth as Auth
import qualified Brig.User.API.Search as Search
Expand Down Expand Up @@ -74,6 +75,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)
Expand Down Expand Up @@ -987,6 +989,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)
Expand All @@ -1009,13 +1012,15 @@ 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,
let Public.BindingNewTeamUser (Public.BindingNewTeam team) _ = creator =
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
Expand Down
Loading