-
-
Notifications
You must be signed in to change notification settings - Fork 11k
🐛 Fixed preview route for published post bypassing redirect to post URL #24653
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Walkthrough
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (3)
ghost/core/core/server/api/endpoints/previews.js (1)
21-23
: Settingframe.isPreview
for member_status previews is correct and fixes status strippingMarking preview frames ensures
clean.post
preservesattrs.status
for Content API previews, restoring the redirect behavior for published posts accessed via preview URLs. Nice, tight fix that aligns with the new serializer logic.As a small clarity nit, consider adding an inline comment to explain why
isPreview
is set here (it’s consumed by serializers to preservestatus
and support redirect logic):// only set apiType when given a member_status to preserve backwards compatibility // where we used to serve "Admin API" content with no gating for all previews frame.apiType = 'content'; + // Mark as preview so serializers can preserve attrs.status (used for redirect decisions) frame.isPreview = true;
ghost/core/core/server/api/endpoints/utils/index.js (1)
50-52
: Defensively handle undefined frames in isPreviewMinor hardening: use optional chaining so the helper gracefully handles unexpected undefined/null frames without throwing.
- isPreview: (frame) => { - return frame.isPreview === true; - } + isPreview: (frame) => { + return frame?.isPreview === true; + }ghost/core/test/e2e-frontend/preview_routes.test.js (1)
148-155
: Good added coverage for published redirect with member_status=paidThis validates the regression path and ensures parity with the non-parameter case. Consider extending coverage to the other allowed values to fully exercise the validation matrix:
?member_status=free
and?member_status=anonymous
.Example additions:
it('should redirect published posts to their live url with ?member_status=free', async function () { await request.get('/p/2ac6b4f6-e1f3-406c-9247-c94a0496d39d/?member_status=free') .expect(301) .expect('Location', '/short-and-sweet/') .expect('Cache-Control', testUtils.cacheRules.year) .expect(assertCorrectFrontendHeaders); }); it('should redirect published posts to their live url with ?member_status=anonymous', async function () { await request.get('/p/2ac6b4f6-e1f3-406c-9247-c94a0496d39d/?member_status=anonymous') .expect(301) .expect('Location', '/short-and-sweet/') .expect('Cache-Control', testUtils.cacheRules.year) .expect(assertCorrectFrontendHeaders); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
ghost/core/core/server/api/endpoints/previews.js
(1 hunks)ghost/core/core/server/api/endpoints/utils/index.js
(1 hunks)ghost/core/core/server/api/endpoints/utils/serializers/output/utils/clean.js
(1 hunks)ghost/core/test/e2e-frontend/preview_routes.test.js
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
ghost/core/test/e2e-frontend/preview_routes.test.js (3)
ghost/core/test/e2e-frontend/advanced_url_config.test.js (2)
request
(8-8)testUtils
(3-3)ghost/core/test/unit/frontend/web/middleware/frontend-caching.test.js (1)
testUtils
(3-3)ghost/core/test/unit/frontend/services/routing/controllers/previews.test.js (1)
testUtils
(3-3)
ghost/core/core/server/api/endpoints/previews.js (1)
ghost/core/test/unit/api/endpoints/previews.test.js (5)
frame
(24-24)frame
(31-31)frame
(38-38)frame
(45-45)frame
(54-54)
ghost/core/core/server/api/endpoints/utils/serializers/output/utils/clean.js
Show resolved
Hide resolved
…RL (#24653) ref https://linear.app/ghost/issue/ENG-2506/its-possible-to-bypass-paywall-on-a-post-by-constructing-a-preview There was a bug in the post preview route that bypassed the normal redirect to the published post. The preview route should only be accessible for posts that are not published. This was caused by the post's status being stripped in some cases, which bypassed the logic to redirect to the published post's normal URL, and instead allowed the preview content to render in its entirety.
…RL (#24653) ref https://linear.app/ghost/issue/ENG-2506/its-possible-to-bypass-paywall-on-a-post-by-constructing-a-preview There was a bug in the post preview route that bypassed the normal redirect to the published post. The preview route should only be accessible for posts that are not published. This was caused by the post's status being stripped in some cases, which bypassed the logic to redirect to the published post's normal URL, and instead allowed the preview content to render in its entirety.
…RL (#24653) ref https://linear.app/ghost/issue/ENG-2506/its-possible-to-bypass-paywall-on-a-post-by-constructing-a-preview There was a bug in the post preview route that bypassed the normal redirect to the published post. The preview route should only be accessible for posts that are not published. This was caused by the post's status being stripped in some cases, which bypassed the logic to redirect to the published post's normal URL, and instead allowed the preview content to render in its entirety.
ref https://linear.app/ghost/issue/ENG-2506/its-possible-to-bypass-paywall-on-a-post-by-constructing-a-preview
There was a bug in the post preview route that bypassed the normal redirect to the published post. The preview route should only be accessible for posts that are not published. This was caused by the post's status being stripped in some cases, which bypassed the logic to redirect to the published post's normal URL, and instead allowed the preview content to render in its entirety.