diff --git a/internal/controllers/common/reconciler.go b/internal/controllers/common/reconciler.go index f27177bb..4fe31826 100644 --- 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,13 +310,13 @@ 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 { @@ -302,7 +324,7 @@ func getSecretNameForIdentityProvider(k8sClient client.Client, ctx context.Conte 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 index 3e73fdde..9a8de2f3 100644 --- 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 index 380504a0..7d51c1d8 100644 --- a/tests/application/authorization-policy/application-assert.yaml +++ b/tests/application/authorization-policy/application-assert.yaml @@ -1,9 +1,8 @@ apiVersion: security.istio.io/v1 kind: AuthorizationPolicy metadata: - name: authorization-policy-deny + name: application-auth-policy spec: - action: DENY rules: - from: - source: @@ -11,8 +10,8 @@ spec: - istio-gateways to: - operation: - paths: + notPaths: - /actuator* selector: matchLabels: - app: authorization-policy + app: application \ No newline at end of file diff --git a/tests/application/authorization-policy/application.yaml b/tests/application/authorization-policy/application.yaml index c6d8edf6..8311e1e3 100644 --- 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 index 19649b87..2798ab04 100644 --- 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 index 9fc20457..08d44f08 100644 --- a/tests/application/authorization-settings/multiple-application-assert.yaml +++ b/tests/application/authorization-settings/multiple-application-assert.yaml @@ -1,9 +1,8 @@ apiVersion: security.istio.io/v1 kind: AuthorizationPolicy metadata: - name: default-deny + name: default-auth-policy spec: - action: DENY rules: - from: - source: @@ -11,7 +10,7 @@ spec: - 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 index 5c286e90..0cf80a30 100644 --- 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 index 10302a90..860d08c4 100644 --- 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,9 +21,8 @@ spec: apiVersion: security.istio.io/v1 kind: AuthorizationPolicy metadata: - name: allow-all-deny + name: allow-all-auth-policy spec: - action: DENY rules: - from: - source: @@ -34,7 +30,7 @@ spec: - 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 index e9930b61..f729b4de 100644 --- 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 index c526607e..58c67e2d 100644 --- 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" \ No newline at end of file diff --git a/tests/application/idporten/application-idporten-auth-assert.yaml b/tests/application/idporten/application-idporten-auth-assert.yaml new file mode 100644 index 00000000..5d577b54 --- /dev/null +++ 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 \ No newline at end of file diff --git a/tests/application/idporten/application.yaml b/tests/application/idporten/application.yaml index be01ec33..d905d585 100644 --- a/tests/application/idporten/application.yaml +++ b/tests/application/idporten/application.yaml @@ -11,4 +11,32 @@ spec: enabled: true integrationType: "api_klient" scopes: - - "openid" \ No newline at end of file + - "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 \ No newline at end of file diff --git a/tests/application/idporten/chainsaw-test.yaml b/tests/application/idporten/chainsaw-test.yaml index 60f4c62f..2920cd3e 100644 --- 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 diff --git a/tests/application/maskinporten/application-maskinporten-auth-assert.yaml b/tests/application/maskinporten/application-maskinporten-auth-assert.yaml new file mode 100644 index 00000000..24d76316 --- /dev/null +++ b/tests/application/maskinporten/application-maskinporten-auth-assert.yaml @@ -0,0 +1,72 @@ +apiVersion: nais.io/v1 +kind: MaskinportenClient +metadata: + name: maskinporten-test-client-with-auth +spec: + scopes: {} +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + name: maskinporten-test-client-with-auth +spec: + selector: + matchLabels: + app: maskinporten-test-client-with-auth + template: + spec: + containers: + - name: maskinporten-test-client-with-auth + volumeMounts: + - mountPath: /tmp + name: tmp + - mountPath: /var/run/secrets/skip/maskinporten + name: "maskinporten-maskinporten-test-client-with-auth-64bbe0ca" + volumes: + - emptyDir: {} + name: tmp + - name: "maskinporten-maskinporten-test-client-with-auth-64bbe0ca" + secret: + defaultMode: 420 + secretName: "maskinporten-maskinporten-test-client-with-auth-64bbe0ca" +--- +apiVersion: security.istio.io/v1 +kind: RequestAuthentication +metadata: + name: maskinporten-test-client-with-auth-jwt-authn +spec: + jwtRules: + - audiences: + - test-client-id-maskinporten + forwardOriginalToken: true + issuer: https://maskinporten.no/ + jwksUri: https://maskinporten.no/jwk + selector: + matchLabels: + app: maskinporten-test-client-with-auth +--- +apiVersion: security.istio.io/v1 +kind: AuthorizationPolicy +metadata: + name: maskinporten-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://maskinporten.no/ + selector: + matchLabels: + app: maskinporten-test-client-with-auth \ No newline at end of file diff --git a/tests/application/maskinporten/application.yaml b/tests/application/maskinporten/application.yaml index 8d0a64ca..d932fb4c 100644 --- a/tests/application/maskinporten/application.yaml +++ b/tests/application/maskinporten/application.yaml @@ -9,3 +9,28 @@ spec: - example.com maskinporten: enabled: true +--- +apiVersion: skiperator.kartverket.no/v1alpha1 +kind: Application +metadata: + name: maskinporten-test-client-with-auth +spec: + image: image + port: 8080 + ingresses: + - example.com + maskinporten: + enabled: true + authentication: + enabled: true + secretName: "maskinporten-maskinporten-test-client-with-auth-64bbe0ca" +--- +apiVersion: v1 +kind: Secret +type: Opaque +metadata: + name: "maskinporten-maskinporten-test-client-with-auth-64bbe0ca" +data: + MASKINPORTEN_CLIENT_ID: dGVzdC1jbGllbnQtaWQtbWFza2lucG9ydGVu #test-client-id-maskinporten + MASKINPORTEN_ISSUER: aHR0cHM6Ly9tYXNraW5wb3J0ZW4ubm8v #https://maskinporten.no/ + MASKINPORTEN_JWKS_URI: aHR0cHM6Ly9tYXNraW5wb3J0ZW4ubm8vandr #https://maskinporten.no/jwk \ No newline at end of file diff --git a/tests/application/maskinporten/chainsaw-test.yaml b/tests/application/maskinporten/chainsaw-test.yaml index ed6cbf41..980506aa 100644 --- a/tests/application/maskinporten/chainsaw-test.yaml +++ b/tests/application/maskinporten/chainsaw-test.yaml @@ -14,6 +14,8 @@ spec: file: application-maskinporten-assert.yaml - assert: file: application-assert.yaml + - assert: + file: application-maskinporten-auth-assert.yaml - try: - apply: file: patch-application.yaml