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

File attachments are publicly accessible by default - if url is known #263

Closed
symbioquine opened this issue Mar 18, 2020 · 8 comments
Closed

Comments

@symbioquine
Copy link
Collaborator

RE: https://farmos.discourse.group/t/file-attachements-public-by-default-if-so-whats-the-best-strategy-for-making-them-private

2020_03_18_uploads_default_public_permissions_issue

Installation created according to: https://farmos.org/development/configure-local-https-reverse-proxy/

image

@mstenta
Copy link
Member

mstenta commented Mar 19, 2020

Thanks for opening this @symbioquine.

Just want to link to my comment here specifically for background: https://farmos.discourse.group/t/file-attachements-public-by-default-if-so-whats-the-best-strategy-for-making-them-private/268/2

And also I will change the title of this to reflect that it is not just logs.

@mstenta mstenta changed the title Log attachments are publicly accessible by default - if url is known File attachments are publicly accessible by default - if url is known Mar 19, 2020
@mstenta
Copy link
Member

mstenta commented Mar 19, 2020

First, I want to emphasize: We are already planning on defaulting to private filesystem uploads in the next version of farmOS on Drupal 8.

So, this is mainly a question/consideration for Drupal 7 farmOS...

There are a few considerations if we want to change this in farmOS on Drupal 7:

  • Drupal has a system for "Private" uploads, which basically means that requests for private files are handled by Drupal itself, rather than the web server (Apache). When Drupal handles the request, it can add an additional layer of access control based on Drupal roles and permissions. That is ultimately the best solution.
  • Drupal's private file handling takes a bit more to configure, which may be harder on some hosting platforms.
  • There are two main places this is configured:
    • There is a site-wide Drupal setting for whether or not files uploads are stored in "Public" or "Private" filesystems. Drupal uses Public by default on new installations.
    • The other place is in the "Field base config" for the actual "File" and "Image" fields that are attached to entity types (eg: on assets, logs, plans, taxonomy terms, etc). The nice thing is we have two base fields that are shared across most record types, so changing those would change all existing instances of those fields:
  • Changing the above settings may work fine for NEW installations of farmOS, but I am not sure how they would affect EXISTING installations. First, because it is the "base field" config that would change, I'm not sure if that is something that can be changed on existing installs or not. Second, because the "Private" and "Public" file systems are in different directories, I'm not sure if that would require automating the move of files from one directory to the other. Or, maybe Drupal is smart enough that it will leave the existing files in Public and put new files in Private. Still: that means we need to decide if we should migrate the old ones.

@mstenta
Copy link
Member

mstenta commented Mar 19, 2020

We are already planning on defaulting to private filesystem uploads in the next version of farmOS on Drupal 8.

This also assumes that there will be an automated migration from D7->D8 that moves all the files to private.

@symbioquine
Copy link
Collaborator Author

Thanks @mstenta, this is really helpful!

So if I'm understanding correctly, fixing this on a one-off basis would require configuring Drupal through the UI so it "knows" how/where to save private uploads, then modifying the code for the farm_fields module to allow for the uploads to be private?

Further if I'm understanding correctly, changing this behavior in the farmOS D7 codebase would require;

  • Find out whether changing the uri_scheme of the field_farm_files and field_farm_images fields to "private" would only affect new uploads or whether it would affect the integrity of the references from existing farm records to any uploaded attachments they may already have
    • If changing the default would break existing attachments, figure out how to avoid that or provide tooling to fix it
  • Find out or decide whether anyone wants or should have public attachments in the future
    • If so, figure out how to make the behavior configurable - either at a system level or at a per-upload level

It occurs to me that one sort of ugly work-around to both problems might be to just add an additional pair of fields. e.g. field_farm_private_files and field_farm_private_images I believe this would guarantee that existing records aren't broken and make it trivial for the UI to allow for either behavior at a system or record level - the trade-off of course being that it would add potentially avoidable complexity to the data model.

@mstenta
Copy link
Member

mstenta commented Mar 21, 2020

So if I'm understanding correctly, fixing this on a one-off basis would require configuring Drupal through the UI so it "knows" how/where to save private uploads, then modifying the code for the farm_fields module to allow for the uploads to be private?

That's correct, as far as I know.

Find out whether changing the uri_scheme of the field_farm_files and field_farm_images fields to "private" would only affect new uploads or whether it would affect the integrity of the references from existing farm records to any uploaded attachments they may already have

Yes, and also: is it even possible to change the schema in the first place through the UI? Some field configuration cannot be changed once a field is created. Anything is possible via code, of course, but that might require more work. So first check to see if it can be changed in the UI (which would then override the config in code).

Find out or decide whether anyone wants or should have public attachments in the future

I would say it's safe to assume private files is always a good starting point. The nice thing about the private file system approach is it will then use Drupal's roles and permissions system, and probably has an access function we could hook into, which means we could also create our own ability to make certain files "public" in the future.

It occurs to me that one sort of ugly work-around to both problems might be to just add an additional pair of fields. ... he trade-off of course being that it would add potentially avoidable complexity to the data model.

Agreed - I'd rather keep the data model simple. And as described above, we may be able to make them "public" on a per-file basis using the access control system, which would be all around better. But we can cross that bridge if/when needed, and as part of a separate feature request.

@mstenta
Copy link
Member

mstenta commented May 19, 2020

Update: I've done some testing with this...

I added the following hook implementation to a custom module, which overrides the base field settings for field_farm_files and field_farm_photos to use the private file system:

/**
 * Implements hook_field_default_field_bases_alter().
 */
function mymodule_field_default_field_bases_alter(&$fields) {

  // Use the private filesystem for file and image fields.
  $file_fields = array(
    'field_farm_files',
    'field_farm_images',
  );
  foreach ($file_fields as $name) {
    if (!empty($fields[$name]['settings']['uri_scheme'])) {
      $fields[$name]['settings']['uri_scheme'] = 'private';
    }
  }
}

Note this does NOT change the actual field base configuration itself that is defined in the farm_fields module in farmOS core, NOR does it actually update the field configuration in the field_config database table. Nevertheless, it appears to override the configuration properly:

Screenshot from 2020-05-19 08-06-40

Once that is done, when a file/photo is uploaded to one of those fields, it is put into the private file system path that is configured at /admin/config/media/file-system.

And: files that were uploaded BEFORE this override continue to work fine, it seems. However, they are NOT automatically moved to the private file directory.

So: this is a quick way to override the setting in a custom module, which will ensure that all files uploaded from that point forward will be private. I am going to investigate what is required for moving old files to the private file system next, but my assumption is that it simply involves:

  • Physically moving the files.
  • Updating the {file_managed} database table to replace public:// with private:// in the uri column of all records.
  • Deleting the old files/styles directory to remove derivative images (thumbnails etc) from public view.

The next things to figure out after that are:

  • Do we want to make "private" the default for all new farmOS installations? I say "yes", but I know that setting up the private files directory is an extra step, so we will need to:
    a. Add instructions somewhere for how to do that.
    b. Test how uploads behave BEFORE that directory is set up. I think Drupal is smart enough to show a message if it detects that it's not possible to upload to the private directory. But this should be confirmed.
  • Do we want to automate the move of old files to the private system on existing installations? Or leave it to the site admin to do that? It could be a risky thing to automate - we absolutely don't want to be the cause of file/data loss. So maybe we adopt a policy of "files uploaded after this update will be private. previously uploaded files will not, unless you follow these steps..."

@mstenta
Copy link
Member

mstenta commented Jun 3, 2020

Here is an update hook sketch that can be used to migrate all files from public to private:

/**
 * Move to private file system for file and image field uploads.
 */
function hook_update_N(&$sandbox) {

  // Define the old (public) and new (private) file paths.
  global $conf;
  $public_path = $conf['file_public_path'] . '/farm';
  $private_path = $conf['file_private_path'] . '/farm';

  // If the public farm files directory does not exist, bail.
  if (!is_dir($public_path)) {
    return;
  }

  // Check that the private farm files directory does not already exist.
  if (is_dir($private_path)) {
    throw new DrupalUpdateException('Private farmOS files already exist. Files were not moved from public.');
  }

  // Move files.
  if (!rename($public_path, $private_path)) {
    throw new DrupalUpdateException('Failed moving farmOS files from private to public.');
  }

  // Flush image styles.
  $styles = image_styles();
  foreach ($styles as $style) {
    image_style_flush($style);
  }

  // Update {file_managed} table.
  db_query("UPDATE {file_managed} SET uri = REPLACE(uri, 'public://', 'private://') WHERE uri LIKE 'public://farm%'");
}

Note that this will only work if no private files have been uploaded already (it checks that private://farm does not exist first). Also note that it only moves files that are in the public://farm directory. So if there are any files uploaded that ended up outside of that directory, they will not be included in this migration.

@mstenta
Copy link
Member

mstenta commented May 25, 2021

I'm going to close this. We've made "private" the default in farmOS 2.x, and instructions/considerations for using private filesystem in 1.x are described above.

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

No branches or pull requests

2 participants