Use config mutex for sessions to prevent data race, added note about bug in Go <1.17 regarding data race in html/template

internal/configuration/Configuration.go

@@ -85,7 +85,7 @@ func Lock() {
 
 // ReleaseAndSave unlocks and saves the configuration
 func ReleaseAndSave() {
-	Save()
+	save()
 	mutex.Unlock()
 }
 
@@ -131,7 +131,7 @@ func updateConfig() {
 	if serverSettings.ConfigVersion < currentConfigVersion {
 		fmt.Println("Successfully upgraded database")
 		serverSettings.ConfigVersion = currentConfigVersion
-		Save()
+		save()
 	}
 }
 
@@ -179,11 +179,11 @@ func generateDefaultConfig() {
 		DataDir:          Environment.DataDir,
 		LengthId:         Environment.LengthId,
 	}
-	Save()
+	save()
 }
 
 // Save the configuration as a json file
-func Save() {
+func save() {
 	file, err := os.OpenFile(Environment.ConfigPath, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0644)
 	if err != nil {
 		fmt.Println("Error reading configuration:", err)
@@ -362,7 +362,7 @@ func addTrailingSlash(url string) string {
 // DisplayPasswordReset shows a password prompt in the CLI and saves the new password
 func DisplayPasswordReset() {
 	serverSettings.AdminPassword = HashPassword(askForPassword(), false)
-	Save()
+	save()
 }
 
 // HashPassword hashes a string with SHA256 and a salt
@@ -384,8 +384,3 @@ func HashPassword(password string, useFileSalt bool) string {
 func GetLengthId() int {
 	return serverSettings.LengthId
 }
-
-// GetSessions returns a pointer to the session storage
-func GetSessions() *map[string]models.Session {
-	return &serverSettings.Sessions
-}

internal/configuration/Configuration_test.go

@@ -89,7 +89,7 @@ func TestUpgradeDb(t *testing.T) {
 	test.IsEqualString(t, serverSettings.DataDir, Environment.DataDir)
 	test.IsEqualInt(t, serverSettings.LengthId, 15)
 	test.IsEqualBool(t, serverSettings.Hotlinks == nil, false)
-	test.IsEqualBool(t, GetSessions() == nil, false)
+	test.IsEqualBool(t, serverSettings.Sessions == nil, false)
 	test.IsEqualBool(t, serverSettings.DownloadStatus == nil, false)
 	test.IsEqualString(t, serverSettings.Files["MgXJLe4XLfpXcL12ec4i"].ContentType, "application/octet-stream")
 	test.IsEqualInt(t, serverSettings.ConfigVersion, currentConfigVersion)

internal/storage/FileServing.go

@@ -162,10 +162,11 @@ func CleanUp(periodic bool) {
 			wasItemDeleted = true
 		}
 	}
-	configuration.Release()
 	if wasItemDeleted {
-		configuration.Save()
+		configuration.ReleaseAndSave()
 		CleanUp(false)
+	} else {
+		configuration.Release()
 	}
 	if periodic {
 		time.Sleep(time.Hour)

internal/webserver/Webserver.go

@@ -154,7 +154,7 @@ func showLogin(w http.ResponseWriter, r *http.Request) {
 	failedLogin := false
 	if pw != "" && user != "" {
 		if strings.ToLower(user) == strings.ToLower(webserverAdminName) && configuration.HashPassword(pw, false) == webserverAdminPassword {
-			sessionmanager.CreateSession(w, false)
+			sessionmanager.CreateSession(w, nil)
 			redirect(w, "admin")
 			return
 		}

internal/webserver/Webserver_test.go

@@ -12,6 +12,9 @@ import (
 	"time"
 )
 
+// Please note that if this test is run with go test -race, it will fail as there is a bug in html/template that
+// causes data race. It will be fixed in Go 1.17, see https://github.com/golang/go/issues/39807
+
 func TestMain(m *testing.M) {
 	testconfiguration2.Create(true)
 	configuration.Load()

internal/webserver/sessionmanager/SessionManager.go

@@ -9,15 +9,12 @@ import (
 	"Gokapi/internal/helper"
 	"Gokapi/internal/models"
 	"net/http"
-	"sync"
 	"time"
 )
 
 // If no login occurred during this time, the admin session will be deleted. Default 30 days
 const cookieLifeAdmin = 30 * 24 * time.Hour
 
-var mutex sync.Mutex
-
 // IsValidSession checks if the user is submitting a valid session token
 // If valid session is found, useSession will be called
 // Returns true if authenticated, otherwise false
@@ -26,49 +23,42 @@ func IsValidSession(w http.ResponseWriter, r *http.Request) bool {
 	if err == nil {
 		sessionString := cookie.Value
 		if sessionString != "" {
-			mutex.Lock()
-			sessions := configuration.GetSessions()
-			defer func() { unlockAndSave() }()
-			_, ok := (*sessions)[sessionString]
+			settings := configuration.GetServerSettings()
+			defer func() { configuration.ReleaseAndSave() }()
+			_, ok := (settings.Sessions)[sessionString]
 			if ok {
-				return useSession(w, sessionString)
+				return useSession(w, sessionString, &settings.Sessions)
 			}
 		}
 	}
 	return false
 }
 
-func unlockAndSave() {
-	configuration.Save()
-	mutex.Unlock()
-}
-
 // useSession checks if a session is still valid. It Changes the session string
 // if it has // been used for more than an hour to limit session hijacking
 // Returns true if session is still valid
 // Returns false if session is invalid (and deletes it)
-func useSession(w http.ResponseWriter, sessionString string) bool {
-	sessions := configuration.GetSessions()
+func useSession(w http.ResponseWriter, sessionString string, sessions *map[string]models.Session) bool {
 	session := (*sessions)[sessionString]
 	if session.ValidUntil < time.Now().Unix() {
 		delete(*sessions, sessionString)
 		return false
 	}
 	if session.RenewAt < time.Now().Unix() {
-		CreateSession(w, true)
+		CreateSession(w, sessions)
 		delete(*sessions, sessionString)
 	}
 	return true
 }
 
 // CreateSession creates a new session - called after login with correct username / password
-func CreateSession(w http.ResponseWriter, isLocked bool) {
-	if !isLocked {
-		mutex.Lock()
-		defer func() { unlockAndSave() }()
+func CreateSession(w http.ResponseWriter, sessions *map[string]models.Session) {
+	if sessions == nil {
+		settings := configuration.GetServerSettings()
+		sessions = &settings.Sessions
+		defer func() { configuration.ReleaseAndSave() }()
 	}
 	sessionString := helper.GenerateRandomString(60)
-	sessions := configuration.GetSessions()
 	(*sessions)[sessionString] = models.Session{
 		RenewAt:    time.Now().Add(time.Hour).Unix(),
 		ValidUntil: time.Now().Add(cookieLifeAdmin).Unix(),
@@ -80,10 +70,9 @@ func CreateSession(w http.ResponseWriter, isLocked bool) {
 func LogoutSession(w http.ResponseWriter, r *http.Request) {
 	cookie, err := r.Cookie("session_token")
 	if err == nil {
-		mutex.Lock()
-		sessions := configuration.GetSessions()
-		delete(*sessions, cookie.Value)
-		unlockAndSave()
+		settings := configuration.GetServerSettings()
+		delete(settings.Sessions, cookie.Value)
+		configuration.ReleaseAndSave()
 	}
 	writeSessionCookie(w, "", time.Now())
 }

internal/webserver/sessionmanager/SessionManager_test.go

@@ -64,7 +64,7 @@ func TestIsValidSession(t *testing.T) {
 
 func TestCreateSession(t *testing.T) {
 	w, _ := getRecorder(nil)
-	CreateSession(w, false)
+	CreateSession(w, nil)
 	cookies := w.Result().Cookies()
 	test.IsEqualInt(t, len(cookies), 1)
 	test.IsEqualString(t, cookies[0].Name, "session_token")