local_o365: only run notifysecretexpiry task for connected Microsoft IdP sites using client secret

[Lai Wei (weilai-irl)]

The task now skips and succeeds unless the site uses a Microsoft IdP, the client secret authentication method, and a working Microsoft 365 connection. Remove the unused errorunsupportedsecretauthenticationmethod string.

[Copilot]

🟢 Approval recommended

The change is small, self-contained, and verified — the moved/added guards are correct, the removed language string is unreferenced, and the required constant is available.

0 open findings

What changed in this PR

This PR adjusts the local_o365 "notify secret expiry" scheduled task so it only performs real work when the preconditions for a client-secret expiry check are all met, and otherwise exits cleanly as a success rather than failing. Previously the task would throw a moodle_exception (error_not_connected) when the site wasn't connected, and later throw errorunsupportedsecretauthenticationmethod when a non-secret auth method was in use — both of which marked the scheduled task as failed on sites where the check simply doesn't apply.

Changes:

• The not-connected case now mtraces and returns true (skips) instead of throwing error_not_connected.
• The client-auth-method guard is moved up front, and now skips (returns true) instead of throwing when the method isn't client secret.
• Removes the now-unused errorunsupportedsecretauthenticationmethod language string, and refines the "not configured" skip message to reference the Microsoft identity provider.

File	Description
local/​o365/​classes/​task/​notifysecretexpiry.php	Reworks execute() precondition checks to skip-and-succeed (Microsoft IdP, client secret auth, active connection) instead of throwing.
local/​o365/​lang/​en/​local_o365.php	Removes the unused errorunsupportedsecretauthenticationmethod string.

I verified that AUTH_OIDC_AUTH_METHOD_SECRET is defined in auth/oidc/lib.php (which is required at the top of the task file), that the removed language string has no remaining references anywhere in the repository, that error_not_connected is still used by other callers (so its string is correctly retained), and that utils::is_configured() genuinely gates on a Microsoft IdP type (so the reworded skip message is accurate). No functional or consistency issues were found in the diff.

🧠 Review effort: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.