Stage 3, step 2. Reviewed by Fable (read-only), fixed by Sonnet with a
failing test before each fix, checked by Opus. No Critical or High
finding: route authorization, tenant isolation by id, the /server/
subtree guard, output encoding and the origin check were found sound.
Fixed
- M1: a session was keyed by user name and had no link to the user. A
deleted or renamed user's cookie stayed alive (renewed on every hit,
before the user was looked up) and became a session of whoever was
later created under that name, with that account's role. The
baseline now has sessions.user_id REFERENCES users(id) ON DELETE
CASCADE; the principal is resolved by id and a session is renewed
only after its user was found. RenameSession is gone.
- M2: any user's password change deleted every other user's sessions.
It ends that user's other sessions only.
- L1: a password set on the user form ends that user's sessions (an
administrator editing their own account keeps the current one).
- L2: store, saslpasswd2 and Postfix reload errors were shown verbatim
on the application form, the inbound upstream / recipients forms and
the limit recalculation - pages a domain administrator reaches.
Validation errors (app.ValidationError, inbound.ValidationError,
store.ErrNotAutoRateLimit) keep their text; anything else is logged
and shown as "Could not save ... Check the logs and try again."
- L3: store.ListDMARCReports treated an empty domain list as "all". It
takes a scope now (AllDomains or a list; an empty list is no rows),
and the DMARC hub is open to the domain role as the plan says: the
reports of the assigned domains, without the instance-wide ingest
box. The menu entry was already shown to them and led to a 404.
- I1: /static/ answered with a listing of the embedded assets. A path
that is not a known asset is a 404.
Consequences
- The global DMARC hub lists reports of domains that were deleted
since, too.
- dev/migrate1x (not in the tree) no longer carries sessions over:
everyone signs in again after the transfer.
- docs/schema-migrations.md: the sessions row.
Recorded, not changed - for the owner:
- L4: the report-address select names the followed user and their
address to every administrator of the domain, by default the global
administrator's login name. The plan asks for it.
- L5: a delegated inbound upstream may be any host and port, loopback
and private ranges included - within the trusted-operator model; to
be listed among the accepted risks in security.md.
- L6: Account's profile is saved without the password; becomes a
requirement once password-reset mails to users.email.
- Still echoing an error's text: backup and domain import (global
only), the domain form's validation messages.
Not checked by the review: anything needing the container (e2e, real
Postfix / DNS), CSP behaviour in a live browser.
go test -count=1 ./internal/web/... ./internal/store/... ./internal/app/...
./internal/inbound/... ./internal/dmarc/... ./cmd/panel
ok (all eleven packages; dev/migrate1x also ok)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
232 lines
6.8 KiB
Go
232 lines
6.8 KiB
Go
package handlers
|
||
|
||
import (
|
||
"errors"
|
||
"fmt"
|
||
"net"
|
||
"net/http"
|
||
"strconv"
|
||
"strings"
|
||
|
||
"github.com/mixeme/selfpost/internal/store"
|
||
)
|
||
|
||
const defaultRateLimitWindowSeconds = 3600
|
||
|
||
type rateLimitInput struct {
|
||
clear bool
|
||
mode string
|
||
ips []string
|
||
maxMessages int
|
||
windowSeconds int
|
||
autoMultiplier float64
|
||
}
|
||
|
||
func (h *Handlers) l1Messages() int {
|
||
if h.cfg.RateLimitMessagesPerIP > 0 {
|
||
return h.cfg.RateLimitMessagesPerIP
|
||
}
|
||
return 100
|
||
}
|
||
|
||
func (h *Handlers) l1Window() int {
|
||
if h.cfg.RateLimitWindowSeconds > 0 {
|
||
return h.cfg.RateLimitWindowSeconds
|
||
}
|
||
return defaultRateLimitWindowSeconds
|
||
}
|
||
|
||
func parseAutoMultiplier(raw string) (float64, error) {
|
||
raw = strings.TrimSpace(raw)
|
||
if raw == "" {
|
||
return store.DefaultAutoMultiplier, nil
|
||
}
|
||
v, err := strconv.ParseFloat(raw, 64)
|
||
if err != nil {
|
||
return 0, fmt.Errorf("enter a valid multiplier (%.1f–%.1f)", store.MinAutoMultiplier, store.MaxAutoMultiplier)
|
||
}
|
||
if v < store.MinAutoMultiplier || v > store.MaxAutoMultiplier {
|
||
return 0, fmt.Errorf("multiplier must be between %.1f and %.1f", store.MinAutoMultiplier, store.MaxAutoMultiplier)
|
||
}
|
||
return v, nil
|
||
}
|
||
|
||
func parseRateLimitMode(raw string) (string, error) {
|
||
raw = strings.TrimSpace(raw)
|
||
if raw == "" {
|
||
return store.RateLimitModeManual, nil
|
||
}
|
||
if raw != store.RateLimitModeManual && raw != store.RateLimitModeAuto {
|
||
return "", fmt.Errorf("choose manual or auto mode")
|
||
}
|
||
return raw, nil
|
||
}
|
||
|
||
func parseRateLimitForm(r *http.Request, l1Max int) (rateLimitInput, error) {
|
||
if err := r.ParseForm(); err != nil {
|
||
return rateLimitInput{}, fmt.Errorf("invalid form submission")
|
||
}
|
||
if r.PostFormValue("clear") != "" {
|
||
return rateLimitInput{clear: true}, nil
|
||
}
|
||
mode, err := parseRateLimitMode(r.PostFormValue("mode"))
|
||
if err != nil {
|
||
return rateLimitInput{}, err
|
||
}
|
||
if mode == store.RateLimitModeAuto {
|
||
mult, err := parseAutoMultiplier(r.PostFormValue("auto_multiplier"))
|
||
if err != nil {
|
||
return rateLimitInput{}, err
|
||
}
|
||
return rateLimitInput{mode: mode, autoMultiplier: mult}, nil
|
||
}
|
||
|
||
rawMax := strings.TrimSpace(r.PostFormValue("max_messages"))
|
||
if rawMax == "" {
|
||
return rateLimitInput{clear: true}, nil
|
||
}
|
||
maxMessages, err := parsePositiveInt(rawMax, 0)
|
||
if err != nil || maxMessages <= 0 {
|
||
return rateLimitInput{}, fmt.Errorf("enter a message limit greater than zero")
|
||
}
|
||
if maxMessages > l1Max {
|
||
return rateLimitInput{}, fmt.Errorf("message limit cannot exceed the level-1 backstop (%d)", l1Max)
|
||
}
|
||
windowSeconds, err := parsePositiveInt(r.PostFormValue("window_seconds"), defaultRateLimitWindowSeconds)
|
||
if err != nil || windowSeconds <= 0 {
|
||
return rateLimitInput{}, fmt.Errorf("enter a time window greater than zero seconds")
|
||
}
|
||
return rateLimitInput{mode: store.RateLimitModeManual, maxMessages: maxMessages, windowSeconds: windowSeconds}, nil
|
||
}
|
||
|
||
func parseAppAuthIPsForm(r *http.Request) (bool, []string, error) {
|
||
if err := r.ParseForm(); err != nil {
|
||
return false, nil, fmt.Errorf("invalid form submission")
|
||
}
|
||
restrict := r.PostFormValue("auth_ip_restrict") != ""
|
||
if !restrict {
|
||
return false, nil, nil
|
||
}
|
||
ips, err := parseIPList(r.PostFormValue("auth_allowed_ips"))
|
||
if err != nil {
|
||
return false, nil, err
|
||
}
|
||
if len(ips) == 0 {
|
||
return false, nil, fmt.Errorf("enter at least one client IP when the allow-list is enabled")
|
||
}
|
||
return true, ips, nil
|
||
}
|
||
|
||
func parseIPList(raw string) ([]string, error) {
|
||
fields := strings.FieldsFunc(raw, func(r rune) bool {
|
||
return r == '\n' || r == '\r' || r == ',' || r == ' ' || r == '\t' || r == ';'
|
||
})
|
||
var out []string
|
||
seen := make(map[string]bool)
|
||
for _, f := range fields {
|
||
ip := net.ParseIP(f)
|
||
if ip == nil {
|
||
return nil, fmt.Errorf("%q is not a valid IP address", f)
|
||
}
|
||
c := ip.String()
|
||
if !seen[c] {
|
||
seen[c] = true
|
||
out = append(out, c)
|
||
}
|
||
}
|
||
return out, nil
|
||
}
|
||
|
||
func parsePositiveInt(raw string, def int) (int, error) {
|
||
raw = strings.TrimSpace(raw)
|
||
if raw == "" {
|
||
return def, nil
|
||
}
|
||
return strconv.Atoi(raw)
|
||
}
|
||
|
||
func (h *Handlers) HandleDomainRateLimit(w http.ResponseWriter, r *http.Request) {
|
||
d, ok := h.lookupDomain(w, r)
|
||
if !ok {
|
||
return
|
||
}
|
||
in, err := parseRateLimitForm(r, h.l1Messages())
|
||
if err != nil {
|
||
h.renderDomainSettings(w, r, http.StatusBadRequest, d, settingsView{Err: err.Error()})
|
||
return
|
||
}
|
||
if err := h.applyRateLimit(in, store.RateLimitScopeDomain, d.ID, h.domains); err != nil {
|
||
logf("panel: domain %d: save rate limit: %v", d.ID, err)
|
||
http.Error(w, "internal error", http.StatusInternalServerError)
|
||
return
|
||
}
|
||
http.Redirect(w, r, fmt.Sprintf("/outbound/domains/%d?ratelimit=1", d.ID), http.StatusSeeOther)
|
||
}
|
||
|
||
func (h *Handlers) HandleDomainRateLimitRecalc(w http.ResponseWriter, r *http.Request) {
|
||
d, ok := h.lookupDomain(w, r)
|
||
if !ok {
|
||
return
|
||
}
|
||
if err := h.recalcRateLimit(store.RateLimitScopeDomain, d.ID); err != nil {
|
||
h.renderDomainSettings(w, r, http.StatusBadRequest, d, settingsView{Err: recalcErrorMessage(err, "domain", d.ID)})
|
||
return
|
||
}
|
||
http.Redirect(w, r, fmt.Sprintf("/outbound/domains/%d?recalculated=1", d.ID), http.StatusSeeOther)
|
||
}
|
||
|
||
func (h *Handlers) HandleAppRateLimitRecalc(w http.ResponseWriter, r *http.Request) {
|
||
a, d, ok := h.lookupDomainApplication(w, r)
|
||
if !ok {
|
||
return
|
||
}
|
||
if err := h.recalcRateLimit(store.RateLimitScopeApp, a.ID); err != nil {
|
||
h.renderApplicationForm(w, r, http.StatusBadRequest, d, &a, nil,
|
||
fmt.Sprintf("%s: %s", a.Login, recalcErrorMessage(err, "application", a.ID)))
|
||
return
|
||
}
|
||
http.Redirect(w, r, fmt.Sprintf("/outbound/domains/%d?recalculated=1", a.DomainID), http.StatusSeeOther)
|
||
}
|
||
|
||
// recalcErrorMessage is what the page says about a recalculation that did not
|
||
// happen. A limit that is not automatic has nothing to recalculate, and the
|
||
// administrator is told so; any other failure goes to the log.
|
||
func recalcErrorMessage(err error, what string, id int64) string {
|
||
if errors.Is(err, store.ErrNotAutoRateLimit) {
|
||
return err.Error()
|
||
}
|
||
logf("panel: %s %d: recalc rate limit: %v", what, id, err)
|
||
return "Could not recalculate the limit. Check the logs and try again."
|
||
}
|
||
|
||
func (h *Handlers) recalcRateLimit(scope string, refID int64) error {
|
||
return h.store.RecalcAutoRateLimit(scope, refID, h.sendLogRetentionDays(), h.l1Messages(), h.l1Window())
|
||
}
|
||
|
||
type rateLimitSaver interface {
|
||
ClearRateLimit(id int64) error
|
||
SaveRateLimit(id int64, rl store.RateLimit) error
|
||
}
|
||
|
||
func (h *Handlers) applyRateLimit(in rateLimitInput, scope string, refID int64, saver rateLimitSaver) error {
|
||
if in.clear {
|
||
return saver.ClearRateLimit(refID)
|
||
}
|
||
rl := store.RateLimit{
|
||
Scope: scope,
|
||
RefID: refID,
|
||
Mode: in.mode,
|
||
MaxMessages: in.maxMessages,
|
||
WindowSeconds: in.windowSeconds,
|
||
AutoMultiplier: in.autoMultiplier,
|
||
}
|
||
if in.mode == store.RateLimitModeAuto {
|
||
rl.WindowSeconds = h.l1Window()
|
||
if err := saver.SaveRateLimit(refID, rl); err != nil {
|
||
return err
|
||
}
|
||
return h.recalcRateLimit(scope, refID)
|
||
}
|
||
return saver.SaveRateLimit(refID, rl)
|
||
}
|