From 65cf274d26335bebfbc7082b1e5c67769c09b62d Mon Sep 17 00:00:00 2001 From: Michiel Kodde Date: Mon, 4 Feb 2019 11:46:33 +0100 Subject: [PATCH 1/3] Provide a custom service overview for admin The admin user now is faced with a potential 504 error after logging on to the application. As the service overview page is causing much data to load for the admin user. In production, over 150 SP's are loaded on this page including all their entity information. This does not work out! The easy fix for now is simply rendering a blank page with a small instruction to the admin user, to use the service switcher and switch to the desired service that way. For now the simplest of solutions is chosen (render a different template if you are an admin user. This could and will be improved upon in an upcoming development effort. https://www.pivotaltracker.com/story/show/163691688 --- CHANGELOG.md | 2 ++ .../DashboardBundle/Controller/ServiceController.php | 4 ++++ .../Resources/translations/messages.en.yml | 5 +++++ .../Resources/views/Service/admin_overview.html.twig | 10 ++++++++++ tests/webtests/ServiceOverviewTest.php | 6 ++---- 5 files changed, 23 insertions(+), 4 deletions(-) create mode 100644 src/Surfnet/ServiceProviderDashboard/Infrastructure/DashboardBundle/Resources/views/Service/admin_overview.html.twig diff --git a/CHANGELOG.md b/CHANGELOG.md index fbf615527..b26a94807 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,7 @@ ## Next release +**Improvement** + - Provide a custom service overview for admin #234 ## 2.0.3 diff --git a/src/Surfnet/ServiceProviderDashboard/Infrastructure/DashboardBundle/Controller/ServiceController.php b/src/Surfnet/ServiceProviderDashboard/Infrastructure/DashboardBundle/Controller/ServiceController.php index d54581fec..ef4cd7529 100644 --- a/src/Surfnet/ServiceProviderDashboard/Infrastructure/DashboardBundle/Controller/ServiceController.php +++ b/src/Surfnet/ServiceProviderDashboard/Infrastructure/DashboardBundle/Controller/ServiceController.php @@ -115,6 +115,10 @@ public function overviewAction() return $this->redirectToRoute('service_add'); } + if ($this->isGranted('ROLE_ADMINISTRATOR')) { + return $this->render("@Dashboard/Service/admin_overview.html.twig"); + } + $serviceObjects = []; foreach ($services as $service) { $entityList = $this->entityService->getEntityListForService($service); diff --git a/src/Surfnet/ServiceProviderDashboard/Infrastructure/DashboardBundle/Resources/translations/messages.en.yml b/src/Surfnet/ServiceProviderDashboard/Infrastructure/DashboardBundle/Resources/translations/messages.en.yml index 2918a32fb..f52040b69 100644 --- a/src/Surfnet/ServiceProviderDashboard/Infrastructure/DashboardBundle/Resources/translations/messages.en.yml +++ b/src/Surfnet/ServiceProviderDashboard/Infrastructure/DashboardBundle/Resources/translations/messages.en.yml @@ -310,6 +310,11 @@ privacy.information.snDpaWhyNot: What items of the SURF model Data Processing Ag privacy.information.surfmarketDpaAgreement: Did you agree a Data Processing Agreement with SURFmarket? privacy.information.surfnetDpaAgreement: Are you willing to sign the model SURF Data Processing Agreement? privacy.information.whatData: Describe what (kind of) data is processed in the service, and pay specific attention to possible processing of personal data. +service: + admin_overview: + title: Service overview + introduction: + html: "Please use the service switcher to manage the entities of one of the services." service.create.title: Add new service service.edit.title: Edit service service.delete.title: "Delete service" diff --git a/src/Surfnet/ServiceProviderDashboard/Infrastructure/DashboardBundle/Resources/views/Service/admin_overview.html.twig b/src/Surfnet/ServiceProviderDashboard/Infrastructure/DashboardBundle/Resources/views/Service/admin_overview.html.twig new file mode 100644 index 000000000..3cb7d5192 --- /dev/null +++ b/src/Surfnet/ServiceProviderDashboard/Infrastructure/DashboardBundle/Resources/views/Service/admin_overview.html.twig @@ -0,0 +1,10 @@ +{% extends '::base.html.twig' %} + +{% block body_container %} +

{% block page_heading %}{{ 'service.admin_overview.title'|trans }}{%endblock%}

+ +
+
{{ 'service.admin_overview.introduction.html'|trans|raw }}
+
+ +{% endblock %} \ No newline at end of file diff --git a/tests/webtests/ServiceOverviewTest.php b/tests/webtests/ServiceOverviewTest.php index 4b10ea0f2..9a0e02019 100644 --- a/tests/webtests/ServiceOverviewTest.php +++ b/tests/webtests/ServiceOverviewTest.php @@ -161,16 +161,14 @@ public function test_service_overview_redirects_to_service_add_when_no_service_e public function test_service_overview_shows_message_when_no_service_selected() { - $this->markTestSkipped('TODO: Determine what the Administrator should see when authenticated'); - $this->loadFixtures(); $this->logIn('ROLE_ADMINISTRATOR'); $this->client->request('GET', '/'); $response = $this->client->getResponse(); - $this->assertContains('No service selected', $response->getContent()); - $this->assertContains('Please select a service', $response->getContent()); + $this->assertContains('Service overview', $response->getContent()); + $this->assertContains('Please use the service switcher to manage the entities of one of the services.', $response->getContent()); } From 771ced802616bb200c8ae556341a0264b617d8c7 Mon Sep 17 00:00:00 2001 From: Michiel Kodde Date: Mon, 4 Feb 2019 11:08:36 +0100 Subject: [PATCH 2/3] Correctly publish OIDC redirectUris to Manage The array that is passed along to manage should be a list, not a map. If the php indexes are updated along the way, by adding/removing items from the form. The index order might not be sequentially pristine. This causes JSON encode to generate a map instead of a list. By resetting the list before publishing, this problem should be remedied https://www.pivotaltracker.com/story/show/163646662 --- CHANGELOG.md | 3 +++ .../Application/Metadata/JsonGenerator.php | 1 + .../ServiceProviderDashboard/Domain/Entity/Entity.php | 2 +- tests/unit/Application/Metadata/JsonGeneratorTest.php | 10 +++++----- 4 files changed, 10 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b26a94807..0fcba82af 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,8 @@ ## Next release +**Bugfix** + - Correctly publish OIDC redirect URIs to Manage #233 + **Improvement** - Provide a custom service overview for admin #234 diff --git a/src/Surfnet/ServiceProviderDashboard/Application/Metadata/JsonGenerator.php b/src/Surfnet/ServiceProviderDashboard/Application/Metadata/JsonGenerator.php index ca22b787f..f11f25f01 100644 --- a/src/Surfnet/ServiceProviderDashboard/Application/Metadata/JsonGenerator.php +++ b/src/Surfnet/ServiceProviderDashboard/Application/Metadata/JsonGenerator.php @@ -267,6 +267,7 @@ private function generateOidcClient(Entity $entity) { $metadata['clientId'] = str_replace('://', '@//', $entity->getEntityId()); $metadata['clientSecret'] = $entity->getClientSecret(); + // Reset the redirect URI list in order to get a correct JSON formatting (See #163646662) $metadata['redirectUris'] = $entity->getRedirectUris(); $metadata['grantType'] = $entity->getGrantType()->getGrantType(); $metadata['scope'] = ['openid']; diff --git a/src/Surfnet/ServiceProviderDashboard/Domain/Entity/Entity.php b/src/Surfnet/ServiceProviderDashboard/Domain/Entity/Entity.php index d053b29d7..bdef75dc6 100644 --- a/src/Surfnet/ServiceProviderDashboard/Domain/Entity/Entity.php +++ b/src/Surfnet/ServiceProviderDashboard/Domain/Entity/Entity.php @@ -975,7 +975,7 @@ public function getRedirectUris() */ public function setRedirectUris($redirectUris) { - $this->redirectUris = $redirectUris; + $this->redirectUris = array_values($redirectUris); } /** diff --git a/tests/unit/Application/Metadata/JsonGeneratorTest.php b/tests/unit/Application/Metadata/JsonGeneratorTest.php index 312a87331..1084d7dbe 100644 --- a/tests/unit/Application/Metadata/JsonGeneratorTest.php +++ b/tests/unit/Application/Metadata/JsonGeneratorTest.php @@ -467,10 +467,10 @@ public function test_it_can_build_oidc_entity_data_for_new_entities() 'clientSecret' => 'test', 'redirectUris' => array ( - 0 => 'uri1', - 1 => 'uri2', - 2 => 'uri3', - 3 => 'http://playground-test', + 'uri1', + 'uri2', + 'uri3', + 'http://playground-test', ), 'grantType' => 'implicit', 'scope' => @@ -628,7 +628,7 @@ private function createOidcEntity() $entity->setOrganizationUrlNl('http://orgnl'); $entity->setClientSecret('test'); - $entity->setRedirectUris(['uri1','uri2', 'uri3']); + $entity->setRedirectUris([0 => 'uri1', 2 => 'uri2', 8 => 'uri3']); $entity->setGrantType(new OidcGrantType('implicit')); $entity->setEnablePlayground(true); From 8357ac335aad0ebf72ecf3edfc90498bf12ced74 Mon Sep 17 00:00:00 2001 From: Michiel Kodde Date: Mon, 4 Feb 2019 14:45:24 +0100 Subject: [PATCH 3/3] Update CHANGELOG.md --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0fcba82af..0125f7307 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,7 @@ ## Next release +## 2.0.4 + **Bugfix** - Correctly publish OIDC redirect URIs to Manage #233