Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion api/comms/chat.go
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ func updatePermissions(db dbv1.DBTX, ctx context.Context, userId int32, permit C
insert into chat_permissions (user_id, permits, allowed, updated_at)
values ($1, $2, $3, $4)
on conflict (user_id, permits)
do update set allowed = $3 where chat_permissions.updated_at < $4
do update set allowed = $3, updated_at = $4 where chat_permissions.updated_at < $4
`, userId, permit, permitAllowed, messageTimestamp.UTC())
return err
}
Expand Down
135 changes: 135 additions & 0 deletions api/comms/chat_inbox_closed_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
package comms

import (
"context"
"fmt"
"testing"
"time"

"api.audius.co/database"
"api.audius.co/trashid"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

// An artist who blasts their followers and later closes their inbox must stop
// receiving new DMs from blast recipients. Threads that hold real messages keep
// working, and a fresh blast sent after closing re-opens replies to it.
func TestChatBlastThenCloseInbox(t *testing.T) {
t0 := time.Now().Add(time.Second * -100).UTC()
t1 := time.Now().Add(time.Second * -90).UTC()
t2 := time.Now().Add(time.Second * -80).UTC()
t3 := time.Now().Add(time.Second * -70).UTC()
t4 := time.Now().Add(time.Second * -60).UTC()
t5 := time.Now().Add(time.Second * -50).UTC()

pool := database.CreateTestDatabase(t, "test_comms")
defer pool.Close()
ctx := context.Background()

// user 1 is the artist. 201, 202, 203 follow them before the blast.
database.Seed(pool, database.FixtureMap{
"users": {
{"user_id": 1, "wallet": "wallet1", "handle": "user1"},
{"user_id": 201, "wallet": "wallet201", "handle": "user201"},
{"user_id": 202, "wallet": "wallet202", "handle": "user202"},
{"user_id": 203, "wallet": "wallet203", "handle": "user203"},
},
"follows": {
{"follower_user_id": 201, "followee_user_id": 1, "created_at": t0},
{"follower_user_id": 202, "followee_user_id": 1, "created_at": t0},
{"follower_user_id": 203, "followee_user_id": 1, "created_at": t0},
},
})
validator := CreateTestValidator(t, pool, DefaultRateLimitConfig, DefaultTestValidatorConfig)

chatAllowed := func(from, to int32) bool {
var ok bool
require.NoError(t, pool.QueryRow(ctx, `select chat_allowed($1, $2)`, from, to).Scan(&ok))
return ok
}
assertMessageAllowed := func(sender int32, chatId string, shouldWork bool) {
rpc := RawRPC{
Params: []byte(fmt.Sprintf(`{"chat_id": "%s", "message_id": "m-%s-%d", "message": "hi"}`, chatId, chatId, sender)),
}
err := validator.validateChatMessage(ctx, sender, rpc)
if shouldWork {
assert.NoError(t, err)
} else {
assert.ErrorContains(t, err, "Not permitted to send messages to this user")
}
}
upgrade := func(follower int32, ts time.Time) string {
chatId := trashid.ChatID(int(follower), 1)
err := chatCreate(pool, ctx, follower, ts, ChatCreateRPCParams{
ChatID: chatId,
Invites: []PurpleInvite{
{UserID: trashid.MustEncodeHashID(int(follower)), InviteCode: "x"},
{UserID: trashid.MustEncodeHashID(1), InviteCode: "x"},
},
})
require.NoError(t, err)
return chatId
}

// artist blasts followers with an open inbox
_, err := chatBlast(pool, ctx, 1, t1, ChatBlastRPCParams{
BlastID: "b_open",
Audience: FollowerAudience,
Message: "hello followers",
})
require.NoError(t, err)

// 202 opens the blast into a thread but never replies
chatId_202 := upgrade(202, t2)

// 203 opens the blast into a thread and replies, so a real conversation exists
chatId_203 := upgrade(203, t2)
require.NoError(t, chatSendMessage(pool, ctx, 203, chatId_203, "reply_203", t2, "203 replying"))

// while the inbox is open, everyone in the audience can reach the artist
assertChatCreateAllowed(t, ctx, validator, 201, 1, true)
assertMessageAllowed(202, chatId_202, true)
assertMessageAllowed(203, chatId_203, true)

// artist closes their inbox
require.NoError(t, chatSetPermissions(pool, ctx, 1, ChatPermissionAll, []ChatPermission{ChatPermissionNone}, boolPtr(true), t3))

// 201 can no longer start a thread off the old blast
assertChatCreateAllowed(t, ctx, validator, 201, 1, false)
assert.False(t, chatAllowed(201, 1))

// 202's thread holds nothing but the blast seed, so it grants no reply rights
assertMessageAllowed(202, chatId_202, false)
assert.False(t, chatAllowed(202, 1))

// 203's thread is a real conversation and keeps working
assertMessageAllowed(203, chatId_203, true)
assert.True(t, chatAllowed(203, 1))

// a follower's own view of the seed-only thread asks the client to recheck permissions
{
chat, err := getUserChat(pool, ctx, chatMembershipParams{UserID: 202, ChatID: chatId_202})
require.NoError(t, err)
assert.True(t, chat.LastMessageIsPlaintext)
}

// artist blasts again while closed: recipients of the new blast may reply to it
_, err = chatBlast(pool, ctx, 1, t4, ChatBlastRPCParams{
BlastID: "b_closed",
Audience: FollowerAudience,
Message: "hello again",
})
require.NoError(t, err)

assertChatCreateAllowed(t, ctx, validator, 201, 1, true)
assert.True(t, chatAllowed(202, 1), "new blast fanned into 202's thread re-opens replies")

// closing the inbox once more after that blast shuts the door again
require.NoError(t, chatSetPermissions(pool, ctx, 1, ChatPermissionAll, []ChatPermission{ChatPermissionNone}, boolPtr(true), t5))
assertChatCreateAllowed(t, ctx, validator, 201, 1, false)
assert.False(t, chatAllowed(202, 1))
assert.True(t, chatAllowed(203, 1))
}

func boolPtr(b bool) *bool { return &b }
7 changes: 7 additions & 0 deletions api/comms/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -576,6 +576,13 @@ func hasNewBlastFromUser(pool *dbv1.DBPools, ctx context.Context, userID int32,
where
blast.from_user_id = $2
and blast.created_at > (select t from last_permission_change)
-- the blaster's own inbox settings: a blast grants reply rights only
-- while it is newer than the blaster's most recent settings change
and blast.created_at > (
select coalesce(max(updated_at), to_timestamp(0))
from chat_permissions
where user_id = $2
)
and chat_allowed(blast.from_user_id, $1)
and not exists (
select 1 from chat_member cm
Expand Down
5 changes: 4 additions & 1 deletion api/dbv1/user_chat_row.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,10 @@ func (row UserChatRow) MarshalJSON() ([]byte, error) {
clearedHistoryAt = row.ClearedHistoryAt.Time.UTC().Format(time.RFC3339Nano)
}

recheckPermissions := false
// A thread whose latest message is a blast may hold nothing but blast
// seeds; whether the viewer may reply then depends on the other member's
// current inbox settings rather than on the thread existing.
recheckPermissions := row.LastMessageIsPlaintext
for _, member := range row.ChatMembers {
if member.ClearedHistoryAt.Valid && (row.LastMessageAt == nil || member.ClearedHistoryAt.Time.After(*row.LastMessageAt)) {
recheckPermissions = true
Expand Down
13 changes: 12 additions & 1 deletion ddl/functions/chat_allowed.sql
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,25 @@ BEGIN
RETURN TRUE;
END IF;

-- existing chat takes priority over permissions
-- existing chat takes priority over permissions.
-- A blast message only counts if the blast is newer than to_user's most
-- recent inbox settings change: otherwise an artist who blasts and then
-- closes their inbox would still be reachable by every blast recipient.
SELECT COUNT(*) > 0 INTO can_message
FROM chat_member member_a
JOIN chat_member member_b USING (chat_id)
JOIN chat_message USING (chat_id)
WHERE member_a.user_id = from_user_id
AND member_b.user_id = to_user_id
AND (member_b.cleared_history_at IS NULL OR chat_message.created_at > member_b.cleared_history_at)
AND (
chat_message.blast_id IS NULL
OR chat_message.created_at > (
SELECT COALESCE(MAX(updated_at), to_timestamp(0))
FROM chat_permissions
WHERE user_id = to_user_id
)
)
;

IF can_message THEN
Expand Down
13 changes: 12 additions & 1 deletion sql/01_schema.sql
Original file line number Diff line number Diff line change
Expand Up @@ -1203,14 +1203,25 @@ BEGIN
RETURN TRUE;
END IF;

-- existing chat takes priority over permissions
-- existing chat takes priority over permissions.
-- A blast message only counts if the blast is newer than to_user's most
-- recent inbox settings change: otherwise an artist who blasts and then
-- closes their inbox would still be reachable by every blast recipient.
SELECT COUNT(*) > 0 INTO can_message
FROM chat_member member_a
JOIN chat_member member_b USING (chat_id)
JOIN chat_message USING (chat_id)
WHERE member_a.user_id = from_user_id
AND member_b.user_id = to_user_id
AND (member_b.cleared_history_at IS NULL OR chat_message.created_at > member_b.cleared_history_at)
AND (
chat_message.blast_id IS NULL
OR chat_message.created_at > (
SELECT COALESCE(MAX(updated_at), to_timestamp(0))
FROM chat_permissions
WHERE user_id = to_user_id
)
)
;

IF can_message THEN
Expand Down
Loading