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

email address is used as primary identifier, which is forbidden by OIDC #160

Open
xi opened this issue Jan 4, 2025 · 1 comment
Open

Comments

@xi
Copy link

xi commented Jan 4, 2025

This plugin uses the email address as a primary identifier, which leads to multiple issues (e.g. #126). The writers of the OpenID Connect (OIDC) spec also figured that this was a bad idea and therefore explicitly prohibited that:

The RP MUST NOT rely upon this value being unique

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 ( https://openid.net/specs/openid-connect-core-1_0.html#ClaimStability)

I understand that this plugin is an implementation of OAuth, and not OIDC specifically. However, a growing number of OAuth IPs follow the OIDC spec, so I think this is relevant nonetheless.

I am not 100% sure what this means in practice though. I am pretty sure that it should be possible to change the email address at the IP and still log in to the same account. However, what should happen if I have an existing non-SSO account and log in using SSO for the first time? I think it would be reasonable to match on the email address in that case.

I think we need a separate table that maps SSO identities to local accounts. I am not sure how that would fit into the existing architecture though.

@xi xi added the bug label Jan 4, 2025
@xi xi changed the title email address is used primary identifier, which is forbidden by OIDC email address is used as primary identifier, which is forbidden by OIDC Jan 4, 2025
@splitbrain splitbrain added enhancement and removed bug labels Jan 6, 2025
@splitbrain
Copy link
Member

You're right. A "proper" implementation would require some kind of account <-> IdP mapping with users manually creating these association. The current implementation is simple and reuses what DokuWiki already has and is good enough for most use cases.
If you want to tackle changing the architecture, please provide a pull request. Please be sure to address how to move existing oauth accounts to the new mechanism.

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

No branches or pull requests

2 participants