trac user mrclay wrote on 42596186-09-26

Another tough problem: for PUBLIC content in closed groups, the global river still shows title/excerpt from the content, but the link will just forward the user back. Only real access levels on the items are dependable for "walling" off the the group content.

trac user sembrestels wrote on 42596450-03-01

I think cash cached this bug before us: #4720. But I prefer this title and description.

trac user sembrestels wrote on 42684967-01-17

Replying to mrclay:

Another tough problem: for PUBLIC content in closed groups, the global river still shows title/excerpt from the content, but the link will just forward the user back.

Actually it shows:

$user published a blog post $post_title 2 days ago

And group link doesn't appear (because group is inaccessible). Entity is accessible yet, while we don't fix #4525 in 1.9, so, it LGTM if we don't do anything in the river.

If you don't want to show entities into inaccessible containers, this can be the solution:

function elgg_view_river_item($item, array $vars = array()) {
    // checking default viewtype since some viewtypes do not have unique views per item (rss)
    if (!$item || !$item->getView() || !elgg_view_exists($item->getView(), 'default')) {
        return '';
    }

    $subject = $item->getSubjectEntity();
    $object = $item->getObjectEntity();
    if (!$subject || !$object) {
        // subject is disabled or subject/object deleted
        return '';
    } elseif (!$object->getContainerEntity()){
         //we cannot access to object's container
        return '';
    }

    $vars['item'] = $item;

    return elgg_view('river/item', $vars);
}

trac user sembrestels wrote on 42684978-01-15

PR: #378

This only includes the ignore access into group_gatekeeper. If you want I include into river the check to don't show items into invisible containers.

Milestone changed to Elgg 1.8.9 by trac user sembrestels on 42684978-01-15

trac user mrclay wrote on 42685409-05-11

Re, PR 378, it forwards the user to a group that may be invisible to him (see the 2nd paragraph in this issue's description). To fix, directly before forwarding, re-get the group entity, and if it returns false forward to the site root instead of the group URL. This helps not expose the existence of hidden groups.

Re: your elgg_view_river_item change, I think for now this would be OK. Filtering on display can break pagination pretty badly depending on how much content is in hidden groups on your site, but I think it's still better than showing a bunch of inaccessible content to users.

Aside: Ideally in the river fetch SQL we could join on visible containers only, but we'd have to change this when #4525 gets resolved. That and containers won't always be groups, and the policy is unclear there.

trac user sembrestels wrote on 42685572-10-22

I see... What about if group is inaccessible return false directly?

if ($group = elgg_get_page_owner_entity()) {
   ···
} else {
   if ($forward) {
      forward();
   } else {
      return false;
   }
}

trac user mrclay wrote on 42686636-08-30

PR #380 (with more explanation)

I think moving the guts of group_gatekeeper into a class simplifies the moving parts quite a bit. I'm definitely going to have to rework all this for 1.9, but I think this is worth getting right in 1.8.

Note that, since the river used to not care about the object's container at all, this fix may hide a lot of river items (that were mostly inaccessible anyway), making pagination...funky: some pages will contain fewer, possibly 0 items.

I see no way around this without some serious elgg_get_river patching, or maybe when river items ARE entities we can give them sensible access levels based on the levels of the subject, object, and their containers.

trac user mrclay wrote on 42691964-06-05

I didn't know the river already has its own access_id. When an item is added to the river, and its object's access_id is PUBLIC, we should analyze the object's container: If the container is a private or closed group, we should set the river item's access_id to the group acl. This will make it so these items don't show up in the queries for people who wouldn't be able to see them anyway, improving pagination.

Milestone changed to Elgg 1.8.10 by trac user mrclay on 42883219-01-09

Milestone changed to Elgg 1.8.12 by brettp on 42934480-07-19

brettp wrote on 42948404-01-02

Steve - Are you suggesting to forego the solution in PR-380 and instead provide the fix in add_to_river()?

trac user mrclay wrote on 42948443-03-26

My suggestion is to apply some of the PR: everything but the changes in elgg_view_river_item(). Then also make the change in add_to_river(). The PR improves redirection and user messages in group_gatekeeper(), but we should let regular access control control visibility for the river (rather than filter on view).

trac user mrclay wrote on 42948454-09-12

BTW, if a group changes between private/public or closed/open, I don't know that we need to go back and change access_id on old river items. Thoughts? That's why the PR's change in elgg_view_river_item() might be helpful; it checks it at view-time.

brettp wrote on 42962603-08-14

I think if a group's visibility is changed it should be reflected in the river entries. If a group were deleted, its river items would disappear.

brettp wrote on 42967650-10-05

Replying to mrclay:

If the container is a private or closed group, we should set the river item's access_id to the group acl.

Closed groups can have public content. I'm going to go ahead with this, but only for private groups.

trac user mrclay wrote on 42967669-01-19

In 1.8 public content inside closed groups is inaccessible. Whether that is seen as a bug or not, it has been the case since 1.8.0. So closed group river items should not be accessible outside the group.

trac user mrclay wrote on 42967678-10-14

Err, that should be MOST public content in closed groups. See my spreadsheet of doom

brettp wrote on 42967679-10-21

Just tried on 1.8.10. If you have a closed group and create a public blog post, the post is shown in the river, and non-group members / logged out users can click through to the post. However, you cannot access the group listing pages (/blog/group/all). This is confusing.

trac user mrclay wrote on 42967690-06-21

https://docs.google.com/a/elgg.org/spreadsheet/ccc?key=0AlAO5SiXoBNGdExuTjgySk1xeDdubm04dmZHU01WQVE#gid=1

brettp wrote on 42967717-05-15

O....k.

The problem for 1.8 is that river entries for visible items will now not be shown. I don't know that there's a great solution for 1.8. We don't want to duplicate the current insanity with new code, nor do we want to make these significant changes to the river that users are probably used to. Thoughts here?

trac user mrclay wrote on 42967835-06-09

The main goal of this ticket was for the group gatekeeper to properly restrict for hidden groups and forward appropriately. Lets just limit it to that and leave river changes for 1.9

trac user Steve Clay <steve@... wrote on 42970136-01-31

In [changeset:"8916fcdca6a2950d210abd2db7e6fb104abec149/elgg"]:

#!CommitTicketReference repository="elgg" revision="8916fcdca6a2950d210abd2db7e6fb104abec149"
Fixes #4789: group_gatekeeper() and river hide closed/invisible group content more reliably

trac user Brett Profitt <brett.profitt@... wrote on 42970136-03-10

In [changeset:"68441b33797e681a03fc61d217e1d110baa9e482/elgg"]:

#!CommitTicketReference repository="elgg" revision="68441b33797e681a03fc61d217e1d110baa9e482"
Refs #4789. Removing the container check for river items.