test(thread): close the integration-tag coverage gap on subchannel blacklist gates

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")
+}

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 必须被拒")
 }