feat(server): pass authenticated userId as header to extensions (#24356)

Signed-off-by: Alexandre Gaudreault <[email protected]>
Co-authored-by: Michael Crenshaw <[email protected]>

Author: agaudreault

docs/developer-guide/extensions/proxy-extensions.md

@@ -1,7 +1,8 @@
 # Proxy Extensions
 
 !!! warning "Beta Feature (Since 2.7.0)"
-    This feature is in the [Beta](https://github.com/argoproj/argoproj/blob/main/community/feature-status.md#beta) stage. 
+
+    This feature is in the [Beta](https://github.com/argoproj/argoproj/blob/main/community/feature-status.md#beta) stage.
     It is generally considered stable, but there may be unhandled edge cases.
 
 ## Overview
@@ -29,7 +30,7 @@ metadata:
   name: argocd-cmd-params-cm
   namespace: argocd
 data:
-  server.enable.proxy.extension: "true"
+  server.enable.proxy.extension: 'true'
 ```
 
 Once the proxy extension is enabled, it can be configured in the main
@@ -102,11 +103,12 @@ respect the new configuration.
 
 Every configuration entry is explained below:
 
-#### `extensions` (*list*)
+#### `extensions` (_list_)
 
 Defines configurations for all extensions enabled.
 
-#### `extensions.name` (*string*)
+#### `extensions.name` (_string_)
+
 (mandatory)
 
 Defines the endpoint that will be used to register the extension
@@ -116,54 +118,61 @@ following url:
 
     <argocd-host>/extensions/my-extension
 
-#### `extensions.backend.connectionTimeout` (*duration string*)
+#### `extensions.backend.connectionTimeout` (_duration string_)
+
 (optional. Default: 2s)
 
 Is the maximum amount of time a dial to the extension server will wait
-for a connect to complete. 
+for a connect to complete.
+
+#### `extensions.backend.keepAlive` (_duration string_)
 
-#### `extensions.backend.keepAlive` (*duration string*)
 (optional. Default: 15s)
 
 Specifies the interval between keep-alive probes for an active network
 connection between the API server and the extension server.
 
-#### `extensions.backend.idleConnectionTimeout` (*duration string*)
+#### `extensions.backend.idleConnectionTimeout` (_duration string_)
+
 (optional. Default: 60s)
 
 Is the maximum amount of time an idle (keep-alive) connection between
 the API server and the extension server will remain idle before
 closing itself.
 
-#### `extensions.backend.maxIdleConnections` (*int*)
+#### `extensions.backend.maxIdleConnections` (_int_)
+
 (optional. Default: 30)
 
 Controls the maximum number of idle (keep-alive) connections between
 the API server and the extension server.
 
-#### `extensions.backend.services` (*list*)
+#### `extensions.backend.services` (_list_)
 
 Defines a list with backend url by cluster.
 
-#### `extensions.backend.services.url` (*string*)
+#### `extensions.backend.services.url` (_string_)
+
 (mandatory)
 
 Is the address where the extension backend must be available.
 
-#### `extensions.backend.services.headers` (*list*)
+#### `extensions.backend.services.headers` (_list_)
 
 If provided, the headers list will be added on all outgoing requests
 for this service config. Existing headers in the incoming request with
 the same name will be overridden by the one in this list. Reserved header
 names will be ignored (see the [headers](#incoming-request-headers) below).
 
-#### `extensions.backend.services.headers.name` (*string*)
+#### `extensions.backend.services.headers.name` (_string_)
+
 (mandatory)
 
 Defines the name of the header. It is a mandatory field if a header is
 provided.
 
-#### `extensions.backend.services.headers.value` (*string*)
+#### `extensions.backend.services.headers.value` (_string_)
+
 (mandatory)
 
 Defines the value of the header. It is a mandatory field if a header is
@@ -178,7 +187,8 @@ Example:
 In the example above, the value will be replaced with the one from
 the argocd-secret with key 'some.argocd.secret.key'.
 
-#### `extensions.backend.services.cluster` (*object*)
+#### `extensions.backend.services.cluster` (_object_)
+
 (optional)
 
 If provided, and multiple services are configured, will have to match
@@ -190,17 +200,19 @@ send requests to the proper backend service. If only one backend
 service is configured, this field is ignored, and all requests are
 forwarded to the configured one.
 
-#### `extensions.backend.services.cluster.name` (*string*)
+#### `extensions.backend.services.cluster.name` (_string_)
+
 (optional)
 
 It will be matched with the value from
 `Application.Spec.Destination.Name`
 
-#### `extensions.backend.services.cluster.server` (*string*)
+#### `extensions.backend.services.cluster.server` (_string_)
+
 (optional)
 
 It will be matched with the value from
-`Application.Spec.Destination.Server`. 
+`Application.Spec.Destination.Server`.
 
 ## Usage
 
@@ -245,7 +257,7 @@ Argo CD UI keeps the authentication token stored in a cookie
 (`argocd.token`). This value needs to be sent in the `Cookie` header
 so the API server can validate its authenticity.
 
-Example: 
+Example:
 
     Cookie: argocd.token=eyJhbGciOiJIUzI1Ni...
 
@@ -299,11 +311,16 @@ section for more details.
 
 #### `Argocd-Username`
 
-Will be populated with the username logged in Argo CD.
+Will be populated with the username logged in Argo CD. This is primarily useful for display purposes. 
+To identify a user for programmatic needs, `Argocd-User-Id` is probably a better choice.
+
+#### `Argocd-User-Id`
+
+Will be populated with the internal user id, most often defined by the `sub` claim, logged in Argo CD.
 
 #### `Argocd-User-Groups`
 
-Will be populated with the 'groups' claim from the user logged in Argo CD.
+Will be populated with the configured RBAC scopes, most often the `groups` claim, from the user logged in Argo CD.
 
 ### Multi Backend Use-Case
 

server/extension/extension.go

@@ -74,10 +74,14 @@ const (
 	// handler.
 	HeaderArgoCDTargetClusterName = "Argocd-Target-Cluster-Name"
 
-	// HeaderArgoCDUsername is the header name that defines the logged
+	// HeaderArgoCDUsername is the header name that defines the username of the logged
 	// in user authenticated by Argo CD.
 	HeaderArgoCDUsername = "Argocd-Username"
 
+	// HeaderArgoCDUserId is the header name that defines the internal user id of the logged
+	// in user authenticated by Argo CD.
+	HeaderArgoCDUserId = "Argocd-User-Id"
+
 	// HeaderArgoCDGroups is the header name that provides the 'groups'
 	// claim from the users authenticated in Argo CD.
 	HeaderArgoCDGroups = "Argocd-User-Groups"
@@ -284,7 +288,8 @@ func (p *DefaultProjectGetter) GetClusters(project string) ([]*v1alpha1.Cluster,
 
 // UserGetter defines the contract to retrieve info from the logged in user.
 type UserGetter interface {
-	GetUser(ctx context.Context) string
+	GetUserId(ctx context.Context) string
+	GetUsername(ctx context.Context) string
 	GetGroups(ctx context.Context) []string
 }
 
@@ -300,11 +305,16 @@ func NewDefaultUserGetter(policyEnf *rbacpolicy.RBACPolicyEnforcer) *DefaultUser
 	}
 }
 
-// GetUser will return the current logged in user
-func (u *DefaultUserGetter) GetUser(ctx context.Context) string {
+// GetUsername will return the username of the current logged in user
+func (u *DefaultUserGetter) GetUsername(ctx context.Context) string {
 	return session.Username(ctx)
 }
 
+// GetUserId will return the user id of the current logged in user
+func (u *DefaultUserGetter) GetUserId(ctx context.Context) string {
+	return session.GetUserIdentifier(ctx)
+}
+
 // GetGroups will return the groups associated with the logged in user.
 func (u *DefaultUserGetter) GetGroups(ctx context.Context) []string {
 	return session.Groups(ctx, u.policyEnf.GetScopes())
@@ -783,11 +793,13 @@ func (m *Manager) CallExtension() func(http.ResponseWriter, *http.Request) {
 			return
 		}
 
-		user := m.userGetter.GetUser(r.Context())
+		userId := m.userGetter.GetUserId(r.Context())
+		username := m.userGetter.GetUsername(r.Context())
 		groups := m.userGetter.GetGroups(r.Context())
-		prepareRequest(r, m.namespace, extName, app, user, groups)
+		prepareRequest(r, m.namespace, extName, app, userId, username, groups)
 		m.log.WithFields(log.Fields{
-			HeaderArgoCDUsername:        user,
+			HeaderArgoCDUserId:          userId,
+			HeaderArgoCDUsername:        username,
 			HeaderArgoCDGroups:          strings.Join(groups, ","),
 			HeaderArgoCDNamespace:       m.namespace,
 			HeaderArgoCDApplicationName: fmt.Sprintf("%s:%s", app.GetNamespace(), app.GetName()),
@@ -819,7 +831,7 @@ func registerMetrics(extName string, metrics httpsnoop.Metrics, extensionMetrics
 //   - Cluster destination name
 //   - Cluster destination server
 //   - Argo CD authenticated username
-func prepareRequest(r *http.Request, namespace string, extName string, app *v1alpha1.Application, username string, groups []string) {
+func prepareRequest(r *http.Request, namespace string, extName string, app *v1alpha1.Application, userId string, username string, groups []string) {
 	r.URL.Path = strings.TrimPrefix(r.URL.Path, fmt.Sprintf("%s/%s", URLPrefix, extName))
 	r.Header.Set(HeaderArgoCDNamespace, namespace)
 	if app.Spec.Destination.Name != "" {
@@ -828,6 +840,9 @@ func prepareRequest(r *http.Request, namespace string, extName string, app *v1al
 	if app.Spec.Destination.Server != "" {
 		r.Header.Set(HeaderArgoCDTargetClusterURL, app.Spec.Destination.Server)
 	}
+	if userId != "" {
+		r.Header.Set(HeaderArgoCDUserId, userId)
+	}
 	if username != "" {
 		r.Header.Set(HeaderArgoCDUsername, username)
 	}

server/extension/extension_test.go

@@ -352,8 +352,9 @@ func TestCallExtension(t *testing.T) {
 		f.rbacMock.On("EnforceErr", mock.Anything, rbac.ResourceExtensions, rbac.ActionInvoke, mock.Anything).Return(extAccessError)
 	}
 
-	withUser := func(f *fixture, username string, groups []string) {
-		f.userMock.On("GetUser", mock.Anything).Return(username)
+	withUser := func(f *fixture, userId string, username string, groups []string) {
+		f.userMock.On("GetUserId", mock.Anything).Return(userId)
+		f.userMock.On("GetUsername", mock.Anything).Return(username)
 		f.userMock.On("GetGroups", mock.Anything).Return(groups)
 	}
 
@@ -411,7 +412,7 @@ func TestCallExtension(t *testing.T) {
 		}))
 		defer backendSrv.Close()
 		withRbac(f, true, true)
-		withUser(f, "some-user", []string{"group1", "group2"})
+		withUser(f, "some-user-id", "some-user", []string{"group1", "group2"})
 		withExtensionConfig(getExtensionConfig(backendEndpoint, backendSrv.URL), f)
 		ts := startTestServer(t, f)
 		defer ts.Close()
@@ -448,6 +449,7 @@ func TestCallExtension(t *testing.T) {
 		assert.Equal(t, clusterURL, resp.Header.Get(extension.HeaderArgoCDTargetClusterURL))
 		assert.Equal(t, "Bearer some-bearer-token", resp.Header.Get("Authorization"))
 		assert.Equal(t, "some-user", resp.Header.Get(extension.HeaderArgoCDUsername))
+		assert.Equal(t, "some-user-id", resp.Header.Get(extension.HeaderArgoCDUserId))
 		assert.Equal(t, "group1,group2", resp.Header.Get(extension.HeaderArgoCDGroups))
 
 		// waitgroup is necessary to make sure assertions aren't executed before
@@ -464,7 +466,7 @@ func TestCallExtension(t *testing.T) {
 		withExtensionConfig(getExtensionConfigString(), f)
 		withRbac(f, true, true)
 		withMetrics(f)
-		withUser(f, "some-user", []string{"group1", "group2"})
+		withUser(f, "some-user-id", "some-user", []string{"group1", "group2"})
 		cluster1Name := "cluster1"
 		f.appGetterMock.On("Get", "namespace", "app-name").Return(getApp(cluster1Name, "", defaultProjectName), nil)
 		withProject(getProjectWithDestinations("project-name", []string{cluster1Name}, []string{"some-url"}), f)
@@ -507,7 +509,7 @@ func TestCallExtension(t *testing.T) {
 		withExtensionConfig(getExtensionConfigWith2Backends(extName, beSrv1.URL, cluster1Name, cluster1URL, beSrv2.URL, cluster2Name, cluster2URL), f)
 		withProject(getProjectWithDestinations("project-name", []string{cluster1Name}, []string{cluster2URL}), f)
 		withMetrics(f)
-		withUser(f, "some-user", []string{"group1", "group2"})
+		withUser(f, "some-user-id", "some-user", []string{"group1", "group2"})
 
 		ts := startTestServer(t, f)
 		defer ts.Close()
@@ -554,7 +556,7 @@ func TestCallExtension(t *testing.T) {
 		withRbac(f, allowApp, allowExtension)
 		withExtensionConfig(getExtensionConfig(extName, "http://fake"), f)
 		withMetrics(f)
-		withUser(f, "some-user", []string{"group1", "group2"})
+		withUser(f, "some-user-id", "some-user", []string{"group1", "group2"})
 		ts := startTestServer(t, f)
 		defer ts.Close()
 		r := newExtensionRequest(t, "Get", fmt.Sprintf("%s/extensions/%s/", ts.URL, extName))
@@ -578,7 +580,7 @@ func TestCallExtension(t *testing.T) {
 		withRbac(f, allowApp, allowExtension)
 		withExtensionConfig(getExtensionConfig(extName, "http://fake"), f)
 		withMetrics(f)
-		withUser(f, "some-user", []string{"group1", "group2"})
+		withUser(f, "some-user-id", "some-user", []string{"group1", "group2"})
 		ts := startTestServer(t, f)
 		defer ts.Close()
 		r := newExtensionRequest(t, "Get", fmt.Sprintf("%s/extensions/%s/", ts.URL, extName))
@@ -603,7 +605,7 @@ func TestCallExtension(t *testing.T) {
 		withRbac(f, allowApp, allowExtension)
 		withExtensionConfig(getExtensionConfig(extName, "http://fake"), f)
 		withMetrics(f)
-		withUser(f, "some-user", []string{"group1", "group2"})
+		withUser(f, "some-user-id", "some-user", []string{"group1", "group2"})
 		ts := startTestServer(t, f)
 		defer ts.Close()
 		r := newExtensionRequest(t, "Get", fmt.Sprintf("%s/extensions/%s/", ts.URL, extName))
@@ -629,7 +631,7 @@ func TestCallExtension(t *testing.T) {
 		withRbac(f, allowApp, allowExtension)
 		withExtensionConfig(getExtensionConfig(extName, "http://fake"), f)
 		withMetrics(f)
-		withUser(f, "some-user", []string{"group1", "group2"})
+		withUser(f, "some-user-id", "some-user", []string{"group1", "group2"})
 		ts := startTestServer(t, f)
 		defer ts.Close()
 		r := newExtensionRequest(t, "Get", fmt.Sprintf("%s/extensions/%s/", ts.URL, extName))
@@ -655,7 +657,7 @@ func TestCallExtension(t *testing.T) {
 		withRbac(f, allowApp, allowExtension)
 		withExtensionConfig(getExtensionConfig(extName, "http://fake"), f)
 		withMetrics(f)
-		withUser(f, "some-user", []string{"group1", "group2"})
+		withUser(f, "some-user-id", "some-user", []string{"group1", "group2"})
 		ts := startTestServer(t, f)
 		defer ts.Close()
 		r := newExtensionRequest(t, "Get", fmt.Sprintf("%s/extensions/%s/", ts.URL, extName))
@@ -687,7 +689,7 @@ func TestCallExtension(t *testing.T) {
 		withExtensionConfig(getExtensionConfigWith2Backends(extName, "url1", "cluster1Name", "cluster1URL", "url2", "cluster2Name", "cluster2URL"), f)
 		withProject(getProjectWithDestinations("project-name", nil, []string{"srv1", destinationServer}), f)
 		withMetrics(f)
-		withUser(f, "some-user", []string{"group1", "group2"})
+		withUser(f, "some-user-id", "some-user", []string{"group1", "group2"})
 
 		ts := startTestServer(t, f)
 		defer ts.Close()
@@ -721,7 +723,7 @@ func TestCallExtension(t *testing.T) {
 		withRbac(f, allowApp, allowExtension)
 		withExtensionConfig(getExtensionConfig(extName, "http://fake"), f)
 		withMetrics(f)
-		withUser(f, "some-user", []string{"group1", "group2"})
+		withUser(f, "some-user-id", "some-user", []string{"group1", "group2"})
 		ts := startTestServer(t, f)
 		defer ts.Close()
 		r := newExtensionRequest(t, "Get", ts.URL+"/extensions/")