Problem/Motivation

Assigning outsider/insider roles to a member is not possible via the UI, but we do not prevent it via code. This can lead to unstable websites where our access policies choke on the unexpected presence of these roles.

We fixed one part of the ordeal here: #3352235: An already assigned individual role can be made out/insider, leading to a crash in the permission calculation
This issue is about fixing the other part.

Steps to reproduce

Assign an outsider role via code, see your site crash.

Proposed resolution

Do not allow the above.

Issue fork group-3480110

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

kristiaanvandeneynde created an issue. See original summary.

kristiaanvandeneynde’s picture

kristiaanvandeneynde
Primary language Dutch
Location Antwerp, Belgium
commented

Need to update the footer of this CR once this is fixed: https://www.drupal.org/node/3480111

kristiaanvandeneynde opened merge request !185

kristiaanvandeneynde’s picture

kristiaanvandeneynde
Primary language Dutch
Location Antwerp, Belgium
commented

I'm hoping tests will go green now. Need to add a CR about memberships now validating role scope. The fact that the other two checks got moved to the same constraint doesn't need a CR because we were already throwing an exception from those checks.

kristiaanvandeneynde’s picture

kristiaanvandeneynde
Primary language Dutch
Location Antwerp, Belgium
commented
Status: Active » Fixed

Status: Fixed » Closed (fixed)

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

mvonfrie’s picture

mvonfrie commented

I stumbled upon this issue when executing a migration from a Drupal 7 site with organic groups which I implemented several months ago and the new site is almost ready for production now. I changed my plugin to filter out all non-individual roles from the mapping and that works. But I observe a strange behavior related to this and would like to ask if that is intended or not.

The Drupal 7 site has an og level "Author" role, which now is a global "Author" role with outsider and insider mappings for the group type. Users, organic groups and almost all contents get migrated from Drupal 7, except migrating the og-level roles to corresponding global roles. The admins have to do this manually. After running the migration I have the following in Drupal 10:

  • User A created
  • Group B created, Insider "Author" role bound to the global "Author" role is defined.
  • Membership of user A in group B created

Now when I assign the global "Author" role to user A, I nowhere see that it also got the "Author (insider)" role in group B implicitely assigned. Neither in the group members view, nor as read-only information in the group membership edit form. How can I check whether the outsider/insider scoped group roles actually are assigned?

kristiaanvandeneynde’s picture

kristiaanvandeneynde
Primary language Dutch
Location Antwerp, Belgium
commented

How can I check whether the outsider/insider scoped group roles actually are assigned?

They aren't assigned, their permissions are by virtue of an access policy: SynchronizedGroupPermissionCalculator

mvonfrie’s picture

mvonfrie commented

Thanks, that seems to work. At least when debugging the SynchronizedGroupPermissionCalculator the calculated permissions look correct.

But still, the information about the implicit (outsider, insider, inherited) roles is missing. I will create a follow-up issue as feature request.

kingdutch’s picture

kingdutch
he/him
Primary language English
commented

There's a slight "API change" this causes that may trip some people up (which I don't think is worth fixing, but I want to share it because it may save people some debugging time).

In previous versions of group it was 'valid' to call $group->addMember($user, ['group_roles' => '']) where group_roles was an empty string. Because of how Drupal treated fields this would just be seen as an empty value and otherwise ignored in downstream code.

Now that the validation exists, the group module will try to load a role with an empty ID which can never exist, thus throwing a validation error. This means that your code should check that you're actually assigning a role and that you didn't receive empty user input. If you're not assigning a role you should unset the group_roles entry of the membership data.

(This scenario occurred for us in test code where scaffold data was being parsed from markdown tables into arrays. Empty fields in this system are interpreted as empty strings.)

kristiaanvandeneynde’s picture

kristiaanvandeneynde
Primary language Dutch
Location Antwerp, Belgium
commented

Good catch, but I agree that this is not worth fixing. I'm sure if you pass it an empty array, the code will work fine. So it's really down to the question: Why are you passing a string where an array is expected? :)

arno_vgh’s picture

arno_vgh
Primary language Dutch
Location Antwerp
commented

I’ve updated the change record at https://www.drupal.org/node/3480111, as this change was also introduced in the 2.3.x branch.