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

The OpenID Connect authenticator does not use the userinfo endpoint by default #9316

Open
tmortagne opened this issue Feb 4, 2021 · 8 comments

Comments

@tmortagne
Copy link
Contributor

I don't get any of the user information (I would have expected to see at least the display name and the email) available through the userinfo endpoint (available in the discovery endpoint) synchronized with my matrix account.

According to https://github.com/matrix-org/synapse/blob/v1.26.0/synapse/handlers/oidc_handler.py#L342 the authenticator seems to assume that the concept of userinfo endpoint is not really part of the OpenID Connect protocol and expect user related claims to be provided by the id token as soon as the scopes contains "oidc".

From what I can see in the OIDC specification the only user related information the id token is supposed to contain is the subject and if the authenticator need to resolve all the user claims (those par of profile and email) it asked the authentication endpoint to authorize it's supposed to go through the userinfo endpoint.

Synapse version: 1.26.0

My oidc config:

oidc_providers:
  - idp_id: adm
    idp_name: "My Provider"
    issuer: "https://myhost/oidc/"
    client_id: "matrix"
    client_secret: "dontcare"
    scopes: ["openid", "profile", "email"]

I'm testing with https://app.element.io as client.

@clokep
Copy link
Member

clokep commented Feb 4, 2021

The user_profile_method might be relevant, but that seems to say that it will fetch the userinfo endpoint if "openid" is in the scopes, which it is in this case. So it should be using it? It is possible that the default mapping provider isn't properly pulling information from that or something though.

@tmortagne
Copy link
Contributor Author

It is possible that the default mapping provider isn't properly pulling information from that or something though.

What I see in https://github.com/matrix-org/synapse/blob/v1.26.0/synapse/handlers/oidc_handler.py#L778 suggest me that the provider is supposed to take in input the userinfo so there is no reason for it to pull it.

@tmortagne
Copy link
Contributor Author

It seems for the display name that I won't have it if I don't set display_name_template anyways according to the documentation:

#             display_name_template: Jinja2 template for the display name to set
#                 on first login. If unset, no displayname will be set.

I feel it would be much nicer to fallback on the standard OIDC "name" claim for example.

@richvdh
Copy link
Member

richvdh commented Feb 8, 2021

This issue seems to conflate a couple of separate issues: one, whether we should use the userinfo endpoint by default; two, whether we can improve the defaults for populating displayname, email, etc.

I think the first is discussed and resolved and #9315. The second is debatable, but if it's still a concern it might be best to open a new issue with a clearer description of the situation?

@tmortagne
Copy link
Contributor Author

tmortagne commented Feb 8, 2021

This issue seems to conflate a couple of separate issues: one, whether we should use the userinfo endpoint by default; two, whether we can improve the defaults for populating displayname, email, etc.

No, this issue really was only related to the fact that the userinfo is not used by default which is quite surprising from OIDC point of view. I worked around it with user_profile_method: "userinfo_endpoint" but it does not feel right.

#9315 is not really the same thing, I just missed that it was expected for the authenticator to ask the user for its id by default and assumed it was because the subject was containing upper case letters.

@clokep
Copy link
Member

clokep commented Feb 8, 2021

Note that this behavior was added in #7658, that PR is quite confusing but there were some backwards compatibility concerns about always fetching it, I think?

@richvdh richvdh changed the title The OpenID Connect authenticator does not seems to be using the userinfo endpoint by default The OpenID Connect authenticator does not use the userinfo endpoint by default Feb 9, 2021
@richvdh
Copy link
Member

richvdh commented Feb 9, 2021

Right, sorry, I was finding it hard to get an overview of the issue. For the record, I believe a summary is:

  • Our OIDC/OAuth2 implementation currently assumes that, if an id_token is available, that will contain all the information we need, so there is no need to call the userinfo_endpoint.
  • However, the id_token may not include useful fields such as the user's display-name, email address, and avatar, so this assumption is sometimes false.

I say sometimes false: many Authorization Servers return extra data in the id_token which means that it is indeed unnecessary to call the userinfo_endpoint - and indeed some IdPs don't even have a userinfo_endpoint (note, for example, that the discovery spec has it as RECOMMENDED, not REQUIRED).

I understand the proposal here is to call the userinfo endpoint by default [at least if it is available?], to maximise the amount of data available to the username mapper.

That's not an unreasonable suggestion, though comes at the cost of a potentially redundant round-trip to the Identity Provider. However, I am a little wary of somehow breaking peoples' existing installations (eg their userinfo_endpoint returns different data to that in the id_token), and I question whether these costs are actually worthwhile when the alternative is simply to set user_profile_method: userinfo_endpoint.

@tmortagne
Copy link
Contributor Author

tmortagne commented Feb 9, 2021

many Authorization Servers return extra data in the id_token

Sure but then it's not really OpenId Connect stuff. The OIDC specification seems to suggest in https://openid.net/specs/openid-connect-basic-1_0.html#rfc.section.2.5 that it is the job of the userinfo enpoint to provide the values of the standard claims related to user details (associated to the profile and email scopes for example). From what I understand the only piece of information about the user which is standardized by the specification as part of the id token is the user subject.

Where I agree is that if the only scope provided is "oidc" then there does not seem to be any point in accessing the userinfo enpoint (since you are supposed to explicitly request scopes like profile and email to get related claims). That might help with the retro compatibility.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants