Skip to content

Commit 79dfec2

Browse files
authored
Merge pull request #1025 from nextcloud/fix/1024/no-token_refresh_expires_in
fix(token-refresh): handle missing attributes in login token
2 parents 8baef6c + 84bfe4c commit 79dfec2

4 files changed

Lines changed: 52 additions & 14 deletions

File tree

docs/token_exchange.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,13 @@
66

77
If your IdP supports token exchange, user_oidc can exchange the login token against another token.
88

9+
:warning: The token exchange feature is disabled by default. You can enable it in `config.php`:
10+
``` php
11+
'user_oidc' => [
12+
'token_exchange' => true,
13+
],
14+
```
15+
916
Keycloak supports token exchange if its "Preview" mode is enabled. See https://www.keycloak.org/securing-apps/token-exchange .
1017

1118
:warning: Your IdP need to be configured accordingly. For example, Keycloak requires that token exchange is explicitely

lib/Controller/LoginController.php

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -518,12 +518,15 @@ public function code(string $state = '', string $code = '', string $scope = '',
518518
$this->eventDispatcher->dispatchTyped(new UserLoggedInEvent($user, $user->getUID(), null, false));
519519
}
520520

521-
// store all token information for potential token exchange requests
522-
$tokenData = array_merge(
523-
$data,
524-
['provider_id' => $providerId],
525-
);
526-
$this->tokenService->storeToken($tokenData);
521+
$tokenExchangeEnabled = (isset($oidcSystemConfig['token_exchange']) && $oidcSystemConfig['token_exchange'] === true);
522+
if ($tokenExchangeEnabled) {
523+
// store all token information for potential token exchange requests
524+
$tokenData = array_merge(
525+
$data,
526+
['provider_id' => $providerId],
527+
);
528+
$this->tokenService->storeToken($tokenData);
529+
}
527530
$this->config->setUserValue($user->getUID(), Application::APP_ID, 'had_token_once', '1');
528531

529532
// Set last password confirm to the future as we don't have passwords to confirm against with SSO

lib/Model/Token.php

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,17 +15,17 @@ class Token implements JsonSerializable {
1515
private ?string $idToken;
1616
private string $accessToken;
1717
private int $expiresIn;
18-
private int $refreshExpiresIn;
19-
private string $refreshToken;
18+
private ?int $refreshExpiresIn;
19+
private ?string $refreshToken;
2020
private int $createdAt;
2121
private ?int $providerId;
2222

2323
public function __construct(array $tokenData) {
2424
$this->idToken = $tokenData['id_token'] ?? null;
2525
$this->accessToken = $tokenData['access_token'];
2626
$this->expiresIn = $tokenData['expires_in'];
27-
$this->refreshExpiresIn = $tokenData['refresh_expires_in'];
28-
$this->refreshToken = $tokenData['refresh_token'];
27+
$this->refreshExpiresIn = $tokenData['refresh_expires_in'] ?? null;
28+
$this->refreshToken = $tokenData['refresh_token'] ?? null;
2929
$this->createdAt = $tokenData['created_at'] ?? time();
3030
$this->providerId = $tokenData['provider_id'] ?? null;
3131
}
@@ -47,16 +47,21 @@ public function getExpiresInFromNow(): int {
4747
return $expiresAt - time();
4848
}
4949

50-
public function getRefreshExpiresIn(): int {
50+
public function getRefreshExpiresIn(): ?int {
5151
return $this->refreshExpiresIn;
5252
}
5353

5454
public function getRefreshExpiresInFromNow(): int {
55+
// if there is no refresh_expires_in, we assume the refresh token never expires
56+
// so we don't need getRefreshExpiresInFromNow
57+
if ($this->refreshExpiresIn === null) {
58+
return 0;
59+
}
5560
$refreshExpiresAt = $this->createdAt + $this->refreshExpiresIn;
5661
return $refreshExpiresAt - time();
5762
}
5863

59-
public function getRefreshToken(): string {
64+
public function getRefreshToken(): ?string {
6065
return $this->refreshToken;
6166
}
6267

@@ -73,10 +78,18 @@ public function isExpiring(): bool {
7378
}
7479

7580
public function refreshIsExpired(): bool {
81+
// if there is no refresh_expires_in, we assume the refresh token never expires
82+
if ($this->refreshExpiresIn === null) {
83+
return false;
84+
}
7685
return time() > ($this->createdAt + $this->refreshExpiresIn);
7786
}
7887

7988
public function refreshIsExpiring(): bool {
89+
// if there is no refresh_expires_in, we assume the refresh token never expires
90+
if ($this->refreshExpiresIn === null) {
91+
return false;
92+
}
8093
return time() > ($this->createdAt + (int)($this->refreshExpiresIn / 2));
8194
}
8295

lib/Service/TokenService.php

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,8 +84,8 @@ public function getToken(bool $refreshIfExpired = true): ?Token {
8484
}
8585

8686
// token has expired
87-
// try to refresh the token if the refresh token is still valid
88-
if ($refreshIfExpired && !$token->refreshIsExpired()) {
87+
// try to refresh the token if there is a refresh token and it is still valid
88+
if ($refreshIfExpired && $token->getRefreshToken() !== null && !$token->refreshIsExpired()) {
8989
$this->logger->debug('[TokenService] getToken: token is expired and refresh token is still valid, refresh expires in ' . $token->getRefreshExpiresInFromNow());
9090
return $this->refresh($token);
9191
}
@@ -102,6 +102,12 @@ public function getToken(bool $refreshIfExpired = true): ?Token {
102102
* @throws PreConditionNotMetException
103103
*/
104104
public function checkLoginToken(): void {
105+
$oidcSystemConfig = $this->config->getSystemValue('user_oidc', []);
106+
$tokenExchangeEnabled = (isset($oidcSystemConfig['token_exchange']) && $oidcSystemConfig['token_exchange'] === true);
107+
if (!$tokenExchangeEnabled) {
108+
return;
109+
}
110+
105111
$currentUser = $this->userSession->getUser();
106112
if (!$this->userSession->isLoggedIn() || $currentUser === null) {
107113
$this->logger->debug('[TokenService] checkLoginToken: user not logged in');
@@ -212,6 +218,15 @@ public function decodeIdToken(Token $token): array {
212218
* @throws \JsonException
213219
*/
214220
public function getExchangedToken(string $targetAudience): Token {
221+
$oidcSystemConfig = $this->config->getSystemValue('user_oidc', []);
222+
$tokenExchangeEnabled = (isset($oidcSystemConfig['token_exchange']) && $oidcSystemConfig['token_exchange'] === true);
223+
if (!$tokenExchangeEnabled) {
224+
throw new TokenExchangeFailedException(
225+
'Failed to exchange token, the token exchange feature is disabled. It can be enabled in config.php',
226+
0,
227+
);
228+
}
229+
215230
$loginToken = $this->getToken();
216231
if ($loginToken === null) {
217232
$this->logger->debug('[TokenService] Failed to exchange token, no login token found in the session');

0 commit comments

Comments
 (0)