From 76cff0494eea65e3807df50bc5257eeeb3eec3dc Mon Sep 17 00:00:00 2001
From: jamey <jamey@server.jamey.stream>
Date: Wed, 4 Mar 2026 12:17:56 +0000
Subject: [PATCH] add forgiving API key validation with inline errors

Add KeyValidation module for format-checking API keys before
attempting connections. Auto-strips whitespace, detects common
mistakes (e.g. pasting a Stripe publishable key), and returns
helpful error messages.

Inline field errors across all three entry points:
- Setup wizard: provider + Stripe keys
- Admin provider form: simplified to single Connect button
- Email settings: per-field errors instead of flash toasts

Also: plain text inputs for all API keys (not password fields),
accessible error states (aria-invalid, role=alert, thick border,
bold text), inner_block slot declaration on error component.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
---
 assets/css/admin/components.css               |  12 +-
 lib/berrypod/key_validation.ex                | 156 ++++++++++++++
 .../components/core_components.ex             |  25 ++-
 lib/berrypod_web/live/admin/email_settings.ex |  52 ++++-
 lib/berrypod_web/live/admin/providers/form.ex |  82 +++-----
 .../live/admin/providers/form.html.heex       |  36 +---
 lib/berrypod_web/live/setup/onboarding.ex     | 151 +++++++++-----
 test/berrypod/key_validation_test.exs         | 194 ++++++++++++++++++
 .../live/admin/email_settings_test.exs        |   8 +-
 .../live/admin/providers_test.exs             |  57 -----
 10 files changed, 557 insertions(+), 216 deletions(-)
 create mode 100644 lib/berrypod/key_validation.ex
 create mode 100644 test/berrypod/key_validation_test.exs

diff --git a/assets/css/admin/components.css b/assets/css/admin/components.css
index bf05c9a..ec7b6b9 100644
--- a/assets/css/admin/components.css
+++ b/assets/css/admin/components.css
@@ -553,6 +553,7 @@
 
 .admin-input-error {
   border-color: var(--t-status-error);
+  border-width: 2px;
 
   &:focus {
     border-color: var(--t-status-error);
@@ -876,13 +877,20 @@
 .admin-error {
   display: flex;
   align-items: center;
-  gap: 0.5rem;
+  gap: 0.375rem;
   margin-top: 0.375rem;
   font-size: 0.875rem;
   line-height: 1.25rem;
+  font-weight: 600;
   color: var(--t-status-error);
 }
 
+.admin-error-icon {
+  width: 1em;
+  height: 1em;
+  flex-shrink: 0;
+}
+
 /* ── Link ── */
 
 .admin-link {
@@ -4774,7 +4782,7 @@
 .truncate { overflow: hidden; text-overflow: ellipsis; white-space: nowrap; }
 .ml-1 { margin-inline-start: 0.25rem; }
 
-.external-link-icon {
+.external-link-icon.external-link-icon {
   width: 0.75em;
   height: 0.75em;
   margin-inline-start: 0.25em;
diff --git a/lib/berrypod/key_validation.ex b/lib/berrypod/key_validation.ex
new file mode 100644
index 0000000..21b183c
--- /dev/null
+++ b/lib/berrypod/key_validation.ex
@@ -0,0 +1,156 @@
+defmodule Berrypod.KeyValidation do
+  @moduledoc """
+  Lightweight format validation for API keys.
+
+  Auto-strips whitespace and checks known formats before attempting
+  network calls, giving users helpful error messages instead of
+  cryptic "connection failed" responses.
+
+  All functions return `{:ok, trimmed_key}` or `{:error, message}`.
+  """
+
+  @doc """
+  Validates a Stripe secret key.
+
+  Checks for the `sk_test_` or `sk_live_` prefix. Detects common
+  mistakes like pasting a publishable key or restricted key.
+  """
+  def validate_stripe_key(key) do
+    key = trim(key)
+
+    cond do
+      key == "" ->
+        {:error, "Please enter your Stripe secret key"}
+
+      String.starts_with?(key, "pk_") ->
+        {:error,
+         "This looks like a publishable key (starts with pk_). You need the secret key, which starts with sk_test_ or sk_live_"}
+
+      String.starts_with?(key, "rk_") ->
+        {:error,
+         "This looks like a restricted key (starts with rk_). You need the secret key, which starts with sk_test_ or sk_live_"}
+
+      not String.starts_with?(key, "sk_test_") and not String.starts_with?(key, "sk_live_") ->
+        {:error,
+         "Stripe secret keys start with sk_test_ or sk_live_. Check you're copying the right key from the Stripe dashboard"}
+
+      String.length(key) < 20 ->
+        {:error, "This key looks too short. Check you copied the full key from Stripe"}
+
+      true ->
+        {:ok, key}
+    end
+  end
+
+  @doc """
+  Validates a print provider API key (Printify, Printful, etc.).
+
+  Provider tokens are opaque, so we just check for empty/too-short values.
+  """
+  def validate_provider_key(key, _provider_type \\ nil) do
+    key = trim(key)
+
+    cond do
+      key == "" ->
+        {:error, "Please enter your API token"}
+
+      String.length(key) < 10 ->
+        {:error, "This looks too short for an API token. Check you copied the full value"}
+
+      true ->
+        {:ok, key}
+    end
+  end
+
+  @doc """
+  Validates an email provider API key or secret.
+
+  Checks format for providers with known key patterns.
+  Falls through to basic validation for unknown providers.
+  """
+  def validate_email_key(key, adapter_key, field_key) do
+    key = trim(key)
+
+    if key == "" do
+      {:error, "Please enter your API key"}
+    else
+      validate_email_format(key, adapter_key, field_key)
+    end
+  end
+
+  # ── Email provider format checks ──
+
+  # SendGrid: SG.{id}.{secret}
+  defp validate_email_format(key, "sendgrid", "api_key") do
+    cond do
+      String.starts_with?(key, "SG.") ->
+        {:ok, key}
+
+      String.contains?(key, ".") ->
+        {:error, "SendGrid keys start with SG. Check you copied the full key"}
+
+      true ->
+        {:error,
+         "SendGrid API keys start with SG. (like SG.xxxxx.xxxxx). This doesn't look right"}
+    end
+  end
+
+  # Postmark: UUID format
+  defp validate_email_format(key, "postmark", "api_key") do
+    uuid_pattern = ~r/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i
+
+    if Regex.match?(uuid_pattern, key) do
+      {:ok, key}
+    else
+      {:error,
+       "Postmark tokens are UUIDs (like abc12345-abcd-1234-abcd-123456789abc). Check you copied the server token, not the account token ID"}
+    end
+  end
+
+  # Resend: re_ prefix
+  defp validate_email_format(key, "resend", "api_key") do
+    if String.starts_with?(key, "re_") do
+      {:ok, key}
+    else
+      {:error, "Resend API keys start with re_. Check you copied the full key"}
+    end
+  end
+
+  # Mailgun: key- prefix (classic keys)
+  defp validate_email_format(key, "mailgun", "api_key") do
+    cond do
+      String.starts_with?(key, "key-") ->
+        {:ok, key}
+
+      # Newer RBAC keys don't have a known prefix, let them through
+      String.length(key) >= 20 ->
+        {:ok, key}
+
+      true ->
+        {:error, "Mailgun API keys usually start with key-. Check you copied the full key"}
+    end
+  end
+
+  # Brevo: xkeysib- prefix
+  defp validate_email_format(key, "brevo", "api_key") do
+    if String.starts_with?(key, "xkeysib-") do
+      {:ok, key}
+    else
+      {:error, "Brevo API keys start with xkeysib-. Check you copied the full key"}
+    end
+  end
+
+  # Unknown provider or non-api_key field, basic length check only
+  defp validate_email_format(key, _adapter_key, _field_key) do
+    if String.length(key) < 5 do
+      {:error, "This value looks too short. Check you copied the full key"}
+    else
+      {:ok, key}
+    end
+  end
+
+  # ── Helpers ──
+
+  defp trim(nil), do: ""
+  defp trim(key) when is_binary(key), do: String.trim(key)
+end
diff --git a/lib/berrypod_web/components/core_components.ex b/lib/berrypod_web/components/core_components.ex
index 27d2539..e8b5377 100644
--- a/lib/berrypod_web/components/core_components.ex
+++ b/lib/berrypod_web/components/core_components.ex
@@ -205,6 +205,8 @@ defmodule BerrypodWeb.CoreComponents do
   end
 
   def input(%{type: "select"} = assigns) do
+    assigns = assign(assigns, :error_id, assigns.id && "#{assigns.id}-error")
+
     ~H"""
     <div class="admin-fieldset">
       <label>
@@ -214,18 +216,22 @@ defmodule BerrypodWeb.CoreComponents do
           name={@name}
           class={[@class || "admin-select", @errors != [] && (@error_class || "admin-input-error")]}
           multiple={@multiple}
+          aria-invalid={@errors != [] && "true"}
+          aria-describedby={@errors != [] && @error_id}
           {@rest}
         >
           <option :if={@prompt} value="">{@prompt}</option>
           {Phoenix.HTML.Form.options_for_select(@options, @value)}
         </select>
       </label>
-      <.error :for={msg <- @errors}>{msg}</.error>
+      <.error :for={msg <- @errors} id={@error_id}>{msg}</.error>
     </div>
     """
   end
 
   def input(%{type: "textarea"} = assigns) do
+    assigns = assign(assigns, :error_id, assigns.id && "#{assigns.id}-error")
+
     ~H"""
     <div class="admin-fieldset">
       <label>
@@ -237,16 +243,20 @@ defmodule BerrypodWeb.CoreComponents do
             @class || "admin-textarea",
             @errors != [] && (@error_class || "admin-input-error")
           ]}
+          aria-invalid={@errors != [] && "true"}
+          aria-describedby={@errors != [] && @error_id}
           {@rest}
         >{Phoenix.HTML.Form.normalize_value("textarea", @value)}</textarea>
       </label>
-      <.error :for={msg <- @errors}>{msg}</.error>
+      <.error :for={msg <- @errors} id={@error_id}>{msg}</.error>
     </div>
     """
   end
 
   # All other inputs text, datetime-local, url, password, etc. are handled here...
   def input(assigns) do
+    assigns = assign(assigns, :error_id, assigns.id && "#{assigns.id}-error")
+
     ~H"""
     <div class="admin-fieldset">
       <label>
@@ -260,19 +270,24 @@ defmodule BerrypodWeb.CoreComponents do
             @class || "admin-input",
             @errors != [] && (@error_class || "admin-input-error")
           ]}
+          aria-invalid={@errors != [] && "true"}
+          aria-describedby={@errors != [] && @error_id}
           {@rest}
         />
       </label>
-      <.error :for={msg <- @errors}>{msg}</.error>
+      <.error :for={msg <- @errors} id={@error_id}>{msg}</.error>
     </div>
     """
   end
 
   # Helper used by inputs to generate form errors
+  attr :id, :string, default: nil
+  slot :inner_block, required: true
+
   defp error(assigns) do
     ~H"""
-    <p class="admin-error">
-      <.icon name="hero-exclamation-circle" class="size-5" />
+    <p class="admin-error" id={@id} role="alert">
+      <.icon name="hero-exclamation-circle" class="admin-error-icon" />
       {render_slot(@inner_block)}
     </p>
     """
diff --git a/lib/berrypod_web/live/admin/email_settings.ex b/lib/berrypod_web/live/admin/email_settings.ex
index 20faa1f..35dd4e8 100644
--- a/lib/berrypod_web/live/admin/email_settings.ex
+++ b/lib/berrypod_web/live/admin/email_settings.ex
@@ -1,6 +1,7 @@
 defmodule BerrypodWeb.Admin.EmailSettings do
   use BerrypodWeb, :live_view
 
+  alias Berrypod.KeyValidation
   alias Berrypod.Mailer
   alias Berrypod.Mailer.Adapters
   alias Berrypod.Settings
@@ -28,6 +29,7 @@ defmodule BerrypodWeb.Admin.EmailSettings do
      )
      |> assign(:sending_test, false)
      |> assign(:from_checklist, false)
+     |> assign(:field_errors, %{})
      |> assign(:form, to_form(%{}, as: :email))}
   end
 
@@ -71,7 +73,12 @@ defmodule BerrypodWeb.Admin.EmailSettings do
   @impl true
   def handle_event("change_adapter", %{"email" => %{"adapter" => key}}, socket) do
     values = load_adapter_values(key)
-    {:noreply, socket |> assign(:adapter_key, key) |> assign(:current_values, values)}
+
+    {:noreply,
+     socket
+     |> assign(:adapter_key, key)
+     |> assign(:current_values, values)
+     |> assign(:field_errors, %{})}
   end
 
   def handle_event("save", %{"email" => params}, socket) do
@@ -137,6 +144,9 @@ defmodule BerrypodWeb.Admin.EmailSettings do
   end
 
   defp save_adapter_config(socket, adapter_info, params) do
+    # Trim all values
+    params = Map.new(params, fn {k, v} -> {k, if(is_binary(v), do: String.trim(v), else: v)} end)
+
     # Validate required fields
     missing =
       adapter_info.fields
@@ -149,9 +159,29 @@ defmodule BerrypodWeb.Admin.EmailSettings do
         empty and not (field.type == :secret and Settings.get_secret(settings_key) != nil)
       end)
 
-    if missing != [] do
-      labels = Enum.map_join(missing, ", ", & &1.label)
-      {:noreply, put_flash(socket, :error, "Missing required fields: #{labels}")}
+    # Build per-field errors for missing required fields
+    missing_errors =
+      for field <- missing, into: %{} do
+        {field.key, "#{field.label} is required"}
+      end
+
+    # Validate secret field formats for known providers
+    format_errors =
+      for field <- adapter_info.fields,
+          field.type == :secret,
+          value = params[field.key],
+          value != nil and value != "",
+          {:error, message} <- [
+            KeyValidation.validate_email_key(value, adapter_info.key, field.key)
+          ],
+          into: %{} do
+        {field.key, message}
+      end
+
+    field_errors = Map.merge(missing_errors, format_errors)
+
+    if field_errors != %{} do
+      {:noreply, assign(socket, :field_errors, field_errors)}
     else
       # Save adapter type
       Settings.put_setting("email_adapter", adapter_info.key)
@@ -199,6 +229,7 @@ defmodule BerrypodWeb.Admin.EmailSettings do
        |> assign(:current_values, current_values)
        |> assign(:from_address, from_address)
        |> assign(:email_configured, Mailer.email_configured?())
+       |> assign(:field_errors, %{})
        |> put_flash(:info, "Email settings saved")}
     end
   end
@@ -293,6 +324,7 @@ defmodule BerrypodWeb.Admin.EmailSettings do
                   field_def={field}
                   value={if selected, do: @current_values[field.key]}
                   disabled={!selected || @env_locked}
+                  error={if selected, do: @field_errors[field.key]}
                 />
               <% end %>
               <%= unless @env_locked do %>
@@ -343,19 +375,23 @@ defmodule BerrypodWeb.Admin.EmailSettings do
   attr :field_def, :map, required: true
   attr :value, :any, default: nil
   attr :disabled, :boolean, default: false
+  attr :error, :string, default: nil
 
   defp adapter_field_static(%{field_def: %{type: :secret}} = assigns) do
+    assigns = assign(assigns, :errors, if(assigns.error, do: [assigns.error], else: []))
+
     ~H"""
     <div>
       <.input
         name={"email[#{@field_def.key}]"}
         value=""
-        type="password"
+        type="text"
         label={@field_def.label}
         autocomplete="off"
         placeholder={if @value, do: @value, else: ""}
         required={@field_def.required && !@value}
         disabled={@disabled}
+        errors={@errors}
       />
       <%= if @value && !@disabled do %>
         <p class="admin-help-text">
@@ -367,6 +403,8 @@ defmodule BerrypodWeb.Admin.EmailSettings do
   end
 
   defp adapter_field_static(%{field_def: %{type: :integer}} = assigns) do
+    assigns = assign(assigns, :errors, if(assigns.error, do: [assigns.error], else: []))
+
     ~H"""
     <.input
       name={"email[#{@field_def.key}]"}
@@ -375,11 +413,14 @@ defmodule BerrypodWeb.Admin.EmailSettings do
       label={@field_def.label}
       required={@field_def.required}
       disabled={@disabled}
+      errors={@errors}
     />
     """
   end
 
   defp adapter_field_static(assigns) do
+    assigns = assign(assigns, :errors, if(assigns.error, do: [assigns.error], else: []))
+
     ~H"""
     <.input
       name={"email[#{@field_def.key}]"}
@@ -388,6 +429,7 @@ defmodule BerrypodWeb.Admin.EmailSettings do
       label={@field_def.label}
       required={@field_def.required}
       disabled={@disabled}
+      errors={@errors}
     />
     """
   end
diff --git a/lib/berrypod_web/live/admin/providers/form.ex b/lib/berrypod_web/live/admin/providers/form.ex
index 8ba2ddd..b449d56 100644
--- a/lib/berrypod_web/live/admin/providers/form.ex
+++ b/lib/berrypod_web/live/admin/providers/form.ex
@@ -1,9 +1,8 @@
 defmodule BerrypodWeb.Admin.Providers.Form do
   use BerrypodWeb, :live_view
 
-  alias Berrypod.Products
+  alias Berrypod.{KeyValidation, Products}
   alias Berrypod.Products.ProviderConnection
-  alias Berrypod.Providers
   alias Berrypod.Providers.Provider
 
   @supported_types Enum.map(Provider.available(), & &1.type)
@@ -23,8 +22,6 @@ defmodule BerrypodWeb.Admin.Providers.Form do
     |> assign(:provider, provider)
     |> assign(:connection, %ProviderConnection{provider_type: provider_type})
     |> assign(:form, to_form(ProviderConnection.changeset(%ProviderConnection{}, %{})))
-    |> assign(:testing, false)
-    |> assign(:test_result, nil)
     |> assign(:pending_api_key, nil)
   end
 
@@ -38,8 +35,6 @@ defmodule BerrypodWeb.Admin.Providers.Form do
     |> assign(:provider, provider)
     |> assign(:connection, connection)
     |> assign(:form, to_form(ProviderConnection.changeset(connection, %{})))
-    |> assign(:testing, false)
-    |> assign(:test_result, nil)
     |> assign(:pending_api_key, nil)
   end
 
@@ -55,27 +50,6 @@ defmodule BerrypodWeb.Admin.Providers.Form do
     {:noreply, assign(socket, form: form, pending_api_key: params["api_key"])}
   end
 
-  @impl true
-  def handle_event("test_connection", _params, socket) do
-    socket = assign(socket, testing: true, test_result: nil)
-
-    api_key =
-      socket.assigns[:pending_api_key] ||
-        ProviderConnection.get_api_key(socket.assigns.connection)
-
-    if api_key && api_key != "" do
-      temp_conn = %ProviderConnection{
-        provider_type: socket.assigns.provider_type,
-        api_key_encrypted: encrypt_api_key(api_key)
-      }
-
-      result = Providers.test_connection(temp_conn)
-      {:noreply, assign(socket, testing: false, test_result: result)}
-    else
-      {:noreply, assign(socket, testing: false, test_result: {:error, :no_api_key})}
-    end
-  end
-
   @impl true
   def handle_event("save", %{"provider_connection" => params}, socket) do
     save_connection(socket, socket.assigns.live_action, params)
@@ -85,15 +59,29 @@ defmodule BerrypodWeb.Admin.Providers.Form do
     api_key = params["api_key"] || socket.assigns[:pending_api_key]
     provider_type = socket.assigns.provider_type
 
-    case Products.connect_provider(api_key, provider_type) do
-      {:ok, _connection} ->
-        {:noreply,
-         socket
-         |> put_flash(:info, "Connected to #{socket.assigns.provider.name}!")
-         |> push_navigate(to: ~p"/admin/settings")}
+    case KeyValidation.validate_provider_key(api_key, provider_type) do
+      {:error, message} ->
+        form = form_with_api_key_error(socket, api_key, message)
+        {:noreply, assign(socket, :form, form)}
 
-      {:error, _reason} ->
-        {:noreply, put_flash(socket, :error, "Could not connect — check your API key")}
+      {:ok, api_key} ->
+        case Products.connect_provider(api_key, provider_type) do
+          {:ok, _connection} ->
+            {:noreply,
+             socket
+             |> put_flash(:info, "Connected to #{socket.assigns.provider.name}!")
+             |> push_navigate(to: ~p"/admin/settings")}
+
+          {:error, _reason} ->
+            form =
+              form_with_api_key_error(
+                socket,
+                api_key,
+                "Could not connect. Check your API key and try again"
+              )
+
+            {:noreply, assign(socket, :form, form)}
+        end
     end
   end
 
@@ -110,26 +98,14 @@ defmodule BerrypodWeb.Admin.Providers.Form do
     end
   end
 
-  defp encrypt_api_key(api_key) do
-    case Berrypod.Vault.encrypt(api_key) do
-      {:ok, encrypted} -> encrypted
-      _ -> nil
-    end
+  defp form_with_api_key_error(socket, api_key, message) do
+    socket.assigns.connection
+    |> ProviderConnection.changeset(%{"api_key" => api_key})
+    |> Ecto.Changeset.add_error(:api_key, message)
+    |> Map.put(:action, :validate)
+    |> to_form()
   end
 
   defp validated_type(type) when type in @supported_types, do: type
   defp validated_type(_), do: "printify"
-
-  # Shared helpers used by the template
-
-  defp connection_name({:ok, %{shop_name: name}}), do: name
-  defp connection_name({:ok, %{store_name: name}}), do: name
-  defp connection_name(_), do: nil
-
-  defp format_error(:no_api_key), do: "Please enter your API key"
-  defp format_error(:unauthorized), do: "That key doesn't seem to be valid"
-  defp format_error(:timeout), do: "Couldn't reach the provider - try again"
-  defp format_error({:http_error, _code}), do: "Something went wrong - try again"
-  defp format_error(error) when is_binary(error), do: error
-  defp format_error(_), do: "Connection failed - check your key and try again"
 end
diff --git a/lib/berrypod_web/live/admin/providers/form.html.heex b/lib/berrypod_web/live/admin/providers/form.html.heex
index a170d7c..fe7398c 100644
--- a/lib/berrypod_web/live/admin/providers/form.html.heex
+++ b/lib/berrypod_web/live/admin/providers/form.html.heex
@@ -32,7 +32,7 @@
 
     <.input
       field={@form[:api_key]}
-      type="password"
+      type="text"
       label={"#{@provider.name} API key"}
       placeholder={
         if @live_action == :edit,
@@ -42,44 +42,14 @@
       autocomplete="off"
     />
 
-    <div class="admin-inline-group">
-      <button
-        type="button"
-        class="admin-btn admin-btn-outline admin-btn-sm"
-        phx-click="test_connection"
-        disabled={@testing}
-      >
-        <.icon
-          name={if @testing, do: "hero-arrow-path", else: "hero-signal"}
-          class={if @testing, do: "size-4 animate-spin", else: "size-4"}
-        />
-        {if @testing, do: "Checking...", else: "Check connection"}
-      </button>
-
-      <%= if @test_result do %>
-        <%= case @test_result do %>
-          <% {:ok, _info} -> %>
-            <span class="admin-status-success">
-              <.icon name="hero-check-circle" class="size-4" />
-              Connected to {connection_name(@test_result) || @provider.name}
-            </span>
-          <% {:error, reason} -> %>
-            <span class="admin-status-error">
-              <.icon name="hero-x-circle" class="size-4" />
-              {format_error(reason)}
-            </span>
-        <% end %>
-      <% end %>
-    </div>
-
     <%= if @live_action == :edit do %>
       <.input field={@form[:enabled]} type="checkbox" label="Connection enabled" />
     <% end %>
 
     <div class="admin-form-actions">
-      <.button type="submit" disabled={@testing}>
+      <.button type="submit">
         {if @live_action == :new,
-          do: "Connect to #{@provider.name}",
+          do: "Connect",
           else: "Save changes"}
       </.button>
       <.link navigate={~p"/admin/providers"} class="admin-btn admin-btn-ghost">
diff --git a/lib/berrypod_web/live/setup/onboarding.ex b/lib/berrypod_web/live/setup/onboarding.ex
index 3f4402a..0c8eb24 100644
--- a/lib/berrypod_web/live/setup/onboarding.ex
+++ b/lib/berrypod_web/live/setup/onboarding.ex
@@ -1,7 +1,7 @@
 defmodule BerrypodWeb.Setup.Onboarding do
   use BerrypodWeb, :live_view
 
-  alias Berrypod.{Accounts, Products, Settings, Setup}
+  alias Berrypod.{Accounts, KeyValidation, Products, Settings, Setup}
   alias Berrypod.Providers.Provider
   alias Berrypod.Stripe.Setup, as: StripeSetup
 
@@ -131,75 +131,114 @@ defmodule BerrypodWeb.Setup.Onboarding do
   def handle_event("connect_provider", %{"provider" => %{"api_key" => api_key}}, socket) do
     type = socket.assigns.selected_provider
 
-    if api_key == "" do
-      {:noreply, put_flash(socket, :error, "Please enter your API token")}
-    else
-      socket = assign(socket, provider_connecting: true)
+    case KeyValidation.validate_provider_key(api_key, type) do
+      {:error, message} ->
+        form =
+          to_form(%{"api_key" => api_key},
+            as: :provider,
+            errors: [api_key: {message, []}],
+            action: :validate
+          )
 
-      case Products.connect_provider(api_key, type) do
-        {:ok, connection} ->
-          setup = Setup.setup_status()
+        {:noreply, assign(socket, :provider_form, form)}
+
+      {:ok, api_key} ->
+        socket = assign(socket, :provider_connecting, true)
+
+        case Products.connect_provider(api_key, type) do
+          {:ok, connection} ->
+            setup = Setup.setup_status()
+
+            if setup.setup_complete do
+              {:noreply,
+               socket
+               |> put_flash(:info, "You're in! Here's your launch checklist.")
+               |> push_navigate(to: ~p"/admin")}
+            else
+              {:noreply,
+               socket
+               |> assign(:provider_connecting, false)
+               |> assign(:provider_conn, connection)
+               |> assign(:setup, setup)
+               |> put_flash(:info, "Connected! Product sync started in the background.")}
+            end
+
+          {:error, :no_api_key} ->
+            form =
+              to_form(%{"api_key" => api_key},
+                as: :provider,
+                errors: [api_key: {"Please enter your API token", []}],
+                action: :validate
+              )
 
-          if setup.setup_complete do
-            {:noreply,
-             socket
-             |> put_flash(:info, "You're in! Here's your launch checklist.")
-             |> push_navigate(to: ~p"/admin")}
-          else
             {:noreply,
              socket
              |> assign(:provider_connecting, false)
-             |> assign(:provider_conn, connection)
-             |> assign(:setup, setup)
-             |> put_flash(:info, "Connected! Product sync started in the background.")}
-          end
+             |> assign(:provider_form, form)}
 
-        {:error, :no_api_key} ->
-          {:noreply,
-           socket
-           |> assign(:provider_connecting, false)
-           |> put_flash(:error, "Please enter your API token")}
+          {:error, _reason} ->
+            form =
+              to_form(%{"api_key" => api_key},
+                as: :provider,
+                errors: [api_key: {"Could not connect. Check your API key and try again", []}],
+                action: :validate
+              )
 
-        {:error, _reason} ->
-          {:noreply,
-           socket
-           |> assign(:provider_connecting, false)
-           |> put_flash(:error, "Could not connect — check your API key and try again")}
-      end
+            {:noreply,
+             socket
+             |> assign(:provider_connecting, false)
+             |> assign(:provider_form, form)}
+        end
     end
   end
 
   # ── Events: Stripe ──
 
   def handle_event("connect_stripe", %{"stripe" => %{"api_key" => api_key}}, socket) do
-    if api_key == "" do
-      {:noreply, put_flash(socket, :error, "Please enter your Stripe secret key")}
-    else
-      socket = assign(socket, stripe_connecting: true)
+    case KeyValidation.validate_stripe_key(api_key) do
+      {:error, message} ->
+        form =
+          to_form(%{"api_key" => api_key},
+            as: :stripe,
+            errors: [api_key: {message, []}],
+            action: :validate
+          )
 
-      case StripeSetup.connect(api_key) do
-        {:ok, _result} ->
-          setup = Setup.setup_status()
+        {:noreply, assign(socket, :stripe_form, form)}
+
+      {:ok, api_key} ->
+        socket = assign(socket, :stripe_connecting, true)
+
+        case StripeSetup.connect(api_key) do
+          {:ok, _result} ->
+            setup = Setup.setup_status()
+
+            if setup.setup_complete do
+              {:noreply,
+               socket
+               |> put_flash(:info, "You're in! Here's your launch checklist.")
+               |> push_navigate(to: ~p"/admin")}
+            else
+              {:noreply,
+               socket
+               |> assign(:stripe_connecting, false)
+               |> assign(:setup, setup)
+               |> put_flash(:info, "Stripe connected")}
+            end
+
+          {:error, message} ->
+            form =
+              to_form(%{"api_key" => api_key},
+                as: :stripe,
+                errors: [api_key: {"Stripe connection failed: #{message}", []}],
+                action: :validate
+              )
 
-          if setup.setup_complete do
-            {:noreply,
-             socket
-             |> put_flash(:info, "You're in! Here's your launch checklist.")
-             |> push_navigate(to: ~p"/admin")}
-          else
             {:noreply,
              socket
              |> assign(:stripe_connecting, false)
-             |> assign(:setup, setup)
-             |> put_flash(:info, "Stripe connected")}
-          end
-
-        {:error, message} ->
-          {:noreply,
-           socket
-           |> assign(:stripe_connecting, false)
-           |> put_flash(:error, "Stripe connection failed: #{message}")}
-      end
+             |> assign(:stripe_form, form)}
+        end
     end
   end
 
@@ -208,6 +247,7 @@ defmodule BerrypodWeb.Setup.Onboarding do
   @impl true
   def render(assigns) do
     ~H"""
+    <Layouts.flash_group flash={@flash} />
     <div class="setup-page">
       <div class="setup-header">
         <h1 class="setup-title">Set up your shop</h1>
@@ -400,7 +440,7 @@ defmodule BerrypodWeb.Setup.Onboarding do
         <.form for={@form} phx-submit="connect_provider">
           <.input
             field={@form[:api_key]}
-            type="password"
+            type="text"
             label="API token"
             placeholder="Paste your token here"
             autocomplete="off"
@@ -436,15 +476,12 @@ defmodule BerrypodWeb.Setup.Onboarding do
       <.form for={@form} phx-submit="connect_stripe">
         <.input
           field={@form[:api_key]}
-          type="password"
+          type="text"
           label="Secret key"
           autocomplete="off"
           placeholder="sk_test_... or sk_live_..."
           phx-mounted={@focus && JS.focus()}
         />
-        <p class="setup-key-hint">
-          Starts with <code>sk_test_</code> or <code>sk_live_</code>. Encrypted at rest.
-        </p>
         <div class="setup-actions">
           <.button phx-disable-with="Connecting...">
             {if @connecting, do: "Connecting...", else: "Connect"}
diff --git a/test/berrypod/key_validation_test.exs b/test/berrypod/key_validation_test.exs
new file mode 100644
index 0000000..fb7e70d
--- /dev/null
+++ b/test/berrypod/key_validation_test.exs
@@ -0,0 +1,194 @@
+defmodule Berrypod.KeyValidationTest do
+  use ExUnit.Case, async: true
+
+  alias Berrypod.KeyValidation
+
+  describe "validate_stripe_key/1" do
+    test "accepts valid test key" do
+      key = "sk_test_51OUJ0abc123XYZdef456"
+      assert {:ok, ^key} = KeyValidation.validate_stripe_key(key)
+    end
+
+    test "accepts valid live key" do
+      key = "sk_live_51OUJ0abc123XYZdef456"
+      assert {:ok, ^key} = KeyValidation.validate_stripe_key(key)
+    end
+
+    test "strips whitespace" do
+      key = "sk_test_51OUJ0abc123XYZdef456"
+      assert {:ok, ^key} = KeyValidation.validate_stripe_key("  #{key}  ")
+    end
+
+    test "strips newlines from paste" do
+      key = "sk_test_51OUJ0abc123XYZdef456"
+      assert {:ok, ^key} = KeyValidation.validate_stripe_key("#{key}\n")
+    end
+
+    test "rejects empty key" do
+      assert {:error, msg} = KeyValidation.validate_stripe_key("")
+      assert msg =~ "Please enter"
+    end
+
+    test "rejects nil" do
+      assert {:error, msg} = KeyValidation.validate_stripe_key(nil)
+      assert msg =~ "Please enter"
+    end
+
+    test "rejects whitespace-only" do
+      assert {:error, msg} = KeyValidation.validate_stripe_key("   ")
+      assert msg =~ "Please enter"
+    end
+
+    test "detects publishable key" do
+      assert {:error, msg} = KeyValidation.validate_stripe_key("pk_test_abc123")
+      assert msg =~ "publishable key"
+      assert msg =~ "secret key"
+    end
+
+    test "detects restricted key" do
+      assert {:error, msg} = KeyValidation.validate_stripe_key("rk_live_abc123")
+      assert msg =~ "restricted key"
+    end
+
+    test "rejects key without correct prefix" do
+      assert {:error, msg} = KeyValidation.validate_stripe_key("some_random_key_value")
+      assert msg =~ "sk_test_"
+    end
+
+    test "rejects too-short key with correct prefix" do
+      assert {:error, msg} = KeyValidation.validate_stripe_key("sk_test_x")
+      assert msg =~ "too short"
+    end
+  end
+
+  describe "validate_provider_key/2" do
+    test "accepts reasonable key" do
+      assert {:ok, "abcdef1234567890abcdef"} =
+               KeyValidation.validate_provider_key("abcdef1234567890abcdef", "printify")
+    end
+
+    test "strips whitespace" do
+      assert {:ok, "abcdef1234567890abcdef"} =
+               KeyValidation.validate_provider_key("  abcdef1234567890abcdef  ", "printful")
+    end
+
+    test "rejects empty key" do
+      assert {:error, msg} = KeyValidation.validate_provider_key("", "printify")
+      assert msg =~ "Please enter"
+    end
+
+    test "rejects nil" do
+      assert {:error, msg} = KeyValidation.validate_provider_key(nil, "printify")
+      assert msg =~ "Please enter"
+    end
+
+    test "rejects too-short key" do
+      assert {:error, msg} = KeyValidation.validate_provider_key("short", "printify")
+      assert msg =~ "too short"
+    end
+
+    test "works without provider type" do
+      assert {:ok, "abcdef1234567890abcdef"} =
+               KeyValidation.validate_provider_key("abcdef1234567890abcdef")
+    end
+  end
+
+  describe "validate_email_key/3 - SendGrid" do
+    test "accepts valid SendGrid key" do
+      key = "SG.abcdefghijklmnopqrstuv.abcdefghijklmnopqrstuvwxyz0123456789ABCDEFG"
+      assert {:ok, ^key} = KeyValidation.validate_email_key(key, "sendgrid", "api_key")
+    end
+
+    test "rejects key without SG. prefix" do
+      assert {:error, msg} =
+               KeyValidation.validate_email_key("not_a_sendgrid_key", "sendgrid", "api_key")
+
+      assert msg =~ "SG."
+    end
+
+    test "strips whitespace" do
+      key = "SG.abcdefghijklmnopqrstuv.abcdefghijklmnopqrstuvwxyz0123456789ABCDEFG"
+      assert {:ok, ^key} = KeyValidation.validate_email_key("  #{key}  ", "sendgrid", "api_key")
+    end
+  end
+
+  describe "validate_email_key/3 - Postmark" do
+    test "accepts valid UUID" do
+      key = "abc12345-abcd-1234-abcd-123456789abc"
+      assert {:ok, ^key} = KeyValidation.validate_email_key(key, "postmark", "api_key")
+    end
+
+    test "accepts uppercase UUID" do
+      key = "ABC12345-ABCD-1234-ABCD-123456789ABC"
+      assert {:ok, ^key} = KeyValidation.validate_email_key(key, "postmark", "api_key")
+    end
+
+    test "rejects non-UUID" do
+      assert {:error, msg} = KeyValidation.validate_email_key("not-a-uuid", "postmark", "api_key")
+      assert msg =~ "UUID"
+    end
+  end
+
+  describe "validate_email_key/3 - Resend" do
+    test "accepts valid Resend key" do
+      assert {:ok, "re_abc123xyz"} =
+               KeyValidation.validate_email_key("re_abc123xyz", "resend", "api_key")
+    end
+
+    test "rejects key without re_ prefix" do
+      assert {:error, msg} = KeyValidation.validate_email_key("not_resend", "resend", "api_key")
+      assert msg =~ "re_"
+    end
+  end
+
+  describe "validate_email_key/3 - Mailgun" do
+    test "accepts classic key- format" do
+      key = "key-abcdef1234567890abcdef1234567890"
+      assert {:ok, ^key} = KeyValidation.validate_email_key(key, "mailgun", "api_key")
+    end
+
+    test "accepts newer long key without prefix" do
+      key = "abcdef1234567890abcdef1234567890abcdef1234"
+      assert {:ok, ^key} = KeyValidation.validate_email_key(key, "mailgun", "api_key")
+    end
+
+    test "rejects short key without prefix" do
+      assert {:error, msg} = KeyValidation.validate_email_key("short", "mailgun", "api_key")
+      assert msg =~ "key-"
+    end
+  end
+
+  describe "validate_email_key/3 - Brevo" do
+    test "accepts valid Brevo key" do
+      key = "xkeysib-" <> String.duplicate("ab", 32)
+      assert {:ok, ^key} = KeyValidation.validate_email_key(key, "brevo", "api_key")
+    end
+
+    test "rejects key without xkeysib- prefix" do
+      assert {:error, msg} = KeyValidation.validate_email_key("not_brevo_key", "brevo", "api_key")
+      assert msg =~ "xkeysib-"
+    end
+  end
+
+  describe "validate_email_key/3 - unknown providers" do
+    test "accepts reasonable key for unknown adapter" do
+      assert {:ok, "some_api_key_12345"} =
+               KeyValidation.validate_email_key("some_api_key_12345", "mailersend", "api_key")
+    end
+
+    test "accepts reasonable key for non-api_key fields" do
+      assert {:ok, "smtp.example.com"} =
+               KeyValidation.validate_email_key("smtp.example.com", "smtp", "relay")
+    end
+
+    test "rejects very short value" do
+      assert {:error, msg} = KeyValidation.validate_email_key("ab", "mailersend", "api_key")
+      assert msg =~ "too short"
+    end
+
+    test "rejects empty" do
+      assert {:error, msg} = KeyValidation.validate_email_key("", "mailersend", "api_key")
+      assert msg =~ "Please enter"
+    end
+  end
+end
diff --git a/test/berrypod_web/live/admin/email_settings_test.exs b/test/berrypod_web/live/admin/email_settings_test.exs
index 381ba89..c07786f 100644
--- a/test/berrypod_web/live/admin/email_settings_test.exs
+++ b/test/berrypod_web/live/admin/email_settings_test.exs
@@ -82,11 +82,11 @@ defmodule BerrypodWeb.Admin.EmailSettingsTest do
       |> form("form[phx-change=\"change_adapter\"]", %{email: %{adapter: "postmark"}})
       |> render_change()
 
-      # Submit with an API key
+      # Submit with an API key (Postmark uses UUID format)
       html =
         view
         |> form("form[phx-submit=\"save\"]", %{
-          email: %{adapter: "postmark", api_key: "pm_test_123"}
+          email: %{adapter: "postmark", api_key: "abc12345-abcd-1234-abcd-123456789abc"}
         })
         |> render_submit()
 
@@ -108,7 +108,7 @@ defmodule BerrypodWeb.Admin.EmailSettingsTest do
         |> form("form[phx-submit=\"save\"]", %{email: %{adapter: "postmark", api_key: ""}})
         |> render_submit()
 
-      assert html =~ "Missing required fields"
+      assert html =~ "API key is required"
     end
 
     test "disconnect clears email configuration", %{conn: conn} do
@@ -167,7 +167,7 @@ defmodule BerrypodWeb.Admin.EmailSettingsTest do
 
       view
       |> form("form[phx-submit=\"save\"]", %{
-        email: %{adapter: "postmark", api_key: "pm_new_key"}
+        email: %{adapter: "postmark", api_key: "def12345-abcd-1234-abcd-123456789def"}
       })
       |> render_submit()
 
diff --git a/test/berrypod_web/live/admin/providers_test.exs b/test/berrypod_web/live/admin/providers_test.exs
index cd77d46..30856f8 100644
--- a/test/berrypod_web/live/admin/providers_test.exs
+++ b/test/berrypod_web/live/admin/providers_test.exs
@@ -167,14 +167,6 @@ defmodule BerrypodWeb.Admin.ProvidersTest do
       assert html =~ "Connect to Printify"
     end
 
-    test "test connection shows error when no api key", %{conn: conn} do
-      {:ok, view, _html} = live(conn, ~p"/admin/providers/new")
-
-      html = render_click(view, "test_connection")
-
-      assert html =~ "Please enter your API key"
-    end
-
     test "saves new connection", %{conn: conn} do
       expect(MockProvider, :test_connection, fn _conn ->
         {:ok, %{shop_name: "My Printify Shop", shop_id: 12345}}
@@ -196,55 +188,6 @@ defmodule BerrypodWeb.Admin.ProvidersTest do
     end
   end
 
-  describe "form - test connection" do
-    setup %{conn: conn, user: user} do
-      Application.put_env(:berrypod, :provider_modules, %{
-        "printify" => MockProvider
-      })
-
-      on_exit(fn -> Application.delete_env(:berrypod, :provider_modules) end)
-
-      %{conn: log_in_user(conn, user)}
-    end
-
-    test "shows success when connection is valid", %{conn: conn} do
-      expect(MockProvider, :test_connection, fn _conn ->
-        {:ok, %{shop_name: "My Printify Shop", shop_id: 12345}}
-      end)
-
-      {:ok, view, _html} = live(conn, ~p"/admin/providers/new")
-
-      # Validate first to set pending_api_key
-      view
-      |> form("#provider-form", %{
-        "provider_connection" => %{"api_key" => "valid_key_123"}
-      })
-      |> render_change()
-
-      html = render_click(view, "test_connection")
-
-      assert html =~ "Connected to My Printify Shop"
-    end
-
-    test "shows error when connection fails", %{conn: conn} do
-      expect(MockProvider, :test_connection, fn _conn ->
-        {:error, :unauthorized}
-      end)
-
-      {:ok, view, _html} = live(conn, ~p"/admin/providers/new")
-
-      view
-      |> form("#provider-form", %{
-        "provider_connection" => %{"api_key" => "bad_key"}
-      })
-      |> render_change()
-
-      html = render_click(view, "test_connection")
-
-      assert html =~ "doesn&#39;t seem to be valid"
-    end
-  end
-
   describe "form - edit" do
     setup %{conn: conn, user: user} do
       connection =