panva / openid-client

OAuth 2 / OpenID Connect Client API for JavaScript Runtimes
MIT License
1.83k stars 392 forks source link

Issuer.discover is returning a spread string #378

Closed ktikku closed 3 years ago

ktikku commented 3 years ago

Describe the bug The request call inside the Issuer.discover method is returning a string instead of the object, which in turn is getting spread by the spread operator.

code from issuer.js

 const response = await request.call(this, {
          method: 'GET',
          responseType: 'json',
          url: wellKnownUri,
        });
        const body = processResponse(response);

The body is coming as string instead of object as expected. Which is then getting spread in the below code, hence the bug.

return new Issuer({
          ...ISSUER_DEFAULTS,
          ...body,
          [AAD_MULTITENANT]: !!AAD_MULTITENANT_DISCOVERY.find(
            (discoveryURL) => wellKnownUri.startsWith(discoveryURL),
          ),
        });

To Reproduce Well Known Configuration link generated by the openid-client package: https://login.microsoftonline.com/18a59a81-eea8-4c30-948a-d8824cdc2580/v2.0/.well-known/openid-configuration Issuer and Client configuration: (inline or gist) - Don't forget to redact your secrets.



Issuer is constructed by discovery.

Steps to reproduce the behaviour:

1.  call `Issuer.discover` method and pass the issuer url. 
`Issuer.discover('https://login.microsoftonline.com/18a59a81-eea8-4c30-948a-d8824cdc2580/v2.0')`

**Expected behaviour**
`Issuer.discover` returns an object containing Issuer and it's metadata.

**Environment:**
 - openid-client version: [e.g. v4.7.4]
 - node version: [e.g. v12.16.2]

**Additional context**
Add any other context about the problem here.

 - [x] the bug is happening on latest openid-client too.
 - [x] i have searched the issues tracker on github for similar issues and couldn't find anything related.
panva commented 3 years ago

@ktikku this works just fine, it's one of the core functionalities and it is well tested. I am unable to reproduce your issue. Issuer.discover as documented returns a Promise that is resolved with an Issuer instance.

❯ node         
Welcome to Node.js v12.16.2.
Type ".help" for more information.
> const { Issuer } = require('openid-client')
> Issuer.discover("https://login.microsoftonline.com/18a59a81-eea8-4c30-948a-d8824cdc2580/v2.0").then(console.log)
> Issuer {
  authorization_endpoint: 'https://login.microsoftonline.com/18a59a81-eea8-4c30-948a-d8824cdc2580/oauth2/v2.0/authorize',
  claim_types_supported: [
    'normal'
  ],
  claims_parameter_supported: false,
  claims_supported: [
    'sub',
    'iss',
    'cloud_instance_name',
    'cloud_instance_host_name',
    'cloud_graph_host_name',
    'msgraph_host',
    'aud',
    'exp',
    'iat',
    'auth_time',
    'acr',
    'nonce',
    'preferred_username',
    'name',
    'tid',
    'ver',
    'at_hash',
    'c_hash',
    'email'
  ],
  cloud_graph_host_name: 'graph.windows.net',
  cloud_instance_name: 'microsoftonline.com',
  device_authorization_endpoint: 'https://login.microsoftonline.com/18a59a81-eea8-4c30-948a-d8824cdc2580/oauth2/v2.0/devicecode',
  end_session_endpoint: 'https://login.microsoftonline.com/18a59a81-eea8-4c30-948a-d8824cdc2580/oauth2/v2.0/logout',
  frontchannel_logout_supported: true,
  grant_types_supported: [
    'authorization_code',
    'implicit'
  ],
  http_logout_supported: true,
  id_token_signing_alg_values_supported: [
    'RS256'
  ],
  issuer: 'https://login.microsoftonline.com/18a59a81-eea8-4c30-948a-d8824cdc2580/v2.0',
  jwks_uri: 'https://login.microsoftonline.com/18a59a81-eea8-4c30-948a-d8824cdc2580/discovery/v2.0/keys',
  kerberos_endpoint: 'https://login.microsoftonline.com/18a59a81-eea8-4c30-948a-d8824cdc2580/kerberos',
  msgraph_host: 'graph.microsoft.com',
  rbac_url: 'https://pas.windows.net',
  request_parameter_supported: false,
  request_uri_parameter_supported: false,
  require_request_uri_registration: false,
  response_modes_supported: [
    'query',
    'fragment',
    'form_post'
  ],
  response_types_supported: [
    'code',
    'id_token',
    'code id_token',
    'id_token token'
  ],
  scopes_supported: [
    'openid',
    'profile',
    'email',
    'offline_access'
  ],
  subject_types_supported: [
    'pairwise'
  ],
  tenant_region_scope: 'NA',
  token_endpoint: 'https://login.microsoftonline.com/18a59a81-eea8-4c30-948a-d8824cdc2580/oauth2/v2.0/token',
  token_endpoint_auth_methods_supported: [
    'client_secret_post',
    'private_key_jwt',
    'client_secret_basic'
  ],
  userinfo_endpoint: 'https://graph.microsoft.com/oidc/userinfo'
}
warrickw commented 3 years ago

I don't have anything helpful to add reproduction wise, but I'm experiencing this too. This makes the resolved discoverer useless since it's not correctly loading properties like authorization_endpoint.

I suspect this is a problem for me due to my use and/or configuration of typescript. Other parts of the library are behaving erratically, even if I sidestep this issue by manually initializing an Issuer.

image

ktikku commented 3 years ago

Hi @panva, Thanks for your reply 😄

I further investigated the issue. I did my setup again (installing dependencies) and tried. I noticed a couple of things:

Well known urls configured by module:


1. https://login.microsoftonline.com/18a59a81-eea8-4c30-948a-d8824cdc2580/v2.0/18a59a81-eea8-4c30-948a-d8824cdc2580/v2.0/.well-known/openid-configuration
2. https://login.microsoftonline.com/18a59a81-eea8-4c30-948a-d8824cdc2580/v2.0/.well-known/oauth-authorization-server/18a59a81-eea8-4c30-948a-d8824cdc2580/v2.0 

url (2) is invalid.
  1. the library is creating a couple of well known metadata uris and one of those aren't valid (url number 2), hence the request is failing
  2. also, for the valid well known metadata url, I'm getting the following error

OPError: expected 200 OK with body but no body was returned

Please find below the aggregate error:

AggregateError: Issuer.discover() failed.
    OPError: expected 200 OK with body but no body was returned
    OPError: expected 200 OK, got: 404 Not Found
    at Function.discover (/Users/karantikku/Work/services-repos/CIRR_OIDC_AUTH_MODULE/node_modules/openid-client/lib/issuer.js:265:17)
    at processTicksAndRejections (internal/process/task_queues.js:97:5)

I'm using node 14 v14.0.0 and openid-client package version: 4.7.4

Please let me know if you need more information from me @panva 😄

panva commented 3 years ago

@ktikku I can reproduce the issue, but it seems to be related to a node.js version, not the library. Upgrading to latest v14 (v14.16.0) the issue is gone.

Also note that the engines version actually covers this problem as the minimum v14 is 14.2.0. It must've been a known issue I just don't remember.

"node": "^10.19.0 || >=12.0.0 < 13 || >=13.7.0 < 14 || >= 14.2.0"
ktikku commented 3 years ago

ya, just checked, it's working fine now with node 14.2.0. Thanks for the assistance @panva 😄