From 8b3413519a78c806843fd509dabfa22dcde007c0 Mon Sep 17 00:00:00 2001 From: Yongjin Chong Date: Mon, 7 Jul 2025 12:29:45 -0600 Subject: [PATCH 1/2] Update email login cases - only 422 email exist case can success now --- lib/recognizer/accounts.ex | 12 +- lib/recognizer/bigcommerce.ex | 83 ++++++-- lib/recognizer/bigcommerce/client.ex | 51 ++++- .../accounts/prompt/two_factor_controller.ex | 22 ++- .../accounts/user_settings_controller.ex | 177 +++++++++++++----- 5 files changed, 273 insertions(+), 72 deletions(-) diff --git a/lib/recognizer/accounts.ex b/lib/recognizer/accounts.ex index 158d279e..e57ded9e 100644 --- a/lib/recognizer/accounts.ex +++ b/lib/recognizer/accounts.ex @@ -652,8 +652,16 @@ defmodule Recognizer.Accounts do settings. """ def check_two_factor_notification_time(user) do - {:ok, attrs} = get_new_two_factor_settings(user) - check_two_factor_notification_time(attrs, 100) + case get_new_two_factor_settings(user) do + {:ok, attrs} when not is_nil(attrs) -> + check_two_factor_notification_time(attrs, 100) + + {:ok, nil} -> + {:error, :no_two_factor_settings} + + {:error, reason} -> + {:error, reason} + end end def check_two_factor_notification_time(attrs, two_factor_issue_time) do diff --git a/lib/recognizer/bigcommerce.ex b/lib/recognizer/bigcommerce.ex index 0ad6afcc..db671743 100644 --- a/lib/recognizer/bigcommerce.ex +++ b/lib/recognizer/bigcommerce.ex @@ -16,16 +16,70 @@ defmodule Recognizer.BigCommerce do def create_customer(user) do case Client.create_customer(user) do - {:ok, bc_id} -> - Repo.insert(%Customer{user_id: user.id, bc_id: bc_id}) - {:ok, user} + {:ok, :email_already_exists} -> + handle_email_already_exists(user) + + {:ok, bc_id} when is_integer(bc_id) -> + handle_new_customer_created(user, bc_id) {:error, e} -> - Logger.error("error creating bigcommerce customer: #{inspect(e)}") - {:error, e} + handle_customer_creation_error(user, e) + end + end + + defp handle_email_already_exists(user) do + Logger.info("BigCommerce customer email already exists for user #{user.id}, attempting to link existing customer") + + case Client.get_customers(emails: [user.email]) do + {:ok, [customer_id | _]} -> + link_existing_customer(user, customer_id) + + {:ok, []} -> + handle_customer_not_found_error(user) + + {:error, e} -> + handle_get_customers_error(user, e) end end + defp link_existing_customer(user, customer_id) do + case Repo.insert(%Customer{user_id: user.id, bc_id: customer_id}) do + {:ok, _} -> + Logger.info("Successfully linked existing BigCommerce customer #{customer_id} to user #{user.id}") + {:ok, user} + + {:error, changeset} -> + Logger.error("Failed to link BigCommerce customer to user: #{inspect(changeset)}") + {:error, {:bigcommerce_link_failed, changeset}} + end + end + + defp handle_customer_not_found_error(user) do + Logger.error("BigCommerce reported email exists but customer not found via API for user #{user.id}") + {:error, {:bigcommerce_customer_not_found, "Email exists but customer not found"}} + end + + defp handle_get_customers_error(user, e) do + Logger.error("Failed to get existing BigCommerce customer for user #{user.id}: #{inspect(e)}") + {:error, {:bigcommerce_api_error, e}} + end + + defp handle_new_customer_created(user, bc_id) do + case Repo.insert(%Customer{user_id: user.id, bc_id: bc_id}) do + {:ok, _} -> + {:ok, user} + + {:error, changeset} -> + Logger.error("Failed to save BigCommerce customer ID to database: #{inspect(changeset)}") + {:ok, user} + end + end + + defp handle_customer_creation_error(user, e) do + Logger.error("BigCommerce customer creation failed: #{inspect(e)}") + {:error, e} + end + def get_or_create_customer(%{email: email, id: id} = user) do Logger.info("Starting BigCommerce get_or_create_customer for user #{id} with email #{email}") @@ -54,17 +108,13 @@ defmodule Recognizer.BigCommerce do {:error, e} -> Logger.error("Error while getting BigCommerce customer: #{inspect(e)}") - # Don't fail account creation due to BigCommerce API errors - # This ensures verification emails are still sent - Logger.info("Continuing account creation process despite BigCommerce error") - {:ok, user} + # Apply strict approach: fail account creation for BigCommerce API errors + {:error, e} e -> Logger.error("Unexpected error while getting or creating BigCommerce customer: #{inspect(e)}") - # Don't fail account creation due to BigCommerce errors - # This ensures verification emails are still sent - Logger.info("Continuing account creation process despite unexpected BigCommerce error") - {:ok, user} + # Apply strict approach: fail account creation for unexpected errors + {:error, "Unexpected BigCommerce error"} end end @@ -75,11 +125,16 @@ defmodule Recognizer.BigCommerce do def update_customer(user) do case Client.update_customer(Repo.preload(user, :bigcommerce_user)) do + {:ok, :email_already_exists} -> + # When updating, email already exists is actually a success condition + Logger.info("BigCommerce customer update: email already exists for user #{user.id}, treating as success") + {:ok, user} + {:ok, _} -> {:ok, user} {:error, e} -> - Logger.error("error creating bigcommerce customer: #{inspect(e)}") + Logger.error("BigCommerce customer update failed: #{inspect(e)}") {:error, e} end end diff --git a/lib/recognizer/bigcommerce/client.ex b/lib/recognizer/bigcommerce/client.ex index f89165aa..7a6e54c9 100644 --- a/lib/recognizer/bigcommerce/client.ex +++ b/lib/recognizer/bigcommerce/client.ex @@ -48,6 +48,9 @@ defmodule Recognizer.BigCommerce.Client do {:ok, %Response{body: response, status_code: 200}} -> {:ok, response} + {:ok, %Response{status_code: 422, body: body}} -> + handle_422_error(body, "create") + {:ok, %Response{status_code: 429, headers: headers}} -> sleep_for_rate_limit(headers) post_customer(customer_json) @@ -65,6 +68,9 @@ defmodule Recognizer.BigCommerce.Client do {:ok, %Response{body: response, status_code: 200}} -> {:ok, response} + {:ok, %Response{status_code: 422, body: body}} -> + handle_422_error(body, "update") + {:ok, %Response{status_code: 429, headers: headers}} -> sleep_for_rate_limit(headers) put_customer(customer_json) @@ -77,6 +83,36 @@ defmodule Recognizer.BigCommerce.Client do end end + # Precisely analyze 422 errors to determine if it's email duplication + defp handle_422_error(body, operation) do + case Jason.decode(body) do + {:ok, %{"errors" => %{".customer_update" => error}}} when is_binary(error) -> + if String.contains?(String.downcase(error), "email") and + String.contains?(String.downcase(error), "already in use") do + Logger.info("BigCommerce customer #{operation}: email already exists, treating as success - #{error}") + {:ok, :email_already_exists} + else + Logger.error("BigCommerce customer #{operation} failed with validation error: #{error}") + {:error, {:validation_error, error}} + end + + {:ok, %{"errors" => errors}} -> + Logger.error("BigCommerce customer #{operation} failed with errors: #{inspect(errors)}") + {:error, {:validation_errors, errors}} + + {:ok, decoded} -> + Logger.error("BigCommerce customer #{operation} failed with unexpected 422 response: #{inspect(decoded)}") + {:error, {:unexpected_422, decoded}} + + {:error, json_error} -> + Logger.error( + "BigCommerce customer #{operation} failed with unparseable 422 response: #{body}, JSON error: #{inspect(json_error)}" + ) + + {:error, {:unparseable_422, body}} + end + end + def get_customers(queries \\ []) do with params <- customer_queries_as_params(queries), full_uri <- customers_uri(), @@ -125,10 +161,17 @@ defmodule Recognizer.BigCommerce.Client do end defp get_id(response) do - case Jason.decode(response) do - {:ok, %{"data" => [%{"id" => id}]}} -> {:ok, id} - {:error, e} -> {:error, e} - e -> {:error, e} + case response do + :email_already_exists -> + # If email already exists, return special response for upper level handling + {:ok, :email_already_exists} + + _ -> + case Jason.decode(response) do + {:ok, %{"data" => [%{"id" => id}]}} -> {:ok, id} + {:error, e} -> {:error, e} + e -> {:error, e} + end end end diff --git a/lib/recognizer_web/controllers/accounts/prompt/two_factor_controller.ex b/lib/recognizer_web/controllers/accounts/prompt/two_factor_controller.ex index c4942adc..526d8cfa 100644 --- a/lib/recognizer_web/controllers/accounts/prompt/two_factor_controller.ex +++ b/lib/recognizer_web/controllers/accounts/prompt/two_factor_controller.ex @@ -25,12 +25,24 @@ defmodule RecognizerWeb.Accounts.Prompt.TwoFactorController do def edit(conn, _params) do user = conn.assigns.user - {:ok, %{two_factor_seed: seed}} = Accounts.get_new_two_factor_settings(user) - render(conn, "confirm.html", - barcode: Authentication.generate_totp_barcode(user, seed), - totp_app_url: Authentication.get_totp_app_url(user, seed) - ) + case Accounts.get_new_two_factor_settings(user) do + {:ok, %{two_factor_seed: seed}} -> + render(conn, "confirm.html", + barcode: Authentication.generate_totp_barcode(user, seed), + totp_app_url: Authentication.get_totp_app_url(user, seed) + ) + + {:ok, nil} -> + conn + |> put_flash(:error, "Two factor setup not found. Please set up two factor authentication first.") + |> redirect(to: Routes.prompt_two_factor_path(conn, :new)) + + {:error, _reason} -> + conn + |> put_flash(:error, "Error retrieving two factor settings. Please try again.") + |> redirect(to: Routes.prompt_two_factor_path(conn, :new)) + end end def update(conn, params) do diff --git a/lib/recognizer_web/controllers/accounts/user_settings_controller.ex b/lib/recognizer_web/controllers/accounts/user_settings_controller.ex index 606c5dd4..6cc207da 100644 --- a/lib/recognizer_web/controllers/accounts/user_settings_controller.ex +++ b/lib/recognizer_web/controllers/accounts/user_settings_controller.ex @@ -48,79 +48,162 @@ defmodule RecognizerWeb.Accounts.UserSettingsController do """ def two_factor_init(conn, _params) do user = Authentication.fetch_current_user(conn) - current_user = Accounts.get_new_two_factor_settings(user) - {:ok, %{two_factor_seed: seed, notification_preference: %{two_factor: method}} = setting_user} = current_user + case Accounts.get_new_two_factor_settings(user) do + {:ok, nil} -> + handle_missing_two_factor_settings(conn) + + {:ok, %{two_factor_seed: seed, notification_preference: %{two_factor: method}} = setting_user} -> + handle_two_factor_init_with_settings(conn, user, setting_user, seed, method) + + {:error, _reason} -> + handle_two_factor_settings_error(conn) + + _ -> + handle_invalid_two_factor_format(conn) + end + end + + defp handle_missing_two_factor_settings(conn) do + conn + |> put_flash( + :error, + "Two factor setup expired or not yet initiated. Please enable two factor authentication first." + ) + |> redirect(to: Routes.user_settings_path(conn, :edit)) + end + + defp handle_two_factor_init_with_settings(conn, user, setting_user, seed, method) do method_atom = normalize_to_atom(method) if method in [:app, "app"] do - render(conn, "confirm_two_factor.html", - barcode: Authentication.generate_totp_barcode(user, seed), - totp_app_url: Authentication.get_totp_app_url(user, seed) - ) + render_app_two_factor(conn, user, seed) + else + render_external_two_factor(conn, setting_user, user, method_atom) + end + end + + defp render_app_two_factor(conn, user, seed) do + render(conn, "confirm_two_factor.html", + barcode: Authentication.generate_totp_barcode(user, seed), + totp_app_url: Authentication.get_totp_app_url(user, seed) + ) + end + + defp render_external_two_factor(conn, setting_user, user, method_atom) do + conn = ensure_two_factor_issue_time_session(conn) + conn = maybe_send_two_factor_notification(conn, setting_user, user, method_atom) + render(conn, "confirm_two_factor_external.html") + end + + defp ensure_two_factor_issue_time_session(conn) do + if get_session(conn, :two_factor_issue_time) == nil do + put_session(conn, :two_factor_issue_time, System.system_time(:second)) else - conn = - if get_session(conn, :two_factor_issue_time) == nil do - put_session(conn, :two_factor_issue_time, System.system_time(:second)) - else - conn - end - - two_factor_sent = get_session(conn, :two_factor_sent) - - conn = - if two_factor_sent do - conn - else - conn = put_session(conn, :two_factor_sent, true) - - conn - |> send_two_factor_notification(setting_user, user, method_atom) - end - - render(conn, "confirm_two_factor_external.html") + conn end end + defp maybe_send_two_factor_notification(conn, setting_user, user, method_atom) do + two_factor_sent = get_session(conn, :two_factor_sent) + + if two_factor_sent do + conn + else + conn + |> put_session(:two_factor_sent, true) + |> send_two_factor_notification(setting_user, user, method_atom) + end + end + + defp handle_two_factor_settings_error(conn) do + conn + |> put_flash(:error, "Error retrieving two factor settings. Please try again.") + |> redirect(to: Routes.user_settings_path(conn, :edit)) + end + + defp handle_invalid_two_factor_format(conn) do + conn + |> put_flash(:error, "Invalid two factor settings format. Please contact support.") + |> redirect(to: Routes.user_settings_path(conn, :edit)) + end + @doc """ Confirming and saving a new two factor setup with user-provided code """ def two_factor_confirm(conn, params) do user = Authentication.fetch_current_user(conn) - current_user = Accounts.get_new_two_factor_settings(user) - two_factor_code = Map.get(params, "two_factor_code") - {:ok, %{notification_preference: %{two_factor: method}} = setting_user} = current_user + case Accounts.get_new_two_factor_settings(user) do + {:ok, nil} -> + handle_missing_two_factor_settings(conn) + + {:ok, %{notification_preference: %{two_factor: method}} = setting_user} -> + handle_two_factor_confirm_with_settings(conn, user, setting_user, two_factor_code, method) + + {:error, _reason} -> + handle_two_factor_settings_error(conn) + + _ -> + handle_invalid_two_factor_format(conn) + end + end + + defp handle_two_factor_confirm_with_settings(conn, user, setting_user, two_factor_code, method) do current_time = System.system_time(:second) conn = ensure_two_factor_issue_time(conn, current_time) - two_factor_issue_time = get_session(conn, :two_factor_issue_time) method_atom = normalize_to_atom(method) if Authentication.valid_token?(method_atom, two_factor_code, two_factor_issue_time, setting_user.two_factor_seed) do - if current_time - two_factor_issue_time > 900 do - conn = - conn - |> put_session(:two_factor_issue_time, current_time) + handle_valid_two_factor_token( + conn, + user, + setting_user, + two_factor_code, + method_atom, + current_time, + two_factor_issue_time + ) + else + handle_invalid_two_factor_token(conn) + end + end - conn - |> send_two_factor_notification(setting_user, user, method_atom) - |> put_flash( - :error, - "Two-factor code has expired. A new code has been sent. Please check your email for the newest two-factor code and try again." - ) - |> redirect(to: Routes.user_settings_path(conn, :two_factor_confirm)) - else - handle_two_factor_settings(conn, user, two_factor_code, method_atom) - end + defp handle_valid_two_factor_token( + conn, + user, + setting_user, + two_factor_code, + method_atom, + current_time, + two_factor_issue_time + ) do + if current_time - two_factor_issue_time > 900 do + handle_expired_two_factor_code(conn, setting_user, user, method_atom, current_time) else - conn - |> put_flash(:error, "Two factor code is invalid") - |> redirect(to: Routes.user_settings_path(conn, :two_factor_confirm)) + handle_two_factor_settings(conn, user, two_factor_code, method_atom) end end + defp handle_expired_two_factor_code(conn, setting_user, user, method_atom, current_time) do + conn + |> put_session(:two_factor_issue_time, current_time) + |> send_two_factor_notification(setting_user, user, method_atom) + |> put_flash( + :error, + "Two-factor code has expired. A new code has been sent. Please check your email for the newest two-factor code and try again." + ) + |> redirect(to: Routes.user_settings_path(conn, :two_factor_confirm)) + end + + defp handle_invalid_two_factor_token(conn) do + conn + |> put_flash(:error, "Two factor code is invalid") + |> redirect(to: Routes.user_settings_path(conn, :two_factor_confirm)) + end + defp ensure_two_factor_issue_time(conn, current_time) do if get_session(conn, :two_factor_issue_time) == nil do put_session(conn, :two_factor_issue_time, current_time) From 774245583f3bbb2825c547f8204e9cdf1c27e6e6 Mon Sep 17 00:00:00 2001 From: Yongjin Chong Date: Mon, 7 Jul 2025 15:05:17 -0600 Subject: [PATCH 2/2] Update email login cases - only 422 email exist case can success now --- lib/recognizer/bigcommerce.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/recognizer/bigcommerce.ex b/lib/recognizer/bigcommerce.ex index db671743..70a14751 100644 --- a/lib/recognizer/bigcommerce.ex +++ b/lib/recognizer/bigcommerce.ex @@ -76,7 +76,7 @@ defmodule Recognizer.BigCommerce do end defp handle_customer_creation_error(user, e) do - Logger.error("BigCommerce customer creation failed: #{inspect(e)}") + Logger.error("BigCommerce customer creation failed for user #{user.id}: #{inspect(e)}") {:error, e} end