Skip to:
Content

BuddyPress.org

Opened 6 years ago

Closed 6 years ago

Last modified 4 years ago

#7936 closed defect (bug) (fixed)

joinleave_group not working on private Group

Reported by: ravipatel's profile ravipatel Owned by:
Milestone: 3.2.0 Priority: high
Severity: normal Version: 3.0.0
Component: Groups Keywords:
Cc: john.deo4566@…

Description

joinleave_group button event not working on private group. Return -1 value.

http://easycaptures.com/fs/uploaded/1205/9630192904.png

Buddypress : 3.1.0

Attachments (3)

7936.diff (5.5 KB) - added by boonebgorges 6 years ago.
site-admin-can-join-private-groups.png (49.0 KB) - added by dcavins 6 years ago.
With .2 patch
7936.2.diff (1.2 KB) - added by dcavins 6 years ago.
If site admins can join private groups (without requesting membership), show them a "join group" button instead of "request membership".

Download all attachments as: .zip

Change History (19)

#1 @ravipatel
6 years ago

bp current user capability not found.
groups_join_group , groups_request_membership

buddypress\bp-templates\bp-legacy\buddypress-functions.php

Step1 : http://easycaptures.com/fs/uploaded/1205/9630192904.png
Request for membership :

Step2: Added code where function exits:
http://easycaptures.com/fs/uploaded/1205/3458617053.png

Step3: Not applied any condition there please how to fix this one?

http://easycaptures.com/fs/uploaded/1205/9165843085.png

Thanks

#2 @ravipatel
6 years ago

<?php
if ( bp_current_user_can( 'groups_join_group', array( 'group_id' => $group->id ) ) ) {
                    
                        check_ajax_referer( 'groups_join_group' );

Not working

#3 @dcavins
6 years ago

Hi @ravipatel-

Thanks for your report. I'm not able to replicate your issue using either template pack (BP Legacy or BP Nouveau)--group membership requests are being created successfully via AJAX on my test and production sites.

If you're seeing a -1 response, that suggests to me that for some reason a check_ajax_referer() validation is failing somewhere along the way. (Check the time zone set for your site, maybe, since nonces are time sensitive?)
https://developer.wordpress.org/reference/functions/check_ajax_referer/

#4 @ravipatel
6 years ago

Yes, when i setup new buddypress with latest wordpress version working fine.
But whats about after update buddypress and wordpress.

Now facing issue in both version :
Wordpress : 4.9.6 and 4.9.7

I have check code with 4.9.7 wordpress.
Return a "groups_join_group" false time zone now based in london.

As per you have suggested we have check nonce are not match on this action only.

Step1 : Group is private (Button nonce action "groups_request_membership") - (admin click on button front end side).

Step2 : Called condition:

<?php
if ( bp_current_user_can( 'groups_join_group', array( 'group_id' => $group->id ) ) ) {

Actually this this one required to move on else condition.

Last edited 6 years ago by ravipatel (previous) (diff)

#5 @ravipatel
6 years ago

http://easycaptures.com/fs/uploaded/1207/1812180597.png

Please check this conditions for admin user. Please suggest whats we do for this right now.

#6 @ravipatel
6 years ago

@dcavins : Please help for for this.

This ticket was mentioned in Slack in #core by ravi. View the logs.


6 years ago

This ticket was mentioned in Slack in #buddypress by ravi. View the logs.


6 years ago

#9 @boonebgorges
6 years ago

  • Milestone changed from Awaiting Review to 3.2.0
  • Version set to 3.0.0

There does appear to be a problem here.

  1. With userA, create a Private group
  2. With userB - who has the 'bp_moderate' cap - go to the directory and click 'Request Membership'

The problem is that 'joinleave_group' requests are ambiguous. The client doesn't specify whether the correct action is a "join" or a "request" or a "leave" or an "accept". Prior to [11776], it used the user's group membership status to infer the purpose of the request. After [11776], permission checks are used to infer the purpose of the request, and the permission checks *contain* a membership-status check. The problem is that the permission checks also do other things.

In particular, 'bp_moderate' users are allowed to do anything, so bp_current_user_can() checks always return true. This means that https://buddypress.trac.wordpress.org/browser/tags/3.1.0/src/bp-templates/bp-legacy/buddypress-functions.php?marks=1515#L1514 returns true for an admin user, even on private groups where membership must be requested. When this happens, check_ajax_referer( 'groups_join_group' ) fails, because the nonce for this request is actually for 'groups_request_membership'.

To fix this, we need to separate out the permission check from the logic check. A suggested patch is attached, for bp-legacy.

Note that the patch doesn't fix what appears to be a typo related to the permission check for accepting invitations - @dcavins is this meant to be 'groups_request_membership'? See also the error message that follows.

@boonebgorges
6 years ago

#10 @dcavins
6 years ago

Hi Boone-

Thanks for looking into this. I see what you mean--the nonce check fails because we're encoding different strings for different cases.

We later added the ability of site admins to basically do anything (@r-a-y caught some issues where old behavior for admins wasn't carrying through), but I was of the understanding that site admins were allowed to join private groups without requesting membership.

If that's the case, I'm wondering if this is a good opportunity to fix the button, because a site admin is allowed join any group at will. So the other way to mesh the front- and back-end request is to change the button to be a Join Group button. I've attached a patch that makes the front end display match our permissions logic more closely.

If that's not the case, then another way to fix the issue if to change the permissions so that site admins have to request membership in the permission structure.

In either case, I think your changes should be rolled in, because they make the logic clearer. Also, the logic as it exists would only allow invitation acceptance to happen for private groups, which can't be right. (I've been using invite anyone for so long I'll admit I'm not 100% sure how vanilla invites work without testing them.)

Thanks again for poking at this. I'm going to think about it some more, and if we can agree on the site admin permission thing, I'll need to fix that in at least one place.

Thanks!

@dcavins
6 years ago

With .2 patch

@dcavins
6 years ago

If site admins can join private groups (without requesting membership), show them a "join group" button instead of "request membership".

#11 @boonebgorges
6 years ago

In 12198:

Separate action logic from permission logic in bp-legacy joinleave_group AJAX callback.

Prior to [11776], join/leave group button intent was calculated based
directly on the user's group membership status. After that changeset,
the intent was calculated based on permission checks, which encompassed
the membership checks but also some additional information. This caused
problems when a user - such as a super admin - had *permission* to
join a group, but was not actually *trying* to join a group (but instead
had clicked Request Membership).

See #7936.

#12 @boonebgorges
6 years ago

Thanks for having a quick look, @dcavins !

because a site admin is allowed join any group at will.

When I was originally looking at this, I considered the same thing. I'm on the fence about whether to do it. For one thing, the current behavior - admins see Request Membership - does not just date from 3.0, but has always been the case (as far as I can see). For another thing, this change would mean that it's impossible for admins to request membership to a private group. Currently, they have the option of *either* joining the group directly (in the admin) *or* requesting membership on the front end (and, optionally, going to Manage > Requests and immediately accepting). If we change it so that the admin always sees Join Group, they'll lose this bit of functionality. It seems to me that in at least some cases, a user who happens to have admin privileges might also like to be able to request membership, at least as a courtesy. So I lean toward leaving the behavior as-is, and allowing the "super power" to be Dashboard-only. Happy to hear other thoughts, though.

#13 @boonebgorges
6 years ago

In 12199:

Separate action logic from permission logic in bp-legacy joinleave_group AJAX callback.

Prior to [11776], join/leave group button intent was calculated based
directly on the user's group membership status. After that changeset,
the intent was calculated based on permission checks, which encompassed
the membership checks but also some additional information. This caused
problems when a user - such as a super admin - had *permission* to
join a group, but was not actually *trying* to join a group (but instead
had clicked Request Membership).

Merges [12198] to the 3.0 branch.

See #7936.

#14 @DJPaul
6 years ago

Is this done?

#15 @imath
6 years ago

I think there's a chance it's not done yet for BP Nouveau. I will take the time to look at it soon.

#16 @DJPaul
6 years ago

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

At a glance I don't see why Nouveau would be affected by a change to bp_group_join_button(). It doesn't seem to obviously duplicate the function and/or customise it, so I think it's done.

I agree with @boonebgorges comment above about not changing the behaviour.

Closing this. Any issues with Nouveau if any, please open a new ticket.

Note: See TracTickets for help on using tickets.