diff sat_pubsub/pgsql_storage.py @ 330:82d1259b3e36

backend, pgsql storage: better items/notification handling, various fixes: - replaced const.VAL_AMODEL_ROSTER by const.VAL_AMODEL_PUBLISHER_ROSTER to follow change in pgsql schema - implemented whitelist access model - fixed bad access check during items retrieval (access was checked on recipient instead of requestor/sender) - getItemsData and notification filtering now use inline callbacks: this make these complexe workflows far mor easy to read, and clarity is imperative in these security critical sections. - publisher-roster access model now need to have only one owner, else it will fail. The idea is to use this model only when owner=publisher, else there is ambiguity on the roster to use to check access - replaced getNodeOwner by node.getOwners, as a node can have several owners - notifications filtering has been fixed in a similar way - psql: simplified withPEP method, pep_table argument is actually not needed - removed error.NotInRoster: error.Forbidden is used instead - notifications now notify all the owners, not only the first one
author Goffi <goffi@goffi.org>
date Sun, 26 Mar 2017 20:52:32 +0200
parents e73e42b4f6ff
children e93a9fd329d9
line wrap: on
line diff
--- a/sat_pubsub/pgsql_storage.py	Sun Mar 26 20:33:18 2017 +0200
+++ b/sat_pubsub/pgsql_storage.py	Sun Mar 26 20:52:32 2017 +0200
@@ -55,7 +55,6 @@
 
 from zope.interface import implements
 
-from twisted.internet import defer
 from twisted.words.protocols.jabber import jid
 from twisted.python import log
 
@@ -74,27 +73,24 @@
 
 # parseXml manage str, but we get unicode
 parseXml = lambda unicode_data: generic.parseXml(unicode_data.encode('utf-8'))
+PEP_COL_NAME = 'pep'
 
 
-def withPEP(query, values, pep, recipient, pep_table=None):
+def withPEP(query, values, pep, recipient):
     """Helper method to facilitate PEP management
 
     @param query: SQL query basis
     @param values: current values to replace in query
     @param pep: True if we are in PEP mode
     @param recipient: jid of the recipient
-    @param pep_table: added before pep if table need to be specified
     @return: query + PEP AND check,
         recipient's bare jid is added to value if needed
     """
-    pep_col_name = "{}pep".format(
-                   '' if pep_table is None
-                   else ".{}".format(pep_table))
     if pep:
-        pep_check="AND {}=%s".format(pep_col_name)
+        pep_check="AND {}=%s".format(PEP_COL_NAME)
         values=list(values) + [recipient.userhost()]
     else:
-        pep_check="AND {} IS NULL".format(pep_col_name)
+        pep_check="AND {} IS NULL".format(PEP_COL_NAME)
     return "{} {}".format(query, pep_check), values
 
 
@@ -261,8 +257,7 @@
                                             WHERE jid=%s) as e""",
                        (node_id, owner))
 
-        #TODO: manage JID access
-        if config[const.OPT_ACCESS_MODEL] == const.VAL_AMODEL_ROSTER:
+        if config[const.OPT_ACCESS_MODEL] == const.VAL_AMODEL_PUBLISHER_ROSTER:
             if const.OPT_ROSTER_GROUPS_ALLOWED in config:
                 allowed_groups = config[const.OPT_ROSTER_GROUPS_ALLOWED]
             else:
@@ -271,6 +266,8 @@
                 #TODO: check that group are actually in roster
                 cursor.execute("""INSERT INTO node_groups_authorized (node_id, groupname)
                                   VALUES (%s,%s)""" , (node_id, group))
+        # XXX: affiliations can't be set on during node creation (at least not with XEP-0060 alone)
+        #      so whitelist affiliations need to be done afterward
 
     def deleteNodeByDbId(self, db_id):
         """Delete a node using directly its database id"""
@@ -294,15 +291,7 @@
         if cursor.rowcount != 1:
             raise error.NodeNotFound()
 
-    def getNodeGroups(self, nodeIdentifier, pep, recipient=None):
-        return self.dbpool.runInteraction(self._getNodeGroups, nodeIdentifier, pep, recipient)
 
-    def _getNodeGroups(self, cursor, nodeIdentifier, pep, recipient):
-        cursor.execute(*withPEP("SELECT groupname FROM node_groups_authorized NATURAL JOIN nodes WHERE node=%s",
-                                (nodeIdentifier,), pep, recipient))
-        rows = cursor.fetchall()
-
-        return [row[0] for row in rows]
 
     def getAffiliations(self, entity, pep, recipient=None):
         d = self.dbpool.runQuery(*withPEP("""SELECT node, affiliation FROM entities
@@ -347,7 +336,6 @@
         self.nodeDbId = nodeDbId
         self.nodeIdentifier = nodeIdentifier
         self._config = config
-        self.owner = None;
 
 
     def _checkNodeExists(self, cursor):
@@ -360,11 +348,9 @@
     def getType(self):
         return self.nodeType
 
-    def getNodeOwner(self):
-        if self.owner:
-            return defer.succeed(self.owner)
-        d = self.dbpool.runQuery("""SELECT jid FROM nodes NATURAL JOIN affiliations NATURAL JOIN entities WHERE node_id=%s""", (self.nodeDbId,))
-        d.addCallback(lambda result: jid.JID(result[0][0]))
+    def getOwners(self):
+        d = self.dbpool.runQuery("""SELECT jid FROM nodes NATURAL JOIN affiliations NATURAL JOIN entities WHERE node_id=%s and affiliation='owner'""", (self.nodeDbId,))
+        d.addCallback(lambda rows: [jid.JID(r[0]) for r in rows])
         return d
 
 
@@ -590,7 +576,7 @@
                        (self.nodeDbId,))
         result = cursor.fetchall()
 
-        return [(jid.internJID(r[0]), r[1]) for r in result]
+        return {jid.internJID(r[0]): r[1] for r in result}
 
 
 
@@ -640,7 +626,7 @@
         item_id = cursor.fetchone()[0];
         self._storeCategories(cursor, item_id, item_data.categories)
 
-        if access_model == const.VAL_AMODEL_ROSTER:
+        if access_model == const.VAL_AMODEL_PUBLISHER_ROSTER:
             if const.OPT_ROSTER_GROUPS_ALLOWED in item_config:
                 item_config.fields[const.OPT_ROSTER_GROUPS_ALLOWED].fieldType='list-multi' #XXX: needed to force list if there is only one value
                 allowed_groups = item_config[const.OPT_ROSTER_GROUPS_ALLOWED]
@@ -650,6 +636,7 @@
                 #TODO: check that group are actually in roster
                 cursor.execute("""INSERT INTO item_groups_authorized (item_id, groupname)
                                   VALUES (%s,%s)""" , (item_id, group))
+        # TODO: whitelist access model
 
     def _storeCategories(self, cursor, item_id, categories, update=False):
         # TODO: handle canonical form
@@ -809,18 +796,20 @@
                 item_id = data[2]
                 date = data[3]
                 access_list = {}
-                if access_model == const.VAL_AMODEL_ROSTER: #TODO: jid access_model
+                if access_model == const.VAL_AMODEL_PUBLISHER_ROSTER:
                     cursor.execute('SELECT groupname FROM item_groups_authorized WHERE item_id=%s', (item_id,))
                     access_list[const.OPT_ROSTER_GROUPS_ALLOWED] = [r[0] for r in cursor.fetchall()]
 
                 ret.append(container.ItemData(item, access_model, access_list, date=date))
+                # TODO: whitelist item access model
             return ret
 
         items_data = [container.ItemData(generic.stripNamespace(parseXml(r[0])), r[1], r[2], date=r[3]) for r in result]
         return items_data
 
     def getItemsById(self, authorized_groups, unrestricted, itemIdentifiers):
-        """ Get items which are in the given list
+        """Get items which are in the given list
+
         @param authorized_groups: we want to get items that these groups can access
         @param unrestricted: if true, don't check permissions
         @param itemIdentifiers: list of ids of the items we want to get
@@ -849,9 +838,10 @@
                 item_id = result[2]
                 date= result[3]
                 access_list = {}
-                if access_model == const.VAL_AMODEL_ROSTER: #TODO: jid access_model
+                if access_model == const.VAL_AMODEL_PUBLISHER_ROSTER:
                     cursor.execute('SELECT groupname FROM item_groups_authorized WHERE item_id=%s', (item_id,))
                     access_list[const.OPT_ROSTER_GROUPS_ALLOWED] = [r[0] for r in cursor.fetchall()]
+                 #TODO: WHITELIST access_model
 
                 ret.append(container.ItemData(item, access_model, access_list, date=date))
         else: #we check permission before returning items