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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions cassandra-schema.cql
Original file line number Diff line number Diff line change
Expand Up @@ -694,6 +694,7 @@ CREATE TABLE brig_test.user (
supported_protocols int,
team uuid,
text_status text,
user_type int,
write_time_bumper int
) WITH bloom_filter_fp_chance = 0.1
AND caching = {'keys': 'ALL', 'rows_per_partition': 'NONE'}
Expand Down
1 change: 1 addition & 0 deletions changelog.d/0-release-notes/WPB-22549-apps-vs-convs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Cassandra (`brig.user`) now keeps track of user types, only for newly created users. **Read this paragraph if you have already created apps before their official support:** For existing users and bots, the user type is inferred, but existing apps will show as regular users. Please remove those users from your team and create them again.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sorry, I know too little about apps: Is the audience of the changelog the same as the people adding apps? (I.e. sys admins deploying Wire?)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

changelog should be read by everybody who is dealing with wire-server i guess, so yes, team admins adding apps are covered?

of in this case: the site admin should read this and then make sure nobody is using any apps in prod already (which they shouldn't, it hasn't been released yet).

19 changes: 18 additions & 1 deletion libs/wire-api/src/Wire/API/User.hs
Original file line number Diff line number Diff line change
Expand Up @@ -470,7 +470,7 @@ instance (1 <= max) => ToJSON (LimitedQualifiedUserIdList max) where
-- UserType

data UserType = UserTypeRegular | UserTypeApp | UserTypeBot
deriving (Eq, Show, Generic)
deriving (Eq, Ord, Show, Generic)
deriving (Arbitrary) via (GenericUniform UserType)
deriving (A.FromJSON, A.ToJSON) via (Schema UserType)

Expand All @@ -486,6 +486,20 @@ instance ToSchema UserType where
Schema.element "bot" UserTypeBot
]

instance C.Cql UserType where
ctype = C.Tagged C.IntColumn

toCql UserTypeRegular = C.CqlInt 0
toCql UserTypeBot = C.CqlInt 1
toCql UserTypeApp = C.CqlInt 2

fromCql (C.CqlInt i) = case i of
0 -> pure UserTypeRegular
1 -> pure UserTypeBot
2 -> pure UserTypeApp
n -> Left $ "unexpected user type: " ++ show n
fromCql _ = Left "user type: int expected"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Have you considered to add the unbound value (_) to the error message? Maybe, it could be helpful in case this goes wrong and needs to be debugged?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

i would, but it's like this in all the other cql instances, and i've just go CI green... :-)


--------------------------------------------------------------------------------
-- UserProfile

Expand Down Expand Up @@ -579,6 +593,7 @@ instance FromJSON SelfProfile where
-- | The data of an existing user.
data User = User
{ userQualifiedId :: Qualified UserId,
userType :: UserType,
-- | User identity. For endpoints like @/self@, it will be present in the response iff
-- the user is activated, and the email/phone contained in it will be guaranteedly
-- verified. {#RefActivation}
Expand Down Expand Up @@ -635,6 +650,8 @@ userObjectSchema =
User
<$> userQualifiedId
.= field "qualified_id" schema
<*> userType
.= field "type" schema
<* userId
.= optional (field "id" (deprecatedSchema "qualified_id" schema))
<*> userIdentity .= maybeUserIdentityObjectSchema
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ testObject_SelfProfile_user_1 =
{ qUnqualified = Id (fromJust (UUID.fromString "00000001-0000-0000-0000-000000000002")),
qDomain = Domain {_domainText = "n0-994.m-226.f91.vg9p-mj-j2"}
},
userType = UserTypeRegular,
userIdentity = Just (EmailIdentity (unsafeEmailAddress "some" "example")),
userEmailUnvalidated = Nothing,
userDisplayName = Name {fromName = "@\1457\2598\66242\US\1104967l+\137302\&6\996495^\162211Mu\t"},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ testObject_User_user_1 =
{ qUnqualified = Id (fromJust (UUID.fromString "00000002-0000-0001-0000-000200000002")),
qDomain = Domain {_domainText = "s-f4.s"}
},
userType = UserTypeRegular,
userIdentity = Nothing,
userEmailUnvalidated = Nothing,
userDisplayName = Name {fromName = "\NULuv\996028su\28209lRi"},
Expand All @@ -80,6 +81,7 @@ testObject_User_user_2 =
{ qUnqualified = Id (fromJust (UUID.fromString "00000000-0000-0001-0000-000200000001")),
qDomain = Domain {_domainText = "k.vbg.p"}
},
userType = UserTypeBot,
userIdentity = Just (EmailIdentity (unsafeEmailAddress "some" "example")),
userEmailUnvalidated = Nothing,
userDisplayName =
Expand Down Expand Up @@ -120,6 +122,7 @@ testObject_User_user_3 =
{ qUnqualified = Id (fromJust (UUID.fromString "00000002-0000-0000-0000-000100000002")),
qDomain = Domain {_domainText = "dt.n"}
},
userType = UserTypeRegular,
userIdentity = Just (EmailIdentity (unsafeEmailAddress "some" "example")),
userEmailUnvalidated = Nothing,
userDisplayName =
Expand Down Expand Up @@ -153,6 +156,7 @@ testObject_User_user_4 =
{ qUnqualified = Id (fromJust (UUID.fromString "00000000-0000-0002-0000-000200000002")),
qDomain = Domain {_domainText = "28b.cqb"}
},
userType = UserTypeRegular,
userIdentity =
Just (SSOIdentity (UserScimExternalId "") (Just (unsafeEmailAddress "some" "example"))),
userEmailUnvalidated = Nothing,
Expand Down Expand Up @@ -191,6 +195,7 @@ testObject_User_user_5 =
{ qUnqualified = Id (fromJust (UUID.fromString "00000000-0000-0002-0000-000200000002")),
qDomain = Domain {_domainText = "28b.cqb"}
},
userType = UserTypeRegular,
userIdentity =
Just (EmailIdentity (unsafeEmailAddress "some" "example")),
userEmailUnvalidated = Nothing,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,7 @@ alice =
{ qUnqualified = Id (fromJust (UUID.fromString "539d9183-32a5-4fc4-ba5c-4634454e7585")),
qDomain = Domain {_domainText = "foo.example.com"}
},
userType = UserTypeRegular,
userIdentity = Nothing,
userEmailUnvalidated = Nothing,
userDisplayName = Name "alice",
Expand Down Expand Up @@ -252,6 +253,7 @@ bob =
{ qUnqualified = Id (fromJust (UUID.fromString "284d1c86-5117-4c58-aa18-c0068f3f7d8c")),
qDomain = Domain {_domainText = "baz.example.com"}
},
userType = UserTypeRegular,
userIdentity = Nothing,
userEmailUnvalidated = Nothing,
userDisplayName = Name "bob",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,5 +23,6 @@
],
"team": "00000001-0000-0002-0000-000000000002",
"text_status": "text status",
"searchable": true
"searchable": true,
"type": "regular"
}
3 changes: 2 additions & 1 deletion libs/wire-api/test/golden/testObject_UserEvent_1.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
],
"team": "bb843450-b2f5-4ec8-90bd-52c7d5f1d22e",
"text_status": "text status",
"searchable": true
"searchable": true,
"type": "regular"
}
}
3 changes: 2 additions & 1 deletion libs/wire-api/test/golden/testObject_UserEvent_2.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
],
"team": "bb843450-b2f5-4ec8-90bd-52c7d5f1d22e",
"text_status": "text status",
"searchable": true
"searchable": true,
"type": "regular"
}
}
3 changes: 2 additions & 1 deletion libs/wire-api/test/golden/testObject_User_user_1.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,6 @@
"supported_protocols": [
"proteus"
],
"searchable": true
"searchable": true,
"type": "regular"
}
3 changes: 2 additions & 1 deletion libs/wire-api/test/golden/testObject_User_user_2.json
Original file line number Diff line number Diff line change
Expand Up @@ -35,5 +35,6 @@
"status": "deleted",
"supported_protocols": [],
"text_status": "text status",
"searchable": true
"searchable": true,
"type": "bot"
}
3 changes: 2 additions & 1 deletion libs/wire-api/test/golden/testObject_User_user_3.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,5 +23,6 @@
"proteus"
],
"team": "00000002-0000-0001-0000-000200000000",
"searchable": true
"searchable": true,
"type": "regular"
}
3 changes: 2 additions & 1 deletion libs/wire-api/test/golden/testObject_User_user_4.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,5 +25,6 @@
"proteus"
],
"team": "00000000-0000-0000-0000-000100000002",
"searchable": true
"searchable": true,
"type": "regular"
}
3 changes: 2 additions & 1 deletion libs/wire-api/test/golden/testObject_User_user_5.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,5 +22,6 @@
"proteus"
],
"team": "00000000-0000-0000-0000-000100000002",
"searchable": true
"searchable": true,
"type": "regular"
}
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,7 @@ appNewStoredUser creator new = do
pure
NewStoredUser
{ id = Id uid,
userType = UserTypeApp,
email = Nothing,
ssoId = Nothing,
name = new.name,
Expand Down
44 changes: 36 additions & 8 deletions libs/wire-subsystems/src/Wire/StoredUser.hs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import Wire.Arbitrary

data StoredUser = StoredUser
{ id :: UserId,
userType :: Maybe UserType,
name :: Name,
textStatus :: Maybe TextStatus,
pict :: Maybe Pict,
Expand Down Expand Up @@ -100,6 +101,7 @@ mkUserFromStored domain defaultLocale storedUser =
svc = newServiceRef <$> storedUser.serviceId <*> storedUser.providerId
in User
{ userQualifiedId = (Qualified storedUser.id domain),
userType = inferUserType (isJust svc) storedUser.userType,
userIdentity = storedUser.identity,
userEmailUnvalidated = storedUser.emailUnvalidated,
userDisplayName = storedUser.name,
Expand All @@ -120,26 +122,48 @@ mkUserFromStored domain defaultLocale storedUser =
userSearchable = (fromMaybe True storedUser.searchable)
}

-- | This function helps us avoid data migrations in cassandra: we
-- allow `User`s to have no type value for backwards compatibility.
-- The type is inferred as "bot" if there is a serviceId, and
-- "regular" otherwise. For newly created apps, the second argument
-- will always be `Just`.
inferUserType :: Bool {- is service -} -> Maybe UserType -> UserType
inferUserType True _ = UserTypeBot
inferUserType False Nothing = UserTypeRegular
inferUserType False (Just t) = t

toLocale :: Locale -> (Maybe Language, Maybe Country) -> Locale
toLocale _ (Just l, c) = Locale l c
toLocale l _ = l

toIdentity ::
Maybe EmailAddress ->
Maybe UserSSOId ->
Maybe UserIdentity
toIdentity (Just e) Nothing = Just $! EmailIdentity e
toIdentity email (Just ssoid) = Just $! SSOIdentity ssoid email
toIdentity Nothing Nothing = Nothing

-- | If the user is not activated, 'toIdentity' will return 'Nothing', because
-- elsewhere we rely on the fact that a non-empty 'UserIdentity' means that the
-- user is activated.
toIdentity ::
--
-- FUTUREWORK(fisx): our account status representation is a mess. we
-- have `activated` for personal users that have an invitation
-- pending; `status` for team members; sometimes no user identity
-- means not activated; and maybe more exceptions that i haven't found
-- today? this needs to be cleaned up!
toIdentityIfActivated ::
-- | Whether the user is activated
Bool ->
Maybe EmailAddress ->
Maybe UserSSOId ->
Maybe UserIdentity
toIdentity True (Just e) Nothing = Just $! EmailIdentity e
toIdentity True email (Just ssoid) = Just $! SSOIdentity ssoid email
toIdentity True Nothing Nothing = Nothing
toIdentity False _ _ = Nothing
toIdentityIfActivated True e s = toIdentity e s
toIdentityIfActivated False _ _ = Nothing

instance HasField "identity" StoredUser (Maybe UserIdentity) where
getField user = toIdentity user.activated user.email user.ssoId
getField user = toIdentityIfActivated user.activated user.email user.ssoId

instance HasField "locale" StoredUser (Maybe Locale) where
getField user = Locale <$> user.language <*> pure user.country
Expand All @@ -148,6 +172,7 @@ instance HasField "locale" StoredUser (Maybe Locale) where

data NewStoredUser = NewStoredUser
{ id :: UserId,
userType :: UserType,
name :: Name,
textStatus :: Maybe TextStatus,
pict :: Pict,
Expand Down Expand Up @@ -180,6 +205,7 @@ recordInstance ''NewStoredUser
deriving instance
Show
( UserId,
UserType,
Name,
Maybe TextStatus,
Pict,
Expand All @@ -205,12 +231,14 @@ deriving instance
instance HasField "service" NewStoredUser (Maybe ServiceRef) where
getField user = ServiceRef <$> user.serviceId <*> user.providerId

-- This saves the identity from `NewStoredUser` even if the user is
-- not activated.
newStoredUserToUser :: Qualified NewStoredUser -> User
newStoredUserToUser (Qualified new domain) =
User
{ userQualifiedId = Qualified new.id domain,
-- save identity even if the user is not activated
userIdentity = toIdentity True new.email new.ssoId,
userType = new.userType,
userIdentity = toIdentity new.email new.ssoId,
userEmailUnvalidated = Nothing,
userDisplayName = new.name,
userTextStatus = new.textStatus,
Expand Down
6 changes: 3 additions & 3 deletions libs/wire-subsystems/src/Wire/UserStore/Cassandra.hs
Original file line number Diff line number Diff line change
Expand Up @@ -442,10 +442,10 @@ lookupServiceUsersForTeamImpl pid sid tid mPagingState =

insertUser :: PrepQuery W (TupleType NewStoredUser) ()
insertUser =
"INSERT INTO user (id, name, text_status, picture, assets, email, sso_id, \
"INSERT INTO user (id, user_type, name, text_status, picture, assets, email, sso_id, \
\accent_id, password, activated, status, expires, language, \
\country, provider, service, handle, team, managed_by, supported_protocols, searchable) \
\VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)"
\VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)"

insertServiceUser :: PrepQuery W (ProviderId, ServiceId, BotId, ConvId, Maybe TeamId) ()
insertServiceUser =
Expand All @@ -460,7 +460,7 @@ insertServiceTeam =
selectUsers :: PrepQuery R (Identity [UserId]) (TupleType StoredUser)
selectUsers =
[sql|
SELECT id, name, text_status, picture, email, email_unvalidated, sso_id, accent_id, assets,
SELECT id, user_type, name, text_status, picture, email, email_unvalidated, sso_id, accent_id, assets,
activated, status, expires, language, country, provider,
service, handle, team, managed_by, supported_protocols, searchable
FROM user WHERE id IN ?
Expand Down
12 changes: 8 additions & 4 deletions libs/wire-subsystems/test/unit/Wire/MiniBackend.hs
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ newtype PendingNotEmptyIdentityStoredUser = PendingNotEmptyIdentityStoredUser St

instance Arbitrary PendingNotEmptyIdentityStoredUser where
arbitrary = do
user <- arbitrary `suchThat` \user -> isJust user.identity
user <- arbitrary `suchThat` \user -> isJust user.identity && user.userType /= Just UserTypeApp
pure $ PendingNotEmptyIdentityStoredUser (user {status = Just PendingInvitation})

newtype NotPendingEmptyIdentityStoredUser = NotPendingEmptyIdentityStoredUser StoredUser
Expand All @@ -147,7 +147,7 @@ newtype NotPendingEmptyIdentityStoredUser = NotPendingEmptyIdentityStoredUser St
-- TODO: make sure this is a valid state
instance Arbitrary NotPendingEmptyIdentityStoredUser where
arbitrary = do
user <- arbitrary `suchThat` \user -> isNothing user.identity
user <- arbitrary `suchThat` \user -> isNothing user.identity && user.userType /= Just UserTypeApp
notPendingStatus <- elements (Nothing : map Just [Active, Suspended, Ephemeral])
pure $ NotPendingEmptyIdentityStoredUser (user {status = notPendingStatus})

Expand All @@ -164,7 +164,7 @@ newtype NotPendingStoredUser = NotPendingStoredUser StoredUser

instance Arbitrary NotPendingStoredUser where
arbitrary = do
user <- arbitrary `suchThat` \user -> isJust user.identity
user <- arbitrary `suchThat` \user -> isJust user.identity && user.userType /= Just UserTypeApp
notPendingStatus <- elements (Nothing : map Just [Active, Suspended, Ephemeral])
pure $ NotPendingStoredUser (user {status = notPendingStatus})

Expand All @@ -173,7 +173,7 @@ newtype NotPendingSSOIdWithEmailStoredUser = NotPendingSSOIdWithEmailStoredUser

instance Arbitrary NotPendingSSOIdWithEmailStoredUser where
arbitrary = do
user <- arbitrary `suchThat` \user -> fmap isUserSSOId user.ssoId == Just True
user <- arbitrary `suchThat` isSsoIsNotApp
notPendingStatus <- elements (Nothing : map Just [Active, Suspended, Ephemeral])
e <- arbitrary
pure $
Expand All @@ -184,6 +184,10 @@ instance Arbitrary NotPendingSSOIdWithEmailStoredUser where
email = Just e
}
)
where
isSsoIsNotApp user =
fmap isUserSSOId user.ssoId == Just True
&& user.userType /= Just UserTypeApp

newtype ActiveStoredUser = ActiveStoredUser StoredUser
deriving (Show, Eq)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,7 @@ newStoredUserToStoredUser :: NewStoredUser -> StoredUser
newStoredUserToStoredUser new =
StoredUser
{ id = new.id,
userType = Just new.userType,
name = new.name,
textStatus = new.textStatus,
pict = Just new.pict,
Expand Down
1 change: 1 addition & 0 deletions services/brig/brig.cabal
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,7 @@ library
Brig.Schema.V89_UpdateDomainRegistrationSchema
Brig.Schema.V90_DomainRegistrationTeamIndex
Brig.Schema.V91_UpdateDomainRegistrationSchema_AddWebappUrl
Brig.Schema.V92_AddUserType
Brig.Team.API
Brig.Team.Template
Brig.Template
Expand Down
2 changes: 2 additions & 0 deletions services/brig/src/Brig/Data/User.hs
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ newStoredUser u inv tid mbHandle = do
user uid l e mPassword =
NewStoredUser
{ id = uid,
userType = UserTypeRegular,
email = ident >>= emailIdentity,
ssoId = ident >>= ssoIdentity,
name,
Expand Down Expand Up @@ -117,6 +118,7 @@ newStoredUserViaScim uid externalId tid locale name email = do
pure $
NewStoredUser
{ id = uid,
userType = UserTypeRegular,
email = Just email,
ssoId = Just (UserScimExternalId externalId),
name,
Expand Down
Loading