Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(server): not need send application if it is not under enabled namespaces #14479

Merged
44 changes: 28 additions & 16 deletions server/application/application.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ import (
"github.com/argoproj/argo-cd/v2/util/db"
"github.com/argoproj/argo-cd/v2/util/env"
"github.com/argoproj/argo-cd/v2/util/git"
"github.com/argoproj/argo-cd/v2/util/glob"
ioutil "github.com/argoproj/argo-cd/v2/util/io"
"github.com/argoproj/argo-cd/v2/util/lua"
"github.com/argoproj/argo-cd/v2/util/manifeststream"
Expand Down Expand Up @@ -225,7 +224,7 @@ func (s *Server) List(ctx context.Context, q *application.ApplicationQuery) (*ap
for _, a := range filteredApps {
// Skip any application that is neither in the control plane's namespace
// nor in the list of enabled namespaces.
if a.Namespace != s.ns && !glob.MatchStringInList(s.enabledNamespaces, a.Namespace, false) {
if !s.isNamespaceEnabled(a.Namespace) {
continue
}
if s.enf.Enforce(ctx.Value("claims"), rbacpolicy.ResourceApplications, rbacpolicy.ActionGet, a.RBACName(s.ns)) {
Expand Down Expand Up @@ -986,6 +985,31 @@ func (s *Server) Delete(ctx context.Context, q *application.ApplicationDeleteReq
return &application.ApplicationResponse{}, nil
}

func (s *Server) isApplicationPermitted(selector labels.Selector, minVersion int, claims any, appName, appNs string, projects map[string]bool, a appv1.Application) bool {
if len(projects) > 0 && !projects[a.Spec.GetProject()] {
return false
}

if appVersion, err := strconv.Atoi(a.ResourceVersion); err == nil && appVersion < minVersion {
return false
}
matchedEvent := (appName == "" || (a.Name == appName && a.Namespace == appNs)) && selector.Matches(labels.Set(a.Labels))
if !matchedEvent {
return false
}

if !s.isNamespaceEnabled(a.Namespace) {
return false
}

if !s.enf.Enforce(claims, rbacpolicy.ResourceApplications, rbacpolicy.ActionGet, a.RBACName(s.ns)) {
// do not emit apps user does not have accessing
return false
}

return true
}

func (s *Server) Watch(q *application.ApplicationQuery, ws application.ApplicationService_WatchServer) error {
appName := q.GetName()
appNs := s.appNamespaceOrDefault(q.GetAppNamespace())
Expand All @@ -1012,20 +1036,8 @@ func (s *Server) Watch(q *application.ApplicationQuery, ws application.Applicati
// sendIfPermitted is a helper to send the application to the client's streaming channel if the
// caller has RBAC privileges permissions to view it
sendIfPermitted := func(a appv1.Application, eventType watch.EventType) {
if len(projects) > 0 && !projects[a.Spec.GetProject()] {
return
}

if appVersion, err := strconv.Atoi(a.ResourceVersion); err == nil && appVersion < minVersion {
return
}
matchedEvent := (appName == "" || (a.Name == appName && a.Namespace == appNs)) && selector.Matches(labels.Set(a.Labels))
if !matchedEvent {
return
}

if !s.enf.Enforce(claims, rbacpolicy.ResourceApplications, rbacpolicy.ActionGet, a.RBACName(s.ns)) {
// do not emit apps user does not have accessing
permitted := s.isApplicationPermitted(selector, minVersion, claims, appName, appNs, projects, a)
if !permitted {
return
}
s.inferResourcesStatusHealth(&a)
Expand Down
53 changes: 53 additions & 0 deletions server/application/application_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
coreerrors "errors"
"fmt"
"io"
"k8s.io/apimachinery/pkg/labels"
"strconv"
"sync/atomic"
"testing"
Expand Down Expand Up @@ -2235,3 +2236,55 @@ func TestRunOldStyleResourceAction(t *testing.T) {
assert.NotNil(t, appResponse)
})
}

func TestIsApplicationPermitted(t *testing.T) {
t.Run("Incorrect project", func(t *testing.T) {
testApp := newTestApp()
appServer := newTestAppServer(t, testApp)
projects := map[string]bool{"test-app": false}
permitted := appServer.isApplicationPermitted(labels.Everything(), 0, nil, "test", "default", projects, *testApp)
assert.False(t, permitted)
})

t.Run("Version is incorrect", func(t *testing.T) {
testApp := newTestApp()
appServer := newTestAppServer(t, testApp)
minVersion := 100000
testApp.ResourceVersion = strconv.Itoa(minVersion - 1)
permitted := appServer.isApplicationPermitted(labels.Everything(), minVersion, nil, "test", "default", nil, *testApp)
assert.False(t, permitted)
})

t.Run("Application name is incorrect", func(t *testing.T) {
testApp := newTestApp()
appServer := newTestAppServer(t, testApp)
appName := "test"
permitted := appServer.isApplicationPermitted(labels.Everything(), 0, nil, appName, "default", nil, *testApp)
assert.False(t, permitted)
})

t.Run("Application namespace is incorrect", func(t *testing.T) {
testApp := newTestApp()
appServer := newTestAppServer(t, testApp)
permitted := appServer.isApplicationPermitted(labels.Everything(), 0, nil, testApp.Name, "demo", nil, *testApp)
assert.False(t, permitted)
})

t.Run("Application is not part of enabled namespace", func(t *testing.T) {
testApp := newTestApp()
appServer := newTestAppServer(t, testApp)
appServer.ns = "server-ns"
appServer.enabledNamespaces = []string{"demo"}
permitted := appServer.isApplicationPermitted(labels.Everything(), 0, nil, testApp.Name, testApp.Namespace, nil, *testApp)
assert.False(t, permitted)
})

t.Run("Application is part of enabled namespace", func(t *testing.T) {
testApp := newTestApp()
appServer := newTestAppServer(t, testApp)
appServer.ns = "server-ns"
appServer.enabledNamespaces = []string{testApp.Namespace}
permitted := appServer.isApplicationPermitted(labels.Everything(), 0, nil, testApp.Name, testApp.Namespace, nil, *testApp)
assert.True(t, permitted)
})
}