From 675a811d2855ec7764d1ccdc9126eaa71d1105fb Mon Sep 17 00:00:00 2001 From: Sixto Martin Date: Tue, 31 Jan 2017 11:16:45 +0100 Subject: [PATCH] Clean index.php. Sanitize GETs. Relax Destination validation. Release 2.4.1 --- auth/onelogin_saml/auth.php | 2 +- auth/onelogin_saml/config.html | 2 +- auth/onelogin_saml/functions.php | 4 +- auth/onelogin_saml/index.php | 197 ++++++++++------------ auth/onelogin_saml/lib/Saml2/Response.php | 5 +- auth/onelogin_saml/version.php | 4 +- 6 files changed, 103 insertions(+), 111 deletions(-) diff --git a/auth/onelogin_saml/auth.php b/auth/onelogin_saml/auth.php index b1951cf..f68e1a4 100644 --- a/auth/onelogin_saml/auth.php +++ b/auth/onelogin_saml/auth.php @@ -6,7 +6,7 @@ * * @originalauthor OneLogin, Inc * @author Harrison Horowitz, Sixto Martin - * @version 2.4.0 + * @version 2.4.1 * @license http://www.gnu.org/copyleft/gpl.html GNU Public License * @package auth/onelogin_saml * @requires XMLSecLibs v2.0.0-mod diff --git a/auth/onelogin_saml/config.html b/auth/onelogin_saml/config.html index 82b06e5..423a9b7 100644 --- a/auth/onelogin_saml/config.html +++ b/auth/onelogin_saml/config.html @@ -5,7 +5,7 @@ * * @originalauthor OneLogin, Inc * @author Harrison Horowitz, Sixto Martin - * @version 2.4.0 + * @version 2.4.1 * @license http://www.gnu.org/copyleft/gpl.html GNU Public License * @package auth/onelogin_saml * @requires XMLSecLibs v2.0.0-mod diff --git a/auth/onelogin_saml/functions.php b/auth/onelogin_saml/functions.php index d11de84..79853d0 100644 --- a/auth/onelogin_saml/functions.php +++ b/auth/onelogin_saml/functions.php @@ -5,7 +5,7 @@ * * @originalauthor OneLogin, Inc * @author Harrison Horowitz, Sixto Martin - * @version 2.4.0 + * @version 2.4.1 * @license http://www.gnu.org/copyleft/gpl.html GNU Public License * @package auth/onelogin_saml * @requires XMLSecLibs v2.0.0-mod @@ -138,7 +138,7 @@ function auth_onelogin_saml_authenticate_user_login($saml_account_matcher, $user } $auths = $authsenabled; - $user = new object(); + $user = new stdClass(); $user->id = 0; // User does not exist } diff --git a/auth/onelogin_saml/index.php b/auth/onelogin_saml/index.php index 3c04a66..bee6f15 100644 --- a/auth/onelogin_saml/index.php +++ b/auth/onelogin_saml/index.php @@ -5,7 +5,7 @@ * * @originalauthor OneLogin, Inc * @author Harrison Horowitz, Sixto Martin - * @version 2.4.0 + * @version 2.4.1 * @license http://www.gnu.org/copyleft/gpl.html GNU Public License * @package auth/onelogin_saml * @requires XMLSecLibs v2.0.0-mod @@ -38,16 +38,16 @@ require_once('functions.php'); - global $CFG, $USER, $SESSION, $POST, $_POST, $_GET, $_SERVER, $DB, $SITE; + global $CFG, $USER, $SESSION, $_POST, $_GET, $_SERVER; // Normal form failed if (isset($_GET['errorcode']) && $_GET['errorcode'] != 4) { - $location = $CFG->wwwroot.'/login/index.php?normal&errorcode='.$_GET['errorcode']; + $errorCode = clean_param($_GET['errorcode'] , PARAM_INT); + $location = $CFG->wwwroot.'/login/index.php?normal&errorcode='.$errorCode; header('Location: '.$location); exit(); } - /** * check that the saml session is OK - if not, send to OneLogin for authentication * if good, then do the Moodle login, and send to the home page, or landing page @@ -66,7 +66,7 @@ // save the jump target - this is checked later that it starts with $CFG->wwwroot, and cleaned if (isset($_GET['wantsurl'])) { - $wantsurl = $SESSION->wantsurl = $_GET['wantsurl']; + $wantsurl = $SESSION->wantsurl = clean_param($_GET['wantsurl'], PARAM_URL); } // check for a wantsurl in the existing Moodle session @@ -86,7 +86,7 @@ if ($logoutActived) { if (isset($_GET['RelayState']) && !empty($_GET['RelayState'])) { - $location = $_GET['RelayState']; + $location = clean_param($_GET['RelayState'], PARAM_URL); } else if (isset($wantsurl)) { $location = $wantsurl; @@ -111,7 +111,7 @@ auth_onelogin_saml_deleteLocalSession(); } else { - print_r(implode(', ', $errors).'

'.$auth->getLastErrorReason()); + print_error('auth_onelogin_saml: '.implode(', ', $errors).'

'.$auth->getLastErrorReason()); exit(); } } @@ -146,107 +146,96 @@ header('Location: '.$location); exit(); - } - - if (!isset($_POST['SAMLResponse']) && !$normalActived && !$normalSessionActivated && !$logoutActived) { - $auth->login(); - } else if (isset($_POST['SAMLResponse']) && $_POST['SAMLResponse'] && !$normalActived && !$normalSessionActivated && !$logoutActived) { - try { - $auth->processResponse(); - $errors = $auth->getErrors(); - if (empty($errors)) { - $SESSION->onelogin_saml_nameID = $onelogin_saml_nameId = $auth->getNameId(); - $SESSION->onelogin_saml_login_attributes = $saml_attributes = $auth->getAttributes(); - $wantsurl = isset($SESSION->wantsurl) ? $SESSION->wantsurl : FALSE; - } else { - $errorMsg = "An invalid SAML response was received from the Identity Provider. Contact the admin."; - if ($pluginconfig->saml_debug_mode) { - $errorMsg .= "
".implode(', ', $errors).'

'.$auth->getLastErrorReason(); - } - } - } catch (Exception $e) { - $errorMsg = "An invalid SAML response was received from the Identity Provider. Contact the admin."; - if ($pluginconfig->saml_debug_mode) { - $errorMsg .= "
".$e->getMessage(); - } - } } else { - // You shouldn't be able to reach here. - print_error("Module Setup Error: Review the OneLogin setup instructions for the SAML authentication module, and be sure to change the following one line of code in Moodle's core in 'login/index.php'.

CHANGE THE FOLLOWING LINE OF CODE (in 'login/index.php')...

if (!empty(\$CFG->alternateloginurl)) {

...to...

if (!empty(\$CFG->alternateloginurl) && !isset(\$_GET['normal'])) { \r\n"); - exit(); - } - - if (isset($errorMsg)) { - print_error($errorMsg); - exit(); - } - - // Valid session. Register or update user in Moodle, log him on, and redirect to Moodle front - // we require the plugin to know that we are now doing a saml login in hook puser_login - $SESSION->onelogin_saml_login = TRUE; - - $samlplugin = get_auth_plugin('onelogin_saml'); - $saml_user = $samlplugin->get_userinfo(null); - - // check user name attribute actually passed - if($saml_user == false){ - error_log('auth_onelogin_saml: auth failed due to missing username/email saml attribute: '.$pluginconfig->saml_username_map); - session_write_close(); - $USER = new object(); - $USER->id = 0; - require_once('../../config.php'); - print_error('auth_onelogin_saml: auth failed due to missing username/email saml attribute: '.$pluginconfig->saml_username_map."
".get_string("auth_onelogin_saml_username_email_error", "auth_onelogin_saml")."\r\n"); - exit(); - } - - - if ($_POST['SAMLResponse']) { - $saml_account_matcher = $pluginconfig->saml_account_matcher; - if (empty($saml_account_matcher)) { - $saml_account_matcher = 'username'; + if (!isset($_POST['SAMLResponse']) && !$normalActived && !$normalSessionActivated && !$logoutActived) { + $auth->login(); + } else if (isset($_POST['SAMLResponse']) && $_POST['SAMLResponse'] && !$normalActived && !$normalSessionActivated && !$logoutActived) { + try { + $auth->processResponse(); + $errors = $auth->getErrors(); + if (empty($errors)) { + $SESSION->onelogin_saml_nameID = $onelogin_saml_nameId = $auth->getNameId(); + $SESSION->onelogin_saml_login_attributes = $saml_attributes = $auth->getAttributes(); + $wantsurl = isset($SESSION->wantsurl) ? $SESSION->wantsurl : FALSE; + + // Valid session. Register or update user in Moodle, log him on, and redirect to Moodle front + // we require the plugin to know that we are now doing a saml login in hook puser_login + $SESSION->onelogin_saml_login = TRUE; + } else { + $errorMsg = "auth_onelogin_saml: An invalid SAML response was received from the Identity Provider. Contact the admin."; + if ($pluginconfig->saml_debug_mode) { + $errorMsg .= "
".implode(', ', $errors).'

'.$auth->getLastErrorReason(); + } + } + } catch (Exception $e) { + $errorMsg = "auth_onelogin_saml: An invalid SAML response was received from the Identity Provider. Contact the admin."; + if ($pluginconfig->saml_debug_mode) { + $errorMsg .= "
".$e->getMessage(); + } + } + } else { + // You shouldn't be able to reach here. + $errorMsg = "auth_onelogin_saml: Module Setup Error: Review the OneLogin setup instructions for the SAML authentication module"; } - $saml_create = $pluginconfig->saml_auto_create_users == 'on'? true : false; - $saml_update = $pluginconfig->saml_auto_update_users == 'on'? true : false; - $USER = auth_onelogin_saml_authenticate_user_login($saml_account_matcher, $saml_user, $saml_create, $saml_update); - } else { - print_error("Info received. Finishing authentication process through regular method hook because no SAML response detected."); - display_object($_POST); - $USER = authenticate_user_login($saml_user[$saml_account_matcher], time()); - } - - // check that the signin worked - if ($USER == false) { - print_error("You could not be identified or created.
Login result: FAILURE
I have...
".htmlspecialchars(print_r($USER, true))); - session_write_close(); - $USER = new object(); - $USER->id = 0; - require_once('../../config.php'); - print_error('pluginauthfailed', 'auth_onelogin_saml', '', (!empty($saml_user['username']) ? $saml_user['username'] : $saml_user['email'])); - exit(); - } - - // complete the user login sequence - $USER->loggedin = true; - $USER->site = $CFG->wwwroot; - $USER = get_complete_user_data('id', $USER->id); - complete_user_login($USER); + if (!isset($errorMsg)) { + $samlplugin = get_auth_plugin('onelogin_saml'); + $saml_user = $samlplugin->get_userinfo(null); + // check user name attribute actually passed + if($saml_user !== false){ + if ($_POST['SAMLResponse']) { + $saml_account_matcher = $pluginconfig->saml_account_matcher; + if (empty($saml_account_matcher)) { + $saml_account_matcher = 'username'; + } - // flag this as a SAML based login - $SESSION->isSAMLSessionControlled = true; - $SESSION->onelogin_saml_session_index = $auth->getSessionIndex(); - $SESSION->onelogin_saml_nameid_format = $auth->getNameIdFormat(); - - $_COOKIE['onelogin_saml_session_index'] = $auth->getSessionIndex(); - $_COOKIE['onelogin_saml_nameid_format'] = $auth->getNameIdFormat(); + $saml_create = $pluginconfig->saml_auto_create_users == 'on'? true : false; + $saml_update = $pluginconfig->saml_auto_update_users == 'on'? true : false; + $USER = auth_onelogin_saml_authenticate_user_login($saml_account_matcher, $saml_user, $saml_create, $saml_update); + + // check that the signin worked + if ($USER != false) { + // complete the user login sequence + $USER->loggedin = true; + $USER->site = $CFG->wwwroot; + $USER = get_complete_user_data('id', $USER->id); + complete_user_login($USER); + + // flag this as a SAML based login + $SESSION->isSAMLSessionControlled = true; + //$SESSION->onelogin_saml_session_index = $auth->getSessionIndex(); + //$SESSION->onelogin_saml_nameid_format = $auth->getNameIdFormat(); + + if (isset($wantsurl)) {// and (strpos($wantsurl, $CFG->wwwroot) === 0) + $urltogo = clean_param($wantsurl, PARAM_URL); + } else { + $urltogo = $CFG->wwwroot.'/'; + } + + if (!$urltogo || $urltogo == "") { + $urltogo = $CFG->wwwroot.'/'; + } + + unset($SESSION->wantsurl); + redirect($urltogo, 0); + } else { + $errorMsg = "auth_onelogin_saml: You could not be identified or created: ".htmlspecialchars((!empty($saml_user['username']) ? $saml_user['username'] : $saml_user['email'])); + } + } else { + $errorMsg = "auth_onelogin_saml: No SAML response detected."; + } + } else { + $errorMsg = 'auth_onelogin_saml: auth failed due to missing username/email saml attribute: '.$pluginconfig->saml_username_map."
".get_string("auth_onelogin_saml_username_email_error", "auth_onelogin_saml"); + } - if (isset($wantsurl)) {// and (strpos($wantsurl, $CFG->wwwroot) === 0) - $urltogo = clean_param($wantsurl, PARAM_URL); - } else { - $urltogo = $CFG->wwwroot.'/'; - } - if (!$urltogo || $urltogo == "") $urltogo = $CFG->wwwroot.'/'; + if (isset($errorMsg)) { + print_error($errorMsg); + exit(); + } - unset($SESSION->wantsurl); - redirect($urltogo, 0); + } else { + print_error($errorMsg); + exit(); + } + } \ No newline at end of file diff --git a/auth/onelogin_saml/lib/Saml2/Response.php b/auth/onelogin_saml/lib/Saml2/Response.php index 3402dc9..dae324f 100644 --- a/auth/onelogin_saml/lib/Saml2/Response.php +++ b/auth/onelogin_saml/lib/Saml2/Response.php @@ -222,12 +222,15 @@ public function isValid($requestId = null) // Check destination if ($this->document->documentElement->hasAttribute('Destination')) { $destination = trim($this->document->documentElement->getAttribute('Destination')); +/* if (empty($destination)) { throw new OneLogin_Saml2_ValidationError( "The response has an empty Destination value", OneLogin_Saml2_ValidationError::EMPTY_DESTINATION ); } else { +*/ + if (!empty($destination)) { if (strpos($destination, $currentURL) !== 0) { $currentURLNoRouted = OneLogin_Saml2_Utils::getSelfURLNoQuery(); @@ -1001,7 +1004,7 @@ protected function _decryptAssertion($dom) if (strpos($encryptedAssertion->tagName, 'saml2:') !== false) { $ns = 'xmlns:saml2'; - } else if (strpos($encryptedAssertion->tagName, 'saml:') != false) { + } else if (strpos($encryptedAssertion->tagName, 'saml:') !== false) { $ns = 'xmlns:saml'; } else { $ns = 'xmlns'; diff --git a/auth/onelogin_saml/version.php b/auth/onelogin_saml/version.php index 8bbffa5..2d2b5a7 100644 --- a/auth/onelogin_saml/version.php +++ b/auth/onelogin_saml/version.php @@ -5,7 +5,7 @@ * * @originalauthor OneLogin, Inc * @author Harrison Horowitz, Sixto Martin - * @version 2.4.0 + * @version 2.4.1 * @license http://www.gnu.org/copyleft/gpl.html GNU Public License * @package auth/onelogin_saml * @requires XMLSecLibs v2.0.0-mod @@ -32,7 +32,7 @@ */ $plugin->component = 'auth_onelogin_saml'; - $plugin->version = 2017011301; // The current module version (Date: YYYYMMDDXX) + $plugin->version = 2017013101; // The current module version (Date: YYYYMMDDXX) $plugin->requires = 2013012801; $plugin->cron = 0; // Period for cron to check this module (secs) $plugin->maturity = MATURITY_STABLE; \ No newline at end of file