#708 mod_storage_ldap - crashing for groups with single member

Reporter adrian.brzezinski
Owner Nobody
Created
Updated
Stars ★ (1)  
Tags
  • Status-New
  • Priority-Medium
  • Type-Defect
  1. adrian.brzezinski on

    What steps will reproduce the problem? 1. Setup LDAP authentication and groups, for eg: authentication = "ldap2" storage = "ldap" ldap = { hostname = "localhost", bind_dn = "uid=readonly,ou=services,ou=users,dc=local", bind_password = "xxxx", use_tls = false, user = { basedn = "ou=users,dc=local", usernamefield = "uid", filter = "(objectclass=posixAccount)", namefield = "cn" }, groups = { basedn = "ou=XMPP Contacts,ou=groups,dc=local", memberfield = "memberUid", namefield = "cn", { name = "IT Administrators", cn = "XMPP Admins", admin = true }, { name = "All Users", cn = "XMPP Users", admin = false }, }, ... 2. Create group in LDAP, for eg: dn: cn=XMPP Admins,ou=XMPP Contacts,ou=groups,dc=local cn: XMPP Admins gidnumber: 1001 memberuid: adrian.brzezinski objectclass: posixGroup objectclass: top 3. Try to login and load roster What is the expected output? What do you see instead? Expected result is user gets properly logged in with roster list loaded. Instead server drops connection, and roster list isn't loaded. Logs: Jul 09 14:44:54 c2s96d90f0 debug Received[c2s]: <iq id='rcAN1-27' type='set'> Jul 09 14:44:54 rostermanager debug load_roster: asked for: xx@local Jul 09 14:44:54 rostermanager debug load_roster: loading for new user: xx@local Jul 09 14:44:54 mod_c2s error Traceback[c2s]: ...rosody/modules/mod_storage_ldap/mod_storage_ldap.lua:119: bad argument #1 to 'ipairs' (table expected, got string) stack traceback: [C]: in function 'ipairs' ...rosody/modules/mod_storage_ldap/mod_storage_ldap.lua:119: in function <...rosody/modules/mod_storage_ldap/mod_storage_ldap.lua:101> (tail call): ? /usr/lib/prosody/core/rostermanager.lua:94: in function 'rm_load_roster' /usr/lib/prosody/core/sessionmanager.lua:166: in function 'sm_bind_resource' /usr/lib/prosody/modules/mod_saslauth.lua:280: in function '?' /usr/lib/prosody/util/events.lua:67: in function </usr/lib/prosody/util/events.lua:63> What version of the product are you using? On what operating system? Version: "prosody 0.9.7-2+deb8u3", system: Debian 8.5 Please provide any additional information below. Code from mod_storage_ldap.lua causing problems: 114 -- XXX this kind of relies on the way we do groups at INOC 115 for _, attrs in ld:search { base = params.groups.basedn, scope = 'onelevel', filter = filter } do 116 if groups[ attrs[namefield] ] then 117 local members = attrs[memberfield]; 118 119 for _, user in ipairs(members) do 120 if user ~= username then 121 local jid = user .. '@' .. module.host; 122 local record = contacts[jid]; Proposed path - please not that it's my first attempt to program in Lua :) # diff -uN /root/mod_storage_ldap.lua mod_storage_ldap.lua --- /root/mod_storage_ldap.lua 2016-07-10 13:24:18.309708593 +0200 +++ mod_storage_ldap.lua 2016-07-10 14:31:21.129656680 +0200 @@ -98,6 +98,22 @@ -- Roster Storage Implementation -- ---------------------------------------- +local function roster_add_contact(user, contacts) + local jid = user .. '@' .. module.host; + local record = contacts[jid]; + + if not record then + record = { + subscription = 'both', + groups = {}, + name = get_alias_for_user(user), + }; + contacts[jid] = record; + end + + return record +end + function adapters.roster:get(username) local ld = ldap.getconnection(); local contacts = {}; @@ -116,21 +132,20 @@ if groups[ attrs[namefield] ] then local members = attrs[memberfield]; - for _, user in ipairs(members) do + -- group with single member + if type(members) == "string" then if user ~= username then - local jid = user .. '@' .. module.host; - local record = contacts[jid]; + local record = roster_add_contact(members, contacts); + record.groups[ groups[ attrs[namefield] ] ] = true; + end - if not record then - record = { - subscription = 'both', - groups = {}, - name = get_alias_for_user(user), - }; - contacts[jid] = record; + -- group with multiple members + elseif type(members) == "table" then + for _, user in ipairs(members) do + if user ~= username then + local record = roster_add_contact(user, contacts); + record.groups[ groups[ attrs[namefield] ] ] = true; end - - record.groups[ groups[ attrs[namefield] ] ] = true; end end end

  2. adrian.brzezinski on

    Update for the path, previous one had small error: # diff -uN /root/mod_storage_ldap.lua mod_storage_ldap.lua --- /root/mod_storage_ldap.lua 2016-07-10 13:24:18.309708593 +0200 +++ mod_storage_ldap.lua 2016-07-10 21:09:39.680163192 +0200 @@ -98,6 +98,22 @@ -- Roster Storage Implementation -- ---------------------------------------- +local function roster_add_contact(user, contacts) + local jid = user .. '@' .. module.host; + local record = contacts[jid]; + + if not record then + record = { + subscription = 'both', + groups = {}, + name = get_alias_for_user(user), + }; + contacts[jid] = record; + end + + return record +end + function adapters.roster:get(username) local ld = ldap.getconnection(); local contacts = {}; @@ -116,22 +132,21 @@ if groups[ attrs[namefield] ] then local members = attrs[memberfield]; - for _, user in ipairs(members) do - if user ~= username then - local jid = user .. '@' .. module.host; - local record = contacts[jid]; - - if not record then - record = { - subscription = 'both', - groups = {}, - name = get_alias_for_user(user), - }; - contacts[jid] = record; - end - + -- group with single member + if type(members) == "string" then + if members ~= username then + local record = roster_add_contact(members, contacts); record.groups[ groups[ attrs[namefield] ] ] = true; end + + -- group with multiple members + elseif type(members) == "table" then + for _, user in ipairs(members) do + if user ~= username then + local record = roster_add_contact(user, contacts); + record.groups[ groups[ attrs[namefield] ] ] = true; + end + end end end end

New comment