github/wonderwall
1
0
Fork 0
mirror of https://github.com/nais/wonderwall.git synced 2026-05-12 03:16:37 +00:00
Code Issues Packages Projects Releases Wiki Activity

refactor(handler/url): remove support for Referer header

Browse Source 
The header isn't guaranteed to be set or sent with requests, and all of
our users prefer the `redirect` query parameter anyways.
This commit is contained in:
Trong Huu Nguyen
2022-09-22 13:05:13 +02:00
parent aaaaaaa38d
commit b651db40e4
3 changed files with 25 additions and 108 deletions
Download Patch File Download Diff File Expand all files Collapse all files

88
pkg/handler/error/error_test.go
View File

@@ -60,12 +60,8 @@ func TestHandler_Retry(t *testing.T) {
handler := idp.RelyingPartyHandler.GetErrorHandler()
httpRequest := func(url string, referer ...string) *http.Request {
req := httptest.NewRequest(http.MethodGet, url, nil)
if len(referer) > 0 {
req.Header.Add("Referer", referer[0])
}
return req
get := func(url string) *http.Request {
return httptest.NewRequest(http.MethodGet, url, nil)
}
for _, test := range []struct {
@@ -77,134 +73,96 @@ func TestHandler_Retry(t *testing.T) {
}{
{
name: "login path",
request: httpRequest("/oauth2/login"),
request: get("/oauth2/login"),
want: "/oauth2/login?redirect-encoded=" + urlpkg.RedirectEncoded("/"),
},
{
name: "callback path",
request: httpRequest("/oauth2/callback"),
request: get("/oauth2/callback"),
want: "/oauth2/login?redirect-encoded=" + urlpkg.RedirectEncoded("/"),
},
{
name: "logout path",
request: httpRequest("/oauth2/logout"),
request: get("/oauth2/logout"),
want: "/oauth2/logout",
},
{
name: "front-channel logout path",
request: httpRequest("/oauth2/logout/frontchannel"),
request: get("/oauth2/logout/frontchannel"),
want: "/oauth2/logout/frontchannel",
},
{
name: "login with non-default ingress",
request: httpRequest("/domene/oauth2/login"),
request: get("/domene/oauth2/login"),
ingress: "https://test.nav.no/domene",
want: "/domene/oauth2/login?redirect-encoded=" + urlpkg.RedirectEncoded("/domene"),
},
{
name: "logout with non-default ingress",
request: httpRequest("/domene/oauth2/logout"),
request: get("/domene/oauth2/logout"),
ingress: "https://test.nav.no/domene",
want: "/domene/oauth2/logout",
},
{
name: "login with referer",
request: httpRequest("/oauth2/login", "/api/me"),
want: "/oauth2/login?redirect-encoded=" + urlpkg.RedirectEncoded("/api/me"),
},
{
name: "login with referer on non-default ingress",
request: httpRequest("/domene/oauth2/login", "/api/me"),
ingress: "https://test.nav.no/domene",
want: "/domene/oauth2/login?redirect-encoded=" + urlpkg.RedirectEncoded("/api/me"),
},
{
name: "login with root referer",
request: httpRequest("/oauth2/login", "/"),
want: "/oauth2/login?redirect-encoded=" + urlpkg.RedirectEncoded("/"),
},
{
name: "login with root referer on non-default ingress",
request: httpRequest("/domene/oauth2/login", "/"),
ingress: "https://test.nav.no/domene",
want: "/domene/oauth2/login?redirect-encoded=" + urlpkg.RedirectEncoded("/"),
},
{
name: "login with cookie referer",
request: httpRequest("/oauth2/login"),
request: get("/oauth2/login"),
loginCookie: &openid.LoginCookie{Referer: "/"},
want: "/oauth2/login?redirect-encoded=" + urlpkg.RedirectEncoded("/"),
},
{
name: "login with empty cookie referer",
request: httpRequest("/oauth2/login"),
request: get("/oauth2/login"),
loginCookie: &openid.LoginCookie{Referer: ""},
want: "/oauth2/login?redirect-encoded=" + urlpkg.RedirectEncoded("/"),
},
{
name: "login with cookie referer takes precedence over referer header",
request: httpRequest("/oauth2/login", "/api/me"),
loginCookie: &openid.LoginCookie{Referer: "/api/headers"},
want: "/oauth2/login?redirect-encoded=" + urlpkg.RedirectEncoded("/api/headers"),
},
{
name: "login with cookie referer on non-default ingress",
request: httpRequest("/domene/oauth2/login"),
request: get("/domene/oauth2/login"),
loginCookie: &openid.LoginCookie{Referer: "/domene/api/me"},
ingress: "https://test.nav.no/domene",
want: "/domene/oauth2/login?redirect-encoded=" + urlpkg.RedirectEncoded("/domene/api/me"),
},
{
name: "login with redirect parameter set",
request: httpRequest("/oauth2/login?redirect=/api/me"),
request: get("/oauth2/login?redirect=/api/me"),
want: "/oauth2/login?redirect-encoded=" + urlpkg.RedirectEncoded("/api/me"),
},
{
name: "login with redirect parameter set and query parameters",
request: httpRequest("/oauth2/login?redirect=/api/me?a=b%26c=d"),
request: get("/oauth2/login?redirect=/api/me?a=b%26c=d"),
want: "/oauth2/login?redirect-encoded=" + urlpkg.RedirectEncoded("/api/me?a=b&c=d"),
},
{
name: "login with redirect parameter set on non-default ingress",
request: httpRequest("/domene/oauth2/login?redirect=/api/me"),
request: get("/domene/oauth2/login?redirect=/api/me"),
ingress: "https://test.nav.no/domene",
want: "/domene/oauth2/login?redirect-encoded=" + urlpkg.RedirectEncoded("/api/me"),
},
{
name: "login with redirect parameter set takes precedence over referer header",
request: httpRequest("/oauth2/login?redirect=/other", "/api/me"),
want: "/oauth2/login?redirect-encoded=" + urlpkg.RedirectEncoded("/other"),
},
{
name: "login with redirect parameter set to relative root takes precedence over referer header",
request: httpRequest("/oauth2/login?redirect=/", "/api/me"),
want: "/oauth2/login?redirect-encoded=" + urlpkg.RedirectEncoded("/"),
},
{
name: "login with redirect parameter set to relative root on non-default ingress takes precedence over referer header",
request: httpRequest("/domene/oauth2/login?redirect=/", "/api/me"),
name: "login with redirect parameter set to relative root on non-default ingress",
request: get("/domene/oauth2/login?redirect=/"),
ingress: "https://test.nav.no/domene",
want: "/domene/oauth2/login?redirect-encoded=" + urlpkg.RedirectEncoded("/"),
},
{
name: "login with redirect parameter set to absolute url takes precedence over referer header",
request: httpRequest("/oauth2/login?redirect=http://localhost:8080", "/api/me"),
name: "login with redirect parameter set to absolute url",
request: get("/oauth2/login?redirect=http://localhost:8080"),
want: "/oauth2/login?redirect-encoded=" + urlpkg.RedirectEncoded("/"),
},
{
name: "login with redirect parameter set to absolute url with trailing slash takes precedence over referer header",
request: httpRequest("/oauth2/login?redirect=http://localhost:8080/", "/api/me"),
name: "login with redirect parameter set to absolute url with trailing slash",
request: get("/oauth2/login?redirect=http://localhost:8080/"),
want: "/oauth2/login?redirect-encoded=" + urlpkg.RedirectEncoded("/"),
},
{
name: "login with redirect parameter set to absolute url on non-default ingress takes precedence over referer header",
request: httpRequest("/domene/oauth2/login?redirect=http://localhost:8080/", "/api/me"),
name: "login with redirect parameter set to absolute url on non-default ingress",
request: get("/domene/oauth2/login?redirect=http://localhost:8080/"),
ingress: "https://test.nav.no/domene",
want: "/domene/oauth2/login?redirect-encoded=" + urlpkg.RedirectEncoded("/"),
},
{
name: "login with cookie referer takes precedence over redirect parameter",
request: httpRequest("/oauth2/login?redirect=/other"),
request: get("/oauth2/login?redirect=/other"),
loginCookie: &openid.LoginCookie{Referer: "/domene/api/me"},
want: "/oauth2/login?redirect-encoded=" + urlpkg.RedirectEncoded("/domene/api/me"),
},

10
pkg/handler/url/url.go
View File

@@ -26,19 +26,13 @@ func CanonicalRedirect(r *http.Request) string {
// 1. Default
redirect := ingressPath
// 2. Referer header is set
referer := r.Referer()
if len(referer) > 0 {
redirect = referer
}
// 3. Redirect parameter is set
// 2. Redirect parameter is set
redirectParam := r.URL.Query().Get(RedirectURLParameter)
if len(redirectParam) > 0 {
redirect = redirectParam
}
// 4. Redirect-encoded parameter is set
// 3. Redirect-encoded parameter is set
redirectEncoded := RedirectDecoded(r)
if len(redirectEncoded) > 0 {
redirect = redirectEncoded

35
pkg/handler/url/url_test.go
View File

@@ -60,41 +60,6 @@ func TestCanonicalRedirect(t *testing.T) {
r := httptest.NewRequest(http.MethodGet, defaultIngress+"/oauth2/login", nil)
r = mw.RequestWithPath(r, "/some-path")
// HTTP Referer header is 2nd priority
t.Run("Referer header is set", func(t *testing.T) {
for _, test := range []struct {
name string
value string
expected string
}{
{
name: "full URL",
value: "http://localhost:8080/foo/bar/baz",
expected: "/foo/bar/baz",
},
{
name: "full URL with query parameters",
value: "http://localhost:8080/foo/bar/baz?gnu=notunix",
expected: "/foo/bar/baz?gnu=notunix",
},
{
name: "absolute path",
value: "/foo/bar/baz",
expected: "/foo/bar/baz",
},
{
name: "absolute path with query parameters",
value: "/foo/bar/baz?gnu=notunix",
expected: "/foo/bar/baz?gnu=notunix",
},
} {
t.Run(test.name, func(t *testing.T) {
r.Header.Set("Referer", test.value)
assert.Equal(t, test.expected, urlpkg.CanonicalRedirect(r))
})
}
})
// If either redirect or redirect-encoded parameter is set, use that
t.Run("redirect parameter is set", func(t *testing.T) {
for _, test := range []struct {