diff --git a/.github/dependabot.yml b/.github/dependabot.yml new file mode 100644 index 000000000..d0d482aa3 --- /dev/null +++ b/.github/dependabot.yml @@ -0,0 +1,11 @@ +version: 2 +updates: + - package-ecosystem: "github-actions" + directory: "/" + groups: + github-actions: + patterns: [ "*" ] + schedule: + interval: "weekly" + cooldown: + default-days: 7 diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ce8a6cbb2..410d69804 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -10,6 +10,9 @@ on: - 'MOODLE_*_STABLE' pull_request: +permissions: + contents: read + jobs: check: runs-on: ubuntu-latest @@ -27,12 +30,7 @@ jobs: mariadb: image: mariadb:10.11 env: - MYSQL_USER: 'root' MYSQL_ALLOW_EMPTY_PASSWORD: "true" - MYSQL_CHARACTER_SET_SERVER: "utf8mb4" - MYSQL_COLLATION_SERVER: "utf8mb4_unicode_ci" - MYSQL_INNODB_FILE_PER_TABLE: "1" - MYSQL_INNODB_FILE_FORMAT: "Barracuda" ports: - 3306:3306 options: --health-cmd="mysqladmin ping" --health-interval 10s --health-timeout 5s --health-retries 3 @@ -40,18 +38,18 @@ jobs: strategy: fail-fast: false matrix: - moodle-branch: ['MOODLE_502_STABLE'] - php: [8.3, 8.4] - database: [pgsql, mariadb] + moodle-branch: [ 'MOODLE_502_STABLE' ] + php: [ 8.3, 8.4 ] + database: [ pgsql, mariadb ] steps: - name: Check out repository code - uses: actions/checkout@v4 + uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6.1.0 with: path: plugin - name: Setup PHP ${{ matrix.php }} - uses: shivammathur/setup-php@v2 + uses: shivammathur/setup-php@f3e473d116dcccaddc5834248c87452386958240 # 2.37.2 with: php-version: ${{ matrix.php }} ini-values: max_input_vars=5000 diff --git a/classes/adminsetting/auth_oidc_admin_setting_loginflow.php b/classes/adminsetting/auth_oidc_admin_setting_loginflow.php index 80cb3c172..d2ed5bfaa 100644 --- a/classes/adminsetting/auth_oidc_admin_setting_loginflow.php +++ b/classes/adminsetting/auth_oidc_admin_setting_loginflow.php @@ -30,7 +30,7 @@ */ class auth_oidc_admin_setting_loginflow extends \admin_setting { /** @var array Array of valid login flow types. */ - protected $flowtypes = ['authcode', 'rocreds']; + protected $flowtypes = ['authcode']; /** * Return the setting diff --git a/classes/adminsetting/auth_oidc_admin_setting_secretexpiryrecipients.php b/classes/adminsetting/auth_oidc_admin_setting_secretexpiryrecipients.php new file mode 100644 index 000000000..b7ec94c98 --- /dev/null +++ b/classes/adminsetting/auth_oidc_admin_setting_secretexpiryrecipients.php @@ -0,0 +1,59 @@ +. + +/** + * Admin setting class for the secret expiry notification recipients setting. + * + * @package auth_oidc + * @author Lai Wei + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @copyright (C) 2026 onwards Microsoft, Inc. (http://microsoft.com/) + */ + +namespace auth_oidc\adminsetting; + +defined('MOODLE_INTERNAL') || die(); + +require_once($CFG->dirroot . '/auth/oidc/lib.php'); + +/** + * Admin setting for the comma-separated list of secret expiry notification recipients. + * + * Extends the standard text setting with validation that rejects the value when any entry is + * not a valid email address, so the local_o365 notifysecretexpiry task is not left silently + * dropping recipients at run time. + */ +class auth_oidc_admin_setting_secretexpiryrecipients extends \admin_setting_configtext { + /** + * Validate the submitted list of recipient email addresses. + * + * @param string $data The submitted value. + * @return string|true True when valid; a translatable error string otherwise. + */ + public function validate($data) { + $result = parent::validate($data); + if ($result !== true) { + return $result; + } + + $invalidemails = auth_oidc_validate_secret_expiry_recipients((string) $data); + if ($invalidemails) { + return get_string('error_secretexpiryrecipients_invalid', 'auth_oidc', implode(', ', $invalidemails)); + } + + return true; + } +} diff --git a/classes/adminsetting/auth_oidc_admin_setting_section_heading.php b/classes/adminsetting/auth_oidc_admin_setting_section_heading.php new file mode 100644 index 000000000..f2a0c69e1 --- /dev/null +++ b/classes/adminsetting/auth_oidc_admin_setting_section_heading.php @@ -0,0 +1,60 @@ +. + +/** + * Definition of a section heading admin setting that can be hidden with hide_if(). + * + * @package auth_oidc + * @author Lai Wei + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @copyright (C) 2021 onwards Microsoft, Inc. (http://microsoft.com/) + */ + +namespace auth_oidc\adminsetting; + +use admin_setting_heading; +use html_writer; + +/** + * A section heading that participates in admin settings hide_if() dependencies. + * + * The core {@see admin_setting_heading} renders only a bare

, with no named + * form control and no .form-item wrapper, so the admin settings show/hide + * JavaScript (lib/amd/src/showhidesettings.js) cannot target it and any + * hide_if() condition applied to it is silently ignored. This subclass wraps the + * heading in a .form-item container and adds a hidden input carrying the + * setting's form field name, so the heading is shown and hidden together with + * the settings it introduces. + */ +class auth_oidc_admin_setting_section_heading extends admin_setting_heading { + /** + * Output the heading wrapped so that hide_if() dependencies can act on it. + * + * @param mixed $data + * @param string $query + * @return string + */ + public function output_html($data, $query = '') { + $heading = parent::output_html($data, $query); + $hiddeninput = html_writer::empty_tag('input', [ + 'type' => 'hidden', + 'name' => 's_' . $this->plugin . '_' . $this->name, + 'value' => 1, + ]); + + return html_writer::div($hiddeninput . $heading, 'form-item', ['id' => 'admin-' . $this->name]); + } +} diff --git a/classes/adminsetting/auth_oidc_admin_setting_stateexpiry.php b/classes/adminsetting/auth_oidc_admin_setting_stateexpiry.php new file mode 100644 index 000000000..dd1c92be9 --- /dev/null +++ b/classes/adminsetting/auth_oidc_admin_setting_stateexpiry.php @@ -0,0 +1,54 @@ +. + +/** + * Admin setting class for the OIDC login state expiry setting. + * + * @package auth_oidc + * @author Lai Wei + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @copyright (C) 2026 onwards Microsoft, Inc. (http://microsoft.com/) + */ + +namespace auth_oidc\adminsetting; + +/** + * Admin setting for the OIDC login state expiry, in minutes. + * + * Extends the standard text setting with validation that rejects values below one minute. A + * value of zero (or less) would cause the state cleanup task to delete login state records + * almost immediately, breaking any login that is currently in progress. + */ +class auth_oidc_admin_setting_stateexpiry extends \admin_setting_configtext { + /** + * Validate the submitted expiry value. + * + * @param string $data The submitted value. + * @return string|true True when valid; a translatable error string otherwise. + */ + public function validate($data) { + $result = parent::validate($data); + if ($result !== true) { + return $result; + } + + if ((int) $data < 1) { + return get_string('error_stateexpiry_min', 'auth_oidc'); + } + + return true; + } +} diff --git a/classes/adminsetting/iconselect.css b/classes/adminsetting/iconselect.css index 6fe4121d4..d55e20f75 100644 --- a/classes/adminsetting/iconselect.css +++ b/classes/adminsetting/iconselect.css @@ -7,6 +7,7 @@ label.iconselect img { width: 25px; height: 25px; padding: 10px; + margin-right: 0; } input.iconselect { display: none; diff --git a/classes/form/application.php b/classes/form/application.php index 98b695a39..c0a00cb7f 100644 --- a/classes/form/application.php +++ b/classes/form/application.php @@ -397,6 +397,15 @@ public function validation($data, $files) { } } + // Validate secret expiry notification recipients (only relevant with secret auth). + if (isset($data['secretexpiryrecipients']) && $data['clientauthmethod'] == AUTH_OIDC_AUTH_METHOD_SECRET) { + $invalidemails = auth_oidc_validate_secret_expiry_recipients((string) $data['secretexpiryrecipients']); + if ($invalidemails) { + $errors['secretexpiryrecipients'] = + get_string('error_secretexpiryrecipients_invalid', 'auth_oidc', implode(', ', $invalidemails)); + } + } + return $errors; } diff --git a/classes/hook/before_login_completed.php b/classes/hook/before_login_completed.php new file mode 100644 index 000000000..0a79b198b --- /dev/null +++ b/classes/hook/before_login_completed.php @@ -0,0 +1,50 @@ +. + +namespace auth_oidc\hook; + +use auth_oidc\jwt; + +/** + * Allow plugins to perform additional checks before a user login is completed. + * + * This hook is dispatched by auth_oidc after authenticate_user_login() has + * succeeded, but before complete_user_login() is called. The hook manager + * does not catch exceptions raised by callbacks, so a callback can reject + * the login by throwing an exception (e.g. \moodle_exception) - doing so + * will propagate out of the hook dispatch and prevent complete_user_login() + * from being called. There is no other signal (e.g. a flag on this hook) to + * reject the login; a plain return from a callback allows the login to + * proceed. + * + * @package auth_oidc + * @copyright 2026 Ariadne + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +#[\core\attribute\label('Allow plugins to perform additional checks before a user login is completed.')] +#[\core\attribute\tags('user', 'login')] +class before_login_completed { + /** + * Constructor for the hook. + * + * @param jwt $idtoken The id_token of the user attempting to log in. + */ + public function __construct( + /** @var jwt The id_token of the user attempting to log in */ + public readonly jwt $idtoken + ) { + } +} diff --git a/classes/loginflow/authcode.php b/classes/loginflow/authcode.php index b12867dca..66009133a 100644 --- a/classes/loginflow/authcode.php +++ b/classes/loginflow/authcode.php @@ -31,13 +31,14 @@ use auth_oidc\event\user_rename_attempt; use auth_oidc\jwt; use auth_oidc\utils; +use core\context\system; use core\output\notification; use core_text; use core_user; use moodle_exception; use core\url; -use pix_icon; use stdClass; +use core\di; defined('MOODLE_INTERNAL') || die(); @@ -59,25 +60,27 @@ public function loginpage_idp_list($wantsurl) { return []; } $showicon = isset($this->config->set_pix) ? $this->config->set_pix : true; + $name = strip_tags(format_text($this->config->opname)); $idpentry = [ 'url' => new url('/auth/oidc/', ['source' => 'loginpage']), - 'name' => strip_tags(format_text($this->config->opname)), + 'name' => $name, ]; if ($showicon) { + global $OUTPUT; if (!empty($this->config->customicon)) { - $iconvalue = new pix_icon('0/customicon', get_string('pluginname', 'auth_oidc'), 'auth_oidc'); + $idpentry['iconurl'] = $OUTPUT->image_url('0/customicon', 'auth_oidc')->out(false); } else { - $icon = (!empty($this->config->icon)) ? $this->config->icon : 'auth_oidc:o365'; + $icon = (!empty($this->config->icon)) ? $this->config->icon : 'auth_oidc:microsoft_365'; $icon = explode(':', $icon); if (isset($icon[1])) { [$iconcomponent, $iconname] = $icon; } else { $iconcomponent = 'auth_oidc'; - $iconname = 'o365'; + $iconname = 'microsoft_365'; } - $iconvalue = new pix_icon($iconname, get_string('pluginname', 'auth_oidc'), $iconcomponent); + $idpentry['iconurl'] = $OUTPUT->image_url($iconname, $iconcomponent)->out(false); } - $idpentry['icon'] = $iconvalue; + $idpentry['name'] = $name; } return [$idpentry]; } @@ -112,6 +115,13 @@ protected function is_valid_local_url(string $urlstring): bool { return false; } + // Path must start with the wwwroot path (handles subdirectory installs). + $wwwrootpath = rtrim($wwwroot['path'] ?? '', '/') . '/'; + $checkpath = $checkurl['path'] ?? '/'; + if (strpos($checkpath . '/', $wwwrootpath) !== 0) { + return false; + } + return true; } @@ -195,7 +205,7 @@ public function handleredirect() { $this->handleauthresponse($requestparams); } else { if (isloggedin() && !isguestuser() && empty($justauth) && empty($promptaconsent)) { - if (isset($SESSION->wantsurl) && (strpos($SESSION->wantsurl, $CFG->wwwroot) === 0)) { + if (isset($SESSION->wantsurl) && $this->is_valid_local_url($SESSION->wantsurl)) { $urltogo = $SESSION->wantsurl; unset($SESSION->wantsurl); } else { @@ -264,6 +274,17 @@ public function initiateauthrequest( array $extraparams = [], bool $selectaccount = false ) { + global $USER; + + if (isloggedin() && !isguestuser()) { + // Record who initiated this request against the state record. The OIDC callback can arrive as a + // cross-site POST (response_mode=form_post), which a SameSite=Lax session cookie is not sent on, so the + // session may not survive the round trip. Storing the user id here lets handleauthresponse() identify + // the initiating user from the (unguessable, single-use) state record instead of the live session. + $stateparams['initiatinguserid'] = $USER->id; + } + + $this->set_csrf_cookie(); $client = $this->get_oidcclient(); $client->authrequest($promptlogin, $stateparams, $extraparams, $selectaccount); } @@ -276,10 +297,125 @@ public function initiateauthrequest( * @return void */ public function initiateadminconsentrequest(array $stateparams = [], array $extraparams = []) { + global $USER; + + if (!isset($stateparams['initiatinguserid']) && isloggedin() && !isguestuser()) { + $stateparams['initiatinguserid'] = $USER->id; + } + + $this->set_csrf_cookie(); $client = $this->get_oidcclient(); $client->adminconsentrequest($stateparams, $extraparams); } + /** + * Handle a login state that could not be matched to a stored state record. + * + * This normally happens when the user takes longer than the state record's lifetime to + * complete login at the identity provider (for example, while approving a multi-factor + * authentication prompt), so the scheduled cleanup task has already deleted the record by + * the time the identity provider redirects back. When the "stateredirect_enabled" setting is + * on, this shows a customizable, friendly message and automatically redirects the user back + * to the login page instead of surfacing the generic Moodle error page. + * + * @return never + * @throws moodle_exception If the friendly redirect setting is disabled. + */ + protected function handlemissingstaterecord(): never { + global $OUTPUT, $PAGE; + + if (empty(get_config('auth_oidc', 'stateredirect_enabled'))) { + throw new moodle_exception('errorauthunknownstate', 'auth_oidc'); + } + + $PAGE->set_url('/auth/oidc/'); + $PAGE->set_context(system::instance()); + $PAGE->set_pagelayout('redirect'); + $PAGE->set_title(get_string('pageshouldredirect')); + + $message = get_config('auth_oidc', 'stateredirect_message'); + if (empty($message)) { + $message = get_string('errorauthunknownstate', 'auth_oidc'); + } + $message = format_text($message, FORMAT_HTML, ['context' => system::instance()]); + + $delay = (int) get_config('auth_oidc', 'stateredirect_delay'); + if ($delay < 0) { + $delay = 0; + } + + $url = new url('/login/index.php'); + + echo $OUTPUT->redirect_message($url->out(), $message, $delay, false, notification::NOTIFY_ERROR); + exit; + } + + /** + * Set a dedicated CSRF cookie (SameSite=None; Secure) before redirecting to the IdP. + * + * The main session cookie carries SameSite=Lax (MDL-83526), which browsers drop on + * cross-site form_post callbacks, so a separate cookie is required to carry the sesskey. + */ + protected function set_csrf_cookie(): void { + global $CFG; + // Always attempt to set the cookie with the Secure flag: browsers on plain HTTP will + // simply refuse to store it (safe no-op), while HTTPS terminated by a reverse proxy that + // Moodle isn't aware of (missing $CFG->sslproxy) will still honour it correctly. + $cookiepath = parse_url($CFG->wwwroot, PHP_URL_PATH) ?: '/'; + setcookie('auth_oidc_csrf', sesskey(), [ + 'expires' => time() + 5 * MINSECS, + 'path' => $cookiepath, + 'secure' => true, + 'httponly' => true, + 'samesite' => 'None', + ]); + } + + /** + * Clear the CSRF cookie set by set_csrf_cookie(). + * + * Public so that \auth_oidc\observers::handle_user_loggedout() can clear a stale cookie + * when a user logs out while an OIDC request is still pending. + */ + public function clear_csrf_cookie(): void { + global $CFG; + setcookie('auth_oidc_csrf', '', [ + 'expires' => time() - HOURSECS, + 'path' => parse_url($CFG->wwwroot, PHP_URL_PATH) ?: '/', + 'secure' => true, + 'httponly' => true, + 'samesite' => 'None', + ]); + } + + /** + * Verify the state record's sesskey against the CSRF cookie, falling back to the current + * session's sesskey when the cookie is absent. Always clears the cookie and, on failure, + * deletes the state record. + * + * @param stdClass $staterec + * @throws moodle_exception if the CSRF check fails. + */ + protected function verify_csrf_cookie(stdClass $staterec): void { + global $DB; + + $csrfcookie = $_COOKIE['auth_oidc_csrf'] ?? null; + $csrftoken = (is_string($csrfcookie) && $csrfcookie !== '') ? $csrfcookie : sesskey(); + $valid = hash_equals((string) $staterec->sesskey, (string) $csrftoken); + + $this->clear_csrf_cookie(); + + if (!$valid) { + $DB->delete_records('auth_oidc_state', ['id' => $staterec->id]); + utils::debug( + $csrfcookie === null ? 'CSRF cookie missing on OIDC callback.' : 'CSRF cookie mismatch on OIDC callback.', + __METHOD__, + ['staterecid' => $staterec->id] + ); + throw new moodle_exception('errorauthunknownstate', 'auth_oidc'); + } + } + /** * Handles the response for certificate-based admin consent authorization. * @@ -297,13 +433,18 @@ protected function handlecertadminconsentresponse(array $authparams) { if (!isset($authparams['state'])) { utils::debug('No state received.', __METHOD__, $authparams); - throw new moodle_exception('errorauthunknownstate', 'auth_oidc'); + $this->handlemissingstaterecord(); } // Validate and expire state. $staterec = $DB->get_record('auth_oidc_state', ['state' => $authparams['state']]); if (empty($staterec)) { - throw new moodle_exception('errorauthunknownstate', 'auth_oidc'); + $this->handlemissingstaterecord(); + } + + $csrfverified = is_https() || !empty($CFG->sslproxy) || !empty($_COOKIE['auth_oidc_csrf']); + if ($csrfverified) { + $this->verify_csrf_cookie($staterec); } $orignonce = $staterec->nonce; @@ -317,9 +458,33 @@ protected function handlecertadminconsentresponse(array $authparams) { $SESSION->stateadditionaldata = $additionaldata; $DB->delete_records('auth_oidc_state', ['id' => $staterec->id]); - // Get token. + // Re-associate the browser session with the admin who initiated the consent request before doing + // anything that might fail below, so a failure in the auto-detection step (see below) does not leave + // the admin looking logged out on the resulting page. + if ($csrfverified && !empty($additionaldata['initiatinguserid'])) { + $initiatinguser = core_user::get_user((int) $additionaldata['initiatinguserid']); + if ( + $initiatinguser && empty($initiatinguser->deleted) && + (!isloggedin() || isguestuser()) + ) { + \core\session\manager::login_user($initiatinguser); + } + } + + // Get token. This app-only token is only used to auto-detect the Microsoft Entra tenant and OneDrive for + // Business URL settings below; admin consent itself has already been granted by Microsoft Entra by this + // point. Conditional Access policies can block this specific app-only token request (AADSTS53003) even + // though consent succeeded, and the tenant/URL can still be auto-detected via other Graph API calls, so + // that failure is not fatal and is silently redirected past. $client = $this->get_oidcclient(); - $tokenparams = $client->app_access_token_request(); + try { + $tokenparams = $client->app_access_token_request(); + } catch (moodle_exception $e) { + if ($e->errorcode === 'settings_adminconsent_error_53003' && $e->module === 'local_o365') { + redirect($e->link); + } + throw $e; + } if (!isset($tokenparams['access_token'])) { throw new moodle_exception('errorauthnoaccesstoken', 'auth_oidc'); } @@ -360,13 +525,18 @@ protected function handleauthresponse(array $authparams) { if (!isset($authparams['state'])) { utils::debug('No state received.', __METHOD__, $authparams); - throw new moodle_exception('errorauthunknownstate', 'auth_oidc'); + $this->handlemissingstaterecord(); } // Validate and expire state. $staterec = $DB->get_record('auth_oidc_state', ['state' => $authparams['state']]); if (empty($staterec)) { - throw new moodle_exception('errorauthunknownstate', 'auth_oidc'); + $this->handlemissingstaterecord(); + } + + $csrfverified = is_https() || !empty($CFG->sslproxy) || !empty($_COOKIE['auth_oidc_csrf']); + if ($csrfverified) { + $this->verify_csrf_cookie($staterec); } $orignonce = $staterec->nonce; @@ -409,13 +579,49 @@ protected function handleauthresponse(array $authparams) { ]; $event = user_authed::create($eventdata); $event->trigger(); + + if ($csrfverified && !empty($additionaldata['initiatinguserid'])) { + $initiatinguser = core_user::get_user((int) $additionaldata['initiatinguserid']); + if ( + $initiatinguser && empty($initiatinguser->deleted) && + (!isloggedin() || isguestuser()) + ) { + \core\session\manager::login_user($initiatinguser); + } + } + + if (!empty($additionaldata['redirect'])) { + redirect(new url($additionaldata['redirect'])); + } + return true; } // Check if OIDC user is already migrated. $tokenrec = $DB->get_record('auth_oidc_token', ['oidcuniqid' => $oidcuniqid]); - if (isloggedin() && !isguestuser() && (empty($tokenrec) || (isset($USER->auth) && $USER->auth !== 'oidc'))) { - // If user is already logged in and trying to link Microsoft 365 account or use it for OIDC. + + // Determine who initiated this request. Prefer the user id stored against the state record over the live + // session: the state record is only returned by a legitimate, single-use round trip through the OP, so it + // can be trusted even when the session cookie did not survive the callback (e.g. SameSite=Lax blocking the + // cross-site POST used by response_mode=form_post). Fall back to the live session for state records created + // before this was captured. + $linkinguser = null; + if (!empty($additionaldata['initiatinguserid'])) { + $linkinguser = core_user::get_user((int)$additionaldata['initiatinguserid']); + if (!$linkinguser || !empty($linkinguser->deleted)) { + $linkinguser = null; + } + } else if (isloggedin() && !isguestuser()) { + $linkinguser = $USER; + } + + if ($linkinguser && (empty($tokenrec) || (isset($linkinguser->auth) && $linkinguser->auth !== 'oidc'))) { + // If the initiating user is trying to link a Microsoft 365 account or use it for OIDC, make sure the + // current session actually belongs to them, restoring it if the cookie was dropped on the callback. + if (!isloggedin() || isguestuser() || (int)$USER->id !== (int)$linkinguser->id) { + \core\session\manager::login_user($linkinguser); + } + // Check if that Microsoft 365 account already exists in moodle. $oidcusername = $this->get_oidc_username_from_token_claim($idtoken); @@ -466,6 +672,12 @@ protected function handleauthresponse(array $authparams) { $authoidsidrecord->userid = $USER->id; $authoidsidrecord->sid = $sid; $authoidsidrecord->timecreated = time(); + // Store the Moodle session id so logout.php can terminate the correct session directly, + // without depending on the MoodleSession cookie being present on the IdP's logout request. + $authoidsidrecord->sessionid = session_id(); + // Store the issuer so logout.php can verify that a front-channel logout request naming + // this sid actually originates from the same IdP/tenant that issued it. + $authoidsidrecord->iss = $idtoken->claim('iss'); $DB->insert_record('auth_oidc_sid', $authoidsidrecord); } redirect(core_login_get_return_url()); @@ -574,7 +786,11 @@ protected function check_for_matched($entraidupn) { global $DB; if (auth_oidc_is_local_365_installed()) { - $match = $DB->get_record('local_o365_connections', ['entraidupn' => $entraidupn]); + $entraidupn = trim($entraidupn); + $sql = 'SELECT * + FROM {local_o365_connections} + WHERE ' . $DB->sql_equal('entraidupn', ':entraidupn', false); + $match = $DB->get_record_sql($sql, ['entraidupn' => $entraidupn]); if (!empty($match) && \local_o365\utils::is_o365_connected($match->muserid) !== true) { return $DB->get_record('user', ['id' => $match->muserid]); } @@ -739,6 +955,10 @@ protected function handlelogin(string $oidcuniqid, array $authparams, array $tok $user = authenticate_user_login($username, '', true); if (!empty($user)) { + // Look for plugins that want to add extra checks before user login is completed. + $hook = new \auth_oidc\hook\before_login_completed($idtoken); + di::get(\core\hook\manager::class)->dispatch($hook); + complete_user_login($user); } else { // There was a problem in authenticate_user_login. @@ -809,6 +1029,10 @@ protected function handlelogin(string $oidcuniqid, array $authparams, array $tok $user = authenticate_user_login($username, '', true); if (!empty($user)) { + // Look for plugins that want to add extra checks before user login is completed. + $hook = new \auth_oidc\hook\before_login_completed($idtoken); + di::get(\core\hook\manager::class)->dispatch($hook); + complete_user_login($user); } else { // There was a problem in authenticate_user_login. @@ -850,6 +1074,10 @@ protected function handlelogin(string $oidcuniqid, array $authparams, array $tok $matchedwith->entraidupn = $username; throw new moodle_exception('errorusermatched', 'auth_oidc', null, $matchedwith); } + // The matched Moodle user is already set to auth 'oidc': bind the login to that user's own + // username rather than the Microsoft-derived one, which may not match it (e.g. a manual match + // keyed on the full UPN while the Moodle username is only the UPN prefix). + $username = $matchedwith->username; } $username = trim(core_text::strtolower($username)); $tokenrec = $this->createtoken($oidcuniqid, $username, $authparams, $tokenparams, $idtoken, 0, $originalupn); @@ -895,6 +1123,11 @@ protected function handlelogin(string $oidcuniqid, array $authparams, array $tok $updatedtokenrec->userid = $user->id; $DB->update_record('auth_oidc_token', $updatedtokenrec); } + + // Look for plugins that want to add extra checks before user login is completed. + $hook = new \auth_oidc\hook\before_login_completed($idtoken); + di::get(\core\hook\manager::class)->dispatch($hook); + complete_user_login($user); } else { // There was a problem in authenticate_user_login. Clean up incomplete token record. diff --git a/classes/loginflow/rocreds.php b/classes/loginflow/rocreds.php deleted file mode 100644 index 0152bae25..000000000 --- a/classes/loginflow/rocreds.php +++ /dev/null @@ -1,210 +0,0 @@ -. - -/** - * Resource Owner Password Credentials Grant login flow. - * - * @package auth_oidc - * @author James McQuillan - * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later - * @copyright (C) 2014 onwards Microsoft, Inc. (http://microsoft.com/) - */ - -namespace auth_oidc\loginflow; - -use auth_oidc\event\user_created; -use auth_oidc\utils; - -defined('MOODLE_INTERNAL') || die(); - -require_once($CFG->dirroot . '/auth/oidc/lib.php'); - -/** - * Login flow for the oauth2 resource owner credentials grant. - */ -class rocreds extends base { - /** - * Check for an existing user object. - * - * @param string $o356username - * - * @return string If there is an existing user object, return the username associated with it. - * If there is no existing user object, return the original username. - */ - protected function check_objects($o356username) { - global $DB; - - $user = null; - if (auth_oidc_is_local_365_installed()) { - $sql = 'SELECT u.username - FROM {local_o365_objects} obj - JOIN {user} u ON u.id = obj.moodleid - WHERE obj.o365name = ? and obj.type = ?'; - $params = [$o356username, 'user']; - $user = $DB->get_record_sql($sql, $params); - } - - return (!empty($user)) ? $user->username : $o356username; - } - - /** - * Provides a hook into the login page. - * - * @param stdClass $frm Form object. - * @param stdClass $user User object. - * @return bool - */ - public function loginpage_hook(&$frm, &$user) { - global $DB; - - if (empty($frm)) { - $frm = data_submitted(); - } - if (empty($frm)) { - return true; - } - - $username = $frm->username; - $password = $frm->password; - $auth = 'oidc'; - - $username = $this->check_objects($username); - if ($username !== $frm->username) { - $success = $this->user_login($username, $password); - if ($success === true) { - $existinguser = $DB->get_record('user', ['username' => $username]); - if (!empty($existinguser)) { - $user = $existinguser; - return true; - } - } - } - - $autoappend = get_config('auth_oidc', 'autoappend'); - if (empty($autoappend)) { - // If we're not doing autoappend, just let things flow naturally. - return true; - } - - $existinguser = $DB->get_record('user', ['username' => $username]); - if (!empty($existinguser)) { - // We don't want to prevent access to existing accounts. - return true; - } - - $username .= $autoappend; - $success = $this->user_login($username, $password); - if ($success !== true) { - // No o365 user, continue normally. - return false; - } - - $existinguser = $DB->get_record('user', ['username' => $username]); - if (!empty($existinguser)) { - $user = $existinguser; - return true; - } - - // The user is authenticated but user creation may be disabled. - if (!empty($CFG->authpreventaccountcreation)) { - $failurereason = AUTH_LOGIN_UNAUTHORISED; - - // Trigger login failed event. - $event = \core\event\user_login_failed::create([ - 'other' => [ - 'username' => $username, - 'reason' => $failurereason, - ], - ]); - $event->trigger(); - - debugging('[client ' . getremoteaddr() . "] $CFG->wwwroot Unknown user, can not create new accounts: $username " . - $_SERVER['HTTP_USER_AGENT']); - - return false; - } - - $user = create_user_record($username, $password, $auth); - - // Trigger user_created event. - $eventdata = [ - 'objectid' => $user->id, - 'userid' => $user->id, - 'relateduserid' => $user->id, - ]; - $event = user_created::create($eventdata); - $event->trigger(); - - return true; - } - - /** - * This is the primary method that is used by the authenticate_user_login() function in moodlelib.php. - * - * @param string $username The username (with system magic quotes) - * @param string $password The password (with system magic quotes) - * @return bool Authentication success or failure. - */ - public function user_login($username, $password = null) { - global $DB; - - $client = $this->get_oidcclient(); - $authparams = ['code' => '']; - - $oidcusername = $username; - $oidctoken = $DB->get_records('auth_oidc_token', ['username' => $username]); - if (!empty($oidctoken)) { - $oidctoken = array_shift($oidctoken); - if (!empty($oidctoken) && !empty($oidctoken->oidcusername)) { - $oidcusername = $oidctoken->oidcusername; - } - } - - // Make request. - $tokenparams = $client->rocredsrequest($oidcusername, $password); - if (!empty($tokenparams) && isset($tokenparams['token_type']) && $tokenparams['token_type'] === 'Bearer') { - [$oidcuniqid, $idtoken] = $this->process_idtoken($tokenparams['id_token']); - - // Check restrictions. - $passed = $this->checkrestrictions($idtoken); - if ($passed !== true) { - $errstr = 'User prevented from logging in due to restrictions.'; - utils::debug($errstr, __METHOD__, $idtoken); - return false; - } - - $tokenrec = $DB->get_record('auth_oidc_token', ['oidcuniqid' => $oidcuniqid]); - if (!empty($tokenrec)) { - $this->updatetoken($tokenrec->id, $authparams, $tokenparams); - } else { - $originalupn = null; - if (auth_oidc_is_local_365_installed()) { - $apiclient = \local_o365\utils::get_api(); - $userdetails = $apiclient->get_user($oidcuniqid); - if ( - !is_null($userdetails) && isset($userdetails['userPrincipalName']) && - stripos($userdetails['userPrincipalName'], '#EXT#') !== false - ) { - $originalupn = $userdetails['userPrincipalName']; - } - } - $this->createtoken($oidcuniqid, $username, $authparams, $tokenparams, $idtoken, 0, $originalupn); - } - return true; - } - return false; - } -} diff --git a/classes/observers.php b/classes/observers.php index 9f715e511..e11917cd4 100644 --- a/classes/observers.php +++ b/classes/observers.php @@ -25,6 +25,7 @@ namespace auth_oidc; +use auth_oidc\loginflow\authcode; use core\event\user_deleted; use core\event\user_loggedout; @@ -47,4 +48,25 @@ public static function handle_user_deleted(user_deleted $event) { $userid = $event->objectid; return $DB->delete_records('auth_oidc_token', ['userid' => $userid]); } + + /** + * Handle user_loggedout event - invalidate any pending OIDC state tied to the CSRF cookie + * of the session that just logged out, so a still-in-flight callback (e.g. admin consent) + * cannot silently re-authenticate a user who explicitly logged out. + * + * @param user_loggedout $event The triggered event. + * @return bool Success/Failure. + */ + public static function handle_user_loggedout(user_loggedout $event) { + global $DB; + + $csrfcookie = $_COOKIE['auth_oidc_csrf'] ?? null; + if (is_string($csrfcookie) && $csrfcookie !== '') { + $DB->delete_records('auth_oidc_state', ['sesskey' => $csrfcookie]); + } + + (new authcode())->clear_csrf_cookie(); + + return true; + } } diff --git a/classes/oidcclient.php b/classes/oidcclient.php index 54bf6b59b..c8d874809 100644 --- a/classes/oidcclient.php +++ b/classes/oidcclient.php @@ -301,44 +301,6 @@ public function adminconsentrequest(array $stateparams = [], array $extraparams redirect($redirecturl); } - /** - * Make a token request using the resource-owner credentials login flow. - * - * @param string $username The resource owner's username. - * @param string $password The resource owner's password. - * @return array Received parameters. - */ - public function rocredsrequest($username, $password) { - if (empty($this->endpoints['token'])) { - throw new moodle_exception('erroroidcclientnotokenendpoint', 'auth_oidc'); - } - - if (strpos($this->endpoints['token'], 'https://') !== 0) { - throw new moodle_exception('erroroidcclientinsecuretokenendpoint', 'auth_oidc'); - } - - $params = [ - 'grant_type' => 'password', - 'username' => $username, - 'password' => $password, - 'scope' => 'openid profile email', - 'client_id' => $this->clientid, - 'client_secret' => $this->clientsecret, - ]; - - if (get_config('auth_oidc', 'idptype') != AUTH_OIDC_IDP_TYPE_MICROSOFT_IDENTITY_PLATFORM) { - $params['resource'] = $this->tokenresource; - } - - try { - $returned = $this->httpclient->post($this->endpoints['token'], $params); - return utils::process_json_response($returned, ['token_type' => null, 'id_token' => null]); - } catch (moodle_exception $e) { - utils::debug('Error in rocredsrequest request', __METHOD__, $e->getMessage()); - return false; - } - } - /** * Exchange an authorization code for an access token. * diff --git a/classes/privacy/provider.php b/classes/privacy/provider.php index 2dda27996..fd79912cc 100644 --- a/classes/privacy/provider.php +++ b/classes/privacy/provider.php @@ -73,6 +73,8 @@ public static function get_metadata(collection $collection): collection { 'userid', 'sid', 'timecreated', + 'sessionid', + 'iss', ], ]; diff --git a/classes/task/cleanup_oidc_sid.php b/classes/task/cleanup_oidc_sid.php index b6ed866a1..b9e5047ed 100644 --- a/classes/task/cleanup_oidc_sid.php +++ b/classes/task/cleanup_oidc_sid.php @@ -25,6 +25,7 @@ namespace auth_oidc\task; +use core\session\manager; use core\task\scheduled_task; /** @@ -40,10 +41,33 @@ public function get_name() { /** * Clean up OIDC SID records. + * + * A mapping is only removed once its Moodle session no longer exists, rather than after a fixed + * time period, so that SSO logout keeps working for sessions that outlive that fixed period. */ public function execute() { global $DB; - $DB->delete_records_select('auth_oidc_sid', 'timecreated < ?', [strtotime('-1 day')]); + // Legacy mappings with no recorded session id (created before sessionid was tracked) can never + // be confirmed to have a live session, so delete them directly without a session_exists() check. + $DB->delete_records_select('auth_oidc_sid', 'sessionid IS NULL OR sessionid = ?', ['']); + + // Fetch only the columns needed to decide what to keep, all into memory up front: issuing + // further queries (session_exists, delete_records) while an unbuffered recordset cursor is open + // can trigger "commands out of sync" errors on some DB drivers. + $records = $DB->get_records('auth_oidc_sid', null, '', 'id, sessionid'); + + $staleids = []; + foreach ($records as $record) { + if (!manager::session_exists($record->sessionid)) { + $staleids[] = $record->id; + } + } + + // Delete in chunks to avoid hitting parameter/query-length limits on some DB drivers if a large + // number of mappings have accumulated between cleanup runs. + foreach (array_chunk($staleids, 1000) as $chunk) { + $DB->delete_records_list('auth_oidc_sid', 'id', $chunk); + } } } diff --git a/classes/task/cleanup_oidc_state_and_token.php b/classes/task/cleanup_oidc_state_and_token.php index eb5304c2f..c6e321238 100644 --- a/classes/task/cleanup_oidc_state_and_token.php +++ b/classes/task/cleanup_oidc_state_and_token.php @@ -45,7 +45,12 @@ public function execute() { global $DB; // Clean up oidc state. - $DB->delete_records_select('auth_oidc_state', 'timecreated < ?', [strtotime('-5 min')]); + $stateexpiry = (int) get_config('auth_oidc', 'stateexpiry'); + if ($stateexpiry <= 0) { + $stateexpiry = 5; + } + $cutoff = time() - ($stateexpiry * MINSECS); + $DB->delete_records_select('auth_oidc_state', 'timecreated < ?', [$cutoff]); // Clean up invalid oidc token. $DB->delete_records('auth_oidc_token', ['userid' => 0]); diff --git a/classes/utils.php b/classes/utils.php index 5dbaf86dd..b68af94bf 100644 --- a/classes/utils.php +++ b/classes/utils.php @@ -28,6 +28,7 @@ use Exception; use moodle_exception; use auth_oidc\event\action_failed; +use core\context\system; use core\url; /** @@ -252,4 +253,161 @@ public static function get_openssl_internal_path() { return $CFG->dataroot . '/microsoft_certs'; } + + /** + * Add a unique constraint on (oidcuniqid, tokenresource) to the auth_oidc_token table, removing duplicate + * tokens first. + * + * Removes duplicate tokens, keeping the latest one for each (oidcuniqid, tokenresource) pair. + * Uses a temporary table to work around MySQL error 1093 and PostgreSQL parameter limits. + * + * The combined length of oidcuniqid (255 chars) and tokenresource (127 chars) exceeds the byte + * limit the XMLDB API enforces on composed indexes (xmldb_index::INDEX_COMPOSED_MAX_BYTES), even + * though both MySQL and PostgreSQL can create the index without issue. So the index is created + * with raw SQL instead of $dbman->add_index(), with the index name manually prefixed with the + * site's table prefix to avoid name collisions with other prefixes (e.g. PHPUnit or Behat test + * tables) sharing the same database/schema. + */ + public static function add_token_unique_constraint(): void { + global $DB; + + $dbman = $DB->get_manager(); + $table = new \xmldb_table('auth_oidc_token'); + $index = new \xmldb_index('oidcuniqid-tokenresource', XMLDB_INDEX_UNIQUE, ['oidcuniqid', 'tokenresource']); + + if ($dbman->index_exists($table, $index)) { + // Unique constraint already present, nothing to do. + return; + } + + // Deliberately let any failure here propagate: swallowing it would let the calling + // upgrade step reach its savepoint even though the uniqueness guarantee was never + // established, silently leaving the database inconsistent with the code. + $temptable = 'auth_oidc_token_keep_ids'; + + // Step 1: Create a temporary table with the IDs to keep. + $sql = "CREATE TEMPORARY TABLE {" . $temptable . "} (id INT PRIMARY KEY)"; + $DB->execute($sql); + + // Step 2: Insert the IDs to keep (latest token for each oidcuniqid, tokenresource pair). + $sql = "INSERT INTO {" . $temptable . "} (id) + SELECT MAX(id) FROM {auth_oidc_token} + GROUP BY oidcuniqid, tokenresource"; + $DB->execute($sql); + + // Step 3: Delete duplicates not in the temporary table. + $sql = "DELETE FROM {auth_oidc_token} WHERE id NOT IN (SELECT id FROM {" . $temptable . "})"; + $DB->execute($sql); + + // Step 4: Drop the temporary table (automatic on transaction end, but explicit for clarity). + // Note: PostgreSQL does not accept the TEMPORARY keyword in DROP TABLE (only in CREATE + // TABLE), so plain DROP TABLE is used here; it works for temporary tables on MySQL too. + $sql = "DROP TABLE IF EXISTS {" . $temptable . "}"; + $DB->execute($sql); + + // Step 5: Add unique constraint on (oidcuniqid, tokenresource) to prevent duplicate tokens. + $indexname = $DB->get_prefix() . 'authoidctoken_uniq_ix'; + $sql = "CREATE UNIQUE INDEX {$indexname} ON {auth_oidc_token} (oidcuniqid, tokenresource)"; + $DB->execute($sql); + } + + /** + * Migrate a site's selected stock icon to the custom icon setting if it used one of the + * icon choices that have been removed from the icon selector. + * + * The 'auth_oidc/icon' setting stores a "component:pix" identifier. The set of stock + * choices has been reduced to a handful of icons relevant to this plugin; any site that had + * selected one of the removed choices (all generic core Moodle icons) needs that icon copied + * into the custom icon file area so the login page keeps showing the same image. + * + * Safe to call more than once: once a site has been migrated (or its 'icon' setting was + * never one of the removed choices), every subsequent call is a no-op, since the checks + * above always return early once either 'auth_oidc/icon' is empty/unset or + * 'auth_oidc/customicon' is populated. The file and config writes are wrapped in a + * delegated transaction so a failure partway through can't leave those two settings out of + * sync with each other, which is what the early-return checks rely on. + */ + public static function migrate_removed_icon_choices(): void { + global $CFG, $DB; + + $currenticon = get_config('auth_oidc', 'icon'); + if (empty($currenticon)) { + return; + } + + if (!empty(get_config('auth_oidc', 'customicon'))) { + // A custom icon is already in use and takes priority, so the stock icon setting is + // not currently affecting what is displayed. Nothing to migrate. + return; + } + + // Icons that have simply been replaced with another stock icon: the choice was removed + // from the selector but a suitable replacement exists, so just point the setting at it. + $remappedicons = [ + 'auth_oidc:o365' => 'auth_oidc:office_365', + // The Microsoft 365 logo is now the single icon used for both Microsoft and Microsoft 365. + 'auth_oidc:microsoft' => 'auth_oidc:microsoft_365', + 'auth_oidc:microsoft_365_copilot' => 'auth_oidc:microsoft_365', + ]; + if (isset($remappedicons[$currenticon])) { + set_config('icon', $remappedicons[$currenticon], 'auth_oidc'); + return; + } + + $keepicons = [ + 'auth_oidc:microsoft_365', + 'auth_oidc:office_365', + 'auth_oidc:openid', + 'auth_oidc:keycloak', + ]; + if (in_array($currenticon, $keepicons, true)) { + return; + } + + $parts = explode(':', $currenticon, 2); + if (count($parts) !== 2) { + return; + } + [, $pix] = $parts; + + $sourcefile = null; + $extension = null; + foreach (['svg', 'png', 'gif', 'jpg', 'jpeg'] as $candidateextension) { + $candidatefile = "{$CFG->dirroot}/pix/{$pix}.{$candidateextension}"; + if (file_exists($candidatefile)) { + $sourcefile = $candidatefile; + $extension = $candidateextension; + break; + } + } + if ($sourcefile === null) { + // Can't locate the source image for the removed choice, so there is nothing to copy. + return; + } + + $systemcontext = system::instance(); + $fs = get_file_storage(); + $filename = 'migrated_' . clean_param(str_replace('/', '_', $pix), PARAM_FILE) . '.' . $extension; + $filerecord = [ + 'contextid' => $systemcontext->id, + 'component' => 'auth_oidc', + 'filearea' => 'customicon', + 'itemid' => 0, + 'filepath' => '/', + 'filename' => $filename, + ]; + + // Wrapped in a transaction so a failure partway through (e.g. the file write succeeding + // but a config write failing) can't leave 'icon' and 'customicon' out of sync, which + // would break the early-return guards above on any later call. + $transaction = $DB->start_delegated_transaction(); + $fs->delete_area_files($systemcontext->id, 'auth_oidc', 'customicon', 0); + $fs->create_file_from_pathname($filerecord, $sourcefile); + set_config('customicon', '/' . $filename, 'auth_oidc'); + unset_config('icon', 'auth_oidc'); + $transaction->allow_commit(); + + require_once($CFG->dirroot . '/auth/oidc/lib.php'); + auth_oidc_initialize_customicon('/' . $filename); + } } diff --git a/cleanupoidctokens.php b/cleanupoidctokens.php index 604f56f10..06f25e6e2 100644 --- a/cleanupoidctokens.php +++ b/cleanupoidctokens.php @@ -56,6 +56,7 @@ $deletetokenid = optional_param('id', 0, PARAM_INT); if ($deletetokenid) { + require_sesskey(); if (array_key_exists($deletetokenid, $tokenstoclean)) { auth_oidc_delete_token($deletetokenid); @@ -89,9 +90,9 @@ foreach ($tokenstoclean as $item) { $table->data[] = [ $item->id, - $item->oidcusername, - $item->useridentifier, - $item->oidcuniqueid, + s($item->oidcusername), + s($item->useridentifier), + s($item->oidcuniqueid), $item->matchingstatus, $item->details, $item->action, diff --git a/db/events.php b/db/events.php index 376c05a7f..0013df416 100644 --- a/db/events.php +++ b/db/events.php @@ -32,4 +32,10 @@ 'priority' => 200, 'internal' => false, ], + [ + 'eventname' => '\core\event\user_loggedout', + 'callback' => '\auth_oidc\observers::handle_user_loggedout', + 'priority' => 200, + 'internal' => false, + ], ]; diff --git a/db/install.php b/db/install.php index b8612d116..31bcadf7d 100644 --- a/db/install.php +++ b/db/install.php @@ -27,18 +27,13 @@ * Installation script. */ function xmldb_auth_oidc_install() { - global $DB; - // Set the default value for the bindingusernameclaim setting. $bindingusernameclaimconfig = get_config('auth_oidc', 'bindingusernameclaim'); if (empty($bindingusernameclaimconfig)) { set_config('bindingusernameclaim', 'auto', 'auth_oidc'); } - // Create unique constraint on (oidcuniqid, tokenresource) to prevent duplicate tokens. - // Use CREATE UNIQUE INDEX which works on both MySQL and PostgreSQL. - // Note: PostgreSQL doesn't support column length prefixes, so we use full columns. - // For MySQL, the columns are naturally short enough (GUID + resource URL). - $sql = 'CREATE UNIQUE INDEX idx_oidc_unique ON {auth_oidc_token} (oidcuniqid, tokenresource)'; - $DB->execute($sql); + // The unique constraint on (oidcuniqid, tokenresource) can't be declared in install.xml: it + // exceeds the XMLDB API's byte limit for composed indexes, so it must be added with raw SQL. + \auth_oidc\utils::add_token_unique_constraint(); } diff --git a/db/install.xml b/db/install.xml index 038f48779..231e8df26 100644 --- a/db/install.xml +++ b/db/install.xml @@ -1,5 +1,5 @@ - @@ -65,12 +65,17 @@ - + + + + + + diff --git a/db/upgrade.php b/db/upgrade.php index c4d776684..b8f965f88 100644 --- a/db/upgrade.php +++ b/db/upgrade.php @@ -24,6 +24,8 @@ * @copyright (C) 2014 onwards Microsoft, Inc. (http://microsoft.com/) */ +use auth_oidc\utils; + defined('MOODLE_INTERNAL') || die(); require_once($CFG->dirroot . '/auth/oidc/lib.php'); @@ -582,48 +584,95 @@ function xmldb_auth_oidc_upgrade($oldversion) { } if ($oldversion < 2025100601.01) { - upgrade_auth_oidc_add_token_constraint(); + utils::add_token_unique_constraint(); upgrade_plugin_savepoint(true, 2025100601.01, 'auth', 'oidc'); } - return true; -} + if ($oldversion < 2026042000.01) { + // The rocreds (Resource Owner Password Credentials Grant) login flow has been removed. + // Reset any site still configured to use it back to the authcode flow. + if (get_config('auth_oidc', 'loginflow') === 'rocreds') { + set_config('loginflow', 'authcode', 'auth_oidc'); + } -/** - * Helper function to add unique constraint and remove duplicate tokens. - * - * Removes duplicate tokens, keeping the latest one for each (oidcuniqid, tokenresource) pair. - * Uses a temporary table to work around MySQL error 1093 and PostgreSQL parameter limits. - */ -function upgrade_auth_oidc_add_token_constraint(): void { - global $DB; + upgrade_plugin_savepoint(true, 2026042000.01, 'auth', 'oidc'); + } + + if ($oldversion < 2026042000.02) { + // Retry adding the unique constraint: the previous (2025100601.01) attempt used a helper + // that silently swallowed failures, so sites where that step failed never got the + // constraint despite passing the savepoint. utils::add_token_unique_constraint() is + // idempotent (checks index_exists() first), so this is a no-op where .01 already succeeded. + utils::add_token_unique_constraint(); + upgrade_plugin_savepoint(true, 2026042000.02, 'auth', 'oidc'); + } + + if ($oldversion < 2026042000.04) { + // Widen the sid field on auth_oidc_sid: it stores the session_state parameter, which can + // exceed 36 characters (e.g. two GUIDs joined by a dot), not a fixed-length identifier. + $table = new xmldb_table('auth_oidc_sid'); + $field = new xmldb_field('sid', XMLDB_TYPE_CHAR, '255', null, XMLDB_NOTNULL, null, null, 'userid'); + $index = new xmldb_index('sid', XMLDB_INDEX_NOTUNIQUE, ['sid']); + + if ($dbman->field_exists($table, $field)) { + // The sid index depends on this field, so the DDL layer refuses to alter it in + // place (ddl_dependency_exception); drop the index first and recreate it once the + // field has been widened. + if ($dbman->index_exists($table, $index)) { + $dbman->drop_index($table, $index); + } + + $dbman->change_field_precision($table, $field); + + if (!$dbman->index_exists($table, $index)) { + $dbman->add_index($table, $index); + } + } + + // Oidc savepoint reached. + upgrade_plugin_savepoint(true, 2026042000.04, 'auth', 'oidc'); + } - try { - $temptable = 'auth_oidc_token_keep_ids'; - - // Step 1: Create a temporary table with the IDs to keep. - $sql = "CREATE TEMPORARY TABLE {" . $temptable . "} (id INT PRIMARY KEY)"; - $DB->execute($sql); - - // Step 2: Insert the IDs to keep (latest token for each oidcuniqid, tokenresource pair). - $sql = "INSERT INTO {" . $temptable . "} (id) - SELECT MAX(id) FROM {auth_oidc_token} - GROUP BY oidcuniqid, tokenresource"; - $DB->execute($sql); - - // Step 3: Delete duplicates not in the temporary table. - $sql = "DELETE FROM {auth_oidc_token} WHERE id NOT IN (SELECT id FROM {" . $temptable . "})"; - $DB->execute($sql); - - // Step 4: Drop the temporary table (automatic on transaction end, but explicit for clarity). - $sql = "DROP TEMPORARY TABLE IF EXISTS {" . $temptable . "}"; - $DB->execute($sql); - - // Step 5: Add unique constraint on (oidcuniqid, tokenresource) to prevent duplicate tokens. - // Use CREATE UNIQUE INDEX which works on both MySQL and PostgreSQL. - $sql = 'CREATE UNIQUE INDEX idx_oidc_unique ON {auth_oidc_token} (oidcuniqid, tokenresource)'; - $DB->execute($sql); - } catch (Exception $e) { - unset($e); + if ($oldversion < 2026042000.05) { + \auth_oidc\utils::migrate_removed_icon_choices(); + upgrade_plugin_savepoint(true, 2026042000.05, 'auth', 'oidc'); } + + if ($oldversion < 2026042000.06) { + // Define field sessionid to be added to auth_oidc_sid. + $table = new xmldb_table('auth_oidc_sid'); + $field = new xmldb_field('sessionid', XMLDB_TYPE_CHAR, '128', null, null, null, null, 'timecreated'); + + // Conditionally launch add field sessionid. + if (!$dbman->field_exists($table, $field)) { + $dbman->add_field($table, $field); + } + + // Define index sid to be added to auth_oidc_sid. + $index = new xmldb_index('sid', XMLDB_INDEX_NOTUNIQUE, ['sid']); + + // Conditionally launch add index sid. + if (!$dbman->index_exists($table, $index)) { + $dbman->add_index($table, $index); + } + + // Define field iss to be added to auth_oidc_sid. + $table = new xmldb_table('auth_oidc_sid'); + $field = new xmldb_field('iss', XMLDB_TYPE_CHAR, '255', null, null, null, null, 'sessionid'); + + // Conditionally launch add field iss. + if (!$dbman->field_exists($table, $field)) { + $dbman->add_field($table, $field); + } + + // Oidc savepoint reached. + upgrade_plugin_savepoint(true, 2026042000.06, 'auth', 'oidc'); + } + + if ($oldversion < 2026042000.08) { + \auth_oidc\utils::migrate_removed_icon_choices(); + upgrade_plugin_savepoint(true, 2026042000.08, 'auth', 'oidc'); + } + + return true; } diff --git a/lang/cs/auth_oidc.php b/lang/cs/auth_oidc.php index 08e46baf9..80961a9d7 100644 --- a/lang/cs/auth_oidc.php +++ b/lang/cs/auth_oidc.php @@ -65,8 +65,6 @@ $string['cfg_loginflow_key'] = 'Postup přihlášení'; $string['cfg_loginflow_authcode'] = 'Požadavek na autorizaci'; $string['cfg_loginflow_authcode_desc'] = 'Při použití tohoto postupu uživatel na přihlašovací stránce Moodlu klikne na ikonu poskytovatele identity (viz výše „Název poskytovatele“) a je následně přesměrován na poskytovatele, aby se přihlásil. Po úspěšném přihlášení je uživatel přesměrován zpět do Moodlu, kde proběhne transparentní přihlášení do Moodlu. Toto je nejstandardizovanější bezpečný způsob přihlašování uživatelů.'; -$string['cfg_loginflow_rocreds'] = 'Ověřování uživatelské_jméno/heslo'; -$string['cfg_loginflow_rocreds_desc'] = 'Při použití tohoto postupu uživatel zadá své uživatelské jméno a heslo do přihlašovacího formuláře Moodlu, obdobně jako při ručním přihlášení. Jeho přihlašovací údaje jsou pak na pozadí předány poskytovateli identity, aby bylo získáno ověření. Tento postup je pro uživatele nejtransparentnější, protože nemá žádný přímý kontakt s poskytovatelem identity. Ne všichni poskytovatelé identity ale tento postup podporují.'; $string['cfg_oidcresource_key'] = 'Zdroj'; $string['cfg_oidcresource_desc'] = 'Zdroj OpenID Connect, pro který se odesílá požadavek.'; $string['cfg_oidcscope_key'] = 'Scope'; diff --git a/lang/de/auth_oidc.php b/lang/de/auth_oidc.php index fb3bd3238..404b1e8be 100644 --- a/lang/de/auth_oidc.php +++ b/lang/de/auth_oidc.php @@ -65,8 +65,6 @@ $string['cfg_loginflow_key'] = 'Anmeldefluss'; $string['cfg_loginflow_authcode'] = 'Autorisierungsanforderung'; $string['cfg_loginflow_authcode_desc'] = 'Mit diesem Fluss klickt der Benutzer auf der Moodle-Anmeldeseite auf den Namen des Identitätsproviders (siehe "Providername" weiter oben) und wird zur Anmeldung zum Provider umgeleitet. Nach erfolgreicher Anmeldung wird der Benutzer zurück zu Moodle umgeleitet, wo die Moodle-Anmeldung transparent durchgeführt wird. Dies ist die am meisten standardisierte und sicherste Möglichkeit der Benutzeranmeldung.'; -$string['cfg_loginflow_rocreds'] = 'Authentifizierung mit Benutzername/Kennwort'; -$string['cfg_loginflow_rocreds_desc'] = 'Mit diesem Fluss gibt der Benutzer wie bei einer manuellen Anmeldung seinen Benutzernamen und sein Kennwort im Moodle-Anmeldeformular ein. Die Anmeldedaten werden dann im Hintergrund zur Authentifizierung an den Identitätsprovider übermittelt. Dieser Fluss ist für den Benutzer am transparentesten, da er keine direkte Interaktion mit dem Identitätsprovider hat. Alle Identitätsprovider unterstützen diesen Fluss.'; $string['cfg_oidcresource_key'] = 'Ressource'; $string['cfg_oidcresource_desc'] = 'Die OpenID Connect-Ressource, für die die Anfrage gesendet wird.'; $string['cfg_oidcscope_key'] = 'Umfang'; diff --git a/lang/en/auth_oidc.php b/lang/en/auth_oidc.php index c6031d7e0..0cec1a856 100644 --- a/lang/en/auth_oidc.php +++ b/lang/en/auth_oidc.php @@ -43,6 +43,8 @@ $string['heading_basic_desc'] = ''; $string['heading_additional_options'] = 'Additional options'; $string['heading_additional_options_desc'] = ''; +$string['heading_stateredirect'] = 'Login state error page'; +$string['heading_stateredirect_desc'] = 'By default, if a user takes too long to complete login at the identity provider (for example, while approving a multi-factor authentication prompt) and the stored login state has since been cleaned up, Moodle shows its generic error page. The settings below let you extend how long login state is kept, and let you show a friendlier, customizable message instead of the generic error page, automatically redirecting the user back to the login page.'; $string['heading_user_restrictions'] = 'User restrictions'; $string['heading_user_restrictions_desc'] = ''; $string['heading_sign_out'] = 'Sign out integration'; @@ -111,38 +113,34 @@ $string['cfg_err_invalidauthendpoint'] = 'Invalid Authorization Endpoint'; $string['cfg_err_invalidtokenendpoint'] = 'Invalid Token Endpoint'; $string['cfg_err_invalidclientid'] = 'Invalid client ID'; -$string['error_masked_secret_not_changed'] = 'Please enter a new value. The masked value cannot be saved.'; $string['cfg_err_invalidclientsecret'] = 'Invalid client secret'; $string['cfg_forceredirect_key'] = 'Force redirect'; $string['cfg_forceredirect_desc'] = 'If enabled, will skip the login index page and redirect to the OpenID Connect page. Can be bypassed with ?noredirect=1 URL param'; +$string['cfg_stateexpiry_key'] = 'Login state expiry (minutes)'; +$string['cfg_stateexpiry_desc'] = 'How long a login state record is kept before the scheduled cleanup task removes it. Increase this if users regularly take longer than this to complete login at the identity provider (for example, due to multi-factor authentication prompts).'; +$string['error_stateexpiry_min'] = 'The login state expiry must be at least 1 minute. A value of zero or less would cause logins that are currently in progress to fail.'; +$string['cfg_stateredirect_enabled_key'] = 'Show friendly error page'; +$string['cfg_stateredirect_enabled_desc'] = 'If enabled, a user who takes too long to complete login at the identity provider (so that the stored login state has since been cleaned up) is shown the customizable message below and automatically redirected back to the login page, instead of Moodle\'s generic error page.'; +$string['cfg_stateredirect_message_key'] = 'Message'; +$string['cfg_stateredirect_message_desc'] = 'The message to display to the user on the friendly error page.'; +$string['cfg_stateredirect_message_default'] = 'There was a problem logging you in. This is most likely because the login took too long to complete (for example, while approving a multi-factor authentication prompt). You will be redirected to the login page automatically so you can try again.'; +$string['cfg_stateredirect_delay_key'] = 'Redirect delay (seconds)'; +$string['cfg_stateredirect_delay_desc'] = 'The number of seconds to show the message above before automatically redirecting the user back to the login page.'; $string['cfg_set_pix_key'] = 'Show icon on login page'; $string['cfg_set_pix_desc'] = 'If enabled, displays an icon next to the provider name on the login page.'; $string['cfg_icon_key'] = 'Icon'; $string['cfg_icon_desc'] = 'An icon to display next to the provider name on the login page.'; -$string['cfg_iconalt_o365'] = 'Microsoft 365 icon'; -$string['cfg_iconalt_locked'] = 'Locked icon'; -$string['cfg_iconalt_lock'] = 'Lock icon'; -$string['cfg_iconalt_go'] = 'Green circle'; -$string['cfg_iconalt_stop'] = 'Red circle'; -$string['cfg_iconalt_user'] = 'User icon'; -$string['cfg_iconalt_user2'] = 'User icon alternate'; -$string['cfg_iconalt_key'] = 'Key icon'; -$string['cfg_iconalt_group'] = 'Group icon'; -$string['cfg_iconalt_group2'] = 'Group icon alternate'; -$string['cfg_iconalt_mnet'] = 'MNET icon'; -$string['cfg_iconalt_userlock'] = 'User with lock icon'; -$string['cfg_iconalt_plus'] = 'Plus icon'; -$string['cfg_iconalt_check'] = 'Checkmark icon'; -$string['cfg_iconalt_rightarrow'] = 'Right-facing arrow icon'; +$string['cfg_iconalt_o365'] = 'Office 365 icon'; +$string['cfg_iconalt_microsoft365'] = 'Microsoft 365 logo'; +$string['cfg_iconalt_openid'] = 'OpenID icon'; +$string['cfg_iconalt_keycloak'] = 'Keycloak icon'; $string['cfg_customicon_key'] = 'Custom Icon'; -$string['cfg_customicon_desc'] = 'If you\'d like to use your own icon, upload it here. This overrides any icon chosen above.

Notes on using custom icons:
  • This image will not be resized on the login page, so we recommend uploading an image no bigger than 35x35 pixels.
  • If you have uploaded a custom icon and want to go back to one of the stock icons, click the custom icon in the box above, then click "Delete", then click "OK", then click "Save Changes" at the bottom of this form. The selected stock icon will now appear on the Moodle login page.
'; +$string['cfg_customicon_desc'] = 'If you\'d like to use your own icon, upload it here. This overrides any icon chosen above.

Notes on using custom icons:
  • The uploaded file is not resized. It will be displayed at a fixed size of 24x24 pixels on the login page, with your browser scaling it to fit, so we recommend uploading a square image to avoid distortion.
  • If you have uploaded a custom icon and want to go back to one of the stock icons, click the custom icon in the box above, then click "Delete", then click "OK", then click "Save Changes" at the bottom of this form. The selected stock icon will now appear on the Moodle login page.
'; $string['cfg_debugmode_key'] = 'Record debug messages'; $string['cfg_debugmode_desc'] = 'If enabled, information will be logged to the Moodle log that can help in identifying problems.'; $string['cfg_loginflow_key'] = 'Login Flow'; $string['cfg_loginflow_authcode'] = 'Authorization Code Flow (recommended)'; $string['cfg_loginflow_authcode_desc'] = 'Using this flow, the user clicks the name of the IdP (See "Provider Display Name" above) on the Moodle login page and is redirected to the provider to log in. Once successfully logged in, the user is redirected back to Moodle where the Moodle login takes place transparently. This is the most standardized, secure way for the user log in.'; -$string['cfg_loginflow_rocreds'] = 'Resource Owner Password Credentials Grant (deprecated)'; -$string['cfg_loginflow_rocreds_desc'] = 'This login flow is deprecated and will be removed from the plugin soon.
Using this flow, the user enters their username and password into the Moodle login form like they would with a manual login. This will authorize the user with the IdP, but will not create a session on the IdP\'s site. For example, if using Microsoft 365 with OpenID Connect, the user will be logged in to Moodle but not the Microsoft 365 web applications. Using the authorization request is recommended if you want users to be logged in to both Moodle and the IdP. Note that not all IdP support this flow. This option should only be used when other authorization grant types are not available.'; $string['cfg_silentloginmode_key'] = 'Silent Login Mode'; $string['cfg_silentloginmode_desc'] = 'If enabled, Moodle will try to use the active session of a user authenticated to the configured authorization endpoint to log the user in.
To use this feature, the following configurations are required: @@ -175,7 +173,8 @@ '; $string['secretexpiryrecipients'] = 'Secret Expiry Notification Recipients'; $string['secretexpiryrecipients_help'] = 'A comma-separated list of email addresses to send secret expiry notifications to.
-If no email address is entered, the main site administrator will be notified.'; +If no email address is entered, the main site administrator will be notified.
+By default, notifications are sent daily from four weeks before expiry until the secret has been renewed.'; $string['cfg_opname_key'] = 'Provider Display Name'; $string['cfg_opname_desc'] = 'This is an end-user-facing label that identifies the type of credentials the user must use to login. This label is used throughout the user-facing portions of this plugin to identify your provider.'; $string['cfg_redirecturi_key'] = 'Redirect URI'; @@ -236,7 +235,6 @@ $string['errorauthdisconnectnewmethod'] = 'Use Login Method'; $string['errorauthdisconnectinvalidmethod'] = 'Invalid login method received.'; $string['errorauthdisconnectifmanual'] = 'If using the manual login method, enter credentials below.'; -$string['errorauthdisconnectinvalidmethod'] = 'Invalid login method received.'; $string['errorauthgeneral'] = 'There was a problem logging you in. Please contact your administrator for assistance.'; $string['errorauthinvalididtoken'] = 'Invalid id_token received.'; $string['errorauthloginfailednouser'] = 'Invalid login: User not found in Moodle. If this site has the "authpreventaccountcreation" setting enabled, this may mean you need an administrator to create an account for you first.'; @@ -283,6 +281,9 @@ $string['error_tenant_specific_endpoint_required'] = 'When using "Microsoft identity platform (v2.0)" IdP type and "Certificate" authentication method, tenant specific endpoint (i.e. not common/organizations/consumers) is required.'; $string['error_empty_oidcresource'] = 'Resource cannot be empty when using Microsoft Entra ID (v1.0) or other types of IdP.'; $string['error_invalid_custom_claim'] = 'Invalid custom claim name. Custom claims can only contain alphanumeric characters, hyphens, and underscores.'; +$string['error_masked_secret_not_changed'] = 'Please enter a new value or uncheck the "Change" checkbox to keep the current value.'; +$string['error_secretexpiryrecipients_invalid'] = 'The following secret expiry notification recipients are not valid email addresses: {$a}'; +$string['auth_settings_validation_error'] = 'Invalid authentication settings detected. The following configuration issues must be resolved to ensure successful authentication:'; $string['errorupnchangeisnotsupported'] = 'Your Microsoft account UPN has changed. Please contact your administrator to update your Moodle account.'; $string['erroruserwithusernamealreadyexists'] = 'Error occurred when trying to rename your Moodle account. A Moodle user with the new username already exists. Ask your site administrator to resolve this first.'; $string['error_no_response_available'] = 'No responses available.'; @@ -320,6 +321,8 @@ $string['privacy:metadata:auth_oidc_sid:userid'] = 'The ID of the Moodle user'; $string['privacy:metadata:auth_oidc_sid:sid'] = 'The IdP session identifier (sid)'; $string['privacy:metadata:auth_oidc_sid:timecreated'] = 'The time when the mapping was created'; +$string['privacy:metadata:auth_oidc_sid:sessionid'] = 'The Moodle session id active at the time the mapping was created'; +$string['privacy:metadata:auth_oidc_sid:iss'] = 'The issuer (iss claim) of the id_token the mapping was created from'; // In the following strings, $a refers to a customizable name for the identity manager. For example, this could be // "Microsoft 365", "OpenID Connect", etc. @@ -461,6 +464,7 @@
  • custom: Custom claim.
  • '; $string['binding_username_claim_updated'] = 'Binding username claim was updated successfully.'; +$string['warning_binding_username_claim_custom_unsupported'] = 'The "Binding username claim" setting is currently set to "Custom", which is not supported for the configured IdP type with user sync enabled. Review the Binding username claim settings.'; $string['examplecsv'] = 'Example upload file'; $string['usernamefile'] = 'File'; $string['csvdelimiter'] = 'CSV separator'; diff --git a/lang/es/auth_oidc.php b/lang/es/auth_oidc.php index d77020ff8..daf1f70b0 100644 --- a/lang/es/auth_oidc.php +++ b/lang/es/auth_oidc.php @@ -65,8 +65,6 @@ $string['cfg_loginflow_key'] = 'Flujo de inicio de sesión'; $string['cfg_loginflow_authcode'] = 'Solicitud de autorización'; $string['cfg_loginflow_authcode_desc'] = 'Al utiliza este flujo, el usuario hace clic en el nombre del proveedor de identidad (consulte "Nombre del proveedor" más arriba) en la página de inicio de sesión de Moodle y es redireccionado al proveedor para iniciar sesión. Una vez que haya iniciado sesión correctamente, es redireccionado de vuelta a Moodle, donde se realiza el inicio de sesión de manera transparente. Esta es la forma más segura y estandarizada de inicio de sesión del usuario.'; -$string['cfg_loginflow_rocreds'] = 'Autenticación de nombre de usuario/contraseña'; -$string['cfg_loginflow_rocreds_desc'] = 'Al usar este flujo, el usuario ingresa el nombre de usuario y la contraseña al formulario de inicio de sesión de Moodle como lo haría de manera manual. Luego, las credenciales se pasan al proveedor de identidad en el segundo plano para obtener la autenticación. Este flujo es la forma más transparente para el usuario ya que no posee interacción directa con el proveedor de identidad. Tenga en cuenta que no todos los proveedores de identidad admiten este flujo.'; $string['cfg_oidcresource_key'] = 'Recurso'; $string['cfg_oidcresource_desc'] = 'El recurso de OpenID Connect para el cual enviar la solicitud.'; $string['cfg_oidcscope_key'] = 'Scope'; diff --git a/lang/fi/auth_oidc.php b/lang/fi/auth_oidc.php index 272309fcf..d5a7f8d4d 100644 --- a/lang/fi/auth_oidc.php +++ b/lang/fi/auth_oidc.php @@ -65,8 +65,6 @@ $string['cfg_loginflow_key'] = 'Kirjautumiskulku'; $string['cfg_loginflow_authcode'] = 'Valtuutuspyyntö'; $string['cfg_loginflow_authcode_desc'] = 'Jos tämä kirjautumiskulku on käytössä, käyttäjä napsauttaa identiteetintarjoajan nimeä (ks. Palveluntarjoajan nimi) Moodlen kirjautumissivulla, jonka jälkeen käyttäjä ohjataan palveluntarjoajan sivulle kirjautumista varten. Jos kirjautuminen onnistuu, käyttäjä ohjataan takaisin Moodleen, jossa Moodle-kirjautuminen tapahtuu läpinäkyvästi. Tämä on standardisoitu ja turvallisin käyttäjien kirjautumismenetelmä.'; -$string['cfg_loginflow_rocreds'] = 'Käyttäjänimen/salasanan todennus'; -$string['cfg_loginflow_rocreds_desc'] = 'Jos tämä kirjautumiskulku on käytössä, käyttäjä kirjautuu Moodleen antamalla käyttäjänimen ja salasanan Moodlen kirjautumislomakkeeseen. Tunnistetiedot välitetään taustalla identiteetintarjoajalle todennusta varten. Tämä kulku on läpinäkyvin käyttäjän kannalta, koska käyttäjä ei ole suoraan tekemisissä identiteetintarjoajan kanssa. Huomaa, että kaikki identiteetintarjoajat eivät tue tätä kulkua.'; $string['cfg_oidcresource_key'] = 'Resurssi'; $string['cfg_oidcresource_desc'] = 'OpenID Connect -resurssi, jota lähetettävä pyyntö koskee.'; $string['cfg_oidcscope_key'] = 'laajuus'; diff --git a/lang/fr/auth_oidc.php b/lang/fr/auth_oidc.php index 885f66c91..08b0e9767 100644 --- a/lang/fr/auth_oidc.php +++ b/lang/fr/auth_oidc.php @@ -68,8 +68,6 @@ $string['cfg_loginflow_key'] = 'Méthode d\'authentification'; $string['cfg_loginflow_authcode'] = 'Demande d\'autorisation (recommandée)'; $string['cfg_loginflow_authcode_desc'] = 'À l\'aide de cette méthode, l\'utilisateur clique sur le fournisseur d\'identité (voir « Nom du fournisseur » ci-dessus) sur la page de connexion Moodle et est redirigé vers le fournisseur pour se connecter. Une fois la connexion réussie, l\'utilisateur est redirigé vers Moodle où la connexion Moodle est effectuée en toute transparence. Il s\'agit pour l\'utilisateur du moyen le plus sécurisé et standardisé pour se connecter.'; -$string['cfg_loginflow_rocreds'] = 'Authentification via nom d\'utilisateur/mot de passe'; -$string['cfg_loginflow_rocreds_desc'] = 'À l\'aide de cette méthode, l\'utilisateur saisit son nom d\'utilisateur et son mot de passe dans le formulaire de connexion Moodle comme il le ferait avec une connexion manuelle. Ses informations d\'identification sont ensuite transmises au fournisseur d\'identité en arrière-plan pour obtenir son authentification. Cette méthode est la plus transparente pour l\'utilisateur car il n\'a aucune interaction directe avec le fournisseur d\'identité. Notez que l\'ensemble des fournisseurs d\'identité prennent en charge ce flux.'; $string['cfg_oidcresource_key'] = 'Ressource'; $string['cfg_oidcresource_desc'] = 'Ressource OpenID Connect pour laquelle envoyer la demande.'; $string['cfg_oidcscope_key'] = 'Porté'; diff --git a/lang/it/auth_oidc.php b/lang/it/auth_oidc.php index eed128572..5dc952dee 100644 --- a/lang/it/auth_oidc.php +++ b/lang/it/auth_oidc.php @@ -65,8 +65,6 @@ $string['cfg_loginflow_key'] = 'Flusso di login'; $string['cfg_loginflow_authcode'] = 'Richiesta di autorizzazione'; $string['cfg_loginflow_authcode_desc'] = 'Utilizzando questo flusso, l\'utente fa clic sul nome dell\'Identity Provider (vedere "Nome provider" in precedenza) nella pagina login di Moodle e viene reindirizzato al provider per il login. Dopo il login, l\'utente viene nuovamente reindirizzato in Moodle dove il login a Moodle viene eseguito in maniera trasparente. Questo è il metodo di login più standardizzato e sicuro.'; -$string['cfg_loginflow_rocreds'] = 'Autenticazione nome utente/password'; -$string['cfg_loginflow_rocreds_desc'] = 'Utilizzando questo flusso, l\'utente inserisce il nome utente e la password nel modulo di login di Moodle seguendo la stessa procedura del login manuale. Le credenziali vengono quindi passate all\'Identity Provider in background per ottenere l\'autenticazione. Questo flusso è quello più trasparente per l\'utente in quanto non esiste interazione diretta con l\'Identity Provider. Osservare che non tutti gli Identity Provider supportano questo flusso.'; $string['cfg_oidcresource_key'] = 'Risorsa'; $string['cfg_oidcresource_desc'] = 'La risorsa OpenID Connect per la quale inviare la richiesta.'; $string['cfg_oidcscope_key'] = 'Scopo'; diff --git a/lang/ja/auth_oidc.php b/lang/ja/auth_oidc.php index 12adba749..038bd27d0 100644 --- a/lang/ja/auth_oidc.php +++ b/lang/ja/auth_oidc.php @@ -65,8 +65,6 @@ $string['cfg_loginflow_key'] = 'ログインフロー'; $string['cfg_loginflow_authcode'] = '認証リクエスト'; $string['cfg_loginflow_authcode_desc'] = 'このフローでは、ユーザはMoodleログインページでアイデンティティプロバイダの名前 (上記の「プロバイダ名」を参照) をクリックします。ユーザはプロバイダにリダイレクトされ、そこでログインします。ログインが成功したら、ユーザはMoodleにリダイレクトされ、透過的にMoodleログインが行われます。これは最も標準化され、最もセキュアなユーザのログイン方法です。'; -$string['cfg_loginflow_rocreds'] = 'ユーザ名/パスワード認証'; -$string['cfg_loginflow_rocreds_desc'] = 'このフローでは、手動によるログインと同様、ユーザはMoodleのログインフォームにユーザ名とパスワードを入力します。これらの認証情報はバックグラウンドでアイデンティティプロバイダに渡され、認証を取得します。ユーザはアイデンティティプロバイダと直接やり取りしないので、このフローはユーザに最も透過的です。すべてのアイデンティティプロバイダがこのフローをサポートしているわけではない点にご注意ください。'; $string['cfg_oidcresource_key'] = 'リソース'; $string['cfg_oidcresource_desc'] = 'リクエストを送る、OpenID Connectのリソース。'; $string['cfg_oidcscope_key'] = '範囲'; diff --git a/lang/nl/auth_oidc.php b/lang/nl/auth_oidc.php index c79ec3585..831a01bd2 100644 --- a/lang/nl/auth_oidc.php +++ b/lang/nl/auth_oidc.php @@ -65,8 +65,6 @@ $string['cfg_loginflow_key'] = 'Aanmeldingsflow'; $string['cfg_loginflow_authcode'] = 'Autorisatieverzoek'; $string['cfg_loginflow_authcode_desc'] = 'In deze flow klikt de gebruiker op de Moodle-aanmeldingspagina op de naam van de identiteitsprovider (zie Naam provider hierboven), waarna de gebruiker naar de provider wordt omgeleid om zich aan te melden. Wanneer de gebruiker is aangemeld, wordt de gebruiker weer teruggeleid naar Moodle, waar de Moodle-aanmelding transparant wordt uitgevoerd. Dit is de meest gestandaardiseerde en veilige manier waarop de gebruiker zich kan aanmelden.'; -$string['cfg_loginflow_rocreds'] = 'Authenticatie met gebruikersnaam/wachtwoord'; -$string['cfg_loginflow_rocreds_desc'] = 'In deze flow voert de gebruiker zijn gebruikersnaam en wachtwoord in het aanmeldingsformulier van Moodle in, net als bij een handmatige aanmelding. De referenties van de gebruiker worden daarna op de achtergrond doorgegeven aan de identiteitsprovider om authenticatie te verkrijgen. Deze werkwijze is de meest transparante voor de gebruiker omdat er geen directe interactie is met de identiteitsprovider. Niet alle identiteitsproviders ondersteunen deze werkwijze.'; $string['cfg_oidcresource_key'] = 'Bron'; $string['cfg_oidcresource_desc'] = 'De OpenID Connect-bron waarvoor het verzoek moet worden verzonden.'; $string['cfg_oidcscope_key'] = 'Reikwijdte'; diff --git a/lang/pl/auth_oidc.php b/lang/pl/auth_oidc.php index 0280a4009..2bbd6e271 100644 --- a/lang/pl/auth_oidc.php +++ b/lang/pl/auth_oidc.php @@ -65,8 +65,6 @@ $string['cfg_loginflow_key'] = 'Przepływ logowania'; $string['cfg_loginflow_authcode'] = 'Żądanie autoryzacji'; $string['cfg_loginflow_authcode_desc'] = 'W przypadku tego przepływu użytkownik klika nazwę dostawcy tożsamości (patrz „Nazwa dostawcy” powyżej) na stronie logowania do platformy Moodle i zostaje przekierowany do dostawcy, aby się zalogować. Po pomyślnym zalogowaniu użytkownik jest ponownie przekierowywany do strony platformy Moodle, na której odbywa się logowanie do platformy Moodle w sposób niewidoczny. Jest to najbardziej ustandaryzowany i bezpieczny sposób logowania się użytkownika.'; -$string['cfg_loginflow_rocreds'] = 'Uwierzytelnienie nazwy użytkownika/hasła'; -$string['cfg_loginflow_rocreds_desc'] = 'W przypadku tego przepływu użytkownik wprowadza nazwę użytkownika i hasło do formularza logowania się do platformy Moodle w taki sam sposób jak w przypadku logowania ręcznego. Dane logowania użytkownika są następnie przesyłane do dostawcy tożsamości w tle w celu uwierzytelnienia. Ten przepływ jest najbardziej niewidoczny dla użytkownika, ponieważ użytkownik nie wchodzi w bezpośrednią interakcję z dostawcą tożsamości. Nie wszyscy dostawcy tożsamości obsługują ten przepływ.'; $string['cfg_oidcresource_key'] = 'Zasób'; $string['cfg_oidcresource_desc'] = 'Zasób wtyczki OpenID Connect, do którego ma zostać wysłane żądanie.'; $string['cfg_oidcscope_key'] = 'Scope'; diff --git a/lang/pt_br/auth_oidc.php b/lang/pt_br/auth_oidc.php index 1d2218978..22831cdb5 100644 --- a/lang/pt_br/auth_oidc.php +++ b/lang/pt_br/auth_oidc.php @@ -65,8 +65,6 @@ $string['cfg_loginflow_key'] = 'Fluxo de login'; $string['cfg_loginflow_authcode'] = 'Solicitação de autorização'; $string['cfg_loginflow_authcode_desc'] = 'Ao usar esse fluxo, o usuário clicará no nome do provedor de identidade (consulte "Nome do provedor" acima) na página de login do Moodle e será redirecionado para o provedor para fazer login. Depois de efetuar com sucesso o login, o usuário será redirecionado de volta para o Moodle, onde o login ocorrerá de modo transparente. Essa é a maneira mais padronizada e segura de realizar o login do usuário.'; -$string['cfg_loginflow_rocreds'] = 'Autenticação de nome de usuário e senha'; -$string['cfg_loginflow_rocreds_desc'] = 'Ao usar esse fluxo, o usuário informará seu nome de usuário e sua senha no formulário de login do Moodle da mesma forma que faria em um login manual. As credenciais serão, então, transmitidas em segundo plano para o provedor de identidade no intuito de obter a autenticação. Esse fluxo é o mais simples para o usuário, pois ele não interage diretamente com o provedor de identidade. Tenha em mente que nem todos os provedores de identidade aceitam a utilização desse fluxo.'; $string['cfg_oidcresource_key'] = 'Recurso'; $string['cfg_oidcresource_desc'] = 'O recurso do OpenID Connect para o qual a solicitação deverá ser enviada.'; $string['cfg_oidcscope_key'] = 'Escopo'; diff --git a/lib.php b/lib.php index 5d13a5ae0..b56b4000e 100644 --- a/lib.php +++ b/lib.php @@ -84,6 +84,17 @@ */ const AUTH_OIDC_AUTH_CERT_SOURCE_FILE = 2; +/** + * File extensions accepted for the 'auth_oidc/customicon' upload setting. + * + * SVG is deliberately excluded: unlike the plugin's own bundled stock icons, this file is + * admin-uploaded and served as-is from dataroot, so allowing SVG here would let an admin + * upload active content (script/event handlers). Shared by the setting's file picker + * (settings.php) and the extension allow-list checked before copying the file into + * pix_plugins (auth_oidc_initialize_customicon()) so the two can't drift apart. + */ +const AUTH_OIDC_CUSTOMICON_ALLOWED_EXTENSIONS = ['png', 'jpg', 'gif']; + /** * Callback invoked when application credentials or endpoint settings are updated. * @@ -133,6 +144,8 @@ function auth_oidc_reset_app_tokens($settingname) { * @return void */ function auth_oidc_validate_auth_settings(string $settingname) { + auth_oidc_validate_binding_username_claim(); + $idptype = get_config('auth_oidc', 'idptype'); $clientauthmethod = get_config('auth_oidc', 'clientauthmethod'); @@ -196,6 +209,27 @@ function auth_oidc_validate_auth_settings(string $settingname) { } } +/** + * Warn the admin if the stored "Custom" binding username claim is no longer supported for the + * currently configured IdP type and user sync setting. + * + * @return void + */ +function auth_oidc_validate_binding_username_claim() { + $idptype = get_config('auth_oidc', 'idptype'); + if (empty($idptype) || get_config('auth_oidc', 'bindingusernameclaim') !== 'custom') { + return; + } + + $mstypes = [AUTH_OIDC_IDP_TYPE_MICROSOFT_ENTRA_ID, AUTH_OIDC_IDP_TYPE_MICROSOFT_IDENTITY_PLATFORM]; + if (in_array($idptype, $mstypes) && auth_oidc_is_local_365_installed() && auth_oidc_is_user_sync_enabled()) { + $bindingclaimurl = new url('/admin/settings.php', ['section' => 'auth_oidc_binding_username_claim']); + \core\notification::warning( + get_string('warning_binding_username_claim_custom_unsupported', 'auth_oidc', $bindingclaimurl->out()) + ); + } +} + /** * Initialize custom icon for OIDC authentication. * @@ -227,7 +261,25 @@ function auth_oidc_initialize_customicon($filefullname) { } if (file_exists($CFG->dataroot . '/pix_plugins/auth/oidc/0')) { - $file->copy_content_to($CFG->dataroot . '/pix_plugins/auth/oidc/0/customicon.jpg'); + // Remove any previously stored custom icon so a stale file with a different + // extension can't take priority when the theme resolves the icon image. + $oldiconfiles = glob($CFG->dataroot . '/pix_plugins/auth/oidc/0/customicon.*'); + foreach ($oldiconfiles ?: [] as $oldiconfile) { + // A failed unlink (e.g. permissions, or the file already being gone) isn't fatal + // here: copy_content_to() below will still overwrite/create the current extension's + // file, so at worst a stale file of a different extension is left behind. + @unlink($oldiconfile); + } + + $extension = strtolower(pathinfo($file->get_filename(), PATHINFO_EXTENSION)); + if (!in_array($extension, AUTH_OIDC_CUSTOMICON_ALLOWED_EXTENSIONS, true)) { + // Unexpected/empty extension: don't create a weird or unvalidated file under + // pix_plugins. The stale files for previously-valid extensions were already + // removed above, so this leaves no custom icon in place. + return false; + } + + $file->copy_content_to($CFG->dataroot . "/pix_plugins/auth/oidc/0/customicon.{$extension}"); theme_reset_all_caches(); } } @@ -307,13 +359,13 @@ function auth_oidc_get_tokens_with_empty_ids() { $item = new stdClass(); $item->id = $record->id; $item->oidcusername = $record->oidcusername; - $item->useriditifier = $record->useridentifier; + $item->useridentifier = $record->useridentifier; $item->moodleusername = $record->username; $item->userid = 0; $item->oidcuniqueid = $record->oidcuniqid; $item->matchingstatus = get_string('unmatched', 'auth_oidc'); $item->details = get_string('na', 'auth_oidc'); - $deletetokenurl = new url('/auth/oidc/cleanupoidctokens.php', ['id' => $record->id]); + $deletetokenurl = new url('/auth/oidc/cleanupoidctokens.php', ['id' => $record->id, 'sesskey' => sesskey()]); $item->action = html_writer::link($deletetokenurl, get_string('delete_token', 'auth_oidc')); $emptyuseridtokens[$record->id] = $item; @@ -350,9 +402,9 @@ function auth_oidc_get_tokens_with_mismatched_usernames() { $item->details = get_string( 'mismatched_details', 'auth_oidc', - ['tokenusername' => $record->tokenusername, 'moodleusername' => $record->musername] + ['tokenusername' => s($record->tokenusername), 'moodleusername' => s($record->musername)] ); - $deletetokenurl = new url('/auth/oidc/cleanupoidctokens.php', ['id' => $record->id]); + $deletetokenurl = new url('/auth/oidc/cleanupoidctokens.php', ['id' => $record->id, 'sesskey' => sesskey()]); $item->action = html_writer::link($deletetokenurl, get_string('delete_token_and_reference', 'auth_oidc')); $mismatchedtokens[$record->id] = $item; @@ -1041,6 +1093,32 @@ function auth_oidc_mask_secret($secret) { return substr($secret, 0, 2) . '**********'; } +/** + * Validate the "secret expiry notification recipients" setting value. + * + * The value is a comma-separated list of email addresses that the local_o365 notifysecretexpiry + * task sends notifications to. Empty entries and surrounding whitespace are ignored, and an empty + * list is valid (notifications then fall back to the site administrator). Only the syntax of each + * address is checked; the domain is not resolved. + * + * @param string $value The raw setting value. + * @return array List of entries that are not valid email addresses. Empty when every entry is valid. + */ +function auth_oidc_validate_secret_expiry_recipients(string $value): array { + $invalidemails = []; + foreach (explode(',', $value) as $email) { + $email = trim($email); + if ($email === '') { + continue; + } + if (!validate_email($email)) { + $invalidemails[] = $email; + } + } + + return $invalidemails; +} + /** * Check if a value appears to be a masked secret. * @@ -1066,7 +1144,7 @@ function auth_oidc_is_masked_secret($value) { */ function auth_oidc_get_settings_nav_html(string $currentpage): string { $pages = [ - 'auth_oidc_application' => get_string('settings_page_application', 'auth_oidc'), + 'authsettingoidc' => get_string('settings_page_application', 'auth_oidc'), ]; // Only include the binding username claim tab if IdP type is configured. diff --git a/logout.php b/logout.php index bdbd30a94..bbf8d6413 100644 --- a/logout.php +++ b/logout.php @@ -24,6 +24,7 @@ */ use core\context\system; +use core\session\manager; // phpcs:ignore moodle.Files.RequireLogin.Missing require_once(__DIR__ . '/../../config.php'); @@ -32,19 +33,35 @@ $PAGE->set_context(system::instance()); $sid = optional_param('sid', '', PARAM_TEXT); +$iss = optional_param('iss', '', PARAM_TEXT); if ($sid) { - if ($authoidcsidrecord = $DB->get_record('auth_oidc_sid', ['sid' => $sid])) { - if ($authoidcsidrecord->userid == $USER->id) { - $authsequence = get_enabled_auth_plugins(); // Auths, in sequence. - foreach ($authsequence as $authname) { - $authplugin = get_auth_plugin($authname); - $authplugin->logoutpage_hook(); - } + // This request is made by the IdP directly (e.g. via a hidden iframe), so it will not carry the + // MoodleSession cookie of the user(s) being logged out, and there is no way to authenticate it. + // Do not call auth plugin logout hooks here: this is an unauthenticated endpoint, so anyone could + // trigger them merely by supplying a known sid. + $conditions = ['sid' => $sid]; + if ($iss) { + // When the IdP includes iss, use it as an extra check that the mapping was created for this + // issuer, so a sid alone (without also knowing the issuer it was created for) cannot be used to + // force a logout. Not all IdPs include iss on front-channel logout requests (e.g. Azure AD + // currently does not), so this is applied only when present rather than required. + $conditions['iss'] = $iss; + } - $DB->delete_records('auth_oidc_sid', ['sid' => $sid]); - require_logout(); + $authoidcsidrecords = $DB->get_records('auth_oidc_sid', $conditions); + if ($authoidcsidrecords) { + // The same IdP sid can be mapped to more than one Moodle session (e.g. logins from different + // browsers/devices during the same IdP session), so destroy every session mapped to this sid. + $matchedids = []; + foreach ($authoidcsidrecords as $authoidcsidrecord) { + if (!empty($authoidcsidrecord->sessionid)) { + manager::destroy($authoidcsidrecord->sessionid); + } + $matchedids[] = $authoidcsidrecord->id; } + + $DB->delete_records_list('auth_oidc_sid', 'id', $matchedids); } } diff --git a/manageapplication.php b/manageapplication.php index aac8d2673..84c2cb7a1 100644 --- a/manageapplication.php +++ b/manageapplication.php @@ -112,7 +112,9 @@ switch ($fromform->clientauthmethod) { case AUTH_OIDC_AUTH_METHOD_SECRET: $configstosave[] = 'clientsecret'; - $configstosave[] = 'secretexpiryrecipients'; + if (isset($fromform->secretexpiryrecipients)) { + $configstosave[] = 'secretexpiryrecipients'; + } break; case AUTH_OIDC_AUTH_METHOD_CERTIFICATE: $configstosave[] = 'clientcertsource'; diff --git a/pix/keycloak.png b/pix/keycloak.png new file mode 100644 index 000000000..48e18430c Binary files /dev/null and b/pix/keycloak.png differ diff --git a/pix/microsoft_365.svg b/pix/microsoft_365.svg new file mode 100644 index 000000000..5334aa7ca --- /dev/null +++ b/pix/microsoft_365.svg @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/pix/o365.png b/pix/o365.png deleted file mode 100644 index 0fa022c25..000000000 Binary files a/pix/o365.png and /dev/null differ diff --git a/pix/office_365.svg b/pix/office_365.svg new file mode 100644 index 000000000..66e8725af --- /dev/null +++ b/pix/office_365.svg @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/pix/openid.svg b/pix/openid.svg new file mode 100644 index 000000000..63a90e05c --- /dev/null +++ b/pix/openid.svg @@ -0,0 +1,31 @@ + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/settings.php b/settings.php index 2c1d9774a..7fa165f75 100644 --- a/settings.php +++ b/settings.php @@ -30,6 +30,9 @@ use auth_oidc\adminsetting\auth_oidc_admin_setting_iconselect; use auth_oidc\adminsetting\auth_oidc_admin_setting_loginflow; use auth_oidc\adminsetting\auth_oidc_admin_setting_redirecturi; +use auth_oidc\adminsetting\auth_oidc_admin_setting_secretexpiryrecipients; +use auth_oidc\adminsetting\auth_oidc_admin_setting_section_heading; +use auth_oidc\adminsetting\auth_oidc_admin_setting_stateexpiry; use auth_oidc\utils; use core\url; @@ -39,7 +42,7 @@ // Redirect the category overview page to the first settings tab, so that the Bootstrap // nav-tabs behave correctly instead of showing all sub-pages' content at once. if ($PAGE->has_set_url() && $PAGE->url->get_param('category') === 'oidcfolder') { - redirect(new \core\url('/admin/settings.php', ['section' => 'auth_oidc_application'])); + redirect(new \core\url('/admin/settings.php', ['section' => 'authsettingoidc'])); } // Add folder for OIDC settings. @@ -47,8 +50,13 @@ $ADMIN->add('authsettings', $oidcfolder); // Application configuration settings page. + // Registered as 'authsettingoidc' (Moodle's "authsetting" convention for auth + // plugins) rather than a custom id, because the core "Manage authentication" page + // (admin_setting_manageauths::output_html in lib/adminlib.php) hardcodes the settings + // link for each auth plugin to section "authsetting". Using any other id here + // leaves that link pointing at a non-existent section and triggers a "section error". $applicationsettings = new admin_settingpage( - 'auth_oidc_application', + 'authsettingoidc', get_string('settings_page_application', 'auth_oidc') ); @@ -56,7 +64,7 @@ $applicationsettings->add(new admin_setting_heading( 'auth_oidc_application_nav', '', - auth_oidc_get_settings_nav_html('auth_oidc_application') + auth_oidc_get_settings_nav_html('authsettingoidc') )); // Link to the guided Application Configuration Wizard. @@ -281,13 +289,13 @@ // Secret expiry notification (only when local_o365 is installed). if (auth_oidc_is_local_365_installed()) { - $applicationsettings->add(new admin_setting_heading( + $applicationsettings->add(new auth_oidc_admin_setting_section_heading( 'auth_oidc/application_secretexpiry_heading', get_string('settings_section_secret_expiry_notification', 'auth_oidc'), '' )); - $applicationsettings->add(new admin_setting_configtext( + $applicationsettings->add(new auth_oidc_admin_setting_secretexpiryrecipients( 'auth_oidc/secretexpiryrecipients', get_string('secretexpiryrecipients', 'auth_oidc'), get_string('secretexpiryrecipients_help', 'auth_oidc'), @@ -384,6 +392,21 @@ 'eq', AUTH_OIDC_IDP_TYPE_OTHER ); + + // Hide the section heading too, so an empty section isn't shown when the only + // setting in it is hidden. + $applicationsettings->hide_if( + 'auth_oidc/application_secretexpiry_heading', + 'auth_oidc/clientauthmethod', + 'neq', + AUTH_OIDC_AUTH_METHOD_SECRET + ); + $applicationsettings->hide_if( + 'auth_oidc/application_secretexpiry_heading', + 'auth_oidc/idptype', + 'eq', + AUTH_OIDC_IDP_TYPE_OTHER + ); } $ADMIN->add('oidcfolder', $applicationsettings); @@ -577,6 +600,60 @@ ) ); + // Login state error page heading. + $settings->add( + new admin_setting_heading( + 'auth_oidc/stateredirect_heading', + get_string('heading_stateredirect', 'auth_oidc'), + get_string('heading_stateredirect_desc', 'auth_oidc') + ) + ); + + // Login state expiry. + $settings->add( + new auth_oidc_admin_setting_stateexpiry( + 'auth_oidc/stateexpiry', + get_string('cfg_stateexpiry_key', 'auth_oidc'), + get_string('cfg_stateexpiry_desc', 'auth_oidc'), + 5, + PARAM_INT + ) + ); + + // Enable friendly login state error page. + $settings->add( + new admin_setting_configcheckbox( + 'auth_oidc/stateredirect_enabled', + get_string('cfg_stateredirect_enabled_key', 'auth_oidc'), + get_string('cfg_stateredirect_enabled_desc', 'auth_oidc'), + '0' + ) + ); + + // Message to display on the friendly login state error page. + $settings->add( + new admin_setting_confightmleditor( + 'auth_oidc/stateredirect_message', + get_string('cfg_stateredirect_message_key', 'auth_oidc'), + get_string('cfg_stateredirect_message_desc', 'auth_oidc'), + get_string('cfg_stateredirect_message_default', 'auth_oidc') + ) + ); + + // Redirect delay for the friendly login state error page. + $settings->add( + new admin_setting_configtext( + 'auth_oidc/stateredirect_delay', + get_string('cfg_stateredirect_delay_key', 'auth_oidc'), + get_string('cfg_stateredirect_delay_desc', 'auth_oidc'), + 5, + PARAM_INT + ) + ); + + $settings->hide_if('auth_oidc/stateredirect_message', 'auth_oidc/stateredirect_enabled', 'notchecked'); + $settings->hide_if('auth_oidc/stateredirect_delay', 'auth_oidc/stateredirect_enabled', 'notchecked'); + // User restrictions heading. $settings->add( new admin_setting_heading( @@ -679,79 +756,24 @@ // Icon. $icons = [ [ - 'pix' => 'o365', - 'alt' => new lang_string('cfg_iconalt_o365', 'auth_oidc'), + 'pix' => 'microsoft_365', + 'alt' => new lang_string('cfg_iconalt_microsoft365', 'auth_oidc'), 'component' => 'auth_oidc', ], [ - 'pix' => 't/locked', - 'alt' => new lang_string('cfg_iconalt_locked', 'auth_oidc'), - 'component' => 'moodle', - ], - [ - 'pix' => 't/lock', - 'alt' => new lang_string('cfg_iconalt_lock', 'auth_oidc'), - 'component' => 'moodle', - ], - [ - 'pix' => 't/go', - 'alt' => new lang_string('cfg_iconalt_go', 'auth_oidc'), - 'component' => 'moodle', - ], - [ - 'pix' => 't/stop', - 'alt' => new lang_string('cfg_iconalt_stop', 'auth_oidc'), - 'component' => 'moodle', - ], - [ - 'pix' => 't/user', - 'alt' => new lang_string('cfg_iconalt_user', 'auth_oidc'), - 'component' => 'moodle', - ], - [ - 'pix' => 'u/user35', - 'alt' => new lang_string('cfg_iconalt_user2', 'auth_oidc'), - 'component' => 'moodle', - ], - [ - 'pix' => 'i/permissions', - 'alt' => new lang_string('cfg_iconalt_key', 'auth_oidc'), - 'component' => 'moodle', - ], - [ - 'pix' => 'i/cohort', - 'alt' => new lang_string('cfg_iconalt_group', 'auth_oidc'), - 'component' => 'moodle', - ], - [ - 'pix' => 'i/groups', - 'alt' => new lang_string('cfg_iconalt_group2', 'auth_oidc'), - 'component' => 'moodle', - ], - [ - 'pix' => 'i/mnethost', - 'alt' => new lang_string('cfg_iconalt_mnet', 'auth_oidc'), - 'component' => 'moodle', - ], - [ - 'pix' => 'i/permissionlock', - 'alt' => new lang_string('cfg_iconalt_userlock', 'auth_oidc'), - 'component' => 'moodle', - ], - [ - 'pix' => 't/more', - 'alt' => new lang_string('cfg_iconalt_plus', 'auth_oidc'), - 'component' => 'moodle', + 'pix' => 'office_365', + 'alt' => new lang_string('cfg_iconalt_o365', 'auth_oidc'), + 'component' => 'auth_oidc', ], [ - 'pix' => 't/approve', - 'alt' => new lang_string('cfg_iconalt_check', 'auth_oidc'), - 'component' => 'moodle', + 'pix' => 'openid', + 'alt' => new lang_string('cfg_iconalt_openid', 'auth_oidc'), + 'component' => 'auth_oidc', ], [ - 'pix' => 't/right', - 'alt' => new lang_string('cfg_iconalt_rightarrow', 'auth_oidc'), - 'component' => 'moodle', + 'pix' => 'keycloak', + 'alt' => new lang_string('cfg_iconalt_keycloak', 'auth_oidc'), + 'component' => 'auth_oidc', ], ]; $settings->add( @@ -759,7 +781,7 @@ 'auth_oidc/icon', get_string('cfg_icon_key', 'auth_oidc'), get_string('cfg_icon_desc', 'auth_oidc'), - 'auth_oidc:o365', + 'auth_oidc:microsoft_365', $icons ) ); @@ -773,7 +795,13 @@ get_string('cfg_customicon_desc', 'auth_oidc'), 'customicon', 0, - ['accepted_types' => ['.png', '.jpg', '.ico'], 'maxbytes' => get_max_upload_file_size()] + [ + 'accepted_types' => array_map( + fn ($extension) => ".{$extension}", + AUTH_OIDC_CUSTOMICON_ALLOWED_EXTENSIONS + ), + 'maxbytes' => get_max_upload_file_size(), + ] ); $customiconsetting->set_updatedcallback('auth_oidc_initialize_customicon'); $settings->add($customiconsetting); diff --git a/tests/lib_test.php b/tests/lib_test.php new file mode 100644 index 000000000..339ae7a46 --- /dev/null +++ b/tests/lib_test.php @@ -0,0 +1,75 @@ +. + +namespace auth_oidc; + +use advanced_testcase; + +/** + * Unit tests for functions in auth/oidc/lib.php + * + * @package auth_oidc + * @author Lai Wei + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @copyright (C) 2026 onwards Microsoft, Inc. (http://microsoft.com/) + * @group auth_oidc + * @group office365 + */ +final class lib_test extends advanced_testcase { + /** + * Data provider for {@see self::test_validate_secret_expiry_recipients()}. + * + * @return array + */ + public static function validate_secret_expiry_recipients_provider(): array { + return [ + 'empty string' => ['', []], + 'whitespace only' => [' ', []], + 'single valid address' => ['admin@example.com', []], + 'multiple valid addresses' => ['admin@example.com, second@example.com', []], + 'valid addresses with surrounding whitespace and trailing comma' => [ + ' admin@example.com , second@example.com , ', + [], + ], + 'single invalid address' => ['not-an-email', ['not-an-email']], + 'mixed valid and invalid' => [ + 'admin@example.com, broken, third@example.com', + ['broken'], + ], + 'multiple invalid addresses' => [ + 'broken, also broken@', + ['broken', 'also broken@'], + ], + ]; + } + + /** + * Test auth_oidc_validate_secret_expiry_recipients(). + * + * @dataProvider validate_secret_expiry_recipients_provider + * @param string $value + * @param array $expected + * @return void + * @covers ::auth_oidc_validate_secret_expiry_recipients + */ + public function test_validate_secret_expiry_recipients(string $value, array $expected): void { + $this->resetAfterTest(true); + + require_once(__DIR__ . '/../lib.php'); + + $this->assertSame($expected, auth_oidc_validate_secret_expiry_recipients($value)); + } +} diff --git a/tests/loginflow/authcode_test.php b/tests/loginflow/authcode_test.php new file mode 100644 index 000000000..d0caae076 --- /dev/null +++ b/tests/loginflow/authcode_test.php @@ -0,0 +1,146 @@ +. + +namespace auth_oidc\loginflow; + +use advanced_testcase; +use auth_oidc\jwt; +use core\plugininfo\auth as auth_plugininfo; +use phpunit_util; + +/** + * Unit tests for the class \auth_oidc\loginflow\authcode. + * + * @package auth_oidc + * @copyright 2026 Enovation Solutions + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @group auth_oidc + * @group office365 + * @coversDefaultClass \auth_oidc\loginflow\authcode + */ +final class authcode_test extends advanced_testcase { + /** + * Set up test environment. + * + * @return void + */ + protected function setUp(): void { + parent::setUp(); + $this->resetAfterTest(true); + + auth_plugininfo::enable_plugin('oidc', 1); + set_config('bindingusernameclaim', 'upn', 'auth_oidc'); + } + + /** + * A manually matched user (auth already 'oidc', local_o365_connections keyed on the full Entra UPN, + * Moodle username only the UPN prefix) must complete on the very first OIDC login: the token must be + * bound to the matched Moodle user, not left dangling because the Moodle username differs from the UPN. + * + * Regression test for https://github.com/microsoft/o365-moodle/issues/2875. + * + * @return void + * @covers ::handlelogin + * @covers ::check_for_matched + */ + public function test_handlelogin_completes_manually_matched_user_with_differing_username(): void { + if (!auth_oidc_is_local_365_installed()) { + $this->markTestSkipped('This test requires local_o365 to be installed (local_o365_connections table).'); + } + + $this->assert_login_completes_for_matched_user('asmith@sc.school.edu.au', 'asmith@sc.school.edu.au'); + } + + /** + * A local_o365_connections row stored with mixed-case entraidupn (e.g. saved before UPNs were + * normalized to lower case, or entered that way by an admin) must still match a differently-cased + * UPN claim from the ID token, including on case-sensitive database collations such as PostgreSQL. + * + * @return void + * @covers ::check_for_matched + */ + public function test_handlelogin_completes_manually_matched_user_with_legacy_mixed_case_upn(): void { + if (!auth_oidc_is_local_365_installed()) { + $this->markTestSkipped('This test requires local_o365 to be installed (local_o365_connections table).'); + } + + $this->assert_login_completes_for_matched_user('ASmith@SC.School.edu.AU', 'asmith@sc.school.edu.au'); + } + + /** + * Creates a Moodle user manually matched to the given (as-stored) Entra UPN, drives handlelogin() with + * an ID token carrying the given (as-received) UPN claim, and asserts the login completed and bound + * the auth_oidc_token to the matched user. + * + * @param string $storedentraidupn The UPN as stored in local_o365_connections.entraidupn. + * @param string $tokenupn The UPN as received in the ID token's upn/preferred_username claims. + * @return void + */ + private function assert_login_completes_for_matched_user(string $storedentraidupn, string $tokenupn): void { + global $DB; + + $user = $this->getDataGenerator()->create_user(['username' => 'asmith', 'auth' => 'oidc']); + + // Simulate an admin manually matching the Moodle user to their Entra UPN via + // Manage User Connections, then flipping the account to auth 'oidc'. + $DB->insert_record('local_o365_connections', (object) [ + 'muserid' => $user->id, + 'entraidupn' => $storedentraidupn, + 'uselogin' => 0, + ]); + + $idtoken = new jwt(); + $idtoken->set_claims([ + 'sub' => 'sub-' . $user->id, + 'upn' => $tokenupn, + 'preferred_username' => $tokenupn, + ]); + + $oidcuniqid = 'oidcuniqid-' . $user->id; + $authparams = ['code' => 'authcode-' . $user->id]; + $tokenparams = [ + 'access_token' => 'access-token', + 'id_token' => 'id-token', + 'expires_in' => 3600, + 'resource' => 'resource', + 'scope' => 'scope', + ]; + + // The user_login() method validates the auth code against the current request, so it has to be + // available via optional_param() the same way it would be on a real OIDC callback request. + $_GET['code'] = $authparams['code']; + + try { + $loginflow = new authcode(); + phpunit_util::call_internal_method( + $loginflow, + 'handlelogin', + [$oidcuniqid, $authparams, $tokenparams, $idtoken], + authcode::class + ); + } finally { + unset($_GET['code']); + } + + $tokenrec = $DB->get_record('auth_oidc_token', ['oidcuniqid' => $oidcuniqid]); + $this->assertNotEmpty($tokenrec); + $this->assertEquals($user->id, $tokenrec->userid); + $this->assertEquals('asmith', $tokenrec->username); + + global $USER; + $this->assertEquals($user->id, $USER->id); + } +} diff --git a/tests/task/cleanup_oidc_sid_test.php b/tests/task/cleanup_oidc_sid_test.php index 9a49b8688..6e8fe7c67 100644 --- a/tests/task/cleanup_oidc_sid_test.php +++ b/tests/task/cleanup_oidc_sid_test.php @@ -32,64 +32,73 @@ */ final class cleanup_oidc_sid_test extends advanced_testcase { /** - * SIDs older than 1 day are deleted. + * Insert a row into the core {sessions} table so that + * \core\session\manager::session_exists() reports it as existing. * - * The cleanup task deletes records where timecreated < strtotime('-1 day'). - * Records created exactly 1 day ago or more recently are kept. + * @param string $sid + * @param int $userid + * @return void + * @throws dml_exception + */ + private function create_moodle_session(string $sid, int $userid): void { + global $DB; + + $DB->insert_record('sessions', [ + 'state' => 0, + 'sid' => $sid, + 'userid' => $userid, + 'sessdata' => '', + 'timecreated' => time(), + 'timemodified' => time(), + ]); + } + + /** + * Mappings whose Moodle session still exists are kept; mappings whose session no longer exists, or + * that were never given a session id, are deleted. * * @return void * @throws dml_exception * @covers ::execute */ - public function test_sids_older_than_yesterday_are_deleted(): void { + public function test_sid_records_are_cleaned_up_based_on_session_existence(): void { global $DB; $this->resetAfterTest(); - // Create a test user to own the SID records. $user = $this->getDataGenerator()->create_user(); - // Use a fixed reference time to avoid timing race conditions. - // The cleanup task will calculate strtotime('-1 day') at execution time, which may differ slightly - // from when we calculate it here. Use a large safety margin to ensure records fall clearly on one side. - $now = time(); - $cutofftime = strtotime('-1 day', $now); + // Mapping tied to a session that still exists: must be kept. + $this->create_moodle_session('session_active', $user->id); + $activeid = $DB->insert_record('auth_oidc_sid', [ + 'userid' => $user->id, + 'sid' => 'sid_active', + 'timecreated' => time(), + 'sessionid' => 'session_active', + ]); - // Create timestamps with clear margins to avoid boundary conditions. - // Add a 5-minute buffer before and after the cutoff to account for execution time variance. - $twodaysago = $cutofftime - DAYSECS; // Well before cutoff, will be deleted. - $beforecutoff = $cutofftime - (MINSECS * 5); // 5 minutes before cutoff, will be deleted. - $aftercutoff = $cutofftime + (MINSECS * 5); // 5 minutes after cutoff, will be kept. - $muchlater = $now; // Current time, will be kept. + // Mapping tied to a session id that does not exist (e.g. the user's session has already + // expired or been terminated some other way): must be deleted. + $expiredid = $DB->insert_record('auth_oidc_sid', [ + 'userid' => $user->id, + 'sid' => 'sid_expired', + 'timecreated' => time(), + 'sessionid' => 'session_does_not_exist', + ]); - // Create entries in auth_oidc_sid with unique SIDs. - $entry1id = $DB->insert_record( - 'auth_oidc_sid', - ['userid' => $user->id, 'sid' => 'sid_old_1', 'timecreated' => $twodaysago], - ); - $entry2id = $DB->insert_record( - 'auth_oidc_sid', - ['userid' => $user->id, 'sid' => 'sid_old_2', 'timecreated' => $beforecutoff], - ); - $entry3id = $DB->insert_record( - 'auth_oidc_sid', - ['userid' => $user->id, 'sid' => 'sid_new_1', 'timecreated' => $muchlater], - ); - $entry4id = $DB->insert_record( - 'auth_oidc_sid', - ['userid' => $user->id, 'sid' => 'sid_new_2', 'timecreated' => $aftercutoff], - ); + // Legacy mapping created before sessionid was tracked: must be deleted, since there is no way + // to confirm whether its session still exists. + $legacyid = $DB->insert_record('auth_oidc_sid', [ + 'userid' => $user->id, + 'sid' => 'sid_legacy', + 'timecreated' => time(), + 'sessionid' => null, + ]); $cleanup = new cleanup_oidc_sid(); - $cleanup->execute(); - $records = $DB->get_records('auth_oidc_sid'); - - $this->assertCount(2, $records); - - $this->assertTrue($DB->record_exists('auth_oidc_sid', ['id' => $entry3id])); - $this->assertFalse($DB->record_exists('auth_oidc_sid', ['id' => $entry1id])); - $this->assertFalse($DB->record_exists('auth_oidc_sid', ['id' => $entry2id])); - $this->assertTrue($DB->record_exists('auth_oidc_sid', ['id' => $entry4id])); + $this->assertTrue($DB->record_exists('auth_oidc_sid', ['id' => $activeid])); + $this->assertFalse($DB->record_exists('auth_oidc_sid', ['id' => $expiredid])); + $this->assertFalse($DB->record_exists('auth_oidc_sid', ['id' => $legacyid])); } } diff --git a/tests/task/cleanup_oidc_state_and_token_test.php b/tests/task/cleanup_oidc_state_and_token_test.php new file mode 100644 index 000000000..83d936366 --- /dev/null +++ b/tests/task/cleanup_oidc_state_and_token_test.php @@ -0,0 +1,139 @@ +. + +namespace auth_oidc\task; + +use advanced_testcase; +use dml_exception; + +/** + * Unit tests for the class cleanup_oidc_state_and_token + * + * @package auth_oidc + * @copyright 2026 Enovation Solutions + * @author Lai Wei + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @group auth_oidc + * @group office365 + * @coversDefaultClass \auth_oidc\task\cleanup_oidc_state_and_token + */ +final class cleanup_oidc_state_and_token_test extends advanced_testcase { + /** + * Insert an auth_oidc_state record with the given state string and timecreated. + * + * @param string $state Unique state string. + * @param int $timecreated Time the state record was created. + * @return int The id of the inserted record. + * @throws dml_exception + */ + private function create_state_record(string $state, int $timecreated): int { + global $DB; + + return $DB->insert_record('auth_oidc_state', [ + 'sesskey' => 'sesskey123', + 'state' => $state, + 'nonce' => 'nonce123', + 'timecreated' => $timecreated, + ]); + } + + /** + * When "stateexpiry" is not configured, state records older than the default 5 minutes are deleted. + * + * @return void + * @throws dml_exception + * @covers ::execute + */ + public function test_default_expiry_is_five_minutes(): void { + global $DB; + $this->resetAfterTest(); + + $now = time(); + $cutofftime = strtotime('-5 min', $now); + + $old = $cutofftime - MINSECS; + $new = $cutofftime + MINSECS; + + $oldid = $this->create_state_record('state_old', $old); + $newid = $this->create_state_record('state_new', $new); + + (new cleanup_oidc_state_and_token())->execute(); + + $this->assertFalse($DB->record_exists('auth_oidc_state', ['id' => $oldid])); + $this->assertTrue($DB->record_exists('auth_oidc_state', ['id' => $newid])); + } + + /** + * A configured "stateexpiry" value is honored, keeping records that the default 5 minute + * window would have deleted. + * + * @return void + * @throws dml_exception + * @covers ::execute + */ + public function test_configured_expiry_is_honored(): void { + global $DB; + $this->resetAfterTest(); + + set_config('stateexpiry', 15, 'auth_oidc'); + + $now = time(); + $cutofftime = strtotime('-15 min', $now); + + // 10 minutes old: would be deleted by the default 5 minute window, but must be kept + // with a configured 15 minute expiry. + $keptbyconfig = $now - (MINSECS * 10); + $old = $cutofftime - MINSECS; + $new = $cutofftime + MINSECS; + + $keptbyconfigid = $this->create_state_record('state_kept', $keptbyconfig); + $oldid = $this->create_state_record('state_old', $old); + $newid = $this->create_state_record('state_new', $new); + + (new cleanup_oidc_state_and_token())->execute(); + + $this->assertTrue($DB->record_exists('auth_oidc_state', ['id' => $keptbyconfigid])); + $this->assertFalse($DB->record_exists('auth_oidc_state', ['id' => $oldid])); + $this->assertTrue($DB->record_exists('auth_oidc_state', ['id' => $newid])); + } + + /** + * A "stateexpiry" of zero or less falls back to the default 5 minute window, rather than + * deleting state records almost immediately. + * + * @return void + * @throws dml_exception + * @covers ::execute + */ + public function test_non_positive_expiry_falls_back_to_default(): void { + global $DB; + $this->resetAfterTest(); + + set_config('stateexpiry', 0, 'auth_oidc'); + + $now = time(); + + // Created just now: would be deleted almost immediately by a literal zero-minute expiry, + // but must survive because zero falls back to the 5 minute default. + $recent = $now - MINSECS; + + $recentid = $this->create_state_record('state_recent', $recent); + + (new cleanup_oidc_state_and_token())->execute(); + + $this->assertTrue($DB->record_exists('auth_oidc_state', ['id' => $recentid])); + } +} diff --git a/version.php b/version.php index cb0064cef..43242e981 100644 --- a/version.php +++ b/version.php @@ -25,8 +25,8 @@ defined('MOODLE_INTERNAL') || die(); -$plugin->version = 2026042000; +$plugin->version = 2026042001; $plugin->requires = 2026042000; -$plugin->release = '5.2.0'; +$plugin->release = '5.2.1'; $plugin->component = 'auth_oidc'; $plugin->maturity = MATURITY_STABLE;