diff --git a/integration-tests/auth_refresh_test.go b/integration-tests/auth_refresh_test.go new file mode 100644 index 00000000..849b9637 --- /dev/null +++ b/integration-tests/auth_refresh_test.go @@ -0,0 +1,62 @@ +package tests + +import ( + "net/http/httptest" + "strings" + "sync" + "testing" + "time" + + "github.com/stretchr/testify/assert" + + "github.com/upsun/cli/pkg/mockapi" +) + +// TestAuthRefresh_Concurrent runs several processes that load the same expired session. +// Only one should use the refresh token: the others must use the token it saved. +func TestAuthRefresh_Concurrent(t *testing.T) { + authServer := mockapi.NewAuthServer(t) + defer authServer.Close() + // Keep the first refresh in progress while the other processes start. + authServer.SetRefreshDelay(2 * time.Second) + authServer.AddRefreshToken("initial-refresh-token") + + apiHandler := mockapi.NewHandler(t) + apiHandler.SetMyUser(&mockapi.User{ID: "u1"}) + apiServer := httptest.NewServer(apiHandler) + defer apiServer.Close() + + f := newCommandFactory(t, apiServer.URL, authServer.URL) + f.extraEnv = append(f.extraEnv, EnvPrefix+"TOKEN=") + // Initialize before running commands concurrently. + f.dir = t.TempDir() + getCommandName(t) + writeOAuthSession(t, f.home, "default", map[string]any{ + "accessToken": "expired-token", + "tokenType": "bearer", + "expires": time.Now().Add(-time.Hour).Unix(), + "refreshToken": "initial-refresh-token", + }) + + const processes = 3 + type result struct { + stdout, stderr string + err error + } + results := make([]result, processes) + var wg sync.WaitGroup + for i := range processes { + wg.Go(func() { + stdout, stderr, err := f.RunCombinedOutput("auth:token", "--no-warn") + results[i] = result{stdout, stderr, err} + }) + } + wg.Wait() + + for i, r := range results { + if assert.NoError(t, r.err, "process %d stderr: %s", i, r.stderr) { + assert.Equal(t, "access-token-1", strings.TrimSpace(r.stdout), "process %d", i) + } + } + assert.False(t, authServer.ReuseDetected(), "a rotated refresh token was reused") +} diff --git a/legacy/composer.json b/legacy/composer.json index 3380afe7..cca78fa2 100644 --- a/legacy/composer.json +++ b/legacy/composer.json @@ -21,7 +21,7 @@ "khill/php-duration": "^1.1", "symfony/polyfill-mbstring": "^1.19", "symfony/polyfill-iconv": "^1.19", - "platformsh/oauth2": "^1@beta", + "platformsh/oauth2": "^1.0.0-beta4", "giggsey/libphonenumber-for-php-lite": "^8.13", "symfony/var-dumper": "^7.3" }, diff --git a/legacy/composer.lock b/legacy/composer.lock index ad2cb224..0b7b9b0b 100644 --- a/legacy/composer.lock +++ b/legacy/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "7b82e9a5b71609c891c5da0ec002ec94", + "content-hash": "aeaacf3fb12f52bfbb228bd5541ea1fa", "packages": [ { "name": "cocur/slugify", @@ -794,12 +794,12 @@ "source": { "type": "git", "url": "https://github.com/platformsh/platformsh-client-php.git", - "reference": "ae46040164a44efaeec15758ec4220e3cbf3e3fd" + "reference": "a263eb88563b3aeb1f9fdcae627144e2acb51e30" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/platformsh/platformsh-client-php/zipball/ae46040164a44efaeec15758ec4220e3cbf3e3fd", - "reference": "ae46040164a44efaeec15758ec4220e3cbf3e3fd", + "url": "https://api.github.com/repos/platformsh/platformsh-client-php/zipball/a263eb88563b3aeb1f9fdcae627144e2acb51e30", + "reference": "a263eb88563b3aeb1f9fdcae627144e2acb51e30", "shasum": "" }, "require": { @@ -835,7 +835,7 @@ "issues": "https://github.com/platformsh/platformsh-client-php/issues", "source": "https://github.com/platformsh/platformsh-client-php/tree/3.x" }, - "time": "2026-08-07T13:05:25+00:00" + "time": "2026-10-01T00:14:21+00:00" }, { "name": "platformsh/console-form", @@ -883,16 +883,16 @@ }, { "name": "platformsh/oauth2", - "version": "1.0.0-beta3", + "version": "1.0.0-beta4", "source": { "type": "git", "url": "https://github.com/platformsh/platformsh-oauth2-php.git", - "reference": "3c0e12549850837a827ca432a50aa052e3cab250" + "reference": "38fd5e7941247ea183a336add0ab993ae7e7422f" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/platformsh/platformsh-oauth2-php/zipball/3c0e12549850837a827ca432a50aa052e3cab250", - "reference": "3c0e12549850837a827ca432a50aa052e3cab250", + "url": "https://api.github.com/repos/platformsh/platformsh-oauth2-php/zipball/38fd5e7941247ea183a336add0ab993ae7e7422f", + "reference": "38fd5e7941247ea183a336add0ab993ae7e7422f", "shasum": "" }, "require": { @@ -921,9 +921,9 @@ "description": "Platform.sh OAuth2 client", "support": { "issues": "https://github.com/platformsh/platformsh-oauth2-php/issues", - "source": "https://github.com/platformsh/platformsh-oauth2-php/tree/1.0.0-beta3" + "source": "https://github.com/platformsh/platformsh-oauth2-php/tree/1.0.0-beta4" }, - "time": "2024-11-25T19:19:41+00:00" + "time": "2026-10-01T00:14:08+00:00" }, { "name": "psr/container", diff --git a/legacy/src/Service/Api.php b/legacy/src/Service/Api.php index 93a30056..6880cf30 100644 --- a/legacy/src/Service/Api.php +++ b/legacy/src/Service/Api.php @@ -312,17 +312,30 @@ private function getConnectorOptions(): array // Acquire a lock to prevent tokens being refreshed at the same time in // different CLI processes. $refreshLockName = 'refresh--' . $this->config->getSessionIdSlug(); - $connectorOptions['on_refresh_start'] = function ($originalRefreshToken) use ($refreshLockName) { + $connectorOptions['on_refresh_start'] = function (string $originalRefreshToken) use ($refreshLockName): ?AccessToken { $this->io->debug('Refreshing access token'); - $connector = $this->getClient(false)->getConnector(); - return $this->fileLock->acquireOrWait($refreshLockName, function (): void { + $this->fileLock->acquireOrWait($refreshLockName, function (): void { $this->stdErr->writeln('Waiting for token refresh lock', OutputInterface::VERBOSITY_VERBOSE); - }, function () use ($connector, $originalRefreshToken) { - $session = $connector->getSession(); - $accessToken = $this->tokenFromSession($session); - return $accessToken && $accessToken->getRefreshToken() !== $originalRefreshToken - ? $accessToken : null; }); + // Without the lock, the token could be refreshed or saved over another process's newer token. + if (!$this->fileLock->isHeld($refreshLockName)) { + throw new \RuntimeException('Timed out waiting for another process to refresh the access token. Please try again.'); + } + + // Refresh tokens are single-use, so use the stored token if another process has refreshed it. + $session = $this->getClient(false)->getConnector()->getSession(); + try { + $session->reload(); + } catch (\RuntimeException $e) { + throw $this->convertStorageException($e); + } + $storedToken = $this->tokenFromSession($session); + if ($storedToken && $storedToken->getRefreshToken() !== $originalRefreshToken) { + return $storedToken; + } + + // The middleware refreshes the token, and saves it before calling on_refresh_end. + return null; }; $connectorOptions['on_refresh_end'] = function () use ($refreshLockName): void { $this->fileLock->release($refreshLockName); @@ -460,6 +473,22 @@ private function isApiTokenInvalid(mixed $body): bool return false; } + /** + * Converts a session storage error into a keyring error, if applicable. + */ + private function convertStorageException(\RuntimeException $e): \RuntimeException + { + if ($this->sessionStorage instanceof CredentialHelperStorage) { + $previous = $e->getPrevious(); + if ($previous instanceof ProcessTimedOutException) { + return KeyringUnavailableException::fromTimeout($previous); + } elseif ($previous instanceof ProcessFailedException) { + return KeyringUnavailableException::fromFailure($previous); + } + } + return $e; + } + /** * Loads and returns an AccessToken, if possible, from a session. * @@ -549,15 +578,7 @@ public function getClient(bool $autoLogin = true, bool $reset = false): Platform try { $session->setStorage($this->sessionStorage); } catch (\RuntimeException $e) { - if ($this->sessionStorage instanceof CredentialHelperStorage) { - $previous = $e->getPrevious(); - if ($previous instanceof ProcessTimedOutException) { - throw KeyringUnavailableException::fromTimeout($previous); - } elseif ($previous instanceof ProcessFailedException) { - throw KeyringUnavailableException::fromFailure($previous); - } - } - throw $e; + throw $this->convertStorageException($e); } } diff --git a/legacy/src/Service/FileLock.php b/legacy/src/Service/FileLock.php index cb57d59a..ceb749b3 100644 --- a/legacy/src/Service/FileLock.php +++ b/legacy/src/Service/FileLock.php @@ -4,25 +4,34 @@ namespace Platformsh\Cli\Service; +/** + * Locks between CLI processes, using OS file locks. + * + * The OS releases a lock when its process exits, even if it crashes. + */ class FileLock { private readonly int $checkIntervalMs; - private readonly int $timeLimit; private readonly bool $disabled; - /** @var array */ + /** @var array */ private array $locks = []; - public function __construct(private readonly Config $config) + /** + * @param int $timeLimit The maximum time to wait for a lock, in seconds. + */ + public function __construct(private readonly Config $config, private readonly int $timeLimit = 30) { $this->checkIntervalMs = 500; - $this->timeLimit = 30; $this->disabled = $this->config->getBool('api.disable_locks'); } /** * Acquires a lock, or waits for one if it already exists. * + * If the lock is not acquired within the time limit, this returns null + * without holding the lock. + * * @param string $lockName * A unique name for the lock. * @param callable|null $onWait @@ -36,20 +45,16 @@ public function __construct(private readonly Config $config) */ public function acquireOrWait(string $lockName, ?callable $onWait = null, ?callable $check = null): mixed { - if ($this->disabled) { + if ($this->disabled || isset($this->locks[$lockName])) { return null; } - $runOnWait = false; - $filename = $this->filename($lockName); + $handle = $this->open($this->filename($lockName)); $start = \time(); - while (\time() - $start < $this->timeLimit) { - if (!\file_exists($filename)) { - break; - } - $content = $this->readWithLock($filename); - $lockedAt = \intval($content); - if ($lockedAt === 0 || \time() >= $lockedAt + $this->timeLimit) { - break; + $runOnWait = false; + while (!\flock($handle, LOCK_EX | LOCK_NB)) { + if (\time() - $start >= $this->timeLimit) { + \fclose($handle); + return null; } if ($onWait !== null && !$runOnWait) { $onWait(); @@ -59,23 +64,31 @@ public function acquireOrWait(string $lockName, ?callable $onWait = null, ?calla if ($check !== null) { $result = $check(); if ($result !== null) { - $this->release($lockName); + \fclose($handle); return $result; } } } - $this->writeWithLock($filename, (string) \time()); - $this->locks[$lockName] = $lockName; + $this->locks[$lockName] = $handle; return null; } /** - * Releases a lock that was created by acquire(). + * Checks whether this process holds a lock, or locks are disabled. + */ + public function isHeld(string $lockName): bool + { + return $this->disabled || isset($this->locks[$lockName]); + } + + /** + * Releases a lock that was created by acquireOrWait(). */ public function release(string $lockName): void { - if (!$this->disabled && isset($this->locks[$lockName])) { - $this->writeWithLock($this->filename($lockName), ''); + if (isset($this->locks[$lockName])) { + \flock($this->locks[$lockName], LOCK_UN); + \fclose($this->locks[$lockName]); unset($this->locks[$lockName]); } } @@ -85,7 +98,7 @@ public function release(string $lockName): void */ public function __destruct() { - foreach ($this->locks as $lockName) { + foreach (\array_keys($this->locks) as $lockName) { $this->release($lockName); } } @@ -103,72 +116,21 @@ private function filename(string $lockName): string } /** - * Reads a file using a shared lock. + * Opens a lock file, creating it if necessary. * - * @param string $filename - * @return string + * @return resource */ - private function readWithLock(string $filename): string - { - $handle = \fopen($filename, 'r'); - if (!$handle) { - throw new \RuntimeException('Failed to open file for reading: ' . $filename); - } - try { - if (!\flock($handle, LOCK_SH)) { - \trigger_error('Failed to lock file: ' . $filename, E_USER_WARNING); - } - $content = \fgets($handle); - if ($content === false && !\feof($handle)) { - throw new \RuntimeException('Failed to read file: ' . $filename); - } - } finally { - if (!\flock($handle, LOCK_UN)) { - \trigger_error('Failed to unlock file: ' . $filename, E_USER_WARNING); - } - if (!\fclose($handle)) { - \trigger_error('Failed to close file: ' . $filename, E_USER_WARNING); - } - } - return (string) $content; - } - - /** - * Writes to a file using an exclusive lock. - * - * @param string $filename - * @param string $content - * @return void - */ - private function writeWithLock(string $filename, string $content): void + private function open(string $filename) { $dir = \dirname($filename); - if (!\is_dir($dir)) { - if (!\mkdir($dir, 0o777, true)) { - throw new \RuntimeException('Failed to create directory: ' . $dir); - } + if (!\is_dir($dir) && !\mkdir($dir, 0o777, true) && !\is_dir($dir)) { + throw new \RuntimeException('Failed to create directory: ' . $dir); } - $handle = \fopen($filename, 'w'); + // Mode "c" creates the file without truncating it. + $handle = \fopen($filename, 'c'); if (!$handle) { - throw new \RuntimeException('Failed to open file for writing: ' . $filename); - } - try { - if (!\flock($handle, LOCK_EX)) { - \trigger_error('Failed to lock file: ' . $filename, E_USER_WARNING); - } - if (\fputs($handle, $content) === false) { - throw new \RuntimeException('Failed to write to file: ' . $filename); - } - if (!\fsync($handle)) { - \trigger_error('Failed to sync file (fsync): ' . $filename, E_USER_WARNING); - } - } finally { - if (!\flock($handle, LOCK_UN)) { - \trigger_error('Failed to unlock file: ' . $filename, E_USER_WARNING); - } - if (!\fclose($handle)) { - \trigger_error('Failed to close file: ' . $filename, E_USER_WARNING); - } + throw new \RuntimeException('Failed to open lock file: ' . $filename); } + return $handle; } } diff --git a/legacy/tests/LockHolderTrait.php b/legacy/tests/LockHolderTrait.php new file mode 100644 index 00000000..5b96e031 --- /dev/null +++ b/legacy/tests/LockHolderTrait.php @@ -0,0 +1,34 @@ + %s])); $l->acquireOrWait(%s); echo "locked\n"; sleep(%d);', + \var_export(\dirname(__DIR__) . '/vendor/autoload.php', true), + FileLock::class, + Config::class, + \var_export($homeDir, true), + \var_export($lockName, true), + $holdSeconds, + ); + $process = \proc_open([\PHP_BINARY, '-r', $script], [1 => ['pipe', 'w']], $pipes); + $this->assertIsResource($process); + $this->assertSame("locked\n", \fgets($pipes[1])); + + return $process; + } +} diff --git a/legacy/tests/Service/ApiRefreshLockTest.php b/legacy/tests/Service/ApiRefreshLockTest.php new file mode 100644 index 00000000..497b8a38 --- /dev/null +++ b/legacy/tests/Service/ApiRefreshLockTest.php @@ -0,0 +1,169 @@ +tempDirSetUp(); + $this->storage = new File($this->config()->getSessionDir()); + } + + public function tearDown(): void + { + // Api caches the client statically: do not leak this session into other tests. + (new \ReflectionProperty(Api::class, 'client'))->setValue(null, null); + } + + public function testUsesTheStoredTokenWhenTheLockIsFree(): void + { + $onRefreshStart = $this->loadStaleSession($this->config()); + + $token = $onRefreshStart('refresh-1'); + + $this->assertInstanceOf(AccessToken::class, $token); + $this->assertSame('access-2', $token->getToken()); + $this->assertSame('refresh-2', $token->getRefreshToken()); + } + + public function testUsesTheStoredTokenAfterWaiting(): void + { + $config = $this->config(); + $onRefreshStart = $this->loadStaleSession($config); + $holder = $this->startLockHolder((string) $this->tempDir, 'refresh--' . $config->getSessionIdSlug(), 1); + + $token = $onRefreshStart('refresh-1'); + \proc_close($holder); + + $this->assertInstanceOf(AccessToken::class, $token); + $this->assertSame('refresh-2', $token->getRefreshToken()); + } + + public function testAllowsARefreshWhenTheStoredTokenIsUnchanged(): void + { + $config = $this->config(); + $this->storage->save('refresh-test', $this->sessionData('access-1', 'refresh-1')); + ['on_refresh_start' => $onRefreshStart, 'on_refresh_end' => $onRefreshEnd] = $this->connector($config)->getConfig(); + $this->assertIsCallable($onRefreshStart); + $this->assertIsCallable($onRefreshEnd); + $lockName = 'refresh--' . $config->getSessionIdSlug(); + $otherProcess = new FileLock($config, 1); + + // The middleware refreshes while the lock is held. + $this->assertNull($onRefreshStart('refresh-1')); + $otherProcess->acquireOrWait($lockName); + $this->assertFalse($otherProcess->isHeld($lockName)); + + $onRefreshEnd('refresh-1'); + $otherProcess->acquireOrWait($lockName); + $this->assertTrue($otherProcess->isHeld($lockName)); + } + + /** + * @return array + */ + public static function timeoutCases(): array + { + return [ + 'unchanged stored token' => [false], + // The lock holder may be about to save an even newer token. + 'newer stored token' => [true], + ]; + } + + #[DataProvider('timeoutCases')] + public function testFailsAfterTimingOut(bool $storedTokenChanged): void + { + $config = $this->config(); + $this->storage->save('refresh-test', $this->sessionData('access-1', 'refresh-1')); + $onRefreshStart = $this->onRefreshStart($config, new FileLock($config, 1)); + if ($storedTokenChanged) { + $this->storage->save('refresh-test', $this->sessionData('access-2', 'refresh-2')); + } + $holder = $this->startLockHolder((string) $this->tempDir, 'refresh--' . $config->getSessionIdSlug()); + + try { + $this->expectExceptionMessage('Timed out waiting for another process to refresh the access token'); + $onRefreshStart('refresh-1'); + } finally { + \proc_terminate($holder, 9); + \proc_close($holder); + } + } + + /** + * @param array $env + */ + private function config(array $env = []): Config + { + return new Config($env + [ + 'PLATFORMSH_CLI_HOME' => (string) $this->tempDir, + 'PLATFORMSH_CLI_SESSION_ID' => 'refresh-test', + 'PLATFORMSH_CLI_API_DISABLE_CREDENTIAL_HELPERS' => '1', + ]); + } + + /** + * Loads the session in memory, then simulates another process refreshing the token. + */ + private function loadStaleSession(Config $config): callable + { + $this->storage->save('refresh-test', $this->sessionData('access-1', 'refresh-1')); + $onRefreshStart = $this->onRefreshStart($config); + $this->storage->save('refresh-test', $this->sessionData('access-2', 'refresh-2')); + + return $onRefreshStart; + } + + private function connector(Config $config, ?FileLock $fileLock = null): Connector + { + $api = new Api($config, new ArrayCache(), new BufferedOutput(), null, null, $fileLock); + $connector = $api->getClient(false, true)->getConnector(); + $this->assertInstanceOf(Connector::class, $connector); + $this->assertSame('refresh-1', $connector->getSession()->get('refreshToken')); + + return $connector; + } + + private function onRefreshStart(Config $config, ?FileLock $fileLock = null): callable + { + $onRefreshStart = $this->connector($config, $fileLock)->getConfig()['on_refresh_start']; + $this->assertIsCallable($onRefreshStart); + + return $onRefreshStart; + } + + /** + * @return array + */ + private function sessionData(string $accessToken, string $refreshToken): array + { + return [ + 'accessToken' => $accessToken, + 'tokenType' => 'bearer', + 'expires' => \time() + 900, + 'refreshToken' => $refreshToken, + ]; + } +} diff --git a/legacy/tests/Service/FileLockTest.php b/legacy/tests/Service/FileLockTest.php new file mode 100644 index 00000000..c5aeeaaa --- /dev/null +++ b/legacy/tests/Service/FileLockTest.php @@ -0,0 +1,61 @@ +tempDirSetUp(); + $this->config = new Config(['PLATFORMSH_CLI_HOME' => (string) $this->tempDir]); + } + + public function testWaitsForAHeldLock(): void + { + $holder = new FileLock($this->config); + $this->assertNull($holder->acquireOrWait('test')); + + $this->assertSame('checked', (new FileLock($this->config))->acquireOrWait('test', null, fn(): string => 'checked')); + + $holder->release('test'); + $this->assertNull((new FileLock($this->config))->acquireOrWait('test', null, fn(): string => 'checked')); + } + + public function testIsHeldOnlyWhenAcquired(): void + { + $holder = new FileLock($this->config); + $holder->acquireOrWait('test'); + $this->assertTrue($holder->isHeld('test')); + + $waiter = new FileLock($this->config, 1); + $this->assertNull($waiter->acquireOrWait('test')); + $this->assertFalse($waiter->isHeld('test')); + + $holder->release('test'); + $this->assertFalse($holder->isHeld('test')); + } + + public function testLockIsFreedWhenTheHolderIsKilled(): void + { + $holder = $this->startLockHolder((string) $this->tempDir, 'test'); + $this->assertSame('checked', (new FileLock($this->config))->acquireOrWait('test', null, fn(): string => 'checked')); + + \proc_terminate($holder, 9); + \proc_close($holder); + + $this->assertNull((new FileLock($this->config))->acquireOrWait('test', null, fn(): string => 'checked')); + } +} diff --git a/pkg/mockapi/auth_server.go b/pkg/mockapi/auth_server.go index 258f39ed..cb0931da 100644 --- a/pkg/mockapi/auth_server.go +++ b/pkg/mockapi/auth_server.go @@ -7,6 +7,7 @@ import ( "crypto/sha256" "encoding/base64" "encoding/json" + "fmt" "net/http" "net/http/httptest" "slices" @@ -24,10 +25,82 @@ var ValidAPITokens = []string{"api-token-1"} var accessTokens = []string{"access-token-1"} // AuthServer is a mock authentication server for testing. +// +// Like the real server, it rotates refresh tokens on every refresh. Reusing a +// rotated refresh token revokes all refresh tokens from the same login. type AuthServer struct { *httptest.Server revokedMu sync.Mutex revokedTokens []string + + refreshMu sync.Mutex + refreshTokens map[string]*refreshToken + refreshDelay time.Duration + refreshCount int + reuseDetected bool + revokedFamilies map[string]bool +} + +type refreshToken struct { + family string + used bool +} + +// AddRefreshToken makes a refresh token valid, as if it had been issued at login. +func (s *AuthServer) AddRefreshToken(token string) { + s.refreshMu.Lock() + defer s.refreshMu.Unlock() + s.refreshTokens[token] = &refreshToken{family: token} +} + +// SetRefreshDelay delays refresh token responses, e.g. to make concurrent refreshes overlap. +func (s *AuthServer) SetRefreshDelay(d time.Duration) { + s.refreshMu.Lock() + defer s.refreshMu.Unlock() + s.refreshDelay = d +} + +// ReuseDetected reports whether a rotated refresh token was sent again. +func (s *AuthServer) ReuseDetected() bool { + s.refreshMu.Lock() + defer s.refreshMu.Unlock() + return s.reuseDetected +} + +// issueRefreshToken returns a new refresh token. It starts a new family if family is empty. +// The caller must hold refreshMu. +func (s *AuthServer) issueRefreshToken(family string) string { + s.refreshCount++ + token := fmt.Sprintf("refresh-token-%d", s.refreshCount) + if family == "" { + family = token + } + s.refreshTokens[token] = &refreshToken{family: family} + return token +} + +// newRefreshToken is issueRefreshToken for a new login. +func (s *AuthServer) newRefreshToken() string { + s.refreshMu.Lock() + defer s.refreshMu.Unlock() + return s.issueRefreshToken("") +} + +// rotateRefreshToken exchanges a refresh token for a new one, or returns an OAuth error code. +func (s *AuthServer) rotateRefreshToken(token string) (newToken, errCode string) { + s.refreshMu.Lock() + defer s.refreshMu.Unlock() + rt, ok := s.refreshTokens[token] + if !ok || s.revokedFamilies[rt.family] { + return "", "invalid_grant" + } + if rt.used { + s.reuseDetected = true + s.revokedFamilies[rt.family] = true + return "", "invalid_grant" + } + rt.used = true + return s.issueRefreshToken(rt.family), "" } // RevokedTokens returns a copy of all tokens that have been revoked. @@ -56,7 +129,10 @@ func NewAuthServer(t *testing.T) *AuthServer { pendingAuths = map[string]pendingAuth{} // code → pendingAuth ) - srv := &AuthServer{} + srv := &AuthServer{ + refreshTokens: map[string]*refreshToken{}, + revokedFamilies: map[string]bool{}, + } mux.Get("/oauth2/authorize", func(w http.ResponseWriter, req *http.Request) { q := req.URL.Query() @@ -81,7 +157,7 @@ func NewAuthServer(t *testing.T) *AuthServer { "access_token": accessTokens[0], "expires_in": 3600, "token_type": "bearer", - "refresh_token": "test-refresh-token", + "refresh_token": srv.newRefreshToken(), }) return } @@ -109,20 +185,25 @@ func NewAuthServer(t *testing.T) *AuthServer { "access_token": accessTokens[0], "expires_in": 3600, "token_type": "bearer", - "refresh_token": "test-refresh-token", + "refresh_token": srv.newRefreshToken(), }) case "refresh_token": - if req.Form.Get("refresh_token") == "test-refresh-token" { - _ = json.NewEncoder(w).Encode(map[string]any{ - "access_token": accessTokens[0], - "expires_in": 3600, - "token_type": "bearer", - "refresh_token": "test-refresh-token", - }) + srv.refreshMu.Lock() + delay := srv.refreshDelay + srv.refreshMu.Unlock() + time.Sleep(delay) + newToken, errCode := srv.rotateRefreshToken(req.Form.Get("refresh_token")) + if errCode != "" { + writeOAuthError(w, errCode, "The refresh token is invalid.") return } - writeOAuthError(w, "invalid_grant", "The refresh token is invalid.") + _ = json.NewEncoder(w).Encode(map[string]any{ + "access_token": accessTokens[0], + "expires_in": 3600, + "token_type": "bearer", + "refresh_token": newToken, + }) default: writeOAuthError(w, "unsupported_grant_type", "Unsupported grant type: "+req.Form.Get("grant_type"))