Skip to content

Commit 436b701

Browse files
committed
Improve MDUI Logo usage
The logo is used unsafely in two placed (EnforcePolicy and Mdui) that was addressed here. And then, the test coverage of the Logo element was quite low. And that was also addressed here. See: https://www.pivotaltracker.com/story/show/185540656
1 parent e88cc71 commit 436b701

File tree

4 files changed

+115
-7
lines changed

4 files changed

+115
-7
lines changed

library/EngineBlock/Corto/Filter/Command/EnforcePolicy.php

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -58,10 +58,11 @@ public function execute()
5858
$policyDecision = $pdp->requestDecisionFor($pdpRequest);
5959
// The IdP logo is set after getting the PolicyDecision as it would be inappropriate to inject this into the
6060
// decision request.
61-
$policyDecision->setIdpLogo(
62-
$this->_identityProvider->getMdui()->getLogo()
63-
);
64-
61+
if ($this->_identityProvider->getMdui()->hasLogo()){
62+
$policyDecision->setIdpLogo(
63+
$this->_identityProvider->getMdui()->getLogo()
64+
);
65+
}
6566
$log->debug("Policy Enforcement Point: PDP decision received.");
6667

6768
$pdpLoas = $policyDecision->getStepupObligations();

src/OpenConext/EngineBlock/Metadata/Logo.php

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
namespace OpenConext\EngineBlock\Metadata;
2020

2121
use JsonSerializable;
22+
use OpenConext\EngineBlock\Exception\MduiRuntimeException;
2223

2324
/**
2425
* A SAML2 metadata logo.
@@ -40,9 +41,18 @@ public function __construct($url)
4041

4142
public static function fromJson(array $multiLingualElement): MultilingualElement
4243
{
44+
if (!array_key_exists('url', $multiLingualElement)) {
45+
throw new MduiRuntimeException(
46+
'Incomplete MDUI Logo data. The URL is missing while serializing the data from JSON'
47+
);
48+
}
4349
$element = new self($multiLingualElement['url']);
44-
$element->height = $multiLingualElement['height'];
45-
$element->width = $multiLingualElement['width'];
50+
if (array_key_exists('height', $multiLingualElement)) {
51+
$element->height = $multiLingualElement['height'];
52+
}
53+
if (array_key_exists('width', $multiLingualElement)) {
54+
$element->width = $multiLingualElement['width'];
55+
}
4656
return $element;
4757
}
4858

src/OpenConext/EngineBlock/Metadata/Mdui.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ public static function fromJson(string $parsedData): Mdui
126126
if ($parsedData) {
127127
foreach ($parsedData as $elementName => $multiLingualElement) {
128128
// The logo element differs from the other MduiElements, it is constructed in its own fashion
129-
if ($elementName === 'Logo') {
129+
if ($elementName === 'Logo' && array_key_exists('url', $multiLingualElement)) {
130130
$output[$elementName] = Logo::fromJson($multiLingualElement);
131131
continue;
132132
}
Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
<?php
2+
3+
/**
4+
* Copyright 2023 SURFnet B.V.
5+
*
6+
* Licensed under the Apache License, Version 2.0 (the "License");
7+
* you may not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing, software
13+
* distributed under the License is distributed on an "AS IS" BASIS,
14+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
* See the License for the specific language governing permissions and
16+
* limitations under the License.
17+
*/
18+
19+
namespace OpenConext\EngineBlock\Metadata;
20+
21+
use JsonSerializable;
22+
use OpenConext\EngineBlock\Exception\MduiRuntimeException;
23+
use PHPUnit\Framework\TestCase;
24+
25+
class LogoTest extends TestCase
26+
{
27+
/**
28+
* @param string $json
29+
* @dataProvider provideCorrectJson
30+
*/
31+
public function test_create_logo_from_json_happy_flow(string $json): void
32+
{
33+
$jsonData = json_decode($json, true);
34+
$logo = Logo::fromJson($jsonData);
35+
self::assertInstanceOf(Logo::class, $logo);
36+
self::assertInstanceOf(MultilingualElement::class, $logo);
37+
self::assertInstanceOf(JsonSerializable::class, $logo);
38+
self::assertEquals($jsonData['url'], $logo->url);
39+
40+
if (array_key_exists('width', $jsonData)) {
41+
self::assertEquals($jsonData['width'], $logo->width);
42+
}
43+
if (array_key_exists('height', $jsonData)) {
44+
self::assertEquals($jsonData['height'], $logo->height);
45+
}
46+
self::assertIsArray($logo->jsonSerialize());
47+
}
48+
49+
/**
50+
* @param string $json
51+
* @dataProvider provideUrlMissingJson
52+
*/
53+
public function test_create_logo_from_json_requires_url(string $json): void
54+
{
55+
self::expectException(MduiRuntimeException::class);
56+
self::expectExceptionMessage(
57+
'Incomplete MDUI Logo data. The URL is missing while serializing the data from JSON'
58+
);
59+
Logo::fromJson(json_decode($json, true));
60+
}
61+
62+
public function test_logo_can_not_be_translated()
63+
{
64+
self::expectException(MduiRuntimeException::class);
65+
self::expectExceptionMessage('We do not implement the Mdui Logo in a multilingual fashion');
66+
$logo = Logo::fromJson(['url' => 'https://foobar.example.com']);
67+
$logo->translate('en');
68+
}
69+
70+
public function test_primary_language_is_en()
71+
{
72+
$logo = Logo::fromJson(['url' => 'https://foobar.example.com']);
73+
$lang = $logo->getConfiguredLanguages();
74+
self::assertEquals(['en'], $lang);
75+
76+
}
77+
78+
public function provideCorrectJson()
79+
{
80+
return [
81+
'all data present' => ['{"url": "https://logo.example.com/logo.gif", "height": 12, "width": 50}'],
82+
'only url set' => ['{"url": "https://logo.example.com/logo.gif"}'],
83+
'only logo and width' => ['{"url": "https://logo.example.com/logo.gif", "width": 50}'],
84+
'only logo and height' => ['{"url": "https://logo.example.com/logo.gif", "height": 50}'],
85+
'additional data is ignored' => ['{"url": "https://logo.example.com/logo.gif", "height": 50, "widtf": 50}'],
86+
'url is not validated' => ['{"url": "you are elle", "height": 50, "width": 50}'],
87+
];
88+
}
89+
90+
public function provideUrlMissingJson()
91+
{
92+
return [
93+
'url not present' => ['{"height": 12, "width": 50}'],
94+
'url misspelled' => ['{"ulr": "you are elle", "height": 12, "width": 50}'],
95+
];
96+
}
97+
}

0 commit comments

Comments
 (0)