From a1ba6d3af690b3d638d99893ac7f2a4cef18d54c Mon Sep 17 00:00:00 2001 From: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> Date: Thu, 2 Jul 2026 02:36:47 -0700 Subject: [PATCH 1/2] fix: allow password change in user settings --- .../widgets/pages/settings/UserSettings.jsx | 77 +++++++++++- .../pages/settings/UserSettingsForm.jsx | 99 ++++++++++++++- apps/codebattle/lib/codebattle/user.ex | 59 ++++++++- .../controllers/api/v1/settings_controller.ex | 19 +++ apps/codebattle/lib/codebattle_web/router.ex | 2 + .../api/v1/settings_controller_test.exs | 119 +++++++++++++++++- 6 files changed, 359 insertions(+), 16 deletions(-) diff --git a/apps/codebattle/assets/js/widgets/pages/settings/UserSettings.jsx b/apps/codebattle/assets/js/widgets/pages/settings/UserSettings.jsx index e91f13a94..47871944e 100644 --- a/apps/codebattle/assets/js/widgets/pages/settings/UserSettings.jsx +++ b/apps/codebattle/assets/js/widgets/pages/settings/UserSettings.jsx @@ -24,6 +24,11 @@ const notifications = { error: { variant: "danger", message: i18n.t("Something went wrong") }, empty: {}, }; +const emptyPasswordValues = { + currentPassword: "", + password: "", + passwordConfirmation: "", +}; const csrfToken = document?.querySelector("meta[name='csrf-token']")?.getAttribute("content"); const updateSettings = async (values) => { @@ -46,6 +51,49 @@ const updateSettings = async (values) => { return data; }; +const updatePassword = async (values) => { + const response = await fetch("/api/v1/settings/password", { + method: "PATCH", + headers: { + "Content-Type": "application/json", + "x-csrf-token": csrfToken, + }, + body: JSON.stringify(decamelizeKeys(values)), + }); + const data = await response.json(); + + if (!response.ok) { + const error = new Error(`Request failed with status ${response.status}`); + error.response = { data, status: response.status }; + throw error; + } + + return data; +}; + +const getPasswordValues = ({ currentPassword, password, passwordConfirmation }) => ({ + currentPassword, + password, + passwordConfirmation, +}); + +const getSettingsValues = ({ + currentPassword, + password, + passwordConfirmation, + ...settingsValues +}) => settingsValues; + +const hasPasswordChanges = (values) => + Object.values(values).some((value) => String(value || "").trim() !== ""); + +const formatFieldErrors = (errors = {}) => + Object.entries(camelizeKeys(errors)).reduce((acc, [fieldName, messages]) => { + const fieldMessages = Array.isArray(messages) ? messages : [messages]; + acc[fieldName] = fieldMessages.map(capitalize).join(", "); + return acc; + }, {}); + function Notification({ notification, onClose }) { const { variant, message } = notification; @@ -103,12 +151,23 @@ function UserSettings() { const dispatch = useDispatch(); const handleUpdateUserSettings = useCallback( - async (values, { setErrors }) => { + async (values, { setErrors, resetForm, settingsDirty }) => { + const passwordValues = getPasswordValues(values); + const settingsValues = getSettingsValues(values); + try { - const data = await updateSettings(values); + if (settingsDirty) { + const data = await updateSettings(settingsValues); + + await i18n.changeLanguage(getSupportedLocale(data.locale)); + dispatch(actions.updateUserSettings(camelizeKeys(data))); + } - await i18n.changeLanguage(getSupportedLocale(data.locale)); - dispatch(actions.updateUserSettings(camelizeKeys(data))); + if (hasPasswordChanges(passwordValues)) { + await updatePassword(passwordValues); + } + + resetForm({ values: { ...settingsValues, ...emptyPasswordValues } }); setNotification(notifications.success); } catch (error) { if (!error.response) { @@ -116,8 +175,14 @@ function UserSettings() { return; } - const { name: userNameErrors = [] } = error.response.data.errors; - setErrors({ name: userNameErrors.map(capitalize).join(", ") }); + const fieldErrors = formatFieldErrors(error.response.data.errors); + + if (Object.keys(fieldErrors).length === 0) { + setNotification(notifications.error); + return; + } + + setErrors(fieldErrors); } }, [dispatch], diff --git a/apps/codebattle/assets/js/widgets/pages/settings/UserSettingsForm.jsx b/apps/codebattle/assets/js/widgets/pages/settings/UserSettingsForm.jsx index fbc4244b7..ff574b6c3 100644 --- a/apps/codebattle/assets/js/widgets/pages/settings/UserSettingsForm.jsx +++ b/apps/codebattle/assets/js/widgets/pages/settings/UserSettingsForm.jsx @@ -21,6 +21,7 @@ const views = { sql: "sql", }; +const passwordFieldNames = ["currentPassword", "password", "passwordConfirmation"]; const playingLanguages = Object.entries(omit(languages, [...cssProcessors, ...dbNames])); const cssLanguages = Object.entries(pick(languages, cssProcessors)); const databaseTypes = Object.entries(pick(languages, dbNames)); @@ -53,6 +54,50 @@ const getPlaceholder = ({ disabled, placeholder }) => { return "No access yet"; }; +const hasPasswordValue = (values) => + passwordFieldNames.some((fieldName) => String(values[fieldName] || "").trim() !== ""); + +const passwordValidationSchema = { + currentPassword: Yup.string().test( + "required-current-password", + "Field can't be empty", + function (value) { + return !hasPasswordValue(this.parent) || String(value || "").trim() !== ""; + }, + ), + password: Yup.string() + .test("required-password", "Field can't be empty", function (value) { + return !hasPasswordValue(this.parent) || String(value || "").trim() !== ""; + }) + .test("password-min-length", "Should be at least 6 characters", function (value) { + return !hasPasswordValue(this.parent) || String(value || "").length >= 6; + }), + passwordConfirmation: Yup.string() + .test("required-password-confirmation", "Field can't be empty", function (value) { + return !hasPasswordValue(this.parent) || String(value || "").trim() !== ""; + }) + .test("password-confirmation", "Passwords must match", function (value) { + return !hasPasswordValue(this.parent) || value === this.parent.password; + }), +}; + +const canChangePassword = (settings) => { + if (typeof settings.hasPassword === "boolean") { + return settings.hasPassword; + } + + const hasLinkedProvider = Boolean( + settings.githubId || + settings.discordId || + settings.externalOauthLogin || + settings.externalPlatformId || + settings.externalPlatformLogin || + settings.firebaseUid, + ); + + return !settings.isGuest && !hasLinkedProvider; +}; + function TextInput({ label, ...props }) { const [field, meta] = useField(props); const { name, disabled, hint, hintHref = "", ...inputProps } = props; @@ -182,11 +227,18 @@ function UserSettingsForm({ onSubmit, settings }) { lang: settings.lang || "", styleLang: settings.styleLang || "", dbType: settings.dbType || "", + currentPassword: "", + password: "", + passwordConfirmation: "", }), [settings], ); - const validationSchema = useMemo(() => Yup.object(schemas.userSettings(settings)), [settings]); + const validationSchema = useMemo( + () => Yup.object({ ...schemas.userSettings(settings), ...passwordValidationSchema }), + [settings], + ); + const showPasswordFields = canChangePassword(settings); return ( + onSubmit(values, { + ...helpers, + settingsDirty: + JSON.stringify(omit(values, passwordFieldNames)) !== + JSON.stringify(omit(initialValues, passwordFieldNames)), + }) + } > {({ handleChange, dirty, isValid, isSubmitting, values }) => (
@@ -252,6 +311,42 @@ function UserSettingsForm({ onSubmit, settings }) { + {showPasswordFields && ( +
+
+
+ + + +
+
+
+ )} +
Select sound type
diff --git a/apps/codebattle/lib/codebattle/user.ex b/apps/codebattle/lib/codebattle/user.ex index 9a14dcc0a..12873ba4d 100644 --- a/apps/codebattle/lib/codebattle/user.ex +++ b/apps/codebattle/lib/codebattle/user.ex @@ -87,7 +87,10 @@ defmodule Codebattle.User do field(:timezone, :string, default: "Etc/UTC") field(:games_played, :integer, virtual: true) + field(:current_password, :string, virtual: true) field(:is_guest, :boolean, virtual: true, default: false) + field(:password, :string, virtual: true) + field(:password_confirmation, :string, virtual: true) embeds_one(:sound_settings, SoundSettings, on_replace: :update) @@ -149,6 +152,17 @@ defmodule Codebattle.User do |> assign_clan(params, user.id) end + def password_changeset(user, params \\ %{}) do + user + |> cast(params, [:current_password, :password, :password_confirmation]) + |> validate_required([:current_password, :password, :password_confirmation]) + |> validate_not_blank(:password) + |> validate_length(:password, min: 6, message: "should be at least 6 character(s)") + |> validate_confirmation(:password) + |> validate_current_password() + |> put_password_hash() + end + def token_changeset(user, params \\ %{}) do user |> cast(params, [ @@ -276,6 +290,8 @@ defmodule Codebattle.User do available_login_methods_count(user) > 1 end + def has_password?(user), do: present?(user.password_hash) + def update_subscription_type(user_id, type) do user_id |> get!() @@ -315,9 +331,11 @@ defmodule Codebattle.User do end end - defp verify_password(_user, nil), do: nil + def verify_password(_user, nil), do: nil + + def verify_password(%__MODULE__{password_hash: password_hash}, _password) when password_hash in [nil, ""], do: nil - defp verify_password(user, password) do + def verify_password(user, password) do if Bcrypt.verify_pass(password, user.password_hash) do user end @@ -330,10 +348,8 @@ defmodule Codebattle.User do end def create_password_hash(user, password) do - hashed_password = Bcrypt.hash_pwd_salt(password) - Repo.update_all(from(u in __MODULE__, where: u.id == ^user.id), - set: [password_hash: hashed_password] + set: [password_hash: hash_password(password)] ) end @@ -353,6 +369,39 @@ defmodule Codebattle.User do defp present?(value), do: not is_nil(value) and value != "" + defp validate_not_blank(changeset, field) do + validate_change(changeset, field, fn _, value -> + if String.trim(to_string(value)) == "" do + [{field, "can't be blank"}] + else + [] + end + end) + end + + defp validate_current_password(%{valid?: true} = changeset) do + if verify_password(changeset.data, get_change(changeset, :current_password)) do + changeset + else + add_error(changeset, :current_password, "is invalid") + end + end + + defp validate_current_password(changeset), do: changeset + + defp put_password_hash(%{valid?: true} = changeset) do + changeset + |> get_change(:password) + |> case do + nil -> changeset + password -> put_change(changeset, :password_hash, hash_password(password)) + end + end + + defp put_password_hash(changeset), do: changeset + + defp hash_password(password), do: Bcrypt.hash_pwd_salt(password) + defp assign_clan(changeset, %{:clan => clan}, _user_id) when clan in ["", nil], do: change(changeset, %{clan: nil, clan_id: nil}) diff --git a/apps/codebattle/lib/codebattle_web/controllers/api/v1/settings_controller.ex b/apps/codebattle/lib/codebattle_web/controllers/api/v1/settings_controller.ex index 806d7e703..3ac17f8a2 100644 --- a/apps/codebattle/lib/codebattle_web/controllers/api/v1/settings_controller.ex +++ b/apps/codebattle/lib/codebattle_web/controllers/api/v1/settings_controller.ex @@ -12,6 +12,7 @@ defmodule CodebattleWeb.Api.V1.SettingsController do clan: current_user.clan, locale: current_user.locale, can_unlink_social: User.can_unlink_social?(current_user), + has_password: User.has_password?(current_user), discord_id: current_user.discord_id, discord_name: current_user.discord_name, github_id: current_user.github_id, @@ -36,6 +37,7 @@ defmodule CodebattleWeb.Api.V1.SettingsController do clan: user.clan, locale: user.locale, can_unlink_social: User.can_unlink_social?(user), + has_password: User.has_password?(user), sound_settings: user.sound_settings, lang: user.lang, style_lang: user.style_lang, @@ -48,4 +50,21 @@ defmodule CodebattleWeb.Api.V1.SettingsController do |> json(%{errors: translate_errors(changeset)}) end end + + def update_password(conn, params) do + current_user = conn.assigns.current_user + + current_user + |> User.password_changeset(params) + |> Repo.update() + |> case do + {:ok, _user} -> + json(conn, %{status: "ok", has_password: true}) + + {:error, %Ecto.Changeset{} = changeset} -> + conn + |> put_status(:unprocessable_entity) + |> json(%{errors: translate_errors(changeset)}) + end + end end diff --git a/apps/codebattle/lib/codebattle_web/router.ex b/apps/codebattle/lib/codebattle_web/router.ex index 99eafb700..e813b9ef2 100644 --- a/apps/codebattle/lib/codebattle_web/router.ex +++ b/apps/codebattle/lib/codebattle_web/router.ex @@ -339,6 +339,8 @@ defmodule CodebattleWeb.Router do scope("/games") do resources("/:game_id/user_game_reports", UserGameReportController, only: [:create]) end + + patch("/settings/password", SettingsController, :update_password) end end diff --git a/apps/codebattle/test/codebattle_web/controllers/api/v1/settings_controller_test.exs b/apps/codebattle/test/codebattle_web/controllers/api/v1/settings_controller_test.exs index d9c380c08..e265d2b28 100644 --- a/apps/codebattle/test/codebattle_web/controllers/api/v1/settings_controller_test.exs +++ b/apps/codebattle/test/codebattle_web/controllers/api/v1/settings_controller_test.exs @@ -2,6 +2,10 @@ defmodule CodebattleWeb.Api.V1.SettingsControllerTest do use CodebattleWeb.ConnCase, async: false alias Codebattle.Repo + alias Codebattle.User + + @old_password "old-password!" + @new_password "new-password!" describe "#show" do test "shows current user settings", %{conn: conn} do @@ -37,7 +41,8 @@ defmodule CodebattleWeb.Api.V1.SettingsControllerTest do "db_type" => "mongodb", "style_lang" => "less", "github_id" => 1, - "github_name" => "g_name" + "github_name" => "g_name", + "has_password" => false } end end @@ -67,8 +72,9 @@ defmodule CodebattleWeb.Api.V1.SettingsControllerTest do new_settings |> Map.put("clan", "Bca") |> Map.put("can_unlink_social", true) + |> Map.put("has_password", false) - updated = Repo.get!(Codebattle.User, user.id) + updated = Repo.get!(User, user.id) assert updated.sound_settings.level == 3 assert updated.sound_settings.tournament_level == 8 @@ -92,7 +98,7 @@ defmodule CodebattleWeb.Api.V1.SettingsControllerTest do assert json_response(conn, 422) == %{"errors" => %{"name" => ["can't be blank"]}} - updated = Repo.get!(Codebattle.User, user.id) + updated = Repo.get!(User, user.id) assert updated.name == user.name end @@ -110,4 +116,111 @@ defmodule CodebattleWeb.Api.V1.SettingsControllerTest do assert json_response(conn, 422) == %{"errors" => %{"name" => ["has already been taken"]}} end end + + describe "#update_password" do + test "updates password for current user", %{conn: conn} do + user = insert_user_with_password() + old_password_hash = user.password_hash + + conn = + conn + |> put_session(:user_id, user.id) + |> patch("/api/v1/settings/password", %{ + "current_password" => @old_password, + "password" => @new_password, + "password_confirmation" => @new_password + }) + + assert json_response(conn, 200) == %{"status" => "ok", "has_password" => true} + + updated = Repo.get!(User, user.id) + + refute updated.password_hash == old_password_hash + assert %User{id: user_id} = User.authenticate(user.name, @new_password) + assert user_id == user.id + refute User.authenticate(user.name, @old_password) + end + + test "does not update password when new password is too short", %{conn: conn} do + user = insert_user_with_password() + + conn = + conn + |> put_session(:user_id, user.id) + |> patch("/api/v1/settings/password", %{ + "current_password" => @old_password, + "password" => "short", + "password_confirmation" => "short" + }) + + assert json_response(conn, 422) == %{ + "errors" => %{"password" => ["should be at least 6 character(s)"]} + } + + updated = Repo.get!(User, user.id) + + assert updated.password_hash == user.password_hash + end + + test "does not update password when new password is blank", %{conn: conn} do + user = insert_user_with_password() + + conn = + conn + |> put_session(:user_id, user.id) + |> patch("/api/v1/settings/password", %{ + "current_password" => @old_password, + "password" => " ", + "password_confirmation" => " " + }) + + response = json_response(conn, 422) + + assert "can't be blank" in response["errors"]["password"] + + updated = Repo.get!(User, user.id) + + assert updated.password_hash == user.password_hash + end + + test "does not update password when current password is wrong", %{conn: conn} do + user = insert_user_with_password() + + conn = + conn + |> put_session(:user_id, user.id) + |> patch("/api/v1/settings/password", %{ + "current_password" => "wrong-password", + "password" => @new_password, + "password_confirmation" => @new_password + }) + + assert json_response(conn, 422) == %{"errors" => %{"current_password" => ["is invalid"]}} + + updated = Repo.get!(User, user.id) + + assert updated.password_hash == user.password_hash + end + + test "rejects guest request", %{conn: conn} do + conn = + patch(conn, "/api/v1/settings/password", %{ + "current_password" => @old_password, + "password" => @new_password, + "password_confirmation" => @new_password + }) + + assert json_response(conn, 401) == %{"error" => "oiblz"} + end + end + + defp insert_user_with_password do + insert(:user, %{ + github_id: nil, + github_name: nil, + discord_id: nil, + discord_name: nil, + password_hash: Bcrypt.hash_pwd_salt(@old_password) + }) + end end From afdcc518445e982d2843fbe7ea1834a9c4f5e771 Mon Sep 17 00:00:00 2001 From: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> Date: Thu, 2 Jul 2026 02:44:51 -0700 Subject: [PATCH 2/2] fix: send has_password in initial gon payload; trust it in settings form --- .../assets/js/__tests__/UserSettings.test.jsx | 49 +++++++++++++++++++ .../pages/settings/UserSettingsForm.jsx | 15 +----- .../lib/codebattle_web/plugs/assign_gon.ex | 1 + .../codebattle_web/plugs/assign_gon_test.exs | 13 +++++ 4 files changed, 64 insertions(+), 14 deletions(-) diff --git a/apps/codebattle/assets/js/__tests__/UserSettings.test.jsx b/apps/codebattle/assets/js/__tests__/UserSettings.test.jsx index e9f19a211..c820d2757 100644 --- a/apps/codebattle/assets/js/__tests__/UserSettings.test.jsx +++ b/apps/codebattle/assets/js/__tests__/UserSettings.test.jsx @@ -52,6 +52,7 @@ const preloadedState = { canUnlinkSocial: false, discordName: null, discordId: null, + hasPassword: false, error: "", }, }, @@ -60,6 +61,19 @@ const store = configureStore({ reducer, preloadedState, }); +const buildStore = (settings = {}) => + configureStore({ + reducer, + preloadedState: { + user: { + settings: { + ...preloadedState.user.settings, + ...settings, + }, + }, + }, + }); + jest.mock( "gon", () => { @@ -218,6 +232,41 @@ describe("UserSettings test cases", () => { expect(await findByRole("alert")).toHaveClass("alert-danger"); }); + test("hides password fields for Firebase user without local password", () => { + const customStore = buildStore({ + firebaseUid: "firebase-user", + hasPassword: false, + }); + + const { queryByTestId } = setup( + + + , + ); + + expect(queryByTestId("currentPasswordInput")).not.toBeInTheDocument(); + expect(queryByTestId("passwordInput")).not.toBeInTheDocument(); + expect(queryByTestId("passwordConfirmationInput")).not.toBeInTheDocument(); + }); + + test("shows password fields for local password user with linked OAuth", () => { + const customStore = buildStore({ + githubId: 19, + githubName: "octocat", + hasPassword: true, + }); + + const { getByTestId } = setup( + + + , + ); + + expect(getByTestId("currentPasswordInput")).toBeInTheDocument(); + expect(getByTestId("passwordInput")).toBeInTheDocument(); + expect(getByTestId("passwordConfirmationInput")).toBeInTheDocument(); + }); + test("unlink button is disabled when it is the last sign-in method", () => { const customStore = configureStore({ reducer, diff --git a/apps/codebattle/assets/js/widgets/pages/settings/UserSettingsForm.jsx b/apps/codebattle/assets/js/widgets/pages/settings/UserSettingsForm.jsx index ff574b6c3..8e23fc579 100644 --- a/apps/codebattle/assets/js/widgets/pages/settings/UserSettingsForm.jsx +++ b/apps/codebattle/assets/js/widgets/pages/settings/UserSettingsForm.jsx @@ -82,20 +82,7 @@ const passwordValidationSchema = { }; const canChangePassword = (settings) => { - if (typeof settings.hasPassword === "boolean") { - return settings.hasPassword; - } - - const hasLinkedProvider = Boolean( - settings.githubId || - settings.discordId || - settings.externalOauthLogin || - settings.externalPlatformId || - settings.externalPlatformLogin || - settings.firebaseUid, - ); - - return !settings.isGuest && !hasLinkedProvider; + return settings.hasPassword === true; }; function TextInput({ label, ...props }) { diff --git a/apps/codebattle/lib/codebattle_web/plugs/assign_gon.ex b/apps/codebattle/lib/codebattle_web/plugs/assign_gon.ex index 8c035175a..dbaae3430 100644 --- a/apps/codebattle/lib/codebattle_web/plugs/assign_gon.ex +++ b/apps/codebattle/lib/codebattle_web/plugs/assign_gon.ex @@ -67,6 +67,7 @@ defmodule CodebattleWeb.Plugs.AssignGon do :sound_settings ]) |> Map.put(:can_unlink_social, Codebattle.User.can_unlink_social?(user)) + |> Map.put(:has_password, Codebattle.User.has_password?(user)) |> Map.put(:is_admin, Codebattle.User.admin?(user)) end end diff --git a/apps/codebattle/test/codebattle_web/plugs/assign_gon_test.exs b/apps/codebattle/test/codebattle_web/plugs/assign_gon_test.exs index 421e0a278..ae78116b3 100644 --- a/apps/codebattle/test/codebattle_web/plugs/assign_gon_test.exs +++ b/apps/codebattle/test/codebattle_web/plugs/assign_gon_test.exs @@ -17,4 +17,17 @@ defmodule CodebattleWeb.Plugs.AssignGonTest do assert %{avatar_url: "https://example.com/avatar.png"} = get_gon(conn, :current_user) end + + test "includes has_password in current_user gon payload", %{conn: conn} do + user = insert(:user, github_id: 1, password_hash: "password-hash") + + conn = + conn + |> PhoenixGon.Pipeline.call(PhoenixGon.Pipeline.init([])) + |> put_private(:phoenix_endpoint, CodebattleWeb.Endpoint) + |> assign(:current_user, user) + |> AssignGon.call([]) + + assert %{has_password: true} = get_gon(conn, :current_user) + end end