Skip to content

Commit c835266

Browse files
committed
fix(saml): enable xml entity loader where necessary
Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
1 parent 5f1bd19 commit c835266

3 files changed

Lines changed: 69 additions & 10 deletions

File tree

lib/Command/GetMetadata.php

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121

2222
namespace OCA\User_SAML\Command;
2323

24+
use OCA\User_SAML\Helper\TXmlHelper;
2425
use OCA\User_SAML\SAMLSettings;
2526
use OneLogin\Saml2\Error;
2627
use OneLogin\Saml2\Settings;
@@ -30,6 +31,7 @@
3031
use Symfony\Component\Console\Output\OutputInterface;
3132

3233
class GetMetadata extends Command {
34+
use TXmlHelper;
3335

3436
/** @var SAMLSettings */
3537
private $SAMLSettings;
@@ -71,7 +73,9 @@ protected function execute(InputInterface $input, OutputInterface $output) {
7173
$idp = (int)$input->getArgument('idp');
7274
$settings = new Settings($this->SAMLSettings->getOneLoginSettingsArray($idp));
7375
$metadata = $settings->getSPMetadata();
74-
$errors = $settings->validateMetadata($metadata);
76+
$errors = $this->callWithXmlEntityLoader(function () use ($settings, $metadata) {
77+
return $settings->validateMetadata($metadata);
78+
});
7579
if (empty($errors)) {
7680
$output->writeln($metadata);
7781
} else {

lib/Controller/SAMLController.php

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
use OC\Core\Controller\ClientFlowLoginV2Controller;
2929
use OCA\User_SAML\Exceptions\NoUserFoundException;
3030
use OCA\User_SAML\Exceptions\UserFilterViolationException;
31+
use OCA\User_SAML\Helper\TXmlHelper;
3132
use OCA\User_SAML\SAMLSettings;
3233
use OCA\User_SAML\UserBackend;
3334
use OCA\User_SAML\UserData;
@@ -48,6 +49,8 @@
4849
use OneLogin\Saml2\ValidationError;
4950

5051
class SAMLController extends Controller {
52+
use TXmlHelper;
53+
5154
/** @var ISession */
5255
private $session;
5356
/** @var IUserSession */
@@ -290,7 +293,9 @@ public function login(int $idp = 1) {
290293
public function getMetadata(int $idp = 1) {
291294
$settings = new Settings($this->samlSettings->getOneLoginSettingsArray($idp));
292295
$metadata = $settings->getSPMetadata();
293-
$errors = $settings->validateMetadata($metadata);
296+
$errors = $this->callWithXmlEntityLoader(function () use ($settings, $metadata) {
297+
return $settings->validateMetadata($metadata);
298+
});
294299
if (empty($errors)) {
295300
return new Http\DataDownloadResponse($metadata, 'metadata.xml', 'text/xml');
296301
} else {
@@ -351,7 +356,10 @@ public function assertionConsumerService(): Http\RedirectResponse {
351356
}
352357

353358
$auth = new Auth($this->samlSettings->getOneLoginSettingsArray($idp));
354-
$auth->processResponse($AuthNRequestID);
359+
// validator (called with processResponse()) needs an XML entity loader
360+
$this->callWithXmlEntityLoader(function () use ($auth, $AuthNRequestID): void {
361+
$auth->processResponse($AuthNRequestID);
362+
});
355363

356364
$this->logger->debug('Attributes send by the IDP: ' . json_encode($auth->getAttributes()));
357365

@@ -518,13 +526,16 @@ private function tryProcessSLOResponse(?int $idp): array {
518526
foreach ($idps as $idp) {
519527
try {
520528
$auth = new Auth($this->samlSettings->getOneLoginSettingsArray($idp));
521-
$targetUrl = $auth->processSLO(
522-
true, // do not let processSLO to delete the entire session. Let userSession->logout do the job
523-
null,
524-
$this->samlSettings->usesSloWebServerDecode($idp),
525-
null,
526-
true
527-
);
529+
// validator (called with processSLO()) needs an XML entity loader
530+
$targetUrl = $this->callWithXmlEntityLoader(function () use ($auth, $idp): string {
531+
return $auth->processSLO(
532+
true, // do not let processSLO to delete the entire session. Let userSession->logout do the job
533+
null,
534+
$this->samlSettings->usesSloWebServerDecode($idp),
535+
null,
536+
true
537+
);
538+
});
528539
if ($auth->getLastErrorReason() === null) {
529540
return [$targetUrl, $auth];
530541
}

lib/Helper/TXmlHelper.php

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* @copyright Copyright (c) 2023 Arthur Schiwon <blizzz@arthur-schiwon.de>
7+
*
8+
* @author Arthur Schiwon <blizzz@arthur-schiwon.de>
9+
*
10+
* @license GNU AGPL version 3 or any later version
11+
*
12+
* This program is free software: you can redistribute it and/or modify
13+
* it under the terms of the GNU Affero General Public License as
14+
* published by the Free Software Foundation, either version 3 of the
15+
* License, or (at your option) any later version.
16+
*
17+
* This program is distributed in the hope that it will be useful,
18+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
19+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
20+
* GNU Affero General Public License for more details.
21+
*
22+
* You should have received a copy of the GNU Affero General Public License
23+
* along with this program. If not, see <https://www.gnu.org/licenses/>.
24+
*
25+
*/
26+
27+
namespace OCA\User_SAML\Helper;
28+
29+
trait TXmlHelper {
30+
31+
/**
32+
* @returns mixed returns the result of the callable parameter
33+
*/
34+
public function callWithXmlEntityLoader(callable $func) {
35+
libxml_set_external_entity_loader(static function ($public, $system) {
36+
return $system;
37+
});
38+
$result = $func();
39+
libxml_set_external_entity_loader(static function () {
40+
return null;
41+
});
42+
return $result;
43+
}
44+
}

0 commit comments

Comments
 (0)