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

Updating user profile data from SSO IdP #5763

Open
ara4n opened this issue Jul 25, 2019 · 10 comments
Open

Updating user profile data from SSO IdP #5763

ara4n opened this issue Jul 25, 2019 · 10 comments
Labels
A-SSO Single Sign-On (maybe OIDC) T-Enhancement New features, changes in functionality, improvements in performance, or user-facing enhancements.

Comments

@ara4n
Copy link
Member

ara4n commented Jul 25, 2019

Profile only gets updated via on_successful_auth at registration, says @slipeer

@slipeer
Copy link
Contributor

slipeer commented Jul 25, 2019

For now displayname is recorded to the user profile only when you first log in using SSO.
If user's record already exists in the synapse database, the displayname from SSO source is ignored.

When I have a single identity provider that provides displayname atribute, I expect that the change of the displayname will spread to all integrated systems.

@richvdh
Copy link
Member

richvdh commented Jul 25, 2019

what would be the mechanism for getting updates from the IdP to synapse? Would the update only happen when the user re-authenticates?

@slipeer
Copy link
Contributor

slipeer commented Jul 25, 2019

Would the update only happen when the user re-authenticates?

I think that would be enough.
In combination with session_lifetime - this will give you the actual display name for homeserver users.

@slipeer
Copy link
Contributor

slipeer commented Jul 25, 2019

This can be made easy with:

self._profile_handler = hs.get_profile_handler()

and

if not registered_user_id:                                                                                                                                                            
    registered_user_id, _ = (                                                                                                                                                         
        yield self._registration_handler.register(                                                                                                                                    
            localpart=localpart,                                                                                                                                                      
            generate_token=False,                                                                                                                                                     
            default_display_name=user_display_name,                                                                                                                                   
        )                                                                                                                                                                             
    )                                                                                                                                                                                 
elif user_display_name:                                                                                                                                                               
    self._profile_handler.set_displayname(
        UserID.from_string(user_id),
        create_requester(user_id),
        user_display_name
    )

@neilisfragile neilisfragile added z-p2 (Deprecated Label) z-bug (Deprecated Label) labels Jul 31, 2019
@benjamin-kirkbride
Copy link

Any update to this?

@dklimpel
Copy link
Contributor

dklimpel commented Mar 3, 2020

See also #7023

@anoadragon453
Copy link
Member

Looks like we'd need to add an extra step somewhere in here:

async def complete_sso_login_request(
self,
auth_provider_id: str,
remote_user_id: str,
request: SynapseRequest,
client_redirect_url: str,
sso_to_matrix_id_mapper: Callable[[int], Awaitable[UserAttributes]],
grandfather_existing_users: Callable[[], Awaitable[Optional[str]]],
extra_login_attributes: Optional[JsonDict] = None,
) -> None:
"""
Given an SSO ID, retrieve the user ID for it and possibly register the user.
This first checks if the SSO ID has previously been linked to a matrix ID,
if it has that matrix ID is returned regardless of the current mapping
logic.
If a callable is provided for grandfathering users, it is called and can
potentially return a matrix ID to use. If it does, the SSO ID is linked to
this matrix ID for subsequent calls.
The mapping function is called (potentially multiple times) to generate
a localpart for the user.
If an unused localpart is generated, the user is registered from the
given user-agent and IP address and the SSO ID is linked to this matrix
ID for subsequent calls.
Finally, we generate a redirect to the supplied redirect uri, with a login token
Args:
auth_provider_id: A unique identifier for this SSO provider, e.g.
"oidc" or "saml".
remote_user_id: The unique identifier from the SSO provider.
request: The request to respond to
client_redirect_url: The redirect URL passed in by the client.
sso_to_matrix_id_mapper: A callable to generate the user attributes.
The only parameter is an integer which represents the amount of
times the returned mxid localpart mapping has failed.
It is expected that the mapper can raise two exceptions, which
will get passed through to the caller:
MappingException if there was a problem mapping the response
to the user.
RedirectException to redirect to an additional page (e.g.
to prompt the user for more information).
grandfather_existing_users: A callable which can return an previously
existing matrix ID. The SSO ID is then linked to the returned
matrix ID.
extra_login_attributes: An optional dictionary of extra
attributes to be provided to the client in the login response.
Raises:
MappingException if there was a problem mapping the response to a user.
RedirectException: if the mapping provider needs to redirect the user
to an additional page. (e.g. to prompt for more information)
"""
new_user = False
# grab a lock while we try to find a mapping for this user. This seems...
# optimistic, especially for implementations that end up redirecting to
# interstitial pages.
with await self._mapping_lock.queue(auth_provider_id):
# first of all, check if we already have a mapping for this user
user_id = await self.get_sso_user_by_remote_user_id(
auth_provider_id,
remote_user_id,
)
# Check for grandfathering of users.
if not user_id:
user_id = await grandfather_existing_users()
if user_id:
# Future logins should also match this user ID.
await self._store.record_user_external_id(
auth_provider_id, remote_user_id, user_id
)
# Otherwise, generate a new user.
if not user_id:
attributes = await self._call_attribute_mapper(sso_to_matrix_id_mapper)
if attributes.localpart is None:
# the mapper doesn't return a username. bail out with a redirect to
# the username picker.
await self._redirect_to_username_picker(
auth_provider_id,
remote_user_id,
attributes,
client_redirect_url,
extra_login_attributes,
)
user_id = await self._register_mapped_user(
attributes,
auth_provider_id,
remote_user_id,
get_request_user_agent(request),
request.getClientIP(),
)
new_user = True
await self._auth_handler.complete_sso_login(
user_id,
auth_provider_id,
request,
client_redirect_url,
extra_login_attributes,
new_user=new_user,
)

that updates the user's mutable attributes (so not their UserID) if they've changed based off the response we get back from the IdP.

@clokep clokep added T-Enhancement New features, changes in functionality, improvements in performance, or user-facing enhancements. A-SSO Single Sign-On (maybe OIDC) and removed z-bug (Deprecated Label) z-p2 (Deprecated Label) labels Apr 14, 2021
@jkanefendt
Copy link
Contributor

A solution is proposed in #10108

@anoadragon453
Copy link
Member

#10108 introduces a new config option, sso.update_profile_information, which when enabled will override Matrix profile information from an SSO identity provider.

For now, only the display name is supported - I'm not sure whether we want to keep this issue open until other fields are supported as well, but given the issue's title I suspect so.

@clokep clokep changed the title Updating user profile data from SAML IdP Updating user profile data from SSO IdP Jul 14, 2022
@squahtx
Copy link
Contributor

squahtx commented Jul 15, 2022

User information from OIDC is only fetch at the initial user creation and never again. If you add the email_template to the user_mapping_provider later existing users will not be updated. If a users email changes this change is never reflected. If you set the log level of the sso and oidc handler and log in with existing and new accounts it's clearly visible that user information is only fetched and successfully stored with the initial user creation.

Originally posted by @loelkes in #12605 (comment)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-SSO Single Sign-On (maybe OIDC) T-Enhancement New features, changes in functionality, improvements in performance, or user-facing enhancements.
Projects
None yet
Development

No branches or pull requests

10 participants