Skip to content
This repository has been archived by the owner on Apr 7, 2024. It is now read-only.

Fix uploaded_resources directory permissions #321

Merged
merged 3 commits into from
Feb 18, 2022
Merged

Conversation

clayliddell
Copy link

@clayliddell clayliddell commented Feb 9, 2022

QA Instructions

  1. Spin up local DKAN instance using this branch of dkan-tools.
  2. Create a dataset with a remote CSV file.
  3. Run cron.
  4. Modify the created dataset with a new uploaded CSV file.
  5. Ensure an error is not encountered during the upload process.

Comment on lines +30 to 33
->exec('chmod -R 777 sites/default/files')
// Workaround for https://www.drupal.org/project/drupal/issues/3091285.
->exec('chmod u+w sites/default')
->dir(Util::getProjectDocroot())
Copy link
Member

Choose a reason for hiding this comment

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

@clayliddell aren't these two lines redundant?

Copy link
Author

Choose a reason for hiding this comment

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

No, the second line just affects sites/default since it's non-recursive. The first line is recursive and targets all files and folders within sites/default/files.

Copy link
Member

Choose a reason for hiding this comment

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

I know, I just was thinking they could be combined but I can see how it might cast too wide a net to just 777 the whole default folder. OK will merge after testing

Copy link
Author

Choose a reason for hiding this comment

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

There's a few settings files in the sites/default directory which are read-only for non-owners by default, and I think it would probably be best to leave them that way:

$ ls -al
drwxr-xr-x  3 clayliddell clayliddell  4096 Feb 17 13:48 .
drwxr-xr-x 11 clayliddell clayliddell  4096 Feb 17 13:48 ..
-rw-r--r--  1 clayliddell clayliddell  7565 Feb 17 13:48 default.services.yml
-rw-r--r--  1 clayliddell clayliddell 32852 Feb 17 13:48 default.settings.php
drwxrwxrwx  7 clayliddell clayliddell  4096 Feb 17 13:50 files
-rw-r--r--  1 clayliddell clayliddell   682 Feb 17 13:46 settings.docker.php
-r--r--r--  1 clayliddell clayliddell   403 Feb 17 13:46 settings.php

@dafeder dafeder merged commit 31e5ea7 into master Feb 18, 2022
@dafeder dafeder deleted the fix-uploaded-resources branch February 18, 2022 20:43
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants