[PATCH] make it possible to enable modules through config

[GoogleCodeExporter]

In order to enable a module in simpleSAMLphp it would be nice if there were 
alternative ways to enable a module. This is needed in the case the software is 
packaged. It is not appropriate to modify files inside the /usr/share structure 
for configuration purposes.

Patch against trunk to make this possible.

Original issue reported on code.google.com by mooknarf@gmail.com on 12 Feb 2012 at 12:48

Attachments:

• simplesamlphp_enable_modules_through_config_option.diff

[GoogleCodeExporter]

Unfortunately this leaves us with two ways to enable modules, that I would 
prefer to avoid, since it will confuse people. I certainly see your point wrt. 
pacakging, but if we make this change, it should be be the default way to 
enable modules.

I'd therefore leave this change as a change we make for version 2.0 at some 
point in the future.

Original comment by olavmrk@gmail.com on 13 Feb 2012 at 7:35

[GoogleCodeExporter]

We also have need for module enabling/disabling from configuration, and here is 
proposed patch.

Original comment by comel...@gmail.com on 16 Oct 2012 at 8:49

Attachments:

• enable-module-array.patch

[GoogleCodeExporter]

Olav, could you please take a look at this? I know that situation with two 
methods for enabling/disabling modules is not ideal, but IMO it's good enough 
for the meantime to the new method. If it happens that configuration method 
some day becomes the default and only method, then at first some warning could 
be logged, and in some version (2.0?) exception could be thrown. I'm willing to 
make any necessary changes.

Original comment by comel...@gmail.com on 5 Nov 2012 at 3:48

• Changed state: NeedsInfo

[GoogleCodeExporter]

Sorry for the late response to this issue.

After thinking about this a bit, and discussing it internally, I think I can 
accept enabling/disabling modules through configuration file. However, I'd like 
to see a simpler syntax:

    'module.enable' => array(
        // Setting to TRUE enables.
        'exampleauth' => TRUE,
        // Setting to FALSE disables.
        'saml' => FALSE,
        // Unset or NULL uses default.
        'core' => NULL,
     ),

The default enable/disable settings can then come from their current location.

Basically, the check would be something like:
1. Check for set in configuration file. If set to TRUE or FALSE, use that.
2. Check for enable/disable file in module directory. If present, use that.
3. Use default-enable/default-disable from module directory.

Original comment by olavmrk@gmail.com on 16 Nov 2012 at 11:11

[GoogleCodeExporter]

The new proposed patch.

Original comment by comel...@gmail.com on 9 Jan 2013 at 8:47

Attachments:

• module-enable-1.patch

[GoogleCodeExporter]

This patch looks good. I only have a couple of nitpicks:

- If you use if isset() instead of array_key_exists(), you will automatically 
ignore the entries set to NULL. The code inside the if-check then becomes 
simpler.

- I'd rather throw an exception than log an error when the value isn't a 
boolean. That ensures that people actually see the error.

Original comment by olavmrk@gmail.com on 9 Jan 2013 at 9:36

[GoogleCodeExporter]

Updated and committed as r3218.

Original comment by comel...@gmail.com on 9 Jan 2013 at 11:24

• Changed state: Fixed