From 9424db193bd5e27902328984c49f90e3db4754d3 Mon Sep 17 00:00:00 2001 From: Joonas Javanainen Date: Wed, 22 Jan 2025 16:15:15 +0200 Subject: [PATCH 1/3] Improve weak login form - replaces Keycloak link when the feature flag is enabled --- frontend/src/citizen-frontend/App.tsx | 9 + .../src/citizen-frontend/login/LoginPage.tsx | 126 +++----------- .../login/WeakLoginFormPage.tsx | 154 ++++++++++++++++++ .../src/citizen-frontend/navigation/const.ts | 6 +- .../defaults/citizen/i18n/en.tsx | 11 +- .../defaults/citizen/i18n/fi.tsx | 11 +- .../defaults/citizen/i18n/sv.tsx | 11 +- 7 files changed, 221 insertions(+), 107 deletions(-) create mode 100644 frontend/src/citizen-frontend/login/WeakLoginFormPage.tsx diff --git a/frontend/src/citizen-frontend/App.tsx b/frontend/src/citizen-frontend/App.tsx index 0388432a46..47ed3190cc 100644 --- a/frontend/src/citizen-frontend/App.tsx +++ b/frontend/src/citizen-frontend/App.tsx @@ -47,6 +47,7 @@ import IncomeStatementView from './income-statements/IncomeStatementView' import IncomeStatements from './income-statements/IncomeStatements' import { Localization, useTranslation } from './localization' import LoginPage from './login/LoginPage' +import LoginFormPage from './login/WeakLoginFormPage' import MapPage from './map/MapPage' import MessagesPage from './messages/MessagesPage' import { MessageContextProvider } from './messages/state' @@ -143,6 +144,14 @@ export default createBrowserRouter([ path: '/', element: , children: [ + { + path: '/login/form', + element: ( + + + + ) + }, { path: '/login', element: ( diff --git a/frontend/src/citizen-frontend/login/LoginPage.tsx b/frontend/src/citizen-frontend/login/LoginPage.tsx index 32851fd1bd..34d098aa1d 100644 --- a/frontend/src/citizen-frontend/login/LoginPage.tsx +++ b/frontend/src/citizen-frontend/login/LoginPage.tsx @@ -3,20 +3,13 @@ // SPDX-License-Identifier: LGPL-2.1-or-later import { FontAwesomeIcon } from '@fortawesome/react-fontawesome' -import React, { useMemo, useState } from 'react' -import { Link, Navigate, useSearchParams } from 'react-router' +import React, { useState } from 'react' +import { Link, Navigate, useNavigate, useSearchParams } from 'react-router' import styled from 'styled-components' -import { wrapResult } from 'lib-common/api' -import { string } from 'lib-common/form/fields' -import { object, validated } from 'lib-common/form/form' -import { useForm, useFormFields } from 'lib-common/form/hooks' import { useQueryResult } from 'lib-common/query' -import { parseUrlWithOrigin } from 'lib-common/utils/parse-url-with-origin' import Main from 'lib-components/atoms/Main' -import { AsyncButton } from 'lib-components/atoms/buttons/AsyncButton' import LinkButton from 'lib-components/atoms/buttons/LinkButton' -import { InputFieldF } from 'lib-components/atoms/form/InputField' import Container, { ContentArea } from 'lib-components/layout/Container' import { FixedSpaceColumn } from 'lib-components/layout/flex-helpers' import { @@ -34,10 +27,13 @@ import { featureFlags } from 'lib-customizations/citizen' import { farMap } from 'lib-icons' import Footer from '../Footer' -import { authWeakLogin } from '../auth/api' import { useUser } from '../auth/state' import { useTranslation } from '../localization' -import { getStrongLoginUri, getWeakLoginUri } from '../navigation/const' +import { + getStrongLoginUri, + getWeakKeycloakLoginUri, + getWeakLoginUri +} from '../navigation/const' import { systemNotificationsQuery } from './queries' @@ -51,6 +47,7 @@ export default React.memo(function LoginPage() { const [searchParams] = useSearchParams() const unvalidatedNextPath = searchParams.get('next') + const navigate = useNavigate() const [showInfoBoxText1, setShowInfoBoxText1] = useState(false) const [showInfoBoxText2, setShowInfoBoxText2] = useState(false) @@ -104,14 +101,24 @@ export default React.memo(function LoginPage() { /> )} - - {i18n.loginPage.login.link} - - {featureFlags.weakLogin && ( - + {featureFlags.weakLogin ? ( + { + e.preventDefault() + void navigate(getWeakLoginUri(unvalidatedNextPath ?? '/')) + }} + data-qa="weak-login" + > + {i18n.loginPage.login.link} + + ) : ( + + {i18n.loginPage.login.link} + )} @@ -159,87 +166,6 @@ export default React.memo(function LoginPage() { ) }) -const weakLoginForm = validated( - object({ - username: string(), - password: string() - }), - (form) => { - if (form.username.length === 0 || form.password.length === 0) { - return 'required' - } - return undefined - } -) - -const authWeakLoginResult = wrapResult(authWeakLogin) - -const WeakLoginForm = React.memo(function WeakLogin({ - unvalidatedNextPath -}: { - unvalidatedNextPath: string | null -}) { - const i18n = useTranslation() - const [rateLimitError, setRateLimitError] = useState(false) - - const nextUrl = useMemo( - () => - unvalidatedNextPath - ? parseUrlWithOrigin(window.location, unvalidatedNextPath) - : undefined, - [unvalidatedNextPath] - ) - - const form = useForm( - weakLoginForm, - () => ({ username: '', password: '' }), - i18n.validationErrors - ) - const { username, password } = useFormFields(form) - return ( - <> - -
e.preventDefault()}> - - {rateLimitError && ( - - )} - - - - authWeakLoginResult(form.state.username, form.state.password) - } - onSuccess={() => window.location.replace(nextUrl ?? '/')} - onFailure={(error) => { - if (error.statusCode === 429) { - setRateLimitError(true) - } - }} - /> - -
- - ) -}) - const MapLink = styled(Link)` text-decoration: none; display: inline-block; diff --git a/frontend/src/citizen-frontend/login/WeakLoginFormPage.tsx b/frontend/src/citizen-frontend/login/WeakLoginFormPage.tsx new file mode 100644 index 0000000000..94c14c13fc --- /dev/null +++ b/frontend/src/citizen-frontend/login/WeakLoginFormPage.tsx @@ -0,0 +1,154 @@ +// SPDX-FileCopyrightText: 2017-2025 City of Espoo +// +// SPDX-License-Identifier: LGPL-2.1-or-later + +import React, { useMemo, useState } from 'react' +import { Navigate, useSearchParams } from 'react-router' + +import { authWeakLogin } from 'citizen-frontend/auth/api' +import { wrapResult } from 'lib-common/api' +import { string } from 'lib-common/form/fields' +import { object, required, validated } from 'lib-common/form/form' +import { useForm, useFormFields } from 'lib-common/form/hooks' +import { nonBlank } from 'lib-common/form/validators' +import { parseUrlWithOrigin } from 'lib-common/utils/parse-url-with-origin' +import Main from 'lib-components/atoms/Main' +import { AsyncButton } from 'lib-components/atoms/buttons/AsyncButton' +import ReturnButton from 'lib-components/atoms/buttons/ReturnButton' +import { InputFieldF } from 'lib-components/atoms/form/InputField' +import Container, { ContentArea } from 'lib-components/layout/Container' +import { FixedSpaceColumn } from 'lib-components/layout/flex-helpers' +import { + MobileOnly, + TabletAndDesktop +} from 'lib-components/layout/responsive-layout' +import ExpandingInfo from 'lib-components/molecules/ExpandingInfo' +import { AlertBox } from 'lib-components/molecules/MessageBoxes' +import { H1, Label } from 'lib-components/typography' +import { Gap } from 'lib-components/white-space' + +import Footer from '../Footer' +import { useUser } from '../auth/state' +import { useTranslation } from '../localization' + +export default React.memo(function WeakLoginFormPage() { + const i18n = useTranslation() + const user = useUser() + + const [searchParams] = useSearchParams() + const unvalidatedNextPath = searchParams.get('next') + + if (user) { + return + } + + return ( +
+ + + + + + + + + + +

{i18n.loginPage.login.title}

+ + + + + +
+
+ ) +}) + +const weakLoginForm = object({ + username: validated(required(string()), nonBlank), + password: validated(required(string()), nonBlank) +}) + +const authWeakLoginResult = wrapResult(authWeakLogin) + +const WeakLoginForm = React.memo(function WeakLogin({ + unvalidatedNextPath +}: { + unvalidatedNextPath: string | null +}) { + const i18n = useTranslation() + const t = i18n.loginPage.login + const [rateLimitError, setRateLimitError] = useState(false) + + const nextUrl = useMemo( + () => + unvalidatedNextPath + ? parseUrlWithOrigin(window.location, unvalidatedNextPath) + : undefined, + [unvalidatedNextPath] + ) + + const form = useForm( + weakLoginForm, + () => ({ username: '', password: '' }), + i18n.validationErrors + ) + const { username, password } = useFormFields(form) + return ( +
e.preventDefault()} + data-qa="weak-login-form" + > + + {rateLimitError && } + + + + + + + + + + authWeakLoginResult(form.state.username, form.state.password) + } + onSuccess={() => window.location.replace(nextUrl ?? '/')} + onFailure={(error) => { + if (error.statusCode === 429) { + setRateLimitError(true) + } + }} + /> + + {t.forgotPassword} + + {t.noUsername} + +
+ ) +}) diff --git a/frontend/src/citizen-frontend/navigation/const.ts b/frontend/src/citizen-frontend/navigation/const.ts index 3c0ad8c6c8..713edaed51 100644 --- a/frontend/src/citizen-frontend/navigation/const.ts +++ b/frontend/src/citizen-frontend/navigation/const.ts @@ -4,10 +4,14 @@ export const logoutUrl = `/api/citizen/auth/logout?RelayState=/` -export const getWeakLoginUri = ( +export const getWeakKeycloakLoginUri = ( url = `${window.location.pathname}${window.location.search}${window.location.hash}` ) => `/api/citizen/auth/keycloak/login?RelayState=${encodeURIComponent(url)}` +export const getWeakLoginUri = ( + url = `${window.location.pathname}${window.location.search}${window.location.hash}` +) => `/login/form?next=${encodeURIComponent(url)}` + export const getStrongLoginUri = ( url = `${window.location.pathname}${window.location.search}${window.location.hash}` ) => `/api/citizen/auth/sfi/login?RelayState=${encodeURIComponent(url)}` diff --git a/frontend/src/lib-customizations/defaults/citizen/i18n/en.tsx b/frontend/src/lib-customizations/defaults/citizen/i18n/en.tsx index 6288b722d0..7c3e2f5473 100644 --- a/frontend/src/lib-customizations/defaults/citizen/i18n/en.tsx +++ b/frontend/src/lib-customizations/defaults/citizen/i18n/en.tsx @@ -120,6 +120,7 @@ const en: Translations = { }, closeModal: 'Close popup', close: 'Close', + goBack: 'Go back', duplicatedChild: { identifier: { DAYCARE: { @@ -216,10 +217,16 @@ const en: Translations = {

), - email: 'E-mail', + username: 'Username', password: 'Password', rateLimitError: - 'Your account has been temporarily locked due to a large number of login attempts. Please try again later.' + 'Your account has been temporarily locked due to a large number of login attempts. Please try again later.', + forgotPassword: 'Forgot your password?', + forgotPasswordInfo: + 'You can change your password in your profile by logging in using strong authentication', + noUsername: 'No username?', + noUsernameInfo: + 'You can create a username by logging in using strong authentication and enabling login via email on the "Personal information" page.' }, applying: { title: 'Sign in using Suomi.fi', diff --git a/frontend/src/lib-customizations/defaults/citizen/i18n/fi.tsx b/frontend/src/lib-customizations/defaults/citizen/i18n/fi.tsx index b67d78711b..653a412fc2 100644 --- a/frontend/src/lib-customizations/defaults/citizen/i18n/fi.tsx +++ b/frontend/src/lib-customizations/defaults/citizen/i18n/fi.tsx @@ -117,6 +117,7 @@ export default { }, closeModal: 'Sulje ponnahdusikkuna', close: 'Sulje', + goBack: 'Takaisin', duplicatedChild: { identifier: { DAYCARE: { @@ -213,10 +214,16 @@ export default {

), - email: 'Sähköpostiosoite', + username: 'Käyttäjätunnus', password: 'Salasana', rateLimitError: - 'Käyttäjätunnuksesi on väliaikaisesti lukittu kirjautumisyritysten määrästä johtuen. Kokeile myöhemmin uudelleen.' + 'Käyttäjätunnuksesi on väliaikaisesti lukittu kirjautumisyritysten määrästä johtuen. Kokeile myöhemmin uudelleen.', + forgotPassword: 'Unohditko salasanasi?', + forgotPasswordInfo: + 'Voit vaihtaa salasanan omissa tiedoissasi kirjautumalla vahvasti.', + noUsername: 'Ei käyttäjätunnuksia?', + noUsernameInfo: + 'Voit luoda käyttäjätunnuksen kirjautumalla vahvasti ja sallimalla kirjautumisen sähköpostilla "Omat tiedot"-sivulla' }, applying: { title: 'Kirjaudu Suomi.fi:ssä', diff --git a/frontend/src/lib-customizations/defaults/citizen/i18n/sv.tsx b/frontend/src/lib-customizations/defaults/citizen/i18n/sv.tsx index 6c7ded358a..d325b19f77 100644 --- a/frontend/src/lib-customizations/defaults/citizen/i18n/sv.tsx +++ b/frontend/src/lib-customizations/defaults/citizen/i18n/sv.tsx @@ -118,6 +118,7 @@ const sv: Translations = { }, closeModal: 'Stäng popup', close: 'Stäng', + goBack: 'Tillbaka', duplicatedChild: { identifier: { DAYCARE: { @@ -214,10 +215,16 @@ const sv: Translations = {

), - email: 'E-post', + username: 'Användarnamn', password: 'Lösenord', rateLimitError: - 'Ditt konto har tillfälligt låsts på grund av ett stort antal inloggningsförsök. Vänligen försök igen senare.' + 'Ditt användarnamn är tillfälligt låst på grund av antalet inloggningsförsök. Försök igen senare.', + forgotPassword: 'Glömt lösenordet?', + forgotPasswordInfo: + 'Du kan byta lösenord i dina egna uppgifter genom stark autentisering.', + noUsername: 'Inget användarnamn?', + noUsernameInfo: + 'Du kan skapa ett användarnamn genom att logga in med stark autentisering och tillåta inloggning ned e-post på sidan "Egna uppgifter"' }, applying: { title: 'Logga in via Suomi.fi', From cbe83902303248ab87d0cbe036f126c0993a7ccf Mon Sep 17 00:00:00 2001 From: Joonas Javanainen Date: Mon, 27 Jan 2025 09:59:50 +0200 Subject: [PATCH 2/3] Replace Keycloak login in E2E tests --- .../pages/citizen/citizen-keycloak.ts | 61 ------ .../pages/citizen/citizen-personal-details.ts | 2 - .../specs/0_citizen/citizen-auth.spec.ts | 31 ++- .../0_citizen/citizen-child-page.spec.ts | 24 +-- .../specs/0_citizen/citizen-keycloak.spec.ts | 154 --------------- .../citizen-personal-details.spec.ts | 25 +-- .../7_messaging/messaging-by-staff.spec.ts | 37 ++-- .../specs/7_messaging/messaging.spec.ts | 43 ++-- frontend/src/e2e-test/utils/keycloak.ts | 184 ------------------ frontend/src/e2e-test/utils/user.ts | 28 +-- 10 files changed, 72 insertions(+), 517 deletions(-) delete mode 100644 frontend/src/e2e-test/pages/citizen/citizen-keycloak.ts delete mode 100644 frontend/src/e2e-test/specs/0_citizen/citizen-keycloak.spec.ts delete mode 100644 frontend/src/e2e-test/utils/keycloak.ts diff --git a/frontend/src/e2e-test/pages/citizen/citizen-keycloak.ts b/frontend/src/e2e-test/pages/citizen/citizen-keycloak.ts deleted file mode 100644 index d7b8a5bad6..0000000000 --- a/frontend/src/e2e-test/pages/citizen/citizen-keycloak.ts +++ /dev/null @@ -1,61 +0,0 @@ -// SPDX-FileCopyrightText: 2017-2024 City of Espoo -// -// SPDX-License-Identifier: LGPL-2.1-or-later - -import { Page, Element, TextInput, ElementCollection } from '../../utils/page' - -export class KeycloakLoginPage { - email: TextInput - password: TextInput - loginButton: Element - forgotPasswordLink: Element - registerLink: Element - - constructor(page: Page) { - this.email = new TextInput(page.findTextExact('Sähköpostiosoite')) - this.password = new TextInput(page.findTextExact('Salasana')) - this.loginButton = page.findTextExact('Kirjaudu sisään') - this.forgotPasswordLink = page.findTextExact('Unohditko salasanasi?') - this.registerLink = page.findTextExact('Luo tunnus') - } -} - -export class ForgotPasswordPage { - email: TextInput - changePasswordButton: Element - backToLoginLink: Element - - constructor(page: Page) { - this.email = new TextInput(page.findTextExact('Sähköpostiosoite')) - this.changePasswordButton = page.findTextExact('Vahvista') - this.backToLoginLink = page.findTextExact('Palaa sisäänkirjautumiseen') - } -} - -export class UpdatePasswordPage { - newPassword: TextInput - confirmPassword: TextInput - changePasswordButton: Element - - constructor(page: Page) { - this.newPassword = new TextInput(page.findTextExact('Salasana')) - this.confirmPassword = new TextInput( - page.findTextExact('Vahvista salasana') - ) - this.changePasswordButton = page.findTextExact('Vahvista') - } -} - -export class ConfirmPage { - heading: Element - email: TextInput - sendButton: Element - allLabels: ElementCollection - - constructor(page: Page) { - this.heading = page.find('.evaka-info-text') - this.email = new TextInput(page.findTextExact('Sähköpostiosoite')) - this.sendButton = page.findTextExact('Lähetä') - this.allLabels = page.findAll('label') - } -} diff --git a/frontend/src/e2e-test/pages/citizen/citizen-personal-details.ts b/frontend/src/e2e-test/pages/citizen/citizen-personal-details.ts index 969cddf584..f66ac87920 100644 --- a/frontend/src/e2e-test/pages/citizen/citizen-personal-details.ts +++ b/frontend/src/e2e-test/pages/citizen/citizen-personal-details.ts @@ -135,7 +135,6 @@ export class CitizenPersonalDetailsSection extends Element { } export class LoginDetailsSection extends Element { - keycloakEmail: Element username: Element activateCredentials: Element weakLoginEnabled: Element @@ -144,7 +143,6 @@ export class LoginDetailsSection extends Element { constructor(element: Element) { super(element) - this.keycloakEmail = element.findByDataQa('keycloak-email') this.username = element.findByDataQa('username') this.activateCredentials = element.findByDataQa('activate-credentials') this.weakLoginEnabled = element.findByDataQa('weak-login-enabled') diff --git a/frontend/src/e2e-test/specs/0_citizen/citizen-auth.spec.ts b/frontend/src/e2e-test/specs/0_citizen/citizen-auth.spec.ts index 29484f1b8f..d8f06158dc 100644 --- a/frontend/src/e2e-test/specs/0_citizen/citizen-auth.spec.ts +++ b/frontend/src/e2e-test/specs/0_citizen/citizen-auth.spec.ts @@ -2,32 +2,31 @@ // // SPDX-License-Identifier: LGPL-2.1-or-later -import { testAdult, Fixture } from '../../dev-api/fixtures' -import { resetServiceState } from '../../generated/api-clients' +import { Fixture, testAdult } from '../../dev-api/fixtures' +import { + resetServiceState, + upsertWeakCredentials +} from '../../generated/api-clients' import CitizenHeader from '../../pages/citizen/citizen-header' -import { KeycloakRealmClient } from '../../utils/keycloak' import { Page } from '../../utils/page' -import { - CitizenWeakAccount, - citizenWeakAccount, - enduserLogin, - enduserLoginWeak -} from '../../utils/user' +import { enduserLogin, enduserLoginWeak } from '../../utils/user' describe('Citizen authentication', () => { let page: Page - let account: CitizenWeakAccount + const credentials = { + username: 'test@example.com', + password: 'TestPassword456!' + } beforeEach(async () => { await resetServiceState() await Fixture.person(testAdult).saveAdult({ updateMockVtjWithDependants: [] }) - const keycloak = await KeycloakRealmClient.createCitizenClient() - await keycloak.deleteAllUsers() - - account = citizenWeakAccount(testAdult) - await keycloak.createUser({ ...account, enabled: true }) + await upsertWeakCredentials({ + id: testAdult.id, + body: credentials + }) page = await Page.open() }) @@ -39,7 +38,7 @@ describe('Citizen authentication', () => { ] as const, [ 'weak login', - async (page: Page) => enduserLoginWeak(page, account) + async (page: Page) => enduserLoginWeak(page, credentials) ] as const ] diff --git a/frontend/src/e2e-test/specs/0_citizen/citizen-child-page.spec.ts b/frontend/src/e2e-test/specs/0_citizen/citizen-child-page.spec.ts index 00ea967f1f..a776056257 100644 --- a/frontend/src/e2e-test/specs/0_citizen/citizen-child-page.spec.ts +++ b/frontend/src/e2e-test/specs/0_citizen/citizen-child-page.spec.ts @@ -24,20 +24,16 @@ import { import { createApplications, createDaycarePlacements, - resetServiceState + resetServiceState, + upsertWeakCredentials } from '../../generated/api-clients' import { DevPlacement } from '../../generated/api-types' import CitizenApplicationsPage from '../../pages/citizen/citizen-applications' import { CitizenChildPage } from '../../pages/citizen/citizen-children' import CitizenHeader from '../../pages/citizen/citizen-header' import { waitUntilEqual } from '../../utils' -import { KeycloakRealmClient } from '../../utils/keycloak' import { Page } from '../../utils/page' -import { - citizenWeakAccount, - enduserLogin, - enduserLoginWeak -} from '../../utils/user' +import { enduserLogin, enduserLoginWeak } from '../../utils/user' let page: Page @@ -614,11 +610,15 @@ describe.each(['desktop', 'mobile'] as const)( endDate: mockedDate }).save() - const keycloak = await KeycloakRealmClient.createCitizenClient() - await keycloak.deleteAllUsers() - const account = citizenWeakAccount(testAdult) - await keycloak.createUser({ ...account, enabled: true }) - await enduserLoginWeak(page, account) + const credentials = { + username: 'test@example.com', + password: 'TestPassword456!' + } + await upsertWeakCredentials({ + id: testAdult.id, + body: credentials + }) + await enduserLoginWeak(page, credentials) const header = new CitizenHeader(page, env) const childPage = new CitizenChildPage(page, env) diff --git a/frontend/src/e2e-test/specs/0_citizen/citizen-keycloak.spec.ts b/frontend/src/e2e-test/specs/0_citizen/citizen-keycloak.spec.ts deleted file mode 100644 index a58b23a021..0000000000 --- a/frontend/src/e2e-test/specs/0_citizen/citizen-keycloak.spec.ts +++ /dev/null @@ -1,154 +0,0 @@ -// SPDX-FileCopyrightText: 2017-2024 City of Espoo -// -// SPDX-License-Identifier: LGPL-2.1-or-later - -import libqp from 'libqp' - -import config from '../../config' -import { Fixture } from '../../dev-api/fixtures' -import { resetServiceState } from '../../generated/api-clients' -import { - DummySuomiFiConfirmPage, - DummySuomiFiLoginPage -} from '../../pages/citizen/citizen-dummy-suomifi' -import { - ConfirmPage, - ForgotPasswordPage, - KeycloakLoginPage, - UpdatePasswordPage -} from '../../pages/citizen/citizen-keycloak' -import { waitUntilEqual } from '../../utils' -import { - createSuomiFiUser, - deleteAllSuomiFiUsers -} from '../../utils/dummy-suomifi' -import { KeycloakRealmClient } from '../../utils/keycloak' -import { - deleteCapturedEmails, - getCapturedEmails, - Messages -} from '../../utils/mailhog' -import { Page } from '../../utils/page' - -let keycloak: KeycloakRealmClient - -beforeEach(async () => { - await resetServiceState() - await deleteCapturedEmails() - await deleteAllSuomiFiUsers() - keycloak = await KeycloakRealmClient.createCitizenClient() - await keycloak.deleteAllUsers() -}) - -test('Forgot my password', async () => { - const user = { - username: 'test@example.com', - enabled: true, - firstName: 'Seppo', - lastName: 'Sorsa', - email: 'test@example.com', - socialSecurityNumber: '070644-937X', - password: 'test123' - } - await Fixture.person({ - ssn: user.socialSecurityNumber, - firstName: user.firstName, - lastName: user.lastName - }).saveAdult({ updateMockVtjWithDependants: [] }) - await keycloak.createUser(user) - - const page = await Page.open() - await page.goto(config.enduserUrl) - await page.findByDataQa('weak-login').click() - - const loginPage = new KeycloakLoginPage(page) - await loginPage.forgotPasswordLink.click() - - const forgotPasswordPage = new ForgotPasswordPage(page) - await forgotPasswordPage.backToLoginLink.click() - await loginPage.forgotPasswordLink.click() - await forgotPasswordPage.email.fill(user.email) - await forgotPasswordPage.changePasswordButton.click() - - const emails = await getCapturedEmails() - const resetPasswordLink = extractResetLink(emails, user.email) - - await page.goto(resetPasswordLink) - const newPassword = 'eurie30ofeec' - const updatePasswordPage = new UpdatePasswordPage(page) - await updatePasswordPage.newPassword.fill(newPassword) - await updatePasswordPage.confirmPassword.fill(newPassword) - await updatePasswordPage.changePasswordButton.click() - - await page.findByDataQa('header-city-logo').waitUntilVisible() -}) - -function extractResetLink(emails: Messages, toAddress: string) { - expect(emails.count).toStrictEqual(1) - const email = emails.items[0] - expect(email.Raw.To).toEqual([toAddress]) - const textPart = email.MIME.Parts.find((part) => - part.Headers['Content-Type']?.some((headerValue) => - headerValue.startsWith('text/plain') - ) - ) - if (!textPart) { - throw new Error('No text/plain MIME part in e-mail') - } - const needsDecoding = textPart.Headers['Content-Transfer-Encoding'].some( - (headerValue) => headerValue === 'quoted-printable' - ) - const emailBody = needsDecoding - ? libqp.decode(textPart.Body).toString() - : textPart.Body - - const linkRegex = /(https?:\/\/\S+)/g - const linkMatches = linkRegex.exec(emailBody) - if (!linkMatches) { - throw new Error('Failed to find link in e-mail') - } - expect(linkMatches.length).toStrictEqual(2) - return linkMatches[1] -} - -test('Registration via suomi.fi', async () => { - const user = { - ssn: '010106A9953', - commonName: 'Eemeli Esimerkki', - givenName: 'Eemeli', - surname: 'Esimerkki' - } - await Fixture.person({ - ssn: user.ssn, - firstName: user.givenName, - lastName: user.surname - }).saveAdult({ updateMockVtjWithDependants: [] }) - const email = 'test@example.com' - await createSuomiFiUser(user) - - const page = await Page.open() - await page.goto(config.enduserUrl) - await page.findByDataQa('weak-login').click() - - const loginPage = new KeycloakLoginPage(page) - await loginPage.registerLink.click() - - const suomiFiLoginPage = new DummySuomiFiLoginPage(page) - await suomiFiLoginPage.userRadio(user.commonName).click() - await suomiFiLoginPage.loginButton.click() - - const suomiFiConfirmPage = new DummySuomiFiConfirmPage(page) - await suomiFiConfirmPage.proceedButton.click() - - const confirmPage = new ConfirmPage(page) - await confirmPage.heading.assertTextEquals('Luo uusi eVaka-tunnus') - // double-check we don't have any extra fields that shouldn't be there - await waitUntilEqual( - () => confirmPage.allLabels.allTexts(), - ['Sähköpostiosoite'] - ) - await confirmPage.email.fill(email) - await confirmPage.sendButton.click() - - await page.findByDataQa('header-city-logo').waitUntilVisible() -}) diff --git a/frontend/src/e2e-test/specs/0_citizen/citizen-personal-details.spec.ts b/frontend/src/e2e-test/specs/0_citizen/citizen-personal-details.spec.ts index ad26f20f53..60dae3dcbf 100644 --- a/frontend/src/e2e-test/specs/0_citizen/citizen-personal-details.spec.ts +++ b/frontend/src/e2e-test/specs/0_citizen/citizen-personal-details.spec.ts @@ -2,20 +2,15 @@ // // SPDX-License-Identifier: LGPL-2.1-or-later -import { testAdult, Fixture } from '../../dev-api/fixtures' +import { Fixture, testAdult } from '../../dev-api/fixtures' import { resetServiceState } from '../../generated/api-clients' import CitizenHeader from '../../pages/citizen/citizen-header' import CitizenPersonalDetailsPage, { CitizenNotificationSettingsSection, CitizenPersonalDetailsSection } from '../../pages/citizen/citizen-personal-details' -import { KeycloakRealmClient } from '../../utils/keycloak' import { Page } from '../../utils/page' -import { - citizenWeakAccount, - enduserLogin, - enduserLoginWeak -} from '../../utils/user' +import { enduserLogin } from '../../utils/user' let header: CitizenHeader let personalDetailsPage: CitizenPersonalDetailsPage @@ -97,22 +92,6 @@ describe('Citizen personal details', () => { }) }) -test('Citizen keycloak email is shown', async () => { - const account = citizenWeakAccount(citizenFixture) - const keycloak = await KeycloakRealmClient.createCitizenClient() - await keycloak.deleteAllUsers() - await keycloak.createUser({ ...account, enabled: true }) - await enduserLoginWeak(page, account) - header = new CitizenHeader(page) - - await header.selectTab('personal-details') - personalDetailsPage = new CitizenPersonalDetailsPage(page) - - await personalDetailsPage.loginDetailsSection.keycloakEmail.assertTextEquals( - account.username - ) -}) - describe('Citizen notification settings', () => { let section: CitizenNotificationSettingsSection diff --git a/frontend/src/e2e-test/specs/7_messaging/messaging-by-staff.spec.ts b/frontend/src/e2e-test/specs/7_messaging/messaging-by-staff.spec.ts index fafbff4a98..26e5f23f14 100644 --- a/frontend/src/e2e-test/specs/7_messaging/messaging-by-staff.spec.ts +++ b/frontend/src/e2e-test/specs/7_messaging/messaging-by-staff.spec.ts @@ -10,34 +10,28 @@ import LocalTime from 'lib-common/local-time' import config from '../../config' import { runPendingAsyncJobs } from '../../dev-api' import { - testDaycareGroup, - testChild2, - testAdult, Fixture, + testAdult, testAdult2, - testChild, testCareArea, - testDaycare + testChild, + testChild2, + testDaycare, + testDaycareGroup } from '../../dev-api/fixtures' import { createDaycareGroups, createMessageAccounts, insertGuardians, - resetServiceState + resetServiceState, + upsertWeakCredentials } from '../../generated/api-clients' import { DevEmployee, DevPerson } from '../../generated/api-types' import CitizenMessagesPage from '../../pages/citizen/citizen-messages' import MessagesPage from '../../pages/employee/messages/messages-page' import { waitUntilEqual } from '../../utils' -import { KeycloakRealmClient } from '../../utils/keycloak' import { Page } from '../../utils/page' -import { - CitizenWeakAccount, - citizenWeakAccount, - employeeLogin, - enduserLogin, - enduserLoginWeak -} from '../../utils/user' +import { employeeLogin, enduserLogin, enduserLoginWeak } from '../../utils/user' let staffPage: Page let unitSupervisorPage: Page @@ -45,7 +39,6 @@ let citizenPage: Page let childId: PersonId let staff: DevEmployee let unitSupervisor: DevEmployee -let account: CitizenWeakAccount const mockedDate = LocalDate.of(2020, 5, 21) const mockedDateAt10 = HelsinkiDateTime.fromLocal( @@ -60,6 +53,10 @@ const mockedDateAt12 = HelsinkiDateTime.fromLocal( mockedDate, LocalTime.of(12, 17) ) +const credentials = { + username: 'test@example.com', + password: 'TestPassword456!' +} beforeEach(async () => { await resetServiceState() await Fixture.careArea(testCareArea).save() @@ -70,10 +67,10 @@ beforeEach(async () => { }).save() await createDaycareGroups({ body: [testDaycareGroup] }) - const keycloak = await KeycloakRealmClient.createCitizenClient() - await keycloak.deleteAllUsers() - account = citizenWeakAccount(testAdult) - await keycloak.createUser({ ...account, enabled: true }) + await upsertWeakCredentials({ + id: testAdult.id, + body: credentials + }) staff = await Fixture.employee() .staff(testDaycare.id) @@ -161,7 +158,7 @@ async function initOtherCitizenPage( async function initCitizenPageWeak(mockedTime: HelsinkiDateTime) { citizenPage = await Page.open({ mockedTime }) - await enduserLoginWeak(citizenPage, account) + await enduserLoginWeak(citizenPage, credentials) } const defaultReply = 'Testivastaus testiviestiin' diff --git a/frontend/src/e2e-test/specs/7_messaging/messaging.spec.ts b/frontend/src/e2e-test/specs/7_messaging/messaging.spec.ts index 478ba13fd9..96edce50f6 100644 --- a/frontend/src/e2e-test/specs/7_messaging/messaging.spec.ts +++ b/frontend/src/e2e-test/specs/7_messaging/messaging.spec.ts @@ -16,17 +16,17 @@ import LocalTime from 'lib-common/local-time' import config from '../../config' import { runPendingAsyncJobs } from '../../dev-api' import { - testDaycare2, - testDaycareGroup, - testChild2, Fixture, - testAdult2, testAdult, + testAdult2, + testCareArea, testChild, - testDaycarePrivateVoucher, + testChild2, + testChildRestricted, testDaycare, - testCareArea, - testChildRestricted + testDaycare2, + testDaycareGroup, + testDaycarePrivateVoucher } from '../../dev-api/fixtures' import { addAclRoleForDaycare, @@ -34,7 +34,8 @@ import { createDaycareGroups, createMessageAccounts, insertGuardians, - resetServiceState + resetServiceState, + upsertWeakCredentials } from '../../generated/api-clients' import { DevCareArea, @@ -44,21 +45,13 @@ import { import CitizenMessagesPage from '../../pages/citizen/citizen-messages' import MessagesPage from '../../pages/employee/messages/messages-page' import { waitUntilEqual } from '../../utils' -import { KeycloakRealmClient } from '../../utils/keycloak' import { Page } from '../../utils/page' -import { - CitizenWeakAccount, - citizenWeakAccount, - employeeLogin, - enduserLogin, - enduserLoginWeak -} from '../../utils/user' +import { employeeLogin, enduserLogin, enduserLoginWeak } from '../../utils/user' let unitSupervisorPage: Page let citizenPage: Page let childId: PersonId let unitSupervisor: DevEmployee -let account: CitizenWeakAccount let careArea: DevCareArea let daycarePlacementFixture: DevPlacement let backupDaycareId: DaycareId @@ -78,6 +71,10 @@ const mockedDateAt12 = HelsinkiDateTime.fromLocal( LocalTime.of(12, 17) ) +const credentials = { + username: 'test@example.com', + password: 'TestPassword456!' +} beforeEach(async () => { await resetServiceState() await Fixture.careArea(testCareArea).save() @@ -94,10 +91,10 @@ beforeEach(async () => { careArea = testCareArea await createDaycareGroups({ body: [testDaycareGroup] }) - const keycloak = await KeycloakRealmClient.createCitizenClient() - await keycloak.deleteAllUsers() - account = citizenWeakAccount(testAdult) - await keycloak.createUser({ ...account, enabled: true }) + await upsertWeakCredentials({ + id: testAdult.id, + body: credentials + }) unitSupervisor = await Fixture.employee({ firstName: 'Essi', @@ -172,7 +169,7 @@ async function openCitizenPageWeak(mockedTime: HelsinkiDateTime) { citizenPage = await Page.open({ mockedTime: mockedTime }) - await enduserLoginWeak(citizenPage, account) + await enduserLoginWeak(citizenPage, credentials) } const defaultTitle = 'Otsikko' @@ -840,7 +837,7 @@ describe('Sending and receiving messages', () => { citizenPage = await Page.open({ mockedTime: mockedDateAt10 }) - await enduserLoginWeak(citizenPage, account) + await enduserLoginWeak(citizenPage, credentials) await citizenPage.goto(config.enduserMessagesUrl) await citizenPage.page.evaluate(() => { if (window.evaka) window.evaka.keepSessionAliveThrottleTime = 300 diff --git a/frontend/src/e2e-test/utils/keycloak.ts b/frontend/src/e2e-test/utils/keycloak.ts deleted file mode 100644 index 8a043ec24a..0000000000 --- a/frontend/src/e2e-test/utils/keycloak.ts +++ /dev/null @@ -1,184 +0,0 @@ -// SPDX-FileCopyrightText: 2017-2024 City of Espoo -// -// SPDX-License-Identifier: LGPL-2.1-or-later - -const keycloakBaseUrl = 'http://localhost:8080' - -export class KeycloakRealmClient { - readonly #baseUrl: string - readonly #headers: object - - constructor( - private realm: string, - private accessToken: string - ) { - this.#baseUrl = `${keycloakBaseUrl}/auth/admin/realms/${encodeURIComponent(this.realm)}` - this.#headers = { - Authorization: `Bearer ${this.accessToken}` - } - } - - static async create({ - realm - }: { - realm: string - }): Promise { - const accessToken = await acquireAccessToken() - return new KeycloakRealmClient(realm, accessToken) - } - - static async createCitizenClient(): Promise { - return KeycloakRealmClient.create({ realm: 'evaka-customer' }) - } - - async getUsers(): Promise { - const res = await fetch(`${this.#baseUrl}/users`, { - method: 'GET', - headers: { - ...this.#headers, - Accept: 'application/json' - } - }) - if (!res.ok) { - throw new Error( - `Failed to get users: ${res.status} ${res.statusText}: ${await res.text()}` - ) - } - // eslint-disable-next-line @typescript-eslint/no-unsafe-return - return res.json() - } - - async deleteUser(userId: string): Promise { - const res = await fetch( - `${this.#baseUrl}/users/${encodeURIComponent(userId)}`, - { - method: 'DELETE', - headers: { - ...this.#headers, - Accept: 'application/json' - } - } - ) - if (!res.ok) { - throw new Error( - `Failed to delete user: ${res.status} ${res.statusText}: ${await res.text()}` - ) - } - } - async createUser(user: { - username: string - enabled: boolean - firstName: string - lastName: string - email: string - socialSecurityNumber: string - password: string - }): Promise { - const res = await fetch(`${this.#baseUrl}/users`, { - method: 'POST', - headers: { - ...this.#headers, - Accept: 'application/json', - 'Content-Type': 'application/json' - }, - body: JSON.stringify({ - username: user.username, - enabled: user.enabled, - firstName: user.firstName, - lastName: user.lastName, - email: user.email, - attributes: { - suomi_sn: user.lastName, - suomi_givenName: user.firstName, - emailConfirm: user.email, - suomi_nationalIdentificationNumber: user.socialSecurityNumber - } - }) - }) - if (!res.ok) { - throw new Error( - `Failed to create user: ${res.status} ${res.statusText}: ${await res.text()}` - ) - } - - const users = await this.getUsers() - const userId = users.find(({ username }) => username === user.username)?.id - if (!userId) { - throw new Error('User not found') - } - await this.resetUserPassword({ - userId, - password: user.password - }) - } - - async resetUserPassword({ - userId, - password - }: { - userId: string - password: string - }): Promise { - const res = await fetch( - `${this.#baseUrl}/users/${encodeURIComponent(userId)}/reset-password`, - { - method: 'PUT', - headers: { - ...this.#headers, - Accept: 'application/json', - 'Content-Type': 'application/json' - }, - body: JSON.stringify({ - type: 'password', - value: password - }) - } - ) - if (!res.ok) { - throw new Error( - `Failed to reset user password: ${res.status} ${res.statusText}: ${await res.text()}` - ) - } - } - - async deleteAllUsers() { - const users = await this.getUsers() - for (const user of users) { - await this.deleteUser(user.id) - } - } -} - -export async function acquireAccessToken(): Promise { - const res = await fetch( - `${keycloakBaseUrl}/auth/realms/master/protocol/openid-connect/token`, - { - method: 'POST', - headers: { - Accept: 'application/json', - 'Content-Type': 'application/x-www-form-urlencoded' - }, - body: new URLSearchParams({ - username: 'admin', - password: 'admin', - grant_type: 'password', - client_id: 'admin-cli' - }) - } - ) - if (!res.ok) { - throw new Error( - `Failed to acquire access token: ${res.status} ${res.statusText}: ${await res.text()}` - ) - } - // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment - const body = await res.json() - // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access,@typescript-eslint/no-unsafe-return - return body.access_token -} - -export interface User { - id: string - username: string - // ...other fields -} diff --git a/frontend/src/e2e-test/utils/user.ts b/frontend/src/e2e-test/utils/user.ts index 3273bbfb47..218aa37c86 100644 --- a/frontend/src/e2e-test/utils/user.ts +++ b/frontend/src/e2e-test/utils/user.ts @@ -39,34 +39,18 @@ export async function enduserLogin(page: Page, person: DevPerson) { await page.goto(config.enduserUrl + '/applications') } -export type CitizenWeakAccount = { - username: string - password: string - email: string - socialSecurityNumber: string - firstName: string - lastName: string -} - -export const citizenWeakAccount = (person: DevPerson): CitizenWeakAccount => ({ - username: 'test@example.com', - password: 'test123', - email: 'test@example.com', - socialSecurityNumber: person.ssn!, - firstName: 'Seppo', - lastName: 'Sorsa' -}) - export async function enduserLoginWeak( page: Page, - account: CitizenWeakAccount + credentials: { username: string; password: string } ) { await page.goto(config.enduserLoginUrl) await page.findByDataQa('weak-login').click() - await new TextInput(page.find('[id="username"]')).fill(account.username) - await new TextInput(page.find('[id="password"]')).fill(account.password) - await page.find('[id="kc-login"]').click() + const form = page.findByDataQa('weak-login-form') + await new TextInput(form.find('[id="username"]')).fill(credentials.username) + await new TextInput(form.find('[id="password"]')).fill(credentials.password) + await form.findByDataQa('login').click() + await form.findByDataQa('login').waitUntilHidden() await page.findByDataQa('header-city-logo').waitUntilVisible() } From b5bc7e526f62f28fb3f670b5296328e4b495e77e Mon Sep 17 00:00:00 2001 From: Joonas Javanainen Date: Mon, 27 Jan 2025 13:04:09 +0200 Subject: [PATCH 3/3] Lift weak login rate limit in local dev / e2e tests --- apigw/src/app.ts | 6 +++- apigw/src/enduser/routes/auth-weak-login.ts | 31 +++++++++++---------- apigw/src/shared/config.ts | 16 +++++++++-- 3 files changed, 35 insertions(+), 18 deletions(-) diff --git a/apigw/src/app.ts b/apigw/src/app.ts index c8e4592900..1e591cc8f2 100644 --- a/apigw/src/app.ts +++ b/apigw/src/app.ts @@ -315,7 +315,11 @@ export function apiRouter(config: Config, redisClient: RedisClient) { router.post( '/citizen/auth/weak-login', express.json(), - authWeakLogin(citizenSessions, redisClient) + authWeakLogin( + citizenSessions, + config.citizen.weakLoginRateLimit, + redisClient + ) ) router.use('/citizen/public/map-api', mapRoutes) router.all('/citizen/public/*', citizenProxy) diff --git a/apigw/src/enduser/routes/auth-weak-login.ts b/apigw/src/enduser/routes/auth-weak-login.ts index 87471ecac8..3473382b82 100644 --- a/apigw/src/enduser/routes/auth-weak-login.ts +++ b/apigw/src/enduser/routes/auth-weak-login.ts @@ -23,10 +23,9 @@ const Request = z.object({ const eventCode = (name: string) => `evaka.citizen_weak.${name}` -const loginAttemptsPerHour = 20 - export const authWeakLogin = ( sessions: Sessions<'citizen'>, + loginAttemptsPerHour: number, redis: RedisClient ) => toRequestHandler(async (req, res) => { @@ -34,19 +33,21 @@ export const authWeakLogin = ( try { const body = Request.parse(req.body) - // Apply rate limit (attempts per hour) - // Reference: Redis Rate Limiting Best Practices - // https://redis.io/glossary/rate-limiting/ - const hour = getHours(new Date()) - const key = `citizen-weak-login:${body.username}:${hour}` - const value = Number.parseInt((await redis.get(key)) ?? '', 10) - if (Number.isNaN(value) || value < loginAttemptsPerHour) { - // expire in 1 hour, so there's no old entry when the hours value repeats the next day - const expirySeconds = 60 * 60 - await redis.multi().incr(key).expire(key, expirySeconds).exec() - } else { - res.sendStatus(429) - return + if (loginAttemptsPerHour > 0) { + // Apply rate limit (attempts per hour) + // Reference: Redis Rate Limiting Best Practices + // https://redis.io/glossary/rate-limiting/ + const hour = getHours(new Date()) + const key = `citizen-weak-login:${body.username}:${hour}` + const value = Number.parseInt((await redis.get(key)) ?? '', 10) + if (Number.isNaN(value) || value < loginAttemptsPerHour) { + // expire in 1 hour, so there's no old entry when the hours value repeats the next day + const expirySeconds = 60 * 60 + await redis.multi().incr(key).expire(key, expirySeconds).exec() + } else { + res.sendStatus(429) + return + } } const { id } = await citizenWeakLogin(body) diff --git a/apigw/src/shared/config.ts b/apigw/src/shared/config.ts index f3183326fb..f8c3f95d66 100644 --- a/apigw/src/shared/config.ts +++ b/apigw/src/shared/config.ts @@ -73,6 +73,13 @@ const envVariables = { */ PIN_SESSION_TIMEOUT_SECONDS: 10 * 60, + /** + * Rate limit for citizen weak logins per username (attempts per hour). + * + * 0 means no limit + */ + CITIZEN_WEAK_LOGIN_RATE_LIMIT: 20, + // ----- Redis configuration ----- /** * Redis server hostname @@ -415,6 +422,7 @@ function createLocalDevelopmentOverrides(): Partial { CITIZEN_COOKIE_SECRET: 'A very hush hush cookie secret.', EMPLOYEE_COOKIE_SECRET: 'A very hush hush cookie secret.', USE_SECURE_COOKIES: false, + CITIZEN_WEAK_LOGIN_RATE_LIMIT: 0, REDIS_HOST: '127.0.0.1', REDIS_PORT: 6379, @@ -456,7 +464,7 @@ function createLocalDevelopmentOverrides(): Partial { } export interface Config { - citizen: SessionConfig + citizen: SessionConfig & { weakLoginRateLimit: number } employee: SessionConfig ad: | { type: 'mock' | 'disabled' } @@ -739,7 +747,11 @@ export function configFromEnv(): Config { ), sessionTimeoutMinutes: optional('CITIZEN_SESSION_TIMEOUT_MINUTES', parseInteger) ?? - defaultSessionTimeoutMinutes + defaultSessionTimeoutMinutes, + weakLoginRateLimit: required( + 'CITIZEN_WEAK_LOGIN_RATE_LIMIT', + parseInteger + ) }, employee: { useSecureCookies,