Access is allowed to files on old node revisions when using the public file system

Im not sure this is a bug. When you 'delete' a file from an entity, the status column of the file_managed table is updated. Here is the description of that database column...

A field indicating the status of the file. Two status are defined in
core: temporary (0) and permanent (1). Temporary files older than
DRUPAL_MAXIMUM_TEMP_FILE_AGE will be removed during a cron run.

In your case, the file is probably getting marked to be deleted, but wont be deleted until sufficient time has passed and cron is run (after that time has passed). Can you check the status of those files in your database?

Maybe setting DRUPAL_MAXIMUM_TEMP_FILE_AGE as a really small value in your settings.php will be the solution you need.

The file cannot be deleted because it is still in use by an older revision of the node. If you wanted to revert to that older node revision, you need the file to still be available, yes?

If you delete all old revisions that use the old file, then the file should also be deleted.

The problem is that when you save a new revision with the new file, the old file is still on the system to account for the situation where someone wants to revert to the old revision.

The problem here is that the old file is still accessible if the user knows the path to the old file.

There should be an option (if not the default bahaviour) where if the file is not on the current node revision access is denied when downloading that file.

Possibly a change to https://api.drupal.org/api/drupal/modules!node!node.module/function/node... would halp here.

I would be tempted to say though that this is only a problem for private file downloads.

Corrected double post.

I don't think this is an issue with private downloads only. In fact, just the opposite. In private file mode access to the page is checked first and if a user does not have access, file access is blocked as well which seems a desired default. The opposite is true for public file downloads. File access continues to be granted no matter what the access to the page is which seems like a potential security issue (the same is true for unpublishing a page which leaves file access in place).

In our case editors keep the same file name for files, even if the file has been updated, when creating new revisions. However, Drupal now adds _0, _1, _2 etc. every time a new revision is created because the old files are never deleted while we keep the revisions.

I would argue that all files attached to a revision require the same access control that the container revision has, i.e. if the default is that revisions are not visible to anonymous users, then files attached to revisions should not be visible to anonymous users.

A possible solution could be to physically move files when a revision is created and the new revision either has an updated version of an existing file (move the old file first to a different [private] file revision location, then upload new file to file field location as configured) or a file was removed (move the file to different file revision location).

That way the files could be retrieved when reverting to a previous revision. Moreover, access control stays intact if the file revision location is in the private file system part and we avoid all kinds of issues with duplicate file names etc.

Someone will probably argue that we should use private file downloads if we want access control, but that seems like a big performance hit just to properly deal with file revisions.

I am most concerned about usability actually. Consider the case of a PDF form. User creates a page and uploads the form (version 0) with title form.pdf. Two months later a revised form is uploaded with some text changes on the page. Now we have a new content revision (same URL as before, i.e. we swap the content, but not URL), the old file still accessible under .../form.pdf and the updated form available at .../form_0.pdf (assuming we kept the same file name). This is horrible! Any user who had the direct link to my original form will still see it and use it, not even realizing that a change has been made. I would rather see

1. 404 error (if file was moved and we don't have a replacement with same file name)
2. 403 error (if it belongs to a revision that the user does not have access to)
3. Seeing the updated form (which would be possible with the proposed solution of moving the file physically before uploading the new one)

In that sense, I think we need to apply the same concept we apply to content to files: we swap files to keep the URL intact, but always show the file that belongs to the current revision! By swapping files into the private file area, we at the same time solve potential other issues with regard to security, privacy and usability.

This issue is sort of related. It is about files that remain accessible after the revisions to which they were attached are deleted.

The problem mentioned in the issue summary only exists because the public file system is used. Once you upload a file to the public file system, it is served directly by the web server so Drupal does not (and can not) have any way to intervene in the process.

While this may be correct in the current setup, I think the consideration should be if this is desired. It would mean that revisions never function in public file system as most people would expect it to behave and that sites should use private file system setup by default to have proper access control when using revisions.

If we were to treat files like any other content than it would make more sense. If I configure Drupal to keep revisions and only allow certain roles access to those revisions, it is un-intuitive that the content is access controlled, but not the attached files. It is a design flaw with security implications if those documents include data that should not be accessible.

One option would be to rename a file that is attached to a revision only and to move it outside of web access. That way it could be restored to public file system again if a user reverts back to that revision.

All did we find any resolution for this? I have a document content type that has revisions enabled and when the content editor replaces the files in this content type the files are not deleted in the system and available in google search creating confusions. How should we handle stuff like this? It almost feels like we need to keep the files for revisions but the files should not be accessible. Any suggestions?

@neelaj82:

You'll need to switch your site over from using the public file system to the private file system.
Ideally, any files that can should still use the public file system (for example maybe non-informational image files) and other files like documents should be in the private file system.
You can configure this per file field.

Sometimes though it is just easier to have everything in the private file system.

Unfortunately changing an existing site from public to private files is a bit more involved than just pressing a button, but there is some information that might be helpful at https://drupal.stackexchange.com/q/98560/10729 and https://www.drupal.org/node/1806220 and there is a module that claims to handle it (at least in some cases) at https://www.drupal.org/project/file_system_switch

Using private files system with large volume of nodes can produce performances issues.

I mean node_access with around 1000 roles, domain_acccess, taxonomy_access and similar solutions. This mix of permissions on 30k+ revisioned nodes can produce 700k+ node_access records. And it is not a architecture mistake. Connecting few dozens of domains requires selective editor roles.

Serving revisioned files from private:// will produce performance issue on every download of files. In this case moving files from older revision to non public path would be the best solution.

I attached one related issue but for Drupal 8.x:
#2925532: Provide bulk action to delete all old revisions of an entity