diff --git a/lib/recognizer/accounts/notification_preference.ex b/lib/recognizer/accounts/notification_preference.ex index 76306af..23acf15 100644 --- a/lib/recognizer/accounts/notification_preference.ex +++ b/lib/recognizer/accounts/notification_preference.ex @@ -9,7 +9,7 @@ defmodule Recognizer.Accounts.NotificationPreference do alias __MODULE__, as: NotificationPreference schema "notification_preferences" do - field :two_factor, Recognizer.TwoFactorPreference, default: :text + field :two_factor, Recognizer.TwoFactorPreference, default: :app belongs_to :user, User diff --git a/lib/recognizer_web/controllers/accounts/user_settings_controller.ex b/lib/recognizer_web/controllers/accounts/user_settings_controller.ex index b444a0b..4ab222e 100644 --- a/lib/recognizer_web/controllers/accounts/user_settings_controller.ex +++ b/lib/recognizer_web/controllers/accounts/user_settings_controller.ex @@ -35,7 +35,6 @@ defmodule RecognizerWeb.Accounts.UserSettingsController do Accounts.get_new_two_factor_settings(user) if method == "text" || method == "voice" do - # TODO error without user phone (redirect here with flash error, or stop at settings update) :ok = Accounts.send_new_two_factor_notification(user, settings) render(conn, "confirm_two_factor_external.html") else @@ -93,6 +92,9 @@ defmodule RecognizerWeb.Accounts.UserSettingsController do end end + @doc """ + Form submission for settings applied + """ def update(conn, %{"action" => "update", "user" => user_params}) do user = Authentication.fetch_current_user(conn) @@ -125,6 +127,7 @@ defmodule RecognizerWeb.Accounts.UserSettingsController do end end + # disable 2fa def update(conn, %{"action" => "update_two_factor", "user" => %{"two_factor_enabled" => "0"}}) do user = Authentication.fetch_current_user(conn) @@ -135,18 +138,27 @@ defmodule RecognizerWeb.Accounts.UserSettingsController do end end - @doc """ - Form submission to begin two factor setup - """ - def update(conn, %{"action" => "update_two_factor", "user" => user_params}) do - user = Authentication.fetch_current_user(conn) - preference = get_in(user_params, ["notification_preference", "two_factor"]) - - Accounts.generate_and_cache_new_two_factor_settings(user, preference) + # enable 2fa + def update(conn, %{ + "action" => "update_two_factor", + "user" => %{"notification_preference" => %{"two_factor" => preference}} + }) do + %{phone_number: phone_number} = user = Authentication.fetch_current_user(conn) - redirect(conn, to: Routes.user_settings_path(conn, :review)) + # phone number required for text/voice + if (preference == "text" || preference == "voice") && phone_number == nil do + conn + |> put_flash(:error, "Phone number required for text and voice two-factor methods") + |> redirect(to: Routes.user_settings_path(conn, :edit)) + else + Accounts.generate_and_cache_new_two_factor_settings(user, preference) + redirect(conn, to: Routes.user_settings_path(conn, :review)) + end end + @doc """ + Review recovery codes for copying. + """ def review(conn, _params) do user = Authentication.fetch_current_user(conn) diff --git a/test/recognizer_web/controllers/accounts/user_settings_controller_test.exs b/test/recognizer_web/controllers/accounts/user_settings_controller_test.exs index 438db30..f00b491 100644 --- a/test/recognizer_web/controllers/accounts/user_settings_controller_test.exs +++ b/test/recognizer_web/controllers/accounts/user_settings_controller_test.exs @@ -114,10 +114,35 @@ defmodule RecognizerWeb.Accounts.UserSettingsControllerTest do assert response =~ "Change Profile" assert response =~ "must not contain special characters" end - end - # todo test "requires account phone number for text" - # TODO another for app bypass + test "update two-factor redirects for text method without phone number", %{conn: conn, user: user} do + stub(HTTPoisonMock, :put, fn _, _, _ -> ok_bigcommerce_response() end) + Accounts.update_user(user, %{phone_number: nil}) + + conn = + put(conn, Routes.user_settings_path(conn, :update), %{ + "action" => "update_two_factor", + "user" => %{"notification_preference" => %{"two_factor" => "text"}} + }) + + assert redirected_to(conn) =~ "/settings" + assert get_flash(conn, :error) =~ "Phone number required" + end + + test "update two-factor allows app setup without a phone number", %{conn: conn, user: user} do + stub(HTTPoisonMock, :put, fn _, _, _ -> ok_bigcommerce_response() end) + Accounts.update_user(user, %{phone_number: nil}) + + conn = + put(conn, Routes.user_settings_path(conn, :edit), %{ + "action" => "update_two_factor", + "user" => %{"notification_preference" => %{"two_factor" => "app"}} + }) + + assert redirected_to(conn) =~ "/settings/two-factor/review" + refute get_flash(conn, :error) + end + end describe "GET /users/settings/two-factor/review (backup codes)" do test "gets review page after 2fa setup", %{conn: conn, user: user} do