IdentityModel / oidc-client-js

OpenID Connect (OIDC) and OAuth2 protocol support for browser-based JavaScript applications
Apache License 2.0
2.43k stars 841 forks source link

feat(MetadataService): add metadata_override #1295

Closed oklas closed 3 years ago

oklas commented 3 years ago

The OpenID providers describes configuration exactly as is. But sometimes it is requires to be override. From #1068 customisation of providers options is no more available. This solution is save #1068 behaviour and adds ability to override metadata of OpenID configuration.

So if token_endpoint need to be overrided it is may be done like this:

  const userManager = new UserManager({
    authority: issuer,
    ...
    //metadata,
    metadataOverride: {
      token_endpoint: tokenUrl,
    },
  })
brockallen commented 3 years ago

Can you give an example why you need to override the metadata?

oklas commented 3 years ago

That is implementation of confidential client type (RFC 6749 - 2.1). The code snippet above overrides token endpoint to receive token via client as demonstrated in authorization code grant scheme RFC 6749 - 4.1:

image
brockallen commented 3 years ago

That is implementation of confidential client type (RFC 6749 - 2.1). The code snippet above overrides token endpoint to receive token via client as demonstrated in authorization code grant scheme RFC 6749 - 4.1:

I don't follow your comment, and the parts that I can understand don't seem to explain the code in this PR.

oklas commented 3 years ago

It is mostly about client_secret also known as client password or google call it client key. Judging by the name, this thing should be specially protected. Moreover the github interface does not allow even the owner to see it again. It show it only once during creation only:

image

So I do not want to embed client_secret to web page so everybody can get it just run something like this:

% curl site.com | grep 'client_secret'

From this It is clear client_secret will be only in the client app on a server (not in the web app).

Docs of this module does not mention client_secret as required (same as another modules).

But the token endpoint at auth servers declares client_secret as required.

What is actually the answer from token endpoints of auth servers without client_secret:

Google requires it:

image

Github does not suppose access from browser at all:

image

So to access token endpoint we need to do request with client_secret (which is available only to the client app) - requests must be done from client app. Client App is a square with word Client on the schema above. The module oidc-client is in the square with words "User-Agent".

The user-agent (actualy oidc-client module) must not access for token to Authorization Server (see schema) but to the Client App. And already on server the client app adds the client_secret to request and pass it to Authorization Server.

To receive access token must be used our Client App responsible url (instead of Auth Server url) as demonstrated in schema above. That is the reason why token_endpoint must be overrided.

brockallen commented 3 years ago

Docs of this module does not mention client_secret as required (same as another modules).

This library is not meant to be used server-side. From the About:

OpenID Connect (OIDC) and OAuth2 protocol support for browser-based JavaScript applications

oklas commented 3 years ago

Docs of this module does not mention client_secret as required (same as another modules).

This library is not meant to be used server-side. From the About:

There is no mentions or requirements above to run this library on server side (for server side is openid-client if need).

OpenID Connect (OIDC) and OAuth2 protocol support for browser-based JavaScript applications

There is no contradictions here.

brockallen commented 3 years ago

I guess I still don't follow. If you have a STS that requires a client secret, even for a public client, then it's not really a secret and there's no concern with just setting it in the config (which is supported). And I don't see how that affects the discovery document. So I'm missing why those two concepts are conflated in this PR.

So I'll try to ask again -- why do you find you need to change the discovery document from the STS your client app trusts?

oklas commented 3 years ago

why do you find you need to change the discovery document from the STS your client app trusts?

It depends on how library is considering the object returned by userManager._settings.metadata::getMetadata()

If that object is an exact mirror of the discovery document just for to be immutable then additional logic or additional function that return overrided metadata or special endpoint getters must be added. And all the library must be rewritten to use that. (Why to keep copy and to have additional logic when all the library works with modified settings?)

Actually the location of mentioned object is _settings. The library uses it as API description. The API for browser point is different from discovery document, because as mentioned before at the token endpoint (and the other client_secret dependent endpoints) requests must be done through our trusted client server that has client_secret.

oklas commented 3 years ago

As of now currently, we have moved from using this excellent library (found in some examples closest to the required cases) to a fully server-side solution. The user interacts only with the provider's authorization page. Maybe it will help someone. I noticed that someone is asking the same questions. So at the moment this PR is no longer required, but it is unclear will this need it in the future. Is it necessary to work like this? I hope someone shares their experience here.

brockallen commented 3 years ago

I still don't understand the need to change the STS' discovery doc... and since you've found a server-side library, then I think it's safe to close this unless I get more requests (and rationale). Thanks.