panva / openid-client

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

Unable to send PAR using private_key_jwt to an MTLS endpoint #486

Closed cruskit closed 2 years ago

cruskit commented 2 years ago

Describe the bug When trying to send a Pushed Authorization Request to a PAR endpoint that uses private_key_jwt authentication and is also requires MTLS, the openid-client will always select the non-mtls version of the endpoint.

In client.js, pushedAuthorizationRequest calls authenticatedPost as follows - note pushed_authorization_request passed as endpoint value:

    const response = await authenticatedPost.call(
      this,
      'pushed_authorization_request',
      {
        responseType: 'json',
        form: body,
      },
      { clientAssertionPayload, endpointAuthMethod: 'token' },
    );

Then in helpers/client.js:authenticatedPost(), the check is as follows:

async function authenticatedPost(
  endpoint,
  opts,
  { clientAssertionPayload, endpointAuthMethod = endpoint, DPoP } = {},
) {
  const auth = await authFor.call(this, endpointAuthMethod, { clientAssertionPayload });
  const requestOpts = merge(opts, auth);

  const mTLS =
    this[`${endpointAuthMethod}_endpoint_auth_method`].includes('tls_client_auth') ||
    (endpoint === 'token' && this.tls_client_certificate_bound_access_tokens);

For the mTLS check, if using private_key_jwt for authentication, the first condition will be false. For a PAR the second condition will always be false as pushed_authorization_request was passed in as the endpoint value which can never === 'token'. As a result mTLS can never be true for a PAR using private_key_jwt.

Suggest the check may need to be along lines of:

  const mTLS = this[`${endpointAuthMethod}_endpoint_auth_method`].includes('tls_client_auth')
    || ( (endpoint === 'token' || endpoint === 'pushed_authorization_request' ) && this.tls_client_certificate_bound_access_tokens);

To Reproduce Issuer and Client configuration: (inline or gist) - Don't forget to redact your secrets.

// Issuer configuration (issuer.metadata) and how it is constructed (discovery or manual?) Discovery via .well-known:

{
  "issuer": "https://demo.certification.openid.net/test/a/pr-rp-sample-app-test/",
  "authorization_endpoint": "https://demo.certification.openid.net/test/a/pr-rp-sample-app-test/authorize",
  "token_endpoint": "https://demo.certification.openid.net/test/a/pr-rp-sample-app-test/token",
  "jwks_uri": "https://demo.certification.openid.net/test/a/pr-rp-sample-app-test/jwks",
  "registration_endpoint": "https://demo.certification.openid.net/test/a/pr-rp-sample-app-test/register",
  "userinfo_endpoint": "https://demo.certification.openid.net/test/a/pr-rp-sample-app-test/userinfo",
  "mtls_endpoint_aliases": {
    "token_endpoint": "https://demo.certification.openid.net/test-mtls/a/pr-rp-sample-app-test/token",
    "registration_endpoint": "https://demo.certification.openid.net/test-mtls/a/pr-rp-sample-app-test/register",
    "userinfo_endpoint": "https://demo.certification.openid.net/test-mtls/a/pr-rp-sample-app-test/userinfo",
    "pushed_authorization_request_endpoint": "https://demo.certification.openid.net/test-mtls/a/pr-rp-sample-app-test/par"
  },
  "response_types_supported": [
    "code"
  ],
  "authorization_response_iss_parameter_supported": true,
  "scopes_supported": [
    "openid"
  ],
  "claims_parameter_supported": true,
  "claims_supported": [
    "name",
    "given_name",
    "family_name",
    "email",
    "birthdate",
    "phone_number",
    "address"
  ],
  "subject_types_supported": [
    "pairwise"
  ],
  "id_token_signing_alg_values_supported": [
    "PS256"
  ],
  "request_object_signing_alg_values_supported": [
    "PS256",
    "ES256"
  ],
  "tls_client_certificate_bound_access_tokens": true,
  "token_endpoint_auth_methods_supported": [
    "private_key_jwt"
  ],
  "pushed_authorization_request_endpoint": "https://demo.certification.openid.net/test/a/pr-rp-sample-app-test/par",
  "require_pushed_authorization_requests": true,
  "token_endpoint_auth_signing_alg_values_supported": [
    "PS256",
    "ES256"
  ]
}

// Client configuration (client.metadata) and how it is constructed (fromUri or manual?) Manual:

  client: {
    client_id: '...',
    organisation_id: '...',
    jwks_uri:
      'https://...../application.jwks',
    redirect_uris: [
      'https://tpp.localhost/cb',
    ],
    organisation_name: 'Test Org',
    organisation_number: 'ABN123123123',
    software_description: 'This is an amazing application that will change your life.',

    application_type: 'web',
    grant_types: ['client_credentials', 'authorization_code', 'implicit'],
    id_token_signed_response_alg: 'PS256',
    post_logout_redirect_uris: [],
    require_auth_time: false,
    response_types: ['code id_token', 'code'],
    subject_type: 'public',
    token_endpoint_auth_method: 'private_key_jwt',
    introspection_endpoint_auth_method: 'private_key_jwt',
    revocation_endpoint_auth_method: 'private_key_jwt',
    request_object_signing_alg: 'PS256',
    require_signed_request_object: true,
    require_pushed_authorization_requests: true,
    authorization_signed_response_alg: 'PS256',
    tls_client_certificate_bound_access_tokens: true,
    backchannel_user_code_parameter: false,
    scope: 'openid',
    software_roles: ['BIS'],
  },

Steps to reproduce the behaviour:

  1. Ensure token_endpoint_auth_method is private_key_jwt, and tls_client_certificate_bound_access_tokens = true
  2. Create a request object and make a call to pushedAuthorizationRequest
  3. The client will always call the non MTLS PAR endpoint.

Expected behaviour With token_endpoint_auth_method set to private_key_jwt and tls_client_certificate_bound_access_tokens = true it should use the MTLS PAR endpoint

Environment:

Additional context Add any other context about the problem here.

panva commented 2 years ago

The client is behaving correctly and this exact behaviour is also certified using the certification suite.

The PAR endpoint is not issuing tokens, ergo the tls_client_certificate_bound_access_tokens has no effect on the behaviour. And you're using private_key_jwt which, likewise, does not preclude the use of an mtls_endpoint_aliases listed endpoint.

What issue are you facing exactly?

cc @jogu

cruskit commented 2 years ago

In our ecosytem we want to require the client to use the MTLS endpoint when performing the PAR. I can't work out / understand the configuration required so that the openid-client will use the MTLS version of the PAR endpoint - it always seems to call the non-mtls version of the endpoint.

RalphBragg commented 2 years ago

Paul,

Filip is correct in his implementation (I’ve never know him not to be). Par is not a token issuing endpoint and so that flag isn’t required. Technically par also doesn’t require the use of mtls when using private key jwt so this behaviour is correct.

I believe the correct behaviour is if a bank wants mtls to enforced on this endpoint is to require it on the standard endpoint.

Now this is where it gets potentially more complicated, @jogu. There’s nothing in the spec that says a bank can’t require mtls for this endpoint but when I last tried it the conformance suite refused to honour the mtls requirement on the non mtls endpoint key and failed.

So the question is, should the client always utilise the mtls endpoints even if using private_key_jwt or should the conformance suite honour a requirement to use mtls if it's not strictly required on the standard par endpoint.

RB

panva commented 2 years ago

In our ecosystem we want to require the client to use the MTLS endpoint when performing the PAR.

You'll need to use one the MTLS client authentication methods. Otherwise there's no need to use the MTLS-specific PAR endpoint.

panva commented 2 years ago

Or you can just not have aliases for this endpoint altogether. Providing the mtls specific request options (key, cert, etc) will then be honoured.

panva commented 2 years ago

I've labeled this as https://github.com/panva/node-openid-client/labels/wontfix, despite there being nothing to "fix" since this behaviour is expected.

If ecosystems want to do wonky stuff like requiring MTLS always despite there being no reason for it (issuing tokens, client auth), they shall not use an aliased endpoint.

cruskit commented 2 years ago

Thanks Filip and Ralph for the feedback.