From 27709be5cf25721f6bd5481c38c9c6844bbee6b0 Mon Sep 17 00:00:00 2001 From: Ion Bazan Date: Thu, 8 Oct 2026 02:39:45 +0800 Subject: [PATCH] fix: limit upload size for profile pictures and application images (#1819) --- .../internal/bootstrap/router_bootstrap.go | 4 +- .../controller/app_images_controller.go | 11 +- .../controller/upload_size_limit_test.go | 110 ++++++++++++++++++ .../internal/controller/user_controller.go | 6 +- .../internal/utils/image/profile_picture.go | 23 +++- 5 files changed, 143 insertions(+), 11 deletions(-) create mode 100644 backend/internal/controller/upload_size_limit_test.go diff --git a/backend/internal/bootstrap/router_bootstrap.go b/backend/internal/bootstrap/router_bootstrap.go index edf4c631..51ef5a0f 100644 --- a/backend/internal/bootstrap/router_bootstrap.go +++ b/backend/internal/bootstrap/router_bootstrap.go @@ -171,10 +171,10 @@ func registerRoutes(r *gin.Engine, db *gorm.DB, svc *services, rateLimitServices rateLimitMiddleware.Add(middleware.RateLimitDeviceLoginVerification), ) controller.NewOidcController(apiGroup, authMiddleware, fileSizeLimitMiddleware, svc.oidcService, svc.appConfigService) - controller.NewUserController(apiGroup, authMiddleware, svc.appConfigService, svc.userService, svc.webauthnModule) + controller.NewUserController(apiGroup, authMiddleware, fileSizeLimitMiddleware, svc.appConfigService, svc.userService, svc.webauthnModule) controller.NewAppConfigController(apiGroup, authMiddleware, svc.appConfigService, svc.emailModule) svc.ldapSyncModule.RegisterRoutes(apiGroup, authMiddleware.Add()) - controller.NewAppImagesController(apiGroup, authMiddleware, svc.appImagesService) + controller.NewAppImagesController(apiGroup, authMiddleware, fileSizeLimitMiddleware, svc.appImagesService) svc.auditLogsModule.RegisterRoutes(apiGroup, authMiddleware.Add(), authMiddleware.WithAdminNotRequired().Add()) controller.NewUserGroupController(apiGroup, authMiddleware, svc.appConfigService, svc.userGroupService) svc.apiModule.RegisterRoutes(apiGroup, authMiddleware.Add()) diff --git a/backend/internal/controller/app_images_controller.go b/backend/internal/controller/app_images_controller.go index 9f506b64..63d3d39e 100644 --- a/backend/internal/controller/app_images_controller.go +++ b/backend/internal/controller/app_images_controller.go @@ -20,6 +20,7 @@ import ( func NewAppImagesController( group *gin.RouterGroup, authMiddleware *middleware.AuthMiddleware, + fileSizeLimitMiddleware *middleware.FileSizeLimitMiddleware, appImagesService *service.AppImagesService, ) { controller := &AppImagesController{ @@ -32,11 +33,11 @@ func NewAppImagesController( group.GET("/application-images/favicon", httpserver.Handle(controller.getFaviconHandler)) group.GET("/application-images/default-profile-picture", authMiddleware.Add(), httpserver.Handle(controller.getDefaultProfilePicture)) - group.PUT("/application-images/logo", authMiddleware.Add(), httpserver.Handle(controller.updateLogoHandler)) - group.PUT("/application-images/email", authMiddleware.Add(), httpserver.Handle(controller.updateEmailLogoHandler)) - group.PUT("/application-images/background", authMiddleware.Add(), httpserver.Handle(controller.updateBackgroundImageHandler)) - group.PUT("/application-images/favicon", authMiddleware.Add(), httpserver.Handle(controller.updateFaviconHandler)) - group.PUT("/application-images/default-profile-picture", authMiddleware.Add(), httpserver.Handle(controller.updateDefaultProfilePicture)) + group.PUT("/application-images/logo", authMiddleware.Add(), fileSizeLimitMiddleware.Add(10<<20), httpserver.Handle(controller.updateLogoHandler)) + group.PUT("/application-images/email", authMiddleware.Add(), fileSizeLimitMiddleware.Add(10<<20), httpserver.Handle(controller.updateEmailLogoHandler)) + group.PUT("/application-images/background", authMiddleware.Add(), fileSizeLimitMiddleware.Add(10<<20), httpserver.Handle(controller.updateBackgroundImageHandler)) + group.PUT("/application-images/favicon", authMiddleware.Add(), fileSizeLimitMiddleware.Add(10<<20), httpserver.Handle(controller.updateFaviconHandler)) + group.PUT("/application-images/default-profile-picture", authMiddleware.Add(), fileSizeLimitMiddleware.Add(10<<20), httpserver.Handle(controller.updateDefaultProfilePicture)) group.DELETE("/application-images/logo", authMiddleware.Add(), httpserver.Handle(controller.deleteLogoHandler)) group.DELETE("/application-images/background", authMiddleware.Add(), httpserver.Handle(controller.deleteBackgroundImageHandler)) diff --git a/backend/internal/controller/upload_size_limit_test.go b/backend/internal/controller/upload_size_limit_test.go new file mode 100644 index 00000000..cbcd5796 --- /dev/null +++ b/backend/internal/controller/upload_size_limit_test.go @@ -0,0 +1,110 @@ +package controller + +import ( + "bytes" + "encoding/json" + "mime/multipart" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/gin-gonic/gin" + "github.com/stretchr/testify/require" + + "github.com/pocket-id/pocket-id/backend/internal/apikey" + "github.com/pocket-id/pocket-id/backend/internal/apperror" + "github.com/pocket-id/pocket-id/backend/internal/common" + "github.com/pocket-id/pocket-id/backend/internal/instanceid" + "github.com/pocket-id/pocket-id/backend/internal/middleware" + "github.com/pocket-id/pocket-id/backend/internal/model" + "github.com/pocket-id/pocket-id/backend/internal/service" + testutils "github.com/pocket-id/pocket-id/backend/internal/utils/testing" +) + +func TestImageUploadRoutesLimitRequestSize(t *testing.T) { + gin.SetMode(gin.TestMode) + + originalEnvConfig := common.EnvConfig + defer func() { + common.EnvConfig = originalEnvConfig + }() + common.EnvConfig.EncryptionKey = []byte("0123456789abcdef0123456789abcdef") + + db := testutils.NewDatabaseForTest(t) + + instanceID, err := instanceid.Load(t.Context(), db) + require.NoError(t, err) + + jwtService, err := service.NewJwtService(t.Context(), db, instanceID) + require.NoError(t, err) + + userService := service.NewUserService(db, jwtService, nil, nil, nil, nil, nil) + apiKeyModule, err := apikey.New(t.Context(), apikey.Dependencies{DB: db, CleanupDisabled: true}) + require.NoError(t, err) + + authMiddleware := middleware.NewAuthMiddleware(apiKeyModule, userService, jwtService) + fileSizeLimitMiddleware := middleware.NewFileSizeLimitMiddleware() + + user := model.User{Username: "upload-admin", IsAdmin: true} + require.NoError(t, db.Create(&user).Error) + + token, err := jwtService.GenerateAccessToken(user, "", time.Hour) + require.NoError(t, err) + + router := gin.New() + router.Use(middleware.NewErrorHandlerMiddleware().Add()) + apiGroup := router.Group("/api") + NewUserController(apiGroup, authMiddleware, fileSizeLimitMiddleware, nil, userService, nil) + NewAppImagesController(apiGroup, authMiddleware, fileSizeLimitMiddleware, nil) + + routes := []string{ + "/api/users/user-id/profile-picture", + "/api/users/me/profile-picture", + "/api/application-images/logo", + "/api/application-images/email", + "/api/application-images/background", + "/api/application-images/favicon", + "/api/application-images/default-profile-picture", + } + + for _, route := range routes { + t.Run("rejects an oversized upload to "+route, func(t *testing.T) { + status, code := uploadFile(t, router, route, token, 10<<20+1) + + require.Equal(t, http.StatusRequestEntityTooLarge, status) + require.Equal(t, apperror.CodeFileTooLarge, code) + }) + } + + t.Run("passes an upload under the limit on to the handler", func(t *testing.T) { + status, code := uploadFile(t, router, "/api/users/me/profile-picture", token, 9<<20) + + require.Equal(t, http.StatusBadRequest, status) + require.Equal(t, apperror.CodeInvalidImage, code) + }) +} + +func uploadFile(t *testing.T, router *gin.Engine, target string, token string, size int) (int, apperror.Code) { + t.Helper() + + var body bytes.Buffer + writer := multipart.NewWriter(&body) + part, err := writer.CreateFormFile("file", "upload.png") + require.NoError(t, err) + _, err = part.Write(bytes.Repeat([]byte("x"), size)) + require.NoError(t, err) + require.NoError(t, writer.Close()) + + request := httptest.NewRequestWithContext(t.Context(), http.MethodPut, target, &body) + request.Header.Set("Content-Type", writer.FormDataContentType()) + request.Header.Set("Authorization", "Bearer "+token) + recorder := httptest.NewRecorder() + router.ServeHTTP(recorder, request) + + var response struct { + Code apperror.Code `json:"code"` + } + require.NoError(t, json.Unmarshal(recorder.Body.Bytes(), &response)) + return recorder.Code, response.Code +} diff --git a/backend/internal/controller/user_controller.go b/backend/internal/controller/user_controller.go index ac3f3b06..6a9923c8 100644 --- a/backend/internal/controller/user_controller.go +++ b/backend/internal/controller/user_controller.go @@ -20,7 +20,7 @@ import ( // @Summary User management controller // @Description Initializes all user-related API endpoints // @Tags Users -func NewUserController(group *gin.RouterGroup, authMiddleware *middleware.AuthMiddleware, appConfigService *appconfig.AppConfigService, userService *service.UserService, webAuthnService *webauthn.Module) { +func NewUserController(group *gin.RouterGroup, authMiddleware *middleware.AuthMiddleware, fileSizeLimitMiddleware *middleware.FileSizeLimitMiddleware, appConfigService *appconfig.AppConfigService, userService *service.UserService, webAuthnService *webauthn.Module) { uc := UserController{ appConfigService: appConfigService, userService: userService, @@ -42,8 +42,8 @@ func NewUserController(group *gin.RouterGroup, authMiddleware *middleware.AuthMi group.GET("/users/:id/profile-picture.png", httpserver.Handle(uc.getUserProfilePictureHandler)) - group.PUT("/users/:id/profile-picture", authMiddleware.Add(), httpserver.Handle(uc.updateUserProfilePictureHandler)) - group.PUT("/users/me/profile-picture", authMiddleware.WithAdminNotRequired().Add(), httpserver.Handle(uc.updateCurrentUserProfilePictureHandler)) + group.PUT("/users/:id/profile-picture", authMiddleware.Add(), fileSizeLimitMiddleware.Add(10<<20), httpserver.Handle(uc.updateUserProfilePictureHandler)) + group.PUT("/users/me/profile-picture", authMiddleware.WithAdminNotRequired().Add(), fileSizeLimitMiddleware.Add(10<<20), httpserver.Handle(uc.updateCurrentUserProfilePictureHandler)) group.DELETE("/users/:id/profile-picture", authMiddleware.Add(), httpserver.Handle(uc.resetUserProfilePictureHandler)) group.DELETE("/users/me/profile-picture", authMiddleware.WithAdminNotRequired().Add(), httpserver.Handle(uc.resetCurrentUserProfilePictureHandler)) diff --git a/backend/internal/utils/image/profile_picture.go b/backend/internal/utils/image/profile_picture.go index f25de259..316b7f95 100644 --- a/backend/internal/utils/image/profile_picture.go +++ b/backend/internal/utils/image/profile_picture.go @@ -22,6 +22,8 @@ const profilePictureSize = 300 var ErrInvalidImage = errors.New("invalid image") +var resizeSlots = make(chan struct{}, 2) + // CreateProfilePicture resizes the profile picture to a square and encodes it as PNG func CreateProfilePicture(file io.ReadSeeker) (io.ReadSeeker, error) { // Reject an oversized pixel count before decoding can allocate a pixel buffer @@ -33,6 +35,9 @@ func CreateProfilePicture(file io.ReadSeeker) (io.ReadSeeker, error) { return nil, fmt.Errorf("%w: %w", ErrInvalidImage, validationErr) } + resizeSlots <- struct{}{} + defer func() { <-resizeSlots }() + // Attempt standard formats first img, _, err := imageorient.Decode(file) if err != nil { @@ -52,7 +57,7 @@ func CreateProfilePicture(file io.ReadSeeker) (io.ReadSeeker, error) { } // Resize to square - img = imaging.Fill(img, profilePictureSize, profilePictureSize, imaging.Center, imaging.Lanczos) + img = imaging.Resize(centerSquare(img), profilePictureSize, profilePictureSize, imaging.Lanczos) // Encode back to PNG var buf bytes.Buffer @@ -63,6 +68,22 @@ func CreateProfilePicture(file io.ReadSeeker) (io.ReadSeeker, error) { return bytes.NewReader(buf.Bytes()), nil } +func centerSquare(img image.Image) image.Image { + b := img.Bounds() + side := min(b.Dx(), b.Dy()) + + sub, ok := img.(interface { + SubImage(r image.Rectangle) image.Image + }) + if !ok { + return imaging.CropCenter(img, side, side) + } + + x := b.Min.X + (b.Dx()-side)/2 + y := b.Min.Y + (b.Dy()-side)/2 + return sub.SubImage(image.Rect(x, y, x+side, y+side)) +} + // CreateDefaultProfilePicture creates a profile picture with the initials func CreateDefaultProfilePicture(initials string) (*bytes.Buffer, error) { // Create a blank image with a white background