feat: Send user groups to proxy extensions (#19855)

* feat: Send user groups to proxy extensions

Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>

* address review comments

Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>

---------

Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>
This commit is contained in:
Leonardo Luz Almeida
2024-09-10 11:37:00 -04:00
committed by GitHub
parent d8c773dd3d
commit 878494f037
6 changed files with 138 additions and 17 deletions

View File

@@ -43,6 +43,7 @@ packages:
ProjectGetter:
RbacEnforcer:
SettingsGetter:
UserGetter:
github.com/argoproj/argo-cd/v2/util/db:
interfaces:
ArgoDB:
@@ -65,4 +66,4 @@ packages:
SessionServiceClient:
github.com/argoproj/argo-cd/v2/pkg/apiclient/cluster:
interfaces:
ClusterServiceServer:
ClusterServiceServer:

View File

@@ -268,6 +268,10 @@ section for more details.
Will be populated with the username logged in Argo CD.
#### `Argocd-User-Groups`
Will be populated with the 'groups' claim from the user logged in Argo CD.
### Multi Backend Use-Case
In some cases when Argo CD is configured to sync with multiple remote

View File

@@ -22,6 +22,7 @@ import (
"github.com/argoproj/argo-cd/v2/util/argo"
"github.com/argoproj/argo-cd/v2/util/db"
"github.com/argoproj/argo-cd/v2/util/security"
"github.com/argoproj/argo-cd/v2/util/session"
"github.com/argoproj/argo-cd/v2/util/settings"
)
@@ -68,6 +69,10 @@ const (
// HeaderArgoCDUsername is the header name that defines the logged
// in user authenticated by Argo CD.
HeaderArgoCDUsername = "Argocd-Username"
// HeaderArgoCDGroups is the header name that provides the 'groups'
// claim from the users authenticated in Argo CD.
HeaderArgoCDGroups = "Argocd-User-Groups"
)
// RequestResources defines the authorization scope for
@@ -269,6 +274,34 @@ func (p *DefaultProjectGetter) GetClusters(project string) ([]*v1alpha1.Cluster,
return p.db.GetProjectClusters(context.TODO(), project)
}
// UserGetter defines the contract to retrieve info from the logged in user.
type UserGetter interface {
GetUser(ctx context.Context) string
GetGroups(ctx context.Context) []string
}
// DefaultUserGetter is the main UserGetter implementation.
type DefaultUserGetter struct {
policyEnf *rbacpolicy.RBACPolicyEnforcer
}
// NewDefaultUserGetter return a new default UserGetter
func NewDefaultUserGetter(policyEnf *rbacpolicy.RBACPolicyEnforcer) *DefaultUserGetter {
return &DefaultUserGetter{
policyEnf: policyEnf,
}
}
// GetUser will return the current logged in user
func (u *DefaultUserGetter) GetUser(ctx context.Context) string {
return session.Username(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())
}
// ApplicationGetter defines the contract to retrieve the application resource.
type ApplicationGetter interface {
Get(ns, name string) (*v1alpha1.Application, error)
@@ -306,13 +339,9 @@ type Manager struct {
rbac RbacEnforcer
registry ExtensionRegistry
metricsReg ExtensionMetricsRegistry
username UsernameGetterFunc
userGetter UserGetter
}
// UsernameGetterFunc defines the function signature to retrieve the username
// from the context.Context. The real implementation is defined in session.Username()
type UsernameGetterFunc func(context.Context) string
// ExtensionMetricsRegistry exposes operations to update http metrics in the Argo CD
// API server.
type ExtensionMetricsRegistry interface {
@@ -326,14 +355,14 @@ type ExtensionMetricsRegistry interface {
}
// NewManager will initialize a new manager.
func NewManager(log *log.Entry, sg SettingsGetter, ag ApplicationGetter, pg ProjectGetter, rbac RbacEnforcer, userFn UsernameGetterFunc) *Manager {
func NewManager(log *log.Entry, sg SettingsGetter, ag ApplicationGetter, pg ProjectGetter, rbac RbacEnforcer, ug UserGetter) *Manager {
return &Manager{
log: log,
settings: sg,
application: ag,
project: pg,
rbac: rbac,
username: userFn,
userGetter: ug,
}
}
@@ -709,7 +738,9 @@ func (m *Manager) CallExtension() func(http.ResponseWriter, *http.Request) {
return
}
prepareRequest(r, extName, app, m.username(r.Context()))
user := m.userGetter.GetUser(r.Context())
groups := m.userGetter.GetGroups(r.Context())
prepareRequest(r, extName, app, user, groups)
m.log.Debugf("proxing request for extension %q", extName)
// httpsnoop package is used to properly wrap the responseWriter
// and avoid optional intefaces issue:
@@ -735,7 +766,7 @@ func registerMetrics(extName string, metrics httpsnoop.Metrics, extensionMetrics
// - Cluster destination name
// - Cluster destination server
// - Argo CD authenticated username
func prepareRequest(r *http.Request, extName string, app *v1alpha1.Application, username string) {
func prepareRequest(r *http.Request, extName string, app *v1alpha1.Application, username string, groups []string) {
r.URL.Path = strings.TrimPrefix(r.URL.Path, fmt.Sprintf("%s/%s", URLPrefix, extName))
if app.Spec.Destination.Name != "" {
r.Header.Set(HeaderArgoCDTargetClusterName, app.Spec.Destination.Name)
@@ -746,6 +777,9 @@ func prepareRequest(r *http.Request, extName string, app *v1alpha1.Application,
if username != "" {
r.Header.Set(HeaderArgoCDUsername, username)
}
if len(groups) > 0 {
r.Header.Set(HeaderArgoCDGroups, strings.Join(groups, ","))
}
}
// AddMetricsRegistry will associate the given metricsReg in the Manager.

View File

@@ -245,24 +245,22 @@ func TestCallExtension(t *testing.T) {
rbacMock *mocks.RbacEnforcer
projMock *mocks.ProjectGetter
metricsMock *mocks.ExtensionMetricsRegistry
userMock *mocks.UserGetter
manager *extension.Manager
}
defaultProjectName := "project-name"
usernameFn := func(context.Context) string {
return "loggedinUser"
}
setup := func() *fixture {
appMock := &mocks.ApplicationGetter{}
settMock := &mocks.SettingsGetter{}
rbacMock := &mocks.RbacEnforcer{}
projMock := &mocks.ProjectGetter{}
metricsMock := &mocks.ExtensionMetricsRegistry{}
userMock := &mocks.UserGetter{}
logger, _ := test.NewNullLogger()
logEntry := logger.WithContext(context.Background())
m := extension.NewManager(logEntry, settMock, appMock, projMock, rbacMock, usernameFn)
m := extension.NewManager(logEntry, settMock, appMock, projMock, rbacMock, userMock)
m.AddMetricsRegistry(metricsMock)
mux := http.NewServeMux()
@@ -276,6 +274,7 @@ func TestCallExtension(t *testing.T) {
rbacMock: rbacMock,
projMock: projMock,
metricsMock: metricsMock,
userMock: userMock,
manager: m,
}
}
@@ -351,6 +350,11 @@ func TestCallExtension(t *testing.T) {
f.rbacMock.On("EnforceErr", mock.Anything, rbacpolicy.ResourceExtensions, rbacpolicy.ActionInvoke, mock.Anything).Return(extAccessError)
}
withUser := func(f *fixture, username string, groups []string) {
f.userMock.On("GetUser", mock.Anything).Return(username)
f.userMock.On("GetGroups", mock.Anything).Return(groups)
}
withExtensionConfig := func(configYaml string, f *fixture) {
secrets := make(map[string]string)
secrets["extension.auth.header"] = "Bearer some-bearer-token"
@@ -407,6 +411,7 @@ func TestCallExtension(t *testing.T) {
}))
defer backendSrv.Close()
withRbac(f, true, true)
withUser(f, "some-user", []string{"group1", "group2"})
withExtensionConfig(getExtensionConfig(backendEndpoint, backendSrv.URL), f)
ts := startTestServer(t, f)
defer ts.Close()
@@ -441,7 +446,8 @@ func TestCallExtension(t *testing.T) {
assert.Equal(t, backendResponse, actual)
assert.Equal(t, clusterURL, resp.Header.Get(extension.HeaderArgoCDTargetClusterURL))
assert.Equal(t, "Bearer some-bearer-token", resp.Header.Get("Authorization"))
assert.Equal(t, "loggedinUser", resp.Header.Get(extension.HeaderArgoCDUsername))
assert.Equal(t, "some-user", resp.Header.Get(extension.HeaderArgoCDUsername))
assert.Equal(t, "group1,group2", resp.Header.Get(extension.HeaderArgoCDGroups))
// waitgroup is necessary to make sure assertions aren't executed before
// the goroutine initiated by extension.CallExtension concludes which would
@@ -457,6 +463,7 @@ func TestCallExtension(t *testing.T) {
withExtensionConfig(getExtensionConfigString(), f)
withRbac(f, true, true)
withMetrics(f)
withUser(f, "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)
@@ -497,6 +504,7 @@ func TestCallExtension(t *testing.T) {
withExtensionConfig(getExtensionConfigWith2Backends(extName, beSrv1.URL, cluster1Name, beSrv2.URL, cluster2URL), f)
withProject(getProjectWithDestinations("project-name", []string{cluster1Name}, []string{cluster2URL}), f)
withMetrics(f)
withUser(f, "some-user", []string{"group1", "group2"})
ts := startTestServer(t, f)
defer ts.Close()
@@ -543,6 +551,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"})
ts := startTestServer(t, f)
defer ts.Close()
r := newExtensionRequest(t, "Get", fmt.Sprintf("%s/extensions/%s/", ts.URL, extName))
@@ -566,6 +575,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"})
ts := startTestServer(t, f)
defer ts.Close()
r := newExtensionRequest(t, "Get", fmt.Sprintf("%s/extensions/%s/", ts.URL, extName))
@@ -590,6 +600,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"})
ts := startTestServer(t, f)
defer ts.Close()
r := newExtensionRequest(t, "Get", fmt.Sprintf("%s/extensions/%s/", ts.URL, extName))
@@ -615,6 +626,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"})
ts := startTestServer(t, f)
defer ts.Close()
r := newExtensionRequest(t, "Get", fmt.Sprintf("%s/extensions/%s/", ts.URL, extName))
@@ -640,6 +652,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"})
ts := startTestServer(t, f)
defer ts.Close()
r := newExtensionRequest(t, "Get", fmt.Sprintf("%s/extensions/%s/", ts.URL, extName))
@@ -671,6 +684,7 @@ func TestCallExtension(t *testing.T) {
withExtensionConfig(getExtensionConfigWith2Backends(extName, "url1", "clusterName", "url2", "clusterURL"), f)
withProject(getProjectWithDestinations("project-name", nil, []string{"srv1", destinationServer}), f)
withMetrics(f)
withUser(f, "some-user", []string{"group1", "group2"})
ts := startTestServer(t, f)
defer ts.Close()
@@ -704,6 +718,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"})
ts := startTestServer(t, f)
defer ts.Close()
r := newExtensionRequest(t, "Get", fmt.Sprintf("%s/extensions/", ts.URL))

66
server/extension/mocks/UserGetter.go generated Normal file
View File

@@ -0,0 +1,66 @@
// Code generated by mockery v2.43.2. DO NOT EDIT.
package mocks
import (
context "context"
mock "github.com/stretchr/testify/mock"
)
// UserGetter is an autogenerated mock type for the UserGetter type
type UserGetter struct {
mock.Mock
}
// GetGroups provides a mock function with given fields: ctx
func (_m *UserGetter) GetGroups(ctx context.Context) []string {
ret := _m.Called(ctx)
if len(ret) == 0 {
panic("no return value specified for GetGroups")
}
var r0 []string
if rf, ok := ret.Get(0).(func(context.Context) []string); ok {
r0 = rf(ctx)
} else {
if ret.Get(0) != nil {
r0 = ret.Get(0).([]string)
}
}
return r0
}
// GetUser provides a mock function with given fields: ctx
func (_m *UserGetter) GetUser(ctx context.Context) string {
ret := _m.Called(ctx)
if len(ret) == 0 {
panic("no return value specified for GetUser")
}
var r0 string
if rf, ok := ret.Get(0).(func(context.Context) string); ok {
r0 = rf(ctx)
} else {
r0 = ret.Get(0).(string)
}
return r0
}
// NewUserGetter creates a new instance of UserGetter. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations.
// The first argument is typically a *testing.T value.
func NewUserGetter(t interface {
mock.TestingT
Cleanup(func())
}) *UserGetter {
mock := &UserGetter{}
mock.Mock.Test(t)
t.Cleanup(func() { mock.AssertExpectations(t) })
return mock
}

View File

@@ -326,7 +326,8 @@ func NewServer(ctx context.Context, opts ArgoCDServerOpts, appsetOpts Applicatio
sg := extension.NewDefaultSettingsGetter(settingsMgr)
ag := extension.NewDefaultApplicationGetter(appLister)
pg := extension.NewDefaultProjectGetter(projLister, dbInstance)
em := extension.NewManager(logger, sg, ag, pg, enf, util_session.Username)
ug := extension.NewDefaultUserGetter(policyEnf)
em := extension.NewManager(logger, sg, ag, pg, enf, ug)
a := &ArgoCDServer{
ArgoCDServerOpts: opts,