Skip to content

Commit

Permalink
Fix failing csp/nonce check due to timed out session
Browse files Browse the repository at this point in the history
The CSP nonce is based on the CSRF token. This token does not change,
unless you log in (or out). In case of the session data being lost,
e.g. because php gets rid of old sessions, a new CSRF token is gen-
erated. While this is fine in theory, it actually caused some annoying
problems where the browser restored a tab and Nextcloud js was blocked
due to an outdated nonce.
The main problem here is that, while processing the request, we write
out security headers relatively early. At that point the CSRF token
is known/generated and transformed into a CSP nonce. During this request,
however, we also log the user in because the session information was
lost. At that point we also refresh the CSRF token, which eventually
causes the browser to block any scripts as the nonce in the header
does not match the one which is used to include scripts.
This patch adds a flag to indicate whether the CSRF token should be
refreshed or not. It is assumed that refreshing is only necessary
if we want to re-generate the session id too. To my knowledge, this
case only happens on fresh logins, not when we recover from a deleted
session file.

Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>
  • Loading branch information
ChristophWurst committed Sep 4, 2017
1 parent 30ca3b7 commit 87aeae2
Showing 1 changed file with 8 additions and 5 deletions.
13 changes: 8 additions & 5 deletions lib/private/User/Session.php
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ public function completeLogin(IUser $user, array $loginDetails, $regenerateSessi
}
$this->manager->emit('\OC\User', 'postLogin', [$user, $loginDetails['password']]);
if($this->isLoggedIn()) {
$this->prepareUserLogin($firstTimeLogin);
$this->prepareUserLogin($firstTimeLogin, $regenerateSessionId);
return true;
} else {
$message = \OC::$server->getL10N('lib')->t('Login canceled by app');
Expand Down Expand Up @@ -468,10 +468,13 @@ public function isTokenPassword($password) {
}
}

protected function prepareUserLogin($firstTimeLogin) {
// TODO: mock/inject/use non-static
// Refresh the token
\OC::$server->getCsrfTokenManager()->refreshToken();
protected function prepareUserLogin($firstTimeLogin, $refreshCsrfToken = true) {
if ($refreshCsrfToken) {
// TODO: mock/inject/use non-static
// Refresh the token
\OC::$server->getCsrfTokenManager()->refreshToken();
}

//we need to pass the user name, which may differ from login name
$user = $this->getUser()->getUID();
OC_Util::setupFS($user);
Expand Down

0 comments on commit 87aeae2

Please sign in to comment.