node-solid-server icon indicating copy to clipboard operation
node-solid-server copied to clipboard

Test for trialing slashes in value for 'issuer' from '/.well-known/openid-configuration' responses

Open pmcb55 opened this issue 4 years ago • 4 comments

Basically our ESS Brokers are advertising themselves (in their /.well-known/openid-configuration responses) as issuers with a trailing slash, e.g. for https://broker.pod.inrupt.com/.well-known/openid-configuration:

{
    "introspection_endpoint": "https://broker.pod.inrupt.com/introspect",
    "scopes_supported": [
        "openid",
        "offline_access",
        "webid"
    ],
    "issuer": "https://broker.pod.inrupt.com/",
    :

Note the trailing slash on the issuer URL (which is technically (slightly!) more correct than having the URL without the trailing slash). So it seems NSS might be blindly taking that issuer value, and appending the string literal /.well-known/openid-configuration to it, without first checking if the issuer value has a trailing slash or not. If the URL constructed has two slashes, then the server responds with it’s standard HTML ‘Page Not Found’ response, which results in the ‘invalid json response…’ error.

If this is correct, then the fix in NSS should be simply checking for a trailing slash before appending /.well-known/openid-configuration.

pmcb55 avatar Nov 09 '20 17:11 pmcb55

While the official spec doesn't explicitly forbid using a trailing slash as long as it matches the iss claim (see the section here https://openid.net/specs/openid-connect-discovery-1_0.html#ProviderMetadata), I would argue that the trailing slash should not be used by other implications of the spec.

The example provided in the specification does not have a trailing slash (https://openid.net/specs/openid-connect-discovery-1_0.html#ProviderConfigurationResponse) and other openid services like google do not have it either (https://accounts.google.com/.well-known/openid-configuration)

As you've pointed out, the main problem with including a trailing slash comes from generating the well-known uri. In section 4 of the spec (https://openid.net/specs/openid-connect-discovery-1_0.html#ProviderConfig), it reads:

OpenID Providers supporting Discovery MUST make a JSON document available at the path formed by concatenating the string /.well-known/openid-configuration to the Issuer.

Concatinating /.well-known/openid-configuration to an issuer with a trailing slash like https://broker.pod.inrupt.com/ would cause a double slash like https://broker.pod.inrupt.com//.well-known/openid-configuration

Consider the "issuer" to be the origin of the identity provider. For cookie cookie reasons, an identity provider should not be deployed at a path (like https://example.com/identiyProvider/.well-known/openid-configuration). This would also violate section 3 of the well-known uri spec (https://tools.ietf.org/html/rfc5785#section-3) which states:

A well-known URI is a URI [RFC3986] whose path component begins with the characters "/.well-known/"

I have not seen any mention that clients must first check for a trailing slash in the spec.

jaxoncreed avatar Nov 09 '20 20:11 jaxoncreed

Section 4.1 of the spec also explicitly mentions that the trailing slash should be removed:

If the Issuer value contains a path component, any terminating / MUST be removed before appending /.well-known/openid-configuration. The RP would make the following request to the Issuer https://example.com/issuer1 to obtain its Configuration information, since the Issuer contains a path component:

I'm not sure if the mere presence of a trailing slash qualifies as a "path component", but it would make sense to me that the presence of the slash is compliant with the spec. I don't have strong feelings one way or the other though :), and the fact that established IdPs don't include this slash may be a motivation to not include it, because the same issue may happen with other systems.

On a side note, the iss claim of the tokens matches the openid-config retrieved at the .well-known IRI.

NSeydoux avatar Nov 10 '20 16:11 NSeydoux

@jaxoncreed the OIDC spec is pretty clear on this point:

If the Issuer value contains a path component, any terminating / MUST be removed before appending /.well-known/openid-configuration.

There are non-Solid, production implementations of OIDC that use trailing slashes for issuers in the wild, so it would be incorrect to draw conclusions from non-normative text, especially when the normative text is clear that such behavior (a trailing slash) is allowed.

acoburn avatar Nov 10 '20 17:11 acoburn

Cool I missed that.

jaxoncreed avatar Nov 11 '20 13:11 jaxoncreed