grist-core icon indicating copy to clipboard operation
grist-core copied to clipboard

Using OIDC, the email is used to identify the user instead of the 'sub' claim

Open illode opened this issue 1 year ago • 4 comments

I'm running the latest (fdc3b96cf7fa) docker image, selfhosted. I wanted to use OIDC instead of dealing with SAML.

I was going to report a different bug, but wanted to change the emails of my test accounts from <mydomain> to example.com first so I could send a screenshot. After doing that and signing in, I realized it had created a new account with the same name instead of signing me in to the original account.

Multiple Test Two users in the users table of home.sqlite3: image and multiple test2@<domain> logins in the logins table of home.sqlite3: image

New personal orgs were also created, leaving the original files in limbo. At least unless I change the emails back.

All of the Test 2 entries in both screenshots are the from user in the OIDC provider (Keycloak), I just changed their emails.

As I understand it, the user should be identified using the sub claim (standard claims / ID token). The connect_id column kind of looks like it should be for that, but I'm not sure as it's all NULL.

As an aside, there were a few other issues I ran into with the selfhosted version. Should I create new issues for them, or add them to https://github.com/gristlabs/grist-core/issues/733 since it seems to have several issues in one?

illode avatar Nov 20 '23 23:11 illode

cc @fflorent

Thanks for reporting this @illode. For other problems, generally one issue per problem is best.

paulfitz avatar Nov 21 '23 20:11 paulfitz

Thanks for your report @illode!

Indeed, the sub property can be used to identify uniquely a user. And you seem to be right, connect_id may be a good fit for storing this property: https://github.com/gristlabs/grist-core/blob/570e4032a416a8442d329dbe3551208a658fd6b6/app/gen-server/lib/HomeDBManager.ts#L533-L544

However, the method above seems not to be called anywhere. @paulfitz Does it make sense to take advantage of it for that purpose?

fflorent avatar Nov 22 '23 11:11 fflorent