Skip to content

Fix missing sesskey validation on state-changing admin endpoints - #3376

Open
Patryk Mroczko (patmr7) wants to merge 1 commit into
MOODLE_501_STABLEfrom
wip-133982-m501
Open

Fix missing sesskey validation on state-changing admin endpoints#3376
Patryk Mroczko (patmr7) wants to merge 1 commit into
MOODLE_501_STABLEfrom
wip-133982-m501

Conversation

@patmr7

Copy link
Copy Markdown
Collaborator

No description provided.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens the local Microsoft 365 integration admin tools by enforcing Moodle sesskey (CSRF) validation on multiple state-changing endpoints, and by ensuring UI-triggered requests include the sesskey where needed.

Changes:

  • Added require_sesskey() checks to state-changing handlers (e.g., cohort sync deletion, user match queue clearing, maintenance actions, Teams connection actions, user connection actions).
  • Updated action URLs to include sesskey => sesskey() so newly-enforced sesskey checks pass when actions are triggered from the UI.
  • Adjusted URL string output for the user match queue clear AJAX calls to avoid HTML-escaped query separators breaking the POST target.

Reviewed changes

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

File Description
local/o365/cohortsync.php Enforces sesskey validation before deleting cohort sync mappings.
local/o365/classes/page/acp.php Adds/enforces sesskey validation across multiple admin modes and updates relevant action URLs.
local/o365/classes/form/cohortsync.php Adds sesskey to the “delete mapping” action link to satisfy the new validation.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants