From 3ccde2b4677cd54fc84ac83366d6edd6f88358e8 Mon Sep 17 00:00:00 2001 From: Johan Cwiklinski Date: Sat, 26 Sep 2026 23:21:56 +0200 Subject: [PATCH 1/4] Make tests independent from the working directory --- _routes.php | 4 ++-- tests/TestsBootstrap.php | 10 +++------- tests/router.php | 2 +- 3 files changed, 6 insertions(+), 10 deletions(-) diff --git a/_routes.php b/_routes.php index 902d028..213ed34 100644 --- a/_routes.php +++ b/_routes.php @@ -21,12 +21,12 @@ use GaletteOAuth2\Middleware\Authentication; //Include specific classes (league/oauth2_server and tools) -require_once 'vendor/autoload.php'; +require_once __DIR__ . '/vendor/autoload.php'; //Constants and classes from plugin require_once $module['root'] . '/_config.inc.php'; -require '_dependencies.php'; +require __DIR__ . '/_dependencies.php'; //login is always called by a http_redirect $app->get( diff --git a/tests/TestsBootstrap.php b/tests/TestsBootstrap.php index 20569c5..f4ecfbc 100644 --- a/tests/TestsBootstrap.php +++ b/tests/TestsBootstrap.php @@ -13,14 +13,10 @@ */ define('GALETTE_PLUGINS_PATH', __DIR__ . '/../../'); -$basepath = '../../../galette/'; +$basepath = __DIR__ . '/../../../'; // phpcs:ignore SlevomatCodingStandard.Variables.UnusedVariable.UnusedVariable -- used from Core testBootstrap define('OAUTH2_CONFIGPATH', __DIR__ . '/config'); include_once __DIR__ . '/../vendor/autoload.php'; -include_once '../../../tests/TestsBootstrap.php'; -include_once __DIR__ . '/../_dependencies.php'; -$module = [ - 'root' => __DIR__ . '/..' -]; -include_once __DIR__ . '/../_routes.php'; +include_once __DIR__ . '/../../../../tests/TestsBootstrap.php'; +require_once __DIR__ . '/../_config.inc.php'; diff --git a/tests/router.php b/tests/router.php index 5e5389d..4c65908 100644 --- a/tests/router.php +++ b/tests/router.php @@ -21,7 +21,7 @@ $db = 'pgsql'; } -$basepath = '../../tests/'; +$basepath = __DIR__ . '/../../../../tests/'; define('GALETTE_CONFIG_PATH', $basepath . 'config/' . $db . '/'); define('OAUTH2_CONFIGPATH', __DIR__ . '/config'); return false; From 353d9e1a919b5c503ca407c9defe8fbb31eb5efc Mon Sep 17 00:00:00 2001 From: Johan Cwiklinski Date: Sat, 26 Sep 2026 23:23:59 +0200 Subject: [PATCH 2/4] Check tests and root files with phpstan level 6, and tests with phpcs --- .github/workflows/ci-linux.yml | 2 +- _dependencies.php | 2 ++ _routes.php | 5 +++++ lib/GaletteOAuth2/Authorization/UserHelper.php | 10 +++++++--- lib/GaletteOAuth2/Controllers/LoginController.php | 5 +++++ lib/GaletteOAuth2/Entities/ClientEntity.php | 7 +++++-- lib/GaletteOAuth2/Repositories/ClientRepository.php | 6 +++--- lib/GaletteOAuth2/Repositories/ScopeRepository.php | 5 +++++ lib/GaletteOAuth2/Tools/Config.php | 3 +++ lib/GaletteOAuth2/Tools/Debug.php | 8 ++------ phpstan.neon | 7 ++++++- .../Authorization/tests/units/UserHelper.php | 11 +++++++---- tests/GaletteOAuth2/GaletteOAuth2.php | 1 - 13 files changed, 51 insertions(+), 21 deletions(-) diff --git a/.github/workflows/ci-linux.yml b/.github/workflows/ci-linux.yml index fc6f9ed..9e4de53 100644 --- a/.github/workflows/ci-linux.yml +++ b/.github/workflows/ci-linux.yml @@ -81,7 +81,7 @@ jobs: - name: CS run: | cd galette-core/galette/plugins/plugin-oauth2 - ../../vendor/bin/phpcs lib/ ./*.php + ../../vendor/bin/phpcs lib/ tests/ ./*.php - name: CS Fixer if: matrix.php-is-min diff --git a/_dependencies.php b/_dependencies.php index 02fb378..4df4089 100644 --- a/_dependencies.php +++ b/_dependencies.php @@ -15,6 +15,7 @@ * @author Johan Cwiklinski */ +use Analog\Analog; use Galette\Core\Preferences; use GaletteOAuth2\Repositories\AccessTokenRepository; use GaletteOAuth2\Repositories\AuthCodeRepository; @@ -31,6 +32,7 @@ use RKA\SessionMiddleware; use Slim\Flash\Messages; +/** @var \Slim\Routing\RouteCollectorProxy<\DI\Container> $app */ $container = $app->getContainer(); $container->set( diff --git a/_routes.php b/_routes.php index 213ed34..9f1eb11 100644 --- a/_routes.php +++ b/_routes.php @@ -20,6 +20,11 @@ use GaletteOAuth2\Controllers\LoginController; use GaletteOAuth2\Middleware\Authentication; +/** + * @var \Slim\Routing\RouteCollectorProxy<\DI\Container> $app + * @var array $module + */ + //Include specific classes (league/oauth2_server and tools) require_once __DIR__ . '/vendor/autoload.php'; diff --git a/lib/GaletteOAuth2/Authorization/UserHelper.php b/lib/GaletteOAuth2/Authorization/UserHelper.php index 3b04b22..8f9f94b 100644 --- a/lib/GaletteOAuth2/Authorization/UserHelper.php +++ b/lib/GaletteOAuth2/Authorization/UserHelper.php @@ -286,6 +286,8 @@ public static function getUserData(Container $container, int $id, string $acl, a * * @param Adherent $member Member * @param bool $legacy Legacy mode for data + * + * @return string[] */ protected static function getUserGroups(Adherent $member, bool $legacy = false): array { @@ -369,10 +371,12 @@ public static function getAuthorization(Config $config, string $client_id): stri /** * Merge requested and configured scopes * - * @param Config $config Config instance + * @param ?Config $config Config instance + * @param string $client_id Client app identifier + * @param string[]|string $requested_scopes Requested scopes from query string + * @param bool $with_default Add default scope * - * @param string $client_id Client app identifier - * @param array|string $requested_scopes Requested scopes from query string + * @return string[] */ public static function mergeScopes( ?Config $config, diff --git a/lib/GaletteOAuth2/Controllers/LoginController.php b/lib/GaletteOAuth2/Controllers/LoginController.php index bc19473..ac21091 100755 --- a/lib/GaletteOAuth2/Controllers/LoginController.php +++ b/lib/GaletteOAuth2/Controllers/LoginController.php @@ -227,6 +227,11 @@ public function error(Request $request, Response $response): Response return $response; } + /** + * Prepare login form variables, null if client is invalid + * + * @return ?array + */ private function prepareVarsForm(): ?array { $client_id = $this->session->request_args['client_id'] ?? null; diff --git a/lib/GaletteOAuth2/Entities/ClientEntity.php b/lib/GaletteOAuth2/Entities/ClientEntity.php index 27b999e..ee6c348 100755 --- a/lib/GaletteOAuth2/Entities/ClientEntity.php +++ b/lib/GaletteOAuth2/Entities/ClientEntity.php @@ -25,12 +25,15 @@ final class ClientEntity implements ClientEntityInterface use EntityTrait; use ClientTrait; - public function setName($name): void + public function setName(string $name): void { $this->name = $name; } - public function setRedirectUri($uri): void + /** + * @param string|string[] $uri Redirect URI(s) + */ + public function setRedirectUri(string|array $uri): void { $this->redirectUri = $uri; } diff --git a/lib/GaletteOAuth2/Repositories/ClientRepository.php b/lib/GaletteOAuth2/Repositories/ClientRepository.php index cfc86d2..a96d127 100755 --- a/lib/GaletteOAuth2/Repositories/ClientRepository.php +++ b/lib/GaletteOAuth2/Repositories/ClientRepository.php @@ -11,7 +11,7 @@ namespace GaletteOAuth2\Repositories; use Analog\Analog; -use DI\Container; +use Psr\Container\ContainerInterface; use GaletteOAuth2\Entities\ClientEntity; use GaletteOAuth2\Tools\Config; use GaletteOAuth2\Tools\Debug; @@ -28,10 +28,10 @@ final class ClientRepository implements ClientRepositoryInterface { private const string EXAMPLE_PASSWORD = 'abc123'; - private Container $container; + private ContainerInterface $container; private Config $config; - public function __construct(Container $container) + public function __construct(ContainerInterface $container) { $this->container = $container; $this->config = $this->container->get(Config::class); diff --git a/lib/GaletteOAuth2/Repositories/ScopeRepository.php b/lib/GaletteOAuth2/Repositories/ScopeRepository.php index 42bf782..e432298 100755 --- a/lib/GaletteOAuth2/Repositories/ScopeRepository.php +++ b/lib/GaletteOAuth2/Repositories/ScopeRepository.php @@ -26,6 +26,11 @@ */ final class ScopeRepository implements ScopeRepositoryInterface { + /** + * Known scopes, with their description + * + * @return array + */ public static function knownScopes(): array { return [ diff --git a/lib/GaletteOAuth2/Tools/Config.php b/lib/GaletteOAuth2/Tools/Config.php index 18fc6a6..d0ab548 100644 --- a/lib/GaletteOAuth2/Tools/Config.php +++ b/lib/GaletteOAuth2/Tools/Config.php @@ -21,6 +21,9 @@ final class Config extends \Noodlehaus\Config /** @var string[]|string */ private array|string $path; + /** + * @param string[]|string $values Configuration file(s) + */ public function __construct(array|string $values) { $this->path = $values; diff --git a/lib/GaletteOAuth2/Tools/Debug.php b/lib/GaletteOAuth2/Tools/Debug.php index 7dcd2a5..7816c6e 100644 --- a/lib/GaletteOAuth2/Tools/Debug.php +++ b/lib/GaletteOAuth2/Tools/Debug.php @@ -30,7 +30,7 @@ final class Debug 'code_verifier', ]; - public static function printVar($expression, bool $return = true) + public static function printVar(mixed $expression): string { $export = print_r($expression, true); $patterns = [ @@ -39,12 +39,8 @@ public static function printVar($expression, bool $return = true) "/=>[ ]?\n[ ]+\\[/" => '=> [', "/([ ]*)(\\'[^\\']+\\') => ([\\[\\'])/" => '$1$2 => $3', ]; - $export = preg_replace(array_keys($patterns), array_values($patterns), $export); - if ($return) { - return $export; - } - echo $export; + return preg_replace(array_keys($patterns), array_values($patterns), $export); } public static function log(string $txt): void diff --git a/phpstan.neon b/phpstan.neon index a8e52ab..67197bf 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -1,9 +1,14 @@ parameters: parallel: maximumNumberOfProcesses: 2 - level: 5 + level: 6 paths: - lib/ + - tests/ + - _config.inc.php + - _define.php + - _dependencies.php + - _routes.php scanFiles: - _config.inc.php - ../../includes/sys_config/paths.inc.php diff --git a/tests/GaletteOAuth2/Authorization/tests/units/UserHelper.php b/tests/GaletteOAuth2/Authorization/tests/units/UserHelper.php index 70ce96c..89a4230 100644 --- a/tests/GaletteOAuth2/Authorization/tests/units/UserHelper.php +++ b/tests/GaletteOAuth2/Authorization/tests/units/UserHelper.php @@ -8,6 +8,7 @@ namespace GaletteOauth2\Authorization\tests\units; +use Analog\Analog; use Galette\Tests\GaletteTestCase; use PHPUnit\Framework\Attributes\DataProvider; @@ -313,7 +314,7 @@ public function testRequireAdmin() /** * Data provider for not found members * - * @return array + * @return array> */ public static function memberNotFoundProvider(): array { @@ -326,10 +327,12 @@ public static function memberNotFoundProvider(): array /** * Test with a not found member * + * @param int $member_id Member ID + * * @return void */ #[DataProvider('memberNotFoundProvider')] - public function testMemberNotFound($member_id) + public function testMemberNotFound(int $member_id): void { global $container; @@ -346,7 +349,7 @@ public function testMemberNotFound($member_id) $this->assertEquals("User not found.", $e->getMessage()); } $this->assertTrue($exception_thrown); - $this->expectLogEntry(\Analog::ERROR, 'No member #' . $member_id); + $this->expectLogEntry(Analog::ERROR, 'No member #' . $member_id); } /** @@ -485,7 +488,7 @@ public function testGetAuthorizations(): void \GaletteOAuth2\Authorization\UserHelper::getAuthorization($config, 'galette_test') ); $this->expectLogEntry( - \Analog::ERROR, + Analog::ERROR, 'Invalid authorization "unknown" for client "galette_test"' ); diff --git a/tests/GaletteOAuth2/GaletteOAuth2.php b/tests/GaletteOAuth2/GaletteOAuth2.php index d69053e..59e60b3 100644 --- a/tests/GaletteOAuth2/GaletteOAuth2.php +++ b/tests/GaletteOAuth2/GaletteOAuth2.php @@ -110,7 +110,6 @@ private function runFlow(array $checked_scopes): array $redirected_uri = $headersRedirect[0]; parse_str(parse_url($redirected_uri, PHP_URL_QUERY), $url_arguments); - $this->assertIsArray($url_arguments); $this->assertArrayHasKey('code', $url_arguments); $this->assertArrayHasKey('state', $url_arguments); From b5735d62133de7448d9dc43e8455b4772f2262db Mon Sep 17 00:00:00 2001 From: Johan Cwiklinski Date: Sat, 26 Sep 2026 23:26:29 +0200 Subject: [PATCH 3/4] Test tokens, user data and authentication middleware --- .../tests/units/AuthorizationCodeFlow.php | 291 ++++++++++++++++++ .../tests/units/AuthorizationController.php | 52 ++++ 2 files changed, 343 insertions(+) create mode 100644 tests/GaletteOAuth2/Controllers/tests/units/AuthorizationCodeFlow.php diff --git a/tests/GaletteOAuth2/Controllers/tests/units/AuthorizationCodeFlow.php b/tests/GaletteOAuth2/Controllers/tests/units/AuthorizationCodeFlow.php new file mode 100644 index 0000000..dc4118c --- /dev/null +++ b/tests/GaletteOAuth2/Controllers/tests/units/AuthorizationCodeFlow.php @@ -0,0 +1,291 @@ + + */ +class AuthorizationCodeFlow extends GaletteRoutingTestCase +{ + protected int $seed = 20260926233000; + protected bool $load_plugins = true; + + private const string CLIENT_ID = 'galette_cli'; + private const string CLIENT_SECRET = 'cli-secret-for-tests'; + private const string REDIRECT_URI = 'http://localhost:8888'; + + /** + * Set up tests + * + * @return void + */ + public function setUp(): void + { + global $session; + parent::setUp(); + + $this->session = $this->container->get('oauth_session'); + $session = $this->session; + } + + /** + * Tear down tests + * + * @return void + */ + public function tearDown(): void + { + unset( + $this->session->isLoggedIn, + $this->session->user_id, + $this->session->client_id + ); + parent::tearDown(); + } + + /** + * Get an authorization code, as if member had logged in and approved + * + * @param int $member_id Member ID + * @param string[] $scopes Scopes checked on the consent screen + * + * @return string + */ + private function getAuthorizationCode(int $member_id, array $scopes): string + { + $this->session->isLoggedIn = 'yes'; + $this->session->user_id = $member_id; + $this->session->client_id = self::CLIENT_ID; + + $request = $this->createRequest( + route_name: OAUTH2_PREFIX . '_doAuthorize', + method: 'POST', + query_params: [ + 'response_type' => 'code', + 'client_id' => self::CLIENT_ID, + 'redirect_uri' => self::REDIRECT_URI, + 'scope' => implode(' ', $scopes), + 'state' => 'flow-state', + ] + ); + $request = $request->withParsedBody(['approve' => '', 'scopes' => $scopes]); + $test_response = $this->app->handle($request); + + $this->assertSame(302, $test_response->getStatusCode()); + $location = $test_response->getHeaderLine('Location'); + $this->assertStringStartsWith(self::REDIRECT_URI . '?', $location); + parse_str((string)parse_url($location, PHP_URL_QUERY), $args); + $this->assertSame('flow-state', $args['state']); + $this->assertIsString($args['code']); + + return $args['code']; + } + + /** + * Send a token request + * + * @param array $params Request parameters + * + * @return ResponseInterface + */ + private function requestToken(array $params): ResponseInterface + { + $request = $this->createRequest( + route_name: OAUTH2_PREFIX . '_token', + method: 'POST', + content_type: 'application/x-www-form-urlencoded' + ); + $request = $request->withParsedBody( + $params + [ + 'client_id' => self::CLIENT_ID, + 'client_secret' => self::CLIENT_SECRET, + ] + ); + return $this->app->handle($request); + } + + /** + * Exchange an authorization code for tokens + * + * @param string $code Authorization code + * + * @return array + */ + private function exchangeCode(string $code): array + { + $test_response = $this->requestToken([ + 'grant_type' => 'authorization_code', + 'code' => $code, + 'redirect_uri' => self::REDIRECT_URI, + ]); + $this->assertSame(200, $test_response->getStatusCode(), (string)$test_response->getBody()); + + $tokens = json_decode((string)$test_response->getBody(), true); + $this->assertIsArray($tokens); + return $tokens; + } + + /** + * Get user data with an access token + * + * @param string $access_token Access token + * + * @return ResponseInterface + */ + private function requestUser(string $access_token): ResponseInterface + { + $request = $this->createRequest(route_name: OAUTH2_PREFIX . '_user'); + $request = $request->withHeader('Authorization', 'Bearer ' . $access_token); + return $this->app->handle($request); + } + + /** + * Test exchanging a code for tokens + * + * @return void + */ + public function testTokenFromAuthorizationCode(): void + { + $member = $this->getAdminMember($this->getMemberOne()); + $tokens = $this->exchangeCode($this->getAuthorizationCode($member->id, ['member'])); + + $this->assertSame('Bearer', $tokens['token_type']); + $this->assertSame(3600, $tokens['expires_in']); + $this->assertNotEmpty($tokens['access_token']); + $this->assertNotEmpty($tokens['refresh_token']); + } + + /** + * Test code exchange requires client secret and same redirect URI + * + * @return void + */ + public function testTokenRequiresClientSecretAndRedirectUri(): void + { + $member = $this->getAdminMember($this->getMemberOne()); + $code = $this->getAuthorizationCode($member->id, ['member']); + + $test_response = $this->requestToken([ + 'grant_type' => 'authorization_code', + 'code' => $code, + 'redirect_uri' => self::REDIRECT_URI, + 'client_secret' => 'wrong-secret', + ]); + $this->assertSame(401, $test_response->getStatusCode()); + $body = json_decode((string)$test_response->getBody(), true); + $this->assertSame('invalid_client', $body['error']); + + $test_response = $this->requestToken([ + 'grant_type' => 'authorization_code', + 'code' => $code, + 'redirect_uri' => 'http://127.0.0.1/callback', + ]); + $this->assertSame(400, $test_response->getStatusCode()); + $body = json_decode((string)$test_response->getBody(), true); + $this->assertSame('invalid_request', $body['error']); + } + + /** + * Test refreshing tokens + * + * @return void + */ + public function testRefreshToken(): void + { + $member = $this->getAdminMember($this->getMemberOne()); + $tokens = $this->exchangeCode($this->getAuthorizationCode($member->id, ['member', 'member:due_date'])); + + $test_response = $this->requestToken([ + 'grant_type' => 'refresh_token', + 'refresh_token' => $tokens['refresh_token'], + ]); + $this->assertSame(200, $test_response->getStatusCode(), (string)$test_response->getBody()); + $refreshed = json_decode((string)$test_response->getBody(), true); + $this->assertNotSame($tokens['access_token'], $refreshed['access_token']); + $this->assertNotSame($tokens['refresh_token'], $refreshed['refresh_token']); + + //refreshed token keeps scopes + $test_response = $this->requestUser($refreshed['access_token']); + $this->assertSame(200, $test_response->getStatusCode()); + $data = json_decode((string)$test_response->getBody(), true); + $this->assertArrayHasKey('due_date', $data); + } + + /** + * Test user data follow granted scopes + * + * @return void + */ + public function testUserData(): void + { + $member = $this->getAdminMember($this->getMemberOne()); + $data = $this->dataAdherentOne(); + + $tokens = $this->exchangeCode($this->getAuthorizationCode($member->id, ['member'])); + $test_response = $this->requestUser($tokens['access_token']); + $this->assertSame(200, $test_response->getStatusCode()); + $this->assertSame('application/json', $test_response->getHeaderLine('Content-Type')); + $user = json_decode((string)$test_response->getBody(), true); + + $this->assertSame($member->id, $user['id']); + $this->assertSame($member->id, $user['sub']); + $this->assertSame($data['login_adh'], $user['username']); + $this->assertSame($data['email_adh'], $user['email']); + foreach (['birthdate', 'address', 'phone', 'socials', 'groups', 'due_date'] as $key) { + $this->assertArrayNotHasKey($key, $user); + } + + $tokens = $this->exchangeCode( + $this->getAuthorizationCode( + $member->id, + ['member', 'member:personal', 'member:localization', 'member:groups', 'member:due_date'] + ) + ); + $test_response = $this->requestUser($tokens['access_token']); + $this->assertSame(200, $test_response->getStatusCode()); + $user = json_decode((string)$test_response->getBody(), true); + + $this->assertArrayHasKey('birthdate', $user); + $this->assertArrayHasKey('address', $user); + $this->assertArrayNotHasKey('street_address', $user['address']); + $this->assertContains('admin', $user['groups']); + $this->assertArrayHasKey('due_date', $user); + } + + /** + * Test user data are refused when member is no longer authorized + * + * @return void + */ + public function testUserDataRequiresAuthorization(): void + { + //galette_cli requires a team member, member one is not admin + $member = $this->getMemberOne(); + + $tokens = $this->exchangeCode($this->getAuthorizationCode($member->id, ['member'])); + $test_response = $this->requestUser($tokens['access_token']); + + $this->assertSame(401, $test_response->getStatusCode()); + $this->assertSame('application/json', $test_response->getHeaderLine('Content-Type')); + $body = json_decode((string)$test_response->getBody(), true); + $this->assertSame( + "Sorry, you can't login because your are not a team member.", + $body['message'] + ); + $this->expectLogEntry( + \Analog\Analog::ERROR, + "api/user() error : Sorry, you can't login because your are not a team member." + ); + } +} diff --git a/tests/GaletteOAuth2/Controllers/tests/units/AuthorizationController.php b/tests/GaletteOAuth2/Controllers/tests/units/AuthorizationController.php index c1948a1..7f86bed 100644 --- a/tests/GaletteOAuth2/Controllers/tests/units/AuthorizationController.php +++ b/tests/GaletteOAuth2/Controllers/tests/units/AuthorizationController.php @@ -105,6 +105,58 @@ public function testAuthorize(): void ); } + /** + * Test authorization for an unknown client shows an error + * + * @return void + */ + public function testAuthorizeUnknownClient(): void + { + $this->logUserIn(); + + $params = $this->getAuthorizeParams('http://flarum.localhost/auth/passport'); + $params['client_id'] = 'galette_unknown'; + $request = $this->createRequest( + route_name: OAUTH2_PREFIX . '_authorize', + query_params: $params + ); + $test_response = $this->app->handle($request); + + $this->assertSame(302, $test_response->getStatusCode()); + $this->assertStringContainsString( + OAUTH2_PREFIX . '/error', + $test_response->getHeaderLine('Location') + ); + $this->expectLogEntry( + \Analog\Analog::WARNING, + 'OAuth2: Invalid or missing client_id "galette_unknown" in authorization request' + ); + } + + /** + * Test authorization asks to log in, then comes back to the same request + * + * @return void + */ + public function testAuthorizeRequiresLogin(): void + { + $params = $this->getAuthorizeParams('http://flarum.localhost/auth/passport'); + $request = $this->createRequest( + route_name: OAUTH2_PREFIX . '_authorize', + query_params: $params + ); + $test_response = $this->app->handle($request); + + $this->assertSame(302, $test_response->getStatusCode()); + $location = $test_response->getHeaderLine('Location'); + $this->assertStringStartsWith($this->routeparser->urlFor(OAUTH2_PREFIX . '_login') . '?', $location); + parse_str((string)parse_url($location, PHP_URL_QUERY), $args); + $this->assertSame( + $this->routeparser->urlFor(OAUTH2_PREFIX . '_authorize', [], $params), + $args['redirect_url'] + ); + } + /** * Test a login checked for a client cannot be used for another one * From d7be273f57a6b4ff643a636e613fe337b8187269 Mon Sep 17 00:00:00 2001 From: Johan Cwiklinski Date: Sat, 26 Sep 2026 23:27:15 +0200 Subject: [PATCH 4/4] Send authorization refusal back to the client application --- .../Controllers/AuthorizationController.php | 11 ++------ .../tests/units/AuthorizationController.php | 26 +++++++++++++++++++ 2 files changed, 28 insertions(+), 9 deletions(-) diff --git a/lib/GaletteOAuth2/Controllers/AuthorizationController.php b/lib/GaletteOAuth2/Controllers/AuthorizationController.php index c73c3ab..c07e789 100755 --- a/lib/GaletteOAuth2/Controllers/AuthorizationController.php +++ b/lib/GaletteOAuth2/Controllers/AuthorizationController.php @@ -165,15 +165,8 @@ public function doAuthorize(Request $request, Response $response): Response|Resp } $authRequest->setScopes($req_scopes); } else { - $authRequest->setAuthorizationApproved(true); - $authRequest->setScopes([]); - - throw OAuthServerException::accessDenied( - sprintf( - _T('Default scope (%s) has not been authorized.', 'oauth2'), - 'member' - ) - ); + //refused: client will be redirected with an access_denied error + $authRequest->setAuthorizationApproved(false); } // Return the HTTP redirect response diff --git a/tests/GaletteOAuth2/Controllers/tests/units/AuthorizationController.php b/tests/GaletteOAuth2/Controllers/tests/units/AuthorizationController.php index 7f86bed..c2737fb 100644 --- a/tests/GaletteOAuth2/Controllers/tests/units/AuthorizationController.php +++ b/tests/GaletteOAuth2/Controllers/tests/units/AuthorizationController.php @@ -157,6 +157,32 @@ public function testAuthorizeRequiresLogin(): void ); } + /** + * Test refusing authorization goes back to the client with an error + * + * @return void + */ + public function testDoAuthorizeRefused(): void + { + $this->logUserIn(); + + $request = $this->createRequest( + route_name: OAUTH2_PREFIX . '_doAuthorize', + method: 'POST', + query_params: $this->getAuthorizeParams('http://flarum.localhost/auth/passport') + ); + $request = $request->withParsedBody(['refuse' => '']); + $test_response = $this->app->handle($request); + + $this->assertSame(302, $test_response->getStatusCode()); + $location = $test_response->getHeaderLine('Location'); + $this->assertStringStartsWith('http://flarum.localhost/auth/passport?', $location); + parse_str((string)parse_url($location, PHP_URL_QUERY), $args); + $this->assertSame('access_denied', $args['error']); + $this->assertSame('7d627422092a7a5ac413ac597312b9b4', $args['state']); + $this->assertArrayNotHasKey('code', $args); + } + /** * Test a login checked for a client cannot be used for another one *