From e065d0e42e722313b45d17bf7f2d1b4d1fb37ba5 Mon Sep 17 00:00:00 2001 From: Matus Mucka Date: Fri, 5 Feb 2021 16:58:24 +0100 Subject: [PATCH 1/7] fix parsing URI with fragment --- driver/config/config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/driver/config/config.go b/driver/config/config.go index 92df3ca5630..a6f6116d684 100644 --- a/driver/config/config.go +++ b/driver/config/config.go @@ -562,7 +562,7 @@ func (p *Config) CourierTemplatesRoot() string { } func (p *Config) parseURIOrFail(key string) *url.URL { - u, err := url.ParseRequestURI(p.p.String(key)) + u, err := url.Parse(p.p.String(key)) if err != nil { p.l.WithError(errors.WithStack(err)). Fatalf("Configuration value from key %s is not a valid URL: %s", key, p.p.String(key)) From 32317d01ff5ec250a412d026f36914672ccc3058 Mon Sep 17 00:00:00 2001 From: Matus Mucka Date: Fri, 5 Feb 2021 17:26:33 +0100 Subject: [PATCH 2/7] add tests for urls containing fragments --- driver/config/config_test.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/driver/config/config_test.go b/driver/config/config_test.go index 755b46e25d9..0cad621aa95 100644 --- a/driver/config/config_test.go +++ b/driver/config/config_test.go @@ -44,6 +44,21 @@ func TestViperProvider(t *testing.T) { "http://return-to-1-test.ory.sh/", "http://return-to-2-test.ory.sh/", }, ds) + + pWithFragments := config.MustNew(logrusx.New("", ""), + configx.WithValues(map[string]interface{}{ + config.ViperKeySelfServiceLoginUI: "http://test.kratos.ory.sh/#/login", + config.ViperKeySelfServiceSettingsURL: "http://test.kratos.ory.sh/#/settings", + config.ViperKeySelfServiceRegistrationUI: "http://test.kratos.ory.sh/#/register", + config.ViperKeySelfServiceErrorUI: "http://test.kratos.ory.sh/#/error", + }), + configx.SkipValidation(), + ) + + assert.Equal(t, "http://test.kratos.ory.sh/#/login", pWithFragments.SelfServiceFlowLoginUI().String()) + assert.Equal(t, "http://test.kratos.ory.sh/#/settings", pWithFragments.SelfServiceFlowSettingsUI().String()) + assert.Equal(t, "http://test.kratos.ory.sh/#/register", pWithFragments.SelfServiceFlowRegistrationUI().String()) + assert.Equal(t, "http://test.kratos.ory.sh/#/error", pWithFragments.SelfServiceFlowErrorURL().String()) }) t.Run("group=default_return_to", func(t *testing.T) { From 84f7b3e87a9216fecad33826b9d4af6201b6e6c6 Mon Sep 17 00:00:00 2001 From: Matus Mucka Date: Sat, 6 Feb 2021 18:05:51 +0100 Subject: [PATCH 3/7] improve parsing URI --- driver/config/config.go | 21 +++++++++++++++++++-- driver/config/config_test.go | 27 +++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 2 deletions(-) diff --git a/driver/config/config.go b/driver/config/config.go index a6f6116d684..115e8855c17 100644 --- a/driver/config/config.go +++ b/driver/config/config.go @@ -10,6 +10,7 @@ import ( "net/url" "os" "runtime" + "strings" "time" "github.com/markbates/pkger" @@ -561,13 +562,29 @@ func (p *Config) CourierTemplatesRoot() string { return p.p.StringF(ViperKeyCourierTemplatesPath, "/courier/template/templates") } +func splitUrlAndFragment(s string) (string, string) { + i := strings.IndexByte(s, '#') + if i < 0 { + return s, "" + } + return s[:i], s[i+1:] +} + func (p *Config) parseURIOrFail(key string) *url.URL { - u, err := url.Parse(p.p.String(key)) + u, frag := splitUrlAndFragment(p.p.String(key)) + url, err := url.ParseRequestURI(u) if err != nil { p.l.WithError(errors.WithStack(err)). Fatalf("Configuration value from key %s is not a valid URL: %s", key, p.p.String(key)) } - return u + if url.Host == "" || (url.Scheme != "http" && url.Scheme != "https") { + p.l.Fatalf("Configuration value from key %s is not a valid URL: %s", key, p.p.String(key)) + } + + if frag != "" { + url.Fragment = frag + } + return url } func (p *Config) Tracing() *tracing.Config { diff --git a/driver/config/config_test.go b/driver/config/config_test.go index 0cad621aa95..7da5883cd6f 100644 --- a/driver/config/config_test.go +++ b/driver/config/config_test.go @@ -11,6 +11,7 @@ import ( "github.com/ory/x/logrusx" "github.com/ory/x/urlx" + "github.com/sirupsen/logrus/hooks/test" _ "github.com/ory/jsonschema/v3/fileloader" @@ -59,6 +60,32 @@ func TestViperProvider(t *testing.T) { assert.Equal(t, "http://test.kratos.ory.sh/#/settings", pWithFragments.SelfServiceFlowSettingsUI().String()) assert.Equal(t, "http://test.kratos.ory.sh/#/register", pWithFragments.SelfServiceFlowRegistrationUI().String()) assert.Equal(t, "http://test.kratos.ory.sh/#/error", pWithFragments.SelfServiceFlowErrorURL().String()) + + for _, v := range []string{ + "#/login", + "/login", + "/", + "smtp://test.kratos.ory.sh/login", + } { + + logger := logrusx.New("", "") + logger.Logger.ExitFunc = func(code int) { panic("") } + hook := new(test.Hook) + logger.Logger.Hooks.Add(hook) + + pWithIncorrectUrls := config.MustNew(logger, + configx.WithValues(map[string]interface{}{ + config.ViperKeySelfServiceLoginUI: v, + }), + configx.SkipValidation(), + ) + + assert.Panics(t, func() { pWithIncorrectUrls.SelfServiceFlowLoginUI() }) + + assert.Equal(t, logrus.FatalLevel, hook.LastEntry().Level) + assert.Equal(t, "Configuration value from key selfservice.flows.login.ui_url is not a valid URL: "+v, hook.LastEntry().Message) + assert.Equal(t, 1, len(hook.Entries)) + } }) t.Run("group=default_return_to", func(t *testing.T) { From 8a960b455f7428ccc038ec94f48cdcf32c988044 Mon Sep 17 00:00:00 2001 From: Matus Mucka Date: Tue, 9 Feb 2021 20:11:49 +0100 Subject: [PATCH 4/7] check for empty schema only --- driver/config/config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/driver/config/config.go b/driver/config/config.go index 115e8855c17..e43fbb3fbe9 100644 --- a/driver/config/config.go +++ b/driver/config/config.go @@ -577,7 +577,7 @@ func (p *Config) parseURIOrFail(key string) *url.URL { p.l.WithError(errors.WithStack(err)). Fatalf("Configuration value from key %s is not a valid URL: %s", key, p.p.String(key)) } - if url.Host == "" || (url.Scheme != "http" && url.Scheme != "https") { + if url.Host == "" || (url.Scheme != "") { p.l.Fatalf("Configuration value from key %s is not a valid URL: %s", key, p.p.String(key)) } From 3dbafee6dbff97887f69da0c90c1be2f62ad2c46 Mon Sep 17 00:00:00 2001 From: Matus Mucka Date: Tue, 9 Feb 2021 20:12:18 +0100 Subject: [PATCH 5/7] remove redundant parentheses --- driver/config/config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/driver/config/config.go b/driver/config/config.go index e43fbb3fbe9..ca436309ec0 100644 --- a/driver/config/config.go +++ b/driver/config/config.go @@ -577,7 +577,7 @@ func (p *Config) parseURIOrFail(key string) *url.URL { p.l.WithError(errors.WithStack(err)). Fatalf("Configuration value from key %s is not a valid URL: %s", key, p.p.String(key)) } - if url.Host == "" || (url.Scheme != "") { + if url.Host == "" || url.Scheme != "" { p.l.Fatalf("Configuration value from key %s is not a valid URL: %s", key, p.p.String(key)) } From 7d536be4abe7ff143304a4039ee150eaa54d2804 Mon Sep 17 00:00:00 2001 From: Matus Mucka Date: Tue, 9 Feb 2021 20:18:05 +0100 Subject: [PATCH 6/7] add test url withou schema --- driver/config/config_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/driver/config/config_test.go b/driver/config/config_test.go index 7da5883cd6f..b8746507382 100644 --- a/driver/config/config_test.go +++ b/driver/config/config_test.go @@ -65,7 +65,7 @@ func TestViperProvider(t *testing.T) { "#/login", "/login", "/", - "smtp://test.kratos.ory.sh/login", + "test.kratos.ory.sh/login", } { logger := logrusx.New("", "") From 307d11b5fb9c08478aa5b9327305218f968bf91e Mon Sep 17 00:00:00 2001 From: Matus Mucka Date: Tue, 9 Feb 2021 20:32:16 +0100 Subject: [PATCH 7/7] fix schema condition --- driver/config/config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/driver/config/config.go b/driver/config/config.go index ca436309ec0..b03b6a247d2 100644 --- a/driver/config/config.go +++ b/driver/config/config.go @@ -577,7 +577,7 @@ func (p *Config) parseURIOrFail(key string) *url.URL { p.l.WithError(errors.WithStack(err)). Fatalf("Configuration value from key %s is not a valid URL: %s", key, p.p.String(key)) } - if url.Host == "" || url.Scheme != "" { + if url.Host == "" || url.Scheme == "" { p.l.Fatalf("Configuration value from key %s is not a valid URL: %s", key, p.p.String(key)) }