From 350252fab3d3f1c7ca087f1850f83a555d1c2d7c Mon Sep 17 00:00:00 2001 From: Alano Terblanche <18033717+Benehiko@users.noreply.github.com> Date: Wed, 21 Jun 2023 16:49:41 +0200 Subject: [PATCH 1/5] feat: add return_to parameters to the `createLogout` handler --- selfservice/flow/logout/handler.go | 35 +++++++++++++++++++++++++++--- 1 file changed, 32 insertions(+), 3 deletions(-) diff --git a/selfservice/flow/logout/handler.go b/selfservice/flow/logout/handler.go index 6b78a5cb5652..61d370e3c8a6 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 @@ -132,14 +140,35 @@ type createBrowserLogoutFlow struct { 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).String() + + // Pre-validate the return to URL which is contained in the HTTP request. + returnTo, err := x.SecureRedirectTo(r, + conf.SelfServiceBrowserDefaultReturnTo(r.Context()), + x.SecureRedirectUseSourceURL(requestURL), + 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(), }) } From 7866cebc1d2c5191d430876b1d17d51d4d8abf7d Mon Sep 17 00:00:00 2001 From: Alano Terblanche <18033717+Benehiko@users.noreply.github.com> Date: Thu, 22 Jun 2023 18:27:21 +0200 Subject: [PATCH 2/5] test: logout take over return_to from create to update --- selfservice/flow/logout/handler.go | 27 +++++---- selfservice/flow/logout/handler_test.go | 76 +++++++++++++++++++++---- 2 files changed, 80 insertions(+), 23 deletions(-) diff --git a/selfservice/flow/logout/handler.go b/selfservice/flow/logout/handler.go index 61d370e3c8a6..077855b1b14d 100644 --- a/selfservice/flow/logout/handler.go +++ b/selfservice/flow/logout/handler.go @@ -135,6 +135,7 @@ 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) { @@ -146,18 +147,22 @@ func (h *Handler) createBrowserLogoutFlow(w http.ResponseWriter, r *http.Request conf := h.d.Config() - requestURL := x.RequestURL(r).String() + requestURL := x.RequestURL(r) - // Pre-validate the return to URL which is contained in the HTTP request. - returnTo, err := x.SecureRedirectTo(r, - conf.SelfServiceBrowserDefaultReturnTo(r.Context()), - x.SecureRedirectUseSourceURL(requestURL), - 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 + 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}} diff --git a/selfservice/flow/logout/handler_test.go b/selfservice/flow/logout/handler_test.go index d1d31f3da995..ebc3ef66fe45 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) + }) } From 2ef5f6e4160bc71fd62a0070316bdc5657174c19 Mon Sep 17 00:00:00 2001 From: Alano Terblanche <18033717+Benehiko@users.noreply.github.com> Date: Fri, 23 Jun 2023 15:17:47 +0200 Subject: [PATCH 3/5] test(e2e): logout return to --- internal/client-go/api_frontend.go | 18 ++ internal/httpclient/api_frontend.go | 18 ++ spec/api.json | 18 ++ spec/swagger.json | 12 + .../profiles/email/logout/success.spec.ts | 28 +- .../integration/profiles/mfa/lookup.spec.ts | 2 +- .../integration/profiles/mfa/mix.spec.ts | 296 +++++++++--------- 7 files changed, 242 insertions(+), 150 deletions(-) diff --git a/internal/client-go/api_frontend.go b/internal/client-go/api_frontend.go index 09fea766eb02..e75bab098fa7 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 09fea766eb02..e75bab098fa7 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/spec/api.json b/spec/api.json index 90334abaa976..abfe99608099 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 c8467e49c068..d5f9a3d1fac7 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 512edd4b708e..70a4ac9eb84e 100644 --- a/test/e2e/cypress/integration/profiles/email/logout/success.spec.ts +++ b/test/e2e/cypress/integration/profiles/email/logout/success.spec.ts @@ -11,13 +11,15 @@ 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() @@ -56,5 +58,29 @@ context("Testing logout flows", () => { cy.url().should("include", "/login") }) }) + + it("should be able to sign out at 2fa page", () => { + cy.sessionRequires2fa() + cy.getSession({ expectAal: "aal1" }) + + // 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.reauth({ + expect: { email }, + type: { email: email, password: password }, + }) + + cy.get("href*=/logout").should("have.", "https://www.ory.sh") + cy.get("href*=/logout").click() + + cy.location("host").should("eq", "www.ory.sh") + }) }) }) diff --git a/test/e2e/cypress/integration/profiles/mfa/lookup.spec.ts b/test/e2e/cypress/integration/profiles/mfa/lookup.spec.ts index 7619aabeaff0..40e6c7e7984a 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 98fe7ba55f81..02e48b56c670 100644 --- a/test/e2e/cypress/integration/profiles/mfa/mix.spec.ts +++ b/test/e2e/cypress/integration/profiles/mfa/mix.spec.ts @@ -10,171 +10,171 @@ context("2FA with various methods", () => { beforeEach(() => { cy.task("resetCRI", {}) }) - ;[ - { - login: react.login, - settings: react.settings, - base: react.base, - app: "react" as "react", - profile: "spa", - }, - { - login: express.login, - settings: express.settings, - base: express.base, - app: "express" as "express", - profile: "mfa", - }, - ].forEach(({ settings, login, profile, app, base }) => { - describe(`for app ${app}`, () => { - before(() => { - cy.useConfigProfile(profile) - cy.proxy(app) - }) - let email = gen.email() - let password = gen.password() - - beforeEach(() => { - cy.clearAllCookies() - email = gen.email() - password = gen.password() - cy.registerApi({ - email, - password, - fields: { "traits.website": website }, + ;[ + { + login: react.login, + settings: react.settings, + base: react.base, + app: "react" as "react", + profile: "spa", + }, + { + login: express.login, + settings: express.settings, + base: express.base, + app: "express" as "express", + profile: "mfa", + }, + ].forEach(({ settings, login, profile, app, base }) => { + describe(`for app ${app}`, () => { + before(() => { + cy.useConfigProfile(profile) + cy.proxy(app) }) - cy.clearAllCookies() - cy.login({ email, password, cookieUrl: base }) - cy.longPrivilegedSessionTime() - cy.task("sendCRI", { - query: "WebAuthn.disable", - opts: {}, + let email = gen.email() + let password = gen.password() + + beforeEach(() => { + cy.clearAllCookies() + email = gen.email() + password = gen.password() + cy.registerApi({ + email, + password, + fields: { "traits.website": website }, + }) + cy.clearAllCookies() + cy.login({ email, password, cookieUrl: base }) + cy.longPrivilegedSessionTime() + cy.task("sendCRI", { + query: "WebAuthn.disable", + opts: {}, + }) }) - }) - it("should set up an use all mfa combinations", () => { - cy.visit(settings) - cy.task("sendCRI", { - query: "WebAuthn.enable", - opts: {}, - }).then(() => { + it("should set up an use all mfa combinations", () => { + cy.visit(settings) cy.task("sendCRI", { - query: "WebAuthn.addVirtualAuthenticator", - opts: { - options: { - protocol: "ctap2", - transport: "usb", - hasResidentKey: true, - hasUserVerification: true, - isUserVerified: true, - }, - }, + query: "WebAuthn.enable", + opts: {}, }).then(() => { - cy.getSession({ - expectAal: "aal1", - expectMethods: ["password"], - }) + cy.task("sendCRI", { + query: "WebAuthn.addVirtualAuthenticator", + opts: { + options: { + protocol: "ctap2", + transport: "usb", + hasResidentKey: true, + hasUserVerification: true, + isUserVerified: true, + }, + }, + }).then(() => { + cy.getSession({ + expectAal: "aal1", + expectMethods: ["password"], + }) - cy.visit(settings) - // Set up TOTP - let secret - cy.get( - appPrefix(app) + '[data-testid="node/text/totp_secret_key/text"]', - ).then(($e) => { - secret = $e.text().trim() - }) - cy.get('[name="totp_code"]').then(($e) => { - cy.wrap($e).type(authenticator.generate(secret)) - }) - cy.get('[name="method"][value="totp"]').click() - cy.expectSettingsSaved() - cy.getSession({ - expectAal: "aal2", - expectMethods: ["password", "totp"], - }) + cy.visit(settings) + // Set up TOTP + let secret: string + cy.get( + appPrefix(app) + '[data-testid="node/text/totp_secret_key/text"]', + ).then(($e) => { + secret = $e.text().trim() + }) + cy.get('[name="totp_code"]').then(($e) => { + cy.wrap($e).type(authenticator.generate(secret)) + }) + cy.get('[name="method"][value="totp"]').click() + cy.expectSettingsSaved() + cy.getSession({ + expectAal: "aal2", + expectMethods: ["password", "totp"], + }) - // Set up lookup secrets - cy.visit(settings) - cy.get('[name="lookup_secret_regenerate"]').click() - let codes - cy.getLookupSecrets().then((c) => { - codes = c - }) - cy.get('[name="lookup_secret_confirm"]').click() - cy.expectSettingsSaved() - cy.getSession({ - expectAal: "aal2", - expectMethods: ["password", "totp", "lookup_secret"], - }) + // Set up lookup secrets + cy.visit(settings) + cy.get('[name="lookup_secret_regenerate"]').click() + let codes: string[] + cy.getLookupSecrets().then((c) => { + codes = c + }) + cy.get('[name="lookup_secret_confirm"]').click() + cy.expectSettingsSaved() + cy.getSession({ + expectAal: "aal2", + expectMethods: ["password", "totp", "lookup_secret"], + }) - // Set up WebAuthn - cy.visit(settings) - cy.get('[name="webauthn_register_displayname"]').type("my-key") - // We need a workaround here. So first we click, then we submit - cy.clickWebAuthButton("register") - cy.expectSettingsSaved() - cy.getSession({ - expectAal: "aal2", - expectMethods: ["password", "totp", "webauthn", "lookup_secret"], - }) + // Set up WebAuthn + cy.visit(settings) + cy.get('[name="webauthn_register_displayname"]').type("my-key") + // We need a workaround here. So first we click, then we submit + cy.clickWebAuthButton("register") + cy.expectSettingsSaved() + cy.getSession({ + expectAal: "aal2", + expectMethods: ["password", "totp", "webauthn", "lookup_secret"], + }) - cy.visit(login + "?aal=aal2&refresh=true") - cy.get('[name="totp_code"]').then(($e) => { - cy.wrap($e).type(authenticator.generate(secret)) - }) + cy.visit(login + "?aal=aal2&refresh=true") + cy.get('[name="totp_code"]').then(($e) => { + cy.wrap($e).type(authenticator.generate(secret)) + }) - cy.get('[name="method"][value="totp"]').click() - cy.location("pathname").should("not.include", "/login") + cy.get('[name="method"][value="totp"]').click() + cy.location("pathname").should("not.include", "/login") - cy.getSession({ - expectAal: "aal2", - expectMethods: [ - "password", - "totp", - "webauthn", - "lookup_secret", - "totp", - ], - }) + cy.getSession({ + expectAal: "aal2", + expectMethods: [ + "password", + "totp", + "webauthn", + "lookup_secret", + "totp", + ], + }) - // Use TOTP - cy.visit(login + "?aal=aal2&refresh=true") - cy.clickWebAuthButton("login") - cy.getSession({ - expectAal: "aal2", - expectMethods: [ - "password", - "totp", - "webauthn", - "lookup_secret", - "totp", - "webauthn", - ], - }) + // Use TOTP + cy.visit(login + "?aal=aal2&refresh=true") + cy.clickWebAuthButton("login") + cy.getSession({ + expectAal: "aal2", + expectMethods: [ + "password", + "totp", + "webauthn", + "lookup_secret", + "totp", + "webauthn", + ], + }) - // Use lookup - cy.visit(login + "?aal=aal2&refresh=true") - cy.get('[name="lookup_secret"]').then(($e) => { - cy.wrap($e).type(codes[1]) - }) - cy.get('[name="method"][value="lookup_secret"]').click() - cy.location("pathname").should("not.include", "/login") + // Use lookup + cy.visit(login + "?aal=aal2&refresh=true") + cy.get('[name="lookup_secret"]').then(($e) => { + cy.wrap($e).type(codes[1]) + }) + cy.get('[name="method"][value="lookup_secret"]').click() + cy.location("pathname").should("not.include", "/login") - cy.getSession({ - expectAal: "aal2", - expectMethods: [ - "password", - "totp", - "webauthn", - "lookup_secret", - "totp", - "webauthn", - "lookup_secret", - ], + cy.getSession({ + expectAal: "aal2", + expectMethods: [ + "password", + "totp", + "webauthn", + "lookup_secret", + "totp", + "webauthn", + "lookup_secret", + ], + }) }) }) }) }) }) - }) }) From f619dfb4b41e8e807c1b66359ec6fc53b807b1cf Mon Sep 17 00:00:00 2001 From: Alano Terblanche <18033717+Benehiko@users.noreply.github.com> Date: Mon, 26 Jun 2023 18:13:33 +0200 Subject: [PATCH 4/5] test(e2e): logout return to --- .../profiles/email/logout/success.spec.ts | 49 +-- .../integration/profiles/mfa/mix.spec.ts | 296 +++++++++--------- test/e2e/cypress/support/commands.ts | 12 + test/e2e/cypress/support/index.d.ts | 10 +- 4 files changed, 197 insertions(+), 170 deletions(-) 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 70a4ac9eb84e..a3a96fddf577 100644 --- a/test/e2e/cypress/integration/profiles/email/logout/success.spec.ts +++ b/test/e2e/cypress/integration/profiles/email/logout/success.spec.ts @@ -21,12 +21,15 @@ context("Testing logout flows", () => { }, ].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, @@ -57,30 +60,36 @@ context("Testing logout flows", () => { cy.noSession() cy.url().should("include", "/login") }) - }) - it("should be able to sign out at 2fa page", () => { - cy.sessionRequires2fa() - cy.getSession({ expectAal: "aal1" }) + it("should be able to sign out at 2fa page", () => { + 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() + // 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.logout() + cy.visit(route + "?return_to=https://www.ory.sh") - cy.reauth({ - expect: { email }, - type: { email: email, password: password }, - }) + cy.get('[name="identifier"]').clear().type(email) - cy.get("href*=/logout").should("have.", "https://www.ory.sh") - cy.get("href*=/logout").click() + cy.reauth({ + expect: { email, success: false }, + type: { password: password }, + }) + + cy.get("a[href*='logout']").click() - cy.location("host").should("eq", "www.ory.sh") + cy.location("host").should("eq", "www.ory.sh") + cy.useLookupSecrets(false) + }) }) }) }) diff --git a/test/e2e/cypress/integration/profiles/mfa/mix.spec.ts b/test/e2e/cypress/integration/profiles/mfa/mix.spec.ts index 02e48b56c670..026d4491a8c7 100644 --- a/test/e2e/cypress/integration/profiles/mfa/mix.spec.ts +++ b/test/e2e/cypress/integration/profiles/mfa/mix.spec.ts @@ -10,171 +10,171 @@ context("2FA with various methods", () => { beforeEach(() => { cy.task("resetCRI", {}) }) - ;[ - { - login: react.login, - settings: react.settings, - base: react.base, - app: "react" as "react", - profile: "spa", - }, - { - login: express.login, - settings: express.settings, - base: express.base, - app: "express" as "express", - profile: "mfa", - }, - ].forEach(({ settings, login, profile, app, base }) => { - describe(`for app ${app}`, () => { - before(() => { - cy.useConfigProfile(profile) - cy.proxy(app) - }) - let email = gen.email() - let password = gen.password() + ;[ + { + login: react.login, + settings: react.settings, + base: react.base, + app: "react" as "react", + profile: "spa", + }, + { + login: express.login, + settings: express.settings, + base: express.base, + app: "express" as "express", + profile: "mfa", + }, + ].forEach(({ settings, login, profile, app, base }) => { + describe(`for app ${app}`, () => { + before(() => { + cy.useConfigProfile(profile) + cy.proxy(app) + }) + let email = gen.email() + let password = gen.password() - beforeEach(() => { - cy.clearAllCookies() - email = gen.email() - password = gen.password() - cy.registerApi({ - email, - password, - fields: { "traits.website": website }, - }) - cy.clearAllCookies() - cy.login({ email, password, cookieUrl: base }) - cy.longPrivilegedSessionTime() - cy.task("sendCRI", { - query: "WebAuthn.disable", - opts: {}, - }) + beforeEach(() => { + cy.clearAllCookies() + email = gen.email() + password = gen.password() + cy.registerApi({ + email, + password, + fields: { "traits.website": website }, + }) + cy.clearAllCookies() + cy.login({ email, password, cookieUrl: base }) + cy.longPrivilegedSessionTime() + cy.task("sendCRI", { + query: "WebAuthn.disable", + opts: {}, }) + }) - it("should set up an use all mfa combinations", () => { - cy.visit(settings) + it("should set up an use all mfa combinations", () => { + cy.visit(settings) + cy.task("sendCRI", { + query: "WebAuthn.enable", + opts: {}, + }).then(() => { cy.task("sendCRI", { - query: "WebAuthn.enable", - opts: {}, - }).then(() => { - cy.task("sendCRI", { - query: "WebAuthn.addVirtualAuthenticator", - opts: { - options: { - protocol: "ctap2", - transport: "usb", - hasResidentKey: true, - hasUserVerification: true, - isUserVerified: true, - }, + query: "WebAuthn.addVirtualAuthenticator", + opts: { + options: { + protocol: "ctap2", + transport: "usb", + hasResidentKey: true, + hasUserVerification: true, + isUserVerified: true, }, - }).then(() => { - cy.getSession({ - expectAal: "aal1", - expectMethods: ["password"], - }) + }, + }).then(() => { + cy.getSession({ + expectAal: "aal1", + expectMethods: ["password"], + }) - cy.visit(settings) - // Set up TOTP - let secret: string - cy.get( - appPrefix(app) + '[data-testid="node/text/totp_secret_key/text"]', - ).then(($e) => { - secret = $e.text().trim() - }) - cy.get('[name="totp_code"]').then(($e) => { - cy.wrap($e).type(authenticator.generate(secret)) - }) - cy.get('[name="method"][value="totp"]').click() - cy.expectSettingsSaved() - cy.getSession({ - expectAal: "aal2", - expectMethods: ["password", "totp"], - }) + cy.visit(settings) + // Set up TOTP + let secret: string + cy.get( + appPrefix(app) + '[data-testid="node/text/totp_secret_key/text"]', + ).then(($e) => { + secret = $e.text().trim() + }) + cy.get('[name="totp_code"]').then(($e) => { + cy.wrap($e).type(authenticator.generate(secret)) + }) + cy.get('[name="method"][value="totp"]').click() + cy.expectSettingsSaved() + cy.getSession({ + expectAal: "aal2", + expectMethods: ["password", "totp"], + }) - // Set up lookup secrets - cy.visit(settings) - cy.get('[name="lookup_secret_regenerate"]').click() - let codes: string[] - cy.getLookupSecrets().then((c) => { - codes = c - }) - cy.get('[name="lookup_secret_confirm"]').click() - cy.expectSettingsSaved() - cy.getSession({ - expectAal: "aal2", - expectMethods: ["password", "totp", "lookup_secret"], - }) + // Set up lookup secrets + cy.visit(settings) + cy.get('[name="lookup_secret_regenerate"]').click() + let codes: string[] + cy.getLookupSecrets().then((c) => { + codes = c + }) + cy.get('[name="lookup_secret_confirm"]').click() + cy.expectSettingsSaved() + cy.getSession({ + expectAal: "aal2", + expectMethods: ["password", "totp", "lookup_secret"], + }) - // Set up WebAuthn - cy.visit(settings) - cy.get('[name="webauthn_register_displayname"]').type("my-key") - // We need a workaround here. So first we click, then we submit - cy.clickWebAuthButton("register") - cy.expectSettingsSaved() - cy.getSession({ - expectAal: "aal2", - expectMethods: ["password", "totp", "webauthn", "lookup_secret"], - }) + // Set up WebAuthn + cy.visit(settings) + cy.get('[name="webauthn_register_displayname"]').type("my-key") + // We need a workaround here. So first we click, then we submit + cy.clickWebAuthButton("register") + cy.expectSettingsSaved() + cy.getSession({ + expectAal: "aal2", + expectMethods: ["password", "totp", "webauthn", "lookup_secret"], + }) - cy.visit(login + "?aal=aal2&refresh=true") - cy.get('[name="totp_code"]').then(($e) => { - cy.wrap($e).type(authenticator.generate(secret)) - }) + cy.visit(login + "?aal=aal2&refresh=true") + cy.get('[name="totp_code"]').then(($e) => { + cy.wrap($e).type(authenticator.generate(secret)) + }) - cy.get('[name="method"][value="totp"]').click() - cy.location("pathname").should("not.include", "/login") + cy.get('[name="method"][value="totp"]').click() + cy.location("pathname").should("not.include", "/login") - cy.getSession({ - expectAal: "aal2", - expectMethods: [ - "password", - "totp", - "webauthn", - "lookup_secret", - "totp", - ], - }) + cy.getSession({ + expectAal: "aal2", + expectMethods: [ + "password", + "totp", + "webauthn", + "lookup_secret", + "totp", + ], + }) - // Use TOTP - cy.visit(login + "?aal=aal2&refresh=true") - cy.clickWebAuthButton("login") - cy.getSession({ - expectAal: "aal2", - expectMethods: [ - "password", - "totp", - "webauthn", - "lookup_secret", - "totp", - "webauthn", - ], - }) + // Use TOTP + cy.visit(login + "?aal=aal2&refresh=true") + cy.clickWebAuthButton("login") + cy.getSession({ + expectAal: "aal2", + expectMethods: [ + "password", + "totp", + "webauthn", + "lookup_secret", + "totp", + "webauthn", + ], + }) - // Use lookup - cy.visit(login + "?aal=aal2&refresh=true") - cy.get('[name="lookup_secret"]').then(($e) => { - cy.wrap($e).type(codes[1]) - }) - cy.get('[name="method"][value="lookup_secret"]').click() - cy.location("pathname").should("not.include", "/login") + // Use lookup + cy.visit(login + "?aal=aal2&refresh=true") + cy.get('[name="lookup_secret"]').then(($e) => { + cy.wrap($e).type(codes[1]) + }) + cy.get('[name="method"][value="lookup_secret"]').click() + cy.location("pathname").should("not.include", "/login") - cy.getSession({ - expectAal: "aal2", - expectMethods: [ - "password", - "totp", - "webauthn", - "lookup_secret", - "totp", - "webauthn", - "lookup_secret", - ], - }) + cy.getSession({ + expectAal: "aal2", + expectMethods: [ + "password", + "totp", + "webauthn", + "lookup_secret", + "totp", + "webauthn", + "lookup_secret", + ], }) }) }) }) }) + }) }) diff --git a/test/e2e/cypress/support/commands.ts b/test/e2e/cypress/support/commands.ts index 2b0d138dd14b..91622e42dd2c 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 6966aa8b4773..c9a52a0c7e8e 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 From 0bc7cf04d06400a180bbaf955c46775a9f29906a Mon Sep 17 00:00:00 2001 From: Alano Terblanche <18033717+Benehiko@users.noreply.github.com> Date: Thu, 29 Jun 2023 11:37:30 +0200 Subject: [PATCH 5/5] test: logout return_to isnt applicable to react --- .../cypress/integration/profiles/email/logout/success.spec.ts | 3 +++ 1 file changed, 3 insertions(+) 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 a3a96fddf577..ca965c2d0c25 100644 --- a/test/e2e/cypress/integration/profiles/email/logout/success.spec.ts +++ b/test/e2e/cypress/integration/profiles/email/logout/success.spec.ts @@ -62,6 +62,9 @@ context("Testing logout flows", () => { }) it("should be able to sign out at 2fa page", () => { + if (app === "react") { + return + } cy.useLookupSecrets(true) cy.sessionRequires2fa() cy.getSession({ expectAal: "aal1" })