From 72f83ba8d1a8ba8f3ac648ffee21ac1c2155184f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Laurent=20St=C3=A9phenne?= Date: Sat, 2 May 2026 16:50:49 -0400 Subject: [PATCH 1/4] Feat: OIDC profile picture --- services/proxy/pkg/command/server.go | 29 ++ services/proxy/pkg/config/config.go | 7 + .../pkg/config/defaults/defaultconfig.go | 4 + .../proxy/pkg/middleware/account_resolver.go | 267 ++++++++++++++++-- services/proxy/pkg/middleware/options.go | 28 ++ 5 files changed, 306 insertions(+), 29 deletions(-) diff --git a/services/proxy/pkg/command/server.go b/services/proxy/pkg/command/server.go index 2debb13dd2..4626d9f2a5 100644 --- a/services/proxy/pkg/command/server.go +++ b/services/proxy/pkg/command/server.go @@ -3,8 +3,10 @@ package command import ( "context" "crypto/tls" + "crypto/x509" "fmt" "net/http" + "os" "os/signal" "time" @@ -276,6 +278,28 @@ func loadMiddlewares(logger log.Logger, cfg *config.Config, Timeout: time.Second * 10, } + backendTLSConfig := &tls.Config{ + MinVersion: tls.VersionTLS12, + InsecureSkipVerify: cfg.InsecureBackends, //nolint:gosec + } + if cfg.BackendHTTPSCACert != "" { + certs := x509.NewCertPool() + pemData, err := os.ReadFile(cfg.BackendHTTPSCACert) + if err != nil { + logger.Fatal().Err(err).Msg("Failed to read backend HTTPS CA certificate") + } + if !certs.AppendCertsFromPEM(pemData) { + logger.Fatal().Msg("Failed to append backend HTTPS CA certificate") + } + backendTLSConfig.RootCAs = certs + } + backendHTTPClient := &http.Client{ + Transport: &http.Transport{ + TLSClientConfig: backendTLSConfig, + }, + Timeout: time.Second * 10, + } + var authenticators []middleware.Authenticator if cfg.EnableBasicAuth { logger.Warn().Msg("basic auth enabled, use only for testing or development") @@ -363,6 +387,11 @@ func loadMiddlewares(logger log.Logger, cfg *config.Config, middleware.TraceProvider(traceProvider), middleware.UserProvider(userProvider), middleware.UserRoleAssigner(roleAssigner), + middleware.HTTPClient(oidcHTTPClient), + middleware.BackendHTTPClient(backendHTTPClient), + middleware.OIDCIss(cfg.OIDC.Issuer), + middleware.ServiceSelector(serviceSelector), + middleware.OIDCProfilePicture(cfg.OIDCProfilePicture), middleware.SkipUserInfo(cfg.OIDC.SkipUserInfo), middleware.UserOIDCClaim(cfg.UserOIDCClaim), middleware.UserCS3Claim(cfg.UserCS3Claim), diff --git a/services/proxy/pkg/config/config.go b/services/proxy/pkg/config/config.go index cdaa3e81e7..fb12f1bd1c 100644 --- a/services/proxy/pkg/config/config.go +++ b/services/proxy/pkg/config/config.go @@ -39,6 +39,7 @@ type Config struct { MachineAuthAPIKey string `yaml:"machine_auth_api_key" env:"OC_MACHINE_AUTH_API_KEY;PROXY_MACHINE_AUTH_API_KEY" desc:"Machine auth API key used to validate internal requests necessary to access resources from other services." introductionVersion:"1.0.0" mask:"password"` AutoprovisionAccounts bool `yaml:"auto_provision_accounts" env:"PROXY_AUTOPROVISION_ACCOUNTS" desc:"Set this to 'true' to automatically provision users that do not yet exist in the users service on-demand upon first sign-in. To use this a write-enabled libregraph user backend needs to be setup an running." introductionVersion:"1.0.0"` AutoProvisionClaims AutoProvisionClaims `yaml:"auto_provision_claims"` + OIDCProfilePicture OIDCProfilePicture `yaml:"oidc_profile_picture"` EnableBasicAuth bool `yaml:"enable_basic_auth" env:"PROXY_ENABLE_BASIC_AUTH" desc:"Set this to true to enable 'basic authentication' (username/password)." introductionVersion:"1.0.0"` InsecureBackends bool `yaml:"insecure_backends" env:"PROXY_INSECURE_BACKENDS" desc:"Disable TLS certificate validation for all HTTP backend connections." introductionVersion:"1.0.0"` BackendHTTPSCACert string `yaml:"backend_https_cacert" env:"PROXY_HTTPS_CACERT" desc:"Path/File for the root CA certificate used to validate the server’s TLS certificate for https enabled backend services." introductionVersion:"1.0.0"` @@ -168,6 +169,12 @@ type AutoProvisionClaims struct { Groups string `yaml:"groups" env:"PROXY_AUTOPROVISION_CLAIM_GROUPS" desc:"The name of the OIDC claim that holds the groups." introductionVersion:"1.0.0"` } +// OIDCProfilePicture configures profile picture sync for OIDC users. +type OIDCProfilePicture struct { + Claim string `yaml:"claim" env:"PROXY_OIDC_PROFILE_PICTURE_CLAIM" desc:"The name of the OIDC claim that holds a URL to the user's profile picture. When set, the profile picture will be synced on login."` + DisableLocalChanges bool `yaml:"disable_local_changes" env:"PROXY_OIDC_PROFILE_PICTURE_DISABLE_LOCAL_CHANGES" desc:"When set, users authenticated via OIDC cannot change their profile picture locally (PUT/PATCH/DELETE on /graph/v1.0/me/photo/$value)."` +} + // PolicySelector is the toplevel-configuration for different selectors type PolicySelector struct { Static *StaticSelectorConf `yaml:"static"` diff --git a/services/proxy/pkg/config/defaults/defaultconfig.go b/services/proxy/pkg/config/defaults/defaultconfig.go index 187e52be6f..f7f556fe71 100644 --- a/services/proxy/pkg/config/defaults/defaultconfig.go +++ b/services/proxy/pkg/config/defaults/defaultconfig.go @@ -92,6 +92,10 @@ func DefaultConfig() *config.Config { DisplayName: "name", Groups: "groups", }, + OIDCProfilePicture: config.OIDCProfilePicture{ + Claim: "", + DisableLocalChanges: false, + }, EnableBasicAuth: false, InsecureBackends: false, CSPConfigFileLocation: "", diff --git a/services/proxy/pkg/middleware/account_resolver.go b/services/proxy/pkg/middleware/account_resolver.go index 04b57b6105..fa7b019af8 100644 --- a/services/proxy/pkg/middleware/account_resolver.go +++ b/services/proxy/pkg/middleware/account_resolver.go @@ -1,10 +1,14 @@ package middleware import ( + "bytes" "context" "errors" "fmt" + "io" "net/http" + "net/url" + "strings" "time" gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1" @@ -14,6 +18,7 @@ import ( "github.com/opencloud-eu/opencloud/services/proxy/pkg/router" "github.com/opencloud-eu/opencloud/services/proxy/pkg/user/backend" "github.com/opencloud-eu/opencloud/services/proxy/pkg/userroles" + "go-micro.dev/v4/selector" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/trace" @@ -27,6 +32,11 @@ import ( "github.com/opencloud-eu/reva/v2/pkg/utils" ) +const ( + graphServiceName = "eu.opencloud.web.graph" + maxProfilePhotoBytes = 10 << 20 +) + // AccountResolver provides a middleware which mints a jwt and adds it to the proxied request based // on the oidc-claims func AccountResolver(optionSetters ...Option) func(next http.Handler) http.Handler { @@ -46,42 +56,63 @@ func AccountResolver(optionSetters ...Option) func(next http.Handler) http.Handl ) go tenantIDCache.Start() + httpClient := options.HTTPClient + if httpClient == nil { + httpClient = &http.Client{Timeout: 10 * time.Second} + } + backendHTTPClient := options.BackendHTTPClient + if backendHTTPClient == nil { + backendHTTPClient = &http.Client{Timeout: 10 * time.Second} + } + return func(next http.Handler) http.Handler { return &accountResolver{ - next: next, - logger: logger, - tracer: tracer, - userProvider: options.UserProvider, - userOIDCClaim: options.UserOIDCClaim, - userCS3Claim: options.UserCS3Claim, - tenantOIDCClaim: options.TenantOIDCClaim, - tenantIDMappingEnabled: options.TenantIDMappingEnabled, - gatewaySelector: options.RevaGatewaySelector, - serviceAccount: options.ServiceAccount, - userRoleAssigner: options.UserRoleAssigner, - autoProvisionAccounts: options.AutoprovisionAccounts, - multiTenantEnabled: options.MultiTenantEnabled, - lastGroupSyncCache: lastGroupSyncCache, - tenantIDCache: tenantIDCache, - eventsPublisher: options.EventsPublisher, + next: next, + logger: logger, + tracer: tracer, + userProvider: options.UserProvider, + userOIDCClaim: options.UserOIDCClaim, + userCS3Claim: options.UserCS3Claim, + tenantOIDCClaim: options.TenantOIDCClaim, + tenantIDMappingEnabled: options.TenantIDMappingEnabled, + gatewaySelector: options.RevaGatewaySelector, + serviceAccount: options.ServiceAccount, + userRoleAssigner: options.UserRoleAssigner, + autoProvisionAccounts: options.AutoprovisionAccounts, + multiTenantEnabled: options.MultiTenantEnabled, + lastGroupSyncCache: lastGroupSyncCache, + tenantIDCache: tenantIDCache, + eventsPublisher: options.EventsPublisher, + profilePictureClaim: options.OIDCProfilePicture.Claim, + disableLocalProfilePictureChange: options.OIDCProfilePicture.DisableLocalChanges, + httpClient: httpClient, + backendHTTPClient: backendHTTPClient, + oidcIssuer: options.OIDCIss, + serviceSelector: options.ServiceSelector, } } } type accountResolver struct { - next http.Handler - logger log.Logger - tracer trace.Tracer - userProvider backend.UserBackend - userRoleAssigner userroles.UserRoleAssigner - autoProvisionAccounts bool - multiTenantEnabled bool - tenantIDMappingEnabled bool - gatewaySelector pool.Selectable[gateway.GatewayAPIClient] - serviceAccount config.ServiceAccount - userOIDCClaim string - userCS3Claim string - tenantOIDCClaim string + next http.Handler + logger log.Logger + tracer trace.Tracer + userProvider backend.UserBackend + userRoleAssigner userroles.UserRoleAssigner + autoProvisionAccounts bool + multiTenantEnabled bool + tenantIDMappingEnabled bool + gatewaySelector pool.Selectable[gateway.GatewayAPIClient] + serviceSelector selector.Selector + serviceAccount config.ServiceAccount + userOIDCClaim string + userCS3Claim string + tenantOIDCClaim string + profilePictureClaim string + disableLocalProfilePictureChange bool + oidcIssuer string + httpClient *http.Client + backendHTTPClient *http.Client // lastGroupSyncCache is used to keep track of when the last sync of group // memberships was done for a specific user. This is used to trigger a sync // with every single request. @@ -126,6 +157,172 @@ func readStringClaim(path string, claims map[string]any) (string, error) { return value, fmt.Errorf("claim path '%s' not set or empty", path) } +func (m accountResolver) isProfilePhotoMutation(req *http.Request) bool { + if req == nil { + return false + } + switch req.Method { + case http.MethodPut, http.MethodPatch, http.MethodDelete: + default: + return false + } + pathValue := strings.TrimSuffix(req.URL.Path, "/") + return strings.HasSuffix(pathValue, "/graph/v1.0/me/photo/$value") +} + +func (m accountResolver) syncProfilePicture(ctx context.Context, req *http.Request, user *cs3user.User, token string, claims map[string]any) error { + if user == nil { + return errors.New("missing user for profile photo sync") + } + if token == "" { + return errors.New("missing user token for profile photo sync") + } + + pictureURL, err := readStringClaim(m.profilePictureClaim, claims) + if err != nil { + m.logger.Debug().Err(err).Str("claim", m.profilePictureClaim).Msg("profile picture claim missing") + return nil + } + if pictureURL == "" { + return nil + } + + parsedURL, err := url.Parse(pictureURL) + if err != nil { + return fmt.Errorf("invalid profile picture URL: %w", err) + } + if parsedURL.Scheme != "http" && parsedURL.Scheme != "https" { + return fmt.Errorf("unsupported profile picture URL scheme: %s", parsedURL.Scheme) + } + if parsedURL.Host == "" { + return fmt.Errorf("profile picture URL is missing a host") + } + + authHeader := "" + if req != nil { + authHeader = req.Header.Get("Authorization") + } + + photo, err := m.fetchProfilePicture(ctx, parsedURL, authHeader) + if err != nil { + return err + } + + return m.updateGraphProfilePhoto(ctx, token, photo) +} + +func (m accountResolver) fetchProfilePicture(ctx context.Context, pictureURL *url.URL, authHeader string) ([]byte, error) { + client := m.httpClient + if client == nil { + client = http.DefaultClient + } + + request, err := http.NewRequestWithContext(ctx, http.MethodGet, pictureURL.String(), nil) + if err != nil { + return nil, err + } + request.Header.Set("Accept", "image/*") + if authHeader != "" && m.shouldAttachOIDCToken(pictureURL) { + request.Header.Set("Authorization", authHeader) + } + + resp, err := client.Do(request) + if err != nil { + return nil, err + } + defer resp.Body.Close() + + if resp.StatusCode < http.StatusOK || resp.StatusCode >= http.StatusMultipleChoices { + return nil, fmt.Errorf("profile picture request returned %s", resp.Status) + } + + limited := io.LimitReader(resp.Body, int64(maxProfilePhotoBytes)+1) + data, err := io.ReadAll(limited) + if err != nil { + return nil, err + } + if len(data) == 0 { + return nil, errors.New("profile picture response was empty") + } + if len(data) > maxProfilePhotoBytes { + return nil, fmt.Errorf("profile picture exceeds %d bytes", maxProfilePhotoBytes) + } + contentType := http.DetectContentType(data) + if !strings.HasPrefix(contentType, "image/") { + return nil, fmt.Errorf("unsupported profile picture content type: %s", contentType) + } + + return data, nil +} + +func (m accountResolver) shouldAttachOIDCToken(pictureURL *url.URL) bool { + if m.oidcIssuer == "" || pictureURL == nil { + return false + } + issuerURL, err := url.Parse(m.oidcIssuer) + if err != nil || issuerURL.Host == "" { + return false + } + return strings.EqualFold(issuerURL.Host, pictureURL.Host) +} + +func (m accountResolver) updateGraphProfilePhoto(ctx context.Context, token string, photo []byte) error { + if token == "" { + return errors.New("missing access token for graph profile photo update") + } + baseURL, err := m.graphBaseURL() + if err != nil { + return err + } + + endpoint := baseURL + "/v1.0/me/photo/$value" + request, err := http.NewRequestWithContext(ctx, http.MethodPut, endpoint, bytes.NewReader(photo)) + if err != nil { + return err + } + request.Header.Set(revactx.TokenHeader, token) + request.Header.Set("Content-Type", http.DetectContentType(photo)) + + client := m.backendHTTPClient + if client == nil { + client = http.DefaultClient + } + resp, err := client.Do(request) + if err != nil { + return err + } + defer resp.Body.Close() + + if resp.StatusCode < http.StatusOK || resp.StatusCode >= http.StatusMultipleChoices { + body, _ := io.ReadAll(io.LimitReader(resp.Body, 2048)) + return fmt.Errorf("graph profile photo update failed: %s (%s)", resp.Status, strings.TrimSpace(string(body))) + } + + return nil +} + +func (m accountResolver) graphBaseURL() (string, error) { + if m.serviceSelector == nil { + return "", errors.New("service selector not configured") + } + selectNext, err := m.serviceSelector.Select(graphServiceName) + if err != nil { + return "", err + } + node, err := selectNext() + if err != nil { + return "", err + } + scheme := node.Metadata["protocol"] + if node.Metadata["use_tls"] == "true" { + scheme = "https" + } + if scheme == "" { + scheme = "http" + } + return fmt.Sprintf("%s://%s/graph", scheme, node.Address), nil +} + // TODO do not use the context to store values: https://medium.com/@cep21/how-to-correctly-use-context-context-in-go-1-7-8f2c0fafdf39 func (m accountResolver) ServeHTTP(w http.ResponseWriter, req *http.Request) { ctx, span := m.tracer.Start(req.Context(), fmt.Sprintf("%s %s", req.Method, req.URL.Path), trace.WithSpanKind(trace.SpanKindServer)) @@ -140,6 +337,12 @@ func (m accountResolver) ServeHTTP(w http.ResponseWriter, req *http.Request) { return } + if m.disableLocalProfilePictureChange && claims != nil && m.isProfilePhotoMutation(req) { + m.logger.Debug().Str("path", req.URL.Path).Msg("profile photo updates disabled for OIDC users") + w.WriteHeader(http.StatusForbidden) + return + } + if user == nil && claims != nil { value, err := readStringClaim(m.userOIDCClaim, claims) if err != nil { @@ -219,6 +422,12 @@ func (m accountResolver) ServeHTTP(w http.ResponseWriter, req *http.Request) { } } + if m.profilePictureClaim != "" && oidc.NewSessionFlagFromContext(ctx) { + if err := m.syncProfilePicture(ctx, req, user, token, claims); err != nil { + m.logger.Warn().Err(err).Str("userid", user.GetId().GetOpaqueId()).Msg("Failed to sync profile picture from OIDC claim") + } + } + // resolve the user's roles user, err = m.userRoleAssigner.UpdateUserRoleAssignment(ctx, user, claims) if err != nil { diff --git a/services/proxy/pkg/middleware/options.go b/services/proxy/pkg/middleware/options.go index 7e57d13ba2..2910fcb20e 100644 --- a/services/proxy/pkg/middleware/options.go +++ b/services/proxy/pkg/middleware/options.go @@ -14,6 +14,7 @@ import ( "github.com/opencloud-eu/opencloud/services/proxy/pkg/userroles" "github.com/opencloud-eu/reva/v2/pkg/events" "github.com/opencloud-eu/reva/v2/pkg/rgrpc/todo/pool" + "go-micro.dev/v4/selector" "go-micro.dev/v4/store" "go.opentelemetry.io/otel/trace" ) @@ -29,6 +30,8 @@ type Options struct { PolicySelector config.PolicySelector // HTTPClient to use for communication with the oidcAuth provider HTTPClient *http.Client + // BackendHTTPClient to use for internal backend HTTP calls + BackendHTTPClient *http.Client // UserProvider backend to use for resolving User UserProvider backend.UserBackend // UserRoleAssigner to user for assign a users default role @@ -43,10 +46,14 @@ type Options struct { OIDCIss string // RevaGatewaySelector to send requests to the reva gateway RevaGatewaySelector pool.Selectable[gateway.GatewayAPIClient] + // ServiceSelector to resolve internal HTTP services + ServiceSelector selector.Selector // PreSignedURLConfig to configure the middleware PreSignedURLConfig config.PreSignedURL // UserOIDCClaim to read from the oidc claims UserOIDCClaim string + // OIDCProfilePicture config for syncing profile pictures from OIDC claims + OIDCProfilePicture config.OIDCProfilePicture // UserCS3Claim to use when looking up a user in the CS3 API UserCS3Claim string // TenantOIDCClaim is a JMESPath expression to extract the tenant ID from the OIDC claims. @@ -116,6 +123,13 @@ func HTTPClient(c *http.Client) Option { } } +// BackendHTTPClient provides a function to set the backend http client config option. +func BackendHTTPClient(c *http.Client) Option { + return func(o *Options) { + o.BackendHTTPClient = c + } +} + // SettingsRoleService provides a function to set the role service option. func SettingsRoleService(rc settingssvc.RoleService) Option { return func(o *Options) { @@ -158,6 +172,13 @@ func WithRevaGatewaySelector(val pool.Selectable[gateway.GatewayAPIClient]) Opti } } +// ServiceSelector provides a function to set the internal service selector option. +func ServiceSelector(val selector.Selector) Option { + return func(o *Options) { + o.ServiceSelector = val + } +} + // PreSignedURLConfig provides a function to set the PreSignedURL config func PreSignedURLConfig(cfg config.PreSignedURL) Option { return func(o *Options) { @@ -172,6 +193,13 @@ func UserOIDCClaim(val string) Option { } } +// OIDCProfilePicture provides a function to set the OIDC profile picture config +func OIDCProfilePicture(val config.OIDCProfilePicture) Option { + return func(o *Options) { + o.OIDCProfilePicture = val + } +} + // UserCS3Claim provides a function to set the UserClaimType config func UserCS3Claim(val string) Option { return func(o *Options) { From 1c13bc02b266eb39e2f1bc6afe5bb96792cfb4bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Laurent=20St=C3=A9phenne?= Date: Mon, 11 May 2026 22:32:57 -0400 Subject: [PATCH 2/4] Removed code to block profile picture updates --- services/proxy/pkg/config/config.go | 8 +------ .../pkg/config/defaults/defaultconfig.go | 5 +--- .../proxy/pkg/middleware/account_resolver.go | 23 +------------------ 3 files changed, 3 insertions(+), 33 deletions(-) diff --git a/services/proxy/pkg/config/config.go b/services/proxy/pkg/config/config.go index fb12f1bd1c..f3df0f10f7 100644 --- a/services/proxy/pkg/config/config.go +++ b/services/proxy/pkg/config/config.go @@ -39,7 +39,6 @@ type Config struct { MachineAuthAPIKey string `yaml:"machine_auth_api_key" env:"OC_MACHINE_AUTH_API_KEY;PROXY_MACHINE_AUTH_API_KEY" desc:"Machine auth API key used to validate internal requests necessary to access resources from other services." introductionVersion:"1.0.0" mask:"password"` AutoprovisionAccounts bool `yaml:"auto_provision_accounts" env:"PROXY_AUTOPROVISION_ACCOUNTS" desc:"Set this to 'true' to automatically provision users that do not yet exist in the users service on-demand upon first sign-in. To use this a write-enabled libregraph user backend needs to be setup an running." introductionVersion:"1.0.0"` AutoProvisionClaims AutoProvisionClaims `yaml:"auto_provision_claims"` - OIDCProfilePicture OIDCProfilePicture `yaml:"oidc_profile_picture"` EnableBasicAuth bool `yaml:"enable_basic_auth" env:"PROXY_ENABLE_BASIC_AUTH" desc:"Set this to true to enable 'basic authentication' (username/password)." introductionVersion:"1.0.0"` InsecureBackends bool `yaml:"insecure_backends" env:"PROXY_INSECURE_BACKENDS" desc:"Disable TLS certificate validation for all HTTP backend connections." introductionVersion:"1.0.0"` BackendHTTPSCACert string `yaml:"backend_https_cacert" env:"PROXY_HTTPS_CACERT" desc:"Path/File for the root CA certificate used to validate the server’s TLS certificate for https enabled backend services." introductionVersion:"1.0.0"` @@ -166,15 +165,10 @@ type AutoProvisionClaims struct { Username string `yaml:"username" env:"PROXY_AUTOPROVISION_CLAIM_USERNAME" desc:"The name of the OIDC claim that holds the username." introductionVersion:"1.0.0"` Email string `yaml:"email" env:"PROXY_AUTOPROVISION_CLAIM_EMAIL" desc:"The name of the OIDC claim that holds the email." introductionVersion:"1.0.0"` DisplayName string `yaml:"display_name" env:"PROXY_AUTOPROVISION_CLAIM_DISPLAYNAME" desc:"The name of the OIDC claim that holds the display name." introductionVersion:"1.0.0"` + ProfilePicture string `yaml:"profile_picture" env:"PROXY_AUTOPROVISION_CLAIM_PROFILE_PICTURE" desc:"The name of the OIDC claim that holds the profile picture URL. When set, the profile picture will be synced on login."` Groups string `yaml:"groups" env:"PROXY_AUTOPROVISION_CLAIM_GROUPS" desc:"The name of the OIDC claim that holds the groups." introductionVersion:"1.0.0"` } -// OIDCProfilePicture configures profile picture sync for OIDC users. -type OIDCProfilePicture struct { - Claim string `yaml:"claim" env:"PROXY_OIDC_PROFILE_PICTURE_CLAIM" desc:"The name of the OIDC claim that holds a URL to the user's profile picture. When set, the profile picture will be synced on login."` - DisableLocalChanges bool `yaml:"disable_local_changes" env:"PROXY_OIDC_PROFILE_PICTURE_DISABLE_LOCAL_CHANGES" desc:"When set, users authenticated via OIDC cannot change their profile picture locally (PUT/PATCH/DELETE on /graph/v1.0/me/photo/$value)."` -} - // PolicySelector is the toplevel-configuration for different selectors type PolicySelector struct { Static *StaticSelectorConf `yaml:"static"` diff --git a/services/proxy/pkg/config/defaults/defaultconfig.go b/services/proxy/pkg/config/defaults/defaultconfig.go index f7f556fe71..8e2c7a7d53 100644 --- a/services/proxy/pkg/config/defaults/defaultconfig.go +++ b/services/proxy/pkg/config/defaults/defaultconfig.go @@ -90,12 +90,9 @@ func DefaultConfig() *config.Config { Username: "preferred_username", Email: "email", DisplayName: "name", + ProfilePicture: "", Groups: "groups", }, - OIDCProfilePicture: config.OIDCProfilePicture{ - Claim: "", - DisableLocalChanges: false, - }, EnableBasicAuth: false, InsecureBackends: false, CSPConfigFileLocation: "", diff --git a/services/proxy/pkg/middleware/account_resolver.go b/services/proxy/pkg/middleware/account_resolver.go index fa7b019af8..fdb5db7957 100644 --- a/services/proxy/pkg/middleware/account_resolver.go +++ b/services/proxy/pkg/middleware/account_resolver.go @@ -83,8 +83,7 @@ func AccountResolver(optionSetters ...Option) func(next http.Handler) http.Handl lastGroupSyncCache: lastGroupSyncCache, tenantIDCache: tenantIDCache, eventsPublisher: options.EventsPublisher, - profilePictureClaim: options.OIDCProfilePicture.Claim, - disableLocalProfilePictureChange: options.OIDCProfilePicture.DisableLocalChanges, + profilePictureClaim: options.AutoProvisionClaims.ProfilePicture, httpClient: httpClient, backendHTTPClient: backendHTTPClient, oidcIssuer: options.OIDCIss, @@ -109,7 +108,6 @@ type accountResolver struct { userCS3Claim string tenantOIDCClaim string profilePictureClaim string - disableLocalProfilePictureChange bool oidcIssuer string httpClient *http.Client backendHTTPClient *http.Client @@ -157,19 +155,6 @@ func readStringClaim(path string, claims map[string]any) (string, error) { return value, fmt.Errorf("claim path '%s' not set or empty", path) } -func (m accountResolver) isProfilePhotoMutation(req *http.Request) bool { - if req == nil { - return false - } - switch req.Method { - case http.MethodPut, http.MethodPatch, http.MethodDelete: - default: - return false - } - pathValue := strings.TrimSuffix(req.URL.Path, "/") - return strings.HasSuffix(pathValue, "/graph/v1.0/me/photo/$value") -} - func (m accountResolver) syncProfilePicture(ctx context.Context, req *http.Request, user *cs3user.User, token string, claims map[string]any) error { if user == nil { return errors.New("missing user for profile photo sync") @@ -337,12 +322,6 @@ func (m accountResolver) ServeHTTP(w http.ResponseWriter, req *http.Request) { return } - if m.disableLocalProfilePictureChange && claims != nil && m.isProfilePhotoMutation(req) { - m.logger.Debug().Str("path", req.URL.Path).Msg("profile photo updates disabled for OIDC users") - w.WriteHeader(http.StatusForbidden) - return - } - if user == nil && claims != nil { value, err := readStringClaim(m.userOIDCClaim, claims) if err != nil { From 5f4f8c53887b3f352522ebef3df155cb8bf770db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Laurent=20St=C3=A9phenne?= Date: Wed, 13 May 2026 15:22:37 -0400 Subject: [PATCH 3/4] Fix: Config now uses AutoProvisionClaims --- services/proxy/pkg/command/server.go | 2 +- .../proxy/pkg/middleware/account_resolver.go | 10 +++++----- services/proxy/pkg/middleware/options.go | 18 +++++++++--------- 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/services/proxy/pkg/command/server.go b/services/proxy/pkg/command/server.go index 4626d9f2a5..b94e48cd16 100644 --- a/services/proxy/pkg/command/server.go +++ b/services/proxy/pkg/command/server.go @@ -391,11 +391,11 @@ func loadMiddlewares(logger log.Logger, cfg *config.Config, middleware.BackendHTTPClient(backendHTTPClient), middleware.OIDCIss(cfg.OIDC.Issuer), middleware.ServiceSelector(serviceSelector), - middleware.OIDCProfilePicture(cfg.OIDCProfilePicture), middleware.SkipUserInfo(cfg.OIDC.SkipUserInfo), middleware.UserOIDCClaim(cfg.UserOIDCClaim), middleware.UserCS3Claim(cfg.UserCS3Claim), middleware.TenantOIDCClaim(cfg.TenantOIDCClaim), + middleware.AutoProvisionClaims(cfg.AutoProvisionClaims), middleware.TenantIDMappingEnabled(cfg.TenantIDMappingEnabled), middleware.ServiceAccount(cfg.ServiceAccount), middleware.WithRevaGatewaySelector(gatewaySelector), diff --git a/services/proxy/pkg/middleware/account_resolver.go b/services/proxy/pkg/middleware/account_resolver.go index fdb5db7957..67be2d4dac 100644 --- a/services/proxy/pkg/middleware/account_resolver.go +++ b/services/proxy/pkg/middleware/account_resolver.go @@ -74,6 +74,7 @@ func AccountResolver(optionSetters ...Option) func(next http.Handler) http.Handl userOIDCClaim: options.UserOIDCClaim, userCS3Claim: options.UserCS3Claim, tenantOIDCClaim: options.TenantOIDCClaim, + autoProvisionClaims: options.AutoProvisionClaims, tenantIDMappingEnabled: options.TenantIDMappingEnabled, gatewaySelector: options.RevaGatewaySelector, serviceAccount: options.ServiceAccount, @@ -83,7 +84,6 @@ func AccountResolver(optionSetters ...Option) func(next http.Handler) http.Handl lastGroupSyncCache: lastGroupSyncCache, tenantIDCache: tenantIDCache, eventsPublisher: options.EventsPublisher, - profilePictureClaim: options.AutoProvisionClaims.ProfilePicture, httpClient: httpClient, backendHTTPClient: backendHTTPClient, oidcIssuer: options.OIDCIss, @@ -107,7 +107,7 @@ type accountResolver struct { userOIDCClaim string userCS3Claim string tenantOIDCClaim string - profilePictureClaim string + autoProvisionClaims config.AutoProvisionClaims oidcIssuer string httpClient *http.Client backendHTTPClient *http.Client @@ -163,9 +163,9 @@ func (m accountResolver) syncProfilePicture(ctx context.Context, req *http.Reque return errors.New("missing user token for profile photo sync") } - pictureURL, err := readStringClaim(m.profilePictureClaim, claims) + pictureURL, err := readStringClaim(m.autoProvisionClaims.ProfilePicture, claims) if err != nil { - m.logger.Debug().Err(err).Str("claim", m.profilePictureClaim).Msg("profile picture claim missing") + m.logger.Debug().Err(err).Str("claim", m.autoProvisionClaims.ProfilePicture).Msg("profile picture claim missing") return nil } if pictureURL == "" { @@ -401,7 +401,7 @@ func (m accountResolver) ServeHTTP(w http.ResponseWriter, req *http.Request) { } } - if m.profilePictureClaim != "" && oidc.NewSessionFlagFromContext(ctx) { + if m.autoProvisionClaims.ProfilePicture != "" && oidc.NewSessionFlagFromContext(ctx) { if err := m.syncProfilePicture(ctx, req, user, token, claims); err != nil { m.logger.Warn().Err(err).Str("userid", user.GetId().GetOpaqueId()).Msg("Failed to sync profile picture from OIDC claim") } diff --git a/services/proxy/pkg/middleware/options.go b/services/proxy/pkg/middleware/options.go index 2910fcb20e..005241b842 100644 --- a/services/proxy/pkg/middleware/options.go +++ b/services/proxy/pkg/middleware/options.go @@ -52,13 +52,13 @@ type Options struct { PreSignedURLConfig config.PreSignedURL // UserOIDCClaim to read from the oidc claims UserOIDCClaim string - // OIDCProfilePicture config for syncing profile pictures from OIDC claims - OIDCProfilePicture config.OIDCProfilePicture // UserCS3Claim to use when looking up a user in the CS3 API UserCS3Claim string // TenantOIDCClaim is a JMESPath expression to extract the tenant ID from the OIDC claims. // When set, the extracted value is verified against the tenant ID on the resolved user. TenantOIDCClaim string + // AutoProvisionClaims to read the user info from the oidc claims + AutoProvisionClaims config.AutoProvisionClaims // AutoprovisionAccounts when an accountResolver does not exist. AutoprovisionAccounts bool // EnableBasicAuth to allow basic auth @@ -193,13 +193,6 @@ func UserOIDCClaim(val string) Option { } } -// OIDCProfilePicture provides a function to set the OIDC profile picture config -func OIDCProfilePicture(val config.OIDCProfilePicture) Option { - return func(o *Options) { - o.OIDCProfilePicture = val - } -} - // UserCS3Claim provides a function to set the UserClaimType config func UserCS3Claim(val string) Option { return func(o *Options) { @@ -214,6 +207,13 @@ func TenantOIDCClaim(val string) Option { } } +// AutoProvisionClaims provides a function to set the AutoProvisionClaims config +func AutoProvisionClaims(cfg config.AutoProvisionClaims) Option { + return func(o *Options) { + o.AutoProvisionClaims = cfg + } +} + // AutoprovisionAccounts provides a function to set the AutoprovisionAccounts config func AutoprovisionAccounts(val bool) Option { return func(o *Options) { From bda8db3314fe4e2aa44860f92db13fc78cfc9463 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Laurent=20St=C3=A9phenne?= Date: Wed, 3 Jun 2026 12:40:42 -0400 Subject: [PATCH 4/4] Switched to event based --- services/graph/pkg/config/config.go | 20 +- .../pkg/config/defaults/defaultconfig.go | 8 + services/graph/pkg/service/v0/graph.go | 2 + services/graph/pkg/service/v0/option.go | 8 + services/graph/pkg/service/v0/service.go | 140 +++++++++- services/proxy/pkg/command/server.go | 28 -- .../proxy/pkg/middleware/account_resolver.go | 252 +++--------------- services/proxy/pkg/middleware/options.go | 23 +- .../opencloud-eu/reva/v2/pkg/events/users.go | 5 +- 9 files changed, 214 insertions(+), 272 deletions(-) diff --git a/services/graph/pkg/config/config.go b/services/graph/pkg/config/config.go index 51571c4607..df569b8a0e 100644 --- a/services/graph/pkg/config/config.go +++ b/services/graph/pkg/config/config.go @@ -25,13 +25,14 @@ type Config struct { TokenManager *TokenManager `yaml:"token_manager"` GRPCClientTLS *shared.GRPCClientTLS `yaml:"grpc_client_tls"` - Application Application `yaml:"application"` - Spaces Spaces `yaml:"spaces"` - Identity Identity `yaml:"identity"` - IncludeOCMSharees bool `yaml:"include_ocm_sharees" env:"OC_ENABLE_OCM;GRAPH_INCLUDE_OCM_SHAREES" desc:"Include OCM sharees when listing users." introductionVersion:"1.0.0"` - Events Events `yaml:"events"` - UnifiedRoles UnifiedRoles `yaml:"unified_roles"` - MaxConcurrency int `yaml:"max_concurrency" env:"OC_MAX_CONCURRENCY;GRAPH_MAX_CONCURRENCY" desc:"The maximum number of concurrent requests the service will handle." introductionVersion:"1.0.0"` + Application Application `yaml:"application"` + Spaces Spaces `yaml:"spaces"` + Identity Identity `yaml:"identity"` + OIDCProfilePicture OIDCProfilePicture `yaml:"oidc_profile_picture"` + IncludeOCMSharees bool `yaml:"include_ocm_sharees" env:"OC_ENABLE_OCM;GRAPH_INCLUDE_OCM_SHAREES" desc:"Include OCM sharees when listing users." introductionVersion:"1.0.0"` + Events Events `yaml:"events"` + UnifiedRoles UnifiedRoles `yaml:"unified_roles"` + MaxConcurrency int `yaml:"max_concurrency" env:"OC_MAX_CONCURRENCY;GRAPH_MAX_CONCURRENCY" desc:"The maximum number of concurrent requests the service will handle." introductionVersion:"1.0.0"` Keycloak Keycloak `yaml:"keycloak"` ServiceAccount ServiceAccount `yaml:"service_account"` @@ -116,6 +117,11 @@ type Identity struct { LDAP LDAP `yaml:"ldap"` } +type OIDCProfilePicture struct { + OIDCIssuer string `yaml:"oidc_issuer" env:"OC_URL;OC_OIDC_ISSUER;GRAPH_OIDC_ISSUER" desc:"URL of the OIDC issuer used to derive the default profile-picture URL allowlist when no explicit allowlist is configured." introductionVersion:"6.3.0"` + URLAllowlist []string `yaml:"url_allowlist" env:"GRAPH_OIDC_PROFILE_PICTURE_URL_ALLOWLIST" desc:"A comma separated allowlist of URL patterns accepted for profile-picture sync events. Patterns can be full URLs with glob support in the host (for example 'https://*.example.com') or '*' to allow all URLs. If empty, only the OIDC issuer host is allowed by default." introductionVersion:"6.3.0"` +} + // API represents API configuration parameters. type API struct { GroupMembersPatchLimit int `yaml:"group_members_patch_limit" env:"GRAPH_GROUP_MEMBERS_PATCH_LIMIT" desc:"The amount of group members allowed to be added with a single patch request." introductionVersion:"1.0.0"` diff --git a/services/graph/pkg/config/defaults/defaultconfig.go b/services/graph/pkg/config/defaults/defaultconfig.go index 9b5352f6ea..8f2afa49a2 100644 --- a/services/graph/pkg/config/defaults/defaultconfig.go +++ b/services/graph/pkg/config/defaults/defaultconfig.go @@ -110,6 +110,10 @@ func DefaultConfig() *config.Config { EducationResourcesEnabled: false, }, }, + OIDCProfilePicture: config.OIDCProfilePicture{ + OIDCIssuer: "https://localhost:9200", + URLAllowlist: []string{}, + }, Cache: &config.Cache{ Store: "memory", Nodes: []string{"127.0.0.1:9233"}, @@ -191,6 +195,10 @@ func EnsureDefaults(cfg *config.Config) { cfg.Metadata.SystemUserID = cfg.Commons.SystemUserID } + if cfg.OIDCProfilePicture.OIDCIssuer == "" && cfg.Commons != nil && cfg.Commons.OpenCloudURL != "" { + cfg.OIDCProfilePicture.OIDCIssuer = cfg.Commons.OpenCloudURL + } + } // Sanitize sanitized the configuration diff --git a/services/graph/pkg/service/v0/graph.go b/services/graph/pkg/service/v0/graph.go index c6ef4fa74c..a48f0f255d 100644 --- a/services/graph/pkg/service/v0/graph.go +++ b/services/graph/pkg/service/v0/graph.go @@ -62,6 +62,7 @@ type Graph struct { permissionsService Permissions valueService settingssvc.ValueService specialDriveItemsCache *ttlcache.Cache[string, any] + userProfilePhotoService UsersUserProfilePhotoProvider eventsPublisher events.Publisher eventsConsumer events.Consumer searchService searchsvc.SearchProviderService @@ -69,6 +70,7 @@ type Graph struct { historyClient ehsvc.EventHistoryService traceProvider trace.TracerProvider natskv jetstream.KeyValue + profilePictureHTTPClient HTTPClient } // ServeHTTP implements the Service interface. diff --git a/services/graph/pkg/service/v0/option.go b/services/graph/pkg/service/v0/option.go index 5330fd5783..88c7b11807 100644 --- a/services/graph/pkg/service/v0/option.go +++ b/services/graph/pkg/service/v0/option.go @@ -28,6 +28,7 @@ type Options struct { Context context.Context Logger log.Logger Config *config.Config + ProfilePictureHTTPClient HTTPClient Middleware []func(http.Handler) http.Handler RequireAdminMiddleware func(http.Handler) http.Handler GatewaySelector pool.Selectable[gateway.GatewayAPIClient] @@ -47,6 +48,13 @@ type Options struct { NatsKeyValue jetstream.KeyValue } +// ProfilePictureHTTPClient provides a function to set the HTTP client used for downloading OIDC profile pictures. +func ProfilePictureHTTPClient(val HTTPClient) Option { + return func(o *Options) { + o.ProfilePictureHTTPClient = val + } +} + // newOptions initializes the available default options. func newOptions(opts ...Option) Options { opt := Options{} diff --git a/services/graph/pkg/service/v0/service.go b/services/graph/pkg/service/v0/service.go index 6c9880ba28..c5b59b369b 100644 --- a/services/graph/pkg/service/v0/service.go +++ b/services/graph/pkg/service/v0/service.go @@ -1,19 +1,25 @@ package svc import ( + "bytes" "context" "crypto/tls" "crypto/x509" "errors" "fmt" + "io" "net/http" + "net/url" "os" + "path" "strconv" + "strings" "time" "github.com/go-chi/chi/v5" "github.com/go-chi/chi/v5/middleware" ldapv3 "github.com/go-ldap/ldap/v3" + "github.com/gobwas/glob" "github.com/jellydator/ttlcache/v3" "github.com/opencloud-eu/opencloud/services/graph/pkg/identity/cache" "github.com/riandyrn/otelchi" @@ -39,8 +45,9 @@ import ( const ( // HeaderPurge defines the header name for the purge header. - HeaderPurge = "Purge" - displayNameAttr = "displayName" + HeaderPurge = "Purge" + displayNameAttr = "displayName" + maxProfilePhotoBytes = 10 << 20 ) // Service defines the service handlers. @@ -191,6 +198,7 @@ func NewService(opts ...Option) (Graph, error) { //nolint:maintidx BaseGraphService: baseGraphService, mux: m, specialDriveItemsCache: spacePropertiesCache, + userProfilePhotoService: options.UserProfilePhotoService, eventsPublisher: options.EventsPublisher, eventsConsumer: options.EventsConsumer, searchService: options.SearchService, @@ -200,6 +208,10 @@ func NewService(opts ...Option) (Graph, error) { //nolint:maintidx traceProvider: options.TraceProvider, valueService: options.ValueService, natskv: options.NatsKeyValue, + profilePictureHTTPClient: options.ProfilePictureHTTPClient, + } + if svc.profilePictureHTTPClient == nil { + svc.profilePictureHTTPClient = http.DefaultClient } if err := setIdentityBackends(options, &svc); err != nil { @@ -578,6 +590,11 @@ func (g *Graph) StartListenForLogonEvents(ctx context.Context, l log.Logger) err 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") } + if ev.PictureURL != "" { + if err := g.syncProfilePictureFromURL(ctx, ev.Executant.GetOpaqueId(), ev.PictureURL); err != nil { + l.Warn().Err(err).Str("userid", ev.Executant.GetOpaqueId()).Msg("Failed to sync profile picture from OIDC claim") + } + } } case <-ctx.Done(): l.Info().Msg("context cancelled") @@ -588,6 +605,125 @@ func (g *Graph) StartListenForLogonEvents(ctx context.Context, l log.Logger) err return nil } +func (g *Graph) syncProfilePictureFromURL(ctx context.Context, userID, rawURL string) error { + if g.userProfilePhotoService == nil { + return errors.New("profile photo service not configured") + } + if userID == "" { + return errors.New("missing user id for profile picture sync") + } + if !g.isProfilePictureURLAllowed(rawURL) { + return fmt.Errorf("profile picture URL not allowed: %s", rawURL) + } + + data, err := g.fetchProfilePicture(ctx, rawURL) + if err != nil { + return err + } + + return g.userProfilePhotoService.UpdatePhoto(ctx, userID, bytes.NewReader(data)) +} + +func (g *Graph) fetchProfilePicture(ctx context.Context, rawURL string) ([]byte, error) { + request, err := http.NewRequestWithContext(ctx, http.MethodGet, rawURL, nil) + if err != nil { + return nil, err + } + request.Header.Set("Accept", "image/*") + + resp, err := g.profilePictureHTTPClient.Do(request) + if err != nil { + return nil, err + } + defer resp.Body.Close() + + if resp.StatusCode < http.StatusOK || resp.StatusCode >= http.StatusMultipleChoices { + return nil, fmt.Errorf("profile picture request returned %s", resp.Status) + } + + limited := io.LimitReader(resp.Body, int64(maxProfilePhotoBytes)+1) + data, err := io.ReadAll(limited) + if err != nil { + return nil, err + } + if len(data) == 0 { + return nil, errors.New("profile picture response was empty") + } + if len(data) > maxProfilePhotoBytes { + return nil, fmt.Errorf("profile picture exceeds %d bytes", maxProfilePhotoBytes) + } + contentType := http.DetectContentType(data) + if !strings.HasPrefix(contentType, "image/") { + return nil, fmt.Errorf("unsupported profile picture content type: %s", contentType) + } + + return data, nil +} + +func (g *Graph) isProfilePictureURLAllowed(rawURL string) bool { + parsedURL, err := url.Parse(rawURL) + if err != nil || parsedURL.Host == "" { + return false + } + if parsedURL.Scheme != "http" && parsedURL.Scheme != "https" { + return false + } + + for _, pattern := range g.profilePictureAllowlistPatterns() { + if pattern == "*" { + return true + } + if urlPatternMatches(pattern, parsedURL) { + return true + } + } + return false +} + +func (g *Graph) profilePictureAllowlistPatterns() []string { + if len(g.config.OIDCProfilePicture.URLAllowlist) > 0 { + return g.config.OIDCProfilePicture.URLAllowlist + } + issuerURL, err := url.Parse(g.config.OIDCProfilePicture.OIDCIssuer) + if err != nil || issuerURL.Host == "" { + return nil + } + return []string{fmt.Sprintf("%s://%s", issuerURL.Scheme, issuerURL.Host)} +} + +func urlPatternMatches(pattern string, target *url.URL) bool { + if target == nil { + return false + } + parsedPattern, err := url.Parse(pattern) + if err == nil && parsedPattern.Host != "" { + if parsedPattern.Scheme != "" && !strings.EqualFold(parsedPattern.Scheme, target.Scheme) { + return false + } + hostMatcher, err := glob.Compile(strings.ToLower(parsedPattern.Host)) + if err != nil { + return false + } + if !hostMatcher.Match(strings.ToLower(target.Host)) { + return false + } + if parsedPattern.Path == "" || parsedPattern.Path == "/" { + return true + } + if strings.HasSuffix(parsedPattern.Path, "*") { + prefix := strings.TrimSuffix(parsedPattern.Path, "*") + return strings.HasPrefix(target.Path, prefix) + } + return path.Clean(parsedPattern.Path) == path.Clean(target.Path) + } + + hostMatcher, err := glob.Compile(strings.ToLower(pattern)) + if err != nil { + return false + } + return hostMatcher.Match(strings.ToLower(target.Host)) +} + // parseHeaderPurge parses the 'Purge' header. // '1', 't', 'T', 'TRUE', 'true', 'True' are parsed as true // all other values are false. diff --git a/services/proxy/pkg/command/server.go b/services/proxy/pkg/command/server.go index b94e48cd16..50dca09c4a 100644 --- a/services/proxy/pkg/command/server.go +++ b/services/proxy/pkg/command/server.go @@ -3,10 +3,8 @@ package command import ( "context" "crypto/tls" - "crypto/x509" "fmt" "net/http" - "os" "os/signal" "time" @@ -278,28 +276,6 @@ func loadMiddlewares(logger log.Logger, cfg *config.Config, Timeout: time.Second * 10, } - backendTLSConfig := &tls.Config{ - MinVersion: tls.VersionTLS12, - InsecureSkipVerify: cfg.InsecureBackends, //nolint:gosec - } - if cfg.BackendHTTPSCACert != "" { - certs := x509.NewCertPool() - pemData, err := os.ReadFile(cfg.BackendHTTPSCACert) - if err != nil { - logger.Fatal().Err(err).Msg("Failed to read backend HTTPS CA certificate") - } - if !certs.AppendCertsFromPEM(pemData) { - logger.Fatal().Msg("Failed to append backend HTTPS CA certificate") - } - backendTLSConfig.RootCAs = certs - } - backendHTTPClient := &http.Client{ - Transport: &http.Transport{ - TLSClientConfig: backendTLSConfig, - }, - Timeout: time.Second * 10, - } - var authenticators []middleware.Authenticator if cfg.EnableBasicAuth { logger.Warn().Msg("basic auth enabled, use only for testing or development") @@ -387,10 +363,6 @@ func loadMiddlewares(logger log.Logger, cfg *config.Config, middleware.TraceProvider(traceProvider), middleware.UserProvider(userProvider), middleware.UserRoleAssigner(roleAssigner), - middleware.HTTPClient(oidcHTTPClient), - middleware.BackendHTTPClient(backendHTTPClient), - middleware.OIDCIss(cfg.OIDC.Issuer), - middleware.ServiceSelector(serviceSelector), middleware.SkipUserInfo(cfg.OIDC.SkipUserInfo), middleware.UserOIDCClaim(cfg.UserOIDCClaim), middleware.UserCS3Claim(cfg.UserCS3Claim), diff --git a/services/proxy/pkg/middleware/account_resolver.go b/services/proxy/pkg/middleware/account_resolver.go index 67be2d4dac..4b54b2e465 100644 --- a/services/proxy/pkg/middleware/account_resolver.go +++ b/services/proxy/pkg/middleware/account_resolver.go @@ -1,14 +1,10 @@ package middleware import ( - "bytes" "context" "errors" "fmt" - "io" "net/http" - "net/url" - "strings" "time" gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1" @@ -18,7 +14,6 @@ import ( "github.com/opencloud-eu/opencloud/services/proxy/pkg/router" "github.com/opencloud-eu/opencloud/services/proxy/pkg/user/backend" "github.com/opencloud-eu/opencloud/services/proxy/pkg/userroles" - "go-micro.dev/v4/selector" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/trace" @@ -32,11 +27,6 @@ import ( "github.com/opencloud-eu/reva/v2/pkg/utils" ) -const ( - graphServiceName = "eu.opencloud.web.graph" - maxProfilePhotoBytes = 10 << 20 -) - // AccountResolver provides a middleware which mints a jwt and adds it to the proxied request based // on the oidc-claims func AccountResolver(optionSetters ...Option) func(next http.Handler) http.Handler { @@ -56,61 +46,44 @@ func AccountResolver(optionSetters ...Option) func(next http.Handler) http.Handl ) go tenantIDCache.Start() - httpClient := options.HTTPClient - if httpClient == nil { - httpClient = &http.Client{Timeout: 10 * time.Second} - } - backendHTTPClient := options.BackendHTTPClient - if backendHTTPClient == nil { - backendHTTPClient = &http.Client{Timeout: 10 * time.Second} - } - return func(next http.Handler) http.Handler { return &accountResolver{ - next: next, - logger: logger, - tracer: tracer, - userProvider: options.UserProvider, - userOIDCClaim: options.UserOIDCClaim, - userCS3Claim: options.UserCS3Claim, - tenantOIDCClaim: options.TenantOIDCClaim, - autoProvisionClaims: options.AutoProvisionClaims, - tenantIDMappingEnabled: options.TenantIDMappingEnabled, - gatewaySelector: options.RevaGatewaySelector, - serviceAccount: options.ServiceAccount, - userRoleAssigner: options.UserRoleAssigner, - autoProvisionAccounts: options.AutoprovisionAccounts, - multiTenantEnabled: options.MultiTenantEnabled, - lastGroupSyncCache: lastGroupSyncCache, - tenantIDCache: tenantIDCache, - eventsPublisher: options.EventsPublisher, - httpClient: httpClient, - backendHTTPClient: backendHTTPClient, - oidcIssuer: options.OIDCIss, - serviceSelector: options.ServiceSelector, + next: next, + logger: logger, + tracer: tracer, + userProvider: options.UserProvider, + userOIDCClaim: options.UserOIDCClaim, + userCS3Claim: options.UserCS3Claim, + tenantOIDCClaim: options.TenantOIDCClaim, + autoProvisionClaims: options.AutoProvisionClaims, + tenantIDMappingEnabled: options.TenantIDMappingEnabled, + gatewaySelector: options.RevaGatewaySelector, + serviceAccount: options.ServiceAccount, + userRoleAssigner: options.UserRoleAssigner, + autoProvisionAccounts: options.AutoprovisionAccounts, + multiTenantEnabled: options.MultiTenantEnabled, + lastGroupSyncCache: lastGroupSyncCache, + tenantIDCache: tenantIDCache, + eventsPublisher: options.EventsPublisher, } } } type accountResolver struct { - next http.Handler - logger log.Logger - tracer trace.Tracer - userProvider backend.UserBackend - userRoleAssigner userroles.UserRoleAssigner - autoProvisionAccounts bool - multiTenantEnabled bool - tenantIDMappingEnabled bool - gatewaySelector pool.Selectable[gateway.GatewayAPIClient] - serviceSelector selector.Selector - serviceAccount config.ServiceAccount - userOIDCClaim string - userCS3Claim string - tenantOIDCClaim string - autoProvisionClaims config.AutoProvisionClaims - oidcIssuer string - httpClient *http.Client - backendHTTPClient *http.Client + next http.Handler + logger log.Logger + tracer trace.Tracer + userProvider backend.UserBackend + userRoleAssigner userroles.UserRoleAssigner + autoProvisionAccounts bool + multiTenantEnabled bool + tenantIDMappingEnabled bool + gatewaySelector pool.Selectable[gateway.GatewayAPIClient] + serviceAccount config.ServiceAccount + userOIDCClaim string + userCS3Claim string + tenantOIDCClaim string + autoProvisionClaims config.AutoProvisionClaims // lastGroupSyncCache is used to keep track of when the last sync of group // memberships was done for a specific user. This is used to trigger a sync // with every single request. @@ -155,157 +128,16 @@ func readStringClaim(path string, claims map[string]any) (string, error) { return value, fmt.Errorf("claim path '%s' not set or empty", path) } -func (m accountResolver) syncProfilePicture(ctx context.Context, req *http.Request, user *cs3user.User, token string, claims map[string]any) error { - if user == nil { - return errors.New("missing user for profile photo sync") +func (m accountResolver) readProfilePictureURL(claims map[string]any) string { + if m.autoProvisionClaims.ProfilePicture == "" { + return "" } - if token == "" { - return errors.New("missing user token for profile photo sync") - } - pictureURL, err := readStringClaim(m.autoProvisionClaims.ProfilePicture, claims) if err != nil { m.logger.Debug().Err(err).Str("claim", m.autoProvisionClaims.ProfilePicture).Msg("profile picture claim missing") - return nil - } - if pictureURL == "" { - return nil - } - - parsedURL, err := url.Parse(pictureURL) - if err != nil { - return fmt.Errorf("invalid profile picture URL: %w", err) - } - if parsedURL.Scheme != "http" && parsedURL.Scheme != "https" { - return fmt.Errorf("unsupported profile picture URL scheme: %s", parsedURL.Scheme) - } - if parsedURL.Host == "" { - return fmt.Errorf("profile picture URL is missing a host") - } - - authHeader := "" - if req != nil { - authHeader = req.Header.Get("Authorization") - } - - photo, err := m.fetchProfilePicture(ctx, parsedURL, authHeader) - if err != nil { - return err - } - - return m.updateGraphProfilePhoto(ctx, token, photo) -} - -func (m accountResolver) fetchProfilePicture(ctx context.Context, pictureURL *url.URL, authHeader string) ([]byte, error) { - client := m.httpClient - if client == nil { - client = http.DefaultClient - } - - request, err := http.NewRequestWithContext(ctx, http.MethodGet, pictureURL.String(), nil) - if err != nil { - return nil, err - } - request.Header.Set("Accept", "image/*") - if authHeader != "" && m.shouldAttachOIDCToken(pictureURL) { - request.Header.Set("Authorization", authHeader) - } - - resp, err := client.Do(request) - if err != nil { - return nil, err - } - defer resp.Body.Close() - - if resp.StatusCode < http.StatusOK || resp.StatusCode >= http.StatusMultipleChoices { - return nil, fmt.Errorf("profile picture request returned %s", resp.Status) - } - - limited := io.LimitReader(resp.Body, int64(maxProfilePhotoBytes)+1) - data, err := io.ReadAll(limited) - if err != nil { - return nil, err - } - if len(data) == 0 { - return nil, errors.New("profile picture response was empty") - } - if len(data) > maxProfilePhotoBytes { - return nil, fmt.Errorf("profile picture exceeds %d bytes", maxProfilePhotoBytes) + return "" } - contentType := http.DetectContentType(data) - if !strings.HasPrefix(contentType, "image/") { - return nil, fmt.Errorf("unsupported profile picture content type: %s", contentType) - } - - return data, nil -} - -func (m accountResolver) shouldAttachOIDCToken(pictureURL *url.URL) bool { - if m.oidcIssuer == "" || pictureURL == nil { - return false - } - issuerURL, err := url.Parse(m.oidcIssuer) - if err != nil || issuerURL.Host == "" { - return false - } - return strings.EqualFold(issuerURL.Host, pictureURL.Host) -} - -func (m accountResolver) updateGraphProfilePhoto(ctx context.Context, token string, photo []byte) error { - if token == "" { - return errors.New("missing access token for graph profile photo update") - } - baseURL, err := m.graphBaseURL() - if err != nil { - return err - } - - endpoint := baseURL + "/v1.0/me/photo/$value" - request, err := http.NewRequestWithContext(ctx, http.MethodPut, endpoint, bytes.NewReader(photo)) - if err != nil { - return err - } - request.Header.Set(revactx.TokenHeader, token) - request.Header.Set("Content-Type", http.DetectContentType(photo)) - - client := m.backendHTTPClient - if client == nil { - client = http.DefaultClient - } - resp, err := client.Do(request) - if err != nil { - return err - } - defer resp.Body.Close() - - if resp.StatusCode < http.StatusOK || resp.StatusCode >= http.StatusMultipleChoices { - body, _ := io.ReadAll(io.LimitReader(resp.Body, 2048)) - return fmt.Errorf("graph profile photo update failed: %s (%s)", resp.Status, strings.TrimSpace(string(body))) - } - - return nil -} - -func (m accountResolver) graphBaseURL() (string, error) { - if m.serviceSelector == nil { - return "", errors.New("service selector not configured") - } - selectNext, err := m.serviceSelector.Select(graphServiceName) - if err != nil { - return "", err - } - node, err := selectNext() - if err != nil { - return "", err - } - scheme := node.Metadata["protocol"] - if node.Metadata["use_tls"] == "true" { - scheme = "https" - } - if scheme == "" { - scheme = "http" - } - return fmt.Sprintf("%s://%s/graph", scheme, node.Address), nil + return pictureURL } // TODO do not use the context to store values: https://medium.com/@cep21/how-to-correctly-use-context-context-in-go-1-7-8f2c0fafdf39 @@ -401,12 +233,6 @@ func (m accountResolver) ServeHTTP(w http.ResponseWriter, req *http.Request) { } } - if m.autoProvisionClaims.ProfilePicture != "" && oidc.NewSessionFlagFromContext(ctx) { - if err := m.syncProfilePicture(ctx, req, user, token, claims); err != nil { - m.logger.Warn().Err(err).Str("userid", user.GetId().GetOpaqueId()).Msg("Failed to sync profile picture from OIDC claim") - } - } - // resolve the user's roles user, err = m.userRoleAssigner.UpdateUserRoleAssignment(ctx, user, claims) if err != nil { @@ -417,9 +243,11 @@ func (m accountResolver) ServeHTTP(w http.ResponseWriter, req *http.Request) { // If this is a new session, publish user login event if newSession := oidc.NewSessionFlagFromContext(ctx); newSession && m.eventsPublisher != nil { + pictureURL := m.readProfilePictureURL(claims) event := events.UserSignedIn{ - Executant: user.Id, - Timestamp: utils.TimeToTS(time.Now()), + Executant: user.Id, + PictureURL: pictureURL, + Timestamp: utils.TimeToTS(time.Now()), } if err := events.Publish(req.Context(), m.eventsPublisher, event); err != nil { m.logger.Error().Err(err).Msg("could not publish user signin event.") diff --git a/services/proxy/pkg/middleware/options.go b/services/proxy/pkg/middleware/options.go index 005241b842..f74de68b30 100644 --- a/services/proxy/pkg/middleware/options.go +++ b/services/proxy/pkg/middleware/options.go @@ -14,7 +14,6 @@ import ( "github.com/opencloud-eu/opencloud/services/proxy/pkg/userroles" "github.com/opencloud-eu/reva/v2/pkg/events" "github.com/opencloud-eu/reva/v2/pkg/rgrpc/todo/pool" - "go-micro.dev/v4/selector" "go-micro.dev/v4/store" "go.opentelemetry.io/otel/trace" ) @@ -30,8 +29,6 @@ type Options struct { PolicySelector config.PolicySelector // HTTPClient to use for communication with the oidcAuth provider HTTPClient *http.Client - // BackendHTTPClient to use for internal backend HTTP calls - BackendHTTPClient *http.Client // UserProvider backend to use for resolving User UserProvider backend.UserBackend // UserRoleAssigner to user for assign a users default role @@ -46,8 +43,6 @@ type Options struct { OIDCIss string // RevaGatewaySelector to send requests to the reva gateway RevaGatewaySelector pool.Selectable[gateway.GatewayAPIClient] - // ServiceSelector to resolve internal HTTP services - ServiceSelector selector.Selector // PreSignedURLConfig to configure the middleware PreSignedURLConfig config.PreSignedURL // UserOIDCClaim to read from the oidc claims @@ -87,8 +82,8 @@ type Options struct { // tenant ID in the OIDC claims via the gateway's TenantAPI before comparing it to the user's stored tenant ID. TenantIDMappingEnabled bool // ServiceAccount holds credentials used to authenticate internal service calls (e.g. TenantAPI lookups). - ServiceAccount config.ServiceAccount - EventsPublisher events.Publisher + ServiceAccount config.ServiceAccount + EventsPublisher events.Publisher } // newOptions initializes the available default options. @@ -123,13 +118,6 @@ func HTTPClient(c *http.Client) Option { } } -// BackendHTTPClient provides a function to set the backend http client config option. -func BackendHTTPClient(c *http.Client) Option { - return func(o *Options) { - o.BackendHTTPClient = c - } -} - // SettingsRoleService provides a function to set the role service option. func SettingsRoleService(rc settingssvc.RoleService) Option { return func(o *Options) { @@ -172,13 +160,6 @@ func WithRevaGatewaySelector(val pool.Selectable[gateway.GatewayAPIClient]) Opti } } -// ServiceSelector provides a function to set the internal service selector option. -func ServiceSelector(val selector.Selector) Option { - return func(o *Options) { - o.ServiceSelector = val - } -} - // PreSignedURLConfig provides a function to set the PreSignedURL config func PreSignedURLConfig(cfg config.PreSignedURL) Option { return func(o *Options) { diff --git a/vendor/github.com/opencloud-eu/reva/v2/pkg/events/users.go b/vendor/github.com/opencloud-eu/reva/v2/pkg/events/users.go index 2d7bc812fb..4f1ba8fb3e 100644 --- a/vendor/github.com/opencloud-eu/reva/v2/pkg/events/users.go +++ b/vendor/github.com/opencloud-eu/reva/v2/pkg/events/users.go @@ -122,8 +122,9 @@ func (BackchannelLogout) Unmarshal(v []byte) (interface{}, error) { // UserSignedIn is emitted when a user signs in type UserSignedIn struct { - Executant *user.UserId - Timestamp *types.Timestamp + Executant *user.UserId + PictureURL string `json:",omitempty"` + Timestamp *types.Timestamp } // Unmarshal to fulfill umarshaller interface