Improve validation by membership state.

includes/og.membership.inc

@@ -75,7 +75,7 @@ class OgMembership extends Entity {
 
     // We can now safely save the entity.
     parent::save();
-    og_membership_invalidate_cache();
+    og_invalidate_cache();
     // Clear the group content entity field cache.
     cache_clear_all('field:'. $entity_type . ':' . $etid, 'cache_field');
 
@@ -88,7 +88,7 @@ class OgMembership extends Entity {
   public function delete() {
     parent::delete();
 
-    og_membership_invalidate_cache();
+    og_invalidate_cache();
     // Clear the group content entity field cache.
     cache_clear_all('field:'. $this->entity_type . ':' . $this->etid, 'cache_field');
 

og.module

@@ -1222,17 +1222,12 @@ function og_membership_type_access($op, $entity, $account = NULL, $entity_type =
 
 /**
  * Reset static cache related to group membership.
+ *
+ * @deprecated
+ *   Use og_invalidate_cache() instead.
  */
 function og_membership_invalidate_cache() {
-  $caches = array(
-    'og_get_entity_groups',
-    'og_get_membership',
-    'og_get_field_og_membership_properties',
-  );
-
-  foreach ($caches as $cache) {
-    drupal_static_reset($cache);
-  }
+  og_invalidate_cache();
 }
 
 /**
@@ -1605,7 +1600,7 @@ function og_membership_delete($id) {
  */
 function og_membership_delete_multiple($ids = array()) {
   entity_delete_multiple('og_membership', $ids);
-  og_membership_invalidate_cache();
+  og_invalidate_cache();
 }
 
 /**
@@ -1834,14 +1829,15 @@ function og_invalidate_cache($gids = array()) {
     'og_get_user_roles',
     'og_get_permissions',
     'og_get_group_audience_fields',
+    'og_get_entity_groups',
+    'og_get_membership',
+    'og_get_field_og_membership_properties',
+    'og_get_user_roles',
   );
 
   foreach ($caches as $cache) {
     drupal_static_reset($cache);
   }
-  // Invalidate group membersihp cache.
-  og_membership_invalidate_cache();
-
   // Let other OG modules know we invalidate cache.
   module_invoke_all('og_invalidate_cache', $gids);
 }
@@ -2155,13 +2151,18 @@ function og_user_access($group_type, $gid, $string, $account = NULL, $skip_alter
   // To reduce the number of SQL queries, we cache the user's permissions
   // in a static variable.
   if (!isset($perm[$identifier][$account->uid])) {
-    $roles = og_get_user_roles($group_type, $gid, $account->uid);
-    $role_permissions = og_role_permissions($roles);
-
     $perms = array();
-    foreach ($role_permissions as $one_role) {
-      $perms += $one_role;
+
+    if ($roles = og_get_user_roles($group_type, $gid, $account->uid)) {
+      // Member might not have roles if they are blocked.
+      // A pending member is treated as a non-member.
+      $role_permissions = og_role_permissions($roles);
+
+      foreach ($role_permissions as $one_role) {
+        $perms += $one_role;
+      }
     }
+
     $perm[$identifier][$account->uid] = $perms;
   }
 
@@ -2724,48 +2725,66 @@ function og_get_default_roles($include = TRUE) {
  * @param $include
  *   (optional) If TRUE also anonymous or authenticated role ID will be
  *   returned. Defaults to TRUE.
+ * @param $check_active
+ *   (optional) If TRUE, and the user is pending, only anonymous role will be
+ *   returned. If blocked, no role will be returned.
  *
  * @return
  *   Array with the role IDs of the user as the key, and the role name as
  *   the value.
  */
-function og_get_user_roles($group_type, $gid, $uid = NULL, $include = TRUE) {
+function og_get_user_roles($group_type, $gid, $uid = NULL, $include = TRUE, $check_active = TRUE) {
   $roles = &drupal_static(__FUNCTION__, array());
   if (empty($uid)) {
     global $user;
     $uid = $user->uid;
   }
 
+  $account = user_load($uid);
+
   $identifier = implode(':', array($group_type, $gid, $uid, $include));
   if (isset($roles[$identifier])) {
     return $roles[$identifier];
   }
 
+  $is_blocked = og_is_member($group_type, $gid, 'user', $account, array(OG_STATE_BLOCKED));
+
+  if ($check_active && $is_blocked) {
+    $roles[$identifier] = array();
+    return $roles[$identifier];
+  }
+
+  $is_member = og_is_member($group_type, $gid, 'user', $account);
+
+
+  $rids = array();
   $group = entity_load_single($group_type, $gid);
   // Get the bundle of the group.
-  list($id, $vid, $bundle) = entity_extract_ids($group_type, $group);
+  list(,, $bundle) = entity_extract_ids($group_type, $group);
 
   // Check if roles are overriden for the group.
   $query_gid = og_is_group_default_access($group_type, $gid) ? 0 : $gid;
 
-  $query = db_select('og_users_roles', 'ogur');
-  $query->innerJoin('og_role', 'ogr', 'ogur.rid = ogr.rid');
+  if (!$check_active || $is_member) {
+    $query = db_select('og_users_roles', 'ogur');
+    $query->innerJoin('og_role', 'ogr', 'ogur.rid = ogr.rid');
+
+    $rids = $query
+      ->fields('ogur', array('rid'))
+      ->fields('ogr', array('name'))
+      ->condition('ogr.group_type', $group_type, '=')
+      ->condition('ogr.group_bundle', $bundle, '=')
+      ->condition('ogr.gid', $query_gid, '=')
+      ->condition('ogur.uid', $uid, '=')
+      ->condition('ogur.gid', $gid, '=')
+      ->orderBy('rid')
+      ->execute()
+      ->fetchAllkeyed();
+  }
 
-  $rids = $query
-    ->fields('ogur', array('rid'))
-    ->fields('ogr', array('name'))
-    ->condition('ogr.group_type', $group_type, '=')
-    ->condition('ogr.group_bundle', $bundle, '=')
-    ->condition('ogr.gid', $query_gid, '=')
-    ->condition('ogur.uid', $uid, '=')
-    ->condition('ogur.gid', $gid, '=')
-    ->orderBy('rid')
-    ->execute()
-    ->fetchAllkeyed();
 
-  if ($include) {
-    $account = user_load($uid);
-    $role_name = og_is_member($group_type, $gid, 'user', $account) ? OG_AUTHENTICATED_ROLE : OG_ANONYMOUS_ROLE;
+  if ($include && !$is_blocked) {
+    $role_name = $is_member ? OG_AUTHENTICATED_ROLE : OG_ANONYMOUS_ROLE;
 
     $rids = db_select('og_role', 'ogr')
       ->fields('ogr', array('rid', 'name'))

og.test

@@ -680,7 +680,6 @@ class OgPermissionsTestCase extends DrupalWebTestCase {
     $user1 = $this->drupalCreateUser();
 
     // Create an entity.
-    $property = OG_GROUP_FIELD;
     $entity = entity_create('entity_test', array('name' => 'main', 'uid' => $user1->uid));
     $wrapper = entity_metadata_wrapper('entity_test', $entity);
     $wrapper->{OG_GROUP_FIELD}->set(1);
@@ -721,6 +720,54 @@ class OgPermissionsTestCase extends DrupalWebTestCase {
     $this->assertTrue(og_user_access('entity_test', $entity->pid, 'delete own article content', $user2), t('User still has "delete own article content" as they have "administer group" permission.'));
     $this->assertTrue(og_user_access('entity_test', $entity->pid, 'administer group', $user2), t('User has "administer group" permission.'));
   }
+
+  /**
+   * Assert blocked and pending roles influence the allowed permissions.
+   */
+  function testBlockedAndPendingRoles() {
+    // Create user.
+    $user1 = $this->drupalCreateUser();
+
+    // Create an entity.
+    $entity = entity_create('entity_test', array('name' => 'main', 'uid' => $user1->uid));
+    $wrapper = entity_metadata_wrapper('entity_test', $entity);
+    $wrapper->{OG_GROUP_FIELD}->set(1);
+    $wrapper->save();
+
+    // Associate user to the group, and grant "admin" role.
+    $user2 = $this->drupalCreateUser();
+    $values = array('entity_type' => 'user', 'entity' => $user2);
+    og_group('entity_test', $entity->pid, $values);
+    $og_roles = og_roles('entity_test', 'main');
+
+    $rid = array_search(OG_ADMINISTRATOR_ROLE, $og_roles);
+    og_role_grant('entity_test', $entity->pid, $user2->uid, $rid);
+
+    // Active member.
+    $roles = og_get_user_roles('entity_test', $entity->pid, $user2->uid);
+    $expected_result = array(
+      array_search(OG_AUTHENTICATED_ROLE, $og_roles) => OG_AUTHENTICATED_ROLE,
+      array_search(OG_ADMINISTRATOR_ROLE, $og_roles) => OG_ADMINISTRATOR_ROLE,
+    );
+    $this->assertEqual($roles, $expected_result, 'Active member has also the admin role.');
+    $this->assertTrue(og_user_access('entity_test', $entity->pid, 'update group', $user2), 'Active member has access.');
+
+    // Pending member.
+    $values['state'] = OG_STATE_PENDING;
+    og_group('entity_test', $entity->pid, $values);
+    $roles = og_get_user_roles('entity_test', $entity->pid, $user2->uid);
+    $rid = array_search(OG_ANONYMOUS_ROLE, $og_roles);
+    $expected_result = array($rid => OG_ANONYMOUS_ROLE);
+    $this->assertEqual($roles, $expected_result, 'Pending member has non-member role.');
+    $this->assertFalse(og_user_access('entity_test', $entity->pid, 'update group', $user2), 'Pending member has no access.');
+
+    // Blocked member.
+    $values['state'] = OG_STATE_BLOCKED;
+    og_group('entity_test', $entity->pid, $values);
+    $roles = og_get_user_roles('entity_test', $entity->pid, $user2->uid);
+    $this->assertEqual($roles,  array(), 'Blocked member has no roles.');
+    $this->assertFalse(og_user_access('entity_test', $entity->pid, 'update group', $user2), 'Blocked member has no access.');
+  }
 }
 
 class OgDefaultAccessFieldTestCase extends DrupalWebTestCase {
@@ -954,7 +1001,7 @@ class OgMigrate7000TestCase extends UpgradePathTestCase {
       // Pass also pending state, so we make sure that even if the user
       // isn't active they are considered members, to check they get the
       // correct role.
-      $this->assertEqual(array_keys(og_get_user_roles('node', 10, $uid)), array_keys($roles), format_string('User @uid has the correct roles in group.', array('@uid' => $uid)));
+      $this->assertEqual(array_keys(og_get_user_roles('node', 10, $uid, TRUE, FALSE)), array_keys($roles), format_string('User @uid has the correct roles in group.', array('@uid' => $uid)));
     }
   }
 

og_ui/og_ui.test

@@ -261,7 +261,7 @@ class OgUiManagePeopleTestCase extends DrupalWebTestCase {
     $this->drupalPost('group/entity_test/' . $this->entity->pid . '/admin/people/add-user', $edit, t('Add users'));
 
     // Reload user.
-    og_membership_invalidate_cache();
+    og_invalidate_cache();
     $this->assertTrue(og_is_member('entity_test', $this->entity->pid, 'user', $this->user2), 'User was added to the group.');
 
     // Add the same user twice.