🐛 Bug Report: Federated client credentials RFC 7523 inconsistency #52

Open
opened 2025-10-06 23:59:28 +03:00 by OVERLORD · 8 comments
Owner

Originally created by @michaelbeaumont on GitHub.

Reproduction steps

Try to create an app with federated client credentials for use with Kubernetes.

Try to figure out what the federated client credentials Subject option does.

Actual Behavior

First of all, the docs mention using Kubernetes API server and even describe the format like of those JWT tokens as system:serviceaccount:envoy-gateway-system:envoy-infra-services-571ea4e6
but then I get the error Client ID can only contain letters, numbers, underscores, and hyphens when trying to create a client with this non-generated clientID.

There also seems to be some inconsistency with clientID vs sub. The RFC says the JWT sub must equal the clientID, but then we allegedly have the Federated Client Credentials optional Subject? ...why is it there? It seems like we both enforce that the clientID is in the JWT as per the RFC but also we verify using the optional subject in some cases.

It looks previously we allowed client_id and the sub to be different but https://github.com/pocket-id/pocket-id/pull/704/files#diff-25f504aed465ee11d0481b80d78561cabe0dff6d344ab071732760841bac5db8R1392 broke this.

Expected Behavior

The feature is currently broken. Previously, the user had to violate RFC 7523 for this to work at all.

Ideally federated client creds should be their own type of top-level OIDC client with a unique key of (iss, sub). In any case, pocket-id needs to be able to lookup and find a unique OIDC client configuration given only (iss, sub) from the RFC 7523 compliant client assertion.

Pocket ID Version

v1.10.0

Database

sqlite

OS and Environment

Not relevant.

Log Output

No response

Originally created by @michaelbeaumont on GitHub. ### Reproduction steps Try to create an app with federated client credentials for use with Kubernetes. Try to figure out what the federated client credentials Subject option does. ### Actual Behavior First of all, the docs mention using Kubernetes API server and even describe the format like of those JWT tokens as `system:serviceaccount:envoy-gateway-system:envoy-infra-services-571ea4e6` but then I get the error `Client ID can only contain letters, numbers, underscores, and hyphens` when trying to create a client with this non-generated clientID. There also seems to be some inconsistency with clientID vs sub. The RFC says the JWT sub must equal the clientID, but then we allegedly have the Federated Client Credentials optional Subject? ...why is it there? It seems like we both [enforce that the clientID](https://github.com/pocket-id/pocket-id/blob/802754c24c019abb099c0f1b7be45bac315ae1dd/backend/internal/service/oidc_service.go#L1688) is [in the JWT as per the RFC](https://github.com/pocket-id/pocket-id/blob/main/backend/internal/service/oidc_service.go#L1535-L1550) but also [we verify using the optional subject in some cases](https://github.com/pocket-id/pocket-id/blob/802754c24c019abb099c0f1b7be45bac315ae1dd/backend/internal/service/oidc_service.go#L1657). It looks previously we allowed client_id and the sub to be different but https://github.com/pocket-id/pocket-id/pull/704/files#diff-25f504aed465ee11d0481b80d78561cabe0dff6d344ab071732760841bac5db8R1392 broke this. ### Expected Behavior The feature is currently broken. Previously, the user _had_ to violate RFC 7523 for this to work at all. Ideally federated client creds should be their own type of top-level OIDC client with a unique key of `(iss, sub)`. In any case, `pocket-id` needs to be able to lookup and find a unique OIDC client configuration given only `(iss, sub)` from the RFC 7523 compliant client assertion. ### Pocket ID Version v1.10.0 ### Database sqlite ### OS and Environment Not relevant. ### Log Output _No response_
Author
Owner

@michaelbeaumont commented on GitHub:

I can see a couple some ways forward:

  • require federated identities to be unique across clients, so that we can unambiguously query from (iss, sub) -> oidc_client and not violate the RFC
  • add new columns (iss, sub) to oidc_client, or just iss, which if set allows JWT bearer auth and deprecate federatedIdentities and don't violate the RFC
  • take the first (iss, sub) that we find in some federatedIdentities -> oidc_client so that users who want to follow the RFC at least can
@michaelbeaumont commented on GitHub: I can see a couple some ways forward: - require federated identities to be unique across clients, so that we can unambiguously query from `(iss, sub)` -> `oidc_client` and not violate the RFC - add new columns `(iss, sub)` to `oidc_client`, or just `iss`, which if set allows JWT bearer auth and deprecate `federatedIdentities` and don't violate the RFC - take the first `(iss, sub)` that we find in some `federatedIdentities` -> `oidc_client` so that users who want to follow the RFC at least _can_
Author
Owner

@michaelbeaumont commented on GitHub:

Also

I am personally not too concerned with the incompliance with the RFC, given there's precedent in the industry.

JWT client auth, especially with auth code grants, is as far as I can tell not (yet) a widespread feature. I am concerned about going to upstream libraries and trying to contribute implementations that I have to explain explicitly violate the corresponding RFC because the IdP I like and use decided to violate the spec. Apparently only because Microsoft did, kind of, in some circumstances.

@michaelbeaumont commented on GitHub: Also > I am personally not too concerned with the incompliance with the RFC, given there's precedent in the industry. JWT client auth, especially with auth code grants, is as far as I can tell not (yet) a widespread feature. I am concerned about going to upstream libraries and trying to contribute implementations that I have to explain explicitly violate the corresponding RFC because the IdP I like and use decided to violate the spec. Apparently only because Microsoft did, kind of, in some circumstances.
Author
Owner

@michaelbeaumont commented on GitHub:

I am personally not too concerned with the incompliance with the RFC, given there's precedent in the industry.

Being too permissive is one thing. Requiring the client to violate the RFC is another, which is how it worked previously AFAICT.

I think the docs you linked for Entra are about 2.1. Using JWTs as Authorization Grants. You can see it in the flow diagram here.

After that trust relationship is created, your software workload can exchange trusted tokens from the external identity provider for access tokens from the Microsoft identity platform. Your software workload then uses that access token to access the Microsoft Entra protected resources to which the workload has access

They support JWT for client authentication apparently, but it seems only with the client_credentials grant.

It looks like Entra supports RFC 7523 for access token requests with auth code grant but they call it "certificate credentials"., it's not quite federation. There they also require sub == client_id:

The "iss" (issuer) claim identifies the principal that issued the JWT, in this case your client application. Use the GUID application ID.
The "sub" (subject) claim identifies the subject of the JWT, in this case also your application. Use the same value as iss.

Ory, for example, enforces client_id==sub when using JWTs for client auth.

This feature must work even if there is no client_id presented to the user and all we have is the JWT token's sub claim. This could be accomplished with a custom client ID, however this has the unfortunate side effect, AFAICT, of making it impossible to use the same sub with different issuers. I.e. we should really lookup OIDC client configurations with (iss, sub), not just sub.

@michaelbeaumont commented on GitHub: > I am personally not too concerned with the incompliance with the RFC, given there's precedent in the industry. Being too permissive is one thing. _Requiring_ the client to violate the RFC is another, which is how it worked previously AFAICT. I think the docs you linked for Entra are about [`2.1. Using JWTs as Authorization Grants`](https://datatracker.ietf.org/doc/html/rfc7523#section-2.1). [You can see it in the flow diagram here](https://learn.microsoft.com/en-us/entra/workload-id/workload-identity-federation#how-it-works). > After that trust relationship is created, your software workload can exchange trusted tokens from the external identity provider for access tokens from the Microsoft identity platform. Your software workload then uses that access token to access the Microsoft Entra protected resources to which the workload has access They support JWT for client authentication [apparently, but it seems only with the `client_credentials` grant.](https://learn.microsoft.com/en-us/entra/identity-platform/v2-oauth2-client-creds-grant-flow#third-case-access-token-request-with-a-federated-credential) [It looks like Entra supports RFC 7523 for access token requests with auth code grant](https://learn.microsoft.com/en-us/entra/identity-platform/v2-oauth2-auth-code-flow#request-an-access-token-with-a-certificate-credential) but [they call it "certificate credentials".](https://learn.microsoft.com/en-us/entra/identity-platform/certificate-credentials#assertion-format), it's not quite federation. There they also require `sub` == `client_id`: > The "iss" (issuer) claim identifies the principal that issued the JWT, in this case your client application. Use the GUID application ID. > The "sub" (subject) claim identifies the subject of the JWT, in this case also your application. Use the same value as iss. [Ory, for example, enforces `client_id`==`sub` when using JWTs for client auth](https://www.ory.sh/docs/hydra/guides/jwt#jwts-for-client-authentication). This feature must work even if there is no client_id presented to the user and all we have is the JWT token's `sub` claim. This could be accomplished with a custom client ID, _however_ this has the unfortunate side effect, AFAICT, of making it impossible to use the same `sub` with different issuers. I.e. we should really lookup OIDC client configurations with `(iss, sub)`, not just `sub`.
Author
Owner

@ItalyPaleAle commented on GitHub:

@michaelbeaumont

I'm a bit confused, sorry. When you say "require federated identities to be unique across clients", what are you referring to? Could you elaborate a bit more?

@ItalyPaleAle commented on GitHub: @michaelbeaumont I'm a bit confused, sorry. When you say "require federated identities to be unique across clients", what are you referring to? Could you elaborate a bit more?
Author
Owner

@ItalyPaleAle commented on GitHub:

I can comment on the willful violation of RFC 7523. Specifically, we have not been complying with this point on section 3:

For client authentication, the subject MUST be the "client_id" of the OAuth client.

Up until Pocket ID 1.10.0 it was not possible to set a custom Client ID, so complying with this point was not possible.

It also seems that other IdPs are not conforming with this requirement. For example, Entra ID (which the feature was modeled after) doesn't require the subject to match the client ID either (something that wouldn't be possible in Entra ID either): https://learn.microsoft.com/en-us/graph/api/resources/federatedidentitycredentials-overview?view=graph-rest-1.0#set-up-federated-identity-credentials-through-microsoft-graph

I am personally not too concerned with the incompliance with the RFC, given there's precedent in the industry.

I am more concerned with the fact that it's currently broken.

@ItalyPaleAle commented on GitHub: I can comment on the willful violation of RFC 7523. Specifically, we have not been complying with this point on section 3: > For client authentication, the subject MUST be the "client_id" of the OAuth client. Up until Pocket ID 1.10.0 it was not possible to set a custom Client ID, so complying with this point was not possible. It also seems that other IdPs are not conforming with this requirement. For example, Entra ID (which the feature was modeled after) doesn't require the subject to match the client ID either (something that wouldn't be possible in Entra ID either): https://learn.microsoft.com/en-us/graph/api/resources/federatedidentitycredentials-overview?view=graph-rest-1.0#set-up-federated-identity-credentials-through-microsoft-graph I am personally not _too_ concerned with the incompliance with the RFC, given there's precedent in the industry. I am more concerned with the fact that it's currently broken.
Author
Owner

@michaelbeaumont commented on GitHub:

When you say "require federated identities to be unique across clients", what are you referring to? Could you elaborate a bit more?

Maybe I'm misinterpreting the spec but it should be possible, without an explicit client_id because it's optional, to map the iss and sub from the token to a single "oidc configuration" aka a row in oidc_clients. If (iss, sub) is not unique, as it's currently not constrainted to be, this is not possible.

@michaelbeaumont commented on GitHub: > When you say "require federated identities to be unique across clients", what are you referring to? Could you elaborate a bit more? Maybe I'm misinterpreting the spec but it should be possible, without an explicit `client_id` because it's optional, to map the `iss` and `sub` from the token to a single "oidc configuration" aka a row in `oidc_clients`. If `(iss, sub)` is not unique, as it's currently not constrainted to be, this is not possible.
Author
Owner

@michaelbeaumont commented on GitHub:

I don't think Mealie supports RFC 7523 yet, as far as I can tell. There isn't a way to get Mealie to craft an RFC 7523 compliant request to pocket-id yet. The underlying library it uses, authlib, does already support this, but Mealie is hardcoded to use client_id/client_secret.

What the feature is intended to do is let you skip giving Mealie a client_id/client_secret and instead allow Mealie to just take the service account token it has from running on Kubernetes and use that instead of a client_id/client_secret. The "Federated Client Credentials" option allows you to configure this.

@michaelbeaumont commented on GitHub: I don't think Mealie supports RFC 7523 yet, as far as I can tell. There isn't a way to get Mealie to craft an RFC 7523 compliant request to pocket-id yet. The underlying library it uses, `authlib`, does already support this, but [Mealie is hardcoded to use client_id/client_secret](https://github.com/mealie-recipes/mealie/blob/3ba2227bc79bc857d4b04f9d9878045b7a098af4/mealie/routes/auth/auth.py#L42-L48). What the feature is intended to do is let you skip giving Mealie a client_id/client_secret and instead allow Mealie to just take the service account token it has from running on Kubernetes and use that instead of a client_id/client_secret. The "Federated Client Credentials" option allows you to configure this.
Author
Owner

@userbradley commented on GitHub:

I am happy to be the test user for this, as I am trying to get this working in k8s, however running in to the below error, which I believe to be based on this github issue

Oct  1 18:27:04 ERR record not found app=pocket-id version=1.11.2 error="record not found" query="SELECT * FROM \"oidc_clients\" WHERE id = 'system:serviceaccount:mealie:mealie' ORDER BY \"oidc_clients\".\"id\" LIMIT 1" duration=1.611551ms rows=0 file=github.com/pocket-id/pocket-id/backend/internal/service/oidc_service.go:1595
Oct  1 18:27:04 WRN Request with errors: Error #01: invalid client assertion
 app=pocket-id version=1.11.2 status=400 method=POST path=/api/oidc/token query="" route=/api/oidc/token ip=169.150.201.8 latency=2.190976ms referer="" user_agent=python-requests/2.32.5 body_size=36

From how I understand this error, and please correct me if I am wrong, is I need to make a client (eg: Administration > OIDC Clients) with the Client ID of system:serviceaccount:mealie:mealie however as mentioned in the top comment, this is not possible. If that was the case, does that not mean that that identity can't access other applications? eg: I have it on Mealie client, but if I was to make a new client, it now cant access mealie?

Once this issue is resolved, I am happy to contribute docs towards this one as it's been a head scratcher. I can't write go to save my life so sadly cant contribute here

@userbradley commented on GitHub: I am happy to be the test user for this, as I am trying to get this working in k8s, however running in to the below error, which I believe to be based on this github issue ```text Oct 1 18:27:04 ERR record not found app=pocket-id version=1.11.2 error="record not found" query="SELECT * FROM \"oidc_clients\" WHERE id = 'system:serviceaccount:mealie:mealie' ORDER BY \"oidc_clients\".\"id\" LIMIT 1" duration=1.611551ms rows=0 file=github.com/pocket-id/pocket-id/backend/internal/service/oidc_service.go:1595 Oct 1 18:27:04 WRN Request with errors: Error #01: invalid client assertion app=pocket-id version=1.11.2 status=400 method=POST path=/api/oidc/token query="" route=/api/oidc/token ip=169.150.201.8 latency=2.190976ms referer="" user_agent=python-requests/2.32.5 body_size=36 ``` From how I understand this error, and please correct me if I am wrong, is I need to make a client (eg: Administration > OIDC Clients) with the `Client ID` of `system:serviceaccount:mealie:mealie` however as mentioned in the top comment, this is not possible. If that was the case, does that not mean that that identity can't access other applications? eg: I have it on `Mealie` client, but if I was to make a new client, it now cant access mealie? Once this issue is resolved, I am happy to contribute docs towards this one as it's been a head scratcher. I can't write go to save my life so sadly cant contribute here
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/pocket-id#52