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 4a310fd commit 46dc99c
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 11 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 @@ -29,6 +30,7 @@
use OneLogin\Saml2\Settings;

class GetMetadata extends Command {
use TXmlHelper;

/** @var SAMLSettings */
private $SAMLSettings;
Expand Down Expand Up @@ -70,7 +72,9 @@ protected function execute(InputInterface $input, OutputInterface $output) {
$idp = $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
30 changes: 20 additions & 10 deletions lib/Controller/SAMLController.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
use OC\Core\Controller\ClientFlowLoginController;
use OC\Core\Controller\ClientFlowLoginV2Controller;
use OCA\User_SAML\Exceptions\NoUserFoundException;
use OCA\User_SAML\Helper\TXmlHelper;
use OCA\User_SAML\SAMLSettings;
use OCA\User_SAML\UserBackend;
use OCA\User_SAML\UserData;
Expand All @@ -48,6 +49,8 @@
use OneLogin\Saml2\ValidationError;

class SAMLController extends Controller {
use TXmlHelper;

/** @var ISession */
private $session;
/** @var IUserSession */
Expand Down Expand Up @@ -246,7 +249,9 @@ public function login($idp) {
public function getMetadata($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)) {
return new Http\DataDownloadResponse($metadata, 'metadata.xml', 'text/xml');
} else {
Expand Down Expand Up @@ -308,7 +313,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 @@ -417,14 +425,16 @@ public function singleLogoutService() {
$auth = new Auth($this->SAMLSettings->getOneLoginSettingsArray($idp));
$stay = true ; // $auth will return the redirect URL but won't perform the redirect himself
if($isFromIDP){
$keepLocalSession = true ; // do not let processSLO to delete the entire session. Let userSession->logout do the job
$targetUrl = $auth->processSLO(
$keepLocalSession,
null,
$this->SAMLSettings->usesSloWebServerDecode(),
null,
$stay
);
// 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
);
});
} else {
// If request is not from IDP, we must send him the logout request
$parameters = array();
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 46dc99c

Please sign in to comment.