OIDC mapping providers have no way to check for existing user localparts

[anoadragon453]

Currently Open ID mapping providers have no method of checking for an existing user localpart when mapping user attributes received from the external service over to matrix.

This can be a problem for e.g some services that identify users via an email address, but also allow custom domains. As the localpart of email addresses can be the same (alice@example.com, alice@example2.com), if the mapping provider wishes to only take the localpart of the email address and use it as the Matrix localpart, there will be a conflict.

SAML mapping providers can overcome this using the failures integer argument to the saml_response_to_user_attributes function. If the localpart returned by the mapping provider maps to an existing Matrix localpart, saml_response_to_user_attributes will be called again with the failures argument incremented.

SAML mapping providers can use this as a sign to change the localpart slightly before returning again. This process continues until a valid, new localpart is generated, or an arbitrary loop limit is hit.

OIDC mappers do not have similar functionality, and thus mapping provider developers are stuck.

It's worth nothing that this problem could be solved by append random characters to the localpart, thus making a collision unlikely, but it is not a very elegant solution.

I'm also aware that the SAML failures solution only gives limited information to the mapping provider, and would consider a different approach. However, whatever is decided on should be made consistent across both SAML and OIDC for ease of development and use.

[clokep]

My inclination is to essentially copy what we do for SAML (which is to just increment a failure count). I'm not sure if this can be done in a backwards compatible manner, but I'll try to.

Do we think there's a better way to do this for both SAML and OIDC? We could give the mapping provider a callable which allows it to check whether a name is in use. I'm not really sure what else would be reasonable here.

[clokep]

This is also made a bit more complicated that the SAML code has an option (grandfathered_mxid_source_attribute) for backwards-compatibility. I don't think this is worth adding to the OIDC code, but is in a somewhat related section.

[anoadragon453]

My inclination is to essentially copy what we do for SAML (which is to just increment a failure count). I'm not sure if this can be done in a backwards compatible manner, but I'll try to.

Add it as an optional argument to the mapping function?

Do we think there's a better way to do this for both SAML and OIDC? We could give the mapping provider a callable which allows it to check whether a name is in use. I'm not really sure what else would be reasonable here.

I do think the SAML implementation is a bit naive. However, it does mean the mapping provider doesn't need to keep track of how many attempts have failed, which is valuable.

[clokep]

The OIDC code has the ability to match to existing users. I suspect we only want to do this with the first result from the mapping provider (e.g. if @foo:bar exists in the database and allow_existing_users is true, we would match it, but if @foo1:bar exists we wouldn't). Any thoughts on this?

Annoyingly this is quite different from SAML where we pull a separate attribute from the SAML response to check first.

[anoadragon453]

The OIDC code has the ability to match to existing users. I suspect we only want to do this with the first result from the mapping provider (e.g. if @foo:bar exists in the database and allow_existing_users is true, we would match it, but if @foo1:bar exists we wouldn't). Any thoughts on this?

@clokep I'm not quite sure I understand what you're asking here. Do you mean if multiple matrix users are matched from the information returned from an OIDC IdP, then we'll log the user in to the first result found?

[clokep]

The OIDC code has the ability to match to existing users. I suspect we only want to do this with the first result from the mapping provider (e.g. if @foo:bar exists in the database and allow_existing_users is true, we would match it, but if @foo1:bar exists we wouldn't). Any thoughts on this?

@clokep I'm not quite sure I understand what you're asking here. Do you mean if multiple matrix users are matched from the information returned from an OIDC IdP, then we'll log the user in to the first result found?

I'm trying to figure out the behavior when allow_existing_users is configured for OIDC. I think I have an idea, but it is somewhat "there be dragons", but I'll try it first.