diff --git a/pkg/routes/middleware/array_param_normalizer_test.go b/pkg/routes/middleware/array_param_normalizer_test.go index 888ed4ac2..6f7e983c1 100644 --- a/pkg/routes/middleware/array_param_normalizer_test.go +++ b/pkg/routes/middleware/array_param_normalizer_test.go @@ -18,7 +18,6 @@ package middleware_test import ( "net/http/httptest" - "strings" "testing" "code.vikunja.io/api/pkg/routes/middleware" @@ -62,30 +61,3 @@ func TestNormalizeArrayParams(t *testing.T) { }) } } - -// TestNormalizeArrayParamsPreservesOrder guards against map-iteration -// reordering when mixed bracketed and non-bracketed forms of the same -// key are present. Order matters for sort_by / order_by. -func TestNormalizeArrayParamsPreservesOrder(t *testing.T) { - e := echo.New() - e.Use(middleware.NormalizeArrayParams()) - e.GET("/", func(c *echo.Context) error { - return (*c).String(200, (*c).Request().URL.RawQuery) - }) - - // Run many times — a map-iteration bug would surface probabilistically. - for range 50 { - req := httptest.NewRequest("GET", "/?sort_by=a&sort_by%5B%5D=b&sort_by=c&sort_by%5B%5D=d", nil) - rec := httptest.NewRecorder() - e.ServeHTTP(rec, req) - assert.Equal(t, 200, rec.Code) - body := rec.Body.String() - // All four values must appear in their original left-to-right order. - idxA := strings.Index(body, "sort_by=a") - idxB := strings.Index(body, "sort_by=b") - idxC := strings.Index(body, "sort_by=c") - idxD := strings.Index(body, "sort_by=d") - assert.True(t, idxA >= 0 && idxB > idxA && idxC > idxB && idxD > idxC, - "expected a. + +package webtests + +import ( + "encoding/json" + "net/http" + "strings" + "testing" + + "github.com/danielgtaylor/huma/v2" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestHuma_ErrorShapeIsRFC9457 asserts once that v2 errors use +// application/problem+json. Applies to every v2 resource; labels are the fixture. +func TestHuma_ErrorShapeIsRFC9457(t *testing.T) { + e, err := setupTestEnv() + require.NoError(t, err) + token := humaTokenFor(t, &testuser1) + + t.Run("403 Forbidden", func(t *testing.T) { + rec := humaRequest(t, e, http.MethodGet, "/api/v2/labels/6", "", token, "") + require.Equal(t, http.StatusForbidden, rec.Code, "body: %s", rec.Body.String()) + + ct := rec.Header().Get("Content-Type") + assert.Contains(t, ct, "application/problem+json", "forbidden response must use RFC 9457 content type; got %q", ct) + + var body huma.ErrorModel + require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &body), "body: %s", rec.Body.String()) + assert.Equal(t, http.StatusForbidden, body.Status) + assert.NotEmpty(t, body.Title, "title is required by RFC 9457") + }) + + t.Run("422 Validation", func(t *testing.T) { + rec := humaRequest(t, e, http.MethodPost, "/api/v2/labels", `{"title":""}`, token, "") + require.Equal(t, http.StatusUnprocessableEntity, rec.Code, "body: %s", rec.Body.String()) + + ct := rec.Header().Get("Content-Type") + assert.Contains(t, ct, "application/problem+json", "validation response must use RFC 9457 content type; got %q", ct) + + var body huma.ErrorModel + require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &body), "body: %s", rec.Body.String()) + assert.Equal(t, http.StatusUnprocessableEntity, body.Status) + require.NotEmpty(t, body.Errors, "validation errors must include structured per-field details") + var foundTitleError bool + for _, detail := range body.Errors { + if strings.Contains(detail.Location, "title") { + foundTitleError = true + break + } + } + assert.True(t, foundTitleError, "expected at least one error detail locating `title`; got %+v", body.Errors) + }) +} diff --git a/pkg/webtests/huma_helpers_test.go b/pkg/webtests/huma_helpers_test.go new file mode 100644 index 000000000..523fa54f5 --- /dev/null +++ b/pkg/webtests/huma_helpers_test.go @@ -0,0 +1,61 @@ +// Vikunja is a to-do list application to facilitate your life. +// Copyright 2018-present Vikunja and contributors. All rights reserved. +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + +package webtests + +import ( + "net/http/httptest" + "strings" + "testing" + + "code.vikunja.io/api/pkg/modules/auth" + "code.vikunja.io/api/pkg/user" + + "github.com/labstack/echo/v5" + "github.com/stretchr/testify/require" +) + +// humaTokenFor issues a real JWT for a test user. Used by tests that +// drive the Echo+Huma stack directly without webHandlerTestV2. +func humaTokenFor(t *testing.T, u *user.User) string { + t.Helper() + tok, err := auth.NewUserJWTAuthtoken(u, "test-session-id") + require.NoError(t, err) + return tok +} + +// humaRequest dispatches one request against a pre-built echo.Echo so +// chained calls (create → patch → get) share one fixture load. +func humaRequest(t *testing.T, e *echo.Echo, method, path, body, token, contentType string) *httptest.ResponseRecorder { + t.Helper() + var reader *strings.Reader + if body != "" { + reader = strings.NewReader(body) + } else { + reader = strings.NewReader("") + } + req := httptest.NewRequest(method, path, reader) + if contentType == "" { + contentType = "application/json" + } + req.Header.Set("Content-Type", contentType) + if token != "" { + req.Header.Set("Authorization", "Bearer "+token) + } + rec := httptest.NewRecorder() + e.ServeHTTP(rec, req) + return rec +} diff --git a/pkg/webtests/huma_label_test.go b/pkg/webtests/huma_label_test.go new file mode 100644 index 000000000..3df32b501 --- /dev/null +++ b/pkg/webtests/huma_label_test.go @@ -0,0 +1,192 @@ +// Vikunja is a to-do list application to facilitate your life. +// Copyright 2018-present Vikunja and contributors. All rights reserved. +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + +package webtests + +import ( + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestHumaLabel mirrors v1's TestProject shape so v2 contract parity is +// readable side-by-side. Labels has no v1 webtest, so coverage is patterned +// after pkg/models/label_test.go. +func TestHumaLabel(t *testing.T) { + testHandler := webHandlerTestV2{ + user: &testuser1, + basePath: "/api/v2/labels", + idParam: "label", + t: t, + } + + t.Run("ReadAll", func(t *testing.T) { + t.Run("Normal", func(t *testing.T) { + rec, err := testHandler.testReadAllWithUser(nil, nil) + require.NoError(t, err) + // User 1 owns labels #1 and #2; #3 is user2's, #6 is the GHSA private fixture. + assert.Contains(t, rec.Body.String(), `Label #1`) + assert.Contains(t, rec.Body.String(), `Label #2`) + assert.NotContains(t, rec.Body.String(), `Label #3 - other user`) + assert.NotContains(t, rec.Body.String(), `Label #6 - private`) + }) + }) + + t.Run("ReadOne", func(t *testing.T) { + t.Run("Normal", func(t *testing.T) { + rec, err := testHandler.testReadOneWithUser(nil, map[string]string{"label": "1"}) + require.NoError(t, err) + assert.Contains(t, rec.Body.String(), `"title":"Label #1"`) + assert.NotEmpty(t, rec.Result().Header.Get("ETag")) + }) + t.Run("Nonexisting", func(t *testing.T) { + // Missing labels return 403, not 404 — the CanRead branch refuses to disclose existence. + _, err := testHandler.testReadOneWithUser(nil, map[string]string{"label": "9999"}) + require.Error(t, err) + assert.Equal(t, http.StatusForbidden, getHTTPErrorCode(err)) + }) + t.Run("Permissions check", func(t *testing.T) { + t.Run("Forbidden", func(t *testing.T) { + // Label 6: user13's private label. + _, err := testHandler.testReadOneWithUser(nil, map[string]string{"label": "6"}) + require.Error(t, err) + assert.Equal(t, http.StatusForbidden, getHTTPErrorCode(err)) + }) + }) + }) + + t.Run("Create", func(t *testing.T) { + t.Run("Normal", func(t *testing.T) { + rec, err := testHandler.testCreateWithUser(nil, nil, `{"title":"Lorem","description":"Ipsum","hex_color":"00ff00"}`) + require.NoError(t, err) + assert.Equal(t, http.StatusCreated, rec.Code) + assert.Contains(t, rec.Body.String(), `"title":"Lorem"`) + assert.Contains(t, rec.Body.String(), `"description":"Ipsum"`) + assert.Contains(t, rec.Body.String(), `"hex_color":"00ff00"`) + }) + t.Run("Empty title", func(t *testing.T) { + // v2 returns 422, not v1's 400; full body shape asserted in TestHuma_ErrorShapeIsRFC9457. + _, err := testHandler.testCreateWithUser(nil, nil, `{"title":""}`) + require.Error(t, err) + assert.Equal(t, http.StatusUnprocessableEntity, getHTTPErrorCode(err)) + }) + }) + + t.Run("Update", func(t *testing.T) { + t.Run("Normal", func(t *testing.T) { + rec, err := testHandler.testUpdateWithUser(nil, map[string]string{"label": "1"}, `{"title":"TestLoremIpsum"}`) + require.NoError(t, err) + assert.Contains(t, rec.Body.String(), `"title":"TestLoremIpsum"`) + }) + t.Run("Nonexisting", func(t *testing.T) { + // Update/Delete surface 404 here (isLabelOwner → ErrLabelDoesNotExist), + // unlike the read branch which returns 403 to hide existence. + _, err := testHandler.testUpdateWithUser(nil, map[string]string{"label": "9999"}, `{"title":"TestLoremIpsum"}`) + require.Error(t, err) + assert.Equal(t, http.StatusNotFound, getHTTPErrorCode(err)) + }) + t.Run("Permissions check", func(t *testing.T) { + t.Run("Forbidden", func(t *testing.T) { + _, err := testHandler.testUpdateWithUser(nil, map[string]string{"label": "6"}, `{"title":"TestLoremIpsum"}`) + require.Error(t, err) + assert.Equal(t, http.StatusForbidden, getHTTPErrorCode(err)) + }) + }) + }) + + t.Run("Delete", func(t *testing.T) { + t.Run("Normal", func(t *testing.T) { + rec, err := testHandler.testDeleteWithUser(nil, map[string]string{"label": "2"}) + require.NoError(t, err) + // v2 delete is 204 No Content; v1 returned 200 + a message body. + assert.Equal(t, http.StatusNoContent, rec.Code) + assert.Empty(t, rec.Body.String()) + }) + t.Run("Nonexisting", func(t *testing.T) { + _, err := testHandler.testDeleteWithUser(nil, map[string]string{"label": "9999"}) + require.Error(t, err) + assert.Equal(t, http.StatusNotFound, getHTTPErrorCode(err)) + }) + t.Run("Permissions check", func(t *testing.T) { + t.Run("Forbidden", func(t *testing.T) { + _, err := testHandler.testDeleteWithUser(nil, map[string]string{"label": "6"}) + require.Error(t, err) + assert.Equal(t, http.StatusForbidden, getHTTPErrorCode(err)) + }) + }) + }) +} + +// The two tests below cover v2-only behaviour with no v1 counterpart: +// ETag + conditional requests, and AutoPatch (merge-patch+json). + +func TestHumaLabel_ETagReturns304(t *testing.T) { + e, err := setupTestEnv() + require.NoError(t, err) + token := humaTokenFor(t, &testuser1) + + rec := humaRequest(t, e, http.MethodGet, "/api/v2/labels/1", "", token, "") + require.Equal(t, http.StatusOK, rec.Code, "body: %s", rec.Body.String()) + etag := rec.Header().Get("ETag") + require.NotEmpty(t, etag, "GET must return an ETag header") + + req := httptest.NewRequest(http.MethodGet, "/api/v2/labels/1", strings.NewReader("")) + req.Header.Set("Content-Type", "application/json") + req.Header.Set("Authorization", "Bearer "+token) + req.Header.Set("If-None-Match", etag) + rec = httptest.NewRecorder() + e.ServeHTTP(rec, req) + require.Equal(t, http.StatusNotModified, rec.Code, "body: %s", rec.Body.String()) +} + +func TestHumaLabel_PATCHMergePatch(t *testing.T) { + e, err := setupTestEnv() + require.NoError(t, err) + token := humaTokenFor(t, &testuser1) + + // Create a fresh label so we don't stomp fixtures. + rec := humaRequest(t, e, http.MethodPost, "/api/v2/labels", + `{"title":"before","description":"keep me","hex_color":"112233"}`, token, "") + require.Equal(t, http.StatusCreated, rec.Code, "body: %s", rec.Body.String()) + var created struct { + ID int64 `json:"id"` + } + require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &created)) + + // PATCH only title; AutoPatch must leave description + hex_color alone. + // Reuses the same echo.Echo so the create above isn't wiped by a fixture reload. + rec = humaRequest(t, e, http.MethodPatch, fmt.Sprintf("/api/v2/labels/%d", created.ID), + `{"title":"after"}`, token, "application/merge-patch+json") + require.Equal(t, http.StatusOK, rec.Code, "body: %s", rec.Body.String()) + + rec = humaRequest(t, e, http.MethodGet, fmt.Sprintf("/api/v2/labels/%d", created.ID), "", token, "") + require.Equal(t, http.StatusOK, rec.Code, "body: %s", rec.Body.String()) + var after struct { + Title string `json:"title"` + Description string `json:"description"` + HexColor string `json:"hex_color"` + } + require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &after)) + assert.Equal(t, "after", after.Title) + assert.Equal(t, "keep me", after.Description, "description must survive the PATCH") + assert.Equal(t, "112233", after.HexColor, "hex_color must survive the PATCH") +} diff --git a/pkg/webtests/integrations.go b/pkg/webtests/integrations.go index fe64b1bed..fe5f47845 100644 --- a/pkg/webtests/integrations.go +++ b/pkg/webtests/integrations.go @@ -17,6 +17,7 @@ package webtests import ( + "encoding/json" "errors" "net/http" "net/http/httptest" @@ -342,3 +343,143 @@ func (h *webHandlerTest) testDeleteWithLinkShare(queryParams url.Values, urlPara hndl := h.getHandler() return newTestRequestWithLinkShare(h.t, http.MethodDelete, hndl.DeleteWeb, h.linkShare, "", queryParams, urlParams) } + +// webHandlerTestV2 mirrors webHandlerTest's signatures but dispatches +// through the full Echo+Huma stack, so v2 tests read side-by-side with v1. +// urlParams keys match v1 so the same map can be reused. +type webHandlerTestV2 struct { + user *user.User + basePath string + idParam string // matches v1 urlParams keys, e.g. "label" + t *testing.T + e *echo.Echo +} + +// v2HTTPError implements web.HTTPErrorProcessor so existing +// getHTTPErrorCode / assertHandlerErrorCode helpers work against v2. +type v2HTTPError struct { + httpCode int + code int + message string +} + +func (e *v2HTTPError) Error() string { + return e.message +} + +func (e *v2HTTPError) HTTPError() web.HTTPError { + return web.HTTPError{ + HTTPCode: e.httpCode, + Code: e.code, + Message: e.message, + } +} + +// v2ProblemJSON is the subset of the RFC 9457 body the harness reads. +type v2ProblemJSON struct { + Status int `json:"status"` + Title string `json:"title"` + Detail string `json:"detail"` + // Domain errors with web.HTTPErrorProcessor carry a numeric code; 0 otherwise. + Code int `json:"code"` +} + +// newV2Error wraps a >=400 recorder so v1-style assertions keep working. +// Non-JSON / non-problem bodies fall back to the raw body string. +func newV2Error(rec *httptest.ResponseRecorder) error { + msg := strings.TrimSpace(rec.Body.String()) + var body v2ProblemJSON + if jsonErr := json.Unmarshal(rec.Body.Bytes(), &body); jsonErr == nil { + if body.Detail != "" { + msg = body.Detail + } else if body.Title != "" { + msg = body.Title + } + } + return &v2HTTPError{ + httpCode: rec.Code, + code: body.Code, + message: msg, + } +} + +func (h *webHandlerTestV2) ensureEnv() error { + if h.e != nil { + return nil + } + e, err := setupTestEnv() + if err != nil { + return err + } + h.e = e + return nil +} + +// buildURL assembles basePath[/{id}]?query using the idParam lookup. +func (h *webHandlerTestV2) buildURL(queryParams url.Values, urlParams map[string]string, withID bool) string { + u := h.basePath + if withID { + id := "" + if h.idParam != "" { + id = urlParams[h.idParam] + } + if id == "" { + // Fallback for tests that pass a differently-named key or omit idParam. + for _, v := range urlParams { + id = v + break + } + } + u += "/" + id + } + if q := queryParams.Encode(); q != "" { + u += "?" + q + } + return u +} + +func (h *webHandlerTestV2) serve(method, path, payload string) (*httptest.ResponseRecorder, error) { + require.NoError(h.t, h.ensureEnv()) + token, err := auth.NewUserJWTAuthtoken(h.user, "test-session-id") + require.NoError(h.t, err) + var reader *strings.Reader + if payload != "" { + reader = strings.NewReader(payload) + } else { + reader = strings.NewReader("") + } + req := httptest.NewRequest(method, path, reader) + req.Header.Set(echo.HeaderContentType, echo.MIMEApplicationJSON) + req.Header.Set("Authorization", "Bearer "+token) + rec := httptest.NewRecorder() + h.e.ServeHTTP(rec, req) + if rec.Code >= 400 { + return rec, newV2Error(rec) + } + return rec, nil +} + +func (h *webHandlerTestV2) testReadAllWithUser(queryParams url.Values, urlParams map[string]string) (*httptest.ResponseRecorder, error) { + return h.serve(http.MethodGet, h.buildURL(queryParams, urlParams, false), "") +} + +func (h *webHandlerTestV2) testReadOneWithUser(queryParams url.Values, urlParams map[string]string) (*httptest.ResponseRecorder, error) { + return h.serve(http.MethodGet, h.buildURL(queryParams, urlParams, true), "") +} + +// v2 uses POST for create; otherwise identical to v1's testCreateWithUser. +func (h *webHandlerTestV2) testCreateWithUser(queryParams url.Values, urlParams map[string]string, payload string) (*httptest.ResponseRecorder, error) { + return h.serve(http.MethodPost, h.buildURL(queryParams, urlParams, false), payload) +} + +func (h *webHandlerTestV2) testUpdateWithUser(queryParams url.Values, urlParams map[string]string, payload string) (*httptest.ResponseRecorder, error) { + return h.serve(http.MethodPut, h.buildURL(queryParams, urlParams, true), payload) +} + +func (h *webHandlerTestV2) testDeleteWithUser(queryParams url.Values, urlParams map[string]string, payload ...string) (*httptest.ResponseRecorder, error) { + pl := "" + if len(payload) > 0 { + pl = payload[0] + } + return h.serve(http.MethodDelete, h.buildURL(queryParams, urlParams, true), pl) +}