From 695b70a19c6c10668ed2e47e8b43a7475f7385cb Mon Sep 17 00:00:00 2001 From: Hugo Gonzalez Labrador Date: Tue, 14 Sep 2021 08:28:11 +0200 Subject: [PATCH 1/7] Fix logic for default mapping --- internal/grpc/services/gateway/appprovider.go | 63 +++++++++++++++---- 1 file changed, 52 insertions(+), 11 deletions(-) diff --git a/internal/grpc/services/gateway/appprovider.go b/internal/grpc/services/gateway/appprovider.go index b834968ced..3a895a98a7 100644 --- a/internal/grpc/services/gateway/appprovider.go +++ b/internal/grpc/services/gateway/appprovider.go @@ -21,6 +21,7 @@ package gateway import ( "context" "crypto/tls" + "fmt" "net/url" "strings" @@ -221,17 +222,51 @@ func (s *svc) findAppProvider(ctx context.Context, ri *storageprovider.ResourceI return nil, err } + // when app is empty it means the user assumes a default behaviour. + // From a web perspective, means the user click on the file itself. + // Normally the file will get downloaded but if a suitable application exists + // the behaviour will change from download to open the file with the app. if app == "" { - // We need to get the default provider in case app is not set - // If the default isn't set as well, we'll return the first provider which matches the mimetype + // If app is empty means that we need to rely on "default" behaviour. + // We currently do not have user preferences implemented so the only default + // we can currently enforce is one configured by the system admins, the + // "system default". + // If a default is not set we raise an error rather that giving the user the first provider in the list + // as the list is built on init time and is not deterministic, giving the user different results on service + // reload. res, err := c.GetDefaultAppProviderForMimeType(ctx, ®istry.GetDefaultAppProviderForMimeTypeRequest{ MimeType: ri.MimeType, }) - if err == nil && res.Status.Code == rpc.Code_CODE_OK && res.Provider != nil { + if err != nil { + err = errors.Wrap(err, "gateway: error calling GetDefaultAppProviderForMimeType") + return nil, err + + } + + // we've found a provider + if res.Status.Code == rpc.Code_CODE_OK && res.Provider != nil { return res.Provider, nil } + + // we did not find a default provider + if res.Status.Code == rpc.Code_CODE_NOT_FOUND { + err := errtypes.NotFound(fmt.Sprintf("gateway: default app rovider for mime type:%s not found", ri.MimeType)) + return nil, err + } + + // response code is something else, bubble up error + // if a default is not set we abort as returning the first application available is not + // deterministic for the end-user as it depends on initialization order of the app approviders with the registry. + // It also provides a good hint to the system admin to configure the defaults accordingly. + err = errtypes.InternalError(fmt.Sprintf("gateway: unexpected grpc response status:%s when calling GetDefaultAppProviderForMimeType", res.Status)) + return nil, err } + // app has been forced and is set, we try to get an app provider that can satisfy it + // Note that we ask for the list of all available providers for a given resource + // even though we're only interested into the one set by the "app" parameter. + // A better call will be to issue a (to be added) GetAppProviderByName(app) method + // to just what we ask for. res, err := c.GetAppProviders(ctx, ®istry.GetAppProvidersRequest{ ResourceInfo: ri, }) @@ -239,6 +274,8 @@ func (s *svc) findAppProvider(ctx context.Context, ri *storageprovider.ResourceI err = errors.Wrap(err, "gateway: error calling GetAppProviders") return nil, err } + + // if the list of app providers is empty means we expect a CODE_NOT_FOUND in the response if res.Status.Code != rpc.Code_CODE_OK { if res.Status.Code == rpc.Code_CODE_NOT_FOUND { return nil, errtypes.NotFound("gateway: app provider not found for resource: " + ri.String()) @@ -246,17 +283,21 @@ func (s *svc) findAppProvider(ctx context.Context, ri *storageprovider.ResourceI return nil, errtypes.InternalError("gateway: error finding app providers") } - if app != "" { - for _, p := range res.Providers { - if p.Name == app { - return p, nil - } + // if we only have one app provider we verify that it matches the requested app name + if len(res.Providers) == 1 { + p := res.Providers[0] + if p.Name == app { + return p, nil } - return nil, errtypes.NotFound("gateway: app provider not found: " + app) + // we return error if we return the wrong app provider + err = errtypes.InternalError(fmt.Sprintf("gateway: user asked for app %q and we gave %q", app, p.Name)) + return nil, err } - // As a fallback, return the first provider in the list - return res.Providers[0], nil + // we should never arrive to the point of having more than one + // provider for the given "app" parameters sent by the user + return nil, errtypes.InternalError(fmt.Sprintf("gateway: user requested app %q and we provided %d applications", app, len(res.Providers))) + } func getGRPCConfig(opaque *typespb.Opaque) (bool, bool) { From 01d864516e2d934169149817fdd99ff914fce4b4 Mon Sep 17 00:00:00 2001 From: Gianmaria Del Monte Date: Mon, 20 Sep 2021 17:18:46 +0200 Subject: [PATCH 2/7] Override AppProvider supported mimetypes with the ones set in the config --- internal/grpc/services/appprovider/appprovider.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/internal/grpc/services/appprovider/appprovider.go b/internal/grpc/services/appprovider/appprovider.go index 8dd446d704..1d36c8b5bb 100644 --- a/internal/grpc/services/appprovider/appprovider.go +++ b/internal/grpc/services/appprovider/appprovider.go @@ -53,6 +53,7 @@ type config struct { Drivers map[string]map[string]interface{} `mapstructure:"drivers"` AppProviderURL string `mapstructure:"app_provider_url"` GatewaySvc string `mapstructure:"gatewaysvc"` + MimeTypes []string `mapstructure:"mime_types"` // define the mimetypes supported by the AppProvider } func (c *config) init() { @@ -106,6 +107,12 @@ func (s *service) registerProvider() { } pInfo.Address = s.conf.AppProviderURL + // Add the supported mime types from the configuration + if len(s.conf.MimeTypes) != 0 { + pInfo.MimeTypes = s.conf.MimeTypes + log.Debug().Msg("app provider: overridden mimetype") + } + client, err := pool.GetGatewayServiceClient(s.conf.GatewaySvc) if err != nil { log.Error().Err(err).Msgf("error registering app provider: could not get gateway client") From 891d5fd0dc33d3ada13a67e7da9a6fa5955e5857 Mon Sep 17 00:00:00 2001 From: Gianmaria Del Monte Date: Thu, 23 Sep 2021 10:44:31 +0200 Subject: [PATCH 3/7] Add configuration in the AppRegistry for MimeTypes --- pkg/app/registry/static/static.go | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/pkg/app/registry/static/static.go b/pkg/app/registry/static/static.go index 73275734af..4aae16e7c8 100644 --- a/pkg/app/registry/static/static.go +++ b/pkg/app/registry/static/static.go @@ -20,6 +20,7 @@ package static import ( "context" + "fmt" "strings" "sync" @@ -35,8 +36,16 @@ func init() { registry.Register("static", New) } +type mimeTypeConfig struct { + Extension string `mapstructure:"extension"` + Name string `mapstructure:"name"` + Description string `mapstructure:"description"` + Icon string `mapstructure:"icon"` +} + type config struct { Providers map[string]*registrypb.ProviderInfo `mapstructure:"providers"` + MimeTypes map[string]mimeTypeConfig `mapstructure:"mime_types"` } func (c *config) init() { @@ -64,6 +73,7 @@ type mimeTypeIndex struct { } type reg struct { + config *config providers map[string]*registrypb.ProviderInfo mimetypes map[string]*mimeTypeIndex // map the mime type to the addresses of the corresponding providers sync.RWMutex @@ -78,6 +88,7 @@ func New(m map[string]interface{}) (app.Registry, error) { c.init() newReg := reg{ + config: c, providers: c.Providers, mimetypes: make(map[string]*mimeTypeIndex), } @@ -161,13 +172,17 @@ func (b *reg) ListSupportedMimeTypes(ctx context.Context) ([]*registrypb.MimeTyp if _, ok := mtmap[m]; ok { mtmap[m].AppProviders = append(mtmap[m].AppProviders, &t) } else { + mime, ok := b.config.MimeTypes[m] + if !ok { + return nil, errtypes.NotFound(fmt.Sprintf("mimetype %s not found in the configuration", m)) + } mtmap[m] = ®istrypb.MimeTypeInfo{ MimeType: m, AppProviders: []*registrypb.ProviderInfo{&t}, - Ext: "", // TODO fetch from config - Name: "", - Description: "", - Icon: "", + Ext: mime.Extension, + Name: mime.Name, + Description: mime.Description, + Icon: mime.Icon, } res = append(res, mtmap[m]) } From 1386564604b7d3f83dca1c0c6d3c10e0ba3abff3 Mon Sep 17 00:00:00 2001 From: Gianmaria Del Monte Date: Thu, 23 Sep 2021 10:54:32 +0200 Subject: [PATCH 4/7] Add default AppProvider from the configuration --- pkg/app/registry/static/static.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/pkg/app/registry/static/static.go b/pkg/app/registry/static/static.go index 4aae16e7c8..19123fcd5d 100644 --- a/pkg/app/registry/static/static.go +++ b/pkg/app/registry/static/static.go @@ -41,6 +41,7 @@ type mimeTypeConfig struct { Name string `mapstructure:"name"` Description string `mapstructure:"description"` Icon string `mapstructure:"icon"` + DefaultApp string `mapstructure:"default_app"` } type config struct { @@ -100,7 +101,12 @@ func New(m map[string]interface{}) (app.Registry, error) { if ok { newReg.mimetypes[m].apps = append(newReg.mimetypes[m].apps, addr) } else { - newReg.mimetypes[m] = &mimeTypeIndex{apps: []string{addr}} + // set a default app provider if provided + mime, in := c.MimeTypes[m] + if !in { + return nil, errtypes.NotFound(fmt.Sprintf("mimetype %s not found in the configuration", m)) + } + newReg.mimetypes[m] = &mimeTypeIndex{apps: []string{addr}, defaultApp: mime.DefaultApp} } } } From 9b637531587711346df334bdb6138e69f7023af0 Mon Sep 17 00:00:00 2001 From: Gianmaria Del Monte Date: Thu, 23 Sep 2021 11:12:47 +0200 Subject: [PATCH 5/7] Add changelog --- changelog/unreleased/whitelist-apps.md | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 changelog/unreleased/whitelist-apps.md diff --git a/changelog/unreleased/whitelist-apps.md b/changelog/unreleased/whitelist-apps.md new file mode 100644 index 0000000000..4721a5403a --- /dev/null +++ b/changelog/unreleased/whitelist-apps.md @@ -0,0 +1,7 @@ +Enhancement: Whitelisting for apps + +AppProvider supported mime types are now overridden in its configuration. +A friendly name, a description, an extension, an icon and a default app, +can be configured in the AppRegistry for each mime type. + +https://github.com/cs3org/reva/pull/2095 From 8130f2ea1c2a1df58b731b94cd1b7f00edd5bfd5 Mon Sep 17 00:00:00 2001 From: Gianmaria Del Monte Date: Fri, 24 Sep 2021 09:47:21 +0200 Subject: [PATCH 6/7] Update AppRegistry tests with the mimetypes infos from the config --- .../services/appregistry/appregistry_test.go | 61 ++++++++++++++++++- 1 file changed, 59 insertions(+), 2 deletions(-) diff --git a/internal/grpc/services/appregistry/appregistry_test.go b/internal/grpc/services/appregistry/appregistry_test.go index bf9c330412..726ef0658a 100644 --- a/internal/grpc/services/appregistry/appregistry_test.go +++ b/internal/grpc/services/appregistry/appregistry_test.go @@ -40,6 +40,7 @@ func Test_ListAppProviders(t *testing.T) { tests := []struct { name string providers map[string]interface{} + mimeTypes map[string]map[string]string want *registrypb.ListAppProvidersResponse }{ { @@ -54,6 +55,20 @@ func Test_ListAppProviders(t *testing.T) { "mimetypes": []string{"currently/ignored"}, }, }, + mimeTypes: map[string]map[string]string{ + "text/json": { + "extension": "json", + "name": "JSON File", + "icon": "https://example.org/icons&file=json.png", + "default_app": "some Address", + }, + "currently/ignored": { + "extension": "unknown", + "name": "Ignored file", + "icon": "https://example.org/icons&file=unknown.png", + "default_app": "some Address", + }, + }, // only Status and Providers will be asserted in the tests want: ®istrypb.ListAppProvidersResponse{ @@ -77,6 +92,7 @@ func Test_ListAppProviders(t *testing.T) { { name: "providers is nil", providers: nil, + mimeTypes: nil, want: ®istrypb.ListAppProvidersResponse{ Status: &rpcv1beta1.Status{ Code: 1, @@ -93,6 +109,7 @@ func Test_ListAppProviders(t *testing.T) { { name: "empty providers", providers: map[string]interface{}{}, + mimeTypes: map[string]map[string]string{}, // only Status and Providers will be asserted in the tests want: ®istrypb.ListAppProvidersResponse{ @@ -114,6 +131,7 @@ func Test_ListAppProviders(t *testing.T) { providers: map[string]interface{}{ "some Address": nil, }, + mimeTypes: map[string]map[string]string{}, // only Status and Providers will be asserted in the tests want: ®istrypb.ListAppProvidersResponse{ @@ -129,7 +147,7 @@ func Test_ListAppProviders(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - rr, err := static.New(map[string]interface{}{"Providers": tt.providers}) + rr, err := static.New(map[string]interface{}{"providers": tt.providers, "mime_types": tt.mimeTypes}) if err != nil { t.Errorf("could not create registry error = %v", err) return @@ -168,6 +186,45 @@ func Test_GetAppProviders(t *testing.T) { }, } + mimeTypes := map[string]map[string]string{ + "text/json": { + "extension": "json", + "name": "JSON File", + "icon": "https://example.org/icons&file=json.png", + "default_app": "some Address", + }, + "text/xml": { + "extension": "xml", + "name": "XML File", + "icon": "https://example.org/icons&file=xml.png", + "default_app": "some Address", + }, + "application/vnd.openxmlformats-officedocument.wordprocessingml.document": { + "extension": "doc", + "name": "Word File", + "icon": "https://example.org/icons&file=doc.png", + "default_app": "some Address", + }, + "application/vnd.oasis.opendocument.presentation": { + "extension": "odf", + "name": "OpenDocument File", + "icon": "https://example.org/icons&file=odf.png", + "default_app": "some Address", + }, + "application/vnd.apple.installer+xml": { + "extension": "mpkg", + "name": "Mpkg File", + "icon": "https://example.org/icons&file=mpkg.png", + "default_app": "some Address", + }, + "image/bmp": { + "extension": "bmp", + "name": "Image File", + "icon": "https://example.org/icons&file=bmp.png", + "default_app": "some Address", + }, + } + tests := []struct { name string search *providerv1beta1.ResourceInfo @@ -258,7 +315,7 @@ func Test_GetAppProviders(t *testing.T) { }, } - rr, err := static.New(map[string]interface{}{"providers": providers}) + rr, err := static.New(map[string]interface{}{"providers": providers, "mime_types": mimeTypes}) if err != nil { t.Errorf("could not create registry error = %v", err) return From 182797f8a4093e7696d331357b71ca410d44a34a Mon Sep 17 00:00:00 2001 From: Gianmaria Del Monte Date: Mon, 27 Sep 2021 15:17:40 +0200 Subject: [PATCH 7/7] Add AppRegistry mime type examples and demo AppProvider --- examples/storage-references/gateway.toml | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/examples/storage-references/gateway.toml b/examples/storage-references/gateway.toml index 10e58e90bd..9185c95d67 100644 --- a/examples/storage-references/gateway.toml +++ b/examples/storage-references/gateway.toml @@ -30,6 +30,21 @@ appauth = "localhost:15000" [grpc.services.ocminvitemanager] [grpc.services.ocmproviderauthorizer] +[grpc.services.appregistry] +[grpc.services.appregistry.drivers.static.mime_types] +"application/vnd.openxmlformats-officedocument.wordprocessingml.document" = {"extension" = "docx", "name" = "Microsoft Word", "description" = "Microsoft Word document"} +"application/vnd.openxmlformats-officedocument.spreadsheetml.sheet" = {"extension" = "xlsx", "name" = "Microsoft Excel", "description" = "Microsoft Excel document"} +"application/vnd.openxmlformats-officedocument.presentationml.presentation" = {"extension" = "pptx", "name" = "Microsoft PowerPoint", "description" = "Microsoft PowerPoint document"} +"application/vnd.oasis.opendocument.text" = {"extension" = "odt", "name" = "OpenDocument", "description" = "OpenDocument text document"} +"application/vnd.oasis.opendocument.spreadsheet" = {"extension" = "ods", "name" = "OpenSpreadsheet", "description" = "OpenDocument spreadsheet document"} +"application/vnd.oasis.opendocument.presentation" = {"extension" = "odp", "name" = "OpenPresentation", "description" = "OpenDocument presentation document"} +"text/plain" = {"extension" = "txt", "name" = "Text file", "description" = "Text file"} +"text/markdown" = {"extension" = "md", "name" = "Markdown file", "description" = "Markdown file"} +"application/vnd.jupyter" = {"extension" = "ipynb", "name" = "Jupyter Notebook", "description" = "Jupyter Notebook"} + +[grpc.services.appprovider] +mime_types = ["text/plain"] + [http.services.datagateway] [http.services.prometheus] [http.services.ocmd]