diff --git a/internal/client-go/api_frontend.go b/internal/client-go/api_frontend.go index 09fea766eb0..e75bab098fa 100644 --- a/internal/client-go/api_frontend.go +++ b/internal/client-go/api_frontend.go @@ -1121,12 +1121,17 @@ type FrontendApiApiCreateBrowserLogoutFlowRequest struct { ctx context.Context ApiService FrontendApi cookie *string + returnTo *string } func (r FrontendApiApiCreateBrowserLogoutFlowRequest) Cookie(cookie string) FrontendApiApiCreateBrowserLogoutFlowRequest { r.cookie = &cookie return r } +func (r FrontendApiApiCreateBrowserLogoutFlowRequest) ReturnTo(returnTo string) FrontendApiApiCreateBrowserLogoutFlowRequest { + r.returnTo = &returnTo + return r +} func (r FrontendApiApiCreateBrowserLogoutFlowRequest) Execute() (*LogoutFlow, *http.Response, error) { return r.ApiService.CreateBrowserLogoutFlowExecute(r) @@ -1179,6 +1184,9 @@ func (a *FrontendApiService) CreateBrowserLogoutFlowExecute(r FrontendApiApiCrea localVarQueryParams := url.Values{} localVarFormParams := url.Values{} + if r.returnTo != nil { + localVarQueryParams.Add("return_to", parameterToString(*r.returnTo, "")) + } // to determine the Content-Type header localVarHTTPContentTypes := []string{} @@ -1221,6 +1229,16 @@ func (a *FrontendApiService) CreateBrowserLogoutFlowExecute(r FrontendApiApiCrea body: localVarBody, error: localVarHTTPResponse.Status, } + if localVarHTTPResponse.StatusCode == 400 { + var v ErrorGeneric + err = a.client.decode(&v, localVarBody, localVarHTTPResponse.Header.Get("Content-Type")) + if err != nil { + newErr.error = err.Error() + return localVarReturnValue, localVarHTTPResponse, newErr + } + newErr.model = v + return localVarReturnValue, localVarHTTPResponse, newErr + } if localVarHTTPResponse.StatusCode == 401 { var v ErrorGeneric err = a.client.decode(&v, localVarBody, localVarHTTPResponse.Header.Get("Content-Type")) diff --git a/internal/httpclient/api_frontend.go b/internal/httpclient/api_frontend.go index 09fea766eb0..e75bab098fa 100644 --- a/internal/httpclient/api_frontend.go +++ b/internal/httpclient/api_frontend.go @@ -1121,12 +1121,17 @@ type FrontendApiApiCreateBrowserLogoutFlowRequest struct { ctx context.Context ApiService FrontendApi cookie *string + returnTo *string } func (r FrontendApiApiCreateBrowserLogoutFlowRequest) Cookie(cookie string) FrontendApiApiCreateBrowserLogoutFlowRequest { r.cookie = &cookie return r } +func (r FrontendApiApiCreateBrowserLogoutFlowRequest) ReturnTo(returnTo string) FrontendApiApiCreateBrowserLogoutFlowRequest { + r.returnTo = &returnTo + return r +} func (r FrontendApiApiCreateBrowserLogoutFlowRequest) Execute() (*LogoutFlow, *http.Response, error) { return r.ApiService.CreateBrowserLogoutFlowExecute(r) @@ -1179,6 +1184,9 @@ func (a *FrontendApiService) CreateBrowserLogoutFlowExecute(r FrontendApiApiCrea localVarQueryParams := url.Values{} localVarFormParams := url.Values{} + if r.returnTo != nil { + localVarQueryParams.Add("return_to", parameterToString(*r.returnTo, "")) + } // to determine the Content-Type header localVarHTTPContentTypes := []string{} @@ -1221,6 +1229,16 @@ func (a *FrontendApiService) CreateBrowserLogoutFlowExecute(r FrontendApiApiCrea body: localVarBody, error: localVarHTTPResponse.Status, } + if localVarHTTPResponse.StatusCode == 400 { + var v ErrorGeneric + err = a.client.decode(&v, localVarBody, localVarHTTPResponse.Header.Get("Content-Type")) + if err != nil { + newErr.error = err.Error() + return localVarReturnValue, localVarHTTPResponse, newErr + } + newErr.model = v + return localVarReturnValue, localVarHTTPResponse, newErr + } if localVarHTTPResponse.StatusCode == 401 { var v ErrorGeneric err = a.client.decode(&v, localVarBody, localVarHTTPResponse.Header.Get("Content-Type")) diff --git a/selfservice/flow/logout/handler.go b/selfservice/flow/logout/handler.go index 6b78a5cb565..077855b1b14 100644 --- a/selfservice/flow/logout/handler.go +++ b/selfservice/flow/logout/handler.go @@ -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 @@ -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(), }) } diff --git a/selfservice/flow/logout/handler_test.go b/selfservice/flow/logout/handler_test.go index d1d31f3da99..ebc3ef66fe4 100644 --- a/selfservice/flow/logout/handler_test.go +++ b/selfservice/flow/logout/handler_test.go @@ -7,6 +7,7 @@ import ( "context" "encoding/json" "fmt" + "io" "net/http" "net/http/cookiejar" "net/url" @@ -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 } @@ -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") @@ -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") @@ -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) @@ -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()) @@ -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) @@ -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()) @@ -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) @@ -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()) @@ -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 @@ -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) + }) } diff --git a/spec/api.json b/spec/api.json index 90334abaa97..abfe9960809 100755 --- a/spec/api.json +++ b/spec/api.json @@ -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": { @@ -5151,6 +5159,16 @@ }, "description": "logoutFlow" }, + "400": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/errorGeneric" + } + } + }, + "description": "errorGeneric" + }, "401": { "content": { "application/json": { diff --git a/spec/swagger.json b/spec/swagger.json index c8467e49c06..d5f9a3d1fac 100755 --- a/spec/swagger.json +++ b/spec/swagger.json @@ -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": { @@ -1771,6 +1777,12 @@ "$ref": "#/definitions/logoutFlow" } }, + "400": { + "description": "errorGeneric", + "schema": { + "$ref": "#/definitions/errorGeneric" + } + }, "401": { "description": "errorGeneric", "schema": { diff --git a/test/e2e/cypress/integration/profiles/email/logout/success.spec.ts b/test/e2e/cypress/integration/profiles/email/logout/success.spec.ts index 512edd4b708..ca965c2d0c2 100644 --- a/test/e2e/cypress/integration/profiles/email/logout/success.spec.ts +++ b/test/e2e/cypress/integration/profiles/email/logout/success.spec.ts @@ -11,20 +11,25 @@ context("Testing logout flows", () => { route: express.login, app: "express" as "express", profile: "email", + settings: express.settings, }, { route: react.login, app: "react" as "react", profile: "spa", + settings: react.settings, }, - ].forEach(({ route, profile, app }) => { + ].forEach(({ route, profile, app, settings }) => { describe(`for app ${app}`, () => { - const email = gen.email() - const password = gen.password() + let email: string + let password: string before(() => { cy.proxy(app) + email = gen.email() + password = gen.password() + cy.useConfigProfile(profile) cy.registerApi({ email, @@ -55,6 +60,39 @@ context("Testing logout flows", () => { cy.noSession() cy.url().should("include", "/login") }) + + it("should be able to sign out at 2fa page", () => { + if (app === "react") { + return + } + cy.useLookupSecrets(true) + cy.sessionRequires2fa() + cy.getSession({ expectAal: "aal1" }) + cy.getCookie("ory_kratos_session").should("not.be.null") + + // add 2fa to account + cy.visit(settings) + cy.get( + appPrefix(app) + 'button[name="lookup_secret_regenerate"]', + ).click() + cy.get('button[name="lookup_secret_confirm"]').click() + cy.expectSettingsSaved() + + cy.logout() + cy.visit(route + "?return_to=https://www.ory.sh") + + cy.get('[name="identifier"]').clear().type(email) + + cy.reauth({ + expect: { email, success: false }, + type: { password: password }, + }) + + cy.get("a[href*='logout']").click() + + cy.location("host").should("eq", "www.ory.sh") + cy.useLookupSecrets(false) + }) }) }) }) diff --git a/test/e2e/cypress/integration/profiles/mfa/lookup.spec.ts b/test/e2e/cypress/integration/profiles/mfa/lookup.spec.ts index 7619aabeaff..40e6c7e7984 100644 --- a/test/e2e/cypress/integration/profiles/mfa/lookup.spec.ts +++ b/test/e2e/cypress/integration/profiles/mfa/lookup.spec.ts @@ -95,7 +95,7 @@ context("2FA lookup secrets", () => { "not.be.empty", ) - let codes + let codes: string[] cy.getLookupSecrets().should((c) => { codes = c }) diff --git a/test/e2e/cypress/integration/profiles/mfa/mix.spec.ts b/test/e2e/cypress/integration/profiles/mfa/mix.spec.ts index 98fe7ba55f8..026d4491a8c 100644 --- a/test/e2e/cypress/integration/profiles/mfa/mix.spec.ts +++ b/test/e2e/cypress/integration/profiles/mfa/mix.spec.ts @@ -77,7 +77,7 @@ context("2FA with various methods", () => { cy.visit(settings) // Set up TOTP - let secret + let secret: string cy.get( appPrefix(app) + '[data-testid="node/text/totp_secret_key/text"]', ).then(($e) => { @@ -96,7 +96,7 @@ context("2FA with various methods", () => { // Set up lookup secrets cy.visit(settings) cy.get('[name="lookup_secret_regenerate"]').click() - let codes + let codes: string[] cy.getLookupSecrets().then((c) => { codes = c }) diff --git a/test/e2e/cypress/support/commands.ts b/test/e2e/cypress/support/commands.ts index 2b0d138dd14..91622e42dd2 100644 --- a/test/e2e/cypress/support/commands.ts +++ b/test/e2e/cypress/support/commands.ts @@ -1147,6 +1147,18 @@ Cypress.Commands.add("useVerificationStrategy", (strategy: Strategy) => { }) }) +Cypress.Commands.add("useLookupSecrets", (value: boolean) => { + cy.updateConfigFile((config) => { + config.selfservice.methods = { + ...config.selfservice.methods, + lookup_secret: { + enabled: value, + }, + } + return config + }) +}) + Cypress.Commands.add("getLookupSecrets", () => cy .get('[data-testid="node/text/lookup_secret_codes/text"] code') diff --git a/test/e2e/cypress/support/index.d.ts b/test/e2e/cypress/support/index.d.ts index 6966aa8b477..c9a52a0c7e8 100644 --- a/test/e2e/cypress/support/index.d.ts +++ b/test/e2e/cypress/support/index.d.ts @@ -262,10 +262,16 @@ declare global { * @param opts */ reauth(opts: { - expect: { email; success?: boolean } + expect: { email: string; success?: boolean } type: { email?: string; password?: string } }): Chainable + /** + * Change the config file to support lookup secrets + * @param value + */ + useLookupSecrets(value: boolean): Chainable + /** * Re-authenticates a user. * @@ -273,7 +279,7 @@ declare global { */ reauthWithOtherAccount(opts: { previousUrl: string - expect: { email; success?: boolean } + expect: { email: string; success?: boolean } type: { email?: string; password?: string } }): Chainable