Validate user is member of one of existing admin groups

.vscode/launch.json

@@ -20,6 +20,8 @@
                 "RADIX_ACTIVE_CLUSTER_EGRESS_IPS": "104.45.84.1",
                 "REQUIRE_APP_CONFIGURATION_ITEM": "true",
                 "REQUIRE_APP_AD_GROUPS": "true",
+                "RADIX_ENVIRONMENT":"qa",
+                "RADIX_APP":"radix-api"
             },
             "args": [
                 "--useOutClusterClient=false"

api/applications/applications_controller_test.go

@@ -91,7 +91,7 @@ func TestGetApplications_HasAccessToSomeRR(t *testing.T) {
 			NewApplicationController(
 				func(_ context.Context, _ kubernetes.Interface, _ v1.RadixRegistration) (bool, error) {
 					return false, nil
-				}, NewApplicationHandlerFactory(ApplicationHandlerConfig{true, true})))
+				}, NewApplicationHandlerFactory(ApplicationHandlerConfig{RequireAppConfigurationItem: true, RequireAppADGroups: true})))
 		responseChannel := controllerTestUtils.ExecuteRequest("GET", "/api/v1/applications")
 		response := <-responseChannel
 
@@ -104,7 +104,7 @@ func TestGetApplications_HasAccessToSomeRR(t *testing.T) {
 		controllerTestUtils := controllertest.NewTestUtils(kubeclient, radixclient, secretproviderclient, NewApplicationController(
 			func(_ context.Context, _ kubernetes.Interface, rr v1.RadixRegistration) (bool, error) {
 				return rr.GetName() == "my-second-app", nil
-			}, NewApplicationHandlerFactory(ApplicationHandlerConfig{true, true})))
+			}, NewApplicationHandlerFactory(ApplicationHandlerConfig{RequireAppConfigurationItem: true, RequireAppADGroups: true})))
 		responseChannel := controllerTestUtils.ExecuteRequest("GET", "/api/v1/applications")
 		response := <-responseChannel
 
@@ -117,7 +117,7 @@ func TestGetApplications_HasAccessToSomeRR(t *testing.T) {
 		controllerTestUtils := controllertest.NewTestUtils(kubeclient, radixclient, secretproviderclient, NewApplicationController(
 			func(_ context.Context, _ kubernetes.Interface, _ v1.RadixRegistration) (bool, error) {
 				return true, nil
-			}, NewApplicationHandlerFactory(ApplicationHandlerConfig{true, true})))
+			}, NewApplicationHandlerFactory(ApplicationHandlerConfig{RequireAppConfigurationItem: true, RequireAppADGroups: true})))
 		responseChannel := controllerTestUtils.ExecuteRequest("GET", "/api/v1/applications")
 		response := <-responseChannel
 
@@ -185,7 +185,7 @@ func TestSearchApplications(t *testing.T) {
 	controllerTestUtils := controllertest.NewTestUtils(kubeclient, radixclient, secretproviderclient, NewApplicationController(
 		func(_ context.Context, _ kubernetes.Interface, _ v1.RadixRegistration) (bool, error) {
 			return true, nil
-		}, NewApplicationHandlerFactory(ApplicationHandlerConfig{true, true})))
+		}, NewApplicationHandlerFactory(ApplicationHandlerConfig{RequireAppConfigurationItem: true, RequireAppADGroups: true})))
 
 	// Tests
 	t.Run("search for "+appNames[0], func(t *testing.T) {
@@ -259,7 +259,7 @@ func TestSearchApplications(t *testing.T) {
 		controllerTestUtils := controllertest.NewTestUtils(kubeclient, radixclient, secretproviderclient, NewApplicationController(
 			func(_ context.Context, _ kubernetes.Interface, _ v1.RadixRegistration) (bool, error) {
 				return false, nil
-			}, NewApplicationHandlerFactory(ApplicationHandlerConfig{true, true})))
+			}, NewApplicationHandlerFactory(ApplicationHandlerConfig{RequireAppConfigurationItem: true, RequireAppADGroups: true})))
 		searchParam := applicationModels.ApplicationsSearchRequest{Names: []string{appNames[0]}}
 		responseChannel := controllerTestUtils.ExecuteRequestWithParameters("POST", "/api/v1/applications/_search", &searchParam)
 		response := <-responseChannel

api/applications/applications_handler.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
-	"k8s.io/client-go/kubernetes/fake"
 	"net/http"
 	"strings"
 	"time"
@@ -17,22 +16,27 @@ import (
 	"github.com/equinor/radix-api/api/kubequery"
 	apimodels "github.com/equinor/radix-api/api/models"
 	"github.com/equinor/radix-api/api/utils"
+	"github.com/equinor/radix-api/api/utils/access"
 	"github.com/equinor/radix-api/models"
 	radixhttp "github.com/equinor/radix-common/net/http"
 	radixutils "github.com/equinor/radix-common/utils"
 	"github.com/equinor/radix-common/utils/slice"
 	"github.com/equinor/radix-operator/pkg/apis/applicationconfig"
 	"github.com/equinor/radix-operator/pkg/apis/defaults"
+	"github.com/equinor/radix-operator/pkg/apis/defaults/k8s"
 	jobPipeline "github.com/equinor/radix-operator/pkg/apis/pipeline"
 	v1 "github.com/equinor/radix-operator/pkg/apis/radix/v1"
 	"github.com/equinor/radix-operator/pkg/apis/radixvalidators"
 	crdUtils "github.com/equinor/radix-operator/pkg/apis/utils"
 	log "github.com/sirupsen/logrus"
-	corev1auth "k8s.io/api/authorization/v1"
+	authorizationapi "k8s.io/api/authorization/v1"
 	corev1 "k8s.io/api/core/v1"
+	rbacv1 "k8s.io/api/rbac/v1"
 	k8serrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/client-go/kubernetes"
+	"k8s.io/client-go/kubernetes/fake"
 )
 
 type patch struct {
@@ -47,6 +51,7 @@ type ApplicationHandler struct {
 	environmentHandler environments.EnvironmentHandler
 	accounts           models.Accounts
 	config             ApplicationHandlerConfig
+	namespace          string
 }
 
 // NewApplicationHandler Constructor
@@ -56,7 +61,15 @@ func NewApplicationHandler(accounts models.Accounts, config ApplicationHandlerCo
 		jobHandler:         job.Init(accounts, deployments.Init(accounts)),
 		environmentHandler: environments.Init(environments.WithAccounts(accounts)),
 		config:             config,
+		namespace:          getApiNamespace(config),
+	}
+}
+
+func getApiNamespace(config ApplicationHandlerConfig) string {
+	if namespace := crdUtils.GetEnvironmentNamespace(config.AppName, config.EnvironmentName); len(namespace) > 0 {
+		return namespace
 	}
+	panic("missing RADIX_APP or RADIX_ENVIRONMENT environment variables")
 }
 
 func (ah *ApplicationHandler) getUserAccount() models.Account {
@@ -179,6 +192,10 @@ func (ah *ApplicationHandler) RegisterApplication(ctx context.Context, applicati
 			return upsertResponse, err
 		}
 	}
+	err = ah.validateUserIsMemberOfAdGroups(ctx, applicationRegistrationRequest.ApplicationRegistration.Name, applicationRegistrationRequest.ApplicationRegistration.AdGroups)
+	if err != nil {
+		return nil, err
+	}
 
 	radixRegistration, err = ah.getUserAccount().RadixClient.RadixV1().RadixRegistrations().Create(ctx, radixRegistration, metav1.CreateOptions{})
 	if err != nil {
@@ -277,7 +294,7 @@ func (ah *ApplicationHandler) ModifyRegistrationDetails(ctx context.Context, app
 		return nil, err
 	}
 
-	payload := []patch{}
+	payload := make([]patch, 0)
 	runUpdate := false
 	updatedRegistration := currentRegistration.DeepCopy()
 
@@ -355,6 +372,10 @@ func (ah *ApplicationHandler) ModifyRegistrationDetails(ctx context.Context, app
 	}
 
 	if runUpdate {
+		err := ah.validateUserIsMemberOfAdGroups(ctx, appName, updatedRegistration.Spec.AdGroups)
+		if err != nil {
+			return nil, err
+		}
 		err = ah.isValidRegistrationUpdate(updatedRegistration, currentRegistration)
 		if err != nil {
 			return nil, err
@@ -554,7 +575,7 @@ func (ah *ApplicationHandler) getRegistrationUpdateWarnings(radixRegistration *v
 }
 
 func (ah *ApplicationHandler) isValidRegistrationInsert(radixRegistration *v1.RadixRegistration) error {
-	// Need to use in-cluster client of the API server, because the user might not have enough priviledges
+	// Need to use in-cluster client of the API server, because the user might not have enough privileges
 	// to run a full validation
 	return radixvalidators.CanRadixRegistrationBeInserted(ah.getServiceAccount().RadixClient, radixRegistration, ah.getAdditionalRadixRegistrationInsertValidators()...)
 }
@@ -713,9 +734,9 @@ func (ah *ApplicationHandler) userIsAppAdmin(ctx context.Context, appName string
 	case *fake.Clientset:
 		return true, nil
 	default:
-		review, err := ah.accounts.UserAccount.Client.AuthorizationV1().SelfSubjectAccessReviews().Create(ctx, &corev1auth.SelfSubjectAccessReview{
-			Spec: corev1auth.SelfSubjectAccessReviewSpec{
-				ResourceAttributes: &corev1auth.ResourceAttributes{
+		review, err := ah.accounts.UserAccount.Client.AuthorizationV1().SelfSubjectAccessReviews().Create(ctx, &authorizationapi.SelfSubjectAccessReview{
+			Spec: authorizationapi.SelfSubjectAccessReviewSpec{
+				ResourceAttributes: &authorizationapi.ResourceAttributes{
 					Verb:     "patch",
 					Group:    "radix.equinor.com",
 					Resource: "radixregistrations",
@@ -727,6 +748,54 @@ func (ah *ApplicationHandler) userIsAppAdmin(ctx context.Context, appName string
 	}
 }
 
+func (ah *ApplicationHandler) validateUserIsMemberOfAdGroups(ctx context.Context, appName string, adGroups []string) error {
+	if len(adGroups) == 0 {
+		if ah.config.RequireAppADGroups {
+			return userShouldBeMemberOfAdminAdGroupError()
+		}
+		return nil
+	}
+	name := fmt.Sprintf("access-validation-%s", appName)
+	labels := map[string]string{"radix-access-validation": "true"}
+	configMapName := fmt.Sprintf("%s-%s", name, strings.ToLower(crdUtils.RandString(6)))
+	role, err := createRoleToGetConfigMap(ctx, ah.accounts.ServiceAccount.Client, ah.namespace, name, labels, configMapName)
+	if err != nil {
+		return err
+	}
+	defer func() {
+		err = deleteRole(context.Background(), ah.accounts.ServiceAccount.Client, ah.namespace, role.GetName())
+		if err != nil {
+			log.Warnf("Failed to delete role %s: %v", role.GetName(), err)
+		}
+	}()
+	roleBinding, err := createRoleBindingForRole(ctx, ah.accounts.ServiceAccount.Client, ah.namespace, role, name, adGroups, labels)
+	if err != nil {
+		return err
+	}
+	defer func() {
+		err = deleteRoleBinding(context.Background(), ah.accounts.ServiceAccount.Client, ah.namespace, roleBinding.GetName())
+		if err != nil {
+			log.Warnf("Failed to delete role binding %s: %v", roleBinding.GetName(), err)
+		}
+	}()
+
+	valid, err := access.HasAccess(ctx, ah.accounts.UserAccount.Client, &authorizationapi.ResourceAttributes{
+		Verb:      "get",
+		Group:     "",
+		Resource:  "configmaps",
+		Version:   "*",
+		Namespace: ah.namespace,
+		Name:      configMapName,
+	})
+	if err != nil {
+		return err
+	}
+	if !valid {
+		return userShouldBeMemberOfAdminAdGroupError()
+	}
+	return nil
+}
+
 func setConfigBranchToFallbackWhenEmpty(existingRegistration *v1.RadixRegistration) bool {
 	// HACK ConfigBranch is required, so we set it to "master" if empty to support existing apps registered before ConfigBranch was introduced
 	if len(strings.TrimSpace(existingRegistration.Spec.ConfigBranch)) > 0 {
@@ -735,3 +804,57 @@ func setConfigBranchToFallbackWhenEmpty(existingRegistration *v1.RadixRegistrati
 	existingRegistration.Spec.ConfigBranch = applicationconfig.ConfigBranchFallback
 	return true
 }
+
+func createRoleToGetConfigMap(ctx context.Context, kubeClient kubernetes.Interface, namespace, roleName string, labels map[string]string, configMapName string) (*rbacv1.Role, error) {
+	return kubeClient.RbacV1().Roles(namespace).Create(ctx, &rbacv1.Role{
+		ObjectMeta: metav1.ObjectMeta{GenerateName: roleName, Labels: labels},
+		Rules: []rbacv1.PolicyRule{{
+			Verbs:         []string{"get"},
+			APIGroups:     []string{""},
+			Resources:     []string{"configmaps"},
+			ResourceNames: []string{configMapName},
+		}},
+	}, metav1.CreateOptions{})
+}
+
+func deleteRole(ctx context.Context, kubeClient kubernetes.Interface, namespace, roleName string) error {
+	deletionPropagation := metav1.DeletePropagationBackground
+	return kubeClient.RbacV1().Roles(namespace).Delete(ctx, roleName, metav1.DeleteOptions{
+		PropagationPolicy: &deletionPropagation,
+	})
+}
+
+func createRoleBindingForRole(ctx context.Context, kubeClient kubernetes.Interface, namespace string, role *rbacv1.Role, roleBindingName string, adGroups []string, labels map[string]string) (*rbacv1.RoleBinding, error) {
+	var subjects []rbacv1.Subject
+	for _, adGroup := range adGroups {
+		subjects = append(subjects, rbacv1.Subject{
+			Kind:     k8s.KindGroup,
+			Name:     adGroup,
+			APIGroup: k8s.RbacApiGroup,
+		})
+	}
+	newRoleBinding := &rbacv1.RoleBinding{
+		ObjectMeta: metav1.ObjectMeta{
+			GenerateName: roleBindingName,
+			Labels:       labels,
+			OwnerReferences: []metav1.OwnerReference{
+				{
+					APIVersion: k8s.RbacApiVersion,
+					Kind:       k8s.KindRole,
+					Name:       role.GetName(),
+					UID:        role.GetUID(),
+				},
+			},
+		},
+		RoleRef: rbacv1.RoleRef{
+			APIGroup: k8s.RbacApiGroup,
+			Kind:     k8s.KindRole,
+			Name:     role.GetName(),
+		}, Subjects: subjects,
+	}
+	return kubeClient.RbacV1().RoleBindings(namespace).Create(ctx, newRoleBinding, metav1.CreateOptions{})
+}
+
+func deleteRoleBinding(ctx context.Context, kubeClient kubernetes.Interface, namespace, roleBindingName string) error {
+	return kubeClient.RbacV1().RoleBindings(namespace).Delete(ctx, roleBindingName, metav1.DeleteOptions{})
+}

api/applications/applications_handler_config.go

@@ -27,14 +27,18 @@ func LoadApplicationHandlerConfig(args []string) (ApplicationHandlerConfig, erro
 }
 
 type ApplicationHandlerConfig struct {
-	RequireAppConfigurationItem bool `cfg:"require_app_configuration_item" flag:"require-app-configuration-item"`
-	RequireAppADGroups          bool `cfg:"require_app_ad_groups" flag:"require-app-ad-groups"`
+	RequireAppConfigurationItem bool   `cfg:"require_app_configuration_item" flag:"require-app-configuration-item"`
+	RequireAppADGroups          bool   `cfg:"require_app_ad_groups" flag:"require-app-ad-groups"`
+	AppName                     string `cfg:"radix_app" flag:"radix-app"`
+	EnvironmentName             string `cfg:"radix_environment" flag:"radix-environment"`
 }
 
 func ApplicationHandlerConfigFlagSet() *pflag.FlagSet {
 	flagset := pflag.NewFlagSet("config", pflag.ExitOnError)
 	flagset.ParseErrorsWhitelist = pflag.ParseErrorsWhitelist{UnknownFlags: true}
 	flagset.Bool("require-app-configuration-item", true, "Require configuration item for application")
 	flagset.Bool("require-app-ad-groups", true, "Require AD groups for application")
+	flagset.String("radix-app", "", "Application name")
+	flagset.String("radix-environment", "", "Environment name")
 	return flagset
 }

api/applications/errors.go

@@ -0,0 +1,7 @@
+package applications
+
+import radixhttp "github.com/equinor/radix-common/net/http"
+
+func userShouldBeMemberOfAdminAdGroupError() error {
+	return radixhttp.ValidationError("Radix Registration", "User should be a member of at least one admin AD group or their sub-members")
+}

api/applications/get_applications_handler.go

@@ -10,6 +10,7 @@ import (
 	deploymentModels "github.com/equinor/radix-api/api/deployments/models"
 	environmentModels "github.com/equinor/radix-api/api/environments/models"
 	jobModels "github.com/equinor/radix-api/api/jobs/models"
+	"github.com/equinor/radix-api/api/utils/access"
 	"golang.org/x/sync/errgroup"
 
 	authorizationapi "k8s.io/api/authorization/v1"
@@ -212,25 +213,11 @@ func (ah *ApplicationHandler) filterRadixRegByAccess(ctx context.Context, radixr
 
 // cannot run as test - does not return correct values
 func hasAccess(ctx context.Context, client kubernetes.Interface, rr v1.RadixRegistration) (bool, error) {
-	sar := authorizationapi.SelfSubjectAccessReview{
-		Spec: authorizationapi.SelfSubjectAccessReviewSpec{
-			ResourceAttributes: &authorizationapi.ResourceAttributes{
-				Verb:     "get",
-				Group:    "radix.equinor.com",
-				Resource: "radixregistrations",
-				Version:  "*",
-				Name:     rr.GetName(),
-			},
-		},
-	}
-
-	r, err := postSelfSubjectAccessReviews(ctx, client, sar)
-	if err != nil {
-		return false, err
-	}
-	return r.Status.Allowed, nil
-}
-
-func postSelfSubjectAccessReviews(ctx context.Context, client kubernetes.Interface, sar authorizationapi.SelfSubjectAccessReview) (*authorizationapi.SelfSubjectAccessReview, error) {
-	return client.AuthorizationV1().SelfSubjectAccessReviews().Create(ctx, &sar, metav1.CreateOptions{})
+	return access.HasAccess(ctx, client, &authorizationapi.ResourceAttributes{
+		Verb:     "get",
+		Group:    "radix.equinor.com",
+		Resource: "radixregistrations",
+		Version:  "*",
+		Name:     rr.GetName(),
+	})
 }

api/applications/models/application_registration_patch.go

@@ -11,7 +11,7 @@ type ApplicationRegistrationPatch struct {
 	// ReaderAdGroups the groups that should be able to read the application
 	//
 	// required: false
-	ReaderAdGroups *[]string `json:"ReaderAdGroups,omitempty"`
+	ReaderAdGroups *[]string `json:"readerAdGroups,omitempty"`
 
 	// Owner of the application - should be an email
 	//

api/utils/access/access.go

@@ -0,0 +1,29 @@
+package access
+
+import (
+	"context"
+
+	authorizationapi "k8s.io/api/authorization/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/client-go/kubernetes"
+)
+
+// HasAccess checks if user has access to a resource
+// cannot run as test - does not return correct values
+func HasAccess(ctx context.Context, client kubernetes.Interface, resourceAttributes *authorizationapi.ResourceAttributes) (bool, error) {
+	sar := authorizationapi.SelfSubjectAccessReview{
+		Spec: authorizationapi.SelfSubjectAccessReviewSpec{
+			ResourceAttributes: resourceAttributes,
+		},
+	}
+
+	r, err := postSelfSubjectAccessReviews(ctx, client, sar)
+	if err != nil {
+		return false, err
+	}
+	return r.Status.Allowed, nil
+}
+
+func postSelfSubjectAccessReviews(ctx context.Context, client kubernetes.Interface, sar authorizationapi.SelfSubjectAccessReview) (*authorizationapi.SelfSubjectAccessReview, error) {
+	return client.AuthorizationV1().SelfSubjectAccessReviews().Create(ctx, &sar, metav1.CreateOptions{})
+}