Skip to content

Commit d07d93c

Browse files
tvdijentvdijen
committed
Move RelayState; it's a characteristic of the binding rather than the message
1 parent da73fe2 commit d07d93c

File tree

9 files changed

+43
-51
lines changed

9 files changed

+43
-51
lines changed

‎src/SAML2/Binding.php‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,13 @@
2424
*/
2525
abstract class Binding
2626
{
27+
/**
28+
* The RelayState associated with the message.
29+
*
30+
* @var string|null
31+
*/
32+
protected ?string $relayState = null;
33+
2734
/**
2835
* The destination of messages.
2936
*
@@ -148,6 +155,28 @@ public function getDestination(): ?string
148155
}
149156

150157

158+
/**
159+
* Set the RelayState associated with he message.
160+
*
161+
* @param string|null $relayState The RelayState.
162+
*/
163+
public function setRelayState(string $relayState = null): void
164+
{
165+
$this->relayState = $relayState;
166+
}
167+
168+
169+
/**
170+
* Get the RelayState associated with the message.
171+
*
172+
* @return string|null The RelayState.
173+
*/
174+
public function getRelayState(): ?string
175+
{
176+
return $this->relayState;
177+
}
178+
179+
151180
/**
152181
* Override the destination of a message.
153182
*

‎src/SAML2/HTTPArtifact.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ public function getRedirectURL(AbstractMessage $message): string
8585

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

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

196196
if (isset($query['RelayState'])) {
197-
$samlResponse->setRelayState($query['RelayState']);
197+
$this->setRelayState($query['RelayState']);
198198
}
199199

200200
return $samlResponse;

‎src/SAML2/HTTPPost.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ public function send(AbstractMessage $message): ResponseInterface
4141
} else {
4242
$destination = $this->destination;
4343
}
44-
$relayState = $message->getRelayState();
44+
$relayState = $this->getRelayState();
4545

4646
$msgStr = $message->toXML();
4747

@@ -108,7 +108,7 @@ public function receive(ServerRequestInterface $request): AbstractMessage
108108
}
109109

110110
if (array_key_exists('RelayState', $query)) {
111-
$msg->setRelayState($query['RelayState']);
111+
$this->setRelayState($query['RelayState']);
112112
}
113113

114114
return $msg;

‎src/SAML2/HTTPRedirect.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ public function getRedirectURL(AbstractMessage $message): string
5353
$destination = $this->destination;
5454
}
5555

56-
$relayState = $message->getRelayState();
56+
$relayState = $this->getRelayState();
5757
$msgStr = $message->toXML();
5858

5959
Utils::getContainer()->debugMessage($msgStr, 'out');
@@ -151,8 +151,8 @@ public function receive(ServerRequestInterface $request): AbstractMessage
151151
$message = MessageFactory::fromXML($document->documentElement);
152152

153153
if (array_key_exists('RelayState', $query)) {
154-
$message->setRelayState($query['RelayState']);
155154
$signedQuery .= '&RelayState=' . urlencode($query['RelayState']);
155+
$this->setRelayState($query['RelayState']);
156156
}
157157

158158
if (!array_key_exists('Signature', $query)) {

‎src/SAML2/XML/samlp/AbstractMessage.php‎

Lines changed: 0 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -38,13 +38,6 @@ abstract class AbstractMessage extends AbstractSamlpElement implements SignableE
3838
use SignedElementTrait;
3939

4040

41-
/**
42-
* The RelayState associated with this message.
43-
*
44-
* @var string|null
45-
*/
46-
protected ?string $relayState = null;
47-
4841
/**
4942
* The \DOMDocument we are currently building.
5043
*
@@ -77,7 +70,6 @@ abstract class AbstractMessage extends AbstractSamlpElement implements SignableE
7770
* @param string|null $destination
7871
* @param string|null $consent
7972
* @param \SimpleSAML\SAML2\XML\samlp\Extensions $extensions
80-
* @param string|null $relayState
8173
*
8274
* @throws \Exception
8375
*/
@@ -89,15 +81,13 @@ protected function __construct(
8981
protected ?string $destination = null,
9082
protected ?string $consent = null,
9183
?Extensions $extensions = null,
92-
?string $relayState = null,
9384
) {
9485
Assert::nullOrSame($issueInstant?->getTimeZone()->getName(), 'Z', ProtocolViolationException::class);
9586
Assert::nullOrValidNCName($id); // Covers the empty string
9687
Assert::nullOrValidURI($destination); // Covers the empty string
9788
Assert::nullOrValidURI($consent); // Covers the empty string
9889

9990
$this->setExtensions($extensions);
100-
$this->setRelayState($relayState);
10191
}
10292

10393

@@ -188,30 +178,6 @@ public function isMessageConstructedWithSignature(): bool
188178
}
189179

190180

191-
/**
192-
* Retrieve the RelayState associated with this message.
193-
*
194-
* @return string|null The RelayState, or NULL if no RelayState is given
195-
*/
196-
public function getRelayState(): ?string
197-
{
198-
return $this->relayState;
199-
}
200-
201-
202-
/**
203-
* Set the RelayState associated with this message.
204-
*
205-
* @param string|null $relayState The new RelayState
206-
*/
207-
public function setRelayState(string $relayState = null): void
208-
{
209-
Assert::nullOrNotWhitespaceOnly($relayState);
210-
211-
$this->relayState = $relayState;
212-
}
213-
214-
215181
/**
216182
* Get the XML element.
217183
*

‎src/SAML2/XML/samlp/AbstractStatusResponse.php‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@ abstract class AbstractStatusResponse extends AbstractMessage
3434
* @param string|null $destination
3535
* @param string|null $consent
3636
* @param \SimpleSAML\SAML2\XML\samlp\Extensions|null $extensions
37-
* @param string|null $relayState
3837
*
3938
* @throws \Exception
4039
*/
@@ -48,11 +47,10 @@ protected function __construct(
4847
?string $destination = null,
4948
?string $consent = null,
5049
?Extensions $extensions = null,
51-
?string $relayState = null,
5250
) {
5351
Assert::nullOrValidNCName($inResponseTo); // Covers the empty string
5452

55-
parent::__construct($issuer, $id, $version, $issueInstant, $destination, $consent, $extensions, $relayState);
53+
parent::__construct($issuer, $id, $version, $issueInstant, $destination, $consent, $extensions);
5654
}
5755

5856

‎src/SAML2/XML/samlp/LogoutResponse.php‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@ final class LogoutResponse extends AbstractStatusResponse
3636
* @param string|null $destination
3737
* @param string|null $consent
3838
* @param \SimpleSAML\SAML2\XML\samlp\Extensions|null $extensions
39-
* @param string|null $relayState
4039
*
4140
* @throws \Exception
4241
*/
@@ -50,7 +49,6 @@ public function __construct(
5049
?string $destination = null,
5150
?string $consent = null,
5251
?Extensions $extensions = null,
53-
?string $relayState = null,
5452
) {
5553
parent::__construct(
5654
$status,
@@ -62,7 +60,6 @@ public function __construct(
6260
$destination,
6361
$consent,
6462
$extensions,
65-
$relayState,
6663
);
6764
}
6865

‎tests/SAML2/HTTPPostTest.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ public function testResponseParsing(): void
7878
$this->assertInstanceOf(Response::class, $response);
7979
$issuer = $response->getIssuer();
8080
$this->assertEquals('https://engine.test.surfconext.nl/authentication/idp/metadata', $issuer->getContent());
81-
$relay = $response->getRelayState();
81+
$relay = $hp->getRelayState();
8282
$this->assertEquals('relaystate001', $relay);
8383
}
8484

@@ -161,14 +161,14 @@ public function testSendAuthnResponse(): void
161161
issuer: $issuer,
162162
destination: 'http://example.org/login?success=yes',
163163
);
164-
$response->setRelayState('http://example.org');
165164
$signer = (new SignatureAlgorithmFactory())->getAlgorithm(
166165
C::SIG_RSA_SHA256,
167166
PEMCertificatesMock::getPrivateKey(PEMCertificatesMock::SELFSIGNED_PRIVATE_KEY),
168167
);
169168
$response->sign($signer);
170169

171170
$hr = new HTTPPost();
171+
$hr->setRelayState('http://example.org');
172172
$hr->send($response);
173173
}
174174
}

‎tests/SAML2/HTTPRedirectTest.php‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ public function testRequestParsingMoreParams(): void
115115
$hr = new HTTPRedirect();
116116
$samlrequest = $hr->receive($request);
117117
$this->assertInstanceOf(AbstractRequest::class, $samlrequest);
118-
$relaystate = $samlrequest->getRelayState();
118+
$relaystate = $hr->getRelayState();
119119
$this->assertEquals('https://profile.surfconext.nl/', $relaystate);
120120
}
121121

@@ -138,7 +138,7 @@ public function testSignedRequestParsing(): void
138138
$hr = new HTTPRedirect();
139139
$samlrequest = $hr->receive($request);
140140
$this->assertInstanceOf(AbstractRequest::class, $samlrequest);
141-
$relaystate = $samlrequest->getRelayState();
141+
$relaystate = $hr->getRelayState();
142142
$this->assertEquals(
143143
'https://demo.moo-archive.nl/module.php/admin/test/default-sp',
144144
$relaystate,
@@ -161,6 +161,7 @@ public function testSignedRequestValidation(): void
161161
$request = $request->withQueryParams($q);
162162

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

166167
// validate with the correct certificate, should verify
@@ -325,8 +326,9 @@ public function testSendAuthnResponse(): void
325326
issuer: $issuer,
326327
destination: 'http://example.org/login?success=yes',
327328
);
328-
$response->setRelayState('http://example.org');
329+
329330
$hr = new HTTPRedirect();
331+
$hr->setRelayState('http://example.org');
330332
$hr->send($response);
331333
}
332334

0 commit comments

Comments
 (0)