Skip to content

Conversation

@andrea-sdl
Copy link
Contributor

Description

This PR adds syncing of the wpcom_vip_wsc_forced_mfa_users_additional_capabilities filter to SDS, sending two new fields

  • has_mfa_additional_capabilities_filter if there's a function
  • mfa_additional_required_capabilities the contents of the additional capabilities.

Pre-review checklist

Please make sure the items below have been covered before requesting a review:

  • This change works and has been tested locally or in Codespaces (or has an appropriate fallback).
  • This change has relevant unit tests (if applicable).
  • This change has relevant documentation additions / updates (if applicable).
  • I've created a changelog description that aligns with the provided examples.

Pre-deploy checklist

  • VIP staff: Ensure any alerts added/updated conform to internal standards (see internal documentation).

Steps to Test

Add this filter to the vip-security-boost.php file

add_filter( 'wpcom_vip_wsc_forced_mfa_users_additional_capabilities', function () {
	return [ 'other_capability', 'edit_posts' ];
} );

Apply (somewhere) the SDS filter via

$test_data = apply_filters('vip_site_details_index_security_boost_data', []);

Verify that the test_data output contains the new fields.

@pandah3
Copy link
Contributor

pandah3 commented Nov 8, 2025

@andrea-sdl What was this PR for again? Can we close it out or is the work for it almost done?

@andrea-sdl andrea-sdl marked this pull request as ready for review November 11, 2025 09:50
Copilot AI review requested due to automatic review settings November 11, 2025 09:50
@andrea-sdl
Copy link
Contributor Author

@pandah3 yes, this should be working properly. It's a way for us to track which custom capabilities a customer has configured. We'll need a change in the backend to ensure we store this in the SDS DB, that part is missing.

Copilot finished reviewing on behalf of andrea-sdl November 11, 2025 09:51
Copy link
Contributor

Copilot AI left a comment

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 adds syncing of the vip_wsc_forced_mfa_users_additional_capabilities filter data to the Site Details Service (SDS). The implementation detects when the filter is present and extracts the additional capabilities as a comma-separated string.

Key changes:

  • Added two new fields to the SDS payload: has_mfa_additional_capabilities_filter (boolean) and mfa_additional_required_capabilities (comma-separated string)
  • Added comprehensive test coverage for the new filter detection and data extraction functionality
  • Updated test setup/teardown to properly clean up the new filter

Reviewed Changes

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

File Description
modules/data-sync/class-data-sync.php Added logic to detect and extract MFA additional capabilities filter data, updated documentation to reflect the new fields in the SDS payload
tests/phpunit/test-data-sync.php Added filter cleanup in setUp/tearDown, updated default test expectations, added new test for MFA additional capabilities filter, and added test for two-factor entirely disabled scenario

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

'has_enable_two_factor_filter' => \has_filter( 'wpcom_vip_enable_two_factor' ) !== false,

'has_mfa_additional_capabilities_filter' => $has_additional_capabilities_filter,
'mfa_additional_required_capabilities' => $has_additional_capabilities_filter ? implode( ',', apply_filters( Forced_MFA_Users::ADDITIONAL_CAPABILITIES_FILTER_NAME, [] ) ) : '',
Copy link

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

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

The filter result should be normalized before using implode(). If the filter returns a non-array value (e.g., string, null, or object), this will cause a type error. Consider using Capability_Utils::normalize_capabilities_input() like the existing Forced_MFA_Users::get_custom_enforced_capabilities() method does, or at least cast the result to an array.

Example:

'mfa_additional_required_capabilities' => $has_additional_capabilities_filter ? implode( ',', (array) apply_filters( Forced_MFA_Users::ADDITIONAL_CAPABILITIES_FILTER_NAME, [] ) ) : '',

Or better yet, use the normalization utility:

'mfa_additional_required_capabilities' => $has_additional_capabilities_filter ? implode( ',', Capability_Utils::normalize_capabilities_input( apply_filters( Forced_MFA_Users::ADDITIONAL_CAPABILITIES_FILTER_NAME, [] ) ) ) : '',

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants