Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add return_to parameters to the createLogout handler #3336

Merged
merged 5 commits into from Jun 29, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 18 additions & 0 deletions internal/client-go/api_frontend.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

18 changes: 18 additions & 0 deletions internal/httpclient/api_frontend.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

40 changes: 37 additions & 3 deletions selfservice/flow/logout/handler.go
Expand Up @@ -103,6 +103,14 @@ type createBrowserLogoutFlow struct {
// in: header
// name: cookie
Cookie string `json:"cookie"`

// Return to URL
//
// The URL to which the browser should be redirected to after the logout
// has been performed.
//
// in: query
ReturnTo string `json:"return_to"`
}

// swagger:route GET /self-service/logout/browser frontend createBrowserLogoutFlow
Expand All @@ -127,19 +135,45 @@ type createBrowserLogoutFlow struct {
//
// Responses:
// 200: logoutFlow
// 400: errorGeneric
// 401: errorGeneric
// 500: errorGeneric
func (h *Handler) createBrowserLogoutFlow(w http.ResponseWriter, r *http.Request, ps httprouter.Params) {
sess, err := h.d.SessionManager().FetchFromRequest(r.Context(), r)
if err != nil {
h.d.Writer().WriteError(w, r, err)
h.d.SelfServiceErrorManager().Forward(r.Context(), w, r, err)
return
}

conf := h.d.Config()

requestURL := x.RequestURL(r)

var returnTo *url.URL

if requestURL.Query().Get("return_to") != "" {
// Pre-validate the return to URL which is contained in the HTTP request.
returnTo, err = x.SecureRedirectTo(r,
h.d.Config().SelfServiceFlowLogoutRedirectURL(r.Context()),
x.SecureRedirectUseSourceURL(requestURL.String()),
x.SecureRedirectAllowURLs(conf.SelfServiceBrowserAllowedReturnToDomains(r.Context())),
x.SecureRedirectAllowSelfServiceURLs(conf.SelfPublicURL(r.Context())),
)
if err != nil {
h.d.SelfServiceErrorManager().Forward(r.Context(), w, r, err)
return
}
}

params := url.Values{"token": {sess.LogoutToken}}

if returnTo != nil {
params.Set("return_to", returnTo.String())
}

h.d.Writer().Write(w, r, &logoutFlow{
LogoutToken: sess.LogoutToken,
LogoutURL: urlx.CopyWithQuery(urlx.AppendPaths(h.d.Config().SelfPublicURL(r.Context()), RouteSubmitFlow),
url.Values{"token": {sess.LogoutToken}}).String(),
LogoutURL: urlx.CopyWithQuery(urlx.AppendPaths(h.d.Config().SelfPublicURL(r.Context()), RouteSubmitFlow), params).String(),
})
}

Expand Down
76 changes: 64 additions & 12 deletions selfservice/flow/logout/handler_test.go
Expand Up @@ -7,6 +7,7 @@ import (
"context"
"encoding/json"
"fmt"
"io"
"net/http"
"net/http/cookiejar"
"net/url"
Expand Down Expand Up @@ -86,15 +87,27 @@ func TestLogout(t *testing.T) {
return x.MustReadAll(res.Body), res
}

getLogoutUrl := func(t *testing.T) (hc *http.Client, logoutUrl string) {
getLogoutUrl := func(t *testing.T, params url.Values) (hc *http.Client, logoutUrl string) {
hc = testhelpers.NewSessionClient(t, public.URL+"/session/browser/set")

body, res := testhelpers.HTTPRequestJSON(t, hc, "GET", public.URL+"/self-service/logout/browser", nil)
u, err := url.Parse(public.URL)
require.NoError(t, err)
u.Path = "/self-service/logout/browser"
if params != nil {
u.RawQuery = params.Encode()
}

body, res := testhelpers.HTTPRequestJSON(t, hc, "GET", u.String(), nil)
assert.EqualValues(t, http.StatusOK, res.StatusCode)

logoutUrl = gjson.GetBytes(body, "logout_url").String()
logoutToken := gjson.GetBytes(body, "logout_token").String()
assert.Contains(t, logoutUrl, public.URL+"/self-service/logout?token="+logoutToken, "%s", body)

logoutUrlParsed, err := url.Parse(logoutUrl)
require.NoError(t, err)

assert.Equalf(t, logoutUrlParsed.Query().Get("token"), logoutToken, "%s", body)
assert.Equalf(t, logoutUrlParsed.Path, "/self-service/logout", "%s", body)
return
}

Expand All @@ -112,7 +125,7 @@ func TestLogout(t *testing.T) {
}

t.Run("type=browser", func(t *testing.T) {
hc, logoutUrl := getLogoutUrl(t)
hc, logoutUrl := getLogoutUrl(t, nil)
originalCookies := hc.Jar.Cookies(urlx.ParseOrPanic(public.URL))
require.Contains(t, fmt.Sprintf("%v", hc.Jar.Cookies(urlx.ParseOrPanic(public.URL))), "ory_kratos_session")

Expand All @@ -128,7 +141,7 @@ func TestLogout(t *testing.T) {
})

t.Run("type=ajax", func(t *testing.T) {
hc, logoutUrl := getLogoutUrl(t)
hc, logoutUrl := getLogoutUrl(t, nil)
originalCookies := hc.Jar.Cookies(urlx.ParseOrPanic(public.URL))
require.Contains(t, fmt.Sprintf("%v", hc.Jar.Cookies(urlx.ParseOrPanic(public.URL))), "ory_kratos_session")

Expand Down Expand Up @@ -163,7 +176,7 @@ func TestLogout(t *testing.T) {

t.Run("case=calling submission with token but without session", func(t *testing.T) {
t.Run("type=browser", func(t *testing.T) {
_, logoutUrl := getLogoutUrl(t)
_, logoutUrl := getLogoutUrl(t, nil)

body, res := makeBrowserLogout(t, http.DefaultClient, logoutUrl)
assert.Contains(t, res.Request.URL.String(), errTS.URL)
Expand All @@ -172,7 +185,7 @@ func TestLogout(t *testing.T) {
})

t.Run("type=ajax", func(t *testing.T) {
_, logoutUrl := getLogoutUrl(t)
_, logoutUrl := getLogoutUrl(t, nil)

body, res := testhelpers.HTTPRequestJSON(t, http.DefaultClient, "GET", logoutUrl, nil)
assert.EqualValues(t, logoutUrl, res.Request.URL.String())
Expand All @@ -184,7 +197,7 @@ func TestLogout(t *testing.T) {
t.Run("case=calling submission with token but with session from another user", func(t *testing.T) {
t.Run("type=browser", func(t *testing.T) {
otherUser := testhelpers.NewSessionClient(t, public.URL+"/session/browser/set")
_, logoutUrl := getLogoutUrl(t)
_, logoutUrl := getLogoutUrl(t, nil)

body, res := makeBrowserLogout(t, otherUser, logoutUrl)
assert.Contains(t, res.Request.URL.String(), errTS.URL)
Expand All @@ -194,7 +207,7 @@ func TestLogout(t *testing.T) {

t.Run("type=ajax", func(t *testing.T) {
otherUser := testhelpers.NewSessionClient(t, public.URL+"/session/browser/set")
_, logoutUrl := getLogoutUrl(t)
_, logoutUrl := getLogoutUrl(t, nil)

body, res := testhelpers.HTTPRequestJSON(t, otherUser, "GET", logoutUrl, nil)
assert.EqualValues(t, logoutUrl, res.Request.URL.String())
Expand All @@ -205,7 +218,7 @@ func TestLogout(t *testing.T) {

t.Run("case=calling submission with invalid token", func(t *testing.T) {
t.Run("type=browser", func(t *testing.T) {
hc, logoutUrl := getLogoutUrl(t)
hc, logoutUrl := getLogoutUrl(t, nil)

body, res := makeBrowserLogout(t, hc, logoutUrl+"invalid")
assert.Contains(t, res.Request.URL.String(), errTS.URL)
Expand All @@ -214,7 +227,7 @@ func TestLogout(t *testing.T) {
})

t.Run("type=ajax", func(t *testing.T) {
hc, logoutUrl := getLogoutUrl(t)
hc, logoutUrl := getLogoutUrl(t, nil)

body, res := testhelpers.HTTPRequestJSON(t, hc, "GET", logoutUrl+"invalid", nil)
assert.EqualValues(t, logoutUrl+"invalid", res.Request.URL.String())
Expand All @@ -231,7 +244,7 @@ func TestLogout(t *testing.T) {

t.Run("case=init logout through browser does 303 redirect", func(t *testing.T) {
// init the logout
hc, logoutUrl := getLogoutUrl(t)
hc, logoutUrl := getLogoutUrl(t, nil)
// prevent the redirect, so we can get check the status code
hc.CheckRedirect = func(req *http.Request, via []*http.Request) error {
return http.ErrUseLastResponse
Expand All @@ -246,4 +259,43 @@ func TestLogout(t *testing.T) {
// here we check that the redirect status is 303
require.Equal(t, http.StatusSeeOther, res.StatusCode)
})

t.Run("case=init logout with return_to should carry over return_to", func(t *testing.T) {
reg.Config().MustSet(context.Background(), config.ViperKeyURLsAllowedReturnToDomains, []string{"https://www.ory.sh"})

hc, logoutUrl := getLogoutUrl(t, url.Values{"return_to": {"https://www.ory.sh"}})

logoutUrlParsed, err := url.Parse(logoutUrl)
require.NoError(t, err)

assert.Equal(t, "https://www.ory.sh", logoutUrlParsed.Query().Get("return_to"))

hc.CheckRedirect = func(req *http.Request, via []*http.Request) error {
return http.ErrUseLastResponse
}

body, res := makeBrowserLogout(t, hc, logoutUrl)
assert.EqualValues(t, http.StatusSeeOther, res.StatusCode, "%s", body)
assert.EqualValues(t, "https://www.ory.sh", res.Header.Get("Location"))
})

t.Run("case=init logout with return_to should not carry over return_to if not allowed", func(t *testing.T) {
hc := testhelpers.NewSessionClient(t, public.URL+"/session/browser/set")

logoutUrl := public.URL + "/self-service/logout/browser?return_to=https://www.ory.com"

r, err := http.NewRequest("GET", logoutUrl, nil)
require.NoError(t, err)
r.Header.Set("Accept", "application/json")

resp, err := hc.Do(r)
require.NoError(t, err)
defer resp.Body.Close()

body, err := io.ReadAll(resp.Body)
require.NoError(t, err)

assert.EqualValues(t, "Requested return_to URL \"https://www.ory.com\" is not allowed.", gjson.GetBytes(body, "error.reason").String(), "%s", body)
assert.EqualValues(t, http.StatusBadRequest, resp.StatusCode, "%s", body)
})
}
18 changes: 18 additions & 0 deletions spec/api.json
Expand Up @@ -5138,6 +5138,14 @@
"schema": {
"type": "string"
}
},
{
"description": "Return to URL\n\nThe URL to which the browser should be redirected to after the logout\nhas been performed.",
"in": "query",
"name": "return_to",
"schema": {
"type": "string"
}
}
],
"responses": {
Expand All @@ -5151,6 +5159,16 @@
},
"description": "logoutFlow"
},
"400": {
"content": {
"application/json": {
"schema": {
"$ref": "#/components/schemas/errorGeneric"
}
}
},
"description": "errorGeneric"
},
"401": {
"content": {
"application/json": {
Expand Down
12 changes: 12 additions & 0 deletions spec/swagger.json
Expand Up @@ -1762,6 +1762,12 @@
"description": "HTTP Cookies\n\nIf you call this endpoint from a backend, please include the\noriginal Cookie header in the request.",
"name": "cookie",
"in": "header"
},
{
"type": "string",
"description": "Return to URL\n\nThe URL to which the browser should be redirected to after the logout\nhas been performed.",
"name": "return_to",
"in": "query"
}
],
"responses": {
Expand All @@ -1771,6 +1777,12 @@
"$ref": "#/definitions/logoutFlow"
}
},
"400": {
"description": "errorGeneric",
"schema": {
"$ref": "#/definitions/errorGeneric"
}
},
"401": {
"description": "errorGeneric",
"schema": {
Expand Down