You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ponymail.apache.org by se...@apache.org on 2016/12/15 23:14:21 UTC

incubator-ponymail git commit: rights checking should be localised

Repository: incubator-ponymail
Updated Branches:
  refs/heads/master b6598b818 -> cba30574f


rights checking should be localised

This fixes #293

Project: http://git-wip-us.apache.org/repos/asf/incubator-ponymail/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-ponymail/commit/cba30574
Tree: http://git-wip-us.apache.org/repos/asf/incubator-ponymail/tree/cba30574
Diff: http://git-wip-us.apache.org/repos/asf/incubator-ponymail/diff/cba30574

Branch: refs/heads/master
Commit: cba30574fa236a054cc66de1638b9c8e0ff41872
Parents: b6598b8
Author: Sebb <se...@apache.org>
Authored: Thu Dec 15 23:14:09 2016 +0000
Committer: Sebb <se...@apache.org>
Committed: Thu Dec 15 23:14:09 2016 +0000

----------------------------------------------------------------------
 CHANGELOG.md               |  3 ++-
 site/api/atom.lua          | 26 ++-------------------
 site/api/email.lua         | 17 +-------------
 site/api/lib/aaa.lua       | 52 ++++++++++++++++++++++++++++++++++++++++-
 site/api/lib/utils.lua     | 47 +------------------------------------
 site/api/mbox.lua          |  7 +-----
 site/api/notifications.lua | 15 +-----------
 site/api/preferences.lua   | 10 ++------
 site/api/source.lua        | 15 ++----------
 site/api/stats.lua         | 13 +----------
 site/api/thread.lua        | 40 ++++---------------------------
 11 files changed, 69 insertions(+), 176 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-ponymail/blob/cba30574/CHANGELOG.md
----------------------------------------------------------------------
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 00c458e..7204544 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -65,7 +65,8 @@
 - inconsistent rights checking (#267)
 - thread.lua fails to fetch rights when fetching private child of public parent (#296)
 - typo in all example aaa modules: xemail != exmail (#299)
-- 
+- lib/aaa.lua overwrites local customisations on updates (#292)
+- rights checking should be localised (#293)
 
 ## CHANGES in 0.9b:
 

http://git-wip-us.apache.org/repos/asf/incubator-ponymail/blob/cba30574/site/api/atom.lua
----------------------------------------------------------------------
diff --git a/site/api/atom.lua b/site/api/atom.lua
index fdb92c4..2b5726b 100644
--- a/site/api/atom.lua
+++ b/site/api/atom.lua
@@ -122,18 +122,7 @@ function handle(r)
         for k = #doc.hits.hits, 1, -1 do
             local v = doc.hits.hits[k]
             local email = v._source
-            local canUse = false
-            if email.private then
-                if account then
-                    if rights then
-                        rights = aaa.rights(r, account)
-                    end
-                    canUse = utils.canAccessDoc(email, rights)
-                end
-            else
-                canUse = true
-            end
-            if canUse then
+            if aaa.canAccessDoc(r, email, account) then
                 local mid = email['message-id']
                 local irt = email['in-reply-to']
                 email.id = v._id
@@ -159,18 +148,7 @@ function handle(r)
                 fetchChildren(r, parent)
                 -- ensure access and process all children
                 for k, doc in pairs(emls_thrd) do
-                    local canUse = false
-                    if doc.private then
-                        if account then
-                            if not rights then
-                                rights = aaa.rights(r, account)
-                            end
-                            canUse = utils.canAccessDoc(doc, rights)
-                        end
-                    else
-                        canUse = true
-                    end
-                    if canUse then
+                    if aaa.canAccessDoc(r, doc, account) then
                         table.insert(emls, doc)
                     end
                 end

http://git-wip-us.apache.org/repos/asf/incubator-ponymail/blob/cba30574/site/api/email.lua
----------------------------------------------------------------------
diff --git a/site/api/email.lua b/site/api/email.lua
index 06c0a21..4c62437 100644
--- a/site/api/email.lua
+++ b/site/api/email.lua
@@ -64,25 +64,10 @@ function handle(r)
     
     -- Did we find an email?
     if doc then
-        local canAccess = false
         local account = user.get(r)
         
-        -- Is this a private email? and if so, does the user have access to view it?
-        if doc.private then
-            if account then
-                canAccess = utils.canAccessDoc(doc, aaa.rights(r, account))
-            else
-                r:puts(JSON.encode{
-                    error = "You must be logged in to view this email"
-                })
-                return cross.OK
-            end
-        else
-            canAccess = true
-        end
-        
         -- If we can access this email, ...
-        if canAccess then
+        if aaa.canAccessDoc(r, doc, account) then
             doc.tid = doc.request_id
             
             -- Are we in fact looking for an attachment inside this email?

http://git-wip-us.apache.org/repos/asf/incubator-ponymail/blob/cba30574/site/api/lib/aaa.lua
----------------------------------------------------------------------
diff --git a/site/api/lib/aaa.lua b/site/api/lib/aaa.lua
index cdc3481..e3b0ec4 100644
--- a/site/api/lib/aaa.lua
+++ b/site/api/lib/aaa.lua
@@ -92,7 +92,57 @@ local function getRights(r, account)
     end
 end
 
+--[[ 
+  parse a listid
+  returns the full lid, listname and the domain from "<listname.domain>"
+   where listname cannot contain any "." chars
+]]--
+local function parseLid(lid)
+    return lid:match("^<(([^.]+)%.(.-))>$")
+end
+
+
+-- does the account have the rights to access the mailing list?
+-- N.B. will fail if rights or list_raw are invalid
+local function canAccessList(r, lid, account)
+    if not account then return false end
+    -- check the rights cache
+    local rights = account._rights_ 
+    if not rights then
+        rights = getRights(r, account)
+        account._rights_ = rights
+    end
+    -- we don't need the name
+    local flid, _ , domain = parseLid(lid)
+    for _, v in pairs(rights) do
+        if v == "*" or v == flid or v == domain then
+            return true
+        end
+    end
+    return false
+end
+
+-- does the account have the rights to access the document?
+-- N.B. will fail if doc is invalid
+local function canAccessDoc(r, doc, account)
+    if doc.private then
+        -- if not account then return false end (done by canAccessList)
+        -- assume that rights are list-based
+        return canAccessList(doc.list_raw, account)
+    else
+        return true
+    end
+end
+
+--[[
+    Note that the functions do not check their parameters.
+    This is because they may be called frequently.
+]]--
+
 -- module defs
 return {
-    rights = getRights
+    rights = getRights,
+    parseLid = parseLid,
+    canAccessList = canAccessList,
+    canAccessDoc = canAccessDoc
 }

http://git-wip-us.apache.org/repos/asf/incubator-ponymail/blob/cba30574/site/api/lib/utils.lua
----------------------------------------------------------------------
diff --git a/site/api/lib/utils.lua b/site/api/lib/utils.lua
index 7442312..1b27645 100644
--- a/site/api/lib/utils.lua
+++ b/site/api/lib/utils.lua
@@ -42,51 +42,6 @@ local function findParent(r, doc, elastic)
 end
 
 
---[[ 
-  parse a listid
-  returns the full lid, listname and the domain from "<listname.domain>"
-   where listname cannot contain any "." chars
-]]--
-local function parseLid(lid)
-    return lid:match("^<(([^.]+)%.(.-))>$")
-end
-
-
--- does the user have the rights to access the mailing list?
--- N.B. will fail if rights or list_raw are invalid
-local function canAccessList(lid, rights)
-    -- we don't need the name
-    local flid, _ , domain = parseLid(lid)
-    for _, v in pairs(rights) do
-        if v == "*" or v == flid or v == domain then
-            return true
-        end
-    end
-    return false
-end
-
--- does the user have the rights to access the document?
--- N.B. will fail if doc is invalid; may fail if rights is invalid
-local function canAccessDoc(doc, rights)
-    if doc.private then
-        return canAccessList(doc.list_raw, rights)
-    else
-        return true
-    end
-end
-
---[[
-    TODO the canAccess functions perhaps belong in aaa.lua.
-    This would allow sites to have their own ways of matching lists to rights and individual docs
-    This should be dealt with if/when aaa.lua is split up into generic and local parts.
-
-    Also the functions do not check their parameters.
-    This is because they may be called frequently.
-]]--
-
 return {
-    findParent = findParent,
-    parseLid = parseLid,
-    canAccessList = canAccessList,
-    canAccessDoc = canAccessDoc
+    findParent = findParent
 }

http://git-wip-us.apache.org/repos/asf/incubator-ponymail/blob/cba30574/site/api/mbox.lua
----------------------------------------------------------------------
diff --git a/site/api/mbox.lua b/site/api/mbox.lua
index 5ce3c6a..0c9aca4 100644
--- a/site/api/mbox.lua
+++ b/site/api/mbox.lua
@@ -121,7 +121,6 @@ function handle(r)
         }
 
         local account = user.get(r)
-        local rights = nil
         local listAccessible = nil -- not yet initialised
         -- for each email, get the actual source of it to plop into the mbox file
         for k, v in pairs(docs.hits.hits) do
@@ -129,11 +128,7 @@ function handle(r)
             -- aaa.rights() can be expensive, so only do it once per download
             if v.private and listAccessible == nil then
                 -- we are dealing with a single list here so only need to check once
-                if account then
-                    listAccessible = utils.canAccessList(lid, aaa.rights(r, account))
-                else
-                    listAccessible = false
-                end
+                listAccessible = aaa.canAccessList(r, lid, account)
             end
             if listAccessible or not v.private then
                 local doc = elastic.get('mbox_source', v.mid)

http://git-wip-us.apache.org/repos/asf/incubator-ponymail/blob/cba30574/site/api/notifications.lua
----------------------------------------------------------------------
diff --git a/site/api/notifications.lua b/site/api/notifications.lua
index 80abc72..1cdb693 100644
--- a/site/api/notifications.lua
+++ b/site/api/notifications.lua
@@ -49,25 +49,12 @@ function handle(r)
             return cross.OK
         end
         local peml = {}
-        local rights = nil
         
         -- Find all recent notification docs, up to 50 latest results
         local docs = elastic.find("recipient:\"" .. r:sha1(account.cid) .. "\"", 50, "notifications")
         for k, doc in pairs(docs) do
-            local canUse = false
-            -- check we have rights to view this notification (it might be from a private email we shouldn't see)
-            if doc.private then
-                if account then
-                  if not rights then
-                      rights = aaa.rights(r, account)
-                  end
-                  canUse = utils.canAccessDoc(doc, rights)
-                end
-            else
-                canUse = true
-            end
             -- if we can see the email, push the notif to the list
-            if canUse then
+            if aaa.canAccessDoc(r, doc, account) then
                 doc.id = doc['message-id']
                 doc.tid = doc.id
                 doc.nid = doc.request_id

http://git-wip-us.apache.org/repos/asf/incubator-ponymail/blob/cba30574/site/api/preferences.lua
----------------------------------------------------------------------
diff --git a/site/api/preferences.lua b/site/api/preferences.lua
index f495947..d5be503 100644
--- a/site/api/preferences.lua
+++ b/site/api/preferences.lua
@@ -311,19 +311,13 @@ Pony Mail - Email for Ponies and People.
             r:ivm_set(PM_LISTS_PRIVATE_KEY, JSON.encode(pdoc))
         end
         
-        local rights = {}
-        if account then
-            rights = aaa.rights(r, account)
-        end
         -- remove any lists containing mails that the user is not allowed to access
         -- N.B. this removes mixed lists
         -- i.e. the user won't see the list name if it contains a single private mail they cannot access
         for x,y in pairs (pdoc.aggregations.from.buckets) do
-            local canAccess = false
-            local _, list, domain = utils.parseLid(y.key:lower())
+            local _, list, domain = aaa.parseLid(y.key:lower())
             if list and domain and #list > 0 and #domain > 2 then
-                canAccess = utils.canAccessList(y.key:lower(), rights)
-                if not canAccess then
+                if not aaa.canAccessList(r, y.key:lower(), account) then
                     lists[domain] = lists[domain] or {}
                     lists[domain][list] = nil
                 end

http://git-wip-us.apache.org/repos/asf/incubator-ponymail/blob/cba30574/site/api/source.lua
----------------------------------------------------------------------
diff --git a/site/api/source.lua b/site/api/source.lua
index f1a003c..892723b 100644
--- a/site/api/source.lua
+++ b/site/api/source.lua
@@ -37,19 +37,8 @@ function handle(r)
         end
     end
     if doc then
-        local canAccess = false
-        if doc.private then
-            local account = user.get(r)
-            if account then
-                canAccess = utils.canAccessDoc(doc, aaa.rights(r, account))
-            else
-                r:puts("You must be logged in to view this email")
-                return cross.OK
-            end
-        else
-            canAccess = true
-        end
-        if canAccess then
+        local account = user.get(r)
+        if aaa.canAccessDoc(r, doc, account) then
             doc.tid = doc.request_id
             local doc_raw = elastic.get('mbox_source', doc.request_id)
             if doc_raw then

http://git-wip-us.apache.org/repos/asf/incubator-ponymail/blob/cba30574/site/api/stats.lua
----------------------------------------------------------------------
diff --git a/site/api/stats.lua b/site/api/stats.lua
index eeed145..a77ef12 100644
--- a/site/api/stats.lua
+++ b/site/api/stats.lua
@@ -579,18 +579,7 @@ function handle(r)
         if eepoch > lastEmail then
             lastEmail = eepoch
         end
-        local canUse = false
-        if email.private then
-            if account then
-                if not rights then
-                    rights = aaa.rights(r, account)
-                end
-                canUse = utils.canAccessDoc(email, rights)
-            end
-        else
-            canUse = true
-        end
-        if canUse then
+        if aaa.canAccessDoc(r, email, account) then
 
             h = h + 1
             

http://git-wip-us.apache.org/repos/asf/incubator-ponymail/blob/cba30574/site/api/thread.lua
----------------------------------------------------------------------
diff --git a/site/api/thread.lua b/site/api/thread.lua
index 5689358..04bde9f 100644
--- a/site/api/thread.lua
+++ b/site/api/thread.lua
@@ -43,7 +43,7 @@ local function anonymize(doc)
 end
 
 -- func that fetches all children of an original topic email thingy
-local function fetchChildren(r, pdoc, c, biglist, rights, account)
+local function fetchChildren(r, pdoc, c, biglist, account)
     c = (c or 0) + 1
     -- don't fetch more than 250 subtrees, we don't want to nest ad nauseam
     if c > 250 then
@@ -56,23 +56,9 @@ local function fetchChildren(r, pdoc, c, biglist, rights, account)
     local docs = elastic.findFast('in-reply-to:"' .. r:escape(pdoc['message-id'])..'"', 50, "mbox")
     for k, doc in pairs(docs) do
         -- if we haven't seen this email before, check for its kids and add it to the bunch
-        local canAccess = false
-        if doc.private then
-            if not rights then
-                if account then
-                    rights = aaa.rights(r, account)
-                else
-                    rights = {}
-                end
-            end
-            canAccess = utils.canAccessDoc(doc, rights)
-        else
-            canAccess = true
-        end
-        
-        if canAccess and (not biglist[doc['message-id']]) then
+        if (not biglist[doc['message-id']]) and aaa.canAccessDoc(r, doc, account) then
             biglist[doc['message-id']] = true
-            local mykids = fetchChildren(r, doc, c, biglist, rights, account)
+            local mykids = fetchChildren(r, doc, c, biglist, account)
             if not account and config.antispam then
                 doc = anonymize(doc)
             end
@@ -123,29 +109,13 @@ function handle(r)
     
     -- did we find an email?
     if doc then
-        local canAccess = false
-        local rights = nil
         local account = user.get(r)
-        -- if private, can we access it?
-        if doc.private then
-            if account then
-                rights = aaa.rights(r, account)
-                canAccess = utils.canAccessDoc(doc, rights)
-            else
-                r:puts(JSON.encode{
-                    error = "You must be logged in to view this email"
-                })
-                return cross.OK
-            end
-        else
-            canAccess = true
-        end
-        if canAccess and doc and doc.mid then
+        if doc and doc.mid and aaa.canAccessDoc(r, doc, account) then
             if not account and config.antispam then
                 doc = anonymize(doc)
             end
             table.insert(emls_thrd, doc)
-            doc.children = fetchChildren(r, doc, 1, nil, rights, account)
+            doc.children = fetchChildren(r, doc, 1, nil, account)
             doc.tid = doc.mid
             doc.id = doc.request_id
             --doc.body = nil