Feat: Pull users' Profile Picture from the OIDC claims by Guibi1 · Pull Request #2704 · opencloud-eu/opencloud

Guibi1 · 2026-05-02T20:57:43Z

Description

This PR adds two settings:

PROXY_OIDC_PROFILE_PICTURE_CLAIM: Sets which oidc claim to use as the profile picture.
PROXY_OIDC_PROFILE_PICTURE_DISABLE_LOCAL_CHANGES: Stops the user from updating their profile pictures by disabling the graph endpoint.

Related Issue

Fixes [FR] Tell Opencloud OIDC where to find the Profile Picture #1499

Motivation and Context

See linked issue.

How Has This Been Tested?

manual testing

Screenshots (if appropriate):

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)
Technical debt
Tests only (no source changes)

Checklist:

Code changes
Unit tests added
Acceptance tests added
Documentation added

Copilot

Pull request overview

This PR adds support in the proxy service to (a) sync a user’s profile photo from a configurable OIDC claim on login, and (b) optionally forbid local profile-photo updates via the Graph “me/photo/$value” endpoint for OIDC-authenticated requests.

Changes:

Add PROXY_OIDC_PROFILE_PICTURE_CLAIM to fetch a profile-photo URL from OIDC claims and sync it on new sessions.
Add PROXY_OIDC_PROFILE_PICTURE_DISABLE_LOCAL_CHANGES to block PUT/PATCH/DELETE to /graph/v1.0/me/photo/$value for OIDC-authenticated requests.
Introduce proxy wiring for internal Graph calls (service discovery + separate backend HTTP client).

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
services/proxy/pkg/middleware/options.go	Extends middleware options with backend HTTP client, internal service selector, and OIDC profile-picture config.
services/proxy/pkg/middleware/account_resolver.go	Implements profile-picture fetch + Graph update on new session; optionally blocks local photo mutations.
services/proxy/pkg/config/defaults/defaultconfig.go	Adds default values for the new `oidc_profile_picture` config section.
services/proxy/pkg/config/config.go	Introduces `OIDCProfilePicture` config and env vars for claim + local-change disable.
services/proxy/pkg/command/server.go	Wires new config into middleware and creates a dedicated backend HTTP client for internal calls.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+	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)


+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)


+	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 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")
+			}
+		}


+	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)."`


+	backendHTTPClient := &http.Client{
+		Transport: &http.Transport{
+			TLSClientConfig: backendTLSConfig,
+		},
+		Timeout: time.Second * 10,
+	}


dschmidt · 2026-05-08T09:41:28Z

First of all: Thanks for the effort, @Guibi1!

I think this might entangle two separate concerns too tightly:

Use profile pictures from oidc claims
Disable profile picture updates for the user

I have a feeling the proxy is not the right place to handle the second concern. To me the route handling is crossing service domains in an unfortunate way.
IMHO it's the graph service which should be responsible for ultimately deciding this - and the frontend should be made aware of this, to hide the upload option etc.

When moving this responsibility to the graph service, this leaves us with the question of how to update the profile picture from the proxy (handling it at all there seems to be the right call as the proxy deals with all claims anyway), as the Graph HTTP Api now rejects updates.

Two possibilities from my view:

Emit an event in the proxy and let the graph service consume it. This is rather easy and auditable, but it loses user auth. Again two possibilities:
1.1 attach the image data to the event, limited by event bus config and feels a bit awkward
1.2 have no user auth for fetching the image (is this really needed in reality? I haven't seen this in the wild anyway)
Add a gRPC interface to the graph service: less auditable, but we can pass on user auth or bigger image data

It might be easier to discuss separate concerns in separate PRs - unfortunately they are heavily influencing each other...

Guibi1 · 2026-05-12T02:37:25Z

Agreed! For simplicity (and because i really want this merged), I straight up removed the second concern.
However, using a gRPC interface sounds like the best option, as long as there is already one on the graph service, it would be weird to have one just for this.

sonarqubecloud · 2026-05-12T02:38:49Z

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

dschmidt · 2026-05-12T05:06:45Z

afaik there is none - personally I don't think it's too bad adding it, we're not coming up with a new pattern in this code base.

But ultimately not up for me to decide.

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 9 comments.

+	// OIDCProfilePicture config for syncing profile pictures from OIDC claims
+	OIDCProfilePicture config.OIDCProfilePicture


+			multiTenantEnabled:               options.MultiTenantEnabled,
+			lastGroupSyncCache:               lastGroupSyncCache,
+			tenantIDCache:                    tenantIDCache,
+			eventsPublisher:                  options.EventsPublisher,
+			profilePictureClaim:              options.AutoProvisionClaims.ProfilePicture,
+			httpClient:                       httpClient,
+			backendHTTPClient:                backendHTTPClient,
+			oidcIssuer:                       options.OIDCIss,
+			serviceSelector:                  options.ServiceSelector,


+			middleware.BackendHTTPClient(backendHTTPClient),
+			middleware.OIDCIss(cfg.OIDC.Issuer),
+			middleware.ServiceSelector(serviceSelector),
+			middleware.OIDCProfilePicture(cfg.OIDCProfilePicture),


 	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"`


 	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."`


+	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")
+	}


+	issuerURL, err := url.Parse(m.oidcIssuer)
+	if err != nil || issuerURL.Host == "" {
+		return false
+	}
+	return strings.EqualFold(issuerURL.Host, pictureURL.Host)


+	backendHTTPClient := &http.Client{
+		Transport: &http.Transport{
+			TLSClientConfig: backendTLSConfig,
+		},
+		Timeout: time.Second * 10,


+		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")
+			}
+		}


codacy-production · 2026-05-12T07:01:06Z

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 217 complexity · 28 duplication

Metric Results

Complexity 217

Duplication 28

View in Codacy

AI Reviewer: first review requested successfully. AI can make mistakes. Always validate suggestions.

_{TIP This summary will be updated as you push new changes.}

codacy-production

Pull Request Overview

This PR cannot be merged in its current state due to high-severity logic and security issues.

Critical Findings

Compilation Errors: The code references options.AutoProvisionClaims and cfg.OIDCProfilePicture, neither of which exists in the provided configuration or middleware structs.
Security Risk: The fetchProfilePicture function lacks host validation, exposing the system to SSRF (Server-Side Request Forgery) by allowing requests to arbitrary URLs provided in OIDC claims.
Requirement Gaps: The setting PROXY_OIDC_PROFILE_PICTURE_DISABLE_LOCAL_CHANGES promised in the description is not implemented in the code.
Performance Concerns: Synchronizing profile pictures is handled synchronously within the middleware, which will block the login flow and increase latency for users during their first request.

About this PR

The account resolver middleware has high complexity and significant new logic for fetching and updating profile pictures, yet no unit tests or integration tests have been provided. This is particularly concerning given the identified logic errors.
There is a significant naming mismatch between the PR description ('PROXY_OIDC_PROFILE_PICTURE_CLAIM') and the code ('PROXY_AUTOPROVISION_CLAIM_PROFILE_PICTURE'). Furthermore, the OIDCProfilePicture type used in options.go and server.go appears to be undefined or incorrectly replaced by a string field in the configuration diff.

Test suggestions

Missing: Successful profile picture sync on new session with valid claim and URL
Missing: Skip profile picture sync if the session is not a new OIDC session
Missing: Reject profile picture sync if the URL scheme is not http or https
Missing: Reject profile picture if the response size exceeds 10MB limit
Missing: Verify Authorization header is only attached to image fetch requests if the host matches the OIDC issuer
Missing: Verify Graph API update request includes the correct CS3 token and content type
Missing: Verify local changes are disabled when PROXY_OIDC_PROFILE_PICTURE_DISABLE_LOCAL_CHANGES is true

Prompt proposal for missing tests

Consider implementing these tests if applicable:
1. Missing: Successful profile picture sync on new session with valid claim and URL
2. Missing: Skip profile picture sync if the session is not a new OIDC session
3. Missing: Reject profile picture sync if the URL scheme is not http or https
4. Missing: Reject profile picture if the response size exceeds 10MB limit
5. Missing: Verify Authorization header is only attached to image fetch requests if the host matches the OIDC issuer
6. Missing: Verify Graph API update request includes the correct CS3 token and content type
7. Missing: Verify local changes are disabled when PROXY_OIDC_PROFILE_PICTURE_DISABLE_LOCAL_CHANGES is true

_{TIP Improve review quality by adding custom instructions}
_{TIP How was this review? Give us feedback}

codacy-production · 2026-05-12T07:02:48Z

+		client = http.DefaultClient
+	}
+
+	request, err := http.NewRequestWithContext(ctx, http.MethodGet, pictureURL.String(), nil)


_{🔴 HIGH RISK}

Fetching the profile picture from an arbitrary URL provided in OIDC claims poses a SSRF risk. You should validate that the URL's host matches the OIDC issuer or an allowed list of trusted domains to prevent internal network scanning.

codacy-production · 2026-05-12T07:02:48Z

 )

+const (
+	graphServiceName      = "eu.opencloud.web.graph"


_{🔴 HIGH RISK}

The requirement 'PROXY_OIDC_PROFILE_PICTURE_DISABLE_LOCAL_CHANGES' from the PR description is not implemented. Logic to intercept or disable the graph endpoint based on this setting is missing.

codacy-production · 2026-05-12T07:02:48Z

+			middleware.BackendHTTPClient(backendHTTPClient),
+			middleware.OIDCIss(cfg.OIDC.Issuer),
+			middleware.ServiceSelector(serviceSelector),
+			middleware.OIDCProfilePicture(cfg.OIDCProfilePicture),


_{🔴 HIGH RISK}

Compilation error: cfg.OIDCProfilePicture is not defined. According to the changes in config.go, this should be cfg.AutoProvisionClaims.ProfilePicture.

codacy-production · 2026-05-12T07:02:49Z

+			lastGroupSyncCache:               lastGroupSyncCache,
+			tenantIDCache:                    tenantIDCache,
+			eventsPublisher:                  options.EventsPublisher,
+			profilePictureClaim:              options.AutoProvisionClaims.ProfilePicture,


_{🔴 HIGH RISK}

Compilation error: The field 'AutoProvisionClaims' is not defined on the 'Options' struct. The middleware should likely use the 'OIDCProfilePicture' config field directly as defined in options.go.

codacy-production · 2026-05-12T07:02:49Z

 	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."`


_{🟡 MEDIUM RISK}

The environment variable name contradicts the PR description. Please use 'PROXY_OIDC_PROFILE_PICTURE_CLAIM' for consistency with the documentation.

codacy-production · 2026-05-12T07:02:49Z


+const (
+	graphServiceName      = "eu.opencloud.web.graph"
+	maxProfilePhotoBytes  = 10 << 20


_{🟡 MEDIUM RISK}

Suggestion: The 10MB limit for profile photos is excessive for a synchronous automated sync process. Consider reducing this to 1-2MB to prevent high memory usage and mitigate resource exhaustion risks during peak login times.

codacy-production · 2026-05-12T07:02:49Z

 		}

+		if m.profilePictureClaim != "" && oidc.NewSessionFlagFromContext(ctx) {
+			if err := m.syncProfilePicture(ctx, req, user, token, claims); err != nil {


_{🟡 MEDIUM RISK}

Suggestion: Syncing the profile picture synchronously blocks the request flow and increases latency for the initial session request. Consider running this in a background goroutine using a detached context (e.g., context.WithoutCancel).

codacy-production · 2026-05-12T07:02:49Z

+	}
+	if cfg.BackendHTTPSCACert != "" {
+		certs := x509.NewCertPool()
+		pemData, err := os.ReadFile(cfg.BackendHTTPSCACert)


_{⚪ LOW RISK}

This block for loading CA certificates from a file is duplicated from the OIDC certificate loading logic. Refactoring this into a shared helper function would improve maintainability.

See Clone in Codacy

codacy-production · 2026-05-12T07:02:49Z

+	if len(data) > maxProfilePhotoBytes {
+		return nil, fmt.Errorf("profile picture exceeds %d bytes", maxProfilePhotoBytes)
+	}
+	contentType := http.DetectContentType(data)


_{⚪ LOW RISK}

Suggestion: The content-type is detected twice for the same image data. Optimize this by returning the detected content-type from fetchProfilePicture and passing it to updateGraphProfilePhoto.

codacy-production

Pull Request Overview

This PR introduces OIDC-based profile picture synchronization and the ability to disable local profile updates. While the implementation aligns with the stated goals via new configuration flags, there is a total lack of automated test coverage for these features.

Codacy analysis indicates the PR is technically up to standards regarding code style; however, the account_resolver.go and options.go files have seen a large increase in complexity (101 and 61 respectively) without any new tests to mitigate regression risk. It is recommended to address the missing test scenarios before merging.

About this PR

This PR lacks unit and acceptance tests for the core logic. Specifically, the mapping of the OIDC claim to the user profile and the logic to block graph API updates remain unverified. Automated tests are required to ensure the OIDC claim parsing handles various formats (URL vs. binary) correctly.
Multiple files (account_resolver.go, options.go, server.go, defaultconfig.go) have undergone significant complexity increases without being covered by tests. This creates a high risk of long-term maintenance issues and hidden bugs in the proxy middleware.

Test suggestions

Verify that the profile picture is correctly mapped from the OIDC claim specified in PROXY_OIDC_PROFILE_PICTURE_CLAIM.
Verify that attempts to update the profile picture via the graph endpoint return an error or are blocked when PROXY_OIDC_PROFILE_PICTURE_DISABLE_LOCAL_CHANGES is true.
Unit tests for new logic in services/proxy/pkg/middleware/account_resolver.go (Complexity: 101, Coverage: 0%).
Unit tests for new logic in services/proxy/pkg/middleware/options.go (Complexity: 61, Coverage: 0%).
Unit tests for new logic in services/proxy/pkg/command/server.go (Complexity: 28, Coverage: 0%).
Unit tests for new logic in services/proxy/pkg/config/defaults/defaultconfig.go (Complexity: 27, Coverage: 0%).

Prompt proposal for missing tests

Consider implementing these tests if applicable:
1. Verify that the profile picture is correctly mapped from the OIDC claim specified in PROXY_OIDC_PROFILE_PICTURE_CLAIM.
2. Verify that attempts to update the profile picture via the graph endpoint return an error or are blocked when PROXY_OIDC_PROFILE_PICTURE_DISABLE_LOCAL_CHANGES is true.
3. Unit tests for new logic in services/proxy/pkg/middleware/account_resolver.go (Complexity: 101, Coverage: 0%).
4. Unit tests for new logic in services/proxy/pkg/middleware/options.go (Complexity: 61, Coverage: 0%).
5. Unit tests for new logic in services/proxy/pkg/command/server.go (Complexity: 28, Coverage: 0%).
6. Unit tests for new logic in services/proxy/pkg/config/defaults/defaultconfig.go (Complexity: 27, Coverage: 0%).

_{TIP Improve review quality by adding custom instructions}
_{TIP How was this review? Give us feedback}

Feat: OIDC profile picture

72f83ba

dschmidt requested a review from Copilot May 8, 2026 08:23

Copilot started reviewing on behalf of dschmidt May 8, 2026 08:24 View session

Copilot AI reviewed May 8, 2026

View reviewed changes

Removed code to block profile picture updates

1c13bc0

dschmidt requested a review from Copilot May 12, 2026 05:02

Copilot started reviewing on behalf of dschmidt May 12, 2026 05:03 View session

Copilot AI reviewed May 12, 2026

View reviewed changes

codacy-production Bot reviewed May 12, 2026

View reviewed changes

		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)."`

		// OIDCProfilePicture config for syncing profile pictures from OIDC claims
		OIDCProfilePicture config.OIDCProfilePicture

Conversation

Guibi1 commented May 2, 2026

Description

Related Issue

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

Checklist:

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

dschmidt commented May 8, 2026

Uh oh!

Guibi1 commented May 12, 2026

Uh oh!

sonarqubecloud Bot commented May 12, 2026

Quality Gate passed

Uh oh!

dschmidt commented May 12, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

codacy-production Bot commented May 12, 2026

Up to standards ✅

Uh oh!

codacy-production Bot left a comment

Choose a reason for hiding this comment

Pull Request Overview

Critical Findings

About this PR

Test suggestions

Uh oh!

codacy-production Bot May 12, 2026

Choose a reason for hiding this comment

Uh oh!

codacy-production Bot May 12, 2026

Choose a reason for hiding this comment

Uh oh!

codacy-production Bot May 12, 2026

Choose a reason for hiding this comment

Uh oh!

codacy-production Bot May 12, 2026

Choose a reason for hiding this comment

Uh oh!

codacy-production Bot May 12, 2026

Choose a reason for hiding this comment

Uh oh!

codacy-production Bot May 12, 2026

Choose a reason for hiding this comment

Uh oh!

codacy-production Bot May 12, 2026

Choose a reason for hiding this comment

Uh oh!

codacy-production Bot May 12, 2026

Choose a reason for hiding this comment

Uh oh!

codacy-production Bot May 12, 2026

Choose a reason for hiding this comment

Uh oh!

codacy-production Bot left a comment

Choose a reason for hiding this comment

Pull Request Overview

About this PR

Test suggestions

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants