diff --git a/services/graph/pkg/service/v0/api_users_user_profile_photo.go b/services/graph/pkg/service/v0/api_users_user_profile_photo.go index 3cd43c1e1f..c8d75f7c71 100644 --- a/services/graph/pkg/service/v0/api_users_user_profile_photo.go +++ b/services/graph/pkg/service/v0/api_users_user_profile_photo.go @@ -3,8 +3,10 @@ package svc import ( "context" "errors" + "fmt" "io" "net/http" + "strings" "github.com/go-chi/render" "github.com/opencloud-eu/reva/v2/pkg/storage/utils/metadata" @@ -33,6 +35,12 @@ var ( // ErrNoBytes is returned when no bytes are found ErrNoBytes = errors.New("no bytes") + + // ErrInvalidContentType is returned when the content type is invalid + ErrInvalidContentType = errors.New("invalid content type") + + // ErrMissingArgument is returned when a required argument is missing + ErrMissingArgument = errors.New("required argument is missing") ) // UsersUserProfilePhotoService is the implementation of the UsersUserProfilePhotoProvider interface @@ -53,12 +61,7 @@ func NewUsersUserProfilePhotoService(storage metadata.Storage) (UsersUserProfile // GetPhoto retrieves the requested photo func (s UsersUserProfilePhotoService) GetPhoto(ctx context.Context, id string) ([]byte, error) { - photo, err := s.storage.SimpleDownload(ctx, id) - if err != nil { - return nil, err - } - - return photo, nil + return s.storage.SimpleDownload(ctx, id) } // DeletePhoto deletes the requested photo @@ -68,6 +71,10 @@ func (s UsersUserProfilePhotoService) DeletePhoto(ctx context.Context, id string // UpdatePhoto updates the requested photo func (s UsersUserProfilePhotoService) UpdatePhoto(ctx context.Context, id string, r io.Reader) error { + if id == "" { + return fmt.Errorf("%w: %s", ErrMissingArgument, "id") + } + photo, err := io.ReadAll(r) if err != nil { return err @@ -77,6 +84,11 @@ func (s UsersUserProfilePhotoService) UpdatePhoto(ctx context.Context, id string return ErrNoBytes } + contentType := http.DetectContentType(photo) + if !strings.HasPrefix(contentType, "image/") { + return fmt.Errorf("%w: %s", ErrInvalidContentType, contentType) + } + return s.storage.SimpleUpload(ctx, id, photo) } diff --git a/services/graph/pkg/service/v0/api_users_user_profile_photo_test.go b/services/graph/pkg/service/v0/api_users_user_profile_photo_test.go index cf38bcddfa..500148809c 100644 --- a/services/graph/pkg/service/v0/api_users_user_profile_photo_test.go +++ b/services/graph/pkg/service/v0/api_users_user_profile_photo_test.go @@ -1,6 +1,7 @@ package svc_test import ( + "bytes" "context" "errors" "io" @@ -17,25 +18,50 @@ import ( svc "github.com/opencloud-eu/opencloud/services/graph/pkg/service/v0" ) +func TestNewUsersUserProfilePhotoService(t *testing.T) { + storage := mocks.NewStorage(t) + storage.EXPECT().Init(mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, id string) error { return nil }) + + service, err := svc.NewUsersUserProfilePhotoService(storage) + assert.NoError(t, err) + + t.Run("UpdatePhoto", func(t *testing.T) { + t.Run("reports an error if id is empty", func(t *testing.T) { + err := service.UpdatePhoto(context.Background(), "", bytes.NewReader([]byte{})) + assert.ErrorIs(t, err, svc.ErrMissingArgument) + }) + + t.Run("reports an error if the reader does not contain any bytes", func(t *testing.T) { + err := service.UpdatePhoto(context.Background(), "123", bytes.NewReader([]byte{})) + assert.ErrorIs(t, err, svc.ErrNoBytes) + }) + + t.Run("reports an error if data is not an image", func(t *testing.T) { + err := service.UpdatePhoto(context.Background(), "234", bytes.NewReader([]byte("not an image"))) + assert.ErrorIs(t, err, svc.ErrInvalidContentType) + }) + }) +} + func TestUsersUserProfilePhotoApi(t *testing.T) { var ( - usersUserProfilePhotoProvider = mocks.NewUsersUserProfilePhotoProvider(t) - dummyDataProvider = func(w http.ResponseWriter, r *http.Request) (string, bool) { + serviceProvider = mocks.NewUsersUserProfilePhotoProvider(t) + dataProvider = func(w http.ResponseWriter, r *http.Request) (string, bool) { return "123", true } ) - api, err := svc.NewUsersUserProfilePhotoApi(usersUserProfilePhotoProvider, log.NopLogger()) + api, err := svc.NewUsersUserProfilePhotoApi(serviceProvider, log.NopLogger()) assert.NoError(t, err) t.Run("GetProfilePhoto", func(t *testing.T) { r := httptest.NewRequest(http.MethodGet, "/", nil) - ep := api.GetProfilePhoto(dummyDataProvider) + ep := api.GetProfilePhoto(dataProvider) t.Run("fails if photo provider errors", func(t *testing.T) { w := httptest.NewRecorder() - usersUserProfilePhotoProvider.EXPECT().GetPhoto(mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, s string) ([]byte, error) { + serviceProvider.EXPECT().GetPhoto(mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, s string) ([]byte, error) { return nil, errors.New("any") }).Once() @@ -47,7 +73,7 @@ func TestUsersUserProfilePhotoApi(t *testing.T) { t.Run("successfully returns the requested photo", func(t *testing.T) { w := httptest.NewRecorder() - usersUserProfilePhotoProvider.EXPECT().GetPhoto(mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, s string) ([]byte, error) { + serviceProvider.EXPECT().GetPhoto(mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, s string) ([]byte, error) { return []byte("photo"), nil }).Once() @@ -60,12 +86,12 @@ func TestUsersUserProfilePhotoApi(t *testing.T) { t.Run("DeleteProfilePhoto", func(t *testing.T) { r := httptest.NewRequest(http.MethodDelete, "/", nil) - ep := api.DeleteProfilePhoto(dummyDataProvider) + ep := api.DeleteProfilePhoto(dataProvider) t.Run("fails if photo provider errors", func(t *testing.T) { w := httptest.NewRecorder() - usersUserProfilePhotoProvider.EXPECT().DeletePhoto(mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, s string) error { + serviceProvider.EXPECT().DeletePhoto(mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, s string) error { return errors.New("any") }).Once() @@ -77,7 +103,7 @@ func TestUsersUserProfilePhotoApi(t *testing.T) { t.Run("successfully deletes the requested photo", func(t *testing.T) { w := httptest.NewRecorder() - usersUserProfilePhotoProvider.EXPECT().DeletePhoto(mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, s string) error { + serviceProvider.EXPECT().DeletePhoto(mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, s string) error { return nil }).Once() @@ -89,12 +115,12 @@ func TestUsersUserProfilePhotoApi(t *testing.T) { t.Run("UpsertProfilePhoto", func(t *testing.T) { r := httptest.NewRequest(http.MethodPut, "/", strings.NewReader("body")) - ep := api.UpsertProfilePhoto(dummyDataProvider) + ep := api.UpsertProfilePhoto(dataProvider) t.Run("fails if photo provider errors", func(t *testing.T) { w := httptest.NewRecorder() - usersUserProfilePhotoProvider.EXPECT().UpdatePhoto(mock.Anything, mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, s string, r io.Reader) error { + serviceProvider.EXPECT().UpdatePhoto(mock.Anything, mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, s string, r io.Reader) error { return errors.New("any") }).Once() @@ -106,7 +132,7 @@ func TestUsersUserProfilePhotoApi(t *testing.T) { t.Run("successfully upserts the photo", func(t *testing.T) { w := httptest.NewRecorder() - usersUserProfilePhotoProvider.EXPECT().UpdatePhoto(mock.Anything, mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, s string, r io.Reader) error { + serviceProvider.EXPECT().UpdatePhoto(mock.Anything, mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, s string, r io.Reader) error { return nil }).Once()