Skip to:
Content

BuddyPress.org

Opened 14 years ago

Closed 14 years ago

#2293 closed defect (bug) (fixed)

Hidden groups activity shows in friends > activity screen of non group members

Reported by: hnla's profile hnla Owned by:
Milestone: 1.2.4 Priority: critical
Severity: Version:
Component: Core Keywords: has-patch, needs-testing
Cc: hnla, boonebgorges

Description

Bob & Alice are friends.

Alice is a member of a Hidden group.

Bob is not a member of that same hidden group.

Bob logs in and navigates to his account/profile

Bob clicks link 'activity > friends'

Bob now sees all activity generated by his friends in all? areas.

Bob notices that Alice has posted to a group he hasn't seen before, he can read the latest comment she has made but he can't access the group via the links 'Group Name' or 'View' as he is correctly denied access - however Bob finds that he can use the 'Reply' link on the update and effectively post a reply to the group! this now appears threaded in the update view on his screen.

Alice logs in and visits the hidden group where she finds a reply to the last update she made but from a user who is not an invited member of this hidden group.

Noticed in 1.2.2.1

Tested and confirmed same behavior in 1.2.3

While this defect exists hidden groups are open and not safe to use as suggested by their description.

Attachments (2)

no_nonmember_comments_on_nonpublic_activity.patch (893 bytes) - added by boonebgorges 14 years ago.
hide_private_hidden_activity_from_friends.patch (546 bytes) - added by boonebgorges 14 years ago.

Download all attachments as: .zip

Change History (17)

#1 @hnla
14 years ago

  • Cc hnla added

#2 @jeffsayre
14 years ago

  • Priority changed from major to critical

I've bumped this up to critical so that it is looked at sooner than later.

#3 follow-up: @boonebgorges
14 years ago

  • Cc boonebgorges added
  • Keywords has-patch needs-testing added; hidden groups activity removed

This patch hides the reply button from nonmembers if the activity item is from a hidden or private group. The activity item itself is still visible, though.

#4 in reply to: ↑ 3 @hnla
14 years ago

Replying to boonebgorges:

This patch hides the reply button from nonmembers if the activity item is from a hidden or private group. The activity item itself is still visible, though.

Tested working as described on local install 1.2.3

To note: unlinks reply button rather than 'hides' which is satisfactory.

'Set as favourite' also functions on friends activity screen.

Issue remains that hidden groups updates, comments, may be viewed by non members of group.

Testing and trying to identify particular function for activity in this screen or in general and apply a variation of the patch posted here with aim of simply knocking out groups from all activity stream if they are hidden.

#5 @hnla
14 years ago

Wondering whether this shouldn't have been placed under 1.2.4 milestone really.

#6 follow-up: @boonebgorges
14 years ago

hnla - The tone of your original bug report "however Bob finds he can stil use the Reply link" made it sound like the reply link was the problem. Do you think that the entire activity item should be hidden? Now that I think about it, I'm leaning toward yes - if I post something in a private group, I am assuming that the content of what I post (even if it's just an excerpt as shown in an activity item) should not be visible to outsiders, even if they are my friends. Does that seem right?

#7 in reply to: ↑ 6 @hnla
14 years ago

Replying to boonebgorges:

hnla - The tone of your original bug report "however Bob finds he can stil use the Reply link" made it sound like the reply link was the problem. Do you think that the entire activity item should be hidden? Now that I think about it, I'm leaning toward yes - if I post something in a private group, I am assuming that the content of what I post (even if it's just an excerpt as shown in an activity item) should not be visible to outsiders, even if they are my friends. Does that seem right?

Sorry if it was confusing it was an attempt to impart all the steps and consequences.

The primary concern was that the Hidden group activity was showing up in an activity stream of a user who wasn't a member of that group but was shown to them due to the fact that they were Friends with someone who was a member of that hidden group, the issue proved to be further compounded by the fact that the user NOT a member of this hidden group was able to use the reply button to add a response to the update made tothe hidden group.

Ergo there is a fatal flaw in the activity logic it hasn't taken into account members being friends BUT NOT necessarily BOTH being members of the same hidden group, the friends activity is broken as such and in a critical manner. A hidden group MUST be just that if we have sensitive discussions underway we do not want them seen by uninvited members.

So yes I do think that the entire activity of that group must be hidden, and I'm leaning towards the safest option being ALWAYS and despite a user having access to that group, simply do not bring Hidden group activity into any site wide stream it's too risky, members of hidden groups will visit that group they do not need to see updates in the general activity streams.

#8 @boonebgorges
14 years ago

hnla - I think I agree with you.

The fine grained control over activity visibility would be nice, but the logic is relatively complicated - it'd take a fair amount of overhead to determine dynamically for each activity item (from the group component) whether the current user should be able to see it. It'd require a full group list, along with a full list of the groups to which the loggedin_user belongs, to be called up every time bp_has_activities was called.

I'm attaching a patch that gives a quick, though less than ideal, fix for the problem. It filters out all hide_sitewide activity items when you're viewing the Friends tab on your activity page.

#9 @hnla
14 years ago

Tested patch and it does indeed hide from Bob's screen all mention of Alice's activity in the hidden group, it had the consequence though of removing any notification of a friend activity if that activity is between a friend and a user who is NOT one of your friends.

Bob originally saw a activity stating that Alice had become friends with Admin

With patch added Bob no longer sees that activity, but then in friends activity he wouldn't have seen comments made by a friend to a non fiend anyway so it's a small loss

As stated it's not an ideal fix but then given the alternative loosing that particular activity between friends and non friends is acceptable?.

#10 @boonebgorges
14 years ago

hnla - That's because hide_sitewide is used in a couple different ways that don't seem entirely related (IMHO). For example: If Alice invites Admin to be friends, an activity item is created on the activity stream of both Alice and Admin. However, Admin's activity item is marked hide_sitewide, with the justification that "we've already got the first entry sitewide". In other words, hide_sitewide is used here to prevent duplicates in the sitewide stream. That means that, in the case you describe, Admin must have asked Alice to become friends - if it had been the other way around, then it would have shown up in Bob's friends stream. I don't think this will apply to very many different kinds of activity item, FWIW.

In contrast to this case, hide_sitewide is used elsewhere to hide things that are in hidden and private groups. That's what I mean when I say that the use of hide_sitewide is a bit inconsistent. It'd be nice in the long run to have different pieces of data play these roles. Probably qualifies as an enhancement request.

#11 @hnla
14 years ago

Ok I get what you mean. The more I toy with the activity stream the more I realise just how complex the logic is in it.

hide_sitewide used in the manner you describe does possibly sound like a recipe for confusion, but I'll leave it to the more experienced to suggest an enhancement ;-)

Thanks for all the help with this one.

Hugo.

#12 @3sixty
14 years ago

  • Milestone changed from 1.3 to 1.2.4

Hi folks - I respectfully suggest this be a 1.2.4 fix, and changed the milestone accordingly, as the core functionality of hidden and private groups is inherently broken, and in my opinion (and hnla's opinion) should not be used until fixed. Discussion here: http://buddypress.org/community/groups/how-to-and-troubleshooting/forum/topic/security-risk-forum-posts-are-promiscuous-even-private-posts-are-not-private/

#14 @apeatling
14 years ago

Patch looks good. hide_sitewide should be replaced in the long run with some clearer and more fine grained. Its usage has morphed which creates the confusion.

#15 @apeatling
14 years ago

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

(In [2983]) Fixes #2293 props boonegorges

Note: See TracTickets for help on using tickets.