feat(access-control): add group subscription identifier to metadata#4746
Merged
Conversation
…4697) * fix(access-control): filter out nonexistent and non-group subs from user results * chore: update deprecated `filter_enabled_fields` method * feat(access-control): add Content_Access_Group ESP metadata field Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(access-control): include owned group sub names in Content_Access_Group Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(access-control): add `group` GA4 param for content access groups Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * fix(access-control): address Copilot review on PR #4697 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(access-control): address review blockers + extract shared name helpers - Restore Institution::user_matches_institution() IP-cache tightening lost in trunk merge: only treat the request as uncached when the visitor is the user being evaluated (or the cache-bypass cookie is set). Prevents the requestor's IP from leaking to non-current users during background metadata sync. - Guard get_group_labels() institution branch against non-array $value to avoid a PHP 8 TypeError when an institution rule has empty/scalar value. - Decouple Group_Subscription::get_group_subscriptions_for_user() from is_group_subscription()'s My-Account/Memberships side effect. Data-layer callers must always see the canonical group-enabled state. - Extract Group_Subscription::get_group_names_for_user() and Institution::get_matching_names_for_user() helpers, each with per-request static memoization. Use html_entity_decode( ENT_QUOTES | ENT_HTML5 ) so names with non-basic entities (—, ’, etc.) decode cleanly for ESP/GA4 transport. Use WooCommerce_Connection::get_active_subscriptions_for_user() for the owned path so status + gifting filters live in one place. - Update Content_Gate metadata get_group_labels() and GoogleSiteKit::get_user_group_names() to delegate to the shared helpers. - GA4 helper now early-returns when ! Reader_Activation::is_user_reader( $user ), matching the framing of the surrounding is_reader / is_subscriber params. - Replace brittle one-shot filter ordering in test_subscription_filter_source with an unconditional filter + unregistered product so the strict per-product lookup naturally falls through. - Add regression tests: malformed institution rule values (data provider) and cross-user IP leak prevention. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(access-control): address PR #4697 review round 2 T10 — Restore Memberships parity at the My-Account UI layer - Gate Group_Subscription_MyAccount::inject_member_group_subscriptions() on ! Memberships::is_active() so the data-layer decoupling done in 7426ab3 doesn't re-expose member group subs in the WC subscriptions list on Memberships sites. Mirrors the suppression that previously lived inside is_group_subscription(), but at the UI layer where it belongs. T7 — $strict filter contract caveat - Document on Access_Rules::has_active_subscription() that $strict only constrains the built-in ownership / group-membership checks. The newspack_access_rules_has_active_subscription filter is always applied; filter authors must opt into the 4th $strict arg and respect it, or callers relying on $strict (e.g. Content_Gate source labels) may misclassify filter-granted access as local ownership. T8 / T9 — Helper cache lifecycle + ID accessors - Move Group_Subscription and Institution per-request name caches to class-level statics so they can be reset. - Add public reset_cache() / reset_matching_cache() methods. - Hook reset on subscription status changes and group-member user-meta add/update/delete events so long-running CLI workers don't serve stale data across jobs. - Refactor each helper to a shared get_settings_map_for_user() / get_matching_map_for_user() that returns [id => decoded_name]; the existing get_*_names_for_user() functions and the new get_*_ids_for_user() accessors project from it. Settings are read once per candidate sub. - Institution cache key now includes get_current_user_id() so the IP-rule branch's current-user dependency doesn't produce stale hits. - Docblock note on the gifting-filter asymmetry between owned and member subscription paths. T11 — Anonymize GA4 group param - GoogleSiteKit::get_user_group_labels() now emits "Group {sub_id}" / "Institution {inst_id}" identifiers instead of display names. The unnamed-group fallback synthesizes a name from the owner's billing full name, so sending names to GA4 would leak PII for every member of an unnamed group. ESP path keeps human-readable names — only GA4 is anonymized. T14 — reader_data source label - get_source_labels() returns [ 'reader_data' ] for the reader_data rule slug. Previously a reader_data-only access rule yielded Content_Access=Yes but empty Content_Access_Source. T12 — Use get_post_field() for institution title transport - Replace get_the_title( $inst_id ) so the value going to GA4/ESP isn't run through the_title filters (texturization or third-party hooks that can introduce entities or markup). T13 — Document cookie-branch IP cross-attribution caveat - One-line caveat on IP_Access_Rule::is_cookie_set() disjunct: it still treats any non-current $user_id as uncached when the current visitor carries the cache-bypass cookie. Narrow pre-existing edge case. T18 — Direct $resolver(...) invocation - Drop call_user_func() in Content_Gate::collect_labels(); PHP 7+ invokes array callables directly. (Reverts the Copilot autofix in d350ee4.) T5 / T6 — Test isolation - Track Institution::create() post IDs in test classes that use them and delete them in tear_down(). Institution::create() inserts real posts not tracked by $this->factory, so they were leaking into later tests. - Reset Group_Subscription / Institution caches in set_up and tear_down. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs: improve docblock for get_user_group_labels memoization --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
dkoo
added
the
[Status] Needs Review
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new “content access group identity” surface across Reader Activation metadata and GA4 (Google Site Kit), including shared label-resolution helpers for group subscriptions and institutions plus accompanying regression/unit tests.
Changes:
- Add
Content_Access_Groupcontact metadata field and refactor label collection to share rule-walking logic. - Add GA4
groupcustom parameter (anonymizedGroup {sub_id}/Institution {inst_id}identifiers). - Introduce per-request memoized helpers for group-subscription and institution name/ID collection, plus expanded unit tests.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/unit-tests/plugins/google-site-kit.php | New unit tests validating GA4 group parameter behavior and anonymization. |
| tests/unit-tests/content-gate/group-subscriptions.php | Adds coverage for filtering stale/non-group subscription IDs from member meta. |
| tests/unit-tests/content-gate/class-institution.php | Adds regression coverage for IP matching not leaking across evaluated users. |
| tests/unit-tests/content-gate/class-content-gate-metadata.php | Expands metadata tests for Content_Access_Source and new Content_Access_Group. |
| includes/reader-activation/sync/contact-metadata/class-content-gate.php | Adds Content_Access_Group and shared label collection; updates source labeling. |
| includes/plugins/woocommerce-subscriptions/group-subscription/class-group-subscription.php | Adds memoized group name/ID helpers and cache invalidation hooks. |
| includes/plugins/woocommerce-subscriptions/group-subscription/class-group-subscription-myaccount.php | Moves Memberships suppression to the UI injection layer. |
| includes/plugins/google-site-kit/class-googlesitekit.php | Adds GA4 group parameter generation with anonymized labels. |
| includes/plugins/class-teams-for-memberships.php | Switches enabled-field filtering to integration-driven filter_enabled_outgoing_fields. |
| includes/content-gate/class-institution.php | Adds memoized institution name/ID helpers; tightens IP “uncached” determination. |
| includes/content-gate/class-access-rules.php | Adds $strict parameter to distinguish owned vs group-member subscription access. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
6 tasks
miguelpeixe
approved these changes
May 21, 2026
github-actions
Bot
added
[Status] Approved
|
Hey @dkoo, good job getting this PR merged! 🎉 Now, the Please check if this PR needs to be included in the "Upcoming Changes" and "Release Notes" doc. If it doesn't, simply remove the label. If it does, please add an entry to our shared document, with screenshots and testing instructions if applicable, then remove the label. Thank you! ❤️ |
|
🎉 This PR is included in version 6.41.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
github-actions Bot
pushed a commit
to Automattic/newspack-workspace
that referenced
this pull request
May 22, 2026
# [6.41.0](Automattic/newspack-plugin@v6.40.1...v6.41.0) (2026-05-21) ### Features * **access-control:** add group subscription identifier to metadata ([#4746](Automattic/newspack-plugin#4746)) ([6447bbf](Automattic/newspack-plugin@6447bbf)), closes [#4697](Automattic/newspack-plugin#4697) [#4697](Automattic/newspack-plugin#4697) [#8217](https://github.com/Automattic/newspack-plugin/issues/8217) [#4697](Automattic/newspack-plugin#4697)
6 tasks
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
All Submissions:
Changes proposed in this Pull Request:
Hotfix version of #4697.
How to test the changes in this Pull Request:
See #4697.
Other information: