From 3af5eb8208591123b4501abe153b1cca51cd67e2 Mon Sep 17 00:00:00 2001 From: kolaente Date: Fri, 12 Jun 2026 10:00:49 +0200 Subject: [PATCH] feat(api/v2): add project background upload on /api/v2 Port PUT /projects/{project}/backgrounds/upload to the Huma-backed v2 API. The multipart handler reuses handler.ValidateAndSaveBackgroundUpload (shared with v1), checks project write access explicitly, and is gated on the upload provider config flag. Adds webtests covering the happy path, auth/permission failures, non-image rejection, the disabled-provider case and the multipart spec shape. --- pkg/routes/api/v2/backgrounds.go | 75 ++++++++++ pkg/webtests/huma_background_upload_test.go | 151 ++++++++++++++++++++ 2 files changed, 226 insertions(+) create mode 100644 pkg/webtests/huma_background_upload_test.go diff --git a/pkg/routes/api/v2/backgrounds.go b/pkg/routes/api/v2/backgrounds.go index c56d1acce..f01fcb4e3 100644 --- a/pkg/routes/api/v2/backgrounds.go +++ b/pkg/routes/api/v2/backgrounds.go @@ -24,6 +24,7 @@ import ( "code.vikunja.io/api/pkg/db" "code.vikunja.io/api/pkg/models" "code.vikunja.io/api/pkg/modules/background" + backgroundHandler "code.vikunja.io/api/pkg/modules/background/handler" "code.vikunja.io/api/pkg/modules/background/unsplash" "github.com/danielgtaylor/huma/v2" @@ -54,6 +55,22 @@ func RegisterBackgroundRoutes(api huma.API) { Tags: tags, }, backgroundRemove) + if config.BackgroundsUploadEnabled.GetBool() { + Register(api, huma.Operation{ + OperationID: "projects-background-upload", + Summary: "Upload a project background", + Description: "Uploads an image via multipart/form-data under the \"background\" field and sets it as the project's background. Requires write access to the project. The image is resized server-side and stored as JPEG; it replaces any previous background (idempotent replace, hence PUT). Returns the updated project.", + Method: http.MethodPut, + Path: "/projects/{project}/backgrounds/upload", + // Return the updated project with 200, the natural code for an idempotent PUT. + DefaultStatus: http.StatusOK, + Tags: tags, + // +2 MB mirrors Echo's global BodyLimit overhead so a max-sized file isn't rejected by multipart boundary/header bytes. + // #nosec G115 - configured value won't exceed int64 max in practice. + MaxBodyBytes: (int64(config.GetMaxFileSizeInMBytes()) + 2) * 1024 * 1024, + }, backgroundUpload) + } + if config.BackgroundsUnsplashEnabled.GetBool() { Register(api, huma.Operation{ OperationID: "backgrounds-unsplash-search", @@ -152,6 +169,64 @@ func backgroundUnsplashSet(ctx context.Context, in *struct { return &singleBody[models.Project]{Body: project}, nil } +type backgroundUploadInput struct { + ProjectID int64 `path:"project" doc:"The id of the project to set the background on."` + // Allow-list mirrors the formats background uploads can actually be decoded as + // (handler.ValidateAndSaveBackgroundUpload's allowedImageMimes); octet-stream covers + // programmatic clients. Huma's MimeTypeValidator rejects the part pre-handler, so the + // byte-level image check in the shared function is the real gate. + RawBody huma.MultipartFormFiles[struct { + Background huma.FormFile `form:"background" contentType:"image/jpeg,image/png,image/gif,image/bmp,image/tiff,image/webp,application/octet-stream" required:"true" doc:"The background image to upload. Must be a decodable raster image (JPEG, PNG, GIF, BMP, TIFF or WebP); it is resized server-side and re-encoded as JPEG."` + }] +} + +// backgroundUpload owns auth, the session and the permission check because there is +// no handler.Do* for multipart uploads (see the api-v2-routes skill's "Non-CRUDable +// / custom routes" section). It shares its body with v1 via +// handler.ValidateAndSaveBackgroundUpload. +func backgroundUpload(ctx context.Context, in *backgroundUploadInput) (*singleBody[models.Project], error) { + a, err := authFromCtx(ctx) + if err != nil { + return nil, err + } + + s := db.NewSession() + defer s.Close() + + project := &models.Project{ID: in.ProjectID} + can, err := project.CanUpdate(s, a) + if err != nil { + _ = s.Rollback() + return nil, translateDomainError(err) + } + if !can { + _ = s.Rollback() + return nil, huma.Error403Forbidden("forbidden") + } + project, err = models.GetProjectSimpleByID(s, in.ProjectID) + if err != nil { + _ = s.Rollback() + return nil, translateDomainError(err) + } + + file := in.RawBody.Data().Background + defer func() { _ = file.Close() }() + + if err := backgroundHandler.ValidateAndSaveBackgroundUpload(s, a, project, file, file.Filename, uint64(file.Size)); err != nil { + _ = s.Rollback() + if backgroundHandler.IsErrFileIsNoImage(err) || backgroundHandler.IsErrFileUnsupportedImageFormat(err) { + return nil, huma.Error400BadRequest(err.Error()) + } + return nil, translateDomainError(err) + } + + if err := s.Commit(); err != nil { + return nil, translateDomainError(err) + } + + return &singleBody[models.Project]{Body: project}, nil +} + func backgroundRemove(ctx context.Context, in *struct { ProjectID int64 `path:"project"` }) (*singleBody[models.Project], error) { diff --git a/pkg/webtests/huma_background_upload_test.go b/pkg/webtests/huma_background_upload_test.go new file mode 100644 index 000000000..68755f53a --- /dev/null +++ b/pkg/webtests/huma_background_upload_test.go @@ -0,0 +1,151 @@ +// 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 ( + "bytes" + "encoding/json" + "mime/multipart" + "net/http" + "net/http/httptest" + "testing" + + "code.vikunja.io/api/pkg/config" + "code.vikunja.io/api/pkg/db" + "code.vikunja.io/api/pkg/models" + "code.vikunja.io/api/pkg/routes" + + "github.com/labstack/echo/v5" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// multipartFileBody builds a multipart body with a single file part under the +// given field name. CreateFormFile sets the part Content-Type to +// application/octet-stream, mirroring how many programmatic clients upload. +func multipartFileBody(t *testing.T, fieldName, filename string, content []byte) (*bytes.Buffer, string) { + t.Helper() + buf := &bytes.Buffer{} + w := multipart.NewWriter(buf) + fw, err := w.CreateFormFile(fieldName, filename) + require.NoError(t, err) + _, err = fw.Write(content) + require.NoError(t, err) + require.NoError(t, w.Close()) + return buf, w.FormDataContentType() +} + +func uploadBackgroundRequest(t *testing.T, e *echo.Echo, project, token string, body *bytes.Buffer, contentType string) *httptest.ResponseRecorder { + t.Helper() + req := httptest.NewRequest(http.MethodPut, "/api/v2/projects/"+project+"/backgrounds/upload", body) + req.Header.Set("Content-Type", contentType) + if token != "" { + req.Header.Set("Authorization", "Bearer "+token) + } + rec := httptest.NewRecorder() + e.ServeHTTP(rec, req) + return rec +} + +func TestHumaProjectBackgroundUpload(t *testing.T) { + e, err := setupTestEnv() + require.NoError(t, err) + + t.Run("Owner uploads a background", func(t *testing.T) { + // testuser1 owns project 1, which starts without a background. + body, contentType := multipartFileBody(t, "background", "bg.png", pngBytes(t)) + rec := uploadBackgroundRequest(t, e, "1", humaTokenFor(t, &testuser1), body, contentType) + require.Equal(t, http.StatusOK, rec.Code, "body: %s", rec.Body.String()) + + s := db.NewSession() + defer s.Close() + project := models.Project{ID: 1} + has, err := s.Get(&project) + require.NoError(t, err) + require.True(t, has) + assert.NotZero(t, project.BackgroundFileID, "the upload must set a background file id") + assert.NotEmpty(t, project.BackgroundBlurHash, "the upload must compute a blur hash") + }) + + t.Run("Non-image rejected with 400", func(t *testing.T) { + body, contentType := multipartFileBody(t, "background", "not-an-image.txt", []byte("this is plain text, not an image")) + rec := uploadBackgroundRequest(t, e, "1", humaTokenFor(t, &testuser1), body, contentType) + require.Equal(t, http.StatusBadRequest, rec.Code, "body: %s", rec.Body.String()) + }) + + t.Run("Read-only user is forbidden", func(t *testing.T) { + // testuser15 has read-only access to project 35. + body, contentType := multipartFileBody(t, "background", "bg.png", pngBytes(t)) + rec := uploadBackgroundRequest(t, e, "35", humaTokenFor(t, &testuser15), body, contentType) + assert.Equal(t, http.StatusForbidden, rec.Code, "body: %s", rec.Body.String()) + }) + + t.Run("No access at all is forbidden", func(t *testing.T) { + // testuser1 has no access to project 35. + body, contentType := multipartFileBody(t, "background", "bg.png", pngBytes(t)) + rec := uploadBackgroundRequest(t, e, "35", humaTokenFor(t, &testuser1), body, contentType) + assert.Equal(t, http.StatusForbidden, rec.Code, "body: %s", rec.Body.String()) + }) + + t.Run("Unauthenticated", func(t *testing.T) { + body, contentType := multipartFileBody(t, "background", "bg.png", pngBytes(t)) + rec := uploadBackgroundRequest(t, e, "1", "", body, contentType) + require.Equal(t, http.StatusUnauthorized, rec.Code, "body: %s", rec.Body.String()) + }) + + t.Run("Renders as multipart in the OpenAPI spec", func(t *testing.T) { + req := httptest.NewRequest(http.MethodGet, "/api/v2/openapi.json", nil) + rec := httptest.NewRecorder() + e.ServeHTTP(rec, req) + require.Equal(t, http.StatusOK, rec.Code) + + var spec map[string]any + require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &spec)) + + paths, _ := spec["paths"].(map[string]any) + op, _ := paths["/projects/{project}/backgrounds/upload"].(map[string]any) + put, ok := op["put"].(map[string]any) + require.True(t, ok, "PUT /projects/{project}/backgrounds/upload must be in the spec") + content, _ := put["requestBody"].(map[string]any) + contentMap, _ := content["content"].(map[string]any) + mp, ok := contentMap["multipart/form-data"].(map[string]any) + require.True(t, ok, "background upload must be modeled as multipart/form-data") + schema, _ := mp["schema"].(map[string]any) + props, _ := schema["properties"].(map[string]any) + bgProp, ok := props["background"].(map[string]any) + require.True(t, ok, "the background field must appear in the multipart schema") + assert.Equal(t, "binary", bgProp["format"], "background field must be a binary file in the spec") + }) +} + +// TestHumaProjectBackgroundUploadDisabledByConfig verifies the upload route is +// absent (404) when the upload provider is disabled, even though backgrounds +// themselves are enabled. +func TestHumaProjectBackgroundUploadDisabledByConfig(t *testing.T) { + _, err := setupTestEnv() + require.NoError(t, err) + + config.BackgroundsUploadEnabled.Set(false) + defer config.BackgroundsUploadEnabled.Set(true) + + e := routes.NewEcho() + routes.RegisterRoutes(e) + + body, contentType := multipartFileBody(t, "background", "bg.png", pngBytes(t)) + rec := uploadBackgroundRequest(t, e, "1", humaTokenFor(t, &testuser1), body, contentType) + assert.Equal(t, http.StatusNotFound, rec.Code, "route must be absent when background upload is disabled; body: %s", rec.Body.String()) +}