Password reset link does not work when logged in #2373

Closed
opened 2025-11-02 04:34:06 -06:00 by GiteaMirror · 7 comments
Owner

Originally created by @grothesque on GitHub (Oct 3, 2018).

With current try.gitea.io, I proceed as follows:

  • Create a new gitea account linked to an existing github account
  • Sign in using the github account
  • Go to gitea settings, click on "forgot password?" and confirm the sending of a reset email
  • While staying signed-in to gitea, copy-and-paste the reset URL into the browser, visit it

The result of the above is that there's a redirect to https://try.gitea.io/. No password reset form is presented.

If I log out from gitea before visiting the password reset URL, I am able to reset the password.

I verified the above behavior with two browsers: Firefox ESR 52.9.0 and Chromium 68.0.3440.75.

Originally created by @grothesque on GitHub (Oct 3, 2018). With current try.gitea.io, I proceed as follows: * Create a new gitea account linked to an existing github account * Sign in using the github account * Go to gitea settings, click on "forgot password?" and confirm the sending of a reset email * While staying signed-in to gitea, copy-and-paste the reset URL into the browser, visit it The result of the above is that there's a redirect to https://try.gitea.io/. No password reset form is presented. If I log out from gitea before visiting the password reset URL, I am able to reset the password. I verified the above behavior with two browsers: Firefox ESR 52.9.0 and Chromium 68.0.3440.75.
GiteaMirror added the issue/staletype/bug labels 2025-11-02 04:34:06 -06:00
Author
Owner

@adelowo commented on GitHub (Oct 6, 2018):

@lafriks what should be the behaviour... It redirects back to the dashboard page because the user is already logged in

@adelowo commented on GitHub (Oct 6, 2018): @lafriks what should be the behaviour... It redirects back to the dashboard page because the user is already logged in
Author
Owner

@lafriks commented on GitHub (Oct 6, 2018):

I think it should allow changing password independently if user is logged in or not (could be that user is logged in by oauth) and does not remember his password anymore. Of course it needs to check if change password request is for the same user as logged in

@lafriks commented on GitHub (Oct 6, 2018): I think it should allow changing password independently if user is logged in or not (could be that user is logged in by oauth) and does not remember his password anymore. Of course it needs to check if change password request is for the same user as logged in
Author
Owner

@coolaj86 commented on GitHub (Oct 6, 2018):

I discovered this when working on https://github.com/go-gitea/gitea/pull/5029. I'm going to take a crack at it.

I'll try to keep it separate from my other two PRs if I can.

@coolaj86 commented on GitHub (Oct 6, 2018): I discovered this when working on https://github.com/go-gitea/gitea/pull/5029. I'm going to take a crack at it. I'll try to keep it separate from my other two PRs if I can.
Author
Owner

@coolaj86 commented on GitHub (Oct 7, 2018):

I've taken a peek, but there's magic going on somewhere that I haven't been able to track down and I could use some help.

It's happening before it ever reaches ResetPassword in routers/user/auth.go.

I've noticed that there are some helper functions in modules/auth/user_form.go that probably get called in the same stage, but the only things I have to go on are ResetPassword, reset_password, Code, and code and I'm not finding it.

Any ideas @daviian ?

@coolaj86 commented on GitHub (Oct 7, 2018): I've taken a peek, but there's magic going on somewhere that I haven't been able to track down and I could use some help. It's happening before it ever reaches `ResetPassword` in `routers/user/auth.go`. I've noticed that there are some helper functions in `modules/auth/user_form.go` that probably get called in the same stage, but the only things I have to go on are `ResetPassword`, `reset_password`, `Code`, and `code` and I'm not finding it. Any ideas @daviian ?
Author
Owner

@coolaj86 commented on GitHub (Oct 7, 2018):

Found it, but this change will require some adult supervision (@lafriks).

For Reference: The Stack

routers/routes/routes.go:

        reqSignOut := context.Toggle(&context.ToggleOptions{SignOutRequired: true})

routers/routes/routes.go:

        m.Group("/user", func() {
                ...
        }, reqSignOut)

Which is loading from modules/context/auth.go

Suggestion

I think that the simplest way to get the most value with the fewest code changes will be to make the result of clicking the link (AND clicking to update the password) perform these actions:

  • log out the current user, if any
  • update the password of the user associated with the account
  • log in that user associated with the link

There are some edge cases, but I don't believe that they're our responsibility:

  • If the user has forwarded the link to someone they shouldn't have, that's on them
  • If the user was logged into two browsers, opened the link in the wrong browser, and didn't intend to log out of their second account... meh, minor inconvenience

Some future improvements I think we could make:

  • show the email address associated with the reset code (I don't see any benefit to hiding it, the person with the link knows their email address already)

Implementation

We create a second /user group and move the reset password functions there (already tried this, it works)

routers/routes/routes.go:

        m.Group("/user", func() {
                // move password reset stuff here
                ...
        })
        m.Group("/user", func() {
                // all other stuff, as is
                ...
        }, reqSignOut)

Next we update ResetPasswordPost in routers/user/auth.go to call the same function as SignOut .

Bonus: Add the retype password, remember me, and and call handleSignInFull directly rather than redirecting to /login.

@coolaj86 commented on GitHub (Oct 7, 2018): Found it, but this change will require some adult supervision (@lafriks). For Reference: The Stack === `routers/routes/routes.go`: ``` reqSignOut := context.Toggle(&context.ToggleOptions{SignOutRequired: true}) ``` `routers/routes/routes.go`: ``` m.Group("/user", func() { ... }, reqSignOut) ``` Which is loading from `modules/context/auth.go` Suggestion ==== I think that the simplest way to get the most value with the fewest code changes will be to make the result of clicking the link (AND clicking to update the password) perform these actions: * log out the current user, if any * update the password of the user associated with the account * log in that user associated with the link There are some edge cases, but I don't believe that they're our responsibility: * If the user has forwarded the link to someone they shouldn't have, that's on them * If the user was logged into two browsers, opened the link in the wrong browser, and didn't intend to log out of their second account... meh, minor inconvenience Some future improvements I think we could make: * show the email address associated with the reset code (I don't see any benefit to hiding it, the person with the link knows their email address already) Implementation ==== We create a second `/user` group and move the reset password functions there (already tried this, it works) `routers/routes/routes.go`: ``` m.Group("/user", func() { // move password reset stuff here ... }) m.Group("/user", func() { // all other stuff, as is ... }, reqSignOut) ``` Next we update `ResetPasswordPost` in `routers/user/auth.go` to call the same function as `SignOut` . Bonus: Add the retype password, remember me, and and call `handleSignInFull` directly rather than redirecting to `/login`.
Author
Owner

@stale[bot] commented on GitHub (Jan 7, 2019):

This issue 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 weeks. Thank you for your contributions.

@stale[bot] commented on GitHub (Jan 7, 2019): This issue 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 weeks. Thank you for your contributions.
Author
Owner

@stale[bot] commented on GitHub (Feb 22, 2019):

This issue has been automatically closed because of inactivity. You can re-open it if needed.

@stale[bot] commented on GitHub (Feb 22, 2019): This issue has been automatically closed because of inactivity. You can re-open it if needed.
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: github-starred/gitea#2373