-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
copy remember-me value when renewing a session token #2353
Conversation
On renew, a session token is duplicated. For some reason we did not copy over the remember-me attribute value. Hence, the new token was deleted too early in the background job and remember-me did not work properly. Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>
@ChristophWurst, thanks for your PR! By analyzing the history of the files in this pull request, we identified @LukasReschke, @rullzer and @icewind1991 to be potential reviewers. |
@@ -368,12 +377,13 @@ public function testRenewSessionTokenWithPassword() { | |||
$newToken->setName('MyTokenName'); | |||
$newToken->setToken(hash('sha512', 'newId' . 'MyInstanceSecret')); | |||
$newToken->setType(IToken::TEMPORARY_TOKEN); | |||
$newToken->setRemember(IToken::REMEMBER); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interestingly, the test method also passes if I omit this line. I thought the line ->with($this->equalTo($newToken))
below would assert that those objects are equal. Apparently it doesn't really care, or at least with my version of phpunit locally.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess this is because the object is the same, but the content is not mandatorily the same. So it's not a in-depth comparison, but only a comparison of the "pointers" to an object.
Note: I applied the remember-me patch #1347 onto my nc10 installation and I've experience some unexpected logouts. I'm pretty sure this is what caused it. |
👍 |
1 similar comment
👍 |
On renew, a session token is duplicated. For some reason we did
not copy over the remember-me attribute value. Hence, the new token
was deleted too early in the background job and remember-me did
not work properly.
Signed-off-by: Christoph Wurst christoph@winzerhof-wurst.at