Skip to content

Commit

Permalink
Move RelayState; it's a characteristic of the binding rather than the…
Browse files Browse the repository at this point in the history
… message
  • Loading branch information
tvdijen committed Aug 14, 2023
1 parent 9baba03 commit c4f0ff3
Show file tree
Hide file tree
Showing 9 changed files with 43 additions and 51 deletions.
29 changes: 29 additions & 0 deletions src/SAML2/Binding.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,13 @@
*/
abstract class Binding
{
/**
* The RelayState associated with the message.
*
* @var string|null
*/
protected ?string $relayState = null;

/**
* The destination of messages.
*
Expand Down Expand Up @@ -149,6 +156,28 @@ public function getDestination(): ?string
}


/**
* Set the RelayState associated with he message.
*
* @param string|null $relayState The RelayState.
*/
public function setRelayState(string $relayState = null): void
{
$this->relayState = $relayState;
}


/**
* Get the RelayState associated with the message.
*
* @return string|null The RelayState.
*/
public function getRelayState(): ?string
{
return $this->relayState;
}


/**
* Override the destination of a message.
*
Expand Down
4 changes: 2 additions & 2 deletions src/SAML2/HTTPArtifact.php
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ public function getRedirectURL(AbstractMessage $message): string

$params = ['SAMLart' => $artifact];

$relayState = $message->getRelayState();
$relayState = $this->getRelayState();
if ($relayState !== null) {
$params['RelayState'] = $relayState;
}
Expand Down Expand Up @@ -194,7 +194,7 @@ public function receive(ServerRequestInterface $request): AbstractMessage
$samlResponse->addValidator([get_class($this), 'validateSignature'], $artifactResponse);

if (isset($query['RelayState'])) {
$samlResponse->setRelayState($query['RelayState']);
$this->setRelayState($query['RelayState']);
}

return $samlResponse;
Expand Down
4 changes: 2 additions & 2 deletions src/SAML2/HTTPPost.php
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ public function send(AbstractMessage $message): ResponseInterface
} else {
$destination = $this->destination;
}
$relayState = $message->getRelayState();
$relayState = $this->getRelayState();

$msgStr = $message->toXML();

Expand Down Expand Up @@ -110,7 +110,7 @@ public function receive(ServerRequestInterface $request): AbstractMessage
}

if (array_key_exists('RelayState', $query)) {
$msg->setRelayState($query['RelayState']);
$this->setRelayState($query['RelayState']);
}

return $msg;
Expand Down
4 changes: 2 additions & 2 deletions src/SAML2/HTTPRedirect.php
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ public function getRedirectURL(AbstractMessage $message): string
$destination = $this->destination;
}

$relayState = $message->getRelayState();
$relayState = $this->getRelayState();
$msgStr = $message->toXML();

Utils::getContainer()->debugMessage($msgStr, 'out');
Expand Down Expand Up @@ -152,7 +152,7 @@ public function receive(ServerRequestInterface $request): AbstractMessage

$msg = MessageFactory::fromXML($document->documentElement);
if (array_key_exists('RelayState', $query)) {
$msg->setRelayState($query['RelayState']);
$this->setRelayState($query['RelayState']);
}
return $msg;
}
Expand Down
34 changes: 0 additions & 34 deletions src/SAML2/XML/samlp/AbstractMessage.php
Original file line number Diff line number Diff line change
Expand Up @@ -41,13 +41,6 @@ abstract class AbstractMessage extends AbstractSamlpElement implements SignableE
use SignedElementTrait;


/**
* The RelayState associated with this message.
*
* @var string|null
*/
protected ?string $relayState = null;

/**
* The \DOMDocument we are currently building.
*
Expand Down Expand Up @@ -80,7 +73,6 @@ abstract class AbstractMessage extends AbstractSamlpElement implements SignableE
* @param string|null $destination
* @param string|null $consent
* @param \SimpleSAML\SAML2\XML\samlp\Extensions $extensions
* @param string|null $relayState
*
* @throws \Exception
*/
Expand All @@ -92,15 +84,13 @@ protected function __construct(
protected ?string $destination = null,
protected ?string $consent = null,
?Extensions $extensions = null,
?string $relayState = null,
) {
Assert::nullOrSame($issueInstant?->getTimeZone()->getName(), 'Z', ProtocolViolationException::class);
Assert::nullOrValidNCName($id); // Covers the empty string
Assert::nullOrValidURI($destination); // Covers the empty string
Assert::nullOrValidURI($consent); // Covers the empty string

$this->setExtensions($extensions);
$this->setRelayState($relayState);
}


Expand Down Expand Up @@ -191,30 +181,6 @@ public function isMessageConstructedWithSignature(): bool
}


/**
* Retrieve the RelayState associated with this message.
*
* @return string|null The RelayState, or NULL if no RelayState is given
*/
public function getRelayState(): ?string
{
return $this->relayState;
}


/**
* Set the RelayState associated with this message.
*
* @param string|null $relayState The new RelayState
*/
public function setRelayState(string $relayState = null): void
{
Assert::nullOrNotWhitespaceOnly($relayState);

$this->relayState = $relayState;
}


/**
* Get the XML element.
*
Expand Down
4 changes: 1 addition & 3 deletions src/SAML2/XML/samlp/AbstractStatusResponse.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ abstract class AbstractStatusResponse extends AbstractMessage
* @param string|null $destination
* @param string|null $consent
* @param \SimpleSAML\SAML2\XML\samlp\Extensions|null $extensions
* @param string|null $relayState
*
* @throws \Exception
*/
Expand All @@ -48,11 +47,10 @@ protected function __construct(
?string $destination = null,
?string $consent = null,
?Extensions $extensions = null,
?string $relayState = null,
) {
Assert::nullOrValidNCName($inResponseTo); // Covers the empty string

parent::__construct($issuer, $id, $version, $issueInstant, $destination, $consent, $extensions, $relayState);
parent::__construct($issuer, $id, $version, $issueInstant, $destination, $consent, $extensions);
}


Expand Down
3 changes: 0 additions & 3 deletions src/SAML2/XML/samlp/LogoutResponse.php
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ final class LogoutResponse extends AbstractStatusResponse
* @param string|null $destination
* @param string|null $consent
* @param \SimpleSAML\SAML2\XML\samlp\Extensions|null $extensions
* @param string|null $relayState
*
* @throws \Exception
*/
Expand All @@ -51,7 +50,6 @@ public function __construct(
?string $destination = null,
?string $consent = null,
?Extensions $extensions = null,
?string $relayState = null,
) {
parent::__construct(
$status,
Expand All @@ -63,7 +61,6 @@ public function __construct(
$destination,
$consent,
$extensions,
$relayState,
);
}

Expand Down
4 changes: 2 additions & 2 deletions tests/SAML2/HTTPPostTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ public function testResponseParsing(): void
$this->assertInstanceOf(Response::class, $response);
$issuer = $response->getIssuer();
$this->assertEquals('https://engine.test.surfconext.nl/authentication/idp/metadata', $issuer->getContent());
$relay = $response->getRelayState();
$relay = $hp->getRelayState();
$this->assertEquals('relaystate001', $relay);
}

Expand Down Expand Up @@ -161,14 +161,14 @@ public function testSendAuthnResponse(): void
issuer: $issuer,
destination: 'http://example.org/login?success=yes',
);
$response->setRelayState('http://example.org');
$signer = (new SignatureAlgorithmFactory())->getAlgorithm(
C::SIG_RSA_SHA256,
PEMCertificatesMock::getPrivateKey(PEMCertificatesMock::SELFSIGNED_PRIVATE_KEY),
);
$response->sign($signer);

$hr = new HTTPPost();
$hr->setRelayState('http://example.org');
$hr->send($response);
}
}
8 changes: 5 additions & 3 deletions tests/SAML2/HTTPRedirectTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ public function testRequestParsingMoreParams(): void
$hr = new HTTPRedirect();
$samlrequest = $hr->receive($request);
$this->assertInstanceOf(AbstractRequest::class, $samlrequest);
$relaystate = $samlrequest->getRelayState();
$relaystate = $hr->getRelayState();
$this->assertEquals('https://profile.surfconext.nl/', $relaystate);
}

Expand All @@ -140,7 +140,7 @@ public function testSignedRequestParsing(): void
$hr = new HTTPRedirect();
$samlrequest = $hr->receive($request);
$this->assertInstanceOf(AbstractRequest::class, $samlrequest);
$relaystate = $samlrequest->getRelayState();
$relaystate = $hr->getRelayState();
$this->assertEquals(
'https://demo.moo-archive.nl/module.php/admin/test/default-sp',
$relaystate,
Expand All @@ -163,6 +163,7 @@ public function testSignedRequestValidation(): void
$request = $request->withQueryParams($q);

$hr = new HTTPRedirect();
$hr->setRelayState(urlencode($q['RelayState']));
$samlrequest = $hr->receive($request);

// validate with the correct certificate, should verify
Expand Down Expand Up @@ -308,8 +309,9 @@ public function testSendAuthnResponse(): void
issuer: $issuer,
destination: 'http://example.org/login?success=yes',
);
$response->setRelayState('http://example.org');

$hr = new HTTPRedirect();
$hr->setRelayState('http://example.org');
$hr->send($response);
}

Expand Down

0 comments on commit c4f0ff3

Please sign in to comment.