diff --git a/AUTH_PLAN.md b/AUTH_PLAN.md index 43dee59..3e4e524 100644 --- a/AUTH_PLAN.md +++ b/AUTH_PLAN.md @@ -37,3 +37,6 @@ - Use `net/http/httptest` to verify happy-path login, signup, logout, invalid credential handling, CSRF failures, and session persistence. - Run `go test -cover ./...` to ensure the new logic maintains regression coverage. - Flesh out dedicated service tests for lookup flows and extend dashboard coverage once integration scaffolding is available. +- Add structured logging: text encoder for development, JSON for production deployments. +- Consolidate templates with a base layout to remove duplication across pages. +- Introduce configuration loading that sources environment variables, validates them, and exposes typed settings at startup. diff --git a/cmd/server/main.go b/cmd/server/main.go index bdf08c1..7e6c4f8 100644 --- a/cmd/server/main.go +++ b/cmd/server/main.go @@ -2,27 +2,34 @@ package main import ( "fmt" - "log" + "log/slog" "net/http" + "os" + "github.com/rjnemo/auth/internal/logging" "github.com/rjnemo/auth/internal/server" + "gorm.io/gorm/logger" ) +const listenAddr = ":8000" + func main() { - if err := run(); err != nil { - log.Fatalf("run: %v", err) + if err := run(logger); err != nil { + logger.Error("server exited", slog.Any("error", err)) + os.Exit(1) } } func run() error { - srv, err := server.New() + logger := logging.New(os.Stdout, logging.ModeText, &slog.HandlerOptions{AddSource: true}) + srv, err := server.New(logger) if err != nil { - return fmt.Errorf("initialise server: %v", err) + return fmt.Errorf("initialise server: %w", err) } - log.Println("Starting server on http://localhost:8000") - if err := http.ListenAndServe(":8000", srv.Router()); err != nil { - return fmt.Errorf("listen: %v", err) + logger.Info("starting server", slog.String("addr", listenAddr)) + if err := http.ListenAndServe(listenAddr, srv.Router()); err != nil { + return fmt.Errorf("listen: %w", err) } return nil diff --git a/internal/logging/logging.go b/internal/logging/logging.go new file mode 100644 index 0000000..40ac4a7 --- /dev/null +++ b/internal/logging/logging.go @@ -0,0 +1,46 @@ +package logging + +import ( + "io" + "log/slog" + "strings" +) + +// Mode selects the output format for structured logs. +type Mode string + +const ( + // ModeText renders human-friendly key/value lines for development. + ModeText Mode = "text" + // ModeJSON emits JSON objects suited for production ingestion. + ModeJSON Mode = "json" +) + +// ParseMode canonicalises textual representations of the logging mode. +func ParseMode(value string) Mode { + switch strings.ToLower(strings.TrimSpace(value)) { + case string(ModeJSON): + return ModeJSON + default: + return ModeText + } +} + +// New constructs a slog.Logger with the desired mode and handler options. +func New(out io.Writer, mode Mode, opts *slog.HandlerOptions) *slog.Logger { + if out == nil { + out = io.Discard + } + if opts == nil { + opts = &slog.HandlerOptions{} + } + + var handler slog.Handler + if mode == ModeJSON { + handler = slog.NewJSONHandler(out, opts) + } else { + handler = slog.NewTextHandler(out, opts) + } + + return slog.New(handler) +} diff --git a/internal/logging/logging_test.go b/internal/logging/logging_test.go new file mode 100644 index 0000000..0aea9a8 --- /dev/null +++ b/internal/logging/logging_test.go @@ -0,0 +1,77 @@ +package logging + +import ( + "bytes" + "encoding/json" + "log/slog" + "strings" + "testing" +) + +func TestParseMode(t *testing.T) { + t.Parallel() + + cases := map[string]Mode{ + "": ModeText, + "text": ModeText, + "TEXT": ModeText, + "json": ModeJSON, + " json ": ModeJSON, + "unknown": ModeText, + } + + for input, want := range cases { + if got := ParseMode(input); got != want { + t.Fatalf("ParseMode(%q) = %q, want %q", input, got, want) + } + } +} + +func TestNewTextLogger(t *testing.T) { + var buf bytes.Buffer + + opts := &slog.HandlerOptions{ReplaceAttr: dropTime} + logger := New(&buf, ModeText, opts) + logger.Info("server start", slog.String("component", "http")) + + output := strings.TrimSpace(buf.String()) + if !strings.Contains(output, "level=INFO") || !strings.Contains(output, "component=http") { + t.Fatalf("unexpected text output: %s", output) + } + if strings.Contains(output, slog.TimeKey) { + t.Fatalf("expected time attribute to be stripped: %s", output) + } +} + +func TestNewJSONLogger(t *testing.T) { + var buf bytes.Buffer + + opts := &slog.HandlerOptions{ReplaceAttr: dropTime} + logger := New(&buf, ModeJSON, opts) + logger.Error("save failed", slog.String("component", "auth")) + + var payload map[string]any + if err := json.Unmarshal(buf.Bytes(), &payload); err != nil { + t.Fatalf("failed to decode json log: %v", err) + } + + if payload["msg"] != "save failed" { + t.Fatalf("unexpected message: %v", payload["msg"]) + } + if payload["component"] != "auth" { + t.Fatalf("unexpected component: %v", payload["component"]) + } + if payload["level"] != "ERROR" { + t.Fatalf("unexpected level: %v", payload["level"]) + } + if _, ok := payload[slog.TimeKey]; ok { + t.Fatalf("expected time key to be stripped") + } +} + +func dropTime(_ []string, attr slog.Attr) slog.Attr { + if attr.Key == slog.TimeKey { + return slog.Attr{} + } + return attr +} diff --git a/internal/server/handler_dashboard.go b/internal/server/handler_dashboard.go index 24f629f..ab9e475 100644 --- a/internal/server/handler_dashboard.go +++ b/internal/server/handler_dashboard.go @@ -1,7 +1,7 @@ package server import ( - "log" + "log/slog" "net/http" "time" @@ -12,6 +12,7 @@ const dashboardTimeDisplayLayout = "02 Jan 2006 15:04 MST" func (s *Server) dashboardPageHandler() http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { + logger := s.logger.With(slog.String("component", "dashboard")) state := sessionFromContext(r.Context()) if !state.Authenticated { @@ -22,14 +23,14 @@ func (s *Server) dashboardPageHandler() http.HandlerFunc { email, err := auth.NewUserEmail(state.Email) if err != nil { - log.Printf("dashboard: invalid session email: %v", err) + logger.Warn("invalid session email", slog.Any("error", err)) http.Error(w, "session invalid", http.StatusUnauthorized) return } account, err := s.authService.LookupByEmail(r.Context(), email) if err != nil { - log.Printf("dashboard: lookup failed: %v", err) + logger.Error("lookup failed", slog.Any("error", err)) http.Error(w, "unable to load account", http.StatusInternalServerError) return } diff --git a/internal/server/handler_login.go b/internal/server/handler_login.go index dc12e5e..0dea2b0 100644 --- a/internal/server/handler_login.go +++ b/internal/server/handler_login.go @@ -2,7 +2,7 @@ package server import ( "errors" - "log" + "log/slog" "net/http" "github.com/rjnemo/auth/internal/service/auth" @@ -21,6 +21,7 @@ func (s *Server) loginPageHandler() http.HandlerFunc { func (s *Server) loginHandler() http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { + logger := s.logger.With(slog.String("component", "login")) state := sessionFromContext(r.Context()) if err := r.ParseForm(); err != nil { @@ -44,7 +45,7 @@ func (s *Server) loginHandler() http.HandlerFunc { state.Authenticated = true state.Email = account.Email.String() if err := s.sessions.Save(w, state); err != nil { - log.Printf("session: save failed: %v", err) + logger.Warn("session save failed", slog.Any("error", err)) } http.Redirect(w, r, "/dashboard", http.StatusSeeOther) @@ -57,7 +58,7 @@ func (s *Server) loginHandler() http.HandlerFunc { case errors.Is(err, auth.ErrInvalidCredentials): s.renderLoginFailure(w, email, state.CSRFToken) default: - log.Printf("auth: authenticate failed: %v", err) + logger.Error("authenticate failed", slog.Any("error", err)) http.Error(w, "unexpected error", http.StatusInternalServerError) } } diff --git a/internal/server/handler_signup.go b/internal/server/handler_signup.go index 5ab006a..43db116 100644 --- a/internal/server/handler_signup.go +++ b/internal/server/handler_signup.go @@ -2,7 +2,7 @@ package server import ( "errors" - "log" + "log/slog" "net/http" "github.com/rjnemo/auth/internal/service/auth" @@ -30,6 +30,7 @@ func (s *Server) signupPageHandler() http.HandlerFunc { func (s *Server) signupHandler() http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { + logger := s.logger.With(slog.String("component", "signup")) state := sessionFromContext(r.Context()) if err := r.ParseForm(); err != nil { @@ -53,7 +54,7 @@ func (s *Server) signupHandler() http.HandlerFunc { state.Authenticated = true state.Email = account.Email.String() if err := s.sessions.Save(w, state); err != nil { - log.Printf("session: save failed: %v", err) + logger.Warn("session save failed", slog.Any("error", err)) } http.Redirect(w, r, "/dashboard", http.StatusSeeOther) case errors.Is(err, auth.ErrWeakPassword): @@ -66,7 +67,7 @@ func (s *Server) signupHandler() http.HandlerFunc { w.WriteHeader(http.StatusConflict) s.render(w, "signup.html", newSignupData(email.String(), duplicateEmailMsg, state.CSRFToken)) default: - log.Printf("auth: register failed: %v", err) + logger.Error("register failed", slog.Any("error", err)) http.Error(w, "unexpected error", http.StatusInternalServerError) } } diff --git a/internal/server/middleware.go b/internal/server/middleware.go index 800bf91..d543fac 100644 --- a/internal/server/middleware.go +++ b/internal/server/middleware.go @@ -3,7 +3,7 @@ package server import ( "context" "crypto/subtle" - "log" + "log/slog" "net/http" ) @@ -11,17 +11,19 @@ type sessionContextKey struct{} func (s *Server) sessionMiddleware(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + logger := s.logger.With(slog.String("component", "session")) + state := s.sessions.Load(r) updated, err := ensureCSRFToken(state) if err != nil { - log.Printf("session: csrf token generation failed: %v", err) + logger.Error("csrf token generation failed", slog.Any("error", err)) http.Error(w, "session error", http.StatusInternalServerError) return } state = updated if err := s.sessions.Save(w, state); err != nil { - log.Printf("session: save failed: %v", err) + logger.Warn("session save failed", slog.Any("error", err)) } ctx := withSession(r.Context(), state) diff --git a/internal/server/server.go b/internal/server/server.go index aaecedb..78c22b7 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -5,8 +5,11 @@ import ( "crypto/rand" "fmt" "html/template" + "io" + "log/slog" "time" + "github.com/rjnemo/auth/internal/logging" "github.com/rjnemo/auth/internal/service/auth" "github.com/rjnemo/auth/web" ) @@ -21,10 +24,11 @@ type Server struct { templates *template.Template authService *auth.Service sessions *SessionStore + logger *slog.Logger } // New constructs a Server with parsed templates and default state. -func New() (*Server, error) { +func New(logger *slog.Logger) (*Server, error) { tmpl, err := template.ParseFS( web.Templates, "templates/login.html", @@ -51,10 +55,16 @@ func New() (*Server, error) { return nil, fmt.Errorf("session store: %w", err) } + if logger == nil { + logger = logging.New(io.Discard, logging.ModeText, nil) + } + logger = logger.With(slog.String("service", "http")) + return &Server{ templates: tmpl, authService: auth.NewService(store), sessions: sessionStore, + logger: logger, }, nil } diff --git a/internal/server/views.go b/internal/server/views.go index 4e3392e..c001b81 100644 --- a/internal/server/views.go +++ b/internal/server/views.go @@ -1,13 +1,16 @@ package server import ( - "log" + "log/slog" "net/http" ) func (s *Server) render(w http.ResponseWriter, name string, data any) { if err := s.templates.ExecuteTemplate(w, name, data); err != nil { - log.Printf("render %s: %v", name, err) + s.logger.With( + slog.String("component", "templates"), + slog.String("template", name), + ).Error("render failed", slog.Any("error", err)) http.Error(w, "template render failed", http.StatusInternalServerError) } }