From 864e20028f15072c73ce35dbeb94767bceca1fea Mon Sep 17 00:00:00 2001 From: Pedro Pinto Silva Date: Wed, 1 Apr 2026 16:05:36 +0200 Subject: [PATCH 1/4] feat(collaboration): add UserExtraInfo with avatar and mail to CheckFileInfo Add UserExtraInfo (avatar + mail) to the WOPI CheckFileInfo response for authenticated, non-public-share users. UserExtraInfo format (per Collabora SDK): https://sdk.collaboraonline.com/docs/advanced_integration.html#userextrainfo ```json { "avatar": "http://url/to/user/avatar", "mail": "user@server.com" } ``` After this change, CheckFileInfo returns: ```json { "BaseFileName": "Pedro-filled-hazcom.docx", "UserFriendlyName": "Admin", "UserId": "346364...39323030", "UserCanWrite": true, "UserCanRename": true, "IsAdminUser": true, "EnableInsertRemoteImage": true, "EnableInsertRemoteFile": true, "EnableOwnerTermination": true, "UserExtraInfo": { "avatar": "https://host:9300/wopi/avatars/{userID}?access_token={wopiToken}", "mail": "admin@example.org" }, "PostMessageOrigin": "https://localhost:9200", "message": "CheckFileInfo: success" } ``` Avatars are served via a new /wopi/avatars/{userID} endpoint on the collaboration service, authenticated by the WOPI token. The endpoint calls the Graph service directly (bypassing the proxy) using the reva access token via x-access-token header. All tests pass: go test ./services/collaboration/... ./services/graph/... ./services/proxy/... Signed-off-by: Pedro Pinto Silva --- services/collaboration/pkg/config/cs3api.go | 7 +- .../pkg/config/defaults/defaultconfig.go | 1 + .../pkg/connector/fileconnector.go | 28 ++++++++ .../pkg/connector/fileconnector_test.go | 13 +++- .../pkg/connector/fileinfo/collabora.go | 14 +++- .../pkg/connector/fileinfo/fileinfo.go | 2 +- .../pkg/connector/httpadapter.go | 67 +++++++++++++++++++ .../pkg/middleware/wopicontext.go | 38 ++++++----- .../collaboration/pkg/server/http/server.go | 15 +++++ 9 files changed, 162 insertions(+), 23 deletions(-) diff --git a/services/collaboration/pkg/config/cs3api.go b/services/collaboration/pkg/config/cs3api.go index c9234ce3c1..8eb6ed5904 100644 --- a/services/collaboration/pkg/config/cs3api.go +++ b/services/collaboration/pkg/config/cs3api.go @@ -9,9 +9,10 @@ import ( // CS3Api defines the available configuration in order to access to the CS3 gateway. type CS3Api struct { Gateway Gateway `yaml:"gateway"` - DataGateway DataGateway `yaml:"datagateway"` - GRPCClientTLS *shared.GRPCClientTLS `yaml:"grpc_client_tls"` - APPRegistrationInterval time.Duration `yaml:"app_registration_interval" env:"COLLABORATION_CS3API_APP_REGISTRATION_INTERVAL" desc:"The interval at which the app provider registers itself." introductionVersion:"4.0.0"` + DataGateway DataGateway `yaml:"datagateway"` + GraphEndpoint string `yaml:"graph_endpoint" env:"COLLABORATION_CS3API_GRAPH_ENDPOINT" desc:"The internal HTTP endpoint of the Graph service, used to fetch user profile photos for avatars." introductionVersion:"4.0.0"` + GRPCClientTLS *shared.GRPCClientTLS `yaml:"grpc_client_tls"` + APPRegistrationInterval time.Duration `yaml:"app_registration_interval" env:"COLLABORATION_CS3API_APP_REGISTRATION_INTERVAL" desc:"The interval at which the app provider registers itself." introductionVersion:"4.0.0"` } // Gateway defines the available configuration for the CS3 API gateway diff --git a/services/collaboration/pkg/config/defaults/defaultconfig.go b/services/collaboration/pkg/config/defaults/defaultconfig.go index 20aaf45b86..cdb30a44c1 100644 --- a/services/collaboration/pkg/config/defaults/defaultconfig.go +++ b/services/collaboration/pkg/config/defaults/defaultconfig.go @@ -65,6 +65,7 @@ func DefaultConfig() *config.Config { DataGateway: config.DataGateway{ Insecure: false, }, + GraphEndpoint: "http://127.0.0.1:9120/graph", APPRegistrationInterval: 30 * time.Second, }, } diff --git a/services/collaboration/pkg/connector/fileconnector.go b/services/collaboration/pkg/connector/fileconnector.go index 20f516f369..bede7b8772 100644 --- a/services/collaboration/pkg/connector/fileconnector.go +++ b/services/collaboration/pkg/connector/fileconnector.go @@ -1302,6 +1302,18 @@ func (f *FileConnector) CheckFileInfo(ctx context.Context) (*ConnectorResponse, } } + if !isPublicShare && !isAnonymousUser { + extraInfo := &fileinfo.UserExtraInfo{ + Mail: user.GetMail(), + } + // Build a WOPI-proxied avatar URL so Collabora can load it via img.src + // without needing auth headers (the token is in the query string). + if avatarURL, err := f.createAvatarURL(wopiContext, collaborationURL, user.GetId().GetOpaqueId()); err == nil { + extraInfo.Avatar = avatarURL + } + infoMap[fileinfo.KeyUserExtraInfo] = extraInfo + } + // if the file content is empty and a template reference is set, add the template source URL if wopiContext.TemplateReference != nil && statRes.GetInfo().GetSize() == 0 { if tu, err := f.createDownloadURL(wopiContext, collaborationURL); err == nil { @@ -1339,6 +1351,22 @@ func (f *FileConnector) createDownloadURL(wopiContext middleware.WopiContext, co return downloadURL.String(), nil } +// createAvatarURL builds a WOPI-proxied avatar URL for the given user. +// The collaboration service's /wopi/avatars/ endpoint will fetch the avatar +// from the Graph API using the WOPI token for authentication. +func (f *FileConnector) createAvatarURL(wopiContext middleware.WopiContext, collaborationURL *url.URL, userID string) (string, error) { + token, _, err := middleware.GenerateWopiToken(wopiContext, f.cfg, f.store) + if err != nil { + return "", err + } + avatarURL := *collaborationURL + avatarURL.Path = path.Join(collaborationURL.Path, "wopi/avatars/", userID) + q := avatarURL.Query() + q.Add("access_token", token) + avatarURL.RawQuery = q.Encode() + return avatarURL.String(), nil +} + func createHostUrl(mode string, u *url.URL, appName string, info *providerv1beta1.ResourceInfo) string { webUrl := createAppExternalURL(u, appName, info) addURLParams(webUrl, map[string]string{"view_mode": mode}) diff --git a/services/collaboration/pkg/connector/fileconnector_test.go b/services/collaboration/pkg/connector/fileconnector_test.go index 74efa85f91..c1cd3662cd 100644 --- a/services/collaboration/pkg/connector/fileconnector_test.go +++ b/services/collaboration/pkg/connector/fileconnector_test.go @@ -1994,13 +1994,24 @@ var _ = Describe("FileConnector", func() { PostMessageOrigin: "https://cloud.opencloud.test", EnableInsertRemoteImage: true, IsAdminUser: true, + UserExtraInfo: &fileinfo.UserExtraInfo{ + Mail: "shaft@example.com", + }, } response, err := fc.CheckFileInfo(ctx) Expect(err).ToNot(HaveOccurred()) Expect(response.Status).To(Equal(200)) - Expect(response.Body.(*fileinfo.Collabora)).To(Equal(expectedFileInfo)) + body := response.Body.(*fileinfo.Collabora) + // Avatar URL contains a dynamic WOPI token, so check prefix separately + Expect(body.UserExtraInfo).ToNot(BeNil()) + Expect(body.UserExtraInfo.Avatar).To(ContainSubstring("/wopi/avatars/aabbcc?access_token=")) + Expect(body.UserExtraInfo.Mail).To(Equal("shaft@example.com")) + // Clear dynamic fields for struct comparison + body.UserExtraInfo.Avatar = "" + expectedFileInfo.UserExtraInfo.Avatar = "" + Expect(body).To(Equal(expectedFileInfo)) }) It("Stat success with template", func() { wopiCtx.TemplateReference = &providerv1beta1.Reference{ diff --git a/services/collaboration/pkg/connector/fileinfo/collabora.go b/services/collaboration/pkg/connector/fileinfo/collabora.go index d23e4e1a4b..a3e0ee2a1c 100644 --- a/services/collaboration/pkg/connector/fileinfo/collabora.go +++ b/services/collaboration/pkg/connector/fileinfo/collabora.go @@ -1,5 +1,13 @@ package fileinfo +// UserExtraInfo contains additional user info shared across collaborative +// editing views, such as the user's avatar image and email. +// https://sdk.collaboraonline.com/docs/advanced_integration.html#userextrainfo +type UserExtraInfo struct { + Avatar string `json:"avatar,omitempty"` + Mail string `json:"mail,omitempty"` +} + // Collabora fileInfo properties // // Collabora WOPI check file info specification: @@ -62,7 +70,8 @@ type Collabora struct { IsAnonymousUser bool `json:"IsAnonymousUser,omitempty"` // JSON object that contains additional info about the user, namely the avatar image. - //UserExtraInfo -> requires definition, currently not used + // Shared among all views in collaborative editing sessions. + UserExtraInfo *UserExtraInfo `json:"UserExtraInfo,omitempty"` // JSON object that contains additional info about the user, but unlike the UserExtraInfo it is not shared among the views in collaborative editing sessions. //UserPrivateInfo -> requires definition, currently not used @@ -131,7 +140,8 @@ func (cinfo *Collabora) SetProperties(props map[string]interface{}) { cinfo.SaveAsPostmessage = value.(bool) case KeyEnableOwnerTermination: cinfo.EnableOwnerTermination = value.(bool) - //UserExtraInfo -> requires definition, currently not used + case KeyUserExtraInfo: + cinfo.UserExtraInfo = value.(*UserExtraInfo) //UserPrivateInfo -> requires definition, currently not used case KeyWatermarkText: cinfo.WatermarkText = value.(string) diff --git a/services/collaboration/pkg/connector/fileinfo/fileinfo.go b/services/collaboration/pkg/connector/fileinfo/fileinfo.go index 8791449d36..2429929641 100644 --- a/services/collaboration/pkg/connector/fileinfo/fileinfo.go +++ b/services/collaboration/pkg/connector/fileinfo/fileinfo.go @@ -113,7 +113,7 @@ const ( KeyDownloadAsPostMessage = "DownloadAsPostMessage" KeySaveAsPostmessage = "SaveAsPostmessage" KeyEnableOwnerTermination = "EnableOwnerTermination" - //KeyUserExtraInfo -> requires definition, currently not used + KeyUserExtraInfo = "UserExtraInfo" //KeyUserPrivateInfo -> requires definition, currently not used KeyWatermarkText = "WatermarkText" diff --git a/services/collaboration/pkg/connector/httpadapter.go b/services/collaboration/pkg/connector/httpadapter.go index 3ee48fac92..5e097e758e 100644 --- a/services/collaboration/pkg/connector/httpadapter.go +++ b/services/collaboration/pkg/connector/httpadapter.go @@ -3,13 +3,18 @@ package connector import ( "encoding/json" "errors" + "io" "net/http" "strconv" + "github.com/go-chi/chi/v5" + gatewayv1beta1 "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1" "github.com/opencloud-eu/opencloud/services/collaboration/pkg/config" "github.com/opencloud-eu/opencloud/services/collaboration/pkg/connector/utf7" "github.com/opencloud-eu/opencloud/services/collaboration/pkg/locks" + "github.com/opencloud-eu/opencloud/services/collaboration/pkg/middleware" + revactx "github.com/opencloud-eu/reva/v2/pkg/ctx" "github.com/opencloud-eu/reva/v2/pkg/rgrpc/todo/pool" "github.com/rs/zerolog" microstore "go-micro.dev/v4/store" @@ -41,12 +46,14 @@ const ( type HttpAdapter struct { con ConnectorService locks locks.LockParser + cfg *config.Config } // NewHttpAdapter will create a new HTTP adapter. A new connector using the // provided gateway API client and configuration will be used in the adapter func NewHttpAdapter(gws pool.Selectable[gatewayv1beta1.GatewayAPIClient], cfg *config.Config, st microstore.Store) *HttpAdapter { httpAdapter := &HttpAdapter{ + cfg: cfg, con: NewConnector( NewFileConnector(gws, cfg, st), NewContentConnector(gws, cfg), @@ -301,6 +308,66 @@ func (h *HttpAdapter) RenameFile(w http.ResponseWriter, r *http.Request) { h.writeConnectorResponse(w, r, response) } +// GetAvatar proxies the user's avatar from the Graph API. +// The WOPI token in the query string provides authentication (validated by +// the WopiContextAuthMiddleware). Collabora loads avatars via img.src which +// is a plain browser GET — it cannot send auth headers. +// +// Internally, the handler calls the Graph service directly (bypassing the +// proxy) and authenticates with the reva access token via the x-access-token +// header — the same mechanism the proxy uses when forwarding requests to +// backend services. +func (h *HttpAdapter) GetAvatar(w http.ResponseWriter, r *http.Request) { + logger := zerolog.Ctx(r.Context()) + userID := chi.URLParam(r, "userID") + if userID == "" { + http.Error(w, http.StatusText(http.StatusBadRequest), http.StatusBadRequest) + return + } + + wopiContext, err := middleware.WopiContextFromCtx(r.Context()) + if err != nil { + logger.Error().Err(err).Msg("GetAvatar: missing WOPI context") + http.Error(w, http.StatusText(http.StatusUnauthorized), http.StatusUnauthorized) + return + } + + // Build the internal Graph API URL for the user's photo. + // We call the Graph service directly (not through the proxy) so we can + // authenticate with the reva token via x-access-token. + graphEndpoint := h.cfg.CS3Api.GraphEndpoint + avatarURL := graphEndpoint + "/v1.0/users/" + userID + "/photo/$value" + + httpReq, err := http.NewRequestWithContext(r.Context(), http.MethodGet, avatarURL, nil) + if err != nil { + logger.Error().Err(err).Msg("GetAvatar: failed to create request") + http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) + return + } + httpReq.Header.Set(revactx.TokenHeader, wopiContext.AccessToken) + + httpResp, err := http.DefaultClient.Do(httpReq) + if err != nil { + logger.Error().Err(err).Msg("GetAvatar: failed to fetch avatar from Graph API") + http.Error(w, http.StatusText(http.StatusBadGateway), http.StatusBadGateway) + return + } + defer httpResp.Body.Close() + + if httpResp.StatusCode != http.StatusOK { + logger.Warn().Int("status", httpResp.StatusCode).Msg("GetAvatar: Graph API returned non-200") + http.Error(w, http.StatusText(httpResp.StatusCode), httpResp.StatusCode) + return + } + + if ct := httpResp.Header.Get("Content-Type"); ct != "" { + w.Header().Set("Content-Type", ct) + } + w.Header().Set("Cache-Control", "public, max-age=300") + w.WriteHeader(http.StatusOK) + io.Copy(w, httpResp.Body) +} + func (h *HttpAdapter) writeConnectorResponse(w http.ResponseWriter, r *http.Request, response *ConnectorResponse) { jsonBody := []byte{} if response.Body != nil { diff --git a/services/collaboration/pkg/middleware/wopicontext.go b/services/collaboration/pkg/middleware/wopicontext.go index 37724ea7c3..15cadb5c06 100644 --- a/services/collaboration/pkg/middleware/wopicontext.go +++ b/services/collaboration/pkg/middleware/wopicontext.go @@ -143,22 +143,28 @@ func WopiContextAuthMiddleware(cfg *config.Config, st microstore.Store, next htt Logger() ctx = wopiLogger.WithContext(ctx) - hashedRef := helpers.HashResourceId(claims.WopiContext.FileReference.GetResourceId()) - fileID := parseWopiFileID(cfg, r.URL.Path) - if claims.WopiContext.TemplateReference != nil { - hashedTemplateRef := helpers.HashResourceId(claims.WopiContext.TemplateReference.GetResourceId()) - // the fileID could be one of the references within the access token if both are set - // because we can use the access token to get the contents of the template file - if fileID != hashedTemplateRef && fileID != hashedRef { - wopiLogger.Error().Msg("file reference in the URL doesn't match the one inside the access token") - http.Error(w, http.StatusText(http.StatusUnauthorized), http.StatusUnauthorized) - return - } - } else { - if fileID != hashedRef { - wopiLogger.Error().Msg("file reference in the URL doesn't match the one inside the access token") - http.Error(w, http.StatusText(http.StatusUnauthorized), http.StatusUnauthorized) - return + // Validate that the file ID in the URL matches the WOPI token's file + // reference. This check only applies to /wopi/files/ and /wopi/templates/ + // paths. Other WOPI-authenticated endpoints (e.g. /wopi/avatars/) don't + // carry a file ID in the URL — they only need a valid WOPI token. + if strings.Contains(r.URL.Path, "/files/") || strings.Contains(r.URL.Path, "/templates/") { + hashedRef := helpers.HashResourceId(claims.WopiContext.FileReference.GetResourceId()) + fileID := parseWopiFileID(cfg, r.URL.Path) + if claims.WopiContext.TemplateReference != nil { + hashedTemplateRef := helpers.HashResourceId(claims.WopiContext.TemplateReference.GetResourceId()) + // the fileID could be one of the references within the access token if both are set + // because we can use the access token to get the contents of the template file + if fileID != hashedTemplateRef && fileID != hashedRef { + wopiLogger.Error().Msg("file reference in the URL doesn't match the one inside the access token") + http.Error(w, http.StatusText(http.StatusUnauthorized), http.StatusUnauthorized) + return + } + } else { + if fileID != hashedRef { + wopiLogger.Error().Msg("file reference in the URL doesn't match the one inside the access token") + http.Error(w, http.StatusText(http.StatusUnauthorized), http.StatusUnauthorized) + return + } } } diff --git a/services/collaboration/pkg/server/http/server.go b/services/collaboration/pkg/server/http/server.go index daba9cf940..bd8953532e 100644 --- a/services/collaboration/pkg/server/http/server.go +++ b/services/collaboration/pkg/server/http/server.go @@ -194,5 +194,20 @@ func prepareRoutes(r *chi.Mux, options Options) { adapter.GetFile(w, r) }) }) + + // Avatar proxy: serves user avatars authenticated by the WOPI token. + // Collabora loads avatars via img.src (plain GET, no auth headers), + // so the token must be in the URL query string. + r.Route("/avatars/{userID}", func(r chi.Router) { + r.Use( + func(h stdhttp.Handler) stdhttp.Handler { + return colabmiddleware.WopiContextAuthMiddleware(options.Config, options.Store, h) + }, + colabmiddleware.CollaborationTracingMiddleware, + ) + r.Get("/", func(w stdhttp.ResponseWriter, r *stdhttp.Request) { + adapter.GetAvatar(w, r) + }) + }) }) } From d05db011e5fc720471ecc6c4708db9f30581c679 Mon Sep 17 00:00:00 2001 From: Michael Barz Date: Tue, 14 Apr 2026 17:46:45 +0200 Subject: [PATCH 2/4] refactor: move implementation to fileconnector --- .bingo/go-xgettext.mod | 2 - .../mocks/file_connector_service.go | 68 ++++++++++++++++++ .../pkg/connector/fileconnector.go | 63 +++++++++++++++++ .../pkg/connector/httpadapter.go | 69 +++++++------------ 4 files changed, 155 insertions(+), 47 deletions(-) diff --git a/.bingo/go-xgettext.mod b/.bingo/go-xgettext.mod index b14946a0b8..8e3c6210b5 100644 --- a/.bingo/go-xgettext.mod +++ b/.bingo/go-xgettext.mod @@ -3,5 +3,3 @@ module _ // Auto generated by https://github.com/bwplotka/bingo. DO NOT EDIT go 1.23.4 require github.com/gosexy/gettext v0.0.0-20160830220431-74466a0a0c4a // go-xgettext - -require github.com/jessevdk/go-flags v1.6.1 // indirect diff --git a/services/collaboration/mocks/file_connector_service.go b/services/collaboration/mocks/file_connector_service.go index 3e9f5041db..7baf292d92 100644 --- a/services/collaboration/mocks/file_connector_service.go +++ b/services/collaboration/mocks/file_connector_service.go @@ -169,6 +169,74 @@ func (_c *FileConnectorService_DeleteFile_Call) RunAndReturn(run func(ctx contex return _c } +// GetAvatar provides a mock function for the type FileConnectorService +func (_mock *FileConnectorService) GetAvatar(ctx context.Context, userID string) (*connector.ConnectorResponse, error) { + ret := _mock.Called(ctx, userID) + + if len(ret) == 0 { + panic("no return value specified for GetAvatar") + } + + var r0 *connector.ConnectorResponse + var r1 error + if returnFunc, ok := ret.Get(0).(func(context.Context, string) (*connector.ConnectorResponse, error)); ok { + return returnFunc(ctx, userID) + } + if returnFunc, ok := ret.Get(0).(func(context.Context, string) *connector.ConnectorResponse); ok { + r0 = returnFunc(ctx, userID) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*connector.ConnectorResponse) + } + } + if returnFunc, ok := ret.Get(1).(func(context.Context, string) error); ok { + r1 = returnFunc(ctx, userID) + } else { + r1 = ret.Error(1) + } + return r0, r1 +} + +// FileConnectorService_GetAvatar_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'GetAvatar' +type FileConnectorService_GetAvatar_Call struct { + *mock.Call +} + +// GetAvatar is a helper method to define mock.On call +// - ctx context.Context +// - userID string +func (_e *FileConnectorService_Expecter) GetAvatar(ctx interface{}, userID interface{}) *FileConnectorService_GetAvatar_Call { + return &FileConnectorService_GetAvatar_Call{Call: _e.mock.On("GetAvatar", ctx, userID)} +} + +func (_c *FileConnectorService_GetAvatar_Call) Run(run func(ctx context.Context, userID string)) *FileConnectorService_GetAvatar_Call { + _c.Call.Run(func(args mock.Arguments) { + var arg0 context.Context + if args[0] != nil { + arg0 = args[0].(context.Context) + } + var arg1 string + if args[1] != nil { + arg1 = args[1].(string) + } + run( + arg0, + arg1, + ) + }) + return _c +} + +func (_c *FileConnectorService_GetAvatar_Call) Return(connectorResponse *connector.ConnectorResponse, err error) *FileConnectorService_GetAvatar_Call { + _c.Call.Return(connectorResponse, err) + return _c +} + +func (_c *FileConnectorService_GetAvatar_Call) RunAndReturn(run func(ctx context.Context, userID string) (*connector.ConnectorResponse, error)) *FileConnectorService_GetAvatar_Call { + _c.Call.Return(run) + return _c +} + // GetLock provides a mock function for the type FileConnectorService func (_mock *FileConnectorService) GetLock(ctx context.Context) (*connector.ConnectorResponse, error) { ret := _mock.Called(ctx) diff --git a/services/collaboration/pkg/connector/fileconnector.go b/services/collaboration/pkg/connector/fileconnector.go index bede7b8772..1b6dcd9ba7 100644 --- a/services/collaboration/pkg/connector/fileconnector.go +++ b/services/collaboration/pkg/connector/fileconnector.go @@ -8,6 +8,7 @@ import ( "encoding/hex" "fmt" "io" + "net/http" "net/url" "path" "strings" @@ -94,6 +95,10 @@ type FileConnectorService interface { // In case of conflict, this method will return the actual lockId in // the file as second return value. RenameFile(ctx context.Context, lockID, target string) (*ConnectorResponse, error) + // GetAvatar fetches the user's avatar image from the Graph API. + // The response Body contains the raw image bytes ([]byte) and the + // Headers contain Content-Type and Cache-Control. + GetAvatar(ctx context.Context, userID string) (*ConnectorResponse, error) } // FileConnector implements the "File" endpoint. @@ -1539,3 +1544,61 @@ func (f *FileConnector) getScopeByKeyPrefix(scopes map[string]*auth.Scope, keyPr } return fmt.Errorf("scope %s not found", keyPrefix) } + +// GetAvatar fetches the user's avatar image from the Graph API. +// The returned ConnectorResponse carries the raw image bytes in Body ([]byte) +// and Content-Type / Cache-Control in Headers. +func (f *FileConnector) GetAvatar(ctx context.Context, userID string) (*ConnectorResponse, error) { + logger := zerolog.Ctx(ctx) + + wopiContext, err := middleware.WopiContextFromCtx(ctx) + if err != nil { + logger.Error().Err(err).Msg("GetAvatar: missing WOPI context") + return nil, NewConnectorError(http.StatusUnauthorized, "missing WOPI context") + } + + avatarURL := f.cfg.CS3Api.GraphEndpoint + "/v1.0/users/" + userID + "/photo/$value" + + httpReq, err := http.NewRequestWithContext(ctx, http.MethodGet, avatarURL, nil) + if err != nil { + logger.Error().Err(err).Msg("GetAvatar: failed to create request") + return nil, NewConnectorError(http.StatusInternalServerError, "failed to create request") + } + httpReq.Header.Set(ctxpkg.TokenHeader, wopiContext.AccessToken) + + httpResp, err := http.DefaultClient.Do(httpReq) + if err != nil { + logger.Error().Err(err).Msg("GetAvatar: failed to fetch avatar from Graph API") + return nil, NewConnectorError(http.StatusBadGateway, "failed to fetch avatar") + } + defer httpResp.Body.Close() + + if httpResp.StatusCode != http.StatusOK { + logger.Warn().Int("status", httpResp.StatusCode).Msg("GetAvatar: Graph API returned non-200") + return nil, NewConnectorError(httpResp.StatusCode, http.StatusText(httpResp.StatusCode)) + } + + data, err := io.ReadAll(httpResp.Body) + if err != nil { + logger.Error().Err(err).Msg("GetAvatar: failed to read avatar body") + return nil, NewConnectorError(http.StatusInternalServerError, "failed to read avatar body") + } + + headers := map[string]string{ + "Cache-Control": "public, max-age=300", + } + + if ct := httpResp.Header.Get("Content-Type"); ct != "" { + headers["Content-Type"] = ct + } + + if cl := httpResp.Header.Get("Content-Length"); cl != "" { + headers["Content-Length"] = cl + } + + return &ConnectorResponse{ + Status: http.StatusOK, + Headers: headers, + Body: data, + }, nil +} diff --git a/services/collaboration/pkg/connector/httpadapter.go b/services/collaboration/pkg/connector/httpadapter.go index 5e097e758e..454a664a6d 100644 --- a/services/collaboration/pkg/connector/httpadapter.go +++ b/services/collaboration/pkg/connector/httpadapter.go @@ -3,7 +3,6 @@ package connector import ( "encoding/json" "errors" - "io" "net/http" "strconv" @@ -13,8 +12,6 @@ import ( "github.com/opencloud-eu/opencloud/services/collaboration/pkg/config" "github.com/opencloud-eu/opencloud/services/collaboration/pkg/connector/utf7" "github.com/opencloud-eu/opencloud/services/collaboration/pkg/locks" - "github.com/opencloud-eu/opencloud/services/collaboration/pkg/middleware" - revactx "github.com/opencloud-eu/reva/v2/pkg/ctx" "github.com/opencloud-eu/reva/v2/pkg/rgrpc/todo/pool" "github.com/rs/zerolog" microstore "go-micro.dev/v4/store" @@ -46,14 +43,12 @@ const ( type HttpAdapter struct { con ConnectorService locks locks.LockParser - cfg *config.Config } // NewHttpAdapter will create a new HTTP adapter. A new connector using the // provided gateway API client and configuration will be used in the adapter func NewHttpAdapter(gws pool.Selectable[gatewayv1beta1.GatewayAPIClient], cfg *config.Config, st microstore.Store) *HttpAdapter { httpAdapter := &HttpAdapter{ - cfg: cfg, con: NewConnector( NewFileConnector(gws, cfg, st), NewContentConnector(gws, cfg), @@ -312,11 +307,6 @@ func (h *HttpAdapter) RenameFile(w http.ResponseWriter, r *http.Request) { // The WOPI token in the query string provides authentication (validated by // the WopiContextAuthMiddleware). Collabora loads avatars via img.src which // is a plain browser GET — it cannot send auth headers. -// -// Internally, the handler calls the Graph service directly (bypassing the -// proxy) and authenticates with the reva access token via the x-access-token -// header — the same mechanism the proxy uses when forwarding requests to -// backend services. func (h *HttpAdapter) GetAvatar(w http.ResponseWriter, r *http.Request) { logger := zerolog.Ctx(r.Context()) userID := chi.URLParam(r, "userID") @@ -325,47 +315,36 @@ func (h *HttpAdapter) GetAvatar(w http.ResponseWriter, r *http.Request) { return } - wopiContext, err := middleware.WopiContextFromCtx(r.Context()) + fileCon := h.con.GetFileConnector() + response, err := fileCon.GetAvatar(r.Context(), userID) if err != nil { - logger.Error().Err(err).Msg("GetAvatar: missing WOPI context") - http.Error(w, http.StatusText(http.StatusUnauthorized), http.StatusUnauthorized) + var connErr *ConnectorError + if errors.As(err, &connErr) { + http.Error(w, http.StatusText(connErr.HttpCodeOut), connErr.HttpCodeOut) + } else { + logger.Error().Err(err).Msg("GetAvatar: failed to fetch avatar") + http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) + } return } + h.writeConnectorAvatarResponse(w, r, response) +} - // Build the internal Graph API URL for the user's photo. - // We call the Graph service directly (not through the proxy) so we can - // authenticate with the reva token via x-access-token. - graphEndpoint := h.cfg.CS3Api.GraphEndpoint - avatarURL := graphEndpoint + "/v1.0/users/" + userID + "/photo/$value" - - httpReq, err := http.NewRequestWithContext(r.Context(), http.MethodGet, avatarURL, nil) +func (h *HttpAdapter) writeConnectorAvatarResponse(w http.ResponseWriter, r *http.Request, response *ConnectorResponse) { + data, _ := response.Body.([]byte) + for key, value := range response.Headers { + w.Header().Set(key, value) + } + w.WriteHeader(response.Status) + written, err := w.Write(data) if err != nil { - logger.Error().Err(err).Msg("GetAvatar: failed to create request") - http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) - return + logger := zerolog.Ctx(r.Context()) + logger.Error(). + Err(err). + Int("TotalBytes", len(data)). + Int("WrittenBytes", written). + Msg("failed to write avatar contents in the HTTP response") } - httpReq.Header.Set(revactx.TokenHeader, wopiContext.AccessToken) - - httpResp, err := http.DefaultClient.Do(httpReq) - if err != nil { - logger.Error().Err(err).Msg("GetAvatar: failed to fetch avatar from Graph API") - http.Error(w, http.StatusText(http.StatusBadGateway), http.StatusBadGateway) - return - } - defer httpResp.Body.Close() - - if httpResp.StatusCode != http.StatusOK { - logger.Warn().Int("status", httpResp.StatusCode).Msg("GetAvatar: Graph API returned non-200") - http.Error(w, http.StatusText(httpResp.StatusCode), httpResp.StatusCode) - return - } - - if ct := httpResp.Header.Get("Content-Type"); ct != "" { - w.Header().Set("Content-Type", ct) - } - w.Header().Set("Cache-Control", "public, max-age=300") - w.WriteHeader(http.StatusOK) - io.Copy(w, httpResp.Body) } func (h *HttpAdapter) writeConnectorResponse(w http.ResponseWriter, r *http.Request, response *ConnectorResponse) { From 940bfe5d65b076225ced527dd00f218c3739abb0 Mon Sep 17 00:00:00 2001 From: Michael Barz Date: Wed, 15 Apr 2026 08:00:35 +0200 Subject: [PATCH 3/4] fix: use libregraph client --- services/collaboration/pkg/command/server.go | 3 +- .../pkg/connector/fileconnector.go | 66 +++++++++++-------- .../pkg/connector/fileconnector_test.go | 2 +- .../pkg/connector/httpadapter.go | 5 +- 4 files changed, 46 insertions(+), 30 deletions(-) diff --git a/services/collaboration/pkg/command/server.go b/services/collaboration/pkg/command/server.go index 92e14c0972..afbd1ebf7f 100644 --- a/services/collaboration/pkg/command/server.go +++ b/services/collaboration/pkg/command/server.go @@ -23,6 +23,7 @@ import ( "github.com/opencloud-eu/reva/v2/pkg/store" "github.com/spf13/cobra" + "go-micro.dev/v4/selector" microstore "go-micro.dev/v4/store" ) @@ -138,7 +139,7 @@ func Server(cfg *config.Config) *cobra.Command { // start HTTP server httpServer, err := http.Server( - http.Adapter(connector.NewHttpAdapter(gatewaySelector, cfg, st)), + http.Adapter(connector.NewHttpAdapter(gatewaySelector, cfg, st, selector.NewSelector(selector.Registry(registry.GetRegistry())))), http.Logger(logger), http.Config(cfg), http.Context(ctx), diff --git a/services/collaboration/pkg/connector/fileconnector.go b/services/collaboration/pkg/connector/fileconnector.go index 1b6dcd9ba7..2325342b99 100644 --- a/services/collaboration/pkg/connector/fileconnector.go +++ b/services/collaboration/pkg/connector/fileconnector.go @@ -14,6 +14,9 @@ import ( "strings" "time" + libregraph "github.com/opencloud-eu/libre-graph-api-go" + "go-micro.dev/v4/selector" + appproviderv1beta1 "github.com/cs3org/go-cs3apis/cs3/app/provider/v1beta1" auth "github.com/cs3org/go-cs3apis/cs3/auth/provider/v1beta1" gatewayv1beta1 "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1" @@ -105,17 +108,19 @@ type FileConnectorService interface { // Currently, it handles file locks and getting the file info. // Note that operations might return any kind of error, not just ConnectorError type FileConnector struct { - gws pool.Selectable[gatewayv1beta1.GatewayAPIClient] - cfg *config.Config - store microstore.Store + gws pool.Selectable[gatewayv1beta1.GatewayAPIClient] + cfg *config.Config + store microstore.Store + graphSelector selector.Selector } // NewFileConnector creates a new file connector -func NewFileConnector(gws pool.Selectable[gatewayv1beta1.GatewayAPIClient], cfg *config.Config, st microstore.Store) *FileConnector { +func NewFileConnector(gws pool.Selectable[gatewayv1beta1.GatewayAPIClient], cfg *config.Config, st microstore.Store, graphSelector selector.Selector) *FileConnector { return &FileConnector{ - gws: gws, - cfg: cfg, - store: st, + gws: gws, + cfg: cfg, + store: st, + graphSelector: graphSelector, } } @@ -1557,28 +1562,23 @@ func (f *FileConnector) GetAvatar(ctx context.Context, userID string) (*Connecto return nil, NewConnectorError(http.StatusUnauthorized, "missing WOPI context") } - avatarURL := f.cfg.CS3Api.GraphEndpoint + "/v1.0/users/" + userID + "/photo/$value" - - httpReq, err := http.NewRequestWithContext(ctx, http.MethodGet, avatarURL, nil) + lgClient, err := f.setupLibregraphClient(ctx, wopiContext.AccessToken) if err != nil { - logger.Error().Err(err).Msg("GetAvatar: failed to create request") - return nil, NewConnectorError(http.StatusInternalServerError, "failed to create request") + logger.Error().Err(err).Msg("GetAvatar: failed to setup libregraph client") + return nil, NewConnectorError(http.StatusInternalServerError, "failed to setup libregraph client") } - httpReq.Header.Set(ctxpkg.TokenHeader, wopiContext.AccessToken) - httpResp, err := http.DefaultClient.Do(httpReq) + photoFile, httpResp, err := lgClient.UserPhotoApi.GetUserPhoto(ctx, userID).Execute() if err != nil { logger.Error().Err(err).Msg("GetAvatar: failed to fetch avatar from Graph API") + if httpResp != nil { + return nil, NewConnectorError(httpResp.StatusCode, http.StatusText(httpResp.StatusCode)) + } return nil, NewConnectorError(http.StatusBadGateway, "failed to fetch avatar") } - defer httpResp.Body.Close() + defer photoFile.Close() - if httpResp.StatusCode != http.StatusOK { - logger.Warn().Int("status", httpResp.StatusCode).Msg("GetAvatar: Graph API returned non-200") - return nil, NewConnectorError(httpResp.StatusCode, http.StatusText(httpResp.StatusCode)) - } - - data, err := io.ReadAll(httpResp.Body) + data, err := io.ReadAll(photoFile) if err != nil { logger.Error().Err(err).Msg("GetAvatar: failed to read avatar body") return nil, NewConnectorError(http.StatusInternalServerError, "failed to read avatar body") @@ -1587,18 +1587,32 @@ func (f *FileConnector) GetAvatar(ctx context.Context, userID string) (*Connecto headers := map[string]string{ "Cache-Control": "public, max-age=300", } - if ct := httpResp.Header.Get("Content-Type"); ct != "" { headers["Content-Type"] = ct } - if cl := httpResp.Header.Get("Content-Length"); cl != "" { - headers["Content-Length"] = cl - } - return &ConnectorResponse{ Status: http.StatusOK, Headers: headers, Body: data, }, nil } + +func (f *FileConnector) setupLibregraphClient(_ context.Context, cs3token string) (*libregraph.APIClient, error) { + next, err := f.graphSelector.Select("eu.opencloud.web.graph") + if err != nil { + return nil, err + } + node, err := next() + if err != nil { + return nil, err + } + lgconf := libregraph.NewConfiguration() + lgconf.Servers = libregraph.ServerConfigurations{ + { + URL: fmt.Sprintf("%s://%s/graph", node.Metadata["protocol"], node.Address), + }, + } + lgconf.DefaultHeader = map[string]string{ctxpkg.TokenHeader: cs3token} + return libregraph.NewAPIClient(lgconf), nil +} diff --git a/services/collaboration/pkg/connector/fileconnector_test.go b/services/collaboration/pkg/connector/fileconnector_test.go index c1cd3662cd..cdfe7e6c9e 100644 --- a/services/collaboration/pkg/connector/fileconnector_test.go +++ b/services/collaboration/pkg/connector/fileconnector_test.go @@ -65,7 +65,7 @@ var _ = Describe("FileConnector", func() { gatewaySelector = mocks.NewSelectable[gateway.GatewayAPIClient](GinkgoT()) gatewaySelector.On("Next").Return(gatewayClient, nil) - fc = connector.NewFileConnector(gatewaySelector, cfg, nil) + fc = connector.NewFileConnector(gatewaySelector, cfg, nil, nil) wopiCtx = middleware.WopiContext{ // a real token is needed for the PutRelativeFileSuggested tests diff --git a/services/collaboration/pkg/connector/httpadapter.go b/services/collaboration/pkg/connector/httpadapter.go index 454a664a6d..a983c2b6d9 100644 --- a/services/collaboration/pkg/connector/httpadapter.go +++ b/services/collaboration/pkg/connector/httpadapter.go @@ -14,6 +14,7 @@ import ( "github.com/opencloud-eu/opencloud/services/collaboration/pkg/locks" "github.com/opencloud-eu/reva/v2/pkg/rgrpc/todo/pool" "github.com/rs/zerolog" + "go-micro.dev/v4/selector" microstore "go-micro.dev/v4/store" ) @@ -47,10 +48,10 @@ type HttpAdapter struct { // NewHttpAdapter will create a new HTTP adapter. A new connector using the // provided gateway API client and configuration will be used in the adapter -func NewHttpAdapter(gws pool.Selectable[gatewayv1beta1.GatewayAPIClient], cfg *config.Config, st microstore.Store) *HttpAdapter { +func NewHttpAdapter(gws pool.Selectable[gatewayv1beta1.GatewayAPIClient], cfg *config.Config, st microstore.Store, graphSelector selector.Selector) *HttpAdapter { httpAdapter := &HttpAdapter{ con: NewConnector( - NewFileConnector(gws, cfg, st), + NewFileConnector(gws, cfg, st, graphSelector), NewContentConnector(gws, cfg), ), } From 35aaf92384e47c235471211cecde7e4cac98926f Mon Sep 17 00:00:00 2001 From: Michael Barz Date: Wed, 15 Apr 2026 08:13:22 +0200 Subject: [PATCH 4/4] test: add tests for new features --- services/collaboration/pkg/config/cs3api.go | 7 +- .../pkg/config/defaults/defaultconfig.go | 1 - .../pkg/connector/fileconnector_test.go | 14 ++++ .../pkg/connector/httpadapter_test.go | 68 +++++++++++++++++++ 4 files changed, 85 insertions(+), 5 deletions(-) diff --git a/services/collaboration/pkg/config/cs3api.go b/services/collaboration/pkg/config/cs3api.go index 8eb6ed5904..c9234ce3c1 100644 --- a/services/collaboration/pkg/config/cs3api.go +++ b/services/collaboration/pkg/config/cs3api.go @@ -9,10 +9,9 @@ import ( // CS3Api defines the available configuration in order to access to the CS3 gateway. type CS3Api struct { Gateway Gateway `yaml:"gateway"` - DataGateway DataGateway `yaml:"datagateway"` - GraphEndpoint string `yaml:"graph_endpoint" env:"COLLABORATION_CS3API_GRAPH_ENDPOINT" desc:"The internal HTTP endpoint of the Graph service, used to fetch user profile photos for avatars." introductionVersion:"4.0.0"` - GRPCClientTLS *shared.GRPCClientTLS `yaml:"grpc_client_tls"` - APPRegistrationInterval time.Duration `yaml:"app_registration_interval" env:"COLLABORATION_CS3API_APP_REGISTRATION_INTERVAL" desc:"The interval at which the app provider registers itself." introductionVersion:"4.0.0"` + DataGateway DataGateway `yaml:"datagateway"` + GRPCClientTLS *shared.GRPCClientTLS `yaml:"grpc_client_tls"` + APPRegistrationInterval time.Duration `yaml:"app_registration_interval" env:"COLLABORATION_CS3API_APP_REGISTRATION_INTERVAL" desc:"The interval at which the app provider registers itself." introductionVersion:"4.0.0"` } // Gateway defines the available configuration for the CS3 API gateway diff --git a/services/collaboration/pkg/config/defaults/defaultconfig.go b/services/collaboration/pkg/config/defaults/defaultconfig.go index cdb30a44c1..20aaf45b86 100644 --- a/services/collaboration/pkg/config/defaults/defaultconfig.go +++ b/services/collaboration/pkg/config/defaults/defaultconfig.go @@ -65,7 +65,6 @@ func DefaultConfig() *config.Config { DataGateway: config.DataGateway{ Insecure: false, }, - GraphEndpoint: "http://127.0.0.1:9120/graph", APPRegistrationInterval: 30 * time.Second, }, } diff --git a/services/collaboration/pkg/connector/fileconnector_test.go b/services/collaboration/pkg/connector/fileconnector_test.go index cdfe7e6c9e..f8f0877f6d 100644 --- a/services/collaboration/pkg/connector/fileconnector_test.go +++ b/services/collaboration/pkg/connector/fileconnector_test.go @@ -2106,4 +2106,18 @@ var _ = Describe("FileConnector", func() { Expect(templateSource).To(HavePrefix(expectedTemplateSource)) }) }) + + Describe("GetAvatar", func() { + It("No valid context returns Unauthorized error", func() { + // GetAvatar returns before touching the gateway selector. + gatewaySelector.EXPECT().Next().Unset() + ctx := context.Background() + response, err := fc.GetAvatar(ctx, "user-123") + Expect(response).To(BeNil()) + Expect(err).To(HaveOccurred()) + var connErr *connector.ConnectorError + Expect(errors.As(err, &connErr)).To(BeTrue()) + Expect(connErr.HttpCodeOut).To(Equal(401)) + }) + }) }) diff --git a/services/collaboration/pkg/connector/httpadapter_test.go b/services/collaboration/pkg/connector/httpadapter_test.go index f03dbc9a5b..58dd4bbdb8 100644 --- a/services/collaboration/pkg/connector/httpadapter_test.go +++ b/services/collaboration/pkg/connector/httpadapter_test.go @@ -1,6 +1,7 @@ package connector_test import ( + "context" "encoding/json" "errors" "io" @@ -8,6 +9,7 @@ import ( "strings" typesv1beta1 "github.com/cs3org/go-cs3apis/cs3/types/v1beta1" + "github.com/go-chi/chi/v5" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "github.com/opencloud-eu/opencloud/services/collaboration/mocks" @@ -538,6 +540,72 @@ var _ = Describe("HttpAdapter", func() { }) }) + Describe("GetAvatar", func() { + It("Missing userID returns 400", func() { + // No chi route context means chi.URLParam returns "" + req := httptest.NewRequest("GET", "/wopi/avatars/", nil) + w := httptest.NewRecorder() + + httpAdapter.GetAvatar(w, req) + Expect(w.Result().StatusCode).To(Equal(400)) + }) + + It("ConnectorError propagates the status code", func() { + req := httptest.NewRequest("GET", "/wopi/avatars/user-123", nil) + rctx := chi.NewRouteContext() + rctx.URLParams.Add("userID", "user-123") + req = req.WithContext(context.WithValue(req.Context(), chi.RouteCtxKey, rctx)) + w := httptest.NewRecorder() + + fc.On("GetAvatar", mock.Anything, "user-123").Times(1). + Return(nil, connector.NewConnectorError(502, "Bad Gateway")) + + httpAdapter.GetAvatar(w, req) + Expect(w.Result().StatusCode).To(Equal(502)) + }) + + It("General error returns 500", func() { + req := httptest.NewRequest("GET", "/wopi/avatars/user-123", nil) + rctx := chi.NewRouteContext() + rctx.URLParams.Add("userID", "user-123") + req = req.WithContext(context.WithValue(req.Context(), chi.RouteCtxKey, rctx)) + w := httptest.NewRecorder() + + fc.On("GetAvatar", mock.Anything, "user-123").Times(1). + Return(nil, errors.New("unexpected failure")) + + httpAdapter.GetAvatar(w, req) + Expect(w.Result().StatusCode).To(Equal(500)) + }) + + It("Success writes Content-Type, Cache-Control and body", func() { + req := httptest.NewRequest("GET", "/wopi/avatars/user-123", nil) + rctx := chi.NewRouteContext() + rctx.URLParams.Add("userID", "user-123") + req = req.WithContext(context.WithValue(req.Context(), chi.RouteCtxKey, rctx)) + w := httptest.NewRecorder() + + avatarData := []byte{0xFF, 0xD8, 0xFF, 0xE0} // fake JPEG bytes + fc.On("GetAvatar", mock.Anything, "user-123").Times(1).Return( + &connector.ConnectorResponse{ + Status: 200, + Headers: map[string]string{ + "Content-Type": "image/jpeg", + "Cache-Control": "public, max-age=300", + }, + Body: avatarData, + }, nil) + + httpAdapter.GetAvatar(w, req) + resp := w.Result() + Expect(resp.StatusCode).To(Equal(200)) + Expect(resp.Header.Get("Content-Type")).To(Equal("image/jpeg")) + Expect(resp.Header.Get("Cache-Control")).To(Equal("public, max-age=300")) + body, _ := io.ReadAll(resp.Body) + Expect(body).To(Equal(avatarData)) + }) + }) + Describe("PutRelativeFile", func() { It("Connector error", func() { contentBody := "this is the new fake content"