Fix setup option "save images always locally" reverting to default in setup #165, added option to proxy downloads from s3 to setup #75, rewording in setup

internal/configuration/setup/Setup.go

@@ -447,8 +447,12 @@ func parseCloudSettings(formObjects *[]jsonFormObject) (*cloudconfig.CloudConfig
 }
 
 func getCloudConfig(formObjects *[]jsonFormObject) (*cloudconfig.CloudConfig, error) {
-	var err error
 	awsConfig := cloudconfig.CloudConfig{}
+	proxyDownload, err := getFormValueString(formObjects, "storage_sel_proxy")
+	if err != nil {
+		return nil, err
+	}
+	awsConfig.Aws.ProxyDownload = proxyDownload == "proxy"
 	awsConfig.Aws.Bucket, err = getFormValueString(formObjects, "s3_bucket")
 	if err != nil {
 		return nil, err

internal/configuration/setup/Setup_test.go

@@ -5,6 +5,7 @@ import (
 	"context"
 	"encoding/json"
 	"errors"
+	"fmt"
 	"github.com/forceu/gokapi/internal/configuration"
 	"github.com/forceu/gokapi/internal/configuration/cloudconfig"
 	"github.com/forceu/gokapi/internal/configuration/database"
@@ -35,6 +36,10 @@ func TestMain(m *testing.M) {
 
 func TestDebugNotSet(t *testing.T) {
 	test.IsEqualBool(t, debugDisableAuth, false)
+	if debugDisableAuth {
+		fmt.Println("Debug mode is still on! Exiting test")
+		os.Exit(1)
+	}
 }
 
 func TestInputToJson(t *testing.T) {
@@ -345,6 +350,7 @@ func TestIntegration(t *testing.T) {
 		test.IsEqualString(t, cconfig.Aws.KeyId, "testapi")
 		test.IsEqualString(t, cconfig.Aws.KeySecret, "testsecret")
 		test.IsEqualString(t, cconfig.Aws.Endpoint, "testendpoint")
+		test.IsEqualBool(t, cconfig.Aws.ProxyDownload, true)
 	}
 	test.FileExists(t, "test/cloudconfig.yml")
 
@@ -418,7 +424,7 @@ func TestIntegration(t *testing.T) {
 	test.IsEqualString(t, settings.ServerUrl, "http://127.0.0.1:53842/")
 	test.IsEqualString(t, settings.RedirectUrl, "https://test.com")
 	test.IsEqualBool(t, settings.PicturesAlwaysLocal, false)
-	_, ok = cloudconfig.Load()
+	cconfig, ok = cloudconfig.Load()
 	if os.Getenv("GOKAPI_AWS_BUCKET") == "" {
 		test.IsEqualBool(t, ok, false)
 	}
@@ -445,6 +451,7 @@ func TestIntegration(t *testing.T) {
 	waitForServer(t, false)
 
 	test.IsEqualBool(t, settings.PicturesAlwaysLocal, true)
+	test.IsEqualBool(t, cconfig.Aws.ProxyDownload, false)
 	test.IsEqualString(t, settings.Authentication.OAuthProvider, "provider")
 	test.IsEqualString(t, settings.Authentication.OAuthClientId, "id")
 	test.IsEqualString(t, settings.Authentication.OAuthClientSecret, "secret")
@@ -481,6 +488,7 @@ type setupValues struct {
 	AuthHeaderUsers       setupEntry `form:"auth_header_users"`
 	StorageSelection      setupEntry `form:"storage_sel"`
 	PicturesAlwaysLocal   setupEntry `form:"storage_sel_image"`
+	ProxyDownloads        setupEntry `form:"storage_sel_proxy"`
 	S3Bucket              setupEntry `form:"s3_bucket"`
 	S3Region              setupEntry `form:"s3_region"`
 	S3ApiKey              setupEntry `form:"s3_api"`
@@ -591,6 +599,7 @@ func createInputInternalAuth() setupValues {
 	values.AuthUsername.Value = "admin"
 	values.AuthPassword.Value = "adminadmin"
 	values.StorageSelection.Value = "cloud"
+	values.ProxyDownloads.Value = "proxy"
 	values.S3Bucket.Value = "testbucket"
 	values.S3Region.Value = "testregion"
 	values.S3ApiKey.Value = "testapi"
@@ -643,6 +652,7 @@ func createInputOAuth() setupValues {
 	values.OAuthAuthorisedGroups.Value = "group1; group2"
 	values.StorageSelection.Value = "local"
 	values.PicturesAlwaysLocal.Value = "local"
+	values.ProxyDownloads.Value = "default"
 	return values
 }
 

internal/configuration/setup/templates/setup.tmpl

@@ -344,7 +344,7 @@
 
 				<div class="wizard-input-section">
 					<p>
-						Select if you want to store files locally or on an S3 compatible cloud infrastructure
+						Choose whether to store files locally or on an S3-compatible cloud infrastructure.
 					</p>
 
 					<select name="storage_sel" id="storage_sel" style="width:350px;" class="select form-control" onchange="storageSelectionChanged(this.selectedIndex);">
@@ -357,14 +357,25 @@
 					<p><br><br><i>This build has been compiled without AWS support, therefore cloud storage is not available.</i></p>
 {{ else }}
 
-					<br><p>
-						Select if you want to store pictures always locally. Choose this if you want to use full encryption but still be able to have hotlink support.
-					</p>
-					<select name="storage_sel_image" id="storage_sel_image" style="width:350px;" class="select form-control">
-							<option value="nochange" selected>As above</option>
-							<option value="local">Always local Storage</option>
+					<div id="cloudstorage_options" style="visibility: hidden">
 
-					</select>
+						<br><p>
+							Location for picture files: Select local storage if you require full encryption but still need hotlink support.
+						</p>
+						<select name="storage_sel_image" id="storage_sel_image" style="width:350px;" class="select form-control">
+								<option value="nochange" selected>Use cloud storage</option>
+								<option value="local">Use local storage for pictures</option>
+
+						</select>
+						<br><p>
+							To conserve bandwidth, Gokapi defaults to redirecting to pre-signed S3 download URLs. If you prefer not to use this feature or if your S3 server is only accessible from your internal network, select the "Proxy downloads" option below.
+						</p>
+						<select name="storage_sel_proxy" id="storage_sel_proxy" style="width:350px;" class="select form-control">
+								<option value="presigned" selected>Default</option>
+								<option value="proxy">Proxy downloads</option>
+
+						</select>
+					</div>
 {{ end }}
 					
 
@@ -651,7 +662,7 @@ function TestAWS(button, isManual) {
 					</div>
 					If you configured Gokapi to use a password during startup, please enter it now in the console before clicking on continue.<br><br>
 	
-					<a class="btn btn-success im-done">Continue</a>
+					<a id="bt_finishsetup" class="btn btn-success im-done">Continue</a>
 				</div>
 			</div>
 		</div>
@@ -676,7 +687,9 @@ function TestAWS(button, isManual) {
 				
 				
 				wizard.el.find(".wizard-success .im-done").click(function() {
-					window.location.href = $("#url").val()+"admin";
+					document.getElementById("bt_finishsetup").disabled=true;
+					document.getElementById("bt_finishsetup").text="Please wait";
+					setTimeout(function(){ window.location.href = $("#url").val()+"admin";}, 1500);
 				});
 			
 			
@@ -735,10 +748,20 @@ function TestAWS(button, isManual) {
 				    document.getElementById("auth_header_users").value = "{{ .HeaderUsers }}";
 				    break;
 				}
+				
 
 	{{ if .CloudSettings.Aws.Bucket }}				
 				document.getElementById("storage_sel").value = "cloud";
 				storageSelectionChanged(1);
+				
+		{{ if .CloudSettings.Aws.ProxyDownload }}
+				document.getElementById("storage_sel_proxy").value = "proxy";
+		{{ end }}
+		
+		{{ if .Settings.PicturesAlwaysLocal }}
+				document.getElementById("storage_sel_image").value = "local";
+		{{ end }}
+				
 		{{ if not .S3EnvProvided }}
 				document.getElementById("s3_bucket").value = "{{ .CloudSettings.Aws.Bucket }}";
 				document.getElementById("s3_region").value =  "{{ .CloudSettings.Aws.Region }}";
@@ -886,10 +909,15 @@ function TestAWS(button, isManual) {
 
 			function storageSelectionChanged(index) {
 				let card = wizard.cards["s3credentials"];
-				if (index==0) 
+				let storageOptDiv = document.getElementById("cloudstorage_options");
+				if (index==0) {
 					card.disable(true);
-				else if (index==1) 
+					storageOptDiv.style.visibility="hidden";
+				} else {
 					card.enable();
+					storageOptDiv.style.visibility="visible";
+				}
+					
 			}
 
 			$(function() {

internal/storage/FileServing.go

@@ -534,11 +534,14 @@ func ServeFile(file models.File, w http.ResponseWriter, r *http.Request, forceDo
 	logging.AddDownload(&file, r, configuration.Get().SaveIp)
 
 	if !file.IsLocalStorage() {
-		// We are not setting a download complete status as there is no reliable way to
+		// If non-blocking, we are not setting a download complete status as there is no reliable way to
 		// confirm that the file has been completely downloaded. It expires automatically after 24 hours.
-		downloadstatus.SetDownload(file)
-		err := aws.ServeFile(w, r, file, forceDownload)
+		statusId := downloadstatus.SetDownload(file)
+		isBlocking, err := aws.ServeFile(w, r, file, forceDownload)
 		helper.Check(err)
+		if isBlocking {
+			downloadstatus.SetComplete(statusId)
+		}
 		return
 	}
 	fileData, size := getFileHandler(file, configuration.Get().DataDir)

internal/storage/filesystem/s3filesystem/aws/Aws.go

@@ -119,17 +119,16 @@ func Download(writer io.WriterAt, file models.File) (int64, error) {
 	return size, nil
 }
 
-// ServeFile either redirects the user to a pre-signed download url (default) or downloads the file and serves it as a proxy (if request
-func ServeFile(w http.ResponseWriter, r *http.Request, file models.File, forceDownload bool) error {
+// ServeFile either redirects the user to a pre-signed download url (default) or downloads the file and serves it as a proxy (depending
+// on configuration). Returns true if blocking operation (in order to set download status) or false if non-blocking.
+func ServeFile(w http.ResponseWriter, r *http.Request, file models.File, forceDownload bool) (bool, error) {
 	if !awsConfig.ProxyDownload {
-		return RedirectToDownload(w, r, file, forceDownload)
+		return false, redirectToDownload(w, r, file, forceDownload)
 	}
-	return ProxyDownload(w, file, forceDownload)
+	return true, proxyDownload(w, file, forceDownload)
 }
 
-// RedirectToDownload creates a presigned link that is valid for 15 seconds and redirects the
-// client to this url
-func RedirectToDownload(w http.ResponseWriter, r *http.Request, file models.File, forceDownload bool) error {
+func getPresignedUrl(file models.File, forceDownload bool) (string, error) {
 	sess := createSession()
 	s3svc := s3.New(sess)
 
@@ -146,7 +145,13 @@ func RedirectToDownload(w http.ResponseWriter, r *http.Request, file models.File
 		ResponseContentType:        aws.String(file.ContentType),
 	})
 
-	url, err := req.Presign(15 * time.Second)
+	return req.Presign(15 * time.Second)
+}
+
+// redirectToDownload creates a presigned link that is valid for 15 seconds and redirects the
+// client to this url
+func redirectToDownload(w http.ResponseWriter, r *http.Request, file models.File, forceDownload bool) error {
+	url, err := getPresignedUrl(file, forceDownload)
 	if err != nil {
 		return err
 	}
@@ -155,28 +160,13 @@ func RedirectToDownload(w http.ResponseWriter, r *http.Request, file models.File
 	return nil
 }
 
-// ProxyDownload streams the file from S3 as a proxy
-func ProxyDownload(w http.ResponseWriter, file models.File, forceDownload bool) error {
-	sess := createSession()
-	s3svc := s3.New(sess)
-
-	contentDisposition := "inline; filename=\"" + file.Name + "\""
-	if forceDownload {
-		contentDisposition = "Attachment; filename=\"" + file.Name + "\""
-	}
-
-	req, _ := s3svc.GetObjectRequest(&s3.GetObjectInput{
-		Bucket:                     aws.String(file.AwsBucket),
-		Key:                        aws.String(file.SHA1),
-		ResponseContentDisposition: aws.String(contentDisposition),
-		ResponseCacheControl:       aws.String("no-store"),
-		ResponseContentType:        aws.String(file.ContentType),
-	})
-
-	url, err := req.Presign(15 * time.Second)
+// proxyDownload streams the file from S3 as a proxy, by downloading a presigned url
+func proxyDownload(w http.ResponseWriter, file models.File, forceDownload bool) error {
+	url, err := getPresignedUrl(file, forceDownload)
 	if err != nil {
 		return err
 	}
+
 	resp, err := http.Get(url)
 	if err != nil {
 		return err
@@ -214,7 +204,8 @@ func fileExists(bucket, filename string) (bool, int64, error) {
 	})
 
 	if err != nil {
-		aerr, ok := err.(awserr.Error)
+		var aerr awserr.Error
+		ok := errors.As(err, &aerr)
 		if ok {
 			if aerr.Code() == "NotFound" {
 				return false, 0, nil
@@ -260,7 +251,8 @@ func IsCorsCorrectlySet(bucket, gokapiUrl string) (bool, error) {
 
 	result, err := svc.GetBucketCorsWithContext(ctx, input)
 	if err != nil {
-		aerr, ok := err.(awserr.Error)
+		var aerr awserr.Error
+		ok := errors.As(err, &aerr)
 		if ok && aerr.Code() == "NoSuchCorsConfiguration" {
 			return false, nil
 		}

internal/storage/filesystem/s3filesystem/aws/Aws_mock.go

@@ -127,9 +127,11 @@ func isUploaded(file models.File) bool {
 	return false
 }
 
-// ServeFile either redirects the user to a pre-signed download url (default) or downloads the file and serves it as a proxy (if request
-func ServeFile(w http.ResponseWriter, r *http.Request, file models.File, forceDownload bool) error {
-	return RedirectToDownload(w, r, file, forceDownload)
+// ServeFile either redirects the user to a pre-signed download url (default) or downloads the file and serves it as a proxy (depending
+// on configuration). Returns true if blocking operation (in order to set download status) or false if non-blocking.
+func ServeFile(w http.ResponseWriter, r *http.Request, file models.File, forceDownload bool) (bool, error) {
+	// TODO implement proxy as well
+	return false, RedirectToDownload(w, r, file, forceDownload)
 }
 
 // RedirectToDownload creates a presigned link that is valid for 15 seconds and redirects the

internal/storage/filesystem/s3filesystem/aws/Aws_slim.go

@@ -57,9 +57,10 @@ func RedirectToDownload(w http.ResponseWriter, r *http.Request, file models.File
 	return errors.New(errorString)
 }
 
-// ServeFile either redirects the user to a pre-signed download url (default) or downloads the file and serves it as a proxy (if request
-func ServeFile(w http.ResponseWriter, r *http.Request, file models.File, forceDownload bool) error {
-	return errors.New(errorString)
+// ServeFile either redirects the user to a pre-signed download url (default) or downloads the file and serves it as a proxy (depending
+// on configuration). Returns true if blocking operation (in order to set download status) or false if non-blocking.
+func ServeFile(w http.ResponseWriter, r *http.Request, file models.File, forceDownload bool) (bool, error) {
+	return false, errors.New(errorString)
 }
 
 // FileExists returns true if the object is stored in S3

internal/storage/filesystem/s3filesystem/aws/Aws_test.go

@@ -8,16 +8,22 @@ import (
 	"github.com/forceu/gokapi/internal/test"
 	"github.com/johannesboyne/gofakes3"
 	"github.com/johannesboyne/gofakes3/backend/s3mem"
+	"io"
+	"net/http"
 	"net/http/httptest"
 	"os"
+	"strings"
 	"testing"
 )
 
 var testFile, invalidFile, invalidBucket, invalidAll models.File
 
+var isRealAwsServer bool
+
 func TestMain(m *testing.M) {
 	testFile.AwsBucket = "gokapi-test"
 	testFile.SHA1 = "testfile"
+	testFile.Name = "Testfile.jpg"
 	invalidFile.AwsBucket = "gokapi-test"
 	invalidFile.SHA1 = "invalid"
 	invalidBucket.AwsBucket = "invalid"
@@ -28,6 +34,8 @@ func TestMain(m *testing.M) {
 		ts := startMockServer()
 		os.Setenv("GOKAPI_AWS_ENDPOINT", ts.URL)
 		defer ts.Close()
+	} else {
+		isRealAwsServer = true
 	}
 	exitVal := m.Run()
 	os.Exit(exitVal)
@@ -87,21 +95,62 @@ func TestDownloadFromAws(t *testing.T) {
 	os.Remove("test")
 }
 
-func TestRedirectToDownload(t *testing.T) {
+func TestServeFile(t *testing.T) {
+	awsConfig.ProxyDownload = false
+	testServing(t, true, false)
+	testServing(t, true, true)
+
+	awsConfig.ProxyDownload = true
+	testServing(t, false, false)
+	testServing(t, false, true)
+	awsConfig.ProxyDownload = false
+}
+
+func testServing(t *testing.T, expectRedirect, forceDownload bool) {
 	w := httptest.NewRecorder()
 	r := httptest.NewRequest("GET", "/download", nil)
-	err := RedirectToDownload(w, r, testFile, false)
-	test.IsNil(t, err)
-	test.ResponseBodyContains(t, w, "<a href=\"http")
-	test.IsEqualInt(t, w.Code, 307)
 
-	// Test with force download
-	w = httptest.NewRecorder()
-	r = httptest.NewRequest("GET", "/download", nil)
-	err = RedirectToDownload(w, r, testFile, true)
+	isBlockng, err := ServeFile(w, r, testFile, forceDownload)
+	test.IsEqualBool(t, isBlockng, !expectRedirect)
 	test.IsNil(t, err)
-	test.ResponseBodyContains(t, w, "<a href=\"http")
-	test.IsEqualInt(t, w.Code, 307)
+
+	response, err := io.ReadAll(w.Result().Body)
+	test.IsNil(t, err)
+
+	if !expectRedirect {
+		test.IsEqualString(t, string(response), "testfile-content")
+		test.IsEqualInt(t, w.Code, 200)
+		// content-disposition not implemented in s3mem
+		if isRealAwsServer {
+			// TODO
+		}
+	} else {
+		for _, s := range []string{"<a href=\"http", "Testfile.jpg"} {
+			test.IsEqualBool(t, strings.Contains(string(response), s), true)
+		}
+		test.IsEqualInt(t, w.Code, 307)
+
+		// Get the redirect URL from the response headers
+		redirectURL := w.Header().Get("Location")
+		if redirectURL == "" {
+			t.Fatal("Expected redirect URL in Location header")
+		}
+
+		// Follow the redirect and download the content
+		resp, err := http.Get(redirectURL)
+		test.IsNil(t, err)
+		defer resp.Body.Close()
+
+		// Read the content of the downloaded file and  check the content of the downloaded file
+		downloadedContent, err := io.ReadAll(resp.Body)
+		test.IsNil(t, err)
+		test.IsEqualString(t, string(downloadedContent), "testfile-content")
+
+		// content-disposition not implemented in s3mem
+		if isRealAwsServer {
+			// TODO
+		}
+	}
 }
 
 func TestFileExists(t *testing.T) {
@@ -136,3 +185,15 @@ func TestLogOut(t *testing.T) {
 	LogOut()
 	test.IsEqualBool(t, isCorrectLogin, false)
 }
+
+func TestGetDefaultBucketName(t *testing.T) {
+	test.IsEqualString(t, GetDefaultBucketName(), awsConfig.Bucket)
+}
+
+func TestIsCorsCorrectlySet(t *testing.T) {
+	// not implemented in s3mem
+	if !isRealAwsServer {
+		return
+	}
+	// TODO
+}

internal/test/TestHelper.go

@@ -40,7 +40,8 @@ func IsEqualString(t MockT, got, want string) {
 // ResponseBodyContains fails test if http response does contain string
 func ResponseBodyContains(t MockT, got *httptest.ResponseRecorder, want string) {
 	t.Helper()
-	result, _ := io.ReadAll(got.Result().Body)
+	result, err := io.ReadAll(got.Result().Body)
+	IsNil(t, err)
 	if !strings.Contains(string(result), want) {
 		t.Errorf("Assertion failed, got: %s, want: %s.", got, want)
 	}