fix: decouple images from app config service (#965)

Co-authored-by: Alessandro (Ale) Segala <43508+ItalyPaleAle@users.noreply.github.com>

backend/internal/bootstrap/app_images_bootstrap.go

@@ -4,11 +4,9 @@ import (
 	"bytes"
 	"encoding/hex"
 	"fmt"
-	"io/fs"
 	"log/slog"
 	"os"
 	"path"
-	"strings"
 
 	"github.com/pocket-id/pocket-id/backend/internal/common"
 	"github.com/pocket-id/pocket-id/backend/internal/utils"
@@ -16,10 +14,8 @@ import (
 )
 
 // initApplicationImages copies the images from the images directory to the application-images directory
-func initApplicationImages() error {
-	// Images that are built into the Pocket ID binary
-	builtInImageHashes := getBuiltInImageHashes()
-
+// and returns a map containing the detected file extensions in the application-images directory.
+func initApplicationImages() (map[string]string, error) {
 	// Previous versions of images
 	// If these are found, they are deleted
 	legacyImageHashes := imageHashMap{
@@ -30,16 +26,20 @@ func initApplicationImages() error {
 
 	sourceFiles, err := resources.FS.ReadDir("images")
 	if err != nil && !os.IsNotExist(err) {
-		return fmt.Errorf("failed to read directory: %w", err)
+		return nil, fmt.Errorf("failed to read directory: %w", err)
 	}
 
 	destinationFiles, err := os.ReadDir(dirPath)
 	if err != nil && !os.IsNotExist(err) {
-		return fmt.Errorf("failed to read directory: %w", err)
+		return nil, fmt.Errorf("failed to read directory: %w", err)
 	}
-	destinationFilesMap := make(map[string]bool, len(destinationFiles))
+	dstNameToExt := make(map[string]string, len(destinationFiles))
 	for _, f := range destinationFiles {
+		if f.IsDir() {
+			continue
+		}
 		name := f.Name()
+		nameWithoutExt, ext := utils.SplitFileName(name)
 		destFilePath := path.Join(dirPath, name)
 
 		// Skip directories
@@ -58,50 +58,43 @@ func initApplicationImages() error {
 			slog.Info("Found legacy application image that will be removed", slog.String("name", name))
 			err = os.Remove(destFilePath)
 			if err != nil {
-				return fmt.Errorf("failed to remove legacy file '%s': %w", name, err)
+				return nil, fmt.Errorf("failed to remove legacy file '%s': %w", name, err)
 			}
 			continue
 		}
 
-		// Check if the file is a built-in one and save it in the map
-		destinationFilesMap[getImageNameWithoutExtension(name)] = builtInImageHashes.Contains(h)
+		// Track existing files
+		dstNameToExt[nameWithoutExt] = ext
 	}
 
 	// Copy images from the images directory to the application-images directory if they don't already exist
 	for _, sourceFile := range sourceFiles {
-		// Skip if it's a directory
 		if sourceFile.IsDir() {
 			continue
 		}
 
 		name := sourceFile.Name()
+		nameWithoutExt, ext := utils.SplitFileName(name)
 		srcFilePath := path.Join("images", name)
 		destFilePath := path.Join(dirPath, name)
 
 		// Skip if there's already an image at the path
 		// We do not check the extension because users could have uploaded a different one
-		if imageAlreadyExists(sourceFile, destinationFilesMap) {
+		if _, exists := dstNameToExt[nameWithoutExt]; exists {
 			continue
 		}
 
 		slog.Info("Writing new application image", slog.String("name", name))
 		err := utils.CopyEmbeddedFileToDisk(srcFilePath, destFilePath)
 		if err != nil {
-			return fmt.Errorf("failed to copy file: %w", err)
+			return nil, fmt.Errorf("failed to copy file: %w", err)
 		}
+
+		// Track the newly copied file so it can be included in the extensions map later
+		dstNameToExt[nameWithoutExt] = ext
 	}
 
-	return nil
-}
-
-func getBuiltInImageHashes() imageHashMap {
-	return imageHashMap{
-		"background.webp": mustDecodeHex("3fc436a66d6b872b01d96a4e75046c46b5c3e2daccd51e98ecdf98fd445599ab"),
-		"favicon.ico":     mustDecodeHex("70f9c4b6bd4781ade5fc96958b1267511751e91957f83c2354fb880b35ec890a"),
-		"logo.svg":        mustDecodeHex("f1e60707df9784152ce0847e3eb59cb68b9015f918ff160376c27ebff1eda796"),
-		"logoDark.svg":    mustDecodeHex("0421a8d93714bacf54c78430f1db378fd0d29565f6de59b6a89090d44a82eb16"),
-		"logoLight.svg":   mustDecodeHex("6d42c88cf6668f7e57c4f2a505e71ecc8a1e0a27534632aa6adec87b812d0bb0"),
-	}
+	return dstNameToExt, nil
 }
 
 type imageHashMap map[string][]byte
@@ -118,21 +111,6 @@ func (m imageHashMap) Contains(target []byte) bool {
 	return false
 }
 
-func imageAlreadyExists(sourceFile fs.DirEntry, destinationFiles map[string]bool) bool {
-	sourceFileWithoutExtension := getImageNameWithoutExtension(sourceFile.Name())
-	_, ok := destinationFiles[sourceFileWithoutExtension]
-	return ok
-}
-
-func getImageNameWithoutExtension(fileName string) string {
-	idx := strings.LastIndexByte(fileName, '.')
-	if idx < 1 {
-		// No dot found, or fileName starts with a dot
-		return fileName
-	}
-	return fileName[:idx]
-}
-
 func mustDecodeHex(str string) []byte {
 	b, err := hex.DecodeString(str)
 	if err != nil {

backend/internal/bootstrap/application_images_bootstrap_test.go

@@ -1,61 +0,0 @@
-package bootstrap
-
-import (
-	"os"
-	"path/filepath"
-	"strings"
-	"testing"
-
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-
-	"github.com/pocket-id/pocket-id/backend/internal/utils"
-)
-
-func TestGetBuiltInImageData(t *testing.T) {
-	// Get the built-in image data map
-	builtInImages := getBuiltInImageHashes()
-
-	// Read the actual images directory from disk
-	imagesDir := filepath.Join("..", "..", "resources", "images")
-	actualFiles, err := os.ReadDir(imagesDir)
-	require.NoError(t, err, "Failed to read images directory")
-
-	// Create a map of actual files for comparison
-	actualFilesMap := make(map[string]struct{})
-
-	// Validate each actual file exists in the built-in data with correct hash
-	for _, file := range actualFiles {
-		fileName := file.Name()
-		if file.IsDir() || strings.HasPrefix(fileName, ".") {
-			continue
-		}
-
-		actualFilesMap[fileName] = struct{}{}
-
-		// Check if the file exists in the built-in data
-		builtInHash, exists := builtInImages[fileName]
-		assert.True(t, exists, "File %s exists in images directory but not in getBuiltInImageData map", fileName)
-
-		if !exists {
-			continue
-		}
-
-		filePath := filepath.Join(imagesDir, fileName)
-
-		// Validate SHA256 hash
-		actualHash, err := utils.CreateSha256FileHash(filePath)
-		require.NoError(t, err, "Failed to compute hash for %s", fileName)
-		assert.Equal(t, actualHash, builtInHash, "SHA256 hash mismatch for file %s", fileName)
-	}
-
-	// Ensure the built-in data doesn't have extra files that don't exist in the directory
-	for fileName := range builtInImages {
-		_, exists := actualFilesMap[fileName]
-		assert.True(t, exists, "File %s exists in getBuiltInImageData map but not in images directory", fileName)
-	}
-
-	// Ensure we have at least some files (sanity check)
-	assert.NotEmpty(t, actualFilesMap, "Images directory should contain at least one file")
-	assert.Len(t, actualFilesMap, len(builtInImages), "Number of files in directory should match number in built-in data map")
-}

backend/internal/bootstrap/bootstrap.go

@@ -21,7 +21,7 @@ func Bootstrap(ctx context.Context) error {
 	}
 	slog.InfoContext(ctx, "Pocket ID is starting")
 
-	err = initApplicationImages()
+	imageExtensions, err := initApplicationImages()
 	if err != nil {
 		return fmt.Errorf("failed to initialize application images: %w", err)
 	}
@@ -33,7 +33,7 @@ func Bootstrap(ctx context.Context) error {
 	}
 
 	// Create all services
-	svc, err := initServices(ctx, db, httpClient)
+	svc, err := initServices(ctx, db, httpClient, imageExtensions)
 	if err != nil {
 		return fmt.Errorf("failed to initialize services: %w", err)
 	}

backend/internal/bootstrap/router_bootstrap.go

@@ -85,6 +85,7 @@ func initRouterInternal(db *gorm.DB, svc *services) (utils.Service, error) {
 	controller.NewOidcController(apiGroup, authMiddleware, fileSizeLimitMiddleware, svc.oidcService, svc.jwtService)
 	controller.NewUserController(apiGroup, authMiddleware, middleware.NewRateLimitMiddleware(), svc.userService, svc.appConfigService)
 	controller.NewAppConfigController(apiGroup, authMiddleware, svc.appConfigService, svc.emailService, svc.ldapService)
+	controller.NewAppImagesController(apiGroup, authMiddleware, svc.appImagesService)
 	controller.NewAuditLogController(apiGroup, svc.auditLogService, authMiddleware)
 	controller.NewUserGroupController(apiGroup, authMiddleware, svc.userGroupService)
 	controller.NewCustomClaimController(apiGroup, authMiddleware, svc.customClaimService)
@@ -181,9 +182,9 @@ func initRouterInternal(db *gorm.DB, svc *services) (utils.Service, error) {
 
 func initLogger(r *gin.Engine) {
 	loggerSkipPathsPrefix := []string{
-		"GET /api/application-configuration/logo",
-		"GET /api/application-configuration/background-image",
-		"GET /api/application-configuration/favicon",
+		"GET /api/application-images/logo",
+		"GET /api/application-images/background",
+		"GET /api/application-images/favicon",
 		"GET /_app",
 		"GET /fonts",
 		"GET /healthz",

backend/internal/bootstrap/services_bootstrap.go

@@ -12,6 +12,7 @@ import (
 
 type services struct {
 	appConfigService   *service.AppConfigService
+	appImagesService   *service.AppImagesService
 	emailService       *service.EmailService
 	geoLiteService     *service.GeoLiteService
 	auditLogService    *service.AuditLogService
@@ -27,7 +28,7 @@ type services struct {
 }
 
 // Initializes all services
-func initServices(ctx context.Context, db *gorm.DB, httpClient *http.Client) (svc *services, err error) {
+func initServices(ctx context.Context, db *gorm.DB, httpClient *http.Client, imageExtensions map[string]string) (svc *services, err error) {
 	svc = &services{}
 
 	svc.appConfigService, err = service.NewAppConfigService(ctx, db)
@@ -35,6 +36,8 @@ func initServices(ctx context.Context, db *gorm.DB, httpClient *http.Client) (sv
 		return nil, fmt.Errorf("failed to create app config service: %w", err)
 	}
 
+	svc.appImagesService = service.NewAppImagesService(imageExtensions)
+
 	svc.emailService, err = service.NewEmailService(db, svc.appConfigService)
 	if err != nil {
 		return nil, fmt.Errorf("failed to create email service: %w", err)