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

cookie to receive the login in the browser can not be read -> user is logged out after closing the browser #24438

Closed
4tler opened this issue Nov 29, 2020 · 1 comment · Fixed by #25302
Labels
0. Needs triage Pending check for reproducibility or if it fits our roadmap bug

Comments

@4tler
Copy link

4tler commented Nov 29, 2020

Version: NC 20.0.2
PHP: 7.4
web: NGinx
tested browsers: Firefox, Chrome

Readjust error:

  1. create new user
  2. insert a space in the user name (e.g. Hans Peter)
  3. close browser
  4. open browser

If the username contains a space, the cookie which indicates that you are already logged in to this system can probably not be read or correctly assigned. This means that you will have to log in again and again as soon as the browser is closed.

This was tested on 3 NC instances by creating one user with and one without spaces. The one without spaces remained logged in, the one with had to log in again and again.

@skjnldsv skjnldsv added bug 0. Needs triage Pending check for reproducibility or if it fits our roadmap labels Dec 30, 2020
@mziech
Copy link
Contributor

mziech commented Jan 6, 2021

I was observing the same behavior on my instance, essentially all users with a space in the name have to login twice after the session expired.

I tracked down the issue to a non-obvious change in PHP 7.4.2 and 7.4.3, which makes cookie encoding and decoding RFC compliant. See:

Basically PHP < 7.4.2 would encode Hans Peter as Hans+Peter and decode back to Hans Peter. However, PHP > 7.4.3 encode Hans Peter to Hans%20Peter and decode Hans+Peter to Hans+Peter leading to a clash in the expected UID.

php > echo(rawurldecode(urlencode('Hans Peter')));
Hans+Peter
php > echo(urldecode(rawurlencode('Hans Peter')));
Hans Peter
php > echo(urldecode(urlencode('Hans Peter')));
Hans Peter
php > echo(rawurldecode(rawurlencode('Hans Peter')));
Hans Peter

Since NextCloud uses its own cookie encoding function, the PHP 7.4.3 encoding fix won't work. I fixed the issue by changing urlencode to rawurlencode in https://github.com/nextcloud/server/blob/master/lib/private/Http/CookieHelper.php#L46

mziech added a commit to mziech/server that referenced this issue Jan 24, 2021
PHP 7.4.2 changed the way how cookies are decoded, applying RFC-compliant raw URL decoding. This leads to a conflict Nextcloud's own cookie encoding, breaking the remember-me function if the UID contains a space character.

Fixes nextcloud#24438
mziech added a commit to mziech/server that referenced this issue Jan 24, 2021
PHP 7.4.2 changed the way how cookies are decoded, applying RFC-compliant raw URL decoding. This leads to a conflict Nextcloud's own cookie encoding, breaking the remember-me function if the UID contains a space character.

Fixes nextcloud#24438

Signed-off-by: Marco Ziech <marco@ziech.net>
backportbot-nextcloud bot pushed a commit that referenced this issue Jan 29, 2021
PHP 7.4.2 changed the way how cookies are decoded, applying RFC-compliant raw URL decoding. This leads to a conflict Nextcloud's own cookie encoding, breaking the remember-me function if the UID contains a space character.

Fixes #24438

Signed-off-by: Marco Ziech <marco@ziech.net>
backportbot-nextcloud bot pushed a commit that referenced this issue Jan 29, 2021
PHP 7.4.2 changed the way how cookies are decoded, applying RFC-compliant raw URL decoding. This leads to a conflict Nextcloud's own cookie encoding, breaking the remember-me function if the UID contains a space character.

Fixes #24438

Signed-off-by: Marco Ziech <marco@ziech.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0. Needs triage Pending check for reproducibility or if it fits our roadmap bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants