Skip to content

Commit

Permalink
Merge pull request #752 from OpenConext/feature/serialized-coins
Browse files Browse the repository at this point in the history
Store coin properties in a single column
  • Loading branch information
pablothedude authored Sep 10, 2019
2 parents 7049fee + 1d5b9c1 commit ff01ed8
Show file tree
Hide file tree
Showing 35 changed files with 875 additions and 126 deletions.
2 changes: 0 additions & 2 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,6 @@ script:
branches:
only:
- master
- release/5.11
- feature/error-page-styling

after_failure:
- sudo tail -500 /var/log/apache2/error.log
Expand Down
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,11 @@ We will continue to post relevant release notes on the GitHub release page. More

More information about our release strategy can be found in the [Development Guidelines](https://github.com/OpenConext/OpenConext-engineblock/wiki/Development-Guidelines#release-notes) on the EngineBlock wiki.

## 5.12.0
We've changed the way how coin entity properties are stored in the database. As of now, they will be stored in a serialized manner, in a single column. This greatly simplifies adding or removing coins in the future, as this no longer requires database schema changes.

* Store coin properties in a single column #752

## 5.11.3
The footer links have been made configurable to a greater extent in this release.

Expand Down
16 changes: 16 additions & 0 deletions UPGRADING.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,21 @@
# UPGRADE NOTES

## 5.11 > 5.12.0

### Serialised column cleanup
In version 5.12.0 multiple `coin` columns are serialised into a single column with serialised data.
This change will allow easier maintenance because then no migrations are needed when adding or removing a `coin`.
There is no migration to populate the old `coin` columns to the new serialised column while this is needed to let EB function properly with the new changes.
Therefore you should push the data from Manage after you have updated the codebase and ran the `Version20190703235333`.

Be aware that you need to be logged in into manage to push the data after updating the codebase and database schema.

In order to let this work you need to do the following:
1. Login into manage
1. Update codebase
1. Run migrations
1. Push metadata

## 5.10 -> 5.11

### Improved error feedback
Expand Down
1 change: 1 addition & 0 deletions app/config/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ doctrine:
types:
engineblock_collab_person_id: OpenConext\EngineBlockBundle\Doctrine\Type\CollabPersonIdType
engineblock_collab_person_uuid: OpenConext\EngineBlockBundle\Doctrine\Type\CollabPersonUuidType
engineblock_metadata_coins: OpenConext\EngineBlockBundle\Doctrine\Type\MetadataCoinType
orm:
auto_generate_proxy_classes: "%kernel.debug%"
proxy_dir: '%kernel.cache_dir%/doctrine/orm/Proxies'
Expand Down
1 change: 1 addition & 0 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
},
"require": {
"php": ">=5.6,<7",
"ext-json": "*",
"ext-mbstring": "*",
"beberlei/assert": "^2.6",
"doctrine/doctrine-bundle": "^1.6",
Expand Down
4 changes: 3 additions & 1 deletion composer.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

34 changes: 34 additions & 0 deletions database/DoctrineMigrations/Version20190703235333.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
<?php

namespace OpenConext\EngineBlock\Doctrine\Migrations;

use Doctrine\DBAL\Migrations\AbstractMigration;
use Doctrine\DBAL\Schema\Schema;

/**
* Auto-generated Migration: Please modify to your needs!
*/
class Version20190703235333 extends AbstractMigration
{
/**
* @param Schema $schema
*/
public function up(Schema $schema)
{
// this up() migration is auto-generated, please modify it to your needs
$this->abortIf($this->connection->getDatabasePlatform()->getName() !== 'mysql', 'Migration can only be executed safely on \'mysql\'.');

$this->addSql('ALTER TABLE sso_provider_roles_eb5 ADD coins LONGTEXT NOT NULL COMMENT \'(DC2Type:engineblock_metadata_coins)\'');
}

/**
* @param Schema $schema
*/
public function down(Schema $schema)
{
// this down() migration is auto-generated, please modify it to your needs
$this->abortIf($this->connection->getDatabasePlatform()->getName() !== 'mysql', 'Migration can only be executed safely on \'mysql\'.');

$this->addSql('ALTER TABLE sso_provider_roles_eb5 DROP coins');
}
}
2 changes: 1 addition & 1 deletion library/EngineBlock/Corto/Adapter.php
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ protected function _filterRemoteEntitiesByRequestSp(EngineBlock_Saml2_AuthnReque
$serviceProvider = $repository->fetchServiceProviderByEntityId($request->getIssuer());

if (!$serviceProvider->allowAll) {
if ($serviceProvider->displayUnconnectedIdpsWayf) {
if ($serviceProvider->getCoins()->displayUnconnectedIdpsWayf()) {
$repository->appendVisitor(
new DisableDisallowedEntitiesInWayfVisitor($serviceProvider->allowedIdpEntityIds)
);
Expand Down
6 changes: 3 additions & 3 deletions library/EngineBlock/Corto/Filter/Command/AddGuestStatus.php
Original file line number Diff line number Diff line change
Expand Up @@ -33,18 +33,18 @@ public function execute()
*/
protected function _addIsMemberOfSurfNlAttribute()
{
if ($this->_identityProvider->guestQualifier === IdentityProvider::GUEST_QUALIFIER_ALL) {
if ($this->_identityProvider->getCoins()->guestQualifier() === IdentityProvider::GUEST_QUALIFIER_ALL) {
// All users from this IdP are guests, so no need to add the isMemberOf
return;
}

if ($this->_identityProvider->guestQualifier === IdentityProvider::GUEST_QUALIFIER_NONE) {
if ($this->_identityProvider->getCoins()->guestQualifier() === IdentityProvider::GUEST_QUALIFIER_NONE) {
$this->_setIsMember();
return;
}

$log = EngineBlock_ApplicationSingleton::getLog();
if ($this->_identityProvider->guestQualifier === IdentityProvider::GUEST_QUALIFIER_SOME) {
if ($this->_identityProvider->getCoins()->guestQualifier() === IdentityProvider::GUEST_QUALIFIER_SOME) {
if (isset($this->_responseAttributes[static::URN_SURF_PERSON_AFFILIATION][0])) {
if ($this->_responseAttributes[static::URN_SURF_PERSON_AFFILIATION][0] === 'member') {
$this->_setIsMember();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ public function getResponseAttributes()

public function execute()
{
if ($this->_serviceProvider->skipDenormalization) {
if ($this->_serviceProvider->getCoins()->skipDenormalization()) {
return;
}

Expand Down
2 changes: 1 addition & 1 deletion library/EngineBlock/Corto/Filter/Command/EnforcePolicy.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ public function execute()
$serviceProvider = $this->_serviceProvider;
}

if (!$serviceProvider->policyEnforcementDecisionRequired) {
if (!$serviceProvider->getCoins()->policyEnforcementDecisionRequired()) {
return;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@ public function execute()
{
// ServiceRegistry override of SchacHomeOrganization, set it and skip validation
$excluded = array();
if ($this->_identityProvider->schacHomeOrganization) {
if ($this->_identityProvider->getCoins()->schacHomeOrganization()) {
$this->_responseAttributes[self::URN_MACE_TERENA_SCHACHOMEORG] = array(
$this->_identityProvider->schacHomeOrganization
$this->_identityProvider->getCoins()->schacHomeOrganization()
);
$excluded[] = static::URN_MACE_TERENA_SCHACHOMEORG;
}
Expand Down
4 changes: 2 additions & 2 deletions library/EngineBlock/Corto/Module/Bindings.php
Original file line number Diff line number Diff line change
Expand Up @@ -568,7 +568,7 @@ public function send(
$sspMessage->setCertificates(array($keyPair->getCertificate()->toPem()));
$sspMessage->setSignatureKey(
$keyPair->getPrivateKey()->toXmlSecurityKey(
$remoteEntity->signatureMethod
$remoteEntity->getCoins()->signatureMethod()
)
);
}
Expand All @@ -588,7 +588,7 @@ public function send(
// This suffices for most SP's. However, some SP's require the outer Response element to be signed directly.
// If configured to do so for that SP, Engineblock will sign the outer response element in addition to the signed Assertion element.
// It might make sense in the future to always sign both, if it has been shown that this causes no problems.
if ($remoteEntity instanceof ServiceProvider && $remoteEntity->signResponse) {
if ($remoteEntity instanceof ServiceProvider && $remoteEntity->getCoins()->signResponse()) {
$messageElement = $sspMessage->toSignedXML();
} else {
$messageElement = $sspMessage->toUnsignedXML();
Expand Down
2 changes: 1 addition & 1 deletion library/EngineBlock/Corto/Module/Service/IdpsMetadata.php
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public function serve($serviceName)
continue;
}

if ($entity->hidden) {
if ($entity->getCoins()->hidden()) {
continue;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ public function serve($serviceName)
private function isConsentDisabled(array $serviceProviders, IdentityProvider $identityProvider)
{
foreach ($serviceProviders as $serviceProvider) {
if (!$serviceProvider->isConsentRequired) {
if (!$serviceProvider->getCoins()->isConsentRequired()) {
return true;
}

Expand Down
6 changes: 3 additions & 3 deletions library/EngineBlock/Corto/Module/Service/SingleSignOn.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ public function serve($serviceName)

// When dealing with an SP that acts as a trusted proxy, we should perform SSO on the proxying SP and not the
// proxy itself.
if ($sp->isTrustedProxy) {
if ($sp->getCoins()->isTrustedProxy()) {
// Overwrite the trusted proxy SP instance with that of the SP that uses the trusted proxy.
$sp = $this->findOriginalServiceProvider($request, $log);
}
Expand Down Expand Up @@ -367,7 +367,7 @@ protected function _showWayf(EngineBlock_Saml2_AuthnRequestAnnotationDecorator $
'backLink' => $container->isUiOptionReturnToSpActive(),
'cutoffPointForShowingUnfilteredIdps' => $container->getCutoffPointForShowingUnfilteredIdps(),
'rememberChoiceFeature' => $rememberChoiceFeature,
'showRequestAccess' => $serviceProvider->displayUnconnectedIdpsWayf,
'showRequestAccess' => $serviceProvider->getCoins()->displayUnconnectedIdpsWayf(),
'requestId' => $request->getId(),
'serviceProvider' => $serviceProvider,
'idpList' => $idpList,
Expand Down Expand Up @@ -420,7 +420,7 @@ protected function _transformIdpsForWayf(array $idpEntityIds, $isDebugRequest, $
continue;
}

if ($identityProvider->hidden) {
if ($identityProvider->getCoins()->hidden()) {
continue;
}

Expand Down
2 changes: 1 addition & 1 deletion library/EngineBlock/Corto/ProxyServer.php
Original file line number Diff line number Diff line change
Expand Up @@ -476,7 +476,7 @@ public function createEnhancedResponse(
// Unless of course we are in 'stealth' / transparent mode, in which case,
// pretend to be the Identity Provider.
$serviceProvider = $this->getRepository()->fetchServiceProviderByEntityId($request->getIssuer());
$mustProxyTransparently = ($request->isTransparent() || $serviceProvider->isTransparentIssuer);
$mustProxyTransparently = ($request->isTransparent() || $serviceProvider->getCoins()->isTransparentIssuer());
if (!$this->isInProcessingMode() && $mustProxyTransparently) {
$newResponse->setIssuer($newResponse->getOriginalIssuer());
$newAssertion->setIssuer($newResponse->getOriginalIssuer());
Expand Down
2 changes: 1 addition & 1 deletion library/EngineBlock/Saml2/AuthnRequestFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public static function createFromRequest(
$sspRequest->setIssuer($server->getUrl('spMetadataService'));
$sspRequest->setNameIdPolicy($nameIdPolicy);

if (empty($idpMetadata->disableScoping)) {
if (empty($idpMetadata->getCoins()->disableScoping())) {
// Copy over the Idps that are allowed to answer this request.
$sspRequest->setIDPList($originalRequest->getIDPList());

Expand Down
6 changes: 3 additions & 3 deletions library/EngineBlock/SamlHelper.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ class EngineBlock_SamlHelper
public static function doRemoteEntitiesRequireAdditionalLogging(array $entities)
{
return array_reduce($entities, function($carry, AbstractRole $entity) {
return $carry | $entity->additionalLogging;
return $carry | $entity->getCoins()->additionalLogging();
}, false);
}

Expand Down Expand Up @@ -75,7 +75,7 @@ public static function findRequesterServiceProvider(
MetadataRepositoryInterface $repository,
\Psr\Log\LoggerInterface $logger = null
) {
if (!$serviceProvider->isTrustedProxy) {
if (!$serviceProvider->getCoins()->isTrustedProxy()) {
return null;
}

Expand All @@ -89,7 +89,7 @@ public static function findRequesterServiceProvider(
$lastRequesterEntityId = end($requesterIds);

if (!$lastRequesterEntityId) {
if ($serviceProvider->requesteridRequired) {
if ($serviceProvider->getCoins()->requesteridRequired()) {
throw new EngineBlock_Exception_UnknownServiceProvider(
$serviceProvider,
'No RequesterID specified'
Expand Down
2 changes: 1 addition & 1 deletion src/OpenConext/EngineBlock/Authentication/Dto/Consent.php
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ function (ContactPerson $contact) {
'en' => $this->serviceProvider->supportUrlEn,
'nl' => $this->serviceProvider->supportUrlNl,
],
'eula_url' => $this->serviceProvider->termsOfServiceUrl,
'eula_url' => $this->serviceProvider->getCoins()->termsOfServiceUrl(),
'support_email' => $supportEmail,
'name_id_format' => $this->serviceProvider->nameIdFormat,
];
Expand Down
Loading

0 comments on commit ff01ed8

Please sign in to comment.