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

Check if email is used when updating user (#21289) #21750

Closed
wants to merge 7 commits into from
Closed

Check if email is used when updating user (#21289) #21750

wants to merge 7 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Nov 10, 2022

Backport #21289

Fix #21075
When updating user data should check if email is used by other users

Backport #21289

Fix #21075
When updating user data should check if email is used by other users
@lunny lunny added this to the 1.17.4 milestone Nov 10, 2022
@lunny lunny added the type/bug label Nov 10, 2022
@GiteaBot GiteaBot added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Nov 10, 2022
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Nov 10, 2022
@lunny
Copy link
Member

lunny commented Nov 10, 2022

CI failure is related.

IsPrimary: false,
}); err != nil {
return err
if has && emailAddress.UID != u.ID {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if email exists and is not active? This way I could add to my account let's say others person's email address and block him from adding his other email

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, but that brings another change with it:
We need to throw out unactivated emails then when someone else changes his email and verifies it.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a backport, should I modify this patch?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think this should be fixed

Xinyu Zhou added 3 commits November 25, 2022 21:22
Signed-off-by: Xinyu Zhou <i@sourcehut.net>
Signed-off-by: Xinyu Zhou <i@sourcehut.net>
@ghost
Copy link
Author

ghost commented Nov 25, 2022

CI failure is related.

wow, just noticed.

@lunny lunny modified the milestones: 1.17.4, 1.17.5 Dec 21, 2022
@jolheiser
Copy link
Member

Going to close this as we've moved on from 1.17.x

@jolheiser jolheiser closed this Jan 11, 2023
@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants