Merge pull request #547 from photoview/viktorstrate/issue540

Fix postgres share token bug

api/graphql/models/actions/share_token_actions.go

@@ -11,7 +11,7 @@ import (
 	"gorm.io/gorm"
 )
 
-func AddMediaShare(db *gorm.DB, userID int, mediaID int, expire *time.Time, password *string) (*models.ShareToken, error) {
+func AddMediaShare(db *gorm.DB, user *models.User, mediaID int, expire *time.Time, password *string) (*models.ShareToken, error) {
 	var media models.Media
 
 	var query string
@@ -22,7 +22,7 @@ func AddMediaShare(db *gorm.DB, userID int, mediaID int, expire *time.Time, pass
 	}
 
 	err := db.Joins("Album").
-		Where(query, userID).
+		Where(query, user.ID).
 		First(&media, mediaID).
 		Error
 
@@ -41,7 +41,7 @@ func AddMediaShare(db *gorm.DB, userID int, mediaID int, expire *time.Time, pass
 
 	shareToken := models.ShareToken{
 		Value:    utils.GenerateToken(),
-		OwnerID:  userID,
+		OwnerID:  user.ID,
 		Expire:   expire,
 		Password: hashedPassword,
 		AlbumID:  nil,
@@ -145,8 +145,15 @@ func hashSharePassword(password *string) (*string, error) {
 
 func getUserToken(db *gorm.DB, userID int, tokenValue string) (*models.ShareToken, error) {
 
+	var query string
+	if db.Dialector.Name() == "postgres" {
+		query = "\"Owner\".id = ? OR \"Owner\".admin = TRUE"
+	} else {
+		query = "Owner.id = ? OR Owner.admin = TRUE"
+	}
+
 	var token models.ShareToken
-	err := db.Where("share_tokens.value = ?", tokenValue).Joins("Owner").Where("Owner.id = ? OR Owner.admin = TRUE", userID).First(&token).Error
+	err := db.Where("share_tokens.value = ?", tokenValue).Joins("Owner").Where(query, userID).First(&token).Error
 
 	if err != nil {
 		return nil, errors.Wrap(err, "failed to get user share token from database")

api/graphql/models/actions/share_token_actions_test.go

@@ -0,0 +1,107 @@
+package actions_test
+
+import (
+	"testing"
+	"time"
+
+	"github.com/photoview/photoview/api/graphql/models"
+	"github.com/photoview/photoview/api/graphql/models/actions"
+	"github.com/photoview/photoview/api/test_utils"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestShareToken(t *testing.T) {
+	db := test_utils.DatabaseTest(t)
+
+	password := "1234"
+	user, err := models.RegisterUser(db, "user", &password, false)
+	assert.NoError(t, err)
+
+	rootAlbum := models.Album{
+		Title: "root",
+		Path:  "/photos",
+	}
+
+	assert.NoError(t, db.Save(&rootAlbum).Error)
+
+	childAlbum := models.Album{
+		Title:         "subalbum",
+		Path:          "/photos/subalbum",
+		ParentAlbumID: &rootAlbum.ID,
+	}
+
+	assert.NoError(t, db.Save(&childAlbum).Error)
+
+	assert.NoError(t, db.Model(&user).Association("Albums").Append(&rootAlbum))
+	assert.NoError(t, db.Model(&user).Association("Albums").Append(&childAlbum))
+
+	media := []models.Media{
+		{
+			Title:   "pic1",
+			Path:    "/photos/pic1",
+			AlbumID: rootAlbum.ID,
+		},
+		{
+			Title:   "pic3",
+			Path:    "/photos/subalbum/pic3",
+			AlbumID: childAlbum.ID,
+		},
+		{
+			Title:   "pic4",
+			Path:    "/photos/subalbum/pic4",
+			AlbumID: childAlbum.ID,
+		},
+	}
+
+	assert.NoError(t, db.Save(&media).Error)
+
+	expireTime := time.Unix(1632866400, 0)
+	sharePassword := "secretSharePassword"
+
+	var mediaShare *models.ShareToken
+	var albumShare *models.ShareToken
+
+	t.Run("Add album share", func(t *testing.T) {
+		share, err := actions.AddAlbumShare(db, user, rootAlbum.ID, &expireTime, nil)
+		albumShare = share
+
+		assert.NoError(t, err)
+		assert.NotNil(t, share)
+
+		assert.NotEmpty(t, share.Value)
+		assert.Equal(t, rootAlbum.ID, *share.AlbumID)
+		assert.Nil(t, share.MediaID)
+	})
+
+	t.Run("Add media share", func(t *testing.T) {
+		share, err := actions.AddMediaShare(db, user, media[0].ID, &expireTime, &sharePassword)
+		mediaShare = share
+
+		assert.NoError(t, err)
+		assert.NotNil(t, share)
+
+		assert.NotEmpty(t, share.Value)
+		assert.Equal(t, media[0].ID, *share.MediaID)
+		assert.Nil(t, share.AlbumID)
+	})
+
+	t.Run("Delete share token", func(t *testing.T) {
+		deletedShare, err := actions.DeleteShareToken(db, user.ID, mediaShare.Value)
+
+		assert.NoError(t, err)
+		assert.Equal(t, mediaShare.ID, deletedShare.ID)
+	})
+
+	t.Run("Protect share token", func(t *testing.T) {
+
+		assert.Empty(t, albumShare.Password)
+
+		share, err := actions.ProtectShareToken(db, user.ID, albumShare.Value, &sharePassword)
+		assert.NoError(t, err)
+		assert.NotEmpty(t, share.Password)
+
+		share, err = actions.ProtectShareToken(db, user.ID, albumShare.Value, nil)
+		assert.NoError(t, err)
+		assert.Empty(t, share.Password)
+	})
+}

api/graphql/resolvers/share_token.go

@@ -108,7 +108,7 @@ func (r *mutationResolver) ShareMedia(ctx context.Context, mediaID int, expire *
 		return nil, auth.ErrUnauthorized
 	}
 
-	return actions.AddMediaShare(r.Database, user.ID, mediaID, expire, password)
+	return actions.AddMediaShare(r.Database, user, mediaID, expire, password)
 }
 
 func (r *mutationResolver) DeleteShareToken(ctx context.Context, tokenValue string) (*models.ShareToken, error) {

api/routes/authenticate_routes_test.go

@@ -69,7 +69,7 @@ func TestAuthenticateRoute(t *testing.T) {
 
 		expire := time.Now().Add(time.Hour * 24 * 30)
 		tokenPassword := "token-password-123"
-		shareToken, err := actions.AddMediaShare(db, user.ID, media.ID, &expire, &tokenPassword)
+		shareToken, err := actions.AddMediaShare(db, user, media.ID, &expire, &tokenPassword)
 		if !assert.NoError(t, err) {
 			return
 		}