fix: Better enforcement of application namespace restrictions

controller/appcontroller.go

@@ -1803,6 +1803,12 @@ func (ctrl *ApplicationController) shouldSelfHeal(app *appv1.Application) (bool,
 	return retryAfter <= 0, retryAfter
 }
 
+// isAppNamespaceAllowed returns whether the application is allowed in the
+// namespace it's residing in.
+func (ctrl *ApplicationController) isAppNamespaceAllowed(app *appv1.Application) bool {
+	return app.Namespace == ctrl.namespace || glob.MatchStringInList(ctrl.applicationNamespaces, app.Namespace, false)
+}
+
 func (ctrl *ApplicationController) canProcessApp(obj interface{}) bool {
 	app, ok := obj.(*appv1.Application)
 	if !ok {
@@ -1811,7 +1817,7 @@ func (ctrl *ApplicationController) canProcessApp(obj interface{}) bool {
 
 	// Only process given app if it exists in a watched namespace, or in the
 	// control plane's namespace.
-	if app.Namespace != ctrl.namespace && !glob.MatchStringInList(ctrl.applicationNamespaces, app.Namespace, false) {
+	if !ctrl.isAppNamespaceAllowed(app) {
 		return false
 	}
 
@@ -1862,7 +1868,7 @@ func (ctrl *ApplicationController) newApplicationInformerAndLister() (cache.Shar
 				}
 				newItems := []appv1.Application{}
 				for _, app := range appList.Items {
-					if ctrl.namespace == app.Namespace || glob.MatchStringInList(ctrl.applicationNamespaces, app.Namespace, false) {
+					if ctrl.isAppNamespaceAllowed(&app) {
 						newItems = append(newItems, app)
 					}
 				}
@@ -1879,6 +1885,14 @@ func (ctrl *ApplicationController) newApplicationInformerAndLister() (cache.Shar
 			cache.NamespaceIndex: func(obj interface{}) ([]string, error) {
 				app, ok := obj.(*appv1.Application)
 				if ok {
+					// If the application is not allowed to use the project,
+					// log an error.
+					if ctrl.isAppNamespaceAllowed(app) {
+						if _, err := ctrl.getAppProj(app); err != nil {
+							ctrl.setAppCondition(app, ctrl.projectErrorToCondition(err, app))
+						}
+					}
+
 					// This call to 'ValidateDestination' ensures that the .spec.destination field of all Applications
 					// returned by the informer/lister will have server field set (if not already set) based on the name.
 					// (or, if not found, an error app condition)
@@ -1889,11 +1903,6 @@ func (ctrl *ApplicationController) newApplicationInformerAndLister() (cache.Shar
 						ctrl.setAppCondition(app, appv1.ApplicationCondition{Type: appv1.ApplicationConditionInvalidSpecError, Message: err.Error()})
 					}
 
-					// If the application is not allowed to use the project,
-					// log an error.
-					if _, err := ctrl.getAppProj(app); err != nil {
-						ctrl.setAppCondition(app, ctrl.projectErrorToCondition(err, app))
-					}
 				}
 
 				return cache.MetaNamespaceIndexFunc(obj)
@@ -1904,6 +1913,10 @@ func (ctrl *ApplicationController) newApplicationInformerAndLister() (cache.Shar
 					return nil, nil
 				}
 
+				if !ctrl.isAppNamespaceAllowed(app) {
+					return nil, nil
+				}
+
 				proj, err := applisters.NewAppProjectLister(ctrl.projInformer.GetIndexer()).AppProjects(ctrl.namespace).Get(app.Spec.GetProject())
 				if err != nil {
 					return nil, nil

server/application/application.go

@@ -233,6 +233,9 @@ func (s *Server) getApplicationEnforceRBACInformer(ctx context.Context, action,
 func (s *Server) getApplicationEnforceRBACClient(ctx context.Context, action, project, namespace, name, resourceVersion string) (*appv1.Application, error) {
 	namespaceOrDefault := s.appNamespaceOrDefault(namespace)
 	return s.getAppEnforceRBAC(ctx, action, project, namespaceOrDefault, name, func() (*appv1.Application, error) {
+		if !s.isNamespaceEnabled(namespaceOrDefault) {
+			return nil, security.NamespaceNotPermittedError(namespaceOrDefault)
+		}
 		return s.appclientset.ArgoprojV1alpha1().Applications(namespaceOrDefault).Get(ctx, name, metav1.GetOptions{
 			ResourceVersion: resourceVersion,
 		})
@@ -644,6 +647,7 @@ func (s *Server) Get(ctx context.Context, q *application.ApplicationQuery) (*app
 	} else if len(projects) > 1 {
 		return nil, status.Errorf(codes.InvalidArgument, "multiple projects specified - the get endpoint accepts either zero or one project")
 	}
+
 	// We must use a client Get instead of an informer Get, because it's common to call Get immediately
 	// following a Watch (which is not yet powered by an informer), and the Get must reflect what was
 	// previously seen by the client.

server/application/application_test.go

@@ -170,6 +170,7 @@ func newTestAppServerWithEnforcerConfigure(f func(*rbac.Enforcer), t *testing.T,
 			Destinations: []appsv1.ApplicationDestination{{Server: "*", Namespace: "*"}},
 		},
 	}
+
 	myProj := &appsv1.AppProject{
 		ObjectMeta: metav1.ObjectMeta{Name: "my-proj", Namespace: "default"},
 		Spec: appsv1.AppProjectSpec{
@@ -2343,3 +2344,145 @@ func TestIsApplicationPermitted(t *testing.T) {
 		assert.True(t, permitted)
 	})
 }
+
+func TestAppNamespaceRestrictions(t *testing.T) {
+	t.Run("List applications in controller namespace", func(t *testing.T) {
+		testApp := newTestApp()
+		appServer := newTestAppServer(t, testApp)
+		apps, err := appServer.List(context.TODO(), &application.ApplicationQuery{})
+		require.NoError(t, err)
+		require.Len(t, apps.Items, 1)
+	})
+
+	t.Run("List applications with non-allowed apps existing", func(t *testing.T) {
+		testApp1 := newTestApp()
+		testApp1.Namespace = "argocd-1"
+		appServer := newTestAppServer(t, testApp1)
+		apps, err := appServer.List(context.TODO(), &application.ApplicationQuery{})
+		require.NoError(t, err)
+		require.Len(t, apps.Items, 0)
+	})
+
+	t.Run("List applications with non-allowed apps existing and explicit ns request", func(t *testing.T) {
+		testApp1 := newTestApp()
+		testApp2 := newTestApp()
+		testApp2.Namespace = "argocd-1"
+		appServer := newTestAppServer(t, testApp1, testApp2)
+		apps, err := appServer.List(context.TODO(), &application.ApplicationQuery{AppNamespace: pointer.String("argocd-1")})
+		require.NoError(t, err)
+		require.Len(t, apps.Items, 0)
+	})
+
+	t.Run("List applications with allowed apps in other namespaces", func(t *testing.T) {
+		testApp1 := newTestApp()
+		testApp1.Namespace = "argocd-1"
+		appServer := newTestAppServer(t, testApp1)
+		appServer.enabledNamespaces = []string{"argocd-1"}
+		apps, err := appServer.List(context.TODO(), &application.ApplicationQuery{})
+		require.NoError(t, err)
+		require.Len(t, apps.Items, 1)
+	})
+
+	t.Run("Get application in control plane namespace", func(t *testing.T) {
+		testApp := newTestApp()
+		appServer := newTestAppServer(t, testApp)
+		app, err := appServer.Get(context.TODO(), &application.ApplicationQuery{
+			Name: pointer.String("test-app"),
+		})
+		require.NoError(t, err)
+		assert.Equal(t, "test-app", app.GetName())
+	})
+	t.Run("Get application in other namespace when forbidden", func(t *testing.T) {
+		testApp := newTestApp()
+		testApp.Namespace = "argocd-1"
+		appServer := newTestAppServer(t, testApp)
+		app, err := appServer.Get(context.TODO(), &application.ApplicationQuery{
+			Name:         pointer.String("test-app"),
+			AppNamespace: pointer.String("argocd-1"),
+		})
+		require.Error(t, err)
+		require.ErrorContains(t, err, "permission denied")
+		require.Nil(t, app)
+	})
+	t.Run("Get application in other namespace when allowed", func(t *testing.T) {
+		testApp := newTestApp()
+		testApp.Namespace = "argocd-1"
+		appServer := newTestAppServer(t, testApp)
+		appServer.enabledNamespaces = []string{"argocd-1"}
+		app, err := appServer.Get(context.TODO(), &application.ApplicationQuery{
+			Name:         pointer.String("test-app"),
+			AppNamespace: pointer.String("argocd-1"),
+		})
+		require.NoError(t, err)
+		require.NotNil(t, app)
+		require.Equal(t, "argocd-1", app.Namespace)
+		require.Equal(t, "test-app", app.Name)
+	})
+	t.Run("Create application in other namespace when allowed", func(t *testing.T) {
+		testApp := newTestApp()
+		testApp.Namespace = "argocd-1"
+		testApp.Spec.Project = "other-ns"
+		otherNsProj := &appsv1.AppProject{
+			ObjectMeta: metav1.ObjectMeta{Name: "other-ns", Namespace: "default"},
+			Spec: appsv1.AppProjectSpec{
+				SourceRepos:      []string{"*"},
+				Destinations:     []appsv1.ApplicationDestination{{Server: "*", Namespace: "*"}},
+				SourceNamespaces: []string{"argocd-1"},
+			},
+		}
+		appServer := newTestAppServer(t, otherNsProj)
+		appServer.enabledNamespaces = []string{"argocd-1"}
+		app, err := appServer.Create(context.TODO(), &application.ApplicationCreateRequest{
+			Application: testApp,
+		})
+		require.NoError(t, err)
+		require.NotNil(t, app)
+		assert.Equal(t, "test-app", app.Name)
+		assert.Equal(t, "argocd-1", app.Namespace)
+	})
+
+	t.Run("Create application in other namespace when not allowed by project", func(t *testing.T) {
+		testApp := newTestApp()
+		testApp.Namespace = "argocd-1"
+		testApp.Spec.Project = "other-ns"
+		otherNsProj := &appsv1.AppProject{
+			ObjectMeta: metav1.ObjectMeta{Name: "other-ns", Namespace: "default"},
+			Spec: appsv1.AppProjectSpec{
+				SourceRepos:      []string{"*"},
+				Destinations:     []appsv1.ApplicationDestination{{Server: "*", Namespace: "*"}},
+				SourceNamespaces: []string{},
+			},
+		}
+		appServer := newTestAppServer(t, otherNsProj)
+		appServer.enabledNamespaces = []string{"argocd-1"}
+		app, err := appServer.Create(context.TODO(), &application.ApplicationCreateRequest{
+			Application: testApp,
+		})
+		require.Error(t, err)
+		require.Nil(t, app)
+		require.ErrorContains(t, err, "not allowed to use project")
+	})
+
+	t.Run("Create application in other namespace when not allowed by configuration", func(t *testing.T) {
+		testApp := newTestApp()
+		testApp.Namespace = "argocd-1"
+		testApp.Spec.Project = "other-ns"
+		otherNsProj := &appsv1.AppProject{
+			ObjectMeta: metav1.ObjectMeta{Name: "other-ns", Namespace: "default"},
+			Spec: appsv1.AppProjectSpec{
+				SourceRepos:      []string{"*"},
+				Destinations:     []appsv1.ApplicationDestination{{Server: "*", Namespace: "*"}},
+				SourceNamespaces: []string{"argocd-1"},
+			},
+		}
+		appServer := newTestAppServer(t, otherNsProj)
+		appServer.enabledNamespaces = []string{"argocd-2"}
+		app, err := appServer.Create(context.TODO(), &application.ApplicationCreateRequest{
+			Application: testApp,
+		})
+		require.Error(t, err)
+		require.Nil(t, app)
+		require.ErrorContains(t, err, "namespace 'argocd-1' is not permitted")
+	})
+
+}