Skip to content

Commit

Permalink
fix(Authentication): correctly catch uniq constraint on token insert
Browse files Browse the repository at this point in the history
Signed-off-by: Grigorii K. Shartsev <[email protected]>
  • Loading branch information
ShGKme committed Apr 30, 2024
1 parent a73773b commit 131dbaf
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 14 deletions.
28 changes: 16 additions & 12 deletions lib/private/Authentication/Token/Manager.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,14 @@
*/
namespace OC\Authentication\Token;

use Doctrine\DBAL\Exception\UniqueConstraintViolationException;
use OC\Authentication\Exceptions\InvalidTokenException as OcInvalidTokenException;
use OC\Authentication\Exceptions\PasswordlessTokenException;
use OCP\Authentication\Exceptions\ExpiredTokenException;
use OCP\Authentication\Exceptions\InvalidTokenException;
use OCP\Authentication\Exceptions\WipeTokenException;
use OCP\Authentication\Token\IProvider as OCPIProvider;
use OCP\Authentication\Token\IToken as OCPIToken;
use OCP\DB\Exception;

class Manager implements IProvider, OCPIProvider {
/** @var PublicKeyTokenProvider */
Expand Down Expand Up @@ -77,18 +77,22 @@ public function generateToken(string $token,
$type,
$remember
);
} catch (UniqueConstraintViolationException $e) {
// It's rare, but if two requests of the same session (e.g. env-based SAML)
// try to create the session token they might end up here at the same time
// because we use the session ID as token and the db token is created anew
// with every request.
//
// If the UIDs match, then this should be fine.
$existing = $this->getToken($token);
if ($existing->getUID() !== $uid) {
throw new \Exception('Token conflict handled, but UIDs do not match. This should not happen', 0, $e);
} catch (Exception $e) {
if ($e->getReason() === Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) {
// It's rare, but if two requests of the same session (e.g. env-based SAML)
// try to create the session token they might end up here at the same time
// because we use the session ID as token and the db token is created anew
// with every request.
//
// If the UIDs match, then this should be fine.
$existing = $this->getToken($token);
if ($existing->getUID() !== $uid) {
throw new \Exception('Token conflict handled, but UIDs do not match. This should not happen', 0, $e);
}
return $existing;
}
return $existing;

throw $e;
}
}

Expand Down
6 changes: 4 additions & 2 deletions tests/lib/Authentication/Token/ManagerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
use OC\Authentication\Token\Manager;
use OC\Authentication\Token\PublicKeyToken;
use OC\Authentication\Token\PublicKeyTokenProvider;
use OC\DB\Exceptions\DbalException;
use PHPUnit\Framework\MockObject\MockObject;
use Test\TestCase;

Expand Down Expand Up @@ -79,8 +80,9 @@ public function testGenerateToken() {
}

public function testGenerateConflictingToken() {
/** @var MockObject|UniqueConstraintViolationException $exception */
$exception = $this->createMock(UniqueConstraintViolationException::class);
/** @var MockObject|UniqueConstraintViolationException $parent */
$parent = $this->createMock(UniqueConstraintViolationException::class);
$exception = DbalException::wrap($parent);

$token = new PublicKeyToken();
$token->setUid('uid');
Expand Down

0 comments on commit 131dbaf

Please sign in to comment.