From 288203b7858f07eac9885d86fa77facf5d2ec2dc Mon Sep 17 00:00:00 2001 From: Christian Richter Date: Wed, 29 May 2024 11:59:43 +0200 Subject: [PATCH 1/5] Map HTTP 403 error correctly. Signed-off-by: Christian Richter Co-authored-by: Julian Koberg --- services/webdav/pkg/service/v0/service.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/services/webdav/pkg/service/v0/service.go b/services/webdav/pkg/service/v0/service.go index ad91475272..d0da62fce7 100644 --- a/services/webdav/pkg/service/v0/service.go +++ b/services/webdav/pkg/service/v0/service.go @@ -354,6 +354,8 @@ func (g Webdav) Thumbnail(w http.ResponseWriter, r *http.Request) { return case http.StatusBadRequest: renderError(w, r, errBadRequest(e.Detail)) + case http.StatusForbidden: + renderError(w, r, errPermissionDenied(e.Detail)) default: renderError(w, r, errInternalError(err.Error())) } @@ -531,6 +533,10 @@ func errBadRequest(msg string) *errResponse { return newErrResponse(http.StatusBadRequest, msg) } +func errPermissionDenied(msg string) *errResponse { + return newErrResponse(http.StatusForbidden, msg) +} + func errNotFound(msg string) *errResponse { return newErrResponse(http.StatusNotFound, msg) } From 1012a399aead7581e831bbdfe213f60fe90999e1 Mon Sep 17 00:00:00 2001 From: Christian Richter Date: Mon, 3 Jun 2024 12:07:22 +0200 Subject: [PATCH 2/5] Don't show thumbnails for secureview shares Co-authored-by: Julian Koberg Signed-off-by: Christian Richter --- ...revent-thumbnailer-from-showing-secureview-previews.md | 6 ++++++ services/thumbnails/pkg/service/grpc/v0/service.go | 8 ++++++++ services/webdav/pkg/service/v0/service.go | 2 ++ 3 files changed, 16 insertions(+) create mode 100644 changelog/unreleased/prevent-thumbnailer-from-showing-secureview-previews.md diff --git a/changelog/unreleased/prevent-thumbnailer-from-showing-secureview-previews.md b/changelog/unreleased/prevent-thumbnailer-from-showing-secureview-previews.md new file mode 100644 index 0000000000..686bd6ea90 --- /dev/null +++ b/changelog/unreleased/prevent-thumbnailer-from-showing-secureview-previews.md @@ -0,0 +1,6 @@ +Bugfix: Don't show thumbnails for secureview shares + +We have fixed a bug where thumbnails were shown for secureview shares. + +https://github.com/owncloud/ocis/pull/9299 +https://github.com/owncloud/ocis/issues/9249 diff --git a/services/thumbnails/pkg/service/grpc/v0/service.go b/services/thumbnails/pkg/service/grpc/v0/service.go index 5982b1ca40..4de5e5adb1 100644 --- a/services/thumbnails/pkg/service/grpc/v0/service.go +++ b/services/thumbnails/pkg/service/grpc/v0/service.go @@ -123,6 +123,10 @@ func (g Thumbnail) handleCS3Source(ctx context.Context, req *thumbnailssvc.GetTh return "", err } + if !sRes.GetInfo().GetPermissionSet().GetInitiateFileDownload() { + return "", merrors.Forbidden(g.serviceID, "no download permission") + } + tType := thumbnail.GetExtForMime(sRes.GetInfo().GetMimeType()) if tType == "" { tType = req.GetThumbnailType().String() @@ -206,6 +210,10 @@ func (g Thumbnail) handleWebdavSource(ctx context.Context, req *thumbnailssvc.Ge return "", err } + if !sRes.GetInfo().GetPermissionSet().GetInitiateFileDownload() { + return "", merrors.Forbidden(g.serviceID, "no download permission") + } + tType := thumbnail.GetExtForMime(sRes.GetInfo().GetMimeType()) if tType == "" { tType = req.GetThumbnailType().String() diff --git a/services/webdav/pkg/service/v0/service.go b/services/webdav/pkg/service/v0/service.go index d0da62fce7..97d3656a3f 100644 --- a/services/webdav/pkg/service/v0/service.go +++ b/services/webdav/pkg/service/v0/service.go @@ -261,6 +261,8 @@ func (g Webdav) SpacesThumbnail(w http.ResponseWriter, r *http.Request) { return case http.StatusBadRequest: renderError(w, r, errBadRequest(e.Detail)) + case http.StatusForbidden: + renderError(w, r, errPermissionDenied(e.Detail)) default: renderError(w, r, errInternalError(err.Error())) } From 4d6cbdf260637448afb18b24e278310196d763ed Mon Sep 17 00:00:00 2001 From: Christian Richter Date: Mon, 3 Jun 2024 14:44:33 +0200 Subject: [PATCH 3/5] Move chi HTTP-Method registering into the NewService func Signed-off-by: Christian Richter --- services/webdav/pkg/service/v0/service.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/services/webdav/pkg/service/v0/service.go b/services/webdav/pkg/service/v0/service.go index 97d3656a3f..3e61be951d 100644 --- a/services/webdav/pkg/service/v0/service.go +++ b/services/webdav/pkg/service/v0/service.go @@ -33,11 +33,6 @@ import ( "github.com/owncloud/ocis/v2/services/webdav/pkg/dav/requests" ) -func init() { - // register method with chi before any routing is set up - chi.RegisterMethod("REPORT") -} - var ( codesEnum = map[int]string{ http.StatusBadRequest: "Sabre\\DAV\\Exception\\BadRequest", @@ -94,6 +89,10 @@ func NewService(opts ...Option) (Service, error) { if svc.config.DisablePreviews { svc.thumbnailsClient = nil } + + // register method with chi before any routing is set up + chi.RegisterMethod("REPORT") + m.Route(options.Config.HTTP.Root, func(r chi.Router) { if !svc.config.DisablePreviews { From db308bea5c6207ad3791461e293aee84a463cd66 Mon Sep 17 00:00:00 2001 From: Christian Richter Date: Tue, 4 Jun 2024 11:24:55 +0200 Subject: [PATCH 4/5] try to remove code duplications Signed-off-by: Christian Richter --- .../thumbnails/pkg/service/grpc/v0/service.go | 50 +++++++++---------- 1 file changed, 23 insertions(+), 27 deletions(-) diff --git a/services/thumbnails/pkg/service/grpc/v0/service.go b/services/thumbnails/pkg/service/grpc/v0/service.go index 4de5e5adb1..3dc897838a 100644 --- a/services/thumbnails/pkg/service/grpc/v0/service.go +++ b/services/thumbnails/pkg/service/grpc/v0/service.go @@ -116,15 +116,10 @@ func (g Thumbnail) GetThumbnail(ctx context.Context, req *thumbnailssvc.GetThumb return nil } -func (g Thumbnail) handleCS3Source(ctx context.Context, req *thumbnailssvc.GetThumbnailRequest) (string, error) { - src := req.GetCs3Source() - sRes, err := g.stat(src.GetPath(), src.GetAuthorization()) - if err != nil { - return "", err - } - +func (g Thumbnail) checkThumbnail(req *thumbnailssvc.GetThumbnailRequest, sRes *provider.StatResponse) (string, thumbnail.Request, error) { + tr := thumbnail.Request{} if !sRes.GetInfo().GetPermissionSet().GetInitiateFileDownload() { - return "", merrors.Forbidden(g.serviceID, "no download permission") + return "", tr, merrors.Forbidden(g.serviceID, "no download permission") } tType := thumbnail.GetExtForMime(sRes.GetInfo().GetMimeType()) @@ -133,11 +128,25 @@ func (g Thumbnail) handleCS3Source(ctx context.Context, req *thumbnailssvc.GetTh } tr, err := thumbnail.PrepareRequest(int(req.GetWidth()), int(req.GetHeight()), tType, sRes.GetInfo().GetChecksum().GetSum(), req.GetProcessor()) if err != nil { - return "", merrors.BadRequest(g.serviceID, err.Error()) + return "", tr, merrors.BadRequest(g.serviceID, err.Error()) } if key, exists := g.manager.CheckThumbnail(tr); exists { - return key, nil + return key, tr, nil + } + return "", tr, nil +} + +func (g Thumbnail) handleCS3Source(ctx context.Context, req *thumbnailssvc.GetThumbnailRequest) (string, error) { + src := req.GetCs3Source() + sRes, err := g.stat(src.GetPath(), src.GetAuthorization()) + if err != nil { + return "", err + } + + key, tr, err := g.checkThumbnail(req, sRes) + if err != nil { + return "", err } ctx = imgsource.ContextSetAuthorization(ctx, src.GetAuthorization()) @@ -155,7 +164,7 @@ func (g Thumbnail) handleCS3Source(ctx context.Context, req *thumbnailssvc.GetTh return "", merrors.InternalServerError(g.serviceID, "could not get image") } - key, err := g.manager.Generate(tr, img) + key, err = g.manager.Generate(tr, img) if err != nil { return "", err } @@ -210,23 +219,10 @@ func (g Thumbnail) handleWebdavSource(ctx context.Context, req *thumbnailssvc.Ge return "", err } - if !sRes.GetInfo().GetPermissionSet().GetInitiateFileDownload() { - return "", merrors.Forbidden(g.serviceID, "no download permission") - } - - tType := thumbnail.GetExtForMime(sRes.GetInfo().GetMimeType()) - if tType == "" { - tType = req.GetThumbnailType().String() - } - tr, err := thumbnail.PrepareRequest(int(req.GetWidth()), int(req.GetHeight()), tType, sRes.GetInfo().GetChecksum().GetSum(), req.GetProcessor()) + key, tr, err := g.checkThumbnail(req, sRes) if err != nil { - return "", merrors.BadRequest(g.serviceID, err.Error()) + return "", err } - - if key, exists := g.manager.CheckThumbnail(tr); exists { - return key, nil - } - if src.GetWebdavAuthorization() != "" { ctx = imgsource.ContextSetAuthorization(ctx, src.GetWebdavAuthorization()) } @@ -252,7 +248,7 @@ func (g Thumbnail) handleWebdavSource(ctx context.Context, req *thumbnailssvc.Ge return "", merrors.InternalServerError(g.serviceID, "could not get image") } - key, err := g.manager.Generate(tr, img) + key, err = g.manager.Generate(tr, img) if err != nil { return "", err } From a145c36fd5c5322c41011ef4d322d988f90200b8 Mon Sep 17 00:00:00 2001 From: Christian Richter Date: Tue, 4 Jun 2024 12:10:01 +0200 Subject: [PATCH 5/5] remove obsolete var Signed-off-by: Christian Richter --- .../thumbnails/pkg/service/grpc/v0/service.go | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/services/thumbnails/pkg/service/grpc/v0/service.go b/services/thumbnails/pkg/service/grpc/v0/service.go index 3dc897838a..7049064b70 100644 --- a/services/thumbnails/pkg/service/grpc/v0/service.go +++ b/services/thumbnails/pkg/service/grpc/v0/service.go @@ -116,10 +116,10 @@ func (g Thumbnail) GetThumbnail(ctx context.Context, req *thumbnailssvc.GetThumb return nil } -func (g Thumbnail) checkThumbnail(req *thumbnailssvc.GetThumbnailRequest, sRes *provider.StatResponse) (string, thumbnail.Request, error) { +func (g Thumbnail) checkThumbnail(req *thumbnailssvc.GetThumbnailRequest, sRes *provider.StatResponse) (thumbnail.Request, error) { tr := thumbnail.Request{} if !sRes.GetInfo().GetPermissionSet().GetInitiateFileDownload() { - return "", tr, merrors.Forbidden(g.serviceID, "no download permission") + return tr, merrors.Forbidden(g.serviceID, "no download permission") } tType := thumbnail.GetExtForMime(sRes.GetInfo().GetMimeType()) @@ -128,13 +128,13 @@ func (g Thumbnail) checkThumbnail(req *thumbnailssvc.GetThumbnailRequest, sRes * } tr, err := thumbnail.PrepareRequest(int(req.GetWidth()), int(req.GetHeight()), tType, sRes.GetInfo().GetChecksum().GetSum(), req.GetProcessor()) if err != nil { - return "", tr, merrors.BadRequest(g.serviceID, err.Error()) + return tr, merrors.BadRequest(g.serviceID, err.Error()) } - if key, exists := g.manager.CheckThumbnail(tr); exists { - return key, tr, nil + if _, exists := g.manager.CheckThumbnail(tr); exists { + return tr, nil } - return "", tr, nil + return tr, nil } func (g Thumbnail) handleCS3Source(ctx context.Context, req *thumbnailssvc.GetThumbnailRequest) (string, error) { @@ -144,7 +144,7 @@ func (g Thumbnail) handleCS3Source(ctx context.Context, req *thumbnailssvc.GetTh return "", err } - key, tr, err := g.checkThumbnail(req, sRes) + tr, err := g.checkThumbnail(req, sRes) if err != nil { return "", err } @@ -164,7 +164,7 @@ func (g Thumbnail) handleCS3Source(ctx context.Context, req *thumbnailssvc.GetTh return "", merrors.InternalServerError(g.serviceID, "could not get image") } - key, err = g.manager.Generate(tr, img) + key, err := g.manager.Generate(tr, img) if err != nil { return "", err } @@ -219,7 +219,7 @@ func (g Thumbnail) handleWebdavSource(ctx context.Context, req *thumbnailssvc.Ge return "", err } - key, tr, err := g.checkThumbnail(req, sRes) + tr, err := g.checkThumbnail(req, sRes) if err != nil { return "", err } @@ -248,7 +248,7 @@ func (g Thumbnail) handleWebdavSource(ctx context.Context, req *thumbnailssvc.Ge return "", merrors.InternalServerError(g.serviceID, "could not get image") } - key, err = g.manager.Generate(tr, img) + key, err := g.manager.Generate(tr, img) if err != nil { return "", err }