`checkPassword` function is incomplete

While performing testing of the following PRs, a bug was detected in the checkPassword function:
https://github.com/nextcloud/server/pull/27929
https://github.com/nextcloud/user_saml/pull/537

Steps to reproduce

1. Check out and apply the PRs noted above.
2. Configure the user_saml app to map a password/secret sent from the IdP (in this case, I'm using Keycloak).
3. Create a new user in the IdP and log into NC.
4. As an example, access the Files app. After five minutes, a call to getstoragestats.php will be triggered resulting in a session authentication check.
5. To manually force the behavior, reset the user's authtoken last_check field by executing UPDATE oc_authtoken SET last_check = 0; in the database. Then, run $.getJSON(OC.filePath('files','ajax','getstoragestats.php')) in the browser's console.

Expected behaviour

The user's session should stay authenticated without issue.

Actual behaviour

When any call is made that triggers a session authentication check, the user's page is automatically redirected back to the IdP. However, since the IdP session is still valid, the IdP then automatically re-authenticates with NC and logs the user back in without any action needed from the user.

Server configuration

Operating system:
Debian 11

Web server:
nginx

Database:
MariaDB

PHP version:
8.0

Nextcloud version: (see Nextcloud admin page)
22.1.0

Where did you install Nextcloud from:
Downloaded from https://download.nextcloud.com/server/releases/

List of activated apps:

encryption (Default encryption module)
user_saml (SSO & SAML authentication)

Nextcloud configuration:

{
    "system": {
        "version": "22.1.0.1",
        "installed": true,
        "dbtype": "mysql",
        "dbport": "",
        "dbtableprefix": "oc_",
        "mysql.utf8mb4": true,
        "maintenance": false,
    }
}

Client configuration

Browser:
Chromium 90

Operating system:
Debian 11

IdP:
Keycloak v15

Logs

Nextcloud log (data/owncloud.log)

{"reqId":"GzoPbebScVMN0QFOnFQ0","level":2,"time":"2021-09-15T04:15:52+00:00","remoteAddr":"::1","user":"testuser","app":"core","method":"GET","url":"/index.php/css/core/cbe0-8fa8-server.css?v=d41d8cd98f00b204e9800998ecf8427e-","message":"Login failed: 'testuser' (Remote IP: '::1')","userAgent":"Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/90.0.4430.212 Safari/537.36","version":"22.1.0.1"}
{"reqId":"GzoPbebScVMN0QFOnFQ0","level":3,"time":"2021-09-15T04:15:52+00:00","remoteAddr":"::1","user":"--","app":"PHP","method":"GET","url":"/index.php/css/core/cbe0-8fa8-server.css?v=d41d8cd98f00b204e9800998ecf8427e-","message":"session_start(): A session had already been started - ignoring at /var/www/nextcloud/lib/private/Session/Internal.php#206","userAgent":"Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/90.0.4430.212 Safari/537.36","version":"22.1.0.1","exception":{"Exception":"Error","Message":"session_start(): A session had already been started - ignoring at /var/www/nextcloud/lib/private/Session/Internal.php#206","Code":0,"Trace":[{"function":"onAll","class":"OC\\Log\\ErrorHandler","type":"::"},{"function":"session_start"},{"file":"/var/www/nextcloud/lib/private/Session/Internal.php","line":206,"function":"call_user_func_array"},{"file":"/var/www/nextcloud/lib/private/Session/Internal.php","line":216,"function":"invoke","class":"OC\\Session\\Internal","type":"->"},{"file":"/var/www/nextcloud/lib/private/Session/Internal.php","line":106,"function":"startSession","class":"OC\\Session\\Internal","type":"->"},{"file":"/var/www/nextcloud/lib/private/Session/CryptoSessionData.php","line":149,"function":"clear","class":"OC\\Session\\Internal","type":"->"},{"file":"/var/www/nextcloud/lib/private/User/Session.php","line":933,"function":"clear","class":"OC\\Session\\CryptoSessionData","type":"->"},{"file":"/var/www/nextcloud/lib/private/User/Session.php","line":270,"function":"logout","class":"OC\\User\\Session","type":"->"},{"file":"/var/www/nextcloud/lib/private/User/Session.php","line":243,"function":"validateSession","class":"OC\\User\\Session","type":"->"},{"file":"/var/www/nextcloud/apps/user_saml/appinfo/app.php","line":108,"function":"getUser","class":"OC\\User\\Session","type":"->"},{"file":"/var/www/nextcloud/lib/private/legacy/OC_App.php","line":303,"args":["/var/www/nextcloud/apps/user_saml/appinfo/app.php"],"function":"require_once"},{"file":"/var/www/nextcloud/lib/private/legacy/OC_App.php","line":185,"function":"requireAppFile","class":"OC_App","type":"::"},{"file":"/var/www/nextcloud/lib/private/legacy/OC_App.php","line":139,"function":"loadApp","class":"OC_App","type":"::"},{"file":"/var/www/nextcloud/lib/base.php","line":979,"function":"loadApps","class":"OC_App","type":"::"},{"file":"/var/www/nextcloud/index.php","line":36,"function":"handleRequest","class":"OC","type":"::"}],"File":"/var/www/nextcloud/lib/private/Log/ErrorHandler.php","Line":99,"CustomMessage":"--"}}

Suggested solution

I have identified the source of the problem, but I'm not entirely sure if my solution is the best method. Before opening a PR, I wanted to have a discussion in an issue submission.

In OCA\User_SAML\UserBackend, replace the checkPassword function with the following (including the two additional private functions):

public function checkPassword($uid, $password) {
	/* @var $qb IQueryBuilder */
	$qb = $this->db->getQueryBuilder();
	$qb->select('token', 'private_key', 'password')
		// Previously used the 'user_saml_auth_token' table, but it is never used
		->from('authtoken')
		->where($qb->expr()->eq('uid', $qb->createNamedParameter($uid)))
		->setMaxResults(1000);
	$result = $qb->execute();
	$data = $result->fetchAll();
	$result->closeCursor();

	$secret = $this->config->getSystemValue('secret');
	$instanceid = $this->config->getSystemValue('instanceid');
	$token = \OC::$server->getRequest()->getCookie($instanceid);
	$hashedToken = hash('sha512', $token . $secret);

	// I'm not sure how extensive these checks need to be. Is it required to
	// decrypt all the way down to the stored private key and password? No
	// idea, but I went to that extent for completeness.
	foreach($data as $passwords) {
		if ($hashedToken == $passwords['token']) {
			$privateKey = $this->decrypt($passwords['private_key'], $token);

			if (!is_null($password)) {
				$decryptedPassword = $this->decryptPassword($passwords['password'], $privateKey);

				if ($decryptedPassword == $password) {
					return $uid;
				}
			}
			else {
				return $uid;
			}
		}
	}

	return false;
}

/**
 * @throws InvalidTokenException
 * Adapted from OC\Authentication\Token\PublicKeyTokenProvider
 */
private function decrypt(string $cipherText, string $token): string {
	$secret = $this->config->getSystemValue('secret');
	$crypto = \OC::$server->getCrypto();
	$provider = \OC::$server->query('OC\Authentication\Token\PublicKeyTokenProvider');

	try {
		return $crypto->decrypt($cipherText, $token . $secret);
	} catch (\Exception $ex) {
		// Delete the invalid token
		$provider->invalidateToken($token);
		throw new \OC\Authentication\Exceptions\InvalidTokenException("Could not decrypt token password: " . $ex->getMessage(), 0, $ex);
	}
}

/**
 * From OC\Authentication\Token\PublicKeyTokenProvider
 */
private function decryptPassword(string $encryptedPassword, string $privateKey): string {
	$encryptedPassword = base64_decode($encryptedPassword);
	openssl_private_decrypt($encryptedPassword, $password, $privateKey, OPENSSL_PKCS1_OAEP_PADDING);

	return $password;
}

Start in OCA\User_SAML\UserBackend at the checkPassword function and review the authentication flow described in PRs 27929 and 537. Reproduce the session authentication check with the user_saml app and verify that a valid IdP session remains authenticated without redirecting back to the IdP.