diff --git a/changelog.d/4272.feature b/changelog.d/4272.feature new file mode 100644 index 0000000000..7a8f286957 --- /dev/null +++ b/changelog.d/4272.feature @@ -0,0 +1 @@ +SAML2 authentication: Initialise user display name from SAML2 data diff --git a/synapse/handlers/register.py b/synapse/handlers/register.py index 0f87c4610e..ba39e67f6f 100644 --- a/synapse/handlers/register.py +++ b/synapse/handlers/register.py @@ -126,6 +126,7 @@ class RegistrationHandler(BaseHandler): make_guest=False, admin=False, threepid=None, + default_display_name=None, ): """Registers a new client on the server. @@ -140,6 +141,8 @@ class RegistrationHandler(BaseHandler): since it offers no means of associating a device_id with the access_token. Instead you should call auth_handler.issue_access_token after registration. + default_display_name (unicode|None): if set, the new user's displayname + will be set to this. Defaults to 'localpart'. Returns: A tuple of (user_id, access_token). Raises: @@ -169,6 +172,13 @@ class RegistrationHandler(BaseHandler): user = UserID(localpart, self.hs.hostname) user_id = user.to_string() + if was_guest: + # If the user was a guest then they already have a profile + default_display_name = None + + elif default_display_name is None: + default_display_name = localpart + token = None if generate_token: token = self.macaroon_gen.generate_access_token(user_id) @@ -178,10 +188,7 @@ class RegistrationHandler(BaseHandler): password_hash=password_hash, was_guest=was_guest, make_guest=make_guest, - create_profile_with_localpart=( - # If the user was a guest then they already have a profile - None if was_guest else user.localpart - ), + create_profile_with_displayname=default_display_name, admin=admin, ) @@ -203,13 +210,15 @@ class RegistrationHandler(BaseHandler): yield self.check_user_id_not_appservice_exclusive(user_id) if generate_token: token = self.macaroon_gen.generate_access_token(user_id) + if default_display_name is None: + default_display_name = localpart try: yield self.store.register( user_id=user_id, token=token, password_hash=password_hash, make_guest=make_guest, - create_profile_with_localpart=user.localpart, + create_profile_with_displayname=default_display_name, ) except SynapseError: # if user id is taken, just generate another @@ -300,7 +309,7 @@ class RegistrationHandler(BaseHandler): user_id=user_id, password_hash="", appservice_id=service_id, - create_profile_with_localpart=user.localpart, + create_profile_with_displayname=user.localpart, ) defer.returnValue(user_id) @@ -478,7 +487,7 @@ class RegistrationHandler(BaseHandler): user_id=user_id, token=token, password_hash=password_hash, - create_profile_with_localpart=user.localpart, + create_profile_with_displayname=user.localpart, ) else: yield self._auth_handler.delete_access_tokens_for_user(user_id) diff --git a/synapse/rest/client/v1/login.py b/synapse/rest/client/v1/login.py index b7c5b58b01..e9d3032498 100644 --- a/synapse/rest/client/v1/login.py +++ b/synapse/rest/client/v1/login.py @@ -451,6 +451,7 @@ class SSOAuthHandler(object): @defer.inlineCallbacks def on_successful_auth( self, username, request, client_redirect_url, + user_display_name=None, ): """Called once the user has successfully authenticated with the SSO. @@ -467,6 +468,9 @@ class SSOAuthHandler(object): client_redirect_url (unicode): the redirect_url the client gave us when it first started the process. + user_display_name (unicode|None): if set, and we have to register a new user, + we will set their displayname to this. + Returns: Deferred[none]: Completes once we have handled the request. """ @@ -478,6 +482,7 @@ class SSOAuthHandler(object): yield self._registration_handler.register( localpart=localpart, generate_token=False, + default_display_name=user_display_name, ) ) diff --git a/synapse/rest/saml2/response_resource.py b/synapse/rest/saml2/response_resource.py index ad2ed157b5..69fb77b322 100644 --- a/synapse/rest/saml2/response_resource.py +++ b/synapse/rest/saml2/response_resource.py @@ -66,6 +66,9 @@ class SAML2ResponseResource(Resource): raise CodeMessageException(400, "uid not in SAML2 response") username = saml2_auth.ava["uid"][0] + + displayName = saml2_auth.ava.get("displayName", [None])[0] return self._sso_auth_handler.on_successful_auth( username, request, relay_state, + user_display_name=displayName, ) diff --git a/synapse/storage/registration.py b/synapse/storage/registration.py index 80d76bf9d7..3d55441e33 100644 --- a/synapse/storage/registration.py +++ b/synapse/storage/registration.py @@ -22,6 +22,7 @@ from twisted.internet import defer from synapse.api.errors import Codes, StoreError from synapse.storage import background_updates from synapse.storage._base import SQLBaseStore +from synapse.types import UserID from synapse.util.caches.descriptors import cached, cachedInlineCallbacks @@ -167,7 +168,7 @@ class RegistrationStore(RegistrationWorkerStore, def register(self, user_id, token=None, password_hash=None, was_guest=False, make_guest=False, appservice_id=None, - create_profile_with_localpart=None, admin=False): + create_profile_with_displayname=None, admin=False): """Attempts to register an account. Args: @@ -181,8 +182,8 @@ class RegistrationStore(RegistrationWorkerStore, make_guest (boolean): True if the the new user should be guest, false to add a regular user account. appservice_id (str): The ID of the appservice registering the user. - create_profile_with_localpart (str): Optionally create a profile for - the given localpart. + create_profile_with_displayname (unicode): Optionally create a profile for + the user, setting their displayname to the given value Raises: StoreError if the user_id could not be registered. """ @@ -195,7 +196,7 @@ class RegistrationStore(RegistrationWorkerStore, was_guest, make_guest, appservice_id, - create_profile_with_localpart, + create_profile_with_displayname, admin ) @@ -208,9 +209,11 @@ class RegistrationStore(RegistrationWorkerStore, was_guest, make_guest, appservice_id, - create_profile_with_localpart, + create_profile_with_displayname, admin, ): + user_id_obj = UserID.from_string(user_id) + now = int(self.clock.time()) next_id = self._access_tokens_id_gen.get_next() @@ -273,12 +276,15 @@ class RegistrationStore(RegistrationWorkerStore, (next_id, user_id, token,) ) - if create_profile_with_localpart: + if create_profile_with_displayname: # set a default displayname serverside to avoid ugly race # between auto-joins and clients trying to set displaynames + # + # *obviously* the 'profiles' table uses localpart for user_id + # while everything else uses the full mxid. txn.execute( "INSERT INTO profiles(user_id, displayname) VALUES (?,?)", - (create_profile_with_localpart, create_profile_with_localpart) + (user_id_obj.localpart, create_profile_with_displayname) ) self._invalidate_cache_and_stream( diff --git a/tests/storage/test_monthly_active_users.py b/tests/storage/test_monthly_active_users.py index 8664bc3d54..9618d57463 100644 --- a/tests/storage/test_monthly_active_users.py +++ b/tests/storage/test_monthly_active_users.py @@ -149,7 +149,7 @@ class MonthlyActiveUsersTestCase(HomeserverTestCase): def test_populate_monthly_users_is_guest(self): # Test that guest users are not added to mau list - user_id = "user_id" + user_id = "@user_id:host" self.store.register( user_id=user_id, token="123", password_hash=None, make_guest=True )