incorporate feedback

Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>

.drone.star

@@ -928,9 +928,7 @@ def wopiValidatorTests(ctx, storage, wopiServerType, accounts_hash_difficulty =
                     "MICRO_REGISTRY_ADDRESS": "ocis-server:9233",
                     "COLLABORATION_LOG_LEVEL": "debug",
                     "COLLABORATION_APP_NAME": "FakeOffice",
-                    "COLLABORATION_HTTP_ADDR": "wopiserver:9300",
-                    "COLLABORATION_HTTP_SCHEME": "http",
-                    "COLLABORATION_WOPIAPP_ADDR": "http://fakeoffice:8080",
+                    "COLLABORATION_WOPIAPP_WOPISRC": "http://fakeoffice:8080",
                     "COLLABORATION_WOPIAPP_INSECURE": "true",
                     "COLLABORATION_CS3API_DATAGATEWAY_INSECURE": "true",
                 },

.vscode/launch.json

@@ -50,6 +50,8 @@
                 "OCIS_JWT_SECRET": "some-ocis-jwt-secret",
                 "OCIS_MACHINE_AUTH_API_KEY": "some-ocis-machine-auth-api-key",
                 "OCIS_TRANSFER_SECRET": "some-ocis-transfer-secret",
+                // collaboration
+                "COLLABORATION_WOPIAPP_SECRET": "some-wopi-secret",
                 // idm ldap
                 "IDM_SVC_PASSWORD": "some-ldap-idm-password",
                 "GRAPH_LDAP_BIND_PASSWORD": "some-ldap-idm-password",

.vscode/settings.json

@@ -1 +0,0 @@
-{}

ocis-pkg/shared/errors.go

@@ -85,3 +85,11 @@ func MissingServiceAccountSecret(service string) error {
 		"the config/corresponding environment variable).",
 		service, defaults.BaseConfigPath())
 }
+
+func MissingWOPISecretError(service string) error {
+	return fmt.Errorf("The WOPI secret has not been set properly in your config for %s. "+
+		"Make sure your %s config contains the proper values "+
+		"(e.g. by running ocis init or setting it manually in "+
+		"the config/corresponding environment variable).",
+		service, defaults.BaseConfigPath())
+}

ocis/pkg/init/init.go

@@ -156,8 +156,13 @@ type Clientlog struct {
 	ServiceAccount ServiceAccount `yaml:"service_account"`
 }
 
+type WopiApp struct {
+	Insecure bool   `yaml:"insecure"`
+	Secret   string `yaml:"secret"`
+}
+
 type Collaboration struct {
-	JWTSecret string `yaml:"jwt_secret"`
+	WopiApp WopiApp `yaml:"wopiapp"`
 }
 
 type Nats struct {
@@ -294,9 +299,9 @@ func CreateConfig(insecure, forceOverwrite bool, configPath, adminPassword strin
 	if err != nil {
 		return fmt.Errorf("could not generate random password for tokenmanager: %s", err)
 	}
-	collaborationJwtSecret, err := generators.GenerateRandomPassword(passwordLength)
+	collaborationWOPISecret, err := generators.GenerateRandomPassword(passwordLength)
 	if err != nil {
-		return fmt.Errorf("could not generate random password for collaboration service: %s", err)
+		return fmt.Errorf("could not generate random wopi secret for collaboration service: %s", err)
 	}
 	machineAuthAPIKey, err := generators.GenerateRandomPassword(passwordLength)
 	if err != nil {
@@ -354,7 +359,9 @@ func CreateConfig(insecure, forceOverwrite bool, configPath, adminPassword strin
 			},
 		},
 		Collaboration: Collaboration{
-			JWTSecret: collaborationJwtSecret,
+			WopiApp: WopiApp{
+				Secret: collaborationWOPISecret,
+			},
 		},
 		Groups: UsersAndGroupsService{
 			Drivers: LdapBasedService{
@@ -429,6 +436,7 @@ func CreateConfig(insecure, forceOverwrite bool, configPath, adminPassword strin
 		cfg.AuthBearer = AuthbearerService{
 			AuthProviders: AuthProviderSettings{Oidc: _insecureService},
 		}
+		cfg.Collaboration.WopiApp.Insecure = true
 		cfg.Frontend.AppHandler = _insecureService
 		cfg.Frontend.Archiver = _insecureService
 		cfg.Graph.Spaces = _insecureService

services/collaboration/README.md

@@ -22,11 +22,11 @@ There are a few variables that you need to set:
   The URL of the WOPI app (onlyoffice, collabora, etc).\
   For example: `https://office.example.com`.
 
-* `COLLABORATION_HTTP_ADDR`:\
-  The external address of the collaboration service. The target app (onlyoffice, collabora, etc) will use this address to read and write files from Infinite Scale.\
-  For example: `https://wopiserver.example.com`.
+* `COLLABORATION_WOPIAPP_INSECURE`:\
+  In case you are using a self signed certificate for the WOPI app you can tell the collaboration service to allow an insecure connection.
 
-* `COLLABORATION_HTTP_SCHEME`:\
-  The scheme to be used when accessing the collaboration service. Either `http` or `https`. This will be used to finally build the URL that the WOPI app needs in order to contact the collaboration service.
+* `COLLABORATION_WOPIAPP_WOPISRC`:\
+  The external address of the collaboration service. The target app (onlyoffice, collabora, etc) will use this address to read and write files from Infinite Scale. \
+  For example: `https://wopi.example.com`.
 
-The rest of the configuration options available can be left with the default values.
+The WOPI application can be customized further by changing the `COLLABORATION_APP_*` options to better describe the application.

services/collaboration/pkg/command/server.go

@@ -71,10 +71,7 @@ func Server(cfg *config.Config) *cli.Command {
 			)
 			defer teardown()
 			if err != nil {
-				logger.Info().
-					Err(err).
-					Str("transport", "grpc").
-					Msg("Failed to initialize server")
+				logger.Error().Err(err).Str("transport", "grpc").Msg("Failed to initialize server")
 				return err
 			}
 
@@ -85,11 +82,8 @@ func Server(cfg *config.Config) *cli.Command {
 				}
 				return grpcServer.Serve(l)
 			},
-				func(_ error) {
-					logger.Error().
-						Err(err).
-						Str("server", "grpc").
-						Msg("shutting down server")
+				func(err error) {
+					logger.Error().Err(err).Str("server", "grpc").Msg("shutting down server")
 					cancel()
 				})
 
@@ -100,7 +94,7 @@ func Server(cfg *config.Config) *cli.Command {
 				debug.Config(cfg),
 			)
 			if err != nil {
-				logger.Info().Err(err).Str("transport", "debug").Msg("Failed to initialize server")
+				logger.Error().Err(err).Str("transport", "debug").Msg("Failed to initialize server")
 				return err
 			}
 
@@ -117,6 +111,10 @@ func Server(cfg *config.Config) *cli.Command {
 				http.Context(ctx),
 				http.TracerProvider(traceProvider),
 			)
+			if err != nil {
+				logger.Error().Err(err).Str("transport", "http").Msg("Failed to initialize server")
+				return err
+			}
 			gr.Add(httpServer.Run, func(_ error) {
 				cancel()
 			})

services/collaboration/pkg/config/config.go

@@ -14,7 +14,7 @@ type Config struct {
 	Service Service `yaml:"-"`
 	App     App     `yaml:"app"`
 
-	JWTSecret string `yaml:"jwt_secret" env:"COLLABORATION_JWT_SECRET" desc:"Used as mint and verify WOPI JWT tokens and encrypt and decrypt the REVA JWT token embedded in the WOPI JWT token." introductionVersion:"5.1"`
+	TokenManager *TokenManager `yaml:"token_manager"`
 
 	GRPC    GRPC    `yaml:"grpc"`
 	HTTP    HTTP    `yaml:"http"`

services/collaboration/pkg/config/defaults/defaultconfig.go

@@ -26,14 +26,12 @@ func DefaultConfig() *config.Config {
 			LockName:    "com.github.owncloud.collaboration",
 		},
 		GRPC: config.GRPC{
-			Addr:      "0.0.0.0:9301",
+			Addr:      "127.0.0.1:9301",
 			Namespace: "com.owncloud.api",
 		},
 		HTTP: config.HTTP{
 			Addr:      "127.0.0.1:9300",
-			BindAddr:  "0.0.0.0:9300",
 			Namespace: "com.owncloud.web",
-			Scheme:    "https",
 		},
 		Debug: config.Debug{
 			Addr:   "127.0.0.1:9304",
@@ -42,8 +40,9 @@ func DefaultConfig() *config.Config {
 			Zpages: false,
 		},
 		WopiApp: config.WopiApp{
-			Addr:     "https://127.0.0.1:8080",
+			Addr:     "https://127.0.0.1:9980",
 			Insecure: false,
+			WopiSrc:  "https://localhost:9300",
 		},
 		CS3Api: config.CS3Api{
 			Gateway: config.Gateway{
@@ -81,6 +80,14 @@ func EnsureDefaults(cfg *config.Config) {
 	} else if cfg.Tracing == nil {
 		cfg.Tracing = &config.Tracing{}
 	}
+
+	if cfg.TokenManager == nil && cfg.Commons != nil && cfg.Commons.TokenManager != nil {
+		cfg.TokenManager = &config.TokenManager{
+			JWTSecret: cfg.Commons.TokenManager.JWTSecret,
+		}
+	} else if cfg.TokenManager == nil {
+		cfg.TokenManager = &config.TokenManager{}
+	}
 }
 
 // Sanitize sanitized the configuration

services/collaboration/pkg/config/http.go

@@ -6,9 +6,7 @@ import (
 
 // HTTP defines the available http configuration.
 type HTTP struct {
-	Addr      string                `yaml:"addr" env:"COLLABORATION_HTTP_ADDR" desc:"The external address of the collaboration service wihout a leading scheme. Either use an IP address or a hostname (127.0.0.1:9301 or wopi.private.prv). The configured 'Scheme' in another envvar will be used to finally build the public URL along with this address." introductionVersion:"5.1"`
-	BindAddr  string                `yaml:"bindaddr" env:"COLLABORATION_HTTP_BINDADDR" desc:"The bind address of the HTTP service. Use '<ip-address>:<port>', for example, '127.0.0.1:9301' or '0.0.0.0:9301'." introductionVersion:"5.1"`
+	Addr      string                `yaml:"addr" env:"COLLABORATION_HTTP_ADDR" desc:"The bind address of the HTTP service." introductionVersion:"5.1"`
 	Namespace string                `yaml:"-"`
-	Scheme    string                `yaml:"scheme" env:"COLLABORATION_HTTP_SCHEME" desc:"The scheme to use for the HTTP address, which is either 'http' or 'https'." introductionVersion:"5.1"`
 	TLS       shared.HTTPServiceTLS `yaml:"tls"`
 }

services/collaboration/pkg/config/parser/parse.go

@@ -4,10 +4,10 @@ import (
 	"errors"
 
 	ociscfg "github.com/owncloud/ocis/v2/ocis-pkg/config"
+	"github.com/owncloud/ocis/v2/ocis-pkg/config/envdecode"
+	"github.com/owncloud/ocis/v2/ocis-pkg/shared"
 	"github.com/owncloud/ocis/v2/services/collaboration/pkg/config"
 	"github.com/owncloud/ocis/v2/services/collaboration/pkg/config/defaults"
-
-	"github.com/owncloud/ocis/v2/ocis-pkg/config/envdecode"
 )
 
 // ParseConfig loads configuration from known paths.
@@ -34,5 +34,11 @@ func ParseConfig(cfg *config.Config) error {
 
 // Validate validates the configuration
 func Validate(cfg *config.Config) error {
+	if cfg.TokenManager.JWTSecret == "" {
+		return shared.MissingJWTTokenError(cfg.Service.Name)
+	}
+	if cfg.WopiApp.Secret == "" {
+		return shared.MissingWOPISecretError(cfg.Service.Name)
+	}
 	return nil
 }

services/collaboration/pkg/config/reva.go

@@ -0,0 +1,6 @@
+package config
+
+// TokenManager is the config for using the reva token manager
+type TokenManager struct {
+	JWTSecret string `yaml:"jwt_secret" env:"OCIS_JWT_SECRET;COLLABORATION_JWT_SECRET" desc:"The secret to mint and validate jwt tokens." introductionVersion:"pre5.0"`
+}

services/collaboration/pkg/config/wopiapp.go

@@ -4,4 +4,6 @@ package config
 type WopiApp struct {
 	Addr     string `yaml:"addr" env:"COLLABORATION_WOPIAPP_ADDR" desc:"The URL where the WOPI app is located, such as https://127.0.0.1:8080." introductionVersion:"5.1"`
 	Insecure bool   `yaml:"insecure" env:"COLLABORATION_WOPIAPP_INSECURE" desc:"Skip TLS certificate verification when connecting to the WOPI app" introductionVersion:"5.1"`
+	WopiSrc  string `yaml:"wopisrc" env:"COLLABORATION_WOPIAPP_WOPISRC" desc:"The WOPISrc base URL containing schema, host and port. Path will be set to /wopi/files/{fileid} if not given. {fileid} will be replaced with the WOPI file id. Set this to the schema and domain where the collaboration service is reachable for the wopi app, such as https://office.owncloud.test." introductionVersion:"5.1"`
+	Secret   string `yaml:"secret" env:"COLLABORATION_WOPIAPP_SECRET" desc:"Used to mint and verify WOPI JWT tokens and encrypt and decrypt the REVA JWT token embedded in the WOPI JWT token." introductionVersion:"5.1"`
 }

services/collaboration/pkg/helpers/registration.go

@@ -11,6 +11,7 @@ import (
 	"github.com/gofrs/uuid"
 	"github.com/owncloud/ocis/v2/ocis-pkg/log"
 	"github.com/owncloud/ocis/v2/ocis-pkg/registry"
+	"github.com/owncloud/ocis/v2/ocis-pkg/version"
 	"github.com/owncloud/ocis/v2/services/collaboration/pkg/config"
 )
 
@@ -18,7 +19,7 @@ import (
 // There are no explicit requirements for the context, and it will be passed
 // without changes to the underlying RegisterService method.
 func RegisterOcisService(ctx context.Context, cfg *config.Config, logger log.Logger) error {
-	svc := registry.BuildGRPCService(cfg.GRPC.Namespace+"."+cfg.Service.Name, uuid.Must(uuid.NewV4()).String(), cfg.GRPC.Addr, "0.0.0")
+	svc := registry.BuildGRPCService(cfg.GRPC.Namespace+"."+cfg.Service.Name, uuid.Must(uuid.NewV4()).String(), cfg.GRPC.Addr, version.GetString())
 	return registry.RegisterService(ctx, svc, logger)
 }
 

services/collaboration/pkg/server/http/server.go

@@ -25,10 +25,11 @@ func Server(opts ...Option) (http.Service, error) {
 		http.TLSConfig(options.Config.HTTP.TLS),
 		http.Logger(options.Logger),
 		http.Namespace(options.Config.HTTP.Namespace),
-		http.Name("wopi"),
+		http.Name(options.Config.Service.Name),
 		http.Version(version.GetString()),
-		http.Address(options.Config.HTTP.BindAddr),
+		http.Address(options.Config.HTTP.Addr),
 		http.Context(options.Context),
+		http.TraceProvider(options.TracerProvider),
 	)
 	if err != nil {
 		options.Logger.Error().
@@ -40,7 +41,7 @@ func Server(opts ...Option) (http.Service, error) {
 	middlewares := []func(stdhttp.Handler) stdhttp.Handler{
 		chimiddleware.RequestID,
 		middleware.Version(
-			"userlog",
+			options.Config.Service.Name,
 			version.GetString(),
 		),
 		middleware.Logger(
@@ -48,7 +49,7 @@ func Server(opts ...Option) (http.Service, error) {
 		),
 		middleware.ExtractAccountUUID(
 			account.Logger(options.Logger),
-			account.JWTSecret(options.Config.JWTSecret), // previously, secret came from Config.TokenManager.JWTSecret
+			account.JWTSecret(options.Config.TokenManager.JWTSecret),
 		),
 		/*
 			// Need CORS? not in the original server
@@ -68,7 +69,7 @@ func Server(opts ...Option) (http.Service, error) {
 
 	mux.Use(
 		otelchi.Middleware(
-			"collaboration",
+			options.Config.Service.Name,
 			otelchi.WithChiRoutes(mux),
 			otelchi.WithTracerProvider(options.TracerProvider),
 			otelchi.WithPropagators(tracing.GetPropagator()),
@@ -77,6 +78,11 @@ func Server(opts ...Option) (http.Service, error) {
 
 	prepareRoutes(mux, options)
 
+	_ = chi.Walk(mux, func(method string, route string, handler stdhttp.Handler, middlewares ...func(stdhttp.Handler) stdhttp.Handler) error {
+		options.Logger.Debug().Str("method", method).Str("route", route).Int("middlewares", len(middlewares)).Msg("serving endpoint")
+		return nil
+	})
+
 	if err := micro.RegisterHandler(service.Server(), mux); err != nil {
 		return http.Service{}, err
 	}
@@ -110,7 +116,7 @@ func prepareRoutes(r *chi.Mux, options Options) {
 
 			r.Use(func(h stdhttp.Handler) stdhttp.Handler {
 				// authentication and wopi context
-				return colabmiddleware.WopiContextAuthMiddleware(options.Config.JWTSecret, h)
+				return colabmiddleware.WopiContextAuthMiddleware(options.Config.WopiApp.Secret, h)
 			},
 			)
 

services/collaboration/pkg/service/grpc/v0/service.go

@@ -8,6 +8,7 @@ import (
 	"net/url"
 	"path"
 	"strconv"
+	"strings"
 
 	appproviderv1beta1 "github.com/cs3org/go-cs3apis/cs3/app/provider/v1beta1"
 	gatewayv1beta1 "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1"
@@ -123,10 +124,12 @@ func (s *Service) OpenInApp(
 		viewAppURL = editAppURL
 	}
 
-	wopiSrcURL := url.URL{
-		Scheme: s.config.HTTP.Scheme,
-		Host:   s.config.HTTP.Addr,
-		Path:   path.Join("wopi", "files", fileRef),
+	wopiSrcURL, err := url.Parse(strings.Replace(s.config.WopiApp.WopiSrc, "{fileid}", fileRef, 1))
+	if err != nil {
+		return nil, err
+	}
+	if wopiSrcURL.Path == "" {
+		wopiSrcURL.Path = path.Join("wopi", "files", fileRef)
 	}
 
 	addWopiSrcQueryParam := func(baseURL string) (string, error) {
@@ -169,7 +172,7 @@ func (s *Service) OpenInApp(
 		appURL = editAppURL
 	}
 
-	cryptedReqAccessToken, err := middleware.EncryptAES([]byte(s.config.JWTSecret), req.GetAccessToken())
+	cryptedReqAccessToken, err := middleware.EncryptAES([]byte(s.config.WopiApp.Secret), req.GetAccessToken())
 	if err != nil {
 		s.logger.Error().
 			Err(err).
@@ -184,7 +187,7 @@ func (s *Service) OpenInApp(
 
 	wopiContext := middleware.WopiContext{
 		AccessToken:   cryptedReqAccessToken,
-		ViewOnlyToken: utils.ReadPlainFromOpaque(req.Opaque, "viewOnlyToken"),
+		ViewOnlyToken: utils.ReadPlainFromOpaque(req.GetOpaque(), "viewOnlyToken"),
 		FileReference: providerFileRef,
 		User:          user,
 		ViewMode:      req.GetViewMode(),
@@ -213,7 +216,7 @@ func (s *Service) OpenInApp(
 	}
 
 	token := jwt.NewWithClaims(jwt.SigningMethodHS256, claims)
-	accessToken, err := token.SignedString([]byte(s.config.JWTSecret))
+	accessToken, err := token.SignedString([]byte(s.config.WopiApp.Secret))
 
 	if err != nil {
 		s.logger.Error().

services/collaboration/pkg/service/grpc/v0/service_suite_test.go

@@ -7,7 +7,7 @@ import (
 	. "github.com/onsi/gomega"
 )
 
-func TestGraph(t *testing.T) {
+func TestService(t *testing.T) {
 	RegisterFailHandler(Fail)
 	RunSpecs(t, "Service Suite")
 }

services/collaboration/pkg/service/grpc/v0/service_test.go

@@ -103,8 +103,7 @@ var _ = Describe("Discovery", func() {
 		It("Invalid access token", func() {
 			ctx := context.Background()
 
-			cfg.HTTP.Addr = "wopiserver.test.prv"
-			cfg.HTTP.Scheme = "https"
+			cfg.WopiApp.WopiSrc = "https://wopiserver.test.prv"
 
 			req := &appproviderv1beta1.OpenInAppRequest{
 				ResourceInfo: &providerv1beta1.ResourceInfo{
@@ -140,9 +139,8 @@ var _ = Describe("Discovery", func() {
 			ctx := context.Background()
 			nowTime := time.Now()
 
-			cfg.HTTP.Addr = "wopiserver.test.prv"
-			cfg.HTTP.Scheme = "https"
-			cfg.JWTSecret = "my_supa_secret"
+			cfg.WopiApp.WopiSrc = "https://wopiserver.test.prv"
+			cfg.WopiApp.Secret = "my_supa_secret"
 
 			myself := &userv1beta1.User{
 				Id: &userv1beta1.UserId{
@@ -163,7 +161,7 @@ var _ = Describe("Discovery", func() {
 					Path: "/path/to/file.docx",
 				},
 				ViewMode:    appproviderv1beta1.ViewMode_VIEW_MODE_READ_WRITE,
-				AccessToken: MintToken(myself, cfg.JWTSecret, nowTime),
+				AccessToken: MintToken(myself, cfg.WopiApp.Secret, nowTime),
 			}
 
 			gatewayClient.On("WhoAmI", mock.Anything, mock.Anything).Times(1).Return(&gatewayv1beta1.WhoAmIResponse{