Skip to content

Commit b6cf4e8

Browse files
tvdijentvdijen
committed
Refactor ds:KeyValue
1 parent f441612 commit b6cf4e8

File tree

2 files changed

+49
-62
lines changed

2 files changed

+49
-62
lines changed

‎src/XML/ds/KeyValue.php‎

Lines changed: 44 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+
use ExtendableElementTrait {
32+
// We use our own getter instead of the trait's one
33+
getElements as private;
34+
setElements as private;
35+
}
2636
use SchemaValidatableElementTrait;
2737

2838

@@ -33,33 +43,41 @@ 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+
(
58+
($keyValue instanceof Chunk)
59+
? $keyValue->getNamespaceURI()
60+
: $keyValue::getNameSpaceURI()
61+
) !== C::NS_XDSIG,
62+
'A <ds:KeyValue> requires either a RSAKeyValue, DSAKeyValue, ECKeyValue '
63+
. 'or an element in namespace ##other',
64+
SchemaViolationException::class,
65+
);
5166
}
5267
}
5368

5469

5570
/**
5671
* Collect the value of the RSAKeyValue-property
5772
*
58-
* @return \SimpleSAML\XMLSecurity\XML\ds\RSAKeyValue|null
73+
* @return \SimpleSAML\XMLSecurity\XML\ds\RSAKeyValue|
74+
* \SimpleSAML\XMLSecurity\XML\ds\DSAKeyValue|
75+
* \SimpleSAML\XMLSecurity\XML\dsig11\ECKeyValue|
76+
* \SimpeSAML\XML\SerializableElementInterface
5977
*/
60-
public function getRSAKeyValue(): ?RSAKeyValue
78+
public function getKeyValue(): RSAKeyValue|DSAKeyValue|ECKeyValue|SerializableElementInterface
6179
{
62-
return $this->RSAKeyValue;
80+
return $this->keyValue;
6381
}
6482

6583

@@ -77,23 +95,20 @@ public static function fromXML(DOMElement $xml): static
7795
Assert::same($xml->localName, 'KeyValue', InvalidDOMElementException::class);
7896
Assert::same($xml->namespaceURI, KeyValue::NS, InvalidDOMElementException::class);
7997

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,
98+
$keyValue = array_merge(
99+
RSAKeyValue::getChildrenOfClass($xml),
100+
DSAKeyValue::getChildrenOfClass($xml),
101+
self::getChildElementsFromXML($xml),
86102
);
87103

88-
$elements = self::getChildElementsFromXML($xml);
89-
Assert::maxCount(
90-
$elements,
104+
Assert::count(
105+
$keyValue,
91106
1,
92-
'A <ds:KeyValue> can contain exactly one element in namespace ##other',
107+
'A <ds:KeyValue> must contain exactly one child element',
93108
TooManyElementsException::class,
94109
);
95110

96-
return new static(array_pop($RSAKeyValue), array_pop($elements));
111+
return new static(array_pop($keyValue));
97112
}
98113

99114

@@ -107,13 +122,7 @@ public function toXML(?DOMElement $parent = null): DOMElement
107122
{
108123
$e = $this->instantiateParentElement($parent);
109124

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

118127
return $e;
119128
}

‎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)