Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use saml 1.1 and ws-security libraries to build metadata and messages #22

Merged
merged 47 commits into from
Oct 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
47 commits
Select commit Hold shift + click to select a range
7d98fbd
Migrate module to use saml11 + ws-security libraries
tvdijen Mar 24, 2024
f018c8e
Drop dependency on robrichards/xmlseclibs
tvdijen Mar 28, 2024
9b7e138
allow hosted metadata again. This lets me see it in the admin ui (#18)
monkeyiq Apr 29, 2024
ec55f61
Depend on feature-branch of SSP
tvdijen Mar 30, 2024
76bf046
Fix coding style
tvdijen Apr 29, 2024
6a20079
A hook to generate metadata (#20)
monkeyiq May 8, 2024
5b72496
Fix constant
tvdijen Aug 31, 2024
1b52f19
Fix unit test
tvdijen Aug 31, 2024
b10eb1c
Back to SSP release-branch
tvdijen Aug 31, 2024
8f5cd02
Fix constants
tvdijen Aug 31, 2024
9cdf489
Fix pre-filled username
tvdijen Sep 1, 2024
dd24db5
Attempted fix for installing on SSP v2.3
tvdijen Sep 1, 2024
233ed25
Fixes
tvdijen Sep 1, 2024
4d5b782
Do not rely on SimpleSAMLphp for metadata-building
tvdijen Sep 1, 2024
5d69d3d
Fix
tvdijen Sep 1, 2024
d3bd32f
Introduce a clokc
tvdijen Sep 1, 2024
2f890b8
Generate ID
tvdijen Sep 1, 2024
f128f1a
Migrate module to use saml11 + ws-security libraries
tvdijen Mar 24, 2024
c8d890a
Drop dependency on robrichards/xmlseclibs
tvdijen Mar 28, 2024
21701e3
allow hosted metadata again. This lets me see it in the admin ui (#18)
monkeyiq Apr 29, 2024
05f50a1
Depend on feature-branch of SSP
tvdijen Mar 30, 2024
b4c9214
Fix coding style
tvdijen Apr 29, 2024
332e6d8
A hook to generate metadata (#20)
monkeyiq May 8, 2024
2ae0e4e
Fix constant
tvdijen Aug 31, 2024
bb2cb62
Fix unit test
tvdijen Aug 31, 2024
dc58209
Back to SSP release-branch
tvdijen Aug 31, 2024
b77a617
Fix constants
tvdijen Aug 31, 2024
11824b8
Attempted fix for installing on SSP v2.3
tvdijen Sep 1, 2024
162d155
Fixes
tvdijen Sep 1, 2024
c7e80ba
Feature/standalone metadata (#21)
tvdijen Sep 1, 2024
07a817c
Cleanup
tvdijen Sep 1, 2024
f4aa39b
Fix syntax error
tvdijen Sep 3, 2024
42572df
Fix undeclared var
tvdijen Sep 23, 2024
3c38b61
Use the correct version of WS addressing to mimic ADFS
tvdijen Sep 23, 2024
0710f56
Drop dependency on saml2-lib for constants
tvdijen Sep 23, 2024
bc8ea2b
Fix namespace
tvdijen Sep 23, 2024
f7717bd
Update ws-security lib
tvdijen Oct 2, 2024
bb735c1
Fix dependencies
tvdijen Oct 2, 2024
2741e5a
Fix codesniffer issue
tvdijen Oct 2, 2024
e82387a
Fix constants
tvdijen Oct 2, 2024
fae939a
Fix signed assertion
tvdijen Oct 8, 2024
eb2f705
Don't try to sign if we don't have keys
tvdijen Oct 15, 2024
040d61a
Merge branch 'feature/standalone-metadata' into feature/library
tvdijen Oct 15, 2024
b8ecaf9
Merge branch 'master' into feature/library
tvdijen Oct 15, 2024
8f9a9d4
Fix
tvdijen Oct 17, 2024
9612d64
Fix
tvdijen Oct 22, 2024
0be6586
Allow both active and passive flow (GET or POST)
tvdijen Oct 22, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 11 additions & 8 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@
"composer/package-versions-deprecated": true,
"dealerdirect/phpcodesniffer-composer-installer": true,
"phpstan/extension-installer": true,
"simplesamlphp/composer-module-installer": true
"simplesamlphp/composer-module-installer": true,
"simplesamlphp/composer-xmlprovider-installer": true
}
},
"autoload": {
Expand All @@ -36,18 +37,20 @@
"php": "^8.1",
"ext-dom": "*",

"robrichards/xmlseclibs": "^3.1",
"simplesamlphp/assert": "^1.0",
"beste/clock": "^3.0",
"psr/clock": "^1.0",
"simplesamlphp/assert": "^1.1",
"simplesamlphp/saml11": "^1.0",
"simplesamlphp/saml2": "^5@dev",
"simplesamlphp/simplesamlphp": "^2.0",
"simplesamlphp/xml-common": "^1.12",
"simplesamlphp/ws-security": "^1.0",
"simplesamlphp/simplesamlphp": "dev-feature/adfs-upgrade",
"simplesamlphp/ws-security": "^1.6",
"simplesamlphp/xml-common": "^1.16",
"simplesamlphp/xml-security": "^1.9",
"symfony/http-foundation": "^6.4"
},
"require-dev": {
"simplesamlphp/simplesamlphp-test-framework": "^1.5",
"simplesamlphp/xml-security": "^1.6"
"simplesamlphp/simplesamlphp-test-framework": "^1.6",
"simplesamlphp/xml-security": "^1.7"
},
"support": {
"issues": "https://github.com/simplesamlphp/simplesamlphp-module-adfs/issues",
Expand Down
33 changes: 33 additions & 0 deletions hooks/hook_generate_metadata.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
<?php

declare(strict_types=1);

use SimpleSAML\Assert\Assert;
use SimpleSAML\SAML2\Constants as C;
use SimpleSAML\{Configuration, Utils, Logger, Module};
use SimpleSAML\Locale\Translate;
use SimpleSAML\Metadata\MetaDataStorageHandler;

function adfs_hook_generate_metadata(array &$hookinfo): void
{
if ($hookinfo['set'] === 'adfs-idp-hosted') {
$property = $hookinfo['property'];
$endpoint = Module::getModuleURL('adfs/idp/prp.php');

switch ($property) {
case 'SingleSignOnService':
$hookinfo['result'] = $endpoint;
break;
case 'SingleSignOnServiceBinding':
$hookinfo['result'] = C::BINDING_HTTP_REDIRECT;
break;
case 'SingleLogoutService':
$hookinfo['result'] = $endpoint;
break;
case 'SingleLogoutServiceBinding':
$hookinfo['result'] = C::BINDING_HTTP_REDIRECT;
break;
}
}
}

4 changes: 2 additions & 2 deletions routing/routes/routes.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ adfs-prp:
defaults: {
_controller: 'SimpleSAML\Module\adfs\Controller\Adfs::prp'
}
methods: [GET]
methods: [GET, POST]

adfs-metadata-legacy:
path: /idp/metadata.php
Expand All @@ -26,4 +26,4 @@ adfs-prp-legacy:
defaults: {
_controller: 'SimpleSAML\Module\adfs\Controller\Adfs::prp'
}
methods: [GET]
methods: [GET, POST]
155 changes: 16 additions & 139 deletions src/Controller/Adfs.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,11 @@
namespace SimpleSAML\Module\adfs\Controller;

use Exception;
use SimpleSAML\Assert\Assert;
use SimpleSAML\Configuration;
use SimpleSAML\{Configuration, IdP, Logger, Metadata, Module, Session, Utils};
use SimpleSAML\Error as SspError;
use SimpleSAML\IdP;
use SimpleSAML\Logger;
use SimpleSAML\Metadata;
use SimpleSAML\Module;
use SimpleSAML\Module\adfs\IdP\ADFS as ADFS_IDP;
use SimpleSAML\SAML2\Constants;
use SimpleSAML\Session;
use SimpleSAML\Utils;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\HttpFoundation\StreamedResponse;
use SimpleSAML\Module\adfs\IdP\MetadataBuilder;
use Symfony\Component\HttpFoundation\{Request, Response, StreamedResponse};

/**
* Controller class for the adfs module.
Expand Down Expand Up @@ -50,7 +41,7 @@
public function __construct(Configuration $config, Session $session)
{
$this->config = $config;
$this->metadata = Metadata\MetaDataStorageHandler::getMetadataHandler($config);

Check failure on line 44 in src/Controller/Adfs.php

View workflow job for this annotation

GitHub Actions / Quality control

TooManyArguments

src/Controller/Adfs.php:44:27: TooManyArguments: Too many arguments for SimpleSAML\Metadata\MetaDataStorageHandler::getMetadataHandler - expecting 0 but saw 1 (see https://psalm.dev/026)

Check failure on line 44 in src/Controller/Adfs.php

View workflow job for this annotation

GitHub Actions / Quality control

TooManyArguments

src/Controller/Adfs.php:44:27: TooManyArguments: Too many arguments for SimpleSAML\Metadata\MetaDataStorageHandler::getMetadataHandler - expecting 0 but saw 1 (see https://psalm.dev/026)
$this->session = $session;
$this->cryptoUtils = new Utils\Crypto();
}
Expand Down Expand Up @@ -78,134 +69,20 @@
} else {
$idpentityid = $this->metadata->getMetaDataCurrentEntityID('adfs-idp-hosted');
}
$idpmeta = $this->metadata->getMetaDataConfig($idpentityid, 'adfs-idp-hosted');

Check failure on line 72 in src/Controller/Adfs.php

View workflow job for this annotation

GitHub Actions / Quality control

ArgumentTypeCoercion

src/Controller/Adfs.php:72:59: ArgumentTypeCoercion: Argument 1 of SimpleSAML\Metadata\MetaDataStorageHandler::getMetaDataConfig expects string, but parent type null|scalar provided (see https://psalm.dev/193)

Check failure on line 72 in src/Controller/Adfs.php

View workflow job for this annotation

GitHub Actions / Quality control

PossiblyNullArgument

src/Controller/Adfs.php:72:59: PossiblyNullArgument: Argument 1 of SimpleSAML\Metadata\MetaDataStorageHandler::getMetaDataConfig cannot be null, possibly null value provided (see https://psalm.dev/078)

Check failure on line 72 in src/Controller/Adfs.php

View workflow job for this annotation

GitHub Actions / Quality control

ArgumentTypeCoercion

src/Controller/Adfs.php:72:59: ArgumentTypeCoercion: Argument 1 of SimpleSAML\Metadata\MetaDataStorageHandler::getMetaDataConfig expects string, but parent type null|scalar provided (see https://psalm.dev/193)

Check failure on line 72 in src/Controller/Adfs.php

View workflow job for this annotation

GitHub Actions / Quality control

PossiblyNullArgument

src/Controller/Adfs.php:72:59: PossiblyNullArgument: Argument 1 of SimpleSAML\Metadata\MetaDataStorageHandler::getMetaDataConfig cannot be null, possibly null value provided (see https://psalm.dev/078)

$availableCerts = [];
$keys = [];
$certInfo = $this->cryptoUtils->loadPublicKey($idpmeta, false, 'new_');
$builder = new MetadataBuilder($this->config, $idpmeta);

if ($certInfo !== null) {
$availableCerts['new_idp.crt'] = $certInfo;
$keys[] = [
'type' => 'X509Certificate',
'signing' => true,
'encryption' => true,
'X509Certificate' => $certInfo['certData'],
];
$hasNewCert = true;
} else {
$hasNewCert = false;
}

/** @var array $certInfo */
$certInfo = $this->cryptoUtils->loadPublicKey($idpmeta, true);
$availableCerts['idp.crt'] = $certInfo;
$keys[] = [
'type' => 'X509Certificate',
'signing' => true,
'encryption' => ($hasNewCert ? false : true),
'X509Certificate' => $certInfo['certData'],
];

if ($idpmeta->hasValue('https.certificate')) {
/** @var array $httpsCert */
$httpsCert = $this->cryptoUtils->loadPublicKey($idpmeta, true, 'https.');
Assert::keyExists($httpsCert, 'certData');
$availableCerts['https.crt'] = $httpsCert;
$keys[] = [
'type' => 'X509Certificate',
'signing' => true,
'encryption' => false,
'X509Certificate' => $httpsCert['certData'],
];
}

$adfs_service_location = Module::getModuleURL('adfs') . '/idp/prp.php';
$metaArray = [
'metadata-set' => 'adfs-idp-remote',
'entityid' => $idpentityid,
'SingleSignOnService' => [
0 => [
'Binding' => Constants::BINDING_HTTP_REDIRECT,
'Location' => $adfs_service_location,
],
],
'SingleLogoutService' => [
0 => [
'Binding' => Constants::BINDING_HTTP_REDIRECT,
'Location' => $adfs_service_location,
],
],
];

if (count($keys) === 1) {
$metaArray['certData'] = $keys[0]['X509Certificate'];
} else {
$metaArray['keys'] = $keys;
}

$metaArray['NameIDFormat'] = $idpmeta->getOptionalString(
'NameIDFormat',
Constants::NAMEID_TRANSIENT,
);

if ($idpmeta->hasValue('OrganizationName')) {
$metaArray['OrganizationName'] = $idpmeta->getLocalizedString('OrganizationName');
$metaArray['OrganizationDisplayName'] = $idpmeta->getOptionalLocalizedString(
'OrganizationDisplayName',
$metaArray['OrganizationName'],
);

if (!$idpmeta->hasValue('OrganizationURL')) {
throw new SspError\Exception('If OrganizationName is set, OrganizationURL must also be set.');
}
$metaArray['OrganizationURL'] = $idpmeta->getLocalizedString('OrganizationURL');
}

if ($idpmeta->hasValue('scope')) {
$metaArray['scope'] = $idpmeta->getArray('scope');
}

if ($idpmeta->hasValue('EntityAttributes')) {
$metaArray['EntityAttributes'] = $idpmeta->getArray('EntityAttributes');
}

if ($idpmeta->hasValue('UIInfo')) {
$metaArray['UIInfo'] = $idpmeta->getArray('UIInfo');
}
$document = $builder->buildDocument()->toXML();
// Some products like DirX are known to break on pretty-printed XML
$document->ownerDocument->formatOutput = false;
$document->ownerDocument->encoding = 'UTF-8';

if ($idpmeta->hasValue('DiscoHints')) {
$metaArray['DiscoHints'] = $idpmeta->getArray('DiscoHints');
}

if ($idpmeta->hasValue('RegistrationInfo')) {
$metaArray['RegistrationInfo'] = $idpmeta->getArray('RegistrationInfo');
}

$metaBuilder = new Metadata\SAMLBuilder($idpentityid);
$metaBuilder->addSecurityTokenServiceType($metaArray);
$metaBuilder->addOrganizationInfo($metaArray);
$technicalContactEmail = $this->config->getOptionalString('technicalcontact_email', null);
if ($technicalContactEmail !== null && $technicalContactEmail !== '[email protected]') {
$metaBuilder->addContact(Utils\Config\Metadata::getContact([
'emailAddress' => $technicalContactEmail,
'givenName' => $this->config->getOptionalString('technicalcontact_name', null),
'contactType' => 'technical',
]));
}
$metaxml = $metaBuilder->getEntityDescriptorText();

// sign the metadata if enabled
$metaxml = Metadata\Signer::sign($metaxml, $idpmeta->toArray(), 'ADFS IdP');
$metaxml = $document->ownerDocument->saveXML();

$response = new Response();
$response->setEtag(hash('sha256', $metaxml));
$response->setCache([
'no_cache' => $protectedMetadata === true,
'public' => $protectedMetadata === false,
'private' => $protectedMetadata === true,
]);

$response->setPublic();
if ($response->isNotModified($request)) {
return $response;
}
Expand All @@ -229,10 +106,10 @@
Logger::info('ADFS - IdP.prp: Accessing ADFS IdP endpoint prp');

$idpEntityId = $this->metadata->getMetaDataCurrentEntityID('adfs-idp-hosted');
$idp = IdP::getById($this->config, 'adfs:' . $idpEntityId);
$idp = IdP::getById('adfs:' . $idpEntityId);

if ($request->query->has('wa')) {
$wa = $request->query->get('wa');
if ($request->get('wa', null) !== null) {

Check failure on line 111 in src/Controller/Adfs.php

View workflow job for this annotation

GitHub Actions / Quality control

InternalMethod

src/Controller/Adfs.php:111:23: InternalMethod: The method Symfony\Component\HttpFoundation\Request::get is internal to Symfony but called from SimpleSAML\Module\adfs\Controller\Adfs::prp (see https://psalm.dev/175)

Check failure on line 111 in src/Controller/Adfs.php

View workflow job for this annotation

GitHub Actions / Quality control

InternalMethod

src/Controller/Adfs.php:111:23: InternalMethod: The method Symfony\Component\HttpFoundation\Request::get is internal to Symfony but called from SimpleSAML\Module\adfs\Controller\Adfs::prp (see https://psalm.dev/175)
$wa = $request->get('wa');

Check warning on line 112 in src/Controller/Adfs.php

View workflow job for this annotation

GitHub Actions / Quality control

MixedAssignment

src/Controller/Adfs.php:112:13: MixedAssignment: Unable to determine the type that $wa is being assigned to (see https://psalm.dev/032)

Check failure on line 112 in src/Controller/Adfs.php

View workflow job for this annotation

GitHub Actions / Quality control

InternalMethod

src/Controller/Adfs.php:112:29: InternalMethod: The method Symfony\Component\HttpFoundation\Request::get is internal to Symfony but called from SimpleSAML\Module\adfs\Controller\Adfs::prp (see https://psalm.dev/175)

Check warning on line 112 in src/Controller/Adfs.php

View workflow job for this annotation

GitHub Actions / Quality control

MixedAssignment

src/Controller/Adfs.php:112:13: MixedAssignment: Unable to determine the type that $wa is being assigned to (see https://psalm.dev/032)

Check failure on line 112 in src/Controller/Adfs.php

View workflow job for this annotation

GitHub Actions / Quality control

InternalMethod

src/Controller/Adfs.php:112:29: InternalMethod: The method Symfony\Component\HttpFoundation\Request::get is internal to Symfony but called from SimpleSAML\Module\adfs\Controller\Adfs::prp (see https://psalm.dev/175)
if ($wa === 'wsignout1.0') {
return new StreamedResponse(
function () use ($idp) {
Expand All @@ -243,18 +120,18 @@
return ADFS_IDP::receiveAuthnRequest($request, $idp);
}
throw new SspError\BadRequest("Unsupported value for 'wa' specified in request.");
} elseif ($request->query->has('assocId')) {
} elseif ($request->get('assocId', null) !== null) {

Check failure on line 123 in src/Controller/Adfs.php

View workflow job for this annotation

GitHub Actions / Quality control

InternalMethod

src/Controller/Adfs.php:123:29: InternalMethod: The method Symfony\Component\HttpFoundation\Request::get is internal to Symfony but called from SimpleSAML\Module\adfs\Controller\Adfs::prp (see https://psalm.dev/175)

Check failure on line 123 in src/Controller/Adfs.php

View workflow job for this annotation

GitHub Actions / Quality control

InternalMethod

src/Controller/Adfs.php:123:29: InternalMethod: The method Symfony\Component\HttpFoundation\Request::get is internal to Symfony but called from SimpleSAML\Module\adfs\Controller\Adfs::prp (see https://psalm.dev/175)
// logout response from ADFS SP
// Association ID of the SP that sent the logout response
$assocId = $request->query->get('assocId');
$assocId = $request->get('assocId');

Check warning on line 126 in src/Controller/Adfs.php

View workflow job for this annotation

GitHub Actions / Quality control

MixedAssignment

src/Controller/Adfs.php:126:13: MixedAssignment: Unable to determine the type that $assocId is being assigned to (see https://psalm.dev/032)

Check failure on line 126 in src/Controller/Adfs.php

View workflow job for this annotation

GitHub Actions / Quality control

InternalMethod

src/Controller/Adfs.php:126:34: InternalMethod: The method Symfony\Component\HttpFoundation\Request::get is internal to Symfony but called from SimpleSAML\Module\adfs\Controller\Adfs::prp (see https://psalm.dev/175)

Check warning on line 126 in src/Controller/Adfs.php

View workflow job for this annotation

GitHub Actions / Quality control

MixedAssignment

src/Controller/Adfs.php:126:13: MixedAssignment: Unable to determine the type that $assocId is being assigned to (see https://psalm.dev/032)

Check failure on line 126 in src/Controller/Adfs.php

View workflow job for this annotation

GitHub Actions / Quality control

InternalMethod

src/Controller/Adfs.php:126:34: InternalMethod: The method Symfony\Component\HttpFoundation\Request::get is internal to Symfony but called from SimpleSAML\Module\adfs\Controller\Adfs::prp (see https://psalm.dev/175)
// Data that was sent in the logout request to the SP. Can be null
$relayState = $request->query->get('relayState');
$relayState = $request->get('relayState');

Check warning on line 128 in src/Controller/Adfs.php

View workflow job for this annotation

GitHub Actions / Quality control

MixedAssignment

src/Controller/Adfs.php:128:13: MixedAssignment: Unable to determine the type that $relayState is being assigned to (see https://psalm.dev/032)

Check failure on line 128 in src/Controller/Adfs.php

View workflow job for this annotation

GitHub Actions / Quality control

InternalMethod

src/Controller/Adfs.php:128:37: InternalMethod: The method Symfony\Component\HttpFoundation\Request::get is internal to Symfony but called from SimpleSAML\Module\adfs\Controller\Adfs::prp (see https://psalm.dev/175)

Check warning on line 128 in src/Controller/Adfs.php

View workflow job for this annotation

GitHub Actions / Quality control

MixedAssignment

src/Controller/Adfs.php:128:13: MixedAssignment: Unable to determine the type that $relayState is being assigned to (see https://psalm.dev/032)

Check failure on line 128 in src/Controller/Adfs.php

View workflow job for this annotation

GitHub Actions / Quality control

InternalMethod

src/Controller/Adfs.php:128:37: InternalMethod: The method Symfony\Component\HttpFoundation\Request::get is internal to Symfony but called from SimpleSAML\Module\adfs\Controller\Adfs::prp (see https://psalm.dev/175)
// null on success, or an instance of a \SimpleSAML\Error\Exception on failure.
$logoutError = null;

return new StreamedResponse(
function () use ($idp, /** @scrutinizer ignore-type */ $assocId, $relayState, $logoutError) {
$idp->handleLogoutResponse($assocId, $relayState, $logoutError);

Check warning on line 134 in src/Controller/Adfs.php

View workflow job for this annotation

GitHub Actions / Quality control

MixedArgument

src/Controller/Adfs.php:134:48: MixedArgument: Argument 1 of SimpleSAML\IdP::handleLogoutResponse cannot be mixed, expecting string (see https://psalm.dev/030)

Check warning on line 134 in src/Controller/Adfs.php

View workflow job for this annotation

GitHub Actions / Quality control

MixedArgument

src/Controller/Adfs.php:134:58: MixedArgument: Argument 2 of SimpleSAML\IdP::handleLogoutResponse cannot be mixed, expecting null|string (see https://psalm.dev/030)

Check warning on line 134 in src/Controller/Adfs.php

View workflow job for this annotation

GitHub Actions / Quality control

MixedArgument

src/Controller/Adfs.php:134:48: MixedArgument: Argument 1 of SimpleSAML\IdP::handleLogoutResponse cannot be mixed, expecting string (see https://psalm.dev/030)

Check warning on line 134 in src/Controller/Adfs.php

View workflow job for this annotation

GitHub Actions / Quality control

MixedArgument

src/Controller/Adfs.php:134:58: MixedArgument: Argument 2 of SimpleSAML\IdP::handleLogoutResponse cannot be mixed, expecting null|string (see https://psalm.dev/030)
},
);
}
Expand Down
Loading
Loading