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

[stable12] Fix duplicate session token after remembered login #7568

Merged
merged 1 commit into from Dec 19, 2017

Conversation

@ChristophWurst
Member

ChristophWurst commented Dec 19, 2017

Backport of #6544 (comment) as requested by @oparoz / @rullzer

Original PR description:

On a remembered login session, we create a new session token
in the database with the values of the old one. As we actually
don't need the old session token anymore, we can delete it right
away.

I found out that one of the two commits I wanted to backport had already been backported and so this is less risky than I originally though. I tested this by logging in with remember-me checked and then deleted the session cookies that would expire when the session ends: On reload, I'm still logged in and there's still only one entry in the list of active browser sessions.

Fix duplicate session token after remembered login
On a remembered login session, we create a new session token
in the database with the values of the old one. As we actually
don't need the old session token anymore, we can delete it right
away.

Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>

@ChristophWurst ChristophWurst added this to the Nextcloud 12.0.5 milestone Dec 19, 2017

@ChristophWurst ChristophWurst self-assigned this Dec 19, 2017

@ChristophWurst ChristophWurst added this to SELECTED in Christoph's Tasks via automation Dec 19, 2017

@ChristophWurst ChristophWurst requested review from rullzer and MorrisJobke Dec 19, 2017

@ChristophWurst ChristophWurst moved this from SELECTED to TO REVIEW (max 4 PRs) in Christoph's Tasks Dec 19, 2017

@ChristophWurst

This comment has been minimized.

Member

ChristophWurst commented Dec 19, 2017

Don't know what happened to this one failing CI build but is seems unrelated to me.

@MorrisJobke

This comment has been minimized.

Member

MorrisJobke commented Dec 19, 2017

Offset within element cannot be scrolled into view: (0, 0): http://acceptance-app-files/index.php/apps/files/?dir=/&#
Build info: version: '2.53.1', revision: 'a36b8b1', time: '2016-06-30 17:37:03'
System info: host: '19da6b35ee7b', ip: '172.17.0.3', os.name: 'Linux', os.arch: 'amd64', os.version: '4.4.0-96-generic', java.version: '1.8.0_91'
Driver info: driver.version: unknown (WebDriver\Exception\MoveTargetOutOfBounds)

@danxuliu Any idea? I have seen this from time to time to fail.

@rullzer

This comment has been minimized.

Member

rullzer commented Dec 19, 2017

Other drone run was green so lets merge :)

@rullzer rullzer merged commit fe9f100 into stable12 Dec 19, 2017

1 of 2 checks passed

continuous-integration/drone/push the build failed
Details
continuous-integration/drone/pr the build was successful
Details

Christoph's Tasks automation moved this from TO REVIEW (max 4 PRs) to DONE Dec 19, 2017

@rullzer rullzer deleted the stable12-fix-duplicate-session-token branch Dec 19, 2017

@danxuliu

This comment has been minimized.

Member

danxuliu commented Dec 20, 2017

@MorrisJobke

Any idea? I have seen this from time to time to fail.

Unfortunately no. I will add taking a look to it to my TODO ;-)

@MorrisJobke MorrisJobke referenced this pull request Jan 8, 2018

Merged

12.0.5 RC1 #7740

13 of 13 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment