fix(theme): route draft permalinks through admin-post.php preview redirect#168
Merged
Merged
Conversation
…irect Closes #151 (problem 2). The editor's hamburger-menu "View" link, the post-list "View" row action, and the admin bar all read get_permalink(). For drafts, cdcf_frontend_path_for() returned null and the permalink fell through to WP's default ?p=<id> — which the Next.js catch-all resolves to the home page in preview mode. The only path that did route through /api/preview was the Gutenberg "Preview" button (via preview_post_link), which isn't surfaced in this editor. Approach A from #151: add a WP admin-post.php redirect endpoint, route draft permalinks at it, capability-check + redirect server-side. - cdcf_build_frontend_preview_url($post): factored out of the preview_post_link closure. Single source of truth for the URL shape (id + type + slug + lang + secret query args on the frontend /api/preview endpoint). - cdcf_should_redirect_to_preview($post): true for any post/page that is not yet published (drafts, auto-drafts, pending). Excludes trashed posts (not surfaced with View links anyway), CPTs (the frontend's /api/preview only allows post/page), and non-object inputs from misbehaving callers. - cdcf_redirect_to_frontend_preview(): the admin-post.php handler. admin_post_ prefix means WP core gates the request as logged-in only; the handler further capability-checks edit_post on the requested id, then wp_redirect()s to cdcf_build_frontend_preview_url(). Read-only, no nonce needed — the edit_post capability IS the auth gate. - cdcf_frontend_permalink(): detects the draft case BEFORE the cdcf_frontend_path_for null bail. For drafts of post/page, returns admin_url('admin-post.php?action=cdcf_preview_redirect&id=<id>'). Drafts of CPTs and unroutable types still fall through to the default permalink (unchanged behavior). Notes on the design: - The shared preview secret never appears in get_permalink() / REST `link` output. It's added server-side at redirect time, by a handler that's already cookie-authenticated and capability-gated. - The preview_post_link callback now reuses cdcf_build_frontend_preview_url so the Gutenberg Preview button and the new admin-post.php redirect produce identical URLs. No drift between the two paths. - CDCF_FRONTEND_PREVIEWABLE_TYPES (new const) mirrors the PREVIEWABLE_TYPES set in app/api/preview/route.ts so the theme and frontend agree on which types support by-id preview. Tests: 9 new cases covering URL construction (id/type/slug/lang/secret propagation, never-published-slug-empty case), the redirect predicate (true for post/page drafts of various statuses; false for published, trash, CPTs, garbage input), and the permalink filter (draft routes through admin-post.php, published still routes to frontend, draft CPTs left alone). composer test: 383 PHPUnit tests passing (was 374). Production deploy needed (theme change): when merging, run `gh workflow run deploy.yml -f environment=production`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughRoutes editor preview flows for draft post/page types through an admin-post redirect: adds a previewable-types constant, builds Next.js /api/preview URLs, detects draft eligibility, wires an admin-post handler that validates and redirects, and adds PHPUnit tests covering construction, predicate, filter routing, and handler branches. ChangesDraft and Preview Permalink Handling
Sequence DiagramsequenceDiagram
participant Editor as Editor (WP Admin)
participant Filter as preview_post_link / cdcf_frontend_permalink
participant Predicate as cdcf_should_redirect_to_preview
participant Handler as cdcf_redirect_to_frontend_preview
participant Frontend as Next.js /api/preview
Editor->>Filter: Request preview URL for a post
Filter->>Predicate: Check post type/status (draft/page & previewable?)
Predicate-->>Filter: yes/no
alt draft & previewable
Filter-->>Editor: Return admin-post.php?action=cdcf_preview_redirect&id=...
Editor->>Handler: GET admin-post.php?action=cdcf_preview_redirect&id=...
Handler->>Handler: validate id, load post, check edit_post capability
Handler->>Frontend: Redirect to /api/preview?id=...&type=...&slug=...&lang=...&token=...
else published or not previewable
Filter-->>Editor: Return normal frontend permalink
end
🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@wordpress/themes/cdcf-headless/includes/frontend-permalinks.php`:
- Around line 170-184: The admin-post handler cdcf_redirect_to_frontend_preview
currently only checks edit capability but not the same preview eligibility as
the frontend; call cdcf_should_redirect_to_preview($post) (after retrieving
$post via get_post($id)) and if it returns false, abort with wp_die('Unsupported
post type for frontend preview.', 'Preview', ['response' => 400]) (or reuse the
existing 400 message), otherwise continue to build the frontend URL with
cdcf_build_frontend_preview_url and redirect; this aligns
cdcf_redirect_to_frontend_preview with cdcf_frontend_permalink and the frontend
allowlist.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f4ab5783-28da-456b-b0b5-59cc09c4c07f
📒 Files selected for processing (3)
wordpress/themes/cdcf-headless/functions.phpwordpress/themes/cdcf-headless/includes/frontend-permalinks.phpwordpress/themes/cdcf-headless/tests/FrontendPostPermalinkTest.php
Up to standards ✅🟢 Issues
|
…cover branches
Two issues from review:
1. CodeRabbit: the admin-post.php redirect handler only checked
edit_post capability, not whether the post is eligible for frontend
preview. A capable user could redirect a published post or a CPT
through here; the frontend's /api/preview would then 400, but the
WP-side contract was muddier than it needed to be. Add a second
gate using the existing cdcf_should_redirect_to_preview($post)
predicate that cdcf_frontend_permalink already uses.
Order matters: capability check first, eligibility second. The
reverse would let an unauthorized caller fingerprint post id as
draft-of-(post|page) (400 from cap-fail before eligibility) vs
published-or-CPT (400 from eligibility) — eligibility-first leaks
post-type-and-status to anyone with the id. Inline comment on the
ordering preserves the reasoning.
2. Codecov: handler body (13 lines) was uncovered. Add 5 PHPUnit cases
covering each exit branch:
- 400 on missing/zero id
- 404 on missing post
- 403 on missing edit_post (capability check first)
- 400 on ineligible post (capable user, published/CPT)
- happy-path redirect to the frontend /api/preview URL
stubHandlerTerminators() stubs wp_die / wp_redirect / absint to
throw typed RuntimeExceptions (response code encoded in the message),
so each branch is observable without actually exiting PHPUnit.
composer test: 388 passing (was 383). No production behavior change
for the happy path — only the previously-implicit "this handler
expects a draft post/page" contract is now enforced.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Codecov flagged 4 lines from the previous commit:
- L116 / L119 (truthy arms of the CDCF_FRONTEND_URL / CDCF_PREVIEW_SECRET
defined() ternaries in cdcf_build_frontend_preview_url) — never hit
because the test process doesn't define those constants
- L123 (": ''" arm of the function_exists('pll_get_post_language')
ternary) — never hit because Brain Monkey auto-declares the function
on Functions\when() and PHP can't undeclare it
- L195 (exit; after wp_redirect) — unreachable; wp_redirect is stubbed
to throw before reaching exit
- L226 (return $link; bail-out for null get_post) — pre-existing line,
landed in the diff because the surrounding context shifted
Fixes:
- New @runInSeparateProcess test defines CDCF_FRONTEND_URL +
CDCF_PREVIEW_SECRET and asserts cdcf_build_frontend_preview_url uses
them (covers L116 + L119). Separate process is required because PHP
constants can't be re-defined.
- Second @runInSeparateProcess test calls the helper WITHOUT stubbing
pll_get_post_language so function_exists() returns false (covers
L123). Same isolation rationale.
- Block-style @codeCoverageIgnoreStart/End around the exit; line —
PCOV doesn't honor the inline `// @codeCoverageIgnore` form.
- New plain test for the null-get_post branch (covers L226).
composer test: 391 passing (was 388). Local clover shows 0 uncovered
patch lines.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
wordpress/themes/cdcf-headless/tests/FrontendPostPermalinkTest.php (1)
425-442: ⚡ Quick win403 test doesn't actually verify the "capability before eligibility" ordering it documents.
The comment claims this proves the cap check runs before eligibility to prevent draft-vs-published fingerprinting, but the fixture uses a
draftpost — which is itself eligible. With either ordering the outcome is403(eligibility passes, thencurrent_user_canreturns false). To genuinely exercise the ordering invariant, use an ineligible post (e.g.publishor a CPT) together withcurrent_user_can => falseand assert403(not400): only the cap-first ordering yields403in that case.💚 Suggested fixture to make the ordering assertion meaningful
- Functions\when('get_post')->justReturn($this->makePost(['ID' => 1377, 'post_status' => 'draft'])); + // Ineligible (published) post: with eligibility-first ordering this + // would 400; cap-first ordering must surface 403. + Functions\when('get_post')->justReturn($this->makePost(['ID' => 1377, 'post_status' => 'publish'])); Functions\when('current_user_can')->justReturn(false);Optionally also assert
current_user_canis invoked with('edit_post', 1377)so the capability/target-id contract is covered.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wordpress/themes/cdcf-headless/tests/FrontendPostPermalinkTest.php` around lines 425 - 442, The test test_handler_403s_when_user_lacks_edit_post currently returns a draft post so it cannot distinguish capability-vs-eligibility ordering; change the get_post fixture to return an ineligible post (e.g. post_status => 'publish' or a non-previewable post_type) while still stubbing current_user_can to false, then call cdcf_redirect_to_frontend_preview() and assert it wp_dies with 403; additionally assert that current_user_can was invoked with the expected capability and ID (current_user_can('edit_post', 1377)) to verify the capability check used the post ID.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@wordpress/themes/cdcf-headless/tests/FrontendPostPermalinkTest.php`:
- Around line 425-442: The test test_handler_403s_when_user_lacks_edit_post
currently returns a draft post so it cannot distinguish
capability-vs-eligibility ordering; change the get_post fixture to return an
ineligible post (e.g. post_status => 'publish' or a non-previewable post_type)
while still stubbing current_user_can to false, then call
cdcf_redirect_to_frontend_preview() and assert it wp_dies with 403; additionally
assert that current_user_can was invoked with the expected capability and ID
(current_user_can('edit_post', 1377)) to verify the capability check used the
post ID.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e57be955-eff4-4050-aeed-12864ef8c680
📒 Files selected for processing (2)
wordpress/themes/cdcf-headless/includes/frontend-permalinks.phpwordpress/themes/cdcf-headless/tests/FrontendPostPermalinkTest.php
🚧 Files skipped from review as they are similar to previous changes (1)
- wordpress/themes/cdcf-headless/includes/frontend-permalinks.php
Per review: the previous 403 test used a draft fixture, so a 403 outcome proved only that the cap check rejected unauthorized callers — it couldn't distinguish whether the rejection came from the cap check or the eligibility check that runs after it. Switch to a PUBLISHED post (which would ALSO fail eligibility) so the test fails if the ordering ever reverses: under eligibility-first, the same fixture yields 400. Also swap Functions\when()->justReturn() for Functions\expect()->with() on current_user_can so the test asserts the cap check used the right capability AND post id, not some looser check. composer test: 391 passing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
2 participants
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 #151 (problem 2). With problem 1 resolved (frontend now has
WP_APP_USERNAME/WP_APP_PASSWORDin its runtime env, so/api/previewURLs render real drafts), the remaining gap is that the editor's hamburger-menu View link, the post-list View row action, and the admin bar all readget_permalink(). For drafts,cdcf_frontend_path_for()returnsnulland the permalink falls through to WP's default?p=<id>— which the Next.js catch-all resolves to/in preview mode. So clicking "View" on a draft sends the editor to the rendered home page with a preview banner, not the draft.This implements approach (A) from #151: route draft permalinks through a WP
admin-post.phpredirect endpoint. The shared preview secret never appears inget_permalink()/ RESTlinkoutput; it's added server-side at redirect time by a handler that's already cookie-authenticated (WP core, viaadmin_post_prefix) and capability-gated (edit_poston the target id).Changes
cdcf_build_frontend_preview_url($post)— factored out of thepreview_post_linkclosure so the Gutenberg "Preview" button and the new admin-post.php redirect produce identical URLs. Single source of truth.cdcf_should_redirect_to_preview($post)— true forpost/pagedrafts (draft,auto-draft,pending). False for published (handled by the existing path mapper), trashed (not surfaced with View links), CPTs (the frontend's/api/previewonly allowspost/page— redirecting a project draft there would 400), and non-object inputs.cdcf_redirect_to_frontend_preview()— theadmin-post.phphandler. Readsid, validates, capability-checksedit_post, thenwp_redirect()s to the frontend URL. Read-only, no nonce needed — the capability check IS the auth gate.cdcf_frontend_permalink()— detects the draft case BEFORE thecdcf_frontend_path_fornull bail; returnsadmin_url('admin-post.php?action=cdcf_preview_redirect&id=<id>')for routable drafts. Drafts of CPTs and unroutable types still fall through to the default permalink (unchanged behavior).CDCF_FRONTEND_PREVIEWABLE_TYPES(new const) — mirrorsPREVIEWABLE_TYPESinapp/api/preview/route.tsso theme and frontend agree.Why no nonce
The handler is read-only (no state change — it just redirects). A CSRF attack would at most force a logged-in editor to view their own draft. The capability check (
edit_post) is the actual auth gate.Test plan
composer test --working-dir=wordpress/themes/cdcf-headless— 383 passing (was 374; +9 for the new helper coverage).admin-post.phpURL on a draft.cms.catholicdigitalcommons.org/wp-admin/admin-post.php?action=cdcf_preview_redirect&id=<id>is rejected by WP core (admin_post_ prefix requires login).edit_poston the target gets a 403 from the handler's capability check.Deploy
Theme change → production env needed:
gh workflow run deploy.yml -f environment=production🤖 Generated with Claude Code
Summary by CodeRabbit