`d30741e`

admin: actually send password reset emails (SR2 C3 + L6)

Pre-fix: userResetPassword minted a token, persisted password_resets,
audited ActionAdminUserPasswordReset, then `_ = tokEnc` discarded
the token. admin.Deps had no email.Sender at all, so the comment
'surfaced via the email path' was false from the start. The audit
row claimed 'sent'; the user never received anything.

Post-fix:
- admin.Deps adds Email + Branding fields, mirroring auth.Deps.
- admin_wiring.buildAdminHandlers takes the same email.Sender
the auth surface uses (server.go threads pickEmailSender(cfg)
through both paths).
- userResetPassword calls email.ResetMessage + sender.Send. The
audit row now carries email_sent: bool plus email_error: string
on failure, so a stuck mailbox is visible in /admin/audit instead
of silently misleading.
- L6: server.go now passes version.Version into buildAdminHandlers
instead of the literal 'dev', so /admin/system reflects the
ld-flag-stamped build version.

admin_deps_test pins the field contract — removing Email or
Branding breaks compile.

Authored by

espadonne 3 days ago

SHA: d30741e3d6190cc6f39e8b72c9d0fcd84edb1d38
Parents: a4f4d82
Tree: 203e4a0

5 changed files

Status	File	+	-
M	`internal/web/admin_wiring.go`	16	4
M	`internal/web/handlers/admin/admin.go`	6	0
A	`internal/web/handlers/admin/admin_deps_test.go`	33	0
M	`internal/web/handlers/admin/users.go`	26	7
M	`internal/web/server.go`	11	1

internal/web/admin_wiring.gomodified

  	"github.com/jackc/pgx/v5/pgxpool"
  	"github.com/tenseleyFlow/shithub/internal/auth/audit"
 +	"github.com/tenseleyFlow/shithub/internal/auth/email"
  	"github.com/tenseleyFlow/shithub/internal/infra/config"
  	adminh "github.com/tenseleyFlow/shithub/internal/web/handlers/admin"
  	"github.com/tenseleyFlow/shithub/internal/web/render"
  // buildAdminHandlers wires the S34 site-admin handler set. The
  // returned handler set is route-only; the wiring layer in server.go
  // composes RequireUser + RequireSiteAdmin around the Mount call.
 +//
 +// emailSender + branding are required for the "Reset password"
 +// admin action to actually deliver. Pass the same sender the
 +// auth handler set uses so behavior matches the public flow.
  func buildAdminHandlers(
  	cfg config.Config,
  	pool *pgxpool.Pool,
  	tmplFS fs.FS,
  	logger *slog.Logger,
  	version string,
 +	emailSender email.Sender,
  ) (*adminh.Handlers, error) {
  	if pool == nil {
  		return nil, errors.New("admin: nil pool")
  		return nil, fmt.Errorf("admin: render.New: %w", err)
+ 	}
  	return adminh.New(adminh.Deps{
 -		Logger:   logger,
 -		Render:   rr,
 -		Pool:     pool,
 -		Audit:    audit.NewRecorder(),
 +		Logger: logger,
 +		Render: rr,
 +		Pool:   pool,
 +		Audit:  audit.NewRecorder(),
 +		Email:  emailSender,
 +		Branding: email.Branding{
 +			SiteName: cfg.Auth.SiteName,
 +			BaseURL:  cfg.Auth.BaseURL,
 +			From:     cfg.Auth.EmailFrom,
 +		},
  		SiteName: cfg.Auth.SiteName,
  		Version:  version,
  	})

internal/web/handlers/admin/admin.gomodified

  	admindb "github.com/tenseleyFlow/shithub/internal/admin/sqlc"
  	"github.com/tenseleyFlow/shithub/internal/auth/audit"
 +	"github.com/tenseleyFlow/shithub/internal/auth/email"
  	usersdb "github.com/tenseleyFlow/shithub/internal/users/sqlc"
  	"github.com/tenseleyFlow/shithub/internal/web/render"
+ )
  	Render *render.Renderer
  	Pool   *pgxpool.Pool
  	Audit  *audit.Recorder
 +	// Email + Branding power the admin "Reset password" send. Required:
 +	// without them userResetPassword mints a token and never delivers
 +	// it (SR2 C3 — pre-fix the audit row falsely claimed "sent").
 +	Email    email.Sender
 +	Branding email.Branding
  	// SiteName is rendered into pages; matches Auth.SiteName from config.
  	SiteName string
  	// Version is the running shithubd version, surfaced on /admin/system.

internal/web/handlers/admin/admin_deps_test.goadded

 +// SPDX-License-Identifier: AGPL-3.0-or-later
++
 +package admin_test
++
 +import (
 +	"testing"
++
 +	"github.com/tenseleyFlow/shithub/internal/auth/email"
 +	adminh "github.com/tenseleyFlow/shithub/internal/web/handlers/admin"
 +)
++
 +// TestDepsShape pins the admin.Deps type contract: Email + Branding
 +// MUST be present (SR2 C3). Pre-SR2 the admin handlers had no email
 +// sender wired, so userResetPassword minted a token, persisted it,
 +// audited "sent", and discarded the token via `_ = tokEnc`. The user
 +// never received the email; the audit row lied.
 +//
 +// This is a build-time contract: removing the fields breaks compile.
 +// If you're here because this test failed, the SR2 C3 remediation
 +// has regressed — restore the fields and the userResetPassword send
 +// path.
 +func TestDepsShape(t *testing.T) {
 +	t.Parallel()
++
 +	// Compile-time field references. If Email or Branding are renamed
 +	// or removed the package fails to build. The runtime assertion
 +	// below is belt-and-suspenders for type drift.
 +	d := adminh.Deps{
 +		Email:    nil,              // type: email.Sender
 +		Branding: email.Branding{}, // type: email.Branding
 +	}
 +	_ = d
 +}

internal/web/handlers/admin/users.gomodified

  		http.Error(w, "create reset row failed", http.StatusInternalServerError)
  		return
+ 	}
 -	// Email send is best-effort (the token is in the DB regardless;
 -	// admin can resend by clicking again). When configured, this hits
 -	// the same sender as the public flow.
 -	_ = email.Branding{}
 -	_ = tokEnc // surfaced via the email path; left as TODO when no sender wired
 -	h.recordAdminAction(r, audit.ActionAdminUserPasswordReset, audit.TargetUser, user.ID,
 -		map[string]any{"email": string(em.Email)})
++
 +	// Send the message (best-effort: the token row is committed; the
 +	// admin can resend by clicking again). The audit row records
 +	// whether the send succeeded, so a stuck mailbox shows up in
 +	// /admin/audit instead of being invisible (SR2 C3 fix).
 +	emailSent := false
 +	emailErr := ""
 +	if h.d.Email != nil {
 +		msg, err := email.ResetMessage(h.d.Branding, string(em.Email), tokEnc)
 +		if err != nil {
 +			emailErr = err.Error()
 +			h.d.Logger.WarnContext(r.Context(), "admin reset: build message", "error", err)
 +		} else if err := h.d.Email.Send(r.Context(), msg); err != nil {
 +			emailErr = err.Error()
 +			h.d.Logger.WarnContext(r.Context(), "admin reset: send", "error", err)
 +		} else {
 +			emailSent = true
 +		}
 +	} else {
 +		emailErr = "no sender wired"
 +	}
 +	auditMeta := map[string]any{"email": string(em.Email), "email_sent": emailSent}
 +	if emailErr != "" {
 +		auditMeta["email_error"] = emailErr
 +	}
 +	h.recordAdminAction(r, audit.ActionAdminUserPasswordReset, audit.TargetUser, user.ID, auditMeta)
  	http.Redirect(w, r, "/admin/users/"+strconv.FormatInt(user.ID, 10)+"?notice=saved", http.StatusSeeOther)
+ }

internal/web/server.gomodified

  	infralog "github.com/tenseleyFlow/shithub/internal/infra/log"
  	"github.com/tenseleyFlow/shithub/internal/infra/metrics"
  	"github.com/tenseleyFlow/shithub/internal/infra/tracing"
 +	"github.com/tenseleyFlow/shithub/internal/version"
  	"github.com/tenseleyFlow/shithub/internal/web/handlers"
  	"github.com/tenseleyFlow/shithub/internal/web/middleware"
+ )
  		// S34 — site admin. Gated by RequireUser + RequireSiteAdmin
  		// (404 not 403 for non-admins). Uses its own renderer so the
  		// admin templates are loaded once at boot.
 -		adminH, err := buildAdminHandlers(cfg, pool, deps.TemplatesFS, logger, "dev")
 +		//
 +		// Email sender is the same one auth uses; the admin "Reset
 +		// password" action sends through it (SR2 C3). Version is the
 +		// build-time-stamped value so /admin/system reports reality
 +		// instead of the literal "dev" (SR2 L6).
 +		adminSender, err := pickEmailSender(cfg)
 +		if err != nil {
 +			return fmt.Errorf("admin handlers: pick email sender: %w", err)
 +		}
 +		adminH, err := buildAdminHandlers(cfg, pool, deps.TemplatesFS, logger, version.Version, adminSender)
  		if err != nil {
  			return fmt.Errorf("admin handlers: %w", err)
+ 		}