MarcelCoding / jitsi-openid

Jitsi OpenID is an authentication adapter that allows Jitsi to authorize users with OpenID Connect.
GNU Affero General Public License v3.0
31 stars 4 forks source link

jitsi-openid should use always `sub` as `context.user.id`, not `preferred_username` #330

Open micolous opened 5 days ago

micolous commented 5 days ago

Jitsi Meet's JWT doc describes the context.token.id claim as an opaque user identifier, and gives a UUID as an example.

However, jitsi-openid uses the OpenID preferred_username claim as the default Jitsi user ID claim (context.user.id), falling back to the sub claim if preferred_username is not present:

https://github.com/MarcelCoding/jitsi-openid/blob/eb7b9d74ca93da85660ea387aadb4da6d3b2e0d7/src/routes.rs#L234-L237

Per the OpenID Connect Core 1.0 specification, section 5.7 (emphasis added):

The sub (subject) and iss (issuer) Claims from the ID Token, used together, are the only Claims that an RP can rely upon as a stable identifier for the End-User, since the sub Claim MUST be locally unique and never reassigned within the Issuer for a particular End-User, as described in Section 2. Therefore, the only guaranteed unique identifier for a given End-User is the combination of the iss Claim and the sub Claim.

All other Claims carry no such guarantees across different issuers in terms of stability over time or uniqueness across users, and Issuers are permitted to apply local restrictions and policies. For instance, an Issuer MAY re-use an email Claim Value across different End-Users at different points in time, and the claimed email address for a given End-User MAY change over time. Therefore, other Claims such as email, phone_number, preferred_username, and name MUST NOT be used as unique identifiers for the End-User, whether obtained from the ID Token or the UserInfo Endpoint.

And per section 5.1 (emphasis added):

preferred_username

Shorthand name by which the End-User wishes to be referred to at the RP, such as janedoe or j.doe. This value MAY be any valid JSON string including special characters such as @, /, or whitespace. The RP MUST NOT rely upon this value being unique, as discussed in Section 5.7.

As preferred_username has no guarantees of stability or uniqueness, it is incorrect to pass it as a unique user identifier to Jitsi.

Depending on how Jitsi uses the id value, this may allow a malicious user to impersonate others or hijack accounts[^1].

[^1]: I have not tested the security impact of this issue.

MarcelCoding commented 5 days ago

Sounds reasonable, it could be that the preferred username is unique in keycloak and therefore I did not think about it .

MarcelCoding commented 5 days ago

As jitsi had no state, this should be a simple change without any breakage?

micolous commented 5 days ago

No worries. For what it's worth, you've done a lot more than other adapters I've reviewed (eg: this adapter actually implements PKCE). :)

I don't know what impact changing this would have on Jitsi directly, because I haven't gotten it running yet.

However, other OIDC adapters I've reviewed either set it from sub or omit the ID entirely (1, 2, 3).

My assumption was you'd done it this way deliberately because the ID pops up somewhere in Jitsi's internals (eg: Prosody/XMPP layer) and preferred_username is more familiar than a sub.