Opened 9 years ago
Last modified 8 years ago
#6843 new defect (bug)
Activity @mentions in private groups for non members
Reported by: | timeuser | Owned by: | |
---|---|---|---|
Milestone: | Awaiting Contributions | Priority: | normal |
Severity: | normal | Version: | 2.4.0 |
Component: | Activity | Keywords: | needs-patch |
Cc: |
Description
The activity index template shows posts from private & hidden groups that the user is not a member of if they are @mentioned in the post. They should not be able to see these posts since they aren't a member of the group.
I can fix this by removing line #825 in bp-activity-filters.php:
'show_hidden' => true
But I'm not sure of the implications of why that override of show_hidden is even there. Removing that doesn't cause other problems I can see yet. Surely this is more complicated as there has to be a reason that override was put there?
Change History (15)
#2
@
9 years ago
- Component changed from API to Component - Activity
- Milestone changed from Awaiting Review to 2.5
Thanks for the report, @timeuser. I'd like to bring @r-a-y in to think about this.
As you note, show_hidden=false
will have side effects. You'll no longer see mentions from private groups of which you *are* a member. That being said, this is the way most of our activity streams already work, so in the short term it's probably the best way forward.
It's a complicated problem to solve more thoroughly, given the way that our activity queries work. The mentions
scope is translated to a set of BP_Activity_Query
args.
The logic of the query should be something like this:
Get all activity items with @username where: 'component' !== 'groups' OR 'component' === 'groups' && the group is public OR 'component' === 'groups' && the group is non-public && the user is a member of the group
But we don't have any group information available at the time of the activity query, and joining against these tables is going to cause lots of problems with existing filters on activity queries. This suggests that we could pull up a list of compatible groups before the main activity query, but it's hard to see what this would look like:
$user_private_groups = get all non-public groups of which the user is not a member $activity_query[] = array( 'column' => item_id' 'compare' => 'NOT IN', 'value' => $user_private_groups, );
(obviously it needs to be more complicated than this, but you get the idea). The problem here is that $user_private_groups
could be a mammoth number of groups.
Alternatively, we could let the query go through, and then filter out off-limits items afterward. But this is asking for trouble - it'll break all limit
and pagination parameters.
It should be noted that any solution to this specific problem would be part of the larger question of activity stream privacy. So let's think about it in those terms. And maybe go with the 'show_hidden' band-aid in the short-term.
Any ideas, @r-a-y or someone else?
#3
@
9 years ago
I think that if userA is a member of a private or hidden group, and userB is not, when userA mentions @userB he/she wants to let userB know about it, so the current BP behavior is correct in my opinion.
If we wanted to keep private and hidden groups strictly private or hidden, we could forbid mentions of non-members inside those groups. Just my two cents!
#4
@
9 years ago
There are other reasons users @mention someone than to let them know. Users @mention somone just to discuss them with other private group members, so there is a link to the @mentioned user. Our users definitely expect a private group to be just that, private and to not leak content and notify non group members no matter what they type in a post. This is how other social networking sites & apps generally treat non-public groups and messaging. Also, how would you actually go about forbidding mentions in any practical way? People can type whatever they want in a post and @mentions are found by pattern matching & queries after the fact.
#5
@
9 years ago
From the description of private and hidden groups included in BuddyPress:
"Group content and activity will only be visible to members of the group."
#6
@
9 years ago
I think that if userA is a member of a private or hidden group, and userB is not, when userA mentions @userB he/she wants to let userB know about it, so the current BP behavior is correct in my opinion.
This is an interesting thought, but I'm going to have to agree with @timeuser here. Private group content should be private, period. If we build it correctly, we could make it possible for a plugin to change this behavior.
#7
@
9 years ago
Yes, I agree that most users expect a private group to be always private, and that BP should not surprise them with exceptions.
#8
@
9 years ago
The "short term" show_hidden=false
option is just too problematic it seems. Not seeing private group @mentions for groups you are a member of is bad. You are notified of those posts and then notifications link you directly to the Activity index mentions tab where the post doesn't even show up.
So, I thought maybe I would pursue the option @boonebgorges mentions:
Alternatively, we could let the query go through, and then filter out off-limits items afterward. But this is asking for trouble - it'll break all limit and pagination parameters.
Does anyone have a suggestion on how best to go about that? What would I hook or filter to check and stop the output of a post into the feed? Limit and pagination problems seem more minor to me since they woudn't be very obvious to users.
#9
@
9 years ago
Does anyone have a suggestion on how best to go about that? What would I hook or filter to check and stop the output of a post into the feed? Limit and pagination problems seem more minor to me since they woudn't be very obvious to users.
Roughly:
<?php function bp6843_filter_out_private_mentions( $has_activities ) { global $activities_template; $as = $activities_template->activities; foreach ( $as as $k => $a ) { if ( 'groups' === $a->component ) { $group_id = $a->item_id; $group = groups_get_group( array( 'group_id' => $group_id ) ); if ( 'public' !== $group && ! groups_is_user_member( bp_loggedin_user_id(), $group_id ) ) { unset( $as[ $k ] ); } } } // reset indexes $as = array_values( $as ); $activities_template->activities = $as; return $has_activities; } add_filter( 'bp_has_activities', 'bp6843_filter_out_private_mentions' );
#10
@
9 years ago
That seems to work pretty well. I had to also set the activity_count
but after that it's alright since it updates in the theme I'm using by ajax there are no page numbers or counts displayed anyway, there are just fewer items there which isn't too noticeable usually.
added before return in your function:
$activities_template->activity_count = count( $as );
#11
@
9 years ago
I had to also set the activity_count
Yeah, I meant to say something like that. I wrote the above off the top of my head, without testing :-D I hope this'll be an OK stopgap until we can get our ducks in a row in BP core.
#12
@
9 years ago
There are other reasons users @mention someone than to let them know. Users @mention somone just to discuss them with other private group members, so there is a link to the @mentioned user.
I agree with @dontdream here.
If you are using at-mentions, you are intending to let the mentioned user know of that particular piece of content.
That being said, at-mentions from private content should not show up in public feeds.
There are primarily two places where at-mentions are filtered in the Activity component:
- Activity Directory: Click on the "Mentions" tab. (example.com/activity/)
- User Profile page: Click on the "Activity > Mentions" tab. (example.com/members/USER/activity/mentions/). (Personally, I would love to remove this page or make it so this page is only accessible if
bp_is_my_profile()
is true.)
Our current method for querying at-mention content is by doing a text search for @USERNAME<
, which is all kinds of ugly.
Perhaps, when an activity item has an @-mention, we record an activity meta item with 'bp_mentioned_user'
as key and user ID as the value. We would only add this meta key if it passes all privacy checks such as group privacy and group membership.
Then, when pulling up a user's activity mentions we do an activity meta query for the mentions activity scope.
The positives with this is we do not have to do another query, but the negative is we would definitely need a backfill routine. An upgrader as described in #6841 would be a great example usecase. In the case of the Groups component, we would also have to watch for group membership status and group privavy changes. We would also have to make a displayed user's "Activity > Mentions" page accessible only for the logged-in user as stated above.
If this is too much, we could roll with @boonebgorges' $user_private_groups
idea as well, but Boone's point about the number of groups is a potential problem.
It should be noted that any solution to this specific problem would be part of the larger question of activity stream privacy.
I was thinking we would tack on activity scopes as needed (member blocking, for example). But, this can complicate the SQL query immensely if other plugins tacked on their activity scopes.
I don't want to go too far off-topic here, but the question is how granular do we want activity privacy to be? Twitter's is relatively simple, but if we move (closer) towards a Facebook model (per-activity privacy with options such as "My Groups Only", "Friends of friends", etc.), the activity SQL query will become more complex and slower.
#15
@
8 years ago
- Keywords needs-patch added
@r-a-y I like your idea about breaking mentions out into structured data, and to query against that rather than doing a LIKE query against the content. (This would also be a good use case for a relationship API.) But I'm not sure I understand this bit:
We would only add this meta key if it passes all privacy checks such as group privacy and group membership.
I guess you mean that if user A mentions user B in a private group PG, then:
if ( PG is public || B is a member of PG ) { bp_activity_record_mention( $activity_id, $b ); }
meaning that if the if
condition fails, this particular activity item won't show up in @-mention queries for B. Am I understanding this correctly? I think it's clever. But I worry about data syncing and invalidation - it's going to be challenging to ensure that we catch all invalidation scenarios (activity edit, group membership changes, group status changes, etc). There would probably also be backward compatibility concerns for non-core cases that are expecting mention queries to include non-public data (like if you're running internal analytics or something like that).
I don't want to go too far off-topic here, but the question is how granular do we want activity privacy to be? Twitter's is relatively simple, but if we move (closer) towards a Facebook model (per-activity privacy with options such as "My Groups Only", "Friends of friends", etc.), the activity SQL query will become more complex and slower.
What people want, and have always wanted since BP 1.0, is extremely fine-grained privacy control. But a full-fledged ACL system, intended for use in many different kinds of BP environments, is probably going to be quite slow. I think that, for the time being, it's probably OK to have a system of stopgaps like the mention-meta that you've described here.
To reproduce this go to Activity index and click the Mentions tab. Posts you are @mentioned in will show up even if they were posted in a private or hidden group you are not a member of.