Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

put error messages in login phase #96

Merged
merged 6 commits into from
Jan 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions src/main/frontend/app/component/ErrorBanner.js
Original file line number Diff line number Diff line change
@@ -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
17 changes: 11 additions & 6 deletions src/main/frontend/app/login/LoginPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 = () => {
Expand Down Expand Up @@ -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}>
Expand All @@ -108,6 +110,8 @@ const Login = (props) => {
<Grid style={{marginTop: '10px'}}>
<Divider/>
</Grid>
{errorMessage ? errorsBanner : ""}


{<form action="login" method="post">
<Box>
Expand Down Expand Up @@ -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'));
}
11 changes: 9 additions & 2 deletions src/main/frontend/app/mfa/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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}>

Expand All @@ -26,6 +31,7 @@ const MfaChallengePage = (props) => {
<Grid style={{marginTop: '10px'}}>
<Divider/>
</Grid>
{errorMessage ? errorsBanner : ""}

{<form action="mfa-challenge" method="post">
<Box>
Expand All @@ -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'));
}
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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))
Expand All @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down

This file was deleted.

27 changes: 25 additions & 2 deletions src/main/kotlin/com/vauthenticator/server/mfa/MfaController.kt
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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(
Expand All @@ -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")
}
}
Expand Down
1 change: 1 addition & 0 deletions src/main/resources/templates/template.html
Original file line number Diff line number Diff line change
Expand Up @@ -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>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

}
Loading