openid / federation

9 stars 4 forks source link

Supported RP/client metadata parameters #12

Open rohe opened 4 months ago

rohe commented 4 months ago

The metadata specification for OIDC RP (https://openid.net/specs/openid-connect-registration-1_0.html) and OAuth2 clients (https://www.rfc-editor.org/rfc/rfc7591.html) contains claims that specifies which of a set of values an RP/client prefers.

Take for instance token_endpoint_auth_method. This claim specifies the requested authentication method for the token endpoint. There are no choices here. The RP/client may support many but it has to choose one.

This works in a context where one OP/Client explicitly registers with one OP/AS. It doesn't work so well in a federation context where the RP/client may be part of more then one federation (with different policies) or even if just a member of one federation and connected to more than one OP/AS using automatic registration.

In this context it would rather like to specifies which methods it supports rather than choosing one of them.

I would therefore propose that the claim client_registration_types which is the only metadata claim added for an RP by this specification should be changed to be client_registration_type (no -s) and furthermore to be accompanied by client_registration_types_supported.

The meaning should be that the RP supports all what is specified in client_registration_types_supported but would prefer to use what is in client_registration_type if possible.

This is just a first step. After this I'd like to propose extensions to the OIDC RP and OAuth2 client metadata specifications to add _supported claims where appropriate but that should be handled elsewhere.

selfissued commented 3 months ago

We could do this in the Federation spec before it becomes final. Note that there are no current venues for extending OpenID Connect or OAuth 2 Client Metadata directly, so if we need this, OpenID Federation 1.0 is our ship vehicle.

selfissued commented 3 months ago

This is essentially a duplicate of https://bitbucket.org/openid/connect/issues/2158/metadata-parameter-value-arrays-for-rp .

selfissued commented 3 months ago

This was discussed on the 5-Aug-24 working group call. It's clear that we could do this and that it would be useful in contexts where a party other than the RP is making some of the choices.

It's an open question whether to do this in the Federation spec itself or as a separate extension to OpenID Connect Dynamic Client Registration metadata.

Razumain commented 3 months ago

Mike, thanks for pointing out that this discussion was here. I had this discussion earlier with Roland and he mentioned that he had brought it up here.

I like the "_supported" extension to single valued metadata parameters.

I agree with Roland that updates to metadata parameters should be handled outside of the Federation specification. The federation services process declared metadata in a fashion that is agnostic to what that metadata is. And I would hope that it stays that way. For this reason I think it is cleaner long term to fix metadata where metadata is defined.

But I support any solution, as long as it works.

selfissued commented 2 months ago

I am creating a new separate specification to achieve this, per the results of the discussion on the 8-Aug-24 OpenID Connect working group call.

selfissued commented 1 month ago

openid-connect-rp-metadata-choices-1_0.zip contains source and HTML output for a spec defining client metadata parameters corresponding to all the single-valued OpenID Connect Dynamic Client Registration metadata parameters.

msalle commented 1 month ago

openid-connect-rp-metadata-choices-1_0.zip contains source and HTML output for a spec defining client metadata parameters corresponding to all the single-valued OpenID Connect Dynamic Client Registration metadata parameters.

In principle this looks very good and useful, I'm only wondering about @rohe 's point about preference in the original issue:

The meaning should be that the RP supports all what is specified in client_registration_types_supported but would prefer to use what is in client_registration_type if possible. in the new spec it's just stated they MUST be in the array?

Related to that, would the order in the arrays have a special meaning? I do think there could be use-cases for having a preferred entry, or a preferred order?

jogu commented 4 weeks ago

Building upon Brian's point in https://bitbucket.org/openid/connect/issues/2158/metadata-parameter-value-arrays-for-rp - I'm unclear how the suggested spec solves some of the problems:

For example when using the various requestobject values and automatic registration, I'm not sure any of those values have meaning - there is one set of values that the OP must either accept or reject, and those are the ones that have been used in the request object that triggers automatic registration. Adding extra metadata does not appear to help other than a validation that the alg that is used is actually one the client intended to use.

Similarly but slightly differently, for token_endpoint_auth_methods_supported + automatic registration, there are two possible flows:

  1. PAR: The AS must accept the token_endpoint_auth_method the client has actually used [as the client has to authenticate at the PAR endpoint using the deemed token_endpoint_auth_method]. (and similarly to the above, the new metadata can only be used as a check that the used method is one the client intended to use)
  2. Non-PAR: There isn't enough information for the AS to decide which token_endpoint_auth_methods_supported to register the client with. If both the client and the AS support both private_key_jwt and mtls, the server can register the client using one the client supports, but I believe there's no way for the AS to tell the client which method it registered it with so the client doesn't know how to authenticate at the token endpoint? The AS would essentially have to register the client with multiple methods then refine the registration when it sees which value the client actually uses?
jogu commented 3 weeks ago

Mandating PAR would make the problem more solvable I think, and arguably would be a good thing for privacy and a bit of a simplification both for people trying to implement the spec and those trying to understand it.

rohe commented 3 weeks ago

First, I can't see that Brian has made any comment in the issue mentioned so I don't know what he said.

Secondly, yes the RP is very much in the driving seat when automatic registration is used but I can't see how that invalidates the usefulness of the proposed spec in other use cases.

The use case we try to solve is that an entity may want to be part of more than one federation, each having their own metadata policy, and it should be able to do so as long as it can accept the federations metadata policy. Since the entity publishes its metadata at the .well-known endpoint it can not know and based on that modify the metadata depending on who is looking at the metadata. Imaging that you have an OAuth2 client that can do both client_secret_post and client_secret_basic but must chose one because the token_endpoint_auth_method parameter is single value. This means it can not be a member of a federation that chooses to accept the one that the client didn't choose to publish. Not because it wasn't able to use the method but because the metadata specification says it can only specify one method. So we would like the client to be able to express all it can do allowing the client to be a member of federations that between them have incompatible policies while each of these policies a totally compatible with what the client can do and accept.

jogu commented 3 weeks ago

First, I can't see that Brian has made any comment in the issue mentioned so I don't know what he said.

The 1st October comment from Mike on https://bitbucket.org/openid/connect/issues/2158/metadata-parameter-value-arrays-for-rp records the point Brian made in one of the calls.

Secondly, yes the RP is very much in the driving seat when automatic registration is used but I can't see how that invalidates the usefulness of the proposed spec in other use cases.

I didn't intend to imply it did - my point was very much that to solve the overall problem more than just this spec is needed. My suggestion above of removing non-PAR automatic registration (and expanding the text on PAR automatic registration to indicate that the method that the client actually uses is the one to be registered, assuming it is also listed in the entity statement) it the easiest way I can see to solve everything.

I can raise a separate issue for that if preferred?

selfissued commented 2 weeks ago

The specification https://openid.net/specs/openid-connect-rp-metadata-choices-1_0.html was adopted to address this issue. The GitHub repository for the specification is https://github.com/openid/rp-metadata-choices.