Skip to content
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

Share draft links do not work when global site settings say you must be logged in to view pages #104

Open
robbieaverill opened this issue Aug 26, 2019 · 5 comments

Comments

@robbieaverill
Copy link
Contributor

CWP 2.4.x-dev and CWP 1.9.3-rc2 (both SS3 and SS4)

  • Go to Settings > Access
  • Set "Who can view pages on this site?" to "Logged-in users" and save
  • Create a draft page, and generate a share link
  • Open the share link in an incognito window

Expected: you should see the shared draft content

Actual: the page returns a 200 response with no content

High, could be medium, because the permission setting is probably uncommon.

@NightJar
Copy link
Contributor

NightJar commented Aug 29, 2019

I feel like this is "working as intended".

Although that point can be argued, dynamically adding a new extension $page->add_extension(DraftContentViewingExtension::class) that implements canView($member = null) { return true; } would probably work around this.

Maybe.

@robbieaverill
Copy link
Contributor Author

I don’t think an empty white screen is intended, it should probably redirect to login as it does when you try to access the draft stage when not logged in

@NightJar
Copy link
Contributor

A fair point.
I guess the shared draft content controller needs to check for an HTTPResponse and return it directly if it finds one.
Currently it just assumes success then swallows any other result by returning a string, rather than relaying the response back up the stack.

return str_replace('</body>', $include . '</body>', (string) $rendered->getBody());

@robbieaverill
Copy link
Contributor Author

Yeah, 30x responses should be honoured

@NightJar
Copy link
Contributor

arguably the return errorpage in the else section could also be moved into it's own if, perhaps in a finally for handling redirects that occur via HTTPResponseExceptions (or whatever they're really called) that can also be used for redirection (or errors in general).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants