Skip to content

Commit 7edf950

Browse files
tvdijentvdijen
committed
Refactor ds:KeyValue
1 parent 1d6d6fa commit 7edf950

File tree

2 files changed

+46
-62
lines changed

2 files changed

+46
-62
lines changed

‎src/XML/ds/KeyValue.php‎

Lines changed: 41 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -6,14 +6,20 @@
66

77
use DOMElement;
88
use SimpleSAML\Assert\Assert;
9-
use SimpleSAML\XML\ElementInterface;
9+
use SimpleSAML\XML\Chunk;
1010
use SimpleSAML\XML\Exception\InvalidDOMElementException;
1111
use SimpleSAML\XML\Exception\SchemaViolationException;
1212
use SimpleSAML\XML\Exception\TooManyElementsException;
1313
use SimpleSAML\XML\ExtendableElementTrait;
1414
use SimpleSAML\XML\SchemaValidatableElementInterface;
1515
use SimpleSAML\XML\SchemaValidatableElementTrait;
16+
use SimpleSAML\XML\SerializableElementInterface;
1617
use SimpleSAML\XML\XsNamespace as NS;
18+
use SimpleSAML\XMLSecurity\Constants as C;
19+
use SimpleSAML\XMLSecurity\XML\dsig11\ECKeyValue;
20+
21+
use function array_merge;
22+
use function array_pop;
1723

1824
/**
1925
* Class representing a ds:KeyValue element.
@@ -22,7 +28,11 @@
2228
*/
2329
final class KeyValue extends AbstractDsElement implements SchemaValidatableElementInterface
2430
{
25-
use ExtendableElementTrait;
31+
// We use our own getter instead of the trait's one, so we prevent their use by marking them private
32+
use ExtendableElementTrait {
33+
getElements as private;
34+
setElements as private;
35+
}
2636
use SchemaValidatableElementTrait;
2737

2838

@@ -33,33 +43,38 @@ final class KeyValue extends AbstractDsElement implements SchemaValidatableEleme
3343
/**
3444
* Initialize an KeyValue.
3545
*
36-
* @param \SimpleSAML\XMLSecurity\XML\ds\RSAKeyValue|null $RSAKeyValue
37-
* @param \SimpleSAML\XML\SerializableElementInterface|null $element
46+
* @param \SimpleSAML\XML\SerializableElementInterface $keyValue
3847
*/
3948
final public function __construct(
40-
protected ?RSAKeyValue $RSAKeyValue,
41-
?ElementInterface $element = null,
49+
protected RSAKeyValue|DSAKeyValue|ECKeyValue|SerializableElementInterface $keyValue,
4250
) {
43-
Assert::false(
44-
is_null($RSAKeyValue) && is_null($element),
45-
'A <ds:KeyValue> requires either a RSAKeyValue or an element in namespace ##other',
46-
SchemaViolationException::class,
47-
);
48-
49-
if ($element !== null) {
50-
$this->setElements([$element]);
51+
if (
52+
!($keyValue instanceof RSAKeyValue
53+
|| $keyValue instanceof DSAKeyValue
54+
|| $keyValue instanceof ECKeyValue)
55+
) {
56+
Assert::true(
57+
(($keyValue instanceof Chunk) ? $keyValue->getNamespaceURI() : $keyValue::getNameSpaceURI())
58+
!== C::NS_XDSIG,
59+
'A <ds:KeyValue> requires either a RSAKeyValue, DSAKeyValue, ECKeyValue '
60+
. 'or an element in namespace ##other',
61+
SchemaViolationException::class,
62+
);
5163
}
5264
}
5365

5466

5567
/**
5668
* Collect the value of the RSAKeyValue-property
5769
*
58-
* @return \SimpleSAML\XMLSecurity\XML\ds\RSAKeyValue|null
70+
* @return \SimpleSAML\XMLSecurity\XML\ds\RSAKeyValue|
71+
* \SimpleSAML\XMLSecurity\XML\ds\DSAKeyValue|
72+
* \SimpleSAML\XMLSecurity\XML\dsig11\ECKeyValue|
73+
* \SimpeSAML\XML\SerializableElementInterface
5974
*/
60-
public function getRSAKeyValue(): ?RSAKeyValue
75+
public function getKeyValue(): RSAKeyValue|DSAKeyValue|ECKeyValue|SerializableElementInterface
6176
{
62-
return $this->RSAKeyValue;
77+
return $this->keyValue;
6378
}
6479

6580

@@ -77,23 +92,20 @@ public static function fromXML(DOMElement $xml): static
7792
Assert::same($xml->localName, 'KeyValue', InvalidDOMElementException::class);
7893
Assert::same($xml->namespaceURI, KeyValue::NS, InvalidDOMElementException::class);
7994

80-
$RSAKeyValue = RSAKeyValue::getChildrenOfClass($xml);
81-
Assert::maxCount(
82-
$RSAKeyValue,
83-
1,
84-
'A <ds:KeyValue> can contain exactly one <ds:RSAKeyValue>',
85-
TooManyElementsException::class,
95+
$keyValue = array_merge(
96+
RSAKeyValue::getChildrenOfClass($xml),
97+
DSAKeyValue::getChildrenOfClass($xml),
98+
self::getChildElementsFromXML($xml),
8699
);
87100

88-
$elements = self::getChildElementsFromXML($xml);
89-
Assert::maxCount(
90-
$elements,
101+
Assert::count(
102+
$keyValue,
91103
1,
92-
'A <ds:KeyValue> can contain exactly one element in namespace ##other',
104+
'A <ds:KeyValue> must contain exactly one child element',
93105
TooManyElementsException::class,
94106
);
95107

96-
return new static(array_pop($RSAKeyValue), array_pop($elements));
108+
return new static(array_pop($keyValue));
97109
}
98110

99111

@@ -107,13 +119,7 @@ public function toXML(?DOMElement $parent = null): DOMElement
107119
{
108120
$e = $this->instantiateParentElement($parent);
109121

110-
$this->getRSAKeyValue()?->toXML($e);
111-
112-
foreach ($this->elements as $elt) {
113-
if (!$elt->isEmptyElement()) {
114-
$elt->toXML($e);
115-
}
116-
}
122+
$this->getKeyValue()->toXML($e);
117123

118124
return $e;
119125
}

‎tests/XML/ds/KeyValueTest.php‎

Lines changed: 5 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -70,9 +70,8 @@ public function testMarshalling(): void
7070
{
7171
$keyValue = new KeyValue(RSAKeyValue::fromXML(self::$rsaKeyValue->documentElement));
7272

73-
$rsaKeyValue = $keyValue->getRSAKeyValue();
73+
$rsaKeyValue = $keyValue->getKeyValue();
7474
$this->assertInstanceOf(RSAKeyValue::class, $rsaKeyValue);
75-
$this->assertEmpty($keyValue->getElements());
7675

7776
$this->assertEquals($rsaKeyValue->getModulus()->getContent(), 'dGhpcyBpcyBzb21lIHJhbmRvbSBtb2R1bHVzCg==');
7877
$this->assertEquals($rsaKeyValue->getExponent()->getContent(), 'dGhpcyBpcyBzb21lIHJhbmRvbSBleHBvbmVudAo=');
@@ -88,13 +87,9 @@ public function testMarshalling(): void
8887
*/
8988
public function testMarshallingWithOtherElement(): void
9089
{
91-
$keyValue = new KeyValue(null, EncryptionProperty::fromXML(self::$encryptionProperty->documentElement));
90+
$keyValue = new KeyValue(EncryptionProperty::fromXML(self::$encryptionProperty->documentElement));
9291

93-
$elements = $keyValue->getElements();
94-
$this->assertEmpty($keyValue->getRSAKeyValue());
95-
$this->assertCount(1, $elements);
96-
97-
$element = reset($elements);
92+
$element = $keyValue->getKeyValue();
9893
$this->assertInstanceOf(EncryptionProperty::class, $element);
9994

10095
$document = self::$empty;
@@ -104,19 +99,6 @@ public function testMarshallingWithOtherElement(): void
10499
}
105100

106101

107-
/**
108-
*/
109-
public function testMarshallingEmpty(): void
110-
{
111-
$this->expectException(SchemaViolationException::class);
112-
$this->expectExceptionMessage(
113-
'A <ds:KeyValue> requires either a RSAKeyValue or an element in namespace ##other',
114-
);
115-
116-
new KeyValue(null, null);
117-
}
118-
119-
120102
/**
121103
*/
122104
public function testUnmarshallingWithOtherElement(): void
@@ -128,11 +110,7 @@ public function testUnmarshallingWithOtherElement(): void
128110

129111
$keyValue = KeyValue::fromXML($document->documentElement);
130112

131-
$elements = $keyValue->getElements();
132-
$this->assertNull($keyValue->getRSAKeyValue());
133-
$this->assertCount(1, $elements);
134-
135-
$element = reset($elements);
113+
$element = $keyValue->getKeyValue();
136114
$this->assertInstanceOf(EncryptionProperty::class, $element);
137115
}
138116

@@ -145,7 +123,7 @@ public function testUnmarshallingEmpty(): void
145123

146124
$this->expectException(SchemaViolationException::class);
147125
$this->expectExceptionMessage(
148-
'A <ds:KeyValue> requires either a RSAKeyValue or an element in namespace ##other',
126+
'A <ds:KeyValue> must contain exactly one child element',
149127
);
150128

151129
KeyValue::fromXML($document->documentElement);

0 commit comments

Comments
 (0)