Files: Serve inline files by type, add SVG, open shared access #5488 #5655

internal/api/files.go

@@ -57,13 +57,14 @@ func GetFile(router *gin.RouterGroup) {
 }
 
 // GetFileBytes streams the original bytes of a file for inline viewing, selecting the
-// served type from the request name's extension (e.g. "file.pdf"). The auth and
+// served type from the request name's extension (e.g. "file.pdf" or "file.svg"). The
-// visibility preamble matches GetFile; an unknown or unsupported type returns 415.
+// auth and visibility preamble matches GetFile; an unknown or unsupported type returns 415.
 //
-//	@Summary	streams the original file bytes for inline viewing (e.g. ".pdf")
+//	@Summary	streams the original file bytes for inline viewing (e.g. ".pdf" or ".svg")
 //	@Id			GetFileBytes
 //	@Tags		Files
 //	@Produce	application/pdf
+//	@Produce	image/svg+xml
 //	@Success	200					{file}		binary
 //	@Failure	401,403,404,415,429	{object}	i18n.Response
 //	@Param		hash				path		string	true	"SHA-1 hash of the file"
@@ -94,38 +95,88 @@ func GetFileBytes(router *gin.RouterGroup) {
 			return
 		}
 
-		// Serve the bytes based on the type selected by the request name's extension.
+		// Select the served type from the request name's extension; an unknown or
-		// Future variants (e.g. ".jpg"/".mp4") add their own case here.
+		// unsupported extension has no descriptor and returns 415.
-		switch fs.FileType(c.Param("name")) {
+		spec, ok := servableTypes[fs.FileType(c.Param("name"))]
-		case fs.DocumentPDF:
+		if !ok {
-			getFilePDF(c, f)
-		default:
-			Abort(c, http.StatusUnsupportedMediaType, i18n.ErrUnsupportedType)
-		}
-	})
-}
-
-// getFilePDF streams the original PDF bytes of a document for inline viewing,
-// reusing the session-scoped authorization already enforced by the caller.
-func getFilePDF(c *gin.Context, f *entity.File) {
-	// A document's primary file is its rendered cover image, so when the request
-	// carries the cover hash, resolve the related PDF for the same photo. This is
-	// deliberate and specific to documents: the original IS the sidecar. A future
-	// ".jpg"/".mp4" variant should serve its own type directly, not cross-resolve.
-	if f.Type() != fs.DocumentPDF {
-		if doc, err := query.DocumentByPhotoUID(f.PhotoUID); err == nil && doc.Type() == fs.DocumentPDF {
-			f = doc
-		} else {
 			Abort(c, http.StatusUnsupportedMediaType, i18n.ErrUnsupportedType)
 			return
 		}
+
+		// Resolve the file actually served. It must belong to the SAME photo that passed the
+		// visibility check above, so a resolver can never hand back a file past the scope gate.
+		target, ok := spec.Resolve(f)
+		if !ok || target.PhotoUID != f.PhotoUID {
+			AbortEntityNotFound(c)
+			return
+		}
+
+		serveInlineFile(c, spec, target)
+	})
+}
+
+// ServableType describes how GetFileBytes streams a file type inline. Fields are
+// exported so additional served types can be registered from the same package.
+type ServableType struct {
+	ContentType string               // response Content-Type
+	Disposition string               // response Content-Disposition; defaults to "inline"
+	SetHeaders  func(c *gin.Context) // optional per-type response headers (e.g. SVG XSS neutralization)
+	// Resolve maps the request file to the file actually served. It must return a
+	// file of the SAME photo that passed the visibility check, which GetFileBytes enforces.
+	Resolve func(f *entity.File) (*entity.File, bool)
+}
+
+// servableTypes lists the file types GetFileBytes streams inline, keyed by fs.Type.
+// Adding a type is a map entry, not a new handler branch.
+var servableTypes = map[fs.Type]ServableType{
+	fs.DocumentPDF: {ContentType: header.ContentTypePDF, Resolve: resolvePDFDocument},
+	fs.VectorSVG:   {ContentType: header.ContentTypeSVG, SetHeaders: svgSafeHeaders, Resolve: serveSelf(fs.VectorSVG)},
+}
+
+// serveSelf returns a resolver that serves the request file itself when its type
+// matches want, used by types whose original is its own primary file.
+func serveSelf(want fs.Type) func(*entity.File) (*entity.File, bool) {
+	return func(f *entity.File) (*entity.File, bool) {
+		if f.Type() == want {
+			return f, true
+		}
+		return nil, false
+	}
+}
+
+// resolvePDFDocument maps a document request to its PDF original. A document's
+// primary file is its rendered cover image, so when the request carries the cover
+// hash the related PDF for the same photo is resolved. This cross-resolution is
+// deliberate and specific to documents: the original IS the sidecar.
+func resolvePDFDocument(f *entity.File) (*entity.File, bool) {
+	if f.Type() == fs.DocumentPDF {
+		return f, true
 	}
 
+	if doc, err := query.DocumentByPhotoUID(f.PhotoUID); err == nil && doc.Type() == fs.DocumentPDF {
+		return doc, true
+	}
+
+	return nil, false
+}
+
+// svgSafeHeaders neutralizes the XSS surface of an inline SVG served same-origin:
+// a restrictive CSP sandboxes the document and blocks script execution, and nosniff
+// stops content-type sniffing. PDF needs none of this, hence the per-type hook.
+func svgSafeHeaders(c *gin.Context) {
+	c.Header(header.ContentSecurityPolicy, "default-src 'none'; style-src 'unsafe-inline'; sandbox")
+	c.Header(header.ContentTypeOptions, header.PolicyNoSniff)
+}
+
+// serveInlineFile streams the original bytes of f inline per the type descriptor,
+// with HTTP Range support via c.File. A missing original is flagged so it drops
+// out of search results and reported as not found.
+func serveInlineFile(c *gin.Context, spec ServableType, f *entity.File) {
 	// Resolve the absolute filename and verify the original still exists.
 	fileName := photoprism.FileName(f.FileRoot, f.FileName)
 
 	if !fs.FileExists(fileName) {
-		log.Errorf("files: pdf %s is missing", clean.Log(f.FileName))
+		log.Errorf("files: %s %s is missing", f.Type().String(), clean.Log(f.FileName))
 
 		// Flag as missing so it no longer shows up in search results.
 		logErr("files", f.Update("FileMissing", true))
@@ -135,11 +186,21 @@ func getFilePDF(c *gin.Context, f *entity.File) {
 	}
 
 	// The response is session-scoped, so cache it privately and never on shared CDNs.
-	AddContentTypeHeader(c, header.ContentTypePDF)
+	AddContentTypeHeader(c, spec.ContentType)
 	header.SetCacheControlImmutable(c, ttl.CacheDefault.Int(), false)
 
-	// Serve the document inline with HTTP Range support via c.File (http.ServeContent),
+	// Apply any per-type response headers (e.g. SVG XSS neutralization).
+	if spec.SetHeaders != nil {
+		spec.SetHeaders(c)
+	}
+
+	// Serve the file inline with HTTP Range support via c.File (http.ServeContent),
 	// which keeps the Content-Type and Content-Disposition set here.
-	c.Header(header.ContentDisposition, "inline")
+	disposition := spec.Disposition
+	if disposition == "" {
+		disposition = "inline"
+	}
+
+	c.Header(header.ContentDisposition, disposition)
 	c.File(fileName)
 }

internal/api/files_test.go

@@ -13,6 +13,7 @@ import (
 
 	"github.com/photoprism/photoprism/internal/config"
 	"github.com/photoprism/photoprism/internal/entity"
+	"github.com/photoprism/photoprism/internal/entity/query"
 	"github.com/photoprism/photoprism/internal/photoprism"
 	"github.com/photoprism/photoprism/pkg/fs"
 	"github.com/photoprism/photoprism/pkg/http/header"
@@ -29,6 +30,53 @@ const jpegFixtureHash = "2cad9168fa6acc5c5c2965ddf6ec465ca42fd818"
 // used to verify the endpoint resolves the related PDF from the cover hash.
 const pdfCoverHash = "pcad9168fa6acc5c5c2965adf6ec465ca42fd9911"
 
+// svgFixtureHash is the file hash of the SVG original inserted by writeSvgFixture.
+const svgFixtureHash = "scad9168fa6acc5c5c2965adf6ec465ca42fd9913"
+
+// privateFileHash is a file of a private photo ("Photo06.png"); guest and visitor
+// sessions are never in scope for it, so the visibility gate reports not found.
+const privateFileHash = "pcad9a68fa6acc5c5ba965adf6ec465ca42fd917"
+
+// writeSvgFixture inserts an SVG original attached to the non-private document
+// photo and writes its physical file, returning a cleanup function. The test
+// storage ships no SVG fixture row or original.
+func writeSvgFixture(t *testing.T) func() {
+	const svgName = "education/university/diagram.svg"
+
+	photo := entity.PhotoFixtures.Pointer("Photo55")
+
+	file := &entity.File{
+		PhotoID:     photo.ID,
+		PhotoUID:    photo.PhotoUID,
+		FileUID:     "fs6sg6bw15bnl3sv",
+		FileName:    svgName,
+		FileRoot:    entity.RootOriginals,
+		FileHash:    svgFixtureHash,
+		FileType:    fs.VectorSVG.String(),
+		FileMime:    header.ContentTypeSVG,
+		FilePrimary: false,
+	}
+
+	if err := entity.Db().Create(file).Error; err != nil {
+		t.Fatal(err)
+	}
+
+	fileName := photoprism.FileName(entity.RootOriginals, svgName)
+
+	if err := os.MkdirAll(filepath.Dir(fileName), fs.ModeDir); err != nil {
+		t.Fatal(err)
+	}
+
+	if err := os.WriteFile(fileName, []byte(`<svg xmlns="http://www.w3.org/2000/svg" width="10" height="10"></svg>`), fs.ModeFile); err != nil {
+		t.Fatal(err)
+	}
+
+	return func() {
+		_ = os.Remove(fileName)
+		entity.Db().Unscoped().Where("file_hash = ?", svgFixtureHash).Delete(&entity.File{})
+	}
+}
+
 // writePdfFixture creates the physical original for the photo55.pdf fixture and
 // returns a cleanup function, since the test storage ships no PDF original.
 func writePdfFixture(t *testing.T) func() {
@@ -65,20 +113,19 @@ func TestGetFile(t *testing.T) {
 		r := PerformRequest(app, "GET", "/api/v1/files/111")
 		assert.Equal(t, http.StatusNotFound, r.Code)
 	})
-	t.Run("SharedOnlySessionDenied", func(t *testing.T) {
+	t.Run("GuestOutOfScopeNotFound", func(t *testing.T) {
 		app, router, conf := NewApiTest()
 		conf.SetAuthMode(config.AuthModePasswd)
 		defer conf.SetAuthMode(config.AuthModePublic)
 
 		GetFile(router)
 
-		// A shared-only (guest) session has no files access, so the endpoint denies the request
+		// A guest session now holds a view-only files grant, so the request passes the ACL and is
-		// at the ACL check before the visibility gate. Per-photo file scope (the gate that returns
+		// scoped by search.FileVisibleToSession instead: a file of a private photo is out of scope
-		// 404 for an out-of-scope file when the role does have files access, e.g. a viewer in
+		// for a guest and reported as not found rather than forbidden.
-		// Plus/Pro) is covered by search.TestFileVisibleToSession.
 		authToken := AuthenticateUser(app, router, "gandalf", "Gandalf123!")
-		r := AuthenticatedRequest(app, "GET", "/api/v1/files/2cad9168fa6acc5c5c2965ddf6ec465ca42fd818", authToken)
+		r := AuthenticatedRequest(app, "GET", "/api/v1/files/"+privateFileHash, authToken)
-		assert.Equal(t, http.StatusForbidden, r.Code)
+		assert.Equal(t, http.StatusNotFound, r.Code)
 	})
 }
 
@@ -114,11 +161,35 @@ func TestGetFileBytes(t *testing.T) {
 		assert.Equal(t, http.StatusPartialContent, w.Code)
 	})
 	t.Run("NotPDF", func(t *testing.T) {
-		// A ".pdf" request for a file whose photo has no related PDF is unsupported.
+		// A ".pdf" request for a file whose photo has no related PDF cannot be resolved, so the
+		// endpoint reports not found (the type is supported, the representation just does not exist).
 		app, router, _ := NewApiTest()
 		GetFileBytes(router)
 		r := PerformRequest(app, "GET", "/api/v1/files/"+jpegFixtureHash+"/file.pdf")
-		assert.Equal(t, http.StatusUnsupportedMediaType, r.Code)
+		assert.Equal(t, http.StatusNotFound, r.Code)
+	})
+	t.Run("SVGSuccess", func(t *testing.T) {
+		cleanup := writeSvgFixture(t)
+		defer cleanup()
+		app, router, _ := NewApiTest()
+		GetFileBytes(router)
+		r := PerformRequest(app, "GET", "/api/v1/files/"+svgFixtureHash+"/file.svg")
+		assert.Equal(t, http.StatusOK, r.Code)
+		assert.Equal(t, header.ContentTypeSVG, r.Header().Get("Content-Type"))
+		assert.Equal(t, "inline", r.Header().Get(header.ContentDisposition))
+	})
+	t.Run("SVGSafeHeaders", func(t *testing.T) {
+		// An inline SVG is served same-origin, so it must carry a sandboxing CSP and nosniff to
+		// neutralize embedded scripts. These are set by the handler because the API test router
+		// does not wire the server security middleware.
+		cleanup := writeSvgFixture(t)
+		defer cleanup()
+		app, router, _ := NewApiTest()
+		GetFileBytes(router)
+		r := PerformRequest(app, "GET", "/api/v1/files/"+svgFixtureHash+"/file.svg")
+		assert.Equal(t, http.StatusOK, r.Code)
+		assert.Equal(t, header.PolicyNoSniff, r.Header().Get(header.ContentTypeOptions))
+		assert.Equal(t, "default-src 'none'; style-src 'unsafe-inline'; sandbox", r.Header().Get(header.ContentSecurityPolicy))
 	})
 	t.Run("UnsupportedType", func(t *testing.T) {
 		// An unknown extension maps to no served type, so the dispatch returns 415.
@@ -133,11 +204,10 @@ func TestGetFileBytes(t *testing.T) {
 		r := PerformRequest(app, "GET", "/api/v1/files/123xxx/file.pdf")
 		assert.Equal(t, http.StatusNotFound, r.Code)
 	})
-	t.Run("SharedOnlySessionDenied", func(t *testing.T) {
+	t.Run("GuestOutOfScopeNotFound", func(t *testing.T) {
-		// The bytes route sits behind the same Auth(ResourceFiles, ActionView) gate as the
+		// A guest holds a view-only files grant, so the bytes route passes the ACL and is scoped
-		// JSON route, so a shared-only (guest) session is denied. Per-photo visibility scoping
+		// by search.FileVisibleToSession. The document photo is neither shared with nor published
-		// (404 for an out-of-scope file when the role has files access) is shared with the JSON
+		// to this guest, so it is reported as not found rather than forbidden.
-		// route and covered by search.TestFileVisibleToSession.
 		cleanup := writePdfFixture(t)
 		defer cleanup()
 		app, router, conf := NewApiTest()
@@ -146,6 +216,71 @@ func TestGetFileBytes(t *testing.T) {
 		GetFileBytes(router)
 		authToken := AuthenticateUser(app, router, "gandalf", "Gandalf123!")
 		r := AuthenticatedRequest(app, "GET", "/api/v1/files/"+pdfFixtureHash+"/file.pdf", authToken)
-		assert.Equal(t, http.StatusForbidden, r.Code)
+		assert.Equal(t, http.StatusNotFound, r.Code)
+	})
+	t.Run("SharedAlbumVisitorSuccess", func(t *testing.T) {
+		// A share-link visitor whose shared album contains the document can fetch the PDF: the
+		// backend grant plus the per-photo scope let the viewer work in shared albums.
+		cleanup := writePdfFixture(t)
+		defer cleanup()
+		app, router, conf := NewApiTest()
+		conf.SetAuthMode(config.AuthModePasswd)
+		defer conf.SetAuthMode(config.AuthModePublic)
+		GetFileBytes(router)
+
+		// Attach the PDF document photo to the album shared by the "visitor" session fixture.
+		const docPhotoUID = "ps6sg6byk7wrbk48"    // Photo55 (the PDF document)
+		const sharedAlbumUID = "as6sg6bxpogaaba8" // album the "visitor" session fixture shares
+		if err := entity.Db().Exec("INSERT INTO photos_albums (photo_uid, album_uid, hidden, missing) VALUES (?, ?, 0, 0)", docPhotoUID, sharedAlbumUID).Error; err != nil {
+			t.Fatal(err)
+		}
+		defer entity.Db().Exec("DELETE FROM photos_albums WHERE photo_uid = ? AND album_uid = ?", docPhotoUID, sharedAlbumUID)
+
+		// The "visitor" session fixture holds a share token scoped to sharedAlbumUID.
+		r := AuthenticatedRequest(app, "GET", "/api/v1/files/"+pdfFixtureHash+"/file.pdf", "69be27ac5ca305b394046a83f6fda18167ca3d3f2dbe7ac3")
+		assert.Equal(t, http.StatusOK, r.Code)
+		assert.Equal(t, header.ContentTypePDF, r.Header().Get("Content-Type"))
+	})
+}
+
+func TestServeSelf(t *testing.T) {
+	resolve := serveSelf(fs.VectorSVG)
+	t.Run("Match", func(t *testing.T) {
+		f := &entity.File{FileType: fs.VectorSVG.String(), PhotoUID: "ps6sg6byk7wrbk48"}
+		got, ok := resolve(f)
+		assert.True(t, ok)
+		assert.Same(t, f, got)
+	})
+	t.Run("Mismatch", func(t *testing.T) {
+		f := &entity.File{FileType: fs.ImageJpeg.String()}
+		got, ok := resolve(f)
+		assert.False(t, ok)
+		assert.Nil(t, got)
+	})
+}
+
+func TestResolvePDFDocument(t *testing.T) {
+	t.Run("FromPDF", func(t *testing.T) {
+		f := &entity.File{FileType: fs.DocumentPDF.String(), PhotoUID: "ps6sg6byk7wrbk48"}
+		got, ok := resolvePDFDocument(f)
+		assert.True(t, ok)
+		assert.Same(t, f, got)
+	})
+	t.Run("FromCover", func(t *testing.T) {
+		// The cover image of the document resolves to the related PDF of the same photo.
+		cover, err := query.FileByHash(pdfCoverHash)
+		assert.NoError(t, err)
+		got, ok := resolvePDFDocument(cover)
+		assert.True(t, ok)
+		assert.Equal(t, fs.DocumentPDF, got.Type())
+		assert.Equal(t, cover.PhotoUID, got.PhotoUID)
+	})
+	t.Run("Unrelated", func(t *testing.T) {
+		// A jpeg whose photo has no related PDF cannot resolve to a document.
+		jpeg, err := query.FileByHash(jpegFixtureHash)
+		assert.NoError(t, err)
+		got, ok := resolvePDFDocument(jpeg)
+		assert.False(t, ok)
+		assert.Nil(t, got)
 	})
 }

internal/api/swagger.json

@@ -8148,7 +8148,8 @@
                     }
                 ],
                 "produces": [
-                    "application/pdf"
+                    "application/pdf",
+                    "image/svg+xml"
                 ],
                 "responses": {
                     "200": {
@@ -8188,7 +8189,7 @@
                         }
                     }
                 },
-                "summary": "streams the original file bytes for inline viewing (e.g. \".pdf\")",
+                "summary": "streams the original file bytes for inline viewing (e.g. \".pdf\" or \".svg\")",
                 "tags": [
                     "Files"
                 ]

internal/auth/acl/acl_test.go

@@ -123,7 +123,23 @@ func TestACL_Deny(t *testing.T) {
 
 func TestACL_DenyAll(t *testing.T) {
 	t.Run("ResourceFilesRoleVisitorActionDefault", func(t *testing.T) {
-		assert.True(t, Rules.DenyAll(ResourceFiles, RoleVisitor, Permissions{FullAccess, AccessShared, ActionView}))
+		// A visitor now holds a view-only grant on files (shared view + download) so that
+		// share-link sessions can open in-scope documents; per-photo scope is still enforced
+		// by search.FileVisibleToSession in the handlers.
+		assert.True(t, Rules.Allow(ResourceFiles, RoleVisitor, ActionView))
+		assert.True(t, Rules.Allow(ResourceFiles, RoleVisitor, AccessShared))
+		assert.True(t, Rules.Allow(ResourceFiles, RoleVisitor, ActionDownload))
+		// The grant must not reach library-wide, management, upload, or all-access branches.
+		assert.True(t, Rules.Deny(ResourceFiles, RoleVisitor, FullAccess))
+		assert.True(t, Rules.Deny(ResourceFiles, RoleVisitor, AccessAll))
+		assert.True(t, Rules.Deny(ResourceFiles, RoleVisitor, AccessLibrary))
+		assert.True(t, Rules.Deny(ResourceFiles, RoleVisitor, ActionManage))
+		assert.True(t, Rules.Deny(ResourceFiles, RoleVisitor, ActionUpload))
+	})
+	t.Run("ResourceFilesRoleGuestActionDefault", func(t *testing.T) {
+		assert.True(t, Rules.Allow(ResourceFiles, RoleGuest, ActionView))
+		assert.True(t, Rules.Deny(ResourceFiles, RoleGuest, AccessAll))
+		assert.True(t, Rules.Deny(ResourceFiles, RoleGuest, ActionManage))
 	})
 	t.Run("ResourceFilesRoleAdminActionDefault", func(t *testing.T) {
 		assert.False(t, Rules.DenyAll(ResourceFiles, RoleAdmin, Permissions{FullAccess, AccessShared, ActionView}))

internal/auth/acl/rules.go

@@ -8,8 +8,10 @@ var RulesMutex = &sync.Mutex{}
 // Rules specifies granted permissions by Resource and Role.
 var Rules = ACL{
 	ResourceFiles: Roles{
-		RoleAdmin:  GrantFullAccess,
+		RoleAdmin:   GrantFullAccess,
-		RoleClient: GrantFullAccess,
+		RoleGuest:   GrantViewShared,
+		RoleVisitor: GrantViewShared,
+		RoleClient:  GrantFullAccess,
 	},
 	ResourceFolders: Roles{
 		RoleAdmin:   GrantFullAccess,