From 03aa47a0157b4062c3b0f2b652f8327661739e1b Mon Sep 17 00:00:00 2001 From: Kim Jun Young Date: Sat, 4 Jul 2026 17:04:14 +0900 Subject: [PATCH 1/2] feat: per-division discord role and announce channel + division update API --- cmd/server/main.go | 2 +- docs/docs/admin.md | 58 +++++++- docs/docs/discord.md | 27 +++- docs/docs/divisions.md | 5 + internal/discord/client.go | 26 ++-- internal/discord/client_test.go | 24 +++- internal/http/handlers/discord_test.go | 8 +- internal/http/handlers/handler.go | 23 +++- internal/http/handlers/handler_test.go | 89 +++++++++++++ internal/http/handlers/testenv_test.go | 10 +- internal/http/handlers/types.go | 10 +- internal/http/integration/testenv_test.go | 2 +- internal/http/router.go | 1 + internal/models/division.go | 10 +- internal/repo/division_repo.go | 22 +++ internal/service/discord_service.go | 78 ++++++++--- internal/service/discord_service_test.go | 125 +++++++++++++++--- internal/service/division_service.go | 56 +++++++- internal/service/division_service_test.go | 124 ++++++++++++++++- internal/service/testenv_test.go | 10 +- internal/service/validation.go | 40 ++++++ invite-bot/.env.example | 8 +- invite-bot/src/config.ts | 4 - invite-bot/src/discord/client.ts | 22 +-- invite-bot/src/http/server.ts | 13 +- .../001_add_division_discord_config.sql | 7 + migrations/2026-07-04/999_rollback.sql | 7 + 27 files changed, 713 insertions(+), 98 deletions(-) create mode 100644 migrations/2026-07-04/001_add_division_discord_config.sql create mode 100644 migrations/2026-07-04/999_rollback.sql diff --git a/cmd/server/main.go b/cmd/server/main.go index eca9e43..ce65d9f 100644 --- a/cmd/server/main.go +++ b/cmd/server/main.go @@ -118,7 +118,7 @@ func main() { RedirectURI: cfg.Discord.RedirectURI, Scopes: cfg.Discord.Scopes, }, cfg.Discord.OAuthTimeout) - discordSvc = service.NewDiscordService(cfg.Discord, discordRepo, userRepo, discordBotClient, discordOAuthClient, redisClient) + discordSvc = service.NewDiscordService(cfg.Discord, discordRepo, userRepo, divisionRepo, discordBotClient, discordOAuthClient, redisClient) } bootstrap.BootstrapAdmin(ctx, cfg, database, userRepo, teamRepo, divisionRepo, logger) diff --git a/docs/docs/admin.md b/docs/docs/admin.md index bae80a2..e7ce788 100644 --- a/docs/docs/admin.md +++ b/docs/docs/admin.md @@ -317,16 +317,25 @@ Request ```json { - "name": "고등부" + "name": "고등부", + "discord_role_id": "1522163303982563458", + "discord_announce_channel_id": "1522218332806447225" } ``` +`discord_role_id` and `discord_announce_channel_id` are optional. `discord_role_id` +is the guild role granted to this division's members when they link Discord; +`discord_announce_channel_id` is the channel that receives this division's solve / +first-blood announcements. Both are omitted from the response when unset. + Response 201 ```json { "id": 2, "name": "고등부", + "discord_role_id": "1522163303982563458", + "discord_announce_channel_id": "1522218332806447225", "created_at": "2026-01-26T12:00:00Z" } ``` @@ -340,6 +349,53 @@ Errors: Validation notes: - `name` must be at most 10 characters (counted by Unicode code point). +- `discord_role_id` / `discord_announce_channel_id`, when present, must be numeric + Discord IDs (snowflakes). An empty string clears the value. + +--- + +## Update Division (Admin) + +`PUT /api/admin/divisions/:id` + +Updates a division's name and per-division Discord configuration. Divisions cannot +be deleted. + +Request + +```json +{ + "name": "고등부", + "discord_role_id": "1522163303982563458", + "discord_announce_channel_id": "1522218332806447225" +} +``` + +Omitting (or sending an empty string for) `discord_role_id` / +`discord_announce_channel_id` clears that setting. + +Response 200 + +```json +{ + "id": 2, + "name": "고등부", + "discord_role_id": "1522163303982563458", + "discord_announce_channel_id": "1522218332806447225", + "created_at": "2026-01-26T12:00:00Z" +} +``` + +Errors: + +- 400 `invalid input` +- 401 `invalid token` or `missing access_token cookie` +- 403 `forbidden` +- 404 `not found` + +Validation notes: + +- Same as Create Division. --- diff --git a/docs/docs/discord.md b/docs/docs/discord.md index d9bb8e1..a5387e9 100644 --- a/docs/docs/discord.md +++ b/docs/docs/discord.md @@ -6,19 +6,35 @@ nav_order: 12 Notes: - Discord account linking uses the OAuth2 authorization-code flow on the backend (`identify`, optionally `guilds.join`). -- Guild membership and role changes are performed by a separate **invite-bot** server over an internal HTTP API; the backend never holds the Discord bot token. The bot token, guild id, and verified-role id live on the bot server only. +- Guild membership and role changes are performed by a separate **invite-bot** server over an internal HTTP API; the backend never holds the Discord bot token. The bot token and guild id live on the bot server only; the role and announcement channel are configured per division in the backend and passed to the bot per request. - Access/refresh tokens are **not** stored. Role granting uses the bot token on the bot server, not the user's OAuth token. - The acting user is identified by the JWT `access_token` cookie. `connect`, `callback`, and `status` require login; `sync-role` and `unlink` additionally require an unblocked (active) user. - For authenticated `POST`, `PUT`, `PATCH`, and `DELETE` requests, send both `csrf_token` cookie and matching `X-CSRF-Token` header. - All endpoints return `503 discord feature disabled` when `DISCORD_ENABLED=false`. +## Per-Division Discord Configuration + +The Discord role granted on link and the solve-announcement channel are configured +**per division** (Admin > Divisions, `discord_role_id` and +`discord_announce_channel_id`), not as bot environment variables. Both are +optional. The backend resolves the acting user's division and passes the relevant +role id / channel id to the invite-bot on each request; the bot no longer holds a +`DISCORD_VERIFIED_ROLE_ID` or `DISCORD_ANNOUNCE_CHANNEL_ID`. + +- **Role grant** — on link, the member is granted the division's `discord_role_id`. + If the division has no role configured, linking is still marked `VERIFIED` and no + role is granted. +- **Announcement channel** — solves post to the division's + `discord_announce_channel_id`. If unset, that division's announcements are + disabled. + ## Solve Announcements & Nickname Sync When `DISCORD_ENABLED=true`, the backend performs two best-effort side effects through the invite-bot (failures are logged, never surfaced to the user): - **Solve announcements** — on a correct flag submission, a message is sent to the - channel `DISCORD_ANNOUNCE_CHANNEL_ID` (no-op when unset): + solving user's division announcement channel (no-op when the division has none): - Normal solve: `**division_team** — username solved **Challenge Title**` - First blood: prefixed with 🩸 `First Blood!` @@ -229,7 +245,9 @@ Discord service options (backend): - `DISCORD_BOT_BASE_URL` (default: `http://localhost:8083`) - `DISCORD_BOT_SECRET` — shared bearer secret for the invite-bot internal API (must equal the bot's `DISCORD_INTERNAL_SECRET`). - `DISCORD_BOT_TIMEOUT` (default: `5s`) -- `DISCORD_ANNOUNCE_CHANNEL_ID` — target channel for solve / first-blood announcements. Empty disables announcements (configured on the invite-bot server). + +The verified role and solve-announcement channel are **not** environment +variables — configure them per division (Admin > Divisions). Validation rules when `DISCORD_ENABLED=true`: @@ -241,4 +259,5 @@ Validation rules when `DISCORD_ENABLED=true`: - `DISCORD_BOT_TIMEOUT > 0` - `DISCORD_OAUTH_TIMEOUT > 0` -The Discord bot token, guild id, and verified-role id are configured on the invite-bot server, not here. +The Discord bot token and guild id are configured on the invite-bot server, not here. +The verified role and announcement channel are configured per division in the backend. diff --git a/docs/docs/divisions.md b/docs/docs/divisions.md index 47df3f2..2bb7972 100644 --- a/docs/docs/divisions.md +++ b/docs/docs/divisions.md @@ -14,6 +14,8 @@ Response 200 { "id": 2, "name": "고등부", + "discord_role_id": "1522163303982563458", + "discord_announce_channel_id": "1522218332806447225", "created_at": "2026-01-26T12:00:00Z" } ] @@ -22,3 +24,6 @@ Response 200 Notes: - Division identifiers are numeric `id` values (slugs are not supported). +- `discord_role_id` and `discord_announce_channel_id` are the per-division Discord + role and announcement channel. They are omitted when unset and are managed by + admins via Create / Update Division. See [Admin](admin.md) and [Discord](discord.md). diff --git a/internal/discord/client.go b/internal/discord/client.go index 88d8314..b61828a 100644 --- a/internal/discord/client.go +++ b/internal/discord/client.go @@ -67,11 +67,11 @@ type Member struct { } type BotAPI interface { - JoinGuild(ctx context.Context, discordUserID, accessToken string) error - GrantRole(ctx context.Context, discordUserID string) error + JoinGuild(ctx context.Context, discordUserID, accessToken, roleID string) error + GrantRole(ctx context.Context, discordUserID, roleID string) error KickMember(ctx context.Context, discordUserID string) error GetMember(ctx context.Context, discordUserID string) (*Member, error) - Announce(ctx context.Context, content string) error + Announce(ctx context.Context, channelID, content string) error SetNickname(ctx context.Context, discordUserID, nickname string) error } @@ -93,30 +93,36 @@ func NewBotClient(baseURL, secret string, timeout time.Duration) *BotClient { type joinRequest struct { AccessToken string `json:"access_token"` + RoleID string `json:"role_id,omitempty"` +} + +type roleRequest struct { + RoleID string `json:"role_id"` } type announceRequest struct { - Content string `json:"content"` + ChannelID string `json:"channel_id"` + Content string `json:"content"` } type nicknameRequest struct { Nickname string `json:"nickname"` } -func (c *BotClient) JoinGuild(ctx context.Context, discordUserID, accessToken string) error { - return c.doJSON(ctx, http.MethodPut, "/internal/guild/members/"+url.PathEscape(discordUserID), joinRequest{AccessToken: accessToken}, nil) +func (c *BotClient) JoinGuild(ctx context.Context, discordUserID, accessToken, roleID string) error { + return c.doJSON(ctx, http.MethodPut, "/internal/guild/members/"+url.PathEscape(discordUserID), joinRequest{AccessToken: accessToken, RoleID: roleID}, nil) } -func (c *BotClient) GrantRole(ctx context.Context, discordUserID string) error { - return c.doJSON(ctx, http.MethodPut, "/internal/guild/members/"+url.PathEscape(discordUserID)+"/role", nil, nil) +func (c *BotClient) GrantRole(ctx context.Context, discordUserID, roleID string) error { + return c.doJSON(ctx, http.MethodPut, "/internal/guild/members/"+url.PathEscape(discordUserID)+"/role", roleRequest{RoleID: roleID}, nil) } func (c *BotClient) KickMember(ctx context.Context, discordUserID string) error { return c.doJSON(ctx, http.MethodDelete, "/internal/guild/members/"+url.PathEscape(discordUserID), nil, nil) } -func (c *BotClient) Announce(ctx context.Context, content string) error { - return c.doJSON(ctx, http.MethodPost, "/internal/announce", announceRequest{Content: content}, nil) +func (c *BotClient) Announce(ctx context.Context, channelID, content string) error { + return c.doJSON(ctx, http.MethodPost, "/internal/announce", announceRequest{ChannelID: channelID, Content: content}, nil) } func (c *BotClient) SetNickname(ctx context.Context, discordUserID, nickname string) error { diff --git a/internal/discord/client_test.go b/internal/discord/client_test.go index 20997a8..51c43c0 100644 --- a/internal/discord/client_test.go +++ b/internal/discord/client_test.go @@ -12,15 +12,17 @@ import ( func TestBotClientGrantRoleSuccess(t *testing.T) { var gotPath, gotAuth string + var body roleRequest srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { gotPath = r.URL.Path gotAuth = r.Header.Get("Authorization") + _ = json.NewDecoder(r.Body).Decode(&body) w.WriteHeader(http.StatusNoContent) })) defer srv.Close() client := NewBotClient(srv.URL, "secret", time.Second) - if err := client.GrantRole(context.Background(), "123"); err != nil { + if err := client.GrantRole(context.Background(), "123", "role-9"); err != nil { t.Fatalf("GrantRole: %v", err) } @@ -31,6 +33,10 @@ func TestBotClientGrantRoleSuccess(t *testing.T) { if gotAuth != "Bearer secret" { t.Errorf("auth = %q", gotAuth) } + + if body.RoleID != "role-9" { + t.Errorf("role id = %q", body.RoleID) + } } func TestBotClientKickMember(t *testing.T) { @@ -82,7 +88,7 @@ func TestBotClientGrantRoleMapsCodes(t *testing.T) { defer srv.Close() client := NewBotClient(srv.URL, "", time.Second) - err := client.GrantRole(context.Background(), "123") + err := client.GrantRole(context.Background(), "123", "role-1") if !errors.Is(err, tc.wantErr) { t.Fatalf("got %v, want %v", err, tc.wantErr) } @@ -103,13 +109,17 @@ func TestBotClientJoinGuildSendsAccessToken(t *testing.T) { defer srv.Close() client := NewBotClient(srv.URL, "", time.Second) - if err := client.JoinGuild(context.Background(), "u1", "tok-abc"); err != nil { + if err := client.JoinGuild(context.Background(), "u1", "tok-abc", "role-5"); err != nil { t.Fatalf("JoinGuild: %v", err) } if body.AccessToken != "tok-abc" { t.Errorf("access token = %q", body.AccessToken) } + + if body.RoleID != "role-5" { + t.Errorf("role id = %q", body.RoleID) + } } func TestBotClientGetMember(t *testing.T) { @@ -132,7 +142,7 @@ func TestBotClientGetMember(t *testing.T) { func TestBotClientUnavailableOnDialError(t *testing.T) { client := NewBotClient("http://127.0.0.1:0", "", 200*time.Millisecond) - err := client.GrantRole(context.Background(), "1") + err := client.GrantRole(context.Background(), "1", "role-1") if !errors.Is(err, ErrUnavailable) { t.Fatalf("got %v, want ErrUnavailable", err) } @@ -150,7 +160,7 @@ func TestBotClientAnnounce(t *testing.T) { defer srv.Close() client := NewBotClient(srv.URL, "secret", time.Second) - if err := client.Announce(context.Background(), "hello world"); err != nil { + if err := client.Announce(context.Background(), "chan-1", "hello world"); err != nil { t.Fatalf("Announce: %v", err) } @@ -161,6 +171,10 @@ func TestBotClientAnnounce(t *testing.T) { if body.Content != "hello world" { t.Errorf("content = %q", body.Content) } + + if body.ChannelID != "chan-1" { + t.Errorf("channel id = %q", body.ChannelID) + } } func TestBotClientSetNickname(t *testing.T) { diff --git a/internal/http/handlers/discord_test.go b/internal/http/handlers/discord_test.go index 0af4ca2..aee40c1 100644 --- a/internal/http/handlers/discord_test.go +++ b/internal/http/handlers/discord_test.go @@ -29,8 +29,8 @@ type fakeBot struct { nickname string } -func (f *fakeBot) JoinGuild(_ context.Context, _, _ string) error { return nil } -func (f *fakeBot) GrantRole(_ context.Context, _ string) error { return f.grantErr } +func (f *fakeBot) JoinGuild(_ context.Context, _, _, _ string) error { return nil } +func (f *fakeBot) GrantRole(_ context.Context, _, _ string) error { return f.grantErr } func (f *fakeBot) KickMember(_ context.Context, _ string) error { f.kicked = true return nil @@ -40,7 +40,7 @@ func (f *fakeBot) GetMember(_ context.Context, _ string) (*discord.Member, error return &discord.Member{}, nil } -func (f *fakeBot) Announce(_ context.Context, content string) error { +func (f *fakeBot) Announce(_ context.Context, _, content string) error { f.mu.Lock() defer f.mu.Unlock() f.announced = append(f.announced, content) @@ -97,7 +97,7 @@ func discordEnabledHandler(env handlerEnv, bot discord.BotAPI, user *discord.Use SuccessRedirect: "http://localhost:3000/profile", InviteURL: "https://discord.gg/invite", } - discordSvc := service.NewDiscordService(cfg.Discord, repo.NewDiscordRepo(env.db), env.userRepo, bot, fakeOAuth{user: user}, env.redis) + discordSvc := service.NewDiscordService(cfg.Discord, repo.NewDiscordRepo(env.db), env.userRepo, env.divisionRepo, bot, fakeOAuth{user: user}, env.redis) return New(cfg, env.authSvc, env.ctfSvc, env.appConfigSvc, env.userSvc, env.scoreSvc, env.divisionSvc, env.teamSvc, env.vmSvc, env.redis, discordSvc) } diff --git a/internal/http/handlers/handler.go b/internal/http/handlers/handler.go index e4844f5..4b155af 100644 --- a/internal/http/handlers/handler.go +++ b/internal/http/handlers/handler.go @@ -1384,7 +1384,7 @@ func (h *Handler) CreateDivision(ctx *gin.Context) { return } - division, err := h.divs.CreateDivision(ctx.Request.Context(), req.Name) + division, err := h.divs.CreateDivision(ctx.Request.Context(), req.Name, req.DiscordRoleID, req.DiscordAnnounceChannelID) if err != nil { writeError(ctx, err) return @@ -1393,6 +1393,27 @@ func (h *Handler) CreateDivision(ctx *gin.Context) { ctx.JSON(http.StatusCreated, division) } +func (h *Handler) UpdateDivision(ctx *gin.Context) { + id, ok := parseIDParamOrError(ctx, "id") + if !ok { + return + } + + var req updateDivisionRequest + if err := ctx.ShouldBindJSON(&req); err != nil { + writeBindError(ctx, err) + return + } + + division, err := h.divs.UpdateDivision(ctx.Request.Context(), id, req.Name, req.DiscordRoleID, req.DiscordAnnounceChannelID) + if err != nil { + writeError(ctx, err) + return + } + + ctx.JSON(http.StatusOK, division) +} + // Team Handlers func (h *Handler) CreateTeam(ctx *gin.Context) { diff --git a/internal/http/handlers/handler_test.go b/internal/http/handlers/handler_test.go index bd9dafd..3043f6d 100644 --- a/internal/http/handlers/handler_test.go +++ b/internal/http/handlers/handler_test.go @@ -2748,3 +2748,92 @@ func TestTimePtrUTC(t *testing.T) { t.Fatalf("expected UTC time, got %v", utc) } } + +func TestHandlerCreateDivisionWithDiscordConfig(t *testing.T) { + env := setupHandlerTest(t) + + ctx, rec := newJSONContext(t, http.MethodPost, "/api/admin/divisions", map[string]any{ + "name": "Uni", + "discord_role_id": "123456789012345678", + "discord_announce_channel_id": "987654321098765432", + }) + env.handler.CreateDivision(ctx) + if rec.Code != http.StatusCreated { + t.Fatalf("create division status %d: %s", rec.Code, rec.Body.String()) + } + + var resp struct { + ID int64 `json:"id"` + Name string `json:"name"` + DiscordRoleID *string `json:"discord_role_id"` + DiscordAnnounceChannelID *string `json:"discord_announce_channel_id"` + } + decodeJSON(t, rec, &resp) + if resp.ID == 0 || resp.Name != "Uni" { + t.Fatalf("unexpected division: %+v", resp) + } + if resp.DiscordRoleID == nil || *resp.DiscordRoleID != "123456789012345678" { + t.Fatalf("role id = %+v", resp.DiscordRoleID) + } + if resp.DiscordAnnounceChannelID == nil || *resp.DiscordAnnounceChannelID != "987654321098765432" { + t.Fatalf("channel id = %+v", resp.DiscordAnnounceChannelID) + } + + // Invalid snowflake -> 400. + ctx, rec = newJSONContext(t, http.MethodPost, "/api/admin/divisions", map[string]any{"name": "Bad", "discord_role_id": "abc"}) + env.handler.CreateDivision(ctx) + if rec.Code != http.StatusBadRequest { + t.Fatalf("expected 400 for invalid snowflake, got %d", rec.Code) + } + + // Missing name -> 400. + ctx, rec = newJSONContext(t, http.MethodPost, "/api/admin/divisions", map[string]any{}) + env.handler.CreateDivision(ctx) + if rec.Code != http.StatusBadRequest { + t.Fatalf("expected 400 for missing name, got %d", rec.Code) + } +} + +func TestHandlerUpdateDivision(t *testing.T) { + env := setupHandlerTest(t) + + created, err := env.divisionSvc.CreateDivision(context.Background(), "Orig", nil, nil) + if err != nil { + t.Fatalf("seed division: %v", err) + } + + ctx, rec := newJSONContext(t, http.MethodPut, "/api/admin/divisions/x", map[string]any{ + "name": "Renamed", + "discord_role_id": "555555555555555555", + }) + ctx.Params = gin.Params{{Key: "id", Value: fmt.Sprintf("%d", created.ID)}} + env.handler.UpdateDivision(ctx) + if rec.Code != http.StatusOK { + t.Fatalf("update status %d: %s", rec.Code, rec.Body.String()) + } + + var resp struct { + Name string `json:"name"` + DiscordRoleID *string `json:"discord_role_id"` + } + decodeJSON(t, rec, &resp) + if resp.Name != "Renamed" || resp.DiscordRoleID == nil || *resp.DiscordRoleID != "555555555555555555" { + t.Fatalf("unexpected update response: %+v", resp) + } + + // Non-numeric id -> 400. + ctx, rec = newJSONContext(t, http.MethodPut, "/api/admin/divisions/abc", map[string]any{"name": "X"}) + ctx.Params = gin.Params{{Key: "id", Value: "abc"}} + env.handler.UpdateDivision(ctx) + if rec.Code != http.StatusBadRequest { + t.Fatalf("expected 400 for bad id, got %d", rec.Code) + } + + // Unknown id -> 404. + ctx, rec = newJSONContext(t, http.MethodPut, "/api/admin/divisions/999999", map[string]any{"name": "X"}) + ctx.Params = gin.Params{{Key: "id", Value: "999999"}} + env.handler.UpdateDivision(ctx) + if rec.Code != http.StatusNotFound { + t.Fatalf("expected 404 for unknown id, got %d", rec.Code) + } +} diff --git a/internal/http/handlers/testenv_test.go b/internal/http/handlers/testenv_test.go index 943787e..1ad58a6 100644 --- a/internal/http/handlers/testenv_test.go +++ b/internal/http/handlers/testenv_test.go @@ -274,9 +274,13 @@ func setupHandlerTest(t *testing.T) handlerEnv { handler: handler, } + defaultRoleID := "111111111111111111" + defaultChannelID := "222222222222222222" division := &models.Division{ - Name: "Default", - CreatedAt: time.Now().UTC(), + Name: "Default", + DiscordRoleID: &defaultRoleID, + DiscordAnnounceChannelID: &defaultChannelID, + CreatedAt: time.Now().UTC(), } if err := divisionRepo.Create(context.Background(), division); err != nil { t.Fatalf("create division: %v", err) @@ -290,7 +294,7 @@ func setupHandlerTest(t *testing.T) handlerEnv { func resetHandlerState(t *testing.T) { t.Helper() - if _, err := handlerDB.ExecContext(context.Background(), "TRUNCATE TABLE app_configs, submissions, registration_key_uses, registration_keys, vms, challenges, users, teams, divisions RESTART IDENTITY CASCADE"); err != nil { + if _, err := handlerDB.ExecContext(context.Background(), "TRUNCATE TABLE app_configs, submissions, registration_key_uses, registration_keys, vms, challenges, discord_connections, users, teams, divisions RESTART IDENTITY CASCADE"); err != nil { t.Fatalf("truncate tables: %v", err) } diff --git a/internal/http/handlers/types.go b/internal/http/handlers/types.go index a1c5a03..5b122fc 100644 --- a/internal/http/handlers/types.go +++ b/internal/http/handlers/types.go @@ -270,7 +270,15 @@ type teamResponse struct { } type createDivisionRequest struct { - Name string `json:"name" binding:"required"` + Name string `json:"name" binding:"required"` + DiscordRoleID *string `json:"discord_role_id"` + DiscordAnnounceChannelID *string `json:"discord_announce_channel_id"` +} + +type updateDivisionRequest struct { + Name string `json:"name" binding:"required"` + DiscordRoleID *string `json:"discord_role_id"` + DiscordAnnounceChannelID *string `json:"discord_announce_channel_id"` } type timelineResponse struct { diff --git a/internal/http/integration/testenv_test.go b/internal/http/integration/testenv_test.go index 8638853..003fdbf 100644 --- a/internal/http/integration/testenv_test.go +++ b/internal/http/integration/testenv_test.go @@ -410,7 +410,7 @@ func setCTFWindow(t *testing.T, env testEnv, startAt, endAt *time.Time) { func resetState(t *testing.T) { t.Helper() - if _, err := testDB.ExecContext(context.Background(), "TRUNCATE TABLE app_configs, submissions, registration_key_uses, registration_keys, vms, challenges, users, teams, divisions RESTART IDENTITY CASCADE"); err != nil { + if _, err := testDB.ExecContext(context.Background(), "TRUNCATE TABLE app_configs, submissions, registration_key_uses, registration_keys, vms, challenges, discord_connections, users, teams, divisions RESTART IDENTITY CASCADE"); err != nil { t.Fatalf("truncate tables: %v", err) } diff --git a/internal/http/router.go b/internal/http/router.go index 8c238fb..afe00ea 100644 --- a/internal/http/router.go +++ b/internal/http/router.go @@ -90,6 +90,7 @@ func NewRouter(cfg config.Config, authSvc *service.AuthService, ctfSvc *service. admin.POST("/registration-keys", h.CreateRegistrationKeys) admin.GET("/registration-keys", h.ListRegistrationKeys) admin.POST("/divisions", h.CreateDivision) + admin.PUT("/divisions/:id", h.UpdateDivision) admin.POST("/teams", h.CreateTeam) admin.GET("/vms", h.AdminListVMs) admin.GET("/vms/:vm_id", h.AdminGetVM) diff --git a/internal/models/division.go b/internal/models/division.go index 4538f8a..b2868c9 100644 --- a/internal/models/division.go +++ b/internal/models/division.go @@ -8,8 +8,10 @@ import ( // Database model for divisions type Division struct { - bun.BaseModel `bun:"table:divisions"` - ID int64 `bun:"id,pk,autoincrement" json:"id"` - Name string `bun:"name,unique,notnull" json:"name"` - CreatedAt time.Time `bun:"created_at,nullzero,notnull,default:current_timestamp" json:"created_at"` + bun.BaseModel `bun:"table:divisions"` + ID int64 `bun:"id,pk,autoincrement" json:"id"` + Name string `bun:"name,unique,notnull" json:"name"` + DiscordRoleID *string `bun:"discord_role_id,nullzero" json:"discord_role_id,omitempty"` + DiscordAnnounceChannelID *string `bun:"discord_announce_channel_id,nullzero" json:"discord_announce_channel_id,omitempty"` + CreatedAt time.Time `bun:"created_at,nullzero,notnull,default:current_timestamp" json:"created_at"` } diff --git a/internal/repo/division_repo.go b/internal/repo/division_repo.go index a45b39c..923e717 100644 --- a/internal/repo/division_repo.go +++ b/internal/repo/division_repo.go @@ -24,6 +24,28 @@ func (r *DivisionRepo) Create(ctx context.Context, division *models.Division) er return nil } +func (r *DivisionRepo) Update(ctx context.Context, division *models.Division) error { + res, err := r.db.NewUpdate(). + Model(division). + Column("name", "discord_role_id", "discord_announce_channel_id"). + WherePK(). + Exec(ctx) + if err != nil { + return wrapError("divisionRepo.Update", err) + } + + affected, err := res.RowsAffected() + if err != nil { + return wrapError("divisionRepo.Update", err) + } + + if affected == 0 { + return ErrNotFound + } + + return nil +} + func (r *DivisionRepo) List(ctx context.Context) ([]models.Division, error) { divisions := make([]models.Division, 0) if err := r.db.NewSelect().Model(&divisions).OrderExpr("id ASC").Scan(ctx); err != nil { diff --git a/internal/service/discord_service.go b/internal/service/discord_service.go index cb9d32d..76a6208 100644 --- a/internal/service/discord_service.go +++ b/internal/service/discord_service.go @@ -26,30 +26,43 @@ type DiscordOAuth interface { } type DiscordService struct { - cfg config.DiscordConfig - repo *repo.DiscordRepo - users *repo.UserRepo - bot discord.BotAPI - oauth DiscordOAuth - redis *redis.Client - wg sync.WaitGroup + cfg config.DiscordConfig + repo *repo.DiscordRepo + users *repo.UserRepo + divisions *repo.DivisionRepo + bot discord.BotAPI + oauth DiscordOAuth + redis *redis.Client + wg sync.WaitGroup } func (s *DiscordService) Wait() { s.wg.Wait() } -func NewDiscordService(cfg config.DiscordConfig, discordRepo *repo.DiscordRepo, userRepo *repo.UserRepo, bot discord.BotAPI, oauth DiscordOAuth, redisClient *redis.Client) *DiscordService { +func NewDiscordService(cfg config.DiscordConfig, discordRepo *repo.DiscordRepo, userRepo *repo.UserRepo, divisionRepo *repo.DivisionRepo, bot discord.BotAPI, oauth DiscordOAuth, redisClient *redis.Client) *DiscordService { return &DiscordService{ - cfg: cfg, - repo: discordRepo, - users: userRepo, - bot: bot, - oauth: oauth, - redis: redisClient, + cfg: cfg, + repo: discordRepo, + users: userRepo, + divisions: divisionRepo, + bot: bot, + oauth: oauth, + redis: redisClient, } } +// divisionForUser resolves the division of the given user, for reading the +// division-scoped Discord role / announcement channel. +func (s *DiscordService) divisionForUser(ctx context.Context, userID int64) (*models.Division, error) { + user, err := s.users.GetByID(ctx, userID) + if err != nil { + return nil, err + } + + return s.divisions.GetByID(ctx, user.DivisionID) +} + func (s *DiscordService) ensureEnabled() error { if s == nil || !s.cfg.Enabled { return ErrDiscordDisabled @@ -249,8 +262,21 @@ func (s *DiscordService) provision(ctx context.Context, conn *models.DiscordConn conn.UpdatedAt = now conn.LastError = nil + // Resolve the division-scoped role to grant. An empty role means this + // division does not assign a Discord role, so linking is considered + // verified without a role grant. + roleID := "" + if div, err := s.divisionForUser(ctx, conn.UserID); err != nil { + slog.Warn("discord: division lookup failed", + slog.Int64("user_id", conn.UserID), + slog.Any("error", err), + ) + } else { + roleID = derefOrEmpty(div.DiscordRoleID) + } + if s.cfg.AutoJoin && accessToken != "" { - if err := s.bot.JoinGuild(ctx, conn.DiscordUserID, accessToken); err != nil { + if err := s.bot.JoinGuild(ctx, conn.DiscordUserID, accessToken, roleID); err != nil { slog.Warn("discord guild join failed", slog.Int64("user_id", conn.UserID), slog.Any("error", err), @@ -258,7 +284,13 @@ func (s *DiscordService) provision(ctx context.Context, conn *models.DiscordConn } } - if err := s.bot.GrantRole(ctx, conn.DiscordUserID); err != nil { + if roleID == "" { + conn.RoleStatus = models.DiscordStatusVerified + conn.VerifiedAt = &now + return + } + + if err := s.bot.GrantRole(ctx, conn.DiscordUserID, roleID); err != nil { s.applyGrantError(conn, err) return } @@ -299,6 +331,18 @@ func (s *DiscordService) announceSolve(ctx context.Context, userID int64, challe return } + div, err := s.divisions.GetByID(ctx, user.DivisionID) + if err != nil { + slog.Warn("discord announce: division lookup failed", slog.Int64("user_id", userID), slog.Any("error", err)) + return + } + + channelID := derefOrEmpty(div.DiscordAnnounceChannelID) + if channelID == "" { + // Announcements are disabled for this division. + return + } + team := fmt.Sprintf("%s_%s", user.DivisionName, user.TeamName) var content string if firstBlood { @@ -307,7 +351,7 @@ func (s *DiscordService) announceSolve(ctx context.Context, userID int64, challe content = fmt.Sprintf("**%s** — %s solved **%s**", team, user.Username, challengeTitle) } - if err := s.bot.Announce(ctx, content); err != nil { + if err := s.bot.Announce(ctx, channelID, content); err != nil { slog.Warn("discord announce failed", slog.Int64("user_id", userID), slog.Any("error", err)) } } diff --git a/internal/service/discord_service_test.go b/internal/service/discord_service_test.go index 90d2a08..ba4941c 100644 --- a/internal/service/discord_service_test.go +++ b/internal/service/discord_service_test.go @@ -15,25 +15,30 @@ import ( ) type fakeBot struct { - joinErr error - grantErr error - kickErr error - joined bool - granted bool - kicked bool - - mu sync.Mutex - announced []string - nickname string + joinErr error + grantErr error + kickErr error + joined bool + granted bool + kicked bool + joinRole string + grantedRole string + + mu sync.Mutex + announced []string + announceChannel string + nickname string } -func (f *fakeBot) JoinGuild(_ context.Context, _, _ string) error { +func (f *fakeBot) JoinGuild(_ context.Context, _, _, roleID string) error { f.joined = true + f.joinRole = roleID return f.joinErr } -func (f *fakeBot) GrantRole(_ context.Context, _ string) error { +func (f *fakeBot) GrantRole(_ context.Context, _, roleID string) error { f.granted = true + f.grantedRole = roleID return f.grantErr } @@ -46,13 +51,20 @@ func (f *fakeBot) GetMember(_ context.Context, _ string) (*discord.Member, error return &discord.Member{}, nil } -func (f *fakeBot) Announce(_ context.Context, content string) error { +func (f *fakeBot) Announce(_ context.Context, channelID, content string) error { f.mu.Lock() defer f.mu.Unlock() + f.announceChannel = channelID f.announced = append(f.announced, content) return nil } +func (f *fakeBot) announceChannelID() string { + f.mu.Lock() + defer f.mu.Unlock() + return f.announceChannel +} + func (f *fakeBot) SetNickname(_ context.Context, _, nickname string) error { f.mu.Lock() defer f.mu.Unlock() @@ -106,7 +118,7 @@ func discordTestConfig() config.DiscordConfig { func newDiscordServiceForTest(env serviceEnv, bot discord.BotAPI, user *discord.User) (*DiscordService, *repo.DiscordRepo) { discordRepo := repo.NewDiscordRepo(env.db) - svc := NewDiscordService(discordTestConfig(), discordRepo, env.userRepo, bot, fakeOAuth{user: user}, env.redis) + svc := NewDiscordService(discordTestConfig(), discordRepo, env.userRepo, env.divisionRepo, bot, fakeOAuth{user: user}, env.redis) return svc, discordRepo } @@ -267,7 +279,7 @@ func TestDiscordGetConnectionNilWhenAbsent(t *testing.T) { func TestDiscordDisabledReturnsError(t *testing.T) { env := setupServiceTest(t) discordRepo := repo.NewDiscordRepo(env.db) - svc := NewDiscordService(config.DiscordConfig{Enabled: false}, discordRepo, env.userRepo, &fakeBot{}, fakeOAuth{user: &discord.User{ID: "1"}}, env.redis) + svc := NewDiscordService(config.DiscordConfig{Enabled: false}, discordRepo, env.userRepo, env.divisionRepo, &fakeBot{}, fakeOAuth{user: &discord.User{ID: "1"}}, env.redis) if _, err := svc.BeginConnect(context.Background(), 1); !errors.Is(err, ErrDiscordDisabled) { t.Fatalf("got %v, want ErrDiscordDisabled", err) @@ -305,6 +317,89 @@ func TestDiscordAnnounceSolve(t *testing.T) { if strings.Contains(bot.announcedAt(1), "First Blood") || !strings.Contains(bot.announcedAt(1), "solved") { t.Errorf("normal msg = %q", bot.announcedAt(1)) } + + if bot.announceChannelID() != "222222222222222222" { + t.Errorf("expected division announce channel, got %q", bot.announceChannelID()) + } +} + +func createUserInDivision(t *testing.T, env serviceEnv, email, username string, divisionID int64) *models.User { + t.Helper() + team := &models.Team{Name: "team-" + username, DivisionID: divisionID, CreatedAt: time.Now().UTC()} + if err := env.teamRepo.Create(context.Background(), team); err != nil { + t.Fatalf("create team: %v", err) + } + + return createUserWithTeam(t, env, email, username, "pass", models.UserRole, team.ID) +} + +func TestDiscordAnnounceSkipsWithoutDivisionChannel(t *testing.T) { + env := setupServiceTest(t) + div, err := env.divisionSvc.CreateDivision(context.Background(), "NoChan", nil, nil) + if err != nil { + t.Fatalf("division: %v", err) + } + user := createUserInDivision(t, env, "nochan@example.com", "nochan", div.ID) + bot := &fakeBot{} + svc, _ := newDiscordServiceForTest(env, bot, &discord.User{ID: "1"}) + + svc.AnnounceSolve(context.Background(), user.ID, "Web-1", false) + svc.Wait() + + if bot.announcedCount() != 0 { + t.Fatalf("expected no announcement when channel unset, got %d", bot.announcedCount()) + } +} + +func TestDiscordProvisionUsesDivisionRole(t *testing.T) { + env := setupServiceTest(t) + div, err := env.divisionSvc.CreateDivision(context.Background(), "RoleDiv", sp("909090909090909090"), nil) + if err != nil { + t.Fatalf("division: %v", err) + } + user := createUserInDivision(t, env, "roldiv@example.com", "roldiv", div.ID) + bot := &fakeBot{} + svc, _ := newDiscordServiceForTest(env, bot, &discord.User{ID: "909", Username: "y"}) + + state := seedState(t, env, user.ID) + if _, err := svc.HandleCallback(context.Background(), user.ID, "code", state); err != nil { + t.Fatalf("callback: %v", err) + } + svc.Wait() + + if bot.grantedRole != "909090909090909090" { + t.Errorf("granted role = %q", bot.grantedRole) + } + + if bot.joinRole != "909090909090909090" { + t.Errorf("join role = %q", bot.joinRole) + } +} + +func TestDiscordProvisionSkipsRoleGrantWhenUnset(t *testing.T) { + env := setupServiceTest(t) + div, err := env.divisionSvc.CreateDivision(context.Background(), "NoRole", nil, nil) + if err != nil { + t.Fatalf("division: %v", err) + } + user := createUserInDivision(t, env, "norole@example.com", "norole", div.ID) + bot := &fakeBot{} + svc, _ := newDiscordServiceForTest(env, bot, &discord.User{ID: "808", Username: "x"}) + + state := seedState(t, env, user.ID) + conn, err := svc.HandleCallback(context.Background(), user.ID, "code", state) + if err != nil { + t.Fatalf("callback: %v", err) + } + svc.Wait() + + if bot.granted { + t.Errorf("role grant should be skipped when division has no role") + } + + if conn.RoleStatus != models.DiscordStatusVerified { + t.Errorf("status = %q, want verified", conn.RoleStatus) + } } func TestDiscordSyncNickname(t *testing.T) { diff --git a/internal/service/division_service.go b/internal/service/division_service.go index 9e63218..97e7784 100644 --- a/internal/service/division_service.go +++ b/internal/service/division_service.go @@ -20,18 +20,25 @@ func NewDivisionService(divisionRepo *repo.DivisionRepo) *DivisionService { return &DivisionService{divisionRepo: divisionRepo} } -func (s *DivisionService) CreateDivision(ctx context.Context, name string) (*models.Division, error) { +func (s *DivisionService) CreateDivision(ctx context.Context, name string, discordRoleID, discordAnnounceChannelID *string) (*models.Division, error) { name = strings.TrimSpace(name) + roleID := normalizeDiscordID(discordRoleID) + channelID := normalizeDiscordID(discordAnnounceChannelID) + validator := newFieldValidator() validator.Required("name", name) validator.MaxLen("name", name, nameMaxLen) + validator.Snowflake("discord_role_id", derefOrEmpty(roleID)) + validator.Snowflake("discord_announce_channel_id", derefOrEmpty(channelID)) if err := validator.Error(); err != nil { return nil, err } division := &models.Division{ - Name: name, - CreatedAt: time.Now().UTC(), + Name: name, + DiscordRoleID: roleID, + DiscordAnnounceChannelID: channelID, + CreatedAt: time.Now().UTC(), } if err := s.divisionRepo.Create(ctx, division); err != nil { @@ -44,6 +51,49 @@ func (s *DivisionService) CreateDivision(ctx context.Context, name string) (*mod return division, nil } +func (s *DivisionService) UpdateDivision(ctx context.Context, id int64, name string, discordRoleID, discordAnnounceChannelID *string) (*models.Division, error) { + name = strings.TrimSpace(name) + roleID := normalizeDiscordID(discordRoleID) + channelID := normalizeDiscordID(discordAnnounceChannelID) + + validator := newFieldValidator() + validator.PositiveID("id", id) + validator.Required("name", name) + validator.MaxLen("name", name, nameMaxLen) + validator.Snowflake("discord_role_id", derefOrEmpty(roleID)) + validator.Snowflake("discord_announce_channel_id", derefOrEmpty(channelID)) + if err := validator.Error(); err != nil { + return nil, err + } + + division, err := s.divisionRepo.GetByID(ctx, id) + if err != nil { + if errors.Is(err, repo.ErrNotFound) { + return nil, ErrNotFound + } + + return nil, fmt.Errorf("division.UpdateDivision get: %w", err) + } + + division.Name = name + division.DiscordRoleID = roleID + division.DiscordAnnounceChannelID = channelID + + if err := s.divisionRepo.Update(ctx, division); err != nil { + if db.IsUniqueViolation(err) { + return nil, NewValidationError(FieldError{Field: "name", Reason: "duplicate"}) + } + + if errors.Is(err, repo.ErrNotFound) { + return nil, ErrNotFound + } + + return nil, fmt.Errorf("division.UpdateDivision: %w", err) + } + + return division, nil +} + func (s *DivisionService) ListDivisions(ctx context.Context) ([]models.Division, error) { rows, err := s.divisionRepo.List(ctx) if err != nil { diff --git a/internal/service/division_service_test.go b/internal/service/division_service_test.go index ba09958..eb0e5e9 100644 --- a/internal/service/division_service_test.go +++ b/internal/service/division_service_test.go @@ -6,23 +6,32 @@ import ( "testing" ) +func sp(s string) *string { return &s } + func TestDivisionServiceCreateListGet(t *testing.T) { env := setupServiceTest(t) - if _, err := env.divisionSvc.CreateDivision(context.Background(), ""); err == nil { + if _, err := env.divisionSvc.CreateDivision(context.Background(), "", nil, nil); err == nil { t.Fatalf("expected validation error") } - if _, err := env.divisionSvc.CreateDivision(context.Background(), "ThisNameIsWayTooLong"); err == nil { + if _, err := env.divisionSvc.CreateDivision(context.Background(), "ThisNameIsWayTooLong", nil, nil); err == nil { t.Fatalf("expected max-length validation error") } - division, err := env.divisionSvc.CreateDivision(context.Background(), "Alpha") + division, err := env.divisionSvc.CreateDivision(context.Background(), "Alpha", sp("123456789012345678"), sp("987654321098765432")) if err != nil { t.Fatalf("create division: %v", err) } - if _, err := env.divisionSvc.CreateDivision(context.Background(), "Alpha"); err == nil { + if division.DiscordRoleID == nil || *division.DiscordRoleID != "123456789012345678" { + t.Fatalf("role id not stored: %+v", division.DiscordRoleID) + } + if division.DiscordAnnounceChannelID == nil || *division.DiscordAnnounceChannelID != "987654321098765432" { + t.Fatalf("channel id not stored: %+v", division.DiscordAnnounceChannelID) + } + + if _, err := env.divisionSvc.CreateDivision(context.Background(), "Alpha", nil, nil); err == nil { t.Fatalf("expected duplicate error") } @@ -45,6 +54,113 @@ func TestDivisionServiceCreateListGet(t *testing.T) { } } +func TestDivisionServiceCreateRejectsInvalidSnowflake(t *testing.T) { + env := setupServiceTest(t) + + if _, err := env.divisionSvc.CreateDivision(context.Background(), "Beta", sp("not-a-number"), nil); err == nil { + t.Fatalf("expected snowflake validation error for role id") + } + + if _, err := env.divisionSvc.CreateDivision(context.Background(), "Beta", nil, sp("12x34")); err == nil { + t.Fatalf("expected snowflake validation error for channel id") + } +} + +func TestDivisionServiceCreateBlankDiscordIDsStoreNil(t *testing.T) { + env := setupServiceTest(t) + + division, err := env.divisionSvc.CreateDivision(context.Background(), "Gamma", sp(" "), sp("")) + if err != nil { + t.Fatalf("create division: %v", err) + } + + if division.DiscordRoleID != nil || division.DiscordAnnounceChannelID != nil { + t.Fatalf("blank discord ids should be nil, got %+v / %+v", division.DiscordRoleID, division.DiscordAnnounceChannelID) + } +} + +func TestDivisionServiceUpdate(t *testing.T) { + env := setupServiceTest(t) + + division, err := env.divisionSvc.CreateDivision(context.Background(), "Delta", nil, nil) + if err != nil { + t.Fatalf("create division: %v", err) + } + + updated, err := env.divisionSvc.UpdateDivision(context.Background(), division.ID, "Delta2", sp("111111111111111111"), sp("222222222222222222")) + if err != nil { + t.Fatalf("update division: %v", err) + } + + if updated.Name != "Delta2" { + t.Errorf("name = %q", updated.Name) + } + + if updated.DiscordRoleID == nil || *updated.DiscordRoleID != "111111111111111111" { + t.Errorf("role id = %+v", updated.DiscordRoleID) + } + + // Persisted? + got, err := env.divisionSvc.GetDivision(context.Background(), division.ID) + if err != nil { + t.Fatalf("get division: %v", err) + } + if got.Name != "Delta2" || got.DiscordAnnounceChannelID == nil || *got.DiscordAnnounceChannelID != "222222222222222222" { + t.Fatalf("update not persisted: %+v", got) + } + + // Clearing the discord config back to nil. + cleared, err := env.divisionSvc.UpdateDivision(context.Background(), division.ID, "Delta2", nil, nil) + if err != nil { + t.Fatalf("clear update: %v", err) + } + + if cleared.DiscordRoleID != nil || cleared.DiscordAnnounceChannelID != nil { + t.Fatalf("expected cleared discord ids, got %+v / %+v", cleared.DiscordRoleID, cleared.DiscordAnnounceChannelID) + } +} + +func TestDivisionServiceUpdateValidation(t *testing.T) { + env := setupServiceTest(t) + + division, err := env.divisionSvc.CreateDivision(context.Background(), "Epsilon", nil, nil) + if err != nil { + t.Fatalf("create division: %v", err) + } + + if _, err := env.divisionSvc.UpdateDivision(context.Background(), 0, "Name", nil, nil); err == nil { + t.Fatalf("expected invalid id error") + } + + if _, err := env.divisionSvc.UpdateDivision(context.Background(), division.ID, "", nil, nil); err == nil { + t.Fatalf("expected required name error") + } + + if _, err := env.divisionSvc.UpdateDivision(context.Background(), division.ID, "Epsilon", sp("bad-id"), nil); err == nil { + t.Fatalf("expected snowflake error") + } + + if _, err := env.divisionSvc.UpdateDivision(context.Background(), 999999, "Missing", nil, nil); !errors.Is(err, ErrNotFound) { + t.Fatalf("expected not found, got %v", err) + } +} + +func TestDivisionServiceUpdateDuplicateName(t *testing.T) { + env := setupServiceTest(t) + + if _, err := env.divisionSvc.CreateDivision(context.Background(), "Zeta", nil, nil); err != nil { + t.Fatalf("create Zeta: %v", err) + } + other, err := env.divisionSvc.CreateDivision(context.Background(), "Eta", nil, nil) + if err != nil { + t.Fatalf("create Eta: %v", err) + } + + if _, err := env.divisionSvc.UpdateDivision(context.Background(), other.ID, "Zeta", nil, nil); err == nil { + t.Fatalf("expected duplicate name error") + } +} + func TestDivisionServiceGetDivisionErrors(t *testing.T) { env := setupServiceTest(t) diff --git a/internal/service/testenv_test.go b/internal/service/testenv_test.go index 2d269a5..cd478cd 100644 --- a/internal/service/testenv_test.go +++ b/internal/service/testenv_test.go @@ -232,9 +232,13 @@ func setupServiceTest(t *testing.T) serviceEnv { vmSvc: vmSvc, } + defaultRoleID := "111111111111111111" + defaultChannelID := "222222222222222222" division := &models.Division{ - Name: "Default", - CreatedAt: time.Now().UTC(), + Name: "Default", + DiscordRoleID: &defaultRoleID, + DiscordAnnounceChannelID: &defaultChannelID, + CreatedAt: time.Now().UTC(), } if err := divisionRepo.Create(context.Background(), division); err != nil { t.Fatalf("create division: %v", err) @@ -248,7 +252,7 @@ func setupServiceTest(t *testing.T) serviceEnv { func resetServiceState(t *testing.T) { t.Helper() - if _, err := serviceDB.ExecContext(context.Background(), "TRUNCATE TABLE app_configs, submissions, registration_key_uses, registration_keys, vms, challenges, users, teams, divisions RESTART IDENTITY CASCADE"); err != nil { + if _, err := serviceDB.ExecContext(context.Background(), "TRUNCATE TABLE app_configs, submissions, registration_key_uses, registration_keys, vms, challenges, discord_connections, users, teams, divisions RESTART IDENTITY CASCADE"); err != nil { t.Fatalf("truncate tables: %v", err) } diff --git a/internal/service/validation.go b/internal/service/validation.go index 12ca156..4a65930 100644 --- a/internal/service/validation.go +++ b/internal/service/validation.go @@ -50,6 +50,25 @@ func (v *fieldValidator) Email(field, value string) { } } +// Snowflake validates an optional Discord ID: empty is allowed, otherwise the +// value must be a numeric snowflake (1-32 digits). +func (v *fieldValidator) Snowflake(field, value string) { + if value == "" { + return + } + + for _, r := range value { + if r < '0' || r > '9' { + v.fields = append(v.fields, FieldError{Field: field, Reason: "must be a numeric Discord ID"}) + return + } + } + + if len(value) > 32 { + v.fields = append(v.fields, FieldError{Field: field, Reason: "must be a numeric Discord ID"}) + } +} + func (v *fieldValidator) MaxLen(field, value string, max int) { if utf8.RuneCountInString(value) > max { v.fields = append(v.fields, FieldError{Field: field, Reason: fmt.Sprintf("max length is %d characters", max)}) @@ -78,6 +97,27 @@ func normalizeTrim(value string) string { return strings.TrimSpace(value) } +func normalizeDiscordID(value *string) *string { + if value == nil { + return nil + } + + trimmed := strings.TrimSpace(*value) + if trimmed == "" { + return nil + } + + return &trimmed +} + +func derefOrEmpty(value *string) string { + if value == nil { + return "" + } + + return *value +} + func normalizeOptional(value *string) *string { if value == nil { return nil diff --git a/invite-bot/.env.example b/invite-bot/.env.example index e40722f..96c0bea 100644 --- a/invite-bot/.env.example +++ b/invite-bot/.env.example @@ -10,10 +10,10 @@ DISCORD_INTERNAL_SECRET=change-me # Discord application / bot DISCORD_BOT_TOKEN= DISCORD_GUILD_ID= -DISCORD_VERIFIED_ROLE_ID= + +# The verified role and solve-announcement channel are configured per Division +# in the backend (Admin > Divisions), not here. The backend passes the relevant +# role id / channel id to the bot on each request. # Optional DISCORD_AUDIT_REASON=External site verification - -# Channel where solve announcements are posted (empty = announcements disabled) -DISCORD_ANNOUNCE_CHANNEL_ID= diff --git a/invite-bot/src/config.ts b/invite-bot/src/config.ts index 72a3ee7..c5152ba 100644 --- a/invite-bot/src/config.ts +++ b/invite-bot/src/config.ts @@ -3,8 +3,6 @@ export interface BotConfig { internalSecret: string botToken: string guildId: string - verifiedRoleId: string - announceChannelId: string auditReason: string } @@ -45,8 +43,6 @@ export function loadConfig(): BotConfig { internalSecret: required('DISCORD_INTERNAL_SECRET'), botToken: required('DISCORD_BOT_TOKEN'), guildId: required('DISCORD_GUILD_ID'), - verifiedRoleId: required('DISCORD_VERIFIED_ROLE_ID'), - announceChannelId: optional('DISCORD_ANNOUNCE_CHANNEL_ID', ''), auditReason: optional('DISCORD_AUDIT_REASON', 'External site verification'), } } diff --git a/invite-bot/src/discord/client.ts b/invite-bot/src/discord/client.ts index 5892ad8..f0e880c 100644 --- a/invite-bot/src/discord/client.ts +++ b/invite-bot/src/discord/client.ts @@ -43,22 +43,25 @@ export class DiscordBot { return this.guild } - async joinGuild(userId: string, accessToken: string): Promise { + async joinGuild(userId: string, accessToken: string, roleId: string): Promise { const guild = this.requireGuild() try { await guild.members.add(userId, { accessToken, - roles: [this.cfg.verifiedRoleId], + roles: roleId ? [roleId] : [], }) } catch (err) { throw mapDiscordError(err) } } - async grantRole(userId: string): Promise { + async grantRole(userId: string, roleId: string): Promise { + if (!roleId) { + return + } const member = await this.fetchMember(userId) try { - await member.roles.add(this.cfg.verifiedRoleId, this.cfg.auditReason) + await member.roles.add(roleId, this.cfg.auditReason) } catch (err) { throw mapDiscordError(err) } @@ -76,12 +79,12 @@ export class DiscordBot { } } - async memberStatus(userId: string): Promise { + async memberStatus(userId: string, roleId: string): Promise { try { const member = await this.fetchMember(userId) return { in_guild: true, - has_role: member.roles.cache.has(this.cfg.verifiedRoleId), + has_role: roleId ? member.roles.cache.has(roleId) : false, } } catch (err) { if (err instanceof BotError && err.code === 'NOT_IN_GUILD') { @@ -91,12 +94,13 @@ export class DiscordBot { } } - async announce(content: string): Promise { - if (!this.cfg.announceChannelId) { + async announce(channelId: string, content: string): Promise { + if (!channelId) { return } + try { - const channel = await this.client.channels.fetch(this.cfg.announceChannelId) + const channel = await this.client.channels.fetch(channelId) if (!channel || !channel.isTextBased() || !('send' in channel)) { throw new BotError(400, 'INVALID', 'announce channel is not text-based') } diff --git a/invite-bot/src/http/server.ts b/invite-bot/src/http/server.ts index 3084b99..325037f 100644 --- a/invite-bot/src/http/server.ts +++ b/invite-bot/src/http/server.ts @@ -78,7 +78,8 @@ export function buildServer(cfg: BotConfig, bot: DiscordBot): Express { return } - await bot.joinGuild(pathUserId(req), accessToken) + const roleId = (req.body?.role_id as string | undefined)?.trim() ?? '' + await bot.joinGuild(pathUserId(req), accessToken, roleId) res.status(204).end() }), ) @@ -86,7 +87,8 @@ export function buildServer(cfg: BotConfig, bot: DiscordBot): Express { internal.put( '/guild/members/:userId/role', asyncRoute(async (req, res) => { - await bot.grantRole(pathUserId(req)) + const roleId = (req.body?.role_id as string | undefined)?.trim() ?? '' + await bot.grantRole(pathUserId(req), roleId) res.status(204).end() }), ) @@ -102,7 +104,9 @@ export function buildServer(cfg: BotConfig, bot: DiscordBot): Express { internal.get( '/guild/members/:userId', asyncRoute(async (req, res) => { - const status = await bot.memberStatus(pathUserId(req)) + const rawRole = req.query.role_id + const roleId = typeof rawRole === 'string' ? rawRole.trim() : '' + const status = await bot.memberStatus(pathUserId(req), roleId) res.json(status) }), ) @@ -115,7 +119,8 @@ export function buildServer(cfg: BotConfig, bot: DiscordBot): Express { res.status(400).json({ error: 'content required', code: 'INVALID' }) return } - await bot.announce(content) + const channelId = (req.body?.channel_id as string | undefined)?.trim() ?? '' + await bot.announce(channelId, content) res.status(204).end() }), ) diff --git a/migrations/2026-07-04/001_add_division_discord_config.sql b/migrations/2026-07-04/001_add_division_discord_config.sql new file mode 100644 index 0000000..d39008b --- /dev/null +++ b/migrations/2026-07-04/001_add_division_discord_config.sql @@ -0,0 +1,7 @@ +BEGIN; + +ALTER TABLE divisions + ADD COLUMN IF NOT EXISTS discord_role_id VARCHAR(32) NULL, + ADD COLUMN IF NOT EXISTS discord_announce_channel_id VARCHAR(32) NULL; + +COMMIT; diff --git a/migrations/2026-07-04/999_rollback.sql b/migrations/2026-07-04/999_rollback.sql new file mode 100644 index 0000000..7c7ae76 --- /dev/null +++ b/migrations/2026-07-04/999_rollback.sql @@ -0,0 +1,7 @@ +BEGIN; + +ALTER TABLE divisions + DROP COLUMN IF EXISTS discord_role_id, + DROP COLUMN IF EXISTS discord_announce_channel_id; + +COMMIT; From abbcc603ec905f06f1918d5620bc7cbdb70bcdd4 Mon Sep 17 00:00:00 2001 From: Kim Jun Young Date: Sat, 4 Jul 2026 19:33:47 +0900 Subject: [PATCH 2/2] chore: applied suggestion --- internal/service/discord_service.go | 13 ++++++------- internal/service/division_service.go | 6 ++++++ internal/service/division_service_test.go | 22 ++++++++++++++++++++++ 3 files changed, 34 insertions(+), 7 deletions(-) diff --git a/internal/service/discord_service.go b/internal/service/discord_service.go index 76a6208..8454405 100644 --- a/internal/service/discord_service.go +++ b/internal/service/discord_service.go @@ -262,19 +262,18 @@ func (s *DiscordService) provision(ctx context.Context, conn *models.DiscordConn conn.UpdatedAt = now conn.LastError = nil - // Resolve the division-scoped role to grant. An empty role means this - // division does not assign a Discord role, so linking is considered - // verified without a role grant. - roleID := "" - if div, err := s.divisionForUser(ctx, conn.UserID); err != nil { + div, err := s.divisionForUser(ctx, conn.UserID) + if err != nil { slog.Warn("discord: division lookup failed", slog.Int64("user_id", conn.UserID), slog.Any("error", err), ) - } else { - roleID = derefOrEmpty(div.DiscordRoleID) + s.applyGrantError(conn, err) + return } + roleID := derefOrEmpty(div.DiscordRoleID) + if s.cfg.AutoJoin && accessToken != "" { if err := s.bot.JoinGuild(ctx, conn.DiscordUserID, accessToken, roleID); err != nil { slog.Warn("discord guild join failed", diff --git a/internal/service/division_service.go b/internal/service/division_service.go index 97e7784..fa3fa33 100644 --- a/internal/service/division_service.go +++ b/internal/service/division_service.go @@ -12,6 +12,8 @@ import ( "smctf/internal/repo" ) +const reservedAdminDivisionName = "Admin" + type DivisionService struct { divisionRepo *repo.DivisionRepo } @@ -75,6 +77,10 @@ func (s *DivisionService) UpdateDivision(ctx context.Context, id int64, name str return nil, fmt.Errorf("division.UpdateDivision get: %w", err) } + if strings.EqualFold(division.Name, reservedAdminDivisionName) && !strings.EqualFold(name, reservedAdminDivisionName) { + return nil, NewValidationError(FieldError{Field: "name", Reason: "reserved"}) + } + division.Name = name division.DiscordRoleID = roleID division.DiscordAnnounceChannelID = channelID diff --git a/internal/service/division_service_test.go b/internal/service/division_service_test.go index eb0e5e9..adc1ce4 100644 --- a/internal/service/division_service_test.go +++ b/internal/service/division_service_test.go @@ -161,6 +161,28 @@ func TestDivisionServiceUpdateDuplicateName(t *testing.T) { } } +func TestDivisionServiceUpdateCannotRenameReservedAdmin(t *testing.T) { + env := setupServiceTest(t) + + admin, err := env.divisionSvc.CreateDivision(context.Background(), "Admin", nil, nil) + if err != nil { + t.Fatalf("create admin division: %v", err) + } + + if _, err := env.divisionSvc.UpdateDivision(context.Background(), admin.ID, "Public", nil, nil); err == nil { + t.Fatalf("expected rename of reserved admin division to be rejected") + } + + updated, err := env.divisionSvc.UpdateDivision(context.Background(), admin.ID, "Admin", sp("123456789012345678"), nil) + if err != nil { + t.Fatalf("expected discord-only update to succeed, got %v", err) + } + + if updated.DiscordRoleID == nil || *updated.DiscordRoleID != "123456789012345678" { + t.Fatalf("role id not updated: %+v", updated.DiscordRoleID) + } +} + func TestDivisionServiceGetDivisionErrors(t *testing.T) { env := setupServiceTest(t)