From 2577f560b80c49e3e5a4b3da547245af98844843 Mon Sep 17 00:00:00 2001 From: Dimitrij Drus Date: Fri, 13 Sep 2024 11:53:21 +0200 Subject: [PATCH] refactor!: Made the usage of `if` clauses in authentication & authorization, and error pipelines consistent (#1784) --- cmd/validate/test_data/config.yaml | 8 +- .../content/docs/configuration/reference.adoc | 12 +- docs/content/docs/mechanisms/catalogue.adoc | 3 - .../docs/mechanisms/error_handlers.adoc | 7 +- docs/content/docs/rules/default_rule.adoc | 6 +- docs/content/docs/rules/regular_rule.adoc | 12 +- example_config.yaml | 8 +- .../quickstarts/heimdall/config.yaml | 4 +- .../proxy-demo/heimdall-config.yaml | 2 +- internal/config/test_data/test_config.yaml | 5 +- internal/rules/cel_execution_condition.go | 16 +- .../rules/cel_execution_condition_test.go | 65 ++++++- internal/rules/composite_error_handler.go | 17 +- .../rules/composite_error_handler_test.go | 8 +- internal/rules/conditional_error_handler.go | 50 +++++ .../rules/conditional_error_handler_test.go | 118 +++++++++++ internal/rules/conditional_subject_handler.go | 2 +- .../rules/conditional_subject_handler_test.go | 19 +- internal/rules/default_execution_condition.go | 6 +- internal/rules/error_handler.go | 2 +- internal/rules/execution_condition.go | 3 +- .../errorhandlers/base_error_handler.go | 68 ------- .../errorhandlers/base_error_handler_test.go | 85 -------- .../errorhandlers/default_error_handler.go | 12 +- .../default_error_handler_test.go | 13 +- .../mechanisms/errorhandlers/error_handler.go | 1 - .../errorhandlers/mocks/error_handler.go | 66 ++----- .../errorhandlers/redirect_error_handler.go | 48 ++--- .../redirect_error_handler_test.go | 184 +++--------------- .../www_authenticate_error_handler.go | 36 +--- .../www_authenticate_error_handler_test.go | 160 ++------------- internal/rules/mocks/error_handler.go | 79 ++++---- internal/rules/mocks/execution_condition.go | 92 +++++++-- internal/rules/rule_factory_impl.go | 182 ++++++++--------- internal/rules/rule_factory_impl_test.go | 46 ++++- internal/rules/rule_impl_test.go | 6 - schema/config.schema.json | 16 -- 37 files changed, 662 insertions(+), 805 deletions(-) create mode 100644 internal/rules/conditional_error_handler.go create mode 100644 internal/rules/conditional_error_handler_test.go delete mode 100644 internal/rules/mechanisms/errorhandlers/base_error_handler.go delete mode 100644 internal/rules/mechanisms/errorhandlers/base_error_handler_test.go diff --git a/cmd/validate/test_data/config.yaml b/cmd/validate/test_data/config.yaml index c3431e2f6..3304a5fdf 100644 --- a/cmd/validate/test_data/config.yaml +++ b/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: diff --git a/docs/content/docs/configuration/reference.adoc b/docs/content/docs/configuration/reference.adoc index 4134a9c2b..003571bbe 100644 --- a/docs/content/docs/configuration/reference.adoc +++ b/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: diff --git a/docs/content/docs/mechanisms/catalogue.adoc b/docs/content/docs/mechanisms/catalogue.adoc index 23e9084b0..c4434f1dc 100644 --- a/docs/content/docs/mechanisms/catalogue.adoc +++ b/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 }} diff --git a/docs/content/docs/mechanisms/error_handlers.adoc b/docs/content/docs/mechanisms/error_handlers.adoc index d49901a99..ddf33495b 100644 --- a/docs/content/docs/mechanisms/error_handlers.adoc +++ b/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 }} ---- diff --git a/docs/content/docs/rules/default_rule.adoc b/docs/content/docs/rules/default_rule.adoc index 28df9cade..0e6134801 100644 --- a/docs/content/docs/rules/default_rule.adoc +++ b/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. diff --git a/docs/content/docs/rules/regular_rule.adoc b/docs/content/docs/rules/regular_rule.adoc index 8c4498183..168c7b849 100644 --- a/docs/content/docs/rules/regular_rule.adoc +++ b/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. diff --git a/example_config.yaml b/example_config.yaml index 1ceff650c..74c6924ab 100644 --- a/example_config.yaml +++ b/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: diff --git a/examples/kubernetes/quickstarts/heimdall/config.yaml b/examples/kubernetes/quickstarts/heimdall/config.yaml index 705503574..3939adf2f 100644 --- a/examples/kubernetes/quickstarts/heimdall/config.yaml +++ b/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: diff --git a/examples/kubernetes/quickstarts/proxy-demo/heimdall-config.yaml b/examples/kubernetes/quickstarts/proxy-demo/heimdall-config.yaml index fda2c157d..c070717d4 100644 --- a/examples/kubernetes/quickstarts/proxy-demo/heimdall-config.yaml +++ b/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: diff --git a/internal/config/test_data/test_config.yaml b/internal/config/test_data/test_config.yaml index 34ecb29d0..42c6a0596 100644 --- a/internal/config/test_data/test_config.yaml +++ b/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: diff --git a/internal/rules/cel_execution_condition.go b/internal/rules/cel_execution_condition.go index 6b650afc2..49c439f6f 100644 --- a/internal/rules/cel_execution_condition.go +++ b/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 } diff --git a/internal/rules/cel_execution_condition_test.go b/internal/rules/cel_execution_condition_test.go index 9ff6e609a..8bb24f47f 100644 --- a/internal/rules/cel_execution_condition_test.go +++ b/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) diff --git a/internal/rules/composite_error_handler.go b/internal/rules/composite_error_handler.go index 66170909d..0e412a0b1 100644 --- a/internal/rules/composite_error_handler.go +++ b/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 } diff --git a/internal/rules/composite_error_handler_test.go b/internal/rules/composite_error_handler_test.go index 6d0b260b5..9a092e41c 100644 --- a/internal/rules/composite_error_handler_test.go +++ b/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} diff --git a/internal/rules/conditional_error_handler.go b/internal/rules/conditional_error_handler.go new file mode 100644 index 000000000..4d5ec823a --- /dev/null +++ b/internal/rules/conditional_error_handler.go @@ -0,0 +1,50 @@ +// Copyright 2023 Dimitrij Drus +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 + +package rules + +import ( + "errors" + + "github.com/rs/zerolog" + + "github.com/dadrus/heimdall/internal/heimdall" +) + +var errErrorHandlerNotApplicable = errors.New("error handler not applicable") + +type conditionalErrorHandler struct { + h errorHandler + c executionCondition +} + +func (h *conditionalErrorHandler) Execute(ctx heimdall.Context, causeErr error) error { + logger := zerolog.Ctx(ctx.AppContext()) + + logger.Debug().Str("_id", h.h.ID()).Msg("Checking error handler execution condition") + + if canExecute, err := h.c.CanExecuteOnError(ctx, causeErr); err != nil { + return err + } else if canExecute { + return h.h.Execute(ctx, causeErr) + } + + logger.Debug().Str("_id", h.h.ID()).Msg("Error handler not applicable") + + return errErrorHandlerNotApplicable +} + +func (h *conditionalErrorHandler) ID() string { return h.h.ID() } diff --git a/internal/rules/conditional_error_handler_test.go b/internal/rules/conditional_error_handler_test.go new file mode 100644 index 000000000..8a88de57e --- /dev/null +++ b/internal/rules/conditional_error_handler_test.go @@ -0,0 +1,118 @@ +// Copyright 2022-2024 Dimitrij Drus +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 + +package rules + +import ( + "context" + "errors" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" + + "github.com/dadrus/heimdall/internal/heimdall/mocks" + rulemocks "github.com/dadrus/heimdall/internal/rules/mocks" +) + +func TestConditionalErrorHandlerExecute(t *testing.T) { + t.Parallel() + + for _, tc := range []struct { + uc string + configureMocks func(t *testing.T, c *rulemocks.ExecutionConditionMock, h *rulemocks.ErrorHandlerMock) + assert func(t *testing.T, err error) + }{ + { + uc: "executes if can", + configureMocks: func(t *testing.T, c *rulemocks.ExecutionConditionMock, h *rulemocks.ErrorHandlerMock) { + t.Helper() + + c.EXPECT().CanExecuteOnError(mock.Anything, mock.Anything).Return(true, nil) + h.EXPECT().Execute(mock.Anything, mock.Anything).Return(nil) + h.EXPECT().ID().Return("test") + }, + assert: func(t *testing.T, err error) { + t.Helper() + + require.NoError(t, err) + }, + }, + { + uc: "does not execute if can not", + configureMocks: func(t *testing.T, c *rulemocks.ExecutionConditionMock, h *rulemocks.ErrorHandlerMock) { + t.Helper() + + c.EXPECT().CanExecuteOnError(mock.Anything, mock.Anything).Return(false, nil) + h.EXPECT().ID().Return("test") + }, + assert: func(t *testing.T, err error) { + t.Helper() + + require.Error(t, err) + require.ErrorIs(t, err, errErrorHandlerNotApplicable) + }, + }, + { + uc: "does not execute if can check fails", + configureMocks: func(t *testing.T, c *rulemocks.ExecutionConditionMock, h *rulemocks.ErrorHandlerMock) { + t.Helper() + + c.EXPECT().CanExecuteOnError(mock.Anything, mock.Anything). + Return(true, errors.New("some error")) + h.EXPECT().ID().Return("test") + }, + assert: func(t *testing.T, err error) { + t.Helper() + + require.Error(t, err) + require.ErrorContains(t, err, "some error") + }, + }, + } { + t.Run(tc.uc, func(t *testing.T) { + // GIVEN + condition := rulemocks.NewExecutionConditionMock(t) + handler := rulemocks.NewErrorHandlerMock(t) + decorator := conditionalErrorHandler{c: condition, h: handler} + + ctx := mocks.NewContextMock(t) + ctx.EXPECT().AppContext().Return(context.Background()) + + tc.configureMocks(t, condition, handler) + + // WHEN + err := decorator.Execute(ctx, errors.New("test error")) + + // THEN + tc.assert(t, err) + }) + } +} + +func TestConditionalErrorHandlerID(t *testing.T) { + t.Parallel() + + condition := rulemocks.NewExecutionConditionMock(t) + handler := rulemocks.NewErrorHandlerMock(t) + handler.EXPECT().ID().Return("test") + + eh := conditionalErrorHandler{c: condition, h: handler} + + id := eh.ID() + assert.Equal(t, "test", id) +} diff --git a/internal/rules/conditional_subject_handler.go b/internal/rules/conditional_subject_handler.go index 267c60fd8..74cb89c4d 100644 --- a/internal/rules/conditional_subject_handler.go +++ b/internal/rules/conditional_subject_handler.go @@ -44,7 +44,7 @@ func (h *conditionalSubjectHandler) Execute(ctx heimdall.Context, sub *subject.S } } - if canExecute, err := h.c.CanExecute(ctx, sub); err != nil { + if canExecute, err := h.c.CanExecuteOnSubject(ctx, sub); err != nil { return err } else if canExecute { return h.h.Execute(ctx, sub) diff --git a/internal/rules/conditional_subject_handler_test.go b/internal/rules/conditional_subject_handler_test.go index 881c2a3e5..c38a32a52 100644 --- a/internal/rules/conditional_subject_handler_test.go +++ b/internal/rules/conditional_subject_handler_test.go @@ -42,7 +42,7 @@ func TestConditionalSubjectHandlerExecute(t *testing.T) { configureMocks: func(t *testing.T, c *rulemocks.ExecutionConditionMock, h *rulemocks.SubjectHandlerMock) { t.Helper() - c.EXPECT().CanExecute(mock.Anything, mock.Anything).Return(true, nil) + c.EXPECT().CanExecuteOnSubject(mock.Anything, mock.Anything).Return(true, nil) h.EXPECT().Execute(mock.Anything, mock.Anything).Return(nil) h.EXPECT().ID().Return("test") }, @@ -57,7 +57,7 @@ func TestConditionalSubjectHandlerExecute(t *testing.T) { configureMocks: func(t *testing.T, c *rulemocks.ExecutionConditionMock, h *rulemocks.SubjectHandlerMock) { t.Helper() - c.EXPECT().CanExecute(mock.Anything, mock.Anything).Return(false, nil) + c.EXPECT().CanExecuteOnSubject(mock.Anything, mock.Anything).Return(false, nil) h.EXPECT().ID().Return("test") }, assert: func(t *testing.T, err error) { @@ -71,7 +71,7 @@ func TestConditionalSubjectHandlerExecute(t *testing.T) { configureMocks: func(t *testing.T, c *rulemocks.ExecutionConditionMock, h *rulemocks.SubjectHandlerMock) { t.Helper() - c.EXPECT().CanExecute(mock.Anything, mock.Anything). + c.EXPECT().CanExecuteOnSubject(mock.Anything, mock.Anything). Return(true, testsupport.ErrTestPurpose) h.EXPECT().ID().Return("test") }, @@ -118,3 +118,16 @@ func TestConditionalSubjectHandlerContinueOnError(t *testing.T) { // THEN assert.True(t, ok) } + +func TestConditionalSubjectHandlerID(t *testing.T) { + t.Parallel() + + condition := rulemocks.NewExecutionConditionMock(t) + handler := rulemocks.NewSubjectHandlerMock(t) + handler.EXPECT().ID().Return("test") + + eh := conditionalSubjectHandler{c: condition, h: handler} + + id := eh.ID() + assert.Equal(t, "test", id) +} diff --git a/internal/rules/default_execution_condition.go b/internal/rules/default_execution_condition.go index 3759be5bf..acf1c67cc 100644 --- a/internal/rules/default_execution_condition.go +++ b/internal/rules/default_execution_condition.go @@ -23,6 +23,10 @@ import ( type defaultExecutionCondition struct{} -func (c defaultExecutionCondition) CanExecute(_ heimdall.Context, _ *subject.Subject) (bool, error) { +func (c defaultExecutionCondition) CanExecuteOnSubject(_ heimdall.Context, _ *subject.Subject) (bool, error) { + return true, nil +} + +func (c defaultExecutionCondition) CanExecuteOnError(_ heimdall.Context, _ error) (bool, error) { return true, nil } diff --git a/internal/rules/error_handler.go b/internal/rules/error_handler.go index 5423b68bf..75ee66aba 100644 --- a/internal/rules/error_handler.go +++ b/internal/rules/error_handler.go @@ -23,6 +23,6 @@ import ( //go:generate mockery --name errorHandler --structname ErrorHandlerMock type errorHandler interface { - CanExecute(ctx heimdall.Context, causeErr error) bool + ID() string Execute(ctx heimdall.Context, causeErr error) error } diff --git a/internal/rules/execution_condition.go b/internal/rules/execution_condition.go index 811664c03..fa4bc24af 100644 --- a/internal/rules/execution_condition.go +++ b/internal/rules/execution_condition.go @@ -24,5 +24,6 @@ import ( //go:generate mockery --name executionCondition --structname ExecutionConditionMock type executionCondition interface { - CanExecute(ctx heimdall.Context, sub *subject.Subject) (bool, error) + CanExecuteOnSubject(ctx heimdall.Context, sub *subject.Subject) (bool, error) + CanExecuteOnError(ctx heimdall.Context, err error) (bool, error) } diff --git a/internal/rules/mechanisms/errorhandlers/base_error_handler.go b/internal/rules/mechanisms/errorhandlers/base_error_handler.go deleted file mode 100644 index 548375ef9..000000000 --- a/internal/rules/mechanisms/errorhandlers/base_error_handler.go +++ /dev/null @@ -1,68 +0,0 @@ -// Copyright 2023 Dimitrij Drus -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -// -// SPDX-License-Identifier: Apache-2.0 - -package errorhandlers - -import ( - "errors" - - "github.com/google/cel-go/cel" - "github.com/rs/zerolog" - - "github.com/dadrus/heimdall/internal/heimdall" - "github.com/dadrus/heimdall/internal/rules/mechanisms/cellib" - "github.com/dadrus/heimdall/internal/x/errorchain" -) - -func newBaseErrorHandler(id, conditionExpression string) (*baseErrorHandler, error) { - env, err := cel.NewEnv(cellib.Library()) - if err != nil { - return nil, errorchain.NewWithMessage(heimdall.ErrInternal, "failed creating CEL environment").CausedBy(err) - } - - condition, err := cellib.CompileExpression(env, conditionExpression, "condition failed") - if err != nil { - return nil, errorchain.NewWithMessagef(heimdall.ErrConfiguration, - "failed to compile %s condition", conditionExpression).CausedBy(err) - } - - return &baseErrorHandler{id: id, c: condition}, nil -} - -type baseErrorHandler struct { - id string - c *cellib.CompiledExpression -} - -func (eh *baseErrorHandler) ID() string { return eh.id } - -func (eh *baseErrorHandler) CanExecute(ctx heimdall.Context, cause error) bool { - logger := zerolog.Ctx(ctx.AppContext()) - logger.Debug().Str("_id", eh.id).Msg("Checking error handler applicability") - - err := eh.c.Eval(map[string]any{"Request": ctx.Request(), "Error": cellib.WrapError(cause)}) - if err != nil { - if errors.Is(err, &cellib.EvalError{}) { - logger.Debug().Err(err).Str("_id", eh.id).Msg("Error handler not applicable") - } else { - logger.Error().Err(err).Str("_id", eh.id).Msg("Failed checking error handler applicability") - } - - return false - } - - return true -} diff --git a/internal/rules/mechanisms/errorhandlers/base_error_handler_test.go b/internal/rules/mechanisms/errorhandlers/base_error_handler_test.go deleted file mode 100644 index 0e93ac8da..000000000 --- a/internal/rules/mechanisms/errorhandlers/base_error_handler_test.go +++ /dev/null @@ -1,85 +0,0 @@ -// Copyright 2023 Dimitrij Drus -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -// -// SPDX-License-Identifier: Apache-2.0 - -package errorhandlers - -import ( - "context" - "net/http" - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - - "github.com/dadrus/heimdall/internal/heimdall" - "github.com/dadrus/heimdall/internal/heimdall/mocks" -) - -func TestNewBaseErrorHandler(t *testing.T) { - t.Parallel() - - for _, tc := range []struct { - expression string - error bool - }{ - {"true == true", false}, - {"foo == true", true}, - } { - t.Run(tc.expression, func(t *testing.T) { - base, err := newBaseErrorHandler("test", tc.expression) - - if tc.error { - require.Error(t, err) - require.Nil(t, base) - } else { - require.NoError(t, err) - require.NotNil(t, base) - assert.Equal(t, "test", base.ID()) - } - }) - } -} - -func TestBaseErrorHandlerCanExecute(t *testing.T) { - t.Parallel() - - for _, tc := range []struct { - expression string - req *heimdall.Request - cause error - expect bool - }{ - {"type(Error) == precondition_error", nil, heimdall.ErrAuthorization, false}, - {"Request.Method == 'GET'", &heimdall.Request{Method: http.MethodGet}, heimdall.ErrArgument, true}, - {"Request.URL == 'http://foo.bar'", nil, heimdall.ErrArgument, false}, - } { - t.Run(tc.expression, func(t *testing.T) { - // GIVEN - mctx := mocks.NewContextMock(t) - mctx.EXPECT().AppContext().Return(context.TODO()) - mctx.EXPECT().Request().Return(tc.req) - - base, err := newBaseErrorHandler("test", tc.expression) - require.NoError(t, err) - - // WHEN - result := base.CanExecute(mctx, tc.cause) - - // THEN - assert.Equal(t, tc.expect, result) - }) - } -} diff --git a/internal/rules/mechanisms/errorhandlers/default_error_handler.go b/internal/rules/mechanisms/errorhandlers/default_error_handler.go index f472c9cd8..9455ab256 100644 --- a/internal/rules/mechanisms/errorhandlers/default_error_handler.go +++ b/internal/rules/mechanisms/errorhandlers/default_error_handler.go @@ -20,6 +20,7 @@ import ( "github.com/rs/zerolog" "github.com/dadrus/heimdall/internal/heimdall" + "github.com/dadrus/heimdall/internal/x/errorchain" ) // by intention. Used only during application bootstrap @@ -44,8 +45,6 @@ func newDefaultErrorHandler(id string) *defaultErrorHandler { return &defaultErrorHandler{id: id} } -func (eh *defaultErrorHandler) CanExecute(_ heimdall.Context, _ error) bool { return true } - func (eh *defaultErrorHandler) Execute(ctx heimdall.Context, causeErr error) error { logger := zerolog.Ctx(ctx.AppContext()) logger.Debug().Str("_id", eh.id).Msg("Handling error using default error handler") @@ -55,6 +54,13 @@ func (eh *defaultErrorHandler) Execute(ctx heimdall.Context, causeErr error) err return nil } -func (eh *defaultErrorHandler) WithConfig(_ map[string]any) (ErrorHandler, error) { return eh, nil } +func (eh *defaultErrorHandler) WithConfig(conf map[string]any) (ErrorHandler, error) { + if len(conf) != 0 { + return nil, errorchain.NewWithMessage(heimdall.ErrConfiguration, + "reconfiguration of the default error handler is not supported") + } + + return eh, nil +} func (eh *defaultErrorHandler) ID() string { return eh.id } diff --git a/internal/rules/mechanisms/errorhandlers/default_error_handler_test.go b/internal/rules/mechanisms/errorhandlers/default_error_handler_test.go index c7cc2f9f3..acdc2a60c 100644 --- a/internal/rules/mechanisms/errorhandlers/default_error_handler_test.go +++ b/internal/rules/mechanisms/errorhandlers/default_error_handler_test.go @@ -38,7 +38,6 @@ func TestDefaultErrorHandlerExecution(t *testing.T) { errorHandler := newDefaultErrorHandler("foo") // WHEN & THEN - require.True(t, errorHandler.CanExecute(nil, nil)) require.NoError(t, errorHandler.Execute(ctx, heimdall.ErrConfiguration)) } @@ -47,16 +46,22 @@ func TestDefaultErrorHandlerPrototype(t *testing.T) { // GIVEN prototype := newDefaultErrorHandler("foo") + assert.Equal(t, "foo", prototype.ID()) // WHEN eh1, err1 := prototype.WithConfig(nil) eh2, err2 := prototype.WithConfig(map[string]any{"foo": "bar"}) + eh3, err3 := prototype.WithConfig(map[string]any{}) // THEN require.NoError(t, err1) assert.Equal(t, prototype, eh1) - require.NoError(t, err2) - assert.Equal(t, prototype, eh2) - assert.Equal(t, "foo", prototype.ID()) + require.Error(t, err2) + require.ErrorIs(t, err2, heimdall.ErrConfiguration) + require.ErrorContains(t, err2, "reconfiguration of the default error handler is not supported") + assert.Nil(t, eh2) + + require.NoError(t, err3) + assert.Equal(t, prototype, eh3) } diff --git a/internal/rules/mechanisms/errorhandlers/error_handler.go b/internal/rules/mechanisms/errorhandlers/error_handler.go index b80705b55..47f9c0011 100644 --- a/internal/rules/mechanisms/errorhandlers/error_handler.go +++ b/internal/rules/mechanisms/errorhandlers/error_handler.go @@ -24,7 +24,6 @@ import ( type ErrorHandler interface { ID() string - CanExecute(ctx heimdall.Context, causeErr error) bool Execute(ctx heimdall.Context, causeErr error) error WithConfig(config map[string]any) (ErrorHandler, error) } diff --git a/internal/rules/mechanisms/errorhandlers/mocks/error_handler.go b/internal/rules/mechanisms/errorhandlers/mocks/error_handler.go index f9d83a31f..8517265d9 100644 --- a/internal/rules/mechanisms/errorhandlers/mocks/error_handler.go +++ b/internal/rules/mechanisms/errorhandlers/mocks/error_handler.go @@ -1,4 +1,4 @@ -// Code generated by mockery v2.23.1. DO NOT EDIT. +// Code generated by mockery v2.42.1. DO NOT EDIT. package mocks @@ -22,53 +22,14 @@ func (_m *ErrorHandlerMock) EXPECT() *ErrorHandlerMock_Expecter { return &ErrorHandlerMock_Expecter{mock: &_m.Mock} } -// CanExecute provides a mock function with given fields: ctx, causeErr -func (_m *ErrorHandlerMock) CanExecute(ctx heimdall.Context, causeErr error) bool { - ret := _m.Called(ctx, causeErr) - - var r0 bool - if rf, ok := ret.Get(0).(func(heimdall.Context, error) bool); ok { - r0 = rf(ctx, causeErr) - } else { - r0 = ret.Get(0).(bool) - } - - return r0 -} - -// ErrorHandlerMock_CanExecute_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'CanExecute' -type ErrorHandlerMock_CanExecute_Call struct { - *mock.Call -} - -// CanExecute is a helper method to define mock.On call -// - ctx heimdall.Context -// - causeErr error -func (_e *ErrorHandlerMock_Expecter) CanExecute(ctx interface{}, causeErr interface{}) *ErrorHandlerMock_CanExecute_Call { - return &ErrorHandlerMock_CanExecute_Call{Call: _e.mock.On("CanExecute", ctx, causeErr)} -} - -func (_c *ErrorHandlerMock_CanExecute_Call) Run(run func(ctx heimdall.Context, causeErr error)) *ErrorHandlerMock_CanExecute_Call { - _c.Call.Run(func(args mock.Arguments) { - run(args[0].(heimdall.Context), args[1].(error)) - }) - return _c -} - -func (_c *ErrorHandlerMock_CanExecute_Call) Return(_a0 bool) *ErrorHandlerMock_CanExecute_Call { - _c.Call.Return(_a0) - return _c -} - -func (_c *ErrorHandlerMock_CanExecute_Call) RunAndReturn(run func(heimdall.Context, error) bool) *ErrorHandlerMock_CanExecute_Call { - _c.Call.Return(run) - return _c -} - // Execute provides a mock function with given fields: ctx, causeErr func (_m *ErrorHandlerMock) Execute(ctx heimdall.Context, causeErr error) error { ret := _m.Called(ctx, causeErr) + if len(ret) == 0 { + panic("no return value specified for Execute") + } + var r0 error if rf, ok := ret.Get(0).(func(heimdall.Context, error) error); ok { r0 = rf(ctx, causeErr) @@ -112,6 +73,10 @@ func (_c *ErrorHandlerMock_Execute_Call) RunAndReturn(run func(heimdall.Context, func (_m *ErrorHandlerMock) ID() string { ret := _m.Called() + if len(ret) == 0 { + panic("no return value specified for ID") + } + var r0 string if rf, ok := ret.Get(0).(func() string); ok { r0 = rf() @@ -153,6 +118,10 @@ func (_c *ErrorHandlerMock_ID_Call) RunAndReturn(run func() string) *ErrorHandle func (_m *ErrorHandlerMock) WithConfig(config map[string]interface{}) (errorhandlers.ErrorHandler, error) { ret := _m.Called(config) + if len(ret) == 0 { + panic("no return value specified for WithConfig") + } + var r0 errorhandlers.ErrorHandler var r1 error if rf, ok := ret.Get(0).(func(map[string]interface{}) (errorhandlers.ErrorHandler, error)); ok { @@ -203,13 +172,12 @@ func (_c *ErrorHandlerMock_WithConfig_Call) RunAndReturn(run func(map[string]int return _c } -type mockConstructorTestingTNewErrorHandlerMock interface { +// NewErrorHandlerMock creates a new instance of ErrorHandlerMock. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +// The first argument is typically a *testing.T value. +func NewErrorHandlerMock(t interface { mock.TestingT Cleanup(func()) -} - -// NewErrorHandlerMock creates a new instance of ErrorHandlerMock. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. -func NewErrorHandlerMock(t mockConstructorTestingTNewErrorHandlerMock) *ErrorHandlerMock { +}) *ErrorHandlerMock { mock := &ErrorHandlerMock{} mock.Mock.Test(t) diff --git a/internal/rules/mechanisms/errorhandlers/redirect_error_handler.go b/internal/rules/mechanisms/errorhandlers/redirect_error_handler.go index 77b211223..9fc0cb9a2 100644 --- a/internal/rules/mechanisms/errorhandlers/redirect_error_handler.go +++ b/internal/rules/mechanisms/errorhandlers/redirect_error_handler.go @@ -44,17 +44,15 @@ func init() { } type redirectErrorHandler struct { - *baseErrorHandler - + id string to template.Template code int } func newRedirectErrorHandler(id string, rawConfig map[string]any) (*redirectErrorHandler, error) { type Config struct { - Condition string `mapstructure:"if" validate:"required"` - To template.Template `mapstructure:"to" validate:"required"` - Code int `mapstructure:"code"` + To template.Template `mapstructure:"to" validate:"required"` + Code int `mapstructure:"code"` } var conf Config @@ -62,18 +60,15 @@ func newRedirectErrorHandler(id string, rawConfig map[string]any) (*redirectErro return nil, err } - base, err := newBaseErrorHandler(id, conf.Condition) - if err != nil { - return nil, err - } - return &redirectErrorHandler{ - baseErrorHandler: base, - to: conf.To, - code: x.IfThenElse(conf.Code != 0, conf.Code, http.StatusFound), + id: id, + to: conf.To, + code: x.IfThenElse(conf.Code != 0, conf.Code, http.StatusFound), }, nil } +func (eh *redirectErrorHandler) ID() string { return eh.id } + func (eh *redirectErrorHandler) Execute(ctx heimdall.Context, _ error) error { logger := zerolog.Ctx(ctx.AppContext()) logger.Debug().Str("_id", eh.id).Msg("Handling error using redirect error handler") @@ -95,28 +90,11 @@ func (eh *redirectErrorHandler) Execute(ctx heimdall.Context, _ error) error { return nil } -func (eh *redirectErrorHandler) WithConfig(rawConfig map[string]any) (ErrorHandler, error) { - if len(rawConfig) == 0 { - return eh, nil - } - - type Config struct { - Condition string `mapstructure:"if" validate:"required"` - } - - var conf Config - if err := decodeConfig(ErrorHandlerRedirect, rawConfig, &conf); err != nil { - return nil, err - } - - base, err := newBaseErrorHandler(eh.id, conf.Condition) - if err != nil { - return nil, err +func (eh *redirectErrorHandler) WithConfig(conf map[string]any) (ErrorHandler, error) { + if len(conf) != 0 { + return nil, errorchain.NewWithMessage(heimdall.ErrConfiguration, + "reconfiguration of a redirect error handler is not supported") } - return &redirectErrorHandler{ - baseErrorHandler: base, - to: eh.to, - code: eh.code, - }, nil + return eh, nil } diff --git a/internal/rules/mechanisms/errorhandlers/redirect_error_handler_test.go b/internal/rules/mechanisms/errorhandlers/redirect_error_handler_test.go index 1c2b4bdaa..596591679 100644 --- a/internal/rules/mechanisms/errorhandlers/redirect_error_handler_test.go +++ b/internal/rules/mechanisms/errorhandlers/redirect_error_handler_test.go @@ -40,7 +40,7 @@ func TestCreateRedirectErrorHandler(t *testing.T) { assert func(t *testing.T, err error, redEH *redirectErrorHandler) }{ { - uc: "configuration without required 'To' parameter", + uc: "configuration without required 'to' parameter", config: []byte(`code: 302`), assert: func(t *testing.T, err error, _ *redirectErrorHandler) { t.Helper() @@ -50,50 +50,10 @@ func TestCreateRedirectErrorHandler(t *testing.T) { assert.Contains(t, err.Error(), "'to' is a required field") }, }, - { - uc: "configuration without required 'if' parameter", - config: []byte(`to: http://foo.bar`), - assert: func(t *testing.T, err error, _ *redirectErrorHandler) { - t.Helper() - - require.Error(t, err) - require.ErrorIs(t, err, heimdall.ErrConfiguration) - assert.Contains(t, err.Error(), "'if' is a required field") - }, - }, - { - uc: "with empty 'if' configuration", - config: []byte(` -to: http://foo.bar -if: "" -`), - assert: func(t *testing.T, err error, _ *redirectErrorHandler) { - t.Helper() - - require.Error(t, err) - require.ErrorIs(t, err, heimdall.ErrConfiguration) - assert.Contains(t, err.Error(), "'if' is a required field") - }, - }, - { - uc: "with invalid 'if' conditions configuration", - config: []byte(` -to: http://foo.bar -if: foo -`), - assert: func(t *testing.T, err error, _ *redirectErrorHandler) { - t.Helper() - - require.Error(t, err) - require.ErrorIs(t, err, heimdall.ErrConfiguration) - assert.Contains(t, err.Error(), "failed to compile") - }, - }, { uc: "with unexpected fields in configuration", config: []byte(` to: http://foo.bar -bar: foo if: true == false `), assert: func(t *testing.T, err error, _ *redirectErrorHandler) { @@ -105,11 +65,8 @@ if: true == false }, }, { - uc: "with minimal valid configuration", - config: []byte(` -to: http://foo.bar -if: Error.Source == "foo" -`), + uc: "with minimal valid configuration", + config: []byte(`to: http://foo.bar`), assert: func(t *testing.T, err error, redEH *redirectErrorHandler) { t.Helper() @@ -122,7 +79,6 @@ if: Error.Source == "foo" assert.Equal(t, "http://foo.bar", toURL) assert.Equal(t, http.StatusFound, redEH.code) - assert.NotNil(t, redEH.c) }, }, { @@ -130,7 +86,6 @@ if: Error.Source == "foo" config: []byte(` to: http://foo.bar?origin={{ .Request.URL | urlenc }} code: 301 -if: type(Error) == authentication_error `), assert: func(t *testing.T, err error, redEH *redirectErrorHandler) { t.Helper() @@ -152,7 +107,6 @@ if: type(Error) == authentication_error assert.Equal(t, "http://foo.bar?origin=http%3A%2F%2Ffoobar.baz%2Fzab", toURL) assert.Equal(t, http.StatusMovedPermanently, redEH.code) - assert.NotNil(t, redEH.c) }, }, } { @@ -179,87 +133,36 @@ func TestCreateRedirectErrorHandlerFromPrototype(t *testing.T) { assert func(t *testing.T, err error, prototype *redirectErrorHandler, configured *redirectErrorHandler) }{ { - uc: "no new configuration provided", - prototypeConfig: []byte(` -to: http://foo.bar -if: type(Error) == authentication_error -`), + uc: "no new configuration provided", + prototypeConfig: []byte(`to: http://foo.bar`), assert: func(t *testing.T, err error, prototype *redirectErrorHandler, configured *redirectErrorHandler) { t.Helper() require.NoError(t, err) assert.Equal(t, prototype, configured) - assert.Equal(t, "no new configuration provided", configured.ID()) }, }, { - uc: "empty configuration provided", - prototypeConfig: []byte(` -to: http://foo.bar -if: type(Error) == authentication_error -`), - config: []byte(``), + uc: "empty configuration provided", + prototypeConfig: []byte(`to: http://foo.bar`), + config: []byte(``), assert: func(t *testing.T, err error, prototype *redirectErrorHandler, configured *redirectErrorHandler) { t.Helper() require.NoError(t, err) assert.Equal(t, prototype, configured) - assert.Equal(t, "empty configuration provided", configured.ID()) - }, - }, - { - uc: "unsupported fields provided", - prototypeConfig: []byte(` -to: http://foo.bar -if: type(Error) == authentication_error -`), - config: []byte(`to: http://foo.bar`), - assert: func(t *testing.T, err error, _ *redirectErrorHandler, _ *redirectErrorHandler) { - t.Helper() - - require.Error(t, err) - require.ErrorIs(t, err, heimdall.ErrConfiguration) - assert.Contains(t, err.Error(), "failed decoding") }, }, { - uc: "invalid 'if' condition", - prototypeConfig: []byte(` -to: http://foo.bar -if: type(Error) == authentication_error -`), - config: []byte(`if: foo`), + uc: "unsupported configuration provided", + prototypeConfig: []byte(`to: http://foo.bar`), + config: []byte(`to: http://foo.bar`), assert: func(t *testing.T, err error, _ *redirectErrorHandler, _ *redirectErrorHandler) { t.Helper() require.Error(t, err) require.ErrorIs(t, err, heimdall.ErrConfiguration) - assert.Contains(t, err.Error(), "failed to compile") - }, - }, - { - uc: "required 'if' field provided", - prototypeConfig: []byte(` -to: http://foo.bar -code: 301 -if: type(Error) in [authentication_error, authorization_error] -`), - config: []byte(`if: type(Error) == precondition_error`), - assert: func(t *testing.T, err error, prototype *redirectErrorHandler, configured *redirectErrorHandler) { - t.Helper() - - ctx := mocks.NewContextMock(t) - ctx.EXPECT().AppContext().Return(context.TODO()) - ctx.EXPECT().Request().Return(nil) - - require.NoError(t, err) - assert.NotEqual(t, prototype, configured) - assert.NotNil(t, configured) - assert.Equal(t, "required 'if' field provided", configured.ID()) - assert.Equal(t, prototype.to, configured.to) - assert.Equal(t, prototype.code, configured.code) - assert.NotEqual(t, prototype.c, configured.c) - assert.True(t, configured.CanExecute(ctx, heimdall.ErrArgument)) + require.ErrorContains(t, err, "reconfiguration of a redirect error handler is not supported") }, }, } { @@ -300,55 +203,29 @@ func TestRedirectErrorHandlerExecute(t *testing.T) { config []byte error error configureContext func(t *testing.T, ctx *mocks.ContextMock) - assert func(t *testing.T, wasResponsible bool, err error) + assert func(t *testing.T, err error) }{ { - uc: "not responsible for error", - config: []byte(` -to: http://foo.bar -if: type(Error) == authentication_error -`), - error: heimdall.ErrInternal, - configureContext: func(t *testing.T, ctx *mocks.ContextMock) { - t.Helper() - - ctx.EXPECT().Request().Return(nil) - }, - assert: func(t *testing.T, wasResponsible bool, err error) { - t.Helper() - - require.NoError(t, err) - assert.False(t, wasResponsible) - }, - }, - { - uc: "responsible for error but with template rendering error", - config: []byte(` -to: http://foo.bar={{ len .foobar }} -if: type(Error) == authentication_error -`), - error: heimdall.ErrAuthentication, + uc: "with template rendering error", + config: []byte(`to: http://foo.bar={{ len .foobar }}`), + error: heimdall.ErrAuthentication, configureContext: func(t *testing.T, ctx *mocks.ContextMock) { t.Helper() ctx.EXPECT().Request().Return(nil) }, - assert: func(t *testing.T, wasResponsible bool, err error) { + assert: func(t *testing.T, err error) { t.Helper() require.Error(t, err) require.ErrorIs(t, err, heimdall.ErrInternal) assert.Contains(t, err.Error(), "failed to render") - assert.True(t, wasResponsible) }, }, { - uc: "responsible without return to url templating", - config: []byte(` -to: http://foo.bar -if: type(Error) == authentication_error -`), - error: heimdall.ErrAuthentication, + uc: "without return to url templating", + config: []byte(`to: http://foo.bar`), + error: heimdall.ErrAuthentication, configureContext: func(t *testing.T, ctx *mocks.ContextMock) { t.Helper() @@ -363,19 +240,17 @@ if: type(Error) == authentication_error return true })) }, - assert: func(t *testing.T, wasResponsible bool, err error) { + assert: func(t *testing.T, err error) { t.Helper() require.NoError(t, err) - assert.True(t, wasResponsible) }, }, { - uc: "responsible with template and code set", + uc: "with template and code set", config: []byte(` to: http://foo.bar?origin={{ .Request.URL | urlenc }} code: 300 -if: type(Error) == authentication_error `), error: heimdall.ErrAuthentication, configureContext: func(t *testing.T, ctx *mocks.ContextMock) { @@ -401,11 +276,10 @@ if: type(Error) == authentication_error return true })) }, - assert: func(t *testing.T, wasResponsible bool, err error) { + assert: func(t *testing.T, err error) { t.Helper() require.NoError(t, err) - assert.True(t, wasResponsible) }, }, } { @@ -422,19 +296,11 @@ if: type(Error) == authentication_error errorHandler, err := newRedirectErrorHandler("foo", conf) require.NoError(t, err) - var ( - isResponsible bool - execErr error - ) - // WHEN - isResponsible = errorHandler.CanExecute(mctx, tc.error) - if isResponsible { - execErr = errorHandler.Execute(mctx, tc.error) - } + execErr := errorHandler.Execute(mctx, tc.error) // THEN - tc.assert(t, isResponsible, execErr) + tc.assert(t, execErr) }) } } diff --git a/internal/rules/mechanisms/errorhandlers/www_authenticate_error_handler.go b/internal/rules/mechanisms/errorhandlers/www_authenticate_error_handler.go index 7ba2fc6ab..1ccf53db1 100644 --- a/internal/rules/mechanisms/errorhandlers/www_authenticate_error_handler.go +++ b/internal/rules/mechanisms/errorhandlers/www_authenticate_error_handler.go @@ -40,15 +40,13 @@ func init() { } type wwwAuthenticateErrorHandler struct { - *baseErrorHandler - + id string realm string } func newWWWAuthenticateErrorHandler(id string, rawConfig map[string]any) (*wwwAuthenticateErrorHandler, error) { type Config struct { - Condition string `mapstructure:"if" validate:"required"` - Realm string `mapstructure:"realm"` + Realm string `mapstructure:"realm"` } var conf Config @@ -56,17 +54,14 @@ func newWWWAuthenticateErrorHandler(id string, rawConfig map[string]any) (*wwwAu return nil, err } - base, err := newBaseErrorHandler(id, conf.Condition) - if err != nil { - return nil, err - } - return &wwwAuthenticateErrorHandler{ - baseErrorHandler: base, - realm: x.IfThenElse(len(conf.Realm) != 0, conf.Realm, "Please authenticate"), + id: id, + realm: x.IfThenElse(len(conf.Realm) != 0, conf.Realm, "Please authenticate"), }, nil } +func (eh *wwwAuthenticateErrorHandler) ID() string { return eh.id } + func (eh *wwwAuthenticateErrorHandler) Execute(ctx heimdall.Context, _ error) error { logger := zerolog.Ctx(ctx.AppContext()) logger.Debug().Str("_id", eh.id).Msg("Handling error using www-authenticate error handler") @@ -83,13 +78,11 @@ func (eh *wwwAuthenticateErrorHandler) WithConfig(rawConfig map[string]any) (Err } type Config struct { - Condition string `mapstructure:"if"` - Realm *string `mapstructure:"realm"` + Realm string `mapstructure:"realm"` } var ( conf Config - base *baseErrorHandler err error ) @@ -97,19 +90,8 @@ func (eh *wwwAuthenticateErrorHandler) WithConfig(rawConfig map[string]any) (Err return nil, err } - if len(conf.Condition) != 0 { - base, err = newBaseErrorHandler(eh.id, conf.Condition) - if err != nil { - return nil, err - } - } else { - base = eh.baseErrorHandler - } - return &wwwAuthenticateErrorHandler{ - baseErrorHandler: base, - realm: x.IfThenElseExec(conf.Realm != nil, - func() string { return *conf.Realm }, - func() string { return eh.realm }), + id: eh.id, + realm: conf.Realm, }, nil } diff --git a/internal/rules/mechanisms/errorhandlers/www_authenticate_error_handler_test.go b/internal/rules/mechanisms/errorhandlers/www_authenticate_error_handler_test.go index 5347a32cb..7366cec2a 100644 --- a/internal/rules/mechanisms/errorhandlers/www_authenticate_error_handler_test.go +++ b/internal/rules/mechanisms/errorhandlers/www_authenticate_error_handler_test.go @@ -38,83 +38,34 @@ func TestCreateWWWAuthenticateErrorHandler(t *testing.T) { config []byte assert func(t *testing.T, err error, errorHandler *wwwAuthenticateErrorHandler) }{ - { - uc: "configuration without required 'if' parameter", - config: []byte(`realm: FooBar`), - assert: func(t *testing.T, err error, _ *wwwAuthenticateErrorHandler) { - t.Helper() - - require.Error(t, err) - require.ErrorIs(t, err, heimdall.ErrConfiguration) - assert.Contains(t, err.Error(), "'if' is a required field") - }, - }, - { - uc: "without provided configuration", - assert: func(t *testing.T, err error, _ *wwwAuthenticateErrorHandler) { - t.Helper() - - require.Error(t, err) - require.ErrorIs(t, err, heimdall.ErrConfiguration) - assert.Contains(t, err.Error(), "'if' is a required field") - }, - }, - { - uc: "with empty 'if' configuration", - config: []byte(`if: ""`), - assert: func(t *testing.T, err error, _ *wwwAuthenticateErrorHandler) { - t.Helper() - - require.Error(t, err) - require.ErrorIs(t, err, heimdall.ErrConfiguration) - assert.Contains(t, err.Error(), "'if' is a required field") - }, - }, - { - uc: "with invalid 'if' configuration", - config: []byte(`if: foo`), - assert: func(t *testing.T, err error, _ *wwwAuthenticateErrorHandler) { - t.Helper() - - require.Error(t, err) - require.ErrorIs(t, err, heimdall.ErrConfiguration) - assert.Contains(t, err.Error(), "failed to compile") - }, - }, { uc: "with configuration containing unsupported fields", config: []byte(` realm: FooBar if: type(Error) == authentication_error -foo: bar `), assert: func(t *testing.T, err error, _ *wwwAuthenticateErrorHandler) { t.Helper() require.Error(t, err) require.ErrorIs(t, err, heimdall.ErrConfiguration) - assert.Contains(t, err.Error(), "failed decoding") + require.ErrorContains(t, err, "failed decoding") }, }, { - uc: "with minimum required configuration", - config: []byte(`if: type(Error) == authentication_error`), + uc: "without configuration (minimal configuration)", assert: func(t *testing.T, err error, errorHandler *wwwAuthenticateErrorHandler) { t.Helper() require.NoError(t, err) require.NotNil(t, errorHandler) - assert.Equal(t, "with minimum required configuration", errorHandler.ID()) + assert.Equal(t, "without configuration (minimal configuration)", errorHandler.ID()) assert.Equal(t, "Please authenticate", errorHandler.realm) - require.NotNil(t, errorHandler.c) }, }, { - uc: "with all possible attributes", - config: []byte(` -realm: "What is your password" -if: type(Error) == precondition_error -`), + uc: "with all possible attributes", + config: []byte(`realm: "What is your password"`), assert: func(t *testing.T, err error, errorHandler *wwwAuthenticateErrorHandler) { t.Helper() @@ -122,7 +73,6 @@ if: type(Error) == precondition_error require.NotNil(t, errorHandler) assert.Equal(t, "with all possible attributes", errorHandler.ID()) assert.Equal(t, "What is your password", errorHandler.realm) - require.NotNil(t, errorHandler.c) }, }, } { @@ -151,7 +101,7 @@ func TestCreateWWWAuthenticateErrorHandlerFromPrototype(t *testing.T) { }{ { uc: "no new configuration provided", - prototypeConfig: []byte(`if: type(Error) == authentication_error`), + prototypeConfig: []byte(`realm: "foo"`), assert: func(t *testing.T, err error, prototype *wwwAuthenticateErrorHandler, configured *wwwAuthenticateErrorHandler, ) { @@ -159,12 +109,11 @@ func TestCreateWWWAuthenticateErrorHandlerFromPrototype(t *testing.T) { require.NoError(t, err) assert.Equal(t, prototype, configured) - assert.Equal(t, "no new configuration provided", configured.ID()) }, }, { uc: "empty configuration provided", - prototypeConfig: []byte(`if: type(Error) == authentication_error`), + prototypeConfig: []byte(`realm: "foo"`), config: []byte(``), assert: func(t *testing.T, err error, prototype *wwwAuthenticateErrorHandler, configured *wwwAuthenticateErrorHandler, @@ -173,45 +122,11 @@ func TestCreateWWWAuthenticateErrorHandlerFromPrototype(t *testing.T) { require.NoError(t, err) assert.Equal(t, prototype, configured) - assert.Equal(t, "empty configuration provided", configured.ID()) - }, - }, - { - uc: "unsupported fields provided", - prototypeConfig: []byte(`if: type(Error) == authentication_error`), - config: []byte(`to: http://foo.bar`), - assert: func(t *testing.T, err error, _ *wwwAuthenticateErrorHandler, - _ *wwwAuthenticateErrorHandler, - ) { - t.Helper() - - require.Error(t, err) - require.ErrorIs(t, err, heimdall.ErrConfiguration) - assert.Contains(t, err.Error(), "failed decoding") }, }, { - uc: "with 'if' reconfigured", - prototypeConfig: []byte(`if: type(Error) in [authentication_error, authorization_error]`), - config: []byte(`if: type(Error) == precondition_error`), - assert: func(t *testing.T, err error, prototype *wwwAuthenticateErrorHandler, - configured *wwwAuthenticateErrorHandler, - ) { - t.Helper() - - require.NoError(t, err) - assert.NotEqual(t, prototype, configured) - assert.NotNil(t, configured) - assert.Equal(t, "with 'if' reconfigured", configured.ID()) - assert.Equal(t, "Please authenticate", prototype.realm) - assert.Equal(t, prototype.realm, configured.realm) - assert.NotEqual(t, prototype.c, configured.c) - }, - }, - { - uc: "with invalid 'if' reconfigured", - prototypeConfig: []byte(`if: type(Error) in [authentication_error, authorization_error]`), - config: []byte(`if: foo`), + uc: "unsupported fields provided", + config: []byte(`to: http://foo.bar`), assert: func(t *testing.T, err error, _ *wwwAuthenticateErrorHandler, _ *wwwAuthenticateErrorHandler, ) { @@ -219,12 +134,12 @@ func TestCreateWWWAuthenticateErrorHandlerFromPrototype(t *testing.T) { require.Error(t, err) require.ErrorIs(t, err, heimdall.ErrConfiguration) - assert.Contains(t, err.Error(), "failed to compile") + require.ErrorContains(t, err, "failed decoding") }, }, { uc: "with 'realm' reconfigured", - prototypeConfig: []byte(`if: type(Error) == authentication_error`), + prototypeConfig: []byte(`realm: "Foobar"`), config: []byte(`realm: "You password please"`), assert: func(t *testing.T, err error, prototype *wwwAuthenticateErrorHandler, configured *wwwAuthenticateErrorHandler, @@ -237,7 +152,6 @@ func TestCreateWWWAuthenticateErrorHandlerFromPrototype(t *testing.T) { assert.Equal(t, "with 'realm' reconfigured", configured.ID()) assert.NotEqual(t, prototype.realm, configured.realm) assert.Equal(t, "You password please", configured.realm) - assert.Equal(t, prototype.c, configured.c) }, }, } { @@ -278,32 +192,14 @@ func TestWWWAuthenticateErrorHandlerExecute(t *testing.T) { config []byte error error configureContext func(t *testing.T, ctx *mocks.ContextMock) - assert func(t *testing.T, wasResponsible bool, err error) + assert func(t *testing.T, err error) }{ { - uc: "not responsible for error", - config: []byte(`if: type(Error) == authentication_error`), - error: heimdall.ErrInternal, - configureContext: func(t *testing.T, ctx *mocks.ContextMock) { - t.Helper() - - ctx.EXPECT().Request().Return(nil) - }, - assert: func(t *testing.T, wasResponsible bool, err error) { - t.Helper() - - require.NoError(t, err) - assert.False(t, wasResponsible) - }, - }, - { - uc: "responsible for error with default realm", - config: []byte(`if: type(Error) == authentication_error`), - error: heimdall.ErrAuthentication, + uc: "with default realm", + error: heimdall.ErrAuthentication, configureContext: func(t *testing.T, ctx *mocks.ContextMock) { t.Helper() - ctx.EXPECT().Request().Return(nil) ctx.EXPECT().SetPipelineError(heimdall.ErrAuthentication) ctx.EXPECT().AddHeaderForUpstream("WWW-Authenticate", mock.MatchedBy(func(val string) bool { @@ -314,24 +210,19 @@ func TestWWWAuthenticateErrorHandlerExecute(t *testing.T) { return true })) }, - assert: func(t *testing.T, wasResponsible bool, err error) { + assert: func(t *testing.T, err error) { t.Helper() require.NoError(t, err) - assert.True(t, wasResponsible) }, }, { - uc: "responsible for error with custom realm", - config: []byte(` -realm: "Your password please" -if: type(Error) == authentication_error -`), - error: heimdall.ErrAuthentication, + uc: "with custom realm", + config: []byte(`realm: "Your password please"`), + error: heimdall.ErrAuthentication, configureContext: func(t *testing.T, ctx *mocks.ContextMock) { t.Helper() - ctx.EXPECT().Request().Return(nil) ctx.EXPECT().SetPipelineError(heimdall.ErrAuthentication) ctx.EXPECT().AddHeaderForUpstream("WWW-Authenticate", mock.MatchedBy(func(val string) bool { @@ -342,11 +233,10 @@ if: type(Error) == authentication_error return true })) }, - assert: func(t *testing.T, wasResponsible bool, err error) { + assert: func(t *testing.T, err error) { t.Helper() require.NoError(t, err) - assert.True(t, wasResponsible) }, }, } { @@ -363,19 +253,11 @@ if: type(Error) == authentication_error errorHandler, err := newWWWAuthenticateErrorHandler("foo", conf) require.NoError(t, err) - var ( - isResponsible bool - execErr error - ) - // WHEN - isResponsible = errorHandler.CanExecute(mctx, tc.error) - if isResponsible { - execErr = errorHandler.Execute(mctx, tc.error) - } + execErr := errorHandler.Execute(mctx, tc.error) // THEN - tc.assert(t, isResponsible, execErr) + tc.assert(t, execErr) }) } } diff --git a/internal/rules/mocks/error_handler.go b/internal/rules/mocks/error_handler.go index 760c9a28d..99cba6552 100644 --- a/internal/rules/mocks/error_handler.go +++ b/internal/rules/mocks/error_handler.go @@ -1,4 +1,4 @@ -// Code generated by mockery v2.23.1. DO NOT EDIT. +// Code generated by mockery v2.42.1. DO NOT EDIT. package mocks @@ -20,99 +20,104 @@ func (_m *ErrorHandlerMock) EXPECT() *ErrorHandlerMock_Expecter { return &ErrorHandlerMock_Expecter{mock: &_m.Mock} } -// CanExecute provides a mock function with given fields: ctx, causeErr -func (_m *ErrorHandlerMock) CanExecute(ctx heimdall.Context, causeErr error) bool { +// Execute provides a mock function with given fields: ctx, causeErr +func (_m *ErrorHandlerMock) Execute(ctx heimdall.Context, causeErr error) error { ret := _m.Called(ctx, causeErr) - var r0 bool - if rf, ok := ret.Get(0).(func(heimdall.Context, error) bool); ok { + if len(ret) == 0 { + panic("no return value specified for Execute") + } + + var r0 error + if rf, ok := ret.Get(0).(func(heimdall.Context, error) error); ok { r0 = rf(ctx, causeErr) } else { - r0 = ret.Get(0).(bool) + r0 = ret.Error(0) } return r0 } -// ErrorHandlerMock_CanExecute_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'CanExecute' -type ErrorHandlerMock_CanExecute_Call struct { +// ErrorHandlerMock_Execute_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Execute' +type ErrorHandlerMock_Execute_Call struct { *mock.Call } -// CanExecute is a helper method to define mock.On call +// Execute is a helper method to define mock.On call // - ctx heimdall.Context // - causeErr error -func (_e *ErrorHandlerMock_Expecter) CanExecute(ctx interface{}, causeErr interface{}) *ErrorHandlerMock_CanExecute_Call { - return &ErrorHandlerMock_CanExecute_Call{Call: _e.mock.On("CanExecute", ctx, causeErr)} +func (_e *ErrorHandlerMock_Expecter) Execute(ctx interface{}, causeErr interface{}) *ErrorHandlerMock_Execute_Call { + return &ErrorHandlerMock_Execute_Call{Call: _e.mock.On("Execute", ctx, causeErr)} } -func (_c *ErrorHandlerMock_CanExecute_Call) Run(run func(ctx heimdall.Context, causeErr error)) *ErrorHandlerMock_CanExecute_Call { +func (_c *ErrorHandlerMock_Execute_Call) Run(run func(ctx heimdall.Context, causeErr error)) *ErrorHandlerMock_Execute_Call { _c.Call.Run(func(args mock.Arguments) { run(args[0].(heimdall.Context), args[1].(error)) }) return _c } -func (_c *ErrorHandlerMock_CanExecute_Call) Return(_a0 bool) *ErrorHandlerMock_CanExecute_Call { +func (_c *ErrorHandlerMock_Execute_Call) Return(_a0 error) *ErrorHandlerMock_Execute_Call { _c.Call.Return(_a0) return _c } -func (_c *ErrorHandlerMock_CanExecute_Call) RunAndReturn(run func(heimdall.Context, error) bool) *ErrorHandlerMock_CanExecute_Call { +func (_c *ErrorHandlerMock_Execute_Call) RunAndReturn(run func(heimdall.Context, error) error) *ErrorHandlerMock_Execute_Call { _c.Call.Return(run) return _c } -// Execute provides a mock function with given fields: ctx, causeErr -func (_m *ErrorHandlerMock) Execute(ctx heimdall.Context, causeErr error) error { - ret := _m.Called(ctx, causeErr) +// ID provides a mock function with given fields: +func (_m *ErrorHandlerMock) ID() string { + ret := _m.Called() - var r0 error - if rf, ok := ret.Get(0).(func(heimdall.Context, error) error); ok { - r0 = rf(ctx, causeErr) + if len(ret) == 0 { + panic("no return value specified for ID") + } + + var r0 string + if rf, ok := ret.Get(0).(func() string); ok { + r0 = rf() } else { - r0 = ret.Error(0) + r0 = ret.Get(0).(string) } return r0 } -// ErrorHandlerMock_Execute_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Execute' -type ErrorHandlerMock_Execute_Call struct { +// ErrorHandlerMock_ID_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'ID' +type ErrorHandlerMock_ID_Call struct { *mock.Call } -// Execute is a helper method to define mock.On call -// - ctx heimdall.Context -// - causeErr error -func (_e *ErrorHandlerMock_Expecter) Execute(ctx interface{}, causeErr interface{}) *ErrorHandlerMock_Execute_Call { - return &ErrorHandlerMock_Execute_Call{Call: _e.mock.On("Execute", ctx, causeErr)} +// ID is a helper method to define mock.On call +func (_e *ErrorHandlerMock_Expecter) ID() *ErrorHandlerMock_ID_Call { + return &ErrorHandlerMock_ID_Call{Call: _e.mock.On("ID")} } -func (_c *ErrorHandlerMock_Execute_Call) Run(run func(ctx heimdall.Context, causeErr error)) *ErrorHandlerMock_Execute_Call { +func (_c *ErrorHandlerMock_ID_Call) Run(run func()) *ErrorHandlerMock_ID_Call { _c.Call.Run(func(args mock.Arguments) { - run(args[0].(heimdall.Context), args[1].(error)) + run() }) return _c } -func (_c *ErrorHandlerMock_Execute_Call) Return(_a0 error) *ErrorHandlerMock_Execute_Call { +func (_c *ErrorHandlerMock_ID_Call) Return(_a0 string) *ErrorHandlerMock_ID_Call { _c.Call.Return(_a0) return _c } -func (_c *ErrorHandlerMock_Execute_Call) RunAndReturn(run func(heimdall.Context, error) error) *ErrorHandlerMock_Execute_Call { +func (_c *ErrorHandlerMock_ID_Call) RunAndReturn(run func() string) *ErrorHandlerMock_ID_Call { _c.Call.Return(run) return _c } -type mockConstructorTestingTNewErrorHandlerMock interface { +// NewErrorHandlerMock creates a new instance of ErrorHandlerMock. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +// The first argument is typically a *testing.T value. +func NewErrorHandlerMock(t interface { mock.TestingT Cleanup(func()) -} - -// NewErrorHandlerMock creates a new instance of ErrorHandlerMock. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. -func NewErrorHandlerMock(t mockConstructorTestingTNewErrorHandlerMock) *ErrorHandlerMock { +}) *ErrorHandlerMock { mock := &ErrorHandlerMock{} mock.Mock.Test(t) diff --git a/internal/rules/mocks/execution_condition.go b/internal/rules/mocks/execution_condition.go index b63e27be9..67c2db025 100644 --- a/internal/rules/mocks/execution_condition.go +++ b/internal/rules/mocks/execution_condition.go @@ -1,4 +1,4 @@ -// Code generated by mockery v2.23.1. DO NOT EDIT. +// Code generated by mockery v2.42.1. DO NOT EDIT. package mocks @@ -22,10 +22,71 @@ func (_m *ExecutionConditionMock) EXPECT() *ExecutionConditionMock_Expecter { return &ExecutionConditionMock_Expecter{mock: &_m.Mock} } -// CanExecute provides a mock function with given fields: ctx, sub -func (_m *ExecutionConditionMock) CanExecute(ctx heimdall.Context, sub *subject.Subject) (bool, error) { +// CanExecuteOnError provides a mock function with given fields: ctx, err +func (_m *ExecutionConditionMock) CanExecuteOnError(ctx heimdall.Context, err error) (bool, error) { + ret := _m.Called(ctx, err) + + if len(ret) == 0 { + panic("no return value specified for CanExecuteOnError") + } + + var r0 bool + var r1 error + if rf, ok := ret.Get(0).(func(heimdall.Context, error) (bool, error)); ok { + return rf(ctx, err) + } + if rf, ok := ret.Get(0).(func(heimdall.Context, error) bool); ok { + r0 = rf(ctx, err) + } else { + r0 = ret.Get(0).(bool) + } + + if rf, ok := ret.Get(1).(func(heimdall.Context, error) error); ok { + r1 = rf(ctx, err) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// ExecutionConditionMock_CanExecuteOnError_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'CanExecuteOnError' +type ExecutionConditionMock_CanExecuteOnError_Call struct { + *mock.Call +} + +// CanExecuteOnError is a helper method to define mock.On call +// - ctx heimdall.Context +// - err error +func (_e *ExecutionConditionMock_Expecter) CanExecuteOnError(ctx interface{}, err interface{}) *ExecutionConditionMock_CanExecuteOnError_Call { + return &ExecutionConditionMock_CanExecuteOnError_Call{Call: _e.mock.On("CanExecuteOnError", ctx, err)} +} + +func (_c *ExecutionConditionMock_CanExecuteOnError_Call) Run(run func(ctx heimdall.Context, err error)) *ExecutionConditionMock_CanExecuteOnError_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(heimdall.Context), args[1].(error)) + }) + return _c +} + +func (_c *ExecutionConditionMock_CanExecuteOnError_Call) Return(_a0 bool, _a1 error) *ExecutionConditionMock_CanExecuteOnError_Call { + _c.Call.Return(_a0, _a1) + return _c +} + +func (_c *ExecutionConditionMock_CanExecuteOnError_Call) RunAndReturn(run func(heimdall.Context, error) (bool, error)) *ExecutionConditionMock_CanExecuteOnError_Call { + _c.Call.Return(run) + return _c +} + +// CanExecuteOnSubject provides a mock function with given fields: ctx, sub +func (_m *ExecutionConditionMock) CanExecuteOnSubject(ctx heimdall.Context, sub *subject.Subject) (bool, error) { ret := _m.Called(ctx, sub) + if len(ret) == 0 { + panic("no return value specified for CanExecuteOnSubject") + } + var r0 bool var r1 error if rf, ok := ret.Get(0).(func(heimdall.Context, *subject.Subject) (bool, error)); ok { @@ -46,42 +107,41 @@ func (_m *ExecutionConditionMock) CanExecute(ctx heimdall.Context, sub *subject. return r0, r1 } -// ExecutionConditionMock_CanExecute_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'CanExecute' -type ExecutionConditionMock_CanExecute_Call struct { +// ExecutionConditionMock_CanExecuteOnSubject_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'CanExecuteOnSubject' +type ExecutionConditionMock_CanExecuteOnSubject_Call struct { *mock.Call } -// CanExecute is a helper method to define mock.On call +// CanExecuteOnSubject is a helper method to define mock.On call // - ctx heimdall.Context // - sub *subject.Subject -func (_e *ExecutionConditionMock_Expecter) CanExecute(ctx interface{}, sub interface{}) *ExecutionConditionMock_CanExecute_Call { - return &ExecutionConditionMock_CanExecute_Call{Call: _e.mock.On("CanExecute", ctx, sub)} +func (_e *ExecutionConditionMock_Expecter) CanExecuteOnSubject(ctx interface{}, sub interface{}) *ExecutionConditionMock_CanExecuteOnSubject_Call { + return &ExecutionConditionMock_CanExecuteOnSubject_Call{Call: _e.mock.On("CanExecuteOnSubject", ctx, sub)} } -func (_c *ExecutionConditionMock_CanExecute_Call) Run(run func(ctx heimdall.Context, sub *subject.Subject)) *ExecutionConditionMock_CanExecute_Call { +func (_c *ExecutionConditionMock_CanExecuteOnSubject_Call) Run(run func(ctx heimdall.Context, sub *subject.Subject)) *ExecutionConditionMock_CanExecuteOnSubject_Call { _c.Call.Run(func(args mock.Arguments) { run(args[0].(heimdall.Context), args[1].(*subject.Subject)) }) return _c } -func (_c *ExecutionConditionMock_CanExecute_Call) Return(_a0 bool, _a1 error) *ExecutionConditionMock_CanExecute_Call { +func (_c *ExecutionConditionMock_CanExecuteOnSubject_Call) Return(_a0 bool, _a1 error) *ExecutionConditionMock_CanExecuteOnSubject_Call { _c.Call.Return(_a0, _a1) return _c } -func (_c *ExecutionConditionMock_CanExecute_Call) RunAndReturn(run func(heimdall.Context, *subject.Subject) (bool, error)) *ExecutionConditionMock_CanExecute_Call { +func (_c *ExecutionConditionMock_CanExecuteOnSubject_Call) RunAndReturn(run func(heimdall.Context, *subject.Subject) (bool, error)) *ExecutionConditionMock_CanExecuteOnSubject_Call { _c.Call.Return(run) return _c } -type mockConstructorTestingTNewExecutionConditionMock interface { +// NewExecutionConditionMock creates a new instance of ExecutionConditionMock. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +// The first argument is typically a *testing.T value. +func NewExecutionConditionMock(t interface { mock.TestingT Cleanup(func()) -} - -// NewExecutionConditionMock creates a new instance of ExecutionConditionMock. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. -func NewExecutionConditionMock(t mockConstructorTestingTNewExecutionConditionMock) *ExecutionConditionMock { +}) *ExecutionConditionMock { mock := &ExecutionConditionMock{} mock.Mock.Test(t) diff --git a/internal/rules/rule_factory_impl.go b/internal/rules/rule_factory_impl.go index d01adba67..00ca464a1 100644 --- a/internal/rules/rule_factory_impl.go +++ b/internal/rules/rule_factory_impl.go @@ -59,92 +59,6 @@ type ruleFactory struct { defaultBacktracking bool } -//nolint:funlen,gocognit,cyclop -func (f *ruleFactory) createExecutePipeline( - version string, - pipeline []config.MechanismConfig, -) (compositeSubjectCreator, compositeSubjectHandler, compositeSubjectHandler, error) { - var ( - authenticators compositeSubjectCreator - subjectHandlers compositeSubjectHandler - finalizers compositeSubjectHandler - ) - - contextualizersCheck := func() error { - if len(finalizers) != 0 { - return errorchain.NewWithMessage(heimdall.ErrConfiguration, - "at least one finalizer is defined before a contextualizer") - } - - return nil - } - - authorizersCheck := func() error { - if len(finalizers) != 0 { - return errorchain.NewWithMessage(heimdall.ErrConfiguration, - "at least one finalizer is defined before an authorizer") - } - - return nil - } - - finalizersCheck := func() error { return nil } - - for _, pipelineStep := range pipeline { - id, found := pipelineStep["authenticator"] - if found { - if len(subjectHandlers) != 0 || len(finalizers) != 0 { - return nil, nil, nil, errorchain.NewWithMessage(heimdall.ErrConfiguration, - "an authenticator is defined after some other non authenticator type") - } - - authenticator, err := f.hf.CreateAuthenticator(version, id.(string), getConfig(pipelineStep["config"])) - if err != nil { - return nil, nil, nil, err - } - - authenticators = append(authenticators, authenticator) - - continue - } - - handler, err := createHandler(version, "authorizer", pipelineStep, authorizersCheck, - f.hf.CreateAuthorizer) - if err != nil && !errors.Is(err, errHandlerNotFound) { - return nil, nil, nil, err - } else if handler != nil { - subjectHandlers = append(subjectHandlers, handler) - - continue - } - - handler, err = createHandler(version, "contextualizer", pipelineStep, contextualizersCheck, - f.hf.CreateContextualizer) - if err != nil && !errors.Is(err, errHandlerNotFound) { - return nil, nil, nil, err - } else if handler != nil { - subjectHandlers = append(subjectHandlers, handler) - - continue - } - - handler, err = createHandler(version, "finalizer", pipelineStep, finalizersCheck, - f.hf.CreateFinalizer) - if err != nil && !errors.Is(err, errHandlerNotFound) { - return nil, nil, nil, err - } else if handler != nil { - finalizers = append(finalizers, handler) - - continue - } - - return nil, nil, nil, errorchain.NewWithMessage(heimdall.ErrConfiguration, - "unsupported configuration in execute") - } - - return authenticators, subjectHandlers, finalizers, nil -} - func (f *ruleFactory) DefaultRule() rule.Rule { return f.defaultRule } func (f *ruleFactory) HasDefaultRule() bool { return f.hasDefaultRule } @@ -234,6 +148,92 @@ func (f *ruleFactory) CreateRule(version, srcID string, ruleConfig config2.Rule) return rul, nil } +//nolint:funlen,gocognit,cyclop +func (f *ruleFactory) createExecutePipeline( + version string, + pipeline []config.MechanismConfig, +) (compositeSubjectCreator, compositeSubjectHandler, compositeSubjectHandler, error) { + var ( + authenticators compositeSubjectCreator + subjectHandlers compositeSubjectHandler + finalizers compositeSubjectHandler + ) + + contextualizersCheck := func() error { + if len(finalizers) != 0 { + return errorchain.NewWithMessage(heimdall.ErrConfiguration, + "at least one finalizer is defined before a contextualizer") + } + + return nil + } + + authorizersCheck := func() error { + if len(finalizers) != 0 { + return errorchain.NewWithMessage(heimdall.ErrConfiguration, + "at least one finalizer is defined before an authorizer") + } + + return nil + } + + finalizersCheck := func() error { return nil } + + for _, pipelineStep := range pipeline { + id, found := pipelineStep["authenticator"] + if found { + if len(subjectHandlers) != 0 || len(finalizers) != 0 { + return nil, nil, nil, errorchain.NewWithMessage(heimdall.ErrConfiguration, + "an authenticator is defined after some other non authenticator type") + } + + authenticator, err := f.hf.CreateAuthenticator(version, id.(string), getConfig(pipelineStep["config"])) + if err != nil { + return nil, nil, nil, err + } + + authenticators = append(authenticators, authenticator) + + continue + } + + handler, err := createHandler(version, "authorizer", pipelineStep, authorizersCheck, + f.hf.CreateAuthorizer) + if err != nil && !errors.Is(err, errHandlerNotFound) { + return nil, nil, nil, err + } else if handler != nil { + subjectHandlers = append(subjectHandlers, handler) + + continue + } + + handler, err = createHandler(version, "contextualizer", pipelineStep, contextualizersCheck, + f.hf.CreateContextualizer) + if err != nil && !errors.Is(err, errHandlerNotFound) { + return nil, nil, nil, err + } else if handler != nil { + subjectHandlers = append(subjectHandlers, handler) + + continue + } + + handler, err = createHandler(version, "finalizer", pipelineStep, finalizersCheck, + f.hf.CreateFinalizer) + if err != nil && !errors.Is(err, errHandlerNotFound) { + return nil, nil, nil, err + } else if handler != nil { + finalizers = append(finalizers, handler) + + continue + } + + return nil, nil, nil, errorchain.NewWithMessage(heimdall.ErrConfiguration, + "unsupported configuration in execute") + } + + return authenticators, subjectHandlers, finalizers, nil +} + func (f *ruleFactory) createOnErrorPipeline( version string, ehConfigs []config.MechanismConfig, @@ -244,18 +244,18 @@ func (f *ruleFactory) createOnErrorPipeline( id, found := ehStep["error_handler"] if found { conf := getConfig(ehStep["config"]) - condition := ehStep["if"] - if condition != nil { - conf["if"] = condition + condition, err := getExecutionCondition(ehStep["if"]) + if err != nil { + return nil, err } - eh, err := f.hf.CreateErrorHandler(version, id.(string), conf) + handler, err := f.hf.CreateErrorHandler(version, id.(string), conf) if err != nil { return nil, err } - errorHandlers = append(errorHandlers, eh) + errorHandlers = append(errorHandlers, &conditionalErrorHandler{h: handler, c: condition}) } else { return nil, errorchain.NewWithMessage(heimdall.ErrConfiguration, "unsupported configuration in error handler") diff --git a/internal/rules/rule_factory_impl_test.go b/internal/rules/rule_factory_impl_test.go index c5119f8bc..24a73d614 100644 --- a/internal/rules/rule_factory_impl_test.go +++ b/internal/rules/rule_factory_impl_test.go @@ -841,7 +841,7 @@ func TestRuleFactoryCreateRule(t *testing.T) { }, }, { - uc: "with conditional execution configuration type error", + uc: "with conditional execution configuration type error in the regular pipeline", config: config2.Rule{ ID: "foobar", Matcher: config2.Matcher{Routes: []config2.Route{{Path: "/foo/bar"}}}, @@ -952,6 +952,42 @@ func TestRuleFactoryCreateRule(t *testing.T) { require.Empty(t, rul.eh) }, }, + { + uc: "with bad conditional expression in the error pipeline", + config: config2.Rule{ + ID: "foobar", + Matcher: config2.Matcher{Routes: []config2.Route{{Path: "/foo/bar"}}}, + Execute: []config.MechanismConfig{ + {"authenticator": "foo"}, + {"authorizer": "bar"}, + {"finalizer": "baz"}, + }, + ErrorHandler: []config.MechanismConfig{ + {"error_handler": "foo", "if": "true", "config": map[string]any{}}, + {"error_handler": "bar", "if": 1, "config": map[string]any{}}, + }, + }, + configureMocks: func(t *testing.T, mhf *mocks3.MechanismFactoryMock) { + t.Helper() + + mhf.EXPECT().CreateAuthenticator("test", "foo", mock.Anything). + Return(&mocks2.AuthenticatorMock{}, nil) + mhf.EXPECT().CreateAuthorizer("test", "bar", mock.Anything). + Return(&mocks4.AuthorizerMock{}, nil) + mhf.EXPECT().CreateFinalizer("test", "baz", mock.Anything). + Return(&mocks7.FinalizerMock{}, nil) + mhf.EXPECT().CreateErrorHandler("test", "foo", config.MechanismConfig{}).Return(&mocks6.ErrorHandlerMock{}, nil) + }, + assert: func(t *testing.T, err error, _ *ruleImpl) { + t.Helper() + + t.Helper() + + require.Error(t, err) + require.ErrorIs(t, err, heimdall.ErrConfiguration) + require.ErrorContains(t, err, "unexpected type") + }, + }, { uc: "with conditional execution for error handler", config: config2.Rule{ @@ -976,12 +1012,8 @@ func TestRuleFactoryCreateRule(t *testing.T) { Return(&mocks4.AuthorizerMock{}, nil) mhf.EXPECT().CreateFinalizer("test", "baz", mock.Anything). Return(&mocks7.FinalizerMock{}, nil) - mhf.EXPECT().CreateErrorHandler("test", "foo", - mock.MatchedBy(func(conf map[string]any) bool { return conf["if"] == "true" }), - ).Return(&mocks6.ErrorHandlerMock{}, nil) - mhf.EXPECT().CreateErrorHandler("test", "bar", - mock.MatchedBy(func(conf map[string]any) bool { return conf["if"] == "false" }), - ).Return(&mocks6.ErrorHandlerMock{}, nil) + mhf.EXPECT().CreateErrorHandler("test", "foo", config.MechanismConfig{}).Return(&mocks6.ErrorHandlerMock{}, nil) + mhf.EXPECT().CreateErrorHandler("test", "bar", config.MechanismConfig{}).Return(&mocks6.ErrorHandlerMock{}, nil) }, assert: func(t *testing.T, err error, rul *ruleImpl) { t.Helper() diff --git a/internal/rules/rule_impl_test.go b/internal/rules/rule_impl_test.go index 6a256d8d1..a1e3d9d17 100644 --- a/internal/rules/rule_impl_test.go +++ b/internal/rules/rule_impl_test.go @@ -63,7 +63,6 @@ func TestRuleExecute(t *testing.T) { authenticator.EXPECT().Execute(ctx).Return(nil, testsupport.ErrTestPurpose) authenticator.EXPECT().IsFallbackOnErrorAllowed().Return(false) - errHandler.EXPECT().CanExecute(ctx, testsupport.ErrTestPurpose).Return(true) errHandler.EXPECT().Execute(ctx, testsupport.ErrTestPurpose).Return(nil) }, assert: func(t *testing.T, err error, backend rule.Backend, _ map[string]string) { @@ -85,7 +84,6 @@ func TestRuleExecute(t *testing.T) { authenticator.EXPECT().Execute(ctx).Return(nil, testsupport.ErrTestPurpose) authenticator.EXPECT().IsFallbackOnErrorAllowed().Return(false) - errHandler.EXPECT().CanExecute(ctx, testsupport.ErrTestPurpose).Return(true) errHandler.EXPECT().Execute(ctx, testsupport.ErrTestPurpose).Return(testsupport.ErrTestPurpose2) }, assert: func(t *testing.T, err error, backend rule.Backend, _ map[string]string) { @@ -111,7 +109,6 @@ func TestRuleExecute(t *testing.T) { authenticator.EXPECT().Execute(ctx).Return(sub, nil) authorizer.EXPECT().Execute(ctx, sub).Return(testsupport.ErrTestPurpose) authorizer.EXPECT().ContinueOnError().Return(false) - errHandler.EXPECT().CanExecute(ctx, testsupport.ErrTestPurpose).Return(true) errHandler.EXPECT().Execute(ctx, testsupport.ErrTestPurpose).Return(nil) }, assert: func(t *testing.T, err error, backend rule.Backend, _ map[string]string) { @@ -136,7 +133,6 @@ func TestRuleExecute(t *testing.T) { authenticator.EXPECT().Execute(ctx).Return(sub, nil) authorizer.EXPECT().Execute(ctx, sub).Return(testsupport.ErrTestPurpose) authorizer.EXPECT().ContinueOnError().Return(false) - errHandler.EXPECT().CanExecute(ctx, testsupport.ErrTestPurpose).Return(true) errHandler.EXPECT().Execute(ctx, testsupport.ErrTestPurpose).Return(testsupport.ErrTestPurpose2) }, assert: func(t *testing.T, err error, backend rule.Backend, _ map[string]string) { @@ -163,7 +159,6 @@ func TestRuleExecute(t *testing.T) { authorizer.EXPECT().Execute(ctx, sub).Return(nil) finalizer.EXPECT().Execute(ctx, sub).Return(testsupport.ErrTestPurpose) finalizer.EXPECT().ContinueOnError().Return(false) - errHandler.EXPECT().CanExecute(ctx, testsupport.ErrTestPurpose).Return(true) errHandler.EXPECT().Execute(ctx, testsupport.ErrTestPurpose).Return(nil) }, assert: func(t *testing.T, err error, backend rule.Backend, _ map[string]string) { @@ -189,7 +184,6 @@ func TestRuleExecute(t *testing.T) { authorizer.EXPECT().Execute(ctx, sub).Return(nil) finalizer.EXPECT().Execute(ctx, sub).Return(testsupport.ErrTestPurpose) finalizer.EXPECT().ContinueOnError().Return(false) - errHandler.EXPECT().CanExecute(ctx, testsupport.ErrTestPurpose).Return(true) errHandler.EXPECT().Execute(ctx, testsupport.ErrTestPurpose).Return(testsupport.ErrTestPurpose2) }, assert: func(t *testing.T, err error, backend rule.Backend, _ map[string]string) { diff --git a/schema/config.schema.json b/schema/config.schema.json index 7d4d00d6a..45fea00f5 100644 --- a/schema/config.schema.json +++ b/schema/config.schema.json @@ -1849,7 +1849,6 @@ "required": [ "id", "type", - "if", "config" ], "properties": { @@ -1860,13 +1859,6 @@ "description": "The unique id of the error handler to be used in the rule definition", "type": "string" }, - "if": { - "description": "Condition, when this error handler should be executed", - "type": "string", - "examples": [ - "type(Error) == authentication_error" - ] - }, "config": { "type": "object", "additionalProperties": false, @@ -1887,7 +1879,6 @@ "required": [ "id", "type", - "if", "config" ], "properties": { @@ -1898,13 +1889,6 @@ "description": "The unique id of the error handler to be used in the rule definition", "type": "string" }, - "if": { - "description": "Condition, when this error handler should be executed", - "type": "string", - "examples": [ - "type(Error) == authentication_error" - ] - }, "config": { "type": "object", "additionalProperties": false,