Skip to content

Bug: github auth users should not be able to change their email #1490

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

Closed
f0ssel opened this issue May 16, 2022 · 7 comments · Fixed by #1863
Closed

Bug: github auth users should not be able to change their email #1490

f0ssel opened this issue May 16, 2022 · 7 comments · Fixed by #1863
Assignees
Labels
api Area: HTTP API site Area: frontend dashboard
Milestone

Comments

@f0ssel
Copy link
Contributor

f0ssel commented May 16, 2022

OS Information

Steps to Reproduce

  1. Login with Github
  2. Go to user preferences and update email to something garbage - canichangethis@coder.com
  3. Sign out
  4. Sign in with Github again - get error:
{"message":"create user: execute transaction: create user: pq: duplicate key value violates unique constraint \"idx_users_username\""}

Expected

If we allow email changes it should not lock the user out and break the user account.

Actual

The user is locked out and cannot log back in.

Logs

Screenshot

image

Notes

It seems like we need a good bit more guardrails and validation around this flow, we should not be able to collide on username or email when dealing with oauth.

@kylecarbs
Copy link
Member

Ahh interesting. GitHub users shouldn't be able to change anything about themselves I suppose.

@f0ssel
Copy link
Contributor Author

f0ssel commented May 16, 2022

Yeah that's my thought, @deansheather and I have discussed also not allowing password auth users from changing email as well, since that's also kind of a security issue - An admin invites garrett@coder.com, I log in and change it to secretperson@spooky.com and the admin now has no clue who this person is on their coder deployment.

@kylecarbs
Copy link
Member

Seems like we should associate OAuth connections separately to the account. I feel like @spikecurtis might have some good thoughts here.

@misskniss misskniss added Community MVP 🏁 site Area: frontend dashboard api Area: HTTP API labels May 19, 2022
@misskniss misskniss added this to the Community MVP milestone May 19, 2022
@spikecurtis
Copy link
Contributor

What are we using "username" and "email" for on the platform?

@f0ssel f0ssel self-assigned this May 23, 2022
@misskniss
Copy link

misskniss commented May 24, 2022

@f0ssel will you ping the frontend crew in the #dev-ux channel when you start this so so we can understand what changes may be needed here? We will need to spin up a ticket for that frontend work.

@Emyrk
Copy link
Member

Emyrk commented May 26, 2022

@f0ssel I have time to do this, want me to pick this up?

@f0ssel
Copy link
Contributor Author

f0ssel commented May 27, 2022

After talking with @deansheather and @Emyrk we've come up with the following proposal:

Since users can have both built-in and github auth for a single user, it's not possible to deterministically tell which users are github users and which ones are built-in. Because of the current ambiguity in the system, we are suggesting that no user should be allowed to change email.

I can just remove the email field from the API and get the frontend to make it a read-only field.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api Area: HTTP API site Area: frontend dashboard
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants