IdentityPython / oidc-op

An implementation of an OIDC Provider (OP)
Apache License 2.0
64 stars 26 forks source link

Remove client_authn_method from provider #198

Closed nsklikas closed 1 year ago

nsklikas commented 2 years ago

Remove the client_authn_method from the provider info endpoint. It is not described in the spec and it is pretty buggy, as each endpoint can over-write the methods of the other endpoints, which means that the result relies on the iteration order.

If we want to expose the client authentication methods supported for the other endpoints, we should do it per endpoint, e.g. introspection_endpoint_auth_methods_supported (like token_endpoint_auth_methods_supported, which is the only one described in the discovery specification)

rohe commented 2 years ago

I think we should spend some time on making get_capabilities better, rather than this fix. client_authn_method should be local to an endpoint and not be exposed to the outside unless in cases like token_endpoint_auth_methods_supported. I think that we can bind provider configuration parameters to specific endpoints or have them being server wide. If the later then the endpoint configuration should not be able to set those parameters.

nsklikas commented 2 years ago

You are right that this PR does not address the true problem with the capabilities generation. Perhaps each endpoint should have a method that will generate it's capabilities and then the server will fetch/merge all of them?

What happens right now is similar to that but the endpoint capabilities are overloaded, as they are used to both configure the endpoints behavior, with client_authn_method, and to create the endpoint's capabilities that are exposed in the discovery endpoint. IMO the user should have to configure the endpoint and then the endpoints capabilities should be generated from the provided configuration, which then the server will merge to create the output of the discovery endpoint. We could also add an extra field to the endpoints for overriding the discovery endpoint's capabilities, in case the user wants some custom behavior.

rohe commented 2 years ago

My thoughts are that the endpoints should register their capabilities with the server. Probably best implemented with the callback method you talked about. Which provider configuration parameters that belongs to which endpoint is defined in the code. These parameters may have default values which depends on the capabilities of the software package but are ultimately set by configuration. Setting the provider configuration parameters should probably best be kept apart from other endpoint capabilities (like client_authn_methods).