diff --git a/.github/workflows/php.yml b/.github/workflows/php.yml index 192a59f9..c3979dd4 100644 --- a/.github/workflows/php.yml +++ b/.github/workflows/php.yml @@ -10,8 +10,8 @@ on: - '.config/phpcs.xml.dist' - '.config/phpunit.xml.dist' - '.github/workflows/php.yml' - - 'composer.json' - - 'composer.lock' + - 'solid/composer.json' + - 'solid/composer.lock' branches: [ main ] types: [ opened, reopened, synchronize ] # This event occurs when there is a push to the repository. @@ -21,8 +21,8 @@ on: - '.config/phpcs.xml.dist' - '.config/phpunit.xml.dist' - '.github/workflows/php.yml' - - 'composer.json' - - 'composer.lock' + - 'solid/composer.json' + - 'solid/composer.lock' # Allow manually triggering the workflow. workflow_dispatch: @@ -57,14 +57,27 @@ jobs: steps: - uses: actions/checkout@v4 - run: >- - composer validate - --check-lock - --no-plugins - --no-scripts - --strict + composer validate + --check-lock + --no-plugins + --no-scripts + --strict working-directory: "solid" # 02.test.php.test-unit.yml php-unittest: + container: + image: ghcr.io/${{ github.repository }}:main-${{ matrix.nextcloud_version }} + env: + COMPOSER_AUTH: '{"github-oauth": {"github.com": "${{ secrets.GITHUB_TOKEN }}"}}' + NEXTCLOUD_PATH: /usr/src/nextcloud/apps + NEXTCLOUD_UPDATE: 1 + XDEBUG_MODE: coverage + volumes: + - /usr/bin/composer:/usr/bin/composer + defaults: + run: + shell: bash + working-directory: /usr/src/nextcloud/apps/solid/ name: PHP Unit Tests needs: - lint-php-syntax @@ -78,27 +91,24 @@ jobs: - 29 - 30 steps: - - uses: actions/checkout@v4 - - uses: shivammathur/setup-php@v2 - with: - ini-values: error_reporting=E_ALL, display_errors=On - php-version: 8.3 + - uses: actions/checkout@v5 + # @CHECKME: cp site.conf /etc/apache2/sites-enabled/000-default.conf (?) + - name: Setup Test Environment + run: | + git config --global --add safe.directory "${NEXTCLOUD_PATH}" + /entrypoint.sh "echo" + bash "${GITHUB_WORKSPACE}/init.sh" + rm -r "${NEXTCLOUD_PATH}/solid/" + cp --archive --verbose "${GITHUB_WORKSPACE}/." "${NEXTCLOUD_PATH}" + working-directory: /usr/src/nextcloud/ - name: Install and Cache Composer dependencies - uses: "ramsey/composer-install@v2" + uses: ramsey/composer-install@v3 with: - working-directory: "solid" + working-directory: /usr/src/nextcloud/apps/solid env: COMPOSER_AUTH: '{"github-oauth": {"github.com": "${{ secrets.GITHUB_TOKEN }}"}}' - - run: | - docker run \ - --env 'XDEBUG_MODE=coverage' \ - --rm \ - --volume="./solid:/var/www/html/apps/solid" \ - ghcr.io/${{ github.repository }}:main-${{ matrix.nextcloud_version }} \ - bash -c 'NEXTCLOUD_UPDATE=1 /entrypoint.sh "echo" \ - && sudo -u www-data bash /init.sh \ - && cd /var/www/html/apps/solid \ - && bin/phpunit --configuration phpunit.xml' + - name: Run PHPUnit + run: bin/phpunit --configuration phpunit.xml # 03.quality.php.scan.dependencies-vulnerabilities.yml scan-dependencies-vulnerabilities: @@ -109,12 +119,12 @@ jobs: steps: - uses: actions/checkout@v4 - run: >- - composer audit - --abandoned=report - --locked - --no-dev - --no-plugins - --no-scripts + composer audit + --abandoned=report + --locked + --no-dev + --no-plugins + --no-scripts working-directory: "solid" # 03.quality.php.lint-quality.yml php-lint-quality: diff --git a/solid/lib/Controller/ServerController.php b/solid/lib/Controller/ServerController.php index b4dc255b..2b558cd1 100644 --- a/solid/lib/Controller/ServerController.php +++ b/solid/lib/Controller/ServerController.php @@ -156,6 +156,10 @@ public function authorize() { // return $result->addHeader('Access-Control-Allow-Origin', '*'); } + if (! isset($_GET['client_id'])) { + return new JSONResponse('Bad request, missing client_id', 400); + } + if (isset($_GET['request'])) { $jwtConfig = Configuration::forSymmetricSigner(new Sha256(), InMemory::plainText($this->config->getPrivateKey())); try { @@ -323,7 +327,9 @@ public function session() { */ public function token() { $request = \Laminas\Diactoros\ServerRequestFactory::fromGlobals($_SERVER, $_GET, $_POST, $_COOKIE, $_FILES); - $grantType = $request->getParsedBody()['grant_type']; + + $grantType = $request->getParsedBody()['grant_type'] ?? null; + switch ($grantType) { case "authorization_code": $code = $request->getParsedBody()['code']; @@ -342,9 +348,9 @@ public function token() { break; } - $clientId = $request->getParsedBody()['client_id']; + $clientId = $request->getParsedBody()['client_id'] ?? null; - $httpDpop = $request->getServerParams()['HTTP_DPOP']; + $httpDpop = $request->getServerParams()['HTTP_DPOP'] ?? null; $response = new \Laminas\Diactoros\Response(); $server = new \Pdsinterop\Solid\Auth\Server($this->authServerFactory, $this->authServerConfig, $response); @@ -361,7 +367,7 @@ public function token() { ); } - return $this->respond($response); // ->addHeader('Access-Control-Allow-Origin', '*'); + return $this->respond($response); } /** @@ -389,8 +395,13 @@ public function logout() { * @NoCSRFRequired */ public function register() { - $clientData = file_get_contents('php://input'); - $clientData = json_decode($clientData, true); + $postData = file_get_contents('php://input'); + $clientData = json_decode($postData, true); + + if (! isset($clientData)) { + return new JSONResponse("Missing client data", Http::STATUS_BAD_REQUEST); + } + if (! isset($clientData['redirect_uris'])) { return new JSONResponse("Missing redirect URIs", Http::STATUS_BAD_REQUEST); } diff --git a/solid/tests/Unit/Controller/ServerControllerTest.php b/solid/tests/Unit/Controller/ServerControllerTest.php index 2920b0dd..32699b40 100644 --- a/solid/tests/Unit/Controller/ServerControllerTest.php +++ b/solid/tests/Unit/Controller/ServerControllerTest.php @@ -119,6 +119,25 @@ public function testAuthorizeWithoutUser() $this->assertEquals($expected, $actual); } + /** + * @testdox ServerController should return a 400 when asked to authorize with a user but without client_id + * + * @covers ::authorize + */ + public function testAuthorizeWithoutClientId() + { + $parameters = $this->createMockConstructorParameters(); + + $this->mockUserManager->method('userExists')->willReturn(true); + + $controller = new ServerController(...array_values($parameters)); + + $actual = $controller->authorize(); + $expected = new JSONResponse('Bad request, missing client_id', Http::STATUS_BAD_REQUEST); + + $this->assertEquals($expected, $actual); + } + /** * @testdox ServerController should return a 400 when asked to authorize with a user but without valid token * @@ -126,6 +145,7 @@ public function testAuthorizeWithoutUser() */ public function testAuthorizeWithoutValidToken() { + $_GET['client_id'] = self::MOCK_CLIENT_ID; $_GET['response_type'] = 'mock-response-type'; $parameters = $this->createMockConstructorParameters(); @@ -193,17 +213,8 @@ public function testAuthorizeWithInvalidRedirectUri() $response = $controller->authorize(); - $expected = [ - 'data' => vsprintf($controller::ERROR_UNREGISTERED_URI, [$_GET['redirect_uri']]), - 'headers' => [ - 'Cache-Control' => 'no-cache, no-store, must-revalidate', - 'Content-Security-Policy' => "default-src 'none';base-uri 'none';manifest-src 'self';frame-ancestors 'none'", - 'Content-Type' => 'application/json; charset=utf-8', - 'Feature-Policy' => "autoplay 'none';camera 'none';fullscreen 'none';geolocation 'none';microphone 'none';payment 'none'", - 'X-Robots-Tag' => 'noindex, nofollow', - ], - 'status' => Http::STATUS_BAD_REQUEST, - ]; + $data = vsprintf($controller::ERROR_UNREGISTERED_URI, [$_GET['redirect_uri']]); + $expected = $this->createExpectedResponse(Http::STATUS_BAD_REQUEST, $data); $actual = [ 'data' => $response->getData(), @@ -248,17 +259,7 @@ public function testAuthorize() $response = $controller->authorize(); - $expected = [ - 'data' => 'ok', - 'headers' => [ - 'Cache-Control' => 'no-cache, no-store, must-revalidate', - 'Content-Security-Policy' => "default-src 'none';base-uri 'none';manifest-src 'self';frame-ancestors 'none'", - 'Content-Type' => 'application/json; charset=utf-8', - 'Feature-Policy' => "autoplay 'none';camera 'none';fullscreen 'none';geolocation 'none';microphone 'none';payment 'none'", - 'X-Robots-Tag' => 'noindex, nofollow', - ], - 'status' => Http::STATUS_FOUND, - ]; + $expected = $this->createExpectedResponse(); $actual = [ 'data' => $response->getData(), @@ -296,12 +297,33 @@ public function testAuthorize() * * @covers ::register */ + public function testRegisterWithoutClientData() + { + $parameters = $this->createMockConstructorParameters(); + + $controller = new ServerController(...array_values($parameters)); + + $actual = $controller->register(); + + $this->assertEquals( + new JSONResponse('Missing client data', Http::STATUS_BAD_REQUEST), + $actual + ); + } + + /** + * @testdox ServerController should return a 400 when asked to register without redirect URIs + * + * @covers ::register + */ public function testRegisterWithoutRedirectUris() { $parameters = $this->createMockConstructorParameters(); $controller = new ServerController(...$parameters); + self::$clientData = json_encode([]); + $actual = $controller->register(); $this->assertEquals( @@ -328,6 +350,20 @@ public function testRegisterWithRedirectUris() $response = $controller->register(); + $data = [ + 'application_type' => 'web', + 'client_id' => 'f4a2d00f7602948a97ff409d7a581ec2', + 'client_secret' => '3b5798fddd49e23662ee6fe801085100', + 'grant_types' => ['implicit'], + 'id_token_signed_response_alg' => 'RS256', + 'redirect_uris' => ['https://mock.client/redirect'], + 'registration_access_token' => 'eyJ0eXAiOiJKV1QiLCJhbGciOiJSUzI1NiJ9.eyJpc3MiOiJodHRwczovL21vY2suc2VydmVyIiwiYXVkIjoiZjRhMmQwMGY3NjAyOTQ4YTk3ZmY0MDlkN2E1ODFlYzIiLCJzdWIiOiJmNGEyZDAwZjc2MDI5NDhhOTdmZjQwOWQ3YTU4MWVjMiJ9.AfOi9YW70rL0EKn4_dvhkyu02iI4yGYV-Xh8hQ9RbHBUnvcXROFfQzn-OL-R3kV3nn8tknmpG-r_8Ouoo7O_Sjo8Hx1QSFfeqjJGOgB8HbXV7WN2spOMicSB-68EyftqfTGH0ksyPyJaNSTbkdIqtawsDaSKUVqTmziEo4IrE5anwDLZrtSUcS0A4KVrOAkJmgYGiC4MC0NMYXeBRxgkr1_h7GN4hekAXs9-5XwRH1mwswUVRL-6prx0IYpPNURFNqkS2NU83xNf-vONThOdLVkADVy-l3PCHT3E1sRdkklCHLjhWiZo7NcMlB0WdS-APnZYCi5hLEr5-jwNI2sxoA', + 'registration_client_uri' => '', + 'response_types' => ['id_token token'], + 'token_endpoint_auth_method' => 'client_secret_basic', + ]; + $expected = $this->createExpectedResponse(Http::STATUS_OK, $data); + $actual = [ 'data' => $response->getData(), 'headers' => $response->getHeaders(), @@ -337,27 +373,27 @@ public function testRegisterWithRedirectUris() // Not comparing time-sensitive data unset($actual['data']['client_id_issued_at'], $actual['headers']['X-Request-Id']); - $this->assertEquals([ - 'data' => [ - 'application_type' => 'web', - 'client_id' => 'f4a2d00f7602948a97ff409d7a581ec2', - 'grant_types' => ['implicit'], - 'id_token_signed_response_alg' => 'RS256', - 'redirect_uris' => ['https://mock.client/redirect'], - 'registration_access_token' => 'eyJ0eXAiOiJKV1QiLCJhbGciOiJSUzI1NiJ9.eyJpc3MiOiJodHRwczovL21vY2suc2VydmVyIiwiYXVkIjoiZjRhMmQwMGY3NjAyOTQ4YTk3ZmY0MDlkN2E1ODFlYzIiLCJzdWIiOiJmNGEyZDAwZjc2MDI5NDhhOTdmZjQwOWQ3YTU4MWVjMiJ9.AfOi9YW70rL0EKn4_dvhkyu02iI4yGYV-Xh8hQ9RbHBUnvcXROFfQzn-OL-R3kV3nn8tknmpG-r_8Ouoo7O_Sjo8Hx1QSFfeqjJGOgB8HbXV7WN2spOMicSB-68EyftqfTGH0ksyPyJaNSTbkdIqtawsDaSKUVqTmziEo4IrE5anwDLZrtSUcS0A4KVrOAkJmgYGiC4MC0NMYXeBRxgkr1_h7GN4hekAXs9-5XwRH1mwswUVRL-6prx0IYpPNURFNqkS2NU83xNf-vONThOdLVkADVy-l3PCHT3E1sRdkklCHLjhWiZo7NcMlB0WdS-APnZYCi5hLEr5-jwNI2sxoA', - 'registration_client_uri' => '', - 'response_types' => ['id_token token'], - 'token_endpoint_auth_method' => 'client_secret_basic', - ], - 'headers' => [ - 'Cache-Control' => 'no-cache, no-store, must-revalidate', - 'Content-Security-Policy' => "default-src 'none';base-uri 'none';manifest-src 'self';frame-ancestors 'none'", - 'Feature-Policy' => "autoplay 'none';camera 'none';fullscreen 'none';geolocation 'none';microphone 'none';payment 'none'", - 'X-Robots-Tag' => 'noindex, nofollow', - 'Content-Type' => 'application/json; charset=utf-8', - ], - 'status' => Http::STATUS_OK, - ], $actual); + $this->assertEquals($expected, $actual); + } + + public function testTokenWithoutPostBody() + { + $parameters = $this->createMockConstructorParameters(); + + $controller = new ServerController(...$parameters); + + $tokenResponse = $controller->token(); + + $expected = $this->createExpectedResponse(Http::STATUS_BAD_REQUEST, 'Bad Request'); + + $actual = [ + 'data' => $tokenResponse->getData(), + 'headers' => $tokenResponse->getHeaders(), + 'status' => $tokenResponse->getStatus(), + ]; + unset($actual['headers']['X-Request-Id']); + + $this->assertEquals($expected, $actual); } /** @@ -369,6 +405,7 @@ public function testToken() { $_POST['client_id'] = self::MOCK_CLIENT_ID; $_POST['code'] = ''; + $_POST['grant_type'] = 'authorization_code'; $_SERVER['HTTP_DPOP'] = 'mock dpop'; $_SESSION['nonce'] = 'mock nonce'; @@ -400,17 +437,7 @@ public function testToken() $tokenResponse = $controller->token(); - $expected = [ - 'data' => "I'm a teapot", - 'headers' => [ - 'Cache-Control' => 'no-cache, no-store, must-revalidate', - 'Content-Security-Policy' => "default-src 'none';base-uri 'none';manifest-src 'self';frame-ancestors 'none'", - 'Feature-Policy' => "autoplay 'none';camera 'none';fullscreen 'none';geolocation 'none';microphone 'none';payment 'none'", - 'X-Robots-Tag' => 'noindex, nofollow', - 'Content-Type' => 'application/json; charset=utf-8', - ], - 'status' => Http::STATUS_IM_A_TEAPOT, - ]; + $expected = $this->createExpectedResponse(Http::STATUS_IM_A_TEAPOT, "I'm a teapot"); $actual = [ 'data' => $tokenResponse->getData(), @@ -474,6 +501,25 @@ public function createMockUserManager(): IUserManager|MockObject return $this->mockUserManager; } + public function createExpectedResponse($status = Http::STATUS_FOUND, $data = 'ok', $headers = []): array + { + if (empty($headers)) { + $headers = [ + 'Cache-Control' => 'no-cache, no-store, must-revalidate', + 'Content-Security-Policy' => "default-src 'none';base-uri 'none';manifest-src 'self';frame-ancestors 'none'", + 'Content-Type' => 'application/json; charset=utf-8', + 'Feature-Policy' => "autoplay 'none';camera 'none';fullscreen 'none';geolocation 'none';microphone 'none';payment 'none'", + 'X-Robots-Tag' => 'noindex, nofollow', + ]; + } + + return [ + 'data' => $data, + 'headers' => $headers, + 'status' => $status, + ]; + } + /////////////////////////////// DATAPROVIDERS \\\\\\\\\\\\\\\\\\\\\\\\\\\\\\ public static function provideConstructorParameterIndex()