Added tests

kartverket · Jan 21, 2025 · 668c022 · 668c022
1 parent c09baed
commit 668c022
Show file tree

Hide file tree

Showing 16 changed files with 262 additions and 49 deletions.
diff --git a/internal/controllers/common/reconciler.go b/internal/controllers/common/reconciler.go
@@ -222,16 +222,24 @@ func (r *ReconcilerBase) GetAuthConfigsForApplication(ctx context.Context, appli
 
 func getIdentityProviderInfoWithAuthenticationEnabled(ctx context.Context, application *v1alpha1.Application, k8sClient client.Client) (*[]reconciliation.IdentityProviderInfo, error) {
 	var providerInfo []reconciliation.IdentityProviderInfo
-	if application.Spec.IDPorten != nil && application.Spec.IDPorten.Enabled && application.Spec.IDPorten.Authentication != nil && application.Spec.IDPorten.Authentication.Enabled {
+	if application.Spec.IDPorten != nil && application.Spec.IDPorten.Authentication != nil && application.Spec.IDPorten.Authentication.Enabled {
 		var secretName *string
 		var err error
 		if application.Spec.IDPorten.Authentication.SecretName != nil {
+			// If secret name is provided, use it regardless of whether IDPorten is enabled
 			secretName = application.Spec.IDPorten.Authentication.SecretName
+		} else if application.Spec.IDPorten.Enabled {
+			// If IDPorten is enabled but no secretName provided, retrieve the generated secret from IDPortenClient
+			secretName, err = getSecretNameForIdentityProvider(k8sClient, ctx,
+				types.NamespacedName{
+					Namespace: application.Namespace,
+					Name:      application.Name,
+				},
+				reconciliation.ID_PORTEN,
+				application.UID)
 		} else {
-			secretName, err = getSecretNameForIdentityProvider(k8sClient, ctx, types.NamespacedName{
-				Namespace: application.Namespace,
-				Name:      application.Name,
-			}, reconciliation.ID_PORTEN, application.UID)
+			// If IDPorten is not enabled and no secretName provided, return error
+			return nil, fmt.Errorf("JWT authentication requires either IDPorten to be enabled or a secretName to be provided")
 		}
 		if err != nil {
 			err := fmt.Errorf("failed to get secret name for IDPortenClient: %w", err)
@@ -250,19 +258,33 @@ func getIdentityProviderInfoWithAuthenticationEnabled(ctx context.Context, appli
 			NotPaths:   notPaths,
 		})
 	}
-	if application.Spec.Maskinporten != nil && application.Spec.Maskinporten.Enabled && application.Spec.Maskinporten.Authentication != nil && application.Spec.Maskinporten.Authentication.Enabled == true {
-		secretName, err := getSecretNameForIdentityProvider(k8sClient, ctx, types.NamespacedName{
-			Namespace: application.Namespace,
-			Name:      application.Name,
-		}, reconciliation.MASKINPORTEN, application.UID)
+	if application.Spec.Maskinporten != nil && application.Spec.Maskinporten.Authentication != nil && application.Spec.Maskinporten.Authentication.Enabled == true {
+		var secretName *string
+		var err error
+		if application.Spec.Maskinporten.Authentication.SecretName != nil {
+			// If secret name is provided, use it regardless of whether Maskinporten is enabled
+			secretName = application.Spec.Maskinporten.Authentication.SecretName
+		} else if application.Spec.Maskinporten.Enabled {
+			// If Maskinporten is enabled but no secretName provided, retrieve the generated secret from MaksinPortenClient
+			secretName, err = getSecretNameForIdentityProvider(k8sClient, ctx,
+				types.NamespacedName{
+					Namespace: application.Namespace,
+					Name:      application.Name,
+				},
+				reconciliation.MASKINPORTEN,
+				application.UID)
+		} else {
+			// If Maskinporten is not enabled and no secretName provided, return error
+			return nil, fmt.Errorf("JWT authentication requires either Maskinporten to be enabled or a secretName to be provided")
+		}
 		if err != nil {
 			err := fmt.Errorf("failed to get secret name for MaskinPortenClient: %w", err)
 			return nil, err
 		}
 
 		var notPaths *[]string
-		if application.Spec.IDPorten.Authentication.IgnorePaths != nil {
-			notPaths = application.Spec.IDPorten.Authentication.IgnorePaths
+		if application.Spec.Maskinporten.Authentication.IgnorePaths != nil {
+			notPaths = application.Spec.Maskinporten.Authentication.IgnorePaths
 		} else {
 			notPaths = nil
 		}
@@ -288,21 +310,21 @@ func getSecretNameForIdentityProvider(k8sClient client.Client, ctx context.Conte
 				return &idPortenClient.Spec.SecretName, nil
 			}
 		}
-		err = fmt.Errorf("no IPPortenClient with ownerRef to (%w) found", namespacedName.String())
+		err = fmt.Errorf("no IDPortenClient with ownerRef to (%w) found", namespacedName.String())
 		return nil, err
 
 	case reconciliation.MASKINPORTEN:
 		maskinPortenClient, err := util.GetMaskinPortenlient(k8sClient, ctx, namespacedName)
 		if err != nil {
-			err := fmt.Errorf("failed to get IDPortenClient: %w", namespacedName.String())
+			err := fmt.Errorf("failed to get MaskinPortenClient: %w", namespacedName.String())
 			return nil, err
 		}
 		for _, ownerReference := range maskinPortenClient.OwnerReferences {
 			if ownerReference.UID == applicationUID {
 				return &maskinPortenClient.Spec.SecretName, nil
 			}
 		}
-		err = fmt.Errorf("no IPPortenClient with ownerRef to (%w) found", namespacedName.String())
+		err = fmt.Errorf("no MaskinPortenClient with ownerRef to (%w) found", namespacedName.String())
 		return nil, err
 
 	default:

diff --git a/pkg/resourcegenerator/istio/authorizationpolicy/authorization_policy.go b/pkg/resourcegenerator/istio/authorizationpolicy/authorization_policy.go
@@ -50,7 +50,11 @@ func getGeneralFromRule() []*securityv1api.Rule_From {
 func getAuthorizationPolicy(application *skiperatorv1alpha1.Application, denyPaths []string, authConfigs *[]reconciliation.AuthConfig) *securityv1.AuthorizationPolicy {
 	authPolicyRules := []*securityv1api.Rule{
 		{
-			To:   []*securityv1api.Rule_To{},
+			To: []*securityv1api.Rule_To{
+				{
+					Operation: &securityv1api.Operation{},
+				},
+			},
 			From: getGeneralFromRule(),
 		},
 	}
@@ -59,7 +63,6 @@ func getAuthorizationPolicy(application *skiperatorv1alpha1.Application, denyPat
 		if application.Spec.AuthorizationSettings.AllowAll == true {
 			return nil
 		}
-		// As of now we only use one rule and one operation for all default denies. No need to loop over them all
 		if len(application.Spec.AuthorizationSettings.AllowList) > 0 {
 			operation := authPolicyRules[0].To[0].Operation
 			for _, endpoint := range application.Spec.AuthorizationSettings.AllowList {

diff --git a/tests/application/authorization-policy/application-assert.yaml b/tests/application/authorization-policy/application-assert.yaml
@@ -1,18 +1,17 @@
 apiVersion: security.istio.io/v1
 kind: AuthorizationPolicy
 metadata:
-  name: authorization-policy-deny
+  name: application-auth-policy
 spec:
-  action: DENY
   rules:
     - from:
         - source:
             namespaces:
               - istio-gateways
       to:
         - operation:
-            paths:
+            notPaths:
               - /actuator*
   selector:
     matchLabels:
-      app: authorization-policy
+      app: application
diff --git a/tests/application/authorization-policy/application.yaml b/tests/application/authorization-policy/application.yaml
@@ -1,7 +1,7 @@
 apiVersion: skiperator.kartverket.no/v1alpha1
 kind: Application
 metadata:
-  name: authorization-policy
+  name: application
 spec:
   image: image
   port: 8080
diff --git a/tests/application/authorization-policy/chainsaw-test.yaml b/tests/application/authorization-policy/chainsaw-test.yaml
@@ -17,5 +17,5 @@ spec:
             ref:
               apiVersion: skiperator.kartverket.no/v1alpha1
               kind: Application
-              name: authorization-policy
+              name: application
 
diff --git a/tests/application/authorization-settings/multiple-application-assert.yaml b/tests/application/authorization-settings/multiple-application-assert.yaml
@@ -1,17 +1,16 @@
 apiVersion: security.istio.io/v1
 kind: AuthorizationPolicy
 metadata:
-  name: default-deny
+  name: default-auth-policy
 spec:
-  action: DENY
   rules:
     - from:
         - source:
             namespaces:
               - istio-gateways
       to:
         - operation:
-            paths:
+            notPaths:
               - /actuator*
   selector:
     matchLabels:
@@ -20,9 +19,8 @@ spec:
 apiVersion: security.istio.io/v1
 kind: AuthorizationPolicy
 metadata:
-  name: allow-list-deny
+  name: allow-list-auth-policy
 spec:
-  action: DENY
   rules:
     - from:
         - source:
@@ -31,8 +29,6 @@ spec:
       to:
         - operation:
             paths:
-              - /actuator*
-            notPaths:
               - /actuator/health
               - /actuator/info
   selector:

diff --git a/tests/application/authorization-settings/multiple-application-errors.yaml b/tests/application/authorization-settings/multiple-application-errors.yaml
@@ -1,4 +1,4 @@
 apiVersion: security.istio.io/v1
 kind: AuthorizationPolicy
 metadata:
-  name: allow-all-deny
+  name: allow-all-auth-policy
diff --git a/tests/application/authorization-settings/patch-application-assert.yaml b/tests/application/authorization-settings/patch-application-assert.yaml
@@ -2,9 +2,8 @@
 apiVersion: security.istio.io/v1
 kind: AuthorizationPolicy
 metadata:
-  name: allow-list-deny
+  name: allow-list-auth-policy
 spec:
-  action: DENY
   rules:
     - from:
         - source:
@@ -13,8 +12,6 @@ spec:
       to:
         - operation:
             paths:
-              - /actuator*
-            notPaths:
               - /actuator/info
               - /actuator/shutdown
   selector:
@@ -24,17 +21,16 @@ spec:
 apiVersion: security.istio.io/v1
 kind: AuthorizationPolicy
 metadata:
-  name: allow-all-deny
+  name: allow-all-auth-policy
 spec:
-  action: DENY
   rules:
     - from:
         - source:
             namespaces:
               - istio-gateways
       to:
         - operation:
-            paths:
+            notPaths:
               - /actuator*
   selector:
     matchLabels:

diff --git a/tests/application/authorization-settings/patch-application-errors.yaml b/tests/application/authorization-settings/patch-application-errors.yaml
@@ -1,9 +1,8 @@
 apiVersion: security.istio.io/v1
 kind: AuthorizationPolicy
 metadata:
-  name: allow-list-deny
+  name: allow-list-auth-policy
 spec:
-  action: DENY
   rules:
     - from:
         - source:
@@ -12,7 +11,5 @@ spec:
       to:
         - operation:
             paths:
-              - /actuator*
-            notPaths:
               - /actuator/health
               - /actuator/info
diff --git a/tests/application/idporten/application-assert.yaml b/tests/application/idporten/application-assert.yaml
@@ -21,9 +21,4 @@ spec:
         - name: "idporten-idporten-test-client-5dd0f829"
           secret:
             defaultMode: 420
-            secretName: "idporten-idporten-test-client-5dd0f829"
-
-
-
-
-
+            secretName: "idporten-idporten-test-client-5dd0f829"
diff --git a/tests/application/idporten/application-idporten-auth-assert.yaml b/tests/application/idporten/application-idporten-auth-assert.yaml
@@ -0,0 +1,76 @@
+apiVersion: nais.io/v1
+kind: IDPortenClient
+metadata:
+  name: idporten-test-client-with-auth
+spec:
+  scopes:
+    - "openid"
+    - "profile"
+---
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: idporten-test-client-with-auth
+spec:
+  selector:
+    matchLabels:
+      app: idporten-test-client-with-auth
+  template:
+    spec:
+      containers:
+        - name: idporten-test-client-with-auth
+          volumeMounts:
+            - mountPath: /tmp
+              name: tmp
+            - mountPath: /var/run/secrets/skip/idporten
+              name: "idporten-idporten-test-client-with-auth-c47dbf96"
+      volumes:
+        - emptyDir: {}
+          name: tmp
+        - name: "idporten-idporten-test-client-with-auth-c47dbf96"
+          secret:
+            defaultMode: 420
+            secretName: "idporten-idporten-test-client-with-auth-c47dbf96"
+---
+apiVersion: security.istio.io/v1
+kind: RequestAuthentication
+metadata:
+  name: idporten-test-client-with-auth-jwt-authn
+spec:
+  jwtRules:
+    - audiences:
+        - test-client-id-idporten
+      forwardOriginalToken: true
+      fromCookies:
+        - BearerToken
+      issuer: https://idporten.no
+      jwksUri: https://idporten.no/jwks.json
+  selector:
+    matchLabels:
+      app: idporten-test-client-with-auth
+---
+apiVersion: security.istio.io/v1
+kind: AuthorizationPolicy
+metadata:
+  name: idporten-test-client-with-auth-auth-policy
+spec:
+  rules:
+    - from:
+        - source:
+            namespaces:
+              - istio-gateways
+      to:
+        - operation:
+            notPaths:
+              - /actuator*
+    - to:
+        - operation:
+            paths:
+              - '*'
+      when:
+        - key: request.auth.claims[iss]
+          values:
+            - https://idporten.no
+  selector:
+    matchLabels:
+      app: idporten-test-client-with-auth
diff --git a/tests/application/idporten/application.yaml b/tests/application/idporten/application.yaml
@@ -11,4 +11,32 @@ spec:
     enabled: true
     integrationType: "api_klient"
     scopes:
-      - "openid"
+      - "openid"
+---
+apiVersion: skiperator.kartverket.no/v1alpha1
+kind: Application
+metadata:
+  name: idporten-test-client-with-auth
+spec:
+  image: image
+  port: 8080
+  ingresses:
+    - example.com
+  idporten:
+    enabled: true
+    integrationType: "api_klient"
+    scopes:
+      - "openid"
+    authentication:
+      enabled: true
+      secretName: "idporten-idporten-test-client-with-auth-c47dbf96"
+---
+apiVersion: v1
+kind: Secret
+type: Opaque
+metadata:
+  name: "idporten-idporten-test-client-with-auth-c47dbf96"
+data:
+  IDPORTEN_CLIENT_ID: dGVzdC1jbGllbnQtaWQtaWRwb3J0ZW4= #test-client-id-idporten
+  IDPORTEN_ISSUER: aHR0cHM6Ly9pZHBvcnRlbi5ubw== #https://idporten.no
+  IDPORTEN_JWKS_URI: aHR0cHM6Ly9pZHBvcnRlbi5uby9qd2tzLmpzb24= #https://idporten.no/jwks.json
diff --git a/tests/application/idporten/chainsaw-test.yaml b/tests/application/idporten/chainsaw-test.yaml
@@ -14,6 +14,8 @@ spec:
             file: application-idporten-assert.yaml
         - assert:
             file: application-assert.yaml
+        - assert:
+            file: application-idporten-auth-assert.yaml
     - try:
         - apply:
             file: patch-application.yaml