fix(presence): enforce visibility and relationship invariants (#1507)

fluxer_gateway/src/session/session.erl

@@ -309,7 +309,8 @@ do_handle_info_misc(_Info, State) ->
 
 -spec handle_presence_rejoin_check(session_state()) -> {noreply, session_state()}.
 handle_presence_rejoin_check(State) ->
-    {noreply, session_connection:repair_presence_connection(State)}.
+    RepairedState = session_connection:repair_presence_connection(State),
+    {noreply, session_dispatch_presence:sync_presence_targets(RepairedState)}.
 
 -spec terminate(term(), session_state()) -> ok.
 terminate(Reason, State) ->

fluxer_gateway/src/session/session_connection_presence.erl

@@ -50,14 +50,14 @@ safe_presence_lookup(Request) ->
 -spec build_presence_request(session_state()) -> map().
 build_presence_request(State) when is_map(State) ->
     FriendIds = presence_targets:friend_ids_from_state(State),
-    DmRecipients = presence_targets:dm_recipients_from_state(State),
+    GroupDmRecipients = presence_targets:group_dm_recipients_from_state(State),
     #{
         user_id => maps:get(user_id, State),
         user_data => maps:get(user_data, State),
         guild_ids => maps:keys(maps:get(guilds, State)),
         status => maps:get(status, State),
         friend_ids => FriendIds,
-        group_dm_recipients => DmRecipients,
+        group_dm_recipients => GroupDmRecipients,
         custom_status => maps:get(custom_status, State, null)
     }.
 
@@ -70,7 +70,7 @@ do_session_connect(Pid, Attempt, State) when is_map(State) ->
     Mobile = maps:get(mobile, State),
     SocketPid = maps:get(socket_pid, State, undefined),
     FriendIds = presence_targets:friend_ids_from_state(State),
-    DmRecipients = presence_targets:dm_recipients_from_state(State),
+    GroupDmRecipients = presence_targets:group_dm_recipients_from_state(State),
     try_session_connect(
         Pid,
         SessionId,
@@ -79,7 +79,7 @@ do_session_connect(Pid, Attempt, State) when is_map(State) ->
         Mobile,
         SocketPid,
         FriendIds,
-        DmRecipients,
+        GroupDmRecipients,
         Attempt,
         State
     ).

fluxer_gateway/src/session/session_dispatch_presence.erl

@@ -10,6 +10,7 @@
     flush_all_pending_presences/1,
     dispatch_presence_now/2,
     maybe_sync_presence_targets/3,
+    sync_presence_targets/1,
     event_changes_presence_targets/1,
     presence_user_id/1,
     relationship_target_id/1
@@ -32,21 +33,28 @@ should_buffer_presence(presence_update, Data, State) ->
         true ->
             true;
         false ->
-            check_non_guild_presence(Data, State)
+            check_presence_scope(Data, State)
     end;
 should_buffer_presence(_, _, _) ->
     false.
 
--spec check_non_guild_presence(map(), session_state()) -> boolean().
-check_non_guild_presence(Data, State) ->
-    HasGuildId =
-        is_map(Data) andalso (maps:get(<<"guild_id">>, Data, undefined) =/= undefined),
-    case HasGuildId of
-        true ->
-            false;
-        false ->
+-spec check_presence_scope(map(), session_state()) -> boolean().
+check_presence_scope(Data, State) ->
+    case maps:find(<<"guild_id">>, Data) of
+        error ->
             UserId = presence_user_id(Data),
-            check_user_presence_buffering(UserId, State)
+            check_user_presence_buffering(UserId, State);
+        {ok, GuildIdValue} ->
+            should_buffer_guild_presence(GuildIdValue, State)
+    end.
+
+-spec should_buffer_guild_presence(term(), session_state()) -> boolean().
+should_buffer_guild_presence(GuildIdValue, State) ->
+    case snowflake_id:parse_maybe(GuildIdValue) of
+        GuildId when is_integer(GuildId) ->
+            not maps:is_key(GuildId, maps:get(guilds, State, #{}));
+        undefined ->
+            true
     end.
 
 -spec check_user_presence_buffering(user_id() | undefined, session_state()) -> boolean().
@@ -55,8 +63,8 @@ check_user_presence_buffering(undefined, _State) ->
 check_user_presence_buffering(UserId, State) ->
     Relationships = maps:get(relationships, State, #{}),
     IsRelationship = relationship_allows_presence(UserId, Relationships),
-    IsDmRecipient = is_dm_recipient(UserId, State),
-    not (IsRelationship orelse IsDmRecipient).
+    IsGroupDmRecipient = is_group_dm_recipient(UserId, State),
+    not (IsRelationship orelse IsGroupDmRecipient).
 
 -spec relationship_allows_presence(user_id(), #{user_id() => integer()}) -> boolean().
 relationship_allows_presence(UserId, Relationships) when
@@ -64,31 +72,30 @@ relationship_allows_presence(UserId, Relationships) when
 ->
     case maps:get(UserId, Relationships, undefined) of
         1 -> true;
-        3 -> true;
         _ -> false
     end;
 relationship_allows_presence(_, _) ->
     false.
 
--spec is_dm_recipient(user_id(), session_state()) -> boolean().
-is_dm_recipient(UserId, State) when is_map(State) ->
-    DmRecipients = presence_targets:dm_recipients_from_state(State),
+-spec is_group_dm_recipient(user_id(), session_state()) -> boolean().
+is_group_dm_recipient(UserId, State) when is_map(State) ->
+    GroupDmRecipients = presence_targets:group_dm_recipients_from_state(State),
     maps:fold(
         fun
             (_, Recipients, false) -> maps:is_key(UserId, Recipients);
             (_, _, true) -> true
         end,
         false,
-        DmRecipients
+        GroupDmRecipients
     ).
 
 -spec buffer_presence(event(), map(), session_state()) -> session_state().
 buffer_presence(Event, Data, State) ->
-    Pending = maps:get(pending_presences, State, []),
+    Pending = ensure_queue(maps:get(pending_presences, State, [])),
     UserId = presence_user_id(Data),
     Entry = #{event => Event, data => Data, user_id => UserId},
-    Trimmed = trim_rev_list(Pending, ?MAX_PENDING_PRESENCE_BUFFER_SIZE - 1),
-    NewPending = [Entry | Trimmed],
+    Trimmed = trim_queue_from_tail(Pending, ?MAX_PENDING_PRESENCE_BUFFER_SIZE - 1),
+    NewPending = queue:in_r(Entry, Trimmed),
     State#{pending_presences => NewPending}.
 
 -spec maybe_flush_pending_presences(event(), map(), session_state()) ->
@@ -110,7 +117,7 @@ maybe_flush_pending_presences(_, _, State) ->
     {session_state(), [user_id()]}.
 flush_dm_channel_pending_presences(Data, State) ->
     SelfUserId = maps:get(user_id, State, undefined),
-    RecipientIds = presence_targets:dm_channel_recipient_ids(Data, SelfUserId),
+    RecipientIds = presence_targets:group_dm_channel_recipient_ids(Data, SelfUserId),
     flush_pending_presences_for_ids(RecipientIds, State).
 
 -spec flush_added_recipient_pending_presences(map(), session_state()) ->
@@ -139,9 +146,6 @@ maybe_flush_relationship_pending_presences(Data, State) ->
         1 ->
             TargetId = relationship_target_id(Data),
             {flush_pending_presences(TargetId, State), flushed_id_list(TargetId)};
-        3 ->
-            TargetId = relationship_target_id(Data),
-            {flush_pending_presences(TargetId, State), flushed_id_list(TargetId)};
         _ ->
             {State, []}
     end.
@@ -207,6 +211,10 @@ maybe_sync_presence_targets(Event, FlushedIds, State) ->
         false -> State
     end.
 
+-spec sync_presence_targets(session_state()) -> session_state().
+sync_presence_targets(State) ->
+    sync_presence_targets([], State).
+
 -spec event_changes_presence_targets(event()) -> boolean().
 event_changes_presence_targets(relationship_add) -> true;
 event_changes_presence_targets(relationship_update) -> true;
@@ -226,9 +234,9 @@ sync_presence_targets(FlushedIds, State) when is_map(State) ->
             State;
         Pid when is_pid(Pid) ->
             FriendIds = presence_targets:friend_ids_from_state(State),
-            DmRecipients = presence_targets:dm_recipients_from_state(State),
+            GroupDmRecipients = presence_targets:group_dm_recipients_from_state(State),
             gen_server:cast(Pid, {sync_friends, FriendIds, FlushedIds}),
-            gen_server:cast(Pid, {sync_group_dm_recipients, DmRecipients}),
+            gen_server:cast(Pid, {sync_group_dm_recipients, GroupDmRecipients}),
             State
     end.
 
@@ -253,9 +261,12 @@ relationship_target_id(Data) when is_map(Data) ->
 ensure_queue(List) when is_list(List) -> queue:from_list(List);
 ensure_queue(Q) -> Q.
 
--spec trim_rev_list([T], non_neg_integer()) -> [T].
-trim_rev_list(List, MaxLen) when length(List) =< MaxLen -> List;
-trim_rev_list(List, MaxLen) -> lists:sublist(List, MaxLen).
+-spec trim_queue_from_tail(queue:queue(T), non_neg_integer()) -> queue:queue(T).
+trim_queue_from_tail(Queue, MaxLen) ->
+    case queue:len(Queue) > MaxLen of
+        true -> trim_queue_from_tail(queue:drop_r(Queue), MaxLen);
+        false -> Queue
+    end.
 
 -spec send_to_socket(pid() | undefined, event(), map(), non_neg_integer()) -> ok.
 send_to_socket(undefined, _Event, _Data, _Seq) ->
@@ -269,7 +280,7 @@ send_to_socket(Pid, Event, Data, Seq) when is_pid(Pid) ->
 
 relationship_allows_presence_test() ->
     ?assertEqual(true, relationship_allows_presence(1, #{1 => 1})),
-    ?assertEqual(true, relationship_allows_presence(1, #{1 => 3})),
+    ?assertEqual(false, relationship_allows_presence(1, #{1 => 3})),
     ?assertEqual(false, relationship_allows_presence(1, #{1 => 0})),
     ?assertEqual(false, relationship_allows_presence(1, #{1 => 2})),
     ?assertEqual(false, relationship_allows_presence(1, #{1 => 4})),
@@ -299,10 +310,10 @@ buffering_test_state() ->
 presence_data(UserIdBin) ->
     #{<<"user">> => #{<<"id">> => UserIdBin}, <<"status">> => <<"online">>}.
 
-should_buffer_presence_passes_one_to_one_dm_recipient_test() ->
+should_buffer_presence_buffers_one_to_one_dm_recipient_test() ->
     State = buffering_test_state(),
     ?assertEqual(
-        false, should_buffer_presence(presence_update, presence_data(<<"2">>), State)
+        true, should_buffer_presence(presence_update, presence_data(<<"2">>), State)
     ).
 
 should_buffer_presence_passes_group_dm_recipient_test() ->
@@ -323,11 +334,16 @@ should_buffer_presence_passes_friend_test() ->
         false, should_buffer_presence(presence_update, presence_data(<<"4">>), State)
     ).
 
-should_buffer_presence_passes_guild_presence_test() ->
-    State = buffering_test_state(),
-    Data = (presence_data(<<"99">>))#{<<"guild_id">> => <<"42">>},
+one_to_one_dm_presence_passes_attached_guild_scope_test() ->
+    State = (buffering_test_state())#{guilds => #{42 => undefined}},
+    Data = (presence_data(<<"2">>))#{<<"guild_id">> => <<"42">>},
     ?assertEqual(false, should_buffer_presence(presence_update, Data, State)).
 
+one_to_one_dm_presence_buffers_unattached_guild_scope_test() ->
+    State = (buffering_test_state())#{guilds => #{42 => undefined}},
+    Data = (presence_data(<<"2">>))#{<<"guild_id">> => <<"99">>},
+    ?assertEqual(true, should_buffer_presence(presence_update, Data, State)).
+
 flush_test_state(PendingUserIds) ->
     #{
         user_id => 1,
@@ -344,7 +360,7 @@ flush_test_state(PendingUserIds) ->
         ]
     }.
 
-channel_create_flushes_pending_dm_recipient_presence_test() ->
+channel_create_does_not_flush_one_to_one_dm_recipient_presence_test() ->
     State = flush_test_state([2, 99]),
     ChannelData = #{
         <<"id">> => <<"100">>,
@@ -354,10 +370,10 @@ channel_create_flushes_pending_dm_recipient_presence_test() ->
     {NewState, FlushedIds} = maybe_flush_pending_presences(
         channel_create, ChannelData, State
     ),
-    ?assertEqual([2], FlushedIds),
-    ?assertEqual(1, maps:get(seq, NewState)),
+    ?assertEqual([], FlushedIds),
+    ?assertEqual(0, maps:get(seq, NewState)),
     Remaining = queue:to_list(ensure_queue(maps:get(pending_presences, NewState))),
-    ?assertEqual([99], [maps:get(user_id, P) || P <- Remaining]).
+    ?assertEqual([2, 99], [maps:get(user_id, P) || P <- Remaining]).
 
 channel_create_ignores_guild_channels_test() ->
     State = flush_test_state([2]),

fluxer_gateway/src/session/session_ready_collect.erl

@@ -52,14 +52,14 @@ collect_ready_presences(State, _CollectedGuilds) ->
 -spec collect_presence_targets(session_state(), user_id()) -> [user_id()].
 collect_presence_targets(State, CurrentUserId) when is_map(State) ->
     FIds = presence_targets:friend_ids_from_state(State),
-    DmMap = presence_targets:dm_recipients_from_state(State),
+    GroupDmMap = presence_targets:group_dm_recipients_from_state(State),
     TargetMap0 = add_presence_target_ids(FIds, CurrentUserId, #{}),
     TargetMap = maps:fold(
         fun(_Cid, Recipients, Acc) ->
             add_presence_target_map(Recipients, CurrentUserId, Acc)
         end,
         TargetMap0,
-        DmMap
+        GroupDmMap
     ),
     maps:keys(TargetMap).
 
@@ -350,7 +350,7 @@ collect_presence_targets_deduplicates_before_fetch_test() ->
             }
         }
     },
-    ?assertEqual([2, 3, 5], lists:sort(collect_presence_targets(State, 1))).
+    ?assertEqual([2, 5], lists:sort(collect_presence_targets(State, 1))).
 
 collect_ready_users_collects_directly_into_dedup_map_test() ->
     UserA = #{<<"id">> => <<"10">>, <<"username">> => <<"a">>},