Keep pw based auth tokens valid when pw-less login happens

[7118path]

Hi,

after publishing the eID-Login app for Nextcloud users reported about the problem, that after doing a passwordless login with an eID, all already existing password based authn tokens (browser sessions and app passwords) become invalidated. See eid-login/eid-login-nextcloud#2

When investigating the issue, I found the same happens when doing a WebAuthn (password ) login. The empty password from the login is used to update existing authn tokens, which invalidates them. They get removed on the next run of

server/lib/private/User/Session.php

Line 729 in c2f62ee

private function checkTokenCredentials(IToken $dbToken, $token) {

To keep the existing authn tokens valid, this pull requests prevents the updating of the tokens, if the given password is an empty string.

Please note: This are the two places I found where token passwords are updated on an login attempt. I am not sure why this must happen twice? There may be more? I don`t know.

Also there is a little inconsistency in the function signatures for setting token passwords. When creating a session token a 'null' value as password can be set, which is also set it as 'null' in the database:

server/lib/private/User/Session.php

Line 679 in c2f62ee

public function createSessionToken(IRequest $request, $uid, $loginName, $password = null, $remember = IToken::DO_NOT_REMEMBER) {

This is resulting in different handling of the token later on:

server/lib/private/Authentication/Token/DefaultTokenProvider.php

Lines 224 to 230 in c4159e3

	* @return string
	*/
	public function getPassword(IToken $savedToken, string $tokenId): string {
	$password = $savedToken->getPassword();
	if (is_null($password)) {
	throw new PasswordlessTokenException();
	}

server/lib/private/User/Session.php

Lines 740 to 748 in c2f62ee

	$pwd = $this->tokenProvider->getPassword($dbToken, $token);
	} catch (InvalidTokenException $ex) {
	// An invalid token password was used -> log user out
	return false;
	} catch (PasswordlessTokenException $ex) {
	// Token has no password
	if (!is_null($this->activeUser) && !$this->activeUser->isEnabled()) {
	$this->tokenProvider->invalidateToken($token);

In these functions 'null' is not allowed but an empty string must be used:

server/lib/private/Authentication/Token/DefaultTokenProvider.php

Line 86 in c4159e3

public function generateToken(string $token,

server/lib/private/Authentication/Token/DefaultTokenProvider.php

Line 242 in c4159e3

public function setPassword(IToken $token, string $tokenId, string $password) {

Maybe this APIs can be changed to handle the 'null' value more consistently?

TIA for looking into this and feedback about the issue!

Greetings!

[nickvergessen]

Conflicting files
lib/private/Authentication/Token/PublicKeyTokenProvider.php

You unluckily need to rebase on latest master

[juliusknorr]

@ecsecta Could you do a rebase of the branch instead of a merge with master and maybe also squash your commits into one?

[7118path]

@juliushaertl The last commit should contain all changes based on the latest master. Merging this commit should be sufficient. I have no idea how to squash all commit, when not merging. If I am wrong, maybe you could point me to some docs how to do what is needed now?

[LukasReschke]

/backport to stable22

[7118path]

Thank you!

[kesselb]

/backport to stable22

[kesselb]

/backport to stable22

[backportbot-nextcloud]

The backport to stable22 failed. Please do this backport manually.

[szaimen]

/backport to stable22

[szaimen]

/backport to stable21

[szaimen]

/backport to stable20

[backportbot-nextcloud]

The backport to stable20 failed. Please do this backport manually.

[backportbot-nextcloud]

The backport to stable21 failed. Please do this backport manually.

[backportbot-nextcloud]

The backport to stable22 failed. Please do this backport manually.

[ChristophWurst]

more luck on Friday 13th? let's fine out

[ChristophWurst]

/backport to stable22

[backportbot-nextcloud]

The backport to stable22 failed. Please do this backport manually.