From e604146cfdb47d0972b03f9d039173ea65dcfce5 Mon Sep 17 00:00:00 2001 From: Vaadin Bot Date: Mon, 27 May 2024 12:56:19 +0200 Subject: [PATCH] feat!(frontend): reload the page on login / logout success (#2432) (CP: 24.4) (#2481) feat!(frontend): reload the page on login / logout success (#2432) * refactor: prefer full page reload instead of AJAX request on logout * Revert "refactor: prefer full page reload instead of AJAX request on logout" This reverts commit 47880c127ee22dc40b7ab247a7ae510c03fa8a7c. * fix: reload the page on login / logout success * fix: restore Spring CSRF handling * fix: normalize redirect as path * test: prevent flackyness of parallel security tests * fix(frontend): support context path in redirects * chore: cleanup * test(security): verify server-side logout redirect location * test(security): make tests more reliable * fix: normalize default redirect URL * test(security): expect page reload from API * chore: Java formatting * fix: more accurate normalize url * test(security): extra wait for page load * test(security): extra wait for page load * test(security): extra wait for page load * test(security): more explicit load waiting * chore: cleanup * test(security): fix load waiting * chore: Java formatting * test: add implicit wait * test: rollback asserting path, use builtin wait for main-view * test: assertin path using script * fix: reload with url mapping * test: wait for invalid session reloads * chore: remove test logger * test: extra wait on open * Revert "test: prevent flackyness of parallel security tests" This reverts commit 77721aeb22cdb4eb63fe5dff28a76e3b20598aa0. * fix(frontend): prevent following requests when pageReloadNavigation is in use * Revert "test: add implicit wait" This reverts commit ed7d66d48e0139e0d80135a39660eba133cd966d. * feat!(frontend): onSuccess login / logout callbacks for custom after-flows * test: use onSuccess callbacks * test: wait for load when asserting current URL * test: apply url mapping to logout path * chore(frontend): remove unnecessary .toString() * feat(frontend): support URL in login / logout options --------- Co-authored-by: Vlad Rindevich Co-authored-by: Anton Platonov Co-authored-by: Soroosh Taefi --- .../frontend/connect-client.ts | 2 + .../tests/spring/security/frontend/auth.ts | 31 +++--- .../security/frontend/connect-client.ts | 2 + .../security/frontend/views/login-view.ts | 9 +- .../spring/fusionsecurity/SecurityConfig.java | 2 +- .../spring/fusionsecurity/SecurityIT.java | 18 +++- packages/ts/frontend/src/Authentication.ts | 95 +++++++++++++++++-- .../ts/frontend/test/Authentication.test.ts | 61 +++++++----- packages/ts/react-auth/src/useAuth.tsx | 10 +- 9 files changed, 179 insertions(+), 51 deletions(-) diff --git a/packages/java/tests/spring/security-urlmapping/frontend/connect-client.ts b/packages/java/tests/spring/security-urlmapping/frontend/connect-client.ts index 7efbf21555..c0276f9fc7 100644 --- a/packages/java/tests/spring/security-urlmapping/frontend/connect-client.ts +++ b/packages/java/tests/spring/security-urlmapping/frontend/connect-client.ts @@ -4,6 +4,8 @@ const client = new ConnectClient({ prefix: '../connect', middlewares: [ new InvalidSessionMiddleware(async () => { + // @ts-ignore + window.reloadPending = true; location.reload(); return { error: true, diff --git a/packages/java/tests/spring/security/frontend/auth.ts b/packages/java/tests/spring/security/frontend/auth.ts index c999b33d29..6ef23667d3 100644 --- a/packages/java/tests/spring/security/frontend/auth.ts +++ b/packages/java/tests/spring/security/frontend/auth.ts @@ -47,19 +47,21 @@ export function isAuthenticated() { * Uses `localStorage` for offline support. */ export async function login(username: string, password: string): Promise { - const result = await loginImpl(username, password); - if (!result.error) { - // Get user info from endpoint - await appStore.fetchUserInfo(); - authentication = { - timestamp: new Date().getTime(), - }; + return await loginImpl(username, password, { + async onSuccess() { + // Get user info from endpoint + await appStore.fetchUserInfo(); + authentication = { + timestamp: new Date().getTime(), + }; - // Save the authentication to local storage - localStorage.setItem(AUTHENTICATION_KEY, JSON.stringify(authentication)); - } + // Save the authentication to local storage + localStorage.setItem(AUTHENTICATION_KEY, JSON.stringify(authentication)); - return result; + // @ts-ignore + window.reloadPending = true; + }, + }); } /** @@ -69,6 +71,9 @@ export async function login(username: string, password: string): Promise { + // @ts-ignore + window.reloadPending = true; location.reload(); return { error: true, diff --git a/packages/java/tests/spring/security/frontend/views/login-view.ts b/packages/java/tests/spring/security/frontend/views/login-view.ts index 977888ef83..55ac66fc8c 100644 --- a/packages/java/tests/spring/security/frontend/views/login-view.ts +++ b/packages/java/tests/spring/security/frontend/views/login-view.ts @@ -22,7 +22,9 @@ export class LoginView extends View implements AfterEnterObserver { // If login was opened directly, use the default URL provided by the server. // As we do not know if the target is a resource or a Fusion view or a Flow view, we cannot just use Router.go - window.location.href = result.redirectUrl || this.returnUrl || ''; + + // Navigation should happen automatically + // window.location.href = result.redirectUrl || this.returnUrl || ''; }; render() { @@ -30,12 +32,17 @@ export class LoginView extends View implements AfterEnterObserver { } async login(event: CustomEvent): Promise { + // @ts-ignore + window.reloadPending = true; this.error = false; const result = await login(event.detail.username, event.detail.password); this.error = result.error; if (!result.error) { this.onSuccess(result); + } else { + // @ts-ignore + window.reloadPending = false; } return result; diff --git a/packages/java/tests/spring/security/src/main/java/com/vaadin/flow/spring/fusionsecurity/SecurityConfig.java b/packages/java/tests/spring/security/src/main/java/com/vaadin/flow/spring/fusionsecurity/SecurityConfig.java index a722c5dc65..4c7b35ffeb 100644 --- a/packages/java/tests/spring/security/src/main/java/com/vaadin/flow/spring/fusionsecurity/SecurityConfig.java +++ b/packages/java/tests/spring/security/src/main/java/com/vaadin/flow/spring/fusionsecurity/SecurityConfig.java @@ -74,7 +74,7 @@ protected void configure(HttpSecurity http) throws Exception { .permitAll(); super.configure(http); - setLoginView(http, "/login"); + setLoginView(http, "/login", applyUrlMapping("/")); http.logout().logoutUrl(applyUrlMapping("/logout")); if (stateless) { diff --git a/packages/java/tests/spring/security/src/test/java/com/vaadin/flow/spring/fusionsecurity/SecurityIT.java b/packages/java/tests/spring/security/src/test/java/com/vaadin/flow/spring/fusionsecurity/SecurityIT.java index 3074cca4f2..2f51111fcb 100644 --- a/packages/java/tests/spring/security/src/test/java/com/vaadin/flow/spring/fusionsecurity/SecurityIT.java +++ b/packages/java/tests/spring/security/src/test/java/com/vaadin/flow/spring/fusionsecurity/SecurityIT.java @@ -93,6 +93,7 @@ protected String getUrlMappingBasePath() { protected void open(String path) { getDriver().get(getRootURL() + getUrlMappingBasePath() + "/" + path); + waitForDocumentReady(); } protected void openResource(String path) { @@ -221,11 +222,13 @@ public void access_restricted_to_logged_in_users() { openResource(path); assertLoginViewShown(); loginUser(); + assertResourceShown(path); assertPageContains(contents); logout(); openResource(path); loginAdmin(); + assertResourceShown(path); assertPageContains(contents); logout(); @@ -246,6 +249,7 @@ public void access_restricted_to_admin() { openResource(path); loginAdmin(); + assertResourceShown(path); String adminResult = getDriver().getPageSource(); Assert.assertTrue(adminResult.contains(contents)); logout(); @@ -307,8 +311,16 @@ protected void navigateTo(String path, boolean assertPathShown) { } } + protected void waitForDocumentReady() { + waitUntil(driver -> Boolean.TRUE + .equals(this.getCommandExecutor().executeScript( + "return !window.reloadPending && window.document.readyState " + + "=== 'complete';"))); + } + private TestBenchElement getMainView() { - return waitUntil(driver -> $("*").id("main-view")); + waitForDocumentReady(); + return $("*").withId("main-view").waitForFirst(); } protected void assertLoginViewShown() { @@ -317,6 +329,7 @@ protected void assertLoginViewShown() { } private void assertRootPageShown() { + assertPathShown(""); waitUntil(drive -> $("h1").attribute("id", "header").exists()); String headerText = $("h1").id("header").getText(); Assert.assertEquals(ROOT_PAGE_HEADER_TEXT, headerText); @@ -343,6 +356,7 @@ private void assertPathShown(String path) { } private void assertPathShown(String path, boolean includeUrlMapping) { + waitForDocumentReady(); waitUntil(driver -> { String url = driver.getCurrentUrl(); String expected = getRootURL(); @@ -377,7 +391,9 @@ private void login(String username, String password) { form.getUsernameField().setValue(username); form.getPasswordField().setValue(password); form.submit(); + waitForDocumentReady(); waitUntilNot(driver -> $(LoginOverlayElement.class).exists()); + waitForDocumentReady(); } protected void refresh() { diff --git a/packages/ts/frontend/src/Authentication.ts b/packages/ts/frontend/src/Authentication.ts index b986a2010b..34c97d6fc5 100644 --- a/packages/ts/frontend/src/Authentication.ts +++ b/packages/ts/frontend/src/Authentication.ts @@ -41,13 +41,15 @@ async function updateCsrfTokensBasedOnResponse(response: Response): Promise) { +async function doLogout(logoutUrl: URL | string, headers: Record) { const response = await fetch(logoutUrl, { headers, method: 'POST' }); if (!response.ok) { throw new Error(`failed to logout with response ${response.status}`); } await updateCsrfTokensBasedOnResponse(response); + + return response; } export interface LoginResult { @@ -59,12 +61,73 @@ export interface LoginResult { defaultUrl?: string; } +export type SuccessCallback = () => Promise | void; + +export type NavigateFunction = (path: string) => void; + export interface LoginOptions { - loginProcessingUrl?: string; + /** + * The URL for login request, defaults to `/login`. + */ + loginProcessingUrl?: URL | string; + + /** + * The success callback. + */ + onSuccess?: SuccessCallback; + + /** + * The navigation callback, called after successful login. The default + * reloads the page. + */ + navigate?: NavigateFunction; } export interface LogoutOptions { - logoutUrl?: string; + /** + * The URL for logout request, defaults to `/logout`. + */ + logoutUrl?: URL | string; + + /** + * The success callback. + */ + onSuccess?: SuccessCallback; + + /** + * The navigation callback, called after successful logout. The default + * reloads the page. + */ + navigate?: NavigateFunction; +} + +function normalizePath(url: string): string { + // URL with context path + const effectiveBaseURL = new URL('.', document.baseURI); + const effectiveBaseURI = effectiveBaseURL.toString(); + + let normalized = url; + + // Strip context path prefix + if (normalized.startsWith(effectiveBaseURL.pathname)) { + return `/${normalized.slice(effectiveBaseURL.pathname.length)}`; + } + + // Strip base URI + normalized = normalized.startsWith(effectiveBaseURI) ? `/${normalized.slice(effectiveBaseURI.length)}` : normalized; + + return normalized; +} + +/** + * Navigates to the provided path using page reload. + * + * @param to - navigation target path + */ +function navigateWithPageReload(to: string) { + // Consider absolute path to be within application context + const url = to.startsWith('/') ? new URL(`.${to}`, document.baseURI) : to; + window.location.replace(url); } /** @@ -109,6 +172,15 @@ export async function login(username: string, password: string, options?: LoginO updateSpringCsrfMetaTags(springCsrfTokenInfo); } + if (options?.onSuccess) { + await options.onSuccess(); + } + + const url = savedUrl ?? defaultUrl ?? document.baseURI; + const toPath = normalizePath(url); + const navigate = options?.navigate ?? navigateWithPageReload; + navigate(toPath); + return { defaultUrl, error: false, @@ -141,16 +213,17 @@ export async function login(username: string, password: string, options?: LoginO export async function logout(options?: LogoutOptions): Promise { // this assumes the default Spring Security logout configuration (handler URL) const logoutUrl = options?.logoutUrl ?? 'logout'; + let response: Response | undefined; try { const headers = getSpringCsrfTokenHeadersForAuthRequest(document); - await doLogout(logoutUrl, headers); + response = await doLogout(logoutUrl, headers); } catch { try { - const response = await fetch('?nocache'); - const responseText = await response.text(); + const noCacheResponse = await fetch('?nocache'); + const responseText = await noCacheResponse.text(); const doc = new DOMParser().parseFromString(responseText, 'text/html'); const headers = getSpringCsrfTokenHeadersForAuthRequest(doc); - await doLogout(logoutUrl, headers); + response = await doLogout(logoutUrl, headers); } catch (error) { // clear the token if the call fails clearSpringCsrfMetaTags(); @@ -158,6 +231,14 @@ export async function logout(options?: LogoutOptions): Promise { } } finally { CookieManager.remove(JWT_COOKIE_NAME); + if (response && response.ok && response.redirected) { + if (options?.onSuccess) { + await options.onSuccess(); + } + const toPath = normalizePath(response.url); + const navigate = options?.navigate ?? navigateWithPageReload; + navigate(toPath); + } } } diff --git a/packages/ts/frontend/test/Authentication.test.ts b/packages/ts/frontend/test/Authentication.test.ts index 5df954e93b..d68c217ef6 100644 --- a/packages/ts/frontend/test/Authentication.test.ts +++ b/packages/ts/frontend/test/Authentication.test.ts @@ -8,9 +8,11 @@ import { VAADIN_CSRF_HEADER } from '../src/CsrfUtils.js'; import { ConnectClient, InvalidSessionMiddleware, - login, + login as originalLogin, + type LoginOptions, type LoginResult, - logout, + logout as originalLogout, + type LogoutOptions, type OnInvalidSessionCallback, } from '../src/index.js'; import { @@ -53,6 +55,20 @@ describe('@vaadin/hilla-frontend', () => { ); } + const navigate = sinon.fake<[string], void>(); + const onSuccess = sinon.fake.resolves(undefined); + + const login = async (username: string, password: string, loginOptions?: LoginOptions) => + originalLogin(username, password, { + navigate, + ...(loginOptions ?? {}), + }); + const logout = async (logoutOptions?: LogoutOptions) => + originalLogout({ + navigate, + ...(logoutOptions ?? {}), + }); + beforeEach(() => { setupSpringCsrfMetaTags(); requestHeaders[TEST_SPRING_CSRF_HEADER_NAME] = TEST_SPRING_CSRF_TOKEN_VALUE; @@ -60,6 +76,8 @@ describe('@vaadin/hilla-frontend', () => { afterEach(() => { // delete window.Vaadin.TypeScript; clearSpringCsrfMetaTags(); + onSuccess.resetHistory(); + navigate.resetHistory(); }); describe('login', () => { @@ -69,7 +87,7 @@ describe('@vaadin/hilla-frontend', () => { it('should return an error on invalid credentials', async () => { fetchMock.post('/login', { redirectUrl: '/login?error' }, { headers: requestHeaders }); - const result = await login('invalid-username', 'invalid-password'); + const result = await login('invalid-username', 'invalid-password', { onSuccess }); const expectedResult: LoginResult = { error: true, errorMessage: 'Check that you have entered the correct username and password and try again.', @@ -78,6 +96,8 @@ describe('@vaadin/hilla-frontend', () => { expect(fetchMock.calls()).to.have.lengthOf(1); expect(result).to.deep.equal(expectedResult); + expect(onSuccess).to.not.be.called; + expect(navigate).to.not.be.called; }); it('should return a CSRF token on valid credentials', async () => { @@ -99,24 +119,7 @@ describe('@vaadin/hilla-frontend', () => { expect(fetchMock.calls()).to.have.lengthOf(1); expect(result).to.deep.equal(expectedResult); - }); - - it('should set the csrf tokens on login', async () => { - fetchMock.post( - '/login', - { - body: happyCaseLoginResponseText, - headers: { - ...happyCaseResponseHeaders, - 'Spring-CSRF-header': TEST_SPRING_CSRF_HEADER_NAME, - 'Spring-CSRF-token': 'some-new-spring-token', - 'Vaadin-CSRF': 'some-new-token', - }, - }, - { headers: requestHeaders }, - ); - await login('valid-username', 'valid-password'); - verifySpringCsrfToken('some-new-spring-token'); + expect(navigate).to.be.calledOnceWithExactly('/'); }); it('should redirect based on request cache after login', async () => { @@ -133,7 +136,7 @@ describe('@vaadin/hilla-frontend', () => { }, { headers: requestHeaders }, ); - const result = await login('valid-username', 'valid-password'); + const result = await login('valid-username', 'valid-password', { onSuccess }); const expectedResult: LoginResult = { defaultUrl: '/', error: false, @@ -143,6 +146,8 @@ describe('@vaadin/hilla-frontend', () => { expect(fetchMock.calls()).to.have.lengthOf(1); expect(result).to.deep.equal(expectedResult); + expect(onSuccess).to.be.calledBefore(navigate); + expect(navigate).to.be.calledOnceWithExactly('/protected-view'); }); }); @@ -164,6 +169,7 @@ describe('@vaadin/hilla-frontend', () => { await logout(); expect(fetchMock.calls()).to.have.lengthOf(1); verifySpringCsrfToken(TEST_SPRING_CSRF_TOKEN_VALUE); + expect(navigate).to.be.calledOnceWithExactly('/logout?login'); }); it('should clear the csrf tokens on failed server logout', async () => { @@ -181,13 +187,14 @@ describe('@vaadin/hilla-frontend', () => { let thrownError; try { - await logout(); + await logout({ onSuccess }); } catch (err) { thrownError = err; } expect(thrownError).to.equal(fakeError); expect(fetchMock.calls()).to.have.lengthOf(3); verifySpringCsrfTokenIsCleared(); + expect(navigate).to.not.be.called; }); it('should clear the JWT cookie on logout', async () => { @@ -205,6 +212,7 @@ describe('@vaadin/hilla-frontend', () => { expect(fetchMock.calls()).to.have.lengthOf(1); expect(CookieManager.get(JWT_COOKIE_NAME)).to.be.undefined; + expect(navigate).to.be.calledOnceWithExactly('/logout?login'); }); it('should clear the JWT cookie on failed server logout', async () => { @@ -225,6 +233,7 @@ describe('@vaadin/hilla-frontend', () => { } expect(thrownError).to.equal(fakeError); expect(CookieManager.get(JWT_COOKIE_NAME)).to.be.undefined; + expect(navigate).to.not.be.called; }); // when started the app offline, the spring csrf meta tags are not available @@ -247,6 +256,7 @@ describe('@vaadin/hilla-frontend', () => { await logout(); expect(fetchMock.calls()).to.have.lengthOf(3); verifySpringCsrfToken(TEST_SPRING_CSRF_TOKEN_VALUE); + expect(navigate).to.be.calledOnceWithExactly('/logout?login'); }); // when started the app offline, the spring csrf meta tags are not available @@ -276,6 +286,7 @@ describe('@vaadin/hilla-frontend', () => { } expect(thrownError).to.equal(fakeError); expect(fetchMock.calls()).to.have.lengthOf(3); + expect(navigate).to.not.be.called; setupSpringCsrfMetaTags(); }); @@ -301,9 +312,11 @@ describe('@vaadin/hilla-frontend', () => { }, { headers: requestHeaders, overwriteRoutes: false, repeat: 1 }, ); - await logout(); + await logout({ onSuccess }); expect(fetchMock.calls()).to.have.lengthOf(3); verifySpringCsrfToken(TEST_SPRING_CSRF_TOKEN_VALUE); + expect(onSuccess).to.be.calledBefore(navigate); + expect(navigate).to.be.calledOnceWithExactly('/logout?login'); }); }); diff --git a/packages/ts/react-auth/src/useAuth.tsx b/packages/ts/react-auth/src/useAuth.tsx index a626f8ba08..bfec9dda42 100644 --- a/packages/ts/react-auth/src/useAuth.tsx +++ b/packages/ts/react-auth/src/useAuth.tsx @@ -1,7 +1,9 @@ import { login as _login, + type LoginOptions, type LoginResult, logout as _logout, + type LogoutOptions, UnauthorizedResponseError, } from '@vaadin/hilla-frontend'; import { createContext, type Dispatch, useContext, useEffect, useReducer } from 'react'; @@ -185,8 +187,8 @@ function AuthProvider({ children, getAuthenticatedUser, config }: AuthPro const authenticate = createAuthenticateThunk(dispatch, getAuthenticatedUser); const unauthenticate = createUnauthenticateThunk(dispatch); - async function login(username: string, password: string): Promise { - const result = await _login(username, password); + async function login(username: string, password: string, options?: LoginOptions): Promise { + const result = await _login(username, password, options); if (!result.error) { await authenticate(); @@ -195,8 +197,8 @@ function AuthProvider({ children, getAuthenticatedUser, config }: AuthPro return result; } - async function logout(): Promise { - await _logout(); + async function logout(options?: LogoutOptions): Promise { + await _logout(options); unauthenticate(); }