fix(url/redirect): ensure fragments are preserved

This commit is contained in:
Trong Huu Nguyen
2023-07-13 12:34:47 +02:00
parent 1843594c31
commit c44fb9889b
2 changed files with 60 additions and 19 deletions

View File

@@ -33,7 +33,7 @@ func NewStandaloneRedirect(ingresses *ingress.Ingresses) *StandaloneRedirect {
func (h *StandaloneRedirect) Canonical(r *http.Request) string {
target := redirectQueryParam(r)
redirect, err := url.ParseRequestURI(target)
redirect, err := url.Parse(target)
if err != nil {
redirect = fallback(r, target, h.getFallbackRedirect(r))
}
@@ -61,7 +61,7 @@ type SSOServerRedirect struct {
}
func NewSSOServerRedirect(config *config.Config) (*SSOServerRedirect, error) {
u, err := url.ParseRequestURI(config.SSO.ServerDefaultRedirectURL)
u, err := url.Parse(config.SSO.ServerDefaultRedirectURL)
if err != nil {
return nil, fmt.Errorf("parsing fallback redirect URL: %w", err)
}
@@ -74,7 +74,7 @@ func NewSSOServerRedirect(config *config.Config) (*SSOServerRedirect, error) {
func (h *SSOServerRedirect) Canonical(r *http.Request) string {
target := redirectQueryParam(r)
redirect, err := url.ParseRequestURI(target)
redirect, err := url.Parse(target)
if err != nil {
redirect = fallback(r, target, h.fallbackRedirect)
}
@@ -109,13 +109,14 @@ func (h *SSOProxyRedirect) Canonical(r *http.Request) string {
// get redirect from request query parameter
target := redirectQueryParam(r)
redirectParamURL, err := url.ParseRequestURI(target)
redirectParamURL, err := url.Parse(target)
if err != nil {
logInvalidRedirect(r, target, redirect.String())
} else {
// copy desired path and query to base redirect
// preserve path, query and fragment from redirect parameter to base redirect
redirect.Path = redirectParamURL.Path
redirect.RawQuery = redirectParamURL.RawQuery
redirect.Fragment = redirectParamURL.Fragment
}
return h.Clean(r, redirect.String())

View File

@@ -142,6 +142,22 @@ func TestDefault(t *testing.T) {
clean: "/",
},
},
{
name: "relative url with fragment in redirect",
url: "/?redirect=%2F%23%2Fsome-path",
expected: expected{
canonical: "/#/some-path",
clean: "/?redirect=%2F%23%2Fsome-path",
},
},
{
name: "absolute url with fragment in redirect",
url: "/?redirect=http%3A%2F%2Fwonderwall%2F%23%2Fsome-path",
expected: expected{
canonical: "/#/some-path",
clean: "/?redirect=http%3A%2F%2Fwonderwall%2F%23%2Fsome-path",
},
},
} {
t.Run(tt.name, func(t *testing.T) {
r := mock.NewGetRequest(tt.url, ingresses)
@@ -306,6 +322,14 @@ func TestSSOServer(t *testing.T) {
clean: "http://wonderwall/?redirect=http%3A%2F%2Fsome.app.wonderwall%2Fsome-path%3Fwith%3Dquery",
},
},
{
name: "absolute url with fragment in redirect",
url: "http://wonderwall/?redirect=http%3A%2F%2Fwonderwall%2F%23%2Fsome-path",
expected: expected{
canonical: "http://wonderwall/#/some-path",
clean: "http://wonderwall/?redirect=http%3A%2F%2Fwonderwall%2F%23%2Fsome-path",
},
},
} {
t.Run(tt.name, func(t *testing.T) {
r := mock.NewGetRequest(tt.url, ingresses)
@@ -389,10 +413,10 @@ func TestSSOProxy(t *testing.T) {
},
{
name: "absolute url",
url: "http://wonderwall/?redirect=%2Fpath",
url: "http://app.wonderwall/?redirect=%2Fpath",
expected: expected{
canonical: "http://app.wonderwall/path",
clean: "http://app.wonderwall",
clean: "http://app.wonderwall/?redirect=%2Fpath",
},
},
{
@@ -405,26 +429,26 @@ func TestSSOProxy(t *testing.T) {
},
{
name: "absolute url with context path",
url: "http://wonderwall/some-path?redirect=%2Fpath",
url: "http://app.wonderwall/some-path?redirect=%2Fpath",
expected: expected{
canonical: "http://app.wonderwall/path",
clean: "http://app.wonderwall",
clean: "http://app.wonderwall/some-path?redirect=%2Fpath",
},
},
{
name: "absolute url with query parameters",
url: "http://wonderwall/some-path?redirect=%2Fpath%3Fgnu%3Dnotunix",
url: "http://app.wonderwall/some-path?redirect=%2Fpath%3Fgnu%3Dnotunix",
expected: expected{
canonical: "http://app.wonderwall/path?gnu=notunix",
clean: "http://app.wonderwall",
clean: "http://app.wonderwall/some-path?redirect=%2Fpath%3Fgnu%3Dnotunix",
},
},
{
name: "absolute url with multiple query parameters",
url: "http://wonderwall/some-path?redirect=%2Fpath%3Fgnu%3Dnotunix%26foo%3Dbar",
url: "http://app.wonderwall/some-path?redirect=%2Fpath%3Fgnu%3Dnotunix%26foo%3Dbar",
expected: expected{
canonical: "http://app.wonderwall/path?gnu=notunix&foo=bar",
clean: "http://app.wonderwall",
clean: "http://app.wonderwall/some-path?redirect=%2Fpath%3Fgnu%3Dnotunix%26foo%3Dbar",
},
},
{
@@ -445,26 +469,42 @@ func TestSSOProxy(t *testing.T) {
},
{
name: "absolute url with different domain in redirect",
url: "http://wonderwall/?redirect=http%3A%2F%2Fnot-wonderwall%2Fsome-path%3Fwith%3Dquery",
url: "http://app.wonderwall/?redirect=http%3A%2F%2Fnot-wonderwall%2Fsome-path%3Fwith%3Dquery",
expected: expected{
canonical: "http://app.wonderwall/some-path?with=query",
clean: "http://app.wonderwall",
clean: "http://app.wonderwall/?redirect=http%3A%2F%2Fnot-wonderwall%2Fsome-path%3Fwith%3Dquery",
},
},
{
name: "absolute url with subdomain in redirect",
url: "http://wonderwall/?redirect=http%3A%2F%2Fanother-app.wonderwall%2Fsome-path%3Fwith%3Dquery",
url: "http://app.wonderwall/?redirect=http%3A%2F%2Fanother-app.wonderwall%2Fsome-path%3Fwith%3Dquery",
expected: expected{
canonical: "http://app.wonderwall/some-path?with=query",
clean: "http://app.wonderwall",
clean: "http://app.wonderwall/?redirect=http%3A%2F%2Fanother-app.wonderwall%2Fsome-path%3Fwith%3Dquery",
},
},
{
name: "absolute url with nested subdomain in redirect",
url: "http://wonderwall/?redirect=http%3A%2F%2Fsome.app.wonderwall%2Fsome-path%3Fwith%3Dquery",
url: "http://app.wonderwall/?redirect=http%3A%2F%2Fsome.app.wonderwall%2Fsome-path%3Fwith%3Dquery",
expected: expected{
canonical: "http://app.wonderwall/some-path?with=query",
clean: "http://app.wonderwall",
clean: "http://app.wonderwall/?redirect=http%3A%2F%2Fsome.app.wonderwall%2Fsome-path%3Fwith%3Dquery",
},
},
{
name: "relative url with fragment in redirect",
url: "http://app.wonderwall/?redirect=%2F%23%2Fsome-path",
expected: expected{
canonical: "http://app.wonderwall/#/some-path",
clean: "http://app.wonderwall/?redirect=%2F%23%2Fsome-path",
},
},
{
name: "absolute url with fragment in redirect",
url: "http://app.wonderwall/?redirect=http%3A%2F%2Fwonderwall%2F%23%2Fsome-path",
expected: expected{
canonical: "http://app.wonderwall/#/some-path",
clean: "http://app.wonderwall/?redirect=http%3A%2F%2Fwonderwall%2F%23%2Fsome-path",
},
},
} {