Podsetnici: sprečen IDOR — provera vlasništva pre izmene/završavanja/brisanja

TODO_pregled_koda.md

@@ -50,7 +50,8 @@ Legenda ozbiljnosti: 🔴 visok · 🟡 srednji · 🟢 nizak
 
 ## 4. Handler sloj — validacija, CSRF, autorizacija
 
-- [ ] 🔴 **IDOR / broken access control na podsetnicima (ličnim podsetnicima).**
+- [x] 🔴 **IDOR / broken access control na podsetnicima (ličnim podsetnicima). ISPRAVLJENO.**
+  Dodata `korisnikSmeDaMenjaPodsetnik(k, p)` provera (`internal/handler/podsetnici.go`) — dozvoljava izmenu/završavanje/brisanje ako je korisnik admin/superadmin (`middleware.JeAdmin`) ili je `p.KorisnikID` tačno njegov ID; u suprotnom `403 Forbidden`. Primenjeno u sve tri funkcije: `SacuvajIzmenePodsetnika` (sad prvo učitava postojeći podsetnik pre izmene), `OznaciPodsetnik` i `ObrisiPodsetnik` (sad prvo učitava podsetnik pre brisanja, ranije je brisao direktno po ID-u bez čitanja). Dodat `internal/handler/podsetnici_test.go` sa 6 test-slučajeva (radnik/tuđi radnik/admin × svoj/tuđi/nedodeljen podsetnik) — svi prolaze.
   `internal/handler/podsetnici.go`:
   - `SacuvajIzmenePodsetnika` (red 166-195) — učitava `id` iz URL-a, poziva `PodsetnikRepo.Izmeni` BEZ provere da `podsetnik.KorisnikID` pripada ulogovanom korisniku (ili je izmenilac admin/superadmin).
   - `OznaciPodsetnik` (red 198-218) — isto, menja status završenosti bilo kog podsetnika po ID-u bez provere vlasništva.

internal/handler/podsetnici.go

@@ -172,6 +172,16 @@ func (h *Handler) SacuvajIzmenePodsetnika(w http.ResponseWriter, r *http.Request
 		return
 	}
 
+	postojeci, err := h.PodsetnikRepo.DohvatiID(r.Context(), id)
+	if err != nil {
+		http.Error(w, "Podsetnik nije pronađen", http.StatusNotFound)
+		return
+	}
+	if !korisnikSmeDaMenjaPodsetnik(k, postojeci) {
+		http.Error(w, "Nemate dozvolu da menjate ovaj podsetnik", http.StatusForbidden)
+		return
+	}
+
 	if err := r.ParseForm(); err != nil {
 		http.Error(w, "Greška pri čitanju forme", http.StatusBadRequest)
 		return
@@ -196,6 +206,8 @@ func (h *Handler) SacuvajIzmenePodsetnika(w http.ResponseWriter, r *http.Request
 
 // OznaciPodsetnik prima POST zahtev i menja status završenosti podsetnika
 func (h *Handler) OznaciPodsetnik(w http.ResponseWriter, r *http.Request) {
+	k := middleware.KorisnikIzKonteksta(r.Context())
+
 	id, err := parseID(chi.URLParam(r, "id"))
 	if err != nil {
 		http.Error(w, "Neispravan ID podsetnika", http.StatusBadRequest)
@@ -208,6 +220,10 @@ func (h *Handler) OznaciPodsetnik(w http.ResponseWriter, r *http.Request) {
 		http.Error(w, "Podsetnik nije pronađen", http.StatusNotFound)
 		return
 	}
+	if !korisnikSmeDaMenjaPodsetnik(k, podsetnik) {
+		http.Error(w, "Nemate dozvolu da menjate ovaj podsetnik", http.StatusForbidden)
+		return
+	}
 
 	if err := h.PodsetnikRepo.OznaciZavrsenim(r.Context(), id, !podsetnik.Zavrseno); err != nil {
 		http.Error(w, "Greška pri ažuriranju statusa", http.StatusInternalServerError)
@@ -219,12 +235,24 @@ func (h *Handler) OznaciPodsetnik(w http.ResponseWriter, r *http.Request) {
 
 // ObrisiPodsetnik prima POST zahtev i briše podsetnik po ID-u
 func (h *Handler) ObrisiPodsetnik(w http.ResponseWriter, r *http.Request) {
+	k := middleware.KorisnikIzKonteksta(r.Context())
+
 	id, err := parseID(chi.URLParam(r, "id"))
 	if err != nil {
 		http.Error(w, "Neispravan ID podsetnika", http.StatusBadRequest)
 		return
 	}
 
+	podsetnik, err := h.PodsetnikRepo.DohvatiID(r.Context(), id)
+	if err != nil {
+		http.Error(w, "Podsetnik nije pronađen", http.StatusNotFound)
+		return
+	}
+	if !korisnikSmeDaMenjaPodsetnik(k, podsetnik) {
+		http.Error(w, "Nemate dozvolu da brišete ovaj podsetnik", http.StatusForbidden)
+		return
+	}
+
 	if err := h.PodsetnikRepo.Obrisi(r.Context(), id); err != nil {
 		http.Error(w, "Greška pri brisanju podsetnika", http.StatusInternalServerError)
 		return
@@ -233,6 +261,17 @@ func (h *Handler) ObrisiPodsetnik(w http.ResponseWriter, r *http.Request) {
 	http.Redirect(w, r, "/podsetnici?obrisan=1", http.StatusSeeOther)
 }
 
+// korisnikSmeDaMenjaPodsetnik proverava vlasništvo nad podsetnikom — sprečava da jedan
+// korisnik (npr. radnik) menja/završava/briše tuđi podsetnik pogađanjem ID-a u URL-u.
+// Admin/superadmin smeju sve (isti kriterijum kao za dodelu podsetnika drugom korisniku
+// u parseFormuPodsetnika); radnik sme samo podsetnik dodeljen njemu lično.
+func korisnikSmeDaMenjaPodsetnik(k *model.Korisnik, p *model.Podsetnik) bool {
+	if middleware.JeAdmin(k) {
+		return true
+	}
+	return p.KorisnikID != nil && k != nil && *p.KorisnikID == k.ID
+}
+
 // parseFormuPodsetnika čita polja iz HTTP forme, validira ih i vraća model i eventualnu grešku
 func parseFormuPodsetnika(r *http.Request, k *model.Korisnik) (model.Podsetnik, string) {
 	naslov := strings.TrimSpace(r.FormValue("naslov"))

internal/handler/podsetnici_test.go

@@ -0,0 +1,43 @@
+package handler
+
+import (
+	"testing"
+
+	"ntech/internal/model"
+)
+
+// TestKorisnikSmeDaMenjaPodsetnik proverava zaštitu od IDOR-a: radnik sme da menja/briše
+// samo sopstveni podsetnik, admin/superadmin smeju bilo koji.
+func TestKorisnikSmeDaMenjaPodsetnik(t *testing.T) {
+	radnik := &model.Korisnik{ID: 1, Uloga: "radnik"}
+	drugiRadnik := &model.Korisnik{ID: 2, Uloga: "radnik"}
+	admin := &model.Korisnik{ID: 3, Uloga: "admin"}
+
+	svojPodsetnik := &model.Podsetnik{ID: 100, KorisnikID: p(1)}
+	tudjPodsetnik := &model.Podsetnik{ID: 101, KorisnikID: p(2)}
+	nedodeljenPodsetnik := &model.Podsetnik{ID: 102, KorisnikID: nil}
+
+	slucajevi := []struct {
+		naziv     string
+		korisnik  *model.Korisnik
+		podsetnik *model.Podsetnik
+		ocekivano bool
+	}{
+		{"radnik menja svoj podsetnik", radnik, svojPodsetnik, true},
+		{"radnik ne sme tuđi podsetnik", radnik, tudjPodsetnik, false},
+		{"drugi radnik ne sme tuđi podsetnik", drugiRadnik, svojPodsetnik, false},
+		{"radnik ne sme nedodeljen podsetnik", radnik, nedodeljenPodsetnik, false},
+		{"admin sme tuđi podsetnik", admin, tudjPodsetnik, true},
+		{"admin sme nedodeljen podsetnik", admin, nedodeljenPodsetnik, true},
+	}
+
+	for _, sc := range slucajevi {
+		t.Run(sc.naziv, func(t *testing.T) {
+			dobijeno := korisnikSmeDaMenjaPodsetnik(sc.korisnik, sc.podsetnik)
+			if dobijeno != sc.ocekivano {
+				t.Errorf("korisnikSmeDaMenjaPodsetnik(%+v, %+v) = %v, očekivano %v",
+					sc.korisnik, sc.podsetnik, dobijeno, sc.ocekivano)
+			}
+		})
+	}
+}