From 73209b35d6c264e888320a66758f7e44eff46ebe Mon Sep 17 00:00:00 2001
From: pashakostohrys <pavel@codefresh.io>
Date: Thu, 13 Jul 2023 15:05:30 +0300
Subject: [PATCH] feat: Move application is permitted outside of watch function
 and cover with unit tests

---
 server/application/application.go      | 45 +++++++++++++---------
 server/application/application_test.go | 53 ++++++++++++++++++++++++++
 2 files changed, 80 insertions(+), 18 deletions(-)

diff --git a/server/application/application.go b/server/application/application.go
index 96e85f2fa1d8c..ef35d76e84208 100644
--- 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
index 2dcefc121dfca..4b9e47957bf12 100644
--- 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)
+	})
+}