Skip to content

Commit db99ab8

Browse files
kayjoostenkayjoosten
authored
Issue-1757 move saml id generator (#1993)
Use SAML ID generator from `simplesamlphp/xml-common` And remove from corto. Resolves #1757
1 parent d9ce098 commit db99ab8

11 files changed

Lines changed: 104 additions & 26 deletions

File tree

‎config/services/compat.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ services:
5454

5555
engineblock.compat.saml2_id_generator:
5656
public: true
57-
class: EngineBlock_Saml2_IdGenerator_Default
57+
alias: OpenConext\EngineBlock\Saml2\IdGenerator
5858

5959
engineblock.compat.attribute_release_policy_enforcer:
6060
public: false

‎config/services/services.yml‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -199,11 +199,17 @@ services:
199199
- '@OpenConext\EngineBlock\Metadata\Factory\ValueObject\EngineBlockConfiguration'
200200
- '@OpenConext\EngineBlockBundle\Url\UrlProvider'
201201

202+
OpenConext\EngineBlock\Saml2\DefaultIdGenerator: ~
203+
204+
OpenConext\EngineBlock\Saml2\IdGenerator:
205+
alias: OpenConext\EngineBlock\Saml2\DefaultIdGenerator
206+
public: true
207+
202208
OpenConext\EngineBlock\Xml\MetadataRenderer:
203209
arguments:
204210
- '@OpenConext\EngineBlockBundle\Localization\LanguageSupportProvider'
205211
- '@twig'
206-
- '@engineblock.compat.saml2_id_generator'
212+
- '@OpenConext\EngineBlock\Saml2\IdGenerator'
207213
- '@OpenConext\EngineBlock\Metadata\X509\KeyPairFactory'
208214
- '@OpenConext\EngineBlock\Xml\DocumentSigner'
209215
- '@OpenConext\EngineBlock\Service\TimeProvider\TimeProvider'

‎library/EngineBlock/Application/DiContainer.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,7 @@ public function getTimeProvider()
177177
}
178178

179179
/**
180-
* @return EngineBlock_Saml2_IdGenerator
180+
* @return \OpenConext\EngineBlock\Saml2\IdGenerator
181181
*/
182182
public function getSaml2IdGenerator()
183183
{

‎library/EngineBlock/Corto/Module/Service/SingleSignOn.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -358,7 +358,7 @@ protected function _createUnsolicitedRequest()
358358
$relayState = !empty($_GET['RelayState']) ? $_GET['RelayState'] : null;
359359

360360
$sspRequest = new AuthnRequest();
361-
$sspRequest->setId($this->_server->getNewId(EngineBlock_Saml2_IdGenerator::ID_USAGE_SAML2_REQUEST));
361+
$sspRequest->setId($this->_server->getNewId(\OpenConext\EngineBlock\Saml2\IdGenerator::ID_USAGE_SAML2_REQUEST));
362362
$issuer = new Issuer();
363363
$issuer->setValue($entityId);
364364
$sspRequest->setIssuer($issuer);
@@ -393,7 +393,7 @@ protected function _createUnsolicitedRequest()
393393
protected function _createDebugRequest()
394394
{
395395
$sspRequest = new AuthnRequest();
396-
$sspRequest->setId($this->_server->getNewId(EngineBlock_Saml2_IdGenerator::ID_USAGE_SAML2_REQUEST));
396+
$sspRequest->setId($this->_server->getNewId(\OpenConext\EngineBlock\Saml2\IdGenerator::ID_USAGE_SAML2_REQUEST));
397397
$issuer = new Issuer();
398398
$issuer->setValue($this->_server->getUrl('spMetadataService'));
399399
$sspRequest->setIssuer($issuer);

‎library/EngineBlock/Corto/ProxyServer.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -811,7 +811,7 @@ public function createEnhancedResponse(
811811
// Create a new assertion by us.
812812
$newAssertion = new Assertion();
813813
$newResponse->setAssertions(array($newAssertion));
814-
$newAssertion->setId($this->getNewId(EngineBlock_Saml2_IdGenerator::ID_USAGE_SAML2_ASSERTION));
814+
$newAssertion->setId($this->getNewId(\OpenConext\EngineBlock\Saml2\IdGenerator::ID_USAGE_SAML2_ASSERTION));
815815
$newAssertion->setIssueInstant(time());
816816
$newAssertion->setIssuer($newResponse->getIssuer());
817817

@@ -903,7 +903,7 @@ protected function _createBaseResponse(EngineBlock_Saml2_AuthnRequestAnnotationD
903903
$response = new Response();
904904
/** @var AuthnRequest $request */
905905
$response->setRelayState($request->getRelayState());
906-
$response->setId($this->getNewId(EngineBlock_Saml2_IdGenerator::ID_USAGE_SAML2_RESPONSE));
906+
$response->setId($this->getNewId(\OpenConext\EngineBlock\Saml2\IdGenerator::ID_USAGE_SAML2_RESPONSE));
907907
$response->setIssueInstant(time());
908908
if (!$requestWasUnsolicited) {
909909
$response->setInResponseTo($request->getId());
@@ -1411,7 +1411,7 @@ public function timeStamp($deltaSeconds = 0)
14111411
return $provider->timestamp($deltaSeconds);
14121412
}
14131413

1414-
public function getNewId($usage = EngineBlock_Saml2_IdGenerator::ID_USAGE_OTHER)
1414+
public function getNewId($usage = \OpenConext\EngineBlock\Saml2\IdGenerator::ID_USAGE_OTHER)
14151415
{
14161416
$generator = EngineBlock_ApplicationSingleton::getInstance()->getDiContainer()->getSaml2IdGenerator();
14171417
return $generator->generate(self::ID_PREFIX, $usage);

‎library/EngineBlock/Saml2/AuthnRequestFactory.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ public static function createFromRequest(
4747
/** @var AuthnRequest $originalRequest */
4848

4949
$sspRequest = new AuthnRequest();
50-
$sspRequest->setId($server->getNewId(EngineBlock_Saml2_IdGenerator::ID_USAGE_SAML2_REQUEST));
50+
$sspRequest->setId($server->getNewId(\OpenConext\EngineBlock\Saml2\IdGenerator::ID_USAGE_SAML2_REQUEST));
5151
$sspRequest->setIssueInstant(time());
5252
$sspRequest->setDestination(isset($idpMetadata->singleSignOnServices[0]) ? $idpMetadata->singleSignOnServices[0]->location : null);
5353
$sspRequest->setForceAuthn($originalRequest->getForceAuthn());

library/EngineBlock/Saml2/IdGenerator/Default.php renamed to src/OpenConext/EngineBlock/Saml2/DefaultIdGenerator.php

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
1-
<?php
1+
<?php declare(strict_types=1);
22

33
/**
4-
* Copyright 2010 SURFnet B.V.
4+
* Copyright 2026 SURFnet B.V.
55
*
66
* Licensed under the Apache License, Version 2.0 (the "License");
77
* you may not use this file except in compliance with the License.
@@ -16,10 +16,15 @@
1616
* limitations under the License.
1717
*/
1818

19-
class EngineBlock_Saml2_IdGenerator_Default implements EngineBlock_Saml2_IdGenerator
19+
namespace OpenConext\EngineBlock\Saml2;
20+
21+
use function bin2hex;
22+
use function random_bytes;
23+
24+
final class DefaultIdGenerator implements IdGenerator
2025
{
21-
public function generate($prefix = 'EB', $usage = EngineBlock_Saml2_IdGenerator::ID_USAGE_OTHER)
26+
public function generate(string $prefix = 'EB', string $usage = self::ID_USAGE_OTHER): string
2227
{
23-
return $prefix . sha1(uniqid(mt_rand(), true));
28+
return $prefix . bin2hex(random_bytes(20));
2429
}
2530
}

library/EngineBlock/Saml2/IdGenerator.php renamed to src/OpenConext/EngineBlock/Saml2/IdGenerator.php

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
1-
<?php
1+
<?php declare(strict_types=1);
22

33
/**
4-
* Copyright 2010 SURFnet B.V.
4+
* Copyright 2026 SURFnet B.V.
55
*
66
* Licensed under the Apache License, Version 2.0 (the "License");
77
* you may not use this file except in compliance with the License.
@@ -16,13 +16,15 @@
1616
* limitations under the License.
1717
*/
1818

19-
interface EngineBlock_Saml2_IdGenerator
19+
namespace OpenConext\EngineBlock\Saml2;
20+
21+
interface IdGenerator
2022
{
2123
const ID_USAGE_SAML2_METADATA = 'saml2-metadata';
2224
const ID_USAGE_OTHER = 'other';
2325
const ID_USAGE_SAML2_RESPONSE = 'saml2-response';
2426
const ID_USAGE_SAML2_REQUEST = 'saml2-request';
2527
const ID_USAGE_SAML2_ASSERTION = 'saml2-assertion';
2628

27-
public function generate($prefix = 'EB', $usage = self::ID_USAGE_OTHER);
29+
public function generate(string $prefix = 'EB', string $usage = self::ID_USAGE_OTHER): string;
2830
}

‎src/OpenConext/EngineBlock/Xml/MetadataRenderer.php‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818

1919
namespace OpenConext\EngineBlock\Xml;
2020

21-
use EngineBlock_Saml2_IdGenerator;
2221
use InvalidArgumentException;
2322
use OpenConext\EngineBlock\Metadata\Factory\Collection\IdentityProviderEntityCollection;
2423
use OpenConext\EngineBlock\Metadata\Factory\Helper\IdentityProviderMetadataHelper;
@@ -27,6 +26,7 @@
2726
use OpenConext\EngineBlock\Metadata\Factory\ServiceProviderEntityInterface;
2827
use OpenConext\EngineBlock\Metadata\X509\KeyPairFactory;
2928
use OpenConext\EngineBlock\Metadata\X509\X509KeyPair;
29+
use OpenConext\EngineBlock\Saml2\IdGenerator as SamlIdGenerator;
3030
use OpenConext\EngineBlock\Service\TimeProvider\TimeProvider;
3131
use OpenConext\EngineBlockBundle\Localization\LanguageSupportProvider;
3232
use Twig\Environment;
@@ -49,7 +49,7 @@ class MetadataRenderer
4949
private $twig;
5050

5151
/**
52-
* @var EngineBlock_Saml2_IdGenerator
52+
* @var SamlIdGenerator
5353
*/
5454
private $samlIdGenerator;
5555

@@ -83,7 +83,7 @@ class MetadataRenderer
8383
public function __construct(
8484
LanguageSupportProvider $languageSupportProvider,
8585
Environment $twig,
86-
EngineBlock_Saml2_IdGenerator $samlIdGenerator,
86+
SamlIdGenerator $samlIdGenerator,
8787
KeyPairFactory $keyPairFactory,
8888
DocumentSigner $documentSigner,
8989
TimeProvider $timeProvider,
@@ -160,7 +160,7 @@ private function renderMetadataXmlServiceProvider(ServiceProviderEntityInterface
160160
}
161161

162162
$params = [
163-
'id' => $this->samlIdGenerator->generate(self::ID_PREFIX, EngineBlock_Saml2_IdGenerator::ID_USAGE_SAML2_METADATA),
163+
'id' => $this->samlIdGenerator->generate(self::ID_PREFIX, SamlIdGenerator::ID_USAGE_SAML2_METADATA),
164164
'validUntil' => $this->getValidUntil(),
165165
'metadata' => $metadata,
166166
'locales' => $this->languageSupportProvider->getSupportedLanguages(),
@@ -175,7 +175,7 @@ private function renderMetadataXmlIdentityProvider(IdentityProviderEntityInterfa
175175
$metadata = new IdentityProviderMetadataHelper($idp, $this->languageSupportProvider);
176176

177177
$params = [
178-
'id' => $this->samlIdGenerator->generate(self::ID_PREFIX, EngineBlock_Saml2_IdGenerator::ID_USAGE_SAML2_METADATA),
178+
'id' => $this->samlIdGenerator->generate(self::ID_PREFIX, SamlIdGenerator::ID_USAGE_SAML2_METADATA),
179179
'validUntil' => $this->getValidUntil(),
180180
'metadata' => $metadata,
181181
'locales' => $this->languageSupportProvider->getSupportedLanguages(),
@@ -192,7 +192,7 @@ private function renderMetadataXmlIdentityProviderCollection(IdentityProviderEnt
192192
}
193193

194194
$params = [
195-
'id' => $this->samlIdGenerator->generate(self::ID_PREFIX, EngineBlock_Saml2_IdGenerator::ID_USAGE_SAML2_METADATA),
195+
'id' => $this->samlIdGenerator->generate(self::ID_PREFIX, SamlIdGenerator::ID_USAGE_SAML2_METADATA),
196196
'validUntil' => $this->getValidUntil(),
197197
'metadataCollection' => $metadataCollection,
198198
'locales' => $this->languageSupportProvider->getSupportedLanguages(),

‎tests/unit/OpenConext/EngineBlock/Saml2/DefaultIdGeneratorTest.php‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
<?php declare(strict_types=1);
2+
3+
/**
4+
* Copyright 2026 SURFnet B.V.
5+
*
6+
* Licensed under the Apache License, Version 2.0 (the "License");
7+
* you may not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing, software
13+
* distributed under the License is distributed on an "AS IS" BASIS,
14+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
* See the License for the specific language governing permissions and
16+
* limitations under the License.
17+
*/
18+
19+
namespace OpenConext\EngineBlock\Saml2;
20+
21+
use PHPUnit\Framework\Attributes\Test;
22+
use PHPUnit\Framework\TestCase;
23+
24+
class DefaultIdGeneratorTest extends TestCase
25+
{
26+
private DefaultIdGenerator $generator;
27+
28+
protected function setUp(): void
29+
{
30+
$this->generator = new DefaultIdGenerator();
31+
}
32+
33+
#[Test]
34+
public function generateReturnsStringWithGivenPrefix(): void
35+
{
36+
$id = $this->generator->generate('EB', IdGenerator::ID_USAGE_OTHER);
37+
38+
self::assertStringStartsWith('EB', $id);
39+
}
40+
41+
#[Test]
42+
public function generateReturnsUniqueIds(): void
43+
{
44+
$id1 = $this->generator->generate();
45+
$id2 = $this->generator->generate();
46+
47+
self::assertNotSame($id1, $id2);
48+
}
49+
50+
#[Test]
51+
public function generateDefaultsToEbPrefix(): void
52+
{
53+
$id = $this->generator->generate();
54+
55+
self::assertStringStartsWith('EB', $id);
56+
}
57+
58+
#[Test]
59+
public function generateReturnsFortyHexCharsAfterPrefix(): void
60+
{
61+
$id = $this->generator->generate('EB', IdGenerator::ID_USAGE_OTHER);
62+
63+
self::assertMatchesRegularExpression('/^EB[0-9a-f]{40}$/', $id);
64+
}
65+
}

0 commit comments

Comments
 (0)