From b9e7043ec165be60dddd411a15c8aaf02f399dc7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dalibor=20Markovi=C4=87?= Date: Fri, 31 Jul 2026 03:00:33 +0200 Subject: [PATCH] =?UTF-8?q?Podsetnici:=20spre=C4=8Den=20IDOR=20=E2=80=94?= =?UTF-8?q?=20provera=20vlasni=C5=A1tva=20pre=20izmene/zavr=C5=A1avanja/br?= =?UTF-8?q?isanja?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- TODO_pregled_koda.md | 3 +- internal/handler/podsetnici.go | 39 ++++++++++++++++++++++++++ internal/handler/podsetnici_test.go | 43 +++++++++++++++++++++++++++++ 3 files changed, 84 insertions(+), 1 deletion(-) create mode 100644 internal/handler/podsetnici_test.go diff --git a/TODO_pregled_koda.md b/TODO_pregled_koda.md index ed7be95..d4231ac 100644 --- a/TODO_pregled_koda.md +++ b/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. diff --git a/internal/handler/podsetnici.go b/internal/handler/podsetnici.go index b23bb1e..fdd8735 100644 --- a/internal/handler/podsetnici.go +++ b/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")) diff --git a/internal/handler/podsetnici_test.go b/internal/handler/podsetnici_test.go new file mode 100644 index 0000000..b514bc0 --- /dev/null +++ b/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) + } + }) + } +}