Support from Acquia helps fund testing for Drupal Acquia logo

Comments

geek-merlin’s picture

Status: Active » Needs review
FileSize
1.15 KB

Here's a patch that opens the door to fixing this *without* breaking legacy installations:
* we already have a variable 'og_group_manager_full_access'
* this patch implements the same for group admin as 'og_group_admin_full_access'

amitaibu’s picture

If a user has global "administer group" permission, og_user_access also gives her all content permissions that exist in a group.

But that's exactly the point of that permission, so I'm not clear about the issue.

geek-merlin’s picture

there are 2 levels about this that should NOT be mixed:
* administer groups
* override all group content permissions

an example that we've been running into:
* we modeled some node-create permissions per language
* these permissions are set per country group

now a group admin would be able to create content in any language which is not wanted.

amitaibu’s picture

Category: bug » support
Status: Needs review » Fixed

> * we modeled some node-create permissions per language

In that case I think you should look at #1865944: Allow implementing modules to change the My/Other groups selection -- where the group-audience logic is custom.

(If you think differently please open the issue)

Status: Fixed » Closed (fixed)

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

geek-merlin’s picture

Status: Closed (fixed) » Needs review
FileSize
1.96 KB

The issue in #4 has its use cases but does not fix the underlying problem here.

As to #2:
We have several projects where we need a different permission for
a) can administer groups in the sense of add/delete users
b) has full access to all group (view, edit etc) rights
there are use cases for a-and-not-b as well as b-and-not-a.

In the meantime i added that variable introduced in #1 to the ui settings.

Status: Needs review » Needs work

The last submitted patch, 0001-Issue-1946424-Fixed-administer-group-permission-does.patch, failed testing.

geek-merlin’s picture

Status: Needs work » Needs review
FileSize
1.97 KB

HEAD ran away, rerolled.

Also see my comment in #6.

amitaibu’s picture

+++ b/og.moduleundefined
@@ -2057,6 +2057,11 @@ function og_ungroup($group_type, $gid, $entity_type = 'user', $etid = NULL) {
+  $admin_permissions = array('administer group', 'update group', 'add user', 'manage members', 'manage roles', 'manage permissions');

I'm not a fan of this approach, it hardcodes assumptions. Why not use hook_og_access_alter() to set the access according to your business logic?

azinck’s picture

Here's a slightly different approach to this problem. This patch creates global permissions for every OG role (and every group type) so that you can grant someone select OG roles in *all* groups of a given type without having to make them a member of all the groups.

geek-merlin’s picture

Title: "administer group" permission does grant all content permissions - it should not » Ability to provide global member administration without global content administration
Category: Support request » Feature request

From a first look i think the patch in #10 is very elegant and should solve the original problem.
Did not come to test it yet but if it works i'm all for it.

geek-merlin’s picture

amitaibu’s picture

> Why not use hook_og_access_alter() to set the access according to your business logic?

in my opinion, this question is still valid.

azinck’s picture

@Amitaibu in #13:

I agree that folks can solve this themselves in hook_og_access_alter(). I've used the same basic approach as in #10 but just implemented via hook_og_occess_alter() on a few projects of my own..

That said, the approach I took avoids specific business logic assumptions and solves a variety of use-cases that I've run across when building sites with OG 2.x. I thought it might be helpful to add it to the module.

amitaibu’s picture

Problem is that it adds a lot of new permissions - that could be overwhelming.

azinck’s picture

What if there were an option on the OG role to choose to expose it as a global permission?

azinck’s picture

Re-rolled against 2.x-dev

azinck’s picture

Updated slightly to work properly when a group has overridden permissions.

Status: Needs review » Needs work

The last submitted patch, 18: og-global_perms_for_og_roles-1946424-18.patch, failed testing.

azinck’s picture

Status: Needs work » Needs review
FileSize
2.88 KB

Oops, that needed parentheses. Fix attached.

azinck’s picture

Status: Needs review » Needs work

This needs more work; I've run into trouble with assumptions made by OG's entityreference widget.

amitaibu’s picture

Since it's touching access, it will require also tests