MSC3861: load the issuer and account management URLs from OIDC discovery

[sandhose]

This will help mitigating any discrepancies between the issuer configured and the one returned by the OIDC provider.

This also removes the need for configuring the account_management_url explicitely, as it will now be loaded from the OIDC discovery, as per MSC2965.

Because we may now fetch stuff for the .well-known/matrix/client endpoint, this also transforms the client well-known resource to be asynchronous.

[sandhose]

Changes here don't change the response when the WK can be built:

Before:

curl -i localhost:8008/.well-known/matrix/client
HTTP/1.1 200 OK
Server: Synapse/1.110.0
Date: Mon, 08 Jul 2024 10:10:19 GMT
Access-Control-Allow-Origin: *
Access-Control-Allow-Methods: GET, HEAD, POST, PUT, DELETE, OPTIONS
Access-Control-Allow-Headers: X-Requested-With, Content-Type, Authorization, Date, If-Match, If-None-Match
Access-Control-Expose-Headers: ETag, Location, X-Max-Bytes
Content-Type: application/json
Content-Length: 178

{"m.homeserver":{"base_url":"https://oidc.sandhose.fr/"},"org.matrix.msc2965.authentication":{"issuer":"https://oidc.sandhose.fr/","account":"https://oidc.sandhose.fr/account/"}}

After:

curl -i localhost:8008/.well-known/matrix/client
HTTP/1.1 200 OK
Transfer-Encoding: chunked
Server: Synapse/1.110.0
Date: Mon, 08 Jul 2024 10:10:57 GMT
Content-Type: application/json
Cache-Control: no-cache, no-store, must-revalidate
Access-Control-Allow-Origin: *
Access-Control-Allow-Methods: GET, HEAD, POST, PUT, DELETE, OPTIONS
Access-Control-Allow-Headers: X-Requested-With, Content-Type, Authorization, Date, If-Match, If-None-Match
Access-Control-Expose-Headers: ETag, Location, X-Max-Bytes

{"m.homeserver":{"base_url":"https://oidc.sandhose.fr/"},"org.matrix.msc2965.authentication":{"issuer":"https://oidc.sandhose.fr/","account":"https://oidc.sandhose.fr/account/"}}

It does although change the response when the public_baseurl isn't set:

Before:

curl -i localhost:8008/.well-known/matrix/client
HTTP/1.1 404 Not Found
Server: Synapse/1.110.0
Date: Mon, 08 Jul 2024 10:13:35 GMT
Access-Control-Allow-Origin: *
Access-Control-Allow-Methods: GET, HEAD, POST, PUT, DELETE, OPTIONS
Access-Control-Allow-Headers: X-Requested-With, Content-Type, Authorization, Date, If-Match, If-None-Match
Access-Control-Expose-Headers: ETag, Location, X-Max-Bytes
Content-Type: text/plain
Content-Length: 25

.well-known not available

After:

curl -i localhost:8008/.well-known/matrix/client
HTTP/1.1 404 Not Found
Transfer-Encoding: chunked
Server: Synapse/1.110.0
Date: Mon, 08 Jul 2024 10:13:10 GMT
Content-Type: application/json
Cache-Control: no-cache, no-store, must-revalidate
Access-Control-Allow-Origin: *
Access-Control-Allow-Methods: GET, HEAD, POST, PUT, DELETE, OPTIONS
Access-Control-Allow-Headers: X-Requested-With, Content-Type, Authorization, Date, If-Match, If-None-Match
Access-Control-Expose-Headers: ETag, Location, X-Max-Bytes

{"errcode":"M_NOT_FOUND","error":".well-known not available"}

Which is IMO perfectly acceptable

[reivilibre]

seems sensible other than a couple of concerns

[reivilibre]

is this the right precedence? Generally I would expect config to override autodetection

[sandhose]

I guess the problem is that the issuer in the config is used for discovery, and might be slightly different than the one you should actually advertise. Client will fail if the issuer advertised by /auth_issuer is not exactly the same as the one in {issuer}/.well-known/openid-configuration. We've had problems in the past of mismatches because of trailing slashes being in one place but not at the other one

So IMO, it can be confusing, but I think that pragmatically this is the right precedence

[reivilibre]

Hmm, even still, it still feels wrong to me. Suppose the homeserver fails to fetch the discovery at some point, you're still going to hand out the wrong issuer value to clients then, it will just be extra confusing that this was caused by the discovery failing.

[sandhose]

That's a really good point. Should we log a warning if there is a mismatch, but still work? Because the discovery has to work just once in Synapse's lifetime, then everything is cached, so hopefully that sort of outage would be very temporary

[reivilibre]

Can we check it on start up and then refuse to start up if it's wrong instead? I think this would be justified for what is a configuration error. The problem with warnings is that virtually nobody reads them.

I think by 'once in Synapse's lifetime' you mean once every process lifetime?

[sandhose]

I think you're right. I preferred to not catch the error if it fails loading. The impact of this, is that /.well-known/matrix/client and /_matrix/client/*/auth_issuer will 501 in case Synapse was never able to contact MAS, instead of potentially showing wrong informations. I think this is better

[reivilibre]

cache changes OK

still not sold about the precedence but if you really think it's best, then I'm not going to insist

[sandhose]

@reivilibre I merged develop back, tweaked stuff after the merging of #17509, and made it so that it 500 when it can't load the issuer metadata, instead of potentially wrongly falling back