New site directory permissions [#1615580]

Hi,

I'm trying to setup a multi-developer aegir installation where access to per-site directories is managed via group permissions. That's all fine in theory, but there is one slight problem: aegir (or Drupal?) sets the permissions of sites/uri to 0755 regardless of what umask I set (and regardless of whether I set it via pam_umask or at the start of /usr/bin/drush).

I've done a bit of grepping, but I can't find the code that sets the permissions on this directory. Can anyone point me at where I should be looking to override this directory permission?

Re-opening as a feature request to make this configurable, rather than a hardcoded code change that needs to be re-applied on each new revision.

Comment	File	Size	Author
#9	1615580-9-aegir3-reroll.patch	5.33 KB	joestewart
#3	1615580-new-site-perms-3.patch	5.29 KB	cafuego

Comments

Comment #1

cafuego commented 4 June 2012 at 06:35

Status:

Active

» Closed (works as designed)

Never mind. More grepping has found me an array of things in _provision_drupal_create_directories() in platform/provision_drupal.drush.inc

Comment #2

cafuego commented 4 June 2012 at 23:17

Title:	Site directory permissions	» New site directory permissions
Version:	6.x-1.9	» 6.x-1.x-dev
Component:	Install process	» Code
Category:	support	» feature
Status:	Closed (works as designed)	» Active

So I resolved my issue by changing the hardcoded permissions in _provision_drupal_create_directories(). However, that's an un-ideal solution as the "fix" will go away at the next aegir update.

I propose we (or I) make the permissions configurable via the included provision/example.drushrc.php file. I'll have a go at a patch later in the week.

Comment #3

cafuego commented 5 June 2012 at 00:13

Status:

Active

» Needs review

Status	File	Size
new	1615580-new-site-perms-3.patch	5.29 KB

Attached patch should in theory do the trick :-)

Comment #4

anarcat commented 5 June 2012 at 01:13

Status:

Needs review

» Needs work

Interesting idea!

If we are going to do this, however, I think we should generalize the concept and allow users to customize not only which permissions to apply, but on which directory too. So we should change the code to be a loop on the options provided so that people could override which directory to set permissions on.

This would, for example, cover the following issues with a more general use case:

* #1397834: Missing support for files/languages folder
* #1569290: Missing support for files/styles folder
* #994210: Add support for some common files/subdirs like css/js/ctools/imagecache

Furthermore:

+++ b/platform/provision_drupal.drush.incundefined
@@ -180,53 +180,80 @@ function _provision_drupal_create_directories($url = NULL, $profile = NULL) {
+  $user_perms = drush_get_option('aegir_site_permissions', array());

why not set the default here instead of above? that would remove the need for an array_merge, and that is what the second argument to drush_get_option() is for, iirc. This is nitpicking however.

Also, keep in mind we are considering merging the provisionacl module in core, and maybe we should look at restructuring the code in consequence.

Comment #5

cafuego commented 5 June 2012 at 03:08

So we should change the code to be a loop on the options provided so that people could override which directory to set permissions on.

The option provides an array of overrides in directory => mode format, so it sort of already does. Mind you, the list of directories that are processed is still the hardcoded list in _provision_drupal_create_directories().

why not set the default here instead of above? that would remove the need for an array_merge, and that is what the second argument to drush_get_option() is for, iirc. This is nitpicking however.

If I didn't use array_merge, the option in drushrc.php would need to explicitly include the permissions for all defined directories. Then provision would use either the user-provided array of permissions or the hard-coded defaults. By using array_merge, the user only needs to specify the values they want to override, not the others as well.

In my case, I would only need to specify

$options['aegir_site_permissions'] = array(
  'site' => 02775,
);

because the defaults on all other directories are fine. So the second argument to drush_get_option() here basically ensures that there are no overrides to apply if the user doesn't specify any.

Comment #6

anarcat commented 5 June 2012 at 18:44

The option provides an array of overrides in directory => mode format, so it sort of already does. Mind you, the list of directories that are processed is still the hardcoded list in _provision_drupal_create_directories().

This is what I mean: I would simplify the code and loop over the array instead of hardcoding each line, would just be cleaner.