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"""
- <.icon name="hero-exclamation-circle" class="size-5" /> +
+ <.icon name="hero-exclamation-circle" class="admin-error-icon" /> {render_slot(@inner_block)}
""" 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"""@@ -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" /> -
- Starts with sk_test_ or sk_live_. Encrypted at rest.
-