-
Notifications
You must be signed in to change notification settings - Fork 2
Closed
Labels
bugSomething isn't workingSomething isn't workinggood first issueGood for newcomersGood for newcomers
Description
Summary
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
- 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 expired
if session.ExpiresAt.Before(time.Now()) {
// Expired session - continue without user
// TODO: Consider cleaning up expired session here
return next(c)
}Expected Fix
Delete the expired session before continuing:
// Check if session is expired
if session.ExpiresAt.Before(time.Now()) {
// Clean up expired session
if err := storage.DeleteSession(c.Request().Context(), cookie.Value); err != nil {
c.Logger().Errorf("Failed to delete expired session: %v", err)
}
return next(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
storageparameter already hasDeleteSessionmethod (seestorage.go:191) - Similar cleanup patterns exist in
handlers.go(lines 896, 1206, 1299) - Error from deletion should be logged but not block the request
Estimated Time
30-45 minutes
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
bugSomething isn't workingSomething isn't workinggood first issueGood for newcomersGood for newcomers
Type
Projects
Status
Done