diff --git a/modules/message/check_channel_access_test.go b/modules/message/check_channel_access_test.go new file mode 100644 index 00000000..7ce7fbb5 --- /dev/null +++ b/modules/message/check_channel_access_test.go @@ -0,0 +1,231 @@ +package message + +// ============================================================================= +// Issue #353（PR #345 review P1-test）— checkChannelAccess requireActive split +// 的 CI 内（非 integration tag）回归覆盖。 +// +// AuthorizeThreadFollow（FollowThread，子区/CommunityTopic 分支）要求父群「活跃 +// 成员」（ExistMemberActive，排除被拉黑）；AuthorizeChannelFollow（FollowChannel， +// GROUP 分支）保留 permissive ExistMember。此前该 split 仅由 +// thread_follow_blacklist_e2e_test.go 覆盖，但该文件带 //go:build integration、 +// CI 的 go test 永不编译——把 AuthorizeThreadFollow 翻回 permissive 的回归会 +// 静默漏过。本文件不带 tag，直接吃 CI 已就绪的 MySQL service，跑生产同款 +// newThreadAuthChecker（真实 group/group_member/thread 表）。 +// +// 测试基建约定（PR #356 round-1 CI 红 + round-2 review 的教训）： +// 1. 不跑 sql-migrate——本包非 integration-tag 测试一律不经 module.Setup（既有 +// testutil.NewTestServer 用例全部 t.Skip；手建表用例与迁移在 -shuffle 下互撞 +// Error 1050）。 +// 2. 不碰共享 test 库——本文件要 DROP+CREATE group/group_member/thread，而 +// go test ./... 默认跨包并行、modules/group、modules/thread 等包用 +// testutil.NewTestServer 连同一个 test 库，破坏性 DDL 会撞别包的查询。 +// 因此先在 MySQL 实例上建独立库再连入，所有 DDL/数据只落在隔离库里。 +// 写法照搬 e2e helper：手建最小表 + 裸 INSERT 种子，ctx 显式 Migration=false。 +// ============================================================================= + +import ( + "errors" + "os" + "strings" + "testing" + + "github.com/Mininglamp-OSS/octo-lib/common" + "github.com/Mininglamp-OSS/octo-lib/config" + "github.com/Mininglamp-OSS/octo-lib/pkg/util" + convext "github.com/Mininglamp-OSS/octo-server/modules/conversation_ext" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +const ccaSpaceID = "s_cca" + +// ccaDBName 是本文件专用的隔离库名：与共享 test 库（testutil.NewTestServer / +// 其它包的迁移与查询）完全隔离，跨包并行的 go test ./... 下破坏性 DDL 不会外溢。 +// 与 thread_ext_blacklist_filter_test.go（PR #356）同名复用一个隔离库——两文件的 +// 每个用例都自建 schema，包内测试顺序执行，互不踩踏。 +const ccaDBName = "octo_msg_blacklist_test" + +// ccaNewCtx 构造指向隔离库的 *config.Context：先用 config.New 默认 DSN（与 CI +// MySQL service 同源的 root:demo@…/test）建隔离库，再把 DSN 的库名换成隔离库连入。 +// 显式 Migration=false——不经 module.Setup。可用 MSG_BLACKLIST_TEST_MYSQL_ADDR +// 覆盖隔离库 DSN（与 newSidebarIntegCtx 的 env 覆盖风格一致）。 +func ccaNewCtx(t *testing.T) *config.Context { + t.Helper() + bootCfg := config.New() + bootCfg.Test = true + bootCfg.DB.Migration = false + boot := config.NewContext(bootCfg) + _, err := boot.DB().Exec( + "CREATE DATABASE IF NOT EXISTS " + ccaDBName + + " CHARACTER SET utf8mb4 COLLATE utf8mb4_general_ci") + require.NoError(t, err, "create isolated db") + + addr := os.Getenv("MSG_BLACKLIST_TEST_MYSQL_ADDR") + if addr == "" { + addr = "root:demo@tcp(127.0.0.1:3306)/" + ccaDBName + "?charset=utf8mb4&parseTime=true" + } + cfg := config.New() + cfg.Test = true + cfg.DB.Migration = false + cfg.DB.MySQLAddr = addr + return config.NewContext(cfg) +} + +// ccaEnsureTables 在隔离库里手建 checkChannelAccess / AuthorizeThreadFollow 触达的最小表 +// （DDL 与 modules/group、modules/thread 迁移中本测试触达的列对齐；写法照搬 +// default_followed_group_guard_e2e_test.go / thread_follow_blacklist_e2e_test.go +// 的同名 helper）。DROP + CREATE 保证每个用例拿到本文件期望的 schema；隔离库 +// 只被本包顺序执行的测试使用，破坏性 DDL 安全。 +func ccaEnsureTables(t *testing.T, ctx *config.Context) { + t.Helper() + for _, tbl := range []string{"thread", "group_member", "`group`"} { + _, err := ctx.DB().Exec("DROP TABLE IF EXISTS " + tbl) + require.NoError(t, err, "drop %s", tbl) + } + stmts := []string{ + "CREATE TABLE `group` (" + + " `id` INT NOT NULL AUTO_INCREMENT PRIMARY KEY," + + " `group_no` VARCHAR(40) NOT NULL DEFAULT ''," + + " `name` VARCHAR(40) NOT NULL DEFAULT ''," + + " `creator` VARCHAR(40) NOT NULL DEFAULT ''," + + " `status` SMALLINT NOT NULL DEFAULT 0," + + " `version` BIGINT NOT NULL DEFAULT 0," + + " `group_type` SMALLINT NOT NULL DEFAULT 0," + + " `space_id` VARCHAR(40) DEFAULT ''," + + " `is_external_group` SMALLINT NOT NULL DEFAULT 0," + + " `created_at` TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP," + + " `updated_at` TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP," + + " UNIQUE KEY `group_groupNo` (`group_no`)" + + ") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4", + "CREATE TABLE `group_member` (" + + " `id` INT NOT NULL AUTO_INCREMENT PRIMARY KEY," + + " `group_no` VARCHAR(40) NOT NULL DEFAULT ''," + + " `uid` VARCHAR(40) NOT NULL DEFAULT ''," + + " `role` SMALLINT NOT NULL DEFAULT 0," + + " `version` BIGINT NOT NULL DEFAULT 0," + + " `is_deleted` SMALLINT NOT NULL DEFAULT 0," + + " `status` SMALLINT NOT NULL DEFAULT 1," + + " `vercode` VARCHAR(100) NOT NULL DEFAULT ''," + + " `robot` SMALLINT NOT NULL DEFAULT 0," + + " `is_external` SMALLINT NOT NULL DEFAULT 0," + + " `source_space_id` VARCHAR(40) NOT NULL DEFAULT ''," + + " `created_at` TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP," + + " `updated_at` TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP," + + " UNIQUE KEY `group_no_uid` (`group_no`, `uid`)" + + ") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4", + "CREATE TABLE `thread` (" + + " `id` INT NOT NULL AUTO_INCREMENT PRIMARY KEY," + + " `group_no` VARCHAR(40) NOT NULL DEFAULT ''," + + " `short_id` VARCHAR(40) NOT NULL DEFAULT ''," + + " `name` VARCHAR(100) NOT NULL DEFAULT ''," + + " `creator_uid` VARCHAR(40) NOT NULL DEFAULT ''," + + " `status` INT NOT NULL DEFAULT 1," + + // QueryActiveByGroupShortIDs（AuthorizeThreadFollow 路径）显式 SELECT + // last_message_at，最小表也必须带上（与 ensureThreadE2ETable 一致）。 + " `last_message_at` TIMESTAMP NULL DEFAULT NULL," + + " `created_at` TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP," + + " `updated_at` TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP," + + " UNIQUE KEY `uk_short` (`short_id`)" + + ") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4", + } + for _, s := range stmts { + _, err := ctx.DB().Exec(s) + require.NoError(t, err, "ccaEnsureTables: %s", s[:40]) + } +} + +// setupCheckChannelAccessData 建父群（space_id 空 → legacy wildcard 可见）+ +// 一个正常成员 + 一个 active 子区，返回生产同款 threadAuthChecker。 +func setupCheckChannelAccessData(t *testing.T) (*config.Context, *threadAuthChecker, string, string, string) { + t.Helper() + ctx := ccaNewCtx(t) + ccaEnsureTables(t, ctx) + + groupNo := strings.ReplaceAll(util.GenerUUID(), "-", "") + memberUID := "u_cca_" + util.GenerUUID()[:8] + shortID := "1489104291682713604" + + _, err := ctx.DB().Exec( + "INSERT INTO `group` (group_no, name, creator, status, version, space_id) VALUES (?, '父群', ?, 1, 1, '')", + groupNo, memberUID, + ) + require.NoError(t, err, "seed group") + _, err = ctx.DB().Exec( + "INSERT INTO group_member (group_no, uid, vercode, is_deleted, status, version) VALUES (?, ?, ?, 0, ?, 1)", + groupNo, memberUID, util.GenerUUID(), int(common.GroupMemberStatusNormal), + ) + require.NoError(t, err, "seed member") + _, err = ctx.DB().Exec( + "INSERT INTO thread (group_no, short_id, name, creator_uid, status) VALUES (?, ?, 'topic', ?, 1)", + groupNo, shortID, memberUID, + ) + require.NoError(t, err, "seed thread") + + return ctx, newThreadAuthChecker(ctx), groupNo, memberUID, shortID +} + +func ccaSetMemberStatus(t *testing.T, ctx *config.Context, groupNo, uid string, status common.GroupMemberStatus) { + t.Helper() + _, err := ctx.DB().Exec( + "UPDATE group_member SET status=? WHERE group_no=? AND uid=?", + int(status), groupNo, uid, + ) + require.NoError(t, err) +} + +// TestCheckChannelAccess_RequireActiveSplit 钉住 requireActive 两档语义： +// true（子区分支）排除被拉黑成员，false（GROUP 分支）保持 permissive。 +func TestCheckChannelAccess_RequireActiveSplit(t *testing.T) { + ctx, checker, groupNo, memberUID, _ := setupCheckChannelAccessData(t) + + t.Run("normal member passes both", func(t *testing.T) { + assert.NoError(t, checker.checkChannelAccess(memberUID, ccaSpaceID, groupNo, true), + "正常成员 requireActive=true 应放行") + assert.NoError(t, checker.checkChannelAccess(memberUID, ccaSpaceID, groupNo, false), + "正常成员 requireActive=false 应放行") + }) + + ccaSetMemberStatus(t, ctx, groupNo, memberUID, common.GroupMemberStatusBlacklist) + + t.Run("blacklisted member denied only when requireActive", func(t *testing.T) { + err := checker.checkChannelAccess(memberUID, ccaSpaceID, groupNo, true) + require.Error(t, err, "被拉黑成员 requireActive=true 必须被拒（#345 split）") + assert.True(t, errors.Is(err, convext.ErrChannelForbidden), + "应返回 ErrChannelForbidden，got %v", err) + assert.NoError(t, checker.checkChannelAccess(memberUID, ccaSpaceID, groupNo, false), + "GROUP 分支保持 permissive，不应 over-block 被拉黑成员") + }) + + t.Run("removed member denied on both", func(t *testing.T) { + _, err := ctx.DB().Exec( + "UPDATE group_member SET is_deleted=1 WHERE group_no=? AND uid=?", groupNo, memberUID) + require.NoError(t, err) + assert.Error(t, checker.checkChannelAccess(memberUID, ccaSpaceID, groupNo, true), + "被移出成员 requireActive=true 必须被拒") + assert.Error(t, checker.checkChannelAccess(memberUID, ccaSpaceID, groupNo, false), + "被移出成员 requireActive=false 也被拒（is_deleted 在两档都生效）") + }) +} + +// TestAuthorizeFollow_BlacklistSplit 钉住对外入口的接线：AuthorizeThreadFollow +// 必须走 requireActive=true（被拉黑 → ErrThreadForbidden），AuthorizeChannelFollow +// 走 false（被拉黑放行）。如果有人把 AuthorizeThreadFollow 翻回 permissive， +// 这里的 deny 断言会在 CI 直接红——这正是 #353 指出的静默回归缺口。 +func TestAuthorizeFollow_BlacklistSplit(t *testing.T) { + ctx, checker, groupNo, memberUID, shortID := setupCheckChannelAccessData(t) + + require.NoError(t, checker.AuthorizeThreadFollow(memberUID, ccaSpaceID, groupNo, shortID), + "正常成员 follow 子区不应被拦") + require.NoError(t, checker.AuthorizeChannelFollow(memberUID, ccaSpaceID, groupNo), + "正常成员 follow GROUP 不应被拦") + + ccaSetMemberStatus(t, ctx, groupNo, memberUID, common.GroupMemberStatusBlacklist) + + err := checker.AuthorizeThreadFollow(memberUID, ccaSpaceID, groupNo, shortID) + require.Error(t, err, "被拉黑父群成员 follow 子区必须被拒") + assert.True(t, errors.Is(err, convext.ErrThreadForbidden), + "应翻译为 ErrThreadForbidden（handler 走 403 路径），got %v", err) + + assert.NoError(t, checker.AuthorizeChannelFollow(memberUID, ccaSpaceID, groupNo), + "GROUP 分支保留 permissive ExistMember，被拉黑成员 channel follow 不被 over-block") +} diff --git a/modules/thread/read_path_blacklist_test.go b/modules/thread/read_path_blacklist_test.go index 4cfbb69e..ddd420ac 100644 --- a/modules/thread/read_path_blacklist_test.go +++ b/modules/thread/read_path_blacklist_test.go @@ -13,6 +13,7 @@ import ( "github.com/Mininglamp-OSS/octo-lib/testutil" "github.com/Mininglamp-OSS/octo-server/modules/group" "github.com/Mininglamp-OSS/octo-server/modules/user" + "github.com/Mininglamp-OSS/octo-server/pkg/errcode" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -67,6 +68,19 @@ func doGet(t *testing.T, s *server.Server, path string) *httptest.ResponseRecord return w } +// assertBlacklistDenied 收紧版 deny 断言（issue #353 P2）：原先四处裸 +// assert.NotEqual(200) 会把 404/500 等无关失败也当成"门禁生效"。deny 必须是 +// 成员门禁产生的业务错误——legacy D14 兼容下 ResponseErrorL wire=400，且 body +// 文案是预期错误码的 DefaultMessage（testutil 服务器用默认 renderer，只透出 +// msg/status，不带 i18n code id）。 +func assertBlacklistDenied(t *testing.T, w *httptest.ResponseRecorder, wantMsg, hint string) { + t.Helper() + assert.Equal(t, http.StatusBadRequest, w.Code, + "%s（deny 走 ResponseErrorL，legacy D14 wire=400），body=%s", hint, w.Body.String()) + assert.Contains(t, w.Body.String(), wantMsg, + "%s：deny 原因应是成员门禁而非其它失败", hint) +} + // TestReadPath_ListThreads_BlacklistTransition 列表读路径：normal 放行， // 被拉黑后直接 deny。 func TestReadPath_ListThreads_BlacklistTransition(t *testing.T) { @@ -78,7 +92,7 @@ func TestReadPath_ListThreads_BlacklistTransition(t *testing.T) { blacklistLoginUser(t, ctx, groupNo) w = doGet(t, s, path) - assert.NotEqual(t, http.StatusOK, w.Code, "被拉黑后列子区必须被拒") + assertBlacklistDenied(t, w, errcode.ErrThreadNotGroupMember.DefaultMessage, "被拉黑后列子区必须被拒") } // TestReadPath_GetThread_BlacklistTransition 详情读路径转换。 @@ -91,7 +105,7 @@ func TestReadPath_GetThread_BlacklistTransition(t *testing.T) { blacklistLoginUser(t, ctx, groupNo) w = doGet(t, s, path) - assert.NotEqual(t, http.StatusOK, w.Code, "被拉黑后读详情必须被拒") + assertBlacklistDenied(t, w, errcode.ErrThreadNotGroupMember.DefaultMessage, "被拉黑后读详情必须被拒") } // TestReadPath_ListMembers_BlacklistTransition 成员列表读路径转换。 @@ -104,7 +118,7 @@ func TestReadPath_ListMembers_BlacklistTransition(t *testing.T) { blacklistLoginUser(t, ctx, groupNo) w = doGet(t, s, path) - assert.NotEqual(t, http.StatusOK, w.Code, "被拉黑后列成员必须被拒") + assertBlacklistDenied(t, w, errcode.ErrThreadNotGroupMember.DefaultMessage, "被拉黑后列成员必须被拒") } // TestReadPath_GetThreadSimple_BlacklistTransition 简化路由读路径转换。 @@ -117,5 +131,21 @@ func TestReadPath_GetThreadSimple_BlacklistTransition(t *testing.T) { blacklistLoginUser(t, ctx, groupNo) w = doGet(t, s, path) - assert.NotEqual(t, http.StatusOK, w.Code, "被拉黑后读子区(简化路由)必须被拒") + assertBlacklistDenied(t, w, errcode.ErrThreadNotGroupMember.DefaultMessage, "被拉黑后读子区(简化路由)必须被拒") +} + +// TestReadPath_ThreadMdGet_BlacklistTransition 子区 GROUP.md 读路径转换 +// （issue #353 P2 点名补强：threadMdGet 返回 GROUP.md 正文，越权读面与 +// 列表/详情同级，此前没有 blacklist 覆盖）。deny 码与其它读路径不同—— +// threadMdGet 的非活跃成员走 ErrThreadPermissionDenied。 +func TestReadPath_ThreadMdGet_BlacklistTransition(t *testing.T) { + s, ctx, groupNo, shortID := setupReadPathBlacklistData(t) + path := "/v1/groups/" + groupNo + "/threads/" + shortID + "/md" + + w := doGet(t, s, path) + assert.Equal(t, http.StatusOK, w.Code, "正常成员应能读子区 GROUP.md（无内容时返回空 content）") + + blacklistLoginUser(t, ctx, groupNo) + w = doGet(t, s, path) + assertBlacklistDenied(t, w, errcode.ErrThreadPermissionDenied.DefaultMessage, "被拉黑后读子区 GROUP.md 必须被拒") }