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

Feature/recover password #113

Open
wants to merge 13 commits into
base: develop
Choose a base branch
from
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package pt.up.fe.ni.website.backend.controller

import jakarta.validation.Valid
import org.springframework.validation.annotation.Validated
import org.springframework.web.bind.annotation.DeleteMapping
import org.springframework.web.bind.annotation.GetMapping
Expand All @@ -13,7 +14,7 @@ import org.springframework.web.bind.annotation.RequestPart
import org.springframework.web.bind.annotation.RestController
import org.springframework.web.multipart.MultipartFile
import pt.up.fe.ni.website.backend.dto.auth.ChangePasswordDto
import pt.up.fe.ni.website.backend.dto.auth.PassRecoveryDto
import pt.up.fe.ni.website.backend.dto.auth.PasswordRecoveryDto
import pt.up.fe.ni.website.backend.dto.entity.account.CreateAccountDto
import pt.up.fe.ni.website.backend.dto.entity.account.UpdateAccountDto
import pt.up.fe.ni.website.backend.model.Account
Expand All @@ -31,11 +32,19 @@ class AccountController(private val service: AccountService) {
fun getAccountById(@PathVariable id: Long) = service.getAccountById(id)

@PutMapping("/recoverPassword/{recoveryToken}")
fun recoverPassword(@RequestBody dto: PassRecoveryDto, @PathVariable recoveryToken: String) =
fun recoverPassword(
@Valid @RequestBody
dto: PasswordRecoveryDto,
@PathVariable recoveryToken: String
) =
service.recoverPassword(recoveryToken, dto)

@PostMapping("/changePassword/{id}")
fun changePassword(@PathVariable id: Long, @RequestBody dto: ChangePasswordDto): Map<String, String> {
fun changePassword(
@Valid @RequestBody
dto: ChangePasswordDto,
@PathVariable id: Long
): Map<String, String> {
service.changePassword(id, dto)
return emptyMap()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ import pt.up.fe.ni.website.backend.service.AuthService
@RestController
@RequestMapping("/auth")
class AuthController(val authService: AuthService) {
@field:Value("\${backend.url}")
private lateinit var backendUrl: String
@field:Value("\${page.recover-password}")
private lateinit var recoverPasswordPage: String
BrunoRosendo marked this conversation as resolved.
Show resolved Hide resolved

@PostMapping("/new")
fun getNewToken(@RequestBody loginDto: LoginDto): Map<String, String> {
Expand All @@ -36,8 +36,8 @@ class AuthController(val authService: AuthService) {
@PostMapping("/recoverPassword/{id}")
fun generateRecoveryToken(@PathVariable id: Long): Map<String, String> {
BrunoRosendo marked this conversation as resolved.
Show resolved Hide resolved
val recoveryToken = authService.generateRecoveryToken(id)
BrunoRosendo marked this conversation as resolved.
Show resolved Hide resolved
// TODO: Change URL Later
return mapOf("recovery_url" to "$backendUrl/accounts/recoverPassword/$recoveryToken")
// TODO: Change to email service
return mapOf("recovery_url" to "$recoverPasswordPage/$recoveryToken")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For security reasons, users should not be able to know if a email is registered in a website, so the request should always be Ok 200

}

@GetMapping
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
package pt.up.fe.ni.website.backend.dto.auth

import jakarta.validation.constraints.Size
import pt.up.fe.ni.website.backend.model.constants.AccountConstants as Constants

data class ChangePasswordDto(
@field:Size(min = Constants.Password.minSize, max = Constants.Password.maxSize)
BrunoRosendo marked this conversation as resolved.
Show resolved Hide resolved
val oldPassword: String,

@field:Size(min = Constants.Password.minSize, max = Constants.Password.maxSize)
LuisDuarte1 marked this conversation as resolved.
Show resolved Hide resolved
val newPassword: String
)

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package pt.up.fe.ni.website.backend.dto.auth

import jakarta.validation.constraints.Size
import pt.up.fe.ni.website.backend.model.constants.AccountConstants as Constants

data class PasswordRecoveryDto(
@field:Size(min = Constants.Password.minSize, max = Constants.Password.maxSize)
LuisDuarte1 marked this conversation as resolved.
Show resolved Hide resolved
val password: String
)
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import org.springframework.security.oauth2.server.resource.InvalidBearerTokenExc
import org.springframework.stereotype.Service
import org.springframework.web.multipart.MultipartFile
import pt.up.fe.ni.website.backend.dto.auth.ChangePasswordDto
import pt.up.fe.ni.website.backend.dto.auth.PassRecoveryDto
import pt.up.fe.ni.website.backend.dto.auth.PasswordRecoveryDto
import pt.up.fe.ni.website.backend.dto.entity.account.CreateAccountDto
import pt.up.fe.ni.website.backend.dto.entity.account.UpdateAccountDto
import pt.up.fe.ni.website.backend.model.Account
Expand Down Expand Up @@ -70,7 +70,7 @@ class AccountService(
fun getAccountByEmail(email: String): Account = repository.findByEmail(email)
?: throw NoSuchElementException(ErrorMessages.emailNotFound(email))

fun recoverPassword(recoveryToken: String, dto: PassRecoveryDto): Account {
fun recoverPassword(recoveryToken: String, dto: PasswordRecoveryDto): Account {
val jwt =
try {
jwtDecoder.decode(recoveryToken)
Expand Down
5 changes: 3 additions & 2 deletions src/main/resources/application.properties
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,9 @@ upload.static-path=classpath:static
# URL that will serve static content
upload.static-serve=http://localhost:3000/static

# Backend URL
backend.url=http://localhost:8080
# Recover password page
# (for now it's the backend endpoint as we don't have the front end page yet)
page.recover-password=http://localhost:8080/accounts/recoverPassword

# Cors Origin
cors.allow-origin = http://localhost:3000
Original file line number Diff line number Diff line change
Expand Up @@ -619,6 +619,57 @@ class AccountControllerTest @Autowired constructor(
hasRequestPayload = true
)
}

@NestedTest
@DisplayName("Input Validation")
inner class InputValidation {
private val validationTester = ValidationTester(
req = { params: Map<String, Any?> ->
mockMvc.perform(
post("/accounts/changePassword/${changePasswordAccount.id}")
.contentType(MediaType.APPLICATION_JSON)
.content(objectMapper.writeValueAsString(params))
)
.andDocumentErrorResponse(documentation, hasRequestPayload = true)
},
requiredFields = mapOf(
"oldPassword" to password,
"newPassword" to "test_password2"
)
)

@NestedTest
@DisplayName("oldPassword")
inner class OldPasswordValidation {
@BeforeAll
fun setParam() {
validationTester.param = "oldPassword"
}

@Test
fun `should be required`() = validationTester.isRequired()

@Test
@DisplayName("size should be between ${Constants.Password.minSize} and ${Constants.Password.maxSize}()")
fun size() = validationTester.hasSizeBetween(Constants.Password.minSize, Constants.Password.maxSize)
}

@NestedTest
@DisplayName("newPassword")
inner class NewPasswordValidation {
@BeforeAll
fun setParam() {
validationTester.param = "newPassword"
}

@Test
fun `should be required`() = validationTester.isRequired()

@Test
@DisplayName("size should be between ${Constants.Password.minSize} and ${Constants.Password.maxSize}()")
fun size() = validationTester.hasSizeBetween(Constants.Password.minSize, Constants.Password.maxSize)
}
}
}

@NestedTest
Expand Down Expand Up @@ -908,8 +959,8 @@ class AccountControllerTest @Autowired constructor(
@NestedTest
@DisplayName("PUT /recoverPassword/{recoveryToken}")
inner class RecoverPassword {
@field:Value("\${backend.url}")
private lateinit var backendUrl: String
@field:Value("\${page.recover-password}")
private lateinit var recoverPasswordPage: String

private val newPassword = "new-password"

Expand All @@ -934,7 +985,7 @@ class AccountControllerTest @Autowired constructor(
mockMvc.perform(post("/auth/recoverPassword/${testAccount.id}"))
.andReturn().response.let { authResponse ->
val recoveryToken = objectMapper.readTree(authResponse.contentAsString)["recovery_url"].asText()
.removePrefix("$backendUrl/accounts/recoverPassword/")
.removePrefix("$recoverPasswordPage/")
mockMvc.perform(
put("/accounts/recoverPassword/{recoveryToken}", recoveryToken)
.contentType(MediaType.APPLICATION_JSON)
Expand All @@ -956,6 +1007,15 @@ class AccountControllerTest @Autowired constructor(
documentRequestPayload = true
)
}
mockMvc.post("/auth/new") {
contentType = MediaType.APPLICATION_JSON
content = objectMapper.writeValueAsString(
mapOf(
"email" to testAccount.email,
"password" to newPassword
)
)
}.andExpect { status { isOk() } }
}

@Test
Expand All @@ -980,6 +1040,40 @@ class AccountControllerTest @Autowired constructor(
hasRequestPayload = true
)
}

@NestedTest
@DisplayName("Input Validation")
inner class InputValidation {
private val validationTester = ValidationTester(
req = { params: Map<String, Any?> ->
mockMvc.perform(
put("/accounts/recoverPassword/random-token")
.contentType(MediaType.APPLICATION_JSON)
.content(objectMapper.writeValueAsString(params))
)
.andDocumentErrorResponse(documentation, hasRequestPayload = true)
},
requiredFields = mapOf(
"password" to "new-password"
)
)

@NestedTest
@DisplayName("password")
inner class PasswordValidation {
@BeforeAll
fun setParam() {
validationTester.param = "password"
}

@Test
fun `should be required`() = validationTester.isRequired()

@Test
@DisplayName("size should be between ${Constants.Password.minSize} and ${Constants.Password.maxSize}()")
fun size() = validationTester.hasSizeBetween(Constants.Password.minSize, Constants.Password.maxSize)
}
}
}

fun Date?.toJson(): String {
Expand Down