OIDC `sub` contains possibly sensitive data and may exceed the max length of 255 characters

[ProbstDJakob]

trafficstars

Preflight Checklist

• [x] I agree to follow the Code of Conduct that this project adheres to.
• [x] I have searched the issue tracker for an issue that matches the one I want to file, without success.
• [x] I am not looking for support or already pursued the available support channels without success.

Version

master

Storage Type

N/A

Installation Type

Other (specify below)

Expected Behavior

The OIDC subject should not leak any data and respect the maximum length of 255 characters as specified in the official specs. Ideally it should be a hash (sha256 or similar) based on the userID, connID, and a secret (used as salt).

Actual Behavior

The generated subject via genSubject does contain the userID encoded with protobuf and base64. Some organizations declare usernames or similar as sensitive data and using them as userID would leak them via the OIDC subject (possibly without the organization knowing). Furthermore this also has the effect that the sub may exceed the maximum length of 255 characters as required by the specs. Especially when using another OIDC IdP which has already a sub with 255 characters or a very long LDAP attribute.

Steps To Reproduce

No response

Additional Information

No response

Configuration

Logs

[nabokihms]

I think it is safe to refactor it because in the scope of the provider even if we start forming the sub claim values differently, they will still be unique in the scope of the provider.

[ProbstDJakob]

I do not think it is possible to change this behaviour without a breaking change. Since a new sub would most likely mean a new/different user on the service provider, this would lead to completely new accounts (or worse, to logging in as another existing user) after changing the sub generation algorithm. The only thing I can think of is to provide a way to select which algorithm to use (on a per SP base), but then the question arises what the default should be. If there is no default or it defaults to the new algorithm it would still be a breaking change. Defaulting to the old algorithm would prevent a breaking change, but then the issue with breaking the standard (subs longer than 255 characters) would still exist and admins of new dex instances may miss the setting and stick with the old algorithm, which is not desirable.