Skip to content

Commit

Permalink
feat: add return_to parameters to the createLogout handler (#3336)
Browse files Browse the repository at this point in the history 
* feat: add return_to parameters to the `createLogout` handler

* test: logout take over return_to from create to update

* test(e2e): logout return to

* test(e2e): logout return to

* test: logout return_to isnt applicable to react
  • Loading branch information
@Benehiko
Benehiko committed Jun 29, 2023
1 parent c48f20e commit 08fed36
Show file tree
Hide file tree
Showing 11 changed files with 231 additions and 23 deletions.
18 changes: 18 additions & 0 deletions internal/client-go/api_frontend.go
View file

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
View file

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
View file
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
View file
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
View file
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
View file
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

0 comments on commit 08fed36

Please sign in to comment.