avoid fatal error when group argument to GroupAccessResult:: allowedIfHasGroupPermissions() is NULL

+++ b/src/Plugin/GroupContentAccessControlHandler.php
@@ -142,9 +142,11 @@ class GroupContentAccessControlHandler extends GroupContentHandlerBase implement
-      $result = GroupAccessResult::allowedIfHasGroupPermissions($group_content->getGroup(), $account, $permissions, 'OR');
-      if ($result->isAllowed()) {
-        break;
+      if ($group = $group_content->getGroup()) {
+        $result = GroupAccessResult::allowedIfHasGroupPermissions($group, $account, $permissions, 'OR');
+        if ($result->isAllowed()) {

This needed additional work because moving the $result assignment inside a conditional can cause the variable to never be created as an AccessResult object, which is expected later on in the code.

I created a MR for the 3.2.x version. I do am worried by skipping an access check here, we may open a security hole.
As an admin user, I did check if I could access group content that no longer belongs to a group, and that's not the case (view or edit gives access denied on them).

I moving this to 3.2.x in an attempt to let tests run on the MR.

The merge request patch fixes the issue and applies cleanly to group version 3.2.2.

I am running into this just upgrading from 3.2 to 3.3

I could never accept a patch that adds if ($group = $group_relationship->getGroup()) { to the code base. Group relationships coming from the DB must have a group and target entity. If they don't, your site is seriously broken.

@kristiaanvandeneynde
This error occurs when the group is deleted, but the group content still exists. Perhaps it would be better to deny access to the content if the group can no longer be found?

An other possibility is to throw an exception in GroupRelationship::getGroup() in case the group entity could not be loaded. That would be more descriptive than the TypeError that we get now.

Something like this:

/**
 * {@inheritdoc}
 */
public function getGroup(): GroupInterface {
  $group = $this->get('gid')->entity;
  if (!$group instanceof GroupInterface) {
    throw new \RuntimeException(sprintf('Group %d could not be loaded.', $this->get('gid')));
  }
  return $group;
}

This error occurs when the group is deleted, but the group content still exists.

But that should never happen. See Group::preDelete().

Something is wrong with your website if deleting a group leaves orphaned relationships. It could be a bug in Group, but judging by the method I just mentioned above, it probably isn't.

I'm going to err on the side of this being a local issue. Without steps to reproduce on a clean install there is nothing I can do here. Please feel free to reopen if you have steps to reproduce.

I see: group relationships should get cleaned up upon group removal. Perhaps it is caused by a local issue then. Or maybe I accidentally patched the wrong file? Because I see that my MR is different from the patch in #3.
Good call on requesting the steps to reproduce the issue first.

Experiencing the same that the database table still contains group relationship items while the according group does not exist anymore.

We're having lots of concurrent requests going on, and one thing I could imagine is that the deletion process may take some time, especially when having many group relationship items. And within that time frame another request might have just added a new relationship to the group that is about to be deleted. But this is just a first guess on finding out what could have gone wrong.