luci-base: dispatcher: rework dispatching and menu filtering logic
authorJo-Philipp Wich <jo@mein.io>
Wed, 28 Jul 2021 07:00:16 +0000 (09:00 +0200)
committerJo-Philipp Wich <jo@mein.io>
Mon, 11 Oct 2021 18:38:04 +0000 (20:38 +0200)
 - Prefer nodes that do not require authentication over nodes that do
 - Honour ACL dependencies while resolving firstchild nodes
 - Consider currently active session while scanning menu tree instead
   of only loading effective ACLs when a login node is encountered
 - Do not consider nodes for firstchild dispatching which specify a
   special "firstchild_ineligible" property
 - Hide menu nodes that have no accessible children

Signed-off-by: Jo-Philipp Wich <jo@mein.io>
modules/luci-base/htdocs/luci-static/resources/ui.js
modules/luci-base/luasrc/dispatcher.lua
modules/luci-base/root/usr/share/luci/menu.d/luci-base.json

index 0e909b6dcc717dbfd76d25930f1b69300f3a976a..9cc62c12db1a1612e21e07811c6a9dba9af32f8c 100644 (file)
@@ -3021,7 +3021,7 @@ function scrubMenu(node) {
                for (var k in node.children) {
                        var child = scrubMenu(node.children[k]);
 
-                       if (child.title)
+                       if (child.title && !child.firstchild_ineligible)
                                hasSatisfiedChild = hasSatisfiedChild || child.satisfied;
                }
        }
index bd1b112f60cdb6816c01d18b371d78f8596684b3..e286430765be69df5739c7e2bc5e44e3bfc60eba 100644 (file)
@@ -587,64 +587,6 @@ local function check_authentication(method)
        return session_retrieve(sid)
 end
 
-local function get_children(node)
-       local children = {}
-
-       if not node.wildcard and type(node.children) == "table" then
-               for name, child in pairs(node.children) do
-                       children[#children+1] = {
-                               name  = name,
-                               node  = child,
-                               order = child.order or 1000
-                       }
-               end
-
-               table.sort(children, function(a, b)
-                       if a.order == b.order then
-                               return a.name < b.name
-                       else
-                               return a.order < b.order
-                       end
-               end)
-       end
-
-       return children
-end
-
-local function find_subnode(root, prefix, recurse, descended)
-       local children = get_children(root)
-
-       if #children > 0 and (not descended or recurse) then
-               local sub_path = { unpack(prefix) }
-
-               if recurse == false then
-                       recurse = nil
-               end
-
-               for _, child in ipairs(children) do
-                       sub_path[#prefix+1] = child.name
-
-                       local res_path = find_subnode(child.node, sub_path, recurse, true)
-
-                       if res_path then
-                               return res_path
-                       end
-               end
-       end
-
-       if descended then
-               if not recurse or
-                  root.action.type == "cbi" or
-                  root.action.type == "form" or
-                  root.action.type == "view" or
-                  root.action.type == "template" or
-                  root.action.type == "arcombine"
-               then
-                       return prefix
-               end
-       end
-end
-
 local function merge_trees(node_a, node_b)
        for k, v in pairs(node_b) do
                if k == "children" then
@@ -786,85 +728,177 @@ local function init_template_engine(ctx)
        return tpl
 end
 
-function dispatch(request)
-       --context._disable_memtrace = require "luci.debug".trap_memtrace("l")
-       local ctx = context
+local function is_authenticated(auth)
+       if type(auth) == "table" and type(auth.methods) == "table" and #auth.methods > 0 then
+               local sid, sdat, sacl
+               for _, method in ipairs(auth.methods) do
+                       sid, sdat, sacl = check_authentication(method)
 
-       local auth, cors, suid, sgid
-       local menu = menu_json()
-       local page = menu
+                       if sid and sdat and sacl then
+                               return sid, sdat, sacl
+                       end
+               end
+       end
+end
 
-       local requested_path_full = {}
-       local requested_path_node = {}
-       local requested_path_args = {}
+local function ctx_append(ctx, name, node)
+       ctx.path = ctx.path or {}
+       ctx.path[#ctx.path + 1] = name
 
-       local required_path_acls = {}
+       ctx.acls = ctx.acls or {}
 
-       for i, s in ipairs(request) do
-               if type(page.children) ~= "table" or not page.children[s] then
-                       page = nil
-                       break
-               end
+       local acls = (type(node.depends) == "table" and type(node.depends.acl) == "table") and node.depends.acl or {}
+       for _, acl in ipairs(acls) do
+               ctx.acls[_] = acl
+       end
 
-               if not page.children[s].satisfied then
-                       page = nil
-                       break
-               end
+       ctx.auth = node.auth or ctx.auth
+       ctx.cors = node.cors or ctx.cors
+       ctx.suid = node.setuser or ctx.suid
+       ctx.sgid = node.setgroup or ctx.sgid
 
-               page = page.children[s]
-               auth = page.auth or auth
-               cors = page.cors or cors
-               suid = page.setuser or suid
-               sgid = page.setgroup or sgid
+       return ctx
+end
 
-               if type(page.depends) == "table" and type(page.depends.acl) == "table" then
-                       for _, group in ipairs(page.depends.acl) do
-                               local found = false
-                               for _, item in ipairs(required_path_acls) do
-                                       if item == group then
-                                               found = true
-                                               break
+local function node_weight(node)
+       local weight = node.order or 9999
+
+       if weight > 9999 then
+               weight = 9999
+       end
+
+       if type(node.auth) == "table" and node.auth.login then
+               weight = weight + 10000
+       end
+
+       return weight
+end
+
+local function resolve_firstchild(node, sacl, login_allowed, ctx)
+       local candidate = nil
+       local candidate_ctx = nil
+
+       for name, child in pairs(node.children) do
+               if child.satisfied then
+                       if not sacl then
+                               local _
+                               _, _, sacl = is_authenticated(node.auth)
+                       end
+
+                       local cacl = (type(child.depends) == "table") and child.depends.acl or nil
+                       local login = login_allowed or (type(child.auth) == "table" and child.auth.login)
+                       if login or check_acl_depends(cacl, sacl and sacl["access-group"]) ~= nil then
+                               if child.title and type(child.action) == "table" then
+                                       local child_ctx = ctx_append(util.clone(ctx, true), name, child)
+                                       if child.action.type == "firstchild" then
+                                               if not candidate or node_weight(candidate) > node_weight(child) then
+                                                       local have_grandchild = resolve_firstchild(child, sacl, login, child_ctx)
+                                                       if have_grandchild then
+                                                               candidate = child
+                                                               candidate_ctx = child_ctx
+                                                       end
+                                               end
+                                       elseif not child.firstchild_ineligible then
+                                               if not candidate or node_weight(candidate) > node_weight(child) then
+                                                       candidate = child
+                                                       candidate_ctx = child_ctx
+                                               end
                                        end
                                end
-                               if not found then
-                                       required_path_acls[#required_path_acls + 1] = group
-                               end
                        end
                end
+       end
+
+       if candidate then
+               for k, v in pairs(candidate_ctx) do
+                       ctx[k] = v
+               end
+
+               return true
+       end
+
+       return false
+end
+
+local function resolve_page(tree, request_path)
+       local node = tree
+       local sacl = nil
+       local login = false
+       local ctx = {}
+
+       for i, s in ipairs(request_path) do
+               node = node.children and node.children[s]
+
+               if not node or not node.satisfied then
+                       break
+               end
+
+               ctx_append(ctx, s, node)
+
+               if not sacl then
+                       local _
+                       _, _, sacl = is_authenticated(node.auth)
+               end
+
+               if not login and type(node.auth) == "table" and node.auth.login then
+                       login = true
+               end
 
-               requested_path_full[i] = s
-               requested_path_node[i] = s
+               if node.wildcard then
+                       ctx.request_args = {}
+                       ctx.request_path = util.clone(ctx.path, true)
 
-               if page.wildcard then
-                       for j = i + 1, #request do
-                               requested_path_args[j - i] = request[j]
-                               requested_path_full[j] = request[j]
+                       for j = i + 1, #request_path do
+                               ctx.request_path[j] = request_path[j]
+                               ctx.request_args[j - i] = request_path[j]
                        end
+
                        break
                end
        end
 
+       if node and type(node.action) == "table" and node.action.type == "firstchild" then
+               resolve_firstchild(node, sacl, login, ctx)
+       end
+
+       ctx.acls = ctx.acls or {}
+       ctx.path = ctx.path or {}
+       ctx.request_args = ctx.request_args or {}
+       ctx.request_path = ctx.request_path or util.clone(request_path, true)
+
+       node = tree
+
+       for _, s in ipairs(ctx.path or {}) do
+               node = node.children[s]
+               assert(node, "Internal node resolve error")
+       end
+
+       return node, ctx
+end
+
+function dispatch(request)
+       --context._disable_memtrace = require "luci.debug".trap_memtrace("l")
+       local ctx = context
+
+       local auth, cors, suid, sgid
+       local menu = menu_json()
+       local page, lookup_ctx = resolve_page(menu, request)
+       local action = (page and type(page.action) == "table") and page.action or {}
+
        local tpl = init_template_engine(ctx)
 
-       ctx.args = requested_path_args
-       ctx.path = requested_path_node
+       ctx.args = lookup_ctx.request_args
+       ctx.path = lookup_ctx.path
        ctx.dispatched = page
 
-       ctx.requestpath = ctx.requestpath or requested_path_full
-       ctx.requestargs = ctx.requestargs or requested_path_args
+       ctx.requestpath = ctx.requestpath or lookup_ctx.request_path
+       ctx.requestargs = ctx.requestargs or lookup_ctx.request_args
        ctx.requested = ctx.requested or page
 
-       if type(auth) == "table" and type(auth.methods) == "table" and #auth.methods > 0 then
-               local sid, sdat, sacl
-               for _, method in ipairs(auth.methods) do
-                       sid, sdat, sacl = check_authentication(method)
+       if type(lookup_ctx.auth) == "table" and next(lookup_ctx.auth) then
+               local sid, sdat, sacl = is_authenticated(lookup_ctx.auth)
 
-                       if sid and sdat and sacl then
-                               break
-                       end
-               end
-
-               if not (sid and sdat and sacl) and auth.login then
+               if not (sid and sdat and sacl) and lookup_ctx.auth.login then
                        local user = http.getenv("HTTP_AUTH_USER")
                        local pass = http.getenv("HTTP_AUTH_PASS")
 
@@ -911,8 +945,8 @@ function dispatch(request)
                ctx.authacl = sacl
        end
 
-       if #required_path_acls > 0 then
-               local perm = check_acl_depends(required_path_acls, ctx.authacl and ctx.authacl["access-group"])
+       if #lookup_ctx.acls > 0 then
+               local perm = check_acl_depends(lookup_ctx.acls, ctx.authacl and ctx.authacl["access-group"])
                if perm == nil then
                        http.status(403, "Forbidden")
                        return
@@ -923,13 +957,11 @@ function dispatch(request)
                end
        end
 
-       local action = (page and type(page.action) == "table") and page.action or {}
-
        if action.type == "arcombine" then
-               action = (#requested_path_args > 0) and action.targets[2] or action.targets[1]
+               action = (#lookup_ctx.request_args > 0) and action.targets[2] or action.targets[1]
        end
 
-       if cors and http.getenv("REQUEST_METHOD") == "OPTIONS" then
+       if lookup_ctx.cors and http.getenv("REQUEST_METHOD") == "OPTIONS" then
                luci.http.status(200, "OK")
                luci.http.header("Access-Control-Allow-Origin", http.getenv("HTTP_ORIGIN") or "*")
                luci.http.header("Access-Control-Allow-Methods", "GET, POST, OPTIONS")
@@ -942,12 +974,12 @@ function dispatch(request)
                end
        end
 
-       if sgid then
-               sys.process.setgroup(sgid)
+       if lookup_ctx.sgid then
+               sys.process.setgroup(lookup_ctx.sgid)
        end
 
-       if suid then
-               sys.process.setuser(suid)
+       if lookup_ctx.suid then
+               sys.process.setuser(lookup_ctx.suid)
        end
 
        if action.type == "view" then
@@ -970,7 +1002,7 @@ function dispatch(request)
                       'of type "' .. type(func) .. '".')
 
                local argv = (type(action.parameters) == "table" and #action.parameters > 0) and { unpack(action.parameters) } or {}
-               for _, s in ipairs(requested_path_args) do
+               for _, s in ipairs(lookup_ctx.request_args) do
                        argv[#argv + 1] = s
                end
 
@@ -979,13 +1011,8 @@ function dispatch(request)
                        error500(err)
                end
 
-       elseif action.type == "firstchild" then
-               local sub_request = find_subnode(page, requested_path_full, action.recurse)
-               if sub_request then
-                       dispatch(sub_request)
-               else
-                       tpl.render("empty_node_placeholder", getfenv(1))
-               end
+       --elseif action.type == "firstchild" then
+       --      tpl.render("empty_node_placeholder", getfenv(1))
 
        elseif action.type == "alias" then
                local sub_request = {}
@@ -993,7 +1020,7 @@ function dispatch(request)
                        sub_request[#sub_request + 1] = name
                end
 
-               for _, s in ipairs(requested_path_args) do
+               for _, s in ipairs(lookup_ctx.request_args) do
                        sub_request[#sub_request + 1] = s
                end
 
@@ -1011,7 +1038,7 @@ function dispatch(request)
                        n = n + 1
                end
 
-               for _, s in ipairs(requested_path_args) do
+               for _, s in ipairs(lookup_ctx.request_args) do
                        sub_request[#sub_request + 1] = s
                end
 
@@ -1021,19 +1048,18 @@ function dispatch(request)
                tpl.render(action.path, getfenv(1))
 
        elseif action.type == "cbi" then
-               _cbi({ config = action.config, model = action.path }, unpack(requested_path_args))
+               _cbi({ config = action.config, model = action.path }, unpack(lookup_ctx.request_args))
 
        elseif action.type == "form" then
-               _form({ model = action.path }, unpack(requested_path_args))
+               _form({ model = action.path }, unpack(lookup_ctx.request_args))
 
        else
-               local root = find_subnode(menu, {}, true)
-               if not root then
+               if not menu.children then
                        error404("No root node was registered, this usually happens if no module was installed.\n" ..
                                 "Install luci-mod-admin-full and retry. " ..
                                 "If the module is already installed, try removing the /tmp/luci-indexcache file.")
                else
-                       error404("No page is registered at '/" .. table.concat(requested_path_full, "/") .. "'.\n" ..
+                       error404("No page is registered at '/" .. table.concat(lookup_ctx.request_path, "/") .. "'.\n" ..
                                 "If this url belongs to an extension, make sure it is properly installed.\n" ..
                                 "If the extension was recently installed, try removing the /tmp/luci-indexcache file.")
                end
@@ -1135,7 +1161,8 @@ function createtree_json()
                setgroup = "string",
                setuser = "string",
                title = "string",
-               wildcard = "boolean"
+               wildcard = "boolean",
+               firstchild_ineligible = "boolean"
        }
 
        local files = {}
index eb72c565dca3126232ca3ff61ae55cd77fb34025..2a99684c2ce05c15e9e5cba6f2f3086c3d181dc5 100644 (file)
@@ -87,7 +87,8 @@
                },
                "depends": {
                        "acl": [ "luci-base" ]
-               }
+               },
+               "firstchild_ineligible": true
        },
 
        "admin/uci": {