Skip to:

Opened 11 years ago

Closed 11 years ago

#5116 closed defect (bug) (fixed)

groups_delete_group() checks user group admin status

Reported by: ericlewis's profile ericlewis Owned by: boonebgorges's profile boonebgorges
Milestone: 1.9 Priority: normal
Severity: normal Version:
Component: Groups Keywords:


Kinda bizarre behavior. This means this function can't be used programmatically (e.g. in a cron job, where no WP user is signed in). You could do something like this as a workaround:

$group = new BP_Groups_Group( 1 );

but I'm surprised the Groups API function checks user caps here.

Change History (3)

#1 @boonebgorges
11 years ago

  • Milestone changed from Awaiting Review to 1.9

I've run into this before, and I agree that it's odd. Your workaround is OK, but there's a bunch of other stuff happening in this function that you'd also need to reproduce.

In any case, for the reasons you cite, we should be doing current_user_can() checks, etc, at the level of the screen function, not here.

In BP, groups_delete_group() is called in three places, and each time we do a separate check for permissions before calling it. So removing the permissions check from the function wouldn't cause any problems in BP. However, it's possible that removing the check would cause security issues in plugins that are currently implicitly relying on the check. My inclination is to do a search of the plugin repo to see if that's the case. If the plugin repo looks good, we'll remove the check, and post to bpdevel and the codex to let people know about the change (in case of non-public plugins etc). (I'm happy to do this search myself, but I won't be at my computer with a checkout of the whole plugin repo for a couple days.)

#2 @boonebgorges
11 years ago

I had a look through the plugin repo and it looks like we're OK to make this change.

#3 @boonebgorges
11 years ago

  • Owner set to boonebgorges
  • Resolution set to fixed
  • Status changed from new to closed

In 7588:

Don't check user permissions inside of groups_delete_group()

This check was preventing programmatic group deletion. Permissions should be
checked in the calling functions, such as screen functions.

Fixes #5116

Props ericlewis

Note: See TracTickets for help on using tickets.