Skip to content

Commit

Permalink
Use HTTP delete verb for deleting docs and users
Browse files Browse the repository at this point in the history
  • Loading branch information
svera authored Jul 25, 2024
1 parent 54ccf08 commit b3529c6
Show file tree
Hide file tree
Showing 12 changed files with 26 additions and 83 deletions.
8 changes: 1 addition & 7 deletions internal/webserver/controller/document/delete.go
Original file line number Diff line number Diff line change
@@ -1,21 +1,15 @@
package document

import (
"fmt"
"log"
"path/filepath"

"github.com/gofiber/fiber/v2"
)

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
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions internal/webserver/controller/user/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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
}

Expand Down
16 changes: 4 additions & 12 deletions internal/webserver/embedded/js/delete.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 => {
Expand All @@ -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") {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ <h1 class="modal-title fs-5" id="delete-modal-label">{{t .Lang .ModalHeader}}</h
"Cancel"}}</button>
<button type="submit" class="btn btn-primary">{{t .Lang "Delete"}}</button>
</div>
<input type="hidden" name="id" value="" class="id">
</form>
</div>
</div>
Expand Down
2 changes: 1 addition & 1 deletion internal/webserver/embedded/views/users/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ <h2>{{t $lang "Users"}}</h2>
{{end}}
</div>
{{ if not (and (eq $admins 1) (eq $user.Role 2)) }}
<a href="#" data-bs-toggle="modal" data-bs-target="#delete-modal" data-id="{{$user.Uuid}}">
<a href="#" data-bs-toggle="modal" data-bs-target="#delete-modal" data-id="{{$user.Username}}">
<svg xmlns="http://www.w3.org/2000/svg" width="16" height="16" fill="currentColor" class="bi bi-trash3-fill" viewBox="0 0 16 16">
<path d="M11 1.5v1h3.5a.5.5 0 0 1 0 1h-.538l-.853 10.66A2 2 0 0 1 11.115 16h-6.23a2 2 0 0 1-1.994-1.84L2.038 3.5H1.5a.5.5 0 0 1 0-1H5v-1A1.5 1.5 0 0 1 6.5 0h3A1.5 1.5 0 0 1 11 1.5Zm-5 0v1h4v-1a.5.5 0 0 0-.5-.5h-3a.5.5 0 0 0-.5.5ZM4.5 5.029l.5 8.5a.5.5 0 1 0 .998-.06l-.5-8.5a.5.5 0 1 0-.998.06Zm6.53-.528a.5.5 0 0 0-.528.47l-.5 8.5a.5.5 0 0 0 .998.058l.5-8.5a.5.5 0 0 0-.47-.528ZM8 4.5a.5.5 0 0 0-.5.5v8.5a.5.5 0 0 0 1 0V5a.5.5 0 0 0-.5-.5Z"/>
</svg>
Expand Down
16 changes: 2 additions & 14 deletions internal/webserver/highlights_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ func TestHighlights(t *testing.T) {
db *gorm.DB
app *fiber.App
adminCookie *http.Cookie
data url.Values
adminUser model.User
)

Expand All @@ -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 = ?", "[email protected]").First(&adminUser)

Expand Down Expand Up @@ -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())
}
Expand Down Expand Up @@ -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())
}
Expand Down
15 changes: 3 additions & 12 deletions internal/webserver/remove_document_test.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
package webserver_test

import (
"fmt"
"log"
"net/http"
"net/url"
"os"
"strings"
"testing"

"github.com/google/uuid"
Expand Down Expand Up @@ -43,8 +43,7 @@ func TestRemoveDocument(t *testing.T) {
slug string
expectedHTTPStatus int
}{
{"Remove no document slug", "[email protected]", "admin", "", "", http.StatusBadRequest},
{"Remove non existing document slug", "[email protected]", "admin", "wrong.epub", "", http.StatusBadRequest},
{"Remove non existing document slug", "[email protected]", "admin", "wrong.epub", "wrong-epub", http.StatusBadRequest},
{"Remove document with a regular user", "[email protected]", "regular", "metadata.epub", "john-doe-test-epub", http.StatusForbidden},
{"Remove document with an admin user", "[email protected]", "admin", "metadata.epub", "john-doe-test-epub", http.StatusOK},
}
Expand All @@ -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())
Expand Down
10 changes: 5 additions & 5 deletions internal/webserver/routes.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
31 changes: 5 additions & 26 deletions internal/webserver/user_management_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
Expand All @@ -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())
}
Expand All @@ -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())
}
Expand All @@ -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())
}
Expand All @@ -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())
}
Expand Down

0 comments on commit b3529c6

Please sign in to comment.