Skip to content

Commit

Permalink
feat: use uri-reference for ui_url etc. to allow relative urls (#617)
Browse files Browse the repository at this point in the history
  • Loading branch information
David Laban committed Aug 10, 2020
1 parent ce8ac61 commit 2dba450
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 24 deletions.
27 changes: 15 additions & 12 deletions .schema/config.schema.json
View file
Expand Up @@ -13,10 +13,11 @@
"title": "Redirect browsers to set URL per default",
"description": "ORY Kratos redirects to this URL per default on completion of self-service flows and other browser interaction. Read this [article for more information on browser redirects](https://www.ory.sh/kratos/docs/concepts/browser-redirect-flow-completion).",
"type": "string",
"format": "uri",
"minLength": 6,
"format": "uri-reference",
"minLength": 1,
"examples": [
"https://my-app.com/dashboard"
"https://my-app.com/dashboard",
"/dashboard"
]
},
"selfServiceSessionRevokerHook": {
Expand Down Expand Up @@ -274,11 +275,12 @@
"type": "array",
"items": {
"type": "string",
"format": "uri"
"format": "uri-reference"
},
"examples": [
[
"https://app.my-app.com/dashboard",
"/dashboard",
"https://www.my-app.com/"
]
],
Expand All @@ -296,7 +298,7 @@
"title": "URL of the Settings page.",
"description": "URL where the Settings UI is hosted. Check the [reference implementation](https://github.com/ory/kratos-selfservice-ui-node).",
"type": "string",
"format": "uri",
"format": "uri-reference",
"examples": [
"https://my-app.com/user/settings"
],
Expand Down Expand Up @@ -350,7 +352,7 @@
"title": "Registration UI URL",
"description": "URL where the Registration UI is hosted. Check the [reference implementation](https://github.com/ory/kratos-selfservice-ui-node).",
"type": "string",
"format": "uri",
"format": "uri-reference",
"examples": [
"https://my-app.com/signup"
],
Expand Down Expand Up @@ -379,7 +381,7 @@
"title": "Login UI URL",
"description": "URL where the Login UI is hosted. Check the [reference implementation](https://github.com/ory/kratos-selfservice-ui-node).",
"type": "string",
"format": "uri",
"format": "uri-reference",
"examples": [
"https://my-app.com/login"
],
Expand Down Expand Up @@ -415,7 +417,7 @@
"title": "Verify UI URL",
"description": "URL where the ORY Verify UI is hosted. This is the page where users activate and / or verify their email or telephone number. Check the [reference implementation](https://github.com/ory/kratos-selfservice-ui-node).",
"type": "string",
"format": "uri",
"format": "uri-reference",
"examples": [
"https://my-app.com/verify"
],
Expand Down Expand Up @@ -459,7 +461,7 @@
"title": "Recovery UI URL",
"description": "URL where the ORY Recovery UI is hosted. This is the page where users request and complete account recovery. Check the [reference implementation](https://github.com/ory/kratos-selfservice-ui-node).",
"type": "string",
"format": "uri",
"format": "uri-reference",
"examples": [
"https://my-app.com/verify"
],
Expand Down Expand Up @@ -496,7 +498,7 @@
"title": "ORY Kratos Error UI URL",
"description": "URL where the ORY Kratos Error UI is hosted. Check the [reference implementation](https://github.com/ory/kratos-selfservice-ui-node).",
"type": "string",
"format": "uri",
"format": "uri-reference",
"examples": [
"https://my-app.com/kratos-error"
],
Expand Down Expand Up @@ -671,9 +673,10 @@
"title": "Public Base URL",
"description": "The URL where the public endpoint is exposed at.",
"type": "string",
"format": "uri",
"format": "uri-reference",
"examples": [
"https://my-app.com/.ory/kratos/public"
"https://my-app.com/.ory/kratos/public",
"/.ory/kratos/public/"
]
},
"host": {
Expand Down
43 changes: 31 additions & 12 deletions x/http_secure_redirect_test.go
View file
Expand Up @@ -75,17 +75,20 @@ func TestSecureContentNegotiationRedirection(t *testing.T) {
}

func TestSecureRedirectTo(t *testing.T) {
const defaultReturnTo = "/default-return-to"

var newServer = func(t *testing.T, isTLS bool, expectErr bool, opts func(ts *httptest.Server) []x.SecureRedirectOption) *httptest.Server {
var newServer = func(t *testing.T, isTLS bool, isRelative bool, expectErr bool, opts func(ts *httptest.Server) []x.SecureRedirectOption) *httptest.Server {
var ts *httptest.Server
f := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if opts == nil {
opts = func(ts *httptest.Server) []x.SecureRedirectOption {
return nil
}
}
ru, err := x.SecureRedirectTo(r, urlx.ParseOrPanic(ts.URL+defaultReturnTo), opts(ts)...)
defaultReturnTo := "/default-return-to"
if !isRelative {
defaultReturnTo = ts.URL+defaultReturnTo
}
ru, err := x.SecureRedirectTo(r, urlx.ParseOrPanic(defaultReturnTo), opts(ts)...)
if expectErr {
require.Error(t, err)
_, _ = w.Write([]byte("error"))
Expand Down Expand Up @@ -117,69 +120,85 @@ func TestSecureRedirectTo(t *testing.T) {
}

t.Run("case=return to default URL if nothing is allowed", func(t *testing.T) {
s := newServer(t, false, false, nil)
s := newServer(t, false, false, false, nil)
_, body := makeRequest(t, s, "?return_to=/foo")
assert.EqualValues(t, body, s.URL+"/default-return-to")
})

t.Run("case=return to foo with server baseURL if allowed", func(t *testing.T) {
s := newServer(t, false, false, func(ts *httptest.Server) []x.SecureRedirectOption {
s := newServer(t, false, false, false, func(ts *httptest.Server) []x.SecureRedirectOption {
return []x.SecureRedirectOption{x.SecureRedirectAllowURLs([]url.URL{*urlx.ParseOrPanic(ts.URL)})}
})
_, body := makeRequest(t, s, "?return_to=/foo")
assert.Equal(t, body, s.URL+"/foo")
})

t.Run("case=return to a relative path works", func(t *testing.T) {
s := newServer(t, false, true, false, func(ts *httptest.Server) []x.SecureRedirectOption {
return []x.SecureRedirectOption{x.SecureRedirectAllowURLs([]url.URL{*urlx.ParseOrPanic("/foo")})}
})
_, body := makeRequest(t, s, "?return_to=/foo/kratos")
assert.Equal(t, body, "/foo/kratos")
})

t.Run("case=return to a fully qualified domain is forbidden if whitelist is relative", func(t *testing.T) {
s := newServer(t, false, true, true, func(ts *httptest.Server) []x.SecureRedirectOption {
return []x.SecureRedirectOption{x.SecureRedirectAllowURLs([]url.URL{*urlx.ParseOrPanic("/foo")})}
})
_, body := makeRequest(t, s, "?return_to=https://www.ory.sh/foo/kratos")
assert.Equal(t, body, "error")
})

t.Run("case=return to another domain works", func(t *testing.T) {
s := newServer(t, false, false, func(ts *httptest.Server) []x.SecureRedirectOption {
s := newServer(t, false, false, false, func(ts *httptest.Server) []x.SecureRedirectOption {
return []x.SecureRedirectOption{x.SecureRedirectAllowURLs([]url.URL{*urlx.ParseOrPanic("https://www.ory.sh/foo")})}
})
_, body := makeRequest(t, s, "?return_to=https://www.ory.sh/foo/kratos")
assert.Equal(t, body, "https://www.ory.sh/foo/kratos")
})

t.Run("case=return to another domain fails if host mismatches", func(t *testing.T) {
s := newServer(t, false, true, func(ts *httptest.Server) []x.SecureRedirectOption {
s := newServer(t, false, false, true, func(ts *httptest.Server) []x.SecureRedirectOption {
return []x.SecureRedirectOption{x.SecureRedirectAllowURLs([]url.URL{*urlx.ParseOrPanic("https://www.not-ory.sh/")})}
})
_, body := makeRequest(t, s, "?return_to=https://www.ory.sh/kratos")
assert.Equal(t, body, "error")
})

t.Run("case=return to another domain fails if path mismatches", func(t *testing.T) {
s := newServer(t, false, true, func(ts *httptest.Server) []x.SecureRedirectOption {
s := newServer(t, false, false, true, func(ts *httptest.Server) []x.SecureRedirectOption {
return []x.SecureRedirectOption{x.SecureRedirectAllowURLs([]url.URL{*urlx.ParseOrPanic("https://www.ory.sh/not-kratos")})}
})
_, body := makeRequest(t, s, "?return_to=https://www.ory.sh/kratos")
assert.Equal(t, body, "error")
})

t.Run("case=return to another domain fails if scheme mismatches", func(t *testing.T) {
s := newServer(t, false, true, func(ts *httptest.Server) []x.SecureRedirectOption {
s := newServer(t, false, false, true, func(ts *httptest.Server) []x.SecureRedirectOption {
return []x.SecureRedirectOption{x.SecureRedirectAllowURLs([]url.URL{*urlx.ParseOrPanic("http://www.ory.sh/")})}
})
_, body := makeRequest(t, s, "?return_to=https://www.ory.sh/kratos")
assert.Equal(t, body, "error")
})

t.Run("case=should work with self-service modifier", func(t *testing.T) {
s := newServer(t, false, false, func(ts *httptest.Server) []x.SecureRedirectOption {
s := newServer(t, false, false, false, func(ts *httptest.Server) []x.SecureRedirectOption {
return []x.SecureRedirectOption{x.SecureRedirectAllowSelfServiceURLs(urlx.ParseOrPanic(ts.URL))}
})
_, body := makeRequest(t, s, "?return_to=/self-service/foo")
assert.Equal(t, body, s.URL+"/self-service/foo")
})

t.Run("case=should work with default return to", func(t *testing.T) {
s := newServer(t, false, false, func(ts *httptest.Server) []x.SecureRedirectOption {
s := newServer(t, false, false, false, func(ts *httptest.Server) []x.SecureRedirectOption {
return []x.SecureRedirectOption{x.SecureRedirectOverrideDefaultReturnTo(urlx.ParseOrPanic(ts.URL + "/another-default"))}
})
_, body := makeRequest(t, s, "")
assert.Equal(t, body, s.URL+"/another-default")
})

t.Run("case=should override return_to", func(t *testing.T) {
s := newServer(t, false, false, func(ts *httptest.Server) []x.SecureRedirectOption {
s := newServer(t, false, false, false, func(ts *httptest.Server) []x.SecureRedirectOption {
return []x.SecureRedirectOption{
x.SecureRedirectAllowURLs([]url.URL{*urlx.ParseOrPanic(ts.URL)}),
x.SecureRedirectUseSourceURL("https://foo/bar?return_to=/override"),
Expand Down

0 comments on commit 2dba450

Please sign in to comment.