From b3529c61bb205d71eebcee343324e6d9410c79be Mon Sep 17 00:00:00 2001 From: Sergio Vera Date: Thu, 25 Jul 2024 10:17:14 +0200 Subject: [PATCH] Use HTTP delete verb for deleting docs and users --- .../webserver/controller/document/delete.go | 8 +---- .../highlight/{highlight.go => create.go} | 2 +- .../highlight/{remove.go => delete.go} | 2 +- .../highlight/{highlights.go => list.go} | 2 +- internal/webserver/controller/user/delete.go | 4 +-- internal/webserver/embedded/js/delete.js | 16 +++------- .../embedded/views/partials/delete-modal.html | 1 - .../webserver/embedded/views/users/index.html | 2 +- internal/webserver/highlights_test.go | 16 ++-------- internal/webserver/remove_document_test.go | 15 ++------- internal/webserver/routes.go | 10 +++--- internal/webserver/user_management_test.go | 31 +++---------------- 12 files changed, 26 insertions(+), 83 deletions(-) rename internal/webserver/controller/highlight/{highlight.go => create.go} (86%) rename internal/webserver/controller/highlight/{remove.go => delete.go} (86%) rename internal/webserver/controller/highlight/{highlights.go => list.go} (97%) diff --git a/internal/webserver/controller/document/delete.go b/internal/webserver/controller/document/delete.go index f5f8876..eee18d7 100644 --- a/internal/webserver/controller/document/delete.go +++ b/internal/webserver/controller/document/delete.go @@ -1,7 +1,6 @@ package document import ( - "fmt" "log" "path/filepath" @@ -9,13 +8,8 @@ import ( ) func (d *Controller) Delete(c *fiber.Ctx) error { - if c.FormValue("id") == "" { - return fiber.ErrBadRequest - } - - document, err := d.idx.Document(c.FormValue("id")) + document, err := d.idx.Document(c.Params("slug")) if err != nil { - fmt.Println(err) return fiber.ErrBadRequest } diff --git a/internal/webserver/controller/highlight/highlight.go b/internal/webserver/controller/highlight/create.go similarity index 86% rename from internal/webserver/controller/highlight/highlight.go rename to internal/webserver/controller/highlight/create.go index 829dd58..f201d4a 100644 --- a/internal/webserver/controller/highlight/highlight.go +++ b/internal/webserver/controller/highlight/create.go @@ -5,7 +5,7 @@ import ( "github.com/svera/coreander/v4/internal/webserver/model" ) -func (h *Controller) Highlight(c *fiber.Ctx) error { +func (h *Controller) Create(c *fiber.Ctx) error { user := c.Locals("Session").(model.Session) document, err := h.idx.Document(c.Params("slug")) diff --git a/internal/webserver/controller/highlight/remove.go b/internal/webserver/controller/highlight/delete.go similarity index 86% rename from internal/webserver/controller/highlight/remove.go rename to internal/webserver/controller/highlight/delete.go index 7813525..3c4f419 100644 --- a/internal/webserver/controller/highlight/remove.go +++ b/internal/webserver/controller/highlight/delete.go @@ -5,7 +5,7 @@ import ( "github.com/svera/coreander/v4/internal/webserver/model" ) -func (h *Controller) Remove(c *fiber.Ctx) error { +func (h *Controller) Delete(c *fiber.Ctx) error { user := c.Locals("Session").(model.Session) document, err := h.idx.Document(c.Params("slug")) diff --git a/internal/webserver/controller/highlight/highlights.go b/internal/webserver/controller/highlight/list.go similarity index 97% rename from internal/webserver/controller/highlight/highlights.go rename to internal/webserver/controller/highlight/list.go index cb209f7..10fcd73 100644 --- a/internal/webserver/controller/highlight/highlights.go +++ b/internal/webserver/controller/highlight/list.go @@ -12,7 +12,7 @@ import ( "github.com/svera/coreander/v4/internal/webserver/view" ) -func (h *Controller) Highlights(c *fiber.Ctx) error { +func (h *Controller) List(c *fiber.Ctx) error { emailSendingConfigured := true if _, ok := h.sender.(*infrastructure.NoEmail); ok { emailSendingConfigured = false diff --git a/internal/webserver/controller/user/delete.go b/internal/webserver/controller/user/delete.go index 89e9c75..6ac9d52 100644 --- a/internal/webserver/controller/user/delete.go +++ b/internal/webserver/controller/user/delete.go @@ -7,7 +7,7 @@ import ( // Delete removes a user from the database func (u *Controller) Delete(c *fiber.Ctx) error { - user, err := u.repository.FindByUuid(c.FormValue("id")) + user, err := u.repository.FindByUsername(c.Params("username")) if err != nil { return fiber.ErrInternalServerError } @@ -20,7 +20,7 @@ func (u *Controller) Delete(c *fiber.Ctx) error { return fiber.ErrForbidden } - if err = u.repository.Delete(c.FormValue("id")); err != nil { + if err = u.repository.Delete(user.Uuid); err != nil { return fiber.ErrInternalServerError } diff --git a/internal/webserver/embedded/js/delete.js b/internal/webserver/embedded/js/delete.js index 788eb76..78a94ba 100644 --- a/internal/webserver/embedded/js/delete.js +++ b/internal/webserver/embedded/js/delete.js @@ -9,13 +9,11 @@ const deleteModal = document.getElementById('delete-modal'); const deleteForm = document.getElementById('delete-form'); +let id deleteModal.addEventListener('show.bs.modal', event => { const link = event.relatedTarget - const id = link.getAttribute('data-id') - const modalInput = deleteModal.querySelector('.id') - - modalInput.value = id; + id = link.getAttribute('data-id') }) deleteModal.addEventListener('hidden.bs.modal', event => { @@ -25,14 +23,8 @@ deleteModal.addEventListener('hidden.bs.modal', event => { deleteForm.addEventListener('submit', event => { event.preventDefault(); - fetch(deleteForm.getAttribute("action"), { - method: "DELETE", - headers: { - 'Content-Type': 'application/x-www-form-urlencoded' - }, - body: new URLSearchParams({ - 'id': deleteForm.elements['id'].value, - }) + fetch(deleteForm.getAttribute("action") + '/' + id, { + method: "DELETE" }) .then((response) => { if (response.ok || response.status == "403") { diff --git a/internal/webserver/embedded/views/partials/delete-modal.html b/internal/webserver/embedded/views/partials/delete-modal.html index cb4f2f9..ebed992 100644 --- a/internal/webserver/embedded/views/partials/delete-modal.html +++ b/internal/webserver/embedded/views/partials/delete-modal.html @@ -17,7 +17,6 @@

{{t .Lang .ModalHeader}} - diff --git a/internal/webserver/embedded/views/users/index.html b/internal/webserver/embedded/views/users/index.html index 4079b9b..c463bbe 100644 --- a/internal/webserver/embedded/views/users/index.html +++ b/internal/webserver/embedded/views/users/index.html @@ -26,7 +26,7 @@

{{t $lang "Users"}}

{{end}} {{ if not (and (eq $admins 1) (eq $user.Role 2)) }} - + diff --git a/internal/webserver/highlights_test.go b/internal/webserver/highlights_test.go index fbc8070..f8f55d7 100644 --- a/internal/webserver/highlights_test.go +++ b/internal/webserver/highlights_test.go @@ -19,7 +19,6 @@ func TestHighlights(t *testing.T) { db *gorm.DB app *fiber.App adminCookie *http.Cookie - data url.Values adminUser model.User ) @@ -34,9 +33,6 @@ func TestHighlights(t *testing.T) { t.Fatalf("Unexpected error: %v", err.Error()) } - data = url.Values{ - "slug": {"john-doe-test-epub"}, - } adminUser = model.User{} db.Where("email = ?", "admin@example.com").First(&adminUser) @@ -116,11 +112,7 @@ func TestHighlights(t *testing.T) { t.Fatalf("Unexpected error: %v", err.Error()) } - data = url.Values{ - "id": {"john-doe-test-epub"}, - } - - _, err = deleteRequest(data, adminCookie, app, "/documents", t) + _, err = deleteRequest(url.Values{}, adminCookie, app, "/documents/john-doe-test-epub", t) if err != nil { t.Fatalf("Unexpected error: %v", err.Error()) } @@ -158,11 +150,7 @@ func TestHighlights(t *testing.T) { t.Fatalf("Unexpected error: %v", err.Error()) } - data := url.Values{ - "id": {regularUser.Uuid}, - } - - _, err = deleteRequest(data, adminCookie, app, "/users", t) + _, err = deleteRequest(url.Values{}, adminCookie, app, fmt.Sprintf("/users/%s", regularUser.Username), t) if err != nil { t.Fatalf("Unexpected error: %v", err.Error()) } diff --git a/internal/webserver/remove_document_test.go b/internal/webserver/remove_document_test.go index 42ca6db..7e29b8e 100644 --- a/internal/webserver/remove_document_test.go +++ b/internal/webserver/remove_document_test.go @@ -1,11 +1,11 @@ package webserver_test import ( + "fmt" "log" "net/http" "net/url" "os" - "strings" "testing" "github.com/google/uuid" @@ -43,8 +43,7 @@ func TestRemoveDocument(t *testing.T) { slug string expectedHTTPStatus int }{ - {"Remove no document slug", "admin@example.com", "admin", "", "", http.StatusBadRequest}, - {"Remove non existing document slug", "admin@example.com", "admin", "wrong.epub", "", http.StatusBadRequest}, + {"Remove non existing document slug", "admin@example.com", "admin", "wrong.epub", "wrong-epub", http.StatusBadRequest}, {"Remove document with a regular user", "regular@example.com", "regular", "metadata.epub", "john-doe-test-epub", http.StatusForbidden}, {"Remove document with an admin user", "admin@example.com", "admin", "metadata.epub", "john-doe-test-epub", http.StatusOK}, } @@ -56,23 +55,15 @@ func TestRemoveDocument(t *testing.T) { err error ) - data := url.Values{ - "id": {tcase.slug}, - } - cookie, err := login(app, tcase.email, tcase.password, t) if err != nil { t.Fatalf("Unexpected error: %v", err.Error()) } - req, err := http.NewRequest(http.MethodDelete, "/documents", strings.NewReader(data.Encode())) + response, err = deleteRequest(url.Values{}, cookie, app, fmt.Sprintf("/documents/%s", tcase.slug), t) if err != nil { t.Fatalf("Unexpected error: %v", err.Error()) } - req.Header.Add("Content-Type", "application/x-www-form-urlencoded") - req.AddCookie(cookie) - - response, err = app.Test(req) if err != nil { t.Fatalf("Unexpected error: %v", err.Error()) diff --git a/internal/webserver/routes.go b/internal/webserver/routes.go index b328cd4..44c4026 100644 --- a/internal/webserver/routes.go +++ b/internal/webserver/routes.go @@ -59,13 +59,13 @@ func routes(app *fiber.App, controllers Controllers, jwtSecret []byte, sender Se usersGroup.Post("/", alwaysRequireAuthentication, RequireAdmin, controllers.Users.Create) usersGroup.Get("/:username", alwaysRequireAuthentication, controllers.Users.Edit) usersGroup.Put("/:username", alwaysRequireAuthentication, controllers.Users.Update) - app.Delete("/users", alwaysRequireAuthentication, RequireAdmin, controllers.Users.Delete) + app.Delete("/users/:username", alwaysRequireAuthentication, RequireAdmin, controllers.Users.Delete) - langGroup.Get("/highlights/:username", alwaysRequireAuthentication, controllers.Highlights.Highlights) - app.Post("/documents/:slug/highlight", alwaysRequireAuthentication, controllers.Highlights.Highlight) - app.Delete("/documents/:slug/highlight", alwaysRequireAuthentication, controllers.Highlights.Remove) + langGroup.Get("/highlights/:username", alwaysRequireAuthentication, controllers.Highlights.List) + app.Post("/documents/:slug/highlight", alwaysRequireAuthentication, controllers.Highlights.Create) + app.Delete("/documents/:slug/highlight", alwaysRequireAuthentication, controllers.Highlights.Delete) - app.Delete("/documents", alwaysRequireAuthentication, RequireAdmin, controllers.Documents.Delete) + app.Delete("/documents/:slug", alwaysRequireAuthentication, RequireAdmin, controllers.Documents.Delete) langGroup.Get("/upload", alwaysRequireAuthentication, RequireAdmin, controllers.Documents.UploadForm) langGroup.Post("/documents", alwaysRequireAuthentication, RequireAdmin, controllers.Documents.Upload) diff --git a/internal/webserver/user_management_test.go b/internal/webserver/user_management_test.go index 0394a3a..d814405 100644 --- a/internal/webserver/user_management_test.go +++ b/internal/webserver/user_management_test.go @@ -304,11 +304,7 @@ func TestUserManagement(t *testing.T) { t.Run("Try to delete a user without an active session", func(t *testing.T) { reset() - regularUserData = url.Values{ - "id": {regularUser.Uuid}, - } - - response, err := deleteRequest(regularUserData, &http.Cookie{}, app, "/users", t) + response, err := deleteRequest(url.Values{}, &http.Cookie{}, app, fmt.Sprintf("/users/%s", regularUser.Username), t) if response == nil { t.Fatalf("Unexpected error: %v", err.Error()) } @@ -319,13 +315,7 @@ func TestUserManagement(t *testing.T) { t.Run("Try to delete a user with a regular user's session", func(t *testing.T) { reset() - regularUserData = url.Values{ - "id": {regularUser.Uuid}, - } - - regularUserData.Set("name", "Updated test user") - - response, err := deleteRequest(regularUserData, regularUserCookie, app, "/users", t) + response, err := deleteRequest(url.Values{}, regularUserCookie, app, fmt.Sprintf("/users/%s", regularUser.Username), t) if response == nil { t.Fatalf("Unexpected error: %v", err.Error()) } @@ -336,11 +326,7 @@ func TestUserManagement(t *testing.T) { t.Run("Try to delete a user with an admin session", func(t *testing.T) { reset() - regularUserData = url.Values{ - "id": {regularUser.Uuid}, - } - - response, err := deleteRequest(regularUserData, adminCookie, app, "/users", t) + response, err := deleteRequest(url.Values{}, adminCookie, app, fmt.Sprintf("/users/%s", regularUser.Username), t) if response == nil { t.Fatalf("Unexpected error: %v", err.Error()) } @@ -355,10 +341,7 @@ func TestUserManagement(t *testing.T) { t.Run("Try to delete the only existing admin user", func(t *testing.T) { reset() - regularUserData = url.Values{ - "id": {adminUser.Uuid}, - } - response, err := deleteRequest(regularUserData, adminCookie, app, "/users", t) + response, err := deleteRequest(url.Values{}, adminCookie, app, fmt.Sprintf("/users/%s", adminUser.Username), t) if response == nil { t.Fatalf("Unexpected error: %v", err.Error()) } @@ -369,11 +352,7 @@ func TestUserManagement(t *testing.T) { t.Run("Try to delete a non existing user with an admin session", func(t *testing.T) { reset() - regularUserData = url.Values{ - "id": {"abcde"}, - } - - response, err := deleteRequest(regularUserData, adminCookie, app, "/users", t) + response, err := deleteRequest(url.Values{}, adminCookie, app, "/users/wrong", t) if response == nil { t.Fatalf("Unexpected error: %v", err.Error()) }