Switch from one auth policy to multiple

pkg/resourcegenerator/istio/authorizationpolicy/authorization_policy.go

@@ -6,9 +6,11 @@ import (
 	"github.com/kartverket/skiperator/pkg/reconciliation"
 	"github.com/kartverket/skiperator/pkg/util"
 	securityv1api "istio.io/api/security/v1"
+	"istio.io/api/security/v1beta1"
 	typev1beta1 "istio.io/api/type/v1beta1"
 	securityv1 "istio.io/client-go/pkg/apis/security/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/types"
 )
 
 func Generate(r reconciliation.Reconciliation) error {
@@ -24,19 +26,105 @@ func Generate(r reconciliation.Reconciliation) error {
 	}
 	ctxLog.Debug("Attempting to generate AuthorizationPolicy for application", "application", application.Name)
 
-	defaultDenyPaths := []string{
-		"/actuator*",
+	if application.Spec.AuthorizationSettings != nil {
+		// Do not create an AuthorizationPolicy if allowAll is set to true
+		if application.Spec.AuthorizationSettings.AllowAll == true {
+			return nil
+		}
 	}
-	authPolicy := getAuthorizationPolicy(application, defaultDenyPaths, r.GetAuthConfigs())
+	defaultDenyPath := []string{"/actuator*"}
 
-	ctxLog.Debug("Finished generating AuthorizationPolicy for application", "application", application.Name)
-	if authPolicy != nil {
-		r.AddResource(authPolicy)
+	allowPaths := []string{}
+	if application.Spec.AuthorizationSettings != nil {
+		if application.Spec.AuthorizationSettings.AllowList != nil {
+			if len(application.Spec.AuthorizationSettings.AllowList) > 0 {
+				allowPaths = application.Spec.AuthorizationSettings.AllowList
+			}
+		}
+	}
+	authConfigs := r.GetAuthConfigs()
+	if authConfigs != nil {
+		if len(*authConfigs) > 0 {
+			for _, authConfig := range *authConfigs {
+				if authConfig.NotPaths != nil {
+					allowPaths = append(allowPaths, *authConfig.NotPaths...)
+				}
+			}
+			r.AddResource(
+				getJwtValidationAuthPolicy(
+					types.NamespacedName{
+						Namespace: application.Namespace,
+						Name:      application.Name + "-jwt-auth",
+					},
+					application.Name,
+					*authConfigs,
+					allowPaths,
+					defaultDenyPath,
+				),
+			)
+		}
 	}
 
+	// Generate an AuthorizationPolicy that allows requests to the list of paths in allowPaths
+	if len(allowPaths) > 0 {
+		r.AddResource(
+			getGeneralAuthPolicy(
+				types.NamespacedName{
+					Name:      application.Name + "-allow-paths",
+					Namespace: application.Namespace,
+				},
+				application.Name,
+				securityv1api.AuthorizationPolicy_ALLOW,
+				allowPaths,
+				[]string{},
+			),
+		)
+	} else {
+		r.AddResource(
+			getGeneralAuthPolicy(
+				types.NamespacedName{
+					Name:      application.Name + "-default-deny",
+					Namespace: application.Namespace,
+				},
+				application.Name,
+				securityv1api.AuthorizationPolicy_DENY,
+				defaultDenyPath,
+				allowPaths,
+			),
+		)
+	}
+	ctxLog.Debug("Finished generating AuthorizationPolicy for application", "application", application.Name)
 	return nil
 }
 
+func getGeneralAuthPolicy(namespacedName types.NamespacedName, applicationName string, action v1beta1.AuthorizationPolicy_Action, paths []string, notPaths []string) *securityv1.AuthorizationPolicy {
+	return &securityv1.AuthorizationPolicy{
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: namespacedName.Namespace,
+			Name:      namespacedName.Name,
+		},
+		Spec: securityv1api.AuthorizationPolicy{
+			Action: action,
+			Rules: []*securityv1api.Rule{
+				{
+					To: []*securityv1api.Rule_To{
+						{
+							Operation: &securityv1api.Operation{
+								Paths:    paths,
+								NotPaths: notPaths,
+							},
+						},
+					},
+					From: getGeneralFromRule(),
+				},
+			},
+			Selector: &typev1beta1.WorkloadSelector{
+				MatchLabels: util.GetPodAppSelector(applicationName),
+			},
+		},
+	}
+}
+
 func getGeneralFromRule() []*securityv1api.Rule_From {
 	return []*securityv1api.Rule_From{
 		{
@@ -47,76 +135,40 @@ func getGeneralFromRule() []*securityv1api.Rule_From {
 	}
 }
 
-func getAuthorizationPolicy(application *skiperatorv1alpha1.Application, denyPaths []string, authConfigs *[]reconciliation.AuthConfig) *securityv1.AuthorizationPolicy {
-	authPolicyRules := []*securityv1api.Rule{
-		{
+func getJwtValidationAuthPolicy(namespacedName types.NamespacedName, applicationName string, authConfigs []reconciliation.AuthConfig, allowPaths []string, denyPaths []string) *securityv1.AuthorizationPolicy {
+	var authPolicyRules []*securityv1api.Rule
+
+	notPaths := allowPaths
+	notPaths = append(allowPaths, denyPaths...)
+	for _, authConfig := range authConfigs {
+		authPolicyRules = append(authPolicyRules, &securityv1api.Rule{
 			To: []*securityv1api.Rule_To{
 				{
-					Operation: &securityv1api.Operation{},
+					Operation: &securityv1api.Operation{
+						NotPaths: notPaths,
+					},
+				},
+			},
+			When: []*securityv1api.Condition{
+				{
+					Key:    "request.auth.claims[iss]",
+					Values: []string{authConfig.ProviderURIs.IssuerURI},
 				},
 			},
 			From: getGeneralFromRule(),
-		},
-	}
-
-	if application.Spec.AuthorizationSettings != nil {
-		if application.Spec.AuthorizationSettings.AllowAll == true {
-			return nil
-		}
-		if len(application.Spec.AuthorizationSettings.AllowList) > 0 {
-			operation := authPolicyRules[0].To[0].Operation
-			for _, endpoint := range application.Spec.AuthorizationSettings.AllowList {
-				operation.Paths = append(operation.Paths, endpoint)
-			}
-			authPolicyRules[0].To[0].Operation = operation
-		} else {
-			authPolicyRules[0].To[0].Operation.NotPaths = denyPaths
-		}
-	} else {
-		authPolicyRules[0].To[0].Operation.NotPaths = denyPaths
-	}
-
-	if authConfigs != nil {
-		authPolicyRules = append(authPolicyRules, getJwtValidationRule(*authConfigs)...)
+		})
 	}
-
 	return &securityv1.AuthorizationPolicy{
 		ObjectMeta: metav1.ObjectMeta{
-			Namespace: application.Namespace,
-			Name:      application.Name + "-auth-policy",
+			Namespace: namespacedName.Namespace,
+			Name:      namespacedName.Name,
 		},
 		Spec: securityv1api.AuthorizationPolicy{
 			Action: securityv1api.AuthorizationPolicy_ALLOW,
 			Rules:  authPolicyRules,
 			Selector: &typev1beta1.WorkloadSelector{
-				MatchLabels: util.GetPodAppSelector(application.Name),
+				MatchLabels: util.GetPodAppSelector(applicationName),
 			},
 		},
 	}
 }
-
-func getJwtValidationRule(authConfigs []reconciliation.AuthConfig) []*securityv1api.Rule {
-	authPolicyRules := make([]*securityv1api.Rule, 0)
-	for _, authConfig := range authConfigs {
-		ruleTo := &securityv1api.Rule_To{}
-		if authConfig.NotPaths != nil {
-			ruleTo.Operation = &securityv1api.Operation{
-				NotPaths: *authConfig.NotPaths,
-			}
-		} else {
-			ruleTo.Operation = &securityv1api.Operation{
-				Paths: []string{"*"},
-			}
-		}
-		authPolicyRules = append(authPolicyRules, &securityv1api.Rule{
-			To: []*securityv1api.Rule_To{ruleTo},
-			When: []*securityv1api.Condition{
-				{
-					Key:    "request.auth.claims[iss]",
-					Values: []string{authConfig.ProviderURIs.IssuerURI},
-				},
-			},
-		})
-	}
-	return authPolicyRules
-}

tests/application/authorization-policy/application-assert.yaml

@@ -1,16 +1,17 @@
 apiVersion: security.istio.io/v1
 kind: AuthorizationPolicy
 metadata:
-  name: application-auth-policy
+  name: application-default-deny
 spec:
+  action: DENY
   rules:
     - from:
         - source:
             namespaces:
               - istio-gateways
       to:
         - operation:
-            notPaths:
+            paths:
               - /actuator*
   selector:
     matchLabels:

tests/application/authorization-settings/multiple-application-assert.yaml

@@ -1,16 +1,17 @@
 apiVersion: security.istio.io/v1
 kind: AuthorizationPolicy
 metadata:
-  name: default-auth-policy
+  name: default-default-deny
 spec:
+  action: DENY
   rules:
     - from:
         - source:
             namespaces:
               - istio-gateways
       to:
         - operation:
-            notPaths:
+            paths:
               - /actuator*
   selector:
     matchLabels:
@@ -19,7 +20,7 @@ spec:
 apiVersion: security.istio.io/v1
 kind: AuthorizationPolicy
 metadata:
-  name: allow-list-auth-policy
+  name: allow-list-allow-paths
 spec:
   rules:
     - from:

tests/application/authorization-settings/multiple-application-errors.yaml

@@ -1,4 +1,9 @@
 apiVersion: security.istio.io/v1
 kind: AuthorizationPolicy
 metadata:
-  name: allow-all-auth-policy
+  name: allow-all-default-deny
+---
+apiVersion: security.istio.io/v1
+kind: AuthorizationPolicy
+metadata:
+  name: allow-all-allow-paths

tests/application/authorization-settings/patch-application-assert.yaml

@@ -2,7 +2,7 @@
 apiVersion: security.istio.io/v1
 kind: AuthorizationPolicy
 metadata:
-  name: allow-list-auth-policy
+  name: allow-list-allow-paths
 spec:
   rules:
     - from:
@@ -21,16 +21,17 @@ spec:
 apiVersion: security.istio.io/v1
 kind: AuthorizationPolicy
 metadata:
-  name: allow-all-auth-policy
+  name: allow-all-default-deny
 spec:
+  action: DENY
   rules:
     - from:
         - source:
             namespaces:
               - istio-gateways
       to:
         - operation:
-            notPaths:
+            paths:
               - /actuator*
   selector:
     matchLabels:

tests/application/authorization-settings/patch-application-errors.yaml

@@ -1,8 +1,9 @@
 apiVersion: security.istio.io/v1
 kind: AuthorizationPolicy
 metadata:
-  name: allow-list-auth-policy
+  name: allow-list-default-deny
 spec:
+  action: DENY
   rules:
     - from:
         - source:
@@ -11,5 +12,7 @@ spec:
       to:
         - operation:
             paths:
+              - /actuator*
+            notPaths:
               - /actuator/health
               - /actuator/info

tests/application/idporten/application-assert.yaml

@@ -1,24 +1,24 @@
 apiVersion: apps/v1
 kind: Deployment
 metadata:
-  name: idporten-test-client
+  name: application
 spec:
   selector:
     matchLabels:
-      app: idporten-test-client
+      app: application
   template:
     spec:
       containers:
-        - name: idporten-test-client
+        - name: application
           volumeMounts:
             - mountPath: /tmp
               name: tmp
             - mountPath: /var/run/secrets/skip/idporten
-              name: "idporten-idporten-test-client-5dd0f829"
+              name: "idporten-application-4749490d"
       volumes:
         - emptyDir: {}
           name: tmp
-        - name: "idporten-idporten-test-client-5dd0f829"
+        - name: "idporten-application-4749490d"
           secret:
             defaultMode: 420
-            secretName: "idporten-idporten-test-client-5dd0f829"
+            secretName: "idporten-application-4749490d"

tests/application/idporten/application-idporten-assert.yaml

@@ -1,7 +1,7 @@
 apiVersion: nais.io/v1
 kind: IDPortenClient
 metadata:
-  name: idporten-test-client
+  name: application
 spec:
   scopes:
     - "openid"