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

Separate phplist\index.php (and other subscriber facing UI) from Admin parts #336

Open
mrojnetsky opened this issue May 29, 2018 · 14 comments

Comments

@mrojnetsky
Copy link

Hi there,

For me, it is a gray area if we should consider it a bug, a design flaw, or a feature request. If you don't agree that it is an issue, please move it to a proper place, or let me know and will recreate it elsewhere.

So in the root folder of my website I have .htaccess file, to which I added the following snippet
RewriteEngine On RewriteCond %{REQUEST_URI} ^/phplist/admin RewriteCond %{REMOTE_ADDR} !=x.x.x.x RewriteRule ^(.*)$ - [R=403,L]
This lets me access the admin section only from my IP and significantly decrease the angles of possible attacks from the bad boys and girls.

However, when I do that the subscriber facing UI looses all the formatting and style. At first I thought I'll do a quick workaround by just copying all the js and css from admin a level up and change the references in index.php. But after quick inspection I doesn't look like it is an easy workaround. The subscriber UI and admin parts are tightly coupled. And untangling them will be a big effort.

Please consider this for future releases. And I will be grateful if anyone has ideas on how to secure Admin without sacrificing Subscriber's UI.

@michield
Copy link
Member

why so complicated with a Rewrite to block access. Just a simple require line would be sufficient.
https://httpd.apache.org/docs/2.4/howto/access.html

I don't think any shipped frontend code is in the admin folder, but there may be other references to it. Try checking the network connections, when you press F12.

The forums are probably a better place to discuss this.

@mrojnetsky
Copy link
Author

mrojnetsky commented May 29, 2018

@michield hi there. As mentioned above, I want to keep my changes separate from phplist, thus I have .htaccess inside my root folder. I don't know how you would tell Apache that you need to restrict "/phplist/admin" only and spit out 403. Anyhow that's not the problem.

Unfortunately, the front end code is shared between Admin and Subscriber UI. See screenshot. All the reds are coming from admin/ui or admin/js. Again, as I said there is more to untangling then just copy-pasting css and js and changing the references somewhere in index.php. When I look at it, the index.php requires bunch of other php files (from admin) that in turn require more php files (also from admin). Somewhere inside admin the needed references (to js and css) are eventually made.

I can gladly move it to the forum, but I don't see "register" link here. If you implied some other forum please let me know. I will repost there and add a cross-reference here.

*reworded

@michield
Copy link
Member

Interesting. Looks like your Rewrite is blocking too much.

Forums are here: https://discuss.phplist.org/
Sign up here: https://www.phplist.org/join/

If you find it's a bug, report it here: https://mantis.phplist.org

@samtuke
Copy link
Collaborator

samtuke commented May 30, 2018

@asdr45fsd35fdf Separating the two areas of the application entirely would require more duplication of libraries and complications for translation. Hopefully you can achieve access restriction to the admin pages that you desire with modified rewrite rules. When you do, please can you share what worked here, so that it can be added to the wiki (or add it to the wiki directly yourself)?

@mrojnetsky
Copy link
Author

@michield thanks for the information.

@samtuke Agree, I will try to relax the RewriteCond and see if I can block php files only or subset of them. If I find a solution I will update the wiki.

Thank you both for suggestions.

@mrojnetsky
Copy link
Author

@samtuke

I come up with something that I think works:

Options -Indexes
<IfModule mod_rewrite.c>
	RewriteEngine On
	RewriteCond %{REQUEST_URI} ^/phplist/admin
	RewriteCond %{REQUEST_URI} !^/phplist/admin/ui
	RewriteCond %{REQUEST_URI} !^/phplist/admin/js
	RewriteCond %{REMOTE_ADDR} !=x.x.x.x
	RewriteRule ^(.*)$ - [R=403,L]
</IfModule> 

At least I've tested multiple scenarios and all looks good. I don't have access to any admin stuff but /phplist/ works fine because css and js from admin are not blocked anymore.

I don't know if it covers all the angles. I also don't know if this is the easiest way. If you find anything better, I'm all ears.

Sorry, I won't have time to register on phplist.com and create a wiki anytime soon. I'm overwhelmed with my todo list at the moment. If you think this is a good solution (and I assume you have an account with phplist.com) please post that to the wiki.

@samtuke
Copy link
Collaborator

samtuke commented Jun 5, 2018

@mrojnetsky Excellent thanks, documented here: https://resources.phplist.com/admin_pages_access_control

@frasersdev
Copy link

I'd like to chime in here if I may.

Because of the way my business works (mobile users), I use user certs to restrict access to admin functionality on websites. IP restriction won't work for me. The current architecture of phplist doesn't allow for this. Its unfortunate as the content seems to be mainly static?

I can understand the developers desire not to duplicate libraries, however the decision to place user content in the admin area should be reviewed as there are security implications.

In the short term it would be beneficial if the static content used in the subscription pages was configurable in the config.php, That would at least give people the option to compartmentalise the user pages if they had a reason to.

Longer term, a suggested architectural state:

  • /admin/ (admin pages for admins to do admin, secured, restricted)
  • /shared/ (shared libraries & static content, public)
  • / (user subscription / tracking pages, public)

@samtuke
Copy link
Collaborator

samtuke commented Jul 2, 2018

@frasersdev Thanks for clearly explaining your use case.

The current architecture of phplist doesn't allow for this. Its unfortunate as the content seems to be mainly static?

The most important content of the subscribe pages is dymanic -- which attributes are offered, plus default values etc. are loaded from the database.

In the short term it would be beneficial if the static content used in the subscription pages was configurable in the config.php, That would at least give people the option to compartmentalise the user pages if they had a reason to.

Do you mean that the location of these files should be configurable, so that other paths can be specified in the config file?

Longer term, a suggested architectural state:

Good idea, though it probably makes most sense to work on this separation for phpList 4 rather than phpList 3. New issue added here: phpList/core#296

@frasersdev
Copy link

frasersdev commented Jul 2, 2018

Hi @samtuke

The most important content of the subscribe pages is dymanic -- which attributes are offered, plus default values etc. are loaded from the database.

sorry perhaps I wasn't clear in what I meant. I understand the subscribe page code running on the server includes libraries from /admin. I was only referring to resources being downloaded over http out of the /admin area. On my vanilla install these are:

  • /admin/js/phplistapp.js
  • /admin/ui/phplist-ui-bootlist/css/bootstrap-select.min.css
  • /admin/ui/phplist-ui-bootlist/css/bootstrap-toggle.min.css
  • /admin/ui/phplist-ui-bootlist/css/style.css
  • /admin/ui/phplist-ui-bootlist/js/dist/phpList_ui_bootlist.min.js
  • /admin/ui/phplist-ui-bootlist/js/jquery-1.12.1.min.js

its all css and js (& static).
You can confirm this using chrome developer tools.

Do you mean that the location of these files should be configurable, so that other paths can be specified in the config file?

Yes, ie
"Set the http path for user subscription page template resources" (default = /admin) = /shared
and then phplist subscription pages would load instead:

  • /shared/js/phplistapp.js
  • /shared/ui/phplist-ui-bootlist/css/bootstrap-select.min.css
  • /shared/ui/phplist-ui-bootlist/css/bootstrap-toggle.min.css
  • /shared/ui/phplist-ui-bootlist/css/style.css
  • /shared/ui/phplist-ui-bootlist/js/dist/phpList_ui_bootlist.min.js
  • /shared/ui/phplist-ui-bootlist/js/jquery-1.12.1.min.js

easy to hardlink /shared/ui -> /admin/ui & /shared/js -> /admin/js
also easy to copy those 6 files over

allows for full gamut of security options operating at the http layer, eg mod_rewrite (as originally posted), .htaccess, users certs...

@frasersdev
Copy link

probably makes most sense to work on this separation for phpList 4 rather than phpList 3. New issue added

agreed and thank you 👍

@michield
Copy link
Member

michield commented Jul 2, 2018

If the references are hard coded, then I can understand this is needed, but the code provided is only there as an example and the expectation is that it is overwritten with a local design, which has its own css and js, pulled from other places (eg from /js and /css and not /lists/js/ or /lists/css/)

But yes, I guess the request in this issue could be done.

@frasersdev
Copy link

frasersdev commented Jul 3, 2018

Hi @michield
Thanks, that expectation wasn't obvious to me anyhow, buts its good to know. Aside from styles (which the manual does mention it in the context of needing to modify css), theres javascript, Bootstrap and jQuery to move as well.

Any new subscribe pages generated will of course refer back to resources in /admin/ui - but thats something that can be easily managed.

cheers

@phpitech
Copy link

I've just logged a separate issue ( #394 ) - thinking through the process as a new user and the decision of adding a index.html (that needs to be edited) and a seperate /lists folder - just want to add a note here in case there is any merit to also looking at the other suggestion if/when doing this :)

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

No branches or pull requests

5 participants