Skip to content

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
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.
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
@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
@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
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

@chrisshyi chrisshyi requested a review from lafriks November 9, 2020 07:40
@lafriks lafriks added this to the 1.14.0 milestone Nov 9, 2020
@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

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.

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
6543 pushed a commit to 6543-forks/gitea that referenced this pull request Nov 21, 2020
* 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 <[email protected]>
Co-authored-by: Lunny Xiao <[email protected]>
Co-authored-by: techknowlogick <[email protected]>
@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
* 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 <[email protected]>
Co-authored-by: Lunny Xiao <[email protected]>
Co-authored-by: techknowlogick <[email protected]>

* 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 <[email protected]>
Co-authored-by: zeripath <[email protected]>
Co-authored-by: Lunny Xiao <[email protected]>
Co-authored-by: techknowlogick <[email protected]>
@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.
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
Development

Successfully merging this pull request may close these issues.

8 participants