From 5d48ffd0519c332c9a96152b5ef7b7f502591d02 Mon Sep 17 00:00:00 2001
From: Alexandre de Souza <alexandre@aledsz.com.br>
Date: Tue, 28 Feb 2023 10:55:52 -0300
Subject: [PATCH] Remove entire app secrets implementation (#1734)

---
 lib/livebook/application.ex                   |  9 +++++
 lib/livebook/ecto_types/secret_origin.ex      |  8 +---
 lib/livebook/hubs.ex                          |  8 +++-
 lib/livebook/hubs/enterprise.ex               |  2 +-
 lib/livebook/hubs/provider.ex                 |  3 +-
 lib/livebook/secrets/secret.ex                |  2 +-
 lib/livebook_web/live/session_live.ex         | 13 ++++--
 .../live/session_live/secrets_component.ex    | 28 +++++++------
 .../session_live/secrets_list_component.ex    | 40 ++++++-------------
 .../web_socket/client_connection_test.exs     |  2 +-
 .../session_live/secrets_component_test.exs   | 24 ++++++++++-
 test/livebook_web/live/session_live_test.exs  | 20 ++++++----
 test/support/factory.ex                       |  5 ++-
 test/support/session_helpers.ex               |  3 +-
 14 files changed, 100 insertions(+), 67 deletions(-)

diff --git a/lib/livebook/application.ex b/lib/livebook/application.ex
index 7a05faa6b..afc79d841 100644
--- a/lib/livebook/application.ex
+++ b/lib/livebook/application.ex
@@ -55,6 +55,7 @@ defmodule Livebook.Application do
         display_startup_info()
         insert_personal_hub()
         Livebook.Hubs.connect_hubs()
+        update_app_secrets_origin()
         result
 
       {:error, error} ->
@@ -213,6 +214,14 @@ defmodule Livebook.Application do
     end
   end
 
+  # TODO: Remove in the future
+  defp update_app_secrets_origin do
+    for %{origin: :app} = secret <- Livebook.Secrets.get_secrets() do
+      {:ok, secret} = Livebook.Secrets.update_secret(secret, %{origin: {:hub, "personal-hub"}})
+      Livebook.Secrets.set_secret(secret)
+    end
+  end
+
   defp iframe_server_specs() do
     server? = Phoenix.Endpoint.server?(:livebook, LivebookWeb.Endpoint)
     port = Livebook.Config.iframe_port()
diff --git a/lib/livebook/ecto_types/secret_origin.ex b/lib/livebook/ecto_types/secret_origin.ex
index 2f38afd43..249063498 100644
--- a/lib/livebook/ecto_types/secret_origin.ex
+++ b/lib/livebook/ecto_types/secret_origin.ex
@@ -3,7 +3,7 @@ defmodule Livebook.EctoTypes.SecretOrigin do
 
   use Ecto.Type
 
-  @type t :: nil | :session | :startup | :app | {:hub, String.t()}
+  @type t :: nil | :session | :startup | {:hub, String.t()}
 
   @impl true
   def type, do: :string
@@ -13,16 +13,14 @@ defmodule Livebook.EctoTypes.SecretOrigin do
 
   @impl true
   def dump(:session), do: {:ok, "session"}
-  def dump(:app), do: {:ok, "app"}
   def dump(:startup), do: {:ok, "startup"}
   def dump({:hub, id}), do: {:ok, "hub-#{id}"}
   def dump(_), do: :error
 
   @impl true
   def cast(:session), do: {:ok, :session}
-  def cast(:app), do: {:ok, :app}
   def cast(:startup), do: {:ok, :startup}
-  def cast({:hub, id}), do: {:hub, id}
+  def cast({:hub, id}), do: {:ok, {:hub, id}}
 
   def cast(encoded) when is_binary(encoded) do
     case decode(encoded) do
@@ -38,7 +36,6 @@ defmodule Livebook.EctoTypes.SecretOrigin do
   """
   @spec encode(t()) :: String.t()
   def encode(:session), do: "session"
-  def encode(:app), do: "app"
   def encode(:startup), do: "startup"
   def encode({:hub, id}), do: "hub-#{id}"
 
@@ -47,7 +44,6 @@ defmodule Livebook.EctoTypes.SecretOrigin do
   """
   @spec decode(String.t()) :: {:ok, t()} | :error
   def decode("session"), do: {:ok, :session}
-  def decode("app"), do: {:ok, :app}
   def decode("startup"), do: {:ok, :startup}
   def decode("hub-" <> id), do: {:ok, {:hub, id}}
   def decode(_other), do: :error
diff --git a/lib/livebook/hubs.ex b/lib/livebook/hubs.ex
index c3456cec7..abdf14354 100644
--- a/lib/livebook/hubs.ex
+++ b/lib/livebook/hubs.ex
@@ -200,7 +200,7 @@ defmodule Livebook.Hubs do
   """
   @spec get_secrets() :: list(Secret.t())
   def get_secrets do
-    for hub <- get_hubs([:secrets]),
+    for hub <- get_hubs([:list_secrets]),
         secret <- Provider.get_secrets(hub),
         do: secret
   end
@@ -238,7 +238,11 @@ defmodule Livebook.Hubs do
     Provider.delete_secret(hub, secret)
   end
 
-  defp capability?(hub, capabilities) do
+  @doc """
+  Checks the hub capability for given hub.
+  """
+  @spec capability?(Provider.t(), list(atom())) :: boolean()
+  def capability?(hub, capabilities) do
     capabilities -- Provider.capabilities(hub) == []
   end
 end
diff --git a/lib/livebook/hubs/enterprise.ex b/lib/livebook/hubs/enterprise.ex
index eb2583c03..a30c40650 100644
--- a/lib/livebook/hubs/enterprise.ex
+++ b/lib/livebook/hubs/enterprise.ex
@@ -145,7 +145,7 @@ defimpl Livebook.Hubs.Provider, for: Livebook.Hubs.Enterprise do
     EnterpriseClient.stop(enterprise.id)
   end
 
-  def capabilities(_enterprise), do: ~w(connect secrets list_secrets create_secret)a
+  def capabilities(_enterprise), do: ~w(connect list_secrets create_secret)a
 
   def get_secrets(enterprise) do
     EnterpriseClient.get_secrets(enterprise.id)
diff --git a/lib/livebook/hubs/provider.ex b/lib/livebook/hubs/provider.ex
index cbaf67314..dc6886e71 100644
--- a/lib/livebook/hubs/provider.ex
+++ b/lib/livebook/hubs/provider.ex
@@ -4,8 +4,7 @@ defprotocol Livebook.Hubs.Provider do
   alias Livebook.Secrets.Secret
 
   @type t :: Livebook.Hubs.Enterprise.t() | Livebook.Hubs.Fly.t() | Livebook.Hubs.Personal.t()
-  @type capability ::
-          :connect | :secrets | :list_secrets | :create_secret | :update_secret | :delete_secret
+  @type capability :: :connect | :list_secrets | :create_secret | :update_secret | :delete_secret
   @type capabilities :: list(capability())
   @type changeset_errors :: %{required(:errors) => list({String.t(), {Stirng.t(), list()}})}
 
diff --git a/lib/livebook/secrets/secret.ex b/lib/livebook/secrets/secret.ex
index fd9349f8e..9328d9f59 100644
--- a/lib/livebook/secrets/secret.ex
+++ b/lib/livebook/secrets/secret.ex
@@ -14,7 +14,7 @@ defmodule Livebook.Secrets.Secret do
   @primary_key {:name, :string, autogenerate: false}
   embedded_schema do
     field :value, :string
-    field :origin, SecretOrigin, default: :app
+    field :origin, SecretOrigin
   end
 
   def changeset(secret, attrs \\ %{}) do
diff --git a/lib/livebook_web/live/session_live.ex b/lib/livebook_web/live/session_live.ex
index d31244715..1922d4a24 100644
--- a/lib/livebook_web/live/session_live.ex
+++ b/lib/livebook_web/live/session_live.ex
@@ -1188,14 +1188,21 @@ defmodule LivebookWeb.SessionLive do
     {:noreply,
      socket
      |> assign(saved_secrets: get_saved_secrets())
-     |> put_flash(:info, "A new secret has been created on your Livebook Enterprise")}
+     |> put_flash(:info, "A new secret has been created on your Livebook Hub")}
   end
 
   def handle_info({:secret_updated, %{origin: {:hub, _id}}}, socket) do
     {:noreply,
      socket
      |> assign(saved_secrets: get_saved_secrets())
-     |> put_flash(:info, "An existing secret has been updated on your Livebook Enterprise")}
+     |> put_flash(:info, "An existing secret has been updated on your Livebook Hub")}
+  end
+
+  def handle_info({:secret_deleted, %{origin: {:hub, _id}}}, socket) do
+    {:noreply,
+     socket
+     |> assign(saved_secrets: get_saved_secrets())
+     |> put_flash(:info, "An existing secret has been deleted on your Livebook Hub")}
   end
 
   def handle_info(:hubs_changed, socket) do
@@ -2040,7 +2047,7 @@ defmodule LivebookWeb.SessionLive do
   end
 
   defp get_saved_secrets do
-    Enum.sort(Hubs.get_secrets() ++ Secrets.get_secrets())
+    Enum.sort(Hubs.get_secrets())
   end
 
   defp app_status_color(nil), do: "bg-gray-400"
diff --git a/lib/livebook_web/live/session_live/secrets_component.ex b/lib/livebook_web/live/session_live/secrets_component.ex
index 77216d055..b4c09b71c 100644
--- a/lib/livebook_web/live/session_live/secrets_component.ex
+++ b/lib/livebook_web/live/session_live/secrets_component.ex
@@ -9,7 +9,7 @@ defmodule LivebookWeb.SessionLive.SecretsComponent do
 
   @impl true
   def mount(socket) do
-    {:ok, assign(socket, title: title(socket), hubs: Livebook.Hubs.get_hubs([:secrets]))}
+    {:ok, assign(socket, title: title(socket), hubs: Livebook.Hubs.get_hubs([:create_secret]))}
   end
 
   @impl true
@@ -62,7 +62,7 @@ defmodule LivebookWeb.SessionLive.SecretsComponent do
                 :for={secret <- @saved_secrets}
                 secret_name={secret.name}
                 secret_origin={secret.origin}
-                stored={stored(secret)}
+                stored={stored(secret, @hubs)}
                 active={false}
                 target={@myself}
               />
@@ -113,7 +113,7 @@ defmodule LivebookWeb.SessionLive.SecretsComponent do
               value={SecretOrigin.encode(f[:origin].value)}
               label="Storage"
               options={
-                [{"session", "only this session"}, {"app", "in the Livebook app"}] ++
+                [{"session", "only this session"}] ++
                   if Livebook.Config.feature_flag_enabled?(:hub) do
                     for hub <- @hubs, do: {"hub-#{hub.id}", "in #{hub.hub_emoji} #{hub.hub_name}"}
                   else
@@ -188,7 +188,7 @@ defmodule LivebookWeb.SessionLive.SecretsComponent do
                 icon="error-warning-fill"
                 class="align-middle text-2xl flex text-gray-100 rounded-lg py-2"
               />
-              <%= if @secret.origin in [:app, :startup] do %>
+              <%= if @secret.origin == :startup do %>
                 <span class="ml-2 text-sm font-normal text-gray-100">
                   There is a secret named
                   <span class="font-semibold text-white"><%= @secret.name %></span>
@@ -218,9 +218,6 @@ defmodule LivebookWeb.SessionLive.SecretsComponent do
     """
   end
 
-  defp stored(%{origin: {:hub, _}}), do: "Hub"
-  defp stored(%{origin: origin}) when origin in [:app, :startup], do: "Livebook"
-
   @impl true
   def handle_event("save", %{"secret" => attrs}, socket) do
     with {:ok, secret} <- Secrets.update_secret(%Secret{}, attrs),
@@ -278,11 +275,6 @@ defmodule LivebookWeb.SessionLive.SecretsComponent do
     Session.set_secret(socket.assigns.session.pid, secret)
   end
 
-  defp set_secret(socket, %Secret{origin: :app} = secret) do
-    Secrets.set_secret(secret)
-    Session.set_secret(socket.assigns.session.pid, secret)
-  end
-
   defp set_secret(socket, %Secret{origin: {:hub, id}} = secret) when is_binary(id) do
     with :ok <- Hubs.create_secret(secret) do
       Session.set_secret(socket.assigns.session.pid, secret)
@@ -294,4 +286,16 @@ defmodule LivebookWeb.SessionLive.SecretsComponent do
 
     if secret, do: Livebook.Session.set_secret(socket.assigns.session.pid, secret)
   end
+
+  defp stored(%{origin: {:hub, id}}, hubs), do: stored(id, hubs)
+  defp stored(%{origin: :startup}, hubs), do: stored("personal-hub", hubs)
+
+  defp stored(id, hubs) do
+    hub = fetch_hub!(id, hubs)
+    "#{hub.hub_emoji} #{hub.hub_name}"
+  end
+
+  defp fetch_hub!(id, hubs) do
+    Enum.find(hubs, &(&1.id == id)) || raise "unknown hub id: #{id}"
+  end
 end
diff --git a/lib/livebook_web/live/session_live/secrets_list_component.ex b/lib/livebook_web/live/session_live/secrets_list_component.ex
index 276d6167c..288f81003 100644
--- a/lib/livebook_web/live/session_live/secrets_list_component.ex
+++ b/lib/livebook_web/live/session_live/secrets_list_component.ex
@@ -67,30 +67,6 @@ defmodule LivebookWeb.SessionLive.SecretsListComponent do
           <span>New secret</span>
         </.link>
 
-        <div class="mt-16">
-          <h3 class="uppercase text-sm font-semibold text-gray-500">
-            App secrets
-          </h3>
-          <span class="text-sm text-gray-500">
-            <%= if @saved_secrets == [] do %>
-              No secrets stored in Livebook so far
-            <% else %>
-              Toggle to share with this session
-            <% end %>
-          </span>
-        </div>
-
-        <div class="flex flex-col space-y-4 mt-6">
-          <.secrets_item
-            :for={secret when secret.origin in [:app, :startup] <- @saved_secrets}
-            secret={secret}
-            prefix={to_string(secret.origin)}
-            data_secrets={@secrets}
-            hubs={@hubs}
-            myself={@myself}
-          />
-        </div>
-
         <div :if={Livebook.Config.feature_flag_enabled?(:hub)} class="mt-16">
           <h3 class="uppercase text-sm font-semibold text-gray-500">
             Hub secrets
@@ -106,9 +82,9 @@ defmodule LivebookWeb.SessionLive.SecretsListComponent do
 
         <div class="flex flex-col space-y-4 mt-6">
           <.secrets_item
-            :for={%{origin: {:hub, id}} = secret <- @saved_secrets}
+            :for={secret <- @saved_secrets}
             secret={secret}
-            prefix={"hub-#{id}"}
+            prefix={prefix(secret)}
             data_secrets={@secrets}
             hubs={@hubs}
             myself={@myself}
@@ -162,7 +138,7 @@ defmodule LivebookWeb.SessionLive.SecretsListComponent do
               <%= @secret.value %>
             </span>
             <button
-              :if={@secret.origin == :app}
+              :if={delete?(@secret, @hubs)}
               id={"#{@prefix}-secret-#{@secret.name}-delete"}
               type="button"
               phx-click={
@@ -238,4 +214,14 @@ defmodule LivebookWeb.SessionLive.SecretsListComponent do
   defp fetch_hub!(id, hubs) do
     Enum.find(hubs, &(&1.id == id)) || raise "unknown hub id: #{id}"
   end
+
+  defp prefix(%{origin: {:hub, id}}), do: "hub-#{id}"
+  defp prefix(%{origin: :startup}), do: "hub-personal-hub"
+
+  defp delete?(%{origin: {:hub, id}}, hubs) do
+    hub = fetch_hub!(id, hubs)
+    Livebook.Hubs.capability?(hub.provider, [:delete_secret])
+  end
+
+  defp delete?(_, _), do: false
 end
diff --git a/test/livebook/web_socket/client_connection_test.exs b/test/livebook/web_socket/client_connection_test.exs
index c391a9b14..2d69cdac1 100644
--- a/test/livebook/web_socket/client_connection_test.exs
+++ b/test/livebook/web_socket/client_connection_test.exs
@@ -147,7 +147,7 @@ defmodule Livebook.WebSocket.ClientConnectionTest do
       assert_receive {:event, :secret_deleted, %{name: ^name, value: ^value}}
     end
 
-    test "receives a session_created event", %{conn: conn, node: node} do
+    test "receives a user_synchronized event", %{conn: conn, node: node} do
       data = LivebookProto.build_handshake_request(app_version: Livebook.Config.app_version())
       assert {:handshake, _} = ClientConnection.send_request(conn, data)
 
diff --git a/test/livebook_web/live/session_live/secrets_component_test.exs b/test/livebook_web/live/session_live/secrets_component_test.exs
index a007f0d92..855060c13 100644
--- a/test/livebook_web/live/session_live/secrets_component_test.exs
+++ b/test/livebook_web/live/session_live/secrets_component_test.exs
@@ -59,7 +59,7 @@ defmodule LivebookWeb.SessionLive.SecretsComponentTest do
       render_submit(form, attrs)
 
       assert_receive {:secret_created, ^secret}
-      assert render(view) =~ "A new secret has been created on your Livebook Enterprise"
+      assert render(view) =~ "A new secret has been created on your Livebook Hub"
       assert has_element?(view, "#hub-#{enterprise.id}-secret-#{secret.name}-wrapper")
       assert has_element?(view, ~s/[data-tooltip="#{enterprise.hub_name}"]/, enterprise.hub_emoji)
     end
@@ -190,5 +190,27 @@ defmodule LivebookWeb.SessionLive.SecretsComponentTest do
 
       assert output == "\e[32m\"#{secret.value}\"\e[0m"
     end
+
+    test "shows secret events from Enterprise hub",
+         %{conn: conn, session: session, enterprise: enterprise, node: node} do
+      {:ok, view, _html} = live(conn, ~p"/sessions/#{session.id}/secrets")
+      secret = build(:secret, name: "EVENT_SECRET", value: "123", origin: {:hub, enterprise.id})
+
+      # We need the `Secret` schema from enterprise to execute
+      # the following functions inside `Enterprise.Integration`
+      enterprise_secret =
+        :erpc.call(node, Enterprise.Integration, :create_secret, [secret.name, secret.value])
+
+      assert_receive {:secret_created, ^secret}
+      assert render(view) =~ "A new secret has been created on your Livebook Hub"
+
+      :erpc.call(node, Enterprise.Integration, :update_secret, [enterprise_secret, secret.value])
+      assert_receive {:secret_updated, ^secret}
+      assert render(view) =~ "An existing secret has been updated on your Livebook Hub"
+
+      :erpc.call(node, Enterprise.Integration, :delete_secret, [enterprise_secret])
+      assert_receive {:secret_deleted, ^secret}
+      assert render(view) =~ "An existing secret has been deleted on your Livebook Hub"
+    end
   end
 end
diff --git a/test/livebook_web/live/session_live_test.exs b/test/livebook_web/live/session_live_test.exs
index 2dd18a73a..20ec501a8 100644
--- a/test/livebook_web/live/session_live_test.exs
+++ b/test/livebook_web/live/session_live_test.exs
@@ -1058,24 +1058,28 @@ defmodule LivebookWeb.SessionLiveTest do
 
     test "adds a livebook secret from form", %{conn: conn, session: session} do
       {:ok, view, _} = live(conn, ~p"/sessions/#{session.id}/secrets")
-      secret = build(:secret, name: "BAR", value: "456", origin: :app)
+      secret = build(:secret, name: "BAR", value: "456")
 
       view
       |> element(~s{form[phx-submit="save"]})
-      |> render_submit(%{secret: %{name: secret.name, value: secret.value, origin: "app"}})
+      |> render_submit(%{
+        secret: %{name: secret.name, value: secret.value, origin: "hub-personal-hub"}
+      })
 
       assert secret in Livebook.Secrets.get_secrets()
     end
 
     test "syncs secrets", %{conn: conn, session: session} do
       session_secret = insert_secret(name: "FOO", value: "123")
-      secret = build(:secret, name: "FOO", value: "456", origin: :app)
+      secret = build(:secret, name: "FOO", value: "456")
 
       {:ok, view, _} = live(conn, ~p"/sessions/#{session.id}/secrets")
 
       view
       |> element(~s{form[phx-submit="save"]})
-      |> render_submit(%{secret: %{name: secret.name, value: secret.value, origin: "app"}})
+      |> render_submit(%{
+        secret: %{name: secret.name, value: secret.value, origin: "hub-personal-hub"}
+      })
 
       assert_session_secret(view, session.pid, secret)
       assert secret in Livebook.Secrets.get_secrets()
@@ -1083,11 +1087,13 @@ defmodule LivebookWeb.SessionLiveTest do
       {:ok, view, _} = live(conn, ~p"/sessions/#{session.id}/secrets")
       Session.set_secret(session.pid, session_secret)
 
-      secret = build(:secret, name: "FOO", value: "789", origin: :app)
+      secret = build(:secret, name: "FOO", value: "789")
 
       view
       |> element(~s{form[phx-submit="save"]})
-      |> render_submit(%{secret: %{name: secret.name, value: secret.value, origin: "app"}})
+      |> render_submit(%{
+        secret: %{name: secret.name, value: secret.value, origin: "hub-personal-hub"}
+      })
 
       assert_session_secret(view, session.pid, secret)
       assert secret in Livebook.Secrets.get_secrets()
@@ -1198,7 +1204,7 @@ defmodule LivebookWeb.SessionLiveTest do
       render_click(add_secret_button)
       secrets_component = with_target(view, "#secrets-modal")
 
-      assert render(secrets_component) =~ "in your Livebook app. Allow this session to access it?"
+      assert render(secrets_component) =~ "in your Livebook Hub. Allow this session to access it?"
 
       grant_access_button = element(secrets_component, "button", "Grant access")
       render_click(grant_access_button)
diff --git a/test/support/factory.ex b/test/support/factory.ex
index 79b972b3d..c5338c098 100644
--- a/test/support/factory.ex
+++ b/test/support/factory.ex
@@ -66,7 +66,7 @@ defmodule Livebook.Factory do
     %Livebook.Secrets.Secret{
       name: "FOO",
       value: "123",
-      origin: :app
+      origin: {:hub, "personal-hub"}
     }
   end
 
@@ -86,7 +86,8 @@ defmodule Livebook.Factory do
 
   def insert_secret(attrs \\ %{}) do
     secret = build(:secret, attrs)
-    Livebook.Secrets.set_secret(secret)
+    :ok = Livebook.Hubs.create_secret(secret)
+    secret
   end
 
   def insert_env_var(factory_name, attrs \\ %{}) do
diff --git a/test/support/session_helpers.ex b/test/support/session_helpers.ex
index 9688097f9..a1f9e096f 100644
--- a/test/support/session_helpers.ex
+++ b/test/support/session_helpers.ex
@@ -57,8 +57,7 @@ defmodule Livebook.SessionHelpers do
     selector =
       case secret do
         %{name: name, origin: :session} -> "#session-secret-#{name}-wrapper"
-        %{name: name, origin: :app} -> "#app-secret-#{name}-wrapper"
-        %{name: name, origin: :startup} -> "#startup-secret-#{name}-wrapper"
+        %{name: name, origin: :startup} -> "#hub-personal-hub-secret-#{name}-wrapper"
         %{name: name, origin: {:hub, id}} -> "#hub-#{id}-secret-#{name}-wrapper"
       end