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.

Sign up for GitHub

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

#1 - Allow multiple idp's with the same metadatalink #2

Merged
merged 6 commits into from
Feb 16, 2024
Merged

#1 - Allow multiple idp's with the same metadatalink #2

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
6b43f65
#1 - Changed keyby for idp's to use internal id instead of entryid
Anders-Oerbaek-Pedersen Feb 2, 2024
295edcc
#1 - Changed plugin version
Anders-Oerbaek-Pedersen Feb 2, 2024
2b5ccec
#1 - Removed line accidentially left after initial changes
Anders-Oerbaek-Pedersen Feb 2, 2024
ccf0f43
#1 - Changed use of entityid as array key to id
Anders-Oerbaek-Pedersen Feb 2, 2024
da70f8d
#1 - Fixed issue with IdP matching when updating settings
Anders-Oerbaek-Pedersen Feb 15, 2024
86a5ded
#1 - bump plugin version
Anders-Oerbaek-Pedersen Feb 15, 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
61 changes: 37 additions & 24 deletions classes/admin/setting_idpmetadata.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ private function process_all_idps_metadata($idps) {
$oldidps[$idpentity->metadataurl] = array();
}

$oldidps[$idpentity->metadataurl][$idpentity->entityid] = $idpentity;
$oldidps[$idpentity->metadataurl][$idpentity->id] = $idpentity;
}

foreach ($idps as $idp) {
Expand Down Expand Up @@ -155,31 +155,44 @@ private function process_idp_xml(idp_data $idp, DOMElement $idpelements, DOMXPat
$logo = $logos->item(0)->textContent;
}

if (isset($oldidps[$idp->idpurl][$entityid])) {
$oldidp = $oldidps[$idp->idpurl][$entityid];

if (!empty($idpname) && $oldidp->defaultname !== $idpname) {
$DB->set_field('auth_saml2_idps', 'defaultname', $idpname, array('id' => $oldidp->id));
}

if (!empty($logo) && $oldidp->logo !== $logo) {
$DB->set_field('auth_saml2_idps', 'logo', $logo, array('id' => $oldidp->id));
// Update the IdP if it already exists.
if (array_key_exists($idp->idpurl, $oldidps)) {
foreach ($oldidps[$idp->idpurl] as $idpkey => $oldidp) {
if ($oldidp->entityid !== $entityid) {
continue;
}

if ($oldidp->defaultname !== $idp->idpname) {
continue;
}

if (!empty($idpname) && $oldidp->defaultname !== $idpname) {
$DB->set_field('auth_saml2_idps', 'defaultname', $idpname, array('id' => $oldidp->id));
}

if (!empty($logo) && $oldidp->logo !== $logo) {
$DB->set_field('auth_saml2_idps', 'logo', $logo, array('id' => $oldidp->id));
}

// Remove the IdP from the current array so that we don't delete it later.
unset($oldidps[$idp->idpurl][$idpkey]);

// We found the IdP, so we can return early.
return;
}

// Remove the idp from the current array so that we don't delete it later.
unset($oldidps[$idp->idpurl][$entityid]);
} else {
$newidp = new \stdClass();
$newidp->metadataurl = $idp->idpurl;
$newidp->entityid = $entityid;
$newidp->activeidp = $activedefault;
$newidp->defaultidp = 0;
$newidp->adminidp = 0;
$newidp->defaultname = $idpname;
$newidp->logo = $logo;

$DB->insert_record('auth_saml2_idps', $newidp);
}

// IdP does not exist, so we create a new one.
$newidp = new \stdClass();
$newidp->metadataurl = $idp->idpurl;
$newidp->entityid = $entityid;
$newidp->activeidp = $activedefault;
$newidp->defaultidp = 0;
$newidp->adminidp = 0;
$newidp->defaultname = $idpname;
$newidp->logo = $logo;

$DB->insert_record('auth_saml2_idps', $newidp);
}

/**
Expand Down
16 changes: 8 additions & 8 deletions classes/auth.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,12 +123,12 @@ public function __construct() {
foreach ($idpentities as $idpentity) {
// Set name.
$idpentity->name = empty($idpentity->displayname) ? $idpentity->defaultname : $idpentity->displayname;
$idpentity->md5entityid = md5($idpentity->entityid);
$idpentity->md5id = md5($idpentity->id);
// Set default IdP if we found one.
if ((bool) $idpentity->defaultidp && !isset($this->defaultidp)) {
$this->defaultidp = $idpentity;
}
$this->metadataentities[$idpentity->md5entityid] = $idpentity;
$this->metadataentities[$idpentity->md5id] = $idpentity;
}

// Check if we have mutiple IdPs configured.
Expand Down Expand Up @@ -252,15 +252,15 @@ public function loginpage_idp_list($wantsurl) {
// Moodle Workplace - Check IdP's tenant availability.
// Check if function exists required for Totara 12 compatibility.
if (class_exists(\tool_tenant\local\auth\saml2\manager::class) && !component_class_callback('\tool_tenant\local\auth\saml2\manager',
'issuer_available', [$idp->md5entityid], true)) {
'issuer_available', [$idp->md5id], true)) {
continue;
}

// The wants url may already be routed via login.php so don't re-re-route it.
if (strpos($wantsurl, '/auth/saml2/login.php') !== false) {
$idpurl = new moodle_url($wantsurl);
} else {
$idpurl = new moodle_url('/auth/saml2/login.php', ['wants' => $wantsurl, 'idp' => $idp->md5entityid]);
$idpurl = new moodle_url('/auth/saml2/login.php', ['wants' => $wantsurl, 'idp' => $idp->md5id]);
}
$idpurl->param('passive', 'off');

Expand Down Expand Up @@ -558,7 +558,7 @@ public function saml_login() {
require_once("$CFG->dirroot/login/lib.php");

// Set the default IdP to be the first in the list. Used when dual login is disabled.
$SESSION->saml2idp = reset($this->metadataentities)->md5entityid;
$SESSION->saml2idp = reset($this->metadataentities)->md5id;

// We store the IdP in the session to generate the config/config.php array with the default local SP.
$idpalias = optional_param('idpalias', '', PARAM_TEXT);
Expand All @@ -567,7 +567,7 @@ public function saml_login() {

foreach ($this->metadataentities as $idpentity) {
if ($idpalias == $idpentity->alias) {
$SESSION->saml2idp = $idpentity->md5entityid;
$SESSION->saml2idp = $idpentity->md5id;
$idpfound = true;
break;
}
Expand All @@ -579,7 +579,7 @@ public function saml_login() {
} else if (isset($_GET['idp'])) {
$SESSION->saml2idp = $_GET['idp'];
} else if (!is_null($this->defaultidp)) {
$SESSION->saml2idp = $this->defaultidp->md5entityid;
$SESSION->saml2idp = $this->defaultidp->md5id;
} else if ($this->multiidp) {
// At this stage there is no alias, get-param or default IdP configured.
// On a multi-idp system, now check for any whitelisted IP address redirection.
Expand Down Expand Up @@ -863,7 +863,7 @@ protected function handle_blocked_access() {
protected function check_whitelisted_ip_redirect() {
foreach ($this->metadataentities as $idpentity) {
if (\core\ip_utils::is_ip_in_subnet_list(getremoteaddr(), $idpentity->whitelist ?? '')) {
return $idpentity->md5entityid;
return $idpentity->md5id;
}
}
return false;
Expand Down
2 changes: 1 addition & 1 deletion classes/auto_login.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ protected static function login(\auth_plugin_saml2 $auth) {
$auth = get_auth_plugin('saml2');

// Set the default IdP to be the first in the list. Used when dual login is disabled.
$SESSION->saml2idp = reset($auth->metadataentities)->md5entityid;
$SESSION->saml2idp = reset($auth->metadataentities)->md5id;

// Target URL is normally the same as current page, but if we got redirected to enrol.php
// with a 'wants' URL, then that means if the login is successful we should try again at
Expand Down
2 changes: 1 addition & 1 deletion classes/form/testidpselect.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public function definition() {
$metadataentities = $this->_customdata['metadataentities'];
$selectvalues = [];
foreach ($metadataentities as $idpentity) {
$selectvalues[$idpentity->md5entityid] = "{$idpentity->metadataurl} ({$idpentity->name})";
$selectvalues[$idpentity->md5id] = "{$idpentity->metadataurl} ({$idpentity->name})";
}

$mform->addElement('select', 'idp', get_string('test_auth_button_login', 'auth_saml2'), $selectvalues);
Expand Down
4 changes: 2 additions & 2 deletions locallib.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -427,13 +427,13 @@ function auth_saml2_get_idps($active = false, $asarray = false) {

foreach ($idpentitiesrs as $idpentity) {
$idpentity->name = empty($idpentity->displayname) ? $idpentity->defaultname : $idpentity->displayname;
$idpentity->md5entityid = md5($idpentity->entityid);
$idpentity->md5id = md5($idpentity->id);

if (!isset($idpentities[$idpentity->metadataurl])) {
$idpentities[$idpentity->metadataurl] = [];
}

$idpentities[$idpentity->metadataurl][$idpentity->md5entityid] = ($asarray) ? (array) $idpentity : $idpentity;
$idpentities[$idpentity->metadataurl][$idpentity->md5id] = ($asarray) ? (array) $idpentity : $idpentity;
}

return $idpentities;
Expand Down
6 changes: 3 additions & 3 deletions test.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@

if (empty($SESSION->saml2idp)) {
// Specify the default IdP to use.
$SESSION->saml2idp = reset($saml2auth->metadataentities)->md5entityid;
$SESSION->saml2idp = reset($saml2auth->metadataentities)->md5id;
echo '<p>Setting IdP to default</p>';
}

Expand All @@ -71,8 +71,8 @@
foreach ($saml2auth->metadataentities as $idpentity) {
echo '<hr>';
echo "<h4>IDP: $idpentity->entityid</h4>";
echo "<p>md5: $idpentity->md5entityid</p>";
echo "<p>check: " . md5($idpentity->entityid) . "</p>";
echo "<p>md5: $idpentity->md5id</p>";
echo "<p>check: " . md5($idpentity->id) . "</p>";
}

if ($logout) {
Expand Down
26 changes: 13 additions & 13 deletions tests/auth_test.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,9 +190,9 @@ public function test_class_constructor(): void {
// Name attribute is matching defaultname.
$this->assertEquals($entity1->defaultname, reset($auth->metadataentities)->name);

// Encoded entityid present as an attribute as well as the key.
$this->assertArrayHasKey(md5($entity1->entityid), $auth->metadataentities);
$this->assertEquals(md5($entity1->entityid), reset($auth->metadataentities)->md5entityid);
// Encoded id present as an attribute as well as the key.
$this->assertArrayHasKey(md5($entity1->id), $auth->metadataentities);
$this->assertEquals(md5($entity1->id), reset($auth->metadataentities)->md5id);

// Multiidp flag is false.
$reflector = new \ReflectionClass($auth);
Expand Down Expand Up @@ -232,19 +232,19 @@ public function test_class_constructor(): void {
if (method_exists($this, 'assertEqualsCanonicalizing')) {
// Check entity name.
$this->assertEqualsCanonicalizing(['Login 1', $entity1->defaultname], array_column($auth->metadataentities, 'name'));
// Encoded entityid present as an attribute as well as the key.
$this->assertEqualsCanonicalizing([md5($entity1->entityid), md5($entity3->entityid)],
array_column($auth->metadataentities, 'md5entityid'));
$this->assertEqualsCanonicalizing([md5($entity1->entityid), md5($entity3->entityid)],
// Encoded id present as an attribute as well as the key.
$this->assertEqualsCanonicalizing([md5($entity1->id), md5($entity3->id)],
array_column($auth->metadataentities, 'md5id'));
$this->assertEqualsCanonicalizing([md5($entity1->id), md5($entity3->id)],
array_keys($auth->metadataentities));
} else {
// Check entity name.
$this->assertEquals(['Login 1', $entity1->defaultname],
array_column($auth->metadataentities, 'name'), '', 0, 10, true);
// Encoded entityid present as an attribute as well as the key.
$this->assertEquals([md5($entity1->entityid), md5($entity3->entityid)],
array_column($auth->metadataentities, 'md5entityid'), '', 0, 10, true);
$this->assertEquals([md5($entity1->entityid), md5($entity3->entityid)],
// Encoded id present as an attribute as well as the key.
$this->assertEquals([md5($entity1->id), md5($entity3->id)],
array_column($auth->metadataentities, 'md5id'), '', 0, 10, true);
$this->assertEquals([md5($entity1->id), md5($entity3->id)],
array_keys($auth->metadataentities), '', 0, 10, true);
}

Expand All @@ -258,7 +258,7 @@ public function test_class_constructor(): void {
$property = $reflector->getParentClass()->getProperty('defaultidp');
$property->setAccessible(true);
$this->assertNotNull($property->getValue($auth));
$this->assertEquals($auth->metadataentities[md5($entity3->entityid)], $property->getValue($auth));
$this->assertEquals($auth->metadataentities[md5($entity3->id)], $property->getValue($auth));
}

public function test_loginpage_idp_list(): void {
Expand All @@ -280,7 +280,7 @@ public function test_loginpage_idp_list(): void {
$this->assertInstanceOf(\moodle_url::class, $url);
$this->assertEquals('/moodle/auth/saml2/login.php', $url->get_path());
$this->assertEquals('/', $url->get_param('wants'));
$this->assertEquals(md5($entity1->entityid), $url->get_param('idp'));
$this->assertEquals(md5($entity1->id), $url->get_param('idp'));
$this->assertEquals('off', $url->get_param('passive'));

// Wantsurl is pointing to auth/saml2/login.php.
Expand Down
5 changes: 3 additions & 2 deletions tests/setting_idpmetadata_test.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,9 @@ public function test_it_saves_all_idps_information_from_single_xml() {
foreach ($metadataidps as $metadataurl => $idps) {
self::assertSame('xml', $metadataurl);

$idp1md5 = md5('https://idp1.example.org/idp/shibboleth');
$idp2md5 = md5('https://idp2.example.org/idp/shibboleth');
$idpids = array_values(array_map(fn($idp) => $idp->id, $idps));
$idp1md5 = md5($idpids[0]);
$idp2md5 = md5($idpids[1]);

self::assertTrue(array_key_exists($idp1md5, $idps));
self::assertTrue(array_key_exists($idp2md5, $idps));
Expand Down
4 changes: 2 additions & 2 deletions version.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@

defined('MOODLE_INTERNAL') || die();

$plugin->version = 2023100300; // The current plugin version (Date: YYYYMMDDXX).
$plugin->release = 2023100300; // Match release exactly to version.
$plugin->version = 2024020201; // The current plugin version (Date: YYYYMMDDXX).
$plugin->release = 2024020201; // Match release exactly to version.
$plugin->requires = 2017051509; // Requires PHP 7, 2017051509 = T12. M3.3
// Strictly we require either Moodle 3.5 OR
// we require Totara 3.3, but the version number
Expand Down