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

#6259 reset cookies on GCLogin failure #6917

Merged
merged 1 commit into from
Jan 20, 2018

Conversation

pstorch
Copy link
Contributor

@pstorch pstorch commented Jan 19, 2018

As discussed in #6259 this PR clears the cookies when a GCLogin failure is happening.

I don't know if this finally solves the issue because I cannot reproduce it myself. As users report that it is happening if c:geo wasn't used for quite some time. So we suspect stale session keys in the cookies.

@cgeo-bot
Copy link

Build finished.

@pstorch
Copy link
Contributor Author

pstorch commented Jan 20, 2018

Do we first want to test it in the nightlies for a while or should it be included in the next release?

@pstorch pstorch mentioned this pull request Jan 20, 2018
6 tasks
@Lineflyer
Copy link
Member

Difficult to test the error case, but we should verify that we have no regression.
As we currently have a beta out we should put it into release, publish a new beta.

We can still also test it in the nightlies in parallel.

@pstorch pstorch changed the base branch from master to release January 20, 2018 12:32
@Lineflyer
Copy link
Member

Questions:
AFAICS it resets on all errors.
Thats fine unless this reset would also delete name/password?!

So if e.g. the password is incorrect, it should not harm to delete the cookie but we should never delete username and password.

@pstorch pstorch force-pushed the 6259_gclogin_issue_cookie_reset branch from de1e916 to 352939b Compare January 20, 2018 12:33
@cgeo cgeo deleted a comment Jan 20, 2018
@pstorch pstorch force-pushed the 6259_gclogin_issue_cookie_reset branch from 352939b to b8e519d Compare January 20, 2018 12:40
@cgeo-bot
Copy link

Build finished.

@pstorch pstorch merged commit e5d3a66 into cgeo:release Jan 20, 2018
@cgeo-bot
Copy link

Build finished.

@pstorch
Copy link
Contributor Author

pstorch commented Jan 20, 2018

So if e.g. the password is incorrect, it should not harm to delete the cookie but we should never delete username and password.

No, username and password are not deleted. Only the login status is resetted.

@cgeo cgeo deleted a comment Jan 20, 2018
@cgeo-bot
Copy link

Build finished.

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

Successfully merging this pull request may close these issues.

None yet

3 participants