Merge pull request #756 from nextcloud/backport/754/stable-5.1

[stable-5.1] fix(saml): enable xml entity loader where necessary

.github/workflows/appstore-build-publish.yml

@@ -66,7 +66,7 @@ jobs:
         run: npm i -g npm@"${{ steps.versions.outputs.npmVersion }}"
 
       - name: Set up php ${{ env.PHP_VERSION }}
-        uses: shivammathur/setup-php@v2
+        uses: shivammathur/setup-php@4bd44f22a98a19e0950cbad5f31095157cc9621b
         with:
           php-version: ${{ env.PHP_VERSION }}
           coverage: none

.github/workflows/lint-info-xml.yml

@@ -3,29 +3,37 @@
 # https://github.com/nextcloud/.github
 # https://docs.github.com/en/actions/learn-github-actions/sharing-workflows-with-your-organization
 
-name: Lint
+name: Lint info.xml
 
 on:
   pull_request:
   push:
     branches:
+      - main
       - master
       - stable*
 
+permissions:
+  contents: read
+
+concurrency:
+  group: lint-info-xml-${{ github.head_ref || github.run_id }}
+  cancel-in-progress: true
+
 jobs:
   xml-linters:
     runs-on: ubuntu-latest
 
     name: info.xml lint
     steps:
       - name: Checkout
-        uses: actions/checkout@master
+        uses: actions/checkout@c85c95e3d7251135ab7dc9ce3241c5835cc595a9 # v3.5.3
 
       - name: Download schema
         run: wget https://raw.githubusercontent.com/nextcloud/appstore/master/nextcloudappstore/api/v1/release/info.xsd
 
       - name: Lint info.xml
-        uses: ChristophWurst/xmllint-action@v1
+        uses: ChristophWurst/xmllint-action@39155a91429af431d65fafc21fa52ba5c4f5cb71 # v1.1
         with:
           xml-file: ./appinfo/info.xml
           xml-schema-file: ./info.xsd

.github/workflows/lint-php-cs.yml

@@ -25,7 +25,7 @@ jobs:
         uses: actions/checkout@v3
 
       - name: Set up php
-        uses: shivammathur/setup-php@v2
+        uses: shivammathur/setup-php@4bd44f22a98a19e0950cbad5f31095157cc9621b
         with:
           php-version: "7.4"
           coverage: none

.github/workflows/lint-php.yml

@@ -34,7 +34,7 @@ jobs:
         uses: actions/checkout@v3
 
       - name: Set up php ${{ matrix.php-versions }}
-        uses: shivammathur/setup-php@v2
+        uses: shivammathur/setup-php@4bd44f22a98a19e0950cbad5f31095157cc9621b
         with:
           php-version: ${{ matrix.php-versions }}
           coverage: none

.github/workflows/phpunit-mysql.yml

@@ -79,11 +79,11 @@ jobs:
           path: apps/${{ env.APP_NAME }}
 
       - name: Set up php ${{ matrix.php-versions }}
-        uses: shivammathur/setup-php@v2
+        uses: shivammathur/setup-php@4bd44f22a98a19e0950cbad5f31095157cc9621b
         with:
           php-version: ${{ matrix.php-versions }}
           tools: phpunit
-          extensions: mbstring, iconv, fileinfo, intl, mysql, pdo_mysql
+          extensions: bz2, ctype, curl, dom, fileinfo, gd, iconv, intl, json, libxml, mbstring, openssl, pcntl, posix, session, simplexml, xmlreader, xmlwriter, zip, zlib, mysql, pdo_mysql
           coverage: none
 
       - name: Check composer file existence

.github/workflows/phpunit-oci.yml

@@ -71,10 +71,10 @@ jobs:
           path: apps/${{ env.APP_NAME }}
 
       - name: Set up php ${{ matrix.php-versions }}
-        uses: shivammathur/setup-php@v2
+        uses: shivammathur/setup-php@4bd44f22a98a19e0950cbad5f31095157cc9621b
         with:
           php-version: ${{ matrix.php-versions }}
-          extensions: mbstring, fileinfo, intl, sqlite, pdo_sqlite, oci8
+          extensions: bz2, ctype, curl, dom, fileinfo, gd, iconv, intl, json, libxml, mbstring, openssl, pcntl, posix, session, simplexml, xmlreader, xmlwriter, zip, zlib, oci8
           tools: phpunit
           coverage: none
 

.github/workflows/phpunit-pgsql.yml

@@ -76,11 +76,11 @@ jobs:
           path: apps/${{ env.APP_NAME }}
 
       - name: Set up php ${{ matrix.php-versions }}
-        uses: shivammathur/setup-php@v2
+        uses: shivammathur/setup-php@4bd44f22a98a19e0950cbad5f31095157cc9621b
         with:
           php-version: ${{ matrix.php-versions }}
           tools: phpunit
-          extensions: mbstring, iconv, fileinfo, intl, pgsql, pdo_pgsql
+          extensions: bz2, ctype, curl, dom, fileinfo, gd, iconv, intl, json, libxml, mbstring, openssl, pcntl, posix, session, simplexml, xmlreader, xmlwriter, zip, zlib, pgsql, pdo_pgsql
           coverage: none
 
       - name: Check composer file existence

.github/workflows/phpunit-sqlite.yml

@@ -67,11 +67,11 @@ jobs:
           path: apps/${{ env.APP_NAME }}
 
       - name: Set up php ${{ matrix.php-versions }}
-        uses: shivammathur/setup-php@v2
+        uses: shivammathur/setup-php@4bd44f22a98a19e0950cbad5f31095157cc9621b
         with:
           php-version: ${{ matrix.php-versions }}
           tools: phpunit
-          extensions: mbstring, iconv, fileinfo, intl, sqlite, pdo_sqlite
+          extensions: bz2, ctype, curl, dom, fileinfo, gd, iconv, intl, json, libxml, mbstring, openssl, pcntl, posix, session, simplexml, xmlreader, xmlwriter, zip, zlib, sqlite, pdo_sqlite
           coverage: none
 
       - name: Check composer file existence

lib/Command/GetMetadata.php

@@ -21,6 +21,7 @@
 
 namespace OCA\User_SAML\Command;
 
+use OCA\User_SAML\Helper\TXmlHelper;
 use Symfony\Component\Console\Command\Command;
 use Symfony\Component\Console\Input\InputArgument;
 use Symfony\Component\Console\Input\InputInterface;
@@ -30,6 +31,7 @@
 use OneLogin\Saml2\Settings;
 
 class GetMetadata extends Command {
+	use TXmlHelper;
 
 	/** @var SAMLSettings */
 	private $SAMLSettings;
@@ -71,7 +73,9 @@ protected function execute(InputInterface $input, OutputInterface $output) {
 		$idp = (int)$input->getArgument('idp');
 		$settings = new Settings($this->SAMLSettings->getOneLoginSettingsArray($idp));
 		$metadata = $settings->getSPMetadata();
-		$errors = $settings->validateMetadata($metadata);
+		$errors = $this->callWithXmlEntityLoader(function () use ($settings, $metadata) {
+			return $settings->validateMetadata($metadata);
+		});
 		if (empty($errors)) {
 			$output->writeln($metadata);
 		} else {

lib/Controller/SAMLController.php

@@ -27,6 +27,7 @@
 use OC\Core\Controller\ClientFlowLoginV2Controller;
 use OCA\User_SAML\Exceptions\NoUserFoundException;
 use OCA\User_SAML\Exceptions\UserFilterViolationException;
+use OCA\User_SAML\Helper\TXmlHelper;
 use OCA\User_SAML\SAMLSettings;
 use OCA\User_SAML\UserBackend;
 use OCA\User_SAML\UserData;
@@ -47,6 +48,8 @@
 use OneLogin\Saml2\ValidationError;
 
 class SAMLController extends Controller {
+	use TXmlHelper;
+
 	/** @var ISession */
 	private $session;
 	/** @var IUserSession */
@@ -289,7 +292,9 @@ public function login(int $idp = 1) {
 	public function getMetadata(int $idp = 1) {
 		$settings = new Settings($this->samlSettings->getOneLoginSettingsArray($idp));
 		$metadata = $settings->getSPMetadata();
-		$errors = $settings->validateMetadata($metadata);
+		$errors = $this->callWithXmlEntityLoader(function () use ($settings, $metadata) {
+			return $settings->validateMetadata($metadata);
+		});
 		if (empty($errors)) {
 			return new Http\DataDownloadResponse($metadata, 'metadata.xml', 'text/xml');
 		} else {
@@ -350,7 +355,10 @@ public function assertionConsumerService(): Http\RedirectResponse {
 		}
 
 		$auth = new Auth($this->samlSettings->getOneLoginSettingsArray($idp));
-		$auth->processResponse($AuthNRequestID);
+		// validator (called with processResponse()) needs an XML entity loader
+		$this->callWithXmlEntityLoader(function () use ($auth, $AuthNRequestID): void {
+			$auth->processResponse($AuthNRequestID);
+		});
 
 		$this->logger->debug('Attributes send by the IDP: ' . json_encode($auth->getAttributes()));
 
@@ -510,13 +518,16 @@ private function tryProcessSLOResponse(?int $idp): array {
 		foreach ($idps as $idp) {
 			try {
 				$auth = new Auth($this->samlSettings->getOneLoginSettingsArray($idp));
-				$targetUrl = $auth->processSLO(
-					true, // do not let processSLO to delete the entire session. Let userSession->logout do the job
-					null,
-					$this->samlSettings->usesSloWebServerDecode($idp),
-					null,
-					true
-				);
+				// validator (called with processSLO()) needs an XML entity loader
+				$targetUrl = $this->callWithXmlEntityLoader(function () use ($auth, $idp): string {
+					return $auth->processSLO(
+						true, // do not let processSLO to delete the entire session. Let userSession->logout do the job
+						null,
+						$this->samlSettings->usesSloWebServerDecode($idp),
+						null,
+						true
+					);
+				});
 				if ($auth->getLastErrorReason() === null) {
 					return [$targetUrl, $auth];
 				}

lib/Helper/TXmlHelper.php

@@ -0,0 +1,44 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * @copyright Copyright (c) 2023 Arthur Schiwon <[email protected]>
+ *
+ * @author Arthur Schiwon <[email protected]>
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <https://www.gnu.org/licenses/>.
+ *
+ */
+
+namespace OCA\User_SAML\Helper;
+
+trait TXmlHelper {
+
+	/**
+	 * @returns mixed returns the result of the callable parameter
+	 */
+	public function callWithXmlEntityLoader(callable $func) {
+		libxml_set_external_entity_loader(static function ($public, $system) {
+			return $system;
+		});
+		$result = $func();
+		libxml_set_external_entity_loader(static function () {
+			return null;
+		});
+		return $result;
+	}
+}