From 7a0eff39aa435c8e893e54a5b01de1328f6d24d7 Mon Sep 17 00:00:00 2001 From: Dimitrij Drus Date: Mon, 26 Dec 2022 20:35:35 +0100 Subject: [PATCH] feat!: Templating support in redirect error handler mechanism (#395) --- cmd/serve/decision.go | 32 ++++---- cmd/serve/decision_test.go | 27 +++++++ cmd/serve/proxy.go | 32 ++++---- cmd/serve/proxy_test.go | 27 +++++++ .../configuration/reference/reference.adoc | 15 ++-- .../pipeline_mechanisms/error_handlers.adoc | 14 ++-- .../rules/pipeline_mechanisms/overview.adoc | 3 +- example_config.yaml | 3 +- internal/config/test_data/test_config.yaml | 3 +- .../middleware/errorhandler/error_handler.go | 2 +- .../errorhandler/error_handler_test.go | 5 +- internal/handler/decision/handler.go | 9 +-- internal/handler/management/handler.go | 14 ++-- internal/handler/management/health_test.go | 18 +---- internal/handler/management/jwks.go | 17 +---- internal/handler/management/jwks_test.go | 16 +++- internal/handler/proxy/handler.go | 9 +-- internal/heimdall/errors.go | 3 +- internal/heimdall/jwt_signer.go | 7 +- internal/heimdall/mocks/jwt_signer.go | 5 ++ internal/module.go | 2 + .../errorhandlers/config_decoder.go | 3 +- .../matcher/mapstructure_decoder.go | 17 ----- .../matcher/mapstructure_decoder_test.go | 32 -------- .../errorhandlers/redirect_error_handler.go | 42 +++++------ .../redirect_error_handler_test.go | 73 +++++++++++++------ .../rules/mechanisms/template/template.go | 14 +++- internal/signer/jwt_signer.go | 24 ++++-- internal/signer/jwt_signer_test.go | 41 ++++++++++- internal/signer/module.go | 9 +++ internal/x/testsupport/freeport.go | 20 +++++ schema/config.schema.json | 7 -- 32 files changed, 323 insertions(+), 222 deletions(-) create mode 100644 cmd/serve/decision_test.go create mode 100644 cmd/serve/proxy_test.go create mode 100644 internal/signer/module.go create mode 100644 internal/x/testsupport/freeport.go diff --git a/cmd/serve/decision.go b/cmd/serve/decision.go index b49746f13..69b56ed52 100644 --- a/cmd/serve/decision.go +++ b/cmd/serve/decision.go @@ -15,20 +15,8 @@ func NewDecisionCommand() *cobra.Command { Use: "decision", Short: "Starts heimdall in Decision operation mode", Example: "heimdall serve decision", - Run: func(cmd *cobra.Command, args []string) { - configPath, _ := cmd.Flags().GetString("config") - envPrefix, _ := cmd.Flags().GetString("env-config-prefix") - - app := fx.New( - fx.NopLogger, - fx.Supply( - config.ConfigurationPath(configPath), - config.EnvVarPrefix(envPrefix)), - internal.Module, - decision.Module, - ) - - err := app.Err() + Run: func(cmd *cobra.Command, _ []string) { + app, err := createDecisionApp(cmd) if err != nil { cmd.PrintErrf("Failed to initialize decision service: %v", err) panic(err) @@ -38,3 +26,19 @@ func NewDecisionCommand() *cobra.Command { }, } } + +func createDecisionApp(cmd *cobra.Command) (*fx.App, error) { + configPath, _ := cmd.Flags().GetString("config") + envPrefix, _ := cmd.Flags().GetString("env-config-prefix") + + app := fx.New( + fx.NopLogger, + fx.Supply( + config.ConfigurationPath(configPath), + config.EnvVarPrefix(envPrefix)), + internal.Module, + decision.Module, + ) + + return app, app.Err() +} diff --git a/cmd/serve/decision_test.go b/cmd/serve/decision_test.go new file mode 100644 index 000000000..f745252a8 --- /dev/null +++ b/cmd/serve/decision_test.go @@ -0,0 +1,27 @@ +package serve + +import ( + "strconv" + "testing" + + "github.com/spf13/cobra" + "github.com/stretchr/testify/require" + + "github.com/dadrus/heimdall/internal/x/testsupport" +) + +func TestCreateDecisionApp(t *testing.T) { + // this test verifies that all dependencies are resolved + // and nothing has been forgotten + port1, err := testsupport.GetFreePort() + require.NoError(t, err) + + port2, err := testsupport.GetFreePort() + require.NoError(t, err) + + t.Setenv("SERVE_DECISION_PORT", strconv.Itoa(port1)) + t.Setenv("SERVE_MANAGEMENT_PORT", strconv.Itoa(port2)) + + _, err = createDecisionApp(&cobra.Command{}) + require.NoError(t, err) +} diff --git a/cmd/serve/proxy.go b/cmd/serve/proxy.go index 6b1dfceab..69395f7b9 100644 --- a/cmd/serve/proxy.go +++ b/cmd/serve/proxy.go @@ -15,20 +15,8 @@ func NewProxyCommand() *cobra.Command { Use: "proxy", Short: "Starts heimdall in Reverse Proxy operation mode", Example: "heimdall serve proxy", - Run: func(cmd *cobra.Command, args []string) { - configPath, _ := cmd.Flags().GetString("config") - envPrefix, _ := cmd.Flags().GetString("env-config-prefix") - - app := fx.New( - fx.NopLogger, - fx.Supply( - config.ConfigurationPath(configPath), - config.EnvVarPrefix(envPrefix)), - internal.Module, - proxy.Module, - ) - - err := app.Err() + Run: func(cmd *cobra.Command, _ []string) { + app, err := createProxyApp(cmd) if err != nil { cmd.PrintErrf("Failed to initialize proxy service: %v", err) panic(err) @@ -38,3 +26,19 @@ func NewProxyCommand() *cobra.Command { }, } } + +func createProxyApp(cmd *cobra.Command) (*fx.App, error) { + configPath, _ := cmd.Flags().GetString("config") + envPrefix, _ := cmd.Flags().GetString("env-config-prefix") + + app := fx.New( + fx.NopLogger, + fx.Supply( + config.ConfigurationPath(configPath), + config.EnvVarPrefix(envPrefix)), + internal.Module, + proxy.Module, + ) + + return app, app.Err() +} diff --git a/cmd/serve/proxy_test.go b/cmd/serve/proxy_test.go new file mode 100644 index 000000000..9eb22df6a --- /dev/null +++ b/cmd/serve/proxy_test.go @@ -0,0 +1,27 @@ +package serve + +import ( + "strconv" + "testing" + + "github.com/spf13/cobra" + "github.com/stretchr/testify/require" + + "github.com/dadrus/heimdall/internal/x/testsupport" +) + +func TestCreateProxyApp(t *testing.T) { + // this test verifies that all dependencies are resolved + // and nothing has been forgotten + port1, err := testsupport.GetFreePort() + require.NoError(t, err) + + port2, err := testsupport.GetFreePort() + require.NoError(t, err) + + t.Setenv("SERVE_PROXY_PORT", strconv.Itoa(port1)) + t.Setenv("SERVE_MANAGEMENT_PORT", strconv.Itoa(port2)) + + _, err = createProxyApp(&cobra.Command{}) + require.NoError(t, err) +} diff --git a/docs/content/docs/configuration/reference/reference.adoc b/docs/content/docs/configuration/reference/reference.adoc index ce3c146e0..99485c8c7 100644 --- a/docs/content/docs/configuration/reference/reference.adoc +++ b/docs/content/docs/configuration/reference/reference.adoc @@ -35,8 +35,8 @@ serve: allow_credentials: true max_age: 1m tls: - key: /path/to/key/file.pem - cert: /path/to/cert/file.pem + key_store: /path/to/key/store.pem + password: VerySecure! trusted_proxies: - 192.168.1.0/24 @@ -61,8 +61,9 @@ serve: allow_credentials: true max_age: 1m tls: - key: /path/to/key/file.pem - cert: /path/to/cert/file.pem + key_store: /path/to/key/store.pem + password: VerySecure! + key_id: first_entry min_version: TLS1.2 cipher_suites: - TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 @@ -93,8 +94,7 @@ serve: allow_credentials: true max_age: 1m tls: - key: /path/to/key/file.pem - cert: /path/to/cert/file.pem + key_store: /path/to/key/store.pem min_version: TLS1.2 log: @@ -269,8 +269,7 @@ rules: - id: authenticate_with_kratos type: redirect config: - to: http://127.0.0.1:4433/self-service/login/browser - return_to_query_parameter: return_to + to: http://127.0.0.1:4433/self-service/login/browser?return_to={{ .Request.URL | urlenc }} when: - error: - type: authentication_error diff --git a/docs/content/docs/configuration/rules/pipeline_mechanisms/error_handlers.adoc b/docs/content/docs/configuration/rules/pipeline_mechanisms/error_handlers.adoc index f7463b839..001c6ae90 100644 --- a/docs/content/docs/configuration/rules/pipeline_mechanisms/error_handlers.adoc +++ b/docs/content/docs/configuration/rules/pipeline_mechanisms/error_handlers.adoc @@ -42,11 +42,9 @@ Configuration using the `config` property is mandatory. Following properties are * *`to`*: _URL_ (mandatory, not overridable) + -The url to redirect the client to. If no `return_to_query_parameter` is defined, the value of the HTTP `Location` hader is set to the configured value. - -* *`return_to_query_parameter`*: _string_ (optional, not overridable) +The url to redirect the client to via the `Location` header. Can be templated and has access to the link:{{< relref "overview.adoc#_request" >}}[`Request`] object. + -If you want to return the user back to the url, heimdall was handling when this error handler mechanism kicked in and your authentication system supports this by considering a specific query parameter, you can configure it here. +NOTE: When creating query parameters by making use of templates, don't forget to encode the values using the `urlenc` function. See also examples below. * *`code`*: _int_ (optional, not overridable) + @@ -63,7 +61,7 @@ Conditions, which must hold true for this error handler to execute. The defined The redirect error handler below is configured to kick in if either -* the first error condition evaluates to true. This condition is for web requests (HTTP `Accept` header contains `text/html`) if an `authentication_error` error occurred (an error raised by authenticators). In this case, it will redirect the client (for web requests, usually a browser) to `\http://127.0.0.1:4433/self-service/login/browser` and also add the `return_to` query parameter with the current url to the redirect url. +* the first error condition evaluates to true. This condition is for web requests (HTTP `Accept` header contains `text/html`) if an `authentication_error` error occurred (an error raised by authenticators). In this case, it will redirect the client (for web requests, usually a browser) to `\http://127.0.0.1:4433/self-service/login/browser` and also add the `return_to` query parameter set to the URL encoded value of the current url. * Or, if the second condition evaluates to true. The only difference to the previous one is the error type, which is `authorization_error` in this case. So, e.g. if Heimdall was handling the request for `\http://my-service.local/foo` and run into an error like described above, the value of the HTTP `Location` header will be set to `\http://127.0.0.1:4433/self-service/login/browser?return_to=http%3A%2F%2Fmy-service.local%2Ffoo` @@ -73,8 +71,7 @@ So, e.g. if Heimdall was handling the request for `\http://my-service.local/foo` id: authenticate_with_kratos type: redirect config: - to: http://127.0.0.1:4433/self-service/login/browser - return_to_query_parameter: return_to + to: http://127.0.0.1:4433/self-service/login/browser?return_to={{ .Request.URL | urlenc }} when: - error: - type: authentication_error @@ -95,8 +92,7 @@ In this case the error condition can actually be simplified, so the mechanism co id: authenticate_with_kratos type: redirect config: - to: http://127.0.0.1:4433/self-service/login/browser - return_to_query_parameter: return_to + to: http://127.0.0.1:4433/self-service/login/browser?return_to={{ .Request.URL | urlenc }} when: - error: - type: authentication_error diff --git a/docs/content/docs/configuration/rules/pipeline_mechanisms/overview.adoc b/docs/content/docs/configuration/rules/pipeline_mechanisms/overview.adoc index 43b59f678..a3e987428 100644 --- a/docs/content/docs/configuration/rules/pipeline_mechanisms/overview.adoc +++ b/docs/content/docs/configuration/rules/pipeline_mechanisms/overview.adoc @@ -130,8 +130,7 @@ rules: - id: authenticate_with_kratos type: redirect config: - to: http://127.0.0.1:4433/self-service/login/browser - return_to_query_parameter: return_to + to: http://127.0.0.1:4433/self-service/login/browser?return_to={{ .Request.URL | urlenc }} when: - error: - type: authentication_error diff --git a/example_config.yaml b/example_config.yaml index abcde465d..fe5f75c22 100644 --- a/example_config.yaml +++ b/example_config.yaml @@ -166,8 +166,7 @@ rules: - id: authenticate_with_kratos type: redirect config: - to: http://127.0.0.1:4433/self-service/login/browser - return_to_query_parameter: origin + to: http://127.0.0.1:4433/self-service/login/browser?origin={{ .Request.URL | urlenc }} when: - error: - type: authentication_error diff --git a/internal/config/test_data/test_config.yaml b/internal/config/test_data/test_config.yaml index d9060f2eb..4efa9d2dc 100644 --- a/internal/config/test_data/test_config.yaml +++ b/internal/config/test_data/test_config.yaml @@ -295,8 +295,7 @@ rules: - id: authenticate_with_kratos type: redirect config: - to: http://127.0.0.1:4433/self-service/login/browser - return_to_query_parameter: return_to + to: http://127.0.0.1:4433/self-service/login/browser?return_to={{ .Request.URL | urlenc }} when: - error: - type: authentication_error diff --git a/internal/fiber/middleware/errorhandler/error_handler.go b/internal/fiber/middleware/errorhandler/error_handler.go index a88f63f8a..3d2320217 100644 --- a/internal/fiber/middleware/errorhandler/error_handler.go +++ b/internal/fiber/middleware/errorhandler/error_handler.go @@ -52,7 +52,7 @@ func (h *handler) handle(ctx *fiber.Ctx) error { //nolint:cyclop errors.As(err, &redirectError) - return ctx.Redirect(redirectError.RedirectTo.String(), redirectError.Code) + return ctx.Redirect(redirectError.RedirectTo, redirectError.Code) default: logger := zerolog.Ctx(ctx.UserContext()) logger.Error().Err(err).Msg("Internal error occurred") diff --git a/internal/fiber/middleware/errorhandler/error_handler_test.go b/internal/fiber/middleware/errorhandler/error_handler_test.go index 09995354e..e8ae87f07 100644 --- a/internal/fiber/middleware/errorhandler/error_handler_test.go +++ b/internal/fiber/middleware/errorhandler/error_handler_test.go @@ -4,7 +4,6 @@ import ( "context" "io" "net/http" - "net/url" "testing" "github.com/gofiber/fiber/v2" @@ -165,13 +164,13 @@ func TestHandlerHandle(t *testing.T) { { uc: "redirect error", handler: New(), - err: &heimdall.RedirectError{RedirectTo: &url.URL{}, Code: http.StatusFound}, + err: &heimdall.RedirectError{RedirectTo: "http://foo.local", Code: http.StatusFound}, expCode: http.StatusFound, }, { uc: "redirect error verbose", handler: New(WithVerboseErrors(true)), - err: &heimdall.RedirectError{RedirectTo: &url.URL{}, Code: http.StatusFound}, + err: &heimdall.RedirectError{RedirectTo: "http://foo.local", Code: http.StatusFound}, expCode: http.StatusFound, }, { diff --git a/internal/handler/decision/handler.go b/internal/handler/decision/handler.go index e18e6ebe6..9e010896e 100644 --- a/internal/handler/decision/handler.go +++ b/internal/handler/decision/handler.go @@ -10,7 +10,6 @@ import ( "github.com/dadrus/heimdall/internal/handler/requestcontext" "github.com/dadrus/heimdall/internal/heimdall" "github.com/dadrus/heimdall/internal/rules" - "github.com/dadrus/heimdall/internal/signer" "github.com/dadrus/heimdall/internal/x" "github.com/dadrus/heimdall/internal/x/errorchain" ) @@ -27,20 +26,16 @@ type handlerArgs struct { App *fiber.App `name:"decision"` RulesRepository rules.Repository Config *config.Configuration + Signer heimdall.JWTSigner Logger zerolog.Logger } func newHandler(args handlerArgs) (*Handler, error) { - jwtSigner, err := signer.NewJWTSigner(&args.Config.Signer, args.Logger) - if err != nil { - return nil, err - } - acceptedCode := args.Config.Serve.Decision.Respond.With.Accepted.Code handler := &Handler{ r: args.RulesRepository, - s: jwtSigner, + s: args.Signer, code: x.IfThenElse(acceptedCode != 0, acceptedCode, fiber.StatusOK), } diff --git a/internal/handler/management/handler.go b/internal/handler/management/handler.go index 5a2d91b24..7b0f64db6 100644 --- a/internal/handler/management/handler.go +++ b/internal/handler/management/handler.go @@ -6,7 +6,7 @@ import ( "github.com/rs/zerolog" "go.uber.org/fx" - "github.com/dadrus/heimdall/internal/keystore" + "github.com/dadrus/heimdall/internal/heimdall" ) type Handler struct{} @@ -14,22 +14,22 @@ type Handler struct{} type handlerArgs struct { fx.In - App *fiber.App `name:"management"` - KeyStore keystore.KeyStore - Logger zerolog.Logger + App *fiber.App `name:"management"` + Signer heimdall.JWTSigner + Logger zerolog.Logger } func newHandler(args handlerArgs) (*Handler, error) { handler := &Handler{} - handler.registerRoutes(args.App.Group("/"), args.Logger, args.KeyStore) + handler.registerRoutes(args.App.Group("/"), args.Logger, args.Signer) return handler, nil } -func (h *Handler) registerRoutes(router fiber.Router, logger zerolog.Logger, ks keystore.KeyStore) { +func (h *Handler) registerRoutes(router fiber.Router, logger zerolog.Logger, signer heimdall.JWTSigner) { logger.Debug().Msg("Registering Management service routes") router.Get(EndpointHealth, health) - router.Get(EndpointJWKS, etag.New(), jwks(ks)) + router.Get(EndpointJWKS, etag.New(), jwks(signer)) } diff --git a/internal/handler/management/health_test.go b/internal/handler/management/health_test.go index 9bd09a8e6..6cdf17968 100644 --- a/internal/handler/management/health_test.go +++ b/internal/handler/management/health_test.go @@ -1,8 +1,6 @@ package management import ( - "crypto/rand" - "crypto/rsa" "io" "net/http" "net/http/httptest" @@ -14,31 +12,21 @@ import ( "github.com/stretchr/testify/require" "github.com/dadrus/heimdall/internal/config" - "github.com/dadrus/heimdall/internal/keystore" ) func TestHealthRequest(t *testing.T) { t.Parallel() // GIVEN - const rsa2048 = 2048 - - privateKey, err := rsa.GenerateKey(rand.Reader, rsa2048) - require.NoError(t, err) - - ks, err := keystore.NewKeyStoreFromKey(privateKey) - require.NoError(t, err) - app := newApp(appArgs{ Config: &config.Configuration{Serve: config.ServeConfig{Management: config.ServiceConfig{}}}, Registerer: prometheus.NewRegistry(), Logger: log.Logger, }) - _, err = newHandler(handlerArgs{ - App: app, - Logger: log.Logger, - KeyStore: ks, + _, err := newHandler(handlerArgs{ + App: app, + Logger: log.Logger, }) require.NoError(t, err) diff --git a/internal/handler/management/jwks.go b/internal/handler/management/jwks.go index 1e0acb962..3fe582c90 100644 --- a/internal/handler/management/jwks.go +++ b/internal/handler/management/jwks.go @@ -4,24 +4,13 @@ import ( "github.com/gofiber/fiber/v2" "gopkg.in/square/go-jose.v2" - "github.com/dadrus/heimdall/internal/keystore" + "github.com/dadrus/heimdall/internal/heimdall" ) // jwks implements an endpoint returning JWKS objects according to // https://datatracker.ietf.org/doc/html/rfc7517 -func jwks(ks keystore.KeyStore) fiber.Handler { - // As of today, key store configuration is part of static configuration. So key store can not be updated - // without a new heimdall deployment. For this reason the conversion is done here. Should the support for - // dynamic key store updates be added in the future, the lines below, will have to be moved into the handler - // implementation. - entries := ks.Entries() - keys := make([]jose.JSONWebKey, len(entries)) - - for idx, entry := range entries { - keys[idx] = entry.JWK() - } - +func jwks(signer heimdall.JWTSigner) fiber.Handler { return func(c *fiber.Ctx) error { - return c.JSON(jose.JSONWebKeySet{Keys: keys}) + return c.JSON(jose.JSONWebKeySet{Keys: signer.Keys()}) } } diff --git a/internal/handler/management/jwks_test.go b/internal/handler/management/jwks_test.go index f09aafac0..dd1a099bf 100644 --- a/internal/handler/management/jwks_test.go +++ b/internal/handler/management/jwks_test.go @@ -21,6 +21,7 @@ import ( "gopkg.in/square/go-jose.v2" "github.com/dadrus/heimdall/internal/config" + "github.com/dadrus/heimdall/internal/heimdall/mocks" "github.com/dadrus/heimdall/internal/keystore" "github.com/dadrus/heimdall/internal/x/pkix/pemx" "github.com/dadrus/heimdall/internal/x/testsupport" @@ -96,10 +97,19 @@ func (suite *JWKSTestSuite) SetupSuite() { Logger: log.Logger, }) + keys := make([]jose.JSONWebKey, len(suite.ks.Entries())) + + for idx, entry := range suite.ks.Entries() { + keys[idx] = entry.JWK() + } + + signer := &mocks.MockJWTSigner{} + signer.On("Keys").Return(keys) + _, err = newHandler(handlerArgs{ - App: suite.app, - Logger: log.Logger, - KeyStore: suite.ks, + App: suite.app, + Logger: log.Logger, + Signer: signer, }) suite.NoError(err) } diff --git a/internal/handler/proxy/handler.go b/internal/handler/proxy/handler.go index 213695c19..68a0f0f59 100644 --- a/internal/handler/proxy/handler.go +++ b/internal/handler/proxy/handler.go @@ -12,7 +12,6 @@ import ( "github.com/dadrus/heimdall/internal/handler/requestcontext" "github.com/dadrus/heimdall/internal/heimdall" "github.com/dadrus/heimdall/internal/rules" - "github.com/dadrus/heimdall/internal/signer" "github.com/dadrus/heimdall/internal/x/errorchain" ) @@ -28,18 +27,14 @@ type handlerArgs struct { App *fiber.App `name:"proxy"` RulesRepository rules.Repository Config *config.Configuration + Signer heimdall.JWTSigner Logger zerolog.Logger } func newHandler(args handlerArgs) (*Handler, error) { - jwtSigner, err := signer.NewJWTSigner(&args.Config.Signer, args.Logger) - if err != nil { - return nil, err - } - handler := &Handler{ r: args.RulesRepository, - s: jwtSigner, + s: args.Signer, t: args.Config.Serve.Proxy.Timeout.Read, } diff --git a/internal/heimdall/errors.go b/internal/heimdall/errors.go index 7ecec6051..6c77527b3 100644 --- a/internal/heimdall/errors.go +++ b/internal/heimdall/errors.go @@ -2,7 +2,6 @@ package heimdall import ( "errors" - "net/url" "reflect" ) @@ -21,7 +20,7 @@ var ( type RedirectError struct { Message string Code int - RedirectTo *url.URL + RedirectTo string } func (e *RedirectError) Error() string { return e.Message } diff --git a/internal/heimdall/jwt_signer.go b/internal/heimdall/jwt_signer.go index adfad7a02..982622782 100644 --- a/internal/heimdall/jwt_signer.go +++ b/internal/heimdall/jwt_signer.go @@ -1,8 +1,13 @@ package heimdall -import "time" +import ( + "time" + + "gopkg.in/square/go-jose.v2" +) type JWTSigner interface { Sign(sub string, ttl time.Duration, claims map[string]any) (string, error) Hash() []byte + Keys() []jose.JSONWebKey } diff --git a/internal/heimdall/mocks/jwt_signer.go b/internal/heimdall/mocks/jwt_signer.go index 96a0863f0..e4c923909 100644 --- a/internal/heimdall/mocks/jwt_signer.go +++ b/internal/heimdall/mocks/jwt_signer.go @@ -4,6 +4,7 @@ import ( "time" "github.com/stretchr/testify/mock" + "gopkg.in/square/go-jose.v2" ) type MockJWTSigner struct { @@ -17,3 +18,7 @@ func (m *MockJWTSigner) Sign(subjectID string, ttl time.Duration, claims map[str return args.String(0), args.Error(1) } + +func (m *MockJWTSigner) Keys() []jose.JSONWebKey { + return convertTo[[]jose.JSONWebKey](m.Called().Get(0)) +} diff --git a/internal/module.go b/internal/module.go index 554264c3e..86d94df3e 100644 --- a/internal/module.go +++ b/internal/module.go @@ -11,6 +11,7 @@ import ( "github.com/dadrus/heimdall/internal/prometheus" "github.com/dadrus/heimdall/internal/rules" "github.com/dadrus/heimdall/internal/rules/mechanisms" + "github.com/dadrus/heimdall/internal/signer" "github.com/dadrus/heimdall/internal/tracing" ) @@ -20,6 +21,7 @@ var Module = fx.Options( logging.Module, tracing.Module, cache.Module, + signer.Module, mechanisms.Module, prometheus.Module, rules.Module, diff --git a/internal/rules/mechanisms/errorhandlers/config_decoder.go b/internal/rules/mechanisms/errorhandlers/config_decoder.go index b3509f3c4..64bb280a9 100644 --- a/internal/rules/mechanisms/errorhandlers/config_decoder.go +++ b/internal/rules/mechanisms/errorhandlers/config_decoder.go @@ -4,6 +4,7 @@ import ( "github.com/mitchellh/mapstructure" "github.com/dadrus/heimdall/internal/rules/mechanisms/errorhandlers/matcher" + "github.com/dadrus/heimdall/internal/rules/mechanisms/template" ) func decodeConfig(input any, output any) error { @@ -12,7 +13,7 @@ func decodeConfig(input any, output any) error { DecodeHook: mapstructure.ComposeDecodeHookFunc( matcher.DecodeCIDRMatcherHookFunc(), matcher.DecodeErrorTypeMatcherHookFunc(), - matcher.StringToURLHookFunc(), + template.DecodeTemplateHookFunc(), ), Result: output, ErrorUnused: true, diff --git a/internal/rules/mechanisms/errorhandlers/matcher/mapstructure_decoder.go b/internal/rules/mechanisms/errorhandlers/matcher/mapstructure_decoder.go index 3f15e6b2a..557ea625b 100644 --- a/internal/rules/mechanisms/errorhandlers/matcher/mapstructure_decoder.go +++ b/internal/rules/mechanisms/errorhandlers/matcher/mapstructure_decoder.go @@ -1,7 +1,6 @@ package matcher import ( - "net/url" "reflect" "github.com/mitchellh/mapstructure" @@ -74,19 +73,3 @@ func DecodeErrorTypeMatcherHookFunc() mapstructure.DecodeHookFunc { return matcher, nil } } - -func StringToURLHookFunc() mapstructure.DecodeHookFunc { - return func(from reflect.Type, to reflect.Type, data any) (any, error) { - if from.Kind() != reflect.String { - return data, nil - } - - if to != reflect.TypeOf(&url.URL{}) { - return data, nil - } - - // Convert it by parsing (type check is already done above) - // nolint: forcetypeassert - return url.Parse(data.(string)) - } -} diff --git a/internal/rules/mechanisms/errorhandlers/matcher/mapstructure_decoder_test.go b/internal/rules/mechanisms/errorhandlers/matcher/mapstructure_decoder_test.go index cca1048fe..ca57eaed0 100644 --- a/internal/rules/mechanisms/errorhandlers/matcher/mapstructure_decoder_test.go +++ b/internal/rules/mechanisms/errorhandlers/matcher/mapstructure_decoder_test.go @@ -1,7 +1,6 @@ package matcher import ( - "net/url" "testing" "github.com/mitchellh/mapstructure" @@ -171,34 +170,3 @@ error: err = dec.Decode(mapConfig) require.Error(t, err) } - -func TestStringToURLHookFunc(t *testing.T) { - t.Parallel() - - type Type struct { - Ref *url.URL `mapstructure:"url"` - } - - rawConfig := []byte("url: http://test.com/foo") - - var typ Type - - dec, err := mapstructure.NewDecoder( - &mapstructure.DecoderConfig{ - DecodeHook: mapstructure.ComposeDecodeHookFunc( - StringToURLHookFunc(), - ), - Result: &typ, - ErrorUnused: true, - }) - require.NoError(t, err) - - mapConfig, err := testsupport.DecodeTestConfig(rawConfig) - require.NoError(t, err) - - err = dec.Decode(mapConfig) - require.NoError(t, err) - - assert.NotNil(t, typ.Ref) - assert.Equal(t, "http://test.com/foo", typ.Ref.String()) -} diff --git a/internal/rules/mechanisms/errorhandlers/redirect_error_handler.go b/internal/rules/mechanisms/errorhandlers/redirect_error_handler.go index bfee13b14..d9376b863 100644 --- a/internal/rules/mechanisms/errorhandlers/redirect_error_handler.go +++ b/internal/rules/mechanisms/errorhandlers/redirect_error_handler.go @@ -2,12 +2,12 @@ package errorhandlers import ( "net/http" - "net/url" "github.com/rs/zerolog" "github.com/dadrus/heimdall/internal/heimdall" "github.com/dadrus/heimdall/internal/rules/mechanisms/errorhandlers/matcher" + "github.com/dadrus/heimdall/internal/rules/mechanisms/template" "github.com/dadrus/heimdall/internal/x" "github.com/dadrus/heimdall/internal/x/errorchain" ) @@ -28,18 +28,16 @@ func init() { } type redirectErrorHandler struct { - to *url.URL - returnTo string - code int - m []matcher.ErrorConditionMatcher + to template.Template + code int + m []matcher.ErrorConditionMatcher } func newRedirectErrorHandler(rawConfig map[string]any) (*redirectErrorHandler, error) { type Config struct { - To *url.URL `mapstructure:"to"` - Code int `mapstructure:"code"` - ReturnTo string `mapstructure:"return_to_query_parameter"` - When []matcher.ErrorConditionMatcher `mapstructure:"when"` + To template.Template `mapstructure:"to"` + Code int `mapstructure:"code"` + When []matcher.ErrorConditionMatcher `mapstructure:"when"` } var conf Config @@ -62,10 +60,9 @@ func newRedirectErrorHandler(rawConfig map[string]any) (*redirectErrorHandler, e } return &redirectErrorHandler{ - to: conf.To, - returnTo: conf.ReturnTo, - code: x.IfThenElse(conf.Code != 0, conf.Code, http.StatusFound), - m: conf.When, + to: conf.To, + code: x.IfThenElse(conf.Code != 0, conf.Code, http.StatusFound), + m: conf.When, }, nil } @@ -80,18 +77,16 @@ func (eh *redirectErrorHandler) Execute(ctx heimdall.Context, err error) (bool, logger.Debug().Msg("Handling error using redirect error handler") - toURL := *eh.to - if len(eh.returnTo) != 0 { - toQuery := toURL.Query() - - toQuery.Add(eh.returnTo, ctx.RequestURL().String()) - toURL.RawQuery = toQuery.Encode() + toURL, err := eh.to.Render(ctx, nil) + if err != nil { + return true, errorchain.NewWithMessage(heimdall.ErrInternal, "failed to render 'to' url"). + CausedBy(err) } ctx.SetPipelineError(&heimdall.RedirectError{ Message: "redirect", Code: eh.code, - RedirectTo: &toURL, + RedirectTo: toURL, }) return true, nil @@ -120,9 +115,8 @@ func (eh *redirectErrorHandler) WithConfig(rawConfig map[string]any) (ErrorHandl } return &redirectErrorHandler{ - to: eh.to, - returnTo: eh.returnTo, - code: eh.code, - m: conf.When, + to: eh.to, + code: eh.code, + m: conf.When, }, nil } diff --git a/internal/rules/mechanisms/errorhandlers/redirect_error_handler_test.go b/internal/rules/mechanisms/errorhandlers/redirect_error_handler_test.go index 6c3b65342..20738cde1 100644 --- a/internal/rules/mechanisms/errorhandlers/redirect_error_handler_test.go +++ b/internal/rules/mechanisms/errorhandlers/redirect_error_handler_test.go @@ -113,12 +113,11 @@ when: require.NoError(t, err) require.NotNil(t, redEH) - toURL, err := url.Parse("http://foo.bar") + toURL, err := redEH.to.Render(nil, nil) require.NoError(t, err) - assert.Equal(t, toURL, redEH.to) + assert.Equal(t, "http://foo.bar", toURL) assert.Equal(t, http.StatusFound, redEH.code) - assert.Len(t, redEH.returnTo, 0) require.Len(t, redEH.m, 1) assert.Nil(t, redEH.m[0].CIDR) assert.Nil(t, redEH.m[0].Headers) @@ -133,9 +132,8 @@ when: { uc: "with full complex valid configuration", config: []byte(` -to: http://foo.bar +to: http://foo.bar?origin={{ .Request.URL | urlenc }} code: 301 -return_to_query_parameter: foobar when: - error: - type: authentication_error @@ -163,12 +161,16 @@ when: require.NoError(t, err) require.NotNil(t, redEH) - toURL, err := url.Parse("http://foo.bar") + ctx := &mocks.MockContext{} + ctx.On("RequestMethod").Return("POST") + ctx.On("RequestURL").Return(&url.URL{Scheme: "http", Host: "foobar.baz", Path: "zab"}) + ctx.On("RequestClientIPs").Return(nil) + + toURL, err := redEH.to.Render(ctx, nil) require.NoError(t, err) - assert.Equal(t, toURL, redEH.to) + assert.Equal(t, "http://foo.bar?origin=http%3A%2F%2Ffoobar.baz%2Fzab", toURL) assert.Equal(t, http.StatusMovedPermanently, redEH.code) - assert.Equal(t, "foobar", redEH.returnTo) require.Len(t, redEH.m, 2) condition1 := redEH.m[0] @@ -279,7 +281,6 @@ when: prototypeConfig: []byte(` to: http://foo.bar code: 301 -return_to_query_parameter: foobar when: - error: - type: authentication_error @@ -298,7 +299,6 @@ when: assert.NotNil(t, configured) assert.Equal(t, prototype.to, configured.to) assert.Equal(t, prototype.code, configured.code) - assert.Equal(t, prototype.returnTo, configured.returnTo) assert.NotEqual(t, prototype.m, configured.m) assert.Len(t, configured.m, 1) assert.Nil(t, configured.m[0].CIDR) @@ -369,7 +369,32 @@ when: }, }, { - uc: "responsible without return_to_query_parameter", + uc: "responsible for error but with template rendering error", + config: []byte(` +to: http://foo.bar={{ .foobar }} +when: + - error: + - type: authentication_error +`), + error: heimdall.ErrAuthentication, + configureContext: func(t *testing.T, ctx *mocks.MockContext) { + t.Helper() + + ctx.On("RequestMethod").Return("POST") + ctx.On("RequestURL").Return(&url.URL{Scheme: "http", Host: "foobar.baz", Path: "zab"}) + ctx.On("RequestClientIPs").Return(nil) + }, + assert: func(t *testing.T, wasResponsible bool, err error) { + t.Helper() + + assert.Error(t, err) + assert.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 when: @@ -380,13 +405,13 @@ when: configureContext: func(t *testing.T, ctx *mocks.MockContext) { t.Helper() + ctx.On("RequestMethod").Return("POST") + ctx.On("RequestURL").Return(&url.URL{Scheme: "http", Host: "foobar.baz", Path: "zab"}) + ctx.On("RequestClientIPs").Return(nil) ctx.On("SetPipelineError", mock.MatchedBy(func(redirErr *heimdall.RedirectError) bool { t.Helper() - redirectURL, err := url.Parse("http://foo.bar") - require.NoError(t, err) - - assert.Equal(t, redirectURL, redirErr.RedirectTo) + assert.Equal(t, "http://foo.bar", redirErr.RedirectTo) assert.Equal(t, http.StatusFound, redirErr.Code) assert.Equal(t, "redirect", redirErr.Message) @@ -401,11 +426,10 @@ when: }, }, { - uc: "responsible with return_to_query_parameter and code set", + uc: "responsible with template and code set", config: []byte(` -to: http://foo.bar +to: http://foo.bar?origin={{ .Request.URL | urlenc }} code: 300 -return_to_query_parameter: foobar when: - error: - type: authentication_error @@ -417,15 +441,20 @@ when: requestURL, err := url.Parse("http://test.org") require.NoError(t, err) + ctx.On("RequestMethod").Return("POST") + ctx.On("RequestClientIPs").Return(nil) ctx.On("RequestURL").Return(requestURL) ctx.On("SetPipelineError", mock.MatchedBy(func(redirErr *heimdall.RedirectError) bool { t.Helper() - assert.Equal(t, "http", redirErr.RedirectTo.Scheme) - assert.Equal(t, "foo.bar", redirErr.RedirectTo.Host) - assert.Len(t, redirErr.RedirectTo.Query(), 1) - assert.Equal(t, "http://test.org", redirErr.RedirectTo.Query().Get("foobar")) + redirectURL, err := url.Parse(redirErr.RedirectTo) + require.NoError(t, err) + + assert.Equal(t, "http", redirectURL.Scheme) + assert.Equal(t, "foo.bar", redirectURL.Host) + assert.Len(t, redirectURL.Query(), 1) + assert.Equal(t, "http://test.org", redirectURL.Query().Get("origin")) assert.Equal(t, http.StatusMultipleChoices, redirErr.Code) assert.Equal(t, "redirect", redirErr.Message) diff --git a/internal/rules/mechanisms/template/template.go b/internal/rules/mechanisms/template/template.go index a04a1256e..fdd356ab6 100644 --- a/internal/rules/mechanisms/template/template.go +++ b/internal/rules/mechanisms/template/template.go @@ -4,6 +4,7 @@ import ( "bytes" "crypto/sha256" "errors" + "fmt" "net/url" "text/template" @@ -33,7 +34,7 @@ func New(val string) (Template, error) { tmpl, err := template.New("Heimdall"). Funcs(funcMap). - Funcs(template.FuncMap{"urlenc": url.QueryEscape}). + Funcs(template.FuncMap{"urlenc": urlEncode}). Parse(val) if err != nil { return nil, errorchain.NewWithMessage(heimdall.ErrConfiguration, "failed to parse template"). @@ -65,3 +66,14 @@ func (t *templateImpl) Render(ctx heimdall.Context, sub *subject.Subject) (strin } func (t *templateImpl) Hash() []byte { return t.hash } + +func urlEncode(value any) string { + switch t := value.(type) { + case string: + return url.QueryEscape(t) + case fmt.Stringer: + return url.QueryEscape(t.String()) + default: + return "" + } +} diff --git a/internal/signer/jwt_signer.go b/internal/signer/jwt_signer.go index 2496f5fa7..f27154b88 100644 --- a/internal/signer/jwt_signer.go +++ b/internal/signer/jwt_signer.go @@ -22,14 +22,14 @@ import ( "github.com/dadrus/heimdall/internal/x/pkix" ) -func NewJWTSigner(conf *config.SignerConfig, logger zerolog.Logger) (heimdall.JWTSigner, error) { +func NewJWTSigner(conf *config.Configuration, logger zerolog.Logger) (heimdall.JWTSigner, error) { var ( ks keystore.KeyStore kse *keystore.Entry err error ) - if len(conf.KeyStore) == 0 { + if len(conf.Signer.KeyStore) == 0 { logger.Warn(). Msg("Key store is not configured. NEVER DO IT IN PRODUCTION!!!! Generating an ECDSA P-384 key pair.") @@ -43,7 +43,7 @@ func NewJWTSigner(conf *config.SignerConfig, logger zerolog.Logger) (heimdall.JW ks, err = keystore.NewKeyStoreFromKey(privateKey) } else { - ks, err = keystore.NewKeyStoreFromPEMFile(conf.KeyStore, conf.Password) + ks, err = keystore.NewKeyStoreFromPEMFile(conf.Signer.KeyStore, conf.Signer.Password) } if err != nil { @@ -60,12 +60,12 @@ func NewJWTSigner(conf *config.SignerConfig, logger zerolog.Logger) (heimdall.JW Msg("Entry info") } - if len(conf.KeyID) == 0 { + if len(conf.Signer.KeyID) == 0 { logger.Warn().Msg("No key id for signer configured. Taking first entry from the key store") kse, err = ks.Entries()[0], nil } else { - kse, err = ks.GetKey(conf.KeyID) + kse, err = ks.GetKey(conf.Signer.KeyID) } if err != nil { @@ -85,9 +85,10 @@ func NewJWTSigner(conf *config.SignerConfig, logger zerolog.Logger) (heimdall.JW logger.Info().Str("_key_id", kse.KeyID).Msg("Signer configured") return &jwtSigner{ - iss: conf.Name, + iss: conf.Signer.Name, jwk: kse.JWK(), key: kse.PrivateKey, + ks: ks, }, nil } @@ -95,6 +96,7 @@ type jwtSigner struct { iss string jwk jose.JSONWebKey key crypto.Signer + ks keystore.KeyStore } func (s *jwtSigner) Hash() []byte { @@ -141,3 +143,13 @@ func (s *jwtSigner) Sign(sub string, ttl time.Duration, custClaims map[string]an return rawJwt, nil } + +func (s *jwtSigner) Keys() []jose.JSONWebKey { + keys := make([]jose.JSONWebKey, len(s.ks.Entries())) + + for idx, entry := range s.ks.Entries() { + keys[idx] = entry.JWK() + } + + return keys +} diff --git a/internal/signer/jwt_signer_test.go b/internal/signer/jwt_signer_test.go index d68f59b08..c605d0cb2 100644 --- a/internal/signer/jwt_signer_test.go +++ b/internal/signer/jwt_signer_test.go @@ -248,7 +248,7 @@ func TestNewJWTSigner(t *testing.T) { } { t.Run("case="+tc.uc, func(t *testing.T) { // WHEN - signer, err := NewJWTSigner(&tc.config, log.Logger) + signer, err := NewJWTSigner(&config.Configuration{Signer: tc.config}, log.Logger) // THEN var ( @@ -416,3 +416,42 @@ func TestJWTSignerHash(t *testing.T) { assert.NotEmpty(t, hash1) assert.Equal(t, hash1, hash2) } + +func TestJwtSignerKeys(t *testing.T) { + t.Parallel() + + // GIVEN + privKey1, err := rsa.GenerateKey(rand.Reader, 2048) + require.NoError(t, err) + + privKey2, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + require.NoError(t, err) + + testDir := t.TempDir() + keyFile, err := os.Create(filepath.Join(testDir, "keys.pem")) + require.NoError(t, err) + + pemBytes, err := pemx.BuildPEM( + pemx.WithRSAPrivateKey(privKey1, pemx.WithHeader("X-Key-ID", "key1")), + pemx.WithECDSAPrivateKey(privKey2, pemx.WithHeader("X-Key-ID", "key2")), + ) + require.NoError(t, err) + + _, err = keyFile.Write(pemBytes) + require.NoError(t, err) + + signer, err := NewJWTSigner( + &config.Configuration{Signer: config.SignerConfig{KeyStore: keyFile.Name()}}, + log.Logger, + ) + require.NoError(t, err) + + // WHEN + keys := signer.Keys() + + // THEN + require.Len(t, keys, 2) + + assert.Equal(t, "PS256", keys[0].Algorithm) + assert.Equal(t, "ES256", keys[1].Algorithm) +} diff --git a/internal/signer/module.go b/internal/signer/module.go new file mode 100644 index 000000000..c9d2b4555 --- /dev/null +++ b/internal/signer/module.go @@ -0,0 +1,9 @@ +package signer + +import "go.uber.org/fx" + +// Module is used on app bootstrap. +// nolint: gochecknoglobals +var Module = fx.Options( + fx.Provide(NewJWTSigner), +) diff --git a/internal/x/testsupport/freeport.go b/internal/x/testsupport/freeport.go new file mode 100644 index 000000000..ac12b5fcb --- /dev/null +++ b/internal/x/testsupport/freeport.go @@ -0,0 +1,20 @@ +package testsupport + +import "net" + +func GetFreePort() (int, error) { + addr, err := net.ResolveTCPAddr("tcp", "localhost:0") + if err != nil { + return 0, err + } + + listener, err := net.ListenTCP("tcp", addr) + if err != nil { + return 0, err + } + + defer listener.Close() + + // nolint: forcetypeassert + return listener.Addr().(*net.TCPAddr).Port, nil +} diff --git a/schema/config.schema.json b/schema/config.schema.json index fcfa63065..893ed2e98 100644 --- a/schema/config.schema.json +++ b/schema/config.schema.json @@ -1540,7 +1540,6 @@ "to": { "description": "Set the redirect target. Can either be a http/https URL, or a relative URL.", "type": "string", - "format": "uri-reference", "examples": [ "http://my-app.com/dashboard", "https://my-app.com/dashboard", @@ -1556,12 +1555,6 @@ ], "default": 302 }, - "return_to_query_parameter": { - "description": "Adds the original URL the request tried to access to the query parameter.", - "type": "string", - "pattern": "^[A-Za-z0-9,._~-]*$", - "default": "" - }, "when": { "$ref": "#/definitions/errorsWhen" }