Require a nonce and lesson access to toggle lesson progress

[kimcoleman]

Problem

pmpro_courses_toggle_lesson_progress_ajax() (includes/progress.php) verified nothing beyond a logged-in user before calling PMPro_Courses_User_Progress::toggle_lesson_progress().

To be fair to the existing code, two things already limited the impact:

• The handler is registered on wp_ajax_ only (no nopriv), so a visitor had to be logged in.
• It always writes for get_current_user_id() and ignores any user passed in, so one member could not alter another member's progress.

Two real gaps remained:

1. No nonce. Any third-party page could drive a logged-in member's browser into marking lessons complete (CSRF).
2. No access check. Any logged-in user could mark progress on any lesson id, including lessons their membership level does not grant.

Change

• Localize a nonce alongside ajaxurl and send it with the toggle request (pmpro-courses.php, js/frontend.js).
• check_ajax_referer( 'pmpro_courses_toggle_lesson_progress', 'nonce' ) in the handler.
• Reject a lid that is not a pmpro_lesson.
• Require pmpro_has_membership_access( $lesson_id ) before writing.

Access is checked on the lesson rather than the parent course because lesson-level restrictions are what pmpro_has_membership_access() evaluates, and going through it picks up the Free Lesson bypass via the existing pmpro_has_membership_access_filter_pmpro_lesson filter.

Testing

Exercised the handler directly against a local site. The DB write path was deliberately not run, so no progress rows were touched.

Case	Result
No nonce	rejected, -1
Invalid nonce	rejected, -1
Valid nonce, lesson the member's level does not grant	403
Valid nonce, non-lesson post id	400
Valid nonce, lid=0	400
Free lesson, logged-in member	guards pass

Note

Found while reviewing the lesson drip work (#142), but this is pre-existing and independent of it, so it is split out here against dev. It composes with drip rather than duplicating it: once #142 lands, pmpro_has_membership_access() also consults the release date through the same post-type filter, so an unreleased lesson stops accepting progress writes with no further change here.

[flintfromthebasement]

PR: #143 — Require a nonce and lesson access to toggle lesson progress
kimcoleman → dev | 3 files, +20 -5
#143

---

Summary
The CSRF fix is solid, but the new authorization check doesn't do what the description claims. Not ready to merge until the access check accounts for course-level restrictions.

---

Issues

• [Critical] includes/progress.php:286 — pmpro_has_membership_access( $lesson_id ) checks restriction rows keyed to the lesson's own post ID only (verified in PMPro core, includes/content.php:65-85: the SQL queries wp_pmpro_memberships_pages/wp_pmpro_memberships_categories by $mypost->ID, with no parent traversal for the pmpro_lesson post type — that fallback only exists for attachment/revision at content.php:43-46). In this plugin's own admin UI, restriction is only ever set on the course, not individual lessons (includes/courses.php:211-228, the "Level" column reads wp_pmpro_memberships_pages keyed by $course_id). That's exactly why the plugin's own frontend gate checks the parent first: includes/courses.php:273-282 calls pmpro_has_membership_access( $post->post_parent ) for a lesson, only falling back to the lesson's own ID to catch the Free Lesson bypass. The new AJAX handler skips that parent check entirely, so for a normal restricted course (no per-lesson restriction row), pmpro_has_membership_access( $lesson_id ) returns true for any logged-in user regardless of their level — the write-access gate this PR adds doesn't actually block the scenario in its own test table ("lesson the member's level does not grant"). Fix: mirror courses.php:273-282 — check pmpro_has_membership_access( $lesson->post_parent ) first, and only use the lesson-level check as the fallback that picks up the Free Lesson bypass.

• [Minor] js/frontend.js:16-20 — wp_die( '', '', array( 'response' => 400/403 ) ) returns a non-2xx status, so jQuery.get()'s success callback never fires and the button silently does nothing. Not blocking — the goal is to block the write, not to be polite — but worth a .fail() handler that reverts the pending UI state so a legitimately-blocked user (stale nonce, page open too long) isn't left staring at an unresponsive toggle.

---

Looks Good

• Nonce action string matches between creation (pmpro-courses.php:207, wp_create_nonce( 'pmpro_courses_toggle_lesson_progress' )) and verification (progress.php:274, check_ajax_referer( 'pmpro_courses_toggle_lesson_progress', 'nonce' )) — no mismatch.
• absint() plus an explicit empty()/get_post_type() check on lid is a real tightening over the old bare intval( $_REQUEST['lid'] ), and rejecting before any DB write closes off pmpro_has_membership_access()'s falsy-$post_id-returns-true trap (content.php:22-23).
• The handler is wp_ajax_-only (no nopriv), so this was never reachable by anonymous visitors — the fix is scoped correctly to the actual CSRF/authorization gap.

---

Questions

• js/frontend.js:15 sends the nonce via jQuery.get, putting it in the URL query string and server access logs. Given the PR's theme is hardening this request, is a switch to jQuery.post in scope, or intentionally deferred to a follow-up?