Merge pull request #6348 from espoon-voltti/auth-small-fixes-part-3

Kirjautumisen pikkukorjauksia, osa 3
espoon-voltti · Feb 6, 2025 · fd1410b · fd1410b
2 parents 4e73968 + 3c9bf83
commit fd1410b
Show file tree

Hide file tree

Showing 9 changed files with 41 additions and 9 deletions.
diff --git a/apigw/src/enduser/routes/auth-weak-login.ts b/apigw/src/enduser/routes/auth-weak-login.ts
@@ -7,7 +7,7 @@ import { z } from 'zod'
 
 import { EvakaSessionUser } from '../../shared/auth/index.js'
 import { toRequestHandler } from '../../shared/express.js'
-import { logAuditEvent } from '../../shared/logging.js'
+import { logAuditEvent, logWarn } from '../../shared/logging.js'
 import { RedisClient } from '../../shared/redis-client.js'
 import { citizenWeakLogin } from '../../shared/service-client.js'
 import { Sessions } from '../../shared/session.js'
@@ -45,6 +45,9 @@ export const authWeakLogin = (
           const expirySeconds = 60 * 60
           await redis.multi().incr(key).expire(key, expirySeconds).exec()
         } else {
+          logWarn('Login request hit rate limit', req, {
+            username: body.username
+          })
           res.sendStatus(429)
           return
         }

diff --git a/frontend/src/citizen-frontend/personal-details/LoginDetailsSection.tsx b/frontend/src/citizen-frontend/personal-details/LoginDetailsSection.tsx
@@ -374,7 +374,9 @@ const WeakCredentialsFormModal = React.memo(function WeakCredentialsFormModal({
               message={t.unacceptablePassword}
             />
           )}
-          {isUsernameConflict && <AlertBox message={t.usernameConflict} />}
+          {isUsernameConflict && (
+            <AlertBox message={t.usernameConflict(username)} />
+          )}
         </FixedSpaceColumn>
       </form>
     </MutateFormModal>

diff --git a/frontend/src/citizen-frontend/personal-details/PersonalDetailsSection.tsx b/frontend/src/citizen-frontend/personal-details/PersonalDetailsSection.tsx
@@ -3,9 +3,10 @@
 // SPDX-License-Identifier: LGPL-2.1-or-later
 
 import { FontAwesomeIcon } from '@fortawesome/react-fontawesome'
-import React, { useCallback, useContext } from 'react'
+import React, { useCallback, useContext, useState } from 'react'
 import styled from 'styled-components'
 
+import { Failure } from 'lib-common/api'
 import { boolean, string } from 'lib-common/form/fields'
 import {
   chained,
@@ -507,6 +508,14 @@ const EmailVerificationForm = React.memo(function EmailVerificationForm({
   )
   const verificationCode = useFormField(form, 'verificationCode')
 
+  const [isUsernameConflict, setUsernameConflict] = useState<boolean>(false)
+  const onFailure = useCallback(
+    (failure: Failure<unknown>) => {
+      setUsernameConflict(failure.statusCode === 409)
+    },
+    [setUsernameConflict]
+  )
+
   return (
     <FixedSpaceColumn>
       <LabelLike>
@@ -541,11 +550,18 @@ const EmailVerificationForm = React.memo(function EmailVerificationForm({
           close={closeInfo}
         />
       )}
+      {isUsernameConflict && (
+        <AlertBox
+          message={i18n.personalDetails.loginDetailsSection.usernameConflict(
+            verification.email
+          )}
+        />
+      )}
       <Gap size="m" />
       <MutateButton
         data-qa="verify-email"
         primary
-        disabled={!form.isValid()}
+        disabled={!form.isValid() || isUsernameConflict}
         text={t.confirmVerification}
         mutation={verifyEmailMutation}
         onClick={() => ({
@@ -562,6 +578,7 @@ const EmailVerificationForm = React.memo(function EmailVerificationForm({
               : t.verifyEmail.toast
           })
         }}
+        onFailure={onFailure}
       />
     </FixedSpaceColumn>
   )

diff --git a/frontend/src/lib-customizations/defaults/citizen/i18n/en.tsx b/frontend/src/lib-customizations/defaults/citizen/i18n/en.tsx
@@ -1889,7 +1889,8 @@ const en: Translations = {
           `at least ${v} ${v > 1 ? 'special characters' : 'special character'}`
       },
       unacceptablePassword: 'The password is too easy to guess',
-      usernameConflict: 'The username is already in use'
+      usernameConflict: (username: string): ReactNode =>
+        `The username ${username} is already in use by another person`
     },
     notificationsSection: {
       title: 'Email notifications',

diff --git a/frontend/src/lib-customizations/defaults/citizen/i18n/fi.tsx b/frontend/src/lib-customizations/defaults/citizen/i18n/fi.tsx
@@ -2129,7 +2129,8 @@ export default {
           `vähintään ${v} ${v > 1 ? 'erikoismerkkiä' : 'erikoismerkki'}`
       },
       unacceptablePassword: 'Salasana on liian helposti arvattava',
-      usernameConflict: 'Käyttäjätunnus on jo käytössä'
+      usernameConflict: (username: string): ReactNode =>
+        `Käyttäjätunnus ${username} on jo käytössä toisella henkilöllä`
     },
     notificationsSection: {
       title: 'Sähköposti-ilmoitukset',

diff --git a/frontend/src/lib-customizations/defaults/citizen/i18n/sv.tsx b/frontend/src/lib-customizations/defaults/citizen/i18n/sv.tsx
@@ -2127,7 +2127,8 @@ const sv: Translations = {
           `${v} eller ${v > 1 ? 'flera specialtecken' : 'fler specialtecken'}`
       },
       unacceptablePassword: 'Lösenordet är för lätt att gissa',
-      usernameConflict: 'Användarnamnet är redan i bruk'
+      usernameConflict: (username: string): ReactNode =>
+        `Användarnamnet ${username} används redan av en annan person`
     },
     notificationsSection: {
       title: 'E-postmeddelanden',

diff --git a/service/src/main/kotlin/fi/espoo/evaka/pis/SystemController.kt b/service/src/main/kotlin/fi/espoo/evaka/pis/SystemController.kt
@@ -114,6 +114,7 @@ class SystemController(
                     }
 
                     tx.updateLastWeakLogin(clock, citizen.id)
+                    tx.updateCitizenOnLogin(clock, citizen.id, keycloakEmail = null)
                     personService.getPersonWithChildren(tx, user, citizen.id)
                     CitizenUserIdentity(citizen.id)
                 }

diff --git a/service/src/main/kotlin/fi/espoo/evaka/pis/controllers/PersonalDataControllerCitizen.kt b/service/src/main/kotlin/fi/espoo/evaka/pis/controllers/PersonalDataControllerCitizen.kt
@@ -25,6 +25,7 @@ import fi.espoo.evaka.shared.utils.EMAIL_PATTERN
 import fi.espoo.evaka.shared.utils.PHONE_PATTERN
 import fi.espoo.evaka.user.hasWeakCredentials
 import fi.espoo.evaka.user.updateWeakLoginCredentials
+import io.github.oshai.kotlinlogging.KotlinLogging
 import java.security.SecureRandom
 import java.time.Duration
 import kotlin.random.asKotlinRandom
@@ -44,6 +45,7 @@ class PersonalDataControllerCitizen(
     private val passwordSpecification: PasswordSpecification,
 ) {
     private val secureRandom = SecureRandom()
+    private val logger = KotlinLogging.logger {}
 
     @PutMapping
     fun updatePersonalData(
@@ -270,7 +272,11 @@ class PersonalDataControllerCitizen(
                     user.id,
                 )
                 tx.verifyAndUpdateEmail(clock.now(), user.id, request.id, request.code)
-                tx.syncWeakLoginUsername(clock.now(), user.id)
+                try {
+                    tx.syncWeakLoginUsername(clock.now(), user.id)
+                } catch (e: Exception) {
+                    throw mapPSQLException(e)
+                }
             }
         }
         Audit.CitizenVerifyEmail.log(targetId = AuditId(request.id))

diff --git a/service/src/main/kotlin/fi/espoo/evaka/shared/job/ScheduledJobs.kt b/service/src/main/kotlin/fi/espoo/evaka/shared/job/ScheduledJobs.kt
@@ -230,7 +230,7 @@ enum class ScheduledJob(
         ScheduledJobs::generateReplacementDraftInvoices,
         ScheduledJobSettings(enabled = true, schedule = JobSchedule.daily(LocalTime.of(4, 15))),
     ),
-    ImportPasswordsBlacklists(
+    ImportPasswordBlacklists(
         ScheduledJobs::importPasswordBlacklists,
         ScheduledJobSettings(enabled = true, schedule = JobSchedule.daily(LocalTime.of(0, 20))),
     ),