From 24192a327f92c4ad376f870ba5070d06a9a7f8bf Mon Sep 17 00:00:00 2001
From: Kailash Nadh <kailash@nadh.in>
Date: Sun, 5 Jul 2020 17:35:05 +0530
Subject: [PATCH] Refactor and fix media uploads.

- Fix path related issues in filesystem and S3.
- Add checks for S3 "/" path prefix.
- Add support for custom S3 domain names.
- Remove obsolete `width` and `height` columns from media table (breaking)
- Add `provider` field to media table (breaking)
---
 admin.go                                      | 14 ++---
 config.toml.sample                            | 15 +++---
 frontend/README.md                            |  2 +-
 frontend/src/api/index.js                     |  3 +-
 frontend/src/components/Editor.vue            |  2 +-
 frontend/src/main.js                          |  4 ++
 frontend/src/utils.js                         |  1 +
 frontend/src/views/Media.vue                  | 10 ++--
 init.go                                       |  5 +-
 internal/media/media.go                       | 10 ++--
 .../media/providers/filesystem/filesystem.go  | 25 ++++-----
 internal/media/providers/s3/s3.go             | 54 +++++++++++--------
 media.go                                      |  8 +--
 queries.sql                                   |  4 +-
 schema.sql                                    |  3 +-
 15 files changed, 91 insertions(+), 69 deletions(-)

diff --git a/admin.go b/admin.go
index 51214a0..8fa6fcc 100644
--- a/admin.go
+++ b/admin.go
@@ -11,9 +11,10 @@ import (
 )
 
 type configScript struct {
-	RootURL    string   `json:"rootURL"`
-	FromEmail  string   `json:"fromEmail"`
-	Messengers []string `json:"messengers"`
+	RootURL       string   `json:"rootURL"`
+	FromEmail     string   `json:"fromEmail"`
+	Messengers    []string `json:"messengers"`
+	MediaProvider string   `json:"media_provider"`
 }
 
 // handleGetConfigScript returns general configuration as a Javascript
@@ -22,9 +23,10 @@ func handleGetConfigScript(c echo.Context) error {
 	var (
 		app = c.Get("app").(*App)
 		out = configScript{
-			RootURL:    app.constants.RootURL,
-			FromEmail:  app.constants.FromEmail,
-			Messengers: app.manager.GetMessengerNames(),
+			RootURL:       app.constants.RootURL,
+			FromEmail:     app.constants.FromEmail,
+			Messengers:    app.manager.GetMessengerNames(),
+			MediaProvider: app.constants.MediaProvider,
 		}
 
 		b = bytes.Buffer{}
diff --git a/config.toml.sample b/config.toml.sample
index 71362e8..b6607fb 100644
--- a/config.toml.sample
+++ b/config.toml.sample
@@ -166,20 +166,23 @@ provider = "filesystem"
         aws_secret_access_key = ""
 
         # AWS Region where S3 bucket is hosted.
-        aws_default_region="ap-south-1"
+        aws_default_region = "ap-south-1"
 
         # Bucket name.
-        bucket=""
+        bucket = ""
 
-        # Path where the files will be stored inside bucket. Empty for root.
-        bucket_path=""
+        # Path where the files will be stored inside bucket. Default is "/".
+        bucket_path = "/"
+
+        # Optional full URL to the bucket. eg: https://files.mycustom.com
+        bucket_url = ""
 
         # "private" or "public".
-        bucket_type="public"
+        bucket_type = "public"
 
         # (Optional) Specify TTL (in seconds) for the generated presigned URL.
         # Expiry value is used only if the bucket is private.
-        expiry="86400"
+        expiry = 86400
 
     [upload.filesystem]
         # Path to the uploads directory where media will be uploaded.
diff --git a/frontend/README.md b/frontend/README.md
index 1ca20fe..51964ae 100644
--- a/frontend/README.md
+++ b/frontend/README.md
@@ -5,7 +5,7 @@ It's best if the `listmonk/frontend` directory is opened in an IDE as a separate
 For developer setup instructions, refer to the main project's README.
 
 ## Globals
-`main.js` is where Buefy is injected globally into Vue. In addition two controllers, `$api` (collection of API calls from `api/index.js`) and `$utils` (util functions from `util.js`), are also attached globaly to Vue. They are accessible within Vue as `this.$api` and `this.$utils`.
+`main.js` is where Buefy is injected globally into Vue. In addition two controllers, `$api` (collection of API calls from `api/index.js`), `$utils` (util functions from `util.js`), `$serverConfig` (loaded form /api/config.js) are also attached globaly to Vue. They are accessible within Vue as `this.$api` and `this.$utils`.
 
 Some constants are defined in `constants.js`.
 
diff --git a/frontend/src/api/index.js b/frontend/src/api/index.js
index bfe0205..bb75a9d 100644
--- a/frontend/src/api/index.js
+++ b/frontend/src/api/index.js
@@ -22,8 +22,7 @@ const http = axios.create({
         return resp;
       }
 
-      const data = humps.camelizeKeys(resp.data);
-      return data;
+      return humps.camelizeKeys(resp.data);
     },
   ],
 
diff --git a/frontend/src/components/Editor.vue b/frontend/src/components/Editor.vue
index 307b894..144b33e 100644
--- a/frontend/src/components/Editor.vue
+++ b/frontend/src/components/Editor.vue
@@ -164,7 +164,7 @@ export default {
     },
 
     onMediaSelect(m) {
-      this.$refs.quill.quill.insertEmbed(10, 'image', m.uri);
+      this.$refs.quill.quill.insertEmbed(10, 'image', m.url);
     },
   },
 
diff --git a/frontend/src/main.js b/frontend/src/main.js
index 1503594..3cc4bc3 100644
--- a/frontend/src/main.js
+++ b/frontend/src/main.js
@@ -1,5 +1,6 @@
 import Vue from 'vue';
 import Buefy from 'buefy';
+import humps from 'humps';
 
 import App from './App.vue';
 import router from './router';
@@ -14,6 +15,9 @@ Vue.config.productionTip = false;
 Vue.prototype.$api = api;
 Vue.prototype.$utils = utils;
 
+// window.CONFIG is loaded from /api/config.js directly in a <script> tag.
+Vue.prototype.$serverConfig = humps.camelizeKeys(window.CONFIG);
+
 new Vue({
   router,
   store,
diff --git a/frontend/src/utils.js b/frontend/src/utils.js
index 9795f3d..1221fb3 100644
--- a/frontend/src/utils.js
+++ b/frontend/src/utils.js
@@ -91,6 +91,7 @@ export default class utils {
       message: msg,
       type: !typ ? 'is-success' : typ,
       queue: false,
+      duration: 3000,
     });
   };
 }
diff --git a/frontend/src/views/Media.vue b/frontend/src/views/Media.vue
index 20b7109..ea6cbc0 100644
--- a/frontend/src/views/Media.vue
+++ b/frontend/src/views/Media.vue
@@ -2,6 +2,8 @@
   <section class="media-files">
     <h1 class="title is-4">Media
       <span v-if="media.length > 0">({{ media.length }})</span>
+
+      <span class="has-text-grey-light"> / {{ $serverConfig.mediaProvider }}</span>
     </h1>
 
     <b-loading :active="isProcessing || loading.media"></b-loading>
@@ -45,16 +47,16 @@
 
         <div class="thumbs">
           <div v-for="m in group.items" :key="m.id" class="box thumb">
-            <a @click="(e) => onMediaSelect(m, e)" :href="m.uri" target="_blank">
-              <img :src="m.thumbUri" :title="m.filename" />
+            <a @click="(e) => onMediaSelect(m, e)" :href="m.url" target="_blank">
+              <img :src="m.thumbUrl" :title="m.filename" />
             </a>
             <span class="caption is-size-7" :title="m.filename">{{ m.filename }}</span>
 
             <div class="actions has-text-right">
-              <a :href="m.uri" target="_blank">
+              <a :href="m.url" target="_blank">
                   <b-icon icon="arrow-top-right" size="is-small" />
               </a>
-              <a href="#" @click.prevent="deleteMedia(m.id)">
+              <a href="#" @click.prevent="$utils.confirm(null, () => deleteMedia(m.id))">
                   <b-icon icon="trash-can-outline" size="is-small" />
               </a>
             </div>
diff --git a/init.go b/init.go
index 8319d01..4e6f926 100644
--- a/init.go
+++ b/init.go
@@ -146,6 +146,8 @@ type constants struct {
 	ViewTrackURL string
 	OptinURL     string
 	MessageURL   string
+
+	MediaProvider string
 }
 
 func initConstants() *constants {
@@ -159,6 +161,7 @@ func initConstants() *constants {
 	}
 	c.RootURL = strings.TrimRight(c.RootURL, "/")
 	c.Privacy.Exportable = maps.StringSliceToLookupMap(ko.Strings("privacy.exportable"))
+	c.MediaProvider = ko.String("upload.provider")
 
 	// Static URLS.
 	// url.com/subscription/{campaign_uuid}/{subscriber_uuid}
@@ -175,7 +178,6 @@ func initConstants() *constants {
 
 	// url.com/campaign/{campaign_uuid}/{subscriber_uuid}/px.png
 	c.ViewTrackURL = fmt.Sprintf("%s/campaign/%%s/%%s/px.png", c.RootURL)
-
 	return &c
 }
 
@@ -272,6 +274,7 @@ func initMediaStore() media.Store {
 	case "filesystem":
 		var opts filesystem.Opts
 		ko.Unmarshal("upload.filesystem", &opts)
+		opts.RootURL = ko.String("app.root")
 		opts.UploadPath = filepath.Clean(opts.UploadPath)
 		opts.UploadURI = filepath.Clean(opts.UploadURI)
 		uplder, err := filesystem.NewDiskStore(opts)
diff --git a/internal/media/media.go b/internal/media/media.go
index 2e89c7c..49bdced 100644
--- a/internal/media/media.go
+++ b/internal/media/media.go
@@ -11,14 +11,14 @@ type Media struct {
 	ID        int       `db:"id" json:"id"`
 	UUID      string    `db:"uuid" json:"uuid"`
 	Filename  string    `db:"filename" json:"filename"`
-	Width     int       `db:"width" json:"width"`
-	Height    int       `db:"height" json:"height"`
+	Thumb     string    `db:"thumb" json:"thumb"`
 	CreatedAt null.Time `db:"created_at" json:"created_at"`
-	ThumbURI  string    `json:"thumb_uri"`
-	URI       string    `json:"uri"`
+	ThumbURL  string    `json:"thumb_url"`
+	Provider  string    `json:"provider"`
+	URL       string    `json:"url"`
 }
 
-// Store represents set of methods to perform upload/delete operations.
+// Store represents functions to store and retrieve media (files).
 type Store interface {
 	Put(string, string, io.ReadSeeker) (string, error)
 	Delete(string) error
diff --git a/internal/media/providers/filesystem/filesystem.go b/internal/media/providers/filesystem/filesystem.go
index 112db93..686894d 100644
--- a/internal/media/providers/filesystem/filesystem.go
+++ b/internal/media/providers/filesystem/filesystem.go
@@ -19,6 +19,7 @@ const tmpFilePrefix = "listmonk"
 type Opts struct {
 	UploadPath string `koanf:"upload_path"`
 	UploadURI  string `koanf:"upload_uri"`
+	RootURL    string `koanf:"root_url"`
 }
 
 // Client implements `media.Store`
@@ -26,6 +27,12 @@ type Client struct {
 	opts Opts
 }
 
+// This matches filenames, sans extensions, of the format
+// filename_(number). The number is incremented in case
+// new file uploads conflict with existing filenames
+// on the filesystem.
+var fnameRegexp = regexp.MustCompile(`(.+?)_([0-9]+)$`)
+
 // NewDiskStore initialises store for Filesystem provider.
 func NewDiskStore(opts Opts) (media.Store, error) {
 	return &Client{
@@ -34,7 +41,7 @@ func NewDiskStore(opts Opts) (media.Store, error) {
 }
 
 // Put accepts the filename, the content type and file object itself and stores the file in disk.
-func (e *Client) Put(filename string, cType string, src io.ReadSeeker) (string, error) {
+func (c *Client) Put(filename string, cType string, src io.ReadSeeker) (string, error) {
 	var out *os.File
 	// There's no explicit name. Use the one posted in the HTTP request.
 	if filename == "" {
@@ -44,7 +51,7 @@ func (e *Client) Put(filename string, cType string, src io.ReadSeeker) (string,
 		}
 	}
 	// Get the directory path
-	dir := getDir(e.opts.UploadPath)
+	dir := getDir(c.opts.UploadPath)
 	filename = assertUniqueFilename(dir, filename)
 	o, err := os.OpenFile(filepath.Join(dir, filename), os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0664)
 	if err != nil {
@@ -60,13 +67,13 @@ func (e *Client) Put(filename string, cType string, src io.ReadSeeker) (string,
 }
 
 // Get accepts a filename and retrieves the full path from disk.
-func (e *Client) Get(name string) string {
-	return fmt.Sprintf("%s/%s", e.opts.UploadURI, name)
+func (c *Client) Get(name string) string {
+	return fmt.Sprintf("%s%s/%s", c.opts.RootURL, c.opts.UploadURI, name)
 }
 
 // Delete accepts a filename and removes it from disk.
-func (e *Client) Delete(file string) error {
-	dir := getDir(e.opts.UploadPath)
+func (c *Client) Delete(file string) error {
+	dir := getDir(c.opts.UploadPath)
 	err := os.Remove(filepath.Join(dir, file))
 	if err != nil {
 		return err
@@ -74,12 +81,6 @@ func (e *Client) Delete(file string) error {
 	return nil
 }
 
-// This matches filenames, sans extensions, of the format
-// filename_(number). The number is incremented in case
-// new file uploads conflict with existing filenames
-// on the filesystem.
-var fnameRegexp = regexp.MustCompile(`(.+?)_([0-9]+)$`)
-
 // assertUniqueFilename takes a file path and check if it exists on the disk. If it doesn't,
 // it returns the same name and if it does, it adds a small random hash to the filename
 // and returns that.
diff --git a/internal/media/providers/s3/s3.go b/internal/media/providers/s3/s3.go
index 0b23686..9ccb0c2 100644
--- a/internal/media/providers/s3/s3.go
+++ b/internal/media/providers/s3/s3.go
@@ -4,13 +4,14 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"strings"
 	"time"
 
 	"github.com/knadh/listmonk/internal/media"
 	"github.com/rhnvrm/simples3"
 )
 
-const amznS3PublicURL = "https://%s.s3.%s.amazonaws.com/%s"
+const amznS3PublicURL = "https://%s.s3.%s.amazonaws.com%s"
 
 // Opts represents AWS S3 specific params
 type Opts struct {
@@ -19,6 +20,7 @@ type Opts struct {
 	Region     string `koanf:"aws_default_region"`
 	Bucket     string `koanf:"bucket"`
 	BucketPath string `koanf:"bucket_path"`
+	BucketURL  string `koanf:"bucket_url"`
 	BucketType string `koanf:"bucket_type"`
 	Expiry     int    `koanf:"expiry"`
 }
@@ -54,55 +56,61 @@ func NewS3Store(opts Opts) (media.Store, error) {
 }
 
 // Put takes in the filename, the content type and file object itself and uploads to S3.
-func (e *Client) Put(name string, cType string, file io.ReadSeeker) (string, error) {
+func (c *Client) Put(name string, cType string, file io.ReadSeeker) (string, error) {
 	// Upload input parameters
 	upParams := simples3.UploadInput{
-		Bucket:      e.opts.Bucket,
-		ObjectKey:   getBucketPath(e.opts.BucketPath, name),
+		Bucket:      c.opts.Bucket,
 		ContentType: cType,
 		FileName:    name,
 		Body:        file,
+
+		// Paths inside the bucket should not start with /.
+		ObjectKey: strings.TrimPrefix(makeBucketPath(c.opts.BucketPath, name), "/"),
 	}
 	// Perform an upload.
-	_, err := e.s3.FileUpload(upParams)
-	if err != nil {
+	if _, err := c.s3.FileUpload(upParams); err != nil {
 		return "", err
 	}
 	return name, nil
 }
 
 // Get accepts the filename of the object stored and retrieves from S3.
-func (e *Client) Get(name string) string {
+func (c *Client) Get(name string) string {
 	// Generate a private S3 pre-signed URL if it's a private bucket.
-	if e.opts.BucketType == "private" {
-		url := e.s3.GeneratePresignedURL(simples3.PresignedInput{
-			Bucket:        e.opts.Bucket,
-			ObjectKey:     getBucketPath(e.opts.BucketPath, name),
+	if c.opts.BucketType == "private" {
+		url := c.s3.GeneratePresignedURL(simples3.PresignedInput{
+			Bucket:        c.opts.Bucket,
+			ObjectKey:     makeBucketPath(c.opts.BucketPath, name),
 			Method:        "GET",
 			Timestamp:     time.Now(),
-			ExpirySeconds: e.opts.Expiry,
+			ExpirySeconds: c.opts.Expiry,
 		})
 		return url
 	}
+
 	// Generate a public S3 URL if it's a public bucket.
-	url := fmt.Sprintf(amznS3PublicURL, e.opts.Bucket, e.opts.Region, getBucketPath(e.opts.BucketPath, name))
+	url := ""
+	if c.opts.BucketURL != "" {
+		url = c.opts.BucketURL + makeBucketPath(c.opts.BucketPath, name)
+	} else {
+		url = fmt.Sprintf(amznS3PublicURL, c.opts.Bucket, c.opts.Region,
+			makeBucketPath(c.opts.BucketPath, name))
+	}
 	return url
 }
 
 // Delete accepts the filename of the object and deletes from S3.
-func (e *Client) Delete(name string) error {
-	err := e.s3.FileDelete(simples3.DeleteInput{
-		Bucket:    e.opts.Bucket,
-		ObjectKey: getBucketPath(e.opts.BucketPath, name),
+func (c *Client) Delete(name string) error {
+	err := c.s3.FileDelete(simples3.DeleteInput{
+		Bucket:    c.opts.Bucket,
+		ObjectKey: strings.TrimPrefix(makeBucketPath(c.opts.BucketPath, name), "/"),
 	})
 	return err
 }
 
-// getBucketPath constructs the key for the object stored in S3.
-// If path is empty, the key is the combination of root of S3 bucket and filename.
-func getBucketPath(path string, name string) string {
-	if path == "" {
-		return fmt.Sprintf("%s", name)
+func makeBucketPath(bucketPath string, name string) string {
+	if bucketPath == "/" {
+		return "/" + name
 	}
-	return fmt.Sprintf("%s/%s", path, name)
+	return fmt.Sprintf("%s/%s", bucketPath, name)
 }
diff --git a/media.go b/media.go
index 7f7c7da..c34cbd4 100644
--- a/media.go
+++ b/media.go
@@ -99,7 +99,7 @@ func handleUploadMedia(c echo.Context) error {
 	}
 
 	// Write to the DB.
-	if _, err := app.queries.InsertMedia.Exec(uu, fName, thumbfName, 0, 0); err != nil {
+	if _, err := app.queries.InsertMedia.Exec(uu, fName, thumbfName, app.constants.MediaProvider); err != nil {
 		cleanUp = true
 		app.log.Printf("error inserting uploaded file to db: %v", err)
 		return echo.NewHTTPError(http.StatusInternalServerError,
@@ -115,14 +115,14 @@ func handleGetMedia(c echo.Context) error {
 		out = []media.Media{}
 	)
 
-	if err := app.queries.GetMedia.Select(&out); err != nil {
+	if err := app.queries.GetMedia.Select(&out, app.constants.MediaProvider); err != nil {
 		return echo.NewHTTPError(http.StatusInternalServerError,
 			fmt.Sprintf("Error fetching media list: %s", pqErrMsg(err)))
 	}
 
 	for i := 0; i < len(out); i++ {
-		out[i].URI = app.media.Get(out[i].Filename)
-		out[i].ThumbURI = app.media.Get(thumbPrefix + out[i].Filename)
+		out[i].URL = app.media.Get(out[i].Filename)
+		out[i].ThumbURL = app.media.Get(out[i].Thumb)
 	}
 
 	return c.JSON(http.StatusOK, okResp{out})
diff --git a/queries.sql b/queries.sql
index 49c95ea..48df172 100644
--- a/queries.sql
+++ b/queries.sql
@@ -654,10 +654,10 @@ UPDATE campaigns SET template_id = (SELECT id FROM def) WHERE (SELECT id FROM tp
 
 -- media
 -- name: insert-media
-INSERT INTO media (uuid, filename, thumb, width, height, created_at) VALUES($1, $2, $3, $4, $5, NOW());
+INSERT INTO media (uuid, filename, thumb, provider, created_at) VALUES($1, $2, $3, $4, NOW());
 
 -- name: get-media
-SELECT * FROM media ORDER BY created_at DESC;
+SELECT * FROM media WHERE provider=$1 ORDER BY created_at DESC;
 
 -- name: delete-media
 DELETE FROM media WHERE id=$1 RETURNING filename;
diff --git a/schema.sql b/schema.sql
index 5c0493b..95a9f32 100644
--- a/schema.sql
+++ b/schema.sql
@@ -128,10 +128,9 @@ DROP TABLE IF EXISTS media CASCADE;
 CREATE TABLE media (
     id               SERIAL PRIMARY KEY,
     uuid uuid        NOT NULL UNIQUE,
+    provider         TEXT NOT NULL,
     filename         TEXT NOT NULL,
     thumb            TEXT NOT NULL,
-    width            INT NOT NULL DEFAULT 0,
-    height           INT NOT NULL DEFAULT 0,
     created_at       TIMESTAMP WITH TIME ZONE DEFAULT NOW()
 );