doorkeeper-gem / doorkeeper-openid_connect

OpenID Connect extension for Doorkeeper
MIT License
173 stars 114 forks source link

Using `root_url` in `#webfinger_response` can violate specification #171

Open sato11 opened 2 years ago

sato11 commented 2 years ago

The documentation says in 2. OpenID Provider Issuer Discovery:

The Issuer location MUST be returned in the WebFinger response as the value of the href member of a links array element with rel member value http://openid.net/specs/connect/1.0/issuer.

which then becomes the premise of 4. Obtaining OpenID Provider Configuration Information:

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.

It seems it is assumed that our href must be exactly what our issuer configuration points to. And since issuer can be configured so that it does not always be the root_url, I feel #webfinger_response should return issuer value and not the root_url.

For example, when issuer is configured like this;

Doorkeeper::OpenidConnect.configure do
  issuer 'https://example.com/issuer1'
end

which is justified by definition, quoting from readme:

issuer: Identifier for the issuer of the response (i.e. your application URL). The value is a case sensitive URL using the https scheme that contains scheme, host, and optionally, port number and path components and no query or fragment components.

the response should be like this, provided that we have provider configuration available at https://example.com/issuer1/.well-known/openid-configuration.

{
  "subject": "test@example.com",
  "links": [
    {
      "rel": "http://openid.net/specs/connect/1.0/issuer",
      "href": "https://example.com/issuer1"
    }
  ]
}
calleluks commented 1 week ago

We had the same issue and merged #172 into the fork we maintain with great success. Thanks for doing the work @sato11! I hope this can also be merged upstream.