From e4d24f07c9e641a18aede2f556bee4361b7a7af6 Mon Sep 17 00:00:00 2001 From: Jo-Philipp Wich Date: Wed, 28 Jul 2021 09:00:16 +0200 Subject: [PATCH] luci-base: dispatcher: rework dispatching and menu filtering logic - 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 --- .../htdocs/luci-static/resources/ui.js | 2 +- modules/luci-base/luasrc/dispatcher.lua | 303 ++++++++++-------- .../root/usr/share/luci/menu.d/luci-base.json | 3 +- 3 files changed, 168 insertions(+), 140 deletions(-) diff --git a/modules/luci-base/htdocs/luci-static/resources/ui.js b/modules/luci-base/htdocs/luci-static/resources/ui.js index 0e909b6dcc..9cc62c12db 100644 --- a/modules/luci-base/htdocs/luci-static/resources/ui.js +++ b/modules/luci-base/htdocs/luci-static/resources/ui.js @@ -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; } } diff --git a/modules/luci-base/luasrc/dispatcher.lua b/modules/luci-base/luasrc/dispatcher.lua index bd1b112f60..e286430765 100644 --- a/modules/luci-base/luasrc/dispatcher.lua +++ b/modules/luci-base/luasrc/dispatcher.lua @@ -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 = {} diff --git a/modules/luci-base/root/usr/share/luci/menu.d/luci-base.json b/modules/luci-base/root/usr/share/luci/menu.d/luci-base.json index eb72c565dc..2a99684c2c 100644 --- a/modules/luci-base/root/usr/share/luci/menu.d/luci-base.json +++ b/modules/luci-base/root/usr/share/luci/menu.d/luci-base.json @@ -87,7 +87,8 @@ }, "depends": { "acl": [ "luci-base" ] - } + }, + "firstchild_ineligible": true }, "admin/uci": { -- 2.30.2