Merge pull request #96 from VAuthenticator/feedback-on-login-error

put error messages in login phase

src/main/frontend/app/component/ErrorBanner.js

@@ -0,0 +1,11 @@
+import {Alert, Grid} from "@mui/material";
+import React from "react";
+
+const ErrorBanner = ({errorMessage}) => {
+    return <Grid style={{marginTop: '10px'}}>
+        <Alert severity="error">{errorMessage}</Alert>
+    </Grid>
+
+}
+
+export default ErrorBanner

src/main/frontend/app/login/LoginPage.js

@@ -7,6 +7,7 @@ import FormInputTextField from "../component/FormInputTextField";
 import {Fingerprint, Person} from "@mui/icons-material";
 import Separator from "../component/Separator";
 import FormButton from "../component/FormButton";
+import ErrorBanner from "../component/ErrorBanner";
 
 
 const VAuthenticatorTitle = () => {
@@ -88,17 +89,18 @@ c-111 1 -132 4 -194 28 -38 15 -96 43 -128 62 -32 19 -64 35 -70 35 -7 0 -30
     </svg>
 }
 
-const Login = (props) => {
-    const {rawFeatures} = props;
+const Login = ({rawFeatures, rawErrors}) => {
     let signUpLink = <div>
         <h3>are you not registered? if you want you can register <a href="/sign-up">here</a></h3>
     </div>
     let resetPasswordLink = <div>
-        <h3>do you have forgot your password? please click <a href='/reset-password/reset-password-challenge-sender'>here</a> to recover
-            your
-            password</h3>
+        <h3>do you have forgot your password? please click
+            <a href='/reset-password/reset-password-challenge-sender'>here</a> to recover your password
+        </h3>
     </div>
     let features = JSON.parse(rawFeatures);
+    let errorMessage = JSON.parse(rawErrors)["login"];
+    const errorsBanner = <ErrorBanner errorMessage={errorMessage}/>
 
     return (
         <ThemeProvider theme={theme}>
@@ -108,6 +110,8 @@ const Login = (props) => {
                 <Grid style={{marginTop: '10px'}}>
                     <Divider/>
                 </Grid>
+                {errorMessage ? errorsBanner : ""}
+
 
                 {<form action="login" method="post">
                     <Box>
@@ -145,5 +149,6 @@ const Login = (props) => {
 
 if (document.getElementById('app')) {
     let features = document.getElementById('features').innerHTML
-    ReactDOM.render(<Login rawFeatures={features}/>, document.getElementById('app'));
+    let errors = document.getElementById('errors').innerHTML
+    ReactDOM.render(<Login rawFeatures={features} rawErrors={errors}/>, document.getElementById('app'));
 }

src/main/frontend/app/mfa/index.js

@@ -7,14 +7,19 @@ import Separator from "../component/Separator";
 import FormButton from "../component/FormButton";
 import React from "react";
 import ReactDOM from "react-dom";
+import ErrorBanner from "../component/ErrorBanner";
 
-const MfaChallengePage = (props) => {
+const MfaChallengePage = ({rawErrors}) => {
     let sendAgainMfaCode = () => {
         fetch("/mfa-challenge/send", {
             method: 'PUT', // *GET, POST, PUT, DELETE, etc.
             credentials: 'same-origin', // include, *same-origin, omit
         });
     }
+
+    let errorMessage = JSON.parse(rawErrors)["mfa-challenge"];
+    const errorsBanner = <ErrorBanner errorMessage={errorMessage}/>
+
     return (
         <ThemeProvider theme={theme}>
 
@@ -26,6 +31,7 @@ const MfaChallengePage = (props) => {
                 <Grid style={{marginTop: '10px'}}>
                     <Divider/>
                 </Grid>
+                {errorMessage ? errorsBanner : ""}
 
                 {<form action="mfa-challenge" method="post">
                     <Box>
@@ -49,6 +55,7 @@ const MfaChallengePage = (props) => {
 
 
 if (document.getElementById('app')) {
+    let errors = document.getElementById('errors').innerHTML
     let features = document.getElementById('features').innerHTML
-    ReactDOM.render(<MfaChallengePage rawFeatures={features}/>, document.getElementById('app'));
+    ReactDOM.render(<MfaChallengePage rawFeatures={features} rawErrors={errors}/>, document.getElementById('app'));
 }

src/main/kotlin/com/vauthenticator/server/config/WebSecurityConfig.kt

@@ -64,7 +64,7 @@ class WebSecurityConfig(
         http.csrf().disable().headers().frameOptions().disable()
 
         http.formLogin()
-            .successHandler(MfaAuthenticationHandler(clientApplicationRepository, "/mfa-challenge"))
+            .successHandler(MfaAuthenticationHandler(clientApplicationRepository, "/mfa-challenge/send"))
             .loginProcessingUrl(LOG_IN_URL_PAGE)
             .loginPage(LOG_IN_URL_PAGE)
             .permitAll()

src/main/kotlin/com/vauthenticator/server/login/LoginPageController.kt

@@ -6,13 +6,15 @@ import com.vauthenticator.server.oauth2.clientapp.ClientAppId
 import com.vauthenticator.server.oauth2.clientapp.ClientApplicationFeatures
 import com.vauthenticator.server.oauth2.clientapp.ClientApplicationRepository
 import com.vauthenticator.server.oauth2.clientapp.Scope
+import jakarta.servlet.http.HttpServletRequest
 import jakarta.servlet.http.HttpSession
 import org.slf4j.Logger
 import org.slf4j.LoggerFactory
 import org.springframework.stereotype.Controller
 import org.springframework.ui.Model
 import org.springframework.web.bind.annotation.GetMapping
 import org.springframework.web.bind.annotation.SessionAttributes
+import java.util.*
 
 
 @Controller
@@ -24,10 +26,26 @@ class LoginPageController(
     val logger: Logger = LoggerFactory.getLogger(LoginPageController::class.java)
 
     @GetMapping("/login")
-    fun loginPage(session: HttpSession, model: Model): String {
+    fun loginPage(session: HttpSession, model: Model, httpServletRequest: HttpServletRequest): String {
         val clientId = session.oauth2ClientId()
 
         val features = defaultFeature()
+        clientAppFeaturesFor(clientId, model, features)
+
+        val errors = errorMessageFor(httpServletRequest)
+
+        model.addAttribute("errors", objectMapper.writeValueAsString(errors))
+        model.addAttribute("features", objectMapper.writeValueAsString(features))
+        model.addAttribute("assetBundle", "login_bundle.js")
+
+        return "template"
+    }
+
+    private fun clientAppFeaturesFor(
+        clientId: Optional<String>,
+        model: Model,
+        features: MutableMap<String, Boolean>
+    ) {
         clientId.ifPresent {
             model.addAttribute("clientId", it)
             clientApplicationRepository.findOne(ClientAppId(it))
@@ -38,12 +56,20 @@ class LoginPageController(
                         clientApp.scopes.content.contains(Scope.RESET_PASSWORD)
                 }
         }
-
-        model.addAttribute("features", objectMapper.writeValueAsString(features))
-        model.addAttribute("assetBundle", "login_bundle.js")
-        return "template"
     }
 
+    private fun errorMessageFor(httpServletRequest: HttpServletRequest) =
+        if (hasBadLoginFrom(httpServletRequest)) {
+            mapOf("login" to "Something goes wrong during your login request")
+        } else {
+            emptyMap()
+        }
+
+    private fun hasBadLoginFrom(httpServletRequest: HttpServletRequest) =
+        !Optional.ofNullable(httpServletRequest.session.getAttribute("SPRING_SECURITY_LAST_EXCEPTION")).isEmpty
+                && httpServletRequest.parameterMap.contains("error")
+
+
     private fun defaultFeature() =
         mutableMapOf(
             ClientApplicationFeatures.SIGNUP.value to false,

src/main/kotlin/com/vauthenticator/server/login/userdetails/AccountUserDetailsService.kt

@@ -11,7 +11,7 @@ import org.springframework.security.core.userdetails.UsernameNotFoundException
 open class AccountUserDetailsService(private val userRepository: AccountRepository) : UserDetailsService {
     private val logger: Logger = LoggerFactory.getLogger(AccountUserDetailsService::class.java)
 
-    override fun loadUserByUsername(username: String) =
+    override fun loadUserByUsername(username: String): User =
             userRepository.accountFor(username)
                     .map {
                         logger.info("Account found for $username username")

src/main/kotlin/com/vauthenticator/server/mfa/MfaController.kt

@@ -1,5 +1,6 @@
 package com.vauthenticator.server.mfa
 
+import com.fasterxml.jackson.databind.ObjectMapper
 import jakarta.servlet.http.HttpServletRequest
 import jakarta.servlet.http.HttpServletResponse
 import org.slf4j.LoggerFactory
@@ -9,26 +10,46 @@ import org.springframework.security.web.authentication.AuthenticationSuccessHand
 import org.springframework.stereotype.Controller
 import org.springframework.ui.Model
 import org.springframework.web.bind.annotation.*
+import java.util.*
 
 @Controller
 class MfaController(
+    private val objectMapper: ObjectMapper,
     private val successHandler: AuthenticationSuccessHandler,
     private val otpMfaSender: OtpMfaSender,
     private val otpMfaVerifier: OtpMfaVerifier
 ) {
 
     private val logger = LoggerFactory.getLogger(MfaController::class.java)
 
+    @GetMapping("/mfa-challenge/send")
+    fun view(authentication: Authentication): String {
+        otpMfaSender.sendMfaChallenge(authentication.name)
+        return "redirect:/mfa-challenge"
+    }
+
     @GetMapping("/mfa-challenge")
     fun view(
+        model: Model,
         authentication: Authentication,
-        model: Model
+        httpServletRequest: HttpServletRequest
     ): String {
-        otpMfaSender.sendMfaChallenge(authentication.name)
+        val errors = errorMessageFor(httpServletRequest)
+        model.addAttribute("errors", objectMapper.writeValueAsString(errors))
+
         model.addAttribute("assetBundle", "mfa_bundle.js")
         return "template"
     }
 
+    private fun errorMessageFor(httpServletRequest: HttpServletRequest) =
+        if (hasBadLoginFrom(httpServletRequest)) {
+            mapOf("mfa-challenge" to "Ops! the MFA code provided is wrong or expired")
+        } else {
+            emptyMap()
+        }
+
+    private fun hasBadLoginFrom(httpServletRequest: HttpServletRequest) =
+        !Optional.ofNullable(httpServletRequest.session.getAttribute("MFA_SPRING_SECURITY_LAST_EXCEPTION")).isEmpty
 
     @PostMapping("/mfa-challenge")
     fun processSecondFactor(
@@ -44,6 +65,8 @@ class MfaController(
             successHandler.onAuthenticationSuccess(request, response, authentication.delegate)
         } catch (e: Exception) {
             logger.error(e.message, e)
+            request.session.setAttribute("MFA_SPRING_SECURITY_LAST_EXCEPTION", MfaException("Invalid mfa code"))
+
             response.sendRedirect("/mfa-challenge?error")
         }
     }

src/main/resources/templates/template.html

@@ -9,6 +9,7 @@
 </head>
 <body>
 
+<div id="errors" th:text="${errors}" style="display: none"></div>
 <div id="features" th:text="${features}" style="display: none"></div>
 <div id="metadata" th:text="${metadata}" style="display: none"></div>
 

src/test/kotlin/com/vauthenticator/server/clientapp/DynamoDbClientApplicationRepositoryTest.kt

@@ -7,73 +7,69 @@ import com.vauthenticator.server.oauth2.clientapp.DynamoDbClientApplicationRepos
 import com.vauthenticator.server.support.DatabaseUtils.dynamoClientApplicationTableName
 import com.vauthenticator.server.support.DatabaseUtils.dynamoDbClient
 import com.vauthenticator.server.support.DatabaseUtils.resetDatabase
-import org.junit.jupiter.api.AfterEach
-import org.junit.jupiter.api.Assertions
+import org.junit.jupiter.api.Assertions.assertEquals
 import org.junit.jupiter.api.BeforeEach
 import org.junit.jupiter.api.Test
 import java.util.*
 
 internal class DynamoDbClientApplicationRepositoryTest {
 
-    lateinit var dynamoDbClientApplicationRepository: DynamoDbClientApplicationRepository
+    private lateinit var underTest: DynamoDbClientApplicationRepository
 
     @BeforeEach
     fun setUp() {
-        dynamoDbClientApplicationRepository =
-                DynamoDbClientApplicationRepository(dynamoDbClient, dynamoClientApplicationTableName)
-    }
-
-    @AfterEach
-    fun tearDown() {
+        underTest = DynamoDbClientApplicationRepository(dynamoDbClient, dynamoClientApplicationTableName)
         resetDatabase()
     }
 
     @Test
     fun `when find one client application by empty client id`() {
-        val clientApp: Optional<ClientApplication> = dynamoDbClientApplicationRepository.findOne(ClientAppId(""))
-        Assertions.assertEquals(clientApp, Optional.empty<ClientApplication>())
+        val clientApp: Optional<ClientApplication> = underTest.findOne(ClientAppId(""))
+        val expected = Optional.empty<ClientApplication>()
+        assertEquals(expected, clientApp)
     }
 
     @Test
     fun `when find one client application by client id that does not exist`() {
         val clientApp: Optional<ClientApplication> =
-                dynamoDbClientApplicationRepository.findOne(ClientAppId("not-existing-one"))
-        Assertions.assertEquals(clientApp, Optional.empty<ClientApplication>())
+            underTest.findOne(ClientAppId("not-existing-one"))
+        val expected = Optional.empty<ClientApplication>()
+        assertEquals(expected, clientApp)
     }
 
     @Test
     fun `when store, check if it exist and then delete a client application by client`() {
         val clientAppId = ClientAppId("client_id")
         val expected = ClientAppFixture.aClientApp(clientAppId)
-        dynamoDbClientApplicationRepository.save(expected)
-        var actual = dynamoDbClientApplicationRepository.findOne(clientAppId)
+        underTest.save(expected)
+        var actual = underTest.findOne(clientAppId)
 
-        Assertions.assertEquals(actual, Optional.of(expected))
+        assertEquals(Optional.of(expected), actual)
 
-        dynamoDbClientApplicationRepository.delete(clientAppId)
-        actual = dynamoDbClientApplicationRepository.findOne(clientAppId)
+        underTest.delete(clientAppId)
+        actual = underTest.findOne(clientAppId)
 
-        Assertions.assertEquals(actual, Optional.empty<ClientApplication>())
+        assertEquals(Optional.empty<ClientApplication>(), actual)
     }
 
     @Test
     fun `when find all client applications`() {
         val clientAppId = ClientAppId("client_id")
         val expected = ClientAppFixture.aClientApp(clientAppId)
-        dynamoDbClientApplicationRepository.save(expected)
-        val actual = dynamoDbClientApplicationRepository.findAll()
+        underTest.save(expected)
+        val actual = underTest.findAll()
 
-        Assertions.assertEquals(actual, listOf(expected))
+        assertEquals(listOf(expected), actual)
     }
 
     @Test
     fun `when find an client application with zero authorities`() {
         val clientAppId = ClientAppId("client_id")
         val expected = ClientAppFixture.aClientApp(clientAppId, authorities = Authorities.empty())
-        dynamoDbClientApplicationRepository.save(expected)
-        val actual = dynamoDbClientApplicationRepository.findAll()
+        underTest.save(expected)
+        val actual = underTest.findAll()
 
-        Assertions.assertEquals(actual, listOf(expected))
+        assertEquals(listOf(expected), actual)
     }
 
 }