Add requirement for scheme in the URL field of deployment group (#2632)

Co-authored-by: Jonatan Kłosko <jonatanklosko@gmail.com>

lib/livebook/teams/deployment_group.ex

@@ -28,15 +28,27 @@ defmodule Livebook.Teams.DeploymentGroup do
       |> validate_required([:name, :mode])
       |> update_change(:url, fn url ->
         if url do
-          url
-          |> String.trim_leading("http://")
-          |> String.trim_leading("https://")
-          |> String.trim_trailing("/")
+          String.trim_trailing(url, "/")
+        end
+      end)
+      |> validate_change(:url, fn :url, url ->
+        case URI.new(url) do
+          {:ok, uri} ->
+            cond do
+              uri.scheme not in ["http", "https"] ->
+                [url: ~s(must start with "http://" or "https://")]
+
+              uri.host in ["", nil] ->
+                [url: "must be a well-formed URL"]
+
+              true ->
+                []
+            end
+
+          {:error, _} ->
+            [url: "must be a well-formed URL"]
         end
       end)
-      |> validate_format(:url, ~r/(^[^\/\.\s]+(\.[^\/\.\s]+)+)(\/[^\s]+)?$/,
-        message: "must be a well-formed URL"
-      )
 
     if get_field(changeset, :zta_provider) do
       validate_required(changeset, [:zta_key])
@@ -44,4 +56,12 @@ defmodule Livebook.Teams.DeploymentGroup do
       changeset
     end
   end
+
+  def url_without_scheme(%__MODULE__{url: url} = _deployment_group) do
+    case url do
+      "http://" <> url -> url
+      "https://" <> url -> url
+      url -> url
+    end
+  end
 end

lib/livebook/utils.ex

@@ -212,8 +212,10 @@ defmodule Livebook.Utils do
   """
   @spec valid_url?(String.t()) :: boolean()
   def valid_url?(url) do
-    uri = URI.parse(url)
-    uri.scheme != nil and uri.host not in [nil, ""]
+    case URI.new(url) do
+      {:ok, uri} -> uri.scheme != nil and uri.host not in [nil, ""]
+      {:error, _} -> false
+    end
   end
 
   @doc """

lib/livebook_web/components/app_components.ex

@@ -93,7 +93,7 @@ defmodule LivebookWeb.AppComponents do
     ~H"""
     <div>
       <div class="flex flex-col gap-4">
-        <.schemaless_url_field
+        <.text_field
           label="URL"
           help={
             ~S'''

lib/livebook_web/components/form_components.ex

@@ -5,40 +5,6 @@ defmodule LivebookWeb.FormComponents do
 
   alias Phoenix.LiveView.JS
 
-  @doc """
-  Renders a text input with label and error messages, as well as a "http(s)://" prefix.
-  """
-  attr :id, :any, default: nil
-  attr :name, :any
-  attr :label, :string, default: nil
-  attr :value, :any
-  attr :errors, :list, default: []
-  attr :field, Phoenix.HTML.FormField, doc: "a form field struct retrieved from the form"
-  attr :help, :string, default: nil
-  attr :class, :string, default: nil
-
-  attr :rest, :global, include: ~w(autocomplete readonly disabled step min max)
-
-  def schemaless_url_field(assigns) do
-    assigns = assigns_from_field(assigns)
-
-    ~H"""
-    <.field_wrapper id={@id} name={@name} label={@label} errors={@errors} help={@help}>
-      <div class="flex flex-row">
-        <div class={[input_classes([]), "flex-1 rounded-e-none border-e-0"]}>http(s)://</div>
-        <input
-          type="text"
-          name={@name}
-          id={@id || @name}
-          value={Phoenix.HTML.Form.normalize_value("text", @value)}
-          class={[input_classes(@errors), "rounded-s-none", @class]}
-          {@rest}
-        />
-      </div>
-    </.field_wrapper>
-    """
-  end
-
   @doc """
   Renders a text input with label and error messages.
   """

lib/livebook_web/live/hub/teams/deployment_group_component.ex

@@ -2,6 +2,7 @@ defmodule LivebookWeb.Hub.Teams.DeploymentGroupComponent do
   use LivebookWeb, :live_component
 
   alias LivebookWeb.NotFoundError
+  alias Livebook.Teams.DeploymentGroup
 
   @impl true
   def update(assigns, socket) do
@@ -42,11 +43,11 @@ defmodule LivebookWeb.Hub.Teams.DeploymentGroupComponent do
             </div>
             <.link
               :if={@deployment_group.url}
-              href={"https://#{@deployment_group.url}"}
+              href={@deployment_group.url}
               class="text-sm font-semibold text-blue-600 hover:text-blue-700 mt-1"
               target="_blank"
             >
-              <%= @deployment_group.url %>
+              <%= DeploymentGroup.url_without_scheme(@deployment_group) %>
             </.link>
           </div>
           <.link

test/livebook_teams/web/hub/deployment_group_test.exs

@@ -20,7 +20,7 @@ defmodule LivebookWeb.Integration.Hub.DeploymentGroupTest do
 
   test "creates a deployment group", %{conn: conn, hub: hub} do
     deployment_group =
-      build(:deployment_group, mode: :offline, hub_id: hub.id, url: "example.com")
+      build(:deployment_group, mode: :offline, hub_id: hub.id, url: "http://example.com")
 
     name = deployment_group.name
     url = deployment_group.url
@@ -75,13 +75,27 @@ defmodule LivebookWeb.Integration.Hub.DeploymentGroupTest do
         name: "other-name",
         value: deployment_group.mode,
         hub_id: deployment_group.hub_id,
-        url: "not a valid url"
+        url: "http://not a valid url"
       }
     }
 
     assert view
            |> element("#deployment-group-form")
            |> render_submit(invalid_attrs) =~ "must be a well-formed URL"
+
+    invalid_attrs = %{
+      deployment_group: %{
+        name: "other-name",
+        value: deployment_group.mode,
+        hub_id: deployment_group.hub_id,
+        url: "url.without.scheme.com"
+      }
+    }
+
+    assert view
+           |> element("#deployment-group-form")
+           |> render_submit(invalid_attrs) =~
+             "must start with "http://" or "https://""
   end
 
   test "creates a secret", %{conn: conn, hub: hub} do