Improve how kalatheme detects if modules and libararies paths are writeable

[soniktrooth]

While looking into #123 I came across kalatheme_has_write_access() and kalatheme_lib_dir()

These functions seem a little fuzzy to me and I have questions for @pirog or anyone who knows better.

function kalatheme_has_write_access() {
  return is_writable(drupal_realpath(kalatheme_lib_dir())) && is_writable(drupal_realpath('sites/all/modules'));
}

Why do we ever need to check if the modules dir is writable? What are we writing to the modules dir and why? My understanding was that if your module's dir is writable you got some serious security problems...

function kalatheme_lib_dir() {
  return is_dir('sites/all/libraries') ? 'sites/all/libraries' : 'sites/all';
}

Seems a little hard coded and sledge hammery to me. Panopoly includes the libraries module which can tell us where the libraries dir is. At worst it's probably a site var anyway. Is there a reason we're not doing that?

[andrewmallis]

the subtheme should write to sites/all/themes generally – that said we assume.
$theme_path can return the admin theme, so we might want to use
drupal_get_path('theme', 'kalatheme');

but I am guessing that https://api.drupal.org/api/drupal/includes%21file.inc/function/drupal_realpath/7
may have been used in this instance in order to resolve symlinks, but I do not recall and would defer to pretty much anyone

[soniktrooth]

drupal_get_path() returns a path relative to drupal root, whereas drupal_realpath() returns an absolute path, which I'm assuming is what is_writable() needs.

Regardless of that, my query is more about why we're checking the modules dir instead of the themes dir which is where—as you said—the subtheme should be written to.

[RobLoach]

Writing to a subtheme directory seems strange too. The Libraries module has libraries_get_path(), which can be a nice wrapper around the path/library discovery. We could transition from having our custom implementation to using something like that.

[RobLoach]

Looks like part of that is to update KalaTheme itself, which seems like it would be something for either Drush, or Update module to take on. Any reason for this to be there at all?

[soniktrooth]

@RobLoach KT has to be able to write to the themes dir in order to generate a sub theme. Are you suggesting this is insecure as well? I haven't really explored the drush integration in KT but the functionality to generate a sub theme exists in the config. I don't have experience with the implementing the update module—are we able to get around any security implications by using it?

[andrewmallis]

I too am confused why we are checking sites/all/modules vs sites/all/themes. But again, we should instead check the location of kalatheme, because multisites would have a pattern instead like sites/mysite.com/themes/ or who knows sites/mysite.com/themes/contrib/ (UCSF stack does this).

I have updated the issue description to better reflect our goals.

To address some other notions raised here:

@soniktrooth is correct we cannot use the file_uri_scheme() with is-writable() so yes, drupal's implementation of php's realpath() function is necessary. More insight here: https://api.drupal.org/api/drupal/includes%21file.inc/function/file_prepare_directory/7

Drush subtheme generation is already possible:
https://github.com/drupalprojects/kalatheme/wiki/Installation-with-Drush
but the usecase the functions you question are meant primarily to allow subtheme generation via the UI for non-technical users.

We already require Libraries
https://github.com/drupalprojects/kalatheme/wiki/Setup-and-Installation
so it could be wise as @RobLoach suggests to implement libraries_get_path()

It is pretty amazing that within a single theme so much is possible without other helper modules or themes. Zen had https://www.drupal.org/project/zenophile and Omega https://www.drupal.org/project/omega_tools but recent versions of these themes now only support Drush to generate subthemes.

The subtheme UI generator is a pretty big differentiator, and something we should retain, even if the implementation needs to be a bit hacky in the end.