diff --git a/pkg/apis/application/v1alpha1/app_project_types.go b/pkg/apis/application/v1alpha1/app_project_types.go index 5243ab7990266..81f95ab624a0d 100644 --- a/pkg/apis/application/v1alpha1/app_project_types.go +++ b/pkg/apis/application/v1alpha1/app_project_types.go @@ -17,6 +17,24 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" ) +type ErrApplicationNotAllowedToUseProject struct { + application string + namespace string + project string +} + +func NewErrApplicationNotAllowedToUseProject(application, namespace, project string) error { + return &ErrApplicationNotAllowedToUseProject{ + application: application, + namespace: namespace, + project: project, + } +} + +func (err *ErrApplicationNotAllowedToUseProject) Error() string { + return fmt.Sprintf("application '%s' in namespace '%s' is not allowed to use project %s", err.application, err.namespace, err.project) +} + // AppProjectList is list of AppProject resources // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object type AppProjectList struct { diff --git a/server/application/application.go b/server/application/application.go index b1f35a17756aa..24782233b48d7 100644 --- a/server/application/application.go +++ b/server/application/application.go @@ -147,7 +147,7 @@ func NewServer( // // If the user does provide a "project," we can respond more specifically. If the user does not have access to the given // app name in the given project, we return "permission denied." If the app exists, but the project is different from -func (s *Server) getAppEnforceRBAC(ctx context.Context, action, project, namespace, name string, getApp func() (*appv1.Application, error)) (*appv1.Application, error) { +func (s *Server) getAppEnforceRBAC(ctx context.Context, action, project, namespace, name string, getApp func() (*appv1.Application, error)) (*appv1.Application, *appv1.AppProject, error) { logCtx := log.WithFields(map[string]interface{}{ "application": name, "namespace": namespace, @@ -164,7 +164,7 @@ func (s *Server) getAppEnforceRBAC(ctx context.Context, action, project, namespa // but the app is in a different project" response. We don't want the user inferring the existence of the // app from response time. _, _ = getApp() - return nil, permissionDeniedErr + return nil, nil, permissionDeniedErr } } a, err := getApp() @@ -172,15 +172,15 @@ func (s *Server) getAppEnforceRBAC(ctx context.Context, action, project, namespa if apierr.IsNotFound(err) { if project != "" { // We know that the user was allowed to get the Application, but the Application does not exist. Return 404. - return nil, status.Errorf(codes.NotFound, apierr.NewNotFound(schema.GroupResource{Group: "argoproj.io", Resource: "applications"}, name).Error()) + return nil, nil, status.Errorf(codes.NotFound, apierr.NewNotFound(schema.GroupResource{Group: "argoproj.io", Resource: "applications"}, name).Error()) } // We don't know if the user was allowed to get the Application, and we don't want to leak information about // the Application's existence. Return 403. logCtx.Warn("application does not exist") - return nil, permissionDeniedErr + return nil, nil, permissionDeniedErr } logCtx.Errorf("failed to get application: %s", err) - return nil, permissionDeniedErr + return nil, nil, permissionDeniedErr } // Even if we performed an initial RBAC check (because the request was fully parameterized), we still need to // perform a second RBAC check to ensure that the user has access to the actual Application's project (not just the @@ -194,11 +194,11 @@ func (s *Server) getAppEnforceRBAC(ctx context.Context, action, project, namespa // The user specified a project. We would have returned a 404 if the user had access to the app, but the app // did not exist. So we have to return a 404 when the app does exist, but the user does not have access. // Otherwise, they could infer that the app exists based on the error code. - return nil, status.Errorf(codes.NotFound, apierr.NewNotFound(schema.GroupResource{Group: "argoproj.io", Resource: "applications"}, name).Error()) + return nil, nil, status.Errorf(codes.NotFound, apierr.NewNotFound(schema.GroupResource{Group: "argoproj.io", Resource: "applications"}, name).Error()) } // The user didn't specify a project. We always return permission denied for both lack of access and lack of // existence. - return nil, permissionDeniedErr + return nil, nil, permissionDeniedErr } effectiveProject := "default" if a.Spec.Project != "" { @@ -211,15 +211,20 @@ func (s *Server) getAppEnforceRBAC(ctx context.Context, action, project, namespa }).Warnf("user tried to %s application in project %s, but the application is in project %s", action, project, effectiveProject) // The user has access to the app, but the app is in a different project. Return 404, meaning "app doesn't // exist in that project". - return nil, status.Errorf(codes.NotFound, apierr.NewNotFound(schema.GroupResource{Group: "argoproj.io", Resource: "applications"}, name).Error()) + return nil, nil, status.Errorf(codes.NotFound, apierr.NewNotFound(schema.GroupResource{Group: "argoproj.io", Resource: "applications"}, name).Error()) } - return a, nil + // Get the app's associated project, and make sure all project restrictions are enforced. + proj, err := s.getAppProject(ctx, a, logCtx) + if err != nil { + return a, nil, err + } + return a, proj, nil } // getApplicationEnforceRBACInformer uses an informer to get an Application. If the app does not exist, permission is // denied, or any other error occurs when getting the app, we return a permission denied error to obscure any sensitive // information. -func (s *Server) getApplicationEnforceRBACInformer(ctx context.Context, action, project, namespace, name string) (*appv1.Application, error) { +func (s *Server) getApplicationEnforceRBACInformer(ctx context.Context, action, project, namespace, name string) (*appv1.Application, *appv1.AppProject, error) { namespaceOrDefault := s.appNamespaceOrDefault(namespace) return s.getAppEnforceRBAC(ctx, action, project, namespaceOrDefault, name, func() (*appv1.Application, error) { return s.appLister.Applications(namespaceOrDefault).Get(name) @@ -229,9 +234,12 @@ func (s *Server) getApplicationEnforceRBACInformer(ctx context.Context, action, // getApplicationEnforceRBACClient uses a client to get an Application. If the app does not exist, permission is denied, // or any other error occurs when getting the app, we return a permission denied error to obscure any sensitive // information. -func (s *Server) getApplicationEnforceRBACClient(ctx context.Context, action, project, namespace, name, resourceVersion string) (*appv1.Application, error) { +func (s *Server) getApplicationEnforceRBACClient(ctx context.Context, action, project, namespace, name, resourceVersion string) (*appv1.Application, *appv1.AppProject, 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, }) @@ -310,7 +318,13 @@ func (s *Server) Create(ctx context.Context, q *application.ApplicationCreateReq if q.Validate != nil { validate = *q.Validate } - err := s.validateAndNormalizeApp(ctx, a, validate) + + proj, err := s.getAppProject(ctx, a, log.WithField("application", a.Name)) + if err != nil { + return nil, err + } + + err = s.validateAndNormalizeApp(ctx, a, proj, validate) if err != nil { return nil, fmt.Errorf("error while validating and normalizing app: %w", err) } @@ -366,7 +380,7 @@ func (s *Server) Create(ctx context.Context, q *application.ApplicationCreateReq return updated, nil } -func (s *Server) queryRepoServer(ctx context.Context, a *appv1.Application, action func( +func (s *Server) queryRepoServer(ctx context.Context, a *appv1.Application, proj *appv1.AppProject, action func( client apiclient.RepoServerServiceClient, repo *appv1.Repository, helmRepos []*appv1.Repository, @@ -393,14 +407,6 @@ func (s *Server) queryRepoServer(ctx context.Context, a *appv1.Application, acti if err != nil { return fmt.Errorf("error getting kustomize settings options: %w", err) } - proj, err := argo.GetAppProject(a, applisters.NewAppProjectLister(s.projInformer.GetIndexer()), s.ns, s.settingsMgr, s.db, ctx) - if err != nil { - if apierr.IsNotFound(err) { - return status.Errorf(codes.InvalidArgument, "application references project %s which does not exist", a.Spec.Project) - } - return fmt.Errorf("error getting application's project: %w", err) - } - helmRepos, err := s.db.ListHelmRepositories(ctx) if err != nil { return fmt.Errorf("error listing helm repositories: %w", err) @@ -434,7 +440,7 @@ func (s *Server) GetManifests(ctx context.Context, q *application.ApplicationMan if q.Name == nil || *q.Name == "" { return nil, fmt.Errorf("invalid request: application name is missing") } - a, err := s.getApplicationEnforceRBACInformer(ctx, rbacpolicy.ActionGet, q.GetProject(), q.GetAppNamespace(), q.GetName()) + a, proj, err := s.getApplicationEnforceRBACInformer(ctx, rbacpolicy.ActionGet, q.GetProject(), q.GetAppNamespace(), q.GetName()) if err != nil { return nil, err } @@ -446,7 +452,7 @@ func (s *Server) GetManifests(ctx context.Context, q *application.ApplicationMan } var manifestInfo *apiclient.ManifestResponse - err = s.queryRepoServer(ctx, a, func( + err = s.queryRepoServer(ctx, a, proj, func( client apiclient.RepoServerServiceClient, repo *appv1.Repository, helmRepos []*appv1.Repository, helmCreds []*appv1.RepoCreds, helmOptions *appv1.HelmOptions, kustomizeOptions *appv1.KustomizeOptions, enableGenerateManifests map[string]bool) error { revision := source.TargetRevision if q.GetRevision() != "" { @@ -532,13 +538,13 @@ func (s *Server) GetManifestsWithFiles(stream application.ApplicationService_Get return fmt.Errorf("invalid request: application name is missing") } - a, err := s.getApplicationEnforceRBACInformer(ctx, rbacpolicy.ActionGet, query.GetProject(), query.GetAppNamespace(), query.GetName()) + a, proj, err := s.getApplicationEnforceRBACInformer(ctx, rbacpolicy.ActionGet, query.GetProject(), query.GetAppNamespace(), query.GetName()) if err != nil { return err } var manifestInfo *apiclient.ManifestResponse - err = s.queryRepoServer(ctx, a, func( + err = s.queryRepoServer(ctx, a, proj, func( client apiclient.RepoServerServiceClient, repo *appv1.Repository, helmRepos []*appv1.Repository, helmCreds []*appv1.RepoCreds, helmOptions *appv1.HelmOptions, kustomizeOptions *appv1.KustomizeOptions, enableGenerateManifests map[string]bool) error { appInstanceLabelKey, err := s.settingsMgr.GetAppInstanceLabelKey() @@ -640,7 +646,7 @@ func (s *Server) Get(ctx context.Context, q *application.ApplicationQuery) (*app // 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. - a, err := s.getApplicationEnforceRBACClient(ctx, rbacpolicy.ActionGet, project, appNs, appName, q.GetResourceVersion()) + a, proj, err := s.getApplicationEnforceRBACClient(ctx, rbacpolicy.ActionGet, project, appNs, appName, q.GetResourceVersion()) if err != nil { return nil, err } @@ -671,7 +677,7 @@ func (s *Server) Get(ctx context.Context, q *application.ApplicationQuery) (*app if refreshType == appv1.RefreshTypeHard { // force refresh cached application details - if err := s.queryRepoServer(ctx, a, func( + if err := s.queryRepoServer(ctx, a, proj, func( client apiclient.RepoServerServiceClient, repo *appv1.Repository, helmRepos []*appv1.Repository, @@ -723,7 +729,7 @@ func (s *Server) Get(ctx context.Context, q *application.ApplicationQuery) (*app // ListResourceEvents returns a list of event resources func (s *Server) ListResourceEvents(ctx context.Context, q *application.ApplicationResourceEventsQuery) (*v1.EventList, error) { - a, err := s.getApplicationEnforceRBACInformer(ctx, rbacpolicy.ActionGet, q.GetProject(), q.GetAppNamespace(), q.GetName()) + a, _, err := s.getApplicationEnforceRBACInformer(ctx, rbacpolicy.ActionGet, q.GetProject(), q.GetAppNamespace(), q.GetName()) if err != nil { return nil, err } @@ -791,12 +797,12 @@ func (s *Server) validateAndUpdateApp(ctx context.Context, newApp *appv1.Applica s.projectLock.RLock(newApp.Spec.GetProject()) defer s.projectLock.RUnlock(newApp.Spec.GetProject()) - app, err := s.getApplicationEnforceRBACClient(ctx, action, currentProject, newApp.Namespace, newApp.Name, "") + app, proj, err := s.getApplicationEnforceRBACClient(ctx, action, currentProject, newApp.Namespace, newApp.Name, "") if err != nil { return nil, err } - err = s.validateAndNormalizeApp(ctx, newApp, validate) + err = s.validateAndNormalizeApp(ctx, newApp, proj, validate) if err != nil { return nil, fmt.Errorf("error validating and normalizing app: %w", err) } @@ -908,7 +914,7 @@ func (s *Server) UpdateSpec(ctx context.Context, q *application.ApplicationUpdat if q.GetSpec() == nil { return nil, fmt.Errorf("error updating application spec: spec is nil in request") } - a, err := s.getApplicationEnforceRBACClient(ctx, rbacpolicy.ActionUpdate, q.GetProject(), q.GetAppNamespace(), q.GetName(), "") + a, _, err := s.getApplicationEnforceRBACClient(ctx, rbacpolicy.ActionUpdate, q.GetProject(), q.GetAppNamespace(), q.GetName(), "") if err != nil { return nil, err } @@ -927,7 +933,7 @@ func (s *Server) UpdateSpec(ctx context.Context, q *application.ApplicationUpdat // Patch patches an application func (s *Server) Patch(ctx context.Context, q *application.ApplicationPatchRequest) (*appv1.Application, error) { - app, err := s.getApplicationEnforceRBACClient(ctx, rbacpolicy.ActionGet, q.GetProject(), q.GetAppNamespace(), q.GetName(), "") + app, _, err := s.getApplicationEnforceRBACClient(ctx, rbacpolicy.ActionGet, q.GetProject(), q.GetAppNamespace(), q.GetName(), "") if err != nil { return nil, err } @@ -970,11 +976,35 @@ func (s *Server) Patch(ctx context.Context, q *application.ApplicationPatchReque return s.validateAndUpdateApp(ctx, newApp, false, true, rbacpolicy.ActionUpdate, q.GetProject()) } +func (s *Server) getAppProject(ctx context.Context, a *appv1.Application, logCtx *log.Entry) (*appv1.AppProject, error) { + proj, err := argo.GetAppProject(a, applisters.NewAppProjectLister(s.projInformer.GetIndexer()), s.ns, s.settingsMgr, s.db, ctx) + if err == nil { + return proj, nil + } + + // If there's a permission issue or the app doesn't exist, return a vague error to avoid letting the user enumerate project names. + vagueError := status.Errorf(codes.InvalidArgument, "app is not allowed in project %q, or the project does not exist", a.Spec.Project) + + if apierr.IsNotFound(err) { + return nil, vagueError + } + + if _, ok := err.(*appv1.ErrApplicationNotAllowedToUseProject); ok { + logCtx.WithFields(map[string]interface{}{ + "project": a.Spec.Project, + argocommon.SecurityField: argocommon.SecurityMedium, + }).Warnf("error getting app project: %s", err) + return nil, vagueError + } + + return nil, vagueError +} + // Delete removes an application and all associated resources func (s *Server) Delete(ctx context.Context, q *application.ApplicationDeleteRequest) (*application.ApplicationResponse, error) { appName := q.GetName() appNs := s.appNamespaceOrDefault(q.GetAppNamespace()) - a, err := s.getApplicationEnforceRBACClient(ctx, rbacpolicy.ActionGet, q.GetProject(), appNs, appName, "") + a, _, err := s.getApplicationEnforceRBACClient(ctx, rbacpolicy.ActionGet, q.GetProject(), appNs, appName, "") if err != nil { return nil, err } @@ -1129,16 +1159,7 @@ func (s *Server) Watch(q *application.ApplicationQuery, ws application.Applicati } } -func (s *Server) validateAndNormalizeApp(ctx context.Context, app *appv1.Application, validate bool) error { - proj, err := argo.GetAppProject(app, applisters.NewAppProjectLister(s.projInformer.GetIndexer()), s.ns, s.settingsMgr, s.db, ctx) - if err != nil { - if apierr.IsNotFound(err) { - // Offer no hint that the project does not exist. - log.Warnf("User attempted to create/update application in non-existent project %q", app.Spec.Project) - return permissionDeniedErr - } - return fmt.Errorf("error getting application's project: %w", err) - } +func (s *Server) validateAndNormalizeApp(ctx context.Context, app *appv1.Application, proj *appv1.AppProject, validate bool) error { if app.GetName() == "" { return fmt.Errorf("resource name may not be empty") } @@ -1241,7 +1262,7 @@ func (s *Server) getAppResources(ctx context.Context, a *appv1.Application) (*ap } func (s *Server) getAppLiveResource(ctx context.Context, action string, q *application.ApplicationResourceRequest) (*appv1.ResourceNode, *rest.Config, *appv1.Application, error) { - a, err := s.getApplicationEnforceRBACInformer(ctx, action, q.GetProject(), q.GetAppNamespace(), q.GetName()) + a, _, err := s.getApplicationEnforceRBACInformer(ctx, action, q.GetProject(), q.GetAppNamespace(), q.GetName()) if err != nil { return nil, nil, nil, err } @@ -1378,7 +1399,7 @@ func (s *Server) DeleteResource(ctx context.Context, q *application.ApplicationR } func (s *Server) ResourceTree(ctx context.Context, q *application.ResourcesQuery) (*appv1.ApplicationTree, error) { - a, err := s.getApplicationEnforceRBACInformer(ctx, rbacpolicy.ActionGet, q.GetProject(), q.GetAppNamespace(), q.GetApplicationName()) + a, _, err := s.getApplicationEnforceRBACInformer(ctx, rbacpolicy.ActionGet, q.GetProject(), q.GetAppNamespace(), q.GetApplicationName()) if err != nil { return nil, err } @@ -1387,7 +1408,7 @@ func (s *Server) ResourceTree(ctx context.Context, q *application.ResourcesQuery } func (s *Server) WatchResourceTree(q *application.ResourcesQuery, ws application.ApplicationService_WatchResourceTreeServer) error { - _, err := s.getApplicationEnforceRBACInformer(ws.Context(), rbacpolicy.ActionGet, q.GetProject(), q.GetAppNamespace(), q.GetApplicationName()) + _, _, err := s.getApplicationEnforceRBACInformer(ws.Context(), rbacpolicy.ActionGet, q.GetProject(), q.GetAppNamespace(), q.GetApplicationName()) if err != nil { return err } @@ -1404,7 +1425,7 @@ func (s *Server) WatchResourceTree(q *application.ResourcesQuery, ws application } func (s *Server) RevisionMetadata(ctx context.Context, q *application.RevisionMetadataQuery) (*appv1.RevisionMetadata, error) { - a, err := s.getApplicationEnforceRBACInformer(ctx, rbacpolicy.ActionGet, q.GetProject(), q.GetAppNamespace(), q.GetName()) + a, proj, err := s.getApplicationEnforceRBACInformer(ctx, rbacpolicy.ActionGet, q.GetProject(), q.GetAppNamespace(), q.GetName()) if err != nil { return nil, err } @@ -1414,12 +1435,6 @@ func (s *Server) RevisionMetadata(ctx context.Context, q *application.RevisionMe if err != nil { return nil, fmt.Errorf("error getting repository by URL: %w", err) } - // We need to get some information with the project associated to the app, - // so we'll know whether GPG signatures are enforced. - proj, err := argo.GetAppProject(a, applisters.NewAppProjectLister(s.projInformer.GetIndexer()), s.ns, s.settingsMgr, s.db, ctx) - if err != nil { - return nil, fmt.Errorf("error getting app project: %w", err) - } conn, repoClient, err := s.repoClientset.NewRepoServerClient() if err != nil { return nil, fmt.Errorf("error creating repo server client: %w", err) @@ -1434,7 +1449,7 @@ func (s *Server) RevisionMetadata(ctx context.Context, q *application.RevisionMe // RevisionChartDetails returns the helm chart metadata, as fetched from the reposerver func (s *Server) RevisionChartDetails(ctx context.Context, q *application.RevisionMetadataQuery) (*appv1.ChartDetails, error) { - a, err := s.getApplicationEnforceRBACInformer(ctx, rbacpolicy.ActionGet, q.GetProject(), q.GetAppNamespace(), q.GetName()) + a, _, err := s.getApplicationEnforceRBACInformer(ctx, rbacpolicy.ActionGet, q.GetProject(), q.GetAppNamespace(), q.GetName()) if err != nil { return nil, err } @@ -1465,7 +1480,7 @@ func isMatchingResource(q *application.ResourcesQuery, key kube.ResourceKey) boo } func (s *Server) ManagedResources(ctx context.Context, q *application.ResourcesQuery) (*application.ManagedResourcesResponse, error) { - a, err := s.getApplicationEnforceRBACInformer(ctx, rbacpolicy.ActionGet, q.GetProject(), q.GetAppNamespace(), q.GetApplicationName()) + a, _, err := s.getApplicationEnforceRBACInformer(ctx, rbacpolicy.ActionGet, q.GetProject(), q.GetAppNamespace(), q.GetApplicationName()) if err != nil { return nil, err } @@ -1522,7 +1537,7 @@ func (s *Server) PodLogs(q *application.ApplicationPodLogsQuery, ws application. } } - a, err := s.getApplicationEnforceRBACInformer(ws.Context(), rbacpolicy.ActionGet, q.GetProject(), q.GetAppNamespace(), q.GetName()) + a, _, err := s.getApplicationEnforceRBACInformer(ws.Context(), rbacpolicy.ActionGet, q.GetProject(), q.GetAppNamespace(), q.GetName()) if err != nil { return err } @@ -1714,19 +1729,11 @@ func isTheSelectedOne(currentNode *appv1.ResourceNode, q *application.Applicatio // Sync syncs an application to its target state func (s *Server) Sync(ctx context.Context, syncReq *application.ApplicationSyncRequest) (*appv1.Application, error) { - a, err := s.getApplicationEnforceRBACClient(ctx, rbacpolicy.ActionGet, syncReq.GetProject(), syncReq.GetAppNamespace(), syncReq.GetName(), "") + a, proj, err := s.getApplicationEnforceRBACClient(ctx, rbacpolicy.ActionGet, syncReq.GetProject(), syncReq.GetAppNamespace(), syncReq.GetName(), "") if err != nil { return nil, err } - proj, err := argo.GetAppProject(a, applisters.NewAppProjectLister(s.projInformer.GetIndexer()), s.ns, s.settingsMgr, s.db, ctx) - if err != nil { - if apierr.IsNotFound(err) { - return a, status.Errorf(codes.InvalidArgument, "application references project %s which does not exist", a.Spec.Project) - } - return a, fmt.Errorf("error getting app project: %w", err) - } - s.inferResourcesStatusHealth(a) if !proj.Spec.SyncWindows.Matches(a).CanSync(true) { @@ -1823,7 +1830,7 @@ func (s *Server) Sync(ctx context.Context, syncReq *application.ApplicationSyncR } func (s *Server) Rollback(ctx context.Context, rollbackReq *application.ApplicationRollbackRequest) (*appv1.Application, error) { - a, err := s.getApplicationEnforceRBACClient(ctx, rbacpolicy.ActionSync, rollbackReq.GetProject(), rollbackReq.GetAppNamespace(), rollbackReq.GetName(), "") + a, _, err := s.getApplicationEnforceRBACClient(ctx, rbacpolicy.ActionSync, rollbackReq.GetProject(), rollbackReq.GetAppNamespace(), rollbackReq.GetName(), "") if err != nil { return nil, err } @@ -1882,7 +1889,7 @@ func (s *Server) Rollback(ctx context.Context, rollbackReq *application.Applicat } func (s *Server) ListLinks(ctx context.Context, req *application.ListAppLinksRequest) (*application.LinksResponse, error) { - a, err := s.getApplicationEnforceRBACClient(ctx, rbacpolicy.ActionGet, req.GetProject(), req.GetNamespace(), req.GetName(), "") + a, proj, err := s.getApplicationEnforceRBACClient(ctx, rbacpolicy.ActionGet, req.GetProject(), req.GetNamespace(), req.GetName(), "") if err != nil { return nil, err } @@ -1897,7 +1904,7 @@ func (s *Server) ListLinks(ctx context.Context, req *application.ListAppLinksReq return nil, fmt.Errorf("failed to read application deep links from configmap: %w", err) } - clstObj, _, err := s.getObjectsForDeepLinks(ctx, a) + clstObj, _, err := s.getObjectsForDeepLinks(ctx, a, proj) if err != nil { return nil, err } @@ -1912,12 +1919,7 @@ func (s *Server) ListLinks(ctx context.Context, req *application.ListAppLinksReq return finalList, nil } -func (s *Server) getObjectsForDeepLinks(ctx context.Context, app *appv1.Application) (cluster *unstructured.Unstructured, project *unstructured.Unstructured, err error) { - proj, err := argo.GetAppProject(app, applisters.NewAppProjectLister(s.projInformer.GetIndexer()), s.ns, s.settingsMgr, s.db, ctx) - if err != nil { - return nil, nil, fmt.Errorf("error getting app project: %w", err) - } - +func (s *Server) getObjectsForDeepLinks(ctx context.Context, app *appv1.Application, proj *appv1.AppProject) (cluster *unstructured.Unstructured, project *unstructured.Unstructured, err error) { // sanitize project jwt tokens proj.Status = appv1.AppProjectStatus{} @@ -1980,7 +1982,12 @@ func (s *Server) ListResourceLinks(ctx context.Context, req *application.Applica return nil, err } - clstObj, projObj, err := s.getObjectsForDeepLinks(ctx, app) + proj, err := s.getAppProject(ctx, app, log.WithField("application", app.GetName())) + if err != nil { + return nil, err + } + + clstObj, projObj, err := s.getObjectsForDeepLinks(ctx, app, proj) if err != nil { return nil, err } @@ -2036,7 +2043,7 @@ func (s *Server) resolveRevision(ctx context.Context, app *appv1.Application, sy func (s *Server) TerminateOperation(ctx context.Context, termOpReq *application.OperationTerminateRequest) (*application.OperationTerminateResponse, error) { appName := termOpReq.GetName() appNs := s.appNamespaceOrDefault(termOpReq.GetAppNamespace()) - a, err := s.getApplicationEnforceRBACClient(ctx, rbacpolicy.ActionSync, termOpReq.GetProject(), appNs, appName, "") + a, _, err := s.getApplicationEnforceRBACClient(ctx, rbacpolicy.ActionSync, termOpReq.GetProject(), appNs, appName, "") if err != nil { return nil, err } @@ -2109,7 +2116,7 @@ func (s *Server) ListResourceActions(ctx context.Context, q *application.Applica func (s *Server) getUnstructuredLiveResourceOrApp(ctx context.Context, rbacRequest string, q *application.ApplicationResourceRequest) (obj *unstructured.Unstructured, res *appv1.ResourceNode, app *appv1.Application, config *rest.Config, err error) { if q.GetKind() == applicationType.ApplicationKind && q.GetGroup() == applicationType.Group && q.GetName() == q.GetResourceName() { - app, err = s.getApplicationEnforceRBACInformer(ctx, rbacRequest, q.GetProject(), q.GetAppNamespace(), q.GetName()) + app, _, err = s.getApplicationEnforceRBACInformer(ctx, rbacRequest, q.GetProject(), q.GetAppNamespace(), q.GetName()) if err != nil { return nil, nil, nil, nil, err } @@ -2205,6 +2212,11 @@ func (s *Server) RunResourceAction(ctx context.Context, q *application.ResourceA } } + proj, err := s.getAppProject(ctx, a, log.WithField("application", a.Name)) + if err != nil { + return nil, err + } + // First, make sure all the returned resources are permitted, for each operation. // Also perform create with dry-runs for all create-operation resources. // This is performed separately to reduce the risk of only some of the resources being successfully created later. @@ -2212,7 +2224,7 @@ func (s *Server) RunResourceAction(ctx context.Context, q *application.ResourceA // the dry-run for relevant apply/delete operation would have to be invoked as well. for _, impactedResource := range newObjects { newObj := impactedResource.UnstructuredObj - err := s.verifyResourcePermitted(ctx, app, newObj) + err := s.verifyResourcePermitted(ctx, app, proj, newObj) if err != nil { return nil, err } @@ -2306,14 +2318,7 @@ func (s *Server) patchResource(ctx context.Context, config *rest.Config, liveObj return &application.ApplicationResponse{}, nil } -func (s *Server) verifyResourcePermitted(ctx context.Context, app *appv1.Application, obj *unstructured.Unstructured) error { - proj, err := argo.GetAppProject(app, applisters.NewAppProjectLister(s.projInformer.GetIndexer()), s.ns, s.settingsMgr, s.db, ctx) - if err != nil { - if apierr.IsNotFound(err) { - return fmt.Errorf("application references project %s which does not exist", app.Spec.Project) - } - return fmt.Errorf("failed to get project %s: %w", app.Spec.Project, err) - } +func (s *Server) verifyResourcePermitted(ctx context.Context, app *appv1.Application, proj *appv1.AppProject, obj *unstructured.Unstructured) error { permitted, err := proj.IsResourcePermitted(schema.GroupKind{Group: obj.GroupVersionKind().Group, Kind: obj.GroupVersionKind().Kind}, obj.GetNamespace(), app.Spec.Destination, func(project string) ([]*appv1.Cluster, error) { clusters, err := s.db.GetProjectClusters(context.TODO(), project) if err != nil { @@ -2373,16 +2378,11 @@ func splitStatusPatch(patch []byte) ([]byte, []byte, error) { } func (s *Server) GetApplicationSyncWindows(ctx context.Context, q *application.ApplicationSyncWindowsQuery) (*application.ApplicationSyncWindowsResponse, error) { - a, err := s.getApplicationEnforceRBACClient(ctx, rbacpolicy.ActionGet, q.GetProject(), q.GetAppNamespace(), q.GetName(), "") + a, proj, err := s.getApplicationEnforceRBACClient(ctx, rbacpolicy.ActionGet, q.GetProject(), q.GetAppNamespace(), q.GetName(), "") if err != nil { return nil, err } - proj, err := argo.GetAppProject(a, applisters.NewAppProjectLister(s.projInformer.GetIndexer()), s.ns, s.settingsMgr, s.db, ctx) - if err != nil { - return nil, fmt.Errorf("error getting app project: %w", err) - } - windows := proj.Spec.SyncWindows.Matches(a) sync := windows.CanSync(true) diff --git a/server/application/application_test.go b/server/application/application_test.go index bb2f2804b53df..21612e96f0ac3 100644 --- a/server/application/application_test.go +++ b/server/application/application_test.go @@ -1817,7 +1817,7 @@ func TestServer_GetApplicationSyncWindowsState(t *testing.T) { appServer := newTestAppServer(t, testApp) active, err := appServer.GetApplicationSyncWindows(context.Background(), &application.ApplicationSyncWindowsQuery{Name: &testApp.Name}) - assert.Contains(t, err.Error(), "not found") + assert.Contains(t, err.Error(), "not exist") assert.Nil(t, active) }) } @@ -2364,3 +2364,255 @@ 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" + 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, testApp, otherNsProj) + 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("Get application in other namespace when project is not 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-2"}, + }, + } + appServer := newTestAppServer(t, testApp, otherNsProj) + appServer.enabledNamespaces = []string{"argocd-1"} + app, err := appServer.Get(context.TODO(), &application.ApplicationQuery{ + Name: pointer.String("test-app"), + AppNamespace: pointer.String("argocd-1"), + }) + require.Error(t, err) + require.Nil(t, app) + require.ErrorContains(t, err, "app is not allowed in project") + }) + 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, "app is not allowed in 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") + }) + t.Run("Get application sync window in other namespace when project is 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, testApp, otherNsProj) + appServer.enabledNamespaces = []string{"argocd-1"} + active, err := appServer.GetApplicationSyncWindows(context.TODO(), &application.ApplicationSyncWindowsQuery{Name: &testApp.Name, AppNamespace: &testApp.Namespace}) + assert.NoError(t, err) + assert.Equal(t, 0, len(active.ActiveWindows)) + }) + t.Run("Get application sync window in other namespace when project is not 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-2"}, + }, + } + appServer := newTestAppServer(t, testApp, otherNsProj) + appServer.enabledNamespaces = []string{"argocd-1"} + active, err := appServer.GetApplicationSyncWindows(context.TODO(), &application.ApplicationSyncWindowsQuery{Name: &testApp.Name, AppNamespace: &testApp.Namespace}) + require.Error(t, err) + require.Nil(t, active) + require.ErrorContains(t, err, "app is not allowed in project") + }) + t.Run("Get list of links in other namespace when project is not 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-2"}, + }, + } + appServer := newTestAppServer(t, testApp, otherNsProj) + appServer.enabledNamespaces = []string{"argocd-1"} + links, err := appServer.ListLinks(context.TODO(), &application.ListAppLinksRequest{ + Name: pointer.String("test-app"), + Namespace: pointer.String("argocd-1"), + }) + require.Error(t, err) + require.Nil(t, links) + require.ErrorContains(t, err, "app is not allowed in project") + }) + t.Run("Get list of links in other namespace when project is 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, testApp, otherNsProj) + appServer.enabledNamespaces = []string{"argocd-1"} + links, err := appServer.ListLinks(context.TODO(), &application.ListAppLinksRequest{ + Name: pointer.String("test-app"), + Namespace: pointer.String("argocd-1"), + }) + require.NoError(t, err) + assert.Equal(t, 0, len(links.Items)) + }) +} diff --git a/util/argo/argo.go b/util/argo/argo.go index b32369ea70c48..62405655a7b18 100644 --- a/util/argo/argo.go +++ b/util/argo/argo.go @@ -689,8 +689,7 @@ func GetAppProject(app *argoappv1.Application, projLister applicationsv1.AppProj return nil, err } if !proj.IsAppNamespacePermitted(app, ns) { - return nil, fmt.Errorf("application '%s' in namespace '%s' is not allowed to use project '%s'", - app.Name, app.Namespace, proj.Name) + return nil, argoappv1.NewErrApplicationNotAllowedToUseProject(app.Name, app.Namespace, proj.Name) } return proj, nil }