Make features assign their own permissions

I had in mind something like this being called from each module's hook_enable():

/**
 * Add a module's permissions to the appropriate roles.
 *
 * @param $module
 *   The name of the module whose permissions we're adding.
 * @param $role_perms
 *   An array of arrays keyed by the role name. The values are the permissions
 *   to enable.
 */
function hosting_add_permissions($module, $role_perms) {
  // Get the permissions defined by the module.
  $perms = call_user_func($module . '_perm');
  // Check whether any of the module's perms are assigned to any role.
  foreach ($perms as $perm) {
    if (user_roles(FALSE, $perm) {
      // Don't proceed, as the feature has previously been enabled, and per-
      // missions may have been customized.
      return;
    }
  }

  // Get all defined roles.
  $roles = user_roles();
  // Get current permissions.
  $current = db_fetch_array(db_query('SELECT rid, perm FROM {permission}'));
  foreach ($role_perms as $role => $perms) {
    $rid = array_search($role, $roles);
    // Merge new permissions into current ones.
    $new_perms = array_merge($perms, explode($current[$rid]));
    // Save permissions for this role.
    db_query("INSERT INTO {permission} (rid, perm) VALUES (%d, '%s')", $rid, implode(', ', $new_perms));
    drupal_set_message(t('Assigned the <em>!role</em> role the following permissions: ' . implode(', ', $perms) . '.', array('!role' => $role)));
  }

  // Clear cached pages.
  cache_clear_all();

}

// EDIT: Note that I haven't tested this yet, so it's probably buggy in any number of ways.

Yes, something like that.

Since 'aegir administrator' should get all Hosting permissions, we could skip listing them individually. In hosting_add_permissions(), we could just add $perm to that role, since it'll already contain all the module's permissions.

here is a patch against Hostmaster to remove the creation of the "administrator" role as this is provided by drupal core in D7,

Are you sure about that? This seems to indicate that it's still done in the install profile: http://drupalcode.org/project/drupal.git/blob/refs/heads/7.x:/profiles/s...

Rather than requiring a hook_enable() to do this, maybe we can just add a 'role_perms' array in hook_hosting_feature(). That would centralize the data, and allow us to optionally display roles/perms on the features page.

We should probably also implement hosting_modules_enabled(), to ensure that any hosting features specific code runs when the module is enabled from admin/modules or via drush.

Finally, I'd added a bit of code in #1 to try and guess whether the permissions had been modified previously. I suspect we'd be better off not doing that, and letting the feature just set the permissions regardless.

I added support for a 'role_perms' array in hook_hosting_feature(), as suggested in #10.

So far, I only added the permissions from the hosting module to its feature. So the rest of the extraction from the profile remains. Also, since we now have the option to display the roles and permissions assigned per feature on the hosting features page, I didn't list out the permissions added in our confirmation message. I'd like feedback on whether that is desired. It would potentially be a lot of info, since there are multiple permissions being assigned across several roles. However, since it has security implications, I'm leaning towards putting it in.

I moved the rest of the permissions from the hostmaster profile to the individual modules, implemented hook_modules_enabled() and list the permissions added by enabling a feature. This should close out this feature request.

Hmm, this has broken hostmaster-install, so definitely needs work. Fixing the dependencies listed in hosting.info is a good start.

Part of the problem was missing dependencies between a number of the hosting modules. But then after that, hosting will try to add a verify task for @hostmaster whenever it enables a feature. but since the profile hasn't created the site node yet, it fails with db errors. I'll just skip that bit for any required modules.

That worked.