Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Initialise user displayname from SAML2 data #4272

Merged
merged 3 commits into from
Dec 7, 2018
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 changelog.d/4272.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
SAML2 authentication: Initialise user display name from SAML2 data
23 changes: 16 additions & 7 deletions synapse/handlers/register.py
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ def register(
make_guest=False,
admin=False,
threepid=None,
default_display_name=None,
):
"""Registers a new client on the server.

Expand All @@ -140,6 +141,8 @@ def register(
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:
Expand Down Expand Up @@ -169,6 +172,13 @@ def register(
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)
Expand All @@ -178,10 +188,7 @@ def register(
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,
)

Expand All @@ -203,13 +210,15 @@ def register(
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
Expand Down Expand Up @@ -300,7 +309,7 @@ def appservice_register(self, user_localpart, as_token):
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)

Expand Down Expand Up @@ -478,7 +487,7 @@ def get_or_create_user(self, requester, localpart, displayname,
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)
Expand Down
5 changes: 5 additions & 0 deletions synapse/rest/client/v1/login.py
Original file line number Diff line number Diff line change
Expand Up @@ -451,6 +451,7 @@ def __init__(self, hs):
@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.

Expand All @@ -467,6 +468,9 @@ def on_successful_auth(
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.
"""
Expand All @@ -478,6 +482,7 @@ def on_successful_auth(
yield self._registration_handler.register(
localpart=localpart,
generate_token=False,
default_display_name=user_display_name,
)
)

Expand Down
3 changes: 3 additions & 0 deletions synapse/rest/saml2/response_resource.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,9 @@ def _async_render_POST(self, request):
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,
)
20 changes: 13 additions & 7 deletions synapse/storage/registration.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
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


Expand Down Expand Up @@ -167,7 +168,7 @@ def add_access_token_to_user(self, user_id, token, device_id=None):

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:
Expand All @@ -181,8 +182,8 @@ def register(self, user_id, token=None, password_hash=None,
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.
"""
Expand All @@ -195,7 +196,7 @@ def register(self, user_id, token=None, password_hash=None,
was_guest,
make_guest,
appservice_id,
create_profile_with_localpart,
create_profile_with_displayname,
admin
)

Expand All @@ -208,9 +209,11 @@ def _register(
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()
Expand Down Expand Up @@ -273,12 +276,15 @@ def _register(
(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(
Expand Down
2 changes: 1 addition & 1 deletion tests/storage/test_monthly_active_users.py
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ def test_reap_monthly_active_users(self):

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
)
Expand Down