Add new settings to restrict diskfreespace, diskfreespacedata, ipaddress, sesskey tags

[durzo]

As a Moodle Partner, there are some tags we do not want our customers having access to.

This patch adds config settings to enable/disable diskfreespace, diskfreespacedata, ipaddress and sesskey tags.

plugin version has not been bumped, and should be done after merging this PR.

[durzo]

Hi Michael, certainly.

1. eThink Education

2. We use non-disk backed storage for moodledata and/or docroot, some of these simply don't have "free disk" features like amazon S3 and amazon EFS. EFS in particular has no limit and the output of the php function shows an enormous float. This information can be misleading or confusing for our customers and leads to lots of support tickets where the customer has the wrong information from this plugin.

3. Our webservers are behind load balancers and proxies, sometimes the webserver does not see the real user IP and we do not want to expose our internal infrastructure.

4. I actually can't think of any reason we would want to expose the sesskey, this is considered sensitive information - yes a user can see their own sesskey but why would you want to display it on the page to them? if someone acted maliciously and tricked a user into exposing their sesskey (like crafting a URL using filtercodes and embedding sesskey as a param) then the users session can be hijacked. The option to disable this is purely preventative - Is there an example of how this code is used in any useful way?

My comment about bumping version was related only to this PR for the new language strings - I didn't want to include version.php in this patch :)

[durzo]

I think its worth noting that by default all the options are enabled so including this patch changes nothing for existing users. it will just allow us and other partners to define them in config.php and force them off for customers.

[michael-milette]

Hi @durzo

Thank you for your comments. I did notice that the defaults were enabled by default and I appreciated that.

With regards to the EFS, you made a very valid point. However, in such a case, I am wondering if we could detect this situation and perhaps display an "∞" (infinity) symbol rather than disabling the tag. As I cannot reproduce this environment right now, I would appreciate it if you could help me devise a way to detect this type of scenario.

With regards to the IP address, FilterCodes uses the same IP address function as Moodle logging does. If you are not seeing the correct IP address, you may have a configuration issue with your Moodle site. I've seen this when using AWS containers behind load balancers and proxies (probably similar to your situation). I found that adding the following lines to your site's config.php helped resolve the issue.

// Enable when using external SSL appliance for performance reasons. // Please note that site may be accessible via http: or https:, but not both! $CFG->sslproxy = true; // Enable when detected IP detection is incorrectly identified. $CFG->getremoteaddrconf = 0;

It also resolves an issue about half way through during the Moodle installation of a new site from scratch in AWS pods, it stops and displays message saying that your IP address has changed.

The {sesskey} tag isn't meant to be displayed. I don't know why anyone would want to do that as it is a pretty meaningless sequence of characters. It is really more for things like the URL in links and is required to make the Contact Forms plugin (a form processor) work. Some examples of URL's that make use of {sesskey} include:

• If you want to add a Log out button or link: /login/logout.php?sesskey={sesskey}
• Turn editing on: /course/view.php?id={courseid}&sesskey={sesskey}&edit=on
• Purge cache: /admin/purgecaches.php?confirm=1&sesskey={sesskey}

We use these a lot when creating custom admin and dev menus for our clients.

Thoughts?

[michael-milette]

I am just reluctant to disabling tags because, then people try it out and complain that the tags don't work - and I end up doing support for an otherwise avoidable situation.

[durzo]

I guess the issue for us regarding storage & IP is that we provide managed hosting, the underlying storage space and IP information is not something the customer should be concerned with. Your points on logging is valid, and we also take measures to work around some of those so that customers either do not see them or they are fixed by code (I'm aware of sslproxy and getremoteaddr) - I can't give away too much details on our network to elaborate further (sorry).

I personally feel like allowing the use of {sesskey} is an unwanted security risk, and I think many at Moodle HQ would agree with me - but this is besides the point and why I made all of these configurable, optional and on by default.

It's completely up to you if you choose to accept this PR or not, without them we are unwilling to install this plugin in our hosted environment. Many of our customers saw your presentation at the moot which is what prompted our review of the plugin.

Thank you for your time

[michael-milette]

Hi @durzo ,

Thank you for your recent comments. I have given serious consideration to everything that has been said so far. As your pull request stands right now, unfortunately your modifications are not in line with the functionality or quality of product I wish to share with Moodlers around the world. You have rejected my proposal to address your concerns regarding free disk space and, as you indicated, your concerns with the {ipaddress} and {sesskey} are based on your feelings, but have not indicated any hard facts to back up such claims.

It is not without appreciation that I close this pull request. My take away from this conversation will be to plan to eventually:

• Update the FAQ to let people know that FilterCodes won't be available with some Moodle Partners and other hosted systems.
• Fix the {diskfreespace}, {diskfreespacedata} tags so that they display the infinity symbol instead of the enormous float number.

The {sesskey} and {ipaddress} tags will remain untouched for now.

It is truly unfortunate that eThink clients be unable to benefit and enjoys using FilterCodes in their websites. I will indicate this as a known issue in the documentation's FAQ. Should you change your mind, please let me know so that I may update the notice.

On a side note, I would like to share a tip with you for your consideration which is inline with your belief that making the sesskey accessible is a security concerns. Should you wish to prevent the session key from appearing in Moodle, you will need to also somehow disable JavaScript. Otherwise, with or without the {sesskey} tag, anyone can simply insert the following JavaScript code in the code of their content to achieve the same results:

Note that any user will still be able to open a console in their web browser and simply enter: M.cfg.sesskey

And, if they don't know how to do that, they can always simply hover over the Logout link in the user menu to reveal the sesskey at the end of the URL.

You will have a lot of work to do to disable all of the different ways someone can access the session key. The {sesskey} tag is just one way to get access to that information.

I trust that you will make a decision which is in the best interest of your customers and your business. Some options for your consideration:

Accept the proposed changes and make this tool available to your clients; OR

You have obviously already forked the plugin and, thanks to the benefits of it being open source. You are welcome to create a customized eThink version of the plugin and make this available to your customers as per the GNU GPL v3 license. You will need to:

• Rename the plugin - It can't be called FilterCodes as the name is a trademark.
• Update all related files and the documentation with the new name.
• Clearly indicate which the version of FilterCodes you are basing it on.
• Keep all copyright notices and attributions intact.
• Add your name/company name and clearly identify modifications.

From that point on, it will be up to you to maintain and update that new plugin.

Thank you again for your proposed contribution. I will remain open to new ideas and pull request from you and eThink in the future.

Best regards (and Happy Thanksgiving - if you are Canadian),

Michael

[durzo]

Thanks Michael, accepted on all points.

Can you please provide the trademark registration details for each country registered so that I may pass them onto legal to ensure we do not infringe on your claims.

Thanks