don't deny access to nodes that are without a group

did you mark those node types as omitted in og? you are using HEAD or 5? do you have any other node_access modules running?

can you explain what is surprising about this, and what you propose to make behavior more intuitive. please consider the available prefs on admon/og/og

1) is arguably a bug in the new 5.0 port.

2) i will add some text about this.

reopening this issue. thanks beginner.

Wondering if there is a patch or someone with a solution for 4.7.6. I currently have og 1.110.2.148 installed and am having same problem.

That is, if neither public nor private group is selected then no group gets set (I'd like it to default to public) and no record is added to node_access table for that nid.

I am starting to agree that this is not expected behavior. i'm inclined to remove that deny grant completely. am a bit worried though that some sites rely on this hiding for whatever reason.

I found a possible culprit in og_submit_group (og.module v.1.110.2.148). Please bear in mind I'm no php expert and even less
so regarding og. That said, here's a place to look:

if(empty($node->og_groups)) {
$node->og_public =1
}

empty() does not return true when all og_groups checkboxes in the node edit form are left unchecked. So, og_public = 1 never
gets set when there are groups on the node form.

That is, empty() does not perceive the og_groups array to be empty because the array keys are set with nids for each group,
with only the array values being empty.

I hope this helps. I have not posted my own patched solution, because it's not very elegant, but it does work for me. Rather than using
empty(), I check each array element individually, and set og_public if I find all array element values to be 0.

Mike.

I want to second (or third) this issue. I upgraded my site from 4.7.x to 5.1 last week and anonymous and registered users could not access my site. They would get the "Access denied" page. For the life of me I couldn't figure out what was wrong until I stumbled accross this issue. To resolve the problem, I had to re-submit each organic group on my site. Fortunately there were only 2! Good luck on finding a solution to this. OG is a very important module IMO.

I am going to try to fix a couple more bugs and then make a release of og5. then this can get committed to HEAD and tip of DRUPAL-5. no timeframe though.

Well, I might take this in the upcoming release. In addition to the latest patch, we need some wording on the Public checkbox like "Posts that are not in any group are Public regardless of this checkbox."

In addition to wording indicating that untargetted posts are public, wouldn't it be nice to have a teeny JS fragment that actually ticked and greyed out the public box if no group was selected, just to make it clearer what's happening?

yes, that javascript would be nice.

Hi all, I was following http://drupal.org/node/83005, which is marginally related to this one..

I don't think that the proposed solution is a good, wouldn't it be better to fill the node_ancestry table for unassigned nodes when enabling og?

Having a 'Public' checkbox with a notice like "Posts that are not in any group are Public regardless of this checkbox." is rather confusing ... I'd expect nodes without the public checkbox to be private.

If filling the node_ancestry table isn't feasible, mabye reversing the logic and using a "Private" checkbox would be less confusing:

• nodes are by default untouched by og
• when you check a group in the audience fieldset the node is assigned to that group but it's public
• to make it restricted to that group only you have to check "private"
• if you check only private and no groups the node is accessible only from its author

hiding nodes to only their author is not part of og's mission. i'm inclined to stick with the track we are discussing (Public is assumed if node not in any groups)

I should have said more. The point of the ancestry table to track ancestry. Thats why I don't want records for nodes that are not in any groups. I want og to not even care about such nodes.

Moshe said

hiding nodes to only their author is not part of og's mission. i'm inclined to stick with the track we are discussing (Public is assumed if node not in any groups)

and

I should have said more. The point of the ancestry table to track ancestry. Thats why I don't want records for nodes that are not in any groups. I want og to not even care about such nodes.

and I for one totally agree with that, its just that the system does not honor this logic and denies access to a node not belonging to any group when Public is not selected. I tested this 1.0 1.x-dev and HEAD.

@jhm - this patch hasn't been committed (look at the issue status). thats why you are seeing this behavior. please be patient. i am just thinking about what would be needed for an update path. also hoping someone will write the javascript to disable public checkbox on node form when no groups are selected. otherwise, the patch in #9 looks good.

I committed this, along with some javascript for the node form. Please do test this, and report back success or failure. I hope to roll a new release of og soon.

I think this is related.

When using Leech, organic groups doesn't seem to get passed on to the node items that Leech creates. It just marks everything as public. Nothing gets added to the node_ancestry table when Leech updates it's items. As a workaround, I added this to the Leech module:

$sql = "INSERT INTO {og_ancestry} (nid, group_nid, is_public) 
                  VALUES (%d, %d, 1)";
          db_query($sql, $n->nid, $gid);

It's not a good way to accomplish this, but it at least puts the items into the correct groups. Hopefully the new OG will fix this issue.

i don't know anything about leech but i guess it created nodes and doesn't call nodeapi('submit') before saving. anyway, thats not relevant here.

og_node_access_records uses

  if ($node->og_public) {
    $grants[] = array('realm' => 'og_public', 'gid' => 0, 'grant_view' => 1, 'grant_update' => 0, 'grant_delete' => 0);
  }

expecting $node->og_public to be set correct. if $node->og_public is not set correct no 'grant_view' is inserted

where does $node->og_public get set? in og_nodeapi ->'load'

   if (!og_is_group_type($node->type)) {
        $node->og_public = db_result(db_query_range("SELECT is_public FROM {og_ancestry} WHERE nid = %d", $node->nid, 0, 1));
      }
      else {
        og_load_group($node);
      }

so when there is no 'og_ancestry' entry for a node (no groups selected, only public) upon next load $node->og_public is not set....

When the code then calls og_node_access_records the node_access table is no longer correct.

propose to rewrite og_node_access_records

function og_node_access_records($node) {
  // don't write records if og access control is disabled or the node type is omitted or node is a group
  // if you want group nodes to be protected too, perhaps write a complementary node_access module. lets discuss.
  if (og_is_omitted_type($node->type) || !variable_get('og_enabled', FALSE) || og_is_group_type($node->type)) {
    return;
  }
  
  if (is_array($node->og_groups) && count($node->og_groups)) {
    foreach ($node->og_groups as $gid) {
      // we write a broad grant here but og_node_grants() only issues it selectively.
      $grants[] = array('realm' => 'og_subscriber', 'gid' => $gid, 'grant_view' => 1, 'grant_update' => 1, 'grant_delete' => 1);
    }
    // $node->og_public was set correct on 'load' use it since there are groups used
    $insertPublicView = $node->og_public;
  }
  else {
    // $node->og_public was not set correct on 'load', no groups, assume public...
    $insertPublicView = TRUE;
  }

  if ($insertPublicView) {
    $grants[] = array('realm' => 'og_public', 'gid' => 0, 'grant_view' => 1, 'grant_update' => 0, 'grant_delete' => 0);
  }
  
  // propose a deny for non public nodes whose type matters to og and aren't in a group
  if (!count($grants)) {
    $grants[] = array('priority' => 10, 'realm' => 'og_public', 'gid' => 0, 'grant_view' => 0, 'grant_update' => 0, 'grant_delete' => 0);
  }

  return $grants;
}

an alternative is make sure such nodes are catched in the initial check....

More info also in http://drupal.org/node/121566

mmmm guess that if http://drupal.org/node/107289#comment-175014 is applied it should be ok too.....

@bwynants - i think you have missed the fact that this was committed yesterday. the code you quote has changed. please test and let me know how it goes.

I looked ad the checkins from yesterday just a few minutes ago and they do seem OK. I'll test them this evening and report back.

Works. I cleared all entries in og_ancestry where gid was zero, did the update.php and all is still well. Even after an edit... However the initial state of the check box is not disabled.... it only gets disabled after a click a on, click off from some group. I'm not yet familiar with this java code so can't fix it.

I also would prefer the checkbox to be ON if no goup is selected (which is a correct visual representation).

patch attached does not insert anything extra in the database :-)

@bynants - the current code assumes that if a node is not in a group, then og doesn't care about it. i simply don't want to set $node->og_public at all. Thus, we don't have to change any code in og_node_access_records() either since $node->og_public will not evaluate to TRUE for these nodes. Thus, I see no benefit from your patch.

As for the javascript - the initial state of the checkbox comes from some complicated rules and i do not want to change that even when there are no selected groups. i want the user to be able to select a group and then checkbox becomes enabled, maintaining its carefully calculated default setting.

@moshe: I do understand your point of view as a programmer. However try to explain to a user if a post is visible or not. The user will not see 'public' selected and it's first reaction is that it will not be visible. Until he reads the small text. Checking it is 'for the user' a more direct approach. The overhead on the code is none existing!

PS: did 2 more sites with the newest code and all still fine :-)

i'm also trying to optimize user experience. this is not about optimizing code ... i wil soon roll a new release of og. thanks for the feedback.

solving the initial state (disabled) for the checkbox is easy. add a $('.og-audience').click();
to the Drupal.ogAttach function. It gets executed when the page opens and it puts the checkbox in a correct state

if someone wants to submit a .js patch i will review it

Correct initial state enabled/disabled state of checkbox

Correct initial state enabled/disabled state of checkbox

committed ... i was confused and didn't realize you were fixing the disabled/enabled state, and not the checked/unchecked state ... thanks.

Since this applies to 4.7 too (see #12 comment), will this be backported to 4.7 as well? Thanks

I'm not sure why this doesn't work on my setup -- but if you're an 'administrator', the 'public' box is grayed out. This is not the case with any other role (with sufficient permissions).

When I disable javascript in my browser, lo and behold, I'm able as an 'administrator' to check/uncheck the 'public' box.

So what I'm wondering, is what is the os.js file reading that it disables the 'public' checkbox just because the user is an administrator?

please discuss that js issue elsewhere.

I am also on 4.7.6 and have the same question as #12 and #43. Can this patch be backported to 4.7? It seems it would be extremely beneficial to many of us.

perhaps it could, if someone volunteered to maintain the 4.7 branch. i think this would be a big change for a stable branch though. IOW, very unlikely.