Repository: dexidp/dex (go)

Issue:
Authproxy should have a separate header for username and preferred_username
### Preflight Checklist

- [X] I agree to follow the [Code of Conduct](https://github.com/dexidp/dex/blob/master/.github/CODE_OF_CONDUCT.md) that this project adheres to.
- [X] I have searched the [issue tracker](https://www.github.com/dexidp/dex/issues) for an issue that matches the one I want to file, without success.

### Problem Description

According to https://openid.net/specs/openid-connect-core-1_0.html#StandardClaims, user name is "End-User's full name in displayable form" (display name), and preferred_username is "Shorthand name by which the End-User wishes to be referred to at the RP, such as janedoe or j.doe. This value MAY be any valid JSON string including special characters such as @, /, or whitespace."

Even though preferred username can have special characters, some implementations don't handle this very well, and they have semantically different meanings (username vs display name).

### Proposed Solution

The [authproxy connector](https://github.com/dexidp/dex/blob/master/connector/authproxy/authproxy.go) should have separate headers for specifying these values, such as a `X-Remote-User-Name` header in addition to the existing `X-Remote-User`.

### Alternatives Considered

_No response_

### Additional Information

_No response_

Pull request description:
Add authproxy preferred_username header
<!--
Thank you for sending a pull request! Here are some tips for contributors:

1. Fill the description template below.
2. Sign a DCO (if you haven't already signed it).
3. Include appropriate tests (if necessary). Make sure that all CI checks passed.
4. If the Pull Request is a work in progress, make use of GitHub's "Draft PR" feature and mark it as such.
-->

#### Overview

Allows proxies to pass `X-Remote-User-Name` header to set preferred username. Also adds tests.
Perhaps authproxy should be extended to allow any claims, instead of adding headers for preferred username, picture, etc? But that's out of scope for this PR.

#### What this PR does / why we need it

<!--
- Please state in detail why we need this PR and what it solves.
- If your PR closes some of the existing issues, please add links to them here.
  Mentioned issues will be automatically closed.
  Usage: "Closes #<issue number>", or "Closes (paste link of issue)"
-->
Resolves #3876.

#### Special notes for your reviewer
Defaults to the value of `X-Remote-User` ([user name](https://openid.net/specs/openid-connect-core-1_0.html#StandardClaims)), to preserve the existing default. But we may want to change this because preferred username = shorthand, and user name = display name.

Patch:
diff --git a/connector/authproxy/authproxy.go b/connector/authproxy/authproxy.go
index 61353382..2419d3b7 100644
--- a/connector/authproxy/authproxy.go
+++ b/connector/authproxy/authproxy.go
@@ -19,11 +19,12 @@ import (
 // Headers retrieved to fetch user's email and group can be configured
 // with userHeader and groupHeader.
 type Config struct {
-	UserIDHeader string   `json:"userIDHeader"`
-	UserHeader   string   `json:"userHeader"`
-	EmailHeader  string   `json:"emailHeader"`
-	GroupHeader  string   `json:"groupHeader"`
-	Groups       []string `json:"staticGroups"`
+	UserIDHeader   string   `json:"userIDHeader"`
+	UserHeader     string   `json:"userHeader"`
+	UserNameHeader string   `json:"userNameHeader"`
+	EmailHeader    string   `json:"emailHeader"`
+	GroupHeader    string   `json:"groupHeader"`
+	Groups         []string `json:"staticGroups"`
 }
 
 // Open returns an authentication strategy which requires no user interaction.
@@ -36,6 +37,10 @@ func (c *Config) Open(id string, logger *slog.Logger) (connector.Connector, erro
 	if userHeader == "" {
 		userHeader = "X-Remote-User"
 	}
+	userNameHeader := c.UserNameHeader
+	if userNameHeader == "" {
+		userNameHeader = "X-Remote-User-Name"
+	}
 	emailHeader := c.EmailHeader
 	if emailHeader == "" {
 		emailHeader = "X-Remote-User-Email"
@@ -46,26 +51,28 @@ func (c *Config) Open(id string, logger *slog.Logger) (connector.Connector, erro
 	}
 
 	return &callback{
-		userIDHeader: userIDHeader,
-		userHeader:   userHeader,
-		emailHeader:  emailHeader,
-		groupHeader:  groupHeader,
-		groups:       c.Groups,
-		logger:       logger.With(slog.Group("connector", "type", "authproxy", "id", id)),
-		pathSuffix:   "/" + id,
+		userIDHeader:   userIDHeader,
+		userHeader:     userHeader,
+		userNameHeader: userNameHeader,
+		emailHeader:    emailHeader,
+		groupHeader:    groupHeader,
+		groups:         c.Groups,
+		logger:         logger.With(slog.Group("connector", "type", "authproxy", "id", id)),
+		pathSuffix:     "/" + id,
 	}, nil
 }
 
 // Callback is a connector which returns an identity with the HTTP header
 // X-Remote-User as verified email.
 type callback struct {
-	userIDHeader string
-	userHeader   string
-	emailHeader  string
-	groupHeader  string
-	groups       []string
-	logger       *slog.Logger
-	pathSuffix   string
+	userIDHeader   string
+	userNameHeader string
+	userHeader     string
+	emailHeader    string
+	groupHeader    string
+	groups         []string
+	logger         *slog.Logger
+	pathSuffix     string
 }
 
 // LoginURL returns the URL to redirect the user to login with.
@@ -87,6 +94,10 @@ func (m *callback) HandleCallback(s connector.Scopes, r *http.Request) (connecto
 	if remoteUser == "" {
 		return connector.Identity{}, fmt.Errorf("required HTTP header %s is not set", m.userHeader)
 	}
+	remoteUserName := r.Header.Get(m.userNameHeader)
+	if remoteUserName == "" {
+		remoteUserName = remoteUser
+	}
 	remoteUserID := r.Header.Get(m.userIDHeader)
 	if remoteUserID == "" {
 		remoteUserID = remoteUser
@@ -107,7 +118,7 @@ func (m *callback) HandleCallback(s connector.Scopes, r *http.Request) (connecto
 	return connector.Identity{
 		UserID:            remoteUserID,
 		Username:          remoteUser,
-		PreferredUsername: remoteUser,
+		PreferredUsername: remoteUserName,
 		Email:             remoteUserEmail,
 		EmailVerified:     true,
 		Groups:            groups,


Test patch:
diff --git a/connector/authproxy/authproxy_test.go b/connector/authproxy/authproxy_test.go
index fdcf4038..fb468ea1 100644
--- a/connector/authproxy/authproxy_test.go
+++ b/connector/authproxy/authproxy_test.go
@@ -11,15 +11,16 @@ import (
 )
 
 const (
-	testEmail        = "testuser@example.com"
-	testGroup1       = "group1"
-	testGroup2       = "group2"
-	testGroup3       = "group 3"
-	testGroup4       = "group 4"
-	testStaticGroup1 = "static1"
-	testStaticGroup2 = "static 2"
-	testUsername     = "testuser"
-	testUserID       = "1234567890"
+	testEmail             = "testuser@example.com"
+	testGroup1            = "group1"
+	testGroup2            = "group2"
+	testGroup3            = "group 3"
+	testGroup4            = "group 4"
+	testStaticGroup1      = "static1"
+	testStaticGroup2      = "static 2"
+	testUsername          = "Test User"
+	testPreferredUsername = "testuser"
+	testUserID            = "1234567890"
 )
 
 var logger = slog.New(slog.NewTextHandler(io.Discard, &slog.HandlerOptions{}))
@@ -49,16 +50,18 @@ func TestUser(t *testing.T) {
 
 func TestExtraHeaders(t *testing.T) {
 	config := Config{
-		UserIDHeader: "X-Remote-User-Id",
-		UserHeader:   "X-Remote-User",
-		EmailHeader:  "X-Remote-User-Email",
+		UserIDHeader:   "X-Remote-User-Id",
+		UserHeader:     "X-Remote-User",
+		UserNameHeader: "X-Remote-User-Name",
+		EmailHeader:    "X-Remote-User-Email",
 	}
 	conn := callback{
-		userHeader:   config.UserHeader,
-		userIDHeader: config.UserIDHeader,
-		emailHeader:  config.EmailHeader,
-		logger:       logger,
-		pathSuffix:   "/test",
+		userHeader:     config.UserHeader,
+		userIDHeader:   config.UserIDHeader,
+		userNameHeader: config.UserNameHeader,
+		emailHeader:    config.EmailHeader,
+		logger:         logger,
+		pathSuffix:     "/test",
 	}
 
 	req, err := http.NewRequest("GET", "/", nil)
@@ -66,6 +69,7 @@ func TestExtraHeaders(t *testing.T) {
 	req.Header = map[string][]string{
 		"X-Remote-User-Id":    {testUserID},
 		"X-Remote-User":       {testUsername},
+		"X-Remote-User-Name":  {testPreferredUsername},
 		"X-Remote-User-Email": {testEmail},
 	}
 
@@ -73,7 +77,7 @@ func TestExtraHeaders(t *testing.T) {
 	expectNil(t, err)
 
 	expectEquals(t, ident.UserID, testUserID)
-	expectEquals(t, ident.PreferredUsername, testUsername)
+	expectEquals(t, ident.PreferredUsername, testPreferredUsername)
 	expectEquals(t, ident.Username, testUsername)
 	expectEquals(t, ident.Email, testEmail)
 	expectEquals(t, len(ident.Groups), 0)