You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
When the SessionMiddleware detects an expired session, it continues without the user but doesn't clean up the expired session from the database. This leads to stale session data accumulating over time.
Before You Start
⚠️Verification Required: Please confirm this issue still exists before starting work:
Open internal/oauth/middleware.go
Find the TODO comment around line 42
Verify the expired session is not being deleted
Comment below confirming the issue exists
Current Code
// Lines 39-44 in middleware.go// Check if session is expiredifsession.ExpiresAt.Before(time.Now()) {
// Expired session - continue without user// TODO: Consider cleaning up expired session herereturnnext(c)
}
Expected Fix
Delete the expired session before continuing:
// Check if session is expiredifsession.ExpiresAt.Before(time.Now()) {
// Clean up expired sessioniferr:=storage.DeleteSession(c.Request().Context(), cookie.Value); err!=nil {
c.Logger().Errorf("Failed to delete expired session: %v", err)
}
returnnext(c)
}
Why This Matters
Expired sessions accumulate in the database indefinitely
Database storage grows unnecessarily
Could impact query performance over time
Acceptance Criteria
Call storage.DeleteSession() when expired session is detected
Log any errors from deletion (don't fail the request)
Add a unit test verifying cleanup is called for expired sessions
Run tests: go test ./internal/oauth/...
Helpful Context
The storage parameter already has DeleteSession method (see storage.go:191)
Similar cleanup patterns exist in handlers.go (lines 896, 1206, 1299)
Error from deletion should be logged but not block the request
@majiayu000 I see you made a commit for this in your own repo. If you make a pull request, I can review it and work with you to get it ready to merge into the main repo.
Verified the issue exists and PR #7 now addresses it: SessionMiddleware deletes expired sessions and logs errors without blocking. I also added a non-e2e unit test to cover expired-session cleanup, delete-error non-blocking, and valid-session path; pushed to the PR. CI (test job) is green.
Activity
tompscanlan commentedon Jan 3, 2026
@majiayu000 I see you made a commit for this in your own repo. If you make a pull request, I can review it and work with you to get it ready to merge into the main repo.
majiayu000 commentedon Jan 4, 2026
Verified the issue exists and PR #7 now addresses it: SessionMiddleware deletes expired sessions and logs errors without blocking. I also added a non-e2e unit test to cover expired-session cleanup, delete-error non-blocking, and valid-session path; pushed to the PR. CI (test job) is green.