From b69f2980582cbddabd144e908c150b1695406bfe Mon Sep 17 00:00:00 2001 From: Thales Macedo Garitezi Date: Fri, 28 Jun 2024 14:42:42 -0300 Subject: [PATCH] fix(dashboard): handle add default user race condition This can happen at least in tests, when nodes boot concurrently. --- apps/emqx_dashboard/src/emqx_dashboard.app.src | 2 +- apps/emqx_dashboard/src/emqx_dashboard_admin.erl | 16 +++++++++++++--- .../test/emqx_dashboard_admin_SUITE.erl | 2 +- 3 files changed, 15 insertions(+), 5 deletions(-) diff --git a/apps/emqx_dashboard/src/emqx_dashboard.app.src b/apps/emqx_dashboard/src/emqx_dashboard.app.src index a4a133df1..2835feb34 100644 --- a/apps/emqx_dashboard/src/emqx_dashboard.app.src +++ b/apps/emqx_dashboard/src/emqx_dashboard.app.src @@ -2,7 +2,7 @@ {application, emqx_dashboard, [ {description, "EMQX Web Dashboard"}, % strict semver, bump manually! - {vsn, "5.1.2"}, + {vsn, "5.1.3"}, {modules, []}, {registered, [emqx_dashboard_sup]}, {applications, [ diff --git a/apps/emqx_dashboard/src/emqx_dashboard_admin.erl b/apps/emqx_dashboard/src/emqx_dashboard_admin.erl index 62e39c64b..04694f434 100644 --- a/apps/emqx_dashboard/src/emqx_dashboard_admin.erl +++ b/apps/emqx_dashboard/src/emqx_dashboard_admin.erl @@ -63,6 +63,8 @@ -type emqx_admin() :: #?ADMIN{}. +-define(USERNAME_ALREADY_EXISTS_ERROR, <<"username_already_exists">>). + %%-------------------------------------------------------------------- %% Mnesia bootstrap %%-------------------------------------------------------------------- @@ -214,7 +216,7 @@ add_user_(Username, Password, Role, Desc) -> username => Username, role => Role }), - mnesia:abort(<<"username_already_exist">>) + mnesia:abort(?USERNAME_ALREADY_EXISTS_ERROR) end. -spec remove_user(dashboard_username()) -> {ok, any()} | {error, any()}. @@ -411,8 +413,16 @@ add_default_user(Username, Password) when ?EMPTY_KEY(Username) orelse ?EMPTY_KEY {ok, empty}; add_default_user(Username, Password) -> case lookup_user(Username) of - [] -> do_add_user(Username, Password, ?ROLE_SUPERUSER, <<"administrator">>); - _ -> {ok, default_user_exists} + [] -> + case do_add_user(Username, Password, ?ROLE_SUPERUSER, <<"administrator">>) of + {error, ?USERNAME_ALREADY_EXISTS_ERROR} -> + %% race condition: multiple nodes booting at the same time? + {ok, default_user_exists}; + Res -> + Res + end; + _ -> + {ok, default_user_exists} end. %% ensure the `role` is correct when it is directly read from the table diff --git a/apps/emqx_dashboard/test/emqx_dashboard_admin_SUITE.erl b/apps/emqx_dashboard/test/emqx_dashboard_admin_SUITE.erl index f0e569670..08beae685 100644 --- a/apps/emqx_dashboard/test/emqx_dashboard_admin_SUITE.erl +++ b/apps/emqx_dashboard/test/emqx_dashboard_admin_SUITE.erl @@ -69,7 +69,7 @@ t_add_user(_) -> false = maps:is_key(password, NewUser), %% add again - {error, <<"username_already_exist">>} = + {error, <<"username_already_exists">>} = emqx_dashboard_admin:add_user(AddUser, AddPassword, ?ROLE_SUPERUSER, AddDescription), %% add bad username