fix: web terminal outside argocd namespace (#11166)

pkg/apis/application/v1alpha1/applicationset_types.go

@@ -46,6 +46,11 @@ type ApplicationSet struct {
 	Status            ApplicationSetStatus `json:"status,omitempty" protobuf:"bytes,3,opt,name=status"`
 }
 
+// RBACName formats fully qualified application name for RBAC check.
+func (a *ApplicationSet) RBACName() string {
+	return fmt.Sprintf("%s/%s", a.Spec.Template.Spec.GetProject(), a.ObjectMeta.Name)
+}
+
 // ApplicationSetSpec represents a class of application set state.
 type ApplicationSetSpec struct {
 	GoTemplate bool                      `json:"goTemplate,omitempty" protobuf:"bytes,1,name=goTemplate"`
@@ -536,7 +541,7 @@ const (
 // prefix "Info" means informational condition
 type ApplicationSetConditionType string
 
-//ErrorOccurred / ParametersGenerated / TemplateRendered / ResourcesUpToDate
+// ErrorOccurred / ParametersGenerated / TemplateRendered / ResourcesUpToDate
 const (
 	ApplicationSetConditionErrorOccurred       ApplicationSetConditionType = "ErrorOccurred"
 	ApplicationSetConditionParametersGenerated ApplicationSetConditionType = "ParametersGenerated"

pkg/apis/application/v1alpha1/types.go

@@ -35,6 +35,7 @@ import (
 
 	"github.com/argoproj/argo-cd/v2/util/collections"
 	"github.com/argoproj/argo-cd/v2/util/helm"
+	"github.com/argoproj/argo-cd/v2/util/security"
 )
 
 // Application is a definition of Application resource.
@@ -2575,9 +2576,5 @@ func (a *Application) QualifiedName() string {
 // RBACName returns the full qualified RBAC resource name for the application
 // in a backwards-compatible way.
 func (a *Application) RBACName(defaultNS string) string {
-	if defaultNS != "" && a.Namespace != defaultNS && a.Namespace != "" {
-		return fmt.Sprintf("%s/%s/%s", a.Spec.GetProject(), a.Namespace, a.Name)
-	} else {
-		return fmt.Sprintf("%s/%s", a.Spec.GetProject(), a.Name)
-	}
+	return security.AppRBACName(defaultNS, a.Spec.GetProject(), a.Namespace, a.Name)
 }

server/application/application.go

@@ -53,6 +53,7 @@ import (
 	"github.com/argoproj/argo-cd/v2/util/lua"
 	"github.com/argoproj/argo-cd/v2/util/manifeststream"
 	"github.com/argoproj/argo-cd/v2/util/rbac"
+	"github.com/argoproj/argo-cd/v2/util/security"
 	"github.com/argoproj/argo-cd/v2/util/session"
 	"github.com/argoproj/argo-cd/v2/util/settings"
 )
@@ -209,7 +210,7 @@ func (s *Server) Create(ctx context.Context, q *application.ApplicationCreateReq
 	appNs := s.appNamespaceOrDefault(a.Namespace)
 
 	if !s.isNamespaceEnabled(appNs) {
-		return nil, namespaceNotPermittedError(appNs)
+		return nil, security.NamespaceNotPermittedError(appNs)
 	}
 
 	created, err := s.appclientset.ArgoprojV1alpha1().Applications(appNs).Create(ctx, a, metav1.CreateOptions{})
@@ -341,7 +342,7 @@ func (s *Server) GetManifests(ctx context.Context, q *application.ApplicationMan
 	}
 
 	if !s.isNamespaceEnabled(a.Namespace) {
-		return nil, namespaceNotPermittedError(a.Namespace)
+		return nil, security.NamespaceNotPermittedError(a.Namespace)
 	}
 
 	var manifestInfo *apiclient.ManifestResponse
@@ -2288,9 +2289,5 @@ func (s *Server) appNamespaceOrDefault(appNs string) string {
 }
 
 func (s *Server) isNamespaceEnabled(namespace string) bool {
-	return namespace == s.ns || glob.MatchStringInList(s.enabledNamespaces, namespace, false)
-}
-
-func namespaceNotPermittedError(namespace string) error {
-	return fmt.Errorf("namespace '%s' is not permitted", namespace)
+	return security.IsNamespaceEnabled(namespace, s.ns, s.enabledNamespaces)
 }

server/application/terminal.go

@@ -2,7 +2,6 @@ package application
 
 import (
 	"context"
-	"fmt"
 	"io"
 	"net/http"
 
@@ -24,6 +23,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/rbac"
+	"github.com/argoproj/argo-cd/v2/util/security"
 	sessionmgr "github.com/argoproj/argo-cd/v2/util/session"
 )
 
@@ -35,10 +35,11 @@ type terminalHandler struct {
 	appResourceTreeFn func(ctx context.Context, app *appv1.Application) (*appv1.ApplicationTree, error)
 	allowedShells     []string
 	namespace         string
+	enabledNamespaces []string
 }
 
 // NewHandler returns a new terminal handler.
-func NewHandler(appLister applisters.ApplicationLister, namespace string, db db.ArgoDB, enf *rbac.Enforcer, cache *servercache.Cache,
+func NewHandler(appLister applisters.ApplicationLister, namespace string, enabledNamespaces []string, db db.ArgoDB, enf *rbac.Enforcer, cache *servercache.Cache,
 	appResourceTree AppResourceTreeFn, allowedShells []string) *terminalHandler {
 	return &terminalHandler{
 		appLister:         appLister,
@@ -107,6 +108,8 @@ func (s *terminalHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	appNamespace := q.Get("appNamespace")
+
 	if !isValidPodName(podName) {
 		http.Error(w, "Pod name is not valid", http.StatusBadRequest)
 		return
@@ -127,11 +130,26 @@ func (s *terminalHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		http.Error(w, "Namespace name is not valid", http.StatusBadRequest)
 		return
 	}
+	if !isValidNamespaceName(appNamespace) {
+		http.Error(w, "App namespace name is not valid", http.StatusBadRequest)
+		return
+	}
+
+	ns := appNamespace
+	if ns == "" {
+		ns = s.namespace
+	}
+
+	if !security.IsNamespaceEnabled(ns, s.namespace, s.enabledNamespaces) {
+		http.Error(w, security.NamespaceNotPermittedError(ns).Error(), http.StatusForbidden)
+		return
+	}
+
 	shell := q.Get("shell") // No need to validate. Will only be used if it's in the allow-list.
 
 	ctx := r.Context()
 
-	appRBACName := fmt.Sprintf("%s/%s", project, app)
+	appRBACName := security.AppRBACName(s.namespace, project, appNamespace, app)
 	if err := s.enf.EnforceErr(ctx.Value("claims"), rbacpolicy.ResourceApplications, rbacpolicy.ActionGet, appRBACName); err != nil {
 		http.Error(w, err.Error(), http.StatusUnauthorized)
 		return
@@ -143,9 +161,9 @@ func (s *terminalHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	}
 
 	fieldLog := log.WithFields(log.Fields{"application": app, "userName": sessionmgr.Username(ctx), "container": container,
-		"podName": podName, "namespace": namespace, "cluster": project})
+		"podName": podName, "namespace": namespace, "project": project, "appNamespace": appNamespace})
 
-	a, err := s.appLister.Applications(s.namespace).Get(app)
+	a, err := s.appLister.Applications(ns).Get(app)
 	if err != nil {
 		if apierr.IsNotFound(err) {
 			http.Error(w, "App not found", http.StatusNotFound)

server/application/terminal_test.go

@@ -10,6 +10,7 @@ import (
 	"github.com/stretchr/testify/assert"
 
 	appv1 "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
+	"github.com/argoproj/argo-cd/v2/util/security"
 )
 
 func TestPodExists(t *testing.T) {
@@ -195,24 +196,24 @@ func TestTerminalHandler_ServeHTTP_empty_params(t *testing.T) {
 		for _, testValue := range testValues {
 			testValueCopy := testValue
 
-			t.Run(testKeyCopy+ " " + testValueCopy, func(t *testing.T) {
+			t.Run(testKeyCopy+" "+testValueCopy, func(t *testing.T) {
 				t.Parallel()
 
 				handler := terminalHandler{}
 				params := map[string]string{
-					"pod": "valid",
+					"pod":       "valid",
 					"container": "valid",
-					"app": "valid",
-					"project": "valid",
+					"app":       "valid",
+					"project":   "valid",
 					"namespace": "valid",
 				}
 				params[testKeyCopy] = testValueCopy
 				var paramsArray []string
 				for key, value := range params {
-					paramsArray = append(paramsArray, key + "=" + value)
+					paramsArray = append(paramsArray, key+"="+value)
 				}
 				paramsString := strings.Join(paramsArray, "&")
-				request := httptest.NewRequest("GET", "https://argocd.example.com/api/v1/terminal?" + paramsString, nil)
+				request := httptest.NewRequest("GET", "https://argocd.example.com/api/v1/terminal?"+paramsString, nil)
 				recorder := httptest.NewRecorder()
 				handler.ServeHTTP(recorder, request)
 				response := recorder.Result()
@@ -221,3 +222,13 @@ func TestTerminalHandler_ServeHTTP_empty_params(t *testing.T) {
 		}
 	}
 }
+
+func TestTerminalHandler_ServeHTTP_disallowed_namespace(t *testing.T) {
+	handler := terminalHandler{namespace: "argocd", enabledNamespaces: []string{"allowed"}}
+	request := httptest.NewRequest("GET", "https://argocd.example.com/api/v1/terminal?pod=valid&container=valid&appName=valid&projectName=valid&namespace=test&appNamespace=disallowed", nil)
+	recorder := httptest.NewRecorder()
+	handler.ServeHTTP(recorder, request)
+	response := recorder.Result()
+	assert.Equal(t, http.StatusForbidden, response.StatusCode)
+	assert.Equal(t, security.NamespaceNotPermittedError("disallowed").Error()+"\n", recorder.Body.String())
+}

server/applicationset/applicationset.go

@@ -27,7 +27,6 @@ import (
 	applisters "github.com/argoproj/argo-cd/v2/pkg/client/listers/application/v1alpha1"
 	servercache "github.com/argoproj/argo-cd/v2/server/cache"
 	"github.com/argoproj/argo-cd/v2/server/rbacpolicy"
-	apputil "github.com/argoproj/argo-cd/v2/util/appset"
 	"github.com/argoproj/argo-cd/v2/util/argo"
 	argoutil "github.com/argoproj/argo-cd/v2/util/argo"
 	"github.com/argoproj/argo-cd/v2/util/db"
@@ -89,7 +88,7 @@ func (s *Server) Get(ctx context.Context, q *applicationset.ApplicationSetGetQue
 	if err != nil {
 		return nil, fmt.Errorf("error getting ApplicationSet: %w", err)
 	}
-	if err = s.enf.EnforceErr(ctx.Value("claims"), rbacpolicy.ResourceApplicationSets, rbacpolicy.ActionGet, apputil.AppSetRBACName(a)); err != nil {
+	if err = s.enf.EnforceErr(ctx.Value("claims"), rbacpolicy.ResourceApplicationSets, rbacpolicy.ActionGet, a.RBACName()); err != nil {
 		return nil, err
 	}
 
@@ -111,7 +110,7 @@ func (s *Server) List(ctx context.Context, q *applicationset.ApplicationSetListQ
 
 	newItems := make([]v1alpha1.ApplicationSet, 0)
 	for _, a := range appsetList.Items {
-		if s.enf.Enforce(ctx.Value("claims"), rbacpolicy.ResourceApplicationSets, rbacpolicy.ActionGet, apputil.AppSetRBACName(&a)) {
+		if s.enf.Enforce(ctx.Value("claims"), rbacpolicy.ResourceApplicationSets, rbacpolicy.ActionGet, a.RBACName()) {
 			newItems = append(newItems, a)
 		}
 	}
@@ -182,7 +181,7 @@ func (s *Server) Create(ctx context.Context, q *applicationset.ApplicationSetCre
 	if !q.Upsert {
 		return nil, status.Errorf(codes.InvalidArgument, "existing ApplicationSet spec is different, use upsert flag to force update")
 	}
-	if err = s.enf.EnforceErr(ctx.Value("claims"), rbacpolicy.ResourceApplicationSets, rbacpolicy.ActionUpdate, apputil.AppSetRBACName(appset)); err != nil {
+	if err = s.enf.EnforceErr(ctx.Value("claims"), rbacpolicy.ResourceApplicationSets, rbacpolicy.ActionUpdate, appset.RBACName()); err != nil {
 		return nil, err
 	}
 	updated, err := s.updateAppSet(existing, appset, ctx, true)
@@ -210,11 +209,11 @@ func (s *Server) updateAppSet(appset *v1alpha1.ApplicationSet, newAppset *v1alph
 	if appset != nil && appset.Spec.Template.Spec.Project != newAppset.Spec.Template.Spec.Project {
 		// When changing projects, caller must have applicationset create and update privileges in new project
 		// NOTE: the update check was already verified in the caller to this function
-		if err := s.enf.EnforceErr(ctx.Value("claims"), rbacpolicy.ResourceApplicationSets, rbacpolicy.ActionCreate, apputil.AppSetRBACName(newAppset)); err != nil {
+		if err := s.enf.EnforceErr(ctx.Value("claims"), rbacpolicy.ResourceApplicationSets, rbacpolicy.ActionCreate, newAppset.RBACName()); err != nil {
 			return nil, err
 		}
 		// They also need 'update' privileges in the old project
-		if err := s.enf.EnforceErr(ctx.Value("claims"), rbacpolicy.ResourceApplicationSets, rbacpolicy.ActionUpdate, apputil.AppSetRBACName(appset)); err != nil {
+		if err := s.enf.EnforceErr(ctx.Value("claims"), rbacpolicy.ResourceApplicationSets, rbacpolicy.ActionUpdate, appset.RBACName()); err != nil {
 			return nil, err
 		}
 	}
@@ -254,7 +253,7 @@ func (s *Server) Delete(ctx context.Context, q *applicationset.ApplicationSetDel
 		return nil, fmt.Errorf("error getting ApplicationSets: %w", err)
 	}
 
-	if err := s.enf.EnforceErr(ctx.Value("claims"), rbacpolicy.ResourceApplicationSets, rbacpolicy.ActionDelete, apputil.AppSetRBACName(appset)); err != nil {
+	if err := s.enf.EnforceErr(ctx.Value("claims"), rbacpolicy.ResourceApplicationSets, rbacpolicy.ActionDelete, appset.RBACName()); err != nil {
 		return nil, err
 	}
 
@@ -290,7 +289,7 @@ func (s *Server) validateAppSet(ctx context.Context, appset *v1alpha1.Applicatio
 
 func (s *Server) checkCreatePermissions(ctx context.Context, appset *v1alpha1.ApplicationSet, projectName string) error {
 
-	if err := s.enf.EnforceErr(ctx.Value("claims"), rbacpolicy.ResourceApplicationSets, rbacpolicy.ActionCreate, apputil.AppSetRBACName(appset)); err != nil {
+	if err := s.enf.EnforceErr(ctx.Value("claims"), rbacpolicy.ResourceApplicationSets, rbacpolicy.ActionCreate, appset.RBACName()); err != nil {
 		return err
 	}
 

server/server.go

@@ -2,6 +2,7 @@ package server
 
 import (
 	"context"
+	netCtx "context"
 	"crypto/tls"
 	"fmt"
 	goio "io"
@@ -24,8 +25,6 @@ import (
 	// nolint:staticcheck
 	golang_proto "github.com/golang/protobuf/proto"
 
-	netCtx "context"
-
 	"github.com/argoproj/notifications-engine/pkg/api"
 	"github.com/argoproj/pkg/sync"
 	"github.com/go-redis/redis/v8"
@@ -61,6 +60,8 @@ import (
 	accountpkg "github.com/argoproj/argo-cd/v2/pkg/apiclient/account"
 	applicationpkg "github.com/argoproj/argo-cd/v2/pkg/apiclient/application"
 
+	"github.com/pkg/errors"
+
 	applicationsetpkg "github.com/argoproj/argo-cd/v2/pkg/apiclient/applicationset"
 	certificatepkg "github.com/argoproj/argo-cd/v2/pkg/apiclient/certificate"
 	clusterpkg "github.com/argoproj/argo-cd/v2/pkg/apiclient/cluster"
@@ -121,7 +122,6 @@ import (
 	"github.com/argoproj/argo-cd/v2/util/swagger"
 	tlsutil "github.com/argoproj/argo-cd/v2/util/tls"
 	"github.com/argoproj/argo-cd/v2/util/webhook"
-	"github.com/pkg/errors"
 )
 
 const maxConcurrentLoginRequestsCountEnv = "ARGOCD_MAX_CONCURRENT_LOGIN_REQUESTS_COUNT"
@@ -860,7 +860,7 @@ func (a *ArgoCDServer) newHTTPServer(ctx context.Context, port int, grpcWebHandl
 	}
 	mux.Handle("/api/", handler)
 
-	terminalHandler := application.NewHandler(a.appLister, a.Namespace, a.db, a.enf, a.Cache, appResourceTreeFn, a.settings.ExecShells)
+	terminalHandler := application.NewHandler(a.appLister, a.Namespace, a.ApplicationNamespaces, a.db, a.enf, a.Cache, appResourceTreeFn, a.settings.ExecShells)
 	mux.HandleFunc("/terminal", func(writer http.ResponseWriter, request *http.Request) {
 		argocdSettings, err := a.settingsMgr.GetSettings()
 		if err != nil {

ui/src/app/applications/components/pod-terminal-viewer/pod-terminal-viewer.tsx

@@ -11,6 +11,7 @@ import {Context} from '../../../shared/context';
 import {ErrorNotification, NotificationType} from 'argo-ui';
 export interface PodTerminalViewerProps {
     applicationName: string;
+    applicationNamespace: string;
     projectName: string;
     selectedNode: models.ResourceNode;
     podState: models.State;
@@ -24,7 +25,15 @@ export interface ShellFrame {
     cols?: number;
 }
 
-export const PodTerminalViewer: React.FC<PodTerminalViewerProps> = ({selectedNode, applicationName, projectName, podState, containerName, onClickContainer}) => {
+export const PodTerminalViewer: React.FC<PodTerminalViewerProps> = ({
+    selectedNode,
+    applicationName,
+    applicationNamespace,
+    projectName,
+    podState,
+    containerName,
+    onClickContainer
+}) => {
     const terminalRef = React.useRef(null);
     const appContext = React.useContext(Context); // used to show toast
     const fitAddon = new FitAddon();
@@ -145,7 +154,7 @@ export const PodTerminalViewer: React.FC<PodTerminalViewerProps> = ({selectedNod
         webSocket = new WebSocket(
             `${
                 location.protocol === 'https:' ? 'wss' : 'ws'
-            }://${url}/terminal?pod=${name}&container=${containerName}&appName=${applicationName}&projectName=${projectName}&namespace=${namespace}`
+            }://${url}/terminal?pod=${name}&container=${containerName}&appName=${applicationName}&appNamespace=${applicationNamespace}&projectName=${projectName}&namespace=${namespace}`
         );
         webSocket.onopen = onConnectionOpen;
         webSocket.onclose = onConnectionClose;

ui/src/app/applications/components/resource-details/resource-details.tsx

@@ -129,6 +129,7 @@ export const ResourceDetails = (props: ResourceDetailsProps) => {
                         content: (
                             <PodTerminalViewer
                                 applicationName={application.metadata.name}
+                                applicationNamespace={application.metadata.namespace}
                                 projectName={application.spec.project}
                                 podState={podState}
                                 selectedNode={selectedNode}

util/rbac/rbac.go

@@ -17,7 +17,7 @@ import (
 	"github.com/casbin/casbin/v2"
 	"github.com/casbin/casbin/v2/model"
 	"github.com/casbin/casbin/v2/util"
-	jwt "github.com/golang-jwt/jwt/v4"
+	"github.com/golang-jwt/jwt/v4"
 	gocache "github.com/patrickmn/go-cache"
 	log "github.com/sirupsen/logrus"
 	"google.golang.org/grpc/codes"

util/security/application_namespaces.go

@@ -0,0 +1,15 @@
+package security
+
+import (
+	"fmt"
+
+	"github.com/argoproj/argo-cd/v2/util/glob"
+)
+
+func IsNamespaceEnabled(namespace string, serverNamespace string, enabledNamespaces []string) bool {
+	return namespace == serverNamespace || glob.MatchStringInList(enabledNamespaces, namespace, false)
+}
+
+func NamespaceNotPermittedError(namespace string) error {
+	return fmt.Errorf("namespace '%s' is not permitted", namespace)
+}