Internal server error when trying to log in with OIDC when external ID is a number

[babolivier]

get_user_by_external_id retrieves an MXID from the ID sent by the authentication portal when using SSO.

In this function, we expect external_id to be a string, however we don't make sure the SSO portal returns with one. We should either add a check to both of the two places this function is called from to make sure the external ID is a string, or change that function to do that (e.g. if the ID isn't a string, try to cast it as such and error in a nicer way if it's not possible).

Sentry issue: https://sentry.matrix.org/sentry/synapse-modular/issues/116450/

[babolivier]

As a sidenote, the main occurrence of that is if the localpart is a number, in which case we'll likely also run into other issues given Synapse reserves such localparts to guest users. It might not be an issue, but it would be nice to check when getting around to fixing this.

[jaller94]

As a user trying to log in, I get this "Internal server error" page on the callback page.

[jaller94]

The synapse OpenID config I used on Modular was:

oidc_config:
   enabled: true
   discover: false
   issuer: "https://github.com/"
   client_id: "69…"
   client_secret: "60…"
   authorization_endpoint: "https://github.com/login/oauth/authorize"
   token_endpoint: "https://github.com/login/oauth/access_token"
   userinfo_endpoint: "https://api.github.com/user"
   scopes: ["read:user"]
   user_mapping_provider:
     config:
       subject_claim: "id"
       localpart_template: "{{ user.preferred_username }}"
       display_name_template: "{{ user.name }}"

The reproduced crash in Kibana, for those who have access:
https://modular-eucentral1-kibana.proxy.matrix.org/app/kibana#/doc/eb391830-aa29-11ea-be54-53b6e16b408b/filebeat-7.6.0-2020.07.07?id=y_n6KHMBxZr2sVMY-iPV&_g=(refreshInterval:(pause:!t,value:0),time:(from:'2020-07-07T11:13:13.852Z',to:'2020-07-07T11:13:14.000Z'))

[clokep]

We should either add a check to both of the two places this function is called from to make sure the external ID is a string, or change that function to do that (e.g. if the ID isn't a string, try to cast it as such and error in a nicer way if it's not possible).

For what it's worth, I believe we expect the mapping providers to return strings, but we should probably assert this somewhere.

[richvdh]

Internal server error when trying to log in with OIDC

I'm assuming there's more to it than that? Presumably this isn't always happening, otherwise the OIDC support would be useless?

[clokep]

Internal server error when trying to log in with OIDC

I'm assuming there's more to it than that? Presumably this isn't always happening, otherwise the OIDC support would be useless?

My understanding is that this happens when the external ID is a number. I've updated the title.