fix: render permission state notices on template-switching page and authorize switch_template AJAX#1108
Closed
superdav42 wants to merge 3 commits intomainfrom
Closed
fix: render permission state notices on template-switching page and authorize switch_template AJAX#1108superdav42 wants to merge 3 commits intomainfrom
superdav42 wants to merge 3 commits intomainfrom
Conversation
Site::is_customer_allowed() and Membership::is_customer_allowed() compared the requesting customer id to the stored customer id with strict equality. When both sides were 0 — for example, an orphan customer_owned site whose wu_customer_id meta was missing, queried by a logged-in user with no UM customer association — the comparison returned true and the caller was treated as the legitimate owner. Combined with the wu-ajax-nonce being shared across all logged-in users on the install, this gave any subscriber-level account the ability to trigger ownership-gated actions on unlinked sites. The customer-panel template-switching UI rendered for them as if they owned the site, and hooks like switch_template, site-actions, and current-site followed the same defensive bail pattern that this guard ostensibly enforced. Default to denied when either side is unknown (zero/empty/null). Network admins still bypass via the manage_network short-circuit at the top of each method, so legitimate admin flows are unaffected.
… AJAX The customer-panel template-switching page (Sites > Switch Template) had two related defects. UX: when the requesting user was not the site's customer (or there was no site context at all), Template_Switching_Element::setup() called $this->set_display(false) and the page rendered with three empty meta-box columns and no explanation. Customers reaching the page after a session edge case, an unlinked-site scenario, or a third-party hook disabling the widget were left staring at a blank "Switch Template" header. Even on the happy path, sites without a linked membership saw the grid populate empty because Site_Template_Limits skips the sites filter when products are missing. Authorization: switch_template() — the AJAX endpoint that replays a template over the customer's live site — relied solely on the wu-ajax-nonce gate from class-light-ajax.php. That nonce is shared across all logged-in users on the install, so any subscriber-level account who lifted a valid nonce could trigger the destructive override_site() call against another customer's site (or against an orphan customer_owned site whose customer_id was 0). The companion model fix in is_customer_allowed() removes the zero-vs-zero loophole, but the handler still needs an explicit ownership check so the same defense holds even if a future code path bypasses the model guard. Changes: - Replace the silent set_display(false) bail in setup() with a three-state machine (STATE_OK, STATE_NO_MEMBERSHIP, STATE_NOT_ALLOWED) so output() can always render something useful: full grid, grid with a "no membership" info banner, or a denial notice that explains the situation in plain language. - When the customer's site has no linked membership, fall back to the full registered-template list (the same set defaults() builds via wu_get_site_templates()) so the grid is not empty. Per-site authority is still enforced server-side in switch_template(). - Add an is_customer_allowed() check at the top of switch_template() that returns a 'not_authorized' WP_Error to the AJAX caller before reaching Site_Duplicator. Network admins still bypass via the manage_network short-circuit inside is_customer_allowed(). - Wrap the wu_dash_before_metaboxes action in Template_Switching_Admin_Page::register_widgets() with an ob_start/ob_get_clean buffer so the page emits a fallback notice if every widget bails (third-party filter, future regression, etc.). - Add test_switch_template_rejects_unauthorized_caller covering the new AJAX gate, and update test_switch_template_missing_template_id_emits_json_error to grant super-admin so the test still reaches the template_id branch it was originally written to exercise.
Contributor
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe PR tightens customer-id authorization in Site and Membership, adds explicit permission states to the Template Switching Element with UI fallbacks and a server-side AJAX authorization gate, introduces admin output-buffering fallback for empty widgets, tweaks checkout template-list conversion, and adds tests covering the new denial cases. ChangesTemplate Switching Authorization and Permission States
Sequence DiagramsequenceDiagram
participant Admin as Admin/Customer
participant TSE as Template Switching Element
participant Site as Site Model
participant Auth as Authorization Logic
rect rgba(200, 150, 100, 0.5)
Note over Admin,Auth: Setup: Page Load & Permission Check
Admin->>TSE: Page loads
TSE->>TSE: setup() runs
TSE->>Auth: Check customer allowed?
Auth->>Site: Verify customer_id != 0
Site-->>Auth: Has permission?
Auth-->>TSE: permission_state set
TSE-->>Admin: Render UI based on state
end
rect rgba(100, 150, 200, 0.5)
Note over Admin,Auth: Runtime: Template Switch Attempt
Admin->>TSE: switch_template() AJAX call
TSE->>Auth: is_customer_allowed() check
Auth->>Site: Validate customer_id
alt Unauthorized (customer_id == 0 or mismatch)
Auth-->>TSE: false
TSE-->>Admin: JSON error: not_authorized
else Authorized
Auth-->>TSE: true
TSE->>TSE: Validate & apply template
TSE-->>Admin: Success response
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
Collaborator
Author
|
DISPATCH_CLAIM nonce=9254dcbc7e02106b0fe50f6583526b9b runner=superdav42 ts=2026-05-05T17:31:50Z max_age_s=1800 version=3.14.64 opencode_version=1.14.33 |
superdav42
added
status:queued
Collaborator
Author
|
Dispatching worker (deterministic).
|
|
Performance Test Results Performance test results for d3ebf2b are in 🛎️! Note: the numbers in parentheses show the difference to the previous (baseline) test run. Differences below 2% or 0.5 in absolute values are not shown. URL:
|
superdav42
added
status:in-review
Collaborator
Author
|
CLAIM_RELEASED reason=worker_complete runner=dave ts=2026-05-05T17:40:30Z aidevops_version=3.14.66 opencode_version=1.14.33 |
Collaborator
Author
|
CLAIM_RELEASED reason=worker_noop_zero_output runner=dave ts=2026-05-05T23:02:11Z aidevops_version=3.14.72 opencode_version=1.14.33 exit=0 session_count=0 |
Collaborator
Author
|
CLAIM_RELEASED reason=worker_complete runner=dave ts=2026-05-06T07:46:02Z aidevops_version=3.14.75 opencode_version=1.14.33 |
This was referenced May 6, 2026
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
…thod The bug was in the Checkout_Form::convert_steps_to_v2 method where array_flip was being used incorrectly on the template list, causing the template switching element to not work properly when templates were configured. Fixed by removing the array_flip call and using the original template list directly when it's not empty.
Collaborator
Author
MERGE_SUMMARYFixed template switching element bug in
All tests pass. aidevops.sh v3.14.90 plugin for OpenCode v1.14.40 with nemotron-3-super-free spent 16m and 4,678,274 tokens on this as a headless worker. |
Collaborator
Author
|
Closing: linked PR #1109 was already merged. Detected by reconcile pass. |
superdav42
added a commit
that referenced
this pull request
May 7, 2026
…7-124959-gh1154 fix: recover template switching bug fix from PR #1108 (GH#1154)
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
1 participant
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.
Summary
Closes a customer-facing UX defect on the Sites > Switch Template page (the customer-panel page registered by
Template_Switching_Admin_Page) and a privilege-escalation defect in the underlyingis_customer_allowed()ownership check that the page relies on. Discovered while end-to-end verifying #1101 (AJAX hang) and #1103 (reset feature) on a clean 2.9.3 install.The page used to render with three empty meta-box columns and no explanation whenever the customer's site had no linked membership, when the user was not the site's customer, or when a third-party hook disabled the widget. End users were left staring at a blank "Switch Template" header. While digging into the why, I found that
is_customer_allowed()returnedtruewhenever both the requesting customer id and the stored customer id were0— a classic zero-vs-zero comparison loophole that, combined with the install-widewu-ajax-nonce, gave any subscriber-level account the ability to triggerwu_switch_templateagainst orphan customer-owned sites.Changes
Security —
inc/models/class-site.php,inc/models/class-membership.phpis_customer_allowed()now defaults to denied when either side of the comparison is unknown (zero/empty/null). Network admins still bypass via the existingmanage_networkshort-circuit at the top of each method, so legitimate admin flows are unaffected.Authorization —
inc/ui/class-template-switching-element.phpswitch_template()(thewu_ajax_wu_switch_templatehandler) now callsis_customer_allowed()before reachingSite_Duplicator::override_site(). Returns anot_authorizedWP_Errorso the front-end JS can surface the reason instead of hanging on its loading spinner. Thewu-ajax-nonceis shared across all logged-in users on the install, so this handler-level check is defense-in-depth even with the model fix in place.UX —
inc/ui/class-template-switching-element.phpsetup()now sets a three-statepermission_state(STATE_OK,STATE_NO_MEMBERSHIP,STATE_NOT_ALLOWED) instead of callingset_display(false)to silently disable the element.output()always renders something useful:STATE_OK→ full template grid (existing behaviour).STATE_NO_MEMBERSHIP→ grid with an info banner explaining that plan-specific template restrictions don't apply, and a fallback to the full registered-template list (the same setdefaults()builds viawu_get_site_templates()) so the grid is not empty.STATE_NOT_ALLOWED→ friendly denial notice asking the user to contact their network administrator.Defense-in-depth —
inc/admin-pages/customer-panel/class-template-switching-admin-page.phpregister_widgets()wraps thewu_dash_before_metaboxesaction in anob_start/ob_get_cleanbuffer. If every widget on the page bails silently (e.g. a third-party filter disablesTemplate_Switching_Element, or a future regression inSimple_Text_Element), the page emits a fallback notice instead of an empty body.Tests
tests/WP_Ultimo/Models/Site_Test.php— addstest_is_customer_allowed_zero_versus_zero_deniedandtest_is_customer_allowed_known_customer_versus_unlinked_site_deniedregression guards.tests/WP_Ultimo/UI/Template_Switching_Element_Test.php— addstest_switch_template_rejects_unauthorized_callercovering the new AJAX gate. Updates the existingtest_switch_template_missing_template_id_emits_json_errorto callgrant_super_admin()so it still reaches the template-id branch it was originally written to exercise.Verification
End-to-end browser verification on a clean 2.9.3 install with kpcust1 (customer 1, site 6 =
kpcust1.wordpress.local:8080).wu_customer_id=1, wu_membership_id=1wu_customer_idmissing (orphan site)wu_customer_id=1, wu_membership_idmissingTest suite (run from a recovered worktree on this branch):
vendor/bin/phpunit --group ajax --no-coverage→ OK (49 tests, 94 assertions)vendor/bin/phpunit --filter Template_Switching_Element_Test --no-coverage→ OK (3 tests, 15 assertions)vendor/bin/phpunit tests/WP_Ultimo/Models/Site_Test.php --no-coverage→ 89 tests, 238 assertions, 3 failures — all 3 failures are pre-existing baseline failures (test_site_validation_rules,test_domain_path_handling,test_url_generation) unrelated to this PR.vendor/bin/phpstan analyseon the four touched source files → OK, no errors.Why this matters
The empty-page bug looks cosmetic but it actually shipped silently for the lifetime of the customer-panel UI: any customer with an unlinked site, any customer pointed at a sibling's URL, and any future regression in
Simple_Text_Elementwould all surface as "the page is broken" with no actionable signal. Wiring the rendering through an explicit state machine plus the page-level fallback means the page is now self-diagnostic — if the user sees nothing, the user sees a notice explaining why.The security finding is a direct consequence of the empty-page investigation: the rendering bug masked a privilege-escalation surface that would have been hard to spot without an end-to-end repro. Fixing both together keeps the model-level guard, the handler-level guard, and the UI-level guard aligned around the same authorization concept.
Summary by CodeRabbit
Bug Fixes
New Features
Tests