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

Booting core in walled garden mode #9881

Closed
hypeJunction opened this Issue Jun 10, 2016 · 7 comments

Comments

Projects
None yet
2 participants
@hypeJunction
Contributor

hypeJunction commented Jun 10, 2016

I am working with a 1.12 install, so will need to verify on 2.x, but I believe the problem still exists.
I need to boot an engine from an external script that resides outside of an Elgg root. When I include start.php with walled garden mode on, I get forwarded to Elgg's landing.

Walled garden API should not be forwarding from 'init', 'system' handler, and rather hook into route.

@mrclay

This comment has been minimized.

Member

mrclay commented Jun 10, 2016

Consider plugins that use start.php to create (unrouted) pages. With the change you suggest, suddenly those pages are available to logged out users. In other words, there may be some reliance on existing behavior. IMO we shouldn't touch 1.12, but we could add a simple way to bypass it in 2.2, and make a bigger change in 3.0 if really necessary.

@hypeJunction

This comment has been minimized.

Contributor

hypeJunction commented Jun 10, 2016

I am fine leaving Elgg 1.12 be. But we should really clean up walled garden logic. forward() in an init,system handler is a huge NO-NO for me. Every time I use walled garden, I end up unregistering a bunch of handler to get things working the way I need them to work. I will dig through some site to find examples.

We should get rid of ElggSite::checkWalledGarden and distribute the logic between _elgg_walled_garden_init() and Elgg\Router::route().

I would think we would want to ensure correct access levels even if the script runs via CLI.

@mrclay

This comment has been minimized.

Member

mrclay commented Jun 10, 2016

We should get rid of ElggSite::checkWalledGarden and distribute the logic ...

👍 same with isPublicPage(). No need for this to live in ElggSite.

I would think we would want to ensure correct access levels even if the script runs via CLI.

I'm not sure I follow. What case are you imagining?

@hypeJunction

This comment has been minimized.

Contributor

hypeJunction commented Jun 10, 2016

Currently, ElggSite::checkWalledGarden bails if PHP_SAPI === 'cli'. I think we need to ensure we change the default access, as well as update write access collections, in spite of SAPI mode.

@hypeJunction

This comment has been minimized.

Contributor

hypeJunction commented Jun 10, 2016

I will make a PR for 2.x.

@hypeJunction

This comment has been minimized.

Contributor

hypeJunction commented Jun 10, 2016

Re: examples
The walled garden init handler is registered with a priority of 1000, so plugins have to register a callback with an even higher priority if they want to override the index page handler.

@mrclay

This comment has been minimized.

Member

mrclay commented Jun 11, 2016

To allow a script using start.php to "opt out" of walled garden protection, we'd probably have to sniff for a defined constant like ELGG_BYPASS_WALLED_GARDEN. A script can't do much before including start.php.

@jdalsem jdalsem closed this in e71784d Jan 3, 2017

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