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

Password reset bug: "Your session has expired" #2382

Closed
timothyjensen opened this issue Sep 28, 2018 · 10 comments
Closed

Password reset bug: "Your session has expired" #2382

timothyjensen opened this issue Sep 28, 2018 · 10 comments

Comments

@timothyjensen
Copy link

timothyjensen commented Sep 28, 2018

I am encountering a problem when attempting to reset a client's password via the domain.com/client/recover_password page. The pasword reset form accepts the client's email address and emails a password reset link, but after clicking the email link and submitting the new password form, the client is redirected to the domain.com/client/session_expired page and sees the message:

Your session has expired. Please click the link in your email again.

Interestingly the new password does seem to work but obviously this is not a great user experience.

EDIT: I'm running v4.5.5

@passionInfinite
Copy link

@timothyjensen Did you kept your screen waiting for more than 4hours?

@turbo124
Copy link
Member

This was an issue with v4.5.2, but is resolved in v4.5.4

are you running the latest version?

@timothyjensen
Copy link
Author

@passionInfinite No I clicked the link right away.

@turbo124 I was running v4.5.2 before @hillelcoren informed me that it was a known bug and to update to v4.5.4. I went ahead and updated to v4.5.5 and am still having the same problem.

@turbo124
Copy link
Member

turbo124 commented Oct 3, 2018

@timothyjensen i can't recreate the issue unfortunately. are there any additional errors logged in storage/logs/laravel-error.log ?

@timothyjensen
Copy link
Author

@turbo124 No there are no relevant errors in the log. I spun up a test Invoice Ninja site with a fresh database, tried several different mail settings, in both FF and Chrome, browser extensions disabled, etc., and still have the problem. If you'd like I can add you as a user to the test install to see if you encounter the same problem.

@turbo124
Copy link
Member

turbo124 commented Oct 3, 2018 via email

@jclg
Copy link

jclg commented Oct 3, 2018

I was able to reproduce this issue (version v4.5.5).
After submitting the reset password form (/client/password/reset/{token}), I was redirected to /client/dashboard
This route matches Route::get('client/{contact_key?}', 'ClientPortalController@dashboard') with the middleware auth:client, but I am not authenticated as a client.
In this middleware, the contact is not valid so it returns \Redirect::to('client/session_expired');

I created a PR that should fix this problem by redirecting to /client/login instead of /client/dashboard

@timothyjensen
Copy link
Author

@jclg Thanks for taking a look at this issue! Your PR is definitely an improvement over the current experience and now I'm taken to the login screen instead of being shown the session expired error.

I'm not sure if this is the intended behavior, however. Shouldn't the user should be automatically logged in after submitting the form? The /password/reset route does automatically log in the user after password reset, which I think is the preferred user experience.

@jclg
Copy link

jclg commented Oct 3, 2018

@timothyjensen You are correct, the user should be automatically logged in after submitting the form. This is the expected behavior in Laravel.

According to the Laravel documentation (https://laravel.com/docs/5.7/passwords#after-resetting-passwords)

After a password is reset, the user will automatically be logged into the application and redirected to...

We can see this here:
https://github.com/laravel/framework/blob/5.4/src/Illuminate/Foundation/Auth/ResetsPasswords.php#L108

We will have to investigate why the client is not automatically logged in.

@turbo124 turbo124 added bug and removed triage labels Oct 4, 2018
@micah
Copy link

micah commented Oct 16, 2019

I'm also seeing this behavior, thanks for looking into this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants