diff --git a/pkg/modules/avatar/avatar.go b/pkg/modules/avatar/avatar.go index b35b307c7..afd5b7b34 100644 --- a/pkg/modules/avatar/avatar.go +++ b/pkg/modules/avatar/avatar.go @@ -17,6 +17,10 @@ package avatar import ( + "errors" + "io" + "strings" + "code.vikunja.io/api/pkg/config" "code.vikunja.io/api/pkg/log" "code.vikunja.io/api/pkg/modules/avatar/botmarble" @@ -29,9 +33,13 @@ import ( "code.vikunja.io/api/pkg/modules/avatar/upload" "code.vikunja.io/api/pkg/user" + "github.com/gabriel-vasile/mimetype" "xorm.io/xorm" ) +// ErrNotAnImage is returned by StoreUploadedAvatar when the uploaded file is not an image. +var ErrNotAnImage = errors.New("uploaded file is no image") + // Provider defines the avatar provider interface type Provider interface { // GetAvatar is the method used to get an actual avatar for a user @@ -122,3 +130,28 @@ func GetProvider(u *user.User) Provider { return &empty.Provider{} } } + +// StoreUploadedAvatar validates that src is an image, switches the user's avatar +// provider to "upload", stores the image as the user's avatar and flushes all +// cached avatars for the user. It returns ErrNotAnImage if src is not an image. +func StoreUploadedAvatar(s *xorm.Session, u *user.User, src io.ReadSeeker) error { + mime, err := mimetype.DetectReader(src) + if err != nil { + return err + } + if !strings.HasPrefix(mime.String(), "image") { + return ErrNotAnImage + } + if _, err := src.Seek(0, io.SeekStart); err != nil { + return err + } + + u.AvatarProvider = "upload" + if err := upload.StoreAvatarFile(s, u, src); err != nil { + return err + } + + FlushAllCaches(u) + + return nil +} diff --git a/pkg/routes/api/v1/avatar.go b/pkg/routes/api/v1/avatar.go index fc3a24092..a80719e72 100644 --- a/pkg/routes/api/v1/avatar.go +++ b/pkg/routes/api/v1/avatar.go @@ -21,15 +21,12 @@ import ( "code.vikunja.io/api/pkg/log" "code.vikunja.io/api/pkg/models" "code.vikunja.io/api/pkg/modules/avatar" - "code.vikunja.io/api/pkg/modules/avatar/upload" "code.vikunja.io/api/pkg/user" - "io" + "errors" "net/http" "strconv" - "strings" - "github.com/gabriel-vasile/mimetype" "github.com/labstack/echo/v5" ) @@ -111,21 +108,11 @@ func UploadAvatar(c *echo.Context) (err error) { } defer src.Close() - // Validate we're dealing with an image - mime, err := mimetype.DetectReader(src) - if err != nil { - _ = s.Rollback() - return err - } - if !strings.HasPrefix(mime.String(), "image") { - return c.JSON(http.StatusBadRequest, models.Message{Message: "Uploaded file is no image."}) - } - _, _ = src.Seek(0, io.SeekStart) - - u.AvatarProvider = "upload" - err = upload.StoreAvatarFile(s, u, src) - if err != nil { + if err := avatar.StoreUploadedAvatar(s, u, src); err != nil { _ = s.Rollback() + if errors.Is(err, avatar.ErrNotAnImage) { + return c.JSON(http.StatusBadRequest, models.Message{Message: "Uploaded file is no image."}) + } return err } @@ -134,7 +121,5 @@ func UploadAvatar(c *echo.Context) (err error) { return err } - avatar.FlushAllCaches(u) - return c.JSON(http.StatusOK, models.Message{Message: "Avatar was uploaded successfully."}) } diff --git a/pkg/routes/api/v2/avatar_upload.go b/pkg/routes/api/v2/avatar_upload.go index 2154d8b30..80893a53b 100644 --- a/pkg/routes/api/v2/avatar_upload.go +++ b/pkg/routes/api/v2/avatar_upload.go @@ -18,23 +18,20 @@ package apiv2 import ( "context" - "io" + "errors" "net/http" - "strings" "code.vikunja.io/api/pkg/config" "code.vikunja.io/api/pkg/db" "code.vikunja.io/api/pkg/models" "code.vikunja.io/api/pkg/modules/avatar" - "code.vikunja.io/api/pkg/modules/avatar/upload" "code.vikunja.io/api/pkg/user" "github.com/danielgtaylor/huma/v2" - "github.com/gabriel-vasile/mimetype" ) type avatarUploadInput struct { - // Broad allow-list because Huma's MimeTypeValidator rejects the part pre-handler; octet-stream covers programmatic clients. The real gate is mimetype.DetectReader in the handler. + // Broad allow-list because Huma's MimeTypeValidator rejects the part pre-handler; octet-stream covers programmatic clients. The real gate is the byte-level image check in avatar.StoreUploadedAvatar. RawBody huma.MultipartFormFiles[struct { Avatar huma.FormFile `form:"avatar" contentType:"image/png,image/jpeg,image/gif,image/webp,image/svg+xml,application/octet-stream" required:"true" doc:"The avatar image to upload. Must be an image; it is resized server-side and re-encoded as PNG."` }] @@ -86,24 +83,11 @@ func avatarUpload(ctx context.Context, in *avatarUploadInput) (*avatarUploadBody src := in.RawBody.Data().Avatar defer func() { _ = src.Close() }() - // Byte-level image check, same allow-list as v1's UploadAvatar. - mime, err := mimetype.DetectReader(src) - if err != nil { - _ = s.Rollback() - return nil, translateDomainError(err) - } - if !strings.HasPrefix(mime.String(), "image") { - _ = s.Rollback() - return nil, huma.Error400BadRequest("Uploaded file is no image.") - } - if _, err := src.Seek(0, io.SeekStart); err != nil { - _ = s.Rollback() - return nil, translateDomainError(err) - } - - u.AvatarProvider = "upload" - if err := upload.StoreAvatarFile(s, u, src); err != nil { + if err := avatar.StoreUploadedAvatar(s, u, src); err != nil { _ = s.Rollback() + if errors.Is(err, avatar.ErrNotAnImage) { + return nil, huma.Error400BadRequest("Uploaded file is no image.") + } return nil, translateDomainError(err) } @@ -112,7 +96,5 @@ func avatarUpload(ctx context.Context, in *avatarUploadInput) (*avatarUploadBody return nil, translateDomainError(err) } - avatar.FlushAllCaches(u) - return &avatarUploadBody{Body: &models.Message{Message: "Avatar was uploaded successfully."}}, nil }