make configurable in which part of the OIDC exchange custom claims should be sent

[restena-sw]

In the authorization code flow, custom claims can be sent either in the "id_token" or by querying the "userinfo".

If the client made a preference as to where the claims should be by setting the "claims parameter", the module honours this and sends accordingly.

If no preference is indicated by the client, the module currently sends custom claims always via the userinfo endpoint. As per OIDC core spec, the claims /could/ alternatively be sent directly in the id_token.

The issue at hand suggests to make configurable where a specific custom claim should be sent. The suggested place is the OIDC-to-SAML attribute mapping table, as this lists every single claim

Example, for a hypothetical custom claim "foobar":

    // The default translate table from SAML attributes to OIDC claims.
    ModuleConfig::OPTION_AUTH_SAML_TO_OIDC_TRANSLATE_TABLE => [
...
          'foobar' => [
             'attribute' => 'urn:x-randomvendor:attibute-xyz',
             'dest' => 'userinfo' # or 'id_token'
          ],

There should be a backwards-compatible default (single-string array defaults to "userinfo", to keep the current behaviour).

An open question is how to prioritise if the client did send a claim parameter, but the configured destination differs from the configured destination - which one wins?

[cicnavi]

@pradtke please share your opinion on this.

[cicnavi]

@restena-sw please state why / benefits / added value / examples...

[restena-sw]

It is a general compatibility improvement. Clients are free to look into either id_token or userinfo to find the claims they need, and it's optional for clients to send a claims parameter. If then a server insists to put a claim into a place the client is not looking for, the two implementations do not interop. It's a classical interop problem.

In my case, the OIDC client needs two claims to work. If it finds the first one in the id_token, it also expects the second one at the same place, and does not query userinfo at all. So, if the first is in id_token but the second would in userinfo, the client does not have the claims it needs, even if they are both in principle available.

One could argue that the client is behaving strangely (I concur, and I have an issue open on their side to change client behaviour) but there is no single point where its current behaviour is not spec-compliant. It just "doesn't work" with no easy path to reconcile.

The benefit of implementing the issue as sketched is that such interop problems can be solved by allowing to steer claims such that the response meets client expectations.

[cicnavi]

To consider: it would be possible that some clients would need custom scope claims available in ID Token, while some clients would expect them from userinfo endpoint. Possible solution: having different custom scopes with different location configuration, but same claim value; or per client configuration...

[cicnavi]

To consider: it would be possible that some clients would need custom scope claims available in ID Token, while some clients would expect them from userinfo endpoint. Possible solution: having different custom scopes with different location configuration, but same claim value; or per client configuration...

And of course also always returning scope claims from userinfo endpoint.

[restena-sw]

Indeed, the need to configure could vary (and conflict) between clients. That rather speaks for making this an option of the client scopes in UI.

[pradtke]

We have had clients that expect everything in the id_token and it is an education effort to tell them to use user_info, so I am in favor of handling this in an improved way.

I would put "include claims in id_token" into the client UI configuration since it seems more like a client issue than an issue with the claims. I would think the user_info endpoint would always return the claims. So the change is the client config could say "id_token and userinfo" or just "userinfo".

FWIW, At one point we had a setting alwaysAddClaimsToIdToken that was global

[tvdijen]

We have had clients that expect everything in the id_token and it is an education effort to tell them to use user_info, so I am in favor of handling this in an improved way.

I would put "include claims in id_token" into the client UI configuration since it seems more like a client issue than an issue with the claims. I would think the user_info endpoint would always return the claims. So the change is the client config could say "id_token and userinfo" or just "userinfo".

FWIW, At one point we had a setting alwaysAddClaimsToIdToken that was global

This is perfect MS-breaks-OIDC-specs example and already found it's way in OpenConext-oidcng as an RP-specific feature-flag. When flag is set, claims will be released in the ID-token. Sadly this has become more of a standard than an exception, really..

[cicnavi]

We have had clients that expect everything in the id_token and it is an education effort to tell them to use user_info, so I am in favor of handling this in an improved way.

I would put "include claims in id_token" into the client UI configuration since it seems more like a client issue than an issue with the claims. I would think the user_info endpoint would always return the claims. So the change is the client config could say "id_token and userinfo" or just "userinfo".

FWIW, At one point we had a setting alwaysAddClaimsToIdToken that was global

Note that the suggestion is to only allow this for custom scope claims. Are you in favor of returning all scope claims (and go against the spec as with alwaysAddClaimsToIdToken)? So no distinction between custom / registered scope claims?

[cicnavi]

This is perfect MS-breaks-OIDC-specs example and already found it's way in OpenConext-oidcng as an RP-specific feature-flag. When flag is set, claims will be released in the ID-token. Sadly this has become more of a standard than an exception, really..

Just to confirm, this will include OIDC registered claims? This ones: