mirror of
https://github.com/pocket-id/pocket-id.git
synced 2025-12-06 09:13:19 +03:00
🐛 Bug Report: Federated client credentials RFC 7523 inconsistency #52
Reference in New Issue
Block a user
Delete Branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
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-571ea4e6but then I get the error
Client ID can only contain letters, numbers, underscores, and hyphenswhen 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-idneeds 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
@michaelbeaumont commented on GitHub:
I can see a couple some ways forward:
(iss, sub)->oidc_clientand not violate the RFC(iss, sub)tooidc_client, or justiss, which if set allows JWT bearer auth and deprecatefederatedIdentitiesand don't violate the RFC(iss, sub)that we find in somefederatedIdentities->oidc_clientso that users who want to follow the RFC at least can@michaelbeaumont commented on GitHub:
Also
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:
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.They support JWT for client authentication apparently, but it seems only with the
client_credentialsgrant.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:Ory, for example, enforces
client_id==subwhen 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
subclaim. This could be accomplished with a custom client ID, however this has the unfortunate side effect, AFAICT, of making it impossible to use the samesubwith different issuers. I.e. we should really lookup OIDC client configurations with(iss, sub), not justsub.@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:
I can comment on the willful violation of RFC 7523. Specifically, we have not been complying with this point on section 3:
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.
@michaelbeaumont commented on GitHub:
Maybe I'm misinterpreting the spec but it should be possible, without an explicit
client_idbecause it's optional, to map theissandsubfrom the token to a single "oidc configuration" aka a row inoidc_clients. If(iss, sub)is not unique, as it's currently not constrainted to be, this is not possible.@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.
@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
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 IDofsystem:serviceaccount:mealie:mealiehowever 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 onMealieclient, 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