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.

Sign up for GitHub

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

Jump to bottom

Add email validity check #13475

Merged
merged 18 commits into from
Nov 14, 2020
Merged

Add email validity check #13475

merged 18 commits into from
Nov 14, 2020

Conversation

chrisshyi
Copy link
Contributor

Added email validity checking for the following actions:

  • User update
  • User email addition
    • Through the web interface
    • Through the /user/emails endpoint
  • User creation:
    • Through the admin interface
    • Through the /admin/users endpoint

Chris Shyi and others added 10 commits October 12, 2020 17:10
Improve error feedback for duplicate deploy keys
df0bcef 
Instead of a generic HTTP 500 error page, a flash message is rendered
with the deploy key page template so inform the user that a key with the
intended title already exists.
API returns 422 error when key with name exists
c4341c2
@zeripath
Merge branch 'master' into master
f46c127
Merge remote-tracking branch 'upstream/master'
e8745e3
Add email validity checking
1a539ef 
Add email validity checking for the following routes:
[Web interface]
1. User registration
2. User creation by admin
3. Adding an email through user settings
[API]
1. POST /admin/users
2. PATCH /admin/users/:username
3. POST /user/emails
@chrisshyi
Merge remote-tracking branch 'upstream/master'
354f957
Merge remote-tracking branch 'upstream/master'
2d2b60d
Add further tests
b400388
@chrisshyi
Add signup email tests
bfdb76c
@chrisshyi
Add email validity check for linking existing account
fb476e2
lunny
lunny requested changes Nov 9, 2020
View reviewed changes
models/user.go Outdated Show resolved Hide resolved
options/locale/locale_zh-TW.ini Outdated Show resolved Hide resolved
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Nov 9, 2020
@lunny lunny added the type/enhancement An improvement of existing functionality label Nov 9, 2020
@chrisshyi chrisshyi requested a review from lunny November 9, 2020 06:05
@chrisshyi chrisshyi changed the title Adding email validity check Add email validity check Nov 9, 2020
lafriks
lafriks approved these changes Nov 9, 2020
View reviewed changes
models/user.go Outdated Show resolved Hide resolved
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Nov 9, 2020
lafriks
lafriks requested changes Nov 9, 2020
View reviewed changes
Copy link
Member

@lafriks lafriks left a comment

Choose a reason for hiding this comment

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

Wrong button, see comment above

@lafriks lafriks added this to the 1.14.0 milestone Nov 9, 2020
techknowlogick
techknowlogick requested changes Nov 10, 2020
View reviewed changes
models/user.go Show resolved Hide resolved
@GiteaBot GiteaBot removed the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Nov 11, 2020
@GiteaBot GiteaBot added the lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. label Nov 11, 2020
@codecov-io
Copy link

codecov-io commented Nov 11, 2020 •
edited

Codecov Report

Merging #13475 (f0df7e6) into master (7d2700c) will increase coverage by 0.06%.
The diff coverage is 50.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #13475      +/-   ##
==========================================
+ Coverage   42.16%   42.22%   +0.06%     
==========================================
  Files         696      696              
  Lines       76480    76518      +38     
==========================================
+ Hits        32247    32311      +64     
+ Misses      38939    38911      -28     
- Partials     5294     5296       +2
Impacted Files Coverage Δ
routers/api/v1/user/email.go 0.00% <0.00%> (ø)
routers/user/setting/account.go 25.51% <0.00%> (-0.54%) ⬇️
models/user_mail.go 50.38% <33.33%> (-0.41%) ⬇️
routers/admin/users.go 27.77% <50.00%> (+2.06%) ⬆️
routers/api/v1/admin/user.go 30.57% <50.00%> (+14.54%) ⬆️
routers/user/auth.go 12.19% <50.00%> (+0.53%) ⬆️
models/user.go 54.36% <75.00%> (+0.12%) ⬆️
models/error.go 36.75% <100.00%> (+1.52%) ⬆️
modules/queue/workerpool.go 57.95% <0.00%> (-0.82%) ⬇️
models/gpg_key.go 53.33% <0.00%> (-0.58%) ⬇️
... and 11 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7d2700c...f0df7e6. Read the comment docs.

techknowlogick
techknowlogick approved these changes Nov 11, 2020
View reviewed changes
Copy link
Member

@techknowlogick techknowlogick left a comment

Choose a reason for hiding this comment

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

Thanks :)

@techknowlogick
Copy link
Member

@lunny please review

@techknowlogick techknowlogick merged commit d025d84 into go-gitea:master Nov 14, 2020
This was referenced Nov 18, 2020
Closed
Merged
6543 pushed a commit to 6543-forks/gitea that referenced this pull request Nov 21, 2020
@chrisshyi @zeripath
@lunny @techknowlogick @6543
Add email validity check (go-gitea#13475)
7913ab8 
* Improve error feedback for duplicate deploy keys

Instead of a generic HTTP 500 error page, a flash message is rendered
with the deploy key page template so inform the user that a key with the
intended title already exists.

* API returns 422 error when key with name exists

* Add email validity checking

Add email validity checking for the following routes:
[Web interface]
1. User registration
2. User creation by admin
3. Adding an email through user settings
[API]
1. POST /admin/users
2. PATCH /admin/users/:username
3. POST /user/emails

* Add further tests

* Add signup email tests

* Add email validity check for linking existing account

* Address PR comments

* Remove unneeded DB session

* Move email check to updateUser

Co-authored-by: zeripath <art27@cantab.net>
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
Co-authored-by: techknowlogick <techknowlogick@gitea.io>
@6543 6543 mentioned this pull request Nov 21, 2020
Merged
@6543 6543 added the backport/done All backports for this PR have been created label Nov 21, 2020
@6543
Copy link
Member

6543 commented Nov 21, 2020

-> #13666

techknowlogick added a commit that referenced this pull request Nov 22, 2020
@6543 @chrisshyi
@zeripath @lunny @techknowlogick
Validate email before inserting/updating (#13475) (#13666)
33431fc 
* Add email validity check (#13475)

* Improve error feedback for duplicate deploy keys

Instead of a generic HTTP 500 error page, a flash message is rendered
with the deploy key page template so inform the user that a key with the
intended title already exists.

* API returns 422 error when key with name exists

* Add email validity checking

Add email validity checking for the following routes:
[Web interface]
1. User registration
2. User creation by admin
3. Adding an email through user settings
[API]
1. POST /admin/users
2. PATCH /admin/users/:username
3. POST /user/emails

* Add further tests

* Add signup email tests

* Add email validity check for linking existing account

* Address PR comments

* Remove unneeded DB session

* Move email check to updateUser

Co-authored-by: zeripath <art27@cantab.net>
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
Co-authored-by: techknowlogick <techknowlogick@gitea.io>

* skip email validation on empty string (#13627)

- move validation into its own function
- use a session for UpdateUserSetting

* rm TODO for backport

Co-authored-by: Chris Shyi <chrisshyi13@gmail.com>
Co-authored-by: zeripath <art27@cantab.net>
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
Co-authored-by: techknowlogick <techknowlogick@gitea.io>
@go-gitea go-gitea locked and limited conversation to collaborators Jan 18, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@lunny lunny lunny approved these changes

@techknowlogick techknowlogick techknowlogick approved these changes

@lafriks lafriks lafriks approved these changes

@6543 6543 6543 approved these changes

Assignees
No one assigned
Labels
backport/done All backports for this PR have been created lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/enhancement An improvement of existing functionality
Projects
None yet
Milestone
1.14.0
Development

Successfully merging this pull request may close these issues.

None yet
8 participants
@chrisshyi @codecov-io @techknowlogick @6543 @lunny @lafriks @GiteaBot @zeripath