From f2d25d35de08ae8c6adba644b587c504fa03fb90 Mon Sep 17 00:00:00 2001 From: Pascal Meunier Date: Fri, 1 May 2026 09:23:25 -0400 Subject: [PATCH] fix Escaping/data corruption; Slow/heavy unfriendly UI. Fix is proper htmlspecialchars escaping, a streaming XMLReader parser over the local federation metadata copy, a simple file-based cache, and a checkbox list UI --- .../shibboleth/assets/js/admin.js | 271 +++++------------- .../shibboleth/fields/institutions.php | 263 +++++++++++------ .../authentication/shibboleth/shibboleth.xml | 2 +- 3 files changed, 250 insertions(+), 286 deletions(-) diff --git a/core/plugins/authentication/shibboleth/assets/js/admin.js b/core/plugins/authentication/shibboleth/assets/js/admin.js index d3430c1c295..d360b3d1fff 100644 --- a/core/plugins/authentication/shibboleth/assets/js/admin.js +++ b/core/plugins/authentication/shibboleth/assets/js/admin.js @@ -22,219 +22,80 @@ jQuery(function($) { jQuery(function($) { $('#jform_params_institutions-lbl').hide(); - var prnt = $('.shibboleth'), - // control values are stored in a JSON string so they fit in the extensions table - serialized = $('.shibboleth input.serialized'), - // initialize from existing params - val = JSON.parse(serialized.val()), - // update hidden input to reflect form state - update = function() { - console.log('upd'); - serialized.val(JSON.stringify(val)); - console.log('update', serialized.val()); - }, - // update active idp list state - updateIdps = function() { - val.activeIdps = []; - var anyInvalid = false; - prnt.find('ul.active li').each(function(_, li) { - addedEntities = {} - var idp = {}, thisInvalid = false; - // copy form data to 'val' - $(li).find('input').each(function(_, inp) { - inp = $(inp); - var name = inp.attr('name'); - idp[name] = inp.val(); - thisInvalid = thisInvalid || (name == 'entity_id' && !idp[name].replace(/\s/g, '')) || (name == 'label' && !idp[name].replace(/\s/g, '')); - anyInvalid = anyInvalid || thisInvalid; - if (name == 'logo') { - //idp.logo_data = inp.data('logo_data'); - } - }); - if (!thisInvalid) { - val.activeIdps.push(idp); - } - }); - if (anyInvalid) { - idpWarning.show(); - } - else { - idpWarning.hide(); - } - // propagate to JSON representation - update(); - }, - idpWarning = $('

Not all ID providers will be saved! Each entry must have an entity ID and a label.

').hide() - ; - // link xml input to JSON encoding - $('.shibboleth input[name="xmlPath"]').change(function() { - val.xmlPath = $(this).val(); - update(); - }); - - // make idp attribute keys slightly more presentable - var keyToLabel = function(str) { - return str[0].toUpperCase() + str.substr(1).replace('_', ' ') + ': '; - }; + var serialized = $('.shibboleth input.serialized'); + if (!serialized.length) { console.log('[shib] no serialized input found'); return; } + console.log('[shib] init, serialized name=', serialized.attr('name')); + console.log('[shib] init, serialized initial value=', serialized.val()); + var val; + try { + val = JSON.parse(serialized.val()); + } catch (e) { + console.error('[shib] failed to parse serialized value:', e, serialized.val()); + val = { xmlPath: '', activeIdps: [] }; + } + console.log('[shib] init, parsed val=', val); - // try to show a preview of the given logo URL - var updateLogo = function(li) { - return; - var logoInp = li.find('input[name="logo"]'), - href = logoInp ? logoInp.val() : null; - if (!href || !href.replace(/s+/g, '')) { - return; - } - var imgData; - li.find('.preview').remove(); - if (href != logoInp.data('orig') || !(imgData = logoInp.data('logo_data'))) { - $.ajax({ - 'url': '/core/plugins/authentication/shibboleth/fields/institutions.php', - 'data': {'img': href}, - 'success': function(res) { - li.append($('').attr('src', res)); - logoInp.data('logo_data', res); - updateIdps(); - }, - 'error': updateIdps - }) - } - else { - li.append($('').attr('src', imgData)); - updateIdps(); - } - }; + // xmlPath field change + $('#shib-xmlpath').on('change', function() { + val.xmlPath = this.value; + serialized.val(JSON.stringify(val)); + console.log('[shib] xmlPath changed; serialized now=', serialized.val()); + }); - // make a new entry in the idp list - var newActiveIdp = function(idp, before) { - var li = $('
  • ') - .append($('')) - .append($('').click(function() { - li.remove(); - updateIdps(); - })) - [before === true ? 'prependTo' : 'appendTo'](existing); - if (before) { - li.animate('pulsate', 'slow'); - } - for (var k in idp) { - if (k === 'logo_data' || k === 'logoData') { - continue; - } - var control = mkInp(k, idp[k]); - if (k == 'logo') { -// control.input.change(function() { -// updateLogo(li); -// }); - } - else { - control.input.change(updateIdps); - } - li.append(control.label); - } -// if (idp.logo_data) { -// li.find('input[name="logo"]').data('logo_data', idp.logo_data); -// } - updateLogo(li); - }; + // Rebuild activeIdps from currently-checked boxes and sync to hidden input + function updateActiveIdps(reason) { + val.activeIdps = []; + $('.shib-idp-checkbox:checked').each(function() { + var entityId = $(this).val(); + var label = $(this).data('label'); + var m = entityId.match(/([^.\/:]+\.[^.\/:]+?)(?:\/|$)/); + val.activeIdps.push({ + entity_id: entityId, + label: label, + host: m ? m[1] : '' + }); + }); + serialized.val(JSON.stringify(val)); + console.log('[shib] updateActiveIdps (' + reason + '): ' + val.activeIdps.length + ' active; hidden input value length=' + serialized.val().length); + console.log('[shib] hidden input current value=', serialized.val()); + } - var normalizeUniversity = function(x) { - if (!x) { - return ''; + // Pre-check boxes for already-active IDPs + var activeIds = {}; + (val.activeIdps || []).forEach(function(idp) { + activeIds[idp.entity_id] = true; + }); + var preChecked = 0; + $('.shib-idp-checkbox').each(function() { + if (activeIds[$(this).val()]) { + $(this).prop('checked', true); + preChecked++; } - return x.toLowerCase().replace(/^((the|of|university|college)\W)+/, ''); - }; + }); + console.log('[shib] pre-checked ' + preChecked + ' of ' + Object.keys(activeIds).length + ' saved active IDPs'); - var addedEntities = {}; - if (val.activeIdps) { - val.activeIdps.forEach(function(idp) { - addedEntities[idp.entity_id] = 1; - }); - } - // list entities read from the shibboleth conf, if any - if (val.xmlRead) { - var ul = $('
      '); - var addAll = $('').click(function(evt) { - evt.stopPropagation(); - ul.find('.add.icon').click(); - setTimeout(function() { - console.log(serialized.val()); - }, 5000); - return false; - }); - prnt.append($('

      ID providers in metadata

      ').append(addAll)); - ul.appendTo(prnt); - val.idps.sort(function(a, b) { - a = normalizeUniversity(a.label); - b = normalizeUniversity(b.label); - return a > b ? 1 : -1; - }).forEach(function(idp) { - if (idp.error || addedEntities[idp.entity_id]) { - return; - } - var li = $('
    • '); - li.append($('').click(function() { - addedEntities[idp.entity_id] = 1; - newActiveIdp(idp, true); - updateIdps(); - $(this).parent().remove(); - })); - for (var k in idp) { - if (k == 'logo') { - continue; - } - li.append($('

      ').append($('

      ').text(val.idps)); - } + $(document).on('change', '.shib-idp-checkbox', function() { + console.log('[shib] checkbox change: ' + $(this).val() + ' -> ' + this.checked); + updateActiveIdps('change'); + }); - prnt.append($('

      ')); - var mkInp = function(lbl, val) { - var inp = $('').val(val).attr('name', lbl).data('orig', val); - return { - 'label': $('

      ').append($('

      '); - ['entity_id', 'label', 'host', 'logo'].forEach(function(key) { - var inp = mkInp(key); - addNew.append(inp.label); + // Backstop: also intercept the toolbar Save button click directly, + // since Hubzero.submitform may bypass jQuery submit handlers. + $(document).on('click', 'button.toolbar-submit, a.toolbar-submit, .toolbar button, .toolbar a', function() { + console.log('[shib] toolbar click detected on:', this); + updateActiveIdps('toolbar-click'); }); - addNew.append($('').click(function(evt) { - evt.stopPropagation(); - var idp = {}; - addNew.find('input').each(function(_, inp) { - inp = $(inp); - idp[inp.attr('name')] = inp.val().replace(/^\s+|\s+$/g, ''); - inp.val(''); - }); - newActiveIdp(idp, true); - updateIdps(); - return false; - })); - // append existing active providers - prnt - .append($('

      Active ID providers

      ')) - .append(idpWarning) - .append(addNew); - var existing = $('
        ').sortable({'stop': updateIdps}).appendTo(prnt); - val.activeIdps.forEach(newActiveIdp); - $('').appendTo(prnt).click(function(evt) { - evt.stopPropagation(); - existing.children('li').sort(function(a, b) { - var lblA = $(a).find('input[name=label]').val(); - var lblB = $(b).find('input[name=label]').val(); - return normalizeUniversity(lblA) > normalizeUniversity(lblB) ? 1 : -1; - }).appendTo(existing); - return false; + // Search / filter checkboxes by display name + $('#shib-idp-search').on('input', function() { + var q = this.value.toLowerCase(); + $('.idp-list label').each(function() { + $(this).toggle(!q || $(this).text().toLowerCase().indexOf(q) !== -1); + }); }); }); diff --git a/core/plugins/authentication/shibboleth/fields/institutions.php b/core/plugins/authentication/shibboleth/fields/institutions.php index 680a65057d1..ab43252d5e4 100644 --- a/core/plugins/authentication/shibboleth/fields/institutions.php +++ b/core/plugins/authentication/shibboleth/fields/institutions.php @@ -11,6 +11,28 @@ class Institutions extends Field { + /** + * Append a debug line to a dedicated log file so we can trace + * the institutions field read/write path without grepping cmsdebug.log. + * + * @param string $msg + * @return void + */ + private static function shibLog($msg) + { + $line = '[' . date('Y-m-d H:i:s') . '] [pid ' . getmypid() . '] ' . $msg . "\n"; + $paths = ['/var/log/hubzero/shib-debug.log', PATH_APP . DS . 'tmp' . DS . 'shib-debug.log']; + foreach ($paths as $p) + { + $dir = dirname($p); + if (is_dir($dir) && is_writable($dir) && @file_put_contents($p, $line, FILE_APPEND | LOCK_EX) !== false) + { + break; + } + } + try { \Log::debug('[shib-field] ' . $msg); } catch (\Exception $e) {} + } + /** * Get field input * @@ -19,123 +41,204 @@ class Institutions extends Field protected function getInput() { Document::addScript('/core/plugins/authentication/shibboleth/assets/js/admin.js'); - // Commented out due to interfering with admin styles - //Document::addStyleSheet('/core/plugins/authentication/shibboleth/assets/css/jquery-ui.css'); Document::addStyleSheet('/core/plugins/authentication/shibboleth/assets/css/admin.css'); - $html = array(); - $a = function($str) + self::shibLog('getInput called; name=' . $this->name + . ' value-type=' . gettype($this->value) + . ' value=' . (is_string($this->value) ? $this->value : json_encode($this->value))); + + // Log what's actually stored in the DB row right now, so we can compare + // against what just got POSTed (if anything). + try + { + $db = \App::get('db'); + $db->setQuery("SELECT params FROM `#__extensions` WHERE folder='authentication' AND element='shibboleth' LIMIT 1"); + $dbParams = $db->loadResult(); + self::shibLog('DB row params=' . $dbParams); + } + catch (\Exception $e) + { + self::shibLog('DB lookup failed: ' . $e->getMessage()); + } + + // Log what was actually POSTed for this field, if anything (only present + // when getInput runs after an "apply" task in the same request). + $posted = \Request::getVar('fields', null, 'post', 'array', 2); + if (is_array($posted) && isset($posted['params']['institutions'])) + { + self::shibLog('POSTed fields[params][institutions]=' . (is_string($posted['params']['institutions']) ? $posted['params']['institutions'] : json_encode($posted['params']['institutions']))); + } + else { - return str_replace('"', '"', $str); - }; + self::shibLog('no POSTed fields[params][institutions] in this request'); + } + $val = is_array($this->value) ? $this->value : json_decode($this->value, true); + if (!$val) + { + self::shibLog('value did not decode, using defaults'); + $val = ['xmlPath' => '/etc/shibboleth/metadata/federation-metadata.xml', 'activeIdps' => []]; + } + if (!isset($val['activeIdps'])) + { + $val['activeIdps'] = []; + } + self::shibLog('decoded activeIdps count=' . count($val['activeIdps']) + . ' entity_ids=' . json_encode(array_column($val['activeIdps'], 'entity_id'))); + + $activeMap = []; + foreach ($val['activeIdps'] as $idp) + { + if (isset($idp['entity_id'])) + { + $activeMap[$idp['entity_id']] = true; + } + } + + $entities = self::loadFederationEntities($val['xmlPath']); + + // Only store xmlPath and activeIdps — never the full entity list + $storedVal = ['xmlPath' => $val['xmlPath'], 'activeIdps' => $val['activeIdps']]; + $hiddenJson = htmlspecialchars(json_encode($storedVal), ENT_QUOTES, 'UTF-8'); + $xmlPathEsc = htmlspecialchars($val['xmlPath'], ENT_QUOTES, 'UTF-8'); + $nameEsc = htmlspecialchars($this->name, ENT_QUOTES, 'UTF-8'); + + $html = []; + $html[] = '
        '; + $html[] = '

        '; + $html[] = ''; + + if (is_array($entities)) + { + $html[] = '

        '; + $html[] = '
        '; + foreach ($entities as $entityId => $displayName) + { + $checked = isset($activeMap[$entityId]) ? ' checked' : ''; + $eidEsc = htmlspecialchars($entityId, ENT_QUOTES, 'UTF-8'); + $nameDisp = htmlspecialchars($displayName, ENT_QUOTES, 'UTF-8'); + $html[] = ''; + } + $html[] = '
        '; + } + else + { + $html[] = '

        ' . htmlspecialchars((string)$entities, ENT_QUOTES, 'UTF-8') . '

        '; + } - $html[] = '
        '; - $html[] = '

        '; - list($val['xmlRead'], $val['idps']) = self::getIdpList($val); - $html[] = '

        Save your changes to retry loading ID providers from this file

        '; - $html[] = ''; $html[] = '
        '; - // rest of the form is managed on the client side return implode("\n", $html); } /** - * Get Ipd list + * Stream-parse the federation metadata XML and return [entityId => displayName]. + * Results are cached to app/tmp to avoid re-parsing on every page load. * - * @param string $val - * @param boolean $alwaysUpdate - * @return array + * @param string $xmlPath Filesystem path to the federation metadata XML + * @return array|string Associative array on success, error string on failure */ - private static function getIdpList($val, $alwaysUpdate = true) + private static function loadFederationEntities($xmlPath) { - // list is up to date - if (!file_exists($val['xmlPath'])) + if (!file_exists($xmlPath)) { - return array(null, 'Invalid XML path'); + return 'Federation metadata XML not found: ' . $xmlPath; } - if (($mtime = $val['xmlPath'] . ':' . filemtime($val['xmlPath'])) == $val['xmlRead'] && !$alwaysUpdate) + + $tmpDir = \Config::get('tmp_path', PATH_APP . DS . 'tmp'); + $cacheFile = $tmpDir . DS . 'shib_entities_' . md5($xmlPath) . '.json'; + + if (file_exists($cacheFile) && filemtime($cacheFile) >= filemtime($xmlPath)) { - return array($mtime, $val['idps']); + $cached = json_decode(file_get_contents($cacheFile), true); + if (is_array($cached)) + { + return $cached; + } } - if (!($xml = simplexml_load_file($val['xmlPath']))) + + $reader = new \XMLReader(); + if (!$reader->open($xmlPath)) { - return array(null, 'Failed to parse XML from this path'); + return 'Failed to open federation metadata XML: ' . $xmlPath; } - $xml->registerXPathNamespace('shib', 'urn:mace:shibboleth:2.0:native:sp:config'); - $curl = curl_init(); - $rv = array(); - foreach ($xml->xpath('//shib:SSO') as $item) + $mduiNs = 'urn:oasis:names:tc:SAML:metadata:ui'; + $xmlLangNs = 'http://www.w3.org/XML/1998/namespace'; + $entities = []; + $currentId = null; + $inIdpSso = false; + $captureNext = false; + + while (@$reader->read()) { - $entityId = (string)$item->attributes()->entityID; + $type = $reader->nodeType; - curl_setopt($curl, CURLOPT_URL, $entityId); - curl_setopt($curl, CURLOPT_HEADER, 0); - curl_setopt($curl, CURLOPT_RETURNTRANSFER, 1); + if ($type === \XMLReader::ELEMENT) + { + $local = $reader->localName; - $item = array( - 'entity_id' => $entityId, - 'label' => null, - 'host' => null, - 'logo' => null - ); + if ($local === 'EntityDescriptor') + { + $currentId = $reader->getAttribute('entityID'); + $inIdpSso = false; + $captureNext = false; + } + elseif ($local === 'IDPSSODescriptor') + { + $inIdpSso = true; + } + elseif ($local === 'DisplayName' + && $reader->namespaceURI === $mduiNs + && $inIdpSso + && $currentId !== null + && !isset($entities[$currentId])) + { + $lang = $reader->getAttributeNs('lang', $xmlLangNs); + if ($lang === 'en') + { + $captureNext = true; + } + } - if (!($idp = curl_exec($curl))) - { - $item['error'] = 'Failed to fetch metadata'; + if ($reader->isEmptyElement) + { + $captureNext = false; + } } - else if (!($idp = simplexml_load_string($idp))) + elseif ($type === \XMLReader::TEXT && $captureNext) { - $item['error'] = 'Failed to parse metadata'; + $entities[$currentId] = trim($reader->value); + $captureNext = false; } - else + elseif ($type === \XMLReader::END_ELEMENT) { - $idp->registerXPathNamespace('saml', 'urn:oasis:names:tc:SAML:2.0:metadata'); - // look in a few places for the display name - foreach (array( - '//mdui:DisplayName', // most preferred, ui extension offers prefs for this specific use case - '//saml:OrganizationDisplayName', // good fallback - '//saml:OrganizationName' // ok fallback - ) as $xp) + $local = $reader->localName; + if ($local === 'DisplayName') { - if (($name = $idp->xpath($xp))) - { - $item['label'] = (string)$name[0]; - break; - } + $captureNext = false; } - if (($orgUrl = $idp->xpath('//saml:OrganizationURL'))) + elseif ($local === 'IDPSSODescriptor') { - $item['host'] = preg_replace('/^.*[.]([^.]+[.][^.]+)$/', '$1', parse_url($orgUrl[0], \PHP_URL_HOST)); + $inIdpSso = false; + } + elseif ($local === 'EntityDescriptor') + { + $currentId = null; + $inIdpSso = false; + $captureNext = false; } - //$item['logo'] = $idp->xpath('//mdui:Logo'); - //$item['logo'] = $item['logo'] ? (string)$item['logo'][0] : null; } - $rv[] = $item; } - $idps = self::getResearchAndScholarshipIdps($curl); - $rv = array_merge($rv, $idps); - curl_close($curl); - return array($mtime, $rv); - } + $reader->close(); - /** - * Get a list of research and scholarship IDs - * - * @param string $ch - * @return array - */ - private static function getResearchAndScholarshipIdps($ch) - { - $rv = []; - exec('php ' . __DIR__ . '/get-rs-entities.php', $out); - $result = json_decode(join('', $out)); - if ($result == null) + uasort($entities, 'strcasecmp'); + + if (is_writable($tmpDir)) { - $result = array(); + file_put_contents($cacheFile, json_encode($entities)); } - return $result; + + return $entities; } } diff --git a/core/plugins/authentication/shibboleth/shibboleth.xml b/core/plugins/authentication/shibboleth/shibboleth.xml index abf60c557e0..268a19aee2d 100644 --- a/core/plugins/authentication/shibboleth/shibboleth.xml +++ b/core/plugins/authentication/shibboleth/shibboleth.xml @@ -41,7 +41,7 @@
        - +