From 60a1021efe09f09f8245b5b9ff92df447e575513 Mon Sep 17 00:00:00 2001 From: Hans Svensson Date: Tue, 12 May 2020 16:08:29 +0200 Subject: [PATCH 1/3] Whitespace fix micro_profile --- apps/roster/src/protocol/micro_profile.erl | 143 +++++++++++---------- 1 file changed, 72 insertions(+), 71 deletions(-) diff --git a/apps/roster/src/protocol/micro_profile.erl b/apps/roster/src/protocol/micro_profile.erl index e1fe37d2b..a046d044d 100644 --- a/apps/roster/src/protocol/micro_profile.erl +++ b/apps/roster/src/protocol/micro_profile.erl @@ -6,84 +6,85 @@ -compile(export_all). info(#'Profile'{phone=LinkPhone, rosters=Rosters, status=create, settings = Settings} = Profile, Req, %% TODO are Rosters sent? - #cx{params = ClientId = <<"sys_", _/binary>>, client_pid = C} = State) -> - Phone = micro:uuid_to_id('LinkProfile', LinkPhone), + #cx{params = ClientId = <<"sys_", _/binary>>, client_pid = C} = State) -> + Phone = micro:uuid_to_id('LinkProfile', LinkPhone), ?LOG_INFO("~p:~p:Profile/create:~p", [LinkPhone,ClientId,Profile]), - IO2 = case kvs:get('Profile', Phone) of - {ok, #'Profile'{}} -> - ?LOG_INFO("~p:~p:Profile/already_exist", [LinkPhone, ClientId]), - #io{code = #error{code = already_exist}, data = LinkPhone}; - _ -> - OldPhone = roster:get_data_val(<<"PHONE">>, Settings), - GetProfile = kvs:get('Profile', OldPhone), - case OldPhone == [] orelse element(1, GetProfile) == error of - true -> - kvs:put(Profile#'Profile'{status = [], rosters = []}), - kvs:put(#'LinkProfile'{id = LinkPhone, phone = LinkPhone}), - Res = lists:flatten([begin - {reply, {bert, IO}, _, _} = - micro_roster:info(Roster#'Roster'{status = add}, Req, State), - case IO of #io{} -> IO#io{data = LinkPhone}; #'Roster'{} -> [] end - end || #'Roster'{} = Roster <- Rosters]), - case Res of - [] -> Profile; - [Err | _] -> - kvs:delete('Profile', LinkPhone), %% Delete the created Profile - kvs:delete('LinkProfile', LinkPhone), %% Delete the created Linkage for Profile - Err - end; - _ -> - StoredProfile = element(2, GetProfile), - P = roster:patch_profile(StoredProfile, Profile), - kvs:put(P2 = P#'Profile'{status = []}), - roster:send_ses(C, OldPhone, P#'Profile'{status = patch}), - micro:link_user(P2, Profile), - Res = lists:flatten( - [begin - {reply, {bert, IO}, _, _} = - micro_roster:info(Roster#'Roster'{status = update}, Req, State), - case IO of #io{} -> IO#io{data = LinkPhone}; #'Roster'{} -> [] end - end || #'Roster'{} = Roster <- Rosters]), - case Res of - [] -> Profile; - [Err | _] -> - kvs:put(StoredProfile), %% Return the old Profile - Err - end - end - end, - ?LOG_INFO("~p:~p:Profile/create.result:~p", [LinkPhone,ClientId, IO2]), + IO2 = + case kvs:get('Profile', Phone) of + {ok, #'Profile'{}} -> + ?LOG_INFO("~p:~p:Profile/already_exist", [LinkPhone, ClientId]), + #io{code = #error{code = already_exist}, data = LinkPhone}; + _ -> + OldPhone = roster:get_data_val(<<"PHONE">>, Settings), + GetProfile = kvs:get('Profile', OldPhone), + case OldPhone == [] orelse element(1, GetProfile) == error of + true -> + kvs:put(Profile#'Profile'{status = [], rosters = []}), + kvs:put(#'LinkProfile'{id = LinkPhone, phone = LinkPhone}), + Res = lists:flatten([begin + {reply, {bert, IO}, _, _} = + micro_roster:info(Roster#'Roster'{status = add}, Req, State), + case IO of #io{} -> IO#io{data = LinkPhone}; #'Roster'{} -> [] end + end || #'Roster'{} = Roster <- Rosters]), + case Res of + [] -> Profile; + [Err | _] -> + kvs:delete('Profile', LinkPhone), %% Delete the created Profile + kvs:delete('LinkProfile', LinkPhone), %% Delete the created Linkage for Profile + Err + end; + false -> + StoredProfile = element(2, GetProfile), + P = roster:patch_profile(StoredProfile, Profile), + kvs:put(P2 = P#'Profile'{status = []}), + roster:send_ses(C, OldPhone, P#'Profile'{status = patch}), + micro:link_user(P2, Profile), + Res = lists:flatten( + [begin + {reply, {bert, IO}, _, _} = + micro_roster:info(Roster#'Roster'{status = update}, Req, State), + case IO of #io{} -> IO#io{data = LinkPhone}; #'Roster'{} -> [] end + end || #'Roster'{} = Roster <- Rosters]), + case Res of + [] -> Profile; + [Err | _] -> + kvs:put(StoredProfile), %% Return the old Profile + Err + end + end + end, + ?LOG_INFO("~p:~p:Profile/create.result:~p", [LinkPhone,ClientId, IO2]), {reply, {bert, IO2}, Req, State}; info(#'Profile'{phone=LinkPhone, rosters=[], status = update} = Profile, Req, - #cx{params = ClientId = <<"sys_", _/binary>>, client_pid = C} = State) -> - Phone = micro:uuid_to_id('LinkProfile', LinkPhone), - ?LOG_INFO("~p:~p:Profile(micro)/update:~s", [LinkPhone,ClientId,io_lib:format("~p",[Profile])]), - IO = case kvs:get('Profile', Phone) of - {ok, #'Profile'{phone = Phone} = StoredProfile} -> - P = roster:patch_profile(StoredProfile, Profile), - kvs:put(P#'Profile'{status = []}), - roster:send_ses(C, Phone, P#'Profile'{status = update}), - P#'Profile'{phone = LinkPhone, status = update}; - {error, _} -> - #io{code = #error{code = profile_not_found}, data = LinkPhone} - end, - {reply, {bert, IO}, Req, State}; + #cx{params = ClientId = <<"sys_", _/binary>>, client_pid = C} = State) -> + Phone = micro:uuid_to_id('LinkProfile', LinkPhone), + ?LOG_INFO("~p:~p:Profile(micro)/update:~s", [LinkPhone,ClientId,io_lib:format("~p",[Profile])]), + IO = case kvs:get('Profile', Phone) of + {ok, #'Profile'{phone = Phone} = StoredProfile} -> + P = roster:patch_profile(StoredProfile, Profile), + kvs:put(P#'Profile'{status = []}), + roster:send_ses(C, Phone, P#'Profile'{status = update}), + P#'Profile'{phone = LinkPhone, status = update}; + {error, _} -> + #io{code = #error{code = profile_not_found}, data = LinkPhone} + end, + {reply, {bert, IO}, Req, State}; info(#'Profile'{status = remove, phone=UUID, rosters =Accounts }=Data, Req, - #cx{params = ClientId= <<"sys_", _/binary>>} = State) -> + #cx{params = ClientId= <<"sys_", _/binary>>} = State) -> ?LOG_INFO("~p:~p:Accounts/remove:~s", [UUID,ClientId,io_lib:format("~p",[Data])]), - Rosters = lists:flatten([begin [_,RId]=roster:parts_phone_id(micro:uuid_to_id('LinkRoster',A)), RId end || A <- Accounts]), - UID = micro:uuid_to_id('LinkProfile',UUID), - case roster_profile:info(#'Profile'{status = delete, phone=UID, rosters = [_|_]=Rosters}, Req,State) of - {reply, {bert, {io, {error, _R}, <<>>}}, Req, State}=E -> E; - {reply, {bert, P=#'Profile'{phone=UID, rosters = NewRosters}}, Req, State} -> - Accs=[ begin kvs:delete('LinkRoster', Id), micro:norm_uuid(Id) end - || RosterId <- Rosters--NewRosters, #'LinkRoster'{id=Id} <- kvs:index('LinkRoster',phone_id,roster:phone_id(UID,RosterId))], - {reply, {bert, P#'Profile'{rosters = Accs}}, Req, State} - end; + Rosters = lists:flatten([begin [_,RId]=roster:parts_phone_id(micro:uuid_to_id('LinkRoster',A)), RId end || A <- Accounts]), + UID = micro:uuid_to_id('LinkProfile',UUID), + case roster_profile:info(#'Profile'{status = delete, phone=UID, rosters = [_|_]=Rosters}, Req,State) of + {reply, {bert, {io, {error, _R}, <<>>}}, Req, State}=E -> E; + {reply, {bert, P=#'Profile'{phone=UID, rosters = NewRosters}}, Req, State} -> + Accs=[ begin kvs:delete('LinkRoster', Id), micro:norm_uuid(Id) end + || RosterId <- Rosters--NewRosters, #'LinkRoster'{id=Id} <- kvs:index('LinkRoster',phone_id,roster:phone_id(UID,RosterId))], + {reply, {bert, P#'Profile'{rosters = Accs}}, Req, State} + end; info(#'Profile'{phone = LinkPhone} = Profile, Req, #cx{params = ClientId} = State) -> - ?LOG_INFO("~p:Profile/unknown:~p", [ClientId, Profile]), - {reply, {bert, #io{code = #error{code = invalid_data}, data = LinkPhone}}, Req, State}. + ?LOG_INFO("~p:Profile/unknown:~p", [ClientId, Profile]), + {reply, {bert, #io{code = #error{code = invalid_data}, data = LinkPhone}}, Req, State}. -- GitLab From 77684a0a1d63e9f79fc346d9fcd0e4ed7b79c4e7 Mon Sep 17 00:00:00 2001 From: Hans Svensson Date: Tue, 12 May 2020 16:12:06 +0200 Subject: [PATCH 2/3] Ensure that we don't double link profiles + test that it works --- apps/roster/src/protocol/micro_profile.erl | 36 ++++++++++--------- test/nynja.erl | 4 ++- test/server_SUITE.erl | 40 ++++++++++++++++++++-- 3 files changed, 60 insertions(+), 20 deletions(-) diff --git a/apps/roster/src/protocol/micro_profile.erl b/apps/roster/src/protocol/micro_profile.erl index a046d044d..6466db228 100644 --- a/apps/roster/src/protocol/micro_profile.erl +++ b/apps/roster/src/protocol/micro_profile.erl @@ -34,22 +34,26 @@ info(#'Profile'{phone=LinkPhone, rosters=Rosters, status=create, settings = Sett Err end; false -> - StoredProfile = element(2, GetProfile), - P = roster:patch_profile(StoredProfile, Profile), - kvs:put(P2 = P#'Profile'{status = []}), - roster:send_ses(C, OldPhone, P#'Profile'{status = patch}), - micro:link_user(P2, Profile), - Res = lists:flatten( - [begin - {reply, {bert, IO}, _, _} = - micro_roster:info(Roster#'Roster'{status = update}, Req, State), - case IO of #io{} -> IO#io{data = LinkPhone}; #'Roster'{} -> [] end - end || #'Roster'{} = Roster <- Rosters]), - case Res of - [] -> Profile; - [Err | _] -> - kvs:put(StoredProfile), %% Return the old Profile - Err + case kvs:get('LinkProfile', OldPhone) of + {ok, _} -> #io{code = #error{code = phone_already_linked}, data = OldPhone}; + _ -> + StoredProfile = element(2, GetProfile), + P = roster:patch_profile(StoredProfile, Profile), + kvs:put(P2 = P#'Profile'{status = []}), + roster:send_ses(C, OldPhone, P#'Profile'{status = patch}), + micro:link_user(P2, Profile), + Res = lists:flatten( + [begin + {reply, {bert, IO}, _, _} = + micro_roster:info(Roster#'Roster'{status = update}, Req, State), + case IO of #io{} -> IO#io{data = LinkPhone}; #'Roster'{} -> [] end + end || #'Roster'{} = Roster <- Rosters]), + case Res of + [] -> Profile; + [Err | _] -> + kvs:put(StoredProfile), %% Return the old Profile + Err + end end end end, diff --git a/test/nynja.erl b/test/nynja.erl index 2c189aaa2..7d583043c 100644 --- a/test/nynja.erl +++ b/test/nynja.erl @@ -61,7 +61,9 @@ register_profile(Sys, Info, Connect) -> update = now_msec(), status = create }, - Profile = #'Profile'{ phone = PhoneLink, rosters = [Roster], update = now_msec(), status = create }, + Settings = maps:get(settings, Info, []), + Profile = #'Profile'{ phone = PhoneLink, rosters = [Roster], update = now_msec(), + settings = Settings, status = create }, #mqtt_packet{ payload = #'Profile'{rosters = [#'Roster'{}]} } = do_register_profile(Sys, Profile), case Connect of diff --git a/test/server_SUITE.erl b/test/server_SUITE.erl index 500de8b59..c9cb0e21a 100644 --- a/test/server_SUITE.erl +++ b/test/server_SUITE.erl @@ -31,10 +31,12 @@ , room_chat_message/1 , room_chat_message_errors/1 , call_bubble_message/1 + , double_link_profile/1 ]). all() -> - [{group, primitives} + [{group, primitives}, + {group, regression} ]. groups() -> @@ -44,7 +46,8 @@ groups() -> p2p_chat_message_post, p2p_chat_message_post_errors, room_chat_message, room_chat_message_errors, create_and_join_room_cri, call_bubble_message - ]} + ]}, + {regression, [double_link_profile]} ]. init_per_suite(Cfg) -> @@ -53,7 +56,7 @@ init_per_suite(Cfg) -> ct:pal("NYNJA_HOST undefined. Suite requires running nynja server!"), {skip, {undefined, "NYNJA_HOST"}}; Host -> - %% Use a random phoen number for testing. Skip tests if number has already been used + %% Use a random phone number for testing. Skip tests if number has already been used Phone = rand:uniform(1000000), %% Rebar config has ct_opts point to sys.config such that we @@ -813,6 +816,37 @@ wait_for_call_bubble(User, StartTime, FeedId, N) -> wait_for_call_bubble(User, StartTime, FeedId, N - 1) end. +double_link_profile(Cfg) -> + Phone0 = proplists:get_value(phone, Cfg), + Phone1 = <>, + Phone2 = <>, + + Sys = nynja:connect_sys(), + U1 = nynja:register_profile(Sys, Phone1), + PhoneLink = nynja:user_phone_uuid(U1), + nynja:ws_close(U1), + + try + nynja:register_profile(Sys, #{ phone => Phone2, nick => Phone2, + fname => <<"FNAME">>, lname => <<"LNAME">>, + settings => [#'Feature'{ id = <<"123">>, key = <<"PHONE">>, value = PhoneLink }] }) + catch error:{badmatch, {mqtt_packet, _, _, {io, {error, phone_already_linked}, _}}} -> + ok; + error:{badmatch, timeout} -> + ct:log("register_profile: {badmatch, timeout}") + end, + + nynja:ws_close(Sys), + + try + U1_1 = nynja:connect_user(Phone1), + nynja:ws_close(U1_1) + catch error:{badmatch, timeout} -> + ct:fail(could_not_connect_user) + end, + + ok. + %%% ------------- helpers --------------------- register_fresh_user() -> -- GitLab From ff510550ba733e4979fdc422188163cd00e1536a Mon Sep 17 00:00:00 2001 From: Hans Svensson Date: Wed, 13 May 2020 08:15:21 +0200 Subject: [PATCH 3/3] Extra protection in roster_db:add_account --- apps/roster/src/roster_db.erl | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/apps/roster/src/roster_db.erl b/apps/roster/src/roster_db.erl index 7598cfcd0..2c62074a0 100644 --- a/apps/roster/src/roster_db.erl +++ b/apps/roster/src/roster_db.erl @@ -868,11 +868,15 @@ new_feature(PhoneId, account_id) -> end. add_account(PhoneId, Settings) -> - case micro:id_to_uuid('LinkRoster', phone_id, PhoneId) of + try micro:id_to_uuid('LinkRoster', phone_id, PhoneId) of [] -> + ?LOG_INFO("No AccountId for ~p", [PhoneId]), Settings; UUID -> [build_feature(UUID)|Settings] + catch _:_ -> + ?LOG_ERROR("Could not resolve AccountId for ~p", [PhoneId]), + Settings end. build_feature(UUID) -> -- GitLab