-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/me grumbles at github for eating my first attempt at this comment
Looks like a great start! A few nits above. More generally though:
We need a way to map the displayName. I suggest the mapper return a dict (which will allow for easier addition of new features in the future: avatar url anyone?)
The day is probably not far off where the mapping for mxid and displayName will require more than a single saml attribute. Easier to pass the entire saml2_auth object into the module (and then the lookups from _mxid_source_attr (?) and displayName can move into the default mapper).
Co-Authored-By: Richard van der Hoff <[email protected]>
…ynapse into anoa/saml_username_provider
6be2f08
to
169d369
Compare
Ok! The plugin now gets the auth response, returns a dict that we pull from. You can provide it a custom config. And there's documentation to boot! |
from synapse.util.async_helpers import Linearizer | ||
|
||
logger = logging.getLogger(__name__) | ||
|
||
|
||
@attr.s | ||
class Saml2SessionData: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that this was just cut and pasted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this looks... untested?
Ahhh, I knew there was a reason I didn't review request this on friday. Sorry :/ |
…l_username_provider
Tested with success! |
synapse/handlers/saml_handler.py
Outdated
SamlConfig: A custom config object for this module | ||
""" | ||
# Parse config options and use defaults where necessary | ||
# We use the 'or' syntax here as these options could be a valid value of None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what do you mean by "could be a valid value of None" ? None isn't valid for these options?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mm, I mean that if we do config.get("mxid_source_attribute", "uid")
, None
could still be returned as the config could be:
user_mapping_provider:
config:
mxid_source_attribute:
Thus, we use mxid_source_attribute = config.get("mxid_source_attribute") or "uid
instead, such that if config.get("mxid_source_attribute")
returns None
, we set mxid_source_attribute
to "uid"
instead.
Yeah the comment is confusing. I've changed it, let me know what you think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm otherwise!
…rch_redacted_events * 'develop' of github.com:matrix-org/synapse: (100 commits) Move get_state methods into FederationHandler (#6503) Allow SAML username provider plugins (#6411) Fix race which caused deleted devices to reappear (#6514) Refactor get_events_from_store_or_dest to return a dict (#6501) Remove redundant code from event authorisation implementation. (#6502) Newsfile Silence mypy errors for files outside those specified Newsfile Phone home stats DB reporting should not assume a single DB. Update comment Drop unused index Convert _censor_redactions to async since it awaits on coroutines Only start censor background job after indices are created Newsfile Newsfile Fix make_deferred_yieldable to work with coroutines Newsfile Fix support for SQLite 3.7. Better errors regarding changing avatar_url (#6497) 1.7.0rc1 ...
* commit '4947de5a1': Allow SAML username provider plugins (#6411)
Fixes: #6477
Allows the ability for an external python module to handle the mapping between a SAML auth response attribute to the localpart of a new mxid.
Done so we don't have to keep adding options to Synapse for tiny regex changes :)
If a module is provided, Synapse will use it's implementation of
mxid_source_to_mxid_localpart
(a new method that was split out ofSamlHandler._map_saml_response_to_user
. Otherwise it'll use the built-in one.The config option
saml_config.mxid_source_attribute
determines which attribute to pull out of the SAML response object for creating the mxid, then whatever the value of that is handed tomxid_source_to_mxid_localpart
to get the mxid localpart. If a user with the generated mxid already exists,mxid_source_to_mxid_localpart
will be run again but with thefailures
argument incremented. This will continue until Synapse will eventually give up after 1000 failures to generate an mxid.