From c6ded3f267724a14159e37b6bc109c1360e7fb69 Mon Sep 17 00:00:00 2001 From: Anthony LC Date: Fri, 23 Jan 2026 10:44:03 +0100 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8(auth)=20add=20silent=20login?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently users already logged in to the SSO have to click on the login button again to be connected. This extra step should not be necessary. This commit uses the "silent=true" parameter to the login endpoint to avoid the extra step. --- CHANGELOG.md | 1 + env.d/development/common | 2 +- src/backend/impress/settings.py | 10 ++++ .../e2e/__tests__/app-impress/login.spec.ts | 16 ++++++ .../features/auth/__tests__/utils.test.tsx | 40 +++++++++----- .../src/features/auth/components/Auth.tsx | 52 ++++++++++++++----- .../apps/impress/src/features/auth/conf.ts | 1 + .../apps/impress/src/features/auth/utils.ts | 31 +++++++++-- .../impress/src/pages/docs/[id]/index.tsx | 24 +++++---- 9 files changed, 139 insertions(+), 38 deletions(-) create mode 100644 src/frontend/apps/e2e/__tests__/app-impress/login.spec.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index eee6de14e..e10a8c9fe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ and this project adheres to - ✨(frontend) integrate configurable Waffle #1795 - ✨ Import of documents #1609 - 🚨(CI) gives warning if theme not updated #1811 +- ✨(auth) add silent login #1690 - 🔧(project) add DJANGO_EMAIL_URL_APP environment variable #1825 ### Changed diff --git a/env.d/development/common b/env.d/development/common index 7f8b13110..fad2bac59 100644 --- a/env.d/development/common +++ b/env.d/development/common @@ -48,7 +48,7 @@ LOGIN_REDIRECT_URL=http://localhost:3000 LOGIN_REDIRECT_URL_FAILURE=http://localhost:3000 LOGOUT_REDIRECT_URL=http://localhost:3000 -OIDC_REDIRECT_ALLOWED_HOSTS=["http://localhost:8083", "http://localhost:3000"] +OIDC_REDIRECT_ALLOWED_HOSTS="localhost:8083,localhost:3000" OIDC_AUTH_REQUEST_EXTRA_PARAMS={"acr_values": "eidas1"} # Store OIDC tokens in the session. Needed by search/ endpoint. diff --git a/src/backend/impress/settings.py b/src/backend/impress/settings.py index 751016031..6b6c4239e 100755 --- a/src/backend/impress/settings.py +++ b/src/backend/impress/settings.py @@ -549,6 +549,16 @@ class Base(Configuration): SESSION_COOKIE_NAME = "docs_sessionid" # OIDC - Authorization Code Flow + OIDC_AUTHENTICATE_CLASS = values.Value( + "lasuite.oidc_login.views.OIDCAuthenticationRequestView", + environ_name="OIDC_AUTHENTICATE_CLASS", + environ_prefix=None, + ) + OIDC_CALLBACK_CLASS = values.Value( + "lasuite.oidc_login.views.OIDCAuthenticationCallbackView", + environ_name="OIDC_CALLBACK_CLASS", + environ_prefix=None, + ) OIDC_CREATE_USER = values.BooleanValue( default=True, environ_name="OIDC_CREATE_USER", diff --git a/src/frontend/apps/e2e/__tests__/app-impress/login.spec.ts b/src/frontend/apps/e2e/__tests__/app-impress/login.spec.ts new file mode 100644 index 000000000..fc72e631d --- /dev/null +++ b/src/frontend/apps/e2e/__tests__/app-impress/login.spec.ts @@ -0,0 +1,16 @@ +import { expect, test } from '@playwright/test'; + +test.describe('Login: Not logged', () => { + test.use({ storageState: { cookies: [], origins: [] } }); + + test('It tries silent login', async ({ page }) => { + const silentLoginRequest = page.waitForRequest((request) => + request.url().includes('/api/v1.0/authenticate/?silent=true'), + ); + + await page.goto('/'); + + await silentLoginRequest; + expect(silentLoginRequest).toBeDefined(); + }); +}); diff --git a/src/frontend/apps/impress/src/features/auth/__tests__/utils.test.tsx b/src/frontend/apps/impress/src/features/auth/__tests__/utils.test.tsx index 1c9bff1bf..b92cc0ab8 100644 --- a/src/frontend/apps/impress/src/features/auth/__tests__/utils.test.tsx +++ b/src/frontend/apps/impress/src/features/auth/__tests__/utils.test.tsx @@ -1,33 +1,40 @@ import fetchMock from 'fetch-mock'; import { afterEach, describe, expect, it, vi } from 'vitest'; -import { gotoLogout } from '../utils'; +import { SILENT_LOGIN_RETRY } from '../conf'; +import { gotoLogout, gotoSilentLogin } from '../utils'; // Mock the Crisp service vi.mock('@/services/Crisp', () => ({ terminateCrispSession: vi.fn(), })); +// Add mock on window.location.replace +const mockReplace = vi.fn(); +Object.defineProperty(window, 'location', { + value: { + ...window.location, + replace: mockReplace, + href: 'http://test.jest/', + }, + writable: true, + configurable: true, +}); + +const setItemSpy = vi.spyOn(Storage.prototype, 'setItem'); + describe('utils', () => { afterEach(() => { vi.clearAllMocks(); fetchMock.restore(); + mockReplace.mockClear(); + setItemSpy.mockClear(); + localStorage.clear(); }); it('checks support session is terminated when logout', async () => { const { terminateCrispSession } = await import('@/services/Crisp'); - // Mock window.location.replace - const mockReplace = vi.fn(); - Object.defineProperty(window, 'location', { - value: { - ...window.location, - replace: mockReplace, - }, - writable: true, - configurable: true, - }); - gotoLogout(); expect(terminateCrispSession).toHaveBeenCalled(); @@ -35,4 +42,13 @@ describe('utils', () => { 'http://test.jest/api/v1.0/logout/', ); }); + + it('checks the gotoSilentLogin', async () => { + gotoSilentLogin(); + + expect(mockReplace).toHaveBeenCalledWith( + 'http://test.jest/api/v1.0/authenticate/?silent=true&next=http%3A%2F%2Ftest.jest%2F', + ); + expect(setItemSpy).toHaveBeenCalledWith(SILENT_LOGIN_RETRY, 'true'); + }); }); diff --git a/src/frontend/apps/impress/src/features/auth/components/Auth.tsx b/src/frontend/apps/impress/src/features/auth/components/Auth.tsx index ea079d1f9..a0fa349da 100644 --- a/src/frontend/apps/impress/src/features/auth/components/Auth.tsx +++ b/src/frontend/apps/impress/src/features/auth/components/Auth.tsx @@ -1,19 +1,37 @@ import { useRouter } from 'next/router'; -import { PropsWithChildren, useEffect, useState } from 'react'; +import { PropsWithChildren, useEffect, useMemo, useState } from 'react'; import { Loading } from '@/components'; import { useConfig } from '@/core'; import { HOME_URL } from '../conf'; import { useAuth } from '../hooks'; -import { getAuthUrl, gotoLogin } from '../utils'; +import { + getAuthUrl, + gotoLogin, + gotoSilentLogin, + hasTrySilent, + resetSilent, +} from '../utils'; export const Auth = ({ children }: PropsWithChildren) => { - const { isLoading, pathAllowed, isFetchedAfterMount, authenticated } = - useAuth(); - const { replace, pathname } = useRouter(); - const { data: config } = useConfig(); + const { + isLoading: isAuthLoading, + pathAllowed, + isFetchedAfterMount, + authenticated, + fetchStatus, + } = useAuth(); + const isLoading = fetchStatus !== 'idle' || isAuthLoading; const [isRedirecting, setIsRedirecting] = useState(false); + const { data: config } = useConfig(); + const shouldTrySilentLogin = useMemo( + () => !authenticated && !hasTrySilent() && !isLoading && !isRedirecting, + [authenticated, isLoading, isRedirecting], + ); + const shouldTryLogin = + !authenticated && !isLoading && !isRedirecting && !pathAllowed; + const { replace, pathname } = useRouter(); /** * If the user is authenticated and initially wanted to access a specific page, redirect him to that page now. @@ -23,6 +41,10 @@ export const Auth = ({ children }: PropsWithChildren) => { return; } + if (hasTrySilent()) { + resetSilent(); + } + const authUrl = getAuthUrl(); if (authUrl) { setIsRedirecting(true); @@ -34,7 +56,13 @@ export const Auth = ({ children }: PropsWithChildren) => { * If the user is not authenticated and not on a allowed pages */ useEffect(() => { - if (isLoading || authenticated || pathAllowed || isRedirecting) { + if (shouldTrySilentLogin) { + setIsRedirecting(true); + gotoSilentLogin(); + return; + } + + if (!shouldTryLogin) { return; } @@ -56,19 +84,17 @@ export const Auth = ({ children }: PropsWithChildren) => { setIsRedirecting(true); gotoLogin(); }, [ - authenticated, - pathAllowed, config?.FRONTEND_HOMEPAGE_FEATURE_ENABLED, - replace, - isLoading, - isRedirecting, pathname, + shouldTryLogin, + shouldTrySilentLogin, ]); const shouldShowLoader = (isLoading && !isFetchedAfterMount) || isRedirecting || - (!authenticated && !pathAllowed); + (!authenticated && !pathAllowed) || + shouldTrySilentLogin; if (shouldShowLoader) { return ; diff --git a/src/frontend/apps/impress/src/features/auth/conf.ts b/src/frontend/apps/impress/src/features/auth/conf.ts index 4e58db0fd..5feae77ce 100644 --- a/src/frontend/apps/impress/src/features/auth/conf.ts +++ b/src/frontend/apps/impress/src/features/auth/conf.ts @@ -4,3 +4,4 @@ export const HOME_URL = '/home/'; export const LOGIN_URL = `${baseApiUrl()}authenticate/`; export const LOGOUT_URL = `${baseApiUrl()}logout/`; export const PATH_AUTH_LOCAL_STORAGE = 'docs-path-auth'; +export const SILENT_LOGIN_RETRY = 'silent-login-retry'; diff --git a/src/frontend/apps/impress/src/features/auth/utils.ts b/src/frontend/apps/impress/src/features/auth/utils.ts index 2e765b242..cbb05c291 100644 --- a/src/frontend/apps/impress/src/features/auth/utils.ts +++ b/src/frontend/apps/impress/src/features/auth/utils.ts @@ -1,19 +1,21 @@ import { terminateCrispSession } from '@/services/Crisp'; +import { safeLocalStorage } from '@/utils/storages'; import { HOME_URL, LOGIN_URL, LOGOUT_URL, PATH_AUTH_LOCAL_STORAGE, + SILENT_LOGIN_RETRY, } from './conf'; /** * Get the stored auth URL from local storage */ export const getAuthUrl = () => { - const path_auth = localStorage.getItem(PATH_AUTH_LOCAL_STORAGE); + const path_auth = safeLocalStorage.getItem(PATH_AUTH_LOCAL_STORAGE); if (path_auth) { - localStorage.removeItem(PATH_AUTH_LOCAL_STORAGE); + safeLocalStorage.removeItem(PATH_AUTH_LOCAL_STORAGE); return path_auth; } }; @@ -27,7 +29,7 @@ export const setAuthUrl = () => { window.location.pathname !== '/' && window.location.pathname !== `${HOME_URL}/` ) { - localStorage.setItem(PATH_AUTH_LOCAL_STORAGE, window.location.href); + safeLocalStorage.setItem(PATH_AUTH_LOCAL_STORAGE, window.location.href); } }; @@ -39,6 +41,29 @@ export const gotoLogin = (withRedirect = true) => { window.location.replace(LOGIN_URL); }; +export const gotoSilentLogin = () => { + // Already tried silent login, dont try again + if (!hasTrySilent()) { + const params = new URLSearchParams({ + silent: 'true', + next: window.location.href, + }); + + safeLocalStorage.setItem(SILENT_LOGIN_RETRY, 'true'); + + const REDIRECT = `${LOGIN_URL}?${params.toString()}`; + window.location.replace(REDIRECT); + } +}; + +export const hasTrySilent = () => { + return !!safeLocalStorage.getItem(SILENT_LOGIN_RETRY); +}; + +export const resetSilent = () => { + safeLocalStorage.removeItem(SILENT_LOGIN_RETRY); +}; + export const gotoLogout = () => { terminateCrispSession(); window.location.replace(LOGOUT_URL); diff --git a/src/frontend/apps/impress/src/pages/docs/[id]/index.tsx b/src/frontend/apps/impress/src/pages/docs/[id]/index.tsx index 3c077b7f5..f0956296e 100644 --- a/src/frontend/apps/impress/src/pages/docs/[id]/index.tsx +++ b/src/frontend/apps/impress/src/pages/docs/[id]/index.tsx @@ -171,19 +171,25 @@ const DocPage = ({ id }: DocProps) => { }); }, [addTask, doc?.id, queryClient]); - if (isError && error) { - if ([404, 401].includes(error.status)) { - let replacePath = `/${error.status}`; + useEffect(() => { + if (!isError || !error?.status || ![404, 401].includes(error.status)) { + return; + } - if (error.status === 401) { - if (authenticated) { - queryClient.setQueryData([KEY_AUTH], null); - } - setAuthUrl(); + let replacePath = `/${error.status}`; + + if (error.status === 401) { + if (authenticated) { + queryClient.setQueryData([KEY_AUTH], null); } + setAuthUrl(); + } - void replace(replacePath); + void replace(replacePath); + }, [isError, error?.status, replace, authenticated, queryClient]); + if (isError && error?.status) { + if ([404, 401].includes(error.status)) { return ; }