From ebd83f690aeeb51515e3ecb5f1597c89dee9050f Mon Sep 17 00:00:00 2001 From: Octo Bot Date: Sat, 30 May 2026 09:19:12 +0800 Subject: [PATCH] fix(config): route group avatars to avatar bucket, not group bucket (#21) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GetGroupAvatarFilePath returned paths prefixed with "group/" which routed group avatar images into the private "group" MinIO/OSS bucket (intended for group exports). The MinIO/OSS backend in octo-server selects the destination bucket from the first path segment via splitBucketAndObject, so "group/..." → private group bucket → 403 on anonymous GET. Changed the prefix to "avatar/group/" so group avatars land in the publicly readable "avatar" bucket alongside user avatars, matching the deployment README bucket policy table. Deployments upgrading from the old path should either re-generate group avatars (trigger a group-member change) or copy existing objects from the "group" bucket to "avatar" bucket under the new key layout. Also fixes Mininglamp-OSS/octo-server#103. Signed-off-by: lifeifei --- config/config.go | 19 ++++++++- config/config_avatar_path_test.go | 70 +++++++++++++++++++++++++++++++ 2 files changed, 87 insertions(+), 2 deletions(-) create mode 100644 config/config_avatar_path_test.go diff --git a/config/config.go b/config/config.go index 993ca75..14592c4 100644 --- a/config/config.go +++ b/config/config.go @@ -876,10 +876,25 @@ func (c *Config) GetAvatarPath(uid string) string { return fmt.Sprintf("users/%s/avatar", uid) } -// GetGroupAvatarFilePath 获取群头像上传路径 +// GetGroupAvatarFilePath returns the object path for a group's avatar image. +// +// The path is routed under the "avatar" bucket so it lands in the publicly +// readable bucket used by all avatars (user, group, community). Before this +// fix (GH#21 / octo-server#103) the path started with "group/", which +// routed the file into the private "group" bucket (intended for group +// exports, not publicly served images). The MinIO / OSS splitBucketAndObject +// helper in octo-server selects the destination bucket from the first path +// segment when it appears in the allow-list, so "avatar/..." → avatar +// bucket (anonymous-readable) and "group/..." → group bucket (private). +// +// Deployments upgrading from a version that stored group avatars under the +// old "group/" path should either: +// - re-generate group avatars (trigger a group-member change), or +// - copy the existing objects from the "group" bucket to "avatar" bucket +// under the new key layout. func (c *Config) GetGroupAvatarFilePath(groupNo string) string { avatarID := crc32.ChecksumIEEE([]byte(groupNo)) % uint32(c.Avatar.Partition) - return fmt.Sprintf("group/%d/%s.png", avatarID, groupNo) + return fmt.Sprintf("avatar/group/%d/%s.png", avatarID, groupNo) } // GetCommunityAvatarFilePath 获取社区头像上传路径 diff --git a/config/config_avatar_path_test.go b/config/config_avatar_path_test.go new file mode 100644 index 0000000..71f841e --- /dev/null +++ b/config/config_avatar_path_test.go @@ -0,0 +1,70 @@ +package config + +import ( + "strings" + "testing" + + "github.com/spf13/viper" +) + +// TestGetGroupAvatarFilePath_BucketPrefix verifies that the group avatar path +// lands in the "avatar" bucket (publicly readable), not the "group" bucket +// (private). This is the regression guard for GH#21 / octo-server#103. +func TestGetGroupAvatarFilePath_BucketPrefix(t *testing.T) { + cfg := New() + vp := viper.New() + cfg.ConfigureWithViper(vp) + + path := cfg.GetGroupAvatarFilePath("g_test_12345") + + // Must start with "avatar/" so splitBucketAndObject routes to the avatar bucket. + if !strings.HasPrefix(path, "avatar/") { + t.Errorf("GetGroupAvatarFilePath should start with 'avatar/' to route to the public avatar bucket, got %q", path) + } + + // Must NOT start with "group/" — that routes to the private group bucket. + if strings.HasPrefix(path, "group/") { + t.Errorf("GetGroupAvatarFilePath must not start with 'group/' (private bucket), got %q", path) + } + + // Must end with ".png" and contain the group number. + if !strings.HasSuffix(path, ".png") { + t.Errorf("GetGroupAvatarFilePath should end with '.png', got %q", path) + } + if !strings.Contains(path, "g_test_12345") { + t.Errorf("GetGroupAvatarFilePath should contain the group number, got %q", path) + } + + // Must contain a "group/" sub-path to namespace group avatars within the avatar bucket. + if !strings.Contains(path, "/group/") { + t.Errorf("GetGroupAvatarFilePath should contain '/group/' namespace, got %q", path) + } +} + +// TestGetGroupAvatarFilePath_Deterministic verifies the same groupNo always +// produces the same path (partition is deterministic via CRC32). +func TestGetGroupAvatarFilePath_Deterministic(t *testing.T) { + cfg := New() + vp := viper.New() + cfg.ConfigureWithViper(vp) + + p1 := cfg.GetGroupAvatarFilePath("g_abc") + p2 := cfg.GetGroupAvatarFilePath("g_abc") + if p1 != p2 { + t.Errorf("GetGroupAvatarFilePath should be deterministic: %q != %q", p1, p2) + } +} + +// TestGetGroupAvatarFilePath_DifferentGroups verifies distinct groups get +// distinct paths (collision-free at the groupNo level). +func TestGetGroupAvatarFilePath_DifferentGroups(t *testing.T) { + cfg := New() + vp := viper.New() + cfg.ConfigureWithViper(vp) + + p1 := cfg.GetGroupAvatarFilePath("g_alice_group") + p2 := cfg.GetGroupAvatarFilePath("g_bob_group") + if p1 == p2 { + t.Errorf("different groups should produce different paths: both %q", p1) + } +}