refactor!: Made the usage of if clauses in authentication & authori…

…zation, and error pipelines consistent (#1784)

cmd/validate/test_data/config.yaml

@@ -168,10 +168,6 @@ mechanisms:
       type: default
     - id: authenticate_with_kratos
       type: redirect
-      if: |
-        ((type(Error) == authentication_error && Error.Source == "kratos_session_authenticator") ||
-        type(Error) == authorization_error) &&
-        Request.Header("Accept").contains("*/*")
       config:
         to: http://127.0.0.1:4433/self-service/login/browser?return_to={{ .Request.URL | urlenc }}
 
@@ -182,6 +178,10 @@ default_rule:
     - finalizer: jwt
   on_error:
     - error_handler: authenticate_with_kratos
+      if: |
+        ((type(Error) == authentication_error && Error.Source == "kratos_session_authenticator") ||
+        type(Error) == authorization_error) &&
+        Request.Header("Accept").contains("*/*")
 
 providers:
   file_system:

docs/content/docs/configuration/reference.adoc

@@ -357,14 +357,6 @@ mechanisms:
     type: redirect
     config:
       to: http://127.0.0.1:4433/self-service/login/browser?return_to={{ .Request.URL | urlenc }}
-      when:
-        - error:
-          - type: authentication_error
-            raised_by: kratos_session_authenticator
-          - type: authorization_error
-          request_headers:
-            Accept:
-            - '*/*'
 
 default_rule:
   backtracking_enabled: false
@@ -373,6 +365,10 @@ default_rule:
   - finalizer: jwt
   on_error:
   - error_handler: authenticate_with_kratos
+    if: |
+      ((type(Error) == authentication_error && Error.Source == "kratos_session_authenticator") ||
+        type(Error) == authorization_error) &&
+        Request.Header("Accept").contains("*/*")
 
 providers:
   file_system:

docs/content/docs/mechanisms/catalogue.adoc

@@ -126,9 +126,6 @@ mechanisms:
   - id: default
     type: default
   - id: authenticate_with_kratos
-    if: |
-      type(Error) in [authentication_error, authorization_error] &&
-      Request.Header("Accept").contains("text/html")
     type: redirect
     config:
       to: http://127.0.0.1:4433/self-service/login/browser?return_to={{ .Request.URL | urlenc }}

docs/content/docs/mechanisms/error_handlers.adoc

@@ -16,7 +16,7 @@ Some of the error handlers may support or require additional configuration. The
 
 == Default
 
-This mechanism is always there and is executed if no other error handler mechanism is responsible for the error. Actually, there is no need to explicitly configure it. The only exception is to allow overriding the link:{{< relref "/docs/rules/default_rule.adoc" >}}[default rule's] error handler chain in a specific rule for performance reasons (if configured error handlers in the default rule should not be considered). This mechanism type doesn't have any configuration options.
+This error handler is always there and is executed if no other error handler mechanism is responsible for the error. Actually, there is no need to explicitly configure it. The only exception is to allow overriding the link:{{< relref "/docs/rules/default_rule.adoc" >}}[default rule's] error handler chain in a specific rule for performance reasons (if configured error handlers in the default rule should not be considered). This mechanism type doesn't have any configuration options.
 
 To enable the usage of this mechanism, you have to set the `type` property to `default`.
 
@@ -37,7 +37,7 @@ This error handler mechanism allows redirecting the client to another endpoint,
 
 To enable the usage of this mechanism, you have to set the `type` property to `redirect`.
 
-Configuration is mandatory by making use of the `if` and `config` properties. The first defines the condition, which must hold true for this error handler to execute and has access to the link:{{< relref "/docs/mechanisms/evaluation_objects.adoc#_request" >}}[`Request`] and the link:{{< relref "/docs/mechanisms/evaluation_objects.adoc#_error" >}}[`Error`] objects. Latter defines the data to drive the redirect and supports the following properties:
+Configuration is mandatory by making use of the `config` property supporting the following settings:
 
 * *`to`*: _URL_ (mandatory, not overridable)
 +
@@ -62,9 +62,6 @@ So, e.g. if heimdall was handling the request for `\http://my-service.local/foo`
 ----
 id: authenticate_with_kratos
 type: redirect
-if: |
-  type(Error) in [authentication_error, authorization_error] &&
-  Request.Header("Accept").contains("text/html")
 config:
   to: http://127.0.0.1:4433/self-service/login/browser?return_to={{ .Request.URL | urlenc }}
 ----

docs/content/docs/rules/default_rule.adoc

@@ -32,9 +32,9 @@ Enables or disables backtracking while matching the rules globally. Defaults to
 +
 Which mechanisms to use for authentication, authorization and finalization stages of the pipeline. At least the authentication stage with at least one link:{{< relref "/docs/mechanisms/authenticators.adoc" >}}[authenticator] must be defined. A specific rule (see also link:{{< relref "regular_rule.adoc" >}}[Regular Rule]) can omit the definition of that stage, if it wants to reuse it from in the default rule. Same is true for other stages (See also link:{{< relref "/docs/concepts/rules.adoc#_default_rule_inheritance" >}}[Rule Inheritance]).
 
-* *`on_error`*: _link:{{< relref "regular_rule.adoc#_error_pipeline" >}}[Error Pipeline]_ (mandatory)
+* *`on_error`*: _link:{{< relref "regular_rule.adoc#_error_pipeline" >}}[Error Pipeline]_ (optional)
 +
-Which error handler mechanisms to use if any of the mechanisms, defined in the `execute` property fail. Allows omitting the definition of error handlers in specific rules. As soon as a specific rule defines at least one error handler mechanism, all error handler mechanisms, defined in the default rule are ignored.
+Which error handler mechanisms to use if any of the mechanisms, defined in the `execute` property fail. Allows omitting the definition of error handlers in specific rules. As soon as a specific rule defines at least one error handler mechanism, all error handler mechanisms, defined in the default rule are ignored. If not specified, the default error handler is used.
 
 .Default rule configuration
 ====
@@ -48,6 +48,8 @@ default_rule:
     - finalizer: create_jwt
   on_error:
     - error_handler: authenticate_with_kratos_eh
+      if: |
+        type(Error) == authentication_error && Error.Source == "session_cookie_from_kratos_authn"
 ----
 
 This example defines a default rule, with the authentication 6 authorization pipeline consisting of two authenticators, with `session_cookie_from_kratos_authn` being the first and `oauth2_introspect_token_from_keycloak_authn` being the fallback one (if the first one fails), a `deny_all_requests_authz` authorizer and the `create_jwt` finalizer. The error pipeline is configured to execute only the `authenticate_with_kratos_eh` error handler.

docs/content/docs/rules/regular_rule.adoc

@@ -144,7 +144,7 @@ Specifies the mechanisms used for authentication, authorization, contextualizati
 
 * *`on_error`*: _link:{{< relref "#_error_pipeline" >}}[Error Pipeline]_ (optional)
 +
-Specifies error handling mechanisms if the pipeline defined by the `execute` property fails. Optional, if a link:{{< relref "default_rule.adoc" >}}[default rule] is configured with `on_error` definition.
+Specifies error handling mechanisms if the pipeline defined by the `execute` property fails. Defaults to the error pipeline defined in the link:{{< relref "default_rule.adoc" >}}[default rule] if not specified.
 
 .An example rule
 ====
@@ -365,21 +365,23 @@ This example uses
 
 == Error Pipeline
 
-Compared to the link:{{< relref "#_authentication_authorization_pipeline" >}}[Authentication & Authorization Pipeline], the error pipeline is pretty simple. It is also a list of mechanism references, but all referenced types are link:{{< relref "/docs/mechanisms/error_handlers.adoc" >}}[error handler types]. Thus, each entry in this list must have `error_handler` as key, followed by the `ìd` of the required error handler, previously defined in the link:{{< relref "/docs/mechanisms/catalogue.adoc" >}}[mechanism catalogue]. Error handlers are always executed as fallbacks. So, if the condition of the first error handler does not match, second is selected, if its condition matches, it is executed, otherwise the next one is selected, etc. If none of the conditions of the defined error handlers match, the link:{{< relref "/docs/mechanisms/error_handlers.adoc#_default" >}}[default error handler] is executed.
+Compared to the link:{{< relref "#_authentication_authorization_pipeline" >}}[Authentication & Authorization Pipeline], the error pipeline is pretty simple. It is also a list of mechanism references, but all referenced types are link:{{< relref "/docs/mechanisms/error_handlers.adoc" >}}[error handler types]. Thus, each entry in this list must have `error_handler` as key, followed by the `ìd` of the required error handler previously defined in the link:{{< relref "/docs/mechanisms/catalogue.adoc" >}}[mechanism catalogue].
 
-As with the authentication & authorization pipeline, partial reconfiguration of the used mechanisms is possible if supported by the corresponding type. Same is true for overrides of the `if` conditions. The overrides are always local to the given rule as well.
+Execution of the error handlers should happen conditionally by making use of a https://github.com/google/cel-spec[CEL] expression in an `if` clause, which has access to the link:{{< relref "/docs/mechanisms/evaluation_objects.adoc#_error" >}}[`Error`] and the link:{{< relref "/docs/mechanisms/evaluation_objects.adoc#_request" >}}[`Request`] objects. Otherwise, the first error handler will be executed and the error pipeline will exit.
+
+As with the authentication & authorization pipeline, partial reconfiguration of the used mechanisms is possible if supported by the corresponding type. The overrides are always local to the given rule as well.
 
 .Two error handlers
 ====
 [source, yaml]
 ----
 - error_handler: foo
-- error_handler: bar
   if: # rule specific condition
+- error_handler: bar
   config:
       # rule specific config
 ----
 ====
 
-This example uses two error handlers, named `foo` and `bar`. `bar` will only be selected by heimdall if `foo` 's error condition (defined in the link:{{< relref "/docs/mechanisms/catalogue.adoc" >}}[mechanism catalogue]) does not match. `bar` does also override the error condition as required by the given rule.
+This example uses two error handlers, named `foo` and `bar`. `bar` will only be executed if `foo` 's error condition does not match. `bar` does also override the error handler configuration as required by the given rule.
 

example_config.yaml

@@ -169,10 +169,6 @@ mechanisms:
       type: default
     - id: authenticate_with_kratos
       type: redirect
-      if: |
-        Error.Source == "kratos_session_authenticator" && 
-        type(Error) == authentication_error &&
-        Request.Header("Accept").contains("*/*")
       config:
         to: http://127.0.0.1:4433/self-service/login/browser?origin={{ .Request.URL | urlenc }}
 
@@ -183,6 +179,10 @@ default_rule:
     - finalizer: jwt
   on_error:
     - error_handler: authenticate_with_kratos
+      if: |
+        Error.Source == "kratos_session_authenticator" && 
+        type(Error) == authentication_error &&
+        Request.Header("Accept").contains("*/*")
 
 providers:
   file_system:

examples/kubernetes/quickstarts/heimdall/config.yaml

@@ -31,7 +31,6 @@ mechanisms:
   error_handlers:
     - id: redirect
       type: redirect
-      if: type(Error) == authentication_error
       config:
         to: http://foo.bar?origin={{ .Request.URL | urlenc }}
 
@@ -40,7 +39,8 @@ default_rule:
     - authenticator: anonymous_authenticator
     - authorizer: deny_all_requests
   on_error:
-    - error_handler: redirect
+    - if: type(Error) == authentication_error
+      error_handler: redirect
 
 providers:
   kubernetes:

examples/kubernetes/quickstarts/proxy-demo/heimdall-config.yaml

@@ -37,7 +37,6 @@ data:
       error_handlers:
         - id: redirect
           type: redirect
-          if: type(Error) == authentication_error
           config:
             to: http://foo.bar?origin={{ .Request.URL | urlenc }}
 
@@ -47,6 +46,7 @@ data:
         - authorizer: deny_all_requests
       on_error:
         - error_handler: redirect
+          if: type(Error) == authentication_error
 
     providers:
       file_system:

internal/config/test_data/test_config.yaml

@@ -467,10 +467,6 @@ mechanisms:
       type: default
     - id: authenticate_with_kratos
       type: redirect
-      if: |
-        ((type(Error) == authentication_error && Error.Source == "kratos_session_authenticator") ||
-        type(Error) == authorization_error) &&
-        Request.Header("Accept").contains("*/*")
       config:
         to: http://127.0.0.1:4433/self-service/login/browser?return_to={{ .Request.URL | urlenc }}
 
@@ -481,6 +477,7 @@ default_rule:
     - finalizer: jwt
   on_error:
     - error_handler: authenticate_with_kratos
+      if: type(Error) == authentication_error
 
 providers:
   file_system:

internal/rules/cel_execution_condition.go

@@ -31,14 +31,20 @@ type celExecutionCondition struct {
 	e *cellib.CompiledExpression
 }
 
-func (c *celExecutionCondition) CanExecute(ctx heimdall.Context, sub *subject.Subject) (bool, error) {
-	obj := map[string]any{"Request": ctx.Request()}
+func (c *celExecutionCondition) CanExecuteOnSubject(ctx heimdall.Context, sub *subject.Subject) (bool, error) {
+	if err := c.e.Eval(map[string]any{"Request": ctx.Request(), "Subject": sub}); err != nil {
+		if errors.Is(err, &cellib.EvalError{}) {
+			return false, nil
+		}
 
-	if sub != nil {
-		obj["Subject"] = sub
+		return false, err
 	}
 
-	if err := c.e.Eval(obj); err != nil {
+	return true, nil
+}
+
+func (c *celExecutionCondition) CanExecuteOnError(ctx heimdall.Context, cause error) (bool, error) {
+	if err := c.e.Eval(map[string]any{"Request": ctx.Request(), "Error": cellib.WrapError(cause)}); err != nil {
 		if errors.Is(err, &cellib.EvalError{}) {
 			return false, nil
 		}

internal/rules/cel_execution_condition_test.go

@@ -27,6 +27,7 @@ import (
 	"github.com/dadrus/heimdall/internal/heimdall"
 	"github.com/dadrus/heimdall/internal/heimdall/mocks"
 	"github.com/dadrus/heimdall/internal/rules/mechanisms/subject"
+	"github.com/dadrus/heimdall/internal/x/errorchain"
 )
 
 func TestNewCelExecutionCondition(t *testing.T) {
@@ -58,7 +59,7 @@ func TestNewCelExecutionCondition(t *testing.T) {
 	}
 }
 
-func TestCelExecutionConditionCanExecute(t *testing.T) {
+func TestCelExecutionConditionCanExecuteOnSubject(t *testing.T) {
 	t.Parallel()
 
 	sub := &subject.Subject{
@@ -117,7 +118,67 @@ func TestCelExecutionConditionCanExecute(t *testing.T) {
 			require.NoError(t, err)
 
 			// WHEN
-			can, err := condition.CanExecute(ctx, sub)
+			can, err := condition.CanExecuteOnSubject(ctx, sub)
+
+			// THEN
+			require.NoError(t, err)
+			assert.Equal(t, tc.expected, can)
+		})
+	}
+}
+
+type testIdentifier string
+
+func (tid testIdentifier) ID() string { return string(tid) }
+
+func TestCelExecutionConditionCanExecuteOnError(t *testing.T) {
+	t.Parallel()
+
+	for _, tc := range []struct {
+		uc         string
+		expression string
+		expected   bool
+	}{
+		{
+			uc: "complex expression evaluating to true",
+			expression: `type(Error) in [communication_error, authorization_error] && 
+                           Error.Source == "foobar" &&
+                           "bar" in Request.URL.Query().foo`,
+			expected: true,
+		},
+		{
+			uc:         "simple expression evaluating to false",
+			expression: `type(Error) == internal_error && Request.Method == "GET"`,
+			expected:   false,
+		},
+		{
+			uc:         "simple expression evaluating to true",
+			expression: `type(Error) == authorization_error && Request.Method == "GET"`,
+			expected:   true,
+		},
+	} {
+		t.Run(tc.uc, func(t *testing.T) {
+			// GIVEN
+			ctx := mocks.NewContextMock(t)
+
+			ctx.EXPECT().Request().Return(&heimdall.Request{
+				Method: http.MethodGet,
+				URL: &heimdall.URL{URL: url.URL{
+					Scheme:   "http",
+					Host:     "localhost",
+					Path:     "/test",
+					RawQuery: "foo=bar&baz=zab",
+				}},
+				ClientIPAddresses: []string{"127.0.0.1", "10.10.10.10"},
+			})
+
+			condition, err := newCelExecutionCondition(tc.expression)
+			require.NoError(t, err)
+
+			// WHEN
+			can, err := condition.CanExecuteOnError(ctx, errorchain.
+				NewWithMessage(heimdall.ErrCommunication, "test").
+				CausedBy(heimdall.ErrAuthorization).WithErrorContext(testIdentifier("foobar")))
 
 			// THEN
 			require.NoError(t, err)

internal/rules/composite_error_handler.go

@@ -17,6 +17,8 @@
 package rules
 
 import (
+	"errors"
+
 	"github.com/rs/zerolog"
 
 	"github.com/dadrus/heimdall/internal/heimdall"
@@ -28,16 +30,21 @@ func (eh compositeErrorHandler) Execute(ctx heimdall.Context, exErr error) error
 	logger := zerolog.Ctx(ctx.AppContext())
 	logger.Debug().Msg("Handling pipeline error")
 
-	for _, eh := range eh {
-		if eh.CanExecute(ctx, exErr) {
-			err := eh.Execute(ctx, exErr)
-			if err != nil {
-				logger.Error().Err(err).Msg("Failed to execute error handler")
+	for _, handler := range eh {
+		if err := handler.Execute(ctx, exErr); err != nil {
+			if errors.Is(err, errErrorHandlerNotApplicable) {
+				continue
 			}
 
+			logger.Error().Err(err).Msg("Failed to execute error handler")
+
 			return err
 		}
+
+		return nil
 	}
 
+	logger.Warn().Msg("No applicable error handler found")
+
 	return exErr
 }

internal/rules/composite_error_handler_test.go

@@ -35,10 +35,9 @@ func TestCompositeErrorHandlerExecutionWithFallback(t *testing.T) {
 	ctx.EXPECT().AppContext().Return(context.Background())
 
 	eh1 := rulemocks.NewErrorHandlerMock(t)
-	eh1.EXPECT().CanExecute(ctx, testsupport.ErrTestPurpose).Return(false)
+	eh1.EXPECT().Execute(ctx, testsupport.ErrTestPurpose).Return(errErrorHandlerNotApplicable)
 
 	eh2 := rulemocks.NewErrorHandlerMock(t)
-	eh2.EXPECT().CanExecute(ctx, testsupport.ErrTestPurpose).Return(true)
 	eh2.EXPECT().Execute(ctx, testsupport.ErrTestPurpose).Return(nil)
 
 	eh := compositeErrorHandler{eh1, eh2}
@@ -58,7 +57,6 @@ func TestCompositeErrorHandlerExecutionWithoutFallback(t *testing.T) {
 	ctx.EXPECT().AppContext().Return(context.Background())
 
 	eh1 := rulemocks.NewErrorHandlerMock(t)
-	eh1.EXPECT().CanExecute(ctx, testsupport.ErrTestPurpose).Return(true)
 	eh1.EXPECT().Execute(ctx, testsupport.ErrTestPurpose).Return(nil)
 
 	eh2 := rulemocks.NewErrorHandlerMock(t)
@@ -80,10 +78,10 @@ func TestCompositeErrorHandlerExecutionWithNoApplicableErrorHandler(t *testing.T
 	ctx.EXPECT().AppContext().Return(context.Background())
 
 	eh1 := rulemocks.NewErrorHandlerMock(t)
-	eh1.EXPECT().CanExecute(ctx, testsupport.ErrTestPurpose).Return(false)
+	eh1.EXPECT().Execute(ctx, testsupport.ErrTestPurpose).Return(errErrorHandlerNotApplicable)
 
 	eh2 := rulemocks.NewErrorHandlerMock(t)
-	eh2.EXPECT().CanExecute(ctx, testsupport.ErrTestPurpose).Return(false)
+	eh2.EXPECT().Execute(ctx, testsupport.ErrTestPurpose).Return(errErrorHandlerNotApplicable)
 
 	eh := compositeErrorHandler{eh1, eh2}