From 116d521e04007b7610e3944d943aeaac4b3b0bfd Mon Sep 17 00:00:00 2001 From: Pascal Bleser Date: Tue, 11 Aug 2026 18:38:33 +0200 Subject: [PATCH 1/2] chore(graph): disable HTTP or eventhandlers by configuration In the scope of the broader issue #1312, this PR deals with performing those changes for the `graph` service, namely to add the ability to disable the HTTP API or to disable the events API handler by configuration. It also adds metrics for the events processing, and tests for the events processing. The previous implementation was combining the HTTP server service and the events consumption, which is why this PR refactors the composition of those services: * the event consumption has been moved into its own service * the identity.Backend is created beforehand, and then injected as a collaborator in both the HTTP service as well as the event consumer service It also adds metrics, mainly for the event processing. To encourage re-use in latter implementations and changes, it also introduces two top-level package changes: * internal/eventstest/events_test_helpers: contains a TestBus implementation to unit-test event consumers without NATS * internal/metricstest/metrics_test_helpers: contains assertion functions to test Prometheus metrics --- internal/eventstest/events_test_helpers.go | 44 +++++ internal/metricstest/metrics_test_helpers.go | 143 ++++++++++++++ services/graph/README.md | 21 ++- services/graph/pkg/command/server.go | 69 ++++++- services/graph/pkg/config/config.go | 14 ++ .../pkg/config/defaults/defaultconfig.go | 8 +- services/graph/pkg/config/http.go | 1 + services/graph/pkg/config/parser/parse.go | 8 + services/graph/pkg/identity/factory.go | 136 +++++++++++++ services/graph/pkg/identity/ldap.go | 2 +- services/graph/pkg/metrics/metrics.go | 50 ++++- services/graph/pkg/server/http/server.go | 24 +-- services/graph/pkg/service/events/service.go | 124 ++++++++++++ .../graph/pkg/service/events/service_test.go | 124 ++++++++++++ services/graph/pkg/service/v0/graph.go | 1 - services/graph/pkg/service/v0/option.go | 8 - services/graph/pkg/service/v0/service.go | 178 +----------------- 17 files changed, 739 insertions(+), 216 deletions(-) create mode 100644 internal/eventstest/events_test_helpers.go create mode 100644 internal/metricstest/metrics_test_helpers.go create mode 100644 services/graph/pkg/identity/factory.go create mode 100644 services/graph/pkg/service/events/service.go create mode 100644 services/graph/pkg/service/events/service_test.go diff --git a/internal/eventstest/events_test_helpers.go b/internal/eventstest/events_test_helpers.go new file mode 100644 index 0000000000..ce8ef22ad6 --- /dev/null +++ b/internal/eventstest/events_test_helpers.go @@ -0,0 +1,44 @@ +package eventstest + +import ( + "encoding/json" + "reflect" + + "github.com/google/uuid" + + rev "github.com/opencloud-eu/reva/v2/pkg/events" + microevents "go-micro.dev/v4/events" +) + +func NewTestBus() TestBus { + return TestBus(make(chan rev.Event)) +} + +type TestBus chan rev.Event + +func (tb TestBus) Consume(_ string, _ ...microevents.ConsumeOption) (<-chan microevents.Event, error) { + ch := make(chan microevents.Event) + go func() { + for ev := range tb { + b, _ := json.Marshal(ev.Event) + ch <- microevents.Event{ + Payload: b, + Metadata: map[string]string{ + rev.MetadatakeyEventID: ev.ID, + rev.MetadatakeyEventType: ev.Type, + }, + } + } + }() + return ch, nil +} + +func (tb TestBus) Publish(e any) string { + ev := rev.Event{ + ID: uuid.New().String(), + Type: reflect.TypeOf(e).String(), + Event: e, + } + tb <- ev + return ev.ID +} diff --git a/internal/metricstest/metrics_test_helpers.go b/internal/metricstest/metrics_test_helpers.go new file mode 100644 index 0000000000..2a47285cd0 --- /dev/null +++ b/internal/metricstest/metrics_test_helpers.go @@ -0,0 +1,143 @@ +package metricstest + +import ( + "fmt" + + "github.com/prometheus/client_golang/prometheus" + dto "github.com/prometheus/client_model/go" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// copied and adapted from Prometheus testutil.ToFloat64(), since we don't import that package +func collect(c prometheus.Collector) []prometheus.Metric { + result := []prometheus.Metric{} + ch := make(chan prometheus.Metric) + done := make(chan struct{}) + go func() { + for m := range ch { + result = append(result, m) + } + close(done) + }() + c.Collect(ch) + close(ch) + <-done + return result +} + +func RequireIsNotSet(t require.TestingT, c prometheus.Collector, msgAndArgs ...any) { + if h, ok := t.(interface{ Helper() }); ok { + h.Helper() + } + if !IsNotSet(t, c, msgAndArgs) { + t.FailNow() + } +} + +func IsNotSet(t assert.TestingT, c prometheus.Collector, msgAndArgs ...any) bool { + if h, ok := t.(interface{ Helper() }); ok { + h.Helper() + } + + m := collect(c) + if len(m) > 0 { + return assert.Fail(t, "Metric exists while expected to not exist", msgAndArgs) + } else { + return true + } +} + +func RequireEqual(t require.TestingT, expected float64, c prometheus.Collector, msgAndArgs ...any) { + if h, ok := t.(interface{ Helper() }); ok { + h.Helper() + } + if !Equal(t, expected, c, msgAndArgs) { + t.FailNow() + } +} + +// copied and adapted from Prometheus testutil.ToFloat64(), since we don't import that package +func Equal(t assert.TestingT, expected float64, c prometheus.Collector, msgAndArgs ...any) bool { + if h, ok := t.(interface{ Helper() }); ok { + h.Helper() + } + + m := collect(c) + if !assert.Len(t, m, 1, msgAndArgs...) { + return false + } + pb := &dto.Metric{} + err := m[0].Write(pb) + if !assert.NoError(t, err, msgAndArgs...) { + return false + } + if pb.Gauge != nil { + return assert.Equal(t, expected, pb.Gauge.GetValue(), msgAndArgs...) + } else if pb.Counter != nil { + return assert.Equal(t, expected, pb.Counter.GetValue(), msgAndArgs...) + } else if pb.Untyped != nil { + return assert.Equal(t, expected, pb.Untyped.GetValue(), msgAndArgs...) + } else { + return assert.Fail(t, fmt.Sprintf("collected a non-gauge/counter/untyped metric: %s", pb), msgAndArgs...) + } +} + +func RequireEqualWithLabels(t require.TestingT, expectedValue float64, expectedLabels map[string]string, c prometheus.Collector, msgAndArgs ...any) { + if h, ok := t.(interface{ Helper() }); ok { + h.Helper() + } + if !EqualWithLabels(t, expectedValue, expectedLabels, c, msgAndArgs) { + t.FailNow() + } +} + +func EqualWithLabels(t assert.TestingT, expectedValue float64, expectedLabels map[string]string, c prometheus.Collector, msgAndArgs ...any) bool { + if h, ok := t.(interface{ Helper() }); ok { + h.Helper() + } + + m := collect(c) + if !assert.Len(t, m, 1, "collected %d metrics instead of exactly 1", len(m)) { + return false + } + pb := &dto.Metric{} + err := m[0].Write(pb) + if !assert.NoError(t, err) { + return false + } + if pb.Gauge != nil { + if !assert.Equal(t, expectedValue, pb.Gauge.GetValue()) { + return false + } + } else if pb.Counter != nil { + if !assert.Equal(t, expectedValue, pb.Counter.GetValue()) { + return false + } + } else if pb.Untyped != nil { + if !assert.Equal(t, expectedValue, pb.Untyped.GetValue()) { + return false + } + } else { + return assert.Fail(t, "collected a non-gauge/counter/untyped metric: %s", pb) + } + + if !assert.NotNil(t, pb.Label) { + return false + } + actualLabels := map[string]string{} + for _, label := range pb.Label { + if !assert.NotNil(t, label) { + return false + } + if !assert.NotNil(t, label.Name) { + return false + } + if !assert.NotNil(t, label.Value) { + return false + } + actualLabels[*label.Name] = *label.Value + } + return assert.Equal(t, expectedLabels, actualLabels, msgAndArgs) +} diff --git a/services/graph/README.md b/services/graph/README.md index cbf84ebdd0..f6ea85fe3b 100644 --- a/services/graph/README.md +++ b/services/graph/README.md @@ -168,7 +168,7 @@ The output of this command includes the following information for each role: * `Condition` * `Allowed resource actions` -**Example output (shortned)** +**Example output (shortened)** ```bash +--------------------------------------+----------+--------------------------------+--------------------------------+------------------------------------------+ @@ -184,3 +184,22 @@ The output of this command includes the following information for each role: +--------------------------------------+----------+--------------------------------+--------------------------------+------------------------------------------+ ``` +## API Handlers + +To specialize `graph` service instances in order to scale them independently, it is possible to disable its API handlers: + +* `GRAPH_HTTP_DISABLE`: when set to `true`, the service does not listen on HTTP and only consumes events (defaults to `false`) +* `GRAPH_EVENTS_DISABLE_CONSUMER`: when set to `true`, the service does not consome events and only listens on HTTP (defaults to `false`) + +## Metrics + +The `graph` service provides the following metrics: + +| Name | Description | +| ---- | ----------- | +| `opencloud_graph_build_info{version=...}` | Contains a label `version` that is set to the current version of the service, and always has a value of `1` | +| `opencloud_graph_events_enabled` | Is set to `1` if the Events API handler is enabled, or `0` if not | +| `opencloud_graph_http_enabled` | Is set to `1` if the HTTP API handler is enabled, or `0` if not | +| `opencloud_graph_events{event=...,result=...}` | Counts the number of events that have been consumed, with a `event` label that contains the name of the event, and a `result` label that is set to `success` or `failure` | +| `opencloud_graph_events_invalid` | Counts the number of invalid events that are malformed or are missing required data | +| `opencloud_graph_events_unsupported` | Counts the numbef of consumed events that cannot be processes by this service, should always be `0` | diff --git a/services/graph/pkg/command/server.go b/services/graph/pkg/command/server.go index b8cdc7f977..3a581124ee 100644 --- a/services/graph/pkg/command/server.go +++ b/services/graph/pkg/command/server.go @@ -7,6 +7,7 @@ import ( "strings" "github.com/opencloud-eu/opencloud/pkg/config/configlog" + "github.com/opencloud-eu/opencloud/pkg/generators" "github.com/opencloud-eu/opencloud/pkg/log" natspkg "github.com/opencloud-eu/opencloud/pkg/nats" "github.com/opencloud-eu/opencloud/pkg/runner" @@ -14,9 +15,14 @@ import ( "github.com/opencloud-eu/opencloud/pkg/version" "github.com/opencloud-eu/opencloud/services/graph/pkg/config" "github.com/opencloud-eu/opencloud/services/graph/pkg/config/parser" + "github.com/opencloud-eu/opencloud/services/graph/pkg/identity" "github.com/opencloud-eu/opencloud/services/graph/pkg/metrics" "github.com/opencloud-eu/opencloud/services/graph/pkg/server/debug" "github.com/opencloud-eu/opencloud/services/graph/pkg/server/http" + evc "github.com/opencloud-eu/opencloud/services/graph/pkg/service/events" + "github.com/opencloud-eu/reva/v2/pkg/events" + "github.com/opencloud-eu/reva/v2/pkg/events/stream" + "github.com/prometheus/client_golang/prometheus" "github.com/nats-io/nats.go" "github.com/nats-io/nats.go/jetstream" @@ -46,7 +52,7 @@ func Server(cfg *config.Config) *cobra.Command { } ctx := cfg.Context - mtrcs := metrics.New() + mtrcs := metrics.New(prometheus.DefaultRegisterer) mtrcs.BuildInfo.WithLabelValues(version.GetString()).Set(1) var kv jetstream.KeyValue @@ -78,9 +84,37 @@ func Server(cfg *config.Config) *cobra.Command { } } + identityBackend, eduBackend, err := identity.CreateIdentityBackends( + cfg.Identity.Backend, + cfg, + &logger, + traceProvider, + ) + if err != nil { + logger.Error().Err(err).Msg("Error initializing the identity backend") + return fmt.Errorf("could not initialize identity backend: %w", err) + } + + var eventsStream events.Stream + if cfg.Events.Endpoint != "" { + var err error + connName := generators.GenerateConnectionName(cfg.Service.Name, generators.NTypeBus) + eventsStream, err = stream.NatsFromConfig(connName, false, cfg.Events.ToNatsConfig()) + if err != nil { + logger.Error().Err(err).Msg("Error initializing events publisher") + return fmt.Errorf("could not initialize events publisher: %w", err) + } + } + gr := runner.NewGroup() - { + + if !cfg.HTTP.Disabled { + mtrcs.HttpEnabled.Set(1) + server, err := http.Server( + identityBackend, + eduBackend, + eventsStream, http.Logger(logger), http.Context(ctx), http.Config(cfg), @@ -92,8 +126,37 @@ func Server(cfg *config.Config) *cobra.Command { logger.Error().Err(err).Str("transport", "http").Msg("Failed to initialize server") return err } - gr.Add(runner.NewGoMicroHttpServerRunner(cfg.Service.Name+".http", server)) + } else { + mtrcs.HttpEnabled.Set(0) + logger.Info().Str("transport", "http").Msg("HTTP server is disabled") + } + + if !cfg.Events.DisabledConsumer { + mtrcs.EventsEnabled.Set(1) + + // even if events are enabled, we still need to differentiate between whether this process + // show be consuming events or not (and even when that is disabled, we still need to be + // able to produce events), which is why this is a separate setting; + // for context, see https://github.com/opencloud-eu/opencloud/issues/1312 + + logger := &log.Logger{Logger: logger.With().Str("transport", "events").Logger()} + eventConsumer, err := evc.NewService(cfg.Context, eventsStream, identityBackend, mtrcs, logger) + if err != nil { + return fmt.Errorf("could not initialize events consumer: %w", err) + } + + gr.Add(runner.New(cfg.Service.Name+".svc", func() error { + return eventConsumer.Start() + }, func() { + err := eventConsumer.Close() + if err != nil { + logger.Error().Err(err).Msg("failed to stop event consumer") + } + })) + } else { + mtrcs.EventsEnabled.Set(0) + logger.Info().Str("transport", "events").Msg("event consumer is disabled") } { diff --git a/services/graph/pkg/config/config.go b/services/graph/pkg/config/config.go index 4f3ed16903..1c01c308cb 100644 --- a/services/graph/pkg/config/config.go +++ b/services/graph/pkg/config/config.go @@ -5,6 +5,7 @@ import ( "time" "github.com/opencloud-eu/opencloud/pkg/shared" + "github.com/opencloud-eu/reva/v2/pkg/events/stream" ) // Config combines all available configuration parts. @@ -129,6 +130,7 @@ type API struct { // Events combines the configuration options for the event bus. type Events struct { + DisabledConsumer bool `yaml:"disabled_consumer" env:"GRAPH_EVENTS_DISABLE_CONSUMER" desc:"Disables consuming events. Set this to true if the service should only handle HTTP requests." introductionVersion:"%NEXT%"` Endpoint string `yaml:"endpoint" env:"OC_EVENTS_ENDPOINT;GRAPH_EVENTS_ENDPOINT" desc:"The address of the event system. The event system is the message queuing service. It is used as message broker for the microservice architecture. Set to a empty string to disable emitting events." introductionVersion:"1.0.0"` Cluster string `yaml:"cluster" env:"OC_EVENTS_CLUSTER;GRAPH_EVENTS_CLUSTER" desc:"The clusterID of the event system. The event system is the message queuing service. It is used as message broker for the microservice architecture." introductionVersion:"1.0.0"` TLSInsecure bool `yaml:"tls_insecure" env:"OC_INSECURE;OC_EVENTS_TLS_INSECURE;GRAPH_EVENTS_TLS_INSECURE" desc:"Whether to verify the server TLS certificates." introductionVersion:"1.0.0"` @@ -138,6 +140,18 @@ type Events struct { AuthPassword string `yaml:"password" env:"OC_EVENTS_AUTH_PASSWORD;GRAPH_EVENTS_AUTH_PASSWORD" desc:"The password to authenticate with the events broker. The events broker is the OpenCloud service which receives and delivers events between the services." introductionVersion:"1.0.0"` } +func (e Events) ToNatsConfig() stream.NatsConfig { + return stream.NatsConfig{ + Endpoint: e.Endpoint, + Cluster: e.Cluster, + TLSInsecure: e.TLSInsecure, + TLSRootCACertificate: e.TLSRootCACertificate, + EnableTLS: e.EnableTLS, + AuthUsername: e.AuthUsername, + AuthPassword: e.AuthPassword, + } +} + // CORS defines the available cors configuration. type CORS struct { AllowedOrigins []string `yaml:"allow_origins" env:"OC_CORS_ALLOW_ORIGINS;GRAPH_CORS_ALLOW_ORIGINS" desc:"A list of allowed CORS origins. See following chapter for more details: *Access-Control-Allow-Origin* at https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Access-Control-Allow-Origin. See the Environment Variable Types description for more details." introductionVersion:"1.0.0"` diff --git a/services/graph/pkg/config/defaults/defaultconfig.go b/services/graph/pkg/config/defaults/defaultconfig.go index b8ad9376f3..7c3c5200f4 100644 --- a/services/graph/pkg/config/defaults/defaultconfig.go +++ b/services/graph/pkg/config/defaults/defaultconfig.go @@ -43,6 +43,7 @@ func DefaultConfig() *config.Config { Token: "", }, HTTP: config.HTTP{ + Disabled: false, Addr: "127.0.0.1:9120", Namespace: "eu.opencloud.web", Root: "/graph", @@ -118,9 +119,10 @@ func DefaultConfig() *config.Config { TTL: time.Hour * 24, }, Events: config.Events{ - Endpoint: "127.0.0.1:9233", - Cluster: "opencloud-cluster", - EnableTLS: false, + DisabledConsumer: false, + Endpoint: "127.0.0.1:9233", + Cluster: "opencloud-cluster", + EnableTLS: false, }, MaxConcurrency: 20, UnifiedRoles: config.UnifiedRoles{ diff --git a/services/graph/pkg/config/http.go b/services/graph/pkg/config/http.go index dca2a55cfd..4859fa69f0 100644 --- a/services/graph/pkg/config/http.go +++ b/services/graph/pkg/config/http.go @@ -4,6 +4,7 @@ import "github.com/opencloud-eu/opencloud/pkg/shared" // HTTP defines the available http configuration. type HTTP struct { + Disabled bool `yaml:"disabled" env:"GRAPH_HTTP_DISABLE" desc:"Disables the HTTP service. Set this to true if the service should only consume events." introductionVersion:"%NEXT%"` Addr string `yaml:"addr" env:"GRAPH_HTTP_ADDR" desc:"The bind address of the HTTP service." introductionVersion:"1.0.0"` Namespace string `yaml:"-"` Root string `yaml:"root" env:"GRAPH_HTTP_ROOT" desc:"Subdirectory that serves as the root for this HTTP service." introductionVersion:"1.0.0"` diff --git a/services/graph/pkg/config/parser/parse.go b/services/graph/pkg/config/parser/parse.go index eb400899b1..58f6a77173 100644 --- a/services/graph/pkg/config/parser/parse.go +++ b/services/graph/pkg/config/parser/parse.go @@ -39,6 +39,14 @@ func ParseConfig(cfg *config.Config) error { } func Validate(cfg *config.Config) error { + if cfg.HTTP.Disabled && cfg.Events.DisabledConsumer { + // might be debatable, but this situation should be treated as an error, + // as the process wouldn't be able to serve either API and would thus be + // completely useless -- in that case, just don't start this service + // in the first place (especially since it's optional) + return errors.New("both HTTP and events consumption APIs are disabled by configuration; at least one must be enabled") + } + if cfg.TokenManager.JWTSecret == "" { return shared.MissingJWTTokenError(cfg.Service.Name) } diff --git a/services/graph/pkg/identity/factory.go b/services/graph/pkg/identity/factory.go new file mode 100644 index 0000000000..52cc1f11ce --- /dev/null +++ b/services/graph/pkg/identity/factory.go @@ -0,0 +1,136 @@ +package identity + +import ( + "crypto/tls" + "crypto/x509" + "errors" + "fmt" + "os" + + ldapv3 "github.com/go-ldap/ldap/v3" + ocldap "github.com/opencloud-eu/opencloud/pkg/ldap" + "github.com/opencloud-eu/opencloud/pkg/log" + "github.com/opencloud-eu/opencloud/pkg/registry" + "github.com/opencloud-eu/opencloud/services/graph/pkg/config" + "github.com/opencloud-eu/reva/v2/pkg/rgrpc/todo/pool" + "github.com/opencloud-eu/reva/v2/pkg/utils/ldap" + "go.opentelemetry.io/otel/trace" +) + +func CreateIdentityBackends(name string, cfg *config.Config, logger *log.Logger, traceProvider trace.TracerProvider) (Backend, EducationBackend, error) { + switch name { + case "cs3": + gatewaySelector, err := pool.GatewaySelector( + cfg.Reva.Address, + append( + cfg.Reva.GetRevaOptions(), + pool.WithRegistry(registry.GetRegistry()), + pool.WithTracerProvider(traceProvider), + )..., + ) + if err != nil { + return nil, nil, err + } + + return &CS3{ + Config: cfg.Reva, + Logger: logger, + GatewaySelector: gatewaySelector, + }, nil, nil + case "ldap": + var err error + + var tlsConf *tls.Config + if cfg.Identity.LDAP.Insecure { + // When insecure is set to true then we don't need a certificate. + cfg.Identity.LDAP.CACert = "" + tlsConf = &tls.Config{ + MinVersion: tls.VersionTLS12, + + //nolint:gosec // We need the ability to run with "insecure" (dev/testing) + InsecureSkipVerify: cfg.Identity.LDAP.Insecure, + } + } + + if cfg.Identity.LDAP.CACert != "" { + if err := ocldap.WaitForCA(*logger, + cfg.Identity.LDAP.Insecure, + cfg.Identity.LDAP.CACert); err != nil { + logger.Fatal().Err(err).Msg("The configured LDAP CA cert does not exist") + } + if tlsConf == nil { + tlsConf = &tls.Config{ + MinVersion: tls.VersionTLS12, + } + } + certs := x509.NewCertPool() + pemData, err := os.ReadFile(cfg.Identity.LDAP.CACert) + if err != nil { + logger.Error().Err(err).Msg("Error initializing LDAP Backend") + return nil, nil, err + } + if !certs.AppendCertsFromPEM(pemData) { + logger.Error().Msg("Error initializing LDAP Backend. Adding CA cert failed") + return nil, nil, err + } + tlsConf.RootCAs = certs + } + + conn := ldap.NewLDAPWithReconnect( + ldap.Config{ + URI: cfg.Identity.LDAP.URI, + BindDN: cfg.Identity.LDAP.BindDN, + BindPassword: cfg.Identity.LDAP.BindPassword, + TLSConfig: tlsConf, + }, + ) + conn.SetLogger(&logger.Logger) + lb, err := NewLDAPBackend(conn, cfg.Identity.LDAP, logger) + if err != nil { + logger.Error().Err(err).Msg("Error initializing LDAP Backend") + return nil, nil, err + } + + identityBackend := lb + var eduBackend EducationBackend = lb + + if !cfg.Identity.LDAP.EducationResourcesEnabled { + eduBackend = &ErrEducationBackend{} + } + + disableMechanismType, err := ParseDisableMechanismType(cfg.Identity.LDAP.DisableUserMechanism) + if err != nil { + logger.Error().Err(err).Msg("Error initializing LDAP Backend") + return nil, nil, err + } + + if disableMechanismType == DisableMechanismGroup { + logger.Info().Msg("LocalUserDisable is true, will create group if not exists") + err := lb.CreateLDAPGroupByDN(cfg.Identity.LDAP.LdapDisabledUsersGroupDN) + if err != nil { + isAnError := false + var lerr *ldapv3.Error + if errors.As(err, &lerr) { + if lerr.ResultCode != ldapv3.LDAPResultEntryAlreadyExists { + isAnError = true + } + } else { + isAnError = true + } + + if isAnError { + msg := "error adding group for disabling users" + logger.Error().Err(err).Str("local_user_disable", cfg.Identity.LDAP.LdapDisabledUsersGroupDN).Msg(msg) + return nil, nil, err + } + } + } + + return identityBackend, eduBackend, nil + + default: + err := fmt.Errorf("unknown identity backend: '%s'", name) + logger.Err(err) + return nil, nil, err + } +} diff --git a/services/graph/pkg/identity/ldap.go b/services/graph/pkg/identity/ldap.go index 0cd28b23f0..a766814bb7 100644 --- a/services/graph/pkg/identity/ldap.go +++ b/services/graph/pkg/identity/ldap.go @@ -703,7 +703,7 @@ func (i *LDAP) usersFromLDAPEntries(entries []*ldap.Entry, exp []string) ([]*lib func (i *LDAP) UpdateLastSignInDate(ctx context.Context, userID string, timestamp time.Time) error { if !i.writeEnabled { i.logger.Debug().Str("backend", "ldap").Msg("The LDAP Server is readonly. Skipping update of last sign in date") - return nil + return nil // TODO: do we really want to just silently do nothing here, rather than returning an error? } e, err := i.getLDAPUserByID(userID) switch { diff --git a/services/graph/pkg/metrics/metrics.go b/services/graph/pkg/metrics/metrics.go index 7e597327a2..437822e662 100644 --- a/services/graph/pkg/metrics/metrics.go +++ b/services/graph/pkg/metrics/metrics.go @@ -12,12 +12,21 @@ var ( // Metrics defines the available metrics of this service. type Metrics struct { - // Counter *prometheus.CounterVec - BuildInfo *prometheus.GaugeVec + BuildInfo *prometheus.GaugeVec + EventsEnabled prometheus.Gauge + HttpEnabled prometheus.Gauge + EventsProcessed *prometheus.CounterVec + InvalidEvents prometheus.Counter + UnsupportedEvents prometheus.Counter } +const ( + ResultSuccess = "success" + ResultFailure = "failure" +) + // New initializes the available metrics. -func New() *Metrics { +func New(registerer prometheus.Registerer) *Metrics { m := &Metrics{ BuildInfo: prometheus.NewGaugeVec(prometheus.GaugeOpts{ Namespace: Namespace, @@ -25,9 +34,44 @@ func New() *Metrics { Name: "build_info", Help: "Build information", }, []string{"version"}), + EventsEnabled: prometheus.NewGauge(prometheus.GaugeOpts{ + Namespace: Namespace, + Subsystem: Subsystem, + Name: "events_enabled", + Help: "Whether this instance consumes events (1) or not (0)", + }), + HttpEnabled: prometheus.NewGauge(prometheus.GaugeOpts{ + Namespace: Namespace, + Subsystem: Subsystem, + Name: "http_enabled", + Help: "Whether this instance processes HTTP API calls (1) or not (0)", + }), + EventsProcessed: prometheus.NewCounterVec(prometheus.CounterOpts{ + Namespace: Namespace, + Subsystem: Subsystem, + Name: "events", + Help: "Number of consumed events", + }, []string{"event", "result"}), + InvalidEvents: prometheus.NewCounter(prometheus.CounterOpts{ + Namespace: Namespace, + Subsystem: Subsystem, + Name: "events_invalid", + Help: "Number of supported events with invalid data", + }), + UnsupportedEvents: prometheus.NewCounter(prometheus.CounterOpts{ + Namespace: Namespace, + Subsystem: Subsystem, + Name: "events_unsupported", + Help: "Number of unsupported events that were consumed and ignored", + }), } _ = prometheus.Register(m.BuildInfo) + _ = prometheus.Register(m.EventsEnabled) + _ = prometheus.Register(m.HttpEnabled) + _ = prometheus.Register(m.EventsProcessed) + _ = prometheus.Register(m.UnsupportedEvents) + _ = prometheus.Register(m.InvalidEvents) // TODO: implement metrics return m } diff --git a/services/graph/pkg/server/http/server.go b/services/graph/pkg/server/http/server.go index c5830949b0..583d01e335 100644 --- a/services/graph/pkg/server/http/server.go +++ b/services/graph/pkg/server/http/server.go @@ -8,7 +8,6 @@ import ( gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1" chimiddleware "github.com/go-chi/chi/v5/middleware" - "github.com/opencloud-eu/reva/v2/pkg/events/stream" "github.com/opencloud-eu/reva/v2/pkg/rgrpc/todo/pool" revaMetadata "github.com/opencloud-eu/reva/v2/pkg/storage/utils/metadata" "go-micro.dev/v4" @@ -16,7 +15,6 @@ import ( "github.com/opencloud-eu/opencloud/pkg/account" "github.com/opencloud-eu/opencloud/pkg/cors" - "github.com/opencloud-eu/opencloud/pkg/generators" "github.com/opencloud-eu/opencloud/pkg/keycloak" "github.com/opencloud-eu/opencloud/pkg/middleware" "github.com/opencloud-eu/opencloud/pkg/registry" @@ -27,12 +25,13 @@ import ( ehsvc "github.com/opencloud-eu/opencloud/protogen/gen/opencloud/services/eventhistory/v0" searchsvc "github.com/opencloud-eu/opencloud/protogen/gen/opencloud/services/search/v0" settingssvc "github.com/opencloud-eu/opencloud/protogen/gen/opencloud/services/settings/v0" + "github.com/opencloud-eu/opencloud/services/graph/pkg/identity" graphMiddleware "github.com/opencloud-eu/opencloud/services/graph/pkg/middleware" svc "github.com/opencloud-eu/opencloud/services/graph/pkg/service/v0" ) // Server initializes the http service and server. -func Server(opts ...Option) (http.Service, error) { +func Server(identityBackend identity.Backend, eduBackend identity.EducationBackend, eventsStream events.Stream, opts ...Option) (http.Service, error) { options := newOptions(opts...) service, err := http.NewService( @@ -53,20 +52,6 @@ func Server(opts ...Option) (http.Service, error) { return http.Service{}, fmt.Errorf("could not initialize http service: %w", err) } - var eventsStream events.Stream - - if options.Config.Events.Endpoint != "" { - var err error - connName := generators.GenerateConnectionName(options.Config.Service.Name, generators.NTypeBus) - eventsStream, err = stream.NatsFromConfig(connName, false, stream.NatsConfig(options.Config.Events)) - if err != nil { - options.Logger.Error(). - Err(err). - Msg("Error initializing events publisher") - return http.Service{}, fmt.Errorf("could not initialize events publisher: %w", err) - } - } - middlewares := []func(stdhttp.Handler) stdhttp.Handler{ middleware.TraceContext, chimiddleware.RequestID, @@ -168,8 +153,7 @@ func Server(opts ...Option) (http.Service, error) { svc.Logger(options.Logger), svc.Config(options.Config), svc.Middleware(middlewares...), - svc.EventsPublisher(eventsStream), - svc.EventsConsumer(eventsStream), + svc.EventsPublisher(eventsStream), // is required even when event consumption is disabled svc.WithRoleService(roleService), svc.WithValueService(valueService), svc.WithRequireAdminMiddleware(requireAdminMiddleware), @@ -179,6 +163,8 @@ func Server(opts ...Option) (http.Service, error) { svc.EventHistoryClient(hClient), svc.TraceProvider(options.TraceProvider), svc.WithNatsKeyValue(options.NatsKeyValue), + svc.WithIdentityBackend(identityBackend), + svc.WithIdentityEducationBackend(eduBackend), ) if err != nil { diff --git a/services/graph/pkg/service/events/service.go b/services/graph/pkg/service/events/service.go new file mode 100644 index 0000000000..21213e68ac --- /dev/null +++ b/services/graph/pkg/service/events/service.go @@ -0,0 +1,124 @@ +package events + +import ( + "context" + "io" + "sync/atomic" + + "github.com/opencloud-eu/reva/v2/pkg/events" + "github.com/opencloud-eu/reva/v2/pkg/utils" + + "github.com/opencloud-eu/opencloud/pkg/log" + "github.com/opencloud-eu/opencloud/services/graph/pkg/identity" + "github.com/opencloud-eu/opencloud/services/graph/pkg/metrics" +) + +func processEvents(ctx context.Context, consumer events.Consumer, stop *atomic.Bool, stopCh chan struct{}, + backend identity.Backend, m *metrics.Metrics, logger *log.Logger) error { + var _registeredEvents = []events.Unmarshaller{ + events.UserSignedIn{}, + } + evChannel, err := events.Consume(consumer, "graph", _registeredEvents...) + if err != nil { + logger.Error().Err(err).Msg("cannot consume from nats") + return err + } + logger.Debug().Msg("listening for events") + for loop := true; loop; { + select { + case e := <-evChannel: + switch ev := e.Event.(type) { + default: + // this branch is currently impossible to test and run into because we pick which events we're interested in + // through the _registeredEvents above, and the stream won't hand us events we didn't register for + m.UnsupportedEvents.Inc() + logger.Error().Interface("event", e).Msg("unhandled event") + case events.UserSignedIn: + name := "UserSignedIn" + userId := "" + if ev.Executant != nil && ev.Executant.OpaqueId != "" { + userId = ev.Executant.OpaqueId + } else { + m.InvalidEvents.Inc() + logger.Error().Err(err).Interface("event", ev).Msg("Received invalid event: executant.opaqueId not set") + continue + } + if err := backend.UpdateLastSignInDate(ctx, userId, utils.TSToTime(ev.Timestamp)); err != nil { + m.EventsProcessed.WithLabelValues(name, metrics.ResultFailure).Inc() + logger.Error().Err(err).Str("userid", userId).Str("event", name).Msg("Error updating last sign in date") + } else { + // TODO: UpdateLastSignInDate() currently returns nil instead of an error when the LDAP server is read-only, so those will be accounted for as a success + m.EventsProcessed.WithLabelValues(name, metrics.ResultSuccess).Inc() + logger.Debug().Str("userid", userId).Str("event", name).Msg("Successfully updated last sign in date") + } + } + if stop.Load() { + loop = false + } + case <-stopCh: + logger.Info().Msg("instructed to stop") + loop = false + case <-ctx.Done(): + logger.Info().Msg("context cancelled") + loop = false + } + } + return nil +} + +type GraphEventConsumer interface { + Start() error + io.Closer +} + +type GraphEventConsumerImpl struct { + ctx context.Context + consumer events.Consumer + backend identity.Backend + metrics *metrics.Metrics + logger *log.Logger + stopped atomic.Bool + stopCh chan struct{} +} + +var _ GraphEventConsumer = &GraphEventConsumerImpl{} + +func (g *GraphEventConsumerImpl) Start() error { + return processEvents(g.ctx, g.consumer, &g.stopped, g.stopCh, g.backend, g.metrics, g.logger) +} + +func (g *GraphEventConsumerImpl) Close() error { + if g.stopped.CompareAndSwap(false, true) { + close(g.stopCh) + } + return nil +} + +type NullGraphEventConsumer struct { +} + +var _ GraphEventConsumer = &NullGraphEventConsumer{} + +func (n *NullGraphEventConsumer) Start() error { + return nil +} + +func (n *NullGraphEventConsumer) Close() error { + return nil +} + +func NewService(ctx context.Context, consumer events.Consumer, backend identity.Backend, metrics *metrics.Metrics, logger *log.Logger) (GraphEventConsumer, error) { + if consumer == nil { + return &NullGraphEventConsumer{}, nil + } else { + stopCh := make(chan struct{}, 1) + return &GraphEventConsumerImpl{ + ctx: ctx, + consumer: consumer, + backend: backend, + metrics: metrics, + logger: logger, + stopCh: stopCh, + }, nil + } +} diff --git a/services/graph/pkg/service/events/service_test.go b/services/graph/pkg/service/events/service_test.go new file mode 100644 index 0000000000..e5e118ed91 --- /dev/null +++ b/services/graph/pkg/service/events/service_test.go @@ -0,0 +1,124 @@ +package events_test + +import ( + "context" + "errors" + "fmt" + "math/rand/v2" + "sync" + "testing" + "time" + + userv1beta1 "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" + "github.com/prometheus/client_golang/prometheus" + "github.com/test-go/testify/mock" + + "github.com/opencloud-eu/opencloud/internal/eventstest" + "github.com/opencloud-eu/opencloud/internal/metricstest" + "github.com/opencloud-eu/opencloud/pkg/log" + "github.com/opencloud-eu/opencloud/services/graph/pkg/identity/mocks" + "github.com/opencloud-eu/opencloud/services/graph/pkg/metrics" + g "github.com/opencloud-eu/opencloud/services/graph/pkg/service/events" + "github.com/opencloud-eu/reva/v2/pkg/events" + "github.com/stretchr/testify/require" +) + +func TestSuccessfulCall(t *testing.T) { + require := require.New(t) + + ctx, cancel := context.WithCancel(t.Context()) + + bus := eventstest.NewTestBus() + + var wg sync.WaitGroup + wg.Add(1) + + userId := fmt.Sprintf("user%d", 1000+rand.IntN(10000)) + + backend := mocks.NewBackend(t) + backend.EXPECT().UpdateLastSignInDate(mock.Anything, mock.Anything, mock.Anything).RunAndReturn(func(_ context.Context, _ string, _ time.Time) error { + defer wg.Done() + return nil + }) + + reg := prometheus.NewRegistry() + m := metrics.New(reg) + + logger := log.NewLogger() + + svc, err := g.NewService(ctx, bus, backend, m, &logger) + require.NoError(err) + t.Cleanup(func() { svc.Close() }) + t.Cleanup(cancel) + go func() { + require.NoError(svc.Start()) + }() + + metricstest.RequireEqual(t, 0, m.UnsupportedEvents) + metricstest.RequireIsNotSet(t, m.EventsProcessed) + + _ = bus.Publish(events.UserSignedIn{ + Timestamp: nil, + Executant: &userv1beta1.UserId{ + OpaqueId: userId, + }, + }) + + wg.Wait() + require.Len(backend.Mock.Calls, 1) + require.Len(backend.Mock.Calls[0].Arguments, 3) + require.Equal(userId, backend.Mock.Calls[0].Arguments[1]) + + metricstest.RequireEqual(t, 0, m.UnsupportedEvents) + metricstest.RequireEqualWithLabels(t, 1, map[string]string{"event": "UserSignedIn", "result": "success"}, m.EventsProcessed) +} + +func TestBackendReturningAnError(t *testing.T) { + require := require.New(t) + + ctx, cancel := context.WithCancel(t.Context()) + + bus := eventstest.NewTestBus() + + var wg sync.WaitGroup + wg.Add(1) + + userId := fmt.Sprintf("user%d", 1000+rand.IntN(10000)) + + backend := mocks.NewBackend(t) + backend.EXPECT().UpdateLastSignInDate(mock.Anything, mock.Anything, mock.Anything).RunAndReturn(func(_ context.Context, _ string, _ time.Time) error { + defer wg.Done() + return errors.New("test") + }) + + reg := prometheus.NewRegistry() + m := metrics.New(reg) + + logger := log.NewLogger() + + svc, err := g.NewService(ctx, bus, backend, m, &logger) + require.NoError(err) + t.Cleanup(func() { svc.Close() }) + t.Cleanup(cancel) + go func() { + require.NoError(svc.Start()) + }() + + metricstest.RequireEqual(t, 0, m.UnsupportedEvents) + metricstest.RequireIsNotSet(t, m.EventsProcessed) + + _ = bus.Publish(events.UserSignedIn{ + Timestamp: nil, + Executant: &userv1beta1.UserId{ + OpaqueId: userId, + }, + }) + + wg.Wait() + require.Len(backend.Mock.Calls, 1) + require.Len(backend.Mock.Calls[0].Arguments, 3) + require.Equal(userId, backend.Mock.Calls[0].Arguments[1]) + + metricstest.RequireEqual(t, 0, m.UnsupportedEvents) + metricstest.RequireEqualWithLabels(t, 1, map[string]string{"event": "UserSignedIn", "result": "failure"}, m.EventsProcessed) +} diff --git a/services/graph/pkg/service/v0/graph.go b/services/graph/pkg/service/v0/graph.go index c6ef4fa74c..38246415e7 100644 --- a/services/graph/pkg/service/v0/graph.go +++ b/services/graph/pkg/service/v0/graph.go @@ -63,7 +63,6 @@ type Graph struct { valueService settingssvc.ValueService specialDriveItemsCache *ttlcache.Cache[string, any] eventsPublisher events.Publisher - eventsConsumer events.Consumer searchService searchsvc.SearchProviderService keycloakClient keycloak.Client historyClient ehsvc.EventHistoryService diff --git a/services/graph/pkg/service/v0/option.go b/services/graph/pkg/service/v0/option.go index 5330fd5783..cc720d7b92 100644 --- a/services/graph/pkg/service/v0/option.go +++ b/services/graph/pkg/service/v0/option.go @@ -39,7 +39,6 @@ type Options struct { ValueService settingssvc.ValueService RoleManager *roles.Manager EventsPublisher events.Publisher - EventsConsumer events.Consumer SearchService searchsvc.SearchProviderService KeycloakClient keycloak.Client EventHistoryClient ehsvc.EventHistoryService @@ -163,13 +162,6 @@ func EventsPublisher(val events.Publisher) Option { } } -// EventsConsumer provides a function to set the EventsConsumer option. -func EventsConsumer(val events.Consumer) Option { - return func(o *Options) { - o.EventsConsumer = val - } -} - // KeycloakClient provides a function to set the KeycloakCient option. func KeycloakClient(val keycloak.Client) Option { return func(o *Options) { diff --git a/services/graph/pkg/service/v0/service.go b/services/graph/pkg/service/v0/service.go index 7445395b4c..6eed744a05 100644 --- a/services/graph/pkg/service/v0/service.go +++ b/services/graph/pkg/service/v0/service.go @@ -1,39 +1,25 @@ package svc import ( - "context" - "crypto/tls" - "crypto/x509" - "errors" "fmt" "net/http" "net/url" - "os" "strconv" "time" "github.com/go-chi/chi/v5" "github.com/go-chi/chi/v5/middleware" - ldapv3 "github.com/go-ldap/ldap/v3" "github.com/jellydator/ttlcache/v3" "github.com/opencloud-eu/opencloud/services/graph/pkg/identity/cache" "github.com/riandyrn/otelchi" microstore "go-micro.dev/v4/store" - "github.com/opencloud-eu/reva/v2/pkg/events" - "github.com/opencloud-eu/reva/v2/pkg/rgrpc/todo/pool" "github.com/opencloud-eu/reva/v2/pkg/store" - "github.com/opencloud-eu/reva/v2/pkg/utils" - "github.com/opencloud-eu/reva/v2/pkg/utils/ldap" - ocldap "github.com/opencloud-eu/opencloud/pkg/ldap" - "github.com/opencloud-eu/opencloud/pkg/log" - "github.com/opencloud-eu/opencloud/pkg/registry" "github.com/opencloud-eu/opencloud/pkg/roles" "github.com/opencloud-eu/opencloud/pkg/service/grpc" "github.com/opencloud-eu/opencloud/pkg/tracing" settingssvc "github.com/opencloud-eu/opencloud/protogen/gen/opencloud/services/settings/v0" - "github.com/opencloud-eu/opencloud/services/graph/pkg/identity" graphm "github.com/opencloud-eu/opencloud/services/graph/pkg/middleware" "github.com/opencloud-eu/opencloud/services/graph/pkg/unifiedrole" ) @@ -199,8 +185,8 @@ func NewService(opts ...Option) (Graph, error) { //nolint:maintidx mux: m, specialDriveItemsCache: spacePropertiesCache, eventsPublisher: options.EventsPublisher, - eventsConsumer: options.EventsConsumer, searchService: options.SearchService, + identityBackend: options.IdentityBackend, identityEducationBackend: options.IdentityEducationBackend, keycloakClient: options.KeycloakClient, historyClient: options.EventHistoryClient, @@ -209,10 +195,6 @@ func NewService(opts ...Option) (Graph, error) { //nolint:maintidx natskv: options.NatsKeyValue, } - if err := setIdentityBackends(options, &svc); err != nil { - return svc, err - } - if options.PermissionService == nil { grpcClient, err := grpc.NewClient(append(grpc.GetClientOptions(options.Config.GRPCClientTLS), grpc.WithTraceProvider(options.TraceProvider))...) if err != nil { @@ -450,164 +432,6 @@ func NewService(opts ...Option) (Graph, error) { //nolint:maintidx return svc, nil } -func setIdentityBackends(options Options, svc *Graph) error { - if options.IdentityBackend == nil { - switch options.Config.Identity.Backend { - case "cs3": - gatewaySelector, err := pool.GatewaySelector( - options.Config.Reva.Address, - append( - options.Config.Reva.GetRevaOptions(), - pool.WithRegistry(registry.GetRegistry()), - pool.WithTracerProvider(options.TraceProvider), - )..., - ) - if err != nil { - return err - } - - svc.identityBackend = &identity.CS3{ - Config: options.Config.Reva, - Logger: &options.Logger, - GatewaySelector: gatewaySelector, - } - case "ldap": - var err error - - var tlsConf *tls.Config - if options.Config.Identity.LDAP.Insecure { - - // When insecure is set to true then we don't need a certificate. - options.Config.Identity.LDAP.CACert = "" - tlsConf = &tls.Config{ - MinVersion: tls.VersionTLS12, - - //nolint:gosec // We need the ability to run with "insecure" (dev/testing) - InsecureSkipVerify: options.Config.Identity.LDAP.Insecure, - } - } - - if options.Config.Identity.LDAP.CACert != "" { - if err := ocldap.WaitForCA(options.Logger, - options.Config.Identity.LDAP.Insecure, - options.Config.Identity.LDAP.CACert); err != nil { - options.Logger.Fatal().Err(err).Msg("The configured LDAP CA cert does not exist") - } - if tlsConf == nil { - tlsConf = &tls.Config{ - MinVersion: tls.VersionTLS12, - } - } - certs := x509.NewCertPool() - pemData, err := os.ReadFile(options.Config.Identity.LDAP.CACert) - if err != nil { - options.Logger.Error().Err(err).Msg("Error initializing LDAP Backend") - return err - } - if !certs.AppendCertsFromPEM(pemData) { - options.Logger.Error().Msg("Error initializing LDAP Backend. Adding CA cert failed") - return err - } - tlsConf.RootCAs = certs - } - - conn := ldap.NewLDAPWithReconnect( - ldap.Config{ - URI: options.Config.Identity.LDAP.URI, - BindDN: options.Config.Identity.LDAP.BindDN, - BindPassword: options.Config.Identity.LDAP.BindPassword, - TLSConfig: tlsConf, - }, - ) - conn.SetLogger(&options.Logger.Logger) - lb, err := identity.NewLDAPBackend(conn, options.Config.Identity.LDAP, &options.Logger) - if err != nil { - options.Logger.Error().Err(err).Msg("Error initializing LDAP Backend") - return err - } - svc.identityBackend = lb - if options.IdentityEducationBackend == nil { - if options.Config.Identity.LDAP.EducationResourcesEnabled { - svc.identityEducationBackend = lb - } else { - errEduBackend := &identity.ErrEducationBackend{} - svc.identityEducationBackend = errEduBackend - } - } - - disableMechanismType, err := identity.ParseDisableMechanismType(options.Config.Identity.LDAP.DisableUserMechanism) - if err != nil { - options.Logger.Error().Err(err).Msg("Error initializing LDAP Backend") - return err - } - - if disableMechanismType == identity.DisableMechanismGroup { - options.Logger.Info().Msg("LocalUserDisable is true, will create group if not exists") - err := lb.CreateLDAPGroupByDN(options.Config.Identity.LDAP.LdapDisabledUsersGroupDN) - if err != nil { - isAnError := false - var lerr *ldapv3.Error - if errors.As(err, &lerr) { - if lerr.ResultCode != ldapv3.LDAPResultEntryAlreadyExists { - isAnError = true - } - } else { - isAnError = true - } - - if isAnError { - msg := "error adding group for disabling users" - options.Logger.Error().Err(err).Str("local_user_disable", options.Config.Identity.LDAP.LdapDisabledUsersGroupDN).Msg(msg) - return err - } - } - } - - default: - err := fmt.Errorf("unknown identity backend: '%s'", options.Config.Identity.Backend) - options.Logger.Err(err) - return err - } - } else { - svc.identityBackend = options.IdentityBackend - } - - return svc.StartListenForLogonEvents(options.Context, options.Logger) -} - -func (g *Graph) StartListenForLogonEvents(ctx context.Context, l log.Logger) error { - if g.eventsConsumer == nil { - return nil - } - var _registeredEvents = []events.Unmarshaller{ - events.UserSignedIn{}, - } - evChannel, err := events.Consume(g.eventsConsumer, "graph", _registeredEvents...) - if err != nil { - l.Error().Err(err).Msg("cannot consume from nats") - return err - } - go func() { - for loop := true; loop; { - select { - case e := <-evChannel: - switch ev := e.Event.(type) { - default: - l.Error().Interface("event", e).Msg("unhandled event") - case events.UserSignedIn: - if err := g.identityBackend.UpdateLastSignInDate(ctx, ev.Executant.OpaqueId, utils.TSToTime(ev.Timestamp)); err != nil { - l.Error().Err(err).Str("userid", ev.Executant.OpaqueId).Msg("Error updating last sign in date") - } - } - case <-ctx.Done(): - l.Info().Msg("context cancelled") - loop = false - } - } - }() - return nil -} - // parseHeaderPurge parses the 'Purge' header. // '1', 't', 'T', 'TRUE', 'true', 'True' are parsed as true // all other values are false. From e27961736a365430aa9d4a727f6539475bd7e103 Mon Sep 17 00:00:00 2001 From: Pascal Bleser Date: Thu, 13 Aug 2026 21:50:40 +0200 Subject: [PATCH 2/2] chore(graph): add metrics for HTTP API and LDAP Introducing gowrap as a build-time tool to generate interface delegate structs from templates: * added as a 'make go-generate' target in services/graph, * added as a build-time dependency in .bingo/ Introduce an LDAP client abstraction interface to be able to wrap the go-ldap client API with metrics transparently (and possibly hooks and such in the future), in order to use delegation patterns to measure the time LDAP (client) operations take to finish, as well as to track their results (success, failure, not-found). Has two implementations that are generated using gowrap: * a go-ldap adapter implementation that directly delegates to a go-ldap connection * a time measuring and metrics collecting implementation that delegates to another LdapClient The metrics collecting one is disabled by default, can be enabled with GRAPH_LDAP_METRICS_DISABLE=false It collects durations of outbound LDAP client operations into a histogram, as well as the number of concurrent outbound LDAP operations in a gauge (via an atomic int and a gauge function, as that performs best). Add an HTTP middleware that measures how long Graph HTTP API requests take, storing taken time into a histogram along with labels for * method, * path pattern (from the chi routes), * Graph API version prefix, * Graph API resource name, * and the resulting status code. It also tracks the number of concurrent inbound Graph API HTTP requests using a gauge (also using an atomic int and a gauge function). Disabled by default, can be enabled with GRAPH_HTTP_METRICS_DISABLE=false Add Backend and EducationBackend delegate implementations that measure execution time on the level of the higher API call operations there (CreateUser, DeleteUser, ..., CreateSchool, ...), generated using gowrap. Disabled by default, can be enabled with GRAPH_IDENTITY_BACKEND_METRICS_DISABLE=false Also added a small k6 script to produce some read-only load on the Graph API, for a casual test of the metrics, as well as k6 in mise.toml. Make an internal changes to how singular LDAP entry searches work in the LDAP identity backends: * check whether searches for a singular entry returns more than one result, in which case a new error TooManyResults is returned, instead of leaving that undetected, blindly taking the first result, and potentially risking data inconsistencies Improve the loggers in identity backends by adding attributes for their request targets (Reva gateway address or LDAP URI, respectively). Also add a "backend" attribute for all Graph API logs (set to "ldap" or "cs3"), to help debug potential issues, and remove them from all the logger debug calls at the beginning of each LDAP-related function as those should really be part of the logger and set beforehand. The LDAP identity backend logger also has two new attributes to help debugging with logs: * write (bool): whether write operations are enabled * refint (bool): whether refint is enabled or not Also adds a dedicated counter metric for user password change operations. Minor campfire improvements: * add a constructor func for the CS3 backend * add a constructor func for the LDAP backend * in the LDAP identity backend, in searchLDAPEntryByFilter (used by all search/get public functions), errors that occur when performing LDAP SEARCH operations were blindly mapped to a ItemNotFound error, instead of being analyzed as it could be caused by a technical error * in the requireadmin middleware, add debug logging to explain why a request is denied * when an LDAP password change fails because the user entry was not found in LDAP, we now have a log message that tracks that --- .bingo/Variables.mk | 6 + .bingo/gowrap.mod | 5 + .bingo/gowrap.sum | 55 ++ .bingo/variables.env | 2 + mise.toml | 3 + services/graph/Makefile | 3 +- services/graph/README.md | 73 ++ services/graph/load_test.js | 80 ++ services/graph/pkg/command/server.go | 20 +- services/graph/pkg/config/config.go | 15 +- .../pkg/config/defaults/defaultconfig.go | 16 + services/graph/pkg/config/http.go | 5 + services/graph/pkg/config/parser/parse.go | 2 +- services/graph/pkg/errorcode/errorcode.go | 16 + services/graph/pkg/identity/backend.go | 53 ++ .../graph/pkg/identity/backend_prometheus.go | 500 ++++++++++ .../pkg/identity/backend_prometheus.tmpl | 73 ++ services/graph/pkg/identity/cs3.go | 16 + .../identity/education_backend_prometheus.go | 871 ++++++++++++++++++ services/graph/pkg/identity/err_education.go | 2 + services/graph/pkg/identity/factory.go | 81 +- services/graph/pkg/identity/ldap.go | 222 ++++- services/graph/pkg/identity/ldap_client.go | 26 + .../graph/pkg/identity/ldap_client_goldap.go | 57 ++ .../pkg/identity/ldap_client_goldap.tmpl | 25 + .../pkg/identity/ldap_client_prometheus.go | 208 +++++ .../pkg/identity/ldap_client_prometheus.tmpl | 62 ++ .../pkg/identity/ldap_education_class.go | 53 +- .../pkg/identity/ldap_education_class_test.go | 2 +- .../pkg/identity/ldap_education_school.go | 181 ++-- .../graph/pkg/identity/ldap_education_user.go | 64 +- services/graph/pkg/identity/ldap_group.go | 100 +- services/graph/pkg/identity/ldap_test.go | 39 +- services/graph/pkg/metrics/metrics.go | 113 ++- services/graph/pkg/metrics/middleware.go | 59 ++ services/graph/pkg/middleware/requireadmin.go | 6 +- services/graph/pkg/server/http/server.go | 15 +- services/graph/pkg/service/events/service.go | 110 +-- .../graph/pkg/service/events/service_test.go | 4 +- .../graph/pkg/service/v0/application_test.go | 4 + .../pkg/service/v0/approleassignments_test.go | 4 + services/graph/pkg/service/v0/base.go | 31 +- .../graph/pkg/service/v0/driveitems_test.go | 4 + .../graph/pkg/service/v0/educationclasses.go | 3 +- .../pkg/service/v0/educationclasses_test.go | 7 + .../graph/pkg/service/v0/educationschools.go | 16 +- .../pkg/service/v0/educationschools_test.go | 4 + .../pkg/service/v0/educationuser_test.go | 4 + services/graph/pkg/service/v0/graph.go | 4 + services/graph/pkg/service/v0/graph_test.go | 5 + services/graph/pkg/service/v0/groups.go | 8 +- services/graph/pkg/service/v0/groups_test.go | 7 + services/graph/pkg/service/v0/option.go | 9 + services/graph/pkg/service/v0/password.go | 22 +- .../graph/pkg/service/v0/password_test.go | 14 +- .../pkg/service/v0/rolemanagement_test.go | 4 + services/graph/pkg/service/v0/service.go | 28 + .../graph/pkg/service/v0/sharedbyme_test.go | 4 + .../graph/pkg/service/v0/sharedwithme_test.go | 4 + services/graph/pkg/service/v0/users.go | 67 +- services/graph/pkg/service/v0/users_test.go | 9 +- 61 files changed, 3148 insertions(+), 357 deletions(-) create mode 100644 .bingo/gowrap.mod create mode 100644 .bingo/gowrap.sum create mode 100644 services/graph/load_test.js create mode 100644 services/graph/pkg/identity/backend_prometheus.go create mode 100644 services/graph/pkg/identity/backend_prometheus.tmpl create mode 100644 services/graph/pkg/identity/education_backend_prometheus.go create mode 100644 services/graph/pkg/identity/ldap_client.go create mode 100644 services/graph/pkg/identity/ldap_client_goldap.go create mode 100644 services/graph/pkg/identity/ldap_client_goldap.tmpl create mode 100644 services/graph/pkg/identity/ldap_client_prometheus.go create mode 100644 services/graph/pkg/identity/ldap_client_prometheus.tmpl create mode 100644 services/graph/pkg/metrics/middleware.go diff --git a/.bingo/Variables.mk b/.bingo/Variables.mk index fbe7a09983..735d0d84e5 100644 --- a/.bingo/Variables.mk +++ b/.bingo/Variables.mk @@ -65,6 +65,12 @@ $(GOVULNCHECK): $(BINGO_DIR)/govulncheck.mod @echo "(re)installing $(GOBIN)/govulncheck-v1.1.4" @cd $(BINGO_DIR) && GOWORK=off $(GO) build -mod=mod -modfile=govulncheck.mod -o=$(GOBIN)/govulncheck-v1.1.4 "golang.org/x/vuln/cmd/govulncheck" +GOWRAP := $(GOBIN)/gowrap-v1.4.3 +$(GOWRAP): $(BINGO_DIR)/gowrap.mod + @# Install binary/ries using Go 1.14+ build command. This is using bwplotka/bingo-controlled, separate go module with pinned dependencies. + @echo "(re)installing $(GOBIN)/gowrap-v1.4.3" + @cd $(BINGO_DIR) && GOWORK=off $(GO) build -mod=mod -modfile=gowrap.mod -o=$(GOBIN)/gowrap-v1.4.3 "github.com/hexdigest/gowrap/cmd/gowrap" + MOCKERY := $(GOBIN)/mockery-v3.4.0 $(MOCKERY): $(BINGO_DIR)/mockery.mod @# Install binary/ries using Go 1.14+ build command. This is using bwplotka/bingo-controlled, separate go module with pinned dependencies. diff --git a/.bingo/gowrap.mod b/.bingo/gowrap.mod new file mode 100644 index 0000000000..37caf9a46b --- /dev/null +++ b/.bingo/gowrap.mod @@ -0,0 +1,5 @@ +module _ // Auto generated by https://github.com/bwplotka/bingo. DO NOT EDIT + +go 1.25.8 + +require github.com/hexdigest/gowrap v1.4.3 // cmd/gowrap diff --git a/.bingo/gowrap.sum b/.bingo/gowrap.sum new file mode 100644 index 0000000000..c50b5c1be4 --- /dev/null +++ b/.bingo/gowrap.sum @@ -0,0 +1,55 @@ +github.com/Masterminds/goutils v1.1.1 h1:5nUrii3FMTL5diU80unEVvNevw1nH4+ZV4DSLVJLSYI= +github.com/Masterminds/goutils v1.1.1/go.mod h1:8cTjp+g8YejhMuvIA5y2vz3BpJxksy863GQaJW2MFNU= +github.com/Masterminds/semver/v3 v3.1.1 h1:hLg3sBzpNErnxhQtUy/mmLR2I9foDujNK030IGemrRc= +github.com/Masterminds/semver/v3 v3.1.1/go.mod h1:VPu/7SZ7ePZ3QOrcuXROw5FAcLl4a0cBrbBpGY/8hQs= +github.com/Masterminds/sprig/v3 v3.2.2 h1:17jRggJu518dr3QaafizSXOjKYp94wKfABxUmyxvxX8= +github.com/Masterminds/sprig/v3 v3.2.2/go.mod h1:UoaO7Yp8KlPnJIYWTFkMaqPUYKTfGFPhxNuwnnxkKlk= +github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/google/uuid v1.1.1/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= +github.com/google/uuid v1.3.0 h1:t6JiXgmwXMjEs8VusXIJk2BXHsn+wx8BZdTaoZ5fu7I= +github.com/google/uuid v1.3.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= +github.com/hexdigest/gowrap v1.4.3 h1:m+t8aj1pUiFQbEiE8QJg2xdYVH5DAMluLgZ9P/qEF0k= +github.com/hexdigest/gowrap v1.4.3/go.mod h1:XWL8oQW2H3fX5ll8oT3Fduh4mt2H3cUAGQHQLMUbmG4= +github.com/huandu/xstrings v1.3.1/go.mod h1:y5/lhBue+AyNmUVz9RLU9xbLR0o4KIIExikq4ovT0aE= +github.com/huandu/xstrings v1.3.2 h1:L18LIDzqlW6xN2rEkpdV8+oL/IXWJ1APd+vsdYy4Wdw= +github.com/huandu/xstrings v1.3.2/go.mod h1:y5/lhBue+AyNmUVz9RLU9xbLR0o4KIIExikq4ovT0aE= +github.com/imdario/mergo v0.3.11/go.mod h1:jmQim1M+e3UYxmgPu/WyfjB3N3VflVyUjjjwH0dnCYA= +github.com/imdario/mergo v0.3.12 h1:b6R2BslTbIEToALKP7LxUvijTsNI9TAe80pLWN2g/HU= +github.com/imdario/mergo v0.3.12/go.mod h1:jmQim1M+e3UYxmgPu/WyfjB3N3VflVyUjjjwH0dnCYA= +github.com/mitchellh/copystructure v1.0.0/go.mod h1:SNtv71yrdKgLRyLFxmLdkAbkKEFWgYaq1OVrnRcwhnw= +github.com/mitchellh/copystructure v1.1.2 h1:Th2TIvG1+6ma3e/0/bopBKohOTY7s4dA8V2q4EUcBJ0= +github.com/mitchellh/copystructure v1.1.2/go.mod h1:EBArHfARyrSWO/+Wyr9zwEkc6XMFB9XyNgFNmRkZZU4= +github.com/mitchellh/reflectwalk v1.0.0/go.mod h1:mSTlrgnPZtwu0c4WaC2kGObEpuNDbx0jmZXqmk4esnw= +github.com/mitchellh/reflectwalk v1.0.1 h1:FVzMWA5RllMAKIdUSC8mdWo3XtwoecrH79BY70sEEpE= +github.com/mitchellh/reflectwalk v1.0.1/go.mod h1:mSTlrgnPZtwu0c4WaC2kGObEpuNDbx0jmZXqmk4esnw= +github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= +github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= +github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= +github.com/shopspring/decimal v1.2.0 h1:abSATXmQEYyShuxI4/vyW3tV1MrKAJzCZ/0zLUXYbsQ= +github.com/shopspring/decimal v1.2.0/go.mod h1:DKyhrW/HYNuLGql+MJL6WCR6knT2jwCFRcu2hWCYk4o= +github.com/spf13/cast v1.3.1/go.mod h1:Qx5cxh0v+4UWYiBimWS+eyWzqEqokIECu5etghLkUJE= +github.com/spf13/cast v1.4.1 h1:s0hze+J0196ZfEMTs80N7UlFt0BDuQ7Q+JDnHiMWKdA= +github.com/spf13/cast v1.4.1/go.mod h1:Qx5cxh0v+4UWYiBimWS+eyWzqEqokIECu5etghLkUJE= +github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= +github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= +github.com/stretchr/testify v1.5.1/go.mod h1:5W2xD1RspED5o8YsWQXVCued0rvSQ+mT+I5cxcmMvtA= +golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= +golang.org/x/crypto v0.0.0-20200414173820-0848c9571904/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= +golang.org/x/crypto v0.35.0 h1:b15kiHdrGCHrP6LvwaQ3c03kgNhhiMgvlhxHQhmg2Xs= +golang.org/x/crypto v0.35.0/go.mod h1:dy7dXNW32cAb/6/PRuTNsix8T+vJAqvuIy5Bli/x0YQ= +golang.org/x/mod v0.27.0 h1:kb+q2PyFnEADO2IEF935ehFUXlWiNjJWtRNgBLSfbxQ= +golang.org/x/mod v0.27.0/go.mod h1:rWI627Fq0DEoudcK+MBkNkCe0EetEaDSwJJkCcjpazc= +golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= +golang.org/x/sync v0.16.0 h1:ycBJEhp9p4vXvUZNszeOq0kGTPghopOL8q0fq3vstxw= +golang.org/x/sync v0.16.0/go.mod h1:1dzgHSNfp02xaA81J2MS99Qcpr2w7fw1gpm99rleRqA= +golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= +golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= +golang.org/x/text v0.22.0 h1:bofq7m3/HAFvbF51jz3Q9wLg3jkvSPuiZu/pD1XwgtM= +golang.org/x/text v0.22.0/go.mod h1:YRoo4H8PVmsu+E3Ou7cqLVH8oXWIHVoX0jqUWALQhfY= +golang.org/x/tools v0.36.0 h1:kWS0uv/zsvHEle1LbV5LE8QujrxB3wfQyxHfhOk0Qkg= +golang.org/x/tools v0.36.0/go.mod h1:WBDiHKJK8YgLHlcQPYQzNCkUxUypCaa5ZegCVutKm+s= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= +gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= +gopkg.in/yaml.v2 v2.3.0/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= diff --git a/.bingo/variables.env b/.bingo/variables.env index cbf53ba655..c6aac05fb7 100644 --- a/.bingo/variables.env +++ b/.bingo/variables.env @@ -24,6 +24,8 @@ GOLANGCI_LINT="${GOBIN}/golangci-lint-v1.64.6" GOVULNCHECK="${GOBIN}/govulncheck-v1.1.4" +GOWRAP="${GOBIN}/gowrap-v1.4.3" + MOCKERY="${GOBIN}/mockery-v3.4.0" MUTAGEN="${GOBIN}/mutagen-v0.18.1" diff --git a/mise.toml b/mise.toml index 0c768e41b1..a58c1020bd 100644 --- a/mise.toml +++ b/mise.toml @@ -4,6 +4,9 @@ node = "24" pnpm = "11.1.3" "go:github.com/go-delve/delve/cmd/dlv" = "1.27.0" "aqua:nats-io/natscli" = "0.4.0" +"go:github.com/hexdigest/gowrap/cmd/gowrap" = "1.4.3" +k6 = "2.2.0" +ginkgo = "latest" [tasks.build] description = "build" diff --git a/services/graph/Makefile b/services/graph/Makefile index ddd0dc830a..c151a51850 100644 --- a/services/graph/Makefile +++ b/services/graph/Makefile @@ -13,7 +13,8 @@ include ../../.make/release.mk include ../../.make/docs.mk .PHONY: go-generate -go-generate: $(MOCKERY) +go-generate: $(MOCKERY) $(GOWRAP) + go generate ./... $(MOCKERY) .PHONY: l10n-pull diff --git a/services/graph/README.md b/services/graph/README.md index f6ea85fe3b..20507b7c98 100644 --- a/services/graph/README.md +++ b/services/graph/README.md @@ -193,6 +193,12 @@ To specialize `graph` service instances in order to scale them independently, it ## Metrics +Metrics are disabled by default, and must be enabled using the following environment variables: + +* `GRAPH_LDAP_METRICS_DISABLE`: set to `false` to enable metrics for the duration of outbound LDAP client operations (defaults to `true`) +* `GRAPH_HTTP_METRICS_DISABLE`: set to `false` to enable metrics for the duration of inbound Graph HTTP API requests (defaults to `true`) +* `GRAPH_IDENTITY_BACKEND_METRICS_DISABLE`: set to `false` to enable metrics for the duration of Graph identity backend operations (defaults to `true`) + The `graph` service provides the following metrics: | Name | Description | @@ -203,3 +209,70 @@ The `graph` service provides the following metrics: | `opencloud_graph_events{event=...,result=...}` | Counts the number of events that have been consumed, with a `event` label that contains the name of the event, and a `result` label that is set to `success` or `failure` | | `opencloud_graph_events_invalid` | Counts the number of invalid events that are malformed or are missing required data | | `opencloud_graph_events_unsupported` | Counts the numbef of consumed events that cannot be processes by this service, should always be `0` | +| `opencloud_graph_user_password_changes{result=...,reason=...}` | Counts the number of user password change attempts, including the reason for failure when `result`=`failure` | +| `opencloud_graph_http_request_duration_seconds{method=...,path=...,version=...,resource=...,code=...,result=...}` | Histogram that measures the duration of Graph HTTP API requests, in buckets | +| `opencloud_graph_http_requests` | Gauge that counts the number of concurrent inbound HTTP requests to the Graph API | +| `opencloud_graph_ldap_client_operation_duration_seconds{uri=...,write=...,operation=...,result=...}` | Histogram that measures the duration of outbound LDAP operations | +| `opencloud_graph_ldap_client_operations{uri=...,write=...}` | Gauge that counts the number of concurrent outbound LDAP operations | +| `opencloud_graph_identity_backend_api_duration_seconds{type=...,operation=...,result=...}` | Histogram that measures the duration of requests to the Graph identity backend, in buckets | + +To create some moderate load on a running `opencloud` instance, one can use the k6 script `load_test.js` as follows: + +```bash +k6 run --vus=10 --duration=3m ./load_test.js +``` + +The following environment variables can be used to influence its behavior: + + * `BASE_URL`: defaults to `https://localhost:9200` + * `USERNAME`: defaults to `alan` + * `PASSWORD`: defaults to `demo` + +For example, to use a different user and a different URL: + +```bash +k6 run --vus=10 --duration=3m -e USERNAME=lynn -e BASE_URL=https://localhost:9201 ./load_test.js +``` + +It is not meant to be a feature test suite, but merely a small k6 script to generate some read-only load in order to make Grafana displays. + +### Graph User Password Change Counter Metric + +For `opencloud_graph_user_password_changes`: + +* `result` is either + * `success`: when the password was changed successfully + * `failure`: when the password could not be changed, the reason being tracked in the `reason` label +* `reason` is either + * empty when `result` is `success` + * `invalid`: when parameters were invalid, such as the new password being an empty password + * `error`: when an error prevented the password change, such as a network failure + * `wrong-password`: when the password change was refused because the current password is wrong + +### Graph Inbound HTTP Request Duration Metrics + +For `opencloud_graph_http_request_duration_seconds`: + +* `method` is the HTTP method (`GET`, `PUT`, ...) +* `path` is the canonical request path with placeholders (e.g. `/v1beta1/drives/{driveID}/root/children`) +* `version` is the Graph API version (`v1beta` or `v1.0`) +* `resource` is the top-level resource after the version (`me`, `application`, `drives`, ...) +* `code` is the resulting HTTP status code (`200`, `404`, `500`, ...) +* `result` is one of `success`, `client-error`, `server-error` + +### Graph Outbound LDAP Operation Duration Metrics + +For `opencloud_graph_ldap_client_operation_duration_seconds`: + +* `operation` is the name of the LDAP operation (`add`, `delete`, `modify`, `modify-dn`, ...) +* `result` is either `success`, `failure`, `read-only` (when attempting a write operation on a LDAP server that is configured as read-only in OpenCloud) or `not-found` +* `uri` contains the LDAP server URI the client is connected to +* `write` is set to `1` if the LDAP client is allowed to perform write operations, or to `0` if it is configured to be read-only + +### Graph Identity Backend API Duration Metrics + +* `type` is the type of the identity backend that is being used (`ldap` or `cs3`) +* `operation` is the name of the API operation (`create-user`, `get-users`, ...) +* `result` is `success`, `failure` or `not-found` + + diff --git a/services/graph/load_test.js b/services/graph/load_test.js new file mode 100644 index 0000000000..3030f63de4 --- /dev/null +++ b/services/graph/load_test.js @@ -0,0 +1,80 @@ +// Small k6 script to generate some load on read-only endpoints of +// the Graph API, for showcasing the metrics. + +import http from 'k6/http'; +import { check, sleep } from 'k6'; +import encoding from 'k6/encoding'; + +// Configuration via environment variables with defaults +const BASE_URL = __ENV.BASE_URL || 'https://localhost:9200'; +const USERNAME = __ENV.USERNAME || 'alan'; +const PASSWORD = __ENV.PASSWORD || 'demo'; + +export const options = { + insecureSkipTLSVerify: true, + vus: 10, + thresholds: { + http_req_failed: ['rate<0.01'], + http_req_duration: ['p(95)<500'], + }, +}; + +const credentials = `${USERNAME}:${PASSWORD}`; +const encodedCredentials = encoding.b64encode(credentials); + +const params = { + headers: { + 'Authorization': `Basic ${encodedCredentials}`, + 'Accept': 'application/json', + }, +}; + +export default function () { + // Fetch current user profile, including the list of groups the user is part of + let resMe = http.get(`${BASE_URL}/graph/v1.0/me?$expand=memberOf`, params); + const meOk = check(resMe, { 'GET /me status is 200': (r) => r.status === 200 }); + sleep(0.1); + // extract the names of the groups the user is part of, because the user is allowed + // to retrieve information about those + let groupNames = []; + if (meOk && resMe.json() && resMe.json().memberOf) { + groupNames = (resMe.json().memberOf || []).map((group) => group.displayName); + } + + // Fetch oneself using the users search API: + let resUsers = http.get(`${BASE_URL}/graph/v1.0/users?$search="${USERNAME}"`, params); + check(resUsers, { 'GET /users status is 200': (r) => r.status === 200 }); + sleep(0.1); + + // Fetch storage drives + let resDrives = http.get(`${BASE_URL}/graph/v1.0/drives`, params); + const drivesOk = check(resDrives, { + 'GET /drives status is 200': (r) => r.status === 200, + }); + sleep(0.1); + + // For each of those drives, retrieve deeper information about each + if (drivesOk && resDrives.json() && resDrives.json().value) { + const drives = resDrives.json().value; + + if (drives.length > 0) { + const driveId = drives[0].id; + let resDrive = http.get(`${BASE_URL}/graph/v1.0/drives/${driveId}`, params); + + check(resDrive, { + 'GET /drives/{id} status is 200': (r) => r.status === 200, + }); + } + } + + // For each of the groups the user is part of, retrieve information about each of them + // using the group searching endpoint + for (const group of groupNames) { + let resGroups = http.get(`${BASE_URL}/graph/v1.0/groups?$search="${group}"`, params); + const groupsOk = check(resGroups, { + 'GET /groups status is 200': (r) => r.status === 200, + }); + } + + sleep(0.2); +} diff --git a/services/graph/pkg/command/server.go b/services/graph/pkg/command/server.go index 3a581124ee..e6690f84b1 100644 --- a/services/graph/pkg/command/server.go +++ b/services/graph/pkg/command/server.go @@ -20,6 +20,7 @@ import ( "github.com/opencloud-eu/opencloud/services/graph/pkg/server/debug" "github.com/opencloud-eu/opencloud/services/graph/pkg/server/http" evc "github.com/opencloud-eu/opencloud/services/graph/pkg/service/events" + svc "github.com/opencloud-eu/opencloud/services/graph/pkg/service/v0" "github.com/opencloud-eu/reva/v2/pkg/events" "github.com/opencloud-eu/reva/v2/pkg/events/stream" "github.com/prometheus/client_golang/prometheus" @@ -52,7 +53,14 @@ func Server(cfg *config.Config) *cobra.Command { } ctx := cfg.Context - mtrcs := metrics.New(prometheus.DefaultRegisterer) + prom := prometheus.DefaultRegisterer + + // note that the function we pass here is tasked with decomposing Graph HTTP API + // request URL patterns into information that is then used for labels in metrics + // to track HTTP request processing durations, and it is located there to be close + // to the HTTP API route definitions, to improve chances of adapting it accordingly + // whenever those routes should change in the future + mtrcs := metrics.New(prom, svc.DecomposeGraphApiRequestPattern) mtrcs.BuildInfo.WithLabelValues(version.GetString()).Set(1) var kv jetstream.KeyValue @@ -84,12 +92,20 @@ func Server(cfg *config.Config) *cobra.Command { } } + identityBackendName := cfg.Identity.Backend // contains the name of the backend implementation to use + + // since the identity backend in use is of prime importance to understand issues through logs, every + // log entry should contain a 'backend' entry with the name of the backend in use from here on: + logger = log.Logger{Logger: logger.With().Str("backend", identityBackendName).Logger()} + identityBackend, eduBackend, err := identity.CreateIdentityBackends( - cfg.Identity.Backend, + identityBackendName, cfg, &logger, + prom, traceProvider, ) + if err != nil { logger.Error().Err(err).Msg("Error initializing the identity backend") return fmt.Errorf("could not initialize identity backend: %w", err) diff --git a/services/graph/pkg/config/config.go b/services/graph/pkg/config/config.go index 1c01c308cb..2a84a5a4e9 100644 --- a/services/graph/pkg/config/config.go +++ b/services/graph/pkg/config/config.go @@ -60,6 +60,10 @@ type Spaces struct { TranslationPath string `yaml:"translation_path" env:"OC_TRANSLATION_PATH;GRAPH_TRANSLATION_PATH" desc:"(optional) Set this to a path with custom translations to overwrite the builtin translations. Note that file and folder naming rules apply, see the documentation for more details." introductionVersion:"1.0.0"` } +type LDAPMetrics struct { + Disabled bool `yaml:"disabled" env:"GRAPH_LDAP_METRICS_DISABLE" desc:"Disables the metrics for outbound LDAP operations." introductionVersion:"%NEXT%"` +} + type LDAP struct { URI string `yaml:"uri" env:"OC_LDAP_URI;GRAPH_LDAP_URI" desc:"URI of the LDAP Server to connect to. Supported URI schemes are 'ldaps://' and 'ldap://'" introductionVersion:"1.0.0"` CACert string `yaml:"cacert" env:"OC_LDAP_CACERT;GRAPH_LDAP_CACERT" desc:"Path/File name for the root CA certificate (in PEM format) used to validate TLS server certificates of the LDAP service. If not defined, the root directory derives from $OC_BASE_DATA_PATH/idm." introductionVersion:"1.0.0"` @@ -97,6 +101,8 @@ type LDAP struct { EducationResourcesEnabled bool `yaml:"education_resources_enabled" env:"GRAPH_LDAP_EDUCATION_RESOURCES_ENABLED" desc:"Enable LDAP support for managing education related resources." introductionVersion:"1.0.0"` EducationConfig LDAPEducationConfig + + Metrics LDAPMetrics `yaml:"metrics"` } // LDAPEducationConfig represents the LDAP configuration for education related resources @@ -114,9 +120,14 @@ type LDAPEducationConfig struct { SchoolTerminationGraceDays int `yaml:"school_termination_min_grace_days" env:"GRAPH_LDAP_SCHOOL_TERMINATION_MIN_GRACE_DAYS" desc:"When setting a 'terminationDate' for a school, require the date to be at least this number of days in the future." introductionVersion:"1.0.0"` } +type IdentityMetrics struct { + Disabled bool `yaml:"disabled" env:"GRAPH_IDENTITY_BACKEND_METRICS_DISABLE" desc:"Disables the metrics for inbound identity backend operations." introductionVersion:"%NEXT%"` +} + type Identity struct { - Backend string `yaml:"backend" env:"GRAPH_IDENTITY_BACKEND" desc:"The user identity backend to use. Supported backend types are 'ldap' and 'cs3'." introductionVersion:"1.0.0"` - LDAP LDAP `yaml:"ldap"` + Backend string `yaml:"backend" env:"GRAPH_IDENTITY_BACKEND" desc:"The user identity backend to use. Supported backend types are 'ldap' and 'cs3'." introductionVersion:"1.0.0"` + LDAP LDAP `yaml:"ldap"` + Metrics IdentityMetrics `yaml:"metrics"` } // API represents API configuration parameters. diff --git a/services/graph/pkg/config/defaults/defaultconfig.go b/services/graph/pkg/config/defaults/defaultconfig.go index 7c3c5200f4..159b7db510 100644 --- a/services/graph/pkg/config/defaults/defaultconfig.go +++ b/services/graph/pkg/config/defaults/defaultconfig.go @@ -53,6 +53,11 @@ func DefaultConfig() *config.Config { AllowedHeaders: []string{"Authorization", "Origin", "Content-Type", "Accept", "X-Requested-With", "X-Request-Id", "Purge", "Restore"}, AllowCredentials: true, }, + Metrics: config.HTTPMetrics{ + // disabling inbound HTTP metrics collection by default for now, since the runtime performance impact is currently unclear; + // it is most likely to be negligible, but has not been measured yet to confirm + Disabled: true, + }, }, Service: config.Service{ Name: "graph", @@ -81,6 +86,12 @@ func DefaultConfig() *config.Config { }, Identity: config.Identity{ Backend: "ldap", + Metrics: config.IdentityMetrics{ + // disabling identity backend opcall metrics collection by default for now, since + // the runtime performance impact is currently unclear; + // it is most likely to be negligible, but has not been measured yet to confirm + Disabled: true, + }, LDAP: config.LDAP{ URI: "ldap://localhost:9236", Insecure: false, @@ -110,6 +121,11 @@ func DefaultConfig() *config.Config { GroupMemberAttribute: "member", GroupIDAttribute: "openCloudUUID", EducationResourcesEnabled: false, + Metrics: config.LDAPMetrics{ + // disabling inbound HTTP metrics collection by default for now, since the runtime performance impact is currently unclear; + // it is most likely to be negligible, but has not been measured yet to confirm + Disabled: true, + }, }, }, Cache: &config.Cache{ diff --git a/services/graph/pkg/config/http.go b/services/graph/pkg/config/http.go index 4859fa69f0..98f49f3f40 100644 --- a/services/graph/pkg/config/http.go +++ b/services/graph/pkg/config/http.go @@ -2,6 +2,10 @@ package config import "github.com/opencloud-eu/opencloud/pkg/shared" +type HTTPMetrics struct { + Disabled bool `yaml:"disabled" env:"GRAPH_HTTP_METRICS_DISABLE" desc:"Disables the metrics for the HTTP service." introductionVersion:"%NEXT%"` +} + // HTTP defines the available http configuration. type HTTP struct { Disabled bool `yaml:"disabled" env:"GRAPH_HTTP_DISABLE" desc:"Disables the HTTP service. Set this to true if the service should only consume events." introductionVersion:"%NEXT%"` @@ -11,4 +15,5 @@ type HTTP struct { TLS shared.HTTPServiceTLS `yaml:"tls"` APIToken string `yaml:"apitoken" env:"GRAPH_HTTP_API_TOKEN" desc:"An optional API bearer token" introductionVersion:"1.0.0"` CORS CORS `yaml:"cors"` + Metrics HTTPMetrics `yaml:"metrics"` } diff --git a/services/graph/pkg/config/parser/parse.go b/services/graph/pkg/config/parser/parse.go index 58f6a77173..8ab051624f 100644 --- a/services/graph/pkg/config/parser/parse.go +++ b/services/graph/pkg/config/parser/parse.go @@ -44,7 +44,7 @@ func Validate(cfg *config.Config) error { // as the process wouldn't be able to serve either API and would thus be // completely useless -- in that case, just don't start this service // in the first place (especially since it's optional) - return errors.New("both HTTP and events consumption APIs are disabled by configuration; at least one must be enabled") + return shared.AllComponentsDisabledError("graph") } if cfg.TokenManager.JWTSecret == "" { diff --git a/services/graph/pkg/errorcode/errorcode.go b/services/graph/pkg/errorcode/errorcode.go index cc9cce63aa..0347d83dbb 100644 --- a/services/graph/pkg/errorcode/errorcode.go +++ b/services/graph/pkg/errorcode/errorcode.go @@ -50,6 +50,8 @@ const ( InvalidRequest // ItemNotFound defines the error if the resource could not be found. ItemNotFound + // TooManyResults defines the error if multiple results are found for a unique resource. + TooManyResults // MalwareDetected defines the error if malware was detected in the requested resource. MalwareDetected // NameAlreadyExists defines the error if the specified item name already exists. @@ -84,6 +86,7 @@ var errorCodes = [...]string{ "invalidRange", "invalidRequest", "itemNotFound", + "tooManyResults", "malwareDetected", "nameAlreadyExists", "notAllowed", @@ -206,3 +209,16 @@ func ToError(err error) (Error, bool) { return Error{}, false } + +// Returns true if the error is of type Error and has an ErrorCode that matches +// the one specified as the second parameter, and false if not. +func IsErrorCode(err error, code ErrorCode) bool { + if err == nil { + return false + } + if e, ok := ToError(err); ok { + return e.errorCode == code + } else { + return false + } +} diff --git a/services/graph/pkg/identity/backend.go b/services/graph/pkg/identity/backend.go index 342af7ade9..f9f9a761d5 100644 --- a/services/graph/pkg/identity/backend.go +++ b/services/graph/pkg/identity/backend.go @@ -1,5 +1,8 @@ package identity +//go:generate gowrap gen -g -i Backend -t ./backend_prometheus.tmpl -o backend_prometheus.go +//go:generate gowrap gen -g -i EducationBackend -t ./backend_prometheus.tmpl -o education_backend_prometheus.go + import ( "context" "net/url" @@ -18,6 +21,8 @@ var ( ErrReadOnly = errorcode.New(errorcode.NotAllowed, "server is configured read-only") // ErrNotFound signals that the requested resource was not found. ErrNotFound = errorcode.New(errorcode.ItemNotFound, "not found") + // ErrTooManyResults signals that multiple results were found when only one was expected + ErrTooManyResults = errorcode.New(errorcode.TooManyResults, "too many results") // ErrUnsupportedFilter signals that the requested filter is not supported by the backend. ErrUnsupportedFilter = godata.NotImplementedError("unsupported filter") ) @@ -28,6 +33,24 @@ const ( UserTypeFederated = "Federated" ) +const ( + MetricOpCreateUser = "create-user" + MetricOpDeleteUser = "delete-user" + MetricOpUpdateUser = "update-user" + MetricOpGetUser = "get-user" + MetricOpGetUsers = "get-users" + MetricOpFilterUsers = "filter-users" + MetricOpUpdateLastSignInDate = "update-last-signin-date" + MetricOpGetGroup = "get-group" + MetricOpGetGroups = "get-groups" + MetricOpCreateGroup = "create-group" + MetricOpDeleteGroup = "delete-group" + MetricOpUpdateGroupName = "update-group-name" + MetricOpAddMembersToGroup = "add-members-to-group" + MetricOpRemoveMemberFromGroup = "remove-member-from-group" + MetricOpGetGroupMembers = "get-group-members" +) + // Backend defines the Interface for an IdentityBackend implementation type Backend interface { // CreateUser creates a given user in the identity backend. @@ -58,6 +81,36 @@ type Backend interface { RemoveMemberFromGroup(ctx context.Context, groupID string, memberID string) error } +const ( + MetricOpCreateEducationSchool = "create-school" + MetricOpUpdateEducationSchool = "update-school" + MetricOpDeleteEducationSchool = "delete-school" + MetricOpGetEducationSchool = "get-school" + MetricOpGetEducationSchools = "get-schools" + MetricOpFilterEducationSchoolsByAttribute = "filter-schools-byattr" + MetricOpAddUsersToEducationSchool = "add-eduusers-to-school" + MetricOpRemoveUserFromEducationSchool = "remove-eduser-from-school" + MetricOpGetEducationSchoolClasses = "get-school-classes" + MetricOpAddClassesToEducationSchool = "add-classes-to-school" + MetricOpRemoveClassFromEducationSchool = "remove-class-from-school" + MetricOpAddTeacherToEducationClass = "add-teacher-to-class" + MetricOpCreateEducationUser = "create-eduser" + MetricOpDeleteEducationClass = "delete-class" + MetricOpDeleteEducationUser = "delete-eduser" + MetricOpFilterEducationUsersByAttribute = "filter-edusers" + MetricOpGetEducationClass = "get-class" + MetricOpGetEducationClassMembers = "get-class-members" + MetricOpGetEducationClassTeachers = "get-class-teachers" + MetricOpGetEducationClasses = "get-classes" + MetricOpGetEducationSchoolUsers = "get-school-edusers" + MetricOpGetEducationUser = "get-eduser" + MetricOpGetEducationUsers = "get-edusers" + MetricOpUpdateEducationUser = "update-eduser" + MetricOpRemoveTeacherFromEducationClass = "remove-teacher-from-class" + MetricOpUpdateEducationClass = "update-class" + MetricOpCreateEducationClass = "create-class" +) + // EducationBackend defines the Interface for an EducationBackend implementation type EducationBackend interface { // CreateEducationSchool creates the supplied school in the identity backend. diff --git a/services/graph/pkg/identity/backend_prometheus.go b/services/graph/pkg/identity/backend_prometheus.go new file mode 100644 index 0000000000..51f4aeeb55 --- /dev/null +++ b/services/graph/pkg/identity/backend_prometheus.go @@ -0,0 +1,500 @@ +// Code generated by gowrap. DO NOT EDIT. +// template: backend_prometheus.tmpl +// gowrap: http://github.com/hexdigest/gowrap + +package identity + +import ( + "context" + "net/url" + "time" + + "github.com/CiscoM31/godata" + libregraph "github.com/opencloud-eu/libre-graph-api-go" + "github.com/opencloud-eu/opencloud/services/graph/pkg/errorcode" + "github.com/prometheus/client_golang/prometheus" +) + +// PrometheusBackend implements Backend interface with all methods wrapped +// with Prometheus metrics +type PrometheusBackend struct { + delegate Backend + metric *prometheus.HistogramVec +} + +var _ Backend = &PrometheusBackend{} + +// returns an instance of the Backend decorated with prometheus metric +func NewPrometheusBackend(delegate Backend, metric *prometheus.HistogramVec) PrometheusBackend { + return PrometheusBackend{ + delegate: delegate, + metric: metric, + } +} + +// AddMembersToGroup implements Backend.AddMembersToGroup +func (_d PrometheusBackend) AddMembersToGroup(ctx context.Context, groupID string, memberID []string) (err error) { + _since := time.Now() + defer func() { + duration := time.Since(_since).Seconds() + result := MetricResultSuccess + if err != nil { + result = MetricResultFailure + if err == ErrReadOnly { + result = MetricResultReadOnly + } + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + } else { + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + + } + + _d.metric.WithLabelValues(MetricOpAddMembersToGroup, result).Observe(duration) + }() + return _d.delegate.AddMembersToGroup(ctx, groupID, memberID) +} + +// CreateGroup implements Backend.CreateGroup +func (_d PrometheusBackend) CreateGroup(ctx context.Context, group libregraph.Group) (gp1 *libregraph.Group, err error) { + _since := time.Now() + defer func() { + duration := time.Since(_since).Seconds() + result := MetricResultSuccess + if err != nil { + result = MetricResultFailure + if err == ErrReadOnly { + result = MetricResultReadOnly + } + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + } else { + + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + + } + + _d.metric.WithLabelValues(MetricOpCreateGroup, result).Observe(duration) + }() + return _d.delegate.CreateGroup(ctx, group) +} + +// CreateUser implements Backend.CreateUser +func (_d PrometheusBackend) CreateUser(ctx context.Context, user libregraph.User) (up1 *libregraph.User, err error) { + _since := time.Now() + defer func() { + duration := time.Since(_since).Seconds() + result := MetricResultSuccess + if err != nil { + result = MetricResultFailure + if err == ErrReadOnly { + result = MetricResultReadOnly + } + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + } else { + + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + + } + + _d.metric.WithLabelValues(MetricOpCreateUser, result).Observe(duration) + }() + return _d.delegate.CreateUser(ctx, user) +} + +// DeleteGroup implements Backend.DeleteGroup +func (_d PrometheusBackend) DeleteGroup(ctx context.Context, id string) (err error) { + _since := time.Now() + defer func() { + duration := time.Since(_since).Seconds() + result := MetricResultSuccess + if err != nil { + result = MetricResultFailure + if err == ErrReadOnly { + result = MetricResultReadOnly + } + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + } else { + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + + } + + _d.metric.WithLabelValues(MetricOpDeleteGroup, result).Observe(duration) + }() + return _d.delegate.DeleteGroup(ctx, id) +} + +// DeleteUser implements Backend.DeleteUser +func (_d PrometheusBackend) DeleteUser(ctx context.Context, nameOrID string) (err error) { + _since := time.Now() + defer func() { + duration := time.Since(_since).Seconds() + result := MetricResultSuccess + if err != nil { + result = MetricResultFailure + if err == ErrReadOnly { + result = MetricResultReadOnly + } + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + } else { + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + + } + + _d.metric.WithLabelValues(MetricOpDeleteUser, result).Observe(duration) + }() + return _d.delegate.DeleteUser(ctx, nameOrID) +} + +// FilterUsers implements Backend.FilterUsers +func (_d PrometheusBackend) FilterUsers(ctx context.Context, oreq *godata.GoDataRequest, filter *godata.ParseNode) (upa1 []*libregraph.User, err error) { + _since := time.Now() + defer func() { + duration := time.Since(_since).Seconds() + result := MetricResultSuccess + if err != nil { + result = MetricResultFailure + if err == ErrReadOnly { + result = MetricResultReadOnly + } + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + } else { + + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + + } + + _d.metric.WithLabelValues(MetricOpFilterUsers, result).Observe(duration) + }() + return _d.delegate.FilterUsers(ctx, oreq, filter) +} + +// GetGroup implements Backend.GetGroup +func (_d PrometheusBackend) GetGroup(ctx context.Context, nameOrID string, queryParam url.Values) (gp1 *libregraph.Group, err error) { + _since := time.Now() + defer func() { + duration := time.Since(_since).Seconds() + result := MetricResultSuccess + if err != nil { + result = MetricResultFailure + if err == ErrReadOnly { + result = MetricResultReadOnly + } + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + } else { + // it's a get operation that returns a pointer (and not an array): check whether that's nil or not + if gp1 == nil { + result = MetricResultNotFound + } + + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + + } + + _d.metric.WithLabelValues(MetricOpGetGroup, result).Observe(duration) + }() + return _d.delegate.GetGroup(ctx, nameOrID, queryParam) +} + +// GetGroupMembers implements Backend.GetGroupMembers +func (_d PrometheusBackend) GetGroupMembers(ctx context.Context, id string, oreq *godata.GoDataRequest) (upa1 []*libregraph.User, err error) { + _since := time.Now() + defer func() { + duration := time.Since(_since).Seconds() + result := MetricResultSuccess + if err != nil { + result = MetricResultFailure + if err == ErrReadOnly { + result = MetricResultReadOnly + } + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + } else { + + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + + } + + _d.metric.WithLabelValues(MetricOpGetGroupMembers, result).Observe(duration) + }() + return _d.delegate.GetGroupMembers(ctx, id, oreq) +} + +// GetGroups implements Backend.GetGroups +func (_d PrometheusBackend) GetGroups(ctx context.Context, oreq *godata.GoDataRequest) (gpa1 []*libregraph.Group, err error) { + _since := time.Now() + defer func() { + duration := time.Since(_since).Seconds() + result := MetricResultSuccess + if err != nil { + result = MetricResultFailure + if err == ErrReadOnly { + result = MetricResultReadOnly + } + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + } else { + + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + + } + + _d.metric.WithLabelValues(MetricOpGetGroups, result).Observe(duration) + }() + return _d.delegate.GetGroups(ctx, oreq) +} + +// GetUser implements Backend.GetUser +func (_d PrometheusBackend) GetUser(ctx context.Context, nameOrID string, oreq *godata.GoDataRequest) (up1 *libregraph.User, err error) { + _since := time.Now() + defer func() { + duration := time.Since(_since).Seconds() + result := MetricResultSuccess + if err != nil { + result = MetricResultFailure + if err == ErrReadOnly { + result = MetricResultReadOnly + } + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + } else { + // it's a get operation that returns a pointer (and not an array): check whether that's nil or not + if up1 == nil { + result = MetricResultNotFound + } + + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + + } + + _d.metric.WithLabelValues(MetricOpGetUser, result).Observe(duration) + }() + return _d.delegate.GetUser(ctx, nameOrID, oreq) +} + +// GetUsers implements Backend.GetUsers +func (_d PrometheusBackend) GetUsers(ctx context.Context, oreq *godata.GoDataRequest) (upa1 []*libregraph.User, err error) { + _since := time.Now() + defer func() { + duration := time.Since(_since).Seconds() + result := MetricResultSuccess + if err != nil { + result = MetricResultFailure + if err == ErrReadOnly { + result = MetricResultReadOnly + } + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + } else { + + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + + } + + _d.metric.WithLabelValues(MetricOpGetUsers, result).Observe(duration) + }() + return _d.delegate.GetUsers(ctx, oreq) +} + +// RemoveMemberFromGroup implements Backend.RemoveMemberFromGroup +func (_d PrometheusBackend) RemoveMemberFromGroup(ctx context.Context, groupID string, memberID string) (err error) { + _since := time.Now() + defer func() { + duration := time.Since(_since).Seconds() + result := MetricResultSuccess + if err != nil { + result = MetricResultFailure + if err == ErrReadOnly { + result = MetricResultReadOnly + } + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + } else { + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + + } + + _d.metric.WithLabelValues(MetricOpRemoveMemberFromGroup, result).Observe(duration) + }() + return _d.delegate.RemoveMemberFromGroup(ctx, groupID, memberID) +} + +// UpdateGroupName implements Backend.UpdateGroupName +func (_d PrometheusBackend) UpdateGroupName(ctx context.Context, groupID string, groupName string) (err error) { + _since := time.Now() + defer func() { + duration := time.Since(_since).Seconds() + result := MetricResultSuccess + if err != nil { + result = MetricResultFailure + if err == ErrReadOnly { + result = MetricResultReadOnly + } + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + } else { + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + + } + + _d.metric.WithLabelValues(MetricOpUpdateGroupName, result).Observe(duration) + }() + return _d.delegate.UpdateGroupName(ctx, groupID, groupName) +} + +// UpdateLastSignInDate implements Backend.UpdateLastSignInDate +func (_d PrometheusBackend) UpdateLastSignInDate(ctx context.Context, userID string, timestamp time.Time) (err error) { + _since := time.Now() + defer func() { + duration := time.Since(_since).Seconds() + result := MetricResultSuccess + if err != nil { + result = MetricResultFailure + if err == ErrReadOnly { + result = MetricResultReadOnly + } + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + } else { + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + + } + + _d.metric.WithLabelValues(MetricOpUpdateLastSignInDate, result).Observe(duration) + }() + return _d.delegate.UpdateLastSignInDate(ctx, userID, timestamp) +} + +// UpdateUser implements Backend.UpdateUser +func (_d PrometheusBackend) UpdateUser(ctx context.Context, nameOrID string, user libregraph.UserUpdate) (up1 *libregraph.User, err error) { + _since := time.Now() + defer func() { + duration := time.Since(_since).Seconds() + result := MetricResultSuccess + if err != nil { + result = MetricResultFailure + if err == ErrReadOnly { + result = MetricResultReadOnly + } + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + } else { + + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + + } + + _d.metric.WithLabelValues(MetricOpUpdateUser, result).Observe(duration) + }() + return _d.delegate.UpdateUser(ctx, nameOrID, user) +} diff --git a/services/graph/pkg/identity/backend_prometheus.tmpl b/services/graph/pkg/identity/backend_prometheus.tmpl new file mode 100644 index 0000000000..3837f15d07 --- /dev/null +++ b/services/graph/pkg/identity/backend_prometheus.tmpl @@ -0,0 +1,73 @@ +import ( + "errors" + "time" + + "github.com/go-ldap/ldap/v3" + "github.com/opencloud-eu/opencloud/services/graph/pkg/metrics" + "github.com/prometheus/client_golang/prometheus" +) + +{{ $decorator := (or .Vars.DecoratorName (printf "Prometheus%s" .Interface.Name)) }} + +// {{$decorator}} implements {{.Interface.Type}} interface with all methods wrapped +// with Prometheus metrics +type {{$decorator}} struct { + delegate {{.Interface.Type}} + metric *prometheus.HistogramVec +} + +var _ {{.Interface.Type}} = &{{$decorator}}{} + +// returns an instance of the {{.Interface.Type}} decorated with prometheus metric +func New{{$decorator}}(delegate {{.Interface.Type}}, metric *prometheus.HistogramVec) {{$decorator}} { + return {{$decorator}} { + delegate: delegate, + metric: metric, + } +} + +{{range $method := .Interface.Methods}} + // {{$method.Name}} implements {{$.Interface.Type}}.{{$method.Name}} + func (_d {{$decorator}}) {{$method.Declaration}} { + _since := time.Now() + defer func() { + duration := time.Since(_since).Seconds() + result := MetricResultSuccess + {{- if $method.ReturnsError}} + if err != nil { + result = MetricResultFailure + if err == ErrReadOnly { + result = MetricResultReadOnly + } + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + } else { + {{- if or (hasPrefix "Get" $method.Name) }} + {{- range $i, $result := $method.Results }} + {{- if and (hasPrefix "*" $result.Type) (not (hasPrefix "*[]" $result.Type)) }} + // it's a get operation that returns a pointer (and not an array): check whether that's nil or not + if {{$result.Name}} == nil { + result = MetricResultNotFound + }{{break}} + {{end}} + {{end}} + {{end}} + {{- range $i, $result := $method.Results }} + {{- if eq $result.Type "error" }} + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + {{end}} + {{end}} + } + {{end}} + _d.metric.WithLabelValues(MetricOp{{upFirst $method.Name}}, result).Observe(duration) + }() + {{$method.Pass "_d.delegate."}} + } +{{end}} diff --git a/services/graph/pkg/identity/cs3.go b/services/graph/pkg/identity/cs3.go index 93de4ecb3d..e022890e1c 100644 --- a/services/graph/pkg/identity/cs3.go +++ b/services/graph/pkg/identity/cs3.go @@ -28,6 +28,22 @@ type CS3 struct { GatewaySelector pool.Selectable[gateway.GatewayAPIClient] } +var _ Backend = &CS3{} + +func NewCS3Backend(config *shared.Reva, gatewaySelector pool.Selectable[gateway.GatewayAPIClient], logger *log.Logger) (*CS3, error) { + logger = &log.Logger{Logger: logger.With(). + // Str("backend", "cs3"). // already added upstream + Str("gateway", config.Address). + Logger(), + } + + return &CS3{ + Config: config, + GatewaySelector: gatewaySelector, + Logger: logger, + }, nil +} + // CreateUser implements the Backend Interface. It's currently not supported for the CS3 backend func (i *CS3) CreateUser(ctx context.Context, user libregraph.User) (*libregraph.User, error) { return nil, errNotImplemented diff --git a/services/graph/pkg/identity/education_backend_prometheus.go b/services/graph/pkg/identity/education_backend_prometheus.go new file mode 100644 index 0000000000..1de1628b65 --- /dev/null +++ b/services/graph/pkg/identity/education_backend_prometheus.go @@ -0,0 +1,871 @@ +// Code generated by gowrap. DO NOT EDIT. +// template: backend_prometheus.tmpl +// gowrap: http://github.com/hexdigest/gowrap + +package identity + +import ( + "context" + "time" + + libregraph "github.com/opencloud-eu/libre-graph-api-go" + "github.com/opencloud-eu/opencloud/services/graph/pkg/errorcode" + "github.com/prometheus/client_golang/prometheus" +) + +// PrometheusEducationBackend implements EducationBackend interface with all methods wrapped +// with Prometheus metrics +type PrometheusEducationBackend struct { + delegate EducationBackend + metric *prometheus.HistogramVec +} + +var _ EducationBackend = &PrometheusEducationBackend{} + +// returns an instance of the EducationBackend decorated with prometheus metric +func NewPrometheusEducationBackend(delegate EducationBackend, metric *prometheus.HistogramVec) PrometheusEducationBackend { + return PrometheusEducationBackend{ + delegate: delegate, + metric: metric, + } +} + +// AddClassesToEducationSchool implements EducationBackend.AddClassesToEducationSchool +func (_d PrometheusEducationBackend) AddClassesToEducationSchool(ctx context.Context, schoolNumberOrID string, memberIDs []string) (err error) { + _since := time.Now() + defer func() { + duration := time.Since(_since).Seconds() + result := MetricResultSuccess + if err != nil { + result = MetricResultFailure + if err == ErrReadOnly { + result = MetricResultReadOnly + } + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + } else { + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + + } + + _d.metric.WithLabelValues(MetricOpAddClassesToEducationSchool, result).Observe(duration) + }() + return _d.delegate.AddClassesToEducationSchool(ctx, schoolNumberOrID, memberIDs) +} + +// AddTeacherToEducationClass implements EducationBackend.AddTeacherToEducationClass +func (_d PrometheusEducationBackend) AddTeacherToEducationClass(ctx context.Context, classID string, teacherID string) (err error) { + _since := time.Now() + defer func() { + duration := time.Since(_since).Seconds() + result := MetricResultSuccess + if err != nil { + result = MetricResultFailure + if err == ErrReadOnly { + result = MetricResultReadOnly + } + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + } else { + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + + } + + _d.metric.WithLabelValues(MetricOpAddTeacherToEducationClass, result).Observe(duration) + }() + return _d.delegate.AddTeacherToEducationClass(ctx, classID, teacherID) +} + +// AddUsersToEducationSchool implements EducationBackend.AddUsersToEducationSchool +func (_d PrometheusEducationBackend) AddUsersToEducationSchool(ctx context.Context, schoolID string, memberID []string) (err error) { + _since := time.Now() + defer func() { + duration := time.Since(_since).Seconds() + result := MetricResultSuccess + if err != nil { + result = MetricResultFailure + if err == ErrReadOnly { + result = MetricResultReadOnly + } + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + } else { + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + + } + + _d.metric.WithLabelValues(MetricOpAddUsersToEducationSchool, result).Observe(duration) + }() + return _d.delegate.AddUsersToEducationSchool(ctx, schoolID, memberID) +} + +// CreateEducationClass implements EducationBackend.CreateEducationClass +func (_d PrometheusEducationBackend) CreateEducationClass(ctx context.Context, class libregraph.EducationClass) (ep1 *libregraph.EducationClass, err error) { + _since := time.Now() + defer func() { + duration := time.Since(_since).Seconds() + result := MetricResultSuccess + if err != nil { + result = MetricResultFailure + if err == ErrReadOnly { + result = MetricResultReadOnly + } + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + } else { + + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + + } + + _d.metric.WithLabelValues(MetricOpCreateEducationClass, result).Observe(duration) + }() + return _d.delegate.CreateEducationClass(ctx, class) +} + +// CreateEducationSchool implements EducationBackend.CreateEducationSchool +func (_d PrometheusEducationBackend) CreateEducationSchool(ctx context.Context, group libregraph.EducationSchool) (ep1 *libregraph.EducationSchool, err error) { + _since := time.Now() + defer func() { + duration := time.Since(_since).Seconds() + result := MetricResultSuccess + if err != nil { + result = MetricResultFailure + if err == ErrReadOnly { + result = MetricResultReadOnly + } + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + } else { + + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + + } + + _d.metric.WithLabelValues(MetricOpCreateEducationSchool, result).Observe(duration) + }() + return _d.delegate.CreateEducationSchool(ctx, group) +} + +// CreateEducationUser implements EducationBackend.CreateEducationUser +func (_d PrometheusEducationBackend) CreateEducationUser(ctx context.Context, user libregraph.EducationUser) (ep1 *libregraph.EducationUser, err error) { + _since := time.Now() + defer func() { + duration := time.Since(_since).Seconds() + result := MetricResultSuccess + if err != nil { + result = MetricResultFailure + if err == ErrReadOnly { + result = MetricResultReadOnly + } + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + } else { + + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + + } + + _d.metric.WithLabelValues(MetricOpCreateEducationUser, result).Observe(duration) + }() + return _d.delegate.CreateEducationUser(ctx, user) +} + +// DeleteEducationClass implements EducationBackend.DeleteEducationClass +func (_d PrometheusEducationBackend) DeleteEducationClass(ctx context.Context, nameOrID string) (err error) { + _since := time.Now() + defer func() { + duration := time.Since(_since).Seconds() + result := MetricResultSuccess + if err != nil { + result = MetricResultFailure + if err == ErrReadOnly { + result = MetricResultReadOnly + } + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + } else { + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + + } + + _d.metric.WithLabelValues(MetricOpDeleteEducationClass, result).Observe(duration) + }() + return _d.delegate.DeleteEducationClass(ctx, nameOrID) +} + +// DeleteEducationSchool implements EducationBackend.DeleteEducationSchool +func (_d PrometheusEducationBackend) DeleteEducationSchool(ctx context.Context, id string) (err error) { + _since := time.Now() + defer func() { + duration := time.Since(_since).Seconds() + result := MetricResultSuccess + if err != nil { + result = MetricResultFailure + if err == ErrReadOnly { + result = MetricResultReadOnly + } + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + } else { + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + + } + + _d.metric.WithLabelValues(MetricOpDeleteEducationSchool, result).Observe(duration) + }() + return _d.delegate.DeleteEducationSchool(ctx, id) +} + +// DeleteEducationUser implements EducationBackend.DeleteEducationUser +func (_d PrometheusEducationBackend) DeleteEducationUser(ctx context.Context, nameOrID string) (err error) { + _since := time.Now() + defer func() { + duration := time.Since(_since).Seconds() + result := MetricResultSuccess + if err != nil { + result = MetricResultFailure + if err == ErrReadOnly { + result = MetricResultReadOnly + } + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + } else { + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + + } + + _d.metric.WithLabelValues(MetricOpDeleteEducationUser, result).Observe(duration) + }() + return _d.delegate.DeleteEducationUser(ctx, nameOrID) +} + +// FilterEducationSchoolsByAttribute implements EducationBackend.FilterEducationSchoolsByAttribute +func (_d PrometheusEducationBackend) FilterEducationSchoolsByAttribute(ctx context.Context, attr string, value string) (epa1 []*libregraph.EducationSchool, err error) { + _since := time.Now() + defer func() { + duration := time.Since(_since).Seconds() + result := MetricResultSuccess + if err != nil { + result = MetricResultFailure + if err == ErrReadOnly { + result = MetricResultReadOnly + } + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + } else { + + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + + } + + _d.metric.WithLabelValues(MetricOpFilterEducationSchoolsByAttribute, result).Observe(duration) + }() + return _d.delegate.FilterEducationSchoolsByAttribute(ctx, attr, value) +} + +// FilterEducationUsersByAttribute implements EducationBackend.FilterEducationUsersByAttribute +func (_d PrometheusEducationBackend) FilterEducationUsersByAttribute(ctx context.Context, attr string, value string) (epa1 []*libregraph.EducationUser, err error) { + _since := time.Now() + defer func() { + duration := time.Since(_since).Seconds() + result := MetricResultSuccess + if err != nil { + result = MetricResultFailure + if err == ErrReadOnly { + result = MetricResultReadOnly + } + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + } else { + + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + + } + + _d.metric.WithLabelValues(MetricOpFilterEducationUsersByAttribute, result).Observe(duration) + }() + return _d.delegate.FilterEducationUsersByAttribute(ctx, attr, value) +} + +// GetEducationClass implements EducationBackend.GetEducationClass +func (_d PrometheusEducationBackend) GetEducationClass(ctx context.Context, namedOrID string) (ep1 *libregraph.EducationClass, err error) { + _since := time.Now() + defer func() { + duration := time.Since(_since).Seconds() + result := MetricResultSuccess + if err != nil { + result = MetricResultFailure + if err == ErrReadOnly { + result = MetricResultReadOnly + } + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + } else { + // it's a get operation that returns a pointer (and not an array): check whether that's nil or not + if ep1 == nil { + result = MetricResultNotFound + } + + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + + } + + _d.metric.WithLabelValues(MetricOpGetEducationClass, result).Observe(duration) + }() + return _d.delegate.GetEducationClass(ctx, namedOrID) +} + +// GetEducationClassMembers implements EducationBackend.GetEducationClassMembers +func (_d PrometheusEducationBackend) GetEducationClassMembers(ctx context.Context, nameOrID string) (epa1 []*libregraph.EducationUser, err error) { + _since := time.Now() + defer func() { + duration := time.Since(_since).Seconds() + result := MetricResultSuccess + if err != nil { + result = MetricResultFailure + if err == ErrReadOnly { + result = MetricResultReadOnly + } + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + } else { + + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + + } + + _d.metric.WithLabelValues(MetricOpGetEducationClassMembers, result).Observe(duration) + }() + return _d.delegate.GetEducationClassMembers(ctx, nameOrID) +} + +// GetEducationClassTeachers implements EducationBackend.GetEducationClassTeachers +func (_d PrometheusEducationBackend) GetEducationClassTeachers(ctx context.Context, classID string) (epa1 []*libregraph.EducationUser, err error) { + _since := time.Now() + defer func() { + duration := time.Since(_since).Seconds() + result := MetricResultSuccess + if err != nil { + result = MetricResultFailure + if err == ErrReadOnly { + result = MetricResultReadOnly + } + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + } else { + + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + + } + + _d.metric.WithLabelValues(MetricOpGetEducationClassTeachers, result).Observe(duration) + }() + return _d.delegate.GetEducationClassTeachers(ctx, classID) +} + +// GetEducationClasses implements EducationBackend.GetEducationClasses +func (_d PrometheusEducationBackend) GetEducationClasses(ctx context.Context) (epa1 []*libregraph.EducationClass, err error) { + _since := time.Now() + defer func() { + duration := time.Since(_since).Seconds() + result := MetricResultSuccess + if err != nil { + result = MetricResultFailure + if err == ErrReadOnly { + result = MetricResultReadOnly + } + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + } else { + + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + + } + + _d.metric.WithLabelValues(MetricOpGetEducationClasses, result).Observe(duration) + }() + return _d.delegate.GetEducationClasses(ctx) +} + +// GetEducationSchool implements EducationBackend.GetEducationSchool +func (_d PrometheusEducationBackend) GetEducationSchool(ctx context.Context, nameOrID string) (ep1 *libregraph.EducationSchool, err error) { + _since := time.Now() + defer func() { + duration := time.Since(_since).Seconds() + result := MetricResultSuccess + if err != nil { + result = MetricResultFailure + if err == ErrReadOnly { + result = MetricResultReadOnly + } + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + } else { + // it's a get operation that returns a pointer (and not an array): check whether that's nil or not + if ep1 == nil { + result = MetricResultNotFound + } + + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + + } + + _d.metric.WithLabelValues(MetricOpGetEducationSchool, result).Observe(duration) + }() + return _d.delegate.GetEducationSchool(ctx, nameOrID) +} + +// GetEducationSchoolClasses implements EducationBackend.GetEducationSchoolClasses +func (_d PrometheusEducationBackend) GetEducationSchoolClasses(ctx context.Context, schoolNumberOrID string) (epa1 []*libregraph.EducationClass, err error) { + _since := time.Now() + defer func() { + duration := time.Since(_since).Seconds() + result := MetricResultSuccess + if err != nil { + result = MetricResultFailure + if err == ErrReadOnly { + result = MetricResultReadOnly + } + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + } else { + + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + + } + + _d.metric.WithLabelValues(MetricOpGetEducationSchoolClasses, result).Observe(duration) + }() + return _d.delegate.GetEducationSchoolClasses(ctx, schoolNumberOrID) +} + +// GetEducationSchoolUsers implements EducationBackend.GetEducationSchoolUsers +func (_d PrometheusEducationBackend) GetEducationSchoolUsers(ctx context.Context, id string) (epa1 []*libregraph.EducationUser, err error) { + _since := time.Now() + defer func() { + duration := time.Since(_since).Seconds() + result := MetricResultSuccess + if err != nil { + result = MetricResultFailure + if err == ErrReadOnly { + result = MetricResultReadOnly + } + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + } else { + + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + + } + + _d.metric.WithLabelValues(MetricOpGetEducationSchoolUsers, result).Observe(duration) + }() + return _d.delegate.GetEducationSchoolUsers(ctx, id) +} + +// GetEducationSchools implements EducationBackend.GetEducationSchools +func (_d PrometheusEducationBackend) GetEducationSchools(ctx context.Context) (epa1 []*libregraph.EducationSchool, err error) { + _since := time.Now() + defer func() { + duration := time.Since(_since).Seconds() + result := MetricResultSuccess + if err != nil { + result = MetricResultFailure + if err == ErrReadOnly { + result = MetricResultReadOnly + } + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + } else { + + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + + } + + _d.metric.WithLabelValues(MetricOpGetEducationSchools, result).Observe(duration) + }() + return _d.delegate.GetEducationSchools(ctx) +} + +// GetEducationUser implements EducationBackend.GetEducationUser +func (_d PrometheusEducationBackend) GetEducationUser(ctx context.Context, nameOrID string) (ep1 *libregraph.EducationUser, err error) { + _since := time.Now() + defer func() { + duration := time.Since(_since).Seconds() + result := MetricResultSuccess + if err != nil { + result = MetricResultFailure + if err == ErrReadOnly { + result = MetricResultReadOnly + } + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + } else { + // it's a get operation that returns a pointer (and not an array): check whether that's nil or not + if ep1 == nil { + result = MetricResultNotFound + } + + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + + } + + _d.metric.WithLabelValues(MetricOpGetEducationUser, result).Observe(duration) + }() + return _d.delegate.GetEducationUser(ctx, nameOrID) +} + +// GetEducationUsers implements EducationBackend.GetEducationUsers +func (_d PrometheusEducationBackend) GetEducationUsers(ctx context.Context) (epa1 []*libregraph.EducationUser, err error) { + _since := time.Now() + defer func() { + duration := time.Since(_since).Seconds() + result := MetricResultSuccess + if err != nil { + result = MetricResultFailure + if err == ErrReadOnly { + result = MetricResultReadOnly + } + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + } else { + + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + + } + + _d.metric.WithLabelValues(MetricOpGetEducationUsers, result).Observe(duration) + }() + return _d.delegate.GetEducationUsers(ctx) +} + +// RemoveClassFromEducationSchool implements EducationBackend.RemoveClassFromEducationSchool +func (_d PrometheusEducationBackend) RemoveClassFromEducationSchool(ctx context.Context, schoolNumberOrID string, memberID string) (err error) { + _since := time.Now() + defer func() { + duration := time.Since(_since).Seconds() + result := MetricResultSuccess + if err != nil { + result = MetricResultFailure + if err == ErrReadOnly { + result = MetricResultReadOnly + } + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + } else { + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + + } + + _d.metric.WithLabelValues(MetricOpRemoveClassFromEducationSchool, result).Observe(duration) + }() + return _d.delegate.RemoveClassFromEducationSchool(ctx, schoolNumberOrID, memberID) +} + +// RemoveTeacherFromEducationClass implements EducationBackend.RemoveTeacherFromEducationClass +func (_d PrometheusEducationBackend) RemoveTeacherFromEducationClass(ctx context.Context, classID string, teacherID string) (err error) { + _since := time.Now() + defer func() { + duration := time.Since(_since).Seconds() + result := MetricResultSuccess + if err != nil { + result = MetricResultFailure + if err == ErrReadOnly { + result = MetricResultReadOnly + } + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + } else { + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + + } + + _d.metric.WithLabelValues(MetricOpRemoveTeacherFromEducationClass, result).Observe(duration) + }() + return _d.delegate.RemoveTeacherFromEducationClass(ctx, classID, teacherID) +} + +// RemoveUserFromEducationSchool implements EducationBackend.RemoveUserFromEducationSchool +func (_d PrometheusEducationBackend) RemoveUserFromEducationSchool(ctx context.Context, schoolID string, memberID string) (err error) { + _since := time.Now() + defer func() { + duration := time.Since(_since).Seconds() + result := MetricResultSuccess + if err != nil { + result = MetricResultFailure + if err == ErrReadOnly { + result = MetricResultReadOnly + } + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + } else { + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + + } + + _d.metric.WithLabelValues(MetricOpRemoveUserFromEducationSchool, result).Observe(duration) + }() + return _d.delegate.RemoveUserFromEducationSchool(ctx, schoolID, memberID) +} + +// UpdateEducationClass implements EducationBackend.UpdateEducationClass +func (_d PrometheusEducationBackend) UpdateEducationClass(ctx context.Context, id string, class libregraph.EducationClass) (ep1 *libregraph.EducationClass, err error) { + _since := time.Now() + defer func() { + duration := time.Since(_since).Seconds() + result := MetricResultSuccess + if err != nil { + result = MetricResultFailure + if err == ErrReadOnly { + result = MetricResultReadOnly + } + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + } else { + + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + + } + + _d.metric.WithLabelValues(MetricOpUpdateEducationClass, result).Observe(duration) + }() + return _d.delegate.UpdateEducationClass(ctx, id, class) +} + +// UpdateEducationSchool implements EducationBackend.UpdateEducationSchool +func (_d PrometheusEducationBackend) UpdateEducationSchool(ctx context.Context, numberOrID string, school libregraph.EducationSchool) (ep1 *libregraph.EducationSchool, err error) { + _since := time.Now() + defer func() { + duration := time.Since(_since).Seconds() + result := MetricResultSuccess + if err != nil { + result = MetricResultFailure + if err == ErrReadOnly { + result = MetricResultReadOnly + } + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + } else { + + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + + } + + _d.metric.WithLabelValues(MetricOpUpdateEducationSchool, result).Observe(duration) + }() + return _d.delegate.UpdateEducationSchool(ctx, numberOrID, school) +} + +// UpdateEducationUser implements EducationBackend.UpdateEducationUser +func (_d PrometheusEducationBackend) UpdateEducationUser(ctx context.Context, nameOrID string, user libregraph.EducationUser) (ep1 *libregraph.EducationUser, err error) { + _since := time.Now() + defer func() { + duration := time.Since(_since).Seconds() + result := MetricResultSuccess + if err != nil { + result = MetricResultFailure + if err == ErrReadOnly { + result = MetricResultReadOnly + } + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + } else { + + if e, ok := errorcode.ToError(err); ok { + if e.GetCode() == errorcode.ItemNotFound { + result = MetricResultNotFound + } + } + + } + + _d.metric.WithLabelValues(MetricOpUpdateEducationUser, result).Observe(duration) + }() + return _d.delegate.UpdateEducationUser(ctx, nameOrID, user) +} diff --git a/services/graph/pkg/identity/err_education.go b/services/graph/pkg/identity/err_education.go index 4138299035..4b7252c386 100644 --- a/services/graph/pkg/identity/err_education.go +++ b/services/graph/pkg/identity/err_education.go @@ -9,6 +9,8 @@ import ( // ErrEducationBackend is a dummy EducationBackend, doing nothing type ErrEducationBackend struct{} +var _ EducationBackend = &ErrEducationBackend{} + // CreateEducationSchool creates the supplied school in the identity backend. func (i *ErrEducationBackend) CreateEducationSchool(ctx context.Context, school libregraph.EducationSchool) (*libregraph.EducationSchool, error) { return nil, errNotImplemented diff --git a/services/graph/pkg/identity/factory.go b/services/graph/pkg/identity/factory.go index 52cc1f11ce..3adc221827 100644 --- a/services/graph/pkg/identity/factory.go +++ b/services/graph/pkg/identity/factory.go @@ -6,20 +6,30 @@ import ( "errors" "fmt" "os" + "strings" ldapv3 "github.com/go-ldap/ldap/v3" ocldap "github.com/opencloud-eu/opencloud/pkg/ldap" "github.com/opencloud-eu/opencloud/pkg/log" "github.com/opencloud-eu/opencloud/pkg/registry" "github.com/opencloud-eu/opencloud/services/graph/pkg/config" + "github.com/opencloud-eu/opencloud/services/graph/pkg/metrics" "github.com/opencloud-eu/reva/v2/pkg/rgrpc/todo/pool" "github.com/opencloud-eu/reva/v2/pkg/utils/ldap" + "github.com/prometheus/client_golang/prometheus" "go.opentelemetry.io/otel/trace" ) -func CreateIdentityBackends(name string, cfg *config.Config, logger *log.Logger, traceProvider trace.TracerProvider) (Backend, EducationBackend, error) { +const ( + cs3Backend = "cs3" + ldapBackend = "ldap" +) + +var supportedBackends = []string{cs3Backend, ldapBackend} + +func CreateIdentityBackends(name string, cfg *config.Config, logger *log.Logger, registrer prometheus.Registerer, traceProvider trace.TracerProvider) (Backend, EducationBackend, error) { switch name { - case "cs3": + case cs3Backend: gatewaySelector, err := pool.GatewaySelector( cfg.Reva.Address, append( @@ -32,12 +42,12 @@ func CreateIdentityBackends(name string, cfg *config.Config, logger *log.Logger, return nil, nil, err } - return &CS3{ - Config: cfg.Reva, - Logger: logger, - GatewaySelector: gatewaySelector, - }, nil, nil - case "ldap": + if cs3, err := NewCS3Backend(cfg.Reva, gatewaySelector, logger); err != nil { + return nil, nil, err + } else { + return cs3, nil, nil + } + case ldapBackend: var err error var tlsConf *tls.Config @@ -76,25 +86,54 @@ func CreateIdentityBackends(name string, cfg *config.Config, logger *log.Logger, tlsConf.RootCAs = certs } - conn := ldap.NewLDAPWithReconnect( - ldap.Config{ - URI: cfg.Identity.LDAP.URI, - BindDN: cfg.Identity.LDAP.BindDN, - BindPassword: cfg.Identity.LDAP.BindPassword, - TLSConfig: tlsConf, - }, - ) + ldapConfig := ldap.Config{ + URI: cfg.Identity.LDAP.URI, + BindDN: cfg.Identity.LDAP.BindDN, + BindPassword: cfg.Identity.LDAP.BindPassword, + TLSConfig: tlsConf, + } + + logger = &log.Logger{Logger: logger.With(). + Str("ldap-uri", ldapConfig.URI). + Logger(), + } + + conn := ldap.NewLDAPWithReconnect(ldapConfig) conn.SetLogger(&logger.Logger) - lb, err := NewLDAPBackend(conn, cfg.Identity.LDAP, logger) + lb, err := NewLDAPBackend(conn, cfg.Identity.LDAP, logger, metrics.Namespace, metrics.Subsystem, registrer) if err != nil { logger.Error().Err(err).Msg("Error initializing LDAP Backend") return nil, nil, err } - identityBackend := lb + var identityBackend Backend = lb var eduBackend EducationBackend = lb + if !cfg.Identity.Metrics.Disabled && registrer != nil { + backendApiOperationDuration := prometheus.NewHistogramVec(prometheus.HistogramOpts{ + Namespace: metrics.Namespace, + Subsystem: metrics.Subsystem, + Name: "identity_backend_api_duration_seconds", + Help: "Duration of API operations performed by the Graph service identity backend in seconds.", + Buckets: prometheus.DefBuckets, + ConstLabels: prometheus.Labels{ + MetricLabelType: name, + }, + }, []string{MetricLabelOperation, metrics.LabelResult}) + + if err := registrer.Register(backendApiOperationDuration); err != nil { + logger.Warn().Err(err).Msg("failed to register backend API operation duration metric") + } + + identityBackend = NewPrometheusBackend(identityBackend, backendApiOperationDuration) + eduBackend = NewPrometheusEducationBackend(eduBackend, backendApiOperationDuration) + } + if !cfg.Identity.LDAP.EducationResourcesEnabled { + // in this case, simply bury the previous eduBackend, no need to wrap or anything: if we had + // a previous implementation in there that wrapped with metrics or such, we don't want to + // have any cross-cutting concerns running here, just use this implementation that returns + // errors on purpose and that's it: eduBackend = &ErrEducationBackend{} } @@ -121,7 +160,7 @@ func CreateIdentityBackends(name string, cfg *config.Config, logger *log.Logger, if isAnError { msg := "error adding group for disabling users" logger.Error().Err(err).Str("local_user_disable", cfg.Identity.LDAP.LdapDisabledUsersGroupDN).Msg(msg) - return nil, nil, err + return nil, nil, fmt.Errorf("%s: %w", msg, err) } } } @@ -129,8 +168,8 @@ func CreateIdentityBackends(name string, cfg *config.Config, logger *log.Logger, return identityBackend, eduBackend, nil default: - err := fmt.Errorf("unknown identity backend: '%s'", name) - logger.Err(err) + err := fmt.Errorf("unknown identity backend: %q, must be one of [%s]", name, strings.Join(supportedBackends, ", ")) + logger.Error().Err(err).Msgf("failed to create identity backend %q", name) return nil, nil, err } } diff --git a/services/graph/pkg/identity/ldap.go b/services/graph/pkg/identity/ldap.go index a766814bb7..e28063b4a7 100644 --- a/services/graph/pkg/identity/ldap.go +++ b/services/graph/pkg/identity/ldap.go @@ -8,6 +8,7 @@ import ( "slices" "strconv" "strings" + "sync/atomic" "time" "github.com/CiscoM31/godata" @@ -15,10 +16,12 @@ import ( "github.com/google/uuid" "github.com/libregraph/idm/pkg/ldapdn" libregraph "github.com/opencloud-eu/libre-graph-api-go" + "github.com/prometheus/client_golang/prometheus" "github.com/opencloud-eu/opencloud/pkg/log" "github.com/opencloud-eu/opencloud/services/graph/pkg/config" "github.com/opencloud-eu/opencloud/services/graph/pkg/errorcode" + "github.com/opencloud-eu/opencloud/services/graph/pkg/metrics" "github.com/opencloud-eu/opencloud/services/graph/pkg/odata" ) @@ -74,9 +77,12 @@ type LDAP struct { educationConfig educationConfig logger *log.Logger - conn ldap.Client + conn LdapClient } +var _ Backend = &LDAP{} +var _ EducationBackend = &LDAP{} + type userAttributeMap struct { displayName string id string @@ -107,11 +113,33 @@ func ParseDisableMechanismType(disableMechanism string) (DisableUserMechanismTyp return t, nil } -func NewLDAPBackend(lc ldap.Client, config config.LDAP, logger *log.Logger) (*LDAP, error) { +const ( + MetricResultSuccess = "success" + MetricResultFailure = "failure" + MetricResultNotFound = "not-found" + MetricResultReadOnly = "read-only" +) + +const ( + MetricLabelOperation = "operation" + MetricLabelType = "type" + MetricLabelUri = "uri" + MetricLabelWrite = "write" +) + +func NewLDAPBackend(lc ldap.Client, config config.LDAP, logger *log.Logger, namespace string, subsystem string, registry prometheus.Registerer) (*LDAP, error) { if config.UserDisplayNameAttribute == "" || config.UserIDAttribute == "" || config.UserEmailAttribute == "" || config.UserNameAttribute == "" { return nil, errors.New("invalid user attribute mappings") } + + logger = &log.Logger{Logger: logger.With(). + // Str("backend", "ldap"). // already added upstream + Bool("write", config.WriteEnabled). + Bool("refint", config.RefintEnabled). + Logger(), + } + uam := userAttributeMap{ displayName: config.UserDisplayNameAttribute, id: config.UserIDAttribute, @@ -154,6 +182,65 @@ func NewLDAPBackend(lc ldap.Client, config config.LDAP, logger *log.Logger) (*LD return nil, fmt.Errorf("error configuring disable user mechanism: %w", err) } + var client LdapClient + client = NewGoLdapLdapClient(lc) + if !config.Metrics.Disabled && registry != nil { + // metrics are enabled for the LDAP identity backend + + // use a 'write' label to indicate whether this instance is read-only + // (write=="0") or allowed to make changes (wrote=="1") + write := "0" + if config.WriteEnabled { + write = "1" + } + + // a metric that tracks the duration of the LDAP operations we perform as an LDAP client, + // will be passed to a Prometheus wrapper around LdapClient below + ldapEgressDuration := prometheus.NewHistogramVec(prometheus.HistogramOpts{ + Namespace: namespace, + Subsystem: subsystem, + Name: "ldap_client_operation_duration_seconds", + Help: "Duration of LDAP operations performed by the Graph service in seconds.", + Buckets: prometheus.DefBuckets, + ConstLabels: prometheus.Labels{ + MetricLabelUri: config.URI, + MetricLabelWrite: write, + }, + }, []string{MetricLabelOperation, metrics.LabelResult}) + if err := registry.Register(ldapEgressDuration); err != nil { + logger.Warn().Err(err).Msg("failed to register LDAP egress duration metric") + } + + // a metric that tracks the number of ongoing concurrent LDAP operations, will also be + // passed to a Prometheus wrapper around LdapClient below; + // note that we use an atomic int as a gauge to count operations up and down using the + // wrapper, and then a gauge func that retrieves the current value of that atomic int + // whenever scraped by Prometheus, as that approach performs better than calling inc/dec + // on a Gauge object directly: + var inFlight atomic.Int64 + ldapEgressInFlight := prometheus.NewGaugeFunc(prometheus.GaugeOpts{ + Namespace: namespace, + Subsystem: subsystem, + Name: "ldap_client_operations", + Help: "Number of LDAP client operations in-flight in the Graph service.", + Unit: "operation", + ConstLabels: prometheus.Labels{ + MetricLabelUri: config.URI, + MetricLabelWrite: write, + }, + }, func() float64 { + // when scraped, we simply read the current value of the atomic int: + return float64(inFlight.Load()) + }) + if err := registry.Register(ldapEgressInFlight); err != nil { + logger.Warn().Err(err).Msg("failed to register LDAP egress in-flight metric") + } + + // we sill use an LdapClient that wraps the "proper" LdapClient with recording the + // metrics referenced above: + client = NewPrometheusLdapClient(client, ldapEgressDuration, &inFlight) + } + return &LDAP{ useServerUUID: config.UseServerUUID, usePwModifyExOp: config.UsePasswordModExOp, @@ -174,7 +261,7 @@ func NewLDAPBackend(lc ldap.Client, config config.LDAP, logger *log.Logger) (*LD disableUserMechanism: disableMechanismType, localUserDisableGroupDN: config.LdapDisabledUsersGroupDN, logger: logger, - conn: lc, + conn: client, writeEnabled: config.WriteEnabled, refintEnabled: config.RefintEnabled, }, nil @@ -185,7 +272,7 @@ func NewLDAPBackend(lc ldap.Client, config config.LDAP, logger *log.Logger) (*LD // configured LDAP server func (i *LDAP) CreateUser(ctx context.Context, user libregraph.User) (*libregraph.User, error) { logger := i.logger.SubloggerWithRequestID(ctx) - logger.Debug().Str("backend", "ldap").Msg("CreateUser") + logger.Debug().Msg("CreateUser") if !i.writeEnabled { return nil, ErrReadOnly } @@ -222,14 +309,14 @@ func (i *LDAP) CreateUser(ctx context.Context, user libregraph.User) (*libregrap if err != nil { return nil, err } - return i.createUserModelFromLDAP(e), nil + return i.createUserModelFromLDAP(e) } // DeleteUser implements the Backend Interface. It permanently deletes a User identified // by name or id from the LDAP server func (i *LDAP) DeleteUser(ctx context.Context, nameOrID string) error { logger := i.logger.SubloggerWithRequestID(ctx) - logger.Debug().Str("backend", "ldap").Msg("DeleteUser") + logger.Debug().Msg("DeleteUser") if !i.writeEnabled { return ErrReadOnly } @@ -237,6 +324,7 @@ func (i *LDAP) DeleteUser(ctx context.Context, nameOrID string) error { if err != nil { return err } + dr := ldap.DelRequest{DN: e.DN} if err = i.conn.Del(&dr); err != nil { msg := "error deleting user" @@ -272,11 +360,11 @@ func (i *LDAP) DeleteUser(ctx context.Context, nameOrID string) error { // UpdateUser implements the Backend Interface for the LDAP Backend func (i *LDAP) UpdateUser(ctx context.Context, nameOrID string, user libregraph.UserUpdate) (*libregraph.User, error) { logger := i.logger.SubloggerWithRequestID(ctx) - logger.Debug().Str("backend", "ldap").Msg("UpdateUser") + logger.Debug().Msg("UpdateUser") if !i.writeEnabled { // still allow to enable/disable user when using DisableMechanismGroup if i.disableUserMechanism == DisableMechanismGroup && isUserEnabledUpdate(user) { - logger.Error().Str("backend", "ldap").Msg("Allowing accountEnabled Update on read-only backend") + logger.Error().Msg("Allowing accountEnabled Update on read-only backend") } else { return nil, ErrReadOnly } @@ -396,12 +484,16 @@ func (i *LDAP) UpdateUser(ctx context.Context, nameOrID string, user libregraph. return nil, err } - returnUser := i.createUserModelFromLDAP(e) - - // To avoid a ldap lookup for group membership, set the enabled flag to same as input value - // since this would have been updated with group membership from the input anyway. - if user.AccountEnabled != nil && i.disableUserMechanism == DisableMechanismGroup { - returnUser.AccountEnabled = user.AccountEnabled + returnUser, err := i.createUserModelFromLDAP(e) + if err != nil { + return nil, err + } + if returnUser != nil { + // To avoid a ldap lookup for group membership, set the enabled flag to same as input value + // since this would have been updated with group membership from the input anyway. + if user.AccountEnabled != nil && i.disableUserMechanism == DisableMechanismGroup { + returnUser.AccountEnabled = user.AccountEnabled + } } return returnUser, nil @@ -445,7 +537,7 @@ func (i *LDAP) getEntryByDN(dn string, attrs []string, filter string) (*ldap.Ent nil, ) - i.logger.Debug().Str("backend", "ldap"). + i.logger.Debug(). Str("base", searchRequest.BaseDN). Str("filter", searchRequest.Filter). Int("scope", searchRequest.Scope). @@ -454,16 +546,33 @@ func (i *LDAP) getEntryByDN(dn string, attrs []string, filter string) (*ldap.Ent Msg("getEntryByDN") res, err := i.conn.Search(searchRequest) if err != nil { - i.logger.Error().Err(err).Str("backend", "ldap").Str("dn", dn).Msg("Search ldap by DN failed") - return nil, errorcode.New(errorcode.ItemNotFound, "user lookup failed") + i.logger.Error().Err(err).Str("dn", dn).Msg("Search ldap by DN failed") + msg := "user lookup failed" + errMap := ldapResultToErrMap{ + ldap.LDAPResultNoSuchObject: errorcode.New(errorcode.ItemNotFound, msg), + ldap.LDAPResultUnwillingToPerform: errorcode.New(errorcode.NotAllowed, msg), + ldap.LDAPResultInsufficientAccessRights: errorcode.New(errorcode.NotAllowed, msg), + ldapGenericErr: errorcode.New(errorcode.GeneralException, msg), + } + return nil, i.mapLDAPError(err, errMap) } - if len(res.Entries) == 0 { + + switch len(res.Entries) { + case 0: return nil, ErrNotFound + case 1: + return res.Entries[0], nil + default: + return nil, ErrTooManyResults } - - return res.Entries[0], nil } +// Retrieves a single entry from LDAP. +// +// It never returns nil for the *ldap.Entry: +// - if no object is found, it returns a ErrNotFound error +// - if more than one object is found, it returns a ErrTooManyResults error +// - if exactly one object is found, it returns that entry and no error func (i *LDAP) searchLDAPEntryByFilter(basedn string, attrs []string, filter string) (*ldap.Entry, error) { if filter == "" { filter = "(objectclass=*)" @@ -478,7 +587,7 @@ func (i *LDAP) searchLDAPEntryByFilter(basedn string, attrs []string, filter str nil, ) - i.logger.Debug().Str("backend", "ldap"). + i.logger.Debug(). Str("base", searchRequest.BaseDN). Str("filter", searchRequest.Filter). Int("scope", searchRequest.Scope). @@ -487,14 +596,24 @@ func (i *LDAP) searchLDAPEntryByFilter(basedn string, attrs []string, filter str Msg("getEntryByFilter") res, err := i.conn.Search(searchRequest) if err != nil { - i.logger.Error().Err(err).Str("backend", "ldap").Str("dn", basedn).Str("filter", filter).Msg("Search user by filter failed") - return nil, errorcode.New(errorcode.ItemNotFound, "user search failed") + i.logger.Error().Err(err).Str("dn", basedn).Str("filter", filter).Msg("Search user by filter failed") + msg := "user search failed" + errMap := ldapResultToErrMap{ + ldap.LDAPResultNoSuchObject: errorcode.New(errorcode.ItemNotFound, msg), + ldap.LDAPResultUnwillingToPerform: errorcode.New(errorcode.NotAllowed, msg), + ldap.LDAPResultInsufficientAccessRights: errorcode.New(errorcode.NotAllowed, msg), + ldapGenericErr: errorcode.New(errorcode.GeneralException, msg), + } + return nil, i.mapLDAPError(err, errMap) } - if len(res.Entries) == 0 { + switch len(res.Entries) { + case 0: return nil, ErrNotFound + case 1: + return res.Entries[0], nil + default: + return nil, ErrTooManyResults } - - return res.Entries[0], nil } func filterEscapeAttribute(attribute string, binary bool, id string) (string, error) { @@ -572,16 +691,16 @@ func (i *LDAP) getLDAPUserByFilter(filter string) (*ldap.Entry, error) { // GetUser implements the Backend Interface. func (i *LDAP) GetUser(ctx context.Context, nameOrID string, oreq *godata.GoDataRequest) (*libregraph.User, error) { logger := i.logger.SubloggerWithRequestID(ctx) - logger.Debug().Str("backend", "ldap").Msg("GetUser") + logger.Debug().Msg("GetUser") e, err := i.getLDAPUserByNameOrID(nameOrID) if err != nil { return nil, err } - u := i.createUserModelFromLDAP(e) - if u == nil { - return nil, ErrNotFound + u, err := i.createUserModelFromLDAP(e) + if err != nil { + return nil, err } if i.disableUserMechanism != DisableMechanismNone { @@ -601,7 +720,11 @@ func (i *LDAP) GetUser(ctx context.Context, nameOrID string, oreq *godata.GoData if err != nil { return nil, err } - u.MemberOf = i.groupsFromLDAPEntries(userGroups) + if memberOf, err := i.groupsFromLDAPEntries(userGroups); err != nil { + // TODO: should we really just silently skip LDAP data model errors here, or rather return this as an error? + } else { + u.MemberOf = memberOf + } } return u, nil } @@ -614,7 +737,7 @@ func (i *LDAP) GetUsers(ctx context.Context, oreq *godata.GoDataRequest) ([]*lib // FilterUsers implements the Backend Interface. func (i *LDAP) FilterUsers(ctx context.Context, oreq *godata.GoDataRequest, filter *godata.ParseNode) ([]*libregraph.User, error) { logger := i.logger.SubloggerWithRequestID(ctx) - logger.Debug().Str("backend", "ldap").Msg("GetUsers") + logger.Debug().Msg("GetUsers") queryFilter, err := i.oDataFilterToLDAPFilter(filter) if err != nil { @@ -648,7 +771,7 @@ func (i *LDAP) FilterUsers(ctx context.Context, oreq *godata.GoDataRequest, filt i.getUserAttrTypesForSearch(), nil, ) - logger.Debug().Str("backend", "ldap"). + logger.Debug(). Str("base", searchRequest.BaseDN). Str("filter", searchRequest.Filter). Int("scope", searchRequest.Scope). @@ -676,9 +799,9 @@ func (i *LDAP) usersFromLDAPEntries(entries []*ldap.Entry, exp []string) ([]*lib } users := make([]*libregraph.User, 0, len(entries)) for _, e := range entries { - u := i.createUserModelFromLDAP(e) - // Skip invalid LDAP users - if u == nil { + u, err := i.createUserModelFromLDAP(e) + if u == nil || err != nil { + // Skip invalid LDAP users continue } @@ -692,7 +815,11 @@ func (i *LDAP) usersFromLDAPEntries(entries []*ldap.Entry, exp []string) ([]*lib if err != nil { return nil, err } - u.MemberOf = i.groupsFromLDAPEntries(userGroups) + if memberOf, err := i.groupsFromLDAPEntries(userGroups); err != nil { + // TODO: should we really just silently skip LDAP data model errors here, or rather return this as an error? + } else { + u.MemberOf = memberOf + } } users = append(users, u) } @@ -702,14 +829,15 @@ func (i *LDAP) usersFromLDAPEntries(entries []*ldap.Entry, exp []string) ([]*lib // UpdateLastSignInDate implements the Backend Interface. func (i *LDAP) UpdateLastSignInDate(ctx context.Context, userID string, timestamp time.Time) error { if !i.writeEnabled { - i.logger.Debug().Str("backend", "ldap").Msg("The LDAP Server is readonly. Skipping update of last sign in date") + i.logger.Debug().Msg("The LDAP Server is readonly. Skipping update of last sign in date") return nil // TODO: do we really want to just silently do nothing here, rather than returning an error? } + e, err := i.getLDAPUserByID(userID) switch { case errors.Is(err, ErrNotFound): i.logger.Warn().Err(err).Str("userID", userID).Msg("Failed to update last sign in date for user") - return nil + return nil // TODO questionable whether this should just fail silently because the user was not found case err != nil: return err } @@ -819,7 +947,7 @@ func (i *LDAP) renameMemberInGroup(ctx context.Context, group *ldap.Entry, oldMe func (i *LDAP) updateUserPassword(ctx context.Context, dn, password string) error { logger := i.logger.SubloggerWithRequestID(ctx) - logger.Debug().Str("backend", "ldap").Msg("updateUserPassword") + logger.Debug().Msg("updateUserPassword") pwMod := ldap.PasswordModifyRequest{ UserIdentity: dn, NewPassword: password, @@ -860,9 +988,9 @@ func (i *LDAP) ldapUUIDtoString(e *ldap.Entry, attribute string, binary bool) (s return e.GetEqualFoldAttributeValue(attribute), nil } -func (i *LDAP) createUserModelFromLDAP(e *ldap.Entry) *libregraph.User { +func (i *LDAP) createUserModelFromLDAP(e *ldap.Entry) (*libregraph.User, error) { if e == nil { - return nil + return nil, nil } opsan := e.GetEqualFoldAttributeValue(i.userAttributeMap.userName) @@ -910,10 +1038,12 @@ func (i *LDAP) createUserModelFromLDAP(e *ldap.Entry) *libregraph.User { case !errors.Is(err, errNotSet): i.logger.Warn().Err(err).Str("dn", e.DN).Msg("Error getting last signin timestamp") } - return user + return user, nil } + + err = errorcode.New(errorcode.GeneralException, "Invalid User. Missing username or id attribute") i.logger.Warn().Str("dn", e.DN).Str("id", id).Str("username", opsan).Msg("Invalid User. Missing username or id attribute") - return nil + return nil, err } func (i *LDAP) userToLDAPAttrValues(user libregraph.User) (map[string][]string, error) { @@ -1082,7 +1212,7 @@ func (i *LDAP) removeEntryByDNAndAttributeFromEntry(entry *ldap.Entry, dn string } } if !found { - i.logger.Error().Str("backend", "ldap").Str("entry", entry.DN).Str("target", dn). + i.logger.Error().Str("entry", entry.DN).Str("target", dn). Msg("The target value is not present in the attribute list") return ErrNotFound } @@ -1126,7 +1256,7 @@ func (i *LDAP) removeEntryByDNAndAttributeFromEntry(entry *ldap.Entry, dn string // expandLDAPAttributeEntries reads an attribute from a ldap entry and expands to users func (i *LDAP) expandLDAPAttributeEntries(ctx context.Context, e *ldap.Entry, attribute, searchTerm string) ([]*ldap.Entry, error) { logger := i.logger.SubloggerWithRequestID(ctx) - logger.Debug().Str("backend", "ldap").Msg("ExpandLDAPAttributeEntries") + logger.Debug().Msg("ExpandLDAPAttributeEntries") result := []*ldap.Entry{} for _, entryDN := range e.GetEqualFoldAttributeValues(attribute) { diff --git a/services/graph/pkg/identity/ldap_client.go b/services/graph/pkg/identity/ldap_client.go new file mode 100644 index 0000000000..6115e07550 --- /dev/null +++ b/services/graph/pkg/identity/ldap_client.go @@ -0,0 +1,26 @@ +package identity + +//go:generate gowrap gen -g -i LdapClient -t ./ldap_client_prometheus.tmpl -o ldap_client_prometheus.go +//go:generate gowrap gen -g -i LdapClient -t ./ldap_client_goldap.tmpl -o ldap_client_goldap.go + +import ( + "github.com/go-ldap/ldap/v3" +) + +const ( + LdapOpAdd = "add" + LdapOpDel = "del" + LdapOpModify = "modify" + LdapOpModifyDN = "modify-dn" + LdapOpPasswordModify = "modify-password" + LdapOpSearch = "search" +) + +type LdapClient interface { + Add(*ldap.AddRequest) error + Del(*ldap.DelRequest) error + Modify(*ldap.ModifyRequest) error + ModifyDN(*ldap.ModifyDNRequest) error + PasswordModify(*ldap.PasswordModifyRequest) (*ldap.PasswordModifyResult, error) + Search(*ldap.SearchRequest) (*ldap.SearchResult, error) +} diff --git a/services/graph/pkg/identity/ldap_client_goldap.go b/services/graph/pkg/identity/ldap_client_goldap.go new file mode 100644 index 0000000000..620d1439a5 --- /dev/null +++ b/services/graph/pkg/identity/ldap_client_goldap.go @@ -0,0 +1,57 @@ +// Code generated by gowrap. DO NOT EDIT. +// template: ldap_client_goldap.tmpl +// gowrap: http://github.com/hexdigest/gowrap + +package identity + +import ( + "github.com/go-ldap/ldap/v3" +) + +// implementation that adapts the go-ldap ldap.Client interface +// and delegates everything to a proper LDAP client +type GoLdapLdapClient struct { + delegate ldap.Client +} + +var _ LdapClient = &GoLdapLdapClient{} + +func NewGoLdapLdapClient(delegate ldap.Client) *GoLdapLdapClient { + return &GoLdapLdapClient{delegate: delegate} +} + +// Add implements LdapClient.Add +// and delegates to ldap.Client.Add +func (_d GoLdapLdapClient) Add(ap1 *ldap.AddRequest) (err error) { + return _d.delegate.Add(ap1) +} + +// Del implements LdapClient.Del +// and delegates to ldap.Client.Del +func (_d GoLdapLdapClient) Del(dp1 *ldap.DelRequest) (err error) { + return _d.delegate.Del(dp1) +} + +// Modify implements LdapClient.Modify +// and delegates to ldap.Client.Modify +func (_d GoLdapLdapClient) Modify(mp1 *ldap.ModifyRequest) (err error) { + return _d.delegate.Modify(mp1) +} + +// ModifyDN implements LdapClient.ModifyDN +// and delegates to ldap.Client.ModifyDN +func (_d GoLdapLdapClient) ModifyDN(mp1 *ldap.ModifyDNRequest) (err error) { + return _d.delegate.ModifyDN(mp1) +} + +// PasswordModify implements LdapClient.PasswordModify +// and delegates to ldap.Client.PasswordModify +func (_d GoLdapLdapClient) PasswordModify(pp1 *ldap.PasswordModifyRequest) (pp2 *ldap.PasswordModifyResult, err error) { + return _d.delegate.PasswordModify(pp1) +} + +// Search implements LdapClient.Search +// and delegates to ldap.Client.Search +func (_d GoLdapLdapClient) Search(sp1 *ldap.SearchRequest) (sp2 *ldap.SearchResult, err error) { + return _d.delegate.Search(sp1) +} diff --git a/services/graph/pkg/identity/ldap_client_goldap.tmpl b/services/graph/pkg/identity/ldap_client_goldap.tmpl new file mode 100644 index 0000000000..fd1170b06f --- /dev/null +++ b/services/graph/pkg/identity/ldap_client_goldap.tmpl @@ -0,0 +1,25 @@ +import ( + "github.com/go-ldap/ldap/v3" +) + +{{ $decorator := (or .Vars.DecoratorName (printf "GoLdap%s" .Interface.Name)) }} + +// implementation that adapts the go-ldap ldap.Client interface +// and delegates everything to a proper LDAP client +type {{$decorator}} struct { + delegate ldap.Client +} + +var _ {{.Interface.Type}} = &{{$decorator}}{} + +func New{{$decorator}}(delegate ldap.Client) *GoLdapLdapClient { + return &{{$decorator}}{delegate: delegate} +} + +{{range $method := .Interface.Methods}} + // {{$method.Name}} implements {{$.Interface.Type}}.{{$method.Name}} + // and delegates to ldap.Client.{{$method.Name}} + func (_d {{$decorator}}) {{$method.Declaration}} { + {{$method.Pass "_d.delegate."}} + } +{{end}} diff --git a/services/graph/pkg/identity/ldap_client_prometheus.go b/services/graph/pkg/identity/ldap_client_prometheus.go new file mode 100644 index 0000000000..87d30785d7 --- /dev/null +++ b/services/graph/pkg/identity/ldap_client_prometheus.go @@ -0,0 +1,208 @@ +// Code generated by gowrap. DO NOT EDIT. +// template: ldap_client_prometheus.tmpl +// gowrap: http://github.com/hexdigest/gowrap + +package identity + +import ( + "errors" + "sync/atomic" + "time" + + "github.com/go-ldap/ldap/v3" + "github.com/opencloud-eu/opencloud/services/graph/pkg/metrics" + "github.com/prometheus/client_golang/prometheus" +) + +// PrometheusLdapClient implements LdapClient interface with all methods wrapped +// with Prometheus metrics +type PrometheusLdapClient struct { + delegate LdapClient + timer *prometheus.HistogramVec + inflight *atomic.Int64 +} + +var _ LdapClient = &PrometheusLdapClient{} + +// returns an instance of the LdapClient decorated with prometheus metric +func NewPrometheusLdapClient(delegate LdapClient, timer *prometheus.HistogramVec, inflight *atomic.Int64) PrometheusLdapClient { + return PrometheusLdapClient{ + delegate: delegate, + timer: timer, + inflight: inflight, + } +} + +// Add implements LdapClient.Add +func (_d PrometheusLdapClient) Add(ap1 *ldap.AddRequest) (err error) { + _d.inflight.Add(1) + defer _d.inflight.Add(-1) + + _since := time.Now() + defer func() { + duration := time.Since(_since).Seconds() + result := metrics.ResultSuccess + if err != nil { + if err == ErrReadOnly { + result = metrics.ResultReadOnly + } else { + result = metrics.ResultFailure + var lerr *ldap.Error + if errors.As(err, &lerr) { + if lerr != nil && lerr.ResultCode == ldap.LDAPResultNoSuchObject { + result = metrics.ResultNotFound + } + } + } + } + + _d.timer.WithLabelValues(LdapOpAdd, result).Observe(duration) + }() + + return _d.delegate.Add(ap1) +} + +// Del implements LdapClient.Del +func (_d PrometheusLdapClient) Del(dp1 *ldap.DelRequest) (err error) { + _d.inflight.Add(1) + defer _d.inflight.Add(-1) + + _since := time.Now() + defer func() { + duration := time.Since(_since).Seconds() + result := metrics.ResultSuccess + if err != nil { + if err == ErrReadOnly { + result = metrics.ResultReadOnly + } else { + result = metrics.ResultFailure + var lerr *ldap.Error + if errors.As(err, &lerr) { + if lerr != nil && lerr.ResultCode == ldap.LDAPResultNoSuchObject { + result = metrics.ResultNotFound + } + } + } + } + + _d.timer.WithLabelValues(LdapOpDel, result).Observe(duration) + }() + + return _d.delegate.Del(dp1) +} + +// Modify implements LdapClient.Modify +func (_d PrometheusLdapClient) Modify(mp1 *ldap.ModifyRequest) (err error) { + _d.inflight.Add(1) + defer _d.inflight.Add(-1) + + _since := time.Now() + defer func() { + duration := time.Since(_since).Seconds() + result := metrics.ResultSuccess + if err != nil { + if err == ErrReadOnly { + result = metrics.ResultReadOnly + } else { + result = metrics.ResultFailure + var lerr *ldap.Error + if errors.As(err, &lerr) { + if lerr != nil && lerr.ResultCode == ldap.LDAPResultNoSuchObject { + result = metrics.ResultNotFound + } + } + } + } + + _d.timer.WithLabelValues(LdapOpModify, result).Observe(duration) + }() + + return _d.delegate.Modify(mp1) +} + +// ModifyDN implements LdapClient.ModifyDN +func (_d PrometheusLdapClient) ModifyDN(mp1 *ldap.ModifyDNRequest) (err error) { + _d.inflight.Add(1) + defer _d.inflight.Add(-1) + + _since := time.Now() + defer func() { + duration := time.Since(_since).Seconds() + result := metrics.ResultSuccess + if err != nil { + if err == ErrReadOnly { + result = metrics.ResultReadOnly + } else { + result = metrics.ResultFailure + var lerr *ldap.Error + if errors.As(err, &lerr) { + if lerr != nil && lerr.ResultCode == ldap.LDAPResultNoSuchObject { + result = metrics.ResultNotFound + } + } + } + } + + _d.timer.WithLabelValues(LdapOpModifyDN, result).Observe(duration) + }() + + return _d.delegate.ModifyDN(mp1) +} + +// PasswordModify implements LdapClient.PasswordModify +func (_d PrometheusLdapClient) PasswordModify(pp1 *ldap.PasswordModifyRequest) (pp2 *ldap.PasswordModifyResult, err error) { + _d.inflight.Add(1) + defer _d.inflight.Add(-1) + + _since := time.Now() + defer func() { + duration := time.Since(_since).Seconds() + result := metrics.ResultSuccess + if err != nil { + if err == ErrReadOnly { + result = metrics.ResultReadOnly + } else { + result = metrics.ResultFailure + var lerr *ldap.Error + if errors.As(err, &lerr) { + if lerr != nil && lerr.ResultCode == ldap.LDAPResultNoSuchObject { + result = metrics.ResultNotFound + } + } + } + } + + _d.timer.WithLabelValues(LdapOpPasswordModify, result).Observe(duration) + }() + + return _d.delegate.PasswordModify(pp1) +} + +// Search implements LdapClient.Search +func (_d PrometheusLdapClient) Search(sp1 *ldap.SearchRequest) (sp2 *ldap.SearchResult, err error) { + _d.inflight.Add(1) + defer _d.inflight.Add(-1) + + _since := time.Now() + defer func() { + duration := time.Since(_since).Seconds() + result := metrics.ResultSuccess + if err != nil { + if err == ErrReadOnly { + result = metrics.ResultReadOnly + } else { + result = metrics.ResultFailure + var lerr *ldap.Error + if errors.As(err, &lerr) { + if lerr != nil && lerr.ResultCode == ldap.LDAPResultNoSuchObject { + result = metrics.ResultNotFound + } + } + } + } + + _d.timer.WithLabelValues(LdapOpSearch, result).Observe(duration) + }() + + return _d.delegate.Search(sp1) +} diff --git a/services/graph/pkg/identity/ldap_client_prometheus.tmpl b/services/graph/pkg/identity/ldap_client_prometheus.tmpl new file mode 100644 index 0000000000..5a2fc019b5 --- /dev/null +++ b/services/graph/pkg/identity/ldap_client_prometheus.tmpl @@ -0,0 +1,62 @@ +import ( + "errors" + "time" + "sync/atomic" + + "github.com/go-ldap/ldap/v3" + "github.com/opencloud-eu/opencloud/services/graph/pkg/metrics" + "github.com/prometheus/client_golang/prometheus" +) + +{{ $decorator := (or .Vars.DecoratorName (printf "Prometheus%s" .Interface.Name)) }} + +// {{$decorator}} implements {{.Interface.Type}} interface with all methods wrapped +// with Prometheus metrics +type {{$decorator}} struct { + delegate {{.Interface.Type}} + timer *prometheus.HistogramVec + inflight *atomic.Int64 +} + +var _ {{.Interface.Type}} = &{{$decorator}}{} + +// returns an instance of the {{.Interface.Type}} decorated with prometheus metric +func New{{$decorator}}(delegate {{.Interface.Type}}, timer *prometheus.HistogramVec, inflight *atomic.Int64) {{$decorator}} { + return {{$decorator}} { + delegate: delegate, + timer: timer, + inflight: inflight, + } +} + +{{range $method := .Interface.Methods}} + // {{$method.Name}} implements {{$.Interface.Type}}.{{$method.Name}} + func (_d {{$decorator}}) {{$method.Declaration}} { + _d.inflight.Add(1) + defer _d.inflight.Add(-1) + + _since := time.Now() + defer func() { + duration := time.Since(_since).Seconds() + result := metrics.ResultSuccess + {{- if $method.ReturnsError}} + if err != nil { + if err == ErrReadOnly { + result = metrics.ResultReadOnly + } else { + result = metrics.ResultFailure + var lerr *ldap.Error + if errors.As(err, &lerr) { + if lerr != nil && lerr.ResultCode == ldap.LDAPResultNoSuchObject { + result = metrics.ResultNotFound + } + } + } + } + {{end}} + _d.timer.WithLabelValues(LdapOp{{upFirst $method.Name}}, result).Observe(duration) + }() + + {{$method.Pass "_d.delegate."}} + } +{{end}} diff --git a/services/graph/pkg/identity/ldap_education_class.go b/services/graph/pkg/identity/ldap_education_class.go index f9c3d47cbc..c5d3230c6f 100644 --- a/services/graph/pkg/identity/ldap_education_class.go +++ b/services/graph/pkg/identity/ldap_education_class.go @@ -28,7 +28,7 @@ func newEducationClassAttributeMap() educationClassAttributeMap { // GetEducationClasses implements the EducationBackend interface for the LDAP backend. func (i *LDAP) GetEducationClasses(ctx context.Context) ([]*libregraph.EducationClass, error) { logger := i.logger.SubloggerWithRequestID(ctx) - logger.Debug().Str("backend", "ldap").Msg("GetEducationClasses") + logger.Debug().Msg("GetEducationClasses") classFilter := fmt.Sprintf("(&%s(objectClass=%s))", i.groupFilter, i.educationConfig.classObjectClass) @@ -40,7 +40,7 @@ func (i *LDAP) GetEducationClasses(ctx context.Context) ([]*libregraph.Education classAttrs, nil, ) - logger.Debug().Str("backend", "ldap"). + logger.Debug(). Str("base", searchRequest.BaseDN). Str("filter", searchRequest.Filter). Int("scope", searchRequest.Scope). @@ -56,7 +56,9 @@ func (i *LDAP) GetEducationClasses(ctx context.Context) ([]*libregraph.Education var c *libregraph.EducationClass for _, e := range res.Entries { - if c = i.createEducationClassModelFromLDAP(e); c == nil { + if c, err = i.createEducationClassModelFromLDAP(e); err != nil { + // TODO: should we really just silently ignore invalid LDAP entries here, or rather return it as an error? + } else if c == nil { continue } classes = append(classes, c) @@ -69,7 +71,7 @@ func (i *LDAP) GetEducationClasses(ctx context.Context) ([]*libregraph.Education // With a few additional Attributes added on top via the "openCloudEducationClass" auxiliary ObjectClass. func (i *LDAP) CreateEducationClass(ctx context.Context, class libregraph.EducationClass) (*libregraph.EducationClass, error) { logger := i.logger.SubloggerWithRequestID(ctx) - logger.Debug().Str("backend", "ldap").Msg("create educationClass") + logger.Debug().Msg("create educationClass") if !i.writeEnabled { return nil, errorcode.New(errorcode.NotAllowed, "server is configured read-only") } @@ -94,19 +96,20 @@ func (i *LDAP) CreateEducationClass(ctx context.Context, class libregraph.Educat if err != nil { return nil, err } - return i.createEducationClassModelFromLDAP(e), nil + return i.createEducationClassModelFromLDAP(e) } // GetEducationClass implements the EducationBackend interface for the LDAP backend. func (i *LDAP) GetEducationClass(ctx context.Context, id string) (*libregraph.EducationClass, error) { logger := i.logger.SubloggerWithRequestID(ctx) - logger.Debug().Str("backend", "ldap").Msg("GetEducationClass") + logger.Debug().Msg("GetEducationClass") e, err := i.getEducationClassByID(id, false) if err != nil { return nil, err } var class *libregraph.EducationClass - if class = i.createEducationClassModelFromLDAP(e); class == nil { + if class, err = i.createEducationClassModelFromLDAP(e); err != nil || class == nil { + // TODO: should we really just mask invalid LDAP entries as 'not found' here, or rather the actual error? return nil, errorcode.New(errorcode.ItemNotFound, "not found") } return class, nil @@ -115,7 +118,7 @@ func (i *LDAP) GetEducationClass(ctx context.Context, id string) (*libregraph.Ed // DeleteEducationClass implements the EducationBackend interface for the LDAP backend. func (i *LDAP) DeleteEducationClass(ctx context.Context, id string) error { logger := i.logger.SubloggerWithRequestID(ctx) - logger.Debug().Str("backend", "ldap").Msg("DeleteEducationClass") + logger.Debug().Msg("DeleteEducationClass") if !i.writeEnabled { return ErrReadOnly } @@ -137,7 +140,7 @@ func (i *LDAP) DeleteEducationClass(ctx context.Context, id string) error { // Only the displayName and externalID are supported to change at this point. func (i *LDAP) UpdateEducationClass(ctx context.Context, id string, class libregraph.EducationClass) (*libregraph.EducationClass, error) { logger := i.logger.SubloggerWithRequestID(ctx) - logger.Debug().Str("backend", "ldap").Msg("UpdateEducationClass") + logger.Debug().Msg("UpdateEducationClass") if !i.writeEnabled { return nil, ErrReadOnly } @@ -203,7 +206,7 @@ func (i *LDAP) UpdateEducationClass(ctx context.Context, id string, class libreg return nil, err } - return i.createEducationClassModelFromLDAP(g), nil + return i.createEducationClassModelFromLDAP(g) } func (i *LDAP) updateClassExternalID(ctx context.Context, dn, externalID string) (string, error) { @@ -211,7 +214,7 @@ func (i *LDAP) updateClassExternalID(ctx context.Context, dn, externalID string) newDN := fmt.Sprintf("openCloudEducationExternalId=%s", externalID) mrdn := ldap.NewModifyDNRequest(dn, newDN, true, "") - i.logger.Debug().Str("Backend", "ldap"). + i.logger.Debug(). Str("dn", mrdn.DN). Str("newrdn", mrdn.NewRDN). Msg("updating class external ID") @@ -233,7 +236,7 @@ func (i *LDAP) updateClassExternalID(ctx context.Context, dn, externalID string) // GetEducationClassMembers implements the EducationBackend interface for the LDAP backend. func (i *LDAP) GetEducationClassMembers(ctx context.Context, id string) ([]*libregraph.EducationUser, error) { logger := i.logger.SubloggerWithRequestID(ctx) - logger.Debug().Str("backend", "ldap").Msg("GetEducationClassMembers") + logger.Debug().Msg("GetEducationClassMembers") e, err := i.getEducationClassByID(id, true) if err != nil { return nil, err @@ -245,7 +248,7 @@ func (i *LDAP) GetEducationClassMembers(ctx context.Context, id string) ([]*libr return nil, err } for _, member := range memberEntries { - if u := i.createEducationUserModelFromLDAP(member); u != nil { + if u, err := i.createEducationUserModelFromLDAP(member); u != nil && err == nil { result = append(result, u) } } @@ -315,12 +318,18 @@ func (i *LDAP) getEducationClassByDN(dn string) (*ldap.Entry, error) { return i.getEntryByDN(dn, i.getEducationClassAttrTypes(false), filter) } -func (i *LDAP) createEducationClassModelFromLDAP(e *ldap.Entry) *libregraph.EducationClass { - group := i.createGroupModelFromLDAP(e) - return i.groupToEducationClass(*group, e) +func (i *LDAP) createEducationClassModelFromLDAP(e *ldap.Entry) (*libregraph.EducationClass, error) { + if e == nil { + return nil, nil + } + if group, err := i.createGroupModelFromLDAP(e); err != nil { + return nil, err + } else { + return i.groupToEducationClass(*group, e) + } } -func (i *LDAP) groupToEducationClass(group libregraph.Group, e *ldap.Entry) *libregraph.EducationClass { +func (i *LDAP) groupToEducationClass(group libregraph.Group, e *ldap.Entry) (*libregraph.EducationClass, error) { class := libregraph.NewEducationClass(group.GetDisplayName(), "") class.SetId(group.GetId()) @@ -334,7 +343,7 @@ func (i *LDAP) groupToEducationClass(group libregraph.Group, e *ldap.Entry) *lib } } - return class + return class, nil } func (i *LDAP) getEducationClassLDAPDN(class libregraph.EducationClass) string { @@ -345,6 +354,12 @@ func (i *LDAP) getEducationClassLDAPDN(class libregraph.EducationClass) string { return fmt.Sprintf("%s,%s", attributeTypeAndValue.String(), i.groupBaseDN) } +// Retrieves a single class from LDAP by its namd or id. +// +// It never returns nil for the *ldap.Entry: +// - if no class is found, it returns a ErrNotFound error +// - if more than one class is found, it returns a ErrTooManyResults error +// - if exactly one class is found, it returns that entry and no error func (i *LDAP) getEducationClassByID(nameOrID string, requestMembers bool) (*ldap.Entry, error) { return i.getEducationObjectByNameOrID( nameOrID, @@ -372,7 +387,7 @@ func (i *LDAP) GetEducationClassTeachers(ctx context.Context, classID string) ([ return nil, err } for _, teacher := range teacherEntries { - if u := i.createEducationUserModelFromLDAP(teacher); u != nil { + if u, err := i.createEducationUserModelFromLDAP(teacher); u != nil && err == nil { result = append(result, u) } } diff --git a/services/graph/pkg/identity/ldap_education_class_test.go b/services/graph/pkg/identity/ldap_education_class_test.go index 4c0467d4ee..767946de0c 100644 --- a/services/graph/pkg/identity/ldap_education_class_test.go +++ b/services/graph/pkg/identity/ldap_education_class_test.go @@ -92,7 +92,7 @@ func TestGetEducationClasses(t *testing.T) { g, err = b.GetEducationClasses(context.Background()) if err != nil { t.Errorf("Expected GetEducationClasses to succeed. Got %s", err.Error()) - } else if *g[0].Id != classEntry.GetEqualFoldAttributeValue(b.groupAttributeMap.id) { + } else if len(g) == 0 || *g[0].Id != classEntry.GetEqualFoldAttributeValue(b.groupAttributeMap.id) { t.Errorf("Expected GetEducationClasses to return a valid group") } } diff --git a/services/graph/pkg/identity/ldap_education_school.go b/services/graph/pkg/identity/ldap_education_school.go index 68ee855b61..b6818e4dcd 100644 --- a/services/graph/pkg/identity/ldap_education_school.go +++ b/services/graph/pkg/identity/ldap_education_school.go @@ -116,7 +116,7 @@ func newSchoolAttributeMap() schoolAttributeMap { // CreateEducationSchool creates the supplied school in the identity backend. func (i *LDAP) CreateEducationSchool(ctx context.Context, school libregraph.EducationSchool) (*libregraph.EducationSchool, error) { logger := i.logger.SubloggerWithRequestID(ctx) - logger.Debug().Str("backend", "ldap").Msg("CreateEducationSchool") + logger.Debug().Msg("CreateEducationSchool") if !i.writeEnabled { return nil, ErrReadOnly } @@ -175,7 +175,7 @@ func (i *LDAP) CreateEducationSchool(ctx context.Context, school libregraph.Educ if err != nil { return nil, err } - return i.createSchoolModelFromLDAP(e), nil + return i.createSchoolModelFromLDAP(e) } // UpdateEducationSchoolOperation contains the logic for which update operation to apply to a school @@ -229,7 +229,7 @@ func (i *LDAP) updateDisplayName(ctx context.Context, dn string, providedDisplay } mrdn := ldap.NewModifyDNRequest(dn, attributeTypeAndValue.String(), true, "") - i.logger.Debug().Str("backend", "ldap"). + i.logger.Debug(). Str("dn", mrdn.DN). Str("newrdn", mrdn.NewRDN). Msg("updateDisplayName") @@ -286,7 +286,7 @@ func (i *LDAP) updateSchoolProperties(ctx context.Context, dn string, currentSch // UpdateEducationSchool updates the supplied school in the identity backend func (i *LDAP) UpdateEducationSchool(ctx context.Context, numberOrID string, school libregraph.EducationSchool) (*libregraph.EducationSchool, error) { logger := i.logger.SubloggerWithRequestID(ctx) - logger.Debug().Str("backend", "ldap").Msg("UpdateEducationSchool") + logger.Debug().Msg("UpdateEducationSchool") if !i.writeEnabled { return nil, ErrReadOnly } @@ -296,12 +296,15 @@ func (i *LDAP) UpdateEducationSchool(ctx context.Context, numberOrID string, sch return nil, err } - currentSchool := i.createSchoolModelFromLDAP(e) + currentSchool, err := i.createSchoolModelFromLDAP(e) + if err != nil { + return nil, err + } switch i.updateEducationSchoolOperation(school, *currentSchool) { case tooManyValues: - return nil, fmt.Errorf("school name and school number cannot be updated in the same request") + return nil, errors.New("school name and school number cannot be updated in the same request") case schoolUnchanged: - logger.Debug().Str("backend", "ldap").Msg("UpdateEducationSchool: Nothing changed") + logger.Debug().Msg("UpdateEducationSchool: Nothing changed") return currentSchool, nil case schoolRenamed: if err := i.updateDisplayName(ctx, e.DN, school.GetDisplayName()); err != nil { @@ -318,16 +321,17 @@ func (i *LDAP) UpdateEducationSchool(ctx context.Context, numberOrID string, sch if err != nil { return nil, err } - return i.createSchoolModelFromLDAP(e), nil + return i.createSchoolModelFromLDAP(e) } // DeleteEducationSchool deletes a given school, identified by id func (i *LDAP) DeleteEducationSchool(ctx context.Context, id string) error { logger := i.logger.SubloggerWithRequestID(ctx) - logger.Debug().Str("backend", "ldap").Msg("DeleteEducationSchool") + logger.Debug().Msg("DeleteEducationSchool") if !i.writeEnabled { return ErrReadOnly } + e, err := i.getSchoolByNumberOrID(id) if err != nil { return err @@ -345,13 +349,14 @@ func (i *LDAP) DeleteEducationSchool(ctx context.Context, id string) error { // GetEducationSchool implements the EducationBackend interface for the LDAP backend. func (i *LDAP) GetEducationSchool(ctx context.Context, numberOrID string) (*libregraph.EducationSchool, error) { logger := i.logger.SubloggerWithRequestID(ctx) - logger.Debug().Str("backend", "ldap").Msg("GetEducationSchool") + logger.Debug().Msg("GetEducationSchool") + e, err := i.getSchoolByNumberOrID(numberOrID) if err != nil { return nil, err } - return i.createSchoolModelFromLDAP(e), nil + return i.createSchoolModelFromLDAP(e) } // GetEducationSchools implements the EducationBackend interface for the LDAP backend. @@ -366,7 +371,7 @@ func (i *LDAP) GetEducationSchools(ctx context.Context) ([]*libregraph.Education // FilterEducationSchoolsByAttribute implements the EducationBackend interface for the LDAP backend. func (i *LDAP) FilterEducationSchoolsByAttribute(ctx context.Context, attr, value string) ([]*libregraph.EducationSchool, error) { logger := i.logger.SubloggerWithRequestID(ctx).With().Str("func", "FilterEducationSchoolsByAttribute").Logger() - logger.Debug().Str("backend", "ldap").Str("attribute", attr).Str("value", value).Msg("") + logger.Debug().Str("attribute", attr).Str("value", value).Send() var ldapAttr string switch attr { @@ -395,7 +400,7 @@ func (i *LDAP) searchEducationSchools(ctx context.Context, filter string) ([]*li nil, ) logger := i.logger.SubloggerWithRequestID(ctx) - logger.Debug().Str("backend", "ldap"). + logger.Debug(). Str("base", searchRequest.BaseDN). Str("filter", searchRequest.Filter). Int("scope", searchRequest.Scope). @@ -405,17 +410,27 @@ func (i *LDAP) searchEducationSchools(ctx context.Context, filter string) ([]*li res, err := i.conn.Search(searchRequest) if err != nil { - return nil, errorcode.New(errorcode.ItemNotFound, err.Error()) + msg := "school search failed" + errMap := ldapResultToErrMap{ + ldap.LDAPResultNoSuchObject: errorcode.New(errorcode.ItemNotFound, msg), + ldap.LDAPResultUnwillingToPerform: errorcode.New(errorcode.NotAllowed, msg), + ldap.LDAPResultInsufficientAccessRights: errorcode.New(errorcode.NotAllowed, msg), + ldapGenericErr: errorcode.New(errorcode.GeneralException, msg), + } + return nil, i.mapLDAPError(err, errMap) + } + if res == nil { + return nil, ErrNotFound } schools := make([]*libregraph.EducationSchool, 0, len(res.Entries)) for _, e := range res.Entries { - school := i.createSchoolModelFromLDAP(e) - // Skip invalid LDAP entries - if school == nil { - continue + if school, err := i.createSchoolModelFromLDAP(e); err != nil || school == nil { + // Skip invalid LDAP entries + // TODO: is it really the best idea to silently skip school LDAP data that is invalid, rather than returning an error? + } else { + schools = append(schools, school) } - schools = append(schools, school) } return schools, nil } @@ -423,7 +438,7 @@ func (i *LDAP) searchEducationSchools(ctx context.Context, filter string) ([]*li // GetEducationSchoolUsers implements the EducationBackend interface for the LDAP backend. func (i *LDAP) GetEducationSchoolUsers(ctx context.Context, schoolNumberOrID string) ([]*libregraph.EducationUser, error) { logger := i.logger.SubloggerWithRequestID(ctx) - logger.Debug().Str("backend", "ldap").Msg("GetEducationSchoolUsers") + logger.Debug().Msg("GetEducationSchoolUsers") entries, err := i.getEducationSchoolEntries( schoolNumberOrID, i.userFilter, i.educationConfig.userObjectClass, i.userBaseDN, i.userScope, i.getEducationUserAttrTypes(), logger, @@ -435,12 +450,12 @@ func (i *LDAP) GetEducationSchoolUsers(ctx context.Context, schoolNumberOrID str users := make([]*libregraph.EducationUser, 0, len(entries)) for _, e := range entries { - u := i.createEducationUserModelFromLDAP(e) - // Skip invalid LDAP users - if u == nil { - continue + if u, err := i.createEducationUserModelFromLDAP(e); u != nil && err == nil { + users = append(users, u) + } else { + // Skip invalid LDAP users + // TODO: is it really the best idea to silently skip school user LDAP data that is invalid, rather than returning an error? } - users = append(users, u) } return users, nil } @@ -448,13 +463,12 @@ func (i *LDAP) GetEducationSchoolUsers(ctx context.Context, schoolNumberOrID str // AddUsersToEducationSchool adds new members (reference by a slice of IDs) to supplied school in the identity backend. func (i *LDAP) AddUsersToEducationSchool(ctx context.Context, schoolNumberOrID string, memberIDs []string) error { logger := i.logger.SubloggerWithRequestID(ctx) - logger.Debug().Str("backend", "ldap").Msg("AddUsersToEducationSchool") + logger.Debug().Msg("AddUsersToEducationSchool") schoolEntry, err := i.getSchoolByNumberOrID(schoolNumberOrID) if err != nil { return err } - if schoolEntry == nil { return ErrNotFound } @@ -463,12 +477,12 @@ func (i *LDAP) AddUsersToEducationSchool(ctx context.Context, schoolNumberOrID s userEntries := make([]*ldap.Entry, 0, len(memberIDs)) for _, memberID := range memberIDs { - user, err := i.getEducationUserByNameOrID(memberID) - if err != nil { - i.logger.Warn().Str("userid", memberID).Msg("User does not exist") + if user, err := i.getEducationUserByNameOrID(memberID); err != nil { + i.logger.Warn().Err(err).Str("userid", memberID).Msg("Failed to retrieve education user") return errorcode.New(errorcode.ItemNotFound, fmt.Sprintf("user '%s' not found", memberID)) + } else { + userEntries = append(userEntries, user) } - userEntries = append(userEntries, user) } for _, userEntry := range userEntries { @@ -494,23 +508,20 @@ func (i *LDAP) addEntryToSchool(entry *ldap.Entry, schoolID string) error { // RemoveUserFromEducationSchool removes a single member (by ID) from a school func (i *LDAP) RemoveUserFromEducationSchool(ctx context.Context, schoolNumberOrID string, memberID string) error { logger := i.logger.SubloggerWithRequestID(ctx) - logger.Debug().Str("backend", "ldap").Msg("RemoveUserFromEducationSchool") + logger.Debug().Msg("RemoveUserFromEducationSchool") schoolEntry, err := i.getSchoolByNumberOrID(schoolNumberOrID) if err != nil { return err } - if schoolEntry == nil { - return ErrNotFound - } - schoolID := schoolEntry.GetEqualFoldAttributeValue(i.educationConfig.schoolAttributeMap.id) user, err := i.getEducationUserByNameOrID(memberID) if err != nil { - i.logger.Warn().Str("userid", memberID).Msg("User does not exist") + i.logger.Warn().Err(err).Str("userid", memberID).Msg("Failed to retrieve education user") return err } + currentSchools := user.GetEqualFoldAttributeValues(i.educationConfig.memberOfSchoolAttribute) for _, currentSchool := range currentSchools { if currentSchool == schoolID { @@ -528,7 +539,7 @@ func (i *LDAP) RemoveUserFromEducationSchool(ctx context.Context, schoolNumberOr // GetEducationSchoolClasses implements the EducationBackend interface for the LDAP backend. func (i *LDAP) GetEducationSchoolClasses(ctx context.Context, schoolNumberOrID string) ([]*libregraph.EducationClass, error) { logger := i.logger.SubloggerWithRequestID(ctx) - logger.Debug().Str("backend", "ldap").Msg("GetEducationSchoolClasses") + logger.Debug().Msg("GetEducationSchoolClasses") entries, err := i.getEducationSchoolEntries( schoolNumberOrID, i.groupFilter, i.educationConfig.classObjectClass, i.groupBaseDN, i.groupScope, i.getEducationClassAttrTypes(false), logger, @@ -540,12 +551,13 @@ func (i *LDAP) GetEducationSchoolClasses(ctx context.Context, schoolNumberOrID s classes := make([]*libregraph.EducationClass, 0, len(entries)) for _, e := range entries { - class := i.createEducationClassModelFromLDAP(e) - // Skip invalid LDAP classes - if class == nil { - continue + if class, err := i.createEducationClassModelFromLDAP(e); err != nil || class == nil { + // Skip invalid LDAP classes + logger.Warn().Err(err).Str("school-number", schoolNumberOrID).Interface("entry", e).Msg("failed to create class model from LDAP") + continue // TODO: should we really silently skip invalid LDAP data here, or rather return this as an error? + } else { + classes = append(classes, class) } - classes = append(classes, class) } return classes, nil } @@ -578,7 +590,7 @@ func (i *LDAP) getEducationSchoolEntries( attributes, nil, ) - logger.Debug().Str("backend", "ldap"). + logger.Debug(). Str("base", searchRequest.BaseDN). Str("filter", searchRequest.Filter). Int("scope", searchRequest.Scope). @@ -587,7 +599,17 @@ func (i *LDAP) getEducationSchoolEntries( Msg("GetEducationClasses") res, err := i.conn.Search(searchRequest) if err != nil { - return nil, errorcode.New(errorcode.ItemNotFound, err.Error()) + msg := "school search failed" + errMap := ldapResultToErrMap{ + ldap.LDAPResultNoSuchObject: ErrNotFound, + ldap.LDAPResultUnwillingToPerform: errorcode.New(errorcode.NotAllowed, msg), + ldap.LDAPResultInsufficientAccessRights: errorcode.New(errorcode.NotAllowed, msg), + ldapGenericErr: errorcode.New(errorcode.GeneralException, msg), + } + return nil, i.mapLDAPError(err, errMap) + } + if res == nil { + return nil, ErrNotFound } return res.Entries, nil } @@ -595,13 +617,12 @@ func (i *LDAP) getEducationSchoolEntries( // AddClassesToEducationSchool adds new members (reference by a slice of IDs) to supplied school in the identity backend. func (i *LDAP) AddClassesToEducationSchool(ctx context.Context, schoolNumberOrID string, memberIDs []string) error { logger := i.logger.SubloggerWithRequestID(ctx) - logger.Debug().Str("backend", "ldap").Msg("AddClassesToEducationSchool") + logger.Debug().Msg("AddClassesToEducationSchool") schoolEntry, err := i.getSchoolByNumberOrID(schoolNumberOrID) if err != nil { return err } - if schoolEntry == nil { return ErrNotFound } @@ -612,7 +633,7 @@ func (i *LDAP) AddClassesToEducationSchool(ctx context.Context, schoolNumberOrID for _, memberID := range memberIDs { class, err := i.getEducationClassByID(memberID, false) if err != nil { - i.logger.Warn().Str("userid", memberID).Msg("Class does not exist") + i.logger.Warn().Err(err).Str("userid", memberID).Msg("Failed to retrieve class") return err } classEntries = append(classEntries, class) @@ -630,13 +651,12 @@ func (i *LDAP) AddClassesToEducationSchool(ctx context.Context, schoolNumberOrID // RemoveClassFromEducationSchool removes a single member (by ID) from a school func (i *LDAP) RemoveClassFromEducationSchool(ctx context.Context, schoolNumberOrID string, memberID string) error { logger := i.logger.SubloggerWithRequestID(ctx) - logger.Debug().Str("backend", "ldap").Msg("RemoveClassFromEducationSchool") + logger.Debug().Msg("RemoveClassFromEducationSchool") schoolEntry, err := i.getSchoolByNumberOrID(schoolNumberOrID) if err != nil { return err } - if schoolEntry == nil { return ErrNotFound } @@ -644,7 +664,7 @@ func (i *LDAP) RemoveClassFromEducationSchool(ctx context.Context, schoolNumberO schoolID := schoolEntry.GetEqualFoldAttributeValue(i.educationConfig.schoolAttributeMap.id) class, err := i.getEducationClassByID(memberID, false) if err != nil { - i.logger.Warn().Str("userid", memberID).Msg("Class does not exist") + i.logger.Warn().Err(err).Str("userid", memberID).Msg("Failed to retrieve class") return err } currentSchools := class.GetEqualFoldAttributeValues(i.educationConfig.memberOfSchoolAttribute) @@ -661,6 +681,12 @@ func (i *LDAP) RemoveClassFromEducationSchool(ctx context.Context, schoolNumberO return nil } +// Retrieves a single school from LDAP by its DN +// +// It never returns nil for the *ldap.Entry: +// - if no school is found, it returns a ErrNotFound error +// - if more than one school is found, it returns a ErrTooManyResults error +// - if exactly one school is found, it returns that entry and no error func (i *LDAP) getSchoolByDN(dn string) (*ldap.Entry, error) { filter := fmt.Sprintf("(objectClass=%s)", i.educationConfig.schoolObjectClass) @@ -670,6 +696,12 @@ func (i *LDAP) getSchoolByDN(dn string) (*ldap.Entry, error) { return i.getEntryByDN(dn, i.getEducationSchoolAttrTypes(), filter) } +// Retrieves a single school from LDAP by a given school number or id +// +// It never returns nil for the *ldap.Entry: +// - if no school is found, it returns a ErrNotFound error +// - if more than one school is found, it returns a ErrTooManyResults error +// - if exactly one school is found, it returns that entry and no error func (i *LDAP) getSchoolByNumberOrID(numberOrID string) (*ldap.Entry, error) { numberOrID = ldap.EscapeFilter(numberOrID) filter := fmt.Sprintf( @@ -682,6 +714,12 @@ func (i *LDAP) getSchoolByNumberOrID(numberOrID string) (*ldap.Entry, error) { return i.getSchoolByFilter(filter) } +// Retrieves a single school from LDAP by a given school number +// +// It never returns nil for the *ldap.Entry: +// - if no school is found, it returns a ErrNotFound error +// - if more than one school is found, it returns a ErrTooManyResults error +// - if exactly one school is found, it returns that entry and no error func (i *LDAP) getSchoolByNumber(schoolNumber string) (*ldap.Entry, error) { schoolNumber = ldap.EscapeFilter(schoolNumber) filter := fmt.Sprintf( @@ -692,6 +730,12 @@ func (i *LDAP) getSchoolByNumber(schoolNumber string) (*ldap.Entry, error) { return i.getSchoolByFilter(filter) } +// Retrieves a single school from LDAP by a given filter +// +// It never returns nil for the *ldap.Entry: +// - if no school is found, it returns a ErrNotFound error +// - if more than one school is found, it returns a ErrTooManyResults error +// - if exactly one school is found, it returns that entry and no error func (i *LDAP) getSchoolByFilter(filter string) (*ldap.Entry, error) { filter = fmt.Sprintf("(&%s(objectClass=%s)%s)", i.educationConfig.schoolFilter, @@ -706,7 +750,7 @@ func (i *LDAP) getSchoolByFilter(filter string) (*ldap.Entry, error) { i.getEducationSchoolAttrTypes(), nil, ) - i.logger.Debug().Str("backend", "ldap"). + i.logger.Debug(). Str("base", searchRequest.BaseDN). Str("filter", searchRequest.Filter). Int("scope", searchRequest.Scope). @@ -715,26 +759,37 @@ func (i *LDAP) getSchoolByFilter(filter string) (*ldap.Entry, error) { Msg("getSchoolByFilter") res, err := i.conn.Search(searchRequest) if err != nil { - var errmsg string if lerr, ok := err.(*ldap.Error); ok { if lerr.ResultCode == ldap.LDAPResultSizeLimitExceeded { - errmsg = fmt.Sprintf("too many results searching for school '%s'", filter) - i.logger.Debug().Str("backend", "ldap").Err(lerr). + errmsg := "too many results searching for school" + i.logger.Warn().Err(lerr). Str("schoolfilter", filter).Msg("too many results searching for school") + return nil, errorcode.New(errorcode.TooManyResults, errmsg) } } - return nil, errorcode.New(errorcode.ItemNotFound, errmsg) + msg := "school search failed" + errMap := ldapResultToErrMap{ + ldap.LDAPResultNoSuchObject: errorcode.New(errorcode.ItemNotFound, msg), + ldap.LDAPResultUnwillingToPerform: errorcode.New(errorcode.NotAllowed, msg), + ldap.LDAPResultInsufficientAccessRights: errorcode.New(errorcode.NotAllowed, msg), + ldapGenericErr: errorcode.New(errorcode.GeneralException, msg), + } + return nil, i.mapLDAPError(err, errMap) } - if len(res.Entries) == 0 { + + switch len(res.Entries) { + case 0: return nil, ErrNotFound + case 1: + return res.Entries[0], nil + default: + return nil, ErrTooManyResults } - - return res.Entries[0], nil } -func (i *LDAP) createSchoolModelFromLDAP(e *ldap.Entry) *libregraph.EducationSchool { +func (i *LDAP) createSchoolModelFromLDAP(e *ldap.Entry) (*libregraph.EducationSchool, error) { if e == nil { - return nil + return nil, nil } displayName := i.getDisplayName(e) @@ -750,7 +805,7 @@ func (i *LDAP) createSchoolModelFromLDAP(e *ldap.Entry) *libregraph.EducationSch if id == "" || displayName == "" { i.logger.Warn().Str("dn", e.DN).Str("id", id).Str("displayName", displayName).Msg("Invalid School. Missing required attribute") - return nil + return nil, errors.New("Invalid school: missing required attribute id or displayName") } school := libregraph.NewEducationSchool() @@ -765,7 +820,7 @@ func (i *LDAP) createSchoolModelFromLDAP(e *ldap.Entry) *libregraph.EducationSch if t != nil { school.SetTerminationDate(*t) } - return school + return school, nil } func (i *LDAP) getSchoolNumber(e *ldap.Entry) string { diff --git a/services/graph/pkg/identity/ldap_education_user.go b/services/graph/pkg/identity/ldap_education_user.go index 88855d52e0..57acc0255c 100644 --- a/services/graph/pkg/identity/ldap_education_user.go +++ b/services/graph/pkg/identity/ldap_education_user.go @@ -25,7 +25,7 @@ func newEducationUserAttributeMap() educationUserAttributeMap { // CreateEducationUser creates a given education user in the identity backend. func (i *LDAP) CreateEducationUser(ctx context.Context, user libregraph.EducationUser) (*libregraph.EducationUser, error) { logger := i.logger.SubloggerWithRequestID(ctx) - logger.Debug().Str("backend", "ldap").Msg("CreateEducationUser") + logger.Debug().Msg("CreateEducationUser") if !i.writeEnabled { return nil, ErrReadOnly } @@ -51,13 +51,13 @@ func (i *LDAP) CreateEducationUser(ctx context.Context, user libregraph.Educatio if err != nil { return nil, err } - return i.createEducationUserModelFromLDAP(e), nil + return i.createEducationUserModelFromLDAP(e) } // DeleteEducationUser deletes a given education user, identified by username or id, from the backend func (i *LDAP) DeleteEducationUser(ctx context.Context, nameOrID string) error { logger := i.logger.SubloggerWithRequestID(ctx) - logger.Debug().Str("backend", "ldap").Msg("DeleteEducationUser") + logger.Debug().Msg("DeleteEducationUser") if !i.writeEnabled { return ErrReadOnly } @@ -77,7 +77,7 @@ func (i *LDAP) DeleteEducationUser(ctx context.Context, nameOrID string) error { // UpdateEducationUser applies changes to given education user, identified by username or id func (i *LDAP) UpdateEducationUser(ctx context.Context, nameOrID string, user libregraph.EducationUser) (*libregraph.EducationUser, error) { logger := i.logger.SubloggerWithRequestID(ctx) - logger.Debug().Str("backend", "ldap").Msg("UpdateEducationUser") + logger.Debug().Msg("UpdateEducationUser") if !i.writeEnabled { return nil, ErrReadOnly } @@ -180,7 +180,10 @@ func (i *LDAP) UpdateEducationUser(ctx context.Context, nameOrID string, user li return nil, err } - returnUser := i.createEducationUserModelFromLDAP(e) + returnUser, err := i.createEducationUserModelFromLDAP(e) + if err != nil { + return nil, err + } // To avoid a ldap lookup for group membership, set the enabled flag to same as input value // since this would have been updated with group membership from the input anyway. @@ -194,14 +197,14 @@ func (i *LDAP) UpdateEducationUser(ctx context.Context, nameOrID string, user li // GetEducationUser implements the EducationBackend interface for the LDAP backend. func (i *LDAP) GetEducationUser(ctx context.Context, nameOrID string) (*libregraph.EducationUser, error) { logger := i.logger.SubloggerWithRequestID(ctx) - logger.Debug().Str("backend", "ldap").Msg("GetEducationUser") + logger.Debug().Msg("GetEducationUser") e, err := i.getEducationUserByNameOrID(nameOrID) if err != nil { return nil, err } - u := i.createEducationUserModelFromLDAP(e) - if u == nil { - return nil, ErrNotFound + u, err := i.createEducationUserModelFromLDAP(e) + if err != nil { + return nil, err } return u, nil } @@ -219,7 +222,7 @@ func (i *LDAP) GetEducationUsers(ctx context.Context) ([]*libregraph.EducationUs func (i *LDAP) FilterEducationUsersByAttribute(ctx context.Context, attr, value string) ([]*libregraph.EducationUser, error) { logger := i.logger.SubloggerWithRequestID(ctx).With().Str("func", "FilterEducationUsersByAttribute").Logger() - logger.Debug().Str("backend", "ldap").Str("attribute", attr).Str("value", value).Msg("") + logger.Debug().Str("attribute", attr).Str("value", value).Msg("") var ldapAttr string switch attr { @@ -251,7 +254,7 @@ func (i *LDAP) searchEducationUsers(ctx context.Context, filter string) ([]*libr nil, ) logger := i.logger.SubloggerWithRequestID(ctx) - logger.Debug().Str("backend", "ldap"). + logger.Debug(). Str("base", searchRequest.BaseDN). Str("filter", searchRequest.Filter). Int("scope", searchRequest.Scope). @@ -266,12 +269,12 @@ func (i *LDAP) searchEducationUsers(ctx context.Context, filter string) ([]*libr users := make([]*libregraph.EducationUser, 0, len(res.Entries)) for _, e := range res.Entries { - u := i.createEducationUserModelFromLDAP(e) - // Skip invalid LDAP users - if u == nil { - continue + if u, err := i.createEducationUserModelFromLDAP(e); u != nil && err == nil { + users = append(users, u) + } else { + // Skip invalid LDAP users + // TODO: is it really the best idea to silently skip education user LDAP data that is invalid, rather than returning an error? } - users = append(users, u) } return users, nil } @@ -288,7 +291,7 @@ func (i *LDAP) educationUserToUser(eduUser libregraph.EducationUser) *libregraph return user } -func (i *LDAP) userToEducationUser(user libregraph.User, e *ldap.Entry) *libregraph.EducationUser { +func (i *LDAP) userToEducationUser(user libregraph.User, e *ldap.Entry) (*libregraph.EducationUser, error) { eduUser := libregraph.NewEducationUser() eduUser.Id = user.Id eduUser.OnPremisesSamAccountName = &user.OnPremisesSamAccountName @@ -310,7 +313,7 @@ func (i *LDAP) userToEducationUser(user libregraph.User, e *ldap.Entry) *libregr } } - return eduUser + return eduUser, nil } func (i *LDAP) educationUserToLDAPAttrValues(user libregraph.EducationUser, attrs ldapAttributeValues) (ldapAttributeValues, error) { @@ -344,8 +347,11 @@ func (i *LDAP) educationUserToAddRequest(user libregraph.EducationUser) (*ldap.A return ar, nil } -func (i *LDAP) createEducationUserModelFromLDAP(e *ldap.Entry) *libregraph.EducationUser { - user := i.createUserModelFromLDAP(e) +func (i *LDAP) createEducationUserModelFromLDAP(e *ldap.Entry) (*libregraph.EducationUser, error) { + user, err := i.createUserModelFromLDAP(e) + if err != nil { + return nil, err + } return i.userToEducationUser(*user, e) } @@ -376,6 +382,12 @@ func (i *LDAP) getEducationUserByDN(dn string) (*ldap.Entry, error) { return i.getEntryByDN(dn, i.getEducationUserAttrTypes(), filter) } +// Retrieves a single education user from LDAP by its namd or id. +// +// It never returns nil for the *ldap.Entry: +// - if no object is found, it returns a ErrNotFound error +// - if more than one object is found, it returns a ErrTooManyResults error +// - if exactly one object is found, it returns that entry and no error func (i *LDAP) getEducationUserByNameOrID(nameOrID string) (*ldap.Entry, error) { return i.getEducationObjectByNameOrID( nameOrID, @@ -388,12 +400,24 @@ func (i *LDAP) getEducationUserByNameOrID(nameOrID string) (*ldap.Entry, error) ) } +// Retrieves a single object from LDAP by its namd or id. +// +// It never returns nil for the *ldap.Entry: +// - if no object is found, it returns a ErrNotFound error +// - if more than one object is found, it returns a ErrTooManyResults error +// - if exactly one object is found, it returns that entry and no error func (i *LDAP) getEducationObjectByNameOrID(nameOrID, nameAttribute, idAttribute, objectFilter, objectClass, baseDN string, attributes []string) (*ldap.Entry, error) { nameOrID = ldap.EscapeFilter(nameOrID) filter := fmt.Sprintf("(|(%s=%s)(%s=%s))", nameAttribute, nameOrID, idAttribute, nameOrID) return i.getEducationObjectByFilter(filter, baseDN, objectFilter, objectClass, attributes) } +// Retrieves a single object from LDAP by a filter. +// +// It never returns nil for the *ldap.Entry: +// - if no object is found, it returns a ErrNotFound error +// - if more than one object is found, it returns a ErrTooManyResults error +// - if exactly one object is found, it returns that entry and no error func (i *LDAP) getEducationObjectByFilter(filter, baseDN, objectFilter, objectClass string, attributes []string) (*ldap.Entry, error) { filter = fmt.Sprintf("(&%s(objectClass=%s)%s)", objectFilter, objectClass, filter) return i.searchLDAPEntryByFilter(baseDN, attributes, filter) diff --git a/services/graph/pkg/identity/ldap_group.go b/services/graph/pkg/identity/ldap_group.go index 3363f7902a..fd274986a0 100644 --- a/services/graph/pkg/identity/ldap_group.go +++ b/services/graph/pkg/identity/ldap_group.go @@ -27,7 +27,8 @@ type groupAttributeMap struct { // GetGroup implements the Backend Interface for the LDAP Backend func (i *LDAP) GetGroup(ctx context.Context, nameOrID string, queryParam url.Values) (*libregraph.Group, error) { logger := i.logger.SubloggerWithRequestID(ctx) - logger.Debug().Str("backend", "ldap").Msg("GetGroup") + logger.Debug().Msg("GetGroup") + e, err := i.getLDAPGroupByNameOrID(nameOrID, true) if err != nil { return nil, err @@ -35,8 +36,8 @@ func (i *LDAP) GetGroup(ctx context.Context, nameOrID string, queryParam url.Val sel := strings.Split(queryParam.Get("$select"), ",") exp := strings.Split(queryParam.Get("$expand"), ",") var g *libregraph.Group - if g = i.createGroupModelFromLDAP(e); g == nil { - return nil, errorcode.New(errorcode.ItemNotFound, "not found") + if g, err = i.createGroupModelFromLDAP(e); err != nil { + return nil, ErrNotFound // TODO: ideally, we would have an error that indicates invalid IDM data instead } if slices.Contains(sel, "members") || slices.Contains(exp, "members") { members, err := i.expandLDAPAttributeEntries(ctx, e, i.groupAttributeMap.member, "") @@ -46,7 +47,7 @@ func (i *LDAP) GetGroup(ctx context.Context, nameOrID string, queryParam url.Val g.Members = make([]libregraph.User, 0, len(members)) if len(members) > 0 { for _, ue := range members { - if u := i.createUserModelFromLDAP(ue); u != nil { + if u, err := i.createUserModelFromLDAP(ue); u != nil && err == nil { g.Members = append(g.Members, *u) } } @@ -58,7 +59,7 @@ func (i *LDAP) GetGroup(ctx context.Context, nameOrID string, queryParam url.Val // GetGroups implements the Backend Interface for the LDAP Backend func (i *LDAP) GetGroups(ctx context.Context, oreq *godata.GoDataRequest) ([]*libregraph.Group, error) { logger := i.logger.SubloggerWithRequestID(ctx) - logger.Debug().Str("backend", "ldap").Msg("GetGroups") + logger.Debug().Msg("GetGroups") search, err := odata.GetSearchValues(oreq.Query) if err != nil { @@ -103,7 +104,7 @@ func (i *LDAP) GetGroups(ctx context.Context, oreq *godata.GoDataRequest) ([]*li groupAttrs, nil, ) - logger.Debug().Str("backend", "ldap"). + logger.Debug(). Str("base", searchRequest.BaseDN). Str("filter", searchRequest.Filter). Int("scope", searchRequest.Scope). @@ -119,8 +120,8 @@ func (i *LDAP) GetGroups(ctx context.Context, oreq *godata.GoDataRequest) ([]*li var g *libregraph.Group for _, e := range res.Entries { - if g = i.createGroupModelFromLDAP(e); g == nil { - continue + if g, err = i.createGroupModelFromLDAP(e); err != nil || g == nil { + continue // TODO: should we really silently skip 'invalid' groups here, or return an error instead? } if expandMembers { members, err := i.expandLDAPAttributeEntries(ctx, e, i.groupAttributeMap.member, "") @@ -130,7 +131,7 @@ func (i *LDAP) GetGroups(ctx context.Context, oreq *godata.GoDataRequest) ([]*li g.Members = make([]libregraph.User, 0, len(members)) if len(members) > 0 { for _, ue := range members { - if u := i.createUserModelFromLDAP(ue); u != nil { + if u, err := i.createUserModelFromLDAP(ue); u != nil && err == nil { g.Members = append(g.Members, *u) } } @@ -144,7 +145,7 @@ func (i *LDAP) GetGroups(ctx context.Context, oreq *godata.GoDataRequest) ([]*li // GetGroupMembers implements the Backend Interface for the LDAP Backend func (i *LDAP) GetGroupMembers(ctx context.Context, groupID string, req *godata.GoDataRequest) ([]*libregraph.User, error) { logger := i.logger.SubloggerWithRequestID(ctx) - logger.Debug().Str("backend", "ldap").Msg("GetGroupMembers") + logger.Debug().Msg("GetGroupMembers") exp, err := odata.GetExpandValues(req.Query) if err != nil { @@ -162,18 +163,22 @@ func (i *LDAP) GetGroupMembers(ctx context.Context, groupID string, req *godata. } memberEntries, err := i.expandLDAPAttributeEntries(ctx, e, i.groupAttributeMap.member, searchTerm) - result := make([]*libregraph.User, 0, len(memberEntries)) if err != nil { return nil, err } + result := make([]*libregraph.User, 0, len(memberEntries)) for _, member := range memberEntries { - if u := i.createUserModelFromLDAP(member); u != nil { + if u, err := i.createUserModelFromLDAP(member); u != nil && err == nil { if slices.Contains(exp, "memberOf") { userGroups, err := i.getGroupsForUser(member.DN) if err != nil { return nil, err } - u.MemberOf = i.groupsFromLDAPEntries(userGroups) + if memberOf, err := i.groupsFromLDAPEntries(userGroups); err != nil { + // TODO: should we really just silently ignore the LDAP data model error here? or return this as an error instead? + } else { + u.MemberOf = memberOf + } } result = append(result, u) } @@ -188,7 +193,7 @@ func (i *LDAP) GetGroupMembers(ctx context.Context, groupID string, req *godata. // without a member) a represented by adding an empty DN as the single member. func (i *LDAP) CreateGroup(ctx context.Context, group libregraph.Group) (*libregraph.Group, error) { logger := i.logger.SubloggerWithRequestID(ctx) - logger.Debug().Str("backend", "ldap").Msg("create group") + logger.Debug().Msg("create group") if !i.writeEnabled && i.groupCreateBaseDN == i.groupBaseDN { return nil, errorcode.New(errorcode.NotAllowed, "server is configured read-only") } @@ -199,7 +204,7 @@ func (i *LDAP) CreateGroup(ctx context.Context, group libregraph.Group) (*libreg if err := i.conn.Add(ar); err != nil { var lerr *ldap.Error - logger.Debug().Str("backend", "ldap").Str("dn", group.GetDisplayName()).Err(err).Msg("Failed to create group") + logger.Debug().Str("dn", group.GetDisplayName()).Err(err).Msg("Failed to create group") if errors.As(err, &lerr) { if lerr.ResultCode == ldap.LDAPResultEntryAlreadyExists { err = errorcode.New(errorcode.NameAlreadyExists, "group already exists") @@ -213,13 +218,13 @@ func (i *LDAP) CreateGroup(ctx context.Context, group libregraph.Group) (*libreg if err != nil { return nil, err } - return i.createGroupModelFromLDAP(e), nil + return i.createGroupModelFromLDAP(e) } // DeleteGroup implements the Backend Interface. func (i *LDAP) DeleteGroup(ctx context.Context, id string) error { logger := i.logger.SubloggerWithRequestID(ctx) - logger.Debug().Str("backend", "ldap").Msg("DeleteGroup") + logger.Debug().Msg("DeleteGroup") if !i.writeEnabled && i.groupCreateBaseDN == i.groupBaseDN { return errorcode.New(errorcode.NotAllowed, "server is configured read-only") } @@ -243,7 +248,7 @@ func (i *LDAP) DeleteGroup(ctx context.Context, id string) error { // UpdateGroupName implements the Backend Interface. func (i *LDAP) UpdateGroupName(ctx context.Context, groupID string, groupName string) error { logger := i.logger.SubloggerWithRequestID(ctx) - logger.Debug().Str("backend", "ldap").Msg("AddMembersToGroup") + logger.Debug().Msg("UpdateGroupName") if !i.writeEnabled && i.groupCreateBaseDN == i.groupBaseDN { return errorcode.New(errorcode.NotAllowed, "server is configured read-only") } @@ -289,7 +294,7 @@ func (i *LDAP) UpdateGroupName(ctx context.Context, groupID string, groupName st // as members is not yet implemented func (i *LDAP) AddMembersToGroup(ctx context.Context, groupID string, memberIDs []string) error { logger := i.logger.SubloggerWithRequestID(ctx) - logger.Debug().Str("backend", "ldap").Msg("AddMembersToGroup") + logger.Debug().Msg("AddMembersToGroup") if !i.writeEnabled && i.groupCreateBaseDN == i.groupBaseDN { return errorcode.New(errorcode.NotAllowed, "server is configured read-only") } @@ -353,7 +358,7 @@ func (i *LDAP) AddMembersToGroup(ctx context.Context, groupID string, memberIDs } nDN, err := ldapdn.ParseNormalize(me.DN) if err != nil { - logger.Error().Str("new member", me.DN).Err(err).Msg("Couldn't parse DN") + logger.Error().Err(err).Str("memberId", memberID).Str("new-member", me.DN).Msg("Couldn't parse DN") return err } if _, present := currentSet[nDN]; !present { @@ -403,14 +408,14 @@ func (i *LDAP) AddMembersToGroup(ctx context.Context, groupID string, memberIDs // RemoveMemberFromGroup implements the Backend Interface. func (i *LDAP) RemoveMemberFromGroup(ctx context.Context, groupID string, memberID string) error { logger := i.logger.SubloggerWithRequestID(ctx) - logger.Debug().Str("backend", "ldap").Msg("RemoveMemberFromGroup") + logger.Debug().Msg("RemoveMemberFromGroup") if !i.writeEnabled && i.groupCreateBaseDN == i.groupBaseDN { return errorcode.New(errorcode.NotAllowed, "server is configured read-only") } ge, err := i.getLDAPGroupByID(groupID, true) if err != nil { - logger.Debug().Str("backend", "ldap").Str("groupID", groupID).Msg("Error looking up group") + logger.Debug().Str("groupID", groupID).Msg("Error looking up group") return err } @@ -420,14 +425,18 @@ func (i *LDAP) RemoveMemberFromGroup(ctx context.Context, groupID string, member me, err := i.getLDAPUserByID(memberID) if err != nil { - logger.Debug().Str("backend", "ldap").Str("memberID", memberID).Msg("Error looking up group member") + logger.Debug().Str("memberID", memberID).Msg("Error looking up group member") return err } - logger.Debug().Str("backend", "ldap").Str("groupdn", ge.DN).Str("member", me.DN).Msg("remove member") + logger.Debug().Str("groupdn", ge.DN).Str("member", me.DN).Msg("removing member") if err = i.removeEntryByDNAndAttributeFromEntry(ge, me.DN, i.groupAttributeMap.member); err != nil { - logger.Error().Err(err).Str("backend", "ldap").Str("group", groupID).Str("member", memberID).Msg("Failed to remove member from group.") + msg := "Failed to remove member from group." + if errorcode.IsErrorCode(err, errorcode.ItemNotFound) { + msg = "Failed to find member to remove in group." + } + logger.Error().Err(err).Str("group", groupID).Str("member", memberID).Msg(msg) } return err } @@ -501,11 +510,15 @@ func (i *LDAP) getLDAPGroupByFilter(filter string, requestMembers bool) (*ldap.E if err != nil { return nil, err } - if len(e) == 0 { - return nil, errorcode.New(errorcode.ItemNotFound, "not found") - } - return e[0], nil + switch len(e) { + case 0: + return nil, ErrNotFound + case 1: + return e[0], nil + default: + return nil, ErrTooManyResults + } } // Search for LDAP Groups matching the specified filter, if requestMembers is true the groupMemberShip @@ -531,7 +544,7 @@ func (i *LDAP) getLDAPGroupsByFilter(filter string, requestMembers, single bool) attrs, nil, ) - i.logger.Debug().Str("backend", "ldap"). + i.logger.Debug(). Str("base", searchRequest.BaseDN). Str("filter", searchRequest.Filter). Int("scope", searchRequest.Scope). @@ -544,7 +557,7 @@ func (i *LDAP) getLDAPGroupsByFilter(filter string, requestMembers, single bool) if lerr, ok := err.(*ldap.Error); ok { if lerr.ResultCode == ldap.LDAPResultSizeLimitExceeded { errmsg = fmt.Sprintf("too many results searching for group '%s'", filter) - i.logger.Debug().Str("backend", "ldap").Err(lerr).Msg(errmsg) + i.logger.Debug().Err(lerr).Msg(errmsg) } } return nil, errorcode.New(errorcode.ItemNotFound, errmsg) @@ -577,11 +590,15 @@ func (i *LDAP) getGroupsForUser(dn string) ([]*ldap.Entry, error) { return userGroups, nil } -func (i *LDAP) createGroupModelFromLDAP(e *ldap.Entry) *libregraph.Group { +func (i *LDAP) createGroupModelFromLDAP(e *ldap.Entry) (*libregraph.Group, error) { + if e == nil { + return nil, nil + } name := e.GetEqualFoldAttributeValue(i.groupAttributeMap.name) id, err := i.ldapUUIDtoString(e, i.groupAttributeMap.id, i.groupIDisOctetString) if err != nil { i.logger.Warn().Str("dn", e.DN).Str(i.groupAttributeMap.id, e.GetEqualFoldAttributeValue(i.groupAttributeMap.id)).Msg("Invalid User. Cannot convert UUID") + return nil, err } groupTypes := []string{} @@ -594,10 +611,10 @@ func (i *LDAP) createGroupModelFromLDAP(e *ldap.Entry) *libregraph.Group { DisplayName: &name, Id: &id, GroupTypes: groupTypes, - } + }, nil } i.logger.Warn().Str("dn", e.DN).Msg("Group is missing name or id") - return nil + return nil, errors.New("LDAP group is missing name or id") } func (i *LDAP) isLDAPGroupReadOnly(e *ldap.Entry) bool { @@ -616,12 +633,21 @@ func (i *LDAP) isLDAPGroupReadOnly(e *ldap.Entry) bool { return !baseDN.AncestorOfFold(groupDN) } -func (i *LDAP) groupsFromLDAPEntries(e []*ldap.Entry) []libregraph.Group { +func (i *LDAP) groupsFromLDAPEntries(e []*ldap.Entry) ([]libregraph.Group, error) { groups := make([]libregraph.Group, 0, len(e)) + errs := []error{} for _, g := range e { - if grp := i.createGroupModelFromLDAP(g); grp != nil { + if grp, err := i.createGroupModelFromLDAP(g); err != nil { + // don't bail out here, continue processing the other elements in range to give + // the caller the opportunity to decide whether to ignore these or not + errs = append(errs, err) + } else if grp != nil { groups = append(groups, *grp) } } - return groups + if len(errs) > 0 { + return nil, errors.Join(errs...) + } else { + return groups, nil + } } diff --git a/services/graph/pkg/identity/ldap_test.go b/services/graph/pkg/identity/ldap_test.go index 0f3caf03e4..32541219ee 100644 --- a/services/graph/pkg/identity/ldap_test.go +++ b/services/graph/pkg/identity/ldap_test.go @@ -14,12 +14,13 @@ import ( "github.com/opencloud-eu/opencloud/pkg/log" "github.com/opencloud-eu/opencloud/services/graph/pkg/config" "github.com/opencloud-eu/opencloud/services/graph/pkg/identity/mocks" + "github.com/prometheus/client_golang/prometheus" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" ) func getMockedBackend(l ldap.Client, lc config.LDAP, logger *log.Logger) (*LDAP, error) { - return NewLDAPBackend(l, lc, logger) + return NewLDAPBackend(l, lc, logger, "opencloud", "test", prometheus.NewRegistry()) } const ( @@ -105,31 +106,35 @@ var ldapUserAttributes = []string{"displayname", "entryUUID", "mail", "uid", "sn func TestNewLDAPBackend(t *testing.T) { l := &mocks.Client{} + newBackend := func(c config.LDAP) (*LDAP, error) { + return NewLDAPBackend(l, c, &logger, "opencloud", "test", prometheus.NewRegistry()) + } + tc := lconfig tc.UserDisplayNameAttribute = "" - if _, err := NewLDAPBackend(l, tc, &logger); err == nil { + if _, err := newBackend(tc); err == nil { t.Error("Should fail with incomplete user attr config") } tc = lconfig tc.GroupIDAttribute = "" - if _, err := NewLDAPBackend(l, tc, &logger); err == nil { + if _, err := newBackend(tc); err == nil { t.Errorf("Should fail with incomplete group config") } tc = lconfig tc.UserSearchScope = "" - if _, err := NewLDAPBackend(l, tc, &logger); err == nil { + if _, err := newBackend(tc); err == nil { t.Errorf("Should fail with invalid user search scope") } tc = lconfig tc.GroupSearchScope = "" - if _, err := NewLDAPBackend(l, tc, &logger); err == nil { + if _, err := newBackend(tc); err == nil { t.Errorf("Should fail with invalid group search scope") } - if _, err := NewLDAPBackend(l, lconfig, &logger); err != nil { + if _, err := newBackend(lconfig); err != nil { t.Errorf("Should fail with invalid group search scope") } } @@ -172,7 +177,7 @@ func TestCreateUser(t *testing.T) { c := lconfig c.UseServerUUID = true - b, _ := NewLDAPBackend(l, c, &logger) + b, _ := NewLDAPBackend(l, c, &logger, "opencloud", "test", prometheus.NewRegistry()) newUser, err := b.CreateUser(context.Background(), *user) assert.Nil(t, err) @@ -189,12 +194,16 @@ func TestCreateUserModelFromLDAP(t *testing.T) { l := &mocks.Client{} logger := log.NewLogger(log.Level("debug")) - b, _ := NewLDAPBackend(l, lconfig, &logger) - if user := b.createUserModelFromLDAP(nil); user != nil { - t.Errorf("createUserModelFromLDAP should return on nil Entry") + b, _ := NewLDAPBackend(l, lconfig, &logger, "opencloud", "test", prometheus.NewRegistry()) + { + res, err := b.createUserModelFromLDAP(nil) + assert.NoError(t, err) + assert.Nil(t, res) } - user := b.createUserModelFromLDAP(userEntry) - if user == nil { + user, err := b.createUserModelFromLDAP(userEntry) + if err != nil { + t.Error("Converting a valid LDAP Entry should succeed and not return an error") + } else if user == nil { t.Error("Converting a valid LDAP Entry should succeed") } else { if user.OnPremisesSamAccountName != userEntry.GetEqualFoldAttributeValue(b.userAttributeMap.userName) { @@ -233,10 +242,10 @@ func TestGetUser(t *testing.T) { } _, err = b.GetUser(context.Background(), "fred", odataReqDefault) - assert.ErrorContains(t, err, "itemNotFound:") + assert.ErrorContains(t, err, "user search failed") _, err = b.GetUser(context.Background(), "fred", odataReqExpand) - assert.ErrorContains(t, err, "itemNotFound:") + assert.ErrorContains(t, err, "user search failed") // Mock an empty Search Result lm = &mocks.Client{} @@ -285,7 +294,7 @@ func TestGetUser(t *testing.T) { b, _ = getMockedBackend(lm, lconfig, &logger) _, err = b.GetUser(context.Background(), "invalid", nil) - assert.ErrorContains(t, err, "itemNotFound:") + assert.ErrorContains(t, err, "Invalid User") } func TestGetUserAD(t *testing.T) { diff --git a/services/graph/pkg/metrics/metrics.go b/services/graph/pkg/metrics/metrics.go index 437822e662..e0ede857d2 100644 --- a/services/graph/pkg/metrics/metrics.go +++ b/services/graph/pkg/metrics/metrics.go @@ -1,6 +1,13 @@ package metrics -import "github.com/prometheus/client_golang/prometheus" +import ( + "strconv" + "strings" + "sync/atomic" + "time" + + "github.com/prometheus/client_golang/prometheus" +) var ( // Namespace defines the namespace for the defines metrics. @@ -12,21 +19,50 @@ var ( // Metrics defines the available metrics of this service. type Metrics struct { - BuildInfo *prometheus.GaugeVec - EventsEnabled prometheus.Gauge - HttpEnabled prometheus.Gauge - EventsProcessed *prometheus.CounterVec - InvalidEvents prometheus.Counter - UnsupportedEvents prometheus.Counter + BuildInfo *prometheus.GaugeVec + EventsEnabled prometheus.Gauge + HttpEnabled prometheus.Gauge + EventsProcessed *prometheus.CounterVec + InvalidEvents prometheus.Counter + UnsupportedEvents prometheus.Counter + UserPasswordChanges *prometheus.CounterVec + httpRequestDuration *prometheus.HistogramVec + httpPathSplitter func(pieces []string) (string, string) } const ( - ResultSuccess = "success" - ResultFailure = "failure" + ResultSuccess = "success" + ResultFailure = "failure" + ResultNotFound = "not-found" + ResultReadOnly = "read-only" + ResultClientError = "client-error" + ResultServerError = "server-error" +) + +const ( + LabelMethod = "method" + LabelPath = "path" + LabelVersion = "version" + LabelResource = "resource" + LabelCode = "code" + LabelResult = "result" + LabelReason = "reason" + LabelEvent = "event" + LabelOperation = "operation" +) + +const ( + ReasonInvalid = "invalid" + ReasonError = "error" + ReasonWrongPassword = "wrong-password" +) + +const ( + UnmatchedRoutePattern = "unknown" ) // New initializes the available metrics. -func New(registerer prometheus.Registerer) *Metrics { +func New(registerer prometheus.Registerer, httpPathSplitter func(pieces []string) (string, string)) *Metrics { m := &Metrics{ BuildInfo: prometheus.NewGaugeVec(prometheus.GaugeOpts{ Namespace: Namespace, @@ -51,7 +87,7 @@ func New(registerer prometheus.Registerer) *Metrics { Subsystem: Subsystem, Name: "events", Help: "Number of consumed events", - }, []string{"event", "result"}), + }, []string{LabelEvent, LabelResult}), InvalidEvents: prometheus.NewCounter(prometheus.CounterOpts{ Namespace: Namespace, Subsystem: Subsystem, @@ -64,14 +100,65 @@ func New(registerer prometheus.Registerer) *Metrics { Name: "events_unsupported", Help: "Number of unsupported events that were consumed and ignored", }), + UserPasswordChanges: prometheus.NewCounterVec(prometheus.CounterOpts{ + Namespace: Namespace, + Subsystem: Subsystem, + Name: "user_password_changes", + Help: "Counts occurences of users changing their password", + }, []string{LabelResult, LabelReason}), + // keeping this one private as it should only be used via the RecordHTTPDuration() method below, + // as its number of labels is too fragile to keep in check if they ever change + httpRequestDuration: prometheus.NewHistogramVec(prometheus.HistogramOpts{ + Namespace: Namespace, + Subsystem: Subsystem, + Name: "http_request_duration_seconds", + Help: "Duration of HTTP operations in seconds.", + Buckets: prometheus.DefBuckets, + }, []string{LabelMethod, LabelPath, LabelVersion, LabelResource, LabelCode, LabelResult}), // when changing these, make sure to also modify the methods below accordingly + httpPathSplitter: httpPathSplitter, } _ = prometheus.Register(m.BuildInfo) _ = prometheus.Register(m.EventsEnabled) _ = prometheus.Register(m.HttpEnabled) _ = prometheus.Register(m.EventsProcessed) - _ = prometheus.Register(m.UnsupportedEvents) _ = prometheus.Register(m.InvalidEvents) - // TODO: implement metrics + _ = prometheus.Register(m.UnsupportedEvents) + _ = prometheus.Register(m.UserPasswordChanges) + _ = prometheus.Register(m.httpRequestDuration) + + // TODO: implement more metrics + return m } + +func (m Metrics) InitHttpInFlightGauge(inFlight *atomic.Int64) { + _ = prometheus.Register(prometheus.NewGaugeFunc(prometheus.GaugeOpts{ + Namespace: Namespace, + Subsystem: Subsystem, + Unit: "requests", + Name: "http_requests", + Help: "Concurrent inbound HTTP requests.", + }, func() float64 { + return float64(inFlight.Load()) + })) +} + +func (m Metrics) RecordHTTPDuration(method string, pattern string, statusCode int, duration time.Duration) { + result := "" + if statusCode < 400 { + result = ResultSuccess + } else if statusCode < 500 { + result = ResultClientError + } else { + result = ResultServerError + } + // all the HTTP routes for the Graph API start with a version (v1.0 or v1beta1), followed by a top level + // resource "module", which might be useful to extract and include as a label, to aggregate metrics and + // statistics before drilling down further + pieces := strings.FieldsFunc(pattern, func(r rune) bool { + return r == '/' + }) + version, resource := m.httpPathSplitter(pieces) + m.httpRequestDuration.WithLabelValues(method, pattern, version, resource, strconv.Itoa(statusCode), result).Observe(duration.Seconds()) +} diff --git a/services/graph/pkg/metrics/middleware.go b/services/graph/pkg/metrics/middleware.go new file mode 100644 index 0000000000..77624d6e7d --- /dev/null +++ b/services/graph/pkg/metrics/middleware.go @@ -0,0 +1,59 @@ +package metrics + +import ( + "net/http" + "sync/atomic" + "time" + + "github.com/go-chi/chi/v5" +) + +type statusResponseWriter struct { + http.ResponseWriter + statusCode int +} + +func (rw *statusResponseWriter) WriteHeader(code int) { + rw.statusCode = code + rw.ResponseWriter.WriteHeader(code) +} + +// A middleware that tracks the duration of every inbound Graph API HTTP call +// and calls a function to delegate the storage of that duration into a +// histogram metric, analyzing the incoming query and deconstructing it into +// method, path pattern, as well as the resulting status code. +// +// It also tracks the number of concurrent HTTP requests that are in flight, +// using a Gauge that it increments and decrements when it wraps the next +// handler. +// +// Note that to avoid a high cardinality on the path label, the URL is matched +// against the chi routing rules, passing the path pattern to the function +// instead of the actual URI. +func HTTPMetrics(inFlight *atomic.Int64, observe func(method, pattern string, statusCode int, duration time.Duration)) func(next http.Handler) http.Handler { + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + start := time.Now() + responseWrapper := &statusResponseWriter{ResponseWriter: w, statusCode: 200} // 200 OK is the default when it's not set + inFlight.Add(1) + defer inFlight.Add(-1) + + next.ServeHTTP(responseWrapper, r) + + duration := time.Since(start) + + method := r.Method + routePattern := UnmatchedRoutePattern + if rctx := chi.RouteContext(r.Context()); rctx != nil { + if pattern := rctx.RoutePattern(); pattern != "" { + routePattern = pattern + if method == "" { + method = rctx.RouteMethod + } + } + } + + observe(r.Method, routePattern, responseWrapper.statusCode, duration) + }) + } +} diff --git a/services/graph/pkg/middleware/requireadmin.go b/services/graph/pkg/middleware/requireadmin.go index 81fc3374c4..4a3f2777bb 100644 --- a/services/graph/pkg/middleware/requireadmin.go +++ b/services/graph/pkg/middleware/requireadmin.go @@ -12,15 +12,16 @@ import ( // RequireAdmin middleware is used to require the user in context to be an admin / have account management permissions func RequireAdmin(rm *roles.Manager, logger log.Logger) func(next http.Handler) http.Handler { + l := log.Logger{Logger: logger.With().Str("middleware", "requireAdmin").Logger()} return func(next http.Handler) http.Handler { - l := logger.With().Str("middleware", "requireAdmin").Logger() return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { u, ok := revactx.ContextGetUser(r.Context()) if !ok { errorcode.AccessDenied.Render(w, r, http.StatusUnauthorized, "Unauthorized") return } - if u.Id == nil || u.Id.OpaqueId == "" { + if u.GetId().GetOpaqueId() == "" { + l.Debug().Msg("Bad request: user does not have an id") errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, "user is missing an id") return } @@ -48,6 +49,7 @@ func RequireAdmin(rm *roles.Manager, logger log.Logger) func(next http.Handler) return } + l.Debug().Str("userid", u.Id.OpaqueId).Str("permission", settings.AccountManagementPermissionID).Msg("Access denied: necessary permission not present in user's roles") errorcode.AccessDenied.Render(w, r, http.StatusForbidden, "Forbidden") }) } diff --git a/services/graph/pkg/server/http/server.go b/services/graph/pkg/server/http/server.go index 583d01e335..f5e656c66d 100644 --- a/services/graph/pkg/server/http/server.go +++ b/services/graph/pkg/server/http/server.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" stdhttp "net/http" + "sync/atomic" gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1" chimiddleware "github.com/go-chi/chi/v5/middleware" @@ -26,6 +27,7 @@ import ( searchsvc "github.com/opencloud-eu/opencloud/protogen/gen/opencloud/services/search/v0" settingssvc "github.com/opencloud-eu/opencloud/protogen/gen/opencloud/services/settings/v0" "github.com/opencloud-eu/opencloud/services/graph/pkg/identity" + "github.com/opencloud-eu/opencloud/services/graph/pkg/metrics" graphMiddleware "github.com/opencloud-eu/opencloud/services/graph/pkg/middleware" svc "github.com/opencloud-eu/opencloud/services/graph/pkg/service/v0" ) @@ -62,6 +64,15 @@ func Server(identityBackend identity.Backend, eduBackend identity.EducationBacke middleware.Logger( options.Logger, ), + } + + if !options.Config.HTTP.Metrics.Disabled { + var inFlight atomic.Int64 + middlewares = append(middlewares, metrics.HTTPMetrics(&inFlight, options.Metrics.RecordHTTPDuration)) + options.Metrics.InitHttpInFlightGauge(&inFlight) + } + + middlewares = append(middlewares, middleware.Cors( cors.Logger(options.Logger), cors.AllowedOrigins(options.Config.HTTP.CORS.AllowedOrigins), @@ -69,7 +80,8 @@ func Server(identityBackend identity.Backend, eduBackend identity.EducationBacke cors.AllowedHeaders(options.Config.HTTP.CORS.AllowedHeaders), cors.AllowCredentials(options.Config.HTTP.CORS.AllowCredentials), ), - } + ) + // how do we secure the api? var requireAdminMiddleware func(stdhttp.Handler) stdhttp.Handler var roleService svc.RoleService @@ -152,6 +164,7 @@ func Server(identityBackend identity.Backend, eduBackend identity.EducationBacke svc.UserProfilePhotoService(userProfilePhotoService), svc.Logger(options.Logger), svc.Config(options.Config), + svc.Metrics(options.Metrics), svc.Middleware(middlewares...), svc.EventsPublisher(eventsStream), // is required even when event consumption is disabled svc.WithRoleService(roleService), diff --git a/services/graph/pkg/service/events/service.go b/services/graph/pkg/service/events/service.go index 21213e68ac..451e504eae 100644 --- a/services/graph/pkg/service/events/service.go +++ b/services/graph/pkg/service/events/service.go @@ -9,63 +9,11 @@ import ( "github.com/opencloud-eu/reva/v2/pkg/utils" "github.com/opencloud-eu/opencloud/pkg/log" + "github.com/opencloud-eu/opencloud/services/graph/pkg/errorcode" "github.com/opencloud-eu/opencloud/services/graph/pkg/identity" "github.com/opencloud-eu/opencloud/services/graph/pkg/metrics" ) -func processEvents(ctx context.Context, consumer events.Consumer, stop *atomic.Bool, stopCh chan struct{}, - backend identity.Backend, m *metrics.Metrics, logger *log.Logger) error { - var _registeredEvents = []events.Unmarshaller{ - events.UserSignedIn{}, - } - evChannel, err := events.Consume(consumer, "graph", _registeredEvents...) - if err != nil { - logger.Error().Err(err).Msg("cannot consume from nats") - return err - } - logger.Debug().Msg("listening for events") - for loop := true; loop; { - select { - case e := <-evChannel: - switch ev := e.Event.(type) { - default: - // this branch is currently impossible to test and run into because we pick which events we're interested in - // through the _registeredEvents above, and the stream won't hand us events we didn't register for - m.UnsupportedEvents.Inc() - logger.Error().Interface("event", e).Msg("unhandled event") - case events.UserSignedIn: - name := "UserSignedIn" - userId := "" - if ev.Executant != nil && ev.Executant.OpaqueId != "" { - userId = ev.Executant.OpaqueId - } else { - m.InvalidEvents.Inc() - logger.Error().Err(err).Interface("event", ev).Msg("Received invalid event: executant.opaqueId not set") - continue - } - if err := backend.UpdateLastSignInDate(ctx, userId, utils.TSToTime(ev.Timestamp)); err != nil { - m.EventsProcessed.WithLabelValues(name, metrics.ResultFailure).Inc() - logger.Error().Err(err).Str("userid", userId).Str("event", name).Msg("Error updating last sign in date") - } else { - // TODO: UpdateLastSignInDate() currently returns nil instead of an error when the LDAP server is read-only, so those will be accounted for as a success - m.EventsProcessed.WithLabelValues(name, metrics.ResultSuccess).Inc() - logger.Debug().Str("userid", userId).Str("event", name).Msg("Successfully updated last sign in date") - } - } - if stop.Load() { - loop = false - } - case <-stopCh: - logger.Info().Msg("instructed to stop") - loop = false - case <-ctx.Done(): - logger.Info().Msg("context cancelled") - loop = false - } - } - return nil -} - type GraphEventConsumer interface { Start() error io.Closer @@ -122,3 +70,59 @@ func NewService(ctx context.Context, consumer events.Consumer, backend identity. }, nil } } + +func processEvents(ctx context.Context, consumer events.Consumer, stop *atomic.Bool, stopCh chan struct{}, + backend identity.Backend, m *metrics.Metrics, logger *log.Logger) error { + var _registeredEvents = []events.Unmarshaller{ + events.UserSignedIn{}, + } + evChannel, err := events.Consume(consumer, "graph", _registeredEvents...) + if err != nil { + logger.Error().Err(err).Msg("cannot consume from nats") + return err + } + logger.Debug().Msg("listening for events") + for loop := true; loop; { + select { + case e := <-evChannel: + switch ev := e.Event.(type) { + default: + // this branch is currently impossible to test and run into because we pick which events we're interested in + // through the _registeredEvents above, and the stream won't hand us events we didn't register for + m.UnsupportedEvents.Inc() + logger.Error().Interface("event", e).Msg("unhandled event") + case events.UserSignedIn: + name := "UserSignedIn" + userId := "" + if ev.Executant != nil && ev.Executant.OpaqueId != "" { + userId = ev.Executant.OpaqueId + } else { + m.InvalidEvents.Inc() + logger.Error().Err(err).Interface("event", ev).Msg("Received invalid event: executant.opaqueId not set") + continue + } + if err := backend.UpdateLastSignInDate(ctx, userId, utils.TSToTime(ev.Timestamp)); err != nil { + result := metrics.ResultFailure + if errorcode.IsErrorCode(err, errorcode.ItemNotFound) { + result = metrics.ResultNotFound + } + m.EventsProcessed.WithLabelValues(name, result).Inc() + logger.Error().Err(err).Str("userid", userId).Str("event", name).Msg("Error updating last sign in date") + } else { + m.EventsProcessed.WithLabelValues(name, metrics.ResultSuccess).Inc() + logger.Debug().Str("userid", userId).Str("event", name).Msg("Successfully updated last sign in date") + } + } + if stop.Load() { + loop = false + } + case <-stopCh: + logger.Info().Msg("instructed to stop") + loop = false + case <-ctx.Done(): + logger.Info().Msg("context cancelled") + loop = false + } + } + return nil +} diff --git a/services/graph/pkg/service/events/service_test.go b/services/graph/pkg/service/events/service_test.go index e5e118ed91..d6a75a58f2 100644 --- a/services/graph/pkg/service/events/service_test.go +++ b/services/graph/pkg/service/events/service_test.go @@ -42,7 +42,7 @@ func TestSuccessfulCall(t *testing.T) { }) reg := prometheus.NewRegistry() - m := metrics.New(reg) + m := metrics.New(reg, func(_ []string) (string, string) { return "", "" }) logger := log.NewLogger() @@ -92,7 +92,7 @@ func TestBackendReturningAnError(t *testing.T) { }) reg := prometheus.NewRegistry() - m := metrics.New(reg) + m := metrics.New(reg, func(_ []string) (string, string) { return "", "" }) logger := log.NewLogger() diff --git a/services/graph/pkg/service/v0/application_test.go b/services/graph/pkg/service/v0/application_test.go index b61714967a..60355cfbfd 100644 --- a/services/graph/pkg/service/v0/application_test.go +++ b/services/graph/pkg/service/v0/application_test.go @@ -14,6 +14,7 @@ import ( libregraph "github.com/opencloud-eu/libre-graph-api-go" "github.com/opencloud-eu/reva/v2/pkg/rgrpc/todo/pool" cs3mocks "github.com/opencloud-eu/reva/v2/tests/cs3mocks/mocks" + "github.com/prometheus/client_golang/prometheus" "github.com/stretchr/testify/mock" "google.golang.org/grpc" @@ -24,6 +25,7 @@ import ( "github.com/opencloud-eu/opencloud/services/graph/pkg/config" "github.com/opencloud-eu/opencloud/services/graph/pkg/config/defaults" identitymocks "github.com/opencloud-eu/opencloud/services/graph/pkg/identity/mocks" + "github.com/opencloud-eu/opencloud/services/graph/pkg/metrics" service "github.com/opencloud-eu/opencloud/services/graph/pkg/service/v0" ) @@ -50,6 +52,7 @@ var _ = Describe("Applications", func() { identityBackend = &identitymocks.Backend{} roleService = &mocks.RoleService{} + metrics := metrics.New(prometheus.NewRegistry(), func([]string) (string, string) { return "", "" }) pool.RemoveSelector("GatewaySelector" + "eu.opencloud.api.gateway") gatewayClient = &cs3mocks.GatewayAPIClient{} @@ -74,6 +77,7 @@ var _ = Describe("Applications", func() { var err error svc, err = service.NewService( service.Config(cfg), + service.Metrics(metrics), service.WithGatewaySelector(gatewaySelector), service.EventsPublisher(&eventsPublisher), service.WithIdentityBackend(identityBackend), diff --git a/services/graph/pkg/service/v0/approleassignments_test.go b/services/graph/pkg/service/v0/approleassignments_test.go index 9cdf1e9925..8590acbcdd 100644 --- a/services/graph/pkg/service/v0/approleassignments_test.go +++ b/services/graph/pkg/service/v0/approleassignments_test.go @@ -18,6 +18,7 @@ import ( revactx "github.com/opencloud-eu/reva/v2/pkg/ctx" "github.com/opencloud-eu/reva/v2/pkg/rgrpc/todo/pool" cs3mocks "github.com/opencloud-eu/reva/v2/tests/cs3mocks/mocks" + "github.com/prometheus/client_golang/prometheus" "github.com/stretchr/testify/mock" "google.golang.org/grpc" @@ -28,6 +29,7 @@ import ( "github.com/opencloud-eu/opencloud/services/graph/pkg/config" "github.com/opencloud-eu/opencloud/services/graph/pkg/config/defaults" identitymocks "github.com/opencloud-eu/opencloud/services/graph/pkg/identity/mocks" + "github.com/opencloud-eu/opencloud/services/graph/pkg/metrics" service "github.com/opencloud-eu/opencloud/services/graph/pkg/service/v0" ) @@ -60,6 +62,7 @@ var _ = Describe("AppRoleAssignments", func() { identityBackend = &identitymocks.Backend{} roleService = &mocks.RoleService{} + metrics := metrics.New(prometheus.NewRegistry(), func([]string) (string, string) { return "", "" }) pool.RemoveSelector("GatewaySelector" + "eu.opencloud.api.gateway") gatewayClient = &cs3mocks.GatewayAPIClient{} @@ -84,6 +87,7 @@ var _ = Describe("AppRoleAssignments", func() { var err error svc, err = service.NewService( service.Config(cfg), + service.Metrics(metrics), service.WithGatewaySelector(gatewaySelector), service.EventsPublisher(&eventsPublisher), service.WithIdentityBackend(identityBackend), diff --git a/services/graph/pkg/service/v0/base.go b/services/graph/pkg/service/v0/base.go index 85076870c2..cc25d30da1 100644 --- a/services/graph/pkg/service/v0/base.go +++ b/services/graph/pkg/service/v0/base.go @@ -416,13 +416,13 @@ func (g BaseGraphService) cs3UserSharesToDriveItems(ctx context.Context, shares } for _, share := range sharesByResource.Shares { perm, err := g.cs3UserShareToPermission(ctx, share, condition) - var errcode errorcode.Error - switch { - case errors.As(err, &errcode) && errcode.GetCode() == errorcode.ItemNotFound: - // The Grantee couldn't be found (user/group does not exist anymore) - continue - case err != nil: - return err + if err != nil { + if errorcode.IsErrorCode(err, errorcode.ItemNotFound) { + // The Grantee couldn't be found (user/group does not exist anymore) + continue + } else { + return err + } } item.Permissions = append(item.Permissions, *perm) } @@ -516,13 +516,13 @@ func (g BaseGraphService) cs3OCMSharesToDriveItems(ctx context.Context, shares [ for _, share := range sharesByResource.Shares { perm, err := g.cs3OCMShareToPermission(ctx, share, condition) - var errcode errorcode.Error - switch { - case errors.As(err, &errcode) && errcode.GetCode() == errorcode.ItemNotFound: - // The Grantee couldn't be found (user/group does not exist anymore) - continue - case err != nil: - return err + if err != nil { + if errorcode.IsErrorCode(err, errorcode.ItemNotFound) { + // The Grantee couldn't be found (user/group does not exist anymore) + continue + } else { + return err + } } item.Permissions = append(item.Permissions, *perm) } @@ -1014,7 +1014,6 @@ func (g BaseGraphService) getOCMPermissionByID(ctx context.Context, permissionID } func (g BaseGraphService) getPermissionByID(ctx context.Context, permissionID string, itemID *storageprovider.ResourceId) (*libregraph.Permission, *storageprovider.ResourceId, error) { - var errcode errorcode.Error gatewayClient, err := g.gatewaySelector.Next() if err != nil { g.logger.Debug().Err(err).Msg("selecting gatewaySelector failed") @@ -1045,7 +1044,7 @@ func (g BaseGraphService) getPermissionByID(ctx context.Context, permissionID st } } } - case errors.As(err, &errcode) && errcode.GetCode() == errorcode.ItemNotFound: + case errorcode.IsErrorCode(err, errorcode.ItemNotFound): // there is no public link with that id, check if this is a user share cs3Share, err := g.getCS3UserShareByID(ctx, permissionID) if err != nil { diff --git a/services/graph/pkg/service/v0/driveitems_test.go b/services/graph/pkg/service/v0/driveitems_test.go index f36f6c06fd..4d2b90f56f 100644 --- a/services/graph/pkg/service/v0/driveitems_test.go +++ b/services/graph/pkg/service/v0/driveitems_test.go @@ -16,6 +16,7 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" libregraph "github.com/opencloud-eu/libre-graph-api-go" + "github.com/prometheus/client_golang/prometheus" "github.com/stretchr/testify/mock" "google.golang.org/grpc" @@ -30,6 +31,7 @@ import ( "github.com/opencloud-eu/opencloud/services/graph/pkg/config" "github.com/opencloud-eu/opencloud/services/graph/pkg/config/defaults" identitymocks "github.com/opencloud-eu/opencloud/services/graph/pkg/identity/mocks" + "github.com/opencloud-eu/opencloud/services/graph/pkg/metrics" service "github.com/opencloud-eu/opencloud/services/graph/pkg/service/v0" ) @@ -72,6 +74,7 @@ var _ = Describe("Driveitems", func() { ) identityBackend = &identitymocks.Backend{} + metrics := metrics.New(prometheus.NewRegistry(), func([]string) (string, string) { return "", "" }) newGroup = libregraph.NewGroup() newGroup.SetMembersodataBind([]string{"/users/user1"}) newGroup.SetId("group1") @@ -88,6 +91,7 @@ var _ = Describe("Driveitems", func() { var err error svc, err = service.NewService( service.Config(cfg), + service.Metrics(metrics), service.WithGatewaySelector(gatewaySelector), service.EventsPublisher(&eventsPublisher), service.WithIdentityBackend(identityBackend), diff --git a/services/graph/pkg/service/v0/educationclasses.go b/services/graph/pkg/service/v0/educationclasses.go index cdb315a43b..d9b9ce27fc 100644 --- a/services/graph/pkg/service/v0/educationclasses.go +++ b/services/graph/pkg/service/v0/educationclasses.go @@ -8,10 +8,10 @@ import ( "strings" "github.com/CiscoM31/godata" + libregraph "github.com/opencloud-eu/libre-graph-api-go" "github.com/opencloud-eu/opencloud/services/graph/pkg/errorcode" revactx "github.com/opencloud-eu/reva/v2/pkg/ctx" "github.com/opencloud-eu/reva/v2/pkg/events" - libregraph "github.com/opencloud-eu/libre-graph-api-go" "github.com/go-chi/chi/v5" "github.com/go-chi/render" @@ -393,7 +393,6 @@ func (g Graph) DeleteEducationClassMember(w http.ResponseWriter, r *http.Request } logger.Debug().Str("classID", classID).Str("memberID", memberID).Msg("calling delete member on backend") err = g.identityBackend.RemoveMemberFromGroup(r.Context(), classID, memberID) - if err != nil { logger.Debug().Err(err).Msg("could not delete class member: backend error") errorcode.RenderError(w, r, err) diff --git a/services/graph/pkg/service/v0/educationclasses_test.go b/services/graph/pkg/service/v0/educationclasses_test.go index 5e17e62435..5651457ce9 100644 --- a/services/graph/pkg/service/v0/educationclasses_test.go +++ b/services/graph/pkg/service/v0/educationclasses_test.go @@ -18,6 +18,7 @@ import ( revactx "github.com/opencloud-eu/reva/v2/pkg/ctx" "github.com/opencloud-eu/reva/v2/pkg/rgrpc/todo/pool" cs3mocks "github.com/opencloud-eu/reva/v2/tests/cs3mocks/mocks" + "github.com/prometheus/client_golang/prometheus" "github.com/stretchr/testify/mock" "google.golang.org/grpc" @@ -27,6 +28,7 @@ import ( "github.com/opencloud-eu/opencloud/services/graph/pkg/config/defaults" "github.com/opencloud-eu/opencloud/services/graph/pkg/errorcode" identitymocks "github.com/opencloud-eu/opencloud/services/graph/pkg/identity/mocks" + "github.com/opencloud-eu/opencloud/services/graph/pkg/metrics" service "github.com/opencloud-eu/opencloud/services/graph/pkg/service/v0" ) @@ -66,6 +68,7 @@ var _ = Describe("EducationClass", func() { identityEducationBackend = &identitymocks.EducationBackend{} identityBackend = &identitymocks.Backend{} + metrics := metrics.New(prometheus.NewRegistry(), func([]string) (string, string) { return "", "" }) newClass = libregraph.NewEducationClass("math", "course") newClass.SetMembersodataBind([]string{"/users/user1"}) newClass.SetId("math") @@ -82,6 +85,7 @@ var _ = Describe("EducationClass", func() { var err error svc, err = service.NewService( service.Config(cfg), + service.Metrics(metrics), service.WithGatewaySelector(gatewaySelector), service.EventsPublisher(&eventsPublisher), service.WithIdentityBackend(identityBackend), @@ -328,10 +332,13 @@ var _ = Describe("EducationClass", func() { updatedClassJson, err := json.Marshal(updatedClass) Expect(err).ToNot(HaveOccurred()) + metrics := metrics.New(prometheus.NewRegistry(), func([]string) (string, string) { return "", "" }) + cfg.API.GroupMembersPatchLimit = 21 svc, err = service.NewService( service.Config(cfg), + service.Metrics(metrics), service.WithGatewaySelector(gatewaySelector), service.EventsPublisher(&eventsPublisher), service.WithIdentityBackend(identityBackend), diff --git a/services/graph/pkg/service/v0/educationschools.go b/services/graph/pkg/service/v0/educationschools.go index d363a43494..8172716188 100644 --- a/services/graph/pkg/service/v0/educationschools.go +++ b/services/graph/pkg/service/v0/educationschools.go @@ -216,6 +216,11 @@ func (g Graph) DeleteEducationSchool(w http.ResponseWriter, r *http.Request) { errorcode.RenderError(w, r, err) return } + if school == nil { + logger.Debug().Str("school-id", schoolID).Msg("failed to find school") + errorcode.ItemNotFound.Render(w, r, http.StatusNotFound, "failed to find school") + return + } termination, ok := school.GetTerminationDateOk() if !ok { logger.Debug().Msg("cannot delete school: not termination date set") @@ -246,15 +251,14 @@ func (g Graph) DeleteEducationSchool(w http.ResponseWriter, r *http.Request) { } logger.Debug().Err(err).Msg("could not delete school member: backend error") errorcode.RenderError(w, r, err) - // TODO Do we need return right hear? + return } } logger.Debug().Str("id", schoolID).Msg("calling delete school on backend") err = g.identityEducationBackend.DeleteEducationSchool(r.Context(), schoolID) - if err != nil { - logger.Debug().Err(err).Msg("could not delete school: backend error") + logger.Debug().Err(err).Str("school-id", schoolID).Msg("could not delete school: backend error") errorcode.RenderError(w, r, err) return } @@ -354,9 +358,8 @@ func (g Graph) PostEducationSchoolUser(w http.ResponseWriter, r *http.Request) { logger.Debug().Str("memberType", memberType).Str("id", id).Msg("calling add user on backend") err = g.identityEducationBackend.AddUsersToEducationSchool(r.Context(), schoolID, []string{id}) - if err != nil { - logger.Debug().Err(err).Msg("could not add school user: backend error") + logger.Debug().Err(err).Str("school-id", schoolID).Msg("could not add school user: backend error") errorcode.RenderError(w, r, err) return } @@ -407,9 +410,8 @@ func (g Graph) DeleteEducationSchoolUser(w http.ResponseWriter, r *http.Request) } logger.Debug().Str("schoolID", schoolID).Str("userID", userID).Msg("calling delete member on backend") err = g.identityEducationBackend.RemoveUserFromEducationSchool(r.Context(), schoolID, userID) - if err != nil { - logger.Debug().Err(err).Msg("could not delete school member: backend error") + logger.Debug().Err(err).Str("school-id", schoolID).Msg("could not delete school member: backend error") errorcode.RenderError(w, r, err) return } diff --git a/services/graph/pkg/service/v0/educationschools_test.go b/services/graph/pkg/service/v0/educationschools_test.go index c2886bc732..18f47f327a 100644 --- a/services/graph/pkg/service/v0/educationschools_test.go +++ b/services/graph/pkg/service/v0/educationschools_test.go @@ -19,6 +19,7 @@ import ( ctxpkg "github.com/opencloud-eu/reva/v2/pkg/ctx" "github.com/opencloud-eu/reva/v2/pkg/rgrpc/todo/pool" cs3mocks "github.com/opencloud-eu/reva/v2/tests/cs3mocks/mocks" + "github.com/prometheus/client_golang/prometheus" "github.com/stretchr/testify/mock" "google.golang.org/grpc" @@ -29,6 +30,7 @@ import ( "github.com/opencloud-eu/opencloud/services/graph/pkg/config/defaults" "github.com/opencloud-eu/opencloud/services/graph/pkg/errorcode" identitymocks "github.com/opencloud-eu/opencloud/services/graph/pkg/identity/mocks" + "github.com/opencloud-eu/opencloud/services/graph/pkg/metrics" service "github.com/opencloud-eu/opencloud/services/graph/pkg/service/v0" ) @@ -67,6 +69,7 @@ var _ = Describe("Schools", func() { ) identityEducationBackend = &identitymocks.EducationBackend{} + metrics := metrics.New(prometheus.NewRegistry(), func([]string) (string, string) { return "", "" }) newSchool = libregraph.NewEducationSchool() newSchool.SetId("school1") @@ -83,6 +86,7 @@ var _ = Describe("Schools", func() { var err error svc, err = service.NewService( service.Config(cfg), + service.Metrics(metrics), service.WithGatewaySelector(gatewaySelector), service.WithIdentityEducationBackend(identityEducationBackend), ) diff --git a/services/graph/pkg/service/v0/educationuser_test.go b/services/graph/pkg/service/v0/educationuser_test.go index 077f270bb1..bf8aecfbca 100644 --- a/services/graph/pkg/service/v0/educationuser_test.go +++ b/services/graph/pkg/service/v0/educationuser_test.go @@ -21,6 +21,7 @@ import ( "github.com/opencloud-eu/reva/v2/pkg/rgrpc/status" "github.com/opencloud-eu/reva/v2/pkg/rgrpc/todo/pool" cs3mocks "github.com/opencloud-eu/reva/v2/tests/cs3mocks/mocks" + "github.com/prometheus/client_golang/prometheus" "github.com/stretchr/testify/mock" "google.golang.org/grpc" @@ -30,6 +31,7 @@ import ( "github.com/opencloud-eu/opencloud/services/graph/pkg/config" "github.com/opencloud-eu/opencloud/services/graph/pkg/config/defaults" identitymocks "github.com/opencloud-eu/opencloud/services/graph/pkg/identity/mocks" + "github.com/opencloud-eu/opencloud/services/graph/pkg/metrics" service "github.com/opencloud-eu/opencloud/services/graph/pkg/service/v0" ) @@ -71,6 +73,7 @@ var _ = Describe("EducationUsers", func() { ) identityEducationBackend = &identitymocks.EducationBackend{} + metrics := metrics.New(prometheus.NewRegistry(), func([]string) (string, string) { return "", "" }) roleService = &mocks.RoleService{} rr = httptest.NewRecorder() @@ -85,6 +88,7 @@ var _ = Describe("EducationUsers", func() { var err error svc, err = service.NewService( service.Config(cfg), + service.Metrics(metrics), service.WithGatewaySelector(gatewaySelector), service.EventsPublisher(&eventsPublisher), service.WithIdentityEducationBackend(identityEducationBackend), diff --git a/services/graph/pkg/service/v0/graph.go b/services/graph/pkg/service/v0/graph.go index 38246415e7..913d2ab087 100644 --- a/services/graph/pkg/service/v0/graph.go +++ b/services/graph/pkg/service/v0/graph.go @@ -26,6 +26,7 @@ import ( settingssvc "github.com/opencloud-eu/opencloud/protogen/gen/opencloud/services/settings/v0" "github.com/opencloud-eu/opencloud/services/graph/pkg/errorcode" "github.com/opencloud-eu/opencloud/services/graph/pkg/identity" + "github.com/opencloud-eu/opencloud/services/graph/pkg/metrics" ) // Permissions is the interface used to access the permissions service @@ -66,10 +67,13 @@ type Graph struct { searchService searchsvc.SearchProviderService keycloakClient keycloak.Client historyClient ehsvc.EventHistoryService + metrics *metrics.Metrics traceProvider trace.TracerProvider natskv jetstream.KeyValue } +var _ Service = Graph{} // ensure that the Graph struct implements all of the Service interface + // ServeHTTP implements the Service interface. func (g Graph) ServeHTTP(w http.ResponseWriter, r *http.Request) { // There was a number of issues with the chi router and parameters with diff --git a/services/graph/pkg/service/v0/graph_test.go b/services/graph/pkg/service/v0/graph_test.go index a960b1cdaf..1bf8027761 100644 --- a/services/graph/pkg/service/v0/graph_test.go +++ b/services/graph/pkg/service/v0/graph_test.go @@ -25,6 +25,7 @@ import ( "github.com/opencloud-eu/reva/v2/pkg/utils" cs3mocks "github.com/opencloud-eu/reva/v2/tests/cs3mocks/mocks" "github.com/pkg/errors" + "github.com/prometheus/client_golang/prometheus" "github.com/stretchr/testify/mock" "github.com/tidwall/gjson" "google.golang.org/grpc" @@ -36,6 +37,7 @@ import ( "github.com/opencloud-eu/opencloud/services/graph/pkg/config" "github.com/opencloud-eu/opencloud/services/graph/pkg/config/defaults" "github.com/opencloud-eu/opencloud/services/graph/pkg/errorcode" + "github.com/opencloud-eu/opencloud/services/graph/pkg/metrics" service "github.com/opencloud-eu/opencloud/services/graph/pkg/service/v0" "github.com/opencloud-eu/opencloud/services/graph/pkg/unifiedrole" ) @@ -61,6 +63,8 @@ var _ = Describe("Graph", func() { BeforeEach(func() { rr = httptest.NewRecorder() + metrics := metrics.New(prometheus.NewRegistry(), func([]string) (string, string) { return "", "" }) + ctx = revactx.ContextSetUser(context.Background(), &userprovider.User{Id: &userprovider.UserId{Type: userprovider.UserType_USER_TYPE_PRIMARY, OpaqueId: "testuser"}, Username: "testuser"}) cfg = defaults.FullDefaultConfig() cfg.Identity.LDAP.CACert = "" // skip the startup checks, we don't use LDAP at all in this tests @@ -84,6 +88,7 @@ var _ = Describe("Graph", func() { var err error svc, err = service.NewService( service.Config(cfg), + service.Metrics(metrics), service.WithGatewaySelector(gatewaySelector), service.EventsPublisher(&eventsPublisher), service.PermissionService(&permissionService), diff --git a/services/graph/pkg/service/v0/groups.go b/services/graph/pkg/service/v0/groups.go index d56b1b4590..eb5eece3c7 100644 --- a/services/graph/pkg/service/v0/groups.go +++ b/services/graph/pkg/service/v0/groups.go @@ -163,7 +163,7 @@ func (g Graph) PatchGroup(w http.ResponseWriter, r *http.Request) { } if reflect.ValueOf(*changes).IsZero() { - logger.Debug().Interface("body", r.Body).Msg("ignoring empyt request body") + logger.Debug().Interface("body", r.Body).Msg("ignoring empty request body") render.Status(r, http.StatusNoContent) render.NoContent(w, r) return @@ -176,7 +176,7 @@ func (g Graph) PatchGroup(w http.ResponseWriter, r *http.Request) { errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, "Invalid displayName") return } - if err = g.identityBackend.UpdateGroupName(r.Context(), groupID, displayName); err != nil { + if err := g.identityBackend.UpdateGroupName(r.Context(), groupID, displayName); err != nil { logger.Debug().Err(err).Msg("could not update group displayName") errorcode.RenderError(w, r, err) return @@ -279,7 +279,6 @@ func (g Graph) DeleteGroup(w http.ResponseWriter, r *http.Request) { logger.Debug().Str("id", groupID).Msg("calling delete group on backend") err = g.identityBackend.DeleteGroup(r.Context(), groupID) - if err != nil { logger.Debug().Err(err).Msg("could not delete group: backend error") errorcode.RenderError(w, r, err) @@ -439,14 +438,15 @@ func (g Graph) DeleteGroupMember(w http.ResponseWriter, r *http.Request) { errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, "missing member id") return } + logger.Debug().Str("groupID", groupID).Str("memberID", memberID).Msg("calling delete member on backend") err = g.identityBackend.RemoveMemberFromGroup(r.Context(), groupID, memberID) - if err != nil { logger.Debug().Err(err).Msg("could not delete group member: backend error") errorcode.RenderError(w, r, err) return } + e := events.GroupMemberRemoved{ GroupID: groupID, UserID: memberID, diff --git a/services/graph/pkg/service/v0/groups_test.go b/services/graph/pkg/service/v0/groups_test.go index 4b61e5e8c2..8e9aef8b5d 100644 --- a/services/graph/pkg/service/v0/groups_test.go +++ b/services/graph/pkg/service/v0/groups_test.go @@ -18,6 +18,7 @@ import ( revactx "github.com/opencloud-eu/reva/v2/pkg/ctx" "github.com/opencloud-eu/reva/v2/pkg/rgrpc/todo/pool" cs3mocks "github.com/opencloud-eu/reva/v2/tests/cs3mocks/mocks" + "github.com/prometheus/client_golang/prometheus" "github.com/stretchr/testify/mock" "google.golang.org/grpc" @@ -29,6 +30,7 @@ import ( "github.com/opencloud-eu/opencloud/services/graph/pkg/config/defaults" "github.com/opencloud-eu/opencloud/services/graph/pkg/errorcode" identitymocks "github.com/opencloud-eu/opencloud/services/graph/pkg/identity/mocks" + "github.com/opencloud-eu/opencloud/services/graph/pkg/metrics" service "github.com/opencloud-eu/opencloud/services/graph/pkg/service/v0" ) @@ -72,6 +74,7 @@ var _ = Describe("Groups", func() { permissionService = &mocks.Permissions{} identityBackend = &identitymocks.Backend{} + metrics := metrics.New(prometheus.NewRegistry(), func([]string) (string, string) { return "", "" }) newGroup = libregraph.NewGroup() newGroup.SetMembersodataBind([]string{"/users/user1"}) newGroup.SetId("group1") @@ -88,6 +91,7 @@ var _ = Describe("Groups", func() { var err error svc, err = service.NewService( service.Config(cfg), + service.Metrics(metrics), service.WithGatewaySelector(gatewaySelector), service.EventsPublisher(&eventsPublisher), service.WithIdentityBackend(identityBackend), @@ -413,9 +417,12 @@ var _ = Describe("Groups", func() { updatedGroupJson, err := json.Marshal(updatedGroup) Expect(err).ToNot(HaveOccurred()) + metrics := metrics.New(prometheus.NewRegistry(), func([]string) (string, string) { return "", "" }) + cfg.API.GroupMembersPatchLimit = 21 svc, err = service.NewService( service.Config(cfg), + service.Metrics(metrics), service.WithGatewaySelector(gatewaySelector), service.EventsPublisher(&eventsPublisher), service.WithIdentityBackend(identityBackend), diff --git a/services/graph/pkg/service/v0/option.go b/services/graph/pkg/service/v0/option.go index cc720d7b92..774fb176d7 100644 --- a/services/graph/pkg/service/v0/option.go +++ b/services/graph/pkg/service/v0/option.go @@ -18,6 +18,7 @@ import ( settingssvc "github.com/opencloud-eu/opencloud/protogen/gen/opencloud/services/settings/v0" "github.com/opencloud-eu/opencloud/services/graph/pkg/config" "github.com/opencloud-eu/opencloud/services/graph/pkg/identity" + "github.com/opencloud-eu/opencloud/services/graph/pkg/metrics" ) // Option defines a single option function. @@ -28,6 +29,7 @@ type Options struct { Context context.Context Logger log.Logger Config *config.Config + Metrics *metrics.Metrics Middleware []func(http.Handler) http.Handler RequireAdminMiddleware func(http.Handler) http.Handler GatewaySelector pool.Selectable[gateway.GatewayAPIClient] @@ -78,6 +80,13 @@ func Config(val *config.Config) Option { } } +// Context provides a function to set the context option. +func Metrics(m *metrics.Metrics) Option { + return func(o *Options) { + o.Metrics = m + } +} + // Middleware provides a function to set the middleware option. func Middleware(val ...func(http.Handler) http.Handler) Option { return func(o *Options) { diff --git a/services/graph/pkg/service/v0/password.go b/services/graph/pkg/service/v0/password.go index 96b98721a9..da431cd2aa 100644 --- a/services/graph/pkg/service/v0/password.go +++ b/services/graph/pkg/service/v0/password.go @@ -11,6 +11,7 @@ import ( libregraph "github.com/opencloud-eu/libre-graph-api-go" "github.com/opencloud-eu/opencloud/services/graph/pkg/errorcode" + "github.com/opencloud-eu/opencloud/services/graph/pkg/metrics" revactx "github.com/opencloud-eu/reva/v2/pkg/ctx" "github.com/opencloud-eu/reva/v2/pkg/events" ) @@ -21,6 +22,7 @@ func (g Graph) ChangeOwnPassword(w http.ResponseWriter, r *http.Request) { ctx := r.Context() u, ok := revactx.ContextGetUser(ctx) if !ok { + g.metrics.UserPasswordChanges.WithLabelValues(metrics.ResultFailure, metrics.ReasonInvalid) g.logger.Error().Msg("user not in context") errorcode.ServiceNotAvailable.Render(w, r, http.StatusInternalServerError, "user not in context") return @@ -29,6 +31,7 @@ func (g Graph) ChangeOwnPassword(w http.ResponseWriter, r *http.Request) { sanitizedPath := strings.TrimPrefix(r.URL.Path, "/graph/v1.0/") _, err := godata.ParseRequest(r.Context(), sanitizedPath, r.URL.Query()) if err != nil { + g.metrics.UserPasswordChanges.WithLabelValues(metrics.ResultFailure, metrics.ReasonInvalid) g.logger.Err(err).Interface("query", r.URL.Query()).Msg("query error") errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, err.Error()) return @@ -36,23 +39,27 @@ func (g Graph) ChangeOwnPassword(w http.ResponseWriter, r *http.Request) { cpw := libregraph.NewPasswordChangeWithDefaults() err = StrictJSONUnmarshal(r.Body, cpw) if err != nil { + g.metrics.UserPasswordChanges.WithLabelValues(metrics.ResultFailure, metrics.ReasonInvalid) errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, err.Error()) return } currentPw := cpw.GetCurrentPassword() if currentPw == "" { + g.metrics.UserPasswordChanges.WithLabelValues(metrics.ResultFailure, metrics.ReasonInvalid) errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, "current password cannot be empty") return } newPw := cpw.GetNewPassword() if newPw == "" { + g.metrics.UserPasswordChanges.WithLabelValues(metrics.ResultFailure, metrics.ReasonInvalid) errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, "new password cannot be empty") return } if newPw == currentPw { + g.metrics.UserPasswordChanges.WithLabelValues(metrics.ResultFailure, metrics.ReasonInvalid) errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, "new password must be different from current password") return } @@ -64,11 +71,13 @@ func (g Graph) ChangeOwnPassword(w http.ResponseWriter, r *http.Request) { } client, err := g.gatewaySelector.Next() if err != nil { + g.metrics.UserPasswordChanges.WithLabelValues(metrics.ResultFailure, metrics.ReasonError) errorcode.ServiceNotAvailable.Render(w, r, http.StatusInternalServerError, "could not select next gateway client, aborting") return } authRes, err := client.Authenticate(r.Context(), authReq) if err != nil { + g.metrics.UserPasswordChanges.WithLabelValues(metrics.ResultFailure, metrics.ReasonError) errorcode.ServiceNotAvailable.Render(w, r, http.StatusInternalServerError, err.Error()) return } @@ -77,9 +86,11 @@ func (g Graph) ChangeOwnPassword(w http.ResponseWriter, r *http.Request) { case cs3rpc.Code_CODE_OK: break case cs3rpc.Code_CODE_UNAUTHENTICATED, cs3rpc.Code_CODE_PERMISSION_DENIED: + g.metrics.UserPasswordChanges.WithLabelValues(metrics.ResultFailure, metrics.ReasonWrongPassword) errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, "wrong current password") return default: + g.metrics.UserPasswordChanges.WithLabelValues(metrics.ResultFailure, metrics.ReasonError) errorcode.InvalidRequest.Render(w, r, http.StatusInternalServerError, "password change failed") return } @@ -88,12 +99,19 @@ func (g Graph) ChangeOwnPassword(w http.ResponseWriter, r *http.Request) { newPwProfile.SetPassword(newPw) changes := libregraph.NewUserUpdate() changes.SetPasswordProfile(*newPwProfile) - _, err = g.identityBackend.UpdateUser(ctx, u.Id.OpaqueId, *changes) + found, err := g.identityBackend.UpdateUser(ctx, u.Id.OpaqueId, *changes) if err != nil { + g.metrics.UserPasswordChanges.WithLabelValues(metrics.ResultFailure, metrics.ReasonError) errorcode.InvalidRequest.Render(w, r, http.StatusInternalServerError, "password change failed") g.logger.Debug().Err(err).Str("userid", u.Id.OpaqueId).Msg("failed to update user password") return } + if found == nil { + g.metrics.UserPasswordChanges.WithLabelValues(metrics.ResultFailure, metrics.ReasonInvalid) + errorcode.ItemNotFound.Render(w, r, http.StatusNotFound, "password change failed") + g.logger.Debug().Err(err).Str("userid", u.Id.OpaqueId).Msg("failed to update user password: user not found in backend") + return + } currentUser := revactx.ContextMustGetUser(r.Context()) g.publishEvent( @@ -107,6 +125,8 @@ func (g Graph) ChangeOwnPassword(w http.ResponseWriter, r *http.Request) { }, ) + g.metrics.UserPasswordChanges.WithLabelValues(metrics.ResultSuccess, "") + render.Status(r, http.StatusNoContent) render.NoContent(w, r) } diff --git a/services/graph/pkg/service/v0/password_test.go b/services/graph/pkg/service/v0/password_test.go index f8464cabb7..8505630bb4 100644 --- a/services/graph/pkg/service/v0/password_test.go +++ b/services/graph/pkg/service/v0/password_test.go @@ -18,6 +18,7 @@ import ( "github.com/opencloud-eu/reva/v2/pkg/rgrpc/status" "github.com/opencloud-eu/reva/v2/pkg/rgrpc/todo/pool" cs3mocks "github.com/opencloud-eu/reva/v2/tests/cs3mocks/mocks" + "github.com/prometheus/client_golang/prometheus" "github.com/stretchr/testify/mock" "google.golang.org/grpc" @@ -28,6 +29,7 @@ import ( "github.com/opencloud-eu/opencloud/services/graph/pkg/config/defaults" "github.com/opencloud-eu/opencloud/services/graph/pkg/identity" identitymocks "github.com/opencloud-eu/opencloud/services/graph/pkg/identity/mocks" + "github.com/opencloud-eu/opencloud/services/graph/pkg/metrics" service "github.com/opencloud-eu/opencloud/services/graph/pkg/service/v0" ) @@ -76,14 +78,17 @@ var _ = Describe("Users changing their own password", func() { GroupSearchScope: "sub", } logger := log.NewLogger() - identityBackend, err = identity.NewLDAPBackend(ldapClient, ldapConfig, &logger) + reg := prometheus.NewRegistry() + identityBackend, err = identity.NewLDAPBackend(ldapClient, ldapConfig, &logger, "opencloud", "test", reg) Expect(err).To(BeNil()) + metrics := metrics.New(reg, func([]string) (string, string) { return "", "" }) eventsPublisher = mocks.Publisher{} var err error svc, err = service.NewService( service.Config(cfg), + service.Metrics(metrics), service.WithGatewaySelector(gatewaySelector), service.WithIdentityBackend(identityBackend), service.EventsPublisher(&eventsPublisher), @@ -145,9 +150,10 @@ func mockedLDAPClient() *identitymocks.Client { lm := &identitymocks.Client{} userEntry := ldap.NewEntry("uid=test", map[string][]string{ - "uid": {"test"}, - "displayName": {"test"}, - "mail": {"test@example.org"}, + "openCloudUUID": {"test"}, + "uid": {"test"}, + "displayName": {"test"}, + "mail": {"test@example.org"}, }) lm.On("Search", mock.Anything, mock.Anything, mock.Anything, mock.Anything, diff --git a/services/graph/pkg/service/v0/rolemanagement_test.go b/services/graph/pkg/service/v0/rolemanagement_test.go index fcb77f7c6b..a75a63b79e 100644 --- a/services/graph/pkg/service/v0/rolemanagement_test.go +++ b/services/graph/pkg/service/v0/rolemanagement_test.go @@ -12,12 +12,14 @@ import ( libregraph "github.com/opencloud-eu/libre-graph-api-go" "github.com/opencloud-eu/reva/v2/pkg/rgrpc/todo/pool" cs3mocks "github.com/opencloud-eu/reva/v2/tests/cs3mocks/mocks" + "github.com/prometheus/client_golang/prometheus" "google.golang.org/grpc" "github.com/opencloud-eu/opencloud/pkg/shared" "github.com/opencloud-eu/opencloud/services/graph/mocks" "github.com/opencloud-eu/opencloud/services/graph/pkg/config" "github.com/opencloud-eu/opencloud/services/graph/pkg/config/defaults" + "github.com/opencloud-eu/opencloud/services/graph/pkg/metrics" service "github.com/opencloud-eu/opencloud/services/graph/pkg/service/v0" "github.com/opencloud-eu/opencloud/services/graph/pkg/unifiedrole" ) @@ -55,10 +57,12 @@ var _ = Describe("RoleManagement", func() { ) eventsPublisher = mocks.Publisher{} permSvc = mocks.Permissions{} + metrics := metrics.New(prometheus.NewRegistry(), func([]string) (string, string) { return "", "" }) var err error svc, err = service.NewService( service.Config(cfg), + service.Metrics(metrics), service.WithGatewaySelector(gatewaySelector), service.EventsPublisher(&eventsPublisher), service.PermissionService(&permSvc), diff --git a/services/graph/pkg/service/v0/service.go b/services/graph/pkg/service/v0/service.go index 6eed744a05..9b9636e370 100644 --- a/services/graph/pkg/service/v0/service.go +++ b/services/graph/pkg/service/v0/service.go @@ -190,6 +190,7 @@ func NewService(opts ...Option) (Graph, error) { //nolint:maintidx identityEducationBackend: options.IdentityEducationBackend, keycloakClient: options.KeycloakClient, historyClient: options.EventHistoryClient, + metrics: options.Metrics, traceProvider: options.TraceProvider, valueService: options.ValueService, natskv: options.NatsKeyValue, @@ -432,6 +433,33 @@ func NewService(opts ...Option) (Graph, error) { //nolint:maintidx return svc, nil } +// this function receives the request URI path chi pattern, split cleanly on '/' +// and is tasked with returning a value for the Graph API version, +// as well as a value for the Graph API resource +// +// e.g. for +// +// '/graph/v1.0/users/{userid}' +// -> receive ['graph', 'v1.0', 'users', '{userid}'] +// <- return ('v1.0', 'users') +func DecomposeGraphApiRequestPattern(pieces []string) (string, string) { + // we keep this function close to the chi routes to improve our changes of + // changing this implementation whenever we change the routes + version := "" + resource := "" + if len(pieces) >= 2 { + // first path element is the /graph prefix, ignore that + // followed by the version (v1.0) + version = pieces[1] + if len(pieces) >= 3 { + // and the resource + resource = pieces[2] + } + } + return version, resource + +} + // parseHeaderPurge parses the 'Purge' header. // '1', 't', 'T', 'TRUE', 'true', 'True' are parsed as true // all other values are false. diff --git a/services/graph/pkg/service/v0/sharedbyme_test.go b/services/graph/pkg/service/v0/sharedbyme_test.go index 80ef743538..5496675337 100644 --- a/services/graph/pkg/service/v0/sharedbyme_test.go +++ b/services/graph/pkg/service/v0/sharedbyme_test.go @@ -24,6 +24,7 @@ import ( "github.com/opencloud-eu/reva/v2/pkg/storagespace" "github.com/opencloud-eu/reva/v2/pkg/utils" cs3mocks "github.com/opencloud-eu/reva/v2/tests/cs3mocks/mocks" + "github.com/prometheus/client_golang/prometheus" "github.com/stretchr/testify/mock" "google.golang.org/grpc" @@ -33,6 +34,7 @@ import ( "github.com/opencloud-eu/opencloud/services/graph/pkg/config/defaults" identitymocks "github.com/opencloud-eu/opencloud/services/graph/pkg/identity/mocks" "github.com/opencloud-eu/opencloud/services/graph/pkg/linktype" + "github.com/opencloud-eu/opencloud/services/graph/pkg/metrics" service "github.com/opencloud-eu/opencloud/services/graph/pkg/service/v0" "github.com/opencloud-eu/opencloud/services/graph/pkg/unifiedrole" ) @@ -236,6 +238,7 @@ var _ = Describe("sharedbyme", func() { ) identityBackend = &identitymocks.Backend{} + metrics := metrics.New(prometheus.NewRegistry(), func([]string) (string, string) { return "", "" }) rr = httptest.NewRecorder() ctx = context.Background() @@ -248,6 +251,7 @@ var _ = Describe("sharedbyme", func() { svc, err = service.NewService( service.Config(cfg), + service.Metrics(metrics), service.WithGatewaySelector(gatewaySelector), service.EventsPublisher(&eventsPublisher), service.WithIdentityBackend(identityBackend), diff --git a/services/graph/pkg/service/v0/sharedwithme_test.go b/services/graph/pkg/service/v0/sharedwithme_test.go index 7d0200e776..07153a2962 100644 --- a/services/graph/pkg/service/v0/sharedwithme_test.go +++ b/services/graph/pkg/service/v0/sharedwithme_test.go @@ -21,6 +21,7 @@ import ( "github.com/opencloud-eu/reva/v2/pkg/storagespace" "github.com/opencloud-eu/reva/v2/pkg/utils" cs3mocks "github.com/opencloud-eu/reva/v2/tests/cs3mocks/mocks" + "github.com/prometheus/client_golang/prometheus" "github.com/stretchr/testify/mock" "github.com/tidwall/gjson" "google.golang.org/grpc" @@ -33,6 +34,7 @@ import ( "github.com/opencloud-eu/opencloud/services/graph/pkg/config/defaults" "github.com/opencloud-eu/opencloud/services/graph/pkg/errorcode" identitymocks "github.com/opencloud-eu/opencloud/services/graph/pkg/identity/mocks" + "github.com/opencloud-eu/opencloud/services/graph/pkg/metrics" service "github.com/opencloud-eu/opencloud/services/graph/pkg/service/v0" // "github.com/opencloud-eu/opencloud/services/graph/pkg/unifiedrole" ) @@ -60,6 +62,7 @@ var _ = Describe("SharedWithMe", func() { ) identityBackend = &identitymocks.Backend{} + metrics := metrics.New(prometheus.NewRegistry(), func([]string) (string, string) { return "", "" }) tape = httptest.NewRecorder() ctx = context.Background() @@ -73,6 +76,7 @@ var _ = Describe("SharedWithMe", func() { var err error svc, err = service.NewService( service.Config(cfg), + service.Metrics(metrics), service.WithGatewaySelector(gatewaySelector), service.WithIdentityBackend(identityBackend), ) diff --git a/services/graph/pkg/service/v0/users.go b/services/graph/pkg/service/v0/users.go index 18b154546b..893464890c 100644 --- a/services/graph/pkg/service/v0/users.go +++ b/services/graph/pkg/service/v0/users.go @@ -757,9 +757,11 @@ func (g Graph) DeleteUser(w http.ResponseWriter, r *http.Request) { if (g.config.UserSoftDeleteRetentionTime > 0 && us.State == userstate.UserStateSoftDeleted && purgeUser) || (g.config.UserSoftDeleteRetentionTime == 0) { logger.Debug().Str("id", user.GetId()).Msg("calling delete user on backend") - err = g.identityBackend.DeleteUser(r.Context(), user.GetId()) + err := g.identityBackend.DeleteUser(r.Context(), user.GetId()) if err != nil { - logger.Debug().Err(err).Msg("could not delete user: backend error") + // since cases where the user cannot be found in the backend don't return an error, + // we can safely log this as an error: + logger.Error().Err(err).Msg("could not delete user: backend error") errorcode.RenderError(w, r, err) return } @@ -769,7 +771,13 @@ func (g Graph) DeleteUser(w http.ResponseWriter, r *http.Request) { if err != nil { logger.Error().Err(err).Str("id", userID).Msg("could not set user state") errorcode.RenderError(w, r, err) + return } + + // user has successfully been hard-deleted + e := events.UserDeleted{UserID: user.GetId()} + e.Executant = currentUser.GetId() + g.publishEvent(r.Context(), e) } else { logger.Debug().Str("id", user.GetId()).Msg("calling soft delete user on backend") userUpdate := *libregraph.NewUserUpdate() @@ -778,33 +786,40 @@ func (g Graph) DeleteUser(w http.ResponseWriter, r *http.Request) { us.RetentionPeriod = g.config.UserSoftDeleteRetentionTime us.Reason = "User soft deleted via Graph API" // TODO: this needs a proper implementation through the request us.TimeStamp = time.Now() - err = g.setUserStateToNatsKeyValue(r.Context(), userID, us) - if err != nil { + if err := g.setUserStateToNatsKeyValue(r.Context(), userID, us); err != nil { logger.Error().Err(err).Str("id", userID).Msg("could not set user state") errorcode.RenderError(w, r, err) return } - g.identityBackend.UpdateUser(r.Context(), user.GetId(), userUpdate) - } - - if g.config.UserSoftDeleteRetentionTime == 0 || - (g.config.UserSoftDeleteRetentionTime > 0 && purgeUser && us.State == userstate.UserStateSoftDeleted) { - e := events.UserDeleted{UserID: user.GetId()} - e.Executant = currentUser.GetId() - g.publishEvent(r.Context(), e) - } else { - e := events.UserSoftDeleted{ - UserID: user.GetId(), - RetentionTime: g.config.UserSoftDeleteRetentionTime, - Timestamp: &v1beta1.Timestamp{ - Seconds: uint64(time.Now().Unix()), - Nanos: uint32(time.Now().Nanosecond()), - }, - Reason: "User deleted via Graph API", // TODO: this needs a proper implementation through the request + // note: logging these as WARN for backwards compatibility reason since they previously were not logged at all + if softDeletedUser, err := g.identityBackend.UpdateUser(r.Context(), user.GetId(), userUpdate); err != nil { + if errorcode.IsErrorCode(err, errorcode.ItemNotFound) { + // in thie case, an attempt to perform a soft-delete on a user failed because the user was not found, + // in which case we don't treat this as an error, as it is most certainly caused by a parallel operation + // that performed a hard-delete on that user. + logger.Warn().Err(err).Str("id", userID).Msg("failed to update user for soft-deletion because the user was not found") + } else { + // any other error does denote a failure to soft-delete that user though, and should be treated as such + logger.Error().Err(err).Str("id", userID).Msg("failed to update user") + errorcode.RenderError(w, r, err) + return + } + } else { + // user has successfully been soft-deleted + e := events.UserSoftDeleted{ + UserID: softDeletedUser.GetId(), + RetentionTime: g.config.UserSoftDeleteRetentionTime, + Timestamp: &v1beta1.Timestamp{ + Seconds: uint64(time.Now().Unix()), + Nanos: uint32(time.Now().Nanosecond()), + }, + Reason: "User deleted via Graph API", // TODO: this needs a proper implementation through the request + } + e.Executant = currentUser.GetId() + g.publishEvent(r.Context(), e) } - e.Executant = currentUser.GetId() - g.publishEvent(r.Context(), e) } + render.Status(r, http.StatusNoContent) render.NoContent(w, r) } @@ -887,6 +902,12 @@ func (g Graph) patchUser(w http.ResponseWriter, r *http.Request, nameOrID string errorcode.RenderError(w, r, err) return } + if oldUserValues == nil { + // this is an error + logger.Debug().Err(err).Interface("query", r.URL.Query()).Msg("could not get user: user not found in backend") + errorcode.ItemNotFound.Render(w, r, http.StatusNotFound, "user not found") + return + } if nameOrID == "" { logger.Debug().Msg("could not update user: missing user id") diff --git a/services/graph/pkg/service/v0/users_test.go b/services/graph/pkg/service/v0/users_test.go index 5db85f90f8..4cf7150efd 100644 --- a/services/graph/pkg/service/v0/users_test.go +++ b/services/graph/pkg/service/v0/users_test.go @@ -24,10 +24,12 @@ import ( "github.com/opencloud-eu/reva/v2/pkg/rgrpc/status" "github.com/opencloud-eu/reva/v2/pkg/rgrpc/todo/pool" cs3mocks "github.com/opencloud-eu/reva/v2/tests/cs3mocks/mocks" + "github.com/prometheus/client_golang/prometheus" "github.com/stretchr/testify/mock" "go-micro.dev/v4/client" "google.golang.org/grpc" + "github.com/opencloud-eu/opencloud/services/graph/pkg/metrics" "github.com/opencloud-eu/opencloud/services/graph/pkg/userstate" "github.com/opencloud-eu/opencloud/pkg/shared" @@ -57,6 +59,7 @@ var _ = Describe("Users", func() { valueService *settingsmocks.ValueService permissionService *mocks.Permissions identityBackend *identitymocks.Backend + mtrics *metrics.Metrics natsKeyValueMock *mocks.KeyValue rr *httptest.ResponseRecorder @@ -86,6 +89,7 @@ var _ = Describe("Users", func() { natsKeyValueMock = &mocks.KeyValue{} valueService = &settingsmocks.ValueService{} permissionService = &mocks.Permissions{} + mtrics = metrics.New(prometheus.NewRegistry(), func([]string) (string, string) { return "", "" }) rr = httptest.NewRecorder() ctx = context.Background() @@ -104,6 +108,7 @@ var _ = Describe("Users", func() { var err error svc, err = service.NewService( service.Config(cfg), + service.Metrics(mtrics), service.WithGatewaySelector(gatewaySelector), service.EventsPublisher(&eventsPublisher), service.WithIdentityBackend(identityBackend), @@ -916,6 +921,7 @@ var _ = Describe("Users", func() { localSvc, err := service.NewService( service.Config(localCfg), + service.Metrics(mtrics), service.WithGatewaySelector(gatewaySelector), service.EventsPublisher(&eventsPublisher), service.WithIdentityBackend(identityBackend), @@ -1091,7 +1097,7 @@ var _ = Describe("Users", func() { lu := libregraph.User{} lu.SetId(otheruser.Id.OpaqueId) identityBackend.On("GetUser", mock.Anything, mock.Anything, mock.Anything).Return(&lu, nil) - //identityBackend.On("DeleteUser", mock.Anything, mock.Anything).Return(nil) + //identityBackend.On("DeleteUser", mock.Anything, mock.Anything).Return(IsFound, nil) identityBackend.On("UpdateUser", mock.Anything, mock.Anything, mock.Anything).Return(&lu, nil) gatewayClient.On("DeleteStorageSpace", mock.Anything, mock.Anything).Return(&provider.DeleteStorageSpaceResponse{ Status: status.NewOK(ctx), @@ -1315,6 +1321,7 @@ var _ = Describe("Users", func() { var err error svc, err = service.NewService( service.Config(cfg), + service.Metrics(mtrics), service.WithGatewaySelector(gatewaySelector), service.EventsPublisher(&eventsPublisher), service.WithIdentityBackend(identityBackend),