Skip to content

refactor: move to ITimeFactory #1392

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
solracsf wants to merge 1 commit into
base: main
Choose a base branch
from
+35 −21
Open

refactor: move to ITimeFactory #1392

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/Controller/Id4meController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,7 @@ public function code(string $state = '', string $code = '', string $scope = '')
}

// Set last password confirm to the future as we don't have passwords to confirm against with SSO
$this->session->set('last-password-confirm', strtotime('+4 year', time()));
$this->session->set('last-password-confirm', $this->timeFactory->getTime() + 4 * 365 * 24 * 3600);

return new RedirectResponse($this->serverVersion->getMajorVersion() >= 32 ? $this->urlGenerator->linkToDefaultPageUrl() : \OC_Util::getDefaultPageUrl());
}
Expand Down
2 changes: 1 addition & 1 deletion lib/Controller/LoginController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -667,7 +667,7 @@ public function code(string $state = '', string $code = '', string $scope = '',
$this->config->setUserValue($user->getUID(), Application::APP_ID, 'had_token_once', '1');

// Set last password confirm to the future as we don't have passwords to confirm against with SSO
$this->session->set('last-password-confirm', strtotime('+4 year', time()));
$this->session->set('last-password-confirm', $this->timeFactory->getTime() + 4 * 365 * 24 * 3600);

// for backchannel logout
try {
Expand Down
20 changes: 12 additions & 8 deletions lib/Model/Token.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
namespace OCA\UserOIDC\Model;

use JsonSerializable;
use OCP\AppFramework\Utility\ITimeFactory;

class Token implements JsonSerializable {

Expand All @@ -20,13 +21,16 @@ class Token implements JsonSerializable {
private int $createdAt;
private ?int $providerId;

public function __construct(array $tokenData) {
public function __construct(
array $tokenData,
private ITimeFactory $timeFactory,
Copy link
Copy Markdown
Member

@julien-nc julien-nc Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could inject ITimeFactory in the constructor (and store it as a class attr as well) with OCP\Server::get to avoid having to pass it when instanciating tokens.

Copy link
Copy Markdown
Member Author

@solracsf solracsf Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would work, but have a trade-off: it makes Token harder to test since you can't inject a mock ITimeFactory without having the full Nextcloud DI container available. WDYT?

) {
$this->idToken = $tokenData['id_token'] ?? null;
$this->accessToken = $tokenData['access_token'];
$this->expiresIn = $tokenData['expires_in'];
$this->refreshExpiresIn = $tokenData['refresh_expires_in'] ?? null;
$this->refreshToken = $tokenData['refresh_token'] ?? null;
$this->createdAt = $tokenData['created_at'] ?? time();
$this->createdAt = $tokenData['created_at'] ?? $this->timeFactory->getTime();
$this->providerId = $tokenData['provider_id'] ?? null;
}

Expand All @@ -44,7 +48,7 @@ public function getExpiresIn(): int {

public function getExpiresInFromNow(): int {
$expiresAt = $this->createdAt + $this->expiresIn;
return $expiresAt - time();
return $expiresAt - $this->timeFactory->getTime();
}

public function getRefreshExpiresIn(): ?int {
Expand All @@ -58,7 +62,7 @@ public function getRefreshExpiresInFromNow(): int {
return 0;
}
$refreshExpiresAt = $this->createdAt + $this->refreshExpiresIn;
return $refreshExpiresAt - time();
return $refreshExpiresAt - $this->timeFactory->getTime();
}

public function getRefreshToken(): ?string {
Expand All @@ -70,27 +74,27 @@ public function getProviderId(): ?int {
}

public function isExpired(): bool {
return time() > ($this->createdAt + $this->expiresIn);
return $this->timeFactory->getTime() > ($this->createdAt + $this->expiresIn);
}

public function isExpiring(): bool {
return time() > ($this->createdAt + (int)($this->expiresIn / 2));
return $this->timeFactory->getTime() > ($this->createdAt + (int)($this->expiresIn / 2));
}

public function refreshIsExpired(): bool {
// if there is no refresh_expires_in, we assume the refresh token never expires
if ($this->refreshExpiresIn === null) {
return false;
}
return time() > ($this->createdAt + $this->refreshExpiresIn);
return $this->timeFactory->getTime() > ($this->createdAt + $this->refreshExpiresIn);
}

public function refreshIsExpiring(): bool {
// if there is no refresh_expires_in, we assume the refresh token never expires
if ($this->refreshExpiresIn === null) {
return false;
}
return time() > ($this->createdAt + (int)($this->refreshExpiresIn / 2));
return $this->timeFactory->getTime() > ($this->createdAt + (int)($this->refreshExpiresIn / 2));
}

public function getCreatedAt() {
Expand Down
6 changes: 4 additions & 2 deletions lib/Service/DiscoveryService.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
use OCA\UserOIDC\Helper\HttpClientHelper;
use OCA\UserOIDC\Vendor\Firebase\JWT\JWK;
use OCA\UserOIDC\Vendor\Firebase\JWT\JWT;
use OCP\AppFramework\Utility\ITimeFactory;
use OCP\ICache;
use OCP\ICacheFactory;
use OCP\IConfig;
Expand Down Expand Up @@ -44,6 +45,7 @@ public function __construct(
private HttpClientHelper $clientService,
private ProviderService $providerService,
private IConfig $config,
private ITimeFactory $timeFactory,
ICacheFactory $cacheFactory,
) {
$this->cache = $cacheFactory->createDistributed('user_oidc');
Expand Down Expand Up @@ -75,7 +77,7 @@ public function obtainDiscovery(Provider $provider): array {
*/
public function obtainJWK(Provider $provider, string $tokenToDecode, bool $useCache = true): array {
$lastJwksRefresh = $this->providerService->getSetting($provider->getId(), ProviderService::SETTING_JWKS_CACHE_TIMESTAMP);
if ($lastJwksRefresh !== '' && $useCache && (int)$lastJwksRefresh > time() - self::INVALIDATE_JWKS_CACHE_AFTER_SECONDS) {
if ($lastJwksRefresh !== '' && $useCache && (int)$lastJwksRefresh > $this->timeFactory->getTime() - self::INVALIDATE_JWKS_CACHE_AFTER_SECONDS) {
$rawJwks = $this->providerService->getSetting($provider->getId(), ProviderService::SETTING_JWKS_CACHE);
$rawJwks = json_decode($rawJwks, true);
$this->logger->debug('[obtainJWK] jwks cache content', ['jwks_cache' => $rawJwks]);
Expand All @@ -87,7 +89,7 @@ public function obtainJWK(Provider $provider, string $tokenToDecode, bool $useCa
// cache jwks
$this->providerService->setSetting($provider->getId(), ProviderService::SETTING_JWKS_CACHE, $responseBody);
$this->logger->debug('[obtainJWK] setting cache', ['jwks_cache' => $responseBody]);
$this->providerService->setSetting($provider->getId(), ProviderService::SETTING_JWKS_CACHE_TIMESTAMP, strval(time()));
$this->providerService->setSetting($provider->getId(), ProviderService::SETTING_JWKS_CACHE_TIMESTAMP, strval($this->timeFactory->getTime()));
}

$fixedJwks = $this->fixJwksAlg($rawJwks, $tokenToDecode);
Expand Down
12 changes: 7 additions & 5 deletions lib/Service/TokenService.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
use OCP\App\IAppManager;
use OCP\AppFramework\Db\DoesNotExistException;
use OCP\AppFramework\Db\MultipleObjectsReturnedException;
use OCP\AppFramework\Utility\ITimeFactory;
use OCP\Authentication\Exceptions\ExpiredTokenException;
use OCP\Authentication\Exceptions\InvalidTokenException;
use OCP\Authentication\Exceptions\WipeTokenException;
Expand Down Expand Up @@ -67,11 +68,12 @@ public function __construct(
private DiscoveryService $discoveryService,
private ProviderMapper $providerMapper,
private ILockingProvider $lockingProvider,
private ITimeFactory $timeFactory,
) {
}

public function storeToken(array $tokenData): Token {
$token = new Token($tokenData);
$token = new Token($tokenData, $this->timeFactory);
$this->session->set(self::SESSION_TOKEN_KEY, json_encode($token, JSON_THROW_ON_ERROR));
$this->logger->debug('[TokenService] Store token in the session', ['session_id' => $this->session->getId()]);
return $token;
Expand All @@ -93,7 +95,7 @@ public function getToken(bool $refreshIfExpired = true): ?Token {
return null;
}

$token = new Token(json_decode($sessionData, true, 512, JSON_THROW_ON_ERROR));
$token = new Token(json_decode($sessionData, true, 512, JSON_THROW_ON_ERROR), $this->timeFactory);
// token is still valid
if (!$token->isExpired()) {
$this->logger->debug('[TokenService] getToken: token is still valid, it expires in ' . strval($token->getExpiresInFromNow()) . ' and refresh expires in ' . strval($token->getRefreshExpiresInFromNow()));
Expand Down Expand Up @@ -225,7 +227,7 @@ public function refresh(Token $token): Token {
// the token expiration and the moment it attempted to acquire the lock
$sessionData = $this->session->get(self::SESSION_TOKEN_KEY);
if ($sessionData) {
$currentToken = new Token(json_decode($sessionData, true, 512, JSON_THROW_ON_ERROR));
$currentToken = new Token(json_decode($sessionData, true, 512, JSON_THROW_ON_ERROR), $this->timeFactory);
if (!$currentToken->isExpired()) {
$this->logger->debug('[TokenService] Token already refreshed by another request');
return $currentToken;
Expand Down Expand Up @@ -368,7 +370,7 @@ public function getExchangedToken(string $targetAudience, array $extraScopes = [
$bodyArray,
['provider_id' => $loginToken->getProviderId()],
);
return new Token($tokenData);
return new Token($tokenData, $this->timeFactory);
} catch (ClientException|ServerException $e) {
$response = $e->getResponse();
$body = (string)$response->getBody();
Expand Down Expand Up @@ -439,6 +441,6 @@ public function getTokenFromOidcProviderApp(string $userId, string $targetAudien
'refresh_expires_in' => method_exists($generationEvent, 'getRefreshExpiresIn')
? $generationEvent->getRefreshExpiresIn()
: $generationEvent->getExpiresIn(),
]);
], $this->timeFactory);
}
}
8 changes: 5 additions & 3 deletions lib/User/Backend.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
use OCA\UserOIDC\User\Validator\SelfEncodedValidator;
use OCA\UserOIDC\User\Validator\UserInfoValidator;
use OCP\AppFramework\Db\DoesNotExistException;
use OCP\AppFramework\Utility\ITimeFactory;
use OCP\Authentication\IApacheBackend;
use OCP\DB\Exception;
use OCP\EventDispatcher\GenericEvent;
Expand Down Expand Up @@ -69,6 +70,7 @@ public function __construct(
private LdapService $ldapService,
private IUserManager $userManager,
private ServerVersion $serverVersion,
private ITimeFactory $timeFactory,
) {
}

Expand Down Expand Up @@ -348,12 +350,12 @@ public function getCurrentUserId(): string {
}
}

$this->session->set('last-password-confirm', strtotime('+4 year', time()));
$this->session->set('last-password-confirm', $this->timeFactory->getTime() + 4 * 365 * 24 * 3600);
$this->setSessionUser($userId);
return $userId;
} elseif ($this->userExists($tokenUserId)) {
$this->checkFirstLogin($tokenUserId);
$this->session->set('last-password-confirm', strtotime('+4 year', time()));
$this->session->set('last-password-confirm', $this->timeFactory->getTime() + 4 * 365 * 24 * 3600);
$this->setSessionUser($tokenUserId);
return $tokenUserId;
} else {
Expand All @@ -375,7 +377,7 @@ public function getCurrentUserId(): string {
return '';
}
$this->checkFirstLogin($tokenUserId);
$this->session->set('last-password-confirm', strtotime('+4 year', time()));
$this->session->set('last-password-confirm', $this->timeFactory->getTime() + 4 * 365 * 24 * 3600);
$this->setSessionUser($tokenUserId);
return $tokenUserId;
}
Expand Down
6 changes: 5 additions & 1 deletion tests/unit/Service/DiscoveryServiceTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
use OCA\UserOIDC\Helper\HttpClientHelper;
use OCA\UserOIDC\Service\DiscoveryService;
use OCA\UserOIDC\Service\ProviderService;
use OCP\AppFramework\Utility\ITimeFactory;
use OCP\ICacheFactory;
use OCP\IConfig;
use PHPUnit\Framework\Assert;
Expand All @@ -30,6 +31,8 @@ class DiscoveryServiceTest extends TestCase {
private $config;
/** @var ICacheFactory|MockObject */
private $cacheFactory;
/** @var ITimeFactory|MockObject */
private $timeFactory;
/** @var DiscoveryService */
private $discoveryService;

Expand All @@ -39,8 +42,9 @@ public function setUp(): void {
$this->clientHelper = $this->createMock(HttpClientHelper::class);
$this->providerService = $this->createMock(ProviderService::class);
$this->config = $this->createMock(IConfig::class);
$this->timeFactory = $this->createMock(ITimeFactory::class);
$this->cacheFactory = $this->createMock(ICacheFactory::class);
$this->discoveryService = new DiscoveryService($this->logger, $this->clientHelper, $this->providerService, $this->config, $this->cacheFactory);
$this->discoveryService = new DiscoveryService($this->logger, $this->clientHelper, $this->providerService, $this->config, $this->timeFactory, $this->cacheFactory);
}

/**
Expand Down