Small code improvements

.github/workflows/php.yml

@@ -65,6 +65,14 @@ jobs:
         working-directory: "solid"
   # 02.test.php.test-unit.yml
   php-unittest:
+    container:
+      image: ghcr.io/${{ github.repository }}:main-${{ matrix.nextcloud_version }}
+      env:
+        NEXTCLOUD_PATH: /usr/src/nextcloud/apps
+        NEXTCLOUD_UPDATE: 1
+        XDEBUG_MODE: coverage
+      volumes:
+        - /usr/bin/composer:/usr/bin/composer
     name: PHP Unit Tests
     needs:
       - lint-php-syntax
@@ -78,27 +86,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
+      - 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}/"
       - 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: |
+          "${NEXTCLOUD_PATH}/solid/bin/phpunit" \
+          --configuration "${NEXTCLOUD_PATH}/solid/phpunit.xml"
 
   # 03.quality.php.scan.dependencies-vulnerabilities.yml
   scan-dependencies-vulnerabilities:

solid/lib/Controller/ServerController.php

@@ -3,6 +3,7 @@
 
 use OCA\Solid\DpopFactoryTrait;
 use OCA\Solid\ServerConfig;
+use OCA\Solid\Service\UserService;
 
 use OCP\AppFramework\Controller;
 use OCP\AppFramework\Http;
@@ -47,6 +48,8 @@ class ServerController extends Controller
 	/* @var \Pdsinterop\Solid\Auth\TokenGenerator */
 	private $tokenGenerator;
 
+	private UserService $userService;
+
 	public function __construct(
 		$AppName,
 		IRequest $request,
@@ -55,7 +58,7 @@ public function __construct(
 		IURLGenerator $urlGenerator,
 		$userId,
 		IConfig $config,
-		\OCA\Solid\Service\UserService $UserService,
+		\OCA\Solid\Service\UserService $userService,
 		IDBConnection $connection,
 	) {
 		parent::__construct($AppName, $request);
@@ -66,6 +69,7 @@ public function __construct(
 		$this->request     = $request;
 		$this->urlGenerator = $urlGenerator;
 		$this->session = $session;
+		$this->userService = $userService;
 
 		$this->setJtiStorage($connection);
 
@@ -150,12 +154,15 @@ public function cors($path) {
 	public function authorize() {
 		// Create a request
 		if (!$this->userManager->userExists($this->userId)) {
-			$result = new JSONResponse('Authorization required');
-			$result->setStatus(401);
-			return $result;
-//			return $result->addHeader('Access-Control-Allow-Origin', '*');
+			return new JSONResponse('Authorization required', 401);
+		}
+
+		if (! isset($_GET['client_id'])) {
+			return new JSONResponse('Bad request, missing client_id', 400);
 		}
+		$clientId = $_GET['client_id'];
 
+		$getVars = $_GET;
 		if (isset($_GET['request'])) {
 			$jwtConfig = Configuration::forSymmetricSigner(new Sha256(), InMemory::plainText($this->config->getPrivateKey()));
 			try {
@@ -164,51 +171,51 @@ public function authorize() {
 			} catch(\Exception $e) {
 				$this->session->set("nonce", $_GET['nonce']);
 			}
-		}
 
-		$getVars = $_GET;
-		if (!isset($getVars['grant_type'])) {
-			$getVars['grant_type'] = 'implicit';
-		}
-		$getVars['response_type'] = $this->getResponseType();
-		$getVars['scope'] = "openid" ;
-
-		if (!isset($getVars['redirect_uri'])) {
-			if (!isset($token)) {
-				$result = new JSONResponse('Bad request, does not contain valid token');
-				$result->setStatus(400);
-				return $result;
-//				return $result->addHeader('Access-Control-Allow-Origin', '*');
+			if (!isset($getVars['grant_type'])) {
+				$getVars['grant_type'] = 'implicit';
 			}
-			try {
-				$getVars['redirect_uri'] = $token->claims()->get("redirect_uri");
-			} catch(\Exception $e) {
-				$result = new JSONResponse('Bad request, missing redirect uri');
-				$result->setStatus(400);
-				return $result;
-//				return $result->addHeader('Access-Control-Allow-Origin', '*');
+			$getVars['response_type'] = $this->getResponseType();
+			$getVars['scope'] = "openid";
+
+			if (!isset($getVars['redirect_uri'])) {
+				if (!isset($token)) {
+					return new JSONResponse('Bad request, does not contain valid token', 400);
+				}
+
+				try {
+					$getVars['redirect_uri'] = $token->claims()->get("redirect_uri");
+				} catch(\Exception $e) {
+					return new JSONResponse('Bad request, missing redirect uri', 400);
+				}
 			}
 		}
 
-		if (preg_match("/^http(s)?:/", $getVars['client_id'])) {
+		$request = \Laminas\Diactoros\ServerRequestFactory::fromGlobals($_SERVER, $getVars, $_POST, $_COOKIE, $_FILES);
+		$response = new \Laminas\Diactoros\Response();
+		$authServer = new \Pdsinterop\Solid\Auth\Server($this->authServerFactory, $this->authServerConfig, $response);
+
+		// @FIXME: Check OIDC Spec for rules regarding Client updates
+		if (preg_match("/^http(s)?:/", $clientId)) {
 			$parsedOrigin = parse_url($getVars['redirect_uri']);
 			$origin = $parsedOrigin['scheme'] . '://' . $parsedOrigin['host'];
 			if (isset($parsedOrigin['port'])) {
 				$origin .= ":" . $parsedOrigin['port'];
 			}
 			$clientData = array(
 				"client_id_issued_at" => time(),
-				"client_name" => $getVars['client_id'],
+				"client_name" => $clientId,
 				"origin" => $origin,
 				"redirect_uris" => array(
 					$getVars['redirect_uri']
 				)
 			);
-			$clientId = $this->config->saveClientRegistration($origin, $clientData)['client_id'];
-			$clientId = $this->config->saveClientRegistration($getVars['client_id'], $clientData)['client_id'];
+
+			$this->config->saveClientRegistration($origin, $clientData);
+			$clientId = $this->config->saveClientRegistration($clientId, $clientData)['client_id'];
+
 			$returnUrl = $getVars['redirect_uri'];
 		} else {
-			$clientId = $getVars['client_id'];
 			$returnUrl = $_SERVER['REQUEST_URI'];
 		}
 
@@ -225,7 +232,8 @@ public function authorize() {
 			$result->setStatus(302);
 			$approvalUrl = $this->urlGenerator->getAbsoluteURL($this->urlGenerator->linkToRoute("solid.page.approval", array("clientId" => $clientId, "returnUrl" => $returnUrl)));
 			$result->addHeader("Location", $approvalUrl);
-			return $result; // ->addHeader('Access-Control-Allow-Origin', '*');
+
+			return $result;
 		}
 
 		if (isset($getVars['redirect_uri'])) {
@@ -260,23 +268,21 @@ public function authorize() {
 			return $result;
 		}
 
+		$webId = $this->getProfilePage();
 		$user = new \Pdsinterop\Solid\Auth\Entity\User();
-		$user->setIdentifier($this->getProfilePage());
+		$user->setIdentifier($webId);
 
-		$request = \Laminas\Diactoros\ServerRequestFactory::fromGlobals($_SERVER, $getVars, $_POST, $_COOKIE, $_FILES);
-		$response = new \Laminas\Diactoros\Response();
-		$server	= new \Pdsinterop\Solid\Auth\Server($this->authServerFactory, $this->authServerConfig, $response);
+		$response = $authServer->respondToAuthorizationRequest($request, $user, $approval);
 
-		$response = $server->respondToAuthorizationRequest($request, $user, $approval);
 		$response = $this->tokenGenerator->addIdTokenToResponse(
 			$response,
 			$clientId,
-			$this->getProfilePage(),
+			$webId,
 			$this->session->get("nonce"),
 			$this->config->getPrivateKey()
 		);
 
-		return $this->respond($response); // ->addHeader('Access-Control-Allow-Origin', '*');
+		return $this->respond($response);
 	}
 
 	private function checkApproval($clientId) {
@@ -389,11 +395,17 @@ 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);
 		}
+
 		$clientData['client_id_issued_at'] = time();
 		$parsedOrigin = parse_url($clientData['redirect_uris'][0]);
 		$origin = $parsedOrigin['scheme'] . '://' . $parsedOrigin['host'];