Skip to content

Commit 333b68f

Browse files
tvdijentvdijen
committed
Add RelayState validation
1 parent 4f98ed9 commit 333b68f

File tree

6 files changed

+105
-3
lines changed

6 files changed

+105
-3
lines changed

‎src/Assert/Assert.php‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,20 +15,23 @@
1515
* @method static void validDomain(mixed $value, string $message = '', string $exception = '')
1616
* @method static void validEntityID(mixed $value, string $message = '', string $exception = '')
1717
* @method static void validGeolocation(mixed $value, string $message = '', string $exception = '')
18+
* @method static void validRelayState(mixed $value, string $message = '', string $exception = '')
1819
* @method static void validSAMLAnyURI(mixed $value, string $message = '', string $exception = '')
1920
* @method static void validSAMLDateTime(mixed $value, string $message = '', string $exception = '')
2021
* @method static void validSAMLString(mixed $value, string $message = '', string $exception = '')
2122
* @method static void nullOrValidCIDR(mixed $value, string $message = '', string $exception = '')
2223
* @method static void nullOrValidDomain(mixed $value, string $message = '', string $exception = '')
2324
* @method static void nullOrValidEntityID(mixed $value, string $message = '', string $exception = '')
2425
* @method static void nullOrValidGeolocation(mixed $value, string $message = '', string $exception = '')
26+
* @method static void nullOrValidRelayState(mixed $value, string $message = '', string $exception = '')
2527
* @method static void nullOrValidSAMLAnyURI(mixed $value, string $message = '', string $exception = '')
2628
* @method static void nullOrValidSAMLDateTime(mixed $value, string $message = '', string $exception = '')
2729
* @method static void nullOrValidSAMLString(mixed $value, string $message = '', string $exception = '')
2830
* @method static void allValidCIDR(mixed $value, string $message = '', string $exception = '')
2931
* @method static void allValidDomain(mixed $value, string $message = '', string $exception = '')
3032
* @method static void allValidEntityID(mixed $value, string $message = '', string $exception = '')
3133
* @method static void allValidGeolocation(mixed $value, string $message = '', string $exception = '')
34+
* @method static void allValidRelayState(mixed $value, string $message = '', string $exception = '')
3235
* @method static void allValidSAMLAnyURI(mixed $value, string $message = '', string $exception = '')
3336
* @method static void allValidSAMLDateTime(mixed $value, string $message = '', string $exception = '')
3437
* @method static void allValidSAMLString(mixed $value, string $message = '', string $exception = '')
@@ -39,6 +42,7 @@ class Assert extends BaseAssert
3942
use DomainTrait;
4043
use EntityIDTrait;
4144
use GeolocationTrait;
45+
use RelayStateTrait;
4246
use SAMLAnyURITrait;
4347
use SAMLDateTimeTrait;
4448
use SAMLStringTrait;

‎src/Assert/RelayStateTrait.php‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace SimpleSAML\SAML2\Assert;
6+
7+
use SimpleSAML\Assert\AssertionFailedException;
8+
use SimpleSAML\SAML2\Constants as C;
9+
use SimpleSAML\SAML2\Exception\ProtocolViolationException;
10+
11+
/**
12+
* @package simplesamlphp/saml2
13+
*/
14+
trait RelayStateTrait
15+
{
16+
/**
17+
* @param string $value
18+
* @param string $message
19+
*/
20+
protected static function validRelayState(string $value, string $message = ''): void
21+
{
22+
parent::notWhitespaceOnly($value, $message); // Not protocol-defined, but makes zero sense
23+
try {
24+
/**
25+
* 3.4.3 RelayState
26+
*
27+
* The value MUST NOT exceed 80 bytes in length [..]
28+
*/
29+
parent::maxLength(
30+
$value,
31+
C::MAX_RELAY_STATE_LENGTH,
32+
$message ?: '%s is not a SAML2.0-compliant RelayState',
33+
);
34+
} catch (AssertionFailedException $e) {
35+
throw new ProtocolViolationException($e->getMessage());
36+
}
37+
}
38+
}

‎src/Binding.php‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@
44

55
namespace SimpleSAML\SAML2;
66

7-
use Psr\Http\Message\ResponseInterface;
8-
use Psr\Http\Message\ServerRequestInterface;
7+
use Psr\Http\Message\{ResponseInterface, ServerRequestInterface};
8+
use SimpleSAML\SAML2\Assert\Assert;
99
use SimpleSAML\SAML2\Binding\{HTTPArtifact, HTTPPost, HTTPRedirect, SOAP};
1010
use SimpleSAML\SAML2\Constants as C;
1111
use SimpleSAML\SAML2\Exception\Protocol\UnsupportedBindingException;
@@ -163,6 +163,7 @@ public function getDestination(): ?string
163163
*/
164164
public function setRelayState(?string $relayState = null): void
165165
{
166+
Assert::nullOrValidRelayState($relayState);
166167
$this->relayState = $relayState;
167168
}
168169

‎src/Constants.php‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -462,6 +462,11 @@ class Constants extends \SimpleSAML\XMLSecurity\Constants
462462
*/
463463
public const ENTITYID_MAX_LENGTH = 1024;
464464

465+
/**
466+
* The maximum size in bytes for any RelayState as per specification
467+
*/
468+
public const MAX_RELAY_STATE_LENGTH = 80;
469+
465470
/**
466471
* The maximum size for any entityid as per SAML2INT-specification
467472
*/

‎tests/SAML2/Assert/RelayStateTest.php‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace SimpleSAML\Test\SAML2\Assert;
6+
7+
use PHPUnit\Framework\Attributes\{CoversClass, DataProvider, Group};
8+
use PHPUnit\Framework\TestCase;
9+
use SimpleSAML\Assert\AssertionFailedException;
10+
use SimpleSAML\SAML2\Assert\Assert;
11+
use SimpleSAML\SAML2\Constants as C;
12+
use SimpleSAML\SAML2\Exception\ProtocolViolationException;
13+
14+
use function strpad;
15+
16+
/**
17+
* Class \SimpleSAML\SAML2\Assert\RelayStateTest
18+
*
19+
* @package simplesamlphp/saml2
20+
*/
21+
#[Group('assert')]
22+
#[CoversClass(Assert::class)]
23+
final class RelayStateTest extends TestCase
24+
{
25+
/**
26+
* @param boolean $shouldPass
27+
* @param string $str
28+
*/
29+
#[DataProvider('provideRelayState')]
30+
public function testValidRelayState(bool $shouldPass, string $str): void
31+
{
32+
try {
33+
Assert::validRelayState($str);
34+
$this->assertTrue($shouldPass);
35+
} catch (ProtocolViolationException | AssertionFailedException $e) {
36+
$this->assertFalse($shouldPass);
37+
}
38+
}
39+
40+
41+
/**
42+
* @return array<string, array{0: bool, 1: string}>
43+
*/
44+
public static function provideRelayState(): array
45+
{
46+
return [
47+
'POST Binding' => [true, 'https://target.local'],
48+
'Redirect Binding' => [true, 'fdcoi3Xgoj32M94ejVh11LtQTMZjNE'],
49+
'spaces' => [true, 'this is silly'],
50+
'empty' => [false, ''],
51+
'too_long' => [false, str_pad('', C::MAX_RELAY_STATE_LENGTH + 1, 'a')],
52+
];
53+
}
54+
}

‎tests/SAML2/Binding/HTTPRedirectTest.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -238,7 +238,7 @@ public function testNoSigAlgSpecified(): void
238238
{
239239
$q = [
240240
'SAMLRequest' => 'nVLBauMwEP0Vo7sjW7FpKpJA2rBsoNuGOruHXhZFHm8EsuRqxtv27yvbWWgvYelFgjfvzbx5zBJVazu56enkHuG5B6TktbUO5VhYsT446RUalE61gJK0rDY/7qSYZbILnrz2ln2QXFYoRAhkvGPJbrtiv7VoygJEoTJ9LOusXDSFuJ4vdH6cxwoIEGUjsrqoFUt+QcCoXLHYKMoRe9g5JOUoQlleprlI8/yQz6W4ksXiiSXbuI1xikbViahDyfkRSM2wD40DmjnL0bSdhcE6Hx7BTd3xqnqoIPw1GmbdqWPJNx80jCGtGIUeWLL5t8mtd9i3EM78n493/zWr9XVvx+58mj39IlUaR/QmKOPq4Dtkyf4c9E1EjPtzOePjREL5/XDYp/uH6sDWy6G3HDML66+5ayO7VlHx2dySf2y9nM7pPprabffeGv02ZNcquux5QEydNiNVUlAODTiKMVvrX24DKIJz8nw9jfx8tOt3',
241-
'RelayState' => 'https://beta.surfnet.nl/simplesaml/module.php/core/authenticate.php?as=Braindrops',
241+
'RelayState' => 'https://profile.surfconext.nl/',
242242
'Signature' => 'b+qe/XGgICOrEL1v9dwuoy0RJtJ/GNAr7gJGYSJzLG0riPKwo7v5CH8GPC2P9IRikaeaNeQrnhBAaf8FCWrO0cLFw4qR6msK9bxRBGk+hIaTUYCh54ETrVCyGlmBneMgC5/iCRvtEW3ESPXCCqt8Ncu98yZmv9LIVyHSl67Se+fbB9sDw3/fzwYIHRMqK2aS8jnsnqlgnBGGOXqIqN3+d/2dwtCfz14s/9odoYzSUv32qfNPiPez6PSNqwhwH7dWE3TlO/jZmz0DnOeQ2ft6qdZEi5ZN5KCV6VmNKpkrLMq6DDPnuwPm/8oCAoT88R2jG7uf9QZB+ArWJKMEhDLsCA==',
243243
];
244244
$request = new ServerRequest('GET', 'http://tnyholm.se');

0 commit comments

Comments
 (0)