From cbc6cfdc5657c0441cdb9bb7854de6088afb5fcd Mon Sep 17 00:00:00 2001 From: Timm Friebe Date: Sun, 4 Jan 2026 00:09:32 +0100 Subject: [PATCH 1/2] Add support for PKCE for OAuth2 --- .../web/auth/oauth/ByCertificate.class.php | 4 +- src/main/php/web/auth/oauth/ByPKCE.class.php | 61 +++++++++++++++++++ .../php/web/auth/oauth/BySecret.class.php | 2 +- .../php/web/auth/oauth/Credentials.class.php | 10 ++- .../web/auth/oauth/OAuth2Endpoint.class.php | 21 +++++-- .../php/web/auth/oauth/OAuth2Flow.class.php | 24 ++++++-- .../auth/unittest/ByCertificateTest.class.php | 2 +- .../web/auth/unittest/ByPKCETest.class.php | 50 +++++++++++++++ .../auth/unittest/OAuth2FlowTest.class.php | 25 ++++---- 9 files changed, 172 insertions(+), 27 deletions(-) create mode 100755 src/main/php/web/auth/oauth/ByPKCE.class.php create mode 100755 src/test/php/web/auth/unittest/ByPKCETest.class.php diff --git a/src/main/php/web/auth/oauth/ByCertificate.class.php b/src/main/php/web/auth/oauth/ByCertificate.class.php index 382dde6..f7f7ac3 100644 --- a/src/main/php/web/auth/oauth/ByCertificate.class.php +++ b/src/main/php/web/auth/oauth/ByCertificate.class.php @@ -34,8 +34,8 @@ public function __construct($clientId, $fingerprint, $privateKey, $validity= 360 } /** Returns parameters to be used in authentication process */ - public function params(string $endpoint, $time= null): array { - $time ?? $time= time(); + public function params(string $endpoint, array $seed= []): array { + $time= $seed['time'] ?? time(); $jwt= new JWT(['alg' => 'RS256', 'typ' => 'JWT', 'x5t' => JWT::encode(hex2bin($this->fingerprint))], [ 'aud' => $endpoint, 'exp' => $time + $this->validity, diff --git a/src/main/php/web/auth/oauth/ByPKCE.class.php b/src/main/php/web/auth/oauth/ByPKCE.class.php new file mode 100755 index 0000000..232b263 --- /dev/null +++ b/src/main/php/web/auth/oauth/ByPKCE.class.php @@ -0,0 +1,61 @@ +challenge= fn($verifier) => JWT::encode(hash('sha256', $verifier, true)); + $this->method= 'S256'; + } else if ('plain' === $method) { + $this->challenge= fn($verifier) => $verifier; + $this->method= 'plain'; + } else { + throw new IllegalArgumentException('Unsupported method '.$method); + } + } + + /** @return string */ + public function method() { return $this->method; } + + /** Returns authorization seed */ + public function seed(): array { + static $UNRESERVED= 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-._~'; + + $random= random_bytes(64); + $verifier= ''; + for ($i= 0; $i < 64; $i++) { + $verifier.= $UNRESERVED[ord($random[$i]) % 66]; + } + return ['verifier' => $verifier]; + } + + /** Returns parameters to be passed on to authorization */ + public function pass(array $seed): array { + return [ + 'code_challenge' => ($this->challenge)($seed['verifier']), + 'code_challenge_method' => $this->method, + ]; + } + + /** Returns parameters to be used in authentication process */ + public function params(string $endpoint, array $seed= []): array { + return [ + 'client_id' => $this->key, + 'code_verifier' => $seed['verifier'], + ]; + } +} \ No newline at end of file diff --git a/src/main/php/web/auth/oauth/BySecret.class.php b/src/main/php/web/auth/oauth/BySecret.class.php index c5aef91..37dc894 100644 --- a/src/main/php/web/auth/oauth/BySecret.class.php +++ b/src/main/php/web/auth/oauth/BySecret.class.php @@ -20,7 +20,7 @@ public function __construct($clientId, $secret) { public function secret() { return $this->secret; } /** Returns parameters to be used in authentication process */ - public function params(string $endpoint, $time= null): array { + public function params(string $endpoint, array $seed= []): array { return [ 'client_id' => $this->key, 'client_secret' => $this->secret->reveal(), diff --git a/src/main/php/web/auth/oauth/Credentials.class.php b/src/main/php/web/auth/oauth/Credentials.class.php index fb83850..9a7bb92 100644 --- a/src/main/php/web/auth/oauth/Credentials.class.php +++ b/src/main/php/web/auth/oauth/Credentials.class.php @@ -8,7 +8,7 @@ abstract class Credentials { static function __static() { self::$UNSET= new class(null) extends Credentials { - public function params(string $endpoint, $time= null): array { + public function params(string $endpoint, array $seed= []): array { throw new IllegalStateException('No credentials set'); } }; @@ -23,6 +23,12 @@ public function __construct($key) { $this->key= $key; } + /** Returns authorization seed */ + public function seed(): array { return []; } + + /** Returns parameters to be passed on to authorization */ + public function pass(array $seed): array { return []; } + /** Returns parameters to be used in authentication process */ - public abstract function params(string $endpoint, $time= null): array; + public abstract function params(string $endpoint, array $seed= []): array; } \ No newline at end of file diff --git a/src/main/php/web/auth/oauth/OAuth2Endpoint.class.php b/src/main/php/web/auth/oauth/OAuth2Endpoint.class.php index 389d3d2..bfc73ac 100644 --- a/src/main/php/web/auth/oauth/OAuth2Endpoint.class.php +++ b/src/main/php/web/auth/oauth/OAuth2Endpoint.class.php @@ -1,8 +1,8 @@ credentials->seed(); } + + /** + * Returns authorization parameters + * + * @param [:string] $grant + * @param [:string] $seed + * @return [:string] + */ + public function pass($auth, $seed= []) { return $this->credentials->pass($seed) + $auth; } + /** * Acquires a grant * * @param [:string] $grant + * @param [:string] $seed * @return [:string] */ - public function acquire($grant) { - return $this->request($this->credentials->params($this->conn->getUrl()->getCanonicalURL()) + $grant); + public function acquire($grant, $seed= []) { + return $this->request($this->credentials->params($this->conn->getUrl()->getCanonicalURL(), $seed) + $grant); } } \ No newline at end of file diff --git a/src/main/php/web/auth/oauth/OAuth2Flow.class.php b/src/main/php/web/auth/oauth/OAuth2Flow.class.php index fe94463..4867e50 100755 --- a/src/main/php/web/auth/oauth/OAuth2Flow.class.php +++ b/src/main/php/web/auth/oauth/OAuth2Flow.class.php @@ -110,8 +110,10 @@ public function authenticate($request, $response, $session) { $server= $request->param('state'); if (null === $server || null === $stored) { $state= bin2hex($this->rand->bytes(16)); + $seed= $this->backend->seed(); + $stored??= ['flow' => []]; - $stored['flow'][$state]= (string)$uri; + $stored['flow'][$state]= ['uri' => (string)$uri, 'seed' => $seed]; $session->register($this->namespace, $stored); $session->transmit($response); @@ -121,9 +123,9 @@ public function authenticate($request, $response, $session) { 'client_id' => $this->backend->clientId(), 'scope' => implode(' ', $this->scopes), 'redirect_uri' => $callback, - 'state' => $state + 'state' => $state, ]; - $target= $this->auth->using()->params($params)->create(); + $target= $this->auth->using()->params($this->backend->pass($params, $seed))->create(); // If a URL fragment is present, append it to the state parameter, which // is passed as the last parameter to the authentication service. @@ -150,18 +152,28 @@ public function authenticate($request, $response, $session) { ) { unset($stored['flow'][$state[0]]); + // Target is an array for old session layout and during transition + if (is_array($target)) { + $uri= $target['uri']; + $seed= $target['seed']; + } else { + $uri= $target; + $seed= []; + } + // Exchange the auth code for an access token - $stored['token']= $this->backend->acquire([ + $params= [ 'grant_type' => 'authorization_code', 'code' => $request->param('code'), 'redirect_uri' => $callback, 'state' => $server - ]); + ]; + $stored['token']= $this->backend->acquire($params, $seed); $session->register($this->namespace, $stored); $session->transmit($response); // Redirect to self, using encoded fragment if present - $this->finalize($response, $target.(isset($state[1]) ? '#'.urldecode($state[1]) : '')); + $this->finalize($response, $uri.(isset($state[1]) ? '#'.urldecode($state[1]) : '')); return null; } diff --git a/src/test/php/web/auth/unittest/ByCertificateTest.class.php b/src/test/php/web/auth/unittest/ByCertificateTest.class.php index 3583440..6dd5d2c 100644 --- a/src/test/php/web/auth/unittest/ByCertificateTest.class.php +++ b/src/test/php/web/auth/unittest/ByCertificateTest.class.php @@ -54,7 +54,7 @@ public function jwt_headers_with($fingerprint) { #[Test, Values([3600, 86400])] public function jwt_payload_with($validity) { $time= time(); - $params= (new ByCertificate(self::CLIENT_ID, self::FINGERPRINT, $this->privateKey, $validity))->params(self::ENDPOINT, $time); + $params= (new ByCertificate(self::CLIENT_ID, self::FINGERPRINT, $this->privateKey, $validity))->params(self::ENDPOINT, ['time' => $time]); $payload= json_decode(base64_decode(explode('.', $params['client_assertion'])[1]), true); Assert::equals( diff --git a/src/test/php/web/auth/unittest/ByPKCETest.class.php b/src/test/php/web/auth/unittest/ByPKCETest.class.php new file mode 100755 index 0000000..240991b --- /dev/null +++ b/src/test/php/web/auth/unittest/ByPKCETest.class.php @@ -0,0 +1,50 @@ + 'test-challenge']; + + /** @return iterable */ + private function challenges() { + yield ['S256', 'Xuq1l4Pllrvf6AJ2BfBwnQFQKBK7dnKAbolZ3zvWFlw']; // base64(sha256(TEST_SEED[verifier])) + yield ['plain', 'test-challenge']; + } + + #[Test, Values(['S256', 'plain'])] + public function can_create_with($method) { + new ByPKCE(self::CLIENT_ID, $method); + } + + #[Test, Values(['S128', 'invalid']), Expect(IllegalArgumentException::class)] + public function unsupported($method) { + new ByPKCE(self::CLIENT_ID, $method); + } + + #[Test] + public function seed_creates_verifier() { + Assert::matches( + '/^[a-zA-Z0-9._~-]{64}$/', + (new ByPKCE(self::CLIENT_ID, 'S256'))->seed()['verifier'] + ); + } + + #[Test, Values(from: 'challenges')] + public function pass($method, $challenge) { + Assert::equals( + ['code_challenge' => $challenge, 'code_challenge_method' => $method], + (new ByPKCE(self::CLIENT_ID, $method))->pass(self::TEST_SEED) + ); + } + + #[Test] + public function params() { + Assert::equals( + ['client_id' => self::CLIENT_ID, 'code_verifier' => 'test-challenge'], + (new ByPKCE(self::CLIENT_ID, 'S256'))->params('https://test/oauth/tokens', self::TEST_SEED) + ); + } +} \ No newline at end of file diff --git a/src/test/php/web/auth/unittest/OAuth2FlowTest.class.php b/src/test/php/web/auth/unittest/OAuth2FlowTest.class.php index 9e9a00d..ad4a90e 100755 --- a/src/test/php/web/auth/unittest/OAuth2FlowTest.class.php +++ b/src/test/php/web/auth/unittest/OAuth2FlowTest.class.php @@ -91,7 +91,7 @@ public function redirects_to_auth($path) { $this->authenticate($fixture, $path, $session), $session ); - Assert::equals('http://localhost'.$path, current($session->value(self::SNS)['flow'])); + Assert::equals(['uri' => 'http://localhost'.$path, 'seed' => []], current($session->value(self::SNS)['flow'])); } #[Test, Values(from: 'paths')] @@ -105,7 +105,7 @@ public function redirects_to_auth_with_relative_callback($path) { $this->authenticate($fixture, $path, $session), $session ); - Assert::equals('http://localhost'.$path, current($session->value(self::SNS)['flow'])); + Assert::equals(['uri' => 'http://localhost'.$path, 'seed' => []], current($session->value(self::SNS)['flow'])); } #[Test, Values(from: 'paths')] @@ -119,7 +119,7 @@ public function redirects_to_auth_using_request($path) { $this->authenticate($fixture->target(new UseRequest()), $path, $session), $session ); - Assert::equals('http://localhost'.$path, current($session->value(self::SNS)['flow'])); + Assert::equals(['uri' => 'http://localhost'.$path, 'seed' => []], current($session->value(self::SNS)['flow'])); } #[Test, Values(from: 'paths')] @@ -133,7 +133,7 @@ public function redirects_to_auth_using_url($path) { $this->authenticate($fixture->target(new UseURL(self::SERVICE)), $path, $session), $session ); - Assert::equals(self::SERVICE.$path, current($session->value(self::SNS)['flow'])); + Assert::equals(['uri' => self::SERVICE.$path, 'seed' => []], current($session->value(self::SNS)['flow'])); } #[Test, Values(from: 'fragments')] @@ -147,7 +147,7 @@ public function redirects_to_sso_with_fragment($fragment) { $this->authenticate($fixture, '/#'.$fragment, $session), $session ); - Assert::equals('http://localhost/#'.$fragment, current($session->value(self::SNS)['flow'])); + Assert::equals(['uri' => 'http://localhost/#'.$fragment, 'seed' => []], current($session->value(self::SNS)['flow'])); } #[Test, Values([[['user']], [['user', 'openid']]])] @@ -196,7 +196,7 @@ public function passes_client_id_and_secret() { ]); $fixture= new OAuth2Flow(self::AUTH, $tokens, $credentials, self::CALLBACK); $session= (new ForTesting())->create(); - $session->register('oauth2::flow', ['flow' => [$state => self::SERVICE]]); + $session->register('oauth2::flow', ['flow' => [$state => ['uri' => self::SERVICE, 'seed' => []]]]); $this->authenticate($fixture, '/?code=SERVER_CODE&state='.$state, $session); Assert::equals('authorization_code', $passed['grant_type']); @@ -214,7 +214,7 @@ public function passes_client_id_assertion_and_rs256_jwt() { ]); $fixture= new OAuth2Flow(self::AUTH, $tokens, $credentials, self::CALLBACK); $session= (new ForTesting())->create(); - $session->register('oauth2::flow', ['flow' => [$state => self::SERVICE]]); + $session->register('oauth2::flow', ['flow' => [$state => ['uri' => self::SERVICE, 'seed' => []]]]); $this->authenticate($fixture, '/?code=SERVER_CODE&state='.$state, $session); Assert::equals('authorization_code', $passed['grant_type']); @@ -233,7 +233,7 @@ public function gets_access_token_and_redirects_to_self() { ]); $fixture= new OAuth2Flow(self::AUTH, $tokens, self::CONSUMER, self::CALLBACK); $session= (new ForTesting())->create(); - $session->register('oauth2::flow', ['flow' => [$state => self::SERVICE]]); + $session->register('oauth2::flow', ['flow' => [$state => ['uri' => self::SERVICE, 'seed' => []]]]); $res= $this->authenticate($fixture, '/?code=SERVER_CODE&state='.$state, $session); Assert::equals(self::SERVICE, $res->headers()['Location']); @@ -266,7 +266,7 @@ public function gets_access_token_and_redirects_to_self_with_fragment($fragment) ]); $fixture= new OAuth2Flow(self::AUTH, $tokens, self::CONSUMER, self::CALLBACK); $session= (new ForTesting())->create(); - $session->register('oauth2::flow', ['flow' => [$state => self::SERVICE]]); + $session->register('oauth2::flow', ['flow' => [$state => ['uri' => self::SERVICE, 'seed' => []]]]); $res= $this->authenticate($fixture, '/?code=SERVER_CODE&state='.$state.OAuth2Flow::FRAGMENT.urlencode($fragment), $session); Assert::equals(self::SERVICE.'#'.$fragment, $res->headers()['Location']); @@ -277,7 +277,7 @@ public function gets_access_token_and_redirects_to_self_with_fragment($fragment) public function raises_exception_on_state_mismatch() { $fixture= new OAuth2Flow(self::AUTH, self::TOKENS, self::CONSUMER, self::CALLBACK); $session= (new ForTesting())->create(); - $session->register('oauth2::flow', ['flow' => ['CLIENTSTATE' => self::SERVICE]]); + $session->register('oauth2::flow', ['flow' => ['CLIENTSTATE' => ['uri' => self::SERVICE, 'seed' => []]]]); $this->authenticate($fixture, '/?state=SERVERSTATE&code=SERVER_CODE', $session); } @@ -414,7 +414,10 @@ public function parallel_requests_stored() { $this->authenticate($fixture, '/favicon.ico', $session); Assert::equals( - ['http://localhost/new', 'http://localhost/favicon.ico'], + [ + ['uri' => 'http://localhost/new', 'seed' => []], + ['uri' => 'http://localhost/favicon.ico', 'seed' => []], + ], array_values($session->value(self::SNS)['flow']) ); } From 208d0dc0b323e21e880904ca06861ae0469d58b2 Mon Sep 17 00:00:00 2001 From: Timm Friebe Date: Sun, 4 Jan 2026 02:23:30 +0100 Subject: [PATCH 2/2] Add ByPKCE::SUPPORTED --- src/main/php/web/auth/oauth/ByPKCE.class.php | 7 ++++--- src/test/php/web/auth/unittest/ByPKCETest.class.php | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/main/php/web/auth/oauth/ByPKCE.class.php b/src/main/php/web/auth/oauth/ByPKCE.class.php index 232b263..a3a90a7 100755 --- a/src/main/php/web/auth/oauth/ByPKCE.class.php +++ b/src/main/php/web/auth/oauth/ByPKCE.class.php @@ -4,6 +4,8 @@ /** @test web.auth.unittest.ByPKCETest */ class ByPKCE extends Credentials { + const SUPPORTED= ['S256', 'plain']; + private $challenge, $method; /** @@ -19,13 +21,12 @@ public function __construct($clientId, $method) { if ('S256' === $method) { $this->challenge= fn($verifier) => JWT::encode(hash('sha256', $verifier, true)); - $this->method= 'S256'; } else if ('plain' === $method) { $this->challenge= fn($verifier) => $verifier; - $this->method= 'plain'; } else { - throw new IllegalArgumentException('Unsupported method '.$method); + throw new IllegalArgumentException('Unsupported method '.$method.', expected one of ['.implode(', ', self::SUPPORTED).']'); } + $this->method= $method; } /** @return string */ diff --git a/src/test/php/web/auth/unittest/ByPKCETest.class.php b/src/test/php/web/auth/unittest/ByPKCETest.class.php index 240991b..26ffe02 100755 --- a/src/test/php/web/auth/unittest/ByPKCETest.class.php +++ b/src/test/php/web/auth/unittest/ByPKCETest.class.php @@ -14,7 +14,7 @@ private function challenges() { yield ['plain', 'test-challenge']; } - #[Test, Values(['S256', 'plain'])] + #[Test, Values(ByPKCE::SUPPORTED)] public function can_create_with($method) { new ByPKCE(self::CLIENT_ID, $method); }