From d48b136412a9994e2895360fcc4035992b9ae768 Mon Sep 17 00:00:00 2001 From: Timm Friebe Date: Wed, 17 Jul 2024 08:21:38 +0200 Subject: [PATCH 1/2] Make it possible to change the session namespace --- .../php/web/auth/oauth/OAuth1Flow.class.php | 13 +++--- .../php/web/auth/oauth/OAuth2Flow.class.php | 11 +++-- .../php/web/auth/oauth/OAuthFlow.class.php | 8 +++- .../auth/unittest/OAuth1FlowTest.class.php | 37 ++++++++++------ .../auth/unittest/OAuth2FlowTest.class.php | 43 ++++++++++--------- 5 files changed, 65 insertions(+), 47 deletions(-) diff --git a/src/main/php/web/auth/oauth/OAuth1Flow.class.php b/src/main/php/web/auth/oauth/OAuth1Flow.class.php index d779680..2cd2752 100755 --- a/src/main/php/web/auth/oauth/OAuth1Flow.class.php +++ b/src/main/php/web/auth/oauth/OAuth1Flow.class.php @@ -7,8 +7,6 @@ /** @test web.auth.unittest.OAuth1FlowTest */ class OAuth1Flow extends OAuthFlow { - const SESSION_KEY= 'oauth1::flow'; - private $service, $signature; /** @@ -19,6 +17,7 @@ class OAuth1Flow extends OAuthFlow { * @param string|util.URI $callback */ public function __construct($service, $consumer, $callback= null) { + $this->namespace= 'oauth1::flow'; $this->service= rtrim($service, '/'); // BC: Support web.auth.oauth.Token instances @@ -77,11 +76,11 @@ protected function request($path, $token= null, $params= []) { * @throws lang.IllegalStateException */ public function authenticate($request, $response, $session) { - $state= $session->value(self::SESSION_KEY); + $state= $session->value($this->namespace); // We have an access token, reset state and return an authenticated session if (isset($state['access'])) { - $session->remove(self::SESSION_KEY); + $session->remove($this->namespace); return new BySignedRequests($this->signature->with(new BySecret($state['oauth_token'], $state['oauth_token_secret']))); } @@ -93,7 +92,7 @@ public function authenticate($request, $response, $session) { $state['target'].= '#'.$fragment; } - $session->register(self::SESSION_KEY, $state); + $session->register($this->namespace, $state); $session->transmit($response); $response->send('document.location.replace(target)', 'text/javascript'); return null; @@ -106,7 +105,7 @@ public function authenticate($request, $response, $session) { $server= $request->param('oauth_token'); if (null === $state || null === $server) { $token= $this->request('/request_token', null, ['oauth_callback' => $callback]); - $session->register(self::SESSION_KEY, $token + ['target' => (string)$uri]); + $session->register($this->namespace, $token + ['target' => (string)$uri]); $session->transmit($response); // Redirect the user to the authorization page @@ -139,7 +138,7 @@ public function authenticate($request, $response, $session) { // Back from authentication redirect, upgrade request token to access token $access= $this->request('/access_token', $state['oauth_token'], ['oauth_verifier' => $request->param('oauth_verifier')]); - $session->register(self::SESSION_KEY, $access + ['access' => true]); + $session->register($this->namespace, $access + ['access' => true]); $session->transmit($response); // Redirect to self diff --git a/src/main/php/web/auth/oauth/OAuth2Flow.class.php b/src/main/php/web/auth/oauth/OAuth2Flow.class.php index a480a48..b75d20e 100755 --- a/src/main/php/web/auth/oauth/OAuth2Flow.class.php +++ b/src/main/php/web/auth/oauth/OAuth2Flow.class.php @@ -8,8 +8,6 @@ /** @test web.auth.unittest.OAuth2FlowTest */ class OAuth2Flow extends OAuthFlow { - const SESSION_KEY= 'oauth2::flow'; - private $auth, $backend, $scopes, $rand; /** @@ -22,6 +20,7 @@ class OAuth2Flow extends OAuthFlow { * @param string[] $scopes */ public function __construct($auth, $tokens, $consumer, $callback= null, $scopes= ['user']) { + $this->namespace= 'oauth2::flow'; $this->auth= $auth instanceof URI ? $auth : new URI($auth); $this->backend= $tokens instanceof OAuth2Endpoint ? $tokens->using($consumer) @@ -85,13 +84,13 @@ public function refresh(array $claims) { * @throws lang.IllegalStateException */ public function authenticate($request, $response, $session) { - $stored= $session->value(self::SESSION_KEY); + $stored= $session->value($this->namespace); // We have an access token, reset state and return an authenticated session // See https://www.oauth.com/oauth2-servers/access-tokens/access-token-response/ // and https://tools.ietf.org/html/rfc6749#section-5.1 if (isset($stored['access_token'])) { - $session->remove(self::SESSION_KEY); + $session->remove($this->namespace); return new ByAccessToken( $stored['access_token'], $stored['token_type'] ?? 'Bearer', @@ -114,7 +113,7 @@ public function authenticate($request, $response, $session) { $state= $stored['state']; } else { $state= bin2hex($this->rand->bytes(16)); - $session->register(self::SESSION_KEY, ['state' => $state, 'target' => (string)$uri]); + $session->register($this->namespace, ['state' => $state, 'target' => (string)$uri]); $session->transmit($response); } @@ -156,7 +155,7 @@ public function authenticate($request, $response, $session) { 'redirect_uri' => $callback, 'state' => $stored['state'] ]); - $session->register(self::SESSION_KEY, $token); + $session->register($this->namespace, $token); $session->transmit($response); // Redirect to self, using encoded fragment if present diff --git a/src/main/php/web/auth/oauth/OAuthFlow.class.php b/src/main/php/web/auth/oauth/OAuthFlow.class.php index a7d062c..0ac253b 100755 --- a/src/main/php/web/auth/oauth/OAuthFlow.class.php +++ b/src/main/php/web/auth/oauth/OAuthFlow.class.php @@ -4,7 +4,7 @@ use web\auth\{Flow, UserInfo, AuthenticationError}; abstract class OAuthFlow extends Flow { - protected $callback; + protected $callback, $namespace; /** @return ?util.URI */ public function callback() { return $this->callback; } @@ -15,6 +15,12 @@ public function calling($callback): self { return $this; } + /** @param string $namespace */ + public function namespaced($namespace) { + $this->namespace= $namespace; + return $this; + } + /** * Returns user info which fetched from the given endpoint using the * authorized OAuth2 client diff --git a/src/test/php/web/auth/unittest/OAuth1FlowTest.class.php b/src/test/php/web/auth/unittest/OAuth1FlowTest.class.php index c05ce8b..33af3c1 100755 --- a/src/test/php/web/auth/unittest/OAuth1FlowTest.class.php +++ b/src/test/php/web/auth/unittest/OAuth1FlowTest.class.php @@ -11,6 +11,7 @@ class OAuth1FlowTest extends FlowTest { use Clients; + const SNS = 'oauth1::flow'; const AUTH = 'https://example.com/oauth'; const ID = 'bf396750'; const SECRET = '5ebe2294ecd0e0f08eab7690d2a6ee69'; @@ -47,7 +48,7 @@ public function fetches_request_token_then_redirects_to_auth($path) { sprintf('%s/authenticate?oauth_token=T&oauth_callback=%s', self::AUTH, urlencode(self::CALLBACK)), $this->redirectTo($this->authenticate($fixture, $path, $session)) ); - Assert::equals('http://localhost'.$path, $session->value(OAuth1Flow::SESSION_KEY)['target']); + Assert::equals('http://localhost'.$path, $session->value(self::SNS)['target']); } #[Test, Values(from: 'fragments')] @@ -62,7 +63,7 @@ public function fetches_request_token_then_redirects_to_auth_with_fragment_in_sp sprintf('%s/authenticate?oauth_token=T&oauth_callback=%s', self::AUTH, urlencode(self::CALLBACK)), $this->redirectTo($this->authenticate($fixture, '/#'.$fragment, $session)) ); - Assert::equals('http://localhost/#'.$fragment, $session->value(OAuth1Flow::SESSION_KEY)['target']); + Assert::equals('http://localhost/#'.$fragment, $session->value(self::SNS)['target']); } #[Test] @@ -72,18 +73,18 @@ public function exchanges_request_token_for_access_token() { 'request' => function($path, $token= null, $params= []) use($access) { return $access; } ]); $session= (new ForTesting())->create(); - $session->register(OAuth1Flow::SESSION_KEY, ['oauth_token' => 'REQUEST-TOKEN', 'target' => self::SERVICE]); + $session->register(self::SNS, ['oauth_token' => 'REQUEST-TOKEN', 'target' => self::SERVICE]); $res= $this->authenticate($fixture, '/?oauth_token=REQUEST-TOKEN&oauth_verifier=ABC', $session); Assert::equals(self::SERVICE, $res->headers()['Location']); - Assert::equals($access, $session->value(OAuth1Flow::SESSION_KEY)); + Assert::equals($access, $session->value(self::SNS)); } #[Test, Expect(IllegalStateException::class)] public function raises_exception_on_state_mismatch() { $fixture= new OAuth1Flow(self::AUTH, [self::ID, self::SECRET], self::CALLBACK); $session= (new ForTesting())->create(); - $session->register(OAuth1Flow::SESSION_KEY, ['oauth_token' => 'REQUEST-TOKEN', 'target' => self::SERVICE]); + $session->register(self::SNS, ['oauth_token' => 'REQUEST-TOKEN', 'target' => self::SERVICE]); $this->authenticate($fixture, '/?oauth_token=MISMATCHED-TOKEN&oauth_verifier=ABC', $session); } @@ -96,7 +97,7 @@ public function returns_client() { $req= new Request(new TestInput('GET', '/')); $res= new Response(new TestOutput()); $session= (new ForTesting())->create(); - $session->register(OAuth1Flow::SESSION_KEY, $access); + $session->register(self::SNS, $access); Assert::instance(Client::class, $fixture->authenticate($req, $res, $session)); } @@ -109,10 +110,10 @@ public function resets_state_after_returning_client() { $req= new Request(new TestInput('GET', '/')); $res= new Response(new TestOutput()); $session= (new ForTesting())->create(); - $session->register(OAuth1Flow::SESSION_KEY, $access); + $session->register(self::SNS, $access); $fixture->authenticate($req, $res, $session); - Assert::null($session->value(OAuth1Flow::SESSION_KEY)); + Assert::null($session->value(self::SNS)); } #[Test, Values(from: 'fragments')] @@ -122,11 +123,11 @@ public function appends_fragment($fragment) { $req= new Request(new TestInput('GET', '/?'.OAuth1Flow::FRAGMENT.'='.urlencode($fragment))); $res= new Response(new TestOutput()); $session= (new ForTesting())->create(); - $session->register(OAuth1Flow::SESSION_KEY, ['target' => 'http://localhost/']); + $session->register(self::SNS, ['target' => 'http://localhost/']); $fixture->authenticate($req, $res, $session); - Assert::equals('http://localhost/#'.$fragment, $session->value(OAuth1Flow::SESSION_KEY)['target']); + Assert::equals('http://localhost/#'.$fragment, $session->value(self::SNS)['target']); } #[Test, Values(from: 'fragments')] @@ -136,11 +137,11 @@ public function replaces_fragment($fragment) { $req= new Request(new TestInput('GET', '/?'.OAuth1Flow::FRAGMENT.'='.urlencode($fragment))); $res= new Response(new TestOutput()); $session= (new ForTesting())->create(); - $session->register(OAuth1Flow::SESSION_KEY, ['target' => 'http://localhost/#original']); + $session->register(self::SNS, ['target' => 'http://localhost/#original']); $fixture->authenticate($req, $res, $session); - Assert::equals('http://localhost/#'.$fragment, $session->value(OAuth1Flow::SESSION_KEY)['target']); + Assert::equals('http://localhost/#'.$fragment, $session->value(self::SNS)['target']); } /** @deprecated */ @@ -180,4 +181,16 @@ public function fetch_user_info() { $fixture($this->responding(200, ['Content-Type' => 'application/json'], '{"id":"root"}')) ); } + + #[Test, Values(['oauth::flow', 'flow'])] + public function session_namespace($namespace) { + $request= ['oauth_token' => 'T']; + $fixture= newinstance(OAuth1Flow::class, [self::AUTH, [self::ID, self::SECRET], self::CALLBACK], [ + 'request' => function($path, $token= null, $params= []) use($request) { return $request; } + ]); + $session= (new ForTesting())->create(); + $this->authenticate($fixture->namespaced($namespace), '/target', $session); + + Assert::equals('http://localhost/target', $session->value($namespace)['target']); + } } \ 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 c07c4b1..004cb8b 100755 --- a/src/test/php/web/auth/unittest/OAuth2FlowTest.class.php +++ b/src/test/php/web/auth/unittest/OAuth2FlowTest.class.php @@ -13,6 +13,7 @@ class OAuth2FlowTest extends FlowTest { use PrivateKey, Clients; + const SNS = 'oauth2::flow'; const AUTH = 'https://example.com/oauth/authorize'; const TOKENS = 'https://example.com/oauth/access_token'; const CONSUMER = ['bf396750', '5ebe2294ecd0e0f08eab7690d2a6ee69']; @@ -36,7 +37,7 @@ private function assertLoginWith($service, $scope, $res, $session) { self::CONSUMER[0], implode('+', $scope), urlencode($service), - $session->value(OAuth2Flow::SESSION_KEY)['state'] + $session->value(self::SNS)['state'] ); Assert::equals($url, $this->redirectTo($res)); } @@ -90,7 +91,7 @@ public function redirects_to_auth($path) { $this->authenticate($fixture, $path, $session), $session ); - Assert::equals('http://localhost'.$path, $session->value(OAuth2Flow::SESSION_KEY)['target']); + Assert::equals('http://localhost'.$path, $session->value(self::SNS)['target']); } #[Test, Values(from: 'paths')] @@ -104,7 +105,7 @@ public function redirects_to_auth_with_relative_callback($path) { $this->authenticate($fixture, $path, $session), $session ); - Assert::equals('http://localhost'.$path, $session->value(OAuth2Flow::SESSION_KEY)['target']); + Assert::equals('http://localhost'.$path, $session->value(self::SNS)['target']); } #[Test, Values(from: 'paths')] @@ -118,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, $session->value(OAuth2Flow::SESSION_KEY)['target']); + Assert::equals('http://localhost'.$path, $session->value(self::SNS)['target']); } #[Test, Values(from: 'paths')] @@ -132,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, $session->value(OAuth2Flow::SESSION_KEY)['target']); + Assert::equals(self::SERVICE.$path, $session->value(self::SNS)['target']); } #[Test, Values(from: 'fragments')] @@ -146,7 +147,7 @@ public function redirects_to_sso_with_fragment($fragment) { $this->authenticate($fixture, '/#'.$fragment, $session), $session ); - Assert::equals('http://localhost/#'.$fragment, $session->value(OAuth2Flow::SESSION_KEY)['target']); + Assert::equals('http://localhost/#'.$fragment, $session->value(self::SNS)['target']); } #[Test, Values([[['user']], [['user', 'openid']]])] @@ -166,7 +167,7 @@ public function redirects_to_auth_and_passes_scope($scopes) { public function redirects_to_auth_when_previous_redirect_incomplete() { $fixture= new OAuth2Flow(self::AUTH, self::TOKENS, self::CONSUMER, self::CALLBACK); $session= (new ForTesting())->create(); - $session->register(OAuth2Flow::SESSION_KEY, ['state' => 'PREVIOUS_STATE', 'target' => self::SERVICE]); + $session->register('oauth2::flow', ['state' => 'PREVIOUS_STATE', 'target' => self::SERVICE]); $this->assertLoginWith( self::CALLBACK, @@ -180,10 +181,10 @@ public function redirects_to_auth_when_previous_redirect_incomplete() { public function reuses_state_when_previous_redirect_incomplete() { $fixture= new OAuth2Flow(self::AUTH, self::TOKENS, self::CONSUMER, self::CALLBACK); $session= (new ForTesting())->create(); - $session->register(OAuth2Flow::SESSION_KEY, ['state' => 'REUSED_STATE', 'target' => self::SERVICE]); + $session->register('oauth2::flow', ['state' => 'REUSED_STATE', 'target' => self::SERVICE]); $this->authenticate($fixture, '/', $session); - Assert::equals('REUSED_STATE', $session->value(OAuth2Flow::SESSION_KEY)['state']); + Assert::equals('REUSED_STATE', $session->value(self::SNS)['state']); } #[Test] @@ -195,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(OAuth2Flow::SESSION_KEY, ['state' => $state, 'target' => self::SERVICE]); + $session->register('oauth2::flow', ['state' => $state, 'target' => self::SERVICE]); $this->authenticate($fixture, '/?code=SERVER_CODE&state='.$state, $session); Assert::equals('authorization_code', $passed['grant_type']); @@ -213,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(OAuth2Flow::SESSION_KEY, ['state' => $state, 'target' => self::SERVICE]); + $session->register('oauth2::flow', ['state' => $state, 'target' => self::SERVICE]); $this->authenticate($fixture, '/?code=SERVER_CODE&state='.$state, $session); Assert::equals('authorization_code', $passed['grant_type']); @@ -232,11 +233,11 @@ 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(OAuth2Flow::SESSION_KEY, ['state' => $state, 'target' => self::SERVICE]); + $session->register('oauth2::flow', ['state' => $state, 'target' => self::SERVICE]); $res= $this->authenticate($fixture, '/?code=SERVER_CODE&state='.$state, $session); Assert::equals(self::SERVICE, $res->headers()['Location']); - Assert::equals($token, $session->value(OAuth2Flow::SESSION_KEY)); + Assert::equals($token, $session->value(self::SNS)); } #[Test, Values(from: 'fragments')] @@ -248,18 +249,18 @@ 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(OAuth2Flow::SESSION_KEY, ['state' => $state, 'target' => self::SERVICE]); + $session->register('oauth2::flow', ['state' => $state, 'target' => self::SERVICE]); $res= $this->authenticate($fixture, '/?code=SERVER_CODE&state='.$state.OAuth2Flow::FRAGMENT.urlencode($fragment), $session); Assert::equals(self::SERVICE.'#'.$fragment, $res->headers()['Location']); - Assert::equals($token, $session->value(OAuth2Flow::SESSION_KEY)); + Assert::equals($token, $session->value(self::SNS)); } #[Test, Expect(IllegalStateException::class)] public function raises_exception_on_state_mismatch() { $fixture= new OAuth2Flow(self::AUTH, self::TOKENS, self::CONSUMER, self::CALLBACK); $session= (new ForTesting())->create(); - $session->register(OAuth2Flow::SESSION_KEY, ['state' => 'CLIENTSTATE', 'target' => self::SERVICE]); + $session->register('oauth2::flow', ['state' => 'CLIENTSTATE', 'target' => self::SERVICE]); $this->authenticate($fixture, '/?state=SERVERSTATE&code=SERVER_CODE', $session); } @@ -271,7 +272,7 @@ public function returns_client_in_final_step($response) { $req= new Request(new TestInput('GET', '/')); $res= new Response(new TestOutput()); $session= (new ForTesting())->create(); - $session->register(OAuth2Flow::SESSION_KEY, $response); + $session->register('oauth2::flow', $response); Assert::instance(Client::class, $fixture->authenticate($req, $res, $session)); } @@ -284,10 +285,10 @@ public function resets_state_after_returning_client() { $req= new Request(new TestInput('GET', '/')); $res= new Response(new TestOutput()); $session= (new ForTesting())->create(); - $session->register(OAuth2Flow::SESSION_KEY, $token); + $session->register('oauth2::flow', $token); $fixture->authenticate($req, $res, $session); - Assert::null($session->value(OAuth2Flow::SESSION_KEY)); + Assert::null($session->value(self::SNS)); } #[Test] @@ -297,7 +298,7 @@ public function claims_returned() { $req= new Request(new TestInput('GET', '/')); $res= new Response(new TestOutput()); $session= (new ForTesting())->create(); - $session->register(OAuth2Flow::SESSION_KEY, ['access_token' => '']); + $session->register('oauth2::flow', ['access_token' => '']); Assert::null($fixture->authenticate($req, $res, $session)->claims()); } @@ -309,7 +310,7 @@ public function claims_returned_with_expiry() { $req= new Request(new TestInput('GET', '/')); $res= new Response(new TestOutput()); $session= (new ForTesting())->create(); - $session->register(OAuth2Flow::SESSION_KEY, ['access_token' => '', 'expires_in' => 3600, 'refresh_token' => '']); + $session->register('oauth2::flow', ['access_token' => '', 'expires_in' => 3600, 'refresh_token' => '']); Assert::equals( ['expires' => time() + 3600, 'refresh' => ''], From f6ff41cea912207a8f6edee2351edb2eb4944156 Mon Sep 17 00:00:00 2001 From: Timm Friebe Date: Wed, 17 Jul 2024 08:34:10 +0200 Subject: [PATCH 2/2] QA: Document namespaced() method --- src/main/php/web/auth/oauth/OAuthFlow.class.php | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/main/php/web/auth/oauth/OAuthFlow.class.php b/src/main/php/web/auth/oauth/OAuthFlow.class.php index 0ac253b..d01d72c 100755 --- a/src/main/php/web/auth/oauth/OAuthFlow.class.php +++ b/src/main/php/web/auth/oauth/OAuthFlow.class.php @@ -15,7 +15,13 @@ public function calling($callback): self { return $this; } - /** @param string $namespace */ + /** + * Sets session namespace for this flow. Used to prevent conflicts + * in session state with multiple OAuth flows in place. + * + * @param string $namespace + * @return self + */ public function namespaced($namespace) { $this->namespace= $namespace; return $this; @@ -23,7 +29,7 @@ public function namespaced($namespace) { /** * Returns user info which fetched from the given endpoint using the - * authorized OAuth2 client + * authorized OAuth client * * @param string|util.URI $endpoint * @return web.auth.UserInfo