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

Why does Dex base64 encode "sub" claims? #1719

Closed
candlerb opened this issue May 20, 2020 · 7 comments
Closed

Why does Dex base64 encode "sub" claims? #1719

candlerb opened this issue May 20, 2020 · 7 comments

Comments

@candlerb
Copy link
Contributor

candlerb commented May 20, 2020

When using Dex with the example-app, the displayed "sub" claim is base64-encoded.

  • With mock connector: "sub": "Cg0wLTM4NS0yODA4OS0wEgRtb2Nr"
    This decodes to 0-385-28089-0mock which is the UserID from connector/mock/connectortest.go plus "mock"
  • With local connector: "sub": "CiQwOGE4Njg0Yi1kYjg4LTRiNzMtOTBhOS0zY2QxNjYxZjU0NjYSBWxvY2Fs"
    This decodes to $08a8684b-db88-4b73-90a9-3cd1661f5466local which is UserID from examples/config-dev.yaml plus "local"
  • With google connector: I get base64-encoded sub plus "googl" (sic).
    This is invalid base64: the trailing == from base64 encoding has been stripped, and if I add it back I get "google"

However before I investigate the stripping of == further, I just wonder what is the reason for base64-encoding the sub claim in the first place? Surely if this comes from an IDP's JWT, it must already be valid JSON and hence valid unicode?

EDIT: is it to discourage people from using sub directly, and instead to request federated:id, which gives the cleartext components?

  "federated_claims": {
    "connector_id": "mock",
    "user_id": "0-385-28089-0"
  }
@mouchar
Copy link

mouchar commented Jan 21, 2021

Just a side note: it's not just plain string concatenation. It's base64-encoded binary protobuf message IDTokenSubject, described in server/internal types.proto file.

$ base64 -d <<< Cg0wLTM4NS0yODA4OS0wEgRtb2Nr | \
  protoc --decode=internal.IDTokenSubject -I server/internal types.proto

user_id: "0-385-28089-0"
conn_id: "mock"

$ base64 -d <<< CiQwOGE4Njg0Yi1kYjg4LTRiNzMtOTBhOS0zY2QxNjYxZjU0NjYSBWxvY2Fs | \
  protoc --decode=internal.IDTokenSubject -I server/internal types.proto

user_id: "08a8684b-db88-4b73-90a9-3cd1661f5466"
conn_id: "local"

I'm not sure what's behind this design decision, but I believe it is related to the need to uniquely distinguish the same user IDs belonging to different connectors.
I like the trick with federated:id, it can simplify token processing a lot.

@candlerb
Copy link
Contributor Author

Thank you for the clarification: I had not considered that protobuf tags might be present (as unprintable characters).

$ echo "Cg0wLTM4NS0yODA4OS0wEgRtb2Nr" | base64 -d | hexdump -C
00000000  0a 0d 30 2d 33 38 35 2d  32 38 30 38 39 2d 30 12  |..0-385-28089-0.|
00000010  04 6d 6f 63 6b                                    |.mock|
00000015

This seems to assume that protobuf encoding is stable, but I think that's implementation-dependent. Here it says:

When a message is serialized, there is no guaranteed order for how its known or unknown fields should be written
...
Do not assume the byte output of a serialized message is stable.

Clearly it would be problematic if the "sub" of a user were to flip between two encoded values.

@nabokihms
Copy link
Member

It is a coincidence that the sub claim contains the id of the connector. The right way to get the connector id, as you have mentioned, is to request the federated:id scope.

I am going to close the issue. Feel free to open a new one if you still have questions.

@pinpox
Copy link
Contributor

pinpox commented Oct 24, 2022

Just to clarify: If I'm building an application that uses dex to login users via OpenID Connect, is it wrong to use sub as the user ID (e.g. to use it as ID for a users table in a database)?

I used the sub until now to keep my application compatilbe with any OpenID provider, in case someones want's to use something different from dex.

@nabokihms
Copy link
Member

nabokihms commented Oct 24, 2022

It is ok to use this claim value as a uniq identifier for a user. You can rely on it.

@ludov04
Copy link

ludov04 commented Apr 5, 2023

Hi all,

Would it be possible to have an option to disable this feature?
I'm trying to use dex with AWS's IAM OIDC federation, unfortunately, they only support doing check on a small set of claims.

One of them includes "sub" (https://docs.aws.amazon.com/IAM/latest/UserGuide/reference_policies_iam-condition-keys.html#condition-keys-wif)

But it's impossible to use it when the value is encoded in the obscure protobuf binary

@tooptoop4
Copy link

relates to #2339

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

No branches or pull requests

6 participants