search

element-hq / matrix-authentication-service

https://element-hq.github.io/matrix-authentication-service/
GNU Affero General Public License v3.0
35 stars 9 forks source link

Trailing slash leads to IssuerUrlsDontMatch #2352

Open matrixbot opened 1 month ago

matrixbot commented 1 month ago

This issue was originally created by @timokoesters at https://github.com/matrix-org/matrix-authentication-service/issues/2352.

Example: The expected issuer is https://example.com, but the well-known/openid-configuration reports https://example.com/ with a trailing slash. Currently, this leads to the Err(ProviderMetadataVerificationError::IssuerUrlsDontMatch) because of this exact string match: https://github.com/matrix-org/matrix-authentication-service/blob/3d90d0861ccf87d10e331a7fccd47c6290887bec/crates/oauth2-types/src/oidc.rs#L504-L506

Should the comparison ignore trailing slashes or is this a misconfiguration by the user?

matrixbot commented 1 month ago

This comment was originally posted by @sandhose at https://github.com/matrix-org/matrix-authentication-service/issues/2352#issuecomment-1938956295.

This is a misconfiguration by the user IMO. We're intentionally being strict as the various specs (1, 2) want you to do strict simple string matches for the issuer. I think it should be MAS and Synapse which need to make sure the configuration is right instead of making the client less strict about this. It doesn't help though that the apps give a cryptic error in those cases :(

I've just gone one step towards making those errors less common by including a check in the mas-cli doctor tool, with guidance to help resolve the configuration issue. The other thing we should do is make Synapse discover the right "issuer" value instead of just outputting it as-is in the well-known

matrixbot commented 1 month ago

This comment was originally posted by @poljar at https://github.com/matrix-org/matrix-authentication-service/issues/2352#issuecomment-1941151460.

This is a misconfiguration by the user IMO. We're intentionally being strict as the various specs (1, 2) want you to do strict simple string matches for the issuer.

As far as I understand, they want you to do simple string matching of the URL after the URL has been normalized:

This comparison MUST use simple string comparison as defined in Section 6.2.1 of [RFC3986].

Then if we look at Section 6.2.1:

False negatives are caused by the production and use of URI aliases. Unnecessary aliases can be reduced, regardless of the comparison method, by consistently providing URI references in an already- normalized form (i.e., a form identical to what would be produced after normalization is applied, as described below).

Finally we look at the ways a URI can be normalized in section 6.2.3:

In general, a URI that uses the generic syntax for authority with an empty path should be normalized to a path of "/".

It even lists that these two are equivalent: