Implements cookie authentication enhancements

Improves cookie authentication by adding error handling for expired and bad format cookies.

Enhances user session management through robust checks and logging, ensuring a secure and reliable authentication process.

Updates the README to reflect changes in the authentication mechanism.

6 files changed, 430 insertions, 147 deletions

diff --git a/kauth/backend.go b/kauth/backend.go
new file mode 100644
index 0000000..e4f5b21
--- /dev/null
+++ b/kauth/backend.go
@@ -0,0 +1,151 @@
+package kauth
+
+import (
+	"context"
+	"errors"
+	"fmt"
+	"net/http"
+	"strconv"
+	"strings"
+	"time"
+
+	"github.com/martinlehoux/kagamigo/kcore"
+	"golang.org/x/exp/slog"
+)
+
+var (
+	ErrUserNotLoggedIn    = errors.New("user not logged in")
+	ErrCookieExpired      = errors.New("cookie expired")
+	ErrBadCookie          = errors.New("invalid cookie")
+	ErrInvalidCredentials = errors.New("invalid credentials")
+)
+
+type identifyable interface {
+	ID() kcore.ID
+}
+
+// A backend needs a user store to authenticate and load users
+type UserStore[U identifyable] interface {
+	LoadUser(ctx context.Context, id kcore.ID) (U, error)
+	Authenticate(ctx context.Context, username string, password string) (U, bool)
+}
+
+// A backend is responsible for authenticating users and managing their sessions
+type Backend[U identifyable] struct {
+	UserStore[U]
+	Domain       string
+	CookieSecret []byte
+	Now          func() time.Time // injectable time provider
+}
+
+// Authenticate and persist the login in the user session
+func (b *Backend[U]) Login(w http.ResponseWriter, ctx context.Context, username, password string) (U, error) {
+	user, ok := b.Authenticate(ctx, username, password)
+	if !ok {
+		return user, ErrInvalidCredentials
+	}
+	now := time.Now
+	if b.Now != nil {
+		now = b.Now
+	}
+	expiresAt := now().Add(24 * time.Hour)
+	cookie := http.Cookie{
+		Domain:  b.Domain,
+		Name:    "authentication",
+		Value:   encrypt(b.CookieSecret, fmt.Sprintf("%s:%d", user.ID().String(), expiresAt.Unix())),
+		Expires: expiresAt,
+		Path:    "/",
+	}
+	http.SetCookie(w, &cookie)
+	return user, nil
+}
+
+// Clears login from the response
+func (b *Backend[U]) Logout(w http.ResponseWriter) {
+	// Set the cookie with MaxAge -1 to delete it
+	http.SetCookie(w, &http.Cookie{
+		Name:     "authentication",
+		Value:    "",
+		Path:     "/",
+		Domain:   b.Domain,
+		Expires:  time.Unix(0, 0),
+		MaxAge:   -1,
+		HttpOnly: true,
+		Secure:   true,
+	})
+}
+
+type userContext struct{}
+
+func (b *Backend[U]) PersistUser(r *http.Request, user U) *http.Request {
+	ctx := context.WithValue(r.Context(), userContext{}, user)
+	return r.WithContext(ctx)
+}
+
+func (b Backend[U]) CookieAuthMiddleware() func(http.Handler) http.Handler {
+	return func(next http.Handler) http.Handler {
+		return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			ctx := r.Context()
+			cookie, err := r.Cookie("authentication")
+			if errors.Is(err, http.ErrNoCookie) {
+				next.ServeHTTP(w, r)
+				return
+			}
+			kcore.Expect(err, "error reading cookie")
+
+			authentication, err := decrypt(b.CookieSecret, cookie.Value)
+			if err != nil {
+				err = kcore.Wrap(err, "error decrypting cookie")
+				slog.Warn(err.Error())
+				http.Error(w, err.Error(), http.StatusBadRequest)
+				return
+			}
+			parts := strings.Split(authentication, ":")
+			if len(parts) != 2 {
+				slog.Warn(ErrBadCookie.Error())
+				http.Error(w, ErrBadCookie.Error(), http.StatusBadRequest)
+				return
+			}
+			userId, err := kcore.ParseID(parts[0])
+			if err != nil {
+				err = kcore.Wrap(err, "error parsing user id")
+				slog.Warn(err.Error())
+				http.Error(w, err.Error(), http.StatusBadRequest)
+				return
+			}
+			expiresAtSeconds, err := strconv.Atoi(parts[1])
+			if err != nil {
+				err = kcore.Wrap(err, "error parsing expires at")
+				slog.Warn(err.Error())
+				http.Error(w, err.Error(), http.StatusBadRequest)
+				return
+			}
+			now := time.Now
+			if b.Now != nil {
+				now = b.Now
+			}
+			expiresAt := time.Unix(int64(expiresAtSeconds), 0)
+			if now().After(expiresAt) {
+				slog.Warn(ErrCookieExpired.Error())
+				http.Error(w, ErrCookieExpired.Error(), http.StatusUnauthorized)
+				return
+			}
+			user, err := b.LoadUser(ctx, userId)
+			if err != nil {
+				err = kcore.Wrap(err, "error loading user")
+				slog.Warn(err.Error())
+				http.Error(w, err.Error(), http.StatusInternalServerError)
+				return
+			}
+			r = b.PersistUser(r, user)
+
+			ctx = context.WithValue(ctx, userContext{}, user)
+			next.ServeHTTP(w, r.WithContext(ctx))
+		})
+	}
+}
+
+func (b *Backend[U]) User(ctx context.Context) (U, bool) {
+	user, ok := ctx.Value(userContext{}).(U)
+	return user, ok
+}
diff --git a/kauth/backend_test.go b/kauth/backend_test.go
new file mode 100644
index 0000000..fb18374
--- /dev/null
+++ b/kauth/backend_test.go
@@ -0,0 +1,252 @@
+package kauth
+
+import (
+	"context"
+	"errors"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+	"time"
+
+	"github.com/martinlehoux/kagamigo/kcore"
+	"github.com/stretchr/testify/assert"
+)
+
+// Test structures
+type testUser struct {
+	id       kcore.ID
+	password string
+}
+
+func (u testUser) ID() kcore.ID {
+	return u.id
+}
+
+type store struct {
+	users map[string]testUser
+}
+
+func (s *store) Authenticate(ctx context.Context, username, password string) (testUser, bool) {
+	if u, ok := s.users[username]; ok {
+		if password == u.password {
+			return u, true
+		}
+	}
+	return testUser{}, false
+}
+
+func (s *store) LoadUser(ctx context.Context, id kcore.ID) (testUser, error) {
+	for _, u := range s.users {
+		if u.id == id {
+			return u, nil
+		}
+	}
+	return testUser{}, errors.New("user not found")
+}
+
+func newUser(password string) testUser {
+	return testUser{id: kcore.NewID(), password: password}
+}
+
+func newBackend() (*Backend[testUser], *store) {
+	s := store{users: map[string]testUser{}}
+	return &Backend[testUser]{
+		UserStore:    &s,
+		Domain:       "localhost",
+		CookieSecret: GenerateCookieSecret(),
+		Now:          time.Now,
+	}, &s
+}
+
+// Helper to extract authentication cookie value from a response recorder
+func getAuthCookie(rr *httptest.ResponseRecorder) *http.Cookie {
+	for _, c := range rr.Result().Cookies() {
+		if c.Name == "authentication" {
+			return c
+		}
+	}
+	return nil
+}
+
+// Test Backend.Login sets cookie on success
+func TestBackend_Login_Success(t *testing.T) {
+	user := newUser("pass")
+	backend, store := newBackend()
+	store.users["user"] = user
+
+	rr := httptest.NewRecorder()
+	ctx := context.Background()
+	_, err := backend.Login(rr, ctx, "user", "pass")
+	assert.NoError(t, err)
+
+	c := getAuthCookie(rr)
+	assert.NotNil(t, c, "authentication cookie should be set")
+	assert.Equal(t, backend.Domain, c.Domain)
+	assert.Equal(t, "/", c.Path)
+	assert.True(t, time.Until(c.Expires) >= 23*time.Hour && time.Until(c.Expires) <= 25*time.Hour, "expected expires about 24h from now")
+}
+
+// Test Backend.Login returns error on invalid credentials
+func TestBackend_Login_InvalidCredentials(t *testing.T) {
+	user := newUser("pass")
+	backend, store := newBackend()
+	store.users["user"] = user
+
+	rr := httptest.NewRecorder()
+	ctx := context.Background()
+	_, err := backend.Login(rr, ctx, "baduser", "badpass")
+	assert.Equal(t, ErrInvalidCredentials, err)
+
+	c := getAuthCookie(rr)
+	assert.Nil(t, c, "did not expect authentication cookie to be set")
+}
+
+// Test Backend.Logout deletes the authentication cookie
+func TestBackend_Logout(t *testing.T) {
+	user := newUser("pass")
+	backend, store := newBackend()
+	store.users["user"] = user
+
+	rr := httptest.NewRecorder()
+	backend.Logout(rr)
+
+	c := getAuthCookie(rr)
+	assert.NotNil(t, c, "authentication cookie should be set for deletion")
+	assert.Equal(t, "", c.Value)
+	assert.Equal(t, "/", c.Path)
+	assert.Equal(t, backend.Domain, c.Domain)
+	assert.Equal(t, int(-1), c.MaxAge)
+	assert.True(t, c.Expires.Equal(time.Unix(0, 0)), "Expires should be Unix epoch")
+	assert.True(t, c.HttpOnly)
+	assert.True(t, c.Secure)
+}
+
+func TestCookieAuthMiddleware_ValidCookie_SetsUserInContext(t *testing.T) {
+	user := newUser("pass")
+	backend, store := newBackend()
+	store.users["user"] = user
+
+	rrLogin := httptest.NewRecorder()
+	ctx := context.Background()
+	_, _ = backend.Login(rrLogin, ctx, "user", "pass")
+	cookie := getAuthCookie(rrLogin)
+	req := httptest.NewRequest("GET", "/", nil)
+	req.AddCookie(cookie)
+
+	var userInHandler testUser
+	handler := backend.CookieAuthMiddleware()(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		u, ok := backend.User(r.Context())
+		assert.True(t, ok, "user should be in context")
+		userInHandler = u
+		w.WriteHeader(http.StatusOK)
+	}))
+
+	rr := httptest.NewRecorder()
+	handler.ServeHTTP(rr, req)
+	assert.Equal(t, user.id, userInHandler.id)
+	assert.Equal(t, http.StatusOK, rr.Code)
+}
+
+func TestCookieAuthMiddleware_NoCookie_PassesThrough(t *testing.T) {
+	user := newUser("pass")
+	backend, store := newBackend()
+	store.users["user"] = user
+	req := httptest.NewRequest("GET", "/", nil)
+
+	called := false
+	handler := backend.CookieAuthMiddleware()(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		called = true
+		w.WriteHeader(http.StatusOK)
+	}))
+	rr := httptest.NewRecorder()
+	handler.ServeHTTP(rr, req)
+	assert.True(t, called, "handler should be called")
+	assert.Equal(t, http.StatusOK, rr.Code)
+}
+
+func TestCookieAuthMiddleware_InvalidCookie_DecryptionFails(t *testing.T) {
+	user := newUser("pass")
+	backend, store := newBackend()
+	store.users["user"] = user
+	req := httptest.NewRequest("GET", "/", nil)
+	req.AddCookie(&http.Cookie{Name: "authentication", Value: "not-encrypted"})
+
+	handler := backend.CookieAuthMiddleware()(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		t.Error("handler should not be called on decryption failure")
+	}))
+	rr := httptest.NewRecorder()
+	handler.ServeHTTP(rr, req)
+	assert.Equal(t, http.StatusBadRequest, rr.Code)
+}
+
+func TestCookieAuthMiddleware_BadFormatCookie(t *testing.T) {
+	user := newUser("pass")
+	backend, store := newBackend()
+	store.users["user"] = user
+	// Encrypt a string that doesn't have the expected "id:expires" format
+	badValue := encrypt(backend.CookieSecret, "badformat")
+	req := httptest.NewRequest("GET", "/", nil)
+	req.AddCookie(&http.Cookie{Name: "authentication", Value: badValue})
+
+	handler := backend.CookieAuthMiddleware()(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		t.Error("handler should not be called on bad format")
+	}))
+	rr := httptest.NewRecorder()
+	handler.ServeHTTP(rr, req)
+	assert.Equal(t, http.StatusBadRequest, rr.Code)
+}
+
+func Must[T any](val T, err error) T {
+	if err != nil {
+		panic(err)
+	}
+	return val
+}
+
+func TestCookieAuthMiddleware_ExpiredCookie(t *testing.T) {
+	user := newUser("pass")
+	backend, store := newBackend()
+	store.users["user"] = user
+
+	backend.Now = func() time.Time { return Must(time.Parse(time.RFC3339, "2025-12-23T12:00:00Z")) }
+	rrLogin := httptest.NewRecorder()
+	ctx := context.Background()
+	_, _ = backend.Login(rrLogin, ctx, "user", "pass")
+	cookie := getAuthCookie(rrLogin)
+	assert.NotNil(t, cookie, "authentication cookie should be set")
+	req := httptest.NewRequest("GET", "/", nil)
+	req.AddCookie(cookie)
+
+	var userSet bool
+	backend.Now = func() time.Time { return Must(time.Parse(time.RFC3339, "2025-12-24T13:00:00Z")) } // after expiry
+	handler := backend.CookieAuthMiddleware()(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		_, ok := backend.User(r.Context())
+		userSet = ok
+		w.WriteHeader(http.StatusUnauthorized)
+	}))
+	rr := httptest.NewRecorder()
+	handler.ServeHTTP(rr, req)
+	assert.False(t, userSet, "user should not be set in context for expired cookie")
+	assert.Equal(t, http.StatusUnauthorized, rr.Code)
+}
+
+func TestCookieAuthMiddleware_InvalidUserID(t *testing.T) {
+	user := newUser("pass")
+	backend, store := newBackend()
+	store.users["user"] = user
+	rrLogin := httptest.NewRecorder()
+	ctx := context.Background()
+	_, _ = backend.Login(rrLogin, ctx, "user", "pass")
+	cookie := getAuthCookie(rrLogin)
+
+	req := httptest.NewRequest("GET", "/", nil)
+	req.AddCookie(cookie)
+	delete(store.users, "user") // User is no longer valid
+
+	handler := backend.CookieAuthMiddleware()(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		t.Error("handler should not be called on invalid user id")
+	}))
+	rr := httptest.NewRecorder()
+	handler.ServeHTTP(rr, req)
+	assert.Equal(t, http.StatusInternalServerError, rr.Code)
+}
diff --git a/kauth/config.go b/kauth/config.go
deleted file mode 100644
index 777a22d..0000000
--- a/kauth/config.go
+++ /dev/null
@@ -1,34 +0,0 @@
-package kauth
-
-import (
-	"encoding/hex"
-	"errors"
-	"os"
-
-	"github.com/martinlehoux/kagamigo/kcore"
-	"golang.org/x/exp/slog"
-)
-
-var (
-	ErrCookieBadLength = errors.New("cookie secret must be 32 bytes")
-)
-
-type AuthConfig struct {
-	Domain       string
-	CookieSecret []byte
-}
-
-func LoadCookieSecret(cookieSecretString string) []byte {
-	cookiesSecret, err := hex.DecodeString(cookieSecretString)
-	if err != nil {
-		err = kcore.Wrap(err, "error decoding cookie secret")
-		slog.Error(err.Error())
-		os.Exit(1)
-	}
-	if len(cookiesSecret) != 32 {
-		slog.Error(ErrCookieBadLength.Error())
-		os.Exit(1)
-	}
-	slog.Info("cookie secret loaded")
-	return cookiesSecret
-}
diff --git a/kauth/cookie.go b/kauth/cookie.go
deleted file mode 100644
index edc30b7..0000000
--- a/kauth/cookie.go
+++ /dev/null
@@ -1,21 +0,0 @@
-package kauth
-
-import (
-	"fmt"
-	"net/http"
-	"time"
-
-	"github.com/martinlehoux/kagamigo/kcore"
-)
-
-func CraftCookie(id kcore.ID, config AuthConfig) http.Cookie {
-	expiresAt := time.Now().Add(24 * time.Hour)
-	cookieValue := encrypt(config.CookieSecret, fmt.Sprintf("%s:%d", id.String(), expiresAt.Unix()))
-	return http.Cookie{
-		Domain:  config.Domain,
-		Name:    "authentication",
-		Value:   cookieValue,
-		Expires: expiresAt,
-		Path:    "/",
-	}
-}
diff --git a/kauth/crypto.go b/kauth/crypto.go
index 117076d..0ab9407 100644
--- a/kauth/crypto.go
+++ b/kauth/crypto.go
@@ -5,15 +5,42 @@ import (
 	"crypto/cipher"
 	"crypto/rand"
 	"encoding/base64"
+	"encoding/hex"
 	"errors"
+	"os"
 
 	"github.com/martinlehoux/kagamigo/kcore"
+	"golang.org/x/exp/slog"
 )
 
 var (
 	ErrEncryptedTooShort = errors.New("encrypted text is too short")
+	ErrCookieBadLength   = errors.New("cookie secret must be 32 bytes")
 )
 
+// Generate a random 32-byte secret for cookies
+func GenerateCookieSecret() []byte {
+	secret := make([]byte, 32)
+	_, err := rand.Read(secret)
+	kcore.Expect(err, "failed to generate random bytes")
+	return secret
+}
+
+func LoadCookieSecret(cookieSecretString string) []byte {
+	cookiesSecret, err := hex.DecodeString(cookieSecretString)
+	if err != nil {
+		err = kcore.Wrap(err, "error decoding cookie secret")
+		slog.Error(err.Error())
+		os.Exit(1)
+	}
+	if len(cookiesSecret) != 32 {
+		slog.Error(ErrCookieBadLength.Error())
+		os.Exit(1)
+	}
+	slog.Info("cookie secret loaded")
+	return cookiesSecret
+}
+
 func encrypt(secret []byte, plainText string) string {
 	block, err := aes.NewCipher(secret)
 	kcore.Expect(err, "error creating AES cipher")
diff --git a/kauth/middlewares.go b/kauth/middlewares.go
deleted file mode 100644
index 23e9e71..0000000
--- a/kauth/middlewares.go
+++ /dev/null
@@ -1,92 +0,0 @@
-package kauth
-
-import (
-	"context"
-	"errors"
-	"net/http"
-	"strconv"
-	"strings"
-	"time"
-
-	"github.com/martinlehoux/kagamigo/kcore"
-	"golang.org/x/exp/slog"
-)
-
-var (
-	ErrUserNotLoggedIn = errors.New("user not logged in")
-	ErrCookieExpired   = errors.New("cookie expired")
-	ErrBadCookie       = errors.New("invalid cookie")
-)
-
-type userContext struct{}
-
-func UserFromContext[U any](ctx context.Context) (U, bool) {
-	user, ok := ctx.Value(userContext{}).(U)
-	return user, ok
-}
-
-func LoginFromContext[U kcore.WithLanguage](ctx context.Context) kcore.Login[U] {
-	user, ok := UserFromContext[U](ctx)
-	return kcore.LoginFromUser(user, ok)
-}
-
-func Unauthorized(w http.ResponseWriter, err error) {
-	w.WriteHeader(http.StatusUnauthorized)
-	if err != nil {
-		_, err = w.Write([]byte(err.Error()))
-		kcore.Expect(err, "error writing response")
-	}
-}
-
-func CookieAuthMiddleware(loadUser func(context.Context, kcore.ID) (any, error), config AuthConfig) func(http.Handler) http.Handler {
-	return func(next http.Handler) http.Handler {
-		return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-			ctx := r.Context()
-			cookie, err := r.Cookie("authentication")
-			if errors.Is(err, http.ErrNoCookie) {
-				next.ServeHTTP(w, r)
-				return
-			}
-			kcore.Expect(err, "error reading cookie")
-
-			authentication, err := decrypt(config.CookieSecret, cookie.Value)
-			if err != nil {
-				err = kcore.Wrap(err, "error decrypting cookie")
-				slog.Warn(err.Error())
-				http.Error(w, err.Error(), http.StatusBadRequest)
-				return
-			}
-			parts := strings.Split(authentication, ":")
-			if len(parts) != 2 {
-				slog.Warn(ErrBadCookie.Error())
-				http.Error(w, ErrBadCookie.Error(), http.StatusBadRequest)
-				return
-			}
-			userId, err := kcore.ParseID(parts[0])
-			if err != nil {
-				err = kcore.Wrap(err, "error parsing user id")
-				slog.Warn(err.Error())
-				http.Error(w, err.Error(), http.StatusBadRequest)
-				return
-			}
-			expiresAtSeconds, err := strconv.Atoi(parts[1])
-			if err != nil {
-				err = kcore.Wrap(err, "error parsing expires at")
-				slog.Warn(err.Error())
-				http.Error(w, err.Error(), http.StatusBadRequest)
-				return
-			}
-			expiresAt := time.Unix(int64(expiresAtSeconds), 0)
-			if time.Now().After(expiresAt) {
-				slog.Warn(ErrCookieExpired.Error())
-				http.Error(w, ErrCookieExpired.Error(), http.StatusUnauthorized)
-				return
-			}
-			user, err := loadUser(ctx, userId)
-			kcore.Expect(err, "error loading user")
-
-			ctx = context.WithValue(ctx, userContext{}, user)
-			next.ServeHTTP(w, r.WithContext(ctx))
-		})
-	}
-}