From 55eddf1e81467317e4cf5c5193fb2400b49d047a Mon Sep 17 00:00:00 2001 From: Martin van Es Date: Tue, 21 Jul 2026 16:33:07 +0200 Subject: [PATCH 1/8] Add some attributes to authz_eb call Adds external Subject-id and email to the EB SRAM interrupt call --- .../Corto/Filter/Command/SramInterruptFilter.php | 6 +++++- .../EngineBlockBundle/Sbs/Dto/AuthzRequest.php | 10 ++++++++-- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/library/EngineBlock/Corto/Filter/Command/SramInterruptFilter.php b/library/EngineBlock/Corto/Filter/Command/SramInterruptFilter.php index 77e5e4d6a7..e5d99be64e 100644 --- a/library/EngineBlock/Corto/Filter/Command/SramInterruptFilter.php +++ b/library/EngineBlock/Corto/Filter/Command/SramInterruptFilter.php @@ -107,6 +107,8 @@ private function buildRequest(ServiceProvider $serviceProvider): AuthzRequest $userId = $this->_collabPersonId ?? ""; $eppn = $attributes['urn:mace:dir:attribute-def:eduPersonPrincipalName'][0] ?? ""; + $externalSubject = $attributes['urn:oasis:names:tc:SAML:attribute:subject-id'][0] ?? ""; + $email = $attributes['urn:mace:dir:attribute-def:mail'][0] ?? ""; $continueUrl = $this->_server->getUrl('SramInterruptService', '') . "?ID=$id"; $serviceId = $serviceProvider->entityId; $issuerId = $this->_identityProvider->entityId; @@ -117,7 +119,9 @@ private function buildRequest(ServiceProvider $serviceProvider): AuthzRequest $eppn, $continueUrl, $serviceId, - $issuerId + $issuerId, + $externalSubject, + $email ); } diff --git a/src/OpenConext/EngineBlockBundle/Sbs/Dto/AuthzRequest.php b/src/OpenConext/EngineBlockBundle/Sbs/Dto/AuthzRequest.php index d8a403ed07..eebec7d08b 100644 --- a/src/OpenConext/EngineBlockBundle/Sbs/Dto/AuthzRequest.php +++ b/src/OpenConext/EngineBlockBundle/Sbs/Dto/AuthzRequest.php @@ -28,13 +28,17 @@ public function __construct( public readonly string $eduPersonPrincipalName, public readonly string $continueUrl, public readonly string $serviceId, - public readonly string $issuerId + public readonly string $issuerId, + public readonly string $externalSubject, + public readonly string $email, ) { Assertion::string($userId, 'The userId must be a string.'); Assertion::string($eduPersonPrincipalName, 'The eduPersonPrincipalName must be a string.'); Assertion::string($continueUrl, 'The continueUrl must be a string.'); Assertion::string($serviceId, 'The serviceId must be a string.'); Assertion::string($issuerId, 'The issuerId must be a string.'); + Assertion::string($externalSubject, 'The externalSubject must be a string.'); + Assertion::string($email, 'The email must be a string.'); } public function jsonSerialize() : array @@ -44,7 +48,9 @@ public function jsonSerialize() : array 'eppn' => $this->eduPersonPrincipalName, 'continue_url' => $this->continueUrl, 'service_id' => $this->serviceId, - 'issuer_id' => $this->issuerId + 'issuer_id' => $this->issuerId, + 'external_subject' => $this->externalSubject, + 'email' => $this->email ]; } } From 7e9cf6394bfd2e783cb4f6d00d5f9a65d8f6ac66 Mon Sep 17 00:00:00 2001 From: Martin van Es Date: Tue, 21 Jul 2026 17:17:59 +0200 Subject: [PATCH 2/8] Add attributes --- .../EngineBlock/Corto/Filter/Command/SramInterruptFilter.php | 4 +++- src/OpenConext/EngineBlockBundle/Sbs/Dto/AuthzRequest.php | 5 ++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/library/EngineBlock/Corto/Filter/Command/SramInterruptFilter.php b/library/EngineBlock/Corto/Filter/Command/SramInterruptFilter.php index e5d99be64e..e68509b95f 100644 --- a/library/EngineBlock/Corto/Filter/Command/SramInterruptFilter.php +++ b/library/EngineBlock/Corto/Filter/Command/SramInterruptFilter.php @@ -109,6 +109,7 @@ private function buildRequest(ServiceProvider $serviceProvider): AuthzRequest $eppn = $attributes['urn:mace:dir:attribute-def:eduPersonPrincipalName'][0] ?? ""; $externalSubject = $attributes['urn:oasis:names:tc:SAML:attribute:subject-id'][0] ?? ""; $email = $attributes['urn:mace:dir:attribute-def:mail'][0] ?? ""; + $attributes = $attributes; $continueUrl = $this->_server->getUrl('SramInterruptService', '') . "?ID=$id"; $serviceId = $serviceProvider->entityId; $issuerId = $this->_identityProvider->entityId; @@ -121,7 +122,8 @@ private function buildRequest(ServiceProvider $serviceProvider): AuthzRequest $serviceId, $issuerId, $externalSubject, - $email + $email, + $attributes ); } diff --git a/src/OpenConext/EngineBlockBundle/Sbs/Dto/AuthzRequest.php b/src/OpenConext/EngineBlockBundle/Sbs/Dto/AuthzRequest.php index eebec7d08b..ef1c625a50 100644 --- a/src/OpenConext/EngineBlockBundle/Sbs/Dto/AuthzRequest.php +++ b/src/OpenConext/EngineBlockBundle/Sbs/Dto/AuthzRequest.php @@ -31,6 +31,7 @@ public function __construct( public readonly string $issuerId, public readonly string $externalSubject, public readonly string $email, + public readonly array $attributes, ) { Assertion::string($userId, 'The userId must be a string.'); Assertion::string($eduPersonPrincipalName, 'The eduPersonPrincipalName must be a string.'); @@ -39,6 +40,7 @@ public function __construct( Assertion::string($issuerId, 'The issuerId must be a string.'); Assertion::string($externalSubject, 'The externalSubject must be a string.'); Assertion::string($email, 'The email must be a string.'); + Assertion::isArray($attributes, 'The attributes must be an array.'); } public function jsonSerialize() : array @@ -50,7 +52,8 @@ public function jsonSerialize() : array 'service_id' => $this->serviceId, 'issuer_id' => $this->issuerId, 'external_subject' => $this->externalSubject, - 'email' => $this->email + 'email' => $this->email, + 'attributes' => $this->attributes ]; } } From e9c2b70ee3c73dc80eaffa8164c6225737e7c1e7 Mon Sep 17 00:00:00 2001 From: Martin van Es Date: Tue, 21 Jul 2026 17:19:32 +0200 Subject: [PATCH 3/8] email is array --- .../EngineBlock/Corto/Filter/Command/SramInterruptFilter.php | 2 +- src/OpenConext/EngineBlockBundle/Sbs/Dto/AuthzRequest.php | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/library/EngineBlock/Corto/Filter/Command/SramInterruptFilter.php b/library/EngineBlock/Corto/Filter/Command/SramInterruptFilter.php index e68509b95f..de80ad6e11 100644 --- a/library/EngineBlock/Corto/Filter/Command/SramInterruptFilter.php +++ b/library/EngineBlock/Corto/Filter/Command/SramInterruptFilter.php @@ -108,7 +108,7 @@ private function buildRequest(ServiceProvider $serviceProvider): AuthzRequest $userId = $this->_collabPersonId ?? ""; $eppn = $attributes['urn:mace:dir:attribute-def:eduPersonPrincipalName'][0] ?? ""; $externalSubject = $attributes['urn:oasis:names:tc:SAML:attribute:subject-id'][0] ?? ""; - $email = $attributes['urn:mace:dir:attribute-def:mail'][0] ?? ""; + $email = $attributes['urn:mace:dir:attribute-def:mail'] ?? []; $attributes = $attributes; $continueUrl = $this->_server->getUrl('SramInterruptService', '') . "?ID=$id"; $serviceId = $serviceProvider->entityId; diff --git a/src/OpenConext/EngineBlockBundle/Sbs/Dto/AuthzRequest.php b/src/OpenConext/EngineBlockBundle/Sbs/Dto/AuthzRequest.php index ef1c625a50..0ebaff2c85 100644 --- a/src/OpenConext/EngineBlockBundle/Sbs/Dto/AuthzRequest.php +++ b/src/OpenConext/EngineBlockBundle/Sbs/Dto/AuthzRequest.php @@ -30,7 +30,7 @@ public function __construct( public readonly string $serviceId, public readonly string $issuerId, public readonly string $externalSubject, - public readonly string $email, + public readonly array $email, public readonly array $attributes, ) { Assertion::string($userId, 'The userId must be a string.'); @@ -39,7 +39,7 @@ public function __construct( Assertion::string($serviceId, 'The serviceId must be a string.'); Assertion::string($issuerId, 'The issuerId must be a string.'); Assertion::string($externalSubject, 'The externalSubject must be a string.'); - Assertion::string($email, 'The email must be a string.'); + Assertion::isArray($email, 'The email must be an array.'); Assertion::isArray($attributes, 'The attributes must be an array.'); } From 89ee9c1d4532de00dc77ad3de6db11eed3eecc8a Mon Sep 17 00:00:00 2001 From: Martin van Es Date: Wed, 22 Jul 2026 11:38:08 +0200 Subject: [PATCH 4/8] Fix tests --- .../Corto/Filter/Command/SramInterruptFilter.php | 4 ++-- .../EngineBlockBundle/Sbs/Dto/AuthzRequest.php | 14 +++++++------- .../Filter/Command/SramInterruptFilterTest.php | 15 ++++++++++----- 3 files changed, 19 insertions(+), 14 deletions(-) diff --git a/library/EngineBlock/Corto/Filter/Command/SramInterruptFilter.php b/library/EngineBlock/Corto/Filter/Command/SramInterruptFilter.php index de80ad6e11..7599f23e43 100644 --- a/library/EngineBlock/Corto/Filter/Command/SramInterruptFilter.php +++ b/library/EngineBlock/Corto/Filter/Command/SramInterruptFilter.php @@ -118,11 +118,11 @@ private function buildRequest(ServiceProvider $serviceProvider): AuthzRequest return new AuthzRequest( $userId, $eppn, + $externalSubject, + $email, $continueUrl, $serviceId, $issuerId, - $externalSubject, - $email, $attributes ); } diff --git a/src/OpenConext/EngineBlockBundle/Sbs/Dto/AuthzRequest.php b/src/OpenConext/EngineBlockBundle/Sbs/Dto/AuthzRequest.php index 0ebaff2c85..4b0a16a35a 100644 --- a/src/OpenConext/EngineBlockBundle/Sbs/Dto/AuthzRequest.php +++ b/src/OpenConext/EngineBlockBundle/Sbs/Dto/AuthzRequest.php @@ -26,20 +26,20 @@ class AuthzRequest implements JsonSerializable public function __construct( public readonly string $userId, public readonly string $eduPersonPrincipalName, + public readonly string $externalSubject, + public readonly array $email, public readonly string $continueUrl, public readonly string $serviceId, public readonly string $issuerId, - public readonly string $externalSubject, - public readonly array $email, - public readonly array $attributes, + public readonly array $attributes ) { Assertion::string($userId, 'The userId must be a string.'); Assertion::string($eduPersonPrincipalName, 'The eduPersonPrincipalName must be a string.'); + Assertion::string($externalSubject, 'The externalSubject must be a string.'); + Assertion::isArray($email, 'The email must be an array.'); Assertion::string($continueUrl, 'The continueUrl must be a string.'); Assertion::string($serviceId, 'The serviceId must be a string.'); Assertion::string($issuerId, 'The issuerId must be a string.'); - Assertion::string($externalSubject, 'The externalSubject must be a string.'); - Assertion::isArray($email, 'The email must be an array.'); Assertion::isArray($attributes, 'The attributes must be an array.'); } @@ -48,11 +48,11 @@ public function jsonSerialize() : array return [ 'user_id' => $this->userId, 'eppn' => $this->eduPersonPrincipalName, + 'external_subject' => $this->externalSubject, + 'email' => $this->email, 'continue_url' => $this->continueUrl, 'service_id' => $this->serviceId, 'issuer_id' => $this->issuerId, - 'external_subject' => $this->externalSubject, - 'email' => $this->email, 'attributes' => $this->attributes ]; } diff --git a/tests/library/EngineBlock/Test/Corto/Filter/Command/SramInterruptFilterTest.php b/tests/library/EngineBlock/Test/Corto/Filter/Command/SramInterruptFilterTest.php index d532e1e9be..40c6f88c55 100644 --- a/tests/library/EngineBlock/Test/Corto/Filter/Command/SramInterruptFilterTest.php +++ b/tests/library/EngineBlock/Test/Corto/Filter/Command/SramInterruptFilterTest.php @@ -138,16 +138,18 @@ public function testItAddsNonceWhenMessageInterrupt(): void ] ]); - $expectedRequest = new AuthzRequest('', '', 'https://example.org?ID=', 'spEntityId', 'idpEntityId'); + $expectedRequest = new AuthzRequest('', '', '', [], 'https://example.org?ID=', 'spEntityId', 'idpEntityId', $initialAttributes); $sbsClient->shouldReceive('authz') ->withArgs(function ($args) use ($expectedRequest) { - return $args->userId === $expectedRequest->userId && $args->eduPersonPrincipalName === $expectedRequest->eduPersonPrincipalName + && $args->externalSubject === $expectedRequest->externalSubject + && $args->email === $expectedRequest->email && strpos($args->continueUrl, $expectedRequest->continueUrl) === 0 && $args->serviceId === $expectedRequest->serviceId - && $args->issuerId === $expectedRequest->issuerId; + && $args->issuerId === $expectedRequest->issuerId + && $args->attributes === $expectedRequest->attributes; }) ->andReturn($response); @@ -206,16 +208,19 @@ public function testItAddsSramAttributesOnStatusAuthorized(): void ], ]); - $expectedRequest = new AuthzRequest('', '', 'https://example.org?ID=', 'spEntityId', 'idpEntityId'); + $expectedRequest = new AuthzRequest('', '', '', [], 'https://example.org?ID=', 'spEntityId', 'idpEntityId', $initialAttributes); $sbsClient->shouldReceive('authz') ->withArgs(function ($args) use ($expectedRequest) { return $args->userId === $expectedRequest->userId && $args->eduPersonPrincipalName === $expectedRequest->eduPersonPrincipalName + && $args->externalSubject === $expectedRequest->externalSubject + && $args->email === $expectedRequest->email && str_starts_with($args->continueUrl, $expectedRequest->continueUrl) && $args->serviceId === $expectedRequest->serviceId - && $args->issuerId === $expectedRequest->issuerId; + && $args->issuerId === $expectedRequest->issuerId + && $args->attributes === $expectedRequest->attributes; }) ->andReturn($response); From 4c820057b286318dca8f0af005f76208dd5562de Mon Sep 17 00:00:00 2001 From: Martin van Es Date: Wed, 22 Jul 2026 13:12:21 +0200 Subject: [PATCH 5/8] Rename externalSubject --- .../Corto/Filter/Command/SramInterruptFilter.php | 4 ++-- src/OpenConext/EngineBlockBundle/Sbs/Dto/AuthzRequest.php | 6 +++--- .../Test/Corto/Filter/Command/SramInterruptFilterTest.php | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/library/EngineBlock/Corto/Filter/Command/SramInterruptFilter.php b/library/EngineBlock/Corto/Filter/Command/SramInterruptFilter.php index 7599f23e43..066d11a58f 100644 --- a/library/EngineBlock/Corto/Filter/Command/SramInterruptFilter.php +++ b/library/EngineBlock/Corto/Filter/Command/SramInterruptFilter.php @@ -107,7 +107,7 @@ private function buildRequest(ServiceProvider $serviceProvider): AuthzRequest $userId = $this->_collabPersonId ?? ""; $eppn = $attributes['urn:mace:dir:attribute-def:eduPersonPrincipalName'][0] ?? ""; - $externalSubject = $attributes['urn:oasis:names:tc:SAML:attribute:subject-id'][0] ?? ""; + $externalSubjectId = $attributes['urn:oasis:names:tc:SAML:attribute:subject-id'][0] ?? ""; $email = $attributes['urn:mace:dir:attribute-def:mail'] ?? []; $attributes = $attributes; $continueUrl = $this->_server->getUrl('SramInterruptService', '') . "?ID=$id"; @@ -118,7 +118,7 @@ private function buildRequest(ServiceProvider $serviceProvider): AuthzRequest return new AuthzRequest( $userId, $eppn, - $externalSubject, + $externalSubjectId, $email, $continueUrl, $serviceId, diff --git a/src/OpenConext/EngineBlockBundle/Sbs/Dto/AuthzRequest.php b/src/OpenConext/EngineBlockBundle/Sbs/Dto/AuthzRequest.php index 4b0a16a35a..611ab80273 100644 --- a/src/OpenConext/EngineBlockBundle/Sbs/Dto/AuthzRequest.php +++ b/src/OpenConext/EngineBlockBundle/Sbs/Dto/AuthzRequest.php @@ -26,7 +26,7 @@ class AuthzRequest implements JsonSerializable public function __construct( public readonly string $userId, public readonly string $eduPersonPrincipalName, - public readonly string $externalSubject, + public readonly string $externalSubjectId, public readonly array $email, public readonly string $continueUrl, public readonly string $serviceId, @@ -35,7 +35,7 @@ public function __construct( ) { Assertion::string($userId, 'The userId must be a string.'); Assertion::string($eduPersonPrincipalName, 'The eduPersonPrincipalName must be a string.'); - Assertion::string($externalSubject, 'The externalSubject must be a string.'); + Assertion::string($externalSubjectId, 'The externalSubjectId must be a string.'); Assertion::isArray($email, 'The email must be an array.'); Assertion::string($continueUrl, 'The continueUrl must be a string.'); Assertion::string($serviceId, 'The serviceId must be a string.'); @@ -48,7 +48,7 @@ public function jsonSerialize() : array return [ 'user_id' => $this->userId, 'eppn' => $this->eduPersonPrincipalName, - 'external_subject' => $this->externalSubject, + 'external_subject_id' => $this->externalSubjectId, 'email' => $this->email, 'continue_url' => $this->continueUrl, 'service_id' => $this->serviceId, diff --git a/tests/library/EngineBlock/Test/Corto/Filter/Command/SramInterruptFilterTest.php b/tests/library/EngineBlock/Test/Corto/Filter/Command/SramInterruptFilterTest.php index 40c6f88c55..7dfbe5acf7 100644 --- a/tests/library/EngineBlock/Test/Corto/Filter/Command/SramInterruptFilterTest.php +++ b/tests/library/EngineBlock/Test/Corto/Filter/Command/SramInterruptFilterTest.php @@ -144,7 +144,7 @@ public function testItAddsNonceWhenMessageInterrupt(): void ->withArgs(function ($args) use ($expectedRequest) { return $args->userId === $expectedRequest->userId && $args->eduPersonPrincipalName === $expectedRequest->eduPersonPrincipalName - && $args->externalSubject === $expectedRequest->externalSubject + && $args->externalSubjectId === $expectedRequest->externalSubjectId && $args->email === $expectedRequest->email && strpos($args->continueUrl, $expectedRequest->continueUrl) === 0 && $args->serviceId === $expectedRequest->serviceId @@ -215,7 +215,7 @@ public function testItAddsSramAttributesOnStatusAuthorized(): void return $args->userId === $expectedRequest->userId && $args->eduPersonPrincipalName === $expectedRequest->eduPersonPrincipalName - && $args->externalSubject === $expectedRequest->externalSubject + && $args->externalSubjectId === $expectedRequest->externalSubjectId && $args->email === $expectedRequest->email && str_starts_with($args->continueUrl, $expectedRequest->continueUrl) && $args->serviceId === $expectedRequest->serviceId From cbc105ca21f7cca522e19e758e69f0de5c759e14 Mon Sep 17 00:00:00 2001 From: Martin van Es Date: Wed, 22 Jul 2026 13:14:53 +0200 Subject: [PATCH 6/8] Add Changelog --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e82ba79038..c973327e32 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,12 @@ More information about our release strategy can be found in the [Development Guidelines](https://github.com/OpenConext/OpenConext-engineblock/wiki/Development-Guidelines#release-notes) on the EngineBlock wiki. +## 7.2.1 + +Features: + +* Added `external_subject_id`, `email` and `attributes` to `SramInterruptFilter` request + ## 7.2.0 Maintenance: From f7e86a54c7d198c4b0fd9e3d28d7876c80e0b8ce Mon Sep 17 00:00:00 2001 From: Kay Joosten Date: Wed, 22 Jul 2026 14:48:36 +0200 Subject: [PATCH 7/8] Remove dead code and improve test coverage in SramInterruptFilter - Remove no-op self-assignment ($attributes = $attributes;) left over from the attribute extraction refactor - Use named arguments for the AuthzRequest constructor call to reduce the risk of argument-order mistakes as the DTO grows - Extend test fixtures with subject-id and mail attributes so the new externalSubjectId/email extraction logic is actually exercised instead of only asserting on empty defaults --- .../Filter/Command/SramInterruptFilter.php | 18 +++++----- .../Command/SramInterruptFilterTest.php | 36 ++++++++++++++++--- 2 files changed, 40 insertions(+), 14 deletions(-) diff --git a/library/EngineBlock/Corto/Filter/Command/SramInterruptFilter.php b/library/EngineBlock/Corto/Filter/Command/SramInterruptFilter.php index 066d11a58f..7ab2a72f9f 100644 --- a/library/EngineBlock/Corto/Filter/Command/SramInterruptFilter.php +++ b/library/EngineBlock/Corto/Filter/Command/SramInterruptFilter.php @@ -109,21 +109,19 @@ private function buildRequest(ServiceProvider $serviceProvider): AuthzRequest $eppn = $attributes['urn:mace:dir:attribute-def:eduPersonPrincipalName'][0] ?? ""; $externalSubjectId = $attributes['urn:oasis:names:tc:SAML:attribute:subject-id'][0] ?? ""; $email = $attributes['urn:mace:dir:attribute-def:mail'] ?? []; - $attributes = $attributes; $continueUrl = $this->_server->getUrl('SramInterruptService', '') . "?ID=$id"; $serviceId = $serviceProvider->entityId; $issuerId = $this->_identityProvider->entityId; - return new AuthzRequest( - $userId, - $eppn, - $externalSubjectId, - $email, - $continueUrl, - $serviceId, - $issuerId, - $attributes + userId: $userId, + eduPersonPrincipalName: $eppn, + externalSubjectId: $externalSubjectId, + email: $email, + continueUrl: $continueUrl, + serviceId: $serviceId, + issuerId: $issuerId, + attributes: $attributes ); } diff --git a/tests/library/EngineBlock/Test/Corto/Filter/Command/SramInterruptFilterTest.php b/tests/library/EngineBlock/Test/Corto/Filter/Command/SramInterruptFilterTest.php index 7dfbe5acf7..43d808d78b 100644 --- a/tests/library/EngineBlock/Test/Corto/Filter/Command/SramInterruptFilterTest.php +++ b/tests/library/EngineBlock/Test/Corto/Filter/Command/SramInterruptFilterTest.php @@ -115,7 +115,11 @@ public function testItAddsNonceWhenMessageInterrupt(): void new NullLogger(), ); - $initialAttributes = ['urn:mace:dir:attribute-def:uid' => ['userIdValue']]; + $initialAttributes = [ + 'urn:mace:dir:attribute-def:uid' => ['userIdValue'], + 'urn:oasis:names:tc:SAML:attribute:subject-id' => ['subjectIdValue'], + 'urn:mace:dir:attribute-def:mail' => ['user@example.org'], + ]; $sramFilter->setResponseAttributes($initialAttributes); $server = Mockery::mock(EngineBlock_Corto_ProxyServer::class); @@ -138,7 +142,16 @@ public function testItAddsNonceWhenMessageInterrupt(): void ] ]); - $expectedRequest = new AuthzRequest('', '', '', [], 'https://example.org?ID=', 'spEntityId', 'idpEntityId', $initialAttributes); + $expectedRequest = new AuthzRequest( + userId: '', + eduPersonPrincipalName: '', + externalSubjectId: 'subjectIdValue', + email: ['user@example.org'], + continueUrl: 'https://example.org?ID=', + serviceId: 'spEntityId', + issuerId: 'idpEntityId', + attributes: $initialAttributes, + ); $sbsClient->shouldReceive('authz') ->withArgs(function ($args) use ($expectedRequest) { @@ -183,7 +196,11 @@ public function testItAddsSramAttributesOnStatusAuthorized(): void new NullLogger() ); - $initialAttributes = ['urn:mace:dir:attribute-def:uid' => ['userIdValue']]; + $initialAttributes = [ + 'urn:mace:dir:attribute-def:uid' => ['userIdValue'], + 'urn:oasis:names:tc:SAML:attribute:subject-id' => ['subjectIdValue'], + 'urn:mace:dir:attribute-def:mail' => ['user@example.org'], + ]; $sramFilter->setResponseAttributes($initialAttributes); $server = Mockery::mock(EngineBlock_Corto_ProxyServer::class); @@ -208,7 +225,16 @@ public function testItAddsSramAttributesOnStatusAuthorized(): void ], ]); - $expectedRequest = new AuthzRequest('', '', '', [], 'https://example.org?ID=', 'spEntityId', 'idpEntityId', $initialAttributes); + $expectedRequest = new AuthzRequest( + userId: '', + eduPersonPrincipalName: '', + externalSubjectId: 'subjectIdValue', + email: ['user@example.org'], + continueUrl: 'https://example.org?ID=', + serviceId: 'spEntityId', + issuerId: 'idpEntityId', + attributes: $initialAttributes, + ); $sbsClient->shouldReceive('authz') ->withArgs(function ($args) use ($expectedRequest) { @@ -236,6 +262,8 @@ public function testItAddsSramAttributesOnStatusAuthorized(): void $expectedAttributes = [ 'urn:mace:dir:attribute-def:uid' => ['userIdValue'], + 'urn:oasis:names:tc:SAML:attribute:subject-id' => ['subjectIdValue'], + 'urn:mace:dir:attribute-def:mail' => ['user@example.org'], 'urn:mace:dir:attribute-def:eduPersonEntitlement' => 'attributes', ]; From 4d6f7018a06374ff1b50fdc9910f7edd624df836 Mon Sep 17 00:00:00 2001 From: Bas Zoetekouw Date: Wed, 22 Jul 2026 17:53:57 +0200 Subject: [PATCH 8/8] Simple attribute filter for SRAM call --- .../Filter/Command/SramInterruptFilter.php | 15 ++++++- .../Command/SramInterruptFilterTest.php | 43 +++++++++++++++++++ 2 files changed, 57 insertions(+), 1 deletion(-) diff --git a/library/EngineBlock/Corto/Filter/Command/SramInterruptFilter.php b/library/EngineBlock/Corto/Filter/Command/SramInterruptFilter.php index 7ab2a72f9f..688f9264a5 100644 --- a/library/EngineBlock/Corto/Filter/Command/SramInterruptFilter.php +++ b/library/EngineBlock/Corto/Filter/Command/SramInterruptFilter.php @@ -100,6 +100,19 @@ public function execute(): void } } + // TODO: duplicated from EngineBlockBundle/AttributeAggregation/Dto/Request.php + private static function filterNonStringValuesFromAttributes($attributes) + { + return array_filter($attributes, function ($attributeValues) { + foreach ($attributeValues as $attributeValue) { + if (!is_string($attributeValue)) { + return false; // drop the whole attribute + } + } + return true; + }); + } + private function buildRequest(ServiceProvider $serviceProvider): AuthzRequest { $attributes = $this->getResponseAttributes(); @@ -121,7 +134,7 @@ private function buildRequest(ServiceProvider $serviceProvider): AuthzRequest continueUrl: $continueUrl, serviceId: $serviceId, issuerId: $issuerId, - attributes: $attributes + attributes: self::filterNonStringValuesFromAttributes($attributes) ); } diff --git a/tests/library/EngineBlock/Test/Corto/Filter/Command/SramInterruptFilterTest.php b/tests/library/EngineBlock/Test/Corto/Filter/Command/SramInterruptFilterTest.php index 43d808d78b..97265a45d3 100644 --- a/tests/library/EngineBlock/Test/Corto/Filter/Command/SramInterruptFilterTest.php +++ b/tests/library/EngineBlock/Test/Corto/Filter/Command/SramInterruptFilterTest.php @@ -26,6 +26,7 @@ use OpenConext\EngineBlockBundle\Sbs\AuthzResponse; use OpenConext\EngineBlockBundle\Sbs\SbsClientInterface; use OpenConext\EngineBlockBundle\Sbs\SbsAttributeMerger; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; use Psr\Log\NullLogger; use SAML2\Assertion; @@ -315,6 +316,48 @@ public function testThrowsEngineBlockExceptionIfPolicyCannotBeChecked() $sramFilter->execute(); } + #[DataProvider('filterNonStringValuesProvider')] + public function testFilterNonStringValuesFromAttributes(array $input, array $expected): void + { + $method = new ReflectionMethod( + EngineBlock_Corto_Filter_Command_SramInterruptFilter::class, + 'filterNonStringValuesFromAttributes' + ); + + $this->assertSame($expected, $method->invoke(null, $input)); + } + + public static function filterNonStringValuesProvider(): array + { + return [ + // ---- happy paths: array of (possibly empty) arrays of strings ---- + 'empty attribute set' => [[], []], + 'single string value' => [['a' => ['x']], ['a' => ['x']]], + 'multiple string values' => [['a' => ['x', 'y']], ['a' => ['x', 'y']]], + 'empty inner array kept' => [['a' => []], ['a' => []]], + 'several string attributes' => [ + ['a' => ['x'], 'b' => ['y', 'z'], 'c' => []], + ['a' => ['x'], 'b' => ['y', 'z'], 'c' => []], + ], + + // ---- failure paths: offending attribute dropped ---- + 'inner has integer' => [['a' => [1]], []], + 'inner has float' => [['a' => [1.5]], []], + 'inner has bool' => [['a' => [true]], []], + 'inner has null' => [['a' => [null]], []], + 'inner has nested array' => [['a' => [['nested']]], []], + 'inner has object' => [['a' => [new stdClass()]], []], + 'inner mixed string + int' => [['a' => ['x', 1]], []], + 'inner mixed string + obj' => [['a' => ['x', new stdClass()]], []], + + // ---- mixed set: only offending attributes dropped, keys preserved ---- + 'keeps good drops bad' => [ + ['good' => ['x'], 'bad' => ['x', 2], 'empty' => []], + ['good' => ['x'], 'empty' => []], + ], + ]; + } + private function mockServiceProvider(string $entityId): ServiceProvider { $sp = Mockery::mock(ServiceProvider::class);