From d11084d32fb3c8230672e627cc3f774fa7aa74ff Mon Sep 17 00:00:00 2001 From: ByeongUk Choi Date: Thu, 28 Apr 2022 20:01:53 +0900 Subject: [PATCH] Add shared mode to Livebook sessions (#1128) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * add shared mode (working) * add markdown write permission * Add authorize handle_event * Revert "add shared mode (working)" This reverts commit c1e9ebac4d71342140bdb9dc2545cb7952644d2e. * Add assert_policy! * Apply suggestions code review * Apply suggestions from code review * Apply suggestions from code reviews (redirect) * Apply suggestions from code review * Apply suggestions from code review * Apply suggestions from code review * Apply suggestions from code review * Add PolicyHook * remove assert_live_action_access!/1 * Apply suggestions from code review * Apply suggestions from code review * Update lib/livebook_web/live/session_live.ex Co-authored-by: Jonatan Kłosko --- lib/livebook_web/live/hooks/policy_hook.ex | 7 ++ lib/livebook_web/live/session_live.ex | 82 ++++++++++++++++------ lib/livebook_web/router.ex | 3 +- 3 files changed, 68 insertions(+), 24 deletions(-) create mode 100644 lib/livebook_web/live/hooks/policy_hook.ex diff --git a/lib/livebook_web/live/hooks/policy_hook.ex b/lib/livebook_web/live/hooks/policy_hook.ex new file mode 100644 index 000000000..f876eb5a4 --- /dev/null +++ b/lib/livebook_web/live/hooks/policy_hook.ex @@ -0,0 +1,7 @@ +defmodule LivebookWeb.PolicyHook do + import Phoenix.LiveView + + def on_mount(:private, _params, _session, socket) do + {:cont, socket |> assign(:policy, %{read: true, execute: true, comment: true, edit: true})} + end +end diff --git a/lib/livebook_web/live/session_live.ex b/lib/livebook_web/live/session_live.ex index a3c32edb1..143625465 100644 --- a/lib/livebook_web/live/session_live.ex +++ b/lib/livebook_web/live/session_live.ex @@ -549,12 +549,14 @@ defmodule LivebookWeb.SessionLive do end @impl true - def handle_params(%{"cell_id" => cell_id}, _url, socket) do + def handle_params(%{"cell_id" => cell_id}, _url, socket) + when socket.assigns.live_action in [:cell_settings, :cell_upload] do {:ok, cell, _} = Notebook.fetch_cell_and_section(socket.private.data.notebook, cell_id) {:noreply, assign(socket, cell: cell)} end - def handle_params(%{"section_id" => section_id}, _url, socket) do + def handle_params(%{"section_id" => section_id}, _url, socket) + when socket.assigns.live_action == :delete_section do {:ok, section} = Notebook.fetch_section(socket.private.data.notebook, section_id) first_section_id = hd(socket.private.data.notebook.sections).id {:noreply, assign(socket, section: section, first_section_id: first_section_id)} @@ -565,17 +567,22 @@ defmodule LivebookWeb.SessionLive do _url, %{assigns: %{live_action: :catch_all}} = socket ) do - path_parts = - Enum.map(path_parts, fn - "__parent__" -> ".." - part -> part - end) + if socket.assigns.policy.edit do + path_parts = + Enum.map(path_parts, fn + "__parent__" -> ".." + part -> part + end) - path = Path.join(path_parts) - {:noreply, handle_relative_path(socket, path)} + path = Path.join(path_parts) + {:noreply, handle_relative_path(socket, path)} + else + {:noreply, socket |> put_flash(:error, "No access to navigate") |> redirect_to_self()} + end end - def handle_params(%{"tab" => tab}, _url, socket) do + def handle_params(%{"tab" => tab}, _url, socket) + when socket.assigns.live_action == :export do {:noreply, assign(socket, tab: tab)} end @@ -585,6 +592,7 @@ defmodule LivebookWeb.SessionLive do @impl true def handle_event("append_section", %{}, socket) do + assert_policy!(socket, :edit) idx = length(socket.private.data.notebook.sections) Session.insert_section(socket.assigns.session.pid, idx) @@ -592,6 +600,8 @@ defmodule LivebookWeb.SessionLive do end def handle_event("insert_section_below", params, socket) do + assert_policy!(socket, :edit) + with {:ok, section, index} <- section_with_next_index( socket.private.data.notebook, @@ -609,18 +619,21 @@ defmodule LivebookWeb.SessionLive do %{"section_id" => section_id, "parent_id" => parent_id}, socket ) do + assert_policy!(socket, :edit) Session.set_section_parent(socket.assigns.session.pid, section_id, parent_id) {:noreply, socket} end def handle_event("unset_section_parent", %{"section_id" => section_id}, socket) do + assert_policy!(socket, :edit) Session.unset_section_parent(socket.assigns.session.pid, section_id) {:noreply, socket} end def handle_event("insert_cell_below", params, socket) do + assert_policy!(socket, :edit) {type, attrs} = cell_type_and_attrs_from_params(params) with {:ok, section, index} <- @@ -636,12 +649,14 @@ defmodule LivebookWeb.SessionLive do end def handle_event("delete_cell", %{"cell_id" => cell_id}, socket) do + assert_policy!(socket, :edit) Session.delete_cell(socket.assigns.session.pid, cell_id) {:noreply, socket} end def handle_event("set_notebook_name", %{"value" => name}, socket) do + assert_policy!(socket, :edit) name = normalize_name(name) Session.set_notebook_name(socket.assigns.session.pid, name) @@ -649,6 +664,7 @@ defmodule LivebookWeb.SessionLive do end def handle_event("set_section_name", %{"metadata" => section_id, "value" => name}, socket) do + assert_policy!(socket, :edit) name = normalize_name(name) Session.set_section_name(socket.assigns.session.pid, section_id, name) @@ -660,6 +676,7 @@ defmodule LivebookWeb.SessionLive do %{"cell_id" => cell_id, "tag" => tag, "delta" => delta, "revision" => revision}, socket ) do + assert_policy!(socket, :edit) tag = String.to_atom(tag) delta = Delta.from_compressed(delta) Session.apply_cell_delta(socket.assigns.session.pid, cell_id, tag, delta, revision) @@ -672,6 +689,7 @@ defmodule LivebookWeb.SessionLive do %{"cell_id" => cell_id, "tag" => tag, "revision" => revision}, socket ) do + assert_policy!(socket, :read) tag = String.to_atom(tag) Session.report_cell_revision(socket.assigns.session.pid, cell_id, tag, revision) @@ -679,6 +697,7 @@ defmodule LivebookWeb.SessionLive do end def handle_event("move_cell", %{"cell_id" => cell_id, "offset" => offset}, socket) do + assert_policy!(socket, :edit) offset = ensure_integer(offset) Session.move_cell(socket.assigns.session.pid, cell_id, offset) @@ -686,6 +705,7 @@ defmodule LivebookWeb.SessionLive do end def handle_event("move_section", %{"section_id" => section_id, "offset" => offset}, socket) do + assert_policy!(socket, :edit) offset = ensure_integer(offset) Session.move_section(socket.assigns.session.pid, section_id, offset) @@ -693,6 +713,8 @@ defmodule LivebookWeb.SessionLive do end def handle_event("delete_section", %{"section_id" => section_id}, socket) do + assert_policy!(socket, :edit) + socket = case Notebook.fetch_section(socket.private.data.notebook, section_id) do {:ok, %{cells: []} = section} -> @@ -718,6 +740,7 @@ defmodule LivebookWeb.SessionLive do end def handle_event("convert_smart_cell", %{"cell_id" => cell_id}, socket) do + assert_policy!(socket, :edit) Session.convert_smart_cell(socket.assigns.session.pid, cell_id) {:noreply, socket} @@ -728,6 +751,8 @@ defmodule LivebookWeb.SessionLive do %{"kind" => kind, "variant_idx" => variant_idx}, socket ) do + assert_policy!(socket, :edit) + with %{requirement: %{variants: variants}} <- Enum.find(socket.private.data.smart_cell_definitions, &(&1.kind == kind)), {:ok, %{dependencies: dependencies}} <- Enum.fetch(variants, variant_idx) do @@ -744,6 +769,7 @@ defmodule LivebookWeb.SessionLive do end def handle_event("queue_cell_evaluation", %{"cell_id" => cell_id}, socket) do + assert_policy!(socket, :execute) data = socket.private.data {status, socket} = @@ -763,24 +789,29 @@ defmodule LivebookWeb.SessionLive do end def handle_event("queue_section_evaluation", %{"section_id" => section_id}, socket) do + assert_policy!(socket, :execute) Session.queue_section_evaluation(socket.assigns.session.pid, section_id) {:noreply, socket} end def handle_event("queue_full_evaluation", %{"forced_cell_ids" => forced_cell_ids}, socket) do + assert_policy!(socket, :execute) Session.queue_full_evaluation(socket.assigns.session.pid, forced_cell_ids) {:noreply, socket} end def handle_event("cancel_cell_evaluation", %{"cell_id" => cell_id}, socket) do + assert_policy!(socket, :execute) Session.cancel_cell_evaluation(socket.assigns.session.pid, cell_id) {:noreply, socket} end def handle_event("save", %{}, socket) do + assert_policy!(socket, :edit) + if socket.private.data.file do Session.save(socket.assigns.session.pid) {:noreply, socket} @@ -793,16 +824,19 @@ defmodule LivebookWeb.SessionLive do end def handle_event("reconnect_runtime", %{}, socket) do + assert_policy!(socket, :execute) {_, socket} = maybe_reconnect_runtime(socket) {:noreply, socket} end def handle_event("connect_runtime", %{}, socket) do + assert_policy!(socket, :execute) {_, socket} = connect_runtime(socket) {:noreply, socket} end def handle_event("setup_default_runtime", %{}, socket) do + assert_policy!(socket, :execute) {status, socket} = connect_runtime(socket) if status == :ok do @@ -813,11 +847,14 @@ defmodule LivebookWeb.SessionLive do end def handle_event("disconnect_runtime", %{}, socket) do + assert_policy!(socket, :execute) Session.disconnect_runtime(socket.assigns.session.pid) {:noreply, socket} end def handle_event("intellisense_request", %{"cell_id" => cell_id} = params, socket) do + assert_policy!(socket, :read) + request = case params do %{"type" => "completion", "hint" => hint} -> @@ -864,6 +901,7 @@ defmodule LivebookWeb.SessionLive do end def handle_event("fork_session", %{}, socket) do + assert_policy!(socket, :edit) %{pid: pid, images_dir: images_dir} = socket.assigns.session # Fetch the data, as we don't keep cells' source in the state data = Session.get_data(pid) @@ -872,11 +910,14 @@ defmodule LivebookWeb.SessionLive do end def handle_event("erase_outputs", %{}, socket) do + assert_policy!(socket, :execute) Session.erase_outputs(socket.assigns.session.pid) {:noreply, socket} end def handle_event("location_report", report, socket) do + assert_policy!(socket, :read) + Phoenix.PubSub.broadcast_from( Livebook.PubSub, self(), @@ -887,19 +928,6 @@ defmodule LivebookWeb.SessionLive do {:noreply, socket} end - def handle_event("format_code", %{"code" => code}, socket) do - formatted = - try do - code - |> Code.format_string!() - |> IO.iodata_to_binary() - rescue - _ -> code - end - - {:reply, %{code: formatted}, socket} - end - @impl true def handle_info({:operation, operation}, socket) do {:noreply, handle_operation(socket, operation)} @@ -1657,4 +1685,12 @@ defmodule LivebookWeb.SessionLive do }} end) end + + defp assert_policy!(socket, key) do + unless socket.assigns.policy |> Map.fetch!(key) do + raise "policy not allowed" + end + + :ok + end end diff --git a/lib/livebook_web/router.ex b/lib/livebook_web/router.ex index c75d68aae..070e4ac7e 100644 --- a/lib/livebook_web/router.ex +++ b/lib/livebook_web/router.ex @@ -39,7 +39,8 @@ defmodule LivebookWeb.Router do get "/sessions/:id/assets/:hash/*file_parts", SessionController, :show_asset end - live_session :default, on_mount: [LivebookWeb.AuthHook, LivebookWeb.UserHook] do + live_session :default, + on_mount: [LivebookWeb.AuthHook, LivebookWeb.UserHook, {LivebookWeb.PolicyHook, :private}] do scope "/", LivebookWeb do pipe_through [:browser, :auth]