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

Issue with session ID being refreshed twice during login #89

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Jehong-Ahn
Copy link

@Jehong-Ahn Jehong-Ahn commented Mar 25, 2025

During login, the Session::migrate() function is already being called within attempt() to refresh the session ID. When regenerate() is subsequently called, it triggers Session::migrate() again, causing the session ID to be refreshed a second time.

This has been fixed by only calling regenerateToken().

@tnylea
Copy link
Contributor

tnylea commented Mar 31, 2025

Hey, @Jehong-Ahn. Thanks for the PR.

Can you help me understand what this is resolving? The regenerate() method essentially calls the regenerateToken() just the same. It is not regenerating the session ID because, by default, you can see that on regenerate($destroy=false) destroy is set to false: https://github.com/laravel/framework/blob/12.x/src/Illuminate/Session/Store.php#L592,

Maybe there's something else I'm not understanding. If so, let me know, and I'll look into it further.

Appreciate it!

@tnylea tnylea added the Awaiting User Response Waiting for developers response label Mar 31, 2025
@Jehong-Ahn
Copy link
Author

Hello, @tnylea. Thank you for your warm welcome.

In the current implementation, the $destroy parameter is passed to the migrate() method. However, regardless of the value of $destroy, migrate() always calls generateSessionId().

As a result, if migrate() is invoked twice during the login process, generateSessionId() will also be executed twice.

https://github.com/laravel/framework/blob/12.x/src/Illuminate/Session/Store.php#L613

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting User Response Waiting for developers response
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants