Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Include sub claim in client credentials JWTs? (RFC 9068) #888

Closed
bmcclory opened this issue Sep 12, 2023 · 3 comments
Closed

Include sub claim in client credentials JWTs? (RFC 9068) #888

bmcclory opened this issue Sep 12, 2023 · 3 comments

Comments

@bmcclory
Copy link

bmcclory commented Sep 12, 2023

Which version of Duende IdentityServer are you using?
6.3.4+1e7db6bee93a039206d5354f0c384e3753c2acd3

Describe the bug

(question) Why doesn't IdentityServer include sub claim in client credential JWTs? It seems out of step with the behavior of other IDPs, and with RFC 9068 which directly addresses this scenario:

REQUIRED - as defined in Section 4.1.2 of [RFC7519]. In cases of access tokens obtained through grants where a resource owner is involved, such as the authorization code grant, the value of "sub" SHOULD correspond to the subject identifier of the resource owner. In cases of access tokens obtained through grants where no resource owner is involved, such as the client credentials grant, the value of "sub" SHOULD correspond to an identifier the authorization server uses to indicate the client application. See Section 5 for more details on this scenario. Also, see Section 6 for a discussion about how different choices in assigning "sub" values can impact privacy.

https://datatracker.ietf.org/doc/html/rfc9068#section-2.2

Omitting the ("REQUIRED") sub claim seems like a violation of the specification to me.

From a pragmatic standpoint, it simplifies resource server authorization policies to base everything on sub and not worry about whether an authenticated principal has the claim or not. Perhaps there's some risk in that approach, thought? If a human user and client happen to have same IDs, then an authorization policy based solely on sub could be exploited?(Edit: read more and realized this concern is already addressed in section 5 of the RFC).

Your expertise would be welcome here.

@leastprivilege
Copy link
Member

The RFC you mention was released years after we made that design decision.

Indeed we had that concern that it might lead to confusion if the sub claim can mean something different based on the flow with which the client has obtained the token. Hence our decision.

If you want to force that behavior from the RFC, simply set a static sub client claim in the client definition (and disable the prefixing of client claims). Would that work?

Would you prefer first-class support with a per-client config switch?

@bmcclory
Copy link
Author

bmcclory commented Sep 12, 2023

it might lead to confusion if the sub claim can mean something different based on the flow with which the client has obtained the token

My take: It's not desirable for the resource server to be aware of the flow with which a JWT was obtained (that seems like an IDP detail.) Having to handle the possibility of missing sub claim forces the resource server to maintain some "flow awareness" in its authorization policies and/or JWT handler.

I find myself in (cautious) agreement with RFC 9068 here, though I understand why IdentityServer made a different design decision (long) before the RFC was released. The literature is pretty consistent that, in a client credential scenario, the client itself should be considered the resource owner for purposes of authorization policy. I think that fits the semantic of the sub claim.

If you want to force that behavior from the RFC, simply set a static sub client claim in the client definition (and disable the prefixing of client claims). Would that work?

Understood. I'm coming from IdentityServer4 and am aware this is long-standing behavior of the framework, and statically configured client claims is how I currently deal with this.

But it always felt like a bit of a hack (and violation of DRY) to have to explicitly configure the sub claim for each client in our system. (Perhaps a custom client store decorator could address that concern, though.)

Would you prefer first-class support with a per-client config switch?

In light of RFC 9068, I was hoping to find a more global option available in Duende IdentityServer, e.g. EmitSubClaimInClientCredentialsJwt, where I can just set it once in IdentityServer options and not give it any more thought. This seems similar to EmitScopesAsSpaceDelimitedStringInJwt, which as I understand, was also introduced to align IdentityServer with RFC 9068.

Oh, and I'm happy to submit a PR for this if you guys are open to it.

@leastprivilege
Copy link
Member

It's not desirable for the resource server to be aware of the flow with which a JWT was obtained

It is not about the flow. Since IdentityServer was built with OIDC in mind from the ground up, the absence of sub means the absence of a human being.

Either way - I can see both sides of the argument.

We will discuss internally if we want to add a per-client config switch. A global one will not work for migration scenarios.

You can track it here: DuendeSoftware/IdentityServer#1417

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants