From bb1540fc4537ffa6810ba12763af610b98123f3f Mon Sep 17 00:00:00 2001 From: Florian Schade Date: Tue, 10 Jan 2023 15:42:13 +0100 Subject: [PATCH] [full-ci] add identitySet displayName property to the group and user sets (#5347) * add identitySet displayName property to the group and user sets --- .../enhancement-identity-set-display-name.md | 7 +++ go.mod | 1 + go.sum | 2 + services/graph/pkg/config/config.go | 2 + .../pkg/config/defaults/defaultconfig.go | 11 +++++ services/graph/pkg/server/http/server.go | 5 +- .../graph/pkg/service/v0/driveitems_test.go | 2 +- services/graph/pkg/service/v0/drives.go | 34 +++++++++++--- .../pkg/service/v0/educationschools_test.go | 2 +- .../pkg/service/v0/educationuser_test.go | 2 +- services/graph/pkg/service/v0/graph.go | 7 ++- services/graph/pkg/service/v0/graph_test.go | 2 +- services/graph/pkg/service/v0/groups_test.go | 2 +- services/graph/pkg/service/v0/instrument.go | 2 +- services/graph/pkg/service/v0/logging.go | 2 +- .../graph/pkg/service/v0/password_test.go | 2 +- services/graph/pkg/service/v0/service.go | 46 +++++++++++++++---- services/graph/pkg/service/v0/tracing.go | 2 +- services/graph/pkg/service/v0/users_test.go | 2 +- 19 files changed, 104 insertions(+), 31 deletions(-) create mode 100644 changelog/unreleased/enhancement-identity-set-display-name.md diff --git a/changelog/unreleased/enhancement-identity-set-display-name.md b/changelog/unreleased/enhancement-identity-set-display-name.md new file mode 100644 index 000000000..6c9c13ddf --- /dev/null +++ b/changelog/unreleased/enhancement-identity-set-display-name.md @@ -0,0 +1,7 @@ +Enhancement: Graph Drives IdentitySet displayName + +We've added the IdentitySet displayName property to the group and user sets for the graph drives endpoint. +The values for groups and users get cached. + +https://github.com/owncloud/ocis/pull/5347 +https://github.com/owncloud/web/pull/8178 diff --git a/go.mod b/go.mod index 3778861b4..429b843e4 100644 --- a/go.mod +++ b/go.mod @@ -42,6 +42,7 @@ require ( github.com/gorilla/mux v1.8.0 github.com/grpc-ecosystem/grpc-gateway/v2 v2.13.0 github.com/jellydator/ttlcache/v2 v2.11.1 + github.com/jellydator/ttlcache/v3 v3.0.1 github.com/justinas/alice v1.2.0 github.com/libregraph/idm v0.3.1-0.20220808071235-17bb032176de github.com/libregraph/lico v0.54.1-0.20220325072321-31efc3995d63 diff --git a/go.sum b/go.sum index c19a0e297..4a009167d 100644 --- a/go.sum +++ b/go.sum @@ -823,6 +823,8 @@ github.com/jbenet/go-context v0.0.0-20150711004518-d14ea06fba99 h1:BQSFePA1RWJOl github.com/jbenet/go-context v0.0.0-20150711004518-d14ea06fba99/go.mod h1:1lJo3i6rXxKeerYnT8Nvf0QmHCRC1n8sfWVwXF2Frvo= github.com/jellydator/ttlcache/v2 v2.11.1 h1:AZGME43Eh2Vv3giG6GeqeLeFXxwxn1/qHItqWZl6U64= github.com/jellydator/ttlcache/v2 v2.11.1/go.mod h1:RtE5Snf0/57e+2cLWFYWCCsLas2Hy3c5Z4n14XmSvTI= +github.com/jellydator/ttlcache/v3 v3.0.1 h1:cHgCSMS7TdQcoprXnWUptJZzyFsqs18Lt8VVhRuZYVU= +github.com/jellydator/ttlcache/v3 v3.0.1/go.mod h1:WwTaEmcXQ3MTjOm4bsZoDFiCu/hMvNWLO1w67RXz6h4= github.com/jessevdk/go-flags v1.4.0/go.mod h1:4FA24M0QyGHXBuZZK/XkWh8h0e1EYbRYJSGM75WSRxI= github.com/jessevdk/go-flags v1.5.0/go.mod h1:Fw0T6WPc1dYxT4mKEZRfG5kJhaTDP9pj1c2EWnYs/m4= github.com/jhump/protoreflect v1.6.0 h1:h5jfMVslIg6l29nsMs0D8Wj17RDVdNYti0vDN/PZZoE= diff --git a/services/graph/pkg/config/config.go b/services/graph/pkg/config/config.go index 5fa9843a3..b3618c72e 100644 --- a/services/graph/pkg/config/config.go +++ b/services/graph/pkg/config/config.go @@ -37,6 +37,8 @@ type Spaces struct { WebDavPath string `yaml:"webdav_path" env:"GRAPH_SPACES_WEBDAV_PATH" desc:"The WebDAV subpath for spaces."` DefaultQuota string `yaml:"default_quota" env:"GRAPH_SPACES_DEFAULT_QUOTA" desc:"The default quota in bytes."` ExtendedSpacePropertiesCacheTTL int `yaml:"extended_space_properties_cache_ttl" env:"GRAPH_SPACES_EXTENDED_SPACE_PROPERTIES_CACHE_TTL" desc:"Max TTL in seconds for the spaces property cache."` + UsersCacheTTL int `yaml:"users_cache_ttl" env:"GRAPH_SPACES_USERS_CACHE_TTL" desc:"Max TTL in seconds for the spaces users cache."` + GroupsCacheTTL int `yaml:"groups_cache_ttl" env:"GRAPH_SPACES_GROUPS_CACHE_TTL" desc:"Max TTL in seconds for the spaces groups cache."` } type LDAP struct { diff --git a/services/graph/pkg/config/defaults/defaultconfig.go b/services/graph/pkg/config/defaults/defaultconfig.go index 5f4d15cdb..1d06dad3f 100644 --- a/services/graph/pkg/config/defaults/defaultconfig.go +++ b/services/graph/pkg/config/defaults/defaultconfig.go @@ -3,6 +3,7 @@ package defaults import ( "path" "strings" + "time" "github.com/owncloud/ocis/v2/ocis-pkg/config/defaults" "github.com/owncloud/ocis/v2/ocis-pkg/shared" @@ -38,6 +39,10 @@ func DefaultConfig() *config.Config { WebDavBase: "https://localhost:9200", WebDavPath: "/dav/spaces/", DefaultQuota: "1000000000", + // 30 minutes + GroupsCacheTTL: 1800, + // 30 minutes + UsersCacheTTL: 1800, }, Identity: config.Identity{ Backend: "ldap", @@ -136,4 +141,10 @@ func Sanitize(cfg *config.Config) { if cfg.HTTP.Root != "/" { cfg.HTTP.Root = strings.TrimSuffix(cfg.HTTP.Root, "/") } + + // convert ttl to millisecond + // the config is in seconds, therefore we need multiply it. + cfg.Spaces.ExtendedSpacePropertiesCacheTTL = cfg.Spaces.ExtendedSpacePropertiesCacheTTL * int(time.Second) + cfg.Spaces.GroupsCacheTTL = cfg.Spaces.GroupsCacheTTL * int(time.Second) + cfg.Spaces.UsersCacheTTL = cfg.Spaces.UsersCacheTTL * int(time.Second) } diff --git a/services/graph/pkg/server/http/server.go b/services/graph/pkg/server/http/server.go index 89070cd11..02a7f177a 100644 --- a/services/graph/pkg/server/http/server.go +++ b/services/graph/pkg/server/http/server.go @@ -122,7 +122,8 @@ func Server(opts ...Option) (http.Service, error) { // no gatewayclient needed } - handle := svc.NewService( + var handle svc.Service + handle, err = svc.NewService( svc.Logger(options.Logger), svc.Config(options.Config), svc.Middleware(middlewares...), @@ -133,7 +134,7 @@ func Server(opts ...Option) (http.Service, error) { svc.WithSearchService(searchsvc.NewSearchProviderService("com.owncloud.api.search", grpc.DefaultClient())), ) - if handle == nil { + if err != nil { return http.Service{}, errors.New("could not initialize graph service") } diff --git a/services/graph/pkg/service/v0/driveitems_test.go b/services/graph/pkg/service/v0/driveitems_test.go index c29952ee0..2feb50ae5 100644 --- a/services/graph/pkg/service/v0/driveitems_test.go +++ b/services/graph/pkg/service/v0/driveitems_test.go @@ -64,7 +64,7 @@ var _ = Describe("Driveitems", func() { cfg.GRPCClientTLS = &shared.GRPCClientTLS{} _ = ogrpc.Configure(ogrpc.GetClientOptions(cfg.GRPCClientTLS)...) - svc = service.NewService( + svc, _ = service.NewService( service.Config(cfg), service.WithGatewayClient(gatewayClient), service.EventsPublisher(&eventsPublisher), diff --git a/services/graph/pkg/service/v0/drives.go b/services/graph/pkg/service/v0/drives.go index 15379c68d..c106d2ed7 100644 --- a/services/graph/pkg/service/v0/drives.go +++ b/services/graph/pkg/service/v0/drives.go @@ -23,6 +23,7 @@ import ( "github.com/cs3org/reva/v2/pkg/utils" "github.com/go-chi/chi/v5" "github.com/go-chi/render" + "github.com/jellydator/ttlcache/v3" libregraph "github.com/owncloud/libre-graph-api-go" "github.com/owncloud/ocis/v2/ocis-pkg/service/grpc" v0 "github.com/owncloud/ocis/v2/protogen/gen/ocis/messages/settings/v0" @@ -575,25 +576,44 @@ func (g Graph) cs3StorageSpaceToDrive(ctx context.Context, baseURL *url.URL, spa // libregraph.Identity and if we pass the pointer from the loop every identity // will have the same id. tmp := id - var identity libregraph.IdentitySet + var identitySet libregraph.IdentitySet + if _, ok := groupsMap[id]; ok { + var group libregraph.Group + if item := g.groupsCache.Get(id); item == nil { + if requestedGroup, err := g.identityBackend.GetGroup(ctx, id, url.Values{}); err == nil { + group = *requestedGroup + g.groupsCache.Set(id, group, ttlcache.DefaultTTL) + } + } else { + group = item.Value() + } - if _, ok := groupsMap[id]; !ok { - identity = libregraph.IdentitySet{User: &libregraph.Identity{Id: &tmp}} + identitySet = libregraph.IdentitySet{Group: &libregraph.Identity{Id: &tmp, DisplayName: group.GetDisplayName()}} } else { - identity = libregraph.IdentitySet{Group: &libregraph.Identity{Id: &tmp}} + var user libregraph.User + if item := g.usersCache.Get(id); item == nil { + if requestedUser, err := g.identityBackend.GetUser(ctx, id, url.Values{}); err == nil { + user = *requestedUser + g.usersCache.Set(id, user, ttlcache.DefaultTTL) + } + } else { + user = item.Value() + } + + identitySet = libregraph.IdentitySet{User: &libregraph.Identity{Id: &tmp, DisplayName: user.GetDisplayName()}} } // we need to map the permissions to the roles switch { // having RemoveGrant qualifies you as a manager case perm.RemoveGrant: - managerIdentities = append(managerIdentities, identity) + managerIdentities = append(managerIdentities, identitySet) // InitiateFileUpload means you are an editor case perm.InitiateFileUpload: - editorIdentities = append(editorIdentities, identity) + editorIdentities = append(editorIdentities, identitySet) // Stat permission at least makes you a viewer case perm.Stat: - viewerIdentities = append(viewerIdentities, identity) + viewerIdentities = append(viewerIdentities, identitySet) } } diff --git a/services/graph/pkg/service/v0/educationschools_test.go b/services/graph/pkg/service/v0/educationschools_test.go index b94878cc3..262f73686 100644 --- a/services/graph/pkg/service/v0/educationschools_test.go +++ b/services/graph/pkg/service/v0/educationschools_test.go @@ -65,7 +65,7 @@ var _ = Describe("Schools", func() { cfg.GRPCClientTLS = &shared.GRPCClientTLS{} _ = ogrpc.Configure(ogrpc.GetClientOptions(cfg.GRPCClientTLS)...) - svc = service.NewService( + svc, _ = service.NewService( service.Config(cfg), service.WithGatewayClient(gatewayClient), service.WithIdentityEducationBackend(identityEducationBackend), diff --git a/services/graph/pkg/service/v0/educationuser_test.go b/services/graph/pkg/service/v0/educationuser_test.go index 98ee06809..adc45de6e 100644 --- a/services/graph/pkg/service/v0/educationuser_test.go +++ b/services/graph/pkg/service/v0/educationuser_test.go @@ -69,7 +69,7 @@ var _ = Describe("EducationUsers", func() { cfg.GRPCClientTLS = &shared.GRPCClientTLS{} _ = ogrpc.Configure(ogrpc.GetClientOptions(cfg.GRPCClientTLS)...) - svc = service.NewService( + svc, _ = service.NewService( service.Config(cfg), service.WithGatewayClient(gatewayClient), service.EventsPublisher(&eventsPublisher), diff --git a/services/graph/pkg/service/v0/graph.go b/services/graph/pkg/service/v0/graph.go index db97c56f8..6978317db 100644 --- a/services/graph/pkg/service/v0/graph.go +++ b/services/graph/pkg/service/v0/graph.go @@ -12,7 +12,8 @@ import ( provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" "github.com/cs3org/reva/v2/pkg/events" "github.com/go-chi/chi/v5" - "github.com/jellydator/ttlcache/v2" + "github.com/jellydator/ttlcache/v3" + libregraph "github.com/owncloud/libre-graph-api-go" "github.com/owncloud/ocis/v2/ocis-pkg/log" searchsvc "github.com/owncloud/ocis/v2/protogen/gen/ocis/services/search/v0" settingssvc "github.com/owncloud/ocis/v2/protogen/gen/ocis/services/settings/v0" @@ -100,7 +101,9 @@ type Graph struct { gatewayClient GatewayClient roleService RoleService permissionsService Permissions - spacePropertiesCache *ttlcache.Cache + spacePropertiesCache *ttlcache.Cache[string, interface{}] + usersCache *ttlcache.Cache[string, libregraph.User] + groupsCache *ttlcache.Cache[string, libregraph.Group] eventsPublisher events.Publisher searchService searchsvc.SearchProviderService } diff --git a/services/graph/pkg/service/v0/graph_test.go b/services/graph/pkg/service/v0/graph_test.go index f5cf00909..9a5cc34c6 100644 --- a/services/graph/pkg/service/v0/graph_test.go +++ b/services/graph/pkg/service/v0/graph_test.go @@ -67,7 +67,7 @@ var _ = Describe("Graph", func() { gatewayClient = &mocks.GatewayClient{} eventsPublisher = mocks.Publisher{} permissionService = mocks.Permissions{} - svc = service.NewService( + svc, _ = service.NewService( service.Config(cfg), service.WithGatewayClient(gatewayClient), service.EventsPublisher(&eventsPublisher), diff --git a/services/graph/pkg/service/v0/groups_test.go b/services/graph/pkg/service/v0/groups_test.go index d5ada43d1..10c341299 100644 --- a/services/graph/pkg/service/v0/groups_test.go +++ b/services/graph/pkg/service/v0/groups_test.go @@ -70,7 +70,7 @@ var _ = Describe("Groups", func() { cfg.GRPCClientTLS = &shared.GRPCClientTLS{} _ = ogrpc.Configure(ogrpc.GetClientOptions(cfg.GRPCClientTLS)...) - svc = service.NewService( + svc, _ = service.NewService( service.Config(cfg), service.WithGatewayClient(gatewayClient), service.EventsPublisher(&eventsPublisher), diff --git a/services/graph/pkg/service/v0/instrument.go b/services/graph/pkg/service/v0/instrument.go index c1567cd40..eafb980fe 100644 --- a/services/graph/pkg/service/v0/instrument.go +++ b/services/graph/pkg/service/v0/instrument.go @@ -7,7 +7,7 @@ import ( ) // NewInstrument returns a service that instruments metrics. -func NewInstrument(next Service, metrics *metrics.Metrics) Service { +func NewInstrument(next Service, metrics *metrics.Metrics) instrument { return instrument{ next: next, metrics: metrics, diff --git a/services/graph/pkg/service/v0/logging.go b/services/graph/pkg/service/v0/logging.go index b5c832b79..1e3369b6c 100644 --- a/services/graph/pkg/service/v0/logging.go +++ b/services/graph/pkg/service/v0/logging.go @@ -7,7 +7,7 @@ import ( ) // NewLogging returns a service that logs messages. -func NewLogging(next Service, logger log.Logger) Service { +func NewLogging(next Service, logger log.Logger) logging { return logging{ next: next, logger: logger, diff --git a/services/graph/pkg/service/v0/password_test.go b/services/graph/pkg/service/v0/password_test.go index 51b272804..0184b5e17 100644 --- a/services/graph/pkg/service/v0/password_test.go +++ b/services/graph/pkg/service/v0/password_test.go @@ -70,7 +70,7 @@ var _ = Describe("Users changing their own password", func() { Expect(err).To(BeNil()) eventsPublisher = mocks.Publisher{} - svc = service.NewService( + svc, _ = service.NewService( service.Config(cfg), service.WithGatewayClient(gatewayClient), service.WithIdentityBackend(identityBackend), diff --git a/services/graph/pkg/service/v0/service.go b/services/graph/pkg/service/v0/service.go index a4858d0f2..98ce2d1a2 100644 --- a/services/graph/pkg/service/v0/service.go +++ b/services/graph/pkg/service/v0/service.go @@ -3,14 +3,16 @@ package svc import ( "crypto/tls" "crypto/x509" + "fmt" "net/http" "os" "strconv" + "time" "github.com/go-chi/chi/v5" "github.com/go-chi/chi/v5/middleware" - "github.com/jellydator/ttlcache/v2" - + "github.com/jellydator/ttlcache/v3" + libregraph "github.com/owncloud/libre-graph-api-go" ocisldap "github.com/owncloud/ocis/v2/ocis-pkg/ldap" "github.com/owncloud/ocis/v2/ocis-pkg/roles" "github.com/owncloud/ocis/v2/ocis-pkg/service/grpc" @@ -76,17 +78,40 @@ type Service interface { } // NewService returns a service implementation for Service. -func NewService(opts ...Option) Service { +func NewService(opts ...Option) (Graph, error) { options := newOptions(opts...) m := chi.NewMux() m.Use(options.Middleware...) + spacePropertiesCache := ttlcache.New[string, interface{}]( + ttlcache.WithTTL[string, interface{}]( + time.Duration(options.Config.Spaces.ExtendedSpacePropertiesCacheTTL), + ), + ) + go spacePropertiesCache.Start() + + usersCache := ttlcache.New[string, libregraph.User]( + ttlcache.WithTTL[string, libregraph.User]( + time.Duration(options.Config.Spaces.UsersCacheTTL), + ), + ) + go usersCache.Start() + + groupsCache := ttlcache.New[string, libregraph.Group]( + ttlcache.WithTTL[string, libregraph.Group]( + time.Duration(options.Config.Spaces.GroupsCacheTTL), + ), + ) + go groupsCache.Start() + svc := Graph{ config: options.Config, mux: m, logger: &options.Logger, - spacePropertiesCache: ttlcache.NewCache(), + spacePropertiesCache: spacePropertiesCache, + usersCache: usersCache, + groupsCache: groupsCache, eventsPublisher: options.EventsPublisher, gatewayClient: options.GatewayClient, searchService: options.SearchService, @@ -129,11 +154,11 @@ func NewService(opts ...Option) Service { pemData, err := os.ReadFile(options.Config.Identity.LDAP.CACert) if err != nil { options.Logger.Error().Err(err).Msgf("Error initializing LDAP Backend") - return nil + return svc, err } if !certs.AppendCertsFromPEM(pemData) { options.Logger.Error().Msgf("Error initializing LDAP Backend. Adding CA cert failed") - return nil + return svc, err } tlsConf.RootCAs = certs } @@ -149,7 +174,7 @@ func NewService(opts ...Option) Service { lb, err := identity.NewLDAPBackend(conn, options.Config.Identity.LDAP, &options.Logger) if err != nil { options.Logger.Error().Msgf("Error initializing LDAP Backend: '%s'", err) - return nil + return svc, err } svc.identityBackend = lb if options.IdentityEducationBackend == nil { @@ -161,8 +186,9 @@ func NewService(opts ...Option) Service { } } default: - options.Logger.Error().Msgf("Unknown Identity Backend: '%s'", options.Config.Identity.Backend) - return nil + err := fmt.Errorf("Unknown Identity Backend: '%s'", options.Config.Identity.Backend) + options.Logger.Err(err) + return svc, err } } else { svc.identityBackend = options.IdentityBackend @@ -284,7 +310,7 @@ func NewService(opts ...Option) Service { }) }) - return svc + return svc, nil } // parseHeaderPurge parses the 'Purge' header. diff --git a/services/graph/pkg/service/v0/tracing.go b/services/graph/pkg/service/v0/tracing.go index 99daa0cb2..160289f74 100644 --- a/services/graph/pkg/service/v0/tracing.go +++ b/services/graph/pkg/service/v0/tracing.go @@ -5,7 +5,7 @@ import ( ) // NewTracing returns a service that instruments traces. -func NewTracing(next Service) Service { +func NewTracing(next Service) tracing { return tracing{ next: next, } diff --git a/services/graph/pkg/service/v0/users_test.go b/services/graph/pkg/service/v0/users_test.go index ddc98a3ab..070f547d8 100644 --- a/services/graph/pkg/service/v0/users_test.go +++ b/services/graph/pkg/service/v0/users_test.go @@ -69,7 +69,7 @@ var _ = Describe("Users", func() { cfg.GRPCClientTLS = &shared.GRPCClientTLS{} _ = ogrpc.Configure(ogrpc.GetClientOptions(cfg.GRPCClientTLS)...) - svc = service.NewService( + svc, _ = service.NewService( service.Config(cfg), service.WithGatewayClient(gatewayClient), service.EventsPublisher(&eventsPublisher),