Some of my users have events associated with a group and members outside of that group are able to see the titles of the events on the member profiles. Clicking the titles leads to an "access denied" error (as it should), but members outside of a group should never see any events unless they're public.

The "Users can view signups" checkbox has no effect on member profiles.

CommentFileSizeAuthor
#10 signup_conflict_rewrite_sql.patch_1.txt2.37 KBdww
#9 signup_conflict_rewrite_sql.patch.txt2.44 KBdww
#4 signup_conflicts_og.patch648 bytesChristefano-oldaccount
#2 signup_og.patch673 bytesChristefano-oldaccount

Comments

Christefano-oldaccount’s picture

Christefano-oldaccount commented

It's only fair if I say it's critical that I try to fix it! I'll post a patch as soon as I make one.

This code adds organic groups interoperatibility:

/**
* let's check if the user is allowed to see the signups
*/

    if (!node_access('view',node_load(array('nid' => $node->nid)))) {
        continue;
    }

/**
 * we now return to our regularly scheduled programming
*/

Add it to the signup_user() function at line 207.

Christefano-oldaccount’s picture

Christefano-oldaccount commented
Status: Active » Needs review
StatusFileSize
new signup_og.patch673 bytes

Here's a patch. I've applied it against signup.module,v 1.45.2.26 2006/08/18 08:21:52 and it seems to work.

moshe weitzman’s picture

moshe weitzman commented

more likely a db_rewrite_sql() is missing

Christefano-oldaccount’s picture

Christefano-oldaccount commented
Title: Organic groups interoperatibility » patch for signup and signup_conflicts that respects OG privacy
StatusFileSize
new signup_conflicts_og.patch648 bytes

Attached is a similar patch as above but for the signup_conflicts.module. I can only say it again: members outside of a group should never see any signups unless the event is marked public.

Please review.

dww’s picture

dww
we/he/they
commented
Title: patch for signup and signup_conflicts that respects OG privacy » signup doesn't respect privacy: db_rewrite_sql() missing
Status: Needs review » Needs work

sorry for the delay, i thought you were going to investigate moshe's suggestion and confirm or deny his claim. after looking briefly at the problem, that's exactly what's going on. it's not just missing a few calls to db_rewrite_sql(), there are NO such calls anywhere in the module. this is bad.

if you can, please read up on this function, and try to go through the signup code looking for any calls to db_query() that really need a db_rewrite_sql(), too. a single patch for all of signup and signup/contrib/* that added the missing rewrites would be MOST appreciated. ;)

thanks!
-derek

dww’s picture

dww
we/he/they
commented
Assigned: Unassigned » dww
Status: Needs work » Fixed

after careful review, it was only a single db_rewrite_sql() that was missing, and only for this one place (the signup listing on the profile page). i had to re-order the query a little to get db_rewrite_sql() to work happily with it, but now it's all fine. committed to HEAD, DRUPAL-5, DRUPAL-4-7, and DRUPAL-4-6.

Christefano-oldaccount’s picture

Christefano-oldaccount commented

Thanks for working on this. I banged my head against it for a while and was never able to get it to work right. After seeing that you spent a few hours on it, I'm very grateful.

dww’s picture

dww
we/he/they
commented
Component: Code » signup_conflicts.module
Status: Fixed » Active

no problem, sorry for the insane delay... signup doesn't get enough lovin' from me since i'm too damn busy.

but, hehe, whoops, i forgot about the silly signup_conflicts.module. :( should be easy, however... stay tuned.

dww’s picture

dww
we/he/they
commented
Status: Active » Needs review
StatusFileSize
new signup_conflict_rewrite_sql.patch.txt2.44 KB

attached patch applies on all branches (cleanly for DRUPAL-4-6, with offsets on DRUPAL-4-7, DRUPAL-5, and HEAD). please test/review, if you can. thanks!
-derek

dww’s picture

dww
we/he/they
commented
StatusFileSize
new signup_conflict_rewrite_sql.patch_1.txt2.37 KB

finally had a chance to setup signup_conflict.module and test this. a SQL syntax error snuck in with my previous patch. this one should be RTBC. any final reviews/tests?

dww’s picture

dww
we/he/they
commented
Status: Needs review » Fixed

after futher testing, committed to HEAD, DRUPAL-5, DRUPAL-4-7, and DRUPAL-4-6.

dww’s picture

dww
we/he/they
commented
Component: signup_conflicts.module » Code

(setting back for posterity). ;)

Anonymous’s picture

(not verified) commented
Status: Fixed » Closed (fixed)