Issue #142.

[sozkan]

I added checks to make sure that a mutually supported token endpoint authn method is used. Also updated some test flows and added checks to make sure that the required client authentication method is supported.

OP-ClientAuth-Basic-Static and OP-ClientAuth-Basic-Dynamic now return PARTIAL_RESULT with "No support for: token_endpoint_auth_methods_supported=client_secret_basic" error when client_secret_basic method is not supported. We should clarify if they should return a failure or not.

How I tested it :

• I used the node-oidc-provider
• I edited example/setttings.js to enable/disable specific client authentication methods and to create static clients. For example :

  ...
  module.exports.config = {
  tokenEndpointAuthMethods: ['private_key_jwt'],  //<<<add this line
  acrValues: ['urn:mace:incommon:iap:bronze'],
  ...

• I tested with both dynamically registered clients and static clients.
• For example, enabled only client_secret_post in OP config and ran OP-ClientAuth-*-Dynamic tests.

Before this change when the OP does not support client_secret_basic, many tests, for example OP-ClientAuth-Any-Dynamic fail with an error similar to the following :

Traceback (most recent call last):
  File "/usr/local/lib/python3.6/dist-packages/otest-0.7.3-py3.6.egg/otest/aus/tool.py", line 87, in run_flow
    resp = _oper()
  File "/usr/local/lib/python3.6/dist-packages/otest-0.7.3-py3.6.egg/otest/operation.py", line 105, in __call__
    res = self.run(*args, **kwargs)
  File "/usr/local/lib/python3.6/dist-packages/oidctest-0.7.5-py3.6.egg/oidctest/op/oper.py", line 133, in run
    **self.req_args)
  File "/usr/local/lib/python3.6/dist-packages/otest-0.7.3-py3.6.egg/otest/operation.py", line 171, in catch_exception_and_error
    res = func(**kwargs)
  File "/usr/local/lib/python3.6/dist-packages/oic-0.14.0-py3.6.egg/oic/oic/__init__.py", line 1386, in register
    return self.handle_registration_info(rsp)
  File "/usr/local/lib/python3.6/dist-packages/oic-0.14.0-py3.6.egg/oic/oic/__init__.py", line 1280, in handle_registration_info
    raise RegistrationError(resp.to_dict())
oic.exception.RegistrationError: {'error': 'invalid_client_metadata', 'error_description': 'token_endpoint_auth_method must be one of [private_key_jwt]'}

Before this change, an OP which only supports private_key_jwt method would not be able to pass conformance tests.

[sozkan]

@selfissued, this pull request is related to https://github.com/openid-certification/oidctest/issues/142 and your inputs would be appreciated :

Before this code change, when OP configuration contained only private_key_jwt under token_endpoint_auth_methods_supported, many tests would fail because oidctest would default to client_secret_basic in many cases, without taking token_endpoint_auth_methods_supported into account. This pull request will change this behavior.

In my opinion, if you only support private_key_jwt then you should not be required to support client_secret_basic because the spec (section 2.3.1 of RFC 6749) reads :

The authorization server MUST support the HTTP Basic authentication scheme for authenticating clients that were issued a client password.

So if you are not issuing any secrets then you should not need to support client secret basic to be compliant. Is my understanding correct? After this code change an OP that only supports private_key_jwt token endpoint auth method will be able to pass certification tests.

@rohe let's wait for @selfissued 's feedback before merging code changes.

[travisspencer]

If the OP supports private_key_jwt, it had to have given the client a secret:

client_secret_jwtClients that have received a client_secretvalue from the Authorization Server create a JWT using an HMAC SHA algorithm, such as HMAC SHA-256.

Therefore, the OP must also support HTTP basic auth and form post with that secret, not just as a signed JWT.

[sozkan]

@travisspencer why does the OP have to give a secret to a client that is going to use private_key_jwt?

If the OP supported client_secret_jwt then you would be right, but my question is about an OP that only supports private_key_jwt which does not require client secrets.

[travisspencer]

Ah, no. You're right, Özkan. I conflated client_secret_jwt and private_key_jwt in mind even as I read one and typed the other. If the OP only supports private_key_jwt, then I agree that no secrets are given to a client and the OP shouldn't have to support basic auth or form post auth. If your PR makes that use case possible to certify, it seems like a good one IMO.

[sozkan]

What's proposed for oidctest/op/oper.py around line 146 is I think already done in pyoidc. So, I don't think it has to be redone here. Similar argumentation for the proposal around line 219. pyoidc should do this for you !

It doesn't work without these changes so I don't think pyoidc is doing it for us.

If this isn't the case then pyoidc should probably be fixed.

So how do you want to do this? Do you want to cancel this pull request and fix it in pyoidc?

[rohe]

On 13 Jan 2019, at 11:06, Serkan Özkan notifications@github.com wrote:

What's proposed for oidctest/op/oper.py around line 146 is I think already done in pyoidc. So, I don't think it has to be redone here. Similar argumentation for the proposal around line 219. pyoidc should do this for you !

It doesn't work without these changes so I don't think pyoidc is doing it for us.

OK If this isn't the case then pyoidc should probably be fixed.

So how do you want to do this? Do you want to cancel this pull request and fix it in pyoidc?

The quick fix I guess is to go with the PR and then look at why pyOIDC doesn’t do it. So I’ll accept the PR.

— Roland

It is curious that physical courage should be so common in the world, and moral courage so rare. -Mark Twain, author and humorist (30 Nov 1835-1910)