From d988d35881a4d3dbdb3d292226206a1f43db4d01 Mon Sep 17 00:00:00 2001 From: Fendy Date: Wed, 26 Aug 2026 23:44:19 +0800 Subject: [PATCH] fix: address branch review issues (json tags, share lockout, session expiry, file unlink) --- internal/server/files.go | 6 ++++++ internal/server/middleware.go | 21 +++++++++++++++++++-- internal/server/server.go | 20 ++++++++++++-------- internal/server/share.go | 23 +++++++++++++++++++++++ internal/store/files.go | 12 ++++++------ tests/integration_test.go | 2 +- 6 files changed, 67 insertions(+), 17 deletions(-) diff --git a/internal/server/files.go b/internal/server/files.go index b75800d..1c676fe 100644 --- a/internal/server/files.go +++ b/internal/server/files.go @@ -80,9 +80,15 @@ func (s *Server) handleDownload(w http.ResponseWriter, r *http.Request) { func (s *Server) handleDeleteFile(w http.ResponseWriter, r *http.Request) { id := r.PathValue("id") + f, err := s.store.FileGet(id) + if err != nil { + http.Error(w, "Not found", http.StatusNotFound) + return + } if err := s.store.FileDelete(id); err != nil { http.Error(w, "Failed to delete", http.StatusInternalServerError) return } + os.Remove(filepath.Join(s.workspace, f.Dir, f.StorageName)) w.WriteHeader(http.StatusNoContent) } diff --git a/internal/server/middleware.go b/internal/server/middleware.go index 8981d1f..4cde4ab 100644 --- a/internal/server/middleware.go +++ b/internal/server/middleware.go @@ -2,8 +2,11 @@ package server import ( "net/http" + "time" ) +const sessionMaxAge = 7 * 86400 + func (s *Server) requireAuth(next http.HandlerFunc) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { cookie, err := r.Cookie("ydropbox_session") @@ -13,7 +16,14 @@ func (s *Server) requireAuth(next http.HandlerFunc) http.HandlerFunc { } s.mu.Lock() - _, exists := s.sessions[cookie.Value] + createdAt, exists := s.sessions[cookie.Value] + if exists && time.Now().Unix()-createdAt > sessionMaxAge { + delete(s.sessions, cookie.Value) + exists = false + } + if exists { + s.sessions[cookie.Value] = time.Now().Unix() + } s.mu.Unlock() if !exists { @@ -34,7 +44,14 @@ func (s *Server) requireAuthPage(next http.HandlerFunc) http.HandlerFunc { } s.mu.Lock() - _, exists := s.sessions[cookie.Value] + createdAt, exists := s.sessions[cookie.Value] + if exists && time.Now().Unix()-createdAt > sessionMaxAge { + delete(s.sessions, cookie.Value) + exists = false + } + if exists { + s.sessions[cookie.Value] = time.Now().Unix() + } s.mu.Unlock() if !exists { diff --git a/internal/server/server.go b/internal/server/server.go index ed8e8cd..590c231 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -11,14 +11,16 @@ import ( ) type Server struct { - store *store.Store - accessToken string - sessions map[string]int64 - shareSessions map[string]string - loginAttempts map[string]int - loginLockout map[string]time.Time - mu sync.Mutex - workspace string + store *store.Store + accessToken string + sessions map[string]int64 + shareSessions map[string]string + loginAttempts map[string]int + loginLockout map[string]time.Time + shareAttempts map[string]int + shareLockout map[string]time.Time + mu sync.Mutex + workspace string } func New(store *store.Store, accessToken string, workspace string) *Server { @@ -29,6 +31,8 @@ func New(store *store.Store, accessToken string, workspace string) *Server { shareSessions: make(map[string]string), loginAttempts: make(map[string]int), loginLockout: make(map[string]time.Time), + shareAttempts: make(map[string]int), + shareLockout: make(map[string]time.Time), workspace: workspace, } } diff --git a/internal/server/share.go b/internal/server/share.go index d5ad892..94c5152 100644 --- a/internal/server/share.go +++ b/internal/server/share.go @@ -108,12 +108,35 @@ func (s *Server) handleSharePassword(w http.ResponseWriter, r *http.Request) { return } + key := share.ID + s.mu.Lock() + if lockTime, locked := s.shareLockout[key]; locked && time.Now().Before(lockTime) { + s.mu.Unlock() + http.Error(w, "Too many attempts, locked for 60s", http.StatusForbidden) + return + } + s.mu.Unlock() + + time.Sleep(500 * time.Millisecond) + password := r.FormValue("password") if err := bcrypt.CompareHashAndPassword([]byte(share.PasswordHash), []byte(password)); err != nil { + s.mu.Lock() + s.shareAttempts[key]++ + if s.shareAttempts[key] >= 5 { + s.shareLockout[key] = time.Now().Add(60 * time.Second) + s.shareAttempts[key] = 0 + } + s.mu.Unlock() http.Error(w, "Wrong password", http.StatusForbidden) return } + s.mu.Lock() + delete(s.shareAttempts, key) + delete(s.shareLockout, key) + s.mu.Unlock() + cookieVal := generateSessionID() s.mu.Lock() if s.shareSessions == nil { diff --git a/internal/store/files.go b/internal/store/files.go index 45dd71a..14ea18f 100644 --- a/internal/store/files.go +++ b/internal/store/files.go @@ -3,12 +3,12 @@ package store import "database/sql" type File struct { - ID string - OriginalName string - StorageName string - Dir string - Size int64 - CreatedAt int64 + ID string `json:"id"` + OriginalName string `json:"name"` + StorageName string `json:"-"` + Dir string `json:"dir"` + Size int64 `json:"size"` + CreatedAt int64 `json:"created_at"` } func (s *Store) FileCreate(f *File) error { diff --git a/tests/integration_test.go b/tests/integration_test.go index e6914e1..54952b9 100644 --- a/tests/integration_test.go +++ b/tests/integration_test.go @@ -60,7 +60,7 @@ func TestIntegration(t *testing.T) { var uploadResp map[string]any json.NewDecoder(uploadRes.Body).Decode(&uploadResp) file := uploadResp["file"].(map[string]any) - fileID := file["ID"].(string) + fileID := file["id"].(string) listReq, _ := http.NewRequest("GET", ts.URL+"/api/files?dir=inbox", nil) for _, cookie := range loginRes.Cookies() {