Skip to content

Commit af8311f

Browse files
julien-ncjulien-nc
committed
feat(oauth2): add PKCE support in the oauth2 code flow
Signed-off-by: Julien Veyssier <julien-nc@posteo.net>
1 parent f7777c3 commit af8311f

11 files changed

Lines changed: 477 additions & 4 deletions

File tree

‎apps/oauth2/appinfo/info.xml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
<name>OAuth 2.0</name>
1010
<summary>Allows OAuth2 compatible authentication from other web applications.</summary>
1111
<description>The OAuth2 app allows administrators to configure the built-in authentication workflow to also allow OAuth2 compatible authentication from other web applications.</description>
12-
<version>1.22.0</version>
12+
<version>1.23.0</version>
1313
<licence>agpl</licence>
1414
<author>Lukas Reschke</author>
1515
<namespace>OAuth2</namespace>

‎apps/oauth2/lib/Controller/LoginRedirectorController.php‎

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,8 @@ public function __construct(
5858
* @param string $state State of the flow
5959
* @param string $response_type Response type for the flow
6060
* @param string $redirect_uri URI to redirect to after the flow (is only used for legacy ownCloud clients)
61+
* @param string $code_challenge PKCE code challenge (optional)
62+
* @param string $code_challenge_method PKCE code challenge method "S256" or "plain" (optional)
6163
* @return TemplateResponse<Http::STATUS_OK, array{}>|RedirectResponse<Http::STATUS_SEE_OTHER, array{}>
6264
*
6365
* 200: Client not found
@@ -69,7 +71,9 @@ public function __construct(
6971
public function authorize($client_id,
7072
$state,
7173
$response_type,
72-
string $redirect_uri = ''): TemplateResponse|RedirectResponse {
74+
string $redirect_uri = '',
75+
string $code_challenge = '',
76+
string $code_challenge_method = ''): TemplateResponse|RedirectResponse {
7377
try {
7478
$client = $this->clientMapper->getByIdentifier($client_id);
7579
} catch (ClientNotFoundException $e) {
@@ -85,6 +89,21 @@ public function authorize($client_id,
8589
return new RedirectResponse($url);
8690
}
8791

92+
if ($code_challenge_method !== '' && $code_challenge_method !== 'S256' && $code_challenge_method !== 'plain') {
93+
$url = $client->getRedirectUri() . '?error=invalid_request&error_description=Invalid+code_challenge_method&state=' . \urlencode($state);
94+
return new RedirectResponse($url);
95+
}
96+
97+
if ($code_challenge !== '' && $code_challenge_method === '') {
98+
$url = $client->getRedirectUri() . '?error=invalid_request&error_description=code_challenge_method+required&state=' . \urlencode($state);
99+
return new RedirectResponse($url);
100+
}
101+
102+
if ($code_challenge === '' && $code_challenge_method !== '') {
103+
$url = $client->getRedirectUri() . '?error=invalid_request&error_description=code_challenge+required&state=' . \urlencode($state);
104+
return new RedirectResponse($url);
105+
}
106+
88107
$enableOcClients = $this->config->getSystemValueBool('oauth2.enable_oc_clients', false);
89108

90109
$providedRedirectUri = '';
@@ -93,6 +112,8 @@ public function authorize($client_id,
93112
}
94113

95114
$this->session->set('oauth.state', $state);
115+
$this->session->set('oauth.code_challenge', $code_challenge);
116+
$this->session->set('oauth.code_challenge_method', $code_challenge_method);
96117

97118
if (in_array($client->getName(), $this->appConfig->getValueArray('oauth2', 'skipAuthPickerApplications', []))) {
98119
/** @see ClientFlowLoginController::showAuthPickerPage **/

‎apps/oauth2/lib/Controller/OauthApiController.php‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ public function __construct(
6161
* @param ?string $refresh_token Refresh token
6262
* @param ?string $client_id Client ID
6363
* @param ?string $client_secret Client secret
64+
* @param ?string $code_verifier PKCE code verifier (required if code_challenge was provided)
6465
* @throws Exception
6566
* @return JSONResponse<Http::STATUS_OK, array{access_token: string, token_type: string, expires_in: int, refresh_token: string, user_id: string}, array{}>|JSONResponse<Http::STATUS_BAD_REQUEST, array{error: string}, array{}>
6667
*
@@ -73,6 +74,7 @@ public function __construct(
7374
public function getToken(
7475
string $grant_type, ?string $code, ?string $refresh_token,
7576
?string $client_id, ?string $client_secret,
77+
?string $code_verifier = null,
7678
): JSONResponse {
7779

7880
// We only handle two types
@@ -124,6 +126,38 @@ public function getToken(
124126
$response->throttle(['invalid_request' => 'authorization_code_expired', 'expired_since' => $expiredSince]);
125127
return $response;
126128
}
129+
130+
// verify PKCE code_verifier if code_challenge was provided
131+
$hashedCodeChallenge = $accessToken->getHashedCodeChallenge();
132+
if ($hashedCodeChallenge !== null && $hashedCodeChallenge !== '') {
133+
if ($code_verifier === null || $code_verifier === '') {
134+
$response = new JSONResponse([
135+
'error' => 'invalid_request',
136+
], Http::STATUS_BAD_REQUEST);
137+
$response->throttle(['invalid_request' => 'code_verifier_required']);
138+
return $response;
139+
}
140+
141+
$codeChallengeMethod = $accessToken->getCodeChallengeMethod();
142+
if ($codeChallengeMethod === 'S256') {
143+
$expectedHash = rtrim(strtr(base64_encode(hash('sha256', $code_verifier, true)), '+/', '-_'), '=');
144+
if (!hash_equals($hashedCodeChallenge, $expectedHash)) {
145+
$response = new JSONResponse([
146+
'error' => 'invalid_grant',
147+
], Http::STATUS_BAD_REQUEST);
148+
$response->throttle(['invalid_grant' => 'pkce_verification_failed']);
149+
return $response;
150+
}
151+
} else {
152+
if (!hash_equals($hashedCodeChallenge, $code_verifier)) {
153+
$response = new JSONResponse([
154+
'error' => 'invalid_grant',
155+
], Http::STATUS_BAD_REQUEST);
156+
$response->throttle(['invalid_grant' => 'pkce_verification_failed']);
157+
return $response;
158+
}
159+
}
160+
}
127161
}
128162

129163
try {

‎apps/oauth2/lib/Db/AccessToken.php‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,10 @@
2424
* @method void setCodeCreatedAt(int $createdAt)
2525
* @method int getTokenCount()
2626
* @method void setTokenCount(int $tokenCount)
27+
* @method string|null getHashedCodeChallenge()
28+
* @method void setHashedCodeChallenge(?string $hashedCodeChallenge)
29+
* @method string|null getCodeChallengeMethod()
30+
* @method void setCodeChallengeMethod(?string $codeChallengeMethod)
2731
*/
2832
class AccessToken extends Entity {
2933
/** @var int */
@@ -38,14 +42,20 @@ class AccessToken extends Entity {
3842
protected $codeCreatedAt;
3943
/** @var int */
4044
protected $tokenCount;
45+
/** @var string|null */
46+
protected $hashedCodeChallenge;
47+
/** @var string|null */
48+
protected $codeChallengeMethod;
4149

4250
public function __construct() {
4351
$this->addType('id', Types::INTEGER);
4452
$this->addType('tokenId', Types::INTEGER);
4553
$this->addType('clientId', Types::INTEGER);
46-
$this->addType('hashedCode', 'string');
47-
$this->addType('encryptedToken', 'string');
54+
$this->addType('hashedCode', Types::STRING);
55+
$this->addType('encryptedToken', Types::STRING);
4856
$this->addType('codeCreatedAt', Types::INTEGER);
4957
$this->addType('tokenCount', Types::INTEGER);
58+
$this->addType('hashedCodeChallenge', Types::STRING);
59+
$this->addType('codeChallengeMethod', Types::STRING);
5060
}
5161
}

‎apps/oauth2/lib/Migration/Version012300Date20250423141506.php‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
7+
* SPDX-License-Identifier: AGPL-3.0-or-later
8+
*/
9+
namespace OCA\OAuth2\Migration;
10+
11+
use Closure;
12+
use OCP\DB\ISchemaWrapper;
13+
use OCP\DB\Types;
14+
use OCP\Migration\IOutput;
15+
use OCP\Migration\SimpleMigrationStep;
16+
17+
class Version012300Date20250423141506 extends SimpleMigrationStep {
18+
19+
public function __construct(
20+
) {
21+
}
22+
23+
public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper {
24+
/** @var ISchemaWrapper $schema */
25+
$schema = $schemaClosure();
26+
27+
if ($schema->hasTable('oauth2_access_tokens')) {
28+
$table = $schema->getTable('oauth2_access_tokens');
29+
$dbChanged = false;
30+
if (!$table->hasColumn('hashed_code_challenge')) {
31+
$table->addColumn('hashed_code_challenge', Types::STRING, [
32+
'notnull' => false,
33+
'length' => 128,
34+
]);
35+
$dbChanged = true;
36+
}
37+
if (!$table->hasColumn('code_challenge_method')) {
38+
$table->addColumn('code_challenge_method', Types::STRING, [
39+
'notnull' => false,
40+
'length' => 10,
41+
]);
42+
$dbChanged = true;
43+
}
44+
if ($dbChanged) {
45+
return $schema;
46+
}
47+
}
48+
49+
return null;
50+
}
51+
}

‎apps/oauth2/openapi.json‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,24 @@
7474
"type": "string",
7575
"default": ""
7676
}
77+
},
78+
{
79+
"name": "code_challenge",
80+
"in": "query",
81+
"description": "PKCE code challenge (optional)",
82+
"schema": {
83+
"type": "string",
84+
"default": ""
85+
}
86+
},
87+
{
88+
"name": "code_challenge_method",
89+
"in": "query",
90+
"description": "PKCE code challenge method \"S256\" or \"plain\" (optional)",
91+
"schema": {
92+
"type": "string",
93+
"default": ""
94+
}
7795
}
7896
],
7997
"responses": {
@@ -153,6 +171,12 @@
153171
"type": "string",
154172
"nullable": true,
155173
"description": "Client secret"
174+
},
175+
"code_verifier": {
176+
"type": "string",
177+
"nullable": true,
178+
"default": null,
179+
"description": "PKCE code verifier (required if code_challenge was provided)"
156180
}
157181
}
158182
}

‎apps/oauth2/tests/Controller/LoginRedirectorControllerTest.php‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,23 @@ public function testAuthorizeWrongResponseType(): void {
165165
$this->assertEquals($expected, $this->loginRedirectorController->authorize('MyClientId', 'MyState', 'wrongcode'));
166166
}
167167

168+
public function testAuthorizeRejectsCodeChallengeMethodWithoutChallenge(): void {
169+
$client = new Client();
170+
$client->setClientIdentifier('MyClientIdentifier');
171+
$client->setRedirectUri('http://foo.bar');
172+
$this->clientMapper
173+
->expects($this->once())
174+
->method('getByIdentifier')
175+
->with('MyClientId')
176+
->willReturn($client);
177+
$this->session
178+
->expects($this->never())
179+
->method('set');
180+
181+
$expected = new RedirectResponse('http://foo.bar?error=invalid_request&error_description=code_challenge+required&state=MyState');
182+
$this->assertEquals($expected, $this->loginRedirectorController->authorize('MyClientId', 'MyState', 'code', '', '', 'S256'));
183+
}
184+
168185
public function testAuthorizeWithLegacyOcClient(): void {
169186
$client = new Client();
170187
$client->setClientIdentifier('MyClientIdentifier');

0 commit comments

Comments
 (0)