/view/:guid route can expose plugin information to logged-out users

[ewinslow]

Just stumbled across this while testing a new routing system. I assume we don't want random people to be able to inspect what plugins are installed on a given site.

This is because plugins have access_id public by default. I don't see a way to get around that since plugins need to work for logged out users too, obviously.

Hmm...

[brettp]

In the plugin view: if (!elgg_in_context('admin')) { forward('/', 403); }

[ewinslow]

I'm not sure what your point is. I'm sitting here logged out looking at /view/2 and seeing the plugin.

[ewinslow]

Oh, I see. You're suggesting to put that in the view? Sorry!

[brettp]

Replying to ewinslow:

Oh, I see. You're suggesting to put that in the view? Sorry!

Right. It's mixing controller logic in the view with the forward, but since the /view/ route is built in, it's the cleanest choice. I suppose we could also just return true if we want to be super strict...

Could also add a registration function like we have with elgg_register_ajax_view(). I don't know how useful the /view/ route is, to be honest.

[ewinslow]

I'd prefer not to add another registration function.

Controller logic in the view is a little gross. I vote we return true.

[brettp]

return true doesn't work because it defaults to the default object view. You can still see the plugin entity.

This leaves a registration function, a forward() in the view, or something more intrusive. I'm leaning toward the registration function because no one uses the default page handler anyway. Something like elgg_register_default_page_handler($type, $subtype).

[ewinslow]

If no one uses it and it's causing security issues we should just remove it. No use cluttering the API.

[brettp]

If this is for 1.8 we can't change the API in either case, so it's up to changing the view.

I said it's not used, but I think that's because it's not advertised, not because it's useless. If we make a registration function for it, it will parallel elgg_register_page_handler() to provide a default page handler for /view/:guid for simple plugins that don't want to handle their own pages. This could eventually be extended to a system-wide default page handler with stuff like /list/:type/:subtype, /add, /edit, /owner, etc.

[ewinslow]

Meh. I'd rather spend the time improving our core routing system so that developers can make pretty urls without tearing their hair out.

My idea is here in case you missed it: ​https://github.com/ewinslow/elgg-evan

I've already created a couple plugins on this system and it makes a world of difference.

[brettp]

Many other similar projects ship with default page handlers for a good reason: not having to write page handlers for simple URLs (which most are) is a huge win. Look at all the boilerplate code in the bundled plugins' page handlers.

[ewinslow]

Here's a little code sample.

// routes.php
return array(
    '/blog/owner/:container_guid' => 'blog/list',
);

// pages/blog/list.php

set_input('type', 'object');
set_input('subtype', 'blog');

// Call out to a different page handler
page_handler('entity/list');

This doesn't seem like excessive boilerplate, and you get things like potential for reverse routing, code reuse, relevant urls, etc.

Is this compelling at all?

[brettp]

Yes, it's compelling, but I'm not saying our routing doesn't need improvement. I'm saying there is definite benefit in having a default routing system. elgg_register_default_page_handler('object', 'blog') is even lighter than what you have there. If we pull in your routing system to core, I'm sure the default routing will use it.

[Brett Profitt]

Fixes #4439. Redirecting if trying to view a plugin object outside of admin.

Changeset: 2c324332f33fca688d782b55a51d38a793a18430

[ewinslow]

2 things:

1. OK. Let's submit this default page handler registration issue as a separate ticket if you haven't already. I'm open to it.

2. This ticket exposes a more general problem with our setup, I think. We need everyone to have access to plugin entities so that the engine can initialize, but we don't want these entities to be inspectable by third parties. This may be a case where we need to use elgg_set_ignore_access when we want access to the plugins and otherwise set the access controls to private. If we don't figure out a lower-level solution, I can pretty much guarantee we're going to have a problem like this again in the future.

[brettp]

Already have to do something similar for the site object view, so I agree we need to discuss this more.

[Brett Profitt]

Fixes #4439. Redirecting if trying to view a plugin object outside of admin.

Changeset: 2c324332f33fca688d782b55a51d38a793a18430