allow current user to reset their own password #5034

coolaj86 · 2018-10-07T05:44:04Z

The current logged-in user (which may have signed in via OAuth) is not able to reset their own password via email reset, because they cannot access the form.

Users can get to reset form logged in and logged out
Can attempt to reset password without being logged out
If password reset will be successful, user is logged out (and is redirected to login)

Beyond fixing the bug, these are some things I'd be open to adding now (but would prefer to wait until after this clears)

show username / email of user
- take some action if that's different from the current user?
treat reset password form the same as register & sign in (remember me, retype password)
invalidate the code once used
set time to 15 minutes rather than 3 hours

codecov-io · 2018-10-07T05:52:43Z

Codecov Report

❗ No coverage uploaded for pull request base (master@c168095). Click here to learn what that means.
The diff coverage is 50%.

@@            Coverage Diff            @@
##             master    #5034   +/-   ##
=========================================
  Coverage          ?   40.47%           
=========================================
  Files             ?      405           
  Lines             ?    54399           
  Branches          ?        0           
=========================================
  Hits              ?    22017           
  Misses            ?    29356           
  Partials          ?     3026

Impacted Files	Coverage Δ
routers/routes/routes.go	`82.02% <100%> (ø)`
routers/user/auth.go	`13.15% <30%> (ø)`

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 c168095...0d701e4. Read the comment docs.

lafriks · 2018-10-07T14:48:43Z

routers/user/auth.go

@@ -1179,6 +1185,10 @@ func ResetPasswdPost(ctx *context.Context) {
 			ctx.ServerError("UpdateUser", err)
 			return
 		}
+
+		// Just in case the user is signed in to another account
+		handleSignOut(ctx)


I would prefer that there would be check for code that was issued for what user and currently authorised user and if they do not much than return error (probably 404?)

This could be another PR.

coolaj86 · 2018-10-07T15:40:10Z

The 404 that's already there is really confusing. I think there should be some sort of error message to the user like "This code is not valid or has expired." to let them know that Gitea doesn't have a bug, it recognized what they were trying to do, but followed a set of rules to disallow the action they intended, and gives them a clue as to what to do next to accomplish their goal.

Although the type of user we have is probably pretty savvy, giving feedback so will also increase security by reducing chances for social engineering attacks or pranks.

coolaj86 · 2018-10-07T17:41:20Z

I've got a second PR on the way that contains the ux fixes, showing the correct error message in each of the edge cases discussed in this issue as well as the original issue.

It's currently visible in use at https://git.coolaj86.com, but I have to fix a conflict between my v1.5.1 backports and master, and I have to head out for a bit, so I may not push it until tonight.

lafriks · 2018-11-08T21:41:18Z

Can these changes be merged into #5042 ?

coolaj86 · 2018-11-09T05:09:37Z

@lafriks I'm still a bit under water right now, but I'll try to make some time to merge them together. Thanks for reviewing. :)

coolaj86 · 2019-04-13T21:51:10Z

Rebased and ready for re-review!

stale · 2019-06-12T22:24:07Z

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs during the next 2 months. Thank you for your contributions.

lunny · 2019-06-26T11:35:55Z

Please resolve the conflicts

zeripath · 2019-06-28T20:09:44Z

It looks like #5042 supersedes this.

What's the envisioned behaviour for this.

lunny · 2019-06-29T14:08:41Z

@solderjs please confirm that this could be closed since #5042 merged.

coolaj86 · 2019-06-30T01:04:09Z

Looking at the commit history, I believe that is correct. These changes were included in and improved upon by #5042

coolaj86 · 2019-06-30T01:04:10Z

Looking at the commit history, I believe that is correct. These changes were included in and improved upon by #5042

zeripath · 2019-06-30T07:48:29Z

Thanks @solderjs for your contributions. Sorry some of them have languished.

coolaj86 mentioned this pull request Oct 7, 2018

UX Optional Password & Captcha for External Registration #5029

Closed

6 tasks

bkcsoft added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Oct 7, 2018

lafriks reviewed Oct 7, 2018

View reviewed changes

lafriks added the type/bug label Oct 7, 2018

lafriks added this to the 1.7.0 milestone Oct 7, 2018

lafriks added the backport/v1.6 label Oct 7, 2018

coolaj86 mentioned this pull request Oct 7, 2018

UX + Security current user password reset #5042

Merged

techknowlogick removed the backport/v1.6 label Nov 12, 2018

lafriks modified the milestones: 1.7.0, 1.8.0 Dec 27, 2018

techknowlogick modified the milestones: 1.8.0, 1.9.0 Feb 19, 2019

allow current user to reset their own password

0d701e4

ryanburnette force-pushed the current-user-password-reset branch from d3a4d76 to 0d701e4 Compare April 13, 2019 21:50

stale bot added the issue/stale label Jun 12, 2019

lunny added the issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented label Jun 26, 2019

stale bot removed the issue/stale label Jun 26, 2019

coolaj86 closed this Jun 30, 2019

go-gitea locked and limited conversation to collaborators Nov 24, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

allow current user to reset their own password #5034

allow current user to reset their own password #5034

coolaj86 commented Oct 7, 2018 •

edited

Loading

codecov-io commented Oct 7, 2018 •

edited

Loading

lafriks Oct 7, 2018

lunny Jun 26, 2019

coolaj86 commented Oct 7, 2018

coolaj86 commented Oct 7, 2018 •

edited

Loading

lafriks commented Nov 8, 2018

coolaj86 commented Nov 9, 2018

coolaj86 commented Apr 13, 2019

stale bot commented Jun 12, 2019

lunny commented Jun 26, 2019

zeripath commented Jun 28, 2019

lunny commented Jun 29, 2019

coolaj86 commented Jun 30, 2019

coolaj86 commented Jun 30, 2019

zeripath commented Jun 30, 2019

allow current user to reset their own password #5034

allow current user to reset their own password #5034

Conversation

coolaj86 commented Oct 7, 2018 • edited Loading

codecov-io commented Oct 7, 2018 • edited Loading

Codecov Report

lafriks Oct 7, 2018

Choose a reason for hiding this comment

lunny Jun 26, 2019

Choose a reason for hiding this comment

coolaj86 commented Oct 7, 2018

coolaj86 commented Oct 7, 2018 • edited Loading

lafriks commented Nov 8, 2018

coolaj86 commented Nov 9, 2018

coolaj86 commented Apr 13, 2019

stale bot commented Jun 12, 2019

lunny commented Jun 26, 2019

zeripath commented Jun 28, 2019

lunny commented Jun 29, 2019

coolaj86 commented Jun 30, 2019

coolaj86 commented Jun 30, 2019

zeripath commented Jun 30, 2019

coolaj86 commented Oct 7, 2018 •

edited

Loading

codecov-io commented Oct 7, 2018 •

edited

Loading

coolaj86 commented Oct 7, 2018 •

edited

Loading