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.
*/
Comment | File | Size | Author |
---|---|---|---|
#14 | view_access.txt | 2.57 KB | dldege |
#7 | view_access_0.patch | 2.46 KB | dldege |
view_access.patch | 1.18 KB | dldege |
Comments
Comment #1
dldege CreditAttribution: dldege commentedFix title typo
Comment #2
fagohm, I think this should go in as an option. Could you make it configurable (through the admin buddylist settings) how access is handled?
Comment #3
dldege CreditAttribution: dldege commentedare 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.
Comment #4
fagoperhaps 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.
Comment #5
dldege CreditAttribution: dldege commentedYeah, 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.
Comment #6
fagothat sounds fine
Comment #7
dldege CreditAttribution: dldege commentedOK, 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
handles the check for all permutations of the access permissions
and the helper function
can be called at any time to find out if A is a buddy of B.
Comment #8
fagook, 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
Comment #9
dldege CreditAttribution: dldege commentedfago, 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?
Comment #10
fagoin 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.
Comment #11
dldege CreditAttribution: dldege commentedI 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.
Comment #12
fagohttp://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..
Comment #13
dldege CreditAttribution: dldege commentedHow about getting this in there?
Comment #14
dldege CreditAttribution: dldege commentedLooks like my previous patch doesn't apply against the current HEAD version.
Here is an update patch.
Comment #15
dldege CreditAttribution: dldege commentedComment #16
RobRoy CreditAttribution: RobRoy commentedJust 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.
Comment #17
STyL3 CreditAttribution: STyL3 commentedany status on this...i find that this would be very helpful!