From 05b3da55ac90fdfcec01ebed41e497c88c692acf Mon Sep 17 00:00:00 2001 From: richjohnrambo Date: Wed, 30 Sep 2026 16:11:30 +0800 Subject: [PATCH] fix: align group private chat permission semantics --- server/db/adapter.go | 1 + server/db/mysql/group_schema_test.go | 21 ++++++++++++++++++ server/db/mysql/tenant_groups.go | 33 ++++++++++++++++++++++++++++ server/im_http.go | 9 -------- server/im_http_test.go | 20 ++++++++--------- server/store/store.go | 15 ++++++++++++- 6 files changed, 79 insertions(+), 20 deletions(-) diff --git a/server/db/adapter.go b/server/db/adapter.go index e679f20..6c6bab0 100644 --- a/server/db/adapter.go +++ b/server/db/adapter.go @@ -406,6 +406,7 @@ type TenantBusinessAdapter interface { TenantGroupMemberGet(t.TenantID, string, t.Uid) (*t.GroupMember, error) TenantGroupMemberList(t.TenantID, string, t.GroupMemberListOptions) ([]t.GroupMember, string, error) TenantGroupMemberUpdate(t.TenantID, string, t.Uid, t.GroupMemberPatch) (*t.GroupMember, error) + TenantUsersShareOpenPrivateGroup(t.TenantID, t.Uid, t.Uid) (bool, error) TenantGroupInvitationCreate(t.TenantID, *t.GroupInvitation) (*t.GroupInvitation, error) TenantGroupInvitationAccept(t.TenantID, string, t.Uid, any, time.Time) (*t.GroupInvitation, error) TenantGroupJoinRequestCreate(t.TenantID, *t.GroupJoinRequest) (*t.GroupJoinRequest, error) diff --git a/server/db/mysql/group_schema_test.go b/server/db/mysql/group_schema_test.go index 90447d0..2263279 100644 --- a/server/db/mysql/group_schema_test.go +++ b/server/db/mysql/group_schema_test.go @@ -171,3 +171,24 @@ func TestSharedDBGroupFileLinkIncludesTenantID(t *testing.T) { t.Fatal("TenantGroupFileCreate must not rely on im_filemsglinks.tenant_id default") } } + +func TestTenantUsersShareOpenPrivateGroupChecksBothMembersAndSetting(t *testing.T) { + source, err := os.ReadFile("tenant_groups.go") + if err != nil { + t.Fatal(err) + } + body := sourceBetween(t, string(source), "func (a *adapter) TenantUsersShareOpenPrivateGroup", "\nfunc (a *adapter) TenantGroupMemberUpdate") + for _, required := range []string{ + "INNER JOIN im_group_members AS gm2", + "gm2.topic=gm1.topic", + "INNER JOIN im_group_settings AS gs", + "gs.member_private_chat=1", + "gm1.member_status=?", + "gm2.member_status=?", + "gs.group_status=?", + } { + if !strings.Contains(body, required) { + t.Fatalf("TenantUsersShareOpenPrivateGroup must check active shared group and setting; missing %q", required) + } + } +} diff --git a/server/db/mysql/tenant_groups.go b/server/db/mysql/tenant_groups.go index 66c87a6..5e55849 100644 --- a/server/db/mysql/tenant_groups.go +++ b/server/db/mysql/tenant_groups.go @@ -394,6 +394,39 @@ func (a *adapter) TenantGroupMemberList(id t.TenantID, topic string, opts t.Grou return items, next, rows.Err() } +func (a *adapter) TenantUsersShareOpenPrivateGroup(id t.TenantID, userID, peerID t.Uid) (bool, error) { + if tenantAdp, err := a.routeGroupTenant(id); err != nil { + return false, err + } else if tenantAdp != a { + return tenantAdp.TenantUsersShareOpenPrivateGroup(id, userID, peerID) + } + if userID.IsZero() || peerID.IsZero() || userID == peerID { + return false, t.ErrMalformed + } + ctx, cancel := a.getContext() + if cancel != nil { + defer cancel() + } + var exists int + err := a.db.GetContext(ctx, &exists, tenantShardSQL(id, ` + SELECT 1 + FROM im_group_members AS gm1 + INNER JOIN im_group_members AS gm2 + ON gm2.tenant_id=gm1.tenant_id AND gm2.topic=gm1.topic + INNER JOIN im_group_settings AS gs + ON gs.tenant_id=gm1.tenant_id AND gs.topic=gm1.topic + WHERE gm1.tenant_id=? AND gm1.user_id=? AND gm1.member_status=? + AND gm2.user_id=? AND gm2.member_status=? + AND gs.group_status=? AND gs.member_private_chat=1 + LIMIT 1`), + id, decodeUID(userID), t.GroupMemberStatusActive, + decodeUID(peerID), t.GroupMemberStatusActive, t.GroupStatusActive) + if err == sql.ErrNoRows { + return false, nil + } + return exists == 1, err +} + func (a *adapter) TenantGroupMemberUpdate(id t.TenantID, topic string, uid t.Uid, patch t.GroupMemberPatch) (*t.GroupMember, error) { if tenantAdp, err := a.routeGroupTenant(id); err != nil { return nil, err diff --git a/server/im_http.go b/server/im_http.go index bb8aa68..c1e42ad 100644 --- a/server/im_http.go +++ b/server/im_http.go @@ -4026,15 +4026,6 @@ func serveIMFriendRequestCreate(wrt http.ResponseWriter, req *http.Request) { writeIMError(wrt, req, http.StatusForbidden, 30005, imReasonPermissionDenied, "group membership required", nil) return } - settings, settingsErr := store.GroupBusiness.GetSettings(principal.TenantID, sourceTopic) - if settingsErr != nil { - writeIMRelationError(wrt, req, settingsErr) - return - } - if settings == nil || !settings.MemberPrivateChat { - writeIMError(wrt, req, http.StatusForbidden, 30005, imReasonPermissionDenied, "member private chat is disabled", nil) - return - } default: writeIMError(wrt, req, http.StatusBadRequest, 30000, imReasonMalformedRequest, "invalid friend request source", nil) return diff --git a/server/im_http_test.go b/server/im_http_test.go index 6fcd5f3..ab23207 100644 --- a/server/im_http_test.go +++ b/server/im_http_test.go @@ -7852,26 +7852,23 @@ func mustP2PNameForUser(t *testing.T, uid types.Uid, topic string) string { return name } -func TestServeIMFriendRequestCreateHonorsGroupPrivateChatSetting(t *testing.T) { +func TestServeIMFriendRequestCreateAllowsGroupSourceWhenPrivateChatDisabled(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() - expectIMBearer(t, ctrl, "token-1", 7, 1) + mockStore := expectIMBearer(t, ctrl, "token-1", 7, 1) + mockStore.EXPECT().GetUidString().Return("group-request-1") oldRelations := store.Relations oldProfiles := store.Profiles oldSubs := store.Subs - oldGroups := store.GroupBusiness - store.Relations = &testRelationsStore{context: &types.RelationshipContext{Relationship: types.RelationshipNone}} + relations := &testRelationsStore{context: &types.RelationshipContext{Relationship: types.RelationshipNone}} + store.Relations = relations store.Profiles = &testProfilesStore{} subs := mock_store.NewMockSubsPersistenceInterface(ctrl) store.Subs = subs - store.GroupBusiness = &testGroupBusinessStore{settings: &types.GroupSettings{ - Topic: "grpAlpha", MemberPrivateChat: false, GroupStatus: types.GroupStatusActive, - }} defer func() { store.Relations = oldRelations store.Profiles = oldProfiles store.Subs = oldSubs - store.GroupBusiness = oldGroups }() memberMode := types.ModeCPublic for _, uid := range []types.Uid{1, 2} { @@ -7886,8 +7883,11 @@ func TestServeIMFriendRequestCreateHonorsGroupPrivateChatSetting(t *testing.T) { req.Header.Set("Idempotency-Key", "fr-group-1") rec := httptest.NewRecorder() serveIMFriendRequests(rec, req) - if rec.Code != http.StatusForbidden { - t.Fatalf("status: got %d, want %d; body=%s", rec.Code, http.StatusForbidden, rec.Body.String()) + if rec.Code != http.StatusOK { + t.Fatalf("status: got %d, want %d; body=%s", rec.Code, http.StatusOK, rec.Body.String()) + } + if relations.created == nil || relations.created.Source != "group" || relations.created.SourceTopic != "grpAlpha" { + t.Fatalf("unexpected group friend request: %#v", relations.created) } } diff --git a/server/store/store.go b/server/store/store.go index 051380d..bb5b613 100644 --- a/server/store/store.go +++ b/server/store/store.go @@ -3220,13 +3220,26 @@ func (relationsMapper) CanSendP2P(tenantID types.TenantID, userID, peerID types. return true, types.RelationshipFriend, nil } } - ctx, err := (relationsMapper{}).GetRelationshipContext(tenantID, userID, peerID, "") + tenantAdp, err := tenantBusinessAdapter(tenantID) + if err != nil { + return false, "", err + } + ctx, err := relationshipContext(tenantAdp, tenantID, userID, peerID, "") if err != nil { return false, "", err } if ctx.Relationship == types.RelationshipFriend { return true, ctx.Relationship, nil } + if ctx.Relationship == types.RelationshipNone { + shared, err := tenantAdp.TenantUsersShareOpenPrivateGroup(tenantID, userID, peerID) + if err != nil { + return false, ctx.Relationship, err + } + if shared { + return true, ctx.Relationship, nil + } + } return false, ctx.Relationship, nil }