Skip to content

Commit 4181fdf

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

File tree

2 files changed

+47
-62
lines changed

2 files changed

+47
-62
lines changed

‎src/XML/ds/KeyValue.php‎

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

77
use DOMElement;
88
use SimpleSAML\Assert\Assert;
9-
use SimpleSAML\XML\ElementInterface;
109
use SimpleSAML\XML\Exception\InvalidDOMElementException;
1110
use SimpleSAML\XML\Exception\SchemaViolationException;
1211
use SimpleSAML\XML\Exception\TooManyElementsException;
1312
use SimpleSAML\XML\ExtendableElementTrait;
1413
use SimpleSAML\XML\SchemaValidatableElementInterface;
1514
use SimpleSAML\XML\SchemaValidatableElementTrait;
15+
use SimpleSAML\XML\SerializableElementInterface;
1616
use SimpleSAML\XML\XsNamespace as NS;
17+
use SimpleSAML\XMLSecurity\Constants as C;
18+
19+
use function array_merge;
20+
use function array_pop;
1721

1822
/**
1923
* Class representing a ds:KeyValue element.
@@ -22,7 +26,11 @@
2226
*/
2327
final class KeyValue extends AbstractDsElement implements SchemaValidatableElementInterface
2428
{
25-
use ExtendableElementTrait;
29+
use ExtendableElementTrait {
30+
// We use our own getter instead of the trait's one
31+
getElements as private;
32+
setElements as private;
33+
}
2634
use SchemaValidatableElementTrait;
2735

2836

@@ -33,33 +41,41 @@ final class KeyValue extends AbstractDsElement implements SchemaValidatableEleme
3341
/**
3442
* Initialize an KeyValue.
3543
*
36-
* @param \SimpleSAML\XMLSecurity\XML\ds\RSAKeyValue|null $RSAKeyValue
37-
* @param \SimpleSAML\XML\SerializableElementInterface|null $element
44+
* @param \SimpleSAML\XML\SerializableElementInterface $keyValue
3845
*/
3946
final public function __construct(
40-
protected ?RSAKeyValue $RSAKeyValue,
41-
?ElementInterface $element = null,
47+
protected RSAKeyValue|DSAKeyValue|ECKeyValue|SerializableElementInterface $keyValue,
4248
) {
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]);
49+
if (!(
50+
$keyValue instanceof RSAKeyValue
51+
|| $keyValue instanceof DSAKeyValue
52+
|| $keyValue instanceof ECKeyValue
53+
)) {
54+
Assert::true(
55+
(
56+
($keyValue instanceof Chunk)
57+
? $keyValue->getNamespaceURI()
58+
: $keyValue::getNameSpaceURI()
59+
) !== C::NS_XDSIG,
60+
'A <ds:KeyValue> requires either a RSAKeyValue, DSAKeyValue, ECKeyValue '
61+
. 'or an element in namespace ##other',
62+
SchemaViolationException::class,
63+
);
5164
}
5265
}
5366

5467

5568
/**
5669
* Collect the value of the RSAKeyValue-property
5770
*
58-
* @return \SimpleSAML\XMLSecurity\XML\ds\RSAKeyValue|null
71+
* @return \SimpleSAML\XMLSecurity\XML\ds\RSAKeyValue|
72+
* \SimpleSAML\XMLSecurity\XML\ds\DSAKeyValue|
73+
* \SimpleSAML\XMLSecurity\XML\dsig11\ECKeyValue|
74+
* \SimpeSAML\XML\SerializableElementInterface
5975
*/
60-
public function getRSAKeyValue(): ?RSAKeyValue
76+
public function getKeyValue(): RSAKeyValue|DSAKeyValue|ECKeyValue|SerializableElementInterface
6177
{
62-
return $this->RSAKeyValue;
78+
return $this->keyValue;
6379
}
6480

6581

@@ -77,23 +93,20 @@ public static function fromXML(DOMElement $xml): static
7793
Assert::same($xml->localName, 'KeyValue', InvalidDOMElementException::class);
7894
Assert::same($xml->namespaceURI, KeyValue::NS, InvalidDOMElementException::class);
7995

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,
96+
$keyValue = array_merge(
97+
RSAKeyValue::getChildrenOfClass($xml),
98+
DSAKeyValue::getChildrenOfClass($xml),
99+
self::getChildElementsFromXML($xml),
86100
);
87101

88-
$elements = self::getChildElementsFromXML($xml);
89-
Assert::maxCount(
90-
$elements,
102+
Assert::count(
103+
$keyValue,
91104
1,
92-
'A <ds:KeyValue> can contain exactly one element in namespace ##other',
105+
'A <ds:KeyValue> must contain exactly one child element',
93106
TooManyElementsException::class,
94107
);
95108

96-
return new static(array_pop($RSAKeyValue), array_pop($elements));
109+
return new static(array_pop($keyValue));
97110
}
98111

99112

@@ -107,13 +120,7 @@ public function toXML(?DOMElement $parent = null): DOMElement
107120
{
108121
$e = $this->instantiateParentElement($parent);
109122

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

118125
return $e;
119126
}

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