treewide: rework rollback/apply workflow
authorJo-Philipp Wich <jo@mein.io>
Thu, 26 Jul 2018 20:12:45 +0000 (22:12 +0200)
committerJo-Philipp Wich <jo@mein.io>
Sat, 28 Jul 2018 15:14:22 +0000 (17:14 +0200)
Rework the apply confirmation mechanism to be session agnostic in order to
circumvent cross domain restrictions which prevent the JS code from issuing
apply confirm requests in some cases, e.g. when changing the LAN IP.

Confirmation calls may now be done from unauthenticated pages, as long as a
matching confirmation token is sent along with the request.

The reasoning behind this is that there is little security impact in
confirming pending apply sessions, especially since those sessions can only
be initiated while being authenticated.

After this change, LuCI will now launch a confirmation process on every
rendered page when a rollback is pending. The confirmation will happen
regardless of whether the user is logged in or not, or if the current page
is a CBI form or static template.

A confirmation request now also requires a random one-time token which is
rendered along with the confirmation JavaScript code in order to succeed.

This token is not meant to provide security but to ensure that the confirm
was triggered from an interactive browser session and not some background
HTTP requests that happened to end up in the admin ui.

As a consequence, the different apply/confirm/rollback code paths in CBI
maps and the UCI change/revert pages have been consolidated into one common
implementation residing in the common global theme agnostic footer template.

Signed-off-by: Jo-Philipp Wich <jo@mein.io>
(cherry picked from commit e5a1ac02289e8fde8ddbd05bbb21ac448c661ae3)

modules/luci-base/luasrc/dispatcher.lua
modules/luci-base/luasrc/model/uci.lua
modules/luci-base/luasrc/view/cbi/apply_widget.htm
modules/luci-base/luasrc/view/cbi/map.htm
modules/luci-base/luasrc/view/footer.htm
modules/luci-mod-admin-full/luasrc/controller/admin/uci.lua
modules/luci-mod-admin-full/luasrc/view/admin_uci/changes.htm
modules/luci-mod-admin-full/luasrc/view/admin_uci/revert.htm

index 6d5a8f4d3db655680a626aa44bc0938357e1c624..6cf2712eb4a862c8771ce68d7d34fe966e9ce68e 100644 (file)
@@ -893,8 +893,6 @@ local function _cbi(self, ...)
        local pageaction = true
        local parsechain = { }
 
-       local is_rollback, time_remaining = uci:rollback_pending()
-
        for i, res in ipairs(maps) do
                if res.apply_needed and res.parsechain then
                        local c
@@ -921,8 +919,6 @@ local function _cbi(self, ...)
        for i, res in ipairs(maps) do
                res:render({
                        firstmap   = (i == 1),
-                       applymap   = applymap,
-                       confirmmap = (is_rollback and time_remaining or nil),
                        redirect   = redirect,
                        messages   = messages,
                        pageaction = pageaction,
@@ -932,11 +928,12 @@ local function _cbi(self, ...)
 
        if not config.nofooter then
                tpl.render("cbi/footer", {
-                       flow       = config,
-                       pageaction = pageaction,
-                       redirect   = redirect,
-                       state      = state,
-                       autoapply  = config.autoapply
+                       flow          = config,
+                       pageaction    = pageaction,
+                       redirect      = redirect,
+                       state         = state,
+                       autoapply     = config.autoapply,
+                       trigger_apply = applymap
                })
        end
 end
index 92c0d8f69998a19c61d3e1827dd3363b91900298..b2c1e463bf97db59b0515195a22e56406884e99f 100644 (file)
@@ -147,19 +147,31 @@ function apply(self, rollback)
        local _, err
 
        if rollback then
+               local sys = require "luci.sys"
                local conf = require "luci.config"
-               local timeout = tonumber(conf and conf.apply and conf.apply.rollback or "") or 0
+               local timeout = tonumber(conf and conf.apply and conf.apply.rollback or 30) or 0
 
                _, err = call("apply", {
-                       timeout  = (timeout > 30) and timeout or 30,
+                       timeout = (timeout > 30) and timeout or 30,
                        rollback = true
                })
 
                if not err then
+                       local now = os.time()
+                       local token = sys.uniqueid(16)
+
                        util.ubus("session", "set", {
-                               ubus_rpc_session = session_id,
-                               values = { rollback = os.time() + timeout }
+                               ubus_rpc_session = "00000000000000000000000000000000",
+                               values = {
+                                       rollback = {
+                                               token   = token,
+                                               session = session_id,
+                                               timeout = now + timeout
+                                       }
+                               }
                        })
+
+                       return token
                end
        else
                _, err = call("changes", {})
@@ -184,40 +196,72 @@ function apply(self, rollback)
        return (err == nil), ERRSTR[err]
 end
 
-function confirm(self)
-       local _, err = call("confirm", {})
-       if not err then
-               util.ubus("session", "set", {
-                       ubus_rpc_session = session_id,
-                       values = { rollback = 0 }
+function confirm(self, token)
+       local is_pending, time_remaining, rollback_sid, rollback_token = self:rollback_pending()
+
+       if is_pending then
+               if token ~= rollback_token then
+                       return false, "Permission denied"
+               end
+
+               local _, err = util.ubus("uci", "confirm", {
+                       ubus_rpc_session = rollback_sid
                })
+
+               if not err then
+                       util.ubus("session", "set", {
+                               ubus_rpc_session = "00000000000000000000000000000000",
+                               values = { rollback = {} }
+                       })
+               end
+
+               return (err == nil), ERRSTR[err]
        end
-       return (err == nil), ERRSTR[err]
+
+       return false, "No data"
 end
 
 function rollback(self)
-       local _, err = call("rollback", {})
-       if not err then
-               util.ubus("session", "set", {
-                       ubus_rpc_session = session_id,
-                       values = { rollback = 0 }
+       local is_pending, time_remaining, rollback_sid = self:rollback_pending()
+
+       if is_pending then
+               local _, err = util.ubus("uci", "rollback", {
+                       ubus_rpc_session = rollback_sid
                })
+
+               if not err then
+                       util.ubus("session", "set", {
+                               ubus_rpc_session = "00000000000000000000000000000000",
+                               values = { rollback = {} }
+                       })
+               end
+
+               return (err == nil), ERRSTR[err]
        end
-       return (err == nil), ERRSTR[err]
+
+       return false, "No data"
 end
 
 function rollback_pending(self)
-       local deadline, err = util.ubus("session", "get", {
-               ubus_rpc_session = session_id,
+       local rv, err = util.ubus("session", "get", {
+               ubus_rpc_session = "00000000000000000000000000000000",
                keys = { "rollback" }
        })
 
-       if type(deadline) == "table" and
-          type(deadline.values) == "table" and
-          type(deadline.values.rollback) == "number" and
-          deadline.values.rollback > os.time()
+       local now = os.time()
+
+       if type(rv) == "table" and
+          type(rv.values) == "table" and
+          type(rv.values.rollback) == "table" and
+          type(rv.values.rollback.token) == "string" and
+          type(rv.values.rollback.session) == "string" and
+          type(rv.values.rollback.timeout) == "number" and
+          rv.values.rollback.timeout > now
        then
-               return true, deadline.values.rollback - os.time()
+               return true,
+                       rv.values.rollback.timeout - now,
+                       rv.values.rollback.session,
+                       rv.values.rollback.token
        end
 
        return false, ERRSTR[err]
index f76846ee8719181ec348fd69a9e032abc203e008..4d7e9c56ea669b321fd3a10babf25c44cf274f3f 100644 (file)
@@ -1,4 +1,4 @@
-<% export("cbi_apply_widget", function(redirect_ok) -%>
+<% export("cbi_apply_widget", function(redirect_ok, rollback_token) -%>
 <style type="text/css">
        #cbi_apply_overlay {
                position: absolute;
@@ -51,6 +51,7 @@
            uci_apply_holdoff = <%=math.max(luci.config and luci.config.apply and luci.config.apply.holdoff or 4, 1)%>,
            uci_apply_timeout = <%=math.max(luci.config and luci.config.apply and luci.config.apply.timeout or 5, 1)%>,
            uci_apply_display = <%=math.max(luci.config and luci.config.apply and luci.config.apply.display or 1.5, 1)%>,
+           uci_confirm_auth = <% if rollback_token then %>{ token: '<%=rollback_token%>' }<% else %>null<% end %>,
            was_xhr_poll_running = false;
 
        function uci_status_message(type, content) {
 
                        var delay = isNaN(duration) ? 0 : Math.max(1000 - duration, 0);
                        window.setTimeout(function() {
-                               xhr.post('<%=url("admin/uci/confirm")%>', uci_apply_auth, call, uci_apply_timeout * 1000);
+                               xhr.post('<%=url("admin/uci/confirm")%>', uci_confirm_auth, call, uci_apply_timeout * 1000);
                        }, delay);
                };
 
                        '<img src="<%=resource%>/icons/loading.gif" alt="" style="vertical-align:middle" /> ' +
                        '<%:Starting configuration apply…%>');
 
-               xhr.post('<%=url("admin/uci")%>/' + (checked ? 'apply_rollback' : 'apply_unchecked'), uci_apply_auth, function(r) {
+               xhr.post('<%=url("admin/uci")%>/' + (checked ? 'apply_rollback' : 'apply_unchecked'), uci_apply_auth, function(r, tok) {
                        if (r.status === (checked ? 200 : 204)) {
+                               if (checked && tok !== null && typeof(tok) === 'object' && typeof(tok.token) === 'string')
+                                       uci_confirm_auth = tok;
+
                                uci_confirm(checked, Date.now() + uci_apply_rollback * 1000);
                        }
                        else if (checked && r.status === 204) {
index 83c3cb2170adaf8330bcf0b8bde86a0a5d43ab8c..d65a16167342f8dd5c90b8482662d79f20eb7172 100644 (file)
@@ -5,21 +5,6 @@
 <div class="cbi-map" id="cbi-<%=self.config%>">
        <% if self.title and #self.title > 0 then %><h2 name="content"><%=self.title%></h2><% end %>
        <% if self.description and #self.description > 0 then %><div class="cbi-map-descr"><%=self.description%></div><% end %>
-       <%- if firstmap and (applymap or confirmmap) then -%>
-               <%+cbi/apply_widget%>
-               <% cbi_apply_widget(redirect) %>
-               <div class="alert-message" id="cbi_apply_status" style="display:none"></div>
-               <script type="text/javascript">
-                       document.addEventListener("DOMContentLoaded", function() {
-                               <% if confirmmap then -%>
-                                       uci_confirm(true, Date.now() + <%=confirmmap%> * 1000);
-                               <%- else -%>
-                                       uci_apply(true);
-                               <%- end %>
-                       });
-               </script>
-       <%- end -%>
-
        <% if self.tabbed then %>
                <ul class="cbi-tabmenu map">
                        <%- self.selected_tab = luci.http.formvalue("tab.m-" .. self.config) %>
index f3574b6b1072e2a3d65b19ef76387bda1e391069..d268d71cfffd436875ca1bc898dfe495486e0bc0 100644 (file)
@@ -4,4 +4,27 @@
  Licensed to the public under the Apache License 2.0.
 -%>
 
-<% include("themes/" .. theme .. "/footer") %>
\ No newline at end of file
+<%
+       include("themes/" .. theme .. "/footer")
+
+       local is_rollback_pending, rollback_time_remaining, rollback_session, rollback_token = luci.model.uci:rollback_pending()
+
+       if is_rollback_pending or trigger_apply or trigger_revert then
+               include("cbi/apply_widget")
+               cbi_apply_widget(redirect, rollback_token)
+%>
+       <div class="alert-message" id="cbi_apply_status" style="display:none"></div>
+       <script type="text/javascript">
+               document.addEventListener("DOMContentLoaded", function() {
+                       <% if trigger_apply then -%>
+                               uci_apply(true);
+                       <%- elseif trigger_revert then -%>
+                               uci_revert();
+                       <%- else -%>
+                               uci_confirm(true, Date.now() + <%=rollback_time_remaining%> * 1000);
+                       <%- end %>
+               });
+       </script>
+<%
+       end
+%>
index 9533ff5e6ea3d89bab823e1a7f3368df42c226ca..1d955dd982deb82a5e4559abec7cf28358aa8183 100644 (file)
@@ -9,7 +9,7 @@ function index()
                or table.concat(luci.dispatcher.context.request, "/")
 
        entry({"admin", "uci"}, nil, _("Configuration"))
-       entry({"admin", "uci", "changes"}, call("action_changes"), _("Changes"), 40).query = {redir=redir}
+       entry({"admin", "uci", "changes"}, post_on({ trigger_apply = true }, "action_changes"), _("Changes"), 40).query = {redir=redir}
        entry({"admin", "uci", "revert"}, post("action_revert"), _("Revert"), 30).query = {redir=redir}
 
        local node
@@ -25,9 +25,9 @@ function index()
        node.cors = true
        node.sysauth_authenticator = authen
 
-       node = entry({"admin", "uci", "confirm"}, post("action_confirm"), nil)
+       node = entry({"admin", "uci", "confirm"}, call("action_confirm"), nil)
        node.cors = true
-       node.sysauth_authenticator = authen
+       node.sysauth = false
 end
 
 
@@ -36,8 +36,9 @@ function action_changes()
        local changes = uci:changes()
 
        luci.template.render("admin_uci/changes", {
-               changes  = next(changes) and changes,
-               timeout  = timeout
+               changes       = next(changes) and changes,
+               timeout       = timeout,
+               trigger_apply = luci.http.formvalue("trigger_apply") and true or false
        })
 end
 
@@ -52,7 +53,8 @@ function action_revert()
        end
 
        luci.template.render("admin_uci/revert", {
-               changes = next(changes) and changes
+               changes        = next(changes) and changes,
+               trigger_revert = true
        })
 end
 
@@ -84,8 +86,13 @@ end
 
 function action_apply_rollback()
        local uci = require "luci.model.uci"
-       local _, errstr = uci:apply(true)
-       ubus_state_to_http(errstr)
+       local token, errstr = uci:apply(true)
+       if token then
+               luci.http.prepare_content("application/json")
+               luci.http.write_json({ token = token })
+       else
+               ubus_state_to_http(errstr)
+       end
 end
 
 function action_apply_unchecked()
@@ -96,6 +103,7 @@ end
 
 function action_confirm()
        local uci = require "luci.model.uci"
-       local _, errstr = uci:confirm()
+       local token = luci.http.formvalue("token")
+       local _, errstr = uci:confirm(token)
        ubus_state_to_http(errstr)
 end
index 6282244757599a1c34842fc42c6b951b89101c39..43bd7c23fb60cc178a7812f3c83fbeab3318b30e 100644 (file)
@@ -8,11 +8,9 @@
 
 <%-
        local node, redir_url = luci.dispatcher.lookup(luci.http.formvalue("redir"))
+       export("redirect", redir_url or url("admin/uci/changes"))
 
-       include("cbi/apply_widget")
        include("admin_uci/changelog")
-
-       cbi_apply_widget(redir_url or url("admin/uci/changes"))
 -%>
 
 <h2 name="content"><%:Configuration%> / <%:Changes%></h2>
        </form>
        <% end %>
 
-       <input class="cbi-button cbi-button-save" type="button" id="apply_button" value="<%:Save & Apply%>" onclick="uci_apply(true); this.blur()" />
+       <form method="post" action="<%=url("admin/uci/changes")%>">
+               <input type="hidden" name="token" value="<%=token%>" />
+               <input type="hidden" name="redir" value="<%=pcdata(luci.http.formvalue("redir"))%>" />
+               <input class="cbi-button cbi-button-save" type="submit" name="trigger_apply" value="<%:Save & Apply%>" />
+       </form>
        <form method="post" action="<%=url("admin/uci/revert")%>">
                <input type="hidden" name="token" value="<%=token%>" />
                <input type="hidden" name="redir" value="<%=pcdata(luci.http.formvalue("redir"))%>" />
index ff23d568dcb35f7da1996d9eba8e522379f95591..d8fd3de01ed4311c1b8ac2a47b6476ffe49a49b1 100644 (file)
@@ -8,11 +8,9 @@
 
 <%-
        local node, redir_url = luci.dispatcher.lookup(luci.http.formvalue("redir"))
+       export("redirect", redir_url or url("admin/uci/changes"))
 
-       include("cbi/apply_widget")
        include("admin_uci/changelog")
-
-       cbi_apply_widget(redir_url or url("admin/uci/revert"))
 -%>
 
 <h2 name="content"><%:Configuration%> / <%:Revert%></h2>
        <p><strong><%:There are no pending changes to revert!%></strong></p>
 <% end %>
 
-<div class="alert-message" id="cbi_apply_status" style="display:none"></div>
-<script type="text/javascript">
-       document.addEventListener("DOMContentLoaded", function() {
-               uci_revert();
-       });
-</script>
-
 <% if redir_url then %>
        <div class="cbi-page-actions">
                <form class="inline" method="get" action="<%=luci.util.pcdata(redir_url)%>">