From f8c6ee8fc7caade9a555bc74118c3836c0cc59ae Mon Sep 17 00:00:00 2001 From: majiayu000 <1835304752@qq.com> Date: Sun, 4 Jan 2026 11:13:29 +0800 Subject: [PATCH 1/2] fix: clean up expired sessions in SessionMiddleware MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, expired sessions were detected but not removed from the database, causing them to accumulate indefinitely. Now when an expired session is encountered, it is deleted from the database before continuing with the request. Fixes #3 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- internal/oauth/middleware.go | 6 ++++-- internal/oauth/middleware_test.go | 12 ++++++++++-- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/internal/oauth/middleware.go b/internal/oauth/middleware.go index b444dec..e9004c5 100644 --- a/internal/oauth/middleware.go +++ b/internal/oauth/middleware.go @@ -38,8 +38,10 @@ func SessionMiddleware(storage *Storage) echo.MiddlewareFunc { // Check if session is expired if session.ExpiresAt.Before(time.Now()) { - // Expired session - continue without user - // TODO: Consider cleaning up expired session here + // Clean up expired session from database + if err := storage.DeleteSession(c.Request().Context(), cookie.Value); err != nil { + c.Logger().Errorf("Failed to delete expired session: %v", err) + } return next(c) } diff --git a/internal/oauth/middleware_test.go b/internal/oauth/middleware_test.go index a11d007..3bc0f2e 100644 --- a/internal/oauth/middleware_test.go +++ b/internal/oauth/middleware_test.go @@ -95,7 +95,7 @@ func TestSessionMiddleware(t *testing.T) { assert.Equal(t, "did:plc:test123", capturedUser.DID) }) - t.Run("expired session - sets nil user in context", func(t *testing.T) { + t.Run("expired session - sets nil user in context and deletes session", func(t *testing.T) { // Create an expired session e := echo.New() setupReq := httptest.NewRequest(http.MethodGet, "/", nil) @@ -103,13 +103,17 @@ func TestSessionMiddleware(t *testing.T) { setupCtx := e.NewContext(setupReq, setupRec) session := OAuthSession{ - ID: "expired-session", + ID: "expired-session-cleanup", DID: "did:plc:expired", ExpiresAt: time.Now().Add(-1 * time.Hour), } err := storage.CreateSession(setupCtx.Request().Context(), session) require.NoError(t, err) + // Verify session exists before middleware call + _, err = storage.GetSessionByID(setupCtx.Request().Context(), session.ID) + require.NoError(t, err, "Session should exist before middleware call") + // Make request with expired session cookie req := httptest.NewRequest(http.MethodGet, "/", nil) req.AddCookie(&http.Cookie{ @@ -128,6 +132,10 @@ func TestSessionMiddleware(t *testing.T) { err = handler(c) require.NoError(t, err) assert.Nil(t, capturedUser) + + // Verify session was deleted from database + _, err = storage.GetSessionByID(setupCtx.Request().Context(), session.ID) + assert.Error(t, err, "Expired session should be deleted from database") }) } From e9ad0a824e8cd8cd8745c56b3dfa8bd0d14f00a6 Mon Sep 17 00:00:00 2001 From: majiayu000 <1835304752@qq.com> Date: Sun, 4 Jan 2026 14:11:03 +0800 Subject: [PATCH 2/2] test: add unit tests for session cleanup --- internal/oauth/middleware.go | 9 +- internal/oauth/middleware_unit_test.go | 137 +++++++++++++++++++++++++ 2 files changed, 145 insertions(+), 1 deletion(-) create mode 100644 internal/oauth/middleware_unit_test.go diff --git a/internal/oauth/middleware.go b/internal/oauth/middleware.go index e9004c5..20493ee 100644 --- a/internal/oauth/middleware.go +++ b/internal/oauth/middleware.go @@ -1,6 +1,7 @@ package oauth import ( + "context" "database/sql" "time" @@ -12,9 +13,15 @@ type User struct { DID string } +// SessionStore defines the session operations needed by the middleware. +type SessionStore interface { + GetSessionByID(ctx context.Context, id string) (*OAuthSession, error) + DeleteSession(ctx context.Context, id string) error +} + // SessionMiddleware creates middleware that reads the session cookie // and adds the user to the context if the session is valid -func SessionMiddleware(storage *Storage) echo.MiddlewareFunc { +func SessionMiddleware(storage SessionStore) echo.MiddlewareFunc { return func(next echo.HandlerFunc) echo.HandlerFunc { return func(c echo.Context) error { // Try to get session cookie diff --git a/internal/oauth/middleware_unit_test.go b/internal/oauth/middleware_unit_test.go new file mode 100644 index 0000000..c21c6a5 --- /dev/null +++ b/internal/oauth/middleware_unit_test.go @@ -0,0 +1,137 @@ +package oauth + +import ( + "context" + "database/sql" + "errors" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/labstack/echo/v4" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +type stubSessionStore struct { + sessions map[string]*OAuthSession + deleteErr error + deleteCalls []string +} + +func (s *stubSessionStore) GetSessionByID(ctx context.Context, id string) (*OAuthSession, error) { + session, ok := s.sessions[id] + if !ok { + return nil, sql.ErrNoRows + } + return session, nil +} + +func (s *stubSessionStore) DeleteSession(ctx context.Context, id string) error { + s.deleteCalls = append(s.deleteCalls, id) + if s.deleteErr != nil { + return s.deleteErr + } + delete(s.sessions, id) + return nil +} + +func TestSessionMiddlewareExpiredSessionDeletes(t *testing.T) { + store := &stubSessionStore{ + sessions: map[string]*OAuthSession{ + "expired-session": { + ID: "expired-session", + DID: "did:plc:expired", + ExpiresAt: time.Now().Add(-1 * time.Minute), + }, + }, + } + + e := echo.New() + req := httptest.NewRequest(http.MethodGet, "/", nil) + req.AddCookie(&http.Cookie{Name: "session", Value: "expired-session"}) + rec := httptest.NewRecorder() + c := e.NewContext(req, rec) + + var capturedUser *User + nextCalled := false + handler := SessionMiddleware(store)(func(c echo.Context) error { + nextCalled = true + capturedUser = GetUser(c) + return c.String(http.StatusOK, "ok") + }) + + err := handler(c) + require.NoError(t, err) + assert.True(t, nextCalled) + assert.Nil(t, capturedUser) + assert.Equal(t, []string{"expired-session"}, store.deleteCalls) + _, exists := store.sessions["expired-session"] + assert.False(t, exists) +} + +func TestSessionMiddlewareDeleteErrorDoesNotBlock(t *testing.T) { + store := &stubSessionStore{ + sessions: map[string]*OAuthSession{ + "expired-session": { + ID: "expired-session", + DID: "did:plc:expired", + ExpiresAt: time.Now().Add(-1 * time.Minute), + }, + }, + deleteErr: errors.New("delete failed"), + } + + e := echo.New() + req := httptest.NewRequest(http.MethodGet, "/", nil) + req.AddCookie(&http.Cookie{Name: "session", Value: "expired-session"}) + rec := httptest.NewRecorder() + c := e.NewContext(req, rec) + + var capturedUser *User + nextCalled := false + handler := SessionMiddleware(store)(func(c echo.Context) error { + nextCalled = true + capturedUser = GetUser(c) + return c.String(http.StatusOK, "ok") + }) + + err := handler(c) + require.NoError(t, err) + assert.True(t, nextCalled) + assert.Nil(t, capturedUser) + assert.Equal(t, []string{"expired-session"}, store.deleteCalls) + _, exists := store.sessions["expired-session"] + assert.True(t, exists) +} + +func TestSessionMiddlewareValidSessionDoesNotDelete(t *testing.T) { + store := &stubSessionStore{ + sessions: map[string]*OAuthSession{ + "valid-session": { + ID: "valid-session", + DID: "did:plc:valid", + ExpiresAt: time.Now().Add(1 * time.Hour), + }, + }, + } + + e := echo.New() + req := httptest.NewRequest(http.MethodGet, "/", nil) + req.AddCookie(&http.Cookie{Name: "session", Value: "valid-session"}) + rec := httptest.NewRecorder() + c := e.NewContext(req, rec) + + var capturedUser *User + handler := SessionMiddleware(store)(func(c echo.Context) error { + capturedUser = GetUser(c) + return c.String(http.StatusOK, "ok") + }) + + err := handler(c) + require.NoError(t, err) + require.NotNil(t, capturedUser) + assert.Equal(t, "did:plc:valid", capturedUser.DID) + assert.Empty(t, store.deleteCalls) +}