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

OIDC authenticator does use the user id sent by the provider if it contains uppercases #9315

Closed
tmortagne opened this issue Feb 4, 2021 · 13 comments
Labels
T-Other Questions, user support, anything else.

Comments

@tmortagne
Copy link
Contributor

tmortagne commented Feb 4, 2021

Synapse version: 1.26.0
Client: https://app.element.io

When the OIDC provider return "ThomasMortagne" as subject I'm asked to provide the user id after the auth ("Please pick your username:"). When I try to enter "ThomasMortagne" in this form I get "Invalid username. Only the following characters are allowed: lowercase letters, digits, ., _, -, /, =" so I guess that's why it did not use it. But then I would have expected it to try to clean up the subject automatically for simple case like upper vs lower case. At the very least It should explain why it's asking for the id.

This sounds like quite a constraint, but I don't see any reference to it in the documentation, so it's possible I'm completely wrong.

@clokep
Copy link
Member

clokep commented Feb 4, 2021

Matrix IDs must be all lower-case.

Are you using a custom mapping provider? v1.26.0 includes a feature to prompt people to enter their username (instead of automatically using the one from SSO), which I think it is enabled by default.

I see you provided a config in #9316:

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

Take a look at the mapping_provider key, maybe you want something like:

oidc_providers:
  - idp_id: adm
    idp_name: "My Provider"
    issuer: "https://myhost/oidc/"
    client_id: "matrix"
    client_secret: "dontcare"
    scopes: ["openid", "profile", "email"]
    mapping_provider:
      config:
        localpart_template: "{{ user.login }}"

@tmortagne
Copy link
Contributor Author

tmortagne commented Feb 4, 2021

Are you using a custom mapping provider?

No I don't actually configure anything for the mapping, just using the default behavior.

v1.26.0 includes a feature to prompt people to enter their username (instead of automatically using the one from SSO), which I think it is enabled by default.

OK that's the part I missed and which was indeed clearly indicated in the documentation.

Matrix IDs must be all lower-case.

So my only solution is to find a way on provider side to send a subject compatible with Matrix id constraints, right ?

@clokep
Copy link
Member

clokep commented Feb 4, 2021

Matrix IDs must be all lower-case.

So my only solution is to find a way on provider side to send a subject compatible with Matrix id constraints, right ?

No, the default mapping provider should convert whatever is used as the localpart_template into a valid MXID automatically.

@tmortagne
Copy link
Contributor Author

tmortagne commented Feb 4, 2021

No, the default mapping provider should convert whatever is used as the localpart_template into a valid MXID automatically.

OK sounds great ! One last thing: it's not clear so me what "login" is since the documentation indicate that "the Jinja2 templates are given a 'user' variable, which is set to the claims returned by the UserInfo Endpoint and/or in the ID Token." and there is no such claim in OIDC. But maybe "login" is a special field injected by Synapse with the value of the sub ?

@clokep
Copy link
Member

clokep commented Feb 4, 2021

What exactly is available on the user variable depends on the claims that are provided by your OIDC provider, sub is a common one which could be accessed via user.sub.

@tmortagne
Copy link
Contributor Author

Yes I know about sub, I was just wondering why you were suggesting to use login.

@tmortagne
Copy link
Contributor Author

So I tried with:

oidc_providers:
  - idp_id: adm
    idp_name: "My Provider"
    issuer: "https://myhost/oidc/"
    client_id: "matrix"
    client_secret: "dontcare"
    scopes: ["openid", "profile", "email"]
    user_profile_method: "userinfo_endpoint"
    mapping_provider:
      config:
        localpart_template: "{{ user.preferred_username }}"

(with the "preferred_username" claim containing "ThomasMortagne")

But it keeps asking me for the id.

The log does not really give much hint on why (but at least it seem it did use the userinfo endpoint thanks to user_profile_method: "userinfo_endpoint"):

2021-02-04 15:34:35,152 - synapse.http.client - 429 - INFO - GET-7 - Received response to POST https://myhost/oidc/token: 200
2021-02-04 15:34:35,392 - synapse.http.client - 429 - INFO - GET-7 - Received response to GET https://myhost/oidc/userinfo: 200
2021-02-04 15:34:35,398 - synapse.handlers.sso - 501 - INFO - GET-7 - Recorded registration session id **************
2021-02-04 15:34:35,399 - synapse.http.server - 120 - INFO - GET-7 - <XForwardedForRequest at 0x7fb6005265f8 method='GET' uri='/_synapse/oidc/callback?code=****&state=****' clientproto='HTTP/1.1' site='8008'> redirect to b'/_synapse/client/pick_username'

@richvdh
Copy link
Member

richvdh commented Feb 4, 2021

mapping_provider:

this should be user_mapping_provider. Looks like @clokep made an error here, though I strongly suggest reading both the sample config and /~https://github.com/matrix-org/synapse/blob/develop/docs/openid.md, both of which contain a wealth of examples, rather than relying directly on asking questions.

@richvdh
Copy link
Member

richvdh commented Feb 4, 2021

incidentally, you'll need to transform preferred_username into a valid matrix id localpart. Maybe something like:

        localpart_template: "{{ user.preferred_username | lower }}"

the template is jinja2, so https://jinja.palletsprojects.com/en/2.11.x/templates/ has documentation on the possibilities here.

@clokep
Copy link
Member

clokep commented Feb 4, 2021

incidentally, you'll need to transform preferred_username into a valid matrix id localpart. Maybe something like:

        localpart_template: "{{ user.preferred_username | lower }}"

the template is jinja2, so jinja.palletsprojects.com/en/2.11.x/templates has documentation on the possibilities here.

This happens automatically due to

# Ensure only valid characters are included in the MXID.
localpart = map_username_to_mxid_localpart(localpart)

Sorry about the typo! 👍 I agree with reading the sample config. If there's parts that are unclear it would be good if we can improve them!

Yes I know about sub, I was just wondering why you were suggesting to use login.

Because I chose a random example claim. I don't know what your IdP offers.

@tmortagne
Copy link
Contributor Author

this should be user_mapping_provider

Indeed, thanks, I should have double-checked.

I strongly suggest reading both the sample config and develop/docs/openid.md, both of which contain a wealth of examples

I did that actually, but I'm not using any of those providers, and I was expecting that it would use standard OIDC metadata by default. So I was not really expecting to have to configure the mapping and did not checked this part much.

All good now for the id and the display name. Thanks a lot for the help @clokep and @richvdh !

There is just one last strange things for which I don't find any reference in the OIDC mapping documentation: it seems my email is not synchronized ("Email addresses" is empty in my profile) but the email is definitely sent back by the provider (I print the json on provider log side). Is that expected ?

Here is how the userinfo looks like:

{
  "sub":"ThomasMortagne",
  "updated_at":1612453235,
  "profile":"https:\/\/myprovider\/ThomasMortagne",
  "name":"Thomas Mortagne",
  "preferred_username":"ThomasMortagne",
  "given_name":"Thomas",
  "family_name":"Mortagne",
  "picture":"https:\/\/myprovider\/ThomasMortagne\/profile.jpg",
  "email":"thomas.mortagne@***.com"
}

@clokep
Copy link
Member

clokep commented Feb 4, 2021

Injecting emails isn't supported until the unreleased v1.27.0 see #9245.

I'm going to close this since it seems you got it working.

@clokep clokep closed this as completed Feb 4, 2021
@clokep clokep added the T-Other Questions, user support, anything else. label Feb 4, 2021
@tmortagne
Copy link
Contributor Author

Injecting emails isn't supported until the unreleased v1.27.0 see #9245.

OK thanks for the info.

I'm going to close this since it seems you got it working.

Definitely, thanks again and sorry for the issue that should have been questions on the chat in the end.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
T-Other Questions, user support, anything else.
Projects
None yet
Development

No branches or pull requests

3 participants