Problem/Motivation

I'm trying out the most recent 3.0.0.-beta3 version, and one key change seems to be, that an admin user may not be able to access a group without having an explicit group role configured that in turn would grant access. In v1, it wasn't required to configure access for admin users. Is that a design change, or may I've overlooked something?

Thanks for your answer.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Comments

mxh created an issue. See original summary.

djdevin’s picture

I'm seeing this too and can't get a group created properly on a clean install, 2 or 3. Upgrading from 1 also made all groups inaccessible.

1. Install Drupal 9.4.6
2. Enable group 2.0.0-beta3 or 3.0.0-beta3
3. Create a new group type, default options
4. Create a new group
5. Go to group listing page
6. No groups display

The group is created but inaccessible, even to user 1.

In step 3 you need to select "Automatically configure an administrative role" and "Automatically assign this administrative role to group creators" to get the group to show up. But user 1 should really have full access...

Artusamak’s picture

We might have a related issue with #3312023: GroupQueryAlter status conditions cancel each other out sometimes, not sure though if you are just pointing that the upgrade path is lacking a step or two.

Artusamak’s picture

djdevin’s picture

Component: User interface » Code
Category: Support request » Bug report

This still happens on a clean install.

What am I missing, how are people using this module?

Creating a group with defaults makes it immediately inaccessible, which I think should be failing tests?

Setting this to bug, this can't be expected...

kristiaanvandeneynde’s picture

Component: Code » User interface
Category: Bug report » Support request

This is absolutely, positively, 100% by design. I could give an entire DrupalCon session and then some about why I believe "god mode" switches are bad and what their technical implications are. Having all roles behave the same and not care about UID 1 or a global god mode switch makes the code cleaner, more consistent and easier to extend.

Having said that, all you need to do is configure the following on any group type you set up where you want site admins to be able to administer all groups of said type: Configure an Outsider role that syncs to the Drupal "Administrator" role and tick the "Admin role" checkbox.

If there is a chance that said admins might want to join the groups themselves, add an additional Insider role that does the same. You could name the former's machine name "outside_admin" and the latter "inside_admin" if you want to be able to differentiate in code. If you don't follow this step and a global admin joins the group, they lose their admin access.

kristiaanvandeneynde’s picture

Status: Active » Fixed

P.S.: Factorial will be launching a video series shortly where all of this is explained in more detail.

Artusamak’s picture

I guess that if there should be no god mode, the user #1 should not be able to access this view unless he/she has explicit access via group permissions as any normal person?

mxh’s picture

Thanks @kristiaanvandeneynde, #6 answered the question raised in the IS.

I think the confusion originates from knowing core's default behavior when UID 1 is being asked for permission. Drupal\Core\Session\UserSession is implemented like this:

public function hasPermission($permission) {
    // User #1 has all privileges.
    if ((int) $this->id() === 1) {
      return TRUE;
    }

    return $this->getRoleStorage()->isPermissionInRoles($permission, $this->getRoles());
  }

I can understand that Group doesn't follow that, meaning someone is actually able to restrict access even for UID 1.

kristiaanvandeneynde’s picture

I guess that if there should be no god mode, the user #1 should not be able to access this view unless he/she has explicit access via group permissions as any normal person?

Exactly, and that's what #6 is showing you how to configure it.

Re #9 yeah at one point I just stopped following core on that matter. It leads to all sorts of nasty code that doesn't translate well into cacheability, tests, etc. Group's tests can run fine on UID 1 because we know that account is not an all-access-pass as far as Group is concerned.

Core and other contrib modules, on the other hand, have had a few security issues slip through the cracks because some tests were running as UID 1.

djdevin’s picture

Just for context, I'm migrating from OG in D7, and Group 1.x in D8+, with our typical setup having multiple:

- group-level administrators
- site administrators who can manage all groups of a certain group type
- site administrators who can manage all groups
- super administrators who can manage all groups and group types

at one point I just stopped following core on that matter

I get that breaking the mold is a good thing sometimes. But this is just a bad experience for a first timer using a contrib, even if they've used 1000 other ones before. I gave up on Group 3.x briefly because I figured it wasn't ready yet if user 1 couldn't do anything. Group 3.x has ~500 installs, compared to 10k+ for 1.x, so I'd imagine once that number bumps up there will be more reports.

If we're trying to break the "god mode" tradition then why can user 1 still create the group initially, but not edit/delete it? (Technically, I know why, because one is entity permissions and the next is group permissions.) But that won't make sense to a lot of users. There are probably a lot of sites where there isn't a need for roles and permissions, as the site is managed by only a few people that most likely have user 1 or some administrator role.

In our distro we don't have the "administrator" role (that's only in the standard profile) but we could make the solution in #6 work for user 1 by adding it, then adding the administrator role as an insider/outsider with all permissions to all of our group types.

Still, I can't think of any other module where as an administrator you are locked out by default. Maybe #6 should be the default on a new group type, but in the automated tests you could use a clean group.

I'm also not a fan of outsider administrators losing permissions once inside the group but that's another story. I understand the design but it results in redundant permission sets (if we update the outsider with more permissions, the insider also has to be updated to match).

djdevin’s picture

kristiaanvandeneynde’s picture

I get that breaking the mold is a good thing sometimes. But this is just a bad experience for a first timer using a contrib

I would counter that with the fact that I deem it my responsibility to improve upon core where security is involved. We can't just wait and sit on our hands while #540008: Add a container parameter that can remove the special behavior of UID#1 is going nowhere anytime soon. Secondly, as I mentioned before, there are significant code implications to UID 1's special behavior that would make it impossible to cache some of Group's heaviest permission calculations. Having that one special user would make your entire website dramatically slower.

There are probably a lot of sites where there isn't a need for roles and permissions, as the site is managed by only a few people that most likely have user 1 or some administrator role.

This is the real issue. Let's forego UID 1 and instead focus on adding an extra checkbox on the Group type creation form saying something like: "Do site admins need full access over all groups? If so, please indicate what their Drupal role is". Where we then set up the outsider/insider role for said Drupal role for you. When all is said and done, I think Group has it right, but the usability could be better. We can fix that on the form, not in the code.

I'm also not a fan of outsider administrators losing permissions once inside the group but that's another story. I understand the design but it results in redundant permission sets

There are use cases where this behavior is actually desired. Group's purpose is to try and serve all possible use cases, not just a few. Regarding redundant permission sets: Another issue we can fix on the form, rather than the code. A "copy permissions from" type of behavior would fix this.

We can look into the usability of these things for 2.1.x and 3.1.x. Please open a feature request linking to this issue and close the other issues you found as duplicates of this one. Then after a 2.0.0 and 3.0.0 release we can try and make this more user friendly.

djdevin’s picture

Understood and thanks for the explanation, I'll open some feature requests.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.