search

textpattern / textpattern

A flexible, elegant, fast and easy-to-use content management system written in PHP.
https://textpattern.com
GNU General Public License v2.0
778 stars 111 forks source link

Forbid direct access to theme assets? #1090

Closed Bloke closed 6 years ago

Bloke commented 6 years ago

Since themes reside in the publicly accessible website area, they are directly accessible in the browser via example.com/themes/theme-name/pages/default.txp. Should we forbid access to .txp files from .htaccess or simply encourage people to move themes outside docroot?

Moving the main themes directory outside docroot does mean other contained assets (e.g. custom js, images, or css) wouldn't be browser accessible, which is an issue.

What's the consensus? Is it a problem to potentially expose template files in their entirety in the browser? I guess so, as it might contain sensitive stuff like <txp:password_protect> tags.

cara-tm commented 6 years ago

@Bloke: Codeigniter has a pretty good feature that allow its installation above the web root (see here for more informations: https://www.codeigniter.com/user_guide/installation/index.html).

Bloke commented 6 years ago

Yes, that's for the whole of the system - something that can be achieved in Textpattern easily with multi-site. Since we don't actually run themes from the file system, we don't need to grant public access to the template files - even .css.

I don't think we want to completely forbid public access to the entire themes directory though, as people may legitimately bundle additional content (such as JavaScript) that does require web access. That would also impact our plans for future expansion of the theme system. But I don't like having template files exposed like this particularly.

Perhaps in future we could permit two locations per theme - one for public portions, one for private template code. It's a little more convoluted and makes bundling up themes more complicated. For now, I think we can get away with just forbidding .txp files from being served.

If we permit .css files, well, that's not so bad as they're likely to become public anyway when the site is rendered. But leaving them exposed could potentially reveal the fact that a particular Theme is in use. What do people think?

petecooper commented 6 years ago

My vote is to forbid access to *.txp from front side via .htaccess or similar, with explanatory info on how stuff can be moved out of docroot for additional safety. Having everyone move something outside of docroot is a UX train wreck waiting to happen, and will generate lots of support traffic.

"Here's a great new thing, but you have to jump through some hoops to make it work."

vs

"Here's a great new thing, and if you want to make it even greater, here's what you do."

philwareham commented 6 years ago

If you can forbid access to .txp files great. I’d need public access for everything else though since themes I’m planning have JavaScript and other items in the bundle.

phiw13 commented 6 years ago

Right now, I see that the /themes folder contains a .htaccess-dist file

# Inhibit directory listing
Options -Indexes
# Inhibit direct file downloads
RedirectMatch 403 .*

changing it to limit access only to *.txp would be a good start (still allows .css, .svg, etc)

# Inhibit directory listing
Options -Indexes
# Inhibit direct file downloads
RedirectMatch 403 .txp
Bloke commented 6 years ago

I concur with @phiw13 here. The local .htaccess is the right place to do this. I'll do this now and then we can try it to see if there's any fallout.

Bloke commented 6 years ago

Should be closed by 4ddb30c7. Testing welcome.

cara-tm commented 6 years ago

Well done.

Please, keep the CodeIngniter like secure option in mind. Could you not give access to all public directories within the config.php file? Quit separating the distrubition of TXP into two folders.

petecooper commented 6 years ago

In my experience, it's common to see admin-side functional (/textpattern/) and front-side presentational (/themes/) split into multiple directories.

petecooper commented 6 years ago

@Bloke - if I duplicate the default (or create a new) theme via the GUI (/textpattern/index.php?event=skin), where should I see the duplicated/new theme appear in the file system? I checked /themes/ and…nothing.

phiw13 commented 6 years ago

@petecooper - as far as I can tell, the duplicate theme is only stored in the DB. You have to export it to have it in the filesystem.

@cara-tm – I don’t have any problem with /themes/ existing alongside (outside) of /textpattern/ personally. It is a clear separation of tasks & functions.

Bloke commented 6 years ago

EDIT: I have a blog post about Themes written, just tidying and sprucing it up.

petecooper commented 6 years ago

@phiw13 & @Bloke - thank you, very much appreciated.

petecooper commented 6 years ago

Tested on http://dev-demo.eu.textpattern.co:

Edit: follow-up with subdirectories test…

Resolved. +1 for issue to be closed.