Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 13 additions & 4 deletions .env.example
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,20 @@ OIDC_CLIENT_ID=<your-client-id>
OIDC_CLIENT_SECRET=<your-client-secret>
CMS_REBUILD_TAXONOMY_COUNTS_ON_STARTUP=false

# Slack app signing secret, from the Slack app's Basic Information page. It is
# the only thing authorizing the Approve/Reject buttons on classified
# notifications; without it POST /v1/integrations/slack/classifieds refuses
# every request.
# Slack classified moderation. Both halves are needed for the loop to work and
# the CMS warns at startup if only one is set.
#
# The incoming-webhook URL the submission notification is posted to. The channel
# is fixed by the webhook. Without it nothing is ever posted, so there is no
# message for anyone to click.
SLACK_WEBHOOK_URL=
# The Slack app's signing secret, from its Basic Information page. It is the
# only thing authorizing the Approve/Reject buttons; without it
# POST /v1/integrations/slack/classifieds refuses every request.
SLACK_SIGNING_SECRET=<your-slack-signing-secret>
# Optional: linked from the notification so a moderator can open the CMS queue
# instead of deciding in Slack.
# SLACK_CLASSIFIEDS_QUEUE_URL=https://cms.thetriangle.org/classifieds

# Delta production variables live in deploy/cms.env.example. Do not put
# production server addresses, DB passwords, OIDC secrets, runner tokens,
Expand Down
7 changes: 7 additions & 0 deletions deploy/cms.env.example
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,14 @@ FRONTEND_ORIGIN=
OIDC_REDIRECT_URI=
CMS_SESSION_TTL_SECONDS=
CMS_REBUILD_TAXONOMY_COUNTS_ON_STARTUP=
# Slack classified moderation. The webhook posts the notification; the signing
# secret authorizes the Approve/Reject clicks that come back. Setting only one
# of the two leaves the loop broken -- the backend warns at startup which half
# is missing. SLACK_CLASSIFIEDS_QUEUE_URL is optional and only adds a link back
# to the CMS queue.
SLACK_WEBHOOK_URL=
SLACK_SIGNING_SECRET=
SLACK_CLASSIFIEDS_QUEUE_URL=
AKISMET_API_KEY=
AKISMET_BLOG_URL=
MEDIA_HOST_PATH=
Expand Down
2 changes: 2 additions & 0 deletions deploy/compose.cms.yml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,9 @@ x-backend-base: &backend-base
OIDC_REDIRECT_URI: ${OIDC_REDIRECT_URI:?OIDC_REDIRECT_URI is required}
CMS_SESSION_TTL_SECONDS: ${CMS_SESSION_TTL_SECONDS:-604800}
CMS_REBUILD_TAXONOMY_COUNTS_ON_STARTUP: ${CMS_REBUILD_TAXONOMY_COUNTS_ON_STARTUP:-false}
SLACK_WEBHOOK_URL: ${SLACK_WEBHOOK_URL:-}
SLACK_SIGNING_SECRET: ${SLACK_SIGNING_SECRET:-}
SLACK_CLASSIFIEDS_QUEUE_URL: ${SLACK_CLASSIFIEDS_QUEUE_URL:-}
AKISMET_API_KEY: ${AKISMET_API_KEY:-}
AKISMET_BLOG_URL: ${AKISMET_BLOG_URL:-}
# Media: legacy WP uploads migrated to CephFS. The upload endpoint writes new
Expand Down
2 changes: 2 additions & 0 deletions docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,9 @@ services:
TLS_KEY_FILE: /app/certs/localhost.key
OIDC_ISSUER_URL: ${OIDC_ISSUER_URL:-}
OIDC_CLIENT_ID: ${OIDC_CLIENT_ID:-}
SLACK_WEBHOOK_URL: ${SLACK_WEBHOOK_URL:-}
SLACK_SIGNING_SECRET: ${SLACK_SIGNING_SECRET:-}
SLACK_CLASSIFIEDS_QUEUE_URL: ${SLACK_CLASSIFIEDS_QUEUE_URL:-}
CMS_REBUILD_TAXONOMY_COUNTS_ON_STARTUP: ${CMS_REBUILD_TAXONOMY_COUNTS_ON_STARTUP:-false}
depends_on:
mariadb:
Expand Down
5 changes: 3 additions & 2 deletions frontend/src/pages/classifiedsView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -142,8 +142,9 @@ export default function ClassifiedsView() {

{!slackConfigured && (
<div className="rounded-lg border border-amber-500/40 bg-amber-500/10 px-4 py-3 text-sm text-amber-800 dark:text-amber-300">
Slack approvals are unavailable — the server has no <code>SLACK_SIGNING_SECRET</code>, so
the Approve/Reject buttons on classified notifications will not work. Moderate here
Slack approvals are unavailable — the server is missing{" "}
<code>SLACK_WEBHOOK_URL</code>, <code>SLACK_SIGNING_SECRET</code>, or both, so submissions
are either never posted to Slack or the Approve/Reject buttons are refused. Moderate here
instead; nothing is lost either way.
</div>
)}
Expand Down
49 changes: 46 additions & 3 deletions server/internal/handlers/classifieds.go
Original file line number Diff line number Diff line change
@@ -1,16 +1,20 @@
package handlers

import (
"context"
"database/sql"
"encoding/json"
"log/slog"
"net/http"
"strconv"
"strings"
"time"

"server/internal/activity"
db "server/internal/database"
"server/internal/middleware"
"server/internal/models"
"server/internal/slack"
)

const (
Expand Down Expand Up @@ -40,10 +44,18 @@ func GetClassifieds(conn *sql.DB) http.Handler {
})
}

// How long the Slack post gets once the reader's response has already been
// written. It is not on the request's critical path, but it must not outlive
// the process's patience either.
const classifiedNotifyTimeout = 10 * time.Second

// PostClassified accepts a submission from the public form. It always lands as
// pending: nothing a reader posts reaches the site without a moderator, whether
// that moderator clicks in the CMS or in Slack.
//
// notifier may be nil, which means Slack is not configured: the submission is
// stored and waits in the CMS queue instead.
//
// @Summary Submit a classified
// @Tags classifieds
// @Accept json
Expand All @@ -53,7 +65,7 @@ func GetClassifieds(conn *sql.DB) http.Handler {
// @Failure 400 {object} models.ErrorResponse
// @Failure 500 {object} models.ErrorResponse
// @Router /v1/classifieds [post]
func PostClassified(conn *sql.DB) http.Handler {
func PostClassified(conn *sql.DB, notifier slack.Notifier) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
var body models.ClassifiedSubmitRequest
if err := json.NewDecoder(r.Body).Decode(&body); err != nil {
Expand Down Expand Up @@ -93,10 +105,41 @@ func PostClassified(conn *sql.DB) http.Handler {
writeError(w, http.StatusInternalServerError, err.Error())
return
}
notifyClassifiedSubmitted(notifier, created)
writeJSON(w, http.StatusCreated, created)
})
}

// notifyClassifiedSubmitted posts the submission to Slack in the background.
//
// It deliberately cannot fail the request. The row is already in the CMS queue,
// so a dead webhook costs a notification, not a reader's submission — and the
// reader must not be shown a 500 for a moderation channel they have no idea
// exists. The context is detached from the request because the response is
// written immediately after this returns, which would otherwise cancel the post
// mid-flight.
func notifyClassifiedSubmitted(notifier slack.Notifier, c models.Classified) {
if notifier == nil {
return
}
go func() {
ctx, cancel := context.WithTimeout(context.Background(), classifiedNotifyTimeout)
defer cancel()

if err := notifier.NotifyClassified(ctx, slack.Classified{
ID: c.ID,
Name: c.Name,
Email: c.Email,
Label: c.Label,
Message: c.Message,
EndDate: c.EndDate,
}); err != nil {
slog.Error("could not post classified to Slack; it is still in the CMS moderation queue",
"classified_id", c.ID, "error", err)
}
}()
}

// GetClassifiedsManage is the moderation queue's listing: every status, with
// per-status counts for the filter tabs.
//
Expand All @@ -111,7 +154,7 @@ func PostClassified(conn *sql.DB) http.Handler {
// @Failure 500 {object} models.ErrorResponse
// @Security BearerAuth
// @Router /v1/classifieds/manage [get]
func GetClassifiedsManage(conn *sql.DB) http.Handler {
func GetClassifiedsManage(conn *sql.DB, notifier slack.Notifier) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
status := strings.ToLower(strings.TrimSpace(r.URL.Query().Get("status")))
if status != "" && status != "all" && !db.ValidClassifiedStatuses[status] {
Expand All @@ -138,7 +181,7 @@ func GetClassifiedsManage(conn *sql.DB) http.Handler {
Classifieds: items,
Pagination: paginationResponse(page, limit, offset, offset+len(items) < totalCount, totalCount),
Counts: counts,
SlackConfigured: SlackInteractivityConfigured(),
SlackConfigured: SlackModerationConfigured(notifier),
})
})
}
Expand Down
21 changes: 16 additions & 5 deletions server/internal/handlers/slack.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"server/internal/activity"
db "server/internal/database"
"server/internal/models"
"server/internal/slack"
)

// Slack's interactivity contract: the request is signed with the app's signing
Expand Down Expand Up @@ -103,9 +104,12 @@ func PostSlackClassifiedAction(conn *sql.DB) http.Handler {
action := payload.Actions[0]
var status string
switch strings.ToLower(strings.TrimSpace(action.ActionID)) {
case "approved", "approve":
// slack.ActionApprove/ActionReject are what the outgoing message sends;
// the past-tense spellings are messages posted before this server owned
// the notification, still sitting in channel history.
case slack.ActionApprove, "approved":
status = models.ClassifiedStatusApproved
case "rejected", "reject":
case slack.ActionReject, "rejected":
status = models.ClassifiedStatusRejected
default:
writeError(w, http.StatusBadRequest, "unknown action")
Expand Down Expand Up @@ -228,13 +232,20 @@ func slackVerificationFailure(secret, timestamp, body, signature string, now tim
}

// SlackInteractivityConfigured reports whether the signing secret is present,
// i.e. whether the Approve/Reject buttons can work at all. The CMS surfaces
// this so the moderation queue does not promise Slack approvals that would
// silently time out in the channel.
// i.e. whether a button click could be verified if one ever arrived.
func SlackInteractivityConfigured() bool {
return strings.TrimSpace(os.Getenv("SLACK_SIGNING_SECRET")) != ""
}

// SlackModerationConfigured reports whether the whole moderation loop works:
// a message goes out (notifier) and the click that comes back can be verified
// (signing secret). Both halves are required, and the signing secret alone is
// not enough — with no webhook there is nothing in the channel to click, and
// the queue would promise moderators an approval path that does not exist.
func SlackModerationConfigured(notifier slack.Notifier) bool {
return notifier != nil && SlackInteractivityConfigured()
}

// A rejected request is worth knowing about — this is the one public write
// path that no session guards — but it is also trivially floodable, so the
// warnings are throttled and the ones dropped in between are counted into the
Expand Down
33 changes: 33 additions & 0 deletions server/internal/handlers/slack_test.go
Original file line number Diff line number Diff line change
@@ -1,13 +1,16 @@
package handlers

import (
"context"
"crypto/hmac"
"crypto/sha256"
"encoding/hex"
"io"
"net/http/httptest"
"testing"
"time"

"server/internal/slack"
)

const testSigningSecret = "8f742231b10e8888abcd99yyyzzz85a5"
Expand Down Expand Up @@ -153,6 +156,36 @@ func TestPostSlackClassifiedAction_RefusesWhenUnconfigured(t *testing.T) {
}
}

type stubNotifier struct{}

func (stubNotifier) NotifyClassified(context.Context, slack.Classified) error { return nil }

// The queue UI tells moderators they can approve from Slack based on this. A
// signing secret with no webhook posts nothing, so there is no notification to
// approve from — reporting "configured" there is the bug this guards.
func TestSlackModerationConfigured_RequiresBothHalves(t *testing.T) {
cases := []struct {
name string
notifier slack.Notifier
secret string
want bool
}{
{"webhook and secret", stubNotifier{}, testSigningSecret, true},
{"secret but no webhook", nil, testSigningSecret, false},
{"webhook but no secret", stubNotifier{}, "", false},
{"neither", nil, "", false},
}

for _, c := range cases {
t.Run(c.name, func(t *testing.T) {
t.Setenv("SLACK_SIGNING_SECRET", c.secret)
if got := SlackModerationConfigured(c.notifier); got != c.want {
t.Fatalf("expected %v, got %v", c.want, got)
}
})
}
}

// An unsigned request must be turned away before the body is parsed or the
// database is touched — the nil *sql.DB here would panic if it were not.
func TestPostSlackClassifiedAction_RejectsUnsignedRequest(t *testing.T) {
Expand Down
7 changes: 4 additions & 3 deletions server/internal/routes/routes.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,15 @@ import (
"server/internal/auth"
"server/internal/handlers"
"server/internal/middleware"
"server/internal/slack"
"time"

"github.com/coreos/go-oidc/v3/oidc"
"github.com/prometheus/client_golang/prometheus/promhttp"
httpSwagger "github.com/swaggo/http-swagger"
)

func Register(mux *http.ServeMux, conn *sql.DB, verifier *oidc.IDTokenVerifier, oidcCfg auth.OIDCConfig, spamChecker akismet.Checker, queryEmbedder handlers.QueryEmbedder) {
func Register(mux *http.ServeMux, conn *sql.DB, verifier *oidc.IDTokenVerifier, oidcCfg auth.OIDCConfig, spamChecker akismet.Checker, slackNotifier slack.Notifier, queryEmbedder handlers.QueryEmbedder) {
mux.Handle("/swagger/", httpSwagger.WrapHandler)
// Prometheus scrape target. Unauthenticated by design, and safe only
// because Nginx proxies just /v1 and /swagger: nothing routes /metrics in
Expand Down Expand Up @@ -69,8 +70,8 @@ func Register(mux *http.ServeMux, conn *sql.DB, verifier *oidc.IDTokenVerifier,
// session — hence no authMW on that route. "manage" is a literal segment,
// so Go's mux prefers it over /v1/classifieds/{id}.
mux.Handle("GET /v1/classifieds", handlers.GetClassifieds(conn))
mux.Handle("POST /v1/classifieds", middleware.RateLimitByIP(5, time.Minute)(handlers.PostClassified(conn)))
mux.Handle("GET /v1/classifieds/manage", authMW(handlers.GetClassifiedsManage(conn)))
mux.Handle("POST /v1/classifieds", middleware.RateLimitByIP(5, time.Minute)(handlers.PostClassified(conn, slackNotifier)))
mux.Handle("GET /v1/classifieds/manage", authMW(handlers.GetClassifiedsManage(conn, slackNotifier)))
mux.Handle("PATCH /v1/classifieds/{id}", authMW(handlers.PatchClassified(conn)))
mux.Handle("DELETE /v1/classifieds/{id}", authMW(adminOnly(handlers.DeleteClassified(conn))))
mux.Handle("POST /v1/integrations/slack/classifieds", middleware.RateLimitByIP(30, time.Minute)(handlers.PostSlackClassifiedAction(conn)))
Expand Down
8 changes: 4 additions & 4 deletions server/internal/routes/routes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ func TestRegister_ReadEndpointsPublicWithVerifier(t *testing.T) {
defer conn.Close()

mux := http.NewServeMux()
Register(mux, conn, verifier, auth.OIDCConfig{}, nil, nil)
Register(mux, conn, verifier, auth.OIDCConfig{}, nil, nil, nil)

public := []string{
"/v1/articles",
Expand Down Expand Up @@ -78,7 +78,7 @@ func TestRegister_SettingsWriteEndpointsGated(t *testing.T) {
})

mux := http.NewServeMux()
Register(mux, nil, verifier, auth.OIDCConfig{}, nil, nil)
Register(mux, nil, verifier, auth.OIDCConfig{}, nil, nil, nil)

tests := []struct {
method string
Expand Down Expand Up @@ -106,7 +106,7 @@ func TestRegister_SettingsWriteEndpointsGated(t *testing.T) {

func TestRegister_PublicRoute(t *testing.T) {
mux := http.NewServeMux()
Register(mux, nil, nil, auth.OIDCConfig{}, nil, nil)
Register(mux, nil, nil, auth.OIDCConfig{}, nil, nil, nil)

tests := []struct {
name string
Expand Down Expand Up @@ -172,7 +172,7 @@ func TestRegister_MediaEndpointsGated(t *testing.T) {
})

mux := http.NewServeMux()
Register(mux, nil, verifier, auth.OIDCConfig{}, nil, nil)
Register(mux, nil, verifier, auth.OIDCConfig{}, nil, nil, nil)

tests := []struct {
method string
Expand Down
Loading
Loading