When running a mass hosting environment, it is often required to shield users from the ability to break the sites hosted on your system and there are very often also good security reasons to remove the ability to configure certain variables.

This patch does two things :

1) Introduces an optional sites/global.php file, that will be loaded after the sites/default/settings.php file and provide a set of global settings for all sites
hosted on a server. This means the administrator only needs to configure the variables once.

2) Modifies variable_get and variable_set to check optional 'variable_default($name)' and 'variable_override($name)' functions, that an administrator can define in the above global.php file.

There already exists the ability to set variables using $conf[varname] in your settings.php file, but there is no distinction made against variables that have been set this way, and normal variables, apart from the fact that trying to change them in the admin section fails silently.

A good example of this would be the need to set up the files dir, and not allow it to be modified, as a user modifying the files dir to another site's files directory could be catastrophic.

  function variable_override($name) {
    $conf['file_directory_path'] = conf_init() . '/files';
    $conf['file_directory_temp'] = conf_init() . '/files/tmp';
    $conf['file_downloads'] = 2;
    return $conf[$name];
  }

These values will now be the only accepted and used values to the variables. If the user tries to modify them, they will revert to these values and
there will be a message stating that the variable has been locked.

Another common requirement is to provide sensible defaults for an install, but not take away the configurability of the option. A common example of this would be the theme setting.

  function variable_default($name) {
    $conf['theme_default'] = 'chameleon';
     return $conf[$name];
  }

This will change the default that variable_get falls back on, from the one provided in the parameter list, to the administrator configured
default. This is a much cleaner method of setting the defaults, than global search replacing the defaults provided in the code.

This patch is one of the changes we have made to the drupal code base for our hosting platform @ bryght.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

killes@www.drop.org’s picture

While I like the idea as such, I'd like to see that the form fields that are affected by such global variables should be disabled. Otherwise we'll have a lot of confused users/ support requests.

adrian’s picture

I agree with you .. unfortunately i don't think it's currently possible. It might be closer to possible with the new forms patch, but I don't think that's a suitable reason for this patch not to go in.

I use form_set_error to display the locked variable message, so it already shows which form items have been disabled, but only after you try to change them.

killes@www.drop.org’s picture

Heh, ok, let's push this patch in core. But I suggest you upload it again, I can't look at it for some reason.

adrian’s picture

FileSize
2.25 KB

uploaded again

Jose Reyero’s picture

I think the idea it's interesting, but the idea would be really useful the other way: having a global configuration that can be overridden by specific site settings.

From a security standpoing you've lost the battle from the beginning, since the admin of a site can create a PHP page and add some db_query code.

And I dont like too much having functions in the config file. Maybe better having $conf, $default, $fixed arrays in that files, and then the variable code handling that.

So +1 to having that global.php settings file, but should be before settings php, and -1 to the rest of the patch.

Patch bingo :-)

adrian’s picture

I need to update the patch, once I have the form stuff done .. since I am planning to filter out the overriden elements on settings pages, meaning they can't be accessed from the interface.

Also, the general idea has been changed to having "$default", and "$override" global variables.

Dries has vetoed the global configuration file though, saying we should just add it to our config file when neccesary.

adrian’s picture

Status: Needs review » Needs work

changing status applicably

moshe weitzman’s picture

adrian has submitted a DEP for review by the devel list. he says that code is forthcoming

adrian’s picture

Here is the latest version of the original patch, for the cvs version.

This is ever so important now that we have install profiles, in that install profiles should be able to specify default values.

By only setting variables using variable_set in the install profiles, when the user hits reset to defaults, it will reset to drupal defaults, and not the defaults as specified by the install profile.

It is also a _lot_ simpler to have an include file shipping with the install profile with

$default['var'] = 'value';

moshe weitzman’s picture

looks nice and simple:
- adjust the comment at bottom of settings.php
- change + if ($value = $GLOBALS['override'][$name]) so that we can default and override items to FALSE

after that, RTBC IMO

Chris Johnson’s picture

Agreed with Moshe. With those 2 small changes, I think this is a good patch and will help out the many hosts with multiple sites (and distributions -- let's not call them install profiles, please).

adrian’s picture

Here's some other helpful code (which can be used inside a php page, and should probably form part of the install system building script)


global $conf;

foreach ($conf as $key => $value) {
  ob_start();
  var_export($value);
  $value = ob_get_contents();
  ob_end_clean();
  $string .= "\$default['$key'] = " . $value . ";<br />";
}
return $string;

This prints a list of the variables set in the site, in a format which can just be copied and pasted into the profiles/default/default.inc (or whatever)
file. And then just remove the things you don't want to default.

What I would like to see happen is http://drupal.org/node/77549 be committed first, then I can update this patch to also have the

include_once "profiles/$profile/$profile.inc";

In it, and it would just work.

adrian’s picture

Title: Administrator configurable defaults and overriding variables. » Administrator configurable defaults to variables.
Status: Needs work » Needs review
FileSize
2.75 KB

I have fixed the following issues with this patch :

1) added some help text to settings.php , explaining the use of the defaults.
2) load profiles/$install_profile/$install_profile.inc when found.
3) fixed the use of $GLOBALS[].
4) removed the $override functionality.

The functionality of $override is already available through $conf, and the only difference really is
that having a separate array lets us know which variables have been overridden. The correct way
to handle this would be to extend forms api to know which fields are mapped to variables in the
variables table, and then disable them when the variables have been overridden.

I have removed that part of the patch to simplify the issue, to try and get this in more easily,
as this is really important to having install profiles work the way we need them to.

adrian’s picture

FileSize
2.96 KB

didn't use isset, and forgot to change the parameter name (since default is a global now)

Chris Johnson’s picture

Does $default need to be declared global in conf_init()? It does not look like it is getting used anywhere in that function.

adrian’s picture

All those variables are declared default, so changes to them in the scope of that function take effect for the whole system.

If you remove the global definition, defining variables will only be local to the scope of conf_init(), and you would have to define defaults in the form
$GLOBALS['defaults'] , which is not desireable.

adrian’s picture

errr
all those variables are declared global ..

sorry. no coffee yet.

Chris Johnson’s picture

Ah, I think I've got it. Now it makes sense. Patch looks good to me.

adrian’s picture

Status: Needs review » Reviewed & tested by the community
Dries’s picture

Status: Reviewed & tested by the community » Needs work

Documentation needs work. In settings.php, it is not clear how $conf differs from $default. The language used in settings.php is too technical (it talks about the variables table etc).

The per-profile settings are not documented. Not in conf_init() and not in variable_get().

I don't like the fact that there is a $conf and a $default. Can't we consider merging those, and maybe use an extra attribute to specify whether they can be overwritten? I think we need to think about the code some more.

Jose Reyero’s picture

I like the idea of this patch but would like to make it a bit more extendable. It could go like this:

- We use global array $variable to hold current variable values

- We use $conf array for values setting in the configuration files, so we could have

$conf['global'] = array(.. global variables...); // cannot be overridden

$conf['default'] = array(... default variables...); // can be overridden

These two just need to be merged into $variables in the right order, and then we just need some fall-back mechanism in variable_get, variable_set to check whether we can override a variable or not

And then we could build on that to have multilingual -translattable- variables and so on... :-)

lilou’s picture

Version: x.y.z » 7.x-dev
adrian’s picture

Status: Needs work » Closed (duplicate)