Logout keeps old URL as redirect URL in Firefox?

[nickvergessen]

@schiessle and me experience this problem in Firefox. Neither Chromium nor Safari suffers from this issue, but we have not idea what's causing it.

1. As an admin log in
2. Go to admin settings
3. Click log out
4. URL is now index.php/login?redirect_url=/index.php/settings/admin/overview
5. Log in as a normal user
6. 💥

Access forbidden
Logged in user must be an admin

Actually the mistake is in step 4. After the logout you should not have the redirect_url anymore. Happens in latest 14 + master, not sure when it started.

[nickvergessen]

The problem is:

server/core/Controller/LoginController.php

Line 134 in 85eb43b

$response->addHeader('Clear-Site-Data', '"cache", "storage", "executionContexts"');

If I remove this, the redirect works as intended. With this line the current page is reloaded, instead of the login page being loaded as requested by the redirect. This causes the middleware to throw NotLoggedIn error, redirecting to the login page with redirect parameter:

server/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php

Lines 246 to 250 in 8c1e75e

	if (isset($this->request->server['REQUEST_URI'])) {
	$params['redirect_url'] = $this->request->server['REQUEST_URI'];
	}
	$url = $this->urlGenerator->linkToRoute('core.login.showLoginForm', $params);
	$response = new RedirectResponse($url);

According to https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Clear-Site-Data that is exactly what should happen. But https://www.w3.org/TR/clear-site-data/#grammardef-executioncontexts says that is how it should be. Any idea what we could do @rullzer

[rullzer]

🙈 so the reason it works on chrome is that is doen't recognize executionContext

[nickvergessen]

Well, or they take location into account too

[rullzer]

Nope I get a console warnings:

Unrecognized type: "executionContexts".

[nickvergessen]

Okay, so remove it as a current workaround?

[rullzer]

So what I think would make sense:

1. Remove for now.

And as a fix we can do

1. In logout add a session variable
2. In login check this variable if set => clear variable and clear execution context

Does that sound sane?

[rullzer]

I'll try to implement that this evening to see if it works.

[nickvergessen]

Sounds good

[ghost]

@rullzer , @nickvergessen

Excuse me if i comment on this one. I am not a professional developer but came here from a google search.

First of all thank you for directing me to the right part of the code.

I have / had this problem too. Looked at the code an verified it against the Clear-Site-Data Header specs.

It seems to me that the quotation marks in the directives are wrong.

I delete the quotation marks around directives on line 134. So my code looks like this...

$response->addHeader('Clear-Site-Data', 'cache, storage, executionContexts');

On my site a have no longer this redirect issue. No errors. Maybe this helps.

By the way... Nextcloud 14.0.4, Firefox 63.0.3 in Linux Mint 19

If it is not the way it should be or i am missing something, ignore my idea :o)

Edit 25.11.
I forgot to mention that the headers are sent.

With the quotation marks:
clear-site-data: "cache", "storage", "executionContexts"

Without the quotation marks:
clear-site-data: cache, storage, executionContexts

I don't know why Firefox does not like the quotation marks in the header directives.

Second edit 25.11.

As stated here : https://github.com/w3c/webappsec-referrer-policy/issues/97 it is possible that the MDN documentation on clear-site-data ( https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Clear-Site-Data ) is also wrong on the quotation marks. Thus sending these in the header causes Firefox to ignore the values as it did expect them without quotation marks.

The documentation on cache-control does not use Quotation marks. https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Cache-Control

Maybe that also explains the error rullzer gets. It is indeed an unrecognized type.

Only my two cents.

[BstAA]

With the changes from tfd0207 it works for me on all browsers.
But even with Nextcloud 15.0.2 it's not fixed ;-)

[m-i-k-e-y]

Can confirm the findings of tfd0207 and BstAA:

changing line 134 in LoginController.php in NC 15.0.2 from

    $response->addHeader('Clear-Site-Data', '"cache", "storage", "executionContexts"');

to

    $response->addHeader('Clear-Site-Data', 'cache, storage, executionContexts');

solves the issue in Firefox.

[rullzer]

Well the fix works because (as stated in #12568 (comment)) the quotation marks are needed per RFC. Without it they are just ignored.

[BstAA]

Well the fix works because (as stated in #12568 (comment)) the quotation marks are needed per RFC. Without it they are just ignored.

Your right and I've seen you closed #13267 just yesterday BUT: Even if the NC code is right as per RFC Firefox users get into trouble. -> As as well Chrome as Edge act right even with the patch. Why not use it as long as Firefox uses it's own interpretation and opening a bug report for Firefox?

By the way, I'm a brand new NC user with nearly no experience in php-programming.

[nickvergessen]

The problem is when you remove the quotes, Firefox ignores it and does not clear executeContexts anymore. So that leaves FF users in an even worse state.

[tgunr]

Seems to still be an issue, got into a state where FF 87.0 (64-bit) on MacOS 11 could not login due to redirect. Removed the " from addHeader line and now able to see dashboard.