fix: prevent Edit Flow nonce checks from killing unrelated forms

[GaryJones]

Summary

Fixes #882

The notifications module was incorrectly checking the generic _wpnonce field against Edit Flow's expected action when posts were saved. This caused contact forms and other plugins that triggered post status transitions to fail, as Edit Flow would call wp_die() when their unrelated nonces didn't verify.

Changes

1. save_post_subscriptions(): Now checks for Edit Flow's own form indicator (ef-save_followers) first, then verifies its own dedicated nonce field (ef_notifications_nonce) with action save_user_usergroups. Returns early instead of calling wp_die().

2. handle_user_post_subscription(): Fixed faulty nonce logic that allowed requests without any nonce to pass through. Changed from ! empty() && ! verify() to ! isset() || ! verify().

---

Walkthrough: How this fixes the reported issue

The Scenario

A user submits a speaker contact form on WordCamp. This form:

1. Has its own $_POST['_wpnonce'] field for action speaker_submission
2. Creates/updates a post behind the scenes (to store the speaker data)
3. This triggers WordPress's transition_post_status hook

Before (The Bug)

public function save_post_subscriptions( $new_status, $old_status, $post ) {
    // Skip revisions...
    
    if ( ! empty( $_POST['_wpnonce'] ) && ! wp_verify_nonce( $_POST['_wpnonce'], 'update-post_' . $post->ID ) ) {
        $this->print_ajax_response( 'error', ... );  // This calls wp_die()!
    }
    
    if ( isset( $_POST['ef-save_followers'] ) && ... ) {
        // Save followers
    }
}

What happened:

1. Contact form submits with $_POST['_wpnonce'] = nonce for speaker_submission
2. Post is created → transition_post_status fires → save_post_subscriptions() runs
3. ! empty( $_POST['_wpnonce'] ) → TRUE (the contact form set it)
4. ! wp_verify_nonce( $_POST['_wpnonce'], 'update-post_123' ) → TRUE (wrong action, verification fails)
5. Both conditions true → print_ajax_response() → wp_die() → Request killed
6. Speaker contact form submission fails with "Cheatin' uh?"

After (The Fix)

public function save_post_subscriptions( $new_status, $old_status, $post ) {
    // Skip revisions...
    
    // Only process if Edit Flow's form was submitted
    if ( ! isset( $_POST['ef-save_followers'] ) ) {
        return;  // ← Exit early, don't interfere
    }
    
    // Check capability
    if ( ! current_user_can( $this->edit_post_subscriptions_cap ) ) {
        return;
    }
    
    // Verify Edit Flow's OWN nonce
    if ( ! isset( $_POST['ef_notifications_nonce'] ) || ! wp_verify_nonce( $_POST['ef_notifications_nonce'], 'save_user_usergroups' ) ) {
        return;  // ← Return, don't die
    }
    
    // Save followers...
}

What happens now:

1. Contact form submits with $_POST['_wpnonce'] = nonce for speaker_submission
2. Post is created → transition_post_status fires → save_post_subscriptions() runs
3. ! isset( $_POST['ef-save_followers'] ) → TRUE (contact form didn't set this)
4. Function returns early → No interference
5. Contact form submission succeeds ✓

Key changes

• Check for Edit Flow's form first (ef-save_followers) before doing anything
• Use Edit Flow's own nonce field (ef_notifications_nonce) instead of the generic _wpnonce
• Return instead of die - even if something fails, don't kill the entire request

---

Test plan

• All existing notifications tests pass
• New tests added for:
  • Accepting Edit Flow's own nonce
  • Ignoring requests without Edit Flow form data
  • Not dying when unrelated form nonce is present (the Cheatin' uh? #882 scenario)
  • Returning early with invalid nonce (not dying)
  • AJAX handler properly requires nonce

🤖 Generated with Claude Code

Co-Authored-By: Claude Opus 4.5 noreply@anthropic.com

[dd32]

@GaryJones A quick visual review seems good to me.

However, I'm questioning if this is correct:

add_action( 'transition_post_status', [ $this, 'save_post_subscriptions' ], 0, 3 );

I think it might be better if that was hooked onto the save_post action instead of the transition.

[GaryJones]

Thanks for the review @dd32!

The transition_post_status hook is intentional here — save_post_subscriptions runs at priority 0 before notification_status_change runs at priority 10 on the same hook. Since save_post fires after transition_post_status in WordPress core, switching just this function would break that ordering.

That said, your instinct is sound — save_post is the more canonical hook for saving meta alongside a post. I've opened #886 to track the potential refactor (moving both functions to save_post), as it would be a larger change with potential breaking implications that's outside the scope of this bug fix.