Fix OIDC login CSRF by verifying sesskey on callback

[Patryk Mroczko (patmr7)]

No description provided.

[Copilot]

Pull request overview

Adds an additional CSRF binding for the OIDC auth-code redirect/callback flow by carrying the initiating session’s sesskey in a dedicated SameSite=None cookie, then verifying it against the stored auth_oidc_state.sesskey during the callback handlers.

Changes:

• Set a dedicated auth_oidc_csrf cookie (Secure, HttpOnly, SameSite=None) before initiating the OIDC auth request.
• On both auth-code and admin-consent callbacks, verify the callback’s CSRF cookie matches the sesskey stored with the state record.
• Clear the CSRF cookie after validation.

Suppressed comments (1)

auth/oidc/classes/loginflow/authcode.php:395

• Same as above: the auth_oidc_csrf delete call should use the same cookie path as was used when setting the cookie (ideally scoped to the Moodle webroot path) to ensure the cookie is actually removed and doesn’t interfere with later logins.

        setcookie('auth_oidc_csrf', '', ['expires' => time() - 3600, 'path' => '/', 'secure' => true, 'httponly' => true, 'samesite' => 'None']);

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (2)

auth/oidc/classes/loginflow/authcode.php:407

• The CSRF cookie is cleared immediately after validating one callback. If the user has multiple in-flight OIDC states (e.g., multiple login/admin-consent flows opened in parallel), the first callback will clear the cookie and subsequent form_post callbacks will fail CSRF validation because the session cookie is typically not sent cross-site.

Consider only clearing the cookie when this was the last outstanding auth_oidc_state record for the current sesskey (or simply let the short-lived cookie expire).

        setcookie('auth_oidc_csrf', '', [
            'expires' => time() - 3600,
            'path' => parse_url($CFG->wwwroot, PHP_URL_PATH) ?: '/',
            'secure' => true,
            'httponly' => true,

auth/oidc/classes/loginflow/authcode.php:333

• The CSRF cookie is cleared immediately after validating one callback. If the user has multiple in-flight OIDC states (e.g., multiple login/admin-consent flows opened in parallel), the first callback will clear the cookie and subsequent form_post callbacks will fail CSRF validation because the session cookie is typically not sent cross-site.

Consider only clearing the cookie when this was the last outstanding auth_oidc_state record for the current sesskey (or simply let the short-lived cookie expire).

This issue also appears on line 403 of the same file.

        setcookie('auth_oidc_csrf', '', [
            'expires' => time() - 3600,
            'path' => parse_url($CFG->wwwroot, PHP_URL_PATH) ?: '/',
            'secure' => true,
            'httponly' => true,

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (5)

auth/oidc/classes/loginflow/authcode.php:340

• When clearing the CSRF cookie, match the same cookie scope (path/domain) used when setting it; otherwise deletion can fail in installations with custom cookie parameters. Using session_get_cookie_params() keeps this consistent with other cookie handling in the codebase.

        setcookie('auth_oidc_csrf', '', [
            'expires' => time() - 3600,
            'path' => parse_url($CFG->wwwroot, PHP_URL_PATH) ?: '/',
            'secure' => true,
            'httponly' => true,

auth/oidc/classes/loginflow/authcode.php:407

• $_COOKIE values can be attacker-controlled and may be parsed as arrays (e.g. auth_oidc_csrf[foo]=bar), which would trigger an "Array to string conversion" warning when cast to string. Validate the cookie is a non-empty string before using it, otherwise fall back to sesskey().

        $csrftoken = $_COOKIE['auth_oidc_csrf'] ?? sesskey();

auth/oidc/classes/loginflow/authcode.php:331

• $_COOKIE values can be attacker-controlled and may be parsed as arrays (e.g. auth_oidc_csrf[foo]=bar), which would trigger an "Array to string conversion" warning when cast to string. Validate the cookie is a non-empty string before using it, otherwise fall back to sesskey().

This issue also appears on line 407 of the same file.

        $csrftoken = $_COOKIE['auth_oidc_csrf'] ?? sesskey();

auth/oidc/classes/loginflow/authcode.php:418

• When clearing the CSRF cookie, match the same cookie scope (path/domain) used when setting it; otherwise deletion can fail in installations with custom cookie parameters. Using session_get_cookie_params() keeps this consistent with other cookie handling in the codebase.

        setcookie('auth_oidc_csrf', '', [
            'expires' => time() - 3600,
            'path' => parse_url($CFG->wwwroot, PHP_URL_PATH) ?: '/',
            'secure' => true,
            'httponly' => true,

auth/oidc/classes/loginflow/authcode.php:301

• The CSRF cookie is set without inheriting the site’s configured session cookie parameters (path/domain). Using session_get_cookie_params() here (as done elsewhere in the codebase) makes cookie scoping consistent across installations with custom cookie settings and ensures later deletion uses the same scope.

This issue also appears on line 336 of the same file.

            $cookiepath = parse_url($CFG->wwwroot, PHP_URL_PATH) ?: '/';
            setcookie('auth_oidc_csrf', sesskey(), [
                'expires' => time() + 5 * MINSECS,
                'path' => $cookiepath,
                'secure' => true,

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (2)

auth/oidc/classes/loginflow/authcode.php:443

• Same issue as above: gating verification on is_https()/sslproxy can skip CSRF validation in a reverse-proxy setup where the browser has HTTPS (and can send the Secure CSRF cookie) but Moodle isn’t aware of it. Including the cookie’s presence in the condition ensures the intended protection is applied when possible.

        if (is_https() || !empty($CFG->sslproxy)) {
            $this->verify_csrf_cookie($staterec);
        }

auth/oidc/classes/loginflow/authcode.php:376

• The CSRF check is skipped when Moodle doesn’t detect HTTPS and $CFG->sslproxy is unset. That includes the reverse-proxy scenario mentioned in set_csrf_cookie() (“missing $CFG->sslproxy”), which would leave the callback unprotected even if the CSRF cookie is present.

This issue also appears on line 441 of the same file.

        if (is_https() || !empty($CFG->sslproxy)) {
            $this->verify_csrf_cookie($staterec);
        }

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Suppressed comments (3)

auth/oidc/classes/loginflow/authcode.php:314

• clear_csrf_cookie() should use the same cookie attributes (especially path/domain) as set_csrf_cookie(). If the session cookie has a custom path/domain, clearing with a different scope can leave the CSRF cookie in place and cause unexpected verification behavior on subsequent requests.

        global $CFG;
        setcookie('auth_oidc_csrf', '', [
            'expires' => time() - HOURSECS,
            'path' => parse_url($CFG->wwwroot, PHP_URL_PATH) ?: '/',
            'secure' => true,

auth/oidc/classes/loginflow/authcode.php:443

• Same as above: the CSRF verification gate should also account for cases where the session cookie is present (allowing sesskey() to be stable) even if HTTPS flags are not set. Otherwise CSRF protection remains disabled for some non-HTTPS deployments.

        if (is_https() || !empty($CFG->sslproxy) || !empty($_COOKIE['auth_oidc_csrf'])) {
            $this->verify_csrf_cookie($staterec);
        }

auth/oidc/classes/loginflow/authcode.php:303

• The CSRF cookie is being scoped using parse_url($CFG->wwwroot, PHP_URL_PATH), but Moodle can be configured with custom session cookie path/domain. If those differ, the auth_oidc_csrf cookie may not be sent on the callback, causing false CSRF failures on HTTPS. Consider aligning path/domain with session_get_cookie_params() (as done in local/o365/sso_login.php:183-190).

This issue also appears on line 310 of the same file.

        $cookiepath = parse_url($CFG->wwwroot, PHP_URL_PATH) ?: '/';
        setcookie('auth_oidc_csrf', sesskey(), [
            'expires' => time() + 5 * MINSECS,
            'path' => $cookiepath,
            'secure' => true,