How to keep disabled grants in the ACL table

Adding a simple ON/OFF switch freed me from writing another module _just_ for that functionality.

I experimented with other modules, like Private, but in the end, this was by far the most elegant solution.

It's only one more field to the ACL table and a new toggle function or toggle parameter to existing functions.

It won't break any existing installations, and it's a nice addition for developers! :)

Btw, what do you mean by don't hack "contrib"?

Thanks for the link, I'm the "in-house developer" that would maintain the code between security updates and version updates.

Regarding the error,

"It works when the permissions table is rebuilt, but it seems that needs to be rebuilt every time!",

the error does not occur anymore. I think it was happening because I had different ACL dependent modules installed and setting permissions on the same node at the same time. Once I narrowed down to only using ACL by itself, everything worked fine.

So the original problem was, how can my users enable a specific ACL via a toggle. After messing around with using other modules that use ACL, I found that simply adding an extra column to the ACL table and modifying one line of code worked perfectly.

I think you should implement this because it's a great feature. Do you have any reason not to? I'm sure I can rebut them.

I can see that you take the security of this module very seriously, and that's awesome. You're protecting all our *sses by scrutinizing which changes to accept. However, let's be reasonable. This is a simple feature, does not break existing installs, and improves the usability.

Cheers,
Y.

Wow, that's a lot of work! :) Thanks for changing from 'wont fix' => 'active', but you can change it back. I don't have enough time to do 1-6.

The change to ACL is literally two steps:

1. Add a new column to acl table, name "enabled", type BOOL, default value 1

2. Change the following line:

 $result = db_query("SELECT n.*, 'acl' AS realm, n.acl_id AS gid, a.module FROM {acl_node} n INNER JOIN {acl} a ON n.acl_id = a.acl_id WHERE nid = %d", $node->nid);

to

 $result = db_query("SELECT n.*, 'acl' AS realm, n.acl_id AS gid, a.module FROM {acl_node} n INNER JOIN {acl} a ON n.acl_id = a.acl_id WHERE a.enabled = 1 AND nid = %d", $node->nid);

Here's a piece of code that very easily enables the user to make his post public to everyone or private to specific users:

function mymodule_form_FORM_ID_alter(&$form, $form_state) {

 if (module_exists('acl') && isset($node->nid))
    {
        $node = $form['#node'];

        $form['privacy'] = array(
            '#type' => 'fieldset',
            '#tree' => TRUE,
            '#title' => t('Privacy Settings'),
            '#weight' => 10,
            '#collapsible' => TRUE,
            '#collapsed' => FALSE,
            );

        $form['privacy']['enable'] = array(
            '#type' => 'checkbox',
            '#title' => t('Make this private'),
            '#return_value' => 1,
            '#default_value' => db_result(db_query("SELECT COUNT(*) FROM acl a, acl_node n WHERE a.enabled = 1 AND n.nid = %d AND n.acl_id = a.acl_id", $node->nid)) == 1,
            );

        foreach (array('view') as $op) {
            $acl_id = mymodule_access_get_acl_id($node, $op);
            acl_node_add_acl($node->nid, $acl_id, $op == 'view', $op == 'update', $op == 'delete');
            $form['privacy'][$op] = acl_edit_form($acl_id, t('Grant !op access', array('!op' => $op)));
            $form['privacy']['#weight'] = 10;
        }
        $form['#submit'][] = 'mymodule_acl_form_submit';
    }
}

function mymodule_access_get_acl_id($node, $op)
{
  $acl_id = acl_get_id_by_name('YOUR_MODULE_NAME_HERE', $op .'_'. $node->nid);
  if (!$acl_id) {
    // Create one:
    $acl_id = acl_create_new_acl('YOUR_MODULE_NAME_HERE', $op .'_'. $node->nid);
  }
  return $acl_id;
}

function mymodule_acl_form_submit($form, &$form_state)
{
    if (module_exists('acl')) {
        $nid = $form_state['values']['nid'];
        $enable = $form_state['values']['privacy']['enable'];
        $module = 'YOUR_MODULE_NAME_HERE';

        foreach (array('view') as $op) {
            acl_save_form($form_state['values']['privacy'][$op]);
            db_query("UPDATE acl SET enabled = %d WHERE module = '%s' AND acl_id = (SELECT acl_id FROM acl_node WHERE nid = %d)", $enable, $module, $nid);
        }
    }
}

Removing rows is conceptually simpler than adding a column to "disable" rows.

Say I want users to list who can view their content when they mark it private. If I remove the row when they say "make this node private", I'd need to store the data somewhere else, which means complexity for me increases compared to adding an enabled column.

This will make it more difficult to maintain and use, it will cause more bugs in third-party modules and more people in my issues queue who claim that ACL is not working correctly.

By default, all rows would be enabled, so they wouldn't see any changes. If a third-party module uses this feature, then it's up to them to understand what a toggle function does if they choose to use it :)

Also, I'd like to get test coverage for ACL, and this will double the required effort.

You could make the enabled/disabled test < 50% of all test cases, so it shouldn't double. Eg. run all test cases as default (enabled=true), and 20% of test cases with enabled=false.

Anywho, I'm happy with leaving this as a post for anyone who wants this functionality like I did, but didn't know how to get it.

Thanks for the heads up, I just renamed my module to "ACL 6x-1.0 - The 10 Year Anniversary Special" :P

Why would he create such a list and then disable it, making the node public, but still want to keep that list?

If the list is sufficiently long, it would be very nice to enable/disable via checkbox if suddenly want to toggle the item's privacy, instead of always reentering the list from scratch.