In the current release "view buddy lists" is a global on/off permission. What I wanted was for buddy lists to be private but mutual buddies could see each others networks at a top level. That is A and B are buddies so A can see B buddylist (C,D) and B can see A's buddylist (E,F). But A can't see the buddylist of C,D and B can't see the buddylist of E,F.

Here is a simple patch to allow this.

A slightly better patch might include making this behaviour an optional settings item.

Index: buddylist.module
===================================================================
--- buddylist.module (revision 727)
+++ buddylist.module (working copy)
@@ -137,7 +137,7 @@
);

// 'view only' tabs
- $viewAccess = (($id == $user->uid) || user_access('view buddy lists'));
+ $viewAccess = buddylist_view_access($user->uid,$id);
$items[] = array(
'path' => 'buddylist/'. $id .'/buddies',
'title' => t('@Buddies', buddylist_translation()),
@@ -417,7 +417,7 @@
buddylist_setmsg_received($thisuser);
}

- if ($type == 'view' && user_access('view buddy lists')) {
+ if ($type == 'view' && buddylist_view_access($user->uid, $thisuser->uid)) {
// if thisuser has friends, show friends
$cnt = variable_get('buddylist_prof_buddies', 5);
$i = 0;
@@ -1235,6 +1235,13 @@
drupal_goto();
}

+function buddylist_view_access($uid, $bid) {
+ return ($uid == $bid) || buddylist_is_buddy($uid,$bid) || user_access('view buddy lists');
+}
+function buddylist_is_buddy($uid,$bid) {
+ $isbuddy = db_result(db_query("SELECT * FROM {buddylist} WHERE uid = %d AND buddy = %d",$uid, $bid));
+ return $isbuddy;
+}
/**
* Confirm and add a buddy.
*/

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dldege’s picture

Title: Mutal buddies can't view each others networks » Mutual buddies can't view each others networks

Fix title typo

fago’s picture

Status: Needs review » Needs work

hm, I think this should go in as an option. Could you make it configurable (through the admin buddylist settings) how access is handled?

dldege’s picture

are you thinking as a setting in the settings page or another access permission in hook_perm? I'll need to merge in all the recent updates and see about making another patch.

fago’s picture

perhaps it would fit better in hook_perm too. however we need a good name so that people know what it does.

perhaps we could split it up like these two permissions:
* view buddies' buddylists
* view buddylists

where the second one would supersede the first one. imo a "view stranger's buddylists" permission wouldn't make much sense. perhaps the name "view all buddylists" would be more obvious.

dldege’s picture

Yeah, I was thinking two permissions and that we'd need to rename the current one

I like

view all buddylists
view buddies' buddylists

Let me know if you concur and I'll try to get a patch put together soon.

fago’s picture

that sounds fine

dldege’s picture

FileSize
2.46 KB

OK, here is the patch for the agreed upon behaviour.

There is one new permission and one renamed

view all buddy lists // View all buddylists
view buddies' buddy list // View buddylists of your buddies -

A new function

function buddylist_view_access($bid) {
  global $user;
  return ($user->uid == $bid) || user_access('view all buddy lists') || (user_access('view buddies\' buddy list') && buddylist_is_buddy($user->uid,$bid));
}

handles the check for all permutations of the access permissions

and the helper function

function buddylist_is_buddy($uid,$bid) {
  $isbuddy = db_result(db_query("SELECT * FROM {buddylist} WHERE uid = %d AND buddy = %d",$uid, $bid));
  return $isbuddy;
}

can be called at any time to find out if A is a buddy of B.

fago’s picture

ok, there are a few things I've to note:

(*) if you look at hook_user() the buddies are already loaded for each user ($user->buddies) - so please make use of this data instead of running an extra query in buddylist_is_buddy()

(*) you introduce the variable name $bid - that's not used elsewhere and so unclear. please use something clear perhaps $buddy_uid

dldege’s picture

fago, I have 4-5 other modifications in buddylist so its fairly time consuming for me to make this specific patch - maybe I'm just not CVS savvy enough. If you want to change the variable names can you just modify the patch?

Concerning hook_user and buddylist_is_buddy - when called from that function there is a redundant lookup but buddylist_is_buddy can and is call from other places (like my other custom modules) where the user has not been loaded and thus I chose to use uid's since I can do the query for is buddy relationship w/o a user_load. What did you have in mind for how it should work?

fago’s picture

in buddylist_view_access() you call buddylist_is_buddy() for $user - but $user already contains an array of his buddies in $user->buddies, so this query isn't necessary.

of course, if you have no user object this function makes sense. I'm ok with leaving this function in just as API function - but please change buddylist_view_access().

it's always a good idea to develop new functionality on the original module, so you can make clean patches, which you can also apply on your modified version.

dldege’s picture

I found that there were cases when $user->buddies was null. The call to buddylist_is_buddy is really cheap taking .19ms vs. .50ms for user_load. I could introduce an if check for $user->buddies but I'm not sure the extra complexity of what is currently a 1 line function is necessary.

fago’s picture

http://drupal.org/node/91786

hm, yep. that sucks as it makes things complicated.. I would like to see a robust caching strategy in buddylist, however for the meanwhile your patch will be ok. I'll have a look at it later..

dldege’s picture

Status: Needs work » Needs review

How about getting this in there?

dldege’s picture

FileSize
2.57 KB

Looks like my previous patch doesn't apply against the current HEAD version.

Here is an update patch.

dldege’s picture

Version: 5.x-1.x-dev » master
RobRoy’s picture

Status: Needs review » Needs work

Just a cursory look, will test these more in a bit.

- A couple of code style issues regarding lack of a space in function args and if there is a single quote in the string it is preferred to use double quotes to wrap (in hook_perm). See coder.module.
- Should have some Doxygen above any new API functions with just a one liner about what it does.

STyL3’s picture

any status on this...i find that this would be very helpful!