Skip to content

[5.0] Allow Joomla be served from a public* folder#40509

Merged
HLeithner merged 49 commits intojoomla:5.0-devfrom
dgrammatiko:5.0-dev-rfc-public
Jun 26, 2023
Merged

[5.0] Allow Joomla be served from a public* folder#40509
HLeithner merged 49 commits intojoomla:5.0-devfrom
dgrammatiko:5.0-dev-rfc-public

Conversation

@dgrammatiko
Copy link
Contributor

@dgrammatiko dgrammatiko commented Apr 30, 2023

Pull Request for Issue # .

public folder == subfolder of the root installation (but not restricted to only this config) that has only the allowed entry points (index.php, administrator/index.php, api/index.php) and any public static files (ie media/, images/)

Summary of Changes

  • This is the actual code needed to get Joomla serving from a public folder. It doesn't include the code for the installation to produce/create the different serving option. Also it does't have the required symbolic links for both the media and images folders and neither the actual public folder (this is supposed to be generated)

  • The purpose of this PR is to have people try to serve Joomla from a subfolder named public instead of the plain root that gives access to all the folders...

  • The reason I opened this RFC is mainly because the actual changes required are minimal (that's since the introduction of the child templates)...

  • Gotcha: legacy templates WON'T work with this!!!

  • There is a new constant JPATH_PUBLIC which is used to conditionally change the code both in the HTMLHelper and the WAM so nothing is broken...

Testing Instructions

  • To try this you need to git clone Joomla
  • install Joomla
  • then apply this PR (check out this PR)
  • then unzip in the root folder of Joomla this public.zip
  • then symlink the root media to {root}\public\media
  • also symlink the root images to {root}\public\images
  • Point your server to the public folder (or if it's a local installation to localhost\{root}\public and you should still have a functioning Joomla instance, eg:

Screenshot 2023-04-30 at 12 28 40

Alternatively download and install from the link at the bottom of this PR (on Github) and

  • then unzip in the root folder of Joomla this public.zip
  • then symlink the root media to {root}\public\media, eg: ln -s /path-to-joomla/media /path-to-joomla/public/media
  • also symlink the root images to {root}\public\images, eg: ln -s /path-to-joomla/images /path-to-joomla/public/images
  • Point your server to the public folder (or if it's a local installation to localhost\{root}\public and you should still have a functioning Joomla instance

Screenshot 2023-04-30 at 12 28 40

Actual result BEFORE applying this Pull Request

All good

Expected result AFTER applying this Pull Request

All good but way safer

@HLeithner @laoneo

@joomla-cms-bot joomla-cms-bot added PR-5.0-dev RFC Request for Comment labels Apr 30, 2023
@brianteeman
Copy link
Contributor

6.0 at the very earliest.

@laoneo
Copy link
Member

laoneo commented Apr 30, 2023

There is no 6.0 branch yet, so he has to open it for 5.

Let it work for both root or public
@dgrammatiko
Copy link
Contributor Author

dgrammatiko commented Apr 30, 2023

6.0 at the very earliest.

Sure, but can the maintainers agree on:

  • the name of the folder This is customisable now
  • consider including the changes of the HTMLHelper + WAM in 5.0 to ease the path forward

@brianteeman
Copy link
Contributor

There is no 6.0 branch yet, so he has to open it for 5.

Then the title should at least be changed to say 6.0 and not 5.0

@brianteeman
Copy link
Contributor

What I am missing here is an explanation of

  • the benefits of this change
  • what happens to existing sites

@dgrammatiko
Copy link
Contributor Author

Then the title should at least be changed to say 6.0 and not 5.0

@brianteeman the public folder maybe could be delayed till 6 but IMHO it will be a good pointer if the changes in the WAM and HTMLHelper are done in the 5.0

the benefits of this change

Security, if you know you know...

what happens to existing sites

Nothing is changed. The idea is that this would be an option at the end of the installation where the user could choose if they will opt in to a safer env or not (for some hosts there might not be an option to select a subfolder as the actual served folder).
Also, I guess the change could be provided through the com admin or config (it's just creating a public folder with some files, few symlinks and copying the .httaccess)...

@brianteeman
Copy link
Contributor

brianteeman commented Apr 30, 2023

I thought that might be the reasons claimed for this change although this implementation does not achive it. It has been discussed many times before and while it is a possible option with hosting that you have full control of it is not possible at all with typical shared hosting. For that reason I am not in favour of this at all and it really cannot be tested on a local environment in the way you described

@HLeithner
Copy link
Member

I like the idea but not sure about the implementation.
Also I would to see a .htaccess protection ruleset for all files first or as alternative.

For example I don't like that the public folder must be a subfolder of jpath_root

@dgrammatiko
Copy link
Contributor Author

I like the idea but not sure about the implementation.

This IS NOT A PROPOSED IMPLEMENTATION!!! I just made the minimum viable testable version here to showcase that the actual needed changes (apart the public folder shenanigans) are less than 20locs (the HTMLHelper and the WAM files, the first 2 on the files list)

Also I would to see a .htaccess protection ruleset for all files first or as alternative.

I cannot help here, I'm using NGINX

For example I don't like that the public folder must be a subfolder of jpath_root

I guess it's doable to have them in completely different trees but then you have to define the paths according to the parent root folder. FWIW all the PHP projects that I'm aware (Laravel, Symphony, etc) are using a subfolder (usually public, thus I also used it here) similar to this example

@dgrammatiko
Copy link
Contributor Author

it really cannot be tested on a local environment in the way you described

testing instructions updated

dgrammatiko and others added 7 commits April 30, 2023 13:29
Signed-off-by: Dimitris Grammatikogiannis <dg@dgrammatiko.dev>
Signed-off-by: Dimitris Grammatikogiannis <dg@dgrammatiko.dev>
Use another constant `JPATH_PUBLICNAME` for the name of the public folder (so now it’s customizable)
@dgrammatiko
Copy link
Contributor Author

@HLeithner now if someone defines correctly both JPATH_PUBLIC and JPATH_PUBLICNAME it's possible to have the public folder:

  • named whatever they want
  • not a direct child of the JPATH_ROOT

@HLeithner
Copy link
Member

if possible I would remove the complexity by assuming that your new constants are always defined.

- create copies of incompatible and build_incomplete at the includes folders
- adjust the indexes and app.php files
- adjust the framework.php
@dgrammatiko dgrammatiko force-pushed the 5.0-dev-rfc-public branch from 2ee2ea3 to 978c562 Compare May 5, 2023 10:21
@HLeithner
Copy link
Member

if you write this was a bad idea, it would be really useful to write why it was a bad idea

@dgrammatiko
Copy link
Contributor Author

dgrammatiko commented May 6, 2023

if you write this was a bad idea, it would be really useful to write why it was a bad idea

So, I thought that having the static html pages (build_incomplete, incomplete) available directly in the includes folder would simplify things and then I checked that PHP is doing a file read. Also the changes here are tweaked to reflect an easy path to actually achieve a public folder (either though CLI or GUI) base on the code from https://github.com/dgrammatiko/public-module/blob/main/src/modules/administrator/publicfolder/src/Helper/PublicfolderHelper.php

BTW this PR should be ready for testing (ie install J from this PR and the module from the other repo and try both the nested public folder and a public_folder not a direct child of the J root)

@dgrammatiko dgrammatiko closed this May 8, 2023
@dgrammatiko dgrammatiko reopened this May 8, 2023
@wilsonge
Copy link
Contributor

I think having this is a good idea overall - especially given our reliance on the composer folder which we've increasingly had to blank out in htaccess rules i suspect many people on shared hosting don't enable anyhow). I don't think it's anything we want to expose though through the UI. I can imagine us using it in our own docker images and documenting for those who want to use it. The impact to extension devs and users is going to be nearly non-existent.

Worth noting this probably would only work on extensions who use the new parent template options (and therefore store their css/js/images in the media directory as opposed to the templates directory.

@dgrammatiko
Copy link
Contributor Author

I don't think it's anything we want to expose though through the UI.

Although I created a module that would create the symlinks and all the other shenanigans I wouldn't suggest that for the core! That said and since there's a CLI installer for Joomla a couple extra params could expose a way to install Joomla directly with a public folder. The CLI version also, by default, is targeting advanced users so it should align nicely with what George wrote above.

Worth noting this probably would only work on extensions who use the new parent template options

Yup, I've mentioned that in the description: Gotcha: legacy templates WON'T work with this!!!

Copy link
Member

@HLeithner HLeithner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I merge this for now to hopefully get more feedback with alpha 2

'{{phpversion}}',
JOOMLA_MINIMUM_PHP,
file_get_contents(dirname(__FILE__) . '/templates/system/incompatible.html')
file_get_contents(dirname(__FILE__) . '/includes/incompatible.html')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm wouldn't it make more sense to move this to the media folder?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

having this file now 2 times looks a bit silly

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's fine because the file is generated by the tools, so the source is 1 file

@HLeithner HLeithner merged commit 12a1274 into joomla:5.0-dev Jun 26, 2023
@HLeithner
Copy link
Member

thanks, please write some documentation at manual.joomla.org

@HLeithner HLeithner added this to the Joomla! 5.0 milestone Jun 26, 2023
@dgrammatiko dgrammatiko deleted the 5.0-dev-rfc-public branch June 26, 2023 08:31
@dgrammatiko
Copy link
Contributor Author

Oh wow, this is a great leap forward for security! Thanks

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants