Skip to content

Commit

Permalink
fix(saml): enable xml entity loader where necessary
Browse files Browse the repository at this point in the history
Signed-off-by: Arthur Schiwon <[email protected]>
  • Loading branch information
blizzz committed Jul 21, 2023
1 parent 5791824 commit 5e57c20
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 10 deletions.
6 changes: 5 additions & 1 deletion lib/Command/GetMetadata.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -30,6 +31,7 @@
use OneLogin\Saml2\Settings;

class GetMetadata extends Command {
use TXmlHelper;

/** @var SAMLSettings */
private $SAMLSettings;
Expand Down Expand Up @@ -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 {
Expand Down
29 changes: 20 additions & 9 deletions lib/Controller/SAMLController.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -47,6 +48,8 @@
use OneLogin\Saml2\ValidationError;

class SAMLController extends Controller {
use TXmlHelper;

/** @var ISession */
private $session;
/** @var IUserSession */
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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()));

Expand Down Expand Up @@ -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];
}
Expand Down
44 changes: 44 additions & 0 deletions lib/Helper/TXmlHelper.php
Original file line number Diff line number Diff line change
@@ -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;
}
}

0 comments on commit 5e57c20

Please sign in to comment.