feat: Move application is permitted outside of watch function and cov…

…er with unit tests
pasha-codefresh · Jul 13, 2023 · 73209b3 · 73209b3
1 parent 8d2e085
commit 73209b3
Show file tree

Hide file tree

Showing 2 changed files with 80 additions and 18 deletions.
diff --git a/server/application/application.go b/server/application/application.go
@@ -985,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())
@@ -1011,24 +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.isNamespaceEnabled(a.Namespace) {
-			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)

diff --git a/server/application/application_test.go b/server/application/application_test.go
@@ -5,6 +5,7 @@ import (
 	coreerrors "errors"
 	"fmt"
 	"io"
+	"k8s.io/apimachinery/pkg/labels"
 	"strconv"
 	"sync/atomic"
 	"testing"
@@ -2202,3 +2203,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, testApp.Namespace, 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)
+	})
+}