From b838bc24d42d07c0424b2a5295a4cb120916073b Mon Sep 17 00:00:00 2001 From: Joaquim Cunha Date: Mon, 20 Feb 2023 15:39:06 +0000 Subject: [PATCH 01/10] added recover request route --- .../ni/website/backend/config/auth/AuthConfigProperties.kt | 3 ++- .../up/fe/ni/website/backend/controller/AuthController.kt | 7 +++++++ .../pt/up/fe/ni/website/backend/service/AuthService.kt | 5 +++++ src/main/resources/application.properties | 1 + 4 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/main/kotlin/pt/up/fe/ni/website/backend/config/auth/AuthConfigProperties.kt b/src/main/kotlin/pt/up/fe/ni/website/backend/config/auth/AuthConfigProperties.kt index 18c9484a..eb76c213 100644 --- a/src/main/kotlin/pt/up/fe/ni/website/backend/config/auth/AuthConfigProperties.kt +++ b/src/main/kotlin/pt/up/fe/ni/website/backend/config/auth/AuthConfigProperties.kt @@ -9,5 +9,6 @@ data class AuthConfigProperties( val publicKey: RSAPublicKey, val privateKey: RSAPrivateKey, val jwtAccessExpirationMinutes: Long, - val jwtRefreshExpirationDays: Long + val jwtRefreshExpirationDays: Long, + val jwtRecoveryExpirationMinutes: Long ) diff --git a/src/main/kotlin/pt/up/fe/ni/website/backend/controller/AuthController.kt b/src/main/kotlin/pt/up/fe/ni/website/backend/controller/AuthController.kt index 2239a3f7..49d54e1a 100644 --- a/src/main/kotlin/pt/up/fe/ni/website/backend/controller/AuthController.kt +++ b/src/main/kotlin/pt/up/fe/ni/website/backend/controller/AuthController.kt @@ -2,6 +2,7 @@ package pt.up.fe.ni.website.backend.controller import org.springframework.security.access.prepost.PreAuthorize import org.springframework.web.bind.annotation.GetMapping +import org.springframework.web.bind.annotation.PathVariable import org.springframework.web.bind.annotation.PostMapping import org.springframework.web.bind.annotation.RequestBody import org.springframework.web.bind.annotation.RequestMapping @@ -27,6 +28,12 @@ class AuthController(val authService: AuthService) { return mapOf("access_token" to accessToken) } + @PostMapping("/recoverPassword/{id}") + fun generateRecoveryToken(@PathVariable id: Long): Map { + val recoveryToken = authService.generateRecoveryToken(id) + return mapOf("recovery_token" to recoveryToken) + } + @GetMapping @PreAuthorize("hasRole('MEMBER')") fun checkAuthentication(): Map { diff --git a/src/main/kotlin/pt/up/fe/ni/website/backend/service/AuthService.kt b/src/main/kotlin/pt/up/fe/ni/website/backend/service/AuthService.kt index f5fb93fe..6e9168f0 100644 --- a/src/main/kotlin/pt/up/fe/ni/website/backend/service/AuthService.kt +++ b/src/main/kotlin/pt/up/fe/ni/website/backend/service/AuthService.kt @@ -57,6 +57,11 @@ class AuthService( return generateAccessToken(account) } + fun generateRecoveryToken(id: Long): String { + val account = accountService.getAccountById(id) + return generateToken(account, Duration.ofMinutes(authConfigProperties.jwtRecoveryExpirationMinutes)) + } + fun getAuthenticatedAccount(): Account { val authentication = SecurityContextHolder.getContext().authentication return accountService.getAccountByEmail(authentication.name) diff --git a/src/main/resources/application.properties b/src/main/resources/application.properties index 12a8773f..141dda9e 100644 --- a/src/main/resources/application.properties +++ b/src/main/resources/application.properties @@ -24,6 +24,7 @@ auth.private-key=classpath:certs/private.pem auth.public-key=classpath:certs/public.pem auth.jwt-access-expiration-minutes=60 auth.jwt-refresh-expiration-days=7 +auth.jwt-recovery-expiration-minutes=15 # Due to a problem with Hibernate, which is using a deprecated property. This should be removed when fixed # See https://github.com/spring-projects/spring-data-jpa/issues/2717 for more information From 36812a348dee281de7af4eafd215309b2cab2ff4 Mon Sep 17 00:00:00 2001 From: Joaquim Cunha Date: Mon, 20 Feb 2023 16:19:33 +0000 Subject: [PATCH 02/10] added password update route --- .../backend/controller/AccountController.kt | 6 +++++ .../backend/controller/AuthController.kt | 3 ++- .../backend/dto/auth/PassRecoveryDto.kt | 5 ++++ .../website/backend/service/AccountService.kt | 26 ++++++++++++++++++- .../website/backend/service/ErrorMessages.kt | 4 +++ 5 files changed, 42 insertions(+), 2 deletions(-) create mode 100644 src/main/kotlin/pt/up/fe/ni/website/backend/dto/auth/PassRecoveryDto.kt diff --git a/src/main/kotlin/pt/up/fe/ni/website/backend/controller/AccountController.kt b/src/main/kotlin/pt/up/fe/ni/website/backend/controller/AccountController.kt index cac7fcae..48d0a00b 100644 --- a/src/main/kotlin/pt/up/fe/ni/website/backend/controller/AccountController.kt +++ b/src/main/kotlin/pt/up/fe/ni/website/backend/controller/AccountController.kt @@ -3,9 +3,11 @@ package pt.up.fe.ni.website.backend.controller import org.springframework.web.bind.annotation.GetMapping import org.springframework.web.bind.annotation.PathVariable import org.springframework.web.bind.annotation.PostMapping +import org.springframework.web.bind.annotation.PutMapping import org.springframework.web.bind.annotation.RequestBody import org.springframework.web.bind.annotation.RequestMapping import org.springframework.web.bind.annotation.RestController +import pt.up.fe.ni.website.backend.dto.auth.PassRecoveryDto import pt.up.fe.ni.website.backend.dto.entity.AccountDto import pt.up.fe.ni.website.backend.service.AccountService @@ -20,4 +22,8 @@ class AccountController(private val service: AccountService) { @PostMapping("/new") fun createAccount(@RequestBody dto: AccountDto) = service.createAccount(dto) + + @PutMapping("/recoverPassword/{recoveryToken}") + fun recoverPassword(@RequestBody dto: PassRecoveryDto, @PathVariable recoveryToken: String) = + service.recoverPassword(recoveryToken, dto) } diff --git a/src/main/kotlin/pt/up/fe/ni/website/backend/controller/AuthController.kt b/src/main/kotlin/pt/up/fe/ni/website/backend/controller/AuthController.kt index 49d54e1a..8eb7cc6a 100644 --- a/src/main/kotlin/pt/up/fe/ni/website/backend/controller/AuthController.kt +++ b/src/main/kotlin/pt/up/fe/ni/website/backend/controller/AuthController.kt @@ -31,7 +31,8 @@ class AuthController(val authService: AuthService) { @PostMapping("/recoverPassword/{id}") fun generateRecoveryToken(@PathVariable id: Long): Map { val recoveryToken = authService.generateRecoveryToken(id) - return mapOf("recovery_token" to recoveryToken) + // TODO: Change URL Later + return mapOf("recovery_url" to "localhost:8080/accounts/recoverPassword/$recoveryToken") } @GetMapping diff --git a/src/main/kotlin/pt/up/fe/ni/website/backend/dto/auth/PassRecoveryDto.kt b/src/main/kotlin/pt/up/fe/ni/website/backend/dto/auth/PassRecoveryDto.kt new file mode 100644 index 00000000..155dd619 --- /dev/null +++ b/src/main/kotlin/pt/up/fe/ni/website/backend/dto/auth/PassRecoveryDto.kt @@ -0,0 +1,5 @@ +package pt.up.fe.ni.website.backend.dto.auth + +data class PassRecoveryDto( + val password: String +) diff --git a/src/main/kotlin/pt/up/fe/ni/website/backend/service/AccountService.kt b/src/main/kotlin/pt/up/fe/ni/website/backend/service/AccountService.kt index 8a0f111a..7180bdd8 100644 --- a/src/main/kotlin/pt/up/fe/ni/website/backend/service/AccountService.kt +++ b/src/main/kotlin/pt/up/fe/ni/website/backend/service/AccountService.kt @@ -2,13 +2,21 @@ package pt.up.fe.ni.website.backend.service import org.springframework.data.repository.findByIdOrNull import org.springframework.security.crypto.password.PasswordEncoder +import org.springframework.security.oauth2.jwt.JwtDecoder +import org.springframework.security.oauth2.server.resource.InvalidBearerTokenException import org.springframework.stereotype.Service +import pt.up.fe.ni.website.backend.dto.auth.PassRecoveryDto import pt.up.fe.ni.website.backend.dto.entity.AccountDto import pt.up.fe.ni.website.backend.model.Account import pt.up.fe.ni.website.backend.repository.AccountRepository +import java.time.Instant @Service -class AccountService(private val repository: AccountRepository, private val encoder: PasswordEncoder) { +class AccountService( + private val repository: AccountRepository, + private val encoder: PasswordEncoder, + private val jwtDecoder: JwtDecoder +) { fun getAllAccounts(): List = repository.findAll().toList() fun createAccount(dto: AccountDto): Account { @@ -28,4 +36,20 @@ class AccountService(private val repository: AccountRepository, private val enco fun getAccountByEmail(email: String): Account = repository.findByEmail(email) ?: throw NoSuchElementException(ErrorMessages.emailNotFound(email)) + + fun recoverPassword(recoveryToken: String, dto: PassRecoveryDto): Account { + val jwt = + try { + jwtDecoder.decode(recoveryToken) + } catch (e: Exception) { + throw InvalidBearerTokenException(ErrorMessages.invalidRecoveryToken) + } + if (jwt.expiresAt?.isBefore(Instant.now()) != false) { + throw InvalidBearerTokenException(ErrorMessages.expiredRecoveryToken) + } + val account = getAccountByEmail(jwt.subject) + + account.password = encoder.encode(dto.password) + return repository.save(account) + } } diff --git a/src/main/kotlin/pt/up/fe/ni/website/backend/service/ErrorMessages.kt b/src/main/kotlin/pt/up/fe/ni/website/backend/service/ErrorMessages.kt index ea8ae17a..15f86cc5 100644 --- a/src/main/kotlin/pt/up/fe/ni/website/backend/service/ErrorMessages.kt +++ b/src/main/kotlin/pt/up/fe/ni/website/backend/service/ErrorMessages.kt @@ -11,6 +11,10 @@ object ErrorMessages { const val expiredRefreshToken = "refresh token has expired" + const val invalidRecoveryToken = "invalid password recovery token" + + const val expiredRecoveryToken = "password recovery token has expired" + fun postNotFound(postId: Long): String = "post not found with id $postId" fun postNotFound(postSlug: String): String = "post not found with slug $postSlug" From c067a58d281f12134ec56a861e4a8e1a43fc434b Mon Sep 17 00:00:00 2001 From: Joaquim Cunha Date: Tue, 28 Feb 2023 22:51:50 +0000 Subject: [PATCH 03/10] added tests --- .../controller/AccountControllerTest.kt | 47 +++++++++++++++++++ .../backend/controller/AuthControllerTest.kt | 28 +++++++++++ 2 files changed, 75 insertions(+) diff --git a/src/test/kotlin/pt/up/fe/ni/website/backend/controller/AccountControllerTest.kt b/src/test/kotlin/pt/up/fe/ni/website/backend/controller/AccountControllerTest.kt index cde2d99c..79aeec65 100644 --- a/src/test/kotlin/pt/up/fe/ni/website/backend/controller/AccountControllerTest.kt +++ b/src/test/kotlin/pt/up/fe/ni/website/backend/controller/AccountControllerTest.kt @@ -11,6 +11,7 @@ import org.springframework.http.MediaType import org.springframework.test.web.servlet.MockMvc import org.springframework.test.web.servlet.get import org.springframework.test.web.servlet.post +import org.springframework.test.web.servlet.put import pt.up.fe.ni.website.backend.model.Account import pt.up.fe.ni.website.backend.model.CustomWebsite import pt.up.fe.ni.website.backend.repository.AccountRepository @@ -419,6 +420,52 @@ class AccountControllerTest @Autowired constructor( } } + @NestedTest + @DisplayName("PUT /recoverPassword/{recoveryToken}") + inner class RecoverPassword { + private val newPassword = "new-password" + + @BeforeEach + fun setup() { + repository.save(testAccount) + } + + @Test + fun `should update the password`() { + mockMvc.post("/auth/recoverPassword/${testAccount.id}") + .andReturn().response.let { authResponse -> + val recoveryLink = objectMapper.readTree(authResponse.contentAsString)["recovery_url"].asText() + .removePrefix("localhost:8080") + mockMvc.put(recoveryLink) { + contentType = MediaType.APPLICATION_JSON + content = objectMapper.writeValueAsString( + mapOf( + "password" to newPassword + ) + ) + }.andExpect { + status { isOk() } + } + } + } + + @Test + fun `should fail when token is invalid`() { + mockMvc.put("/accounts/recoverPassword/invalid_token") { + contentType = MediaType.APPLICATION_JSON + content = objectMapper.writeValueAsString( + mapOf( + "password" to newPassword + ) + ) + }.andExpect { + status { isUnauthorized() } + jsonPath("$.errors.length()") { value(1) } + jsonPath("$.errors[0].message") { value("invalid password recovery token") } + } + } + } + fun Date?.toJson(): String { val quotedDate = objectMapper.writeValueAsString(this) // objectMapper adds quotes to the date, so remove them diff --git a/src/test/kotlin/pt/up/fe/ni/website/backend/controller/AuthControllerTest.kt b/src/test/kotlin/pt/up/fe/ni/website/backend/controller/AuthControllerTest.kt index abb05f57..0975e607 100644 --- a/src/test/kotlin/pt/up/fe/ni/website/backend/controller/AuthControllerTest.kt +++ b/src/test/kotlin/pt/up/fe/ni/website/backend/controller/AuthControllerTest.kt @@ -131,6 +131,34 @@ class AuthControllerTest @Autowired constructor( } } + @NestedTest + @DisplayName("POST /auth/recoverPassword/{id}") + inner class RecoverPassword { + @BeforeEach + fun setup() { + repository.save(testAccount) + } + + @Test + fun `should fail if id is not found`() { + mockMvc.post("/auth/recoverPassword/1234") + .andExpect { + status { isNotFound() } + jsonPath("$.errors.length()") { value(1) } + jsonPath("$.errors[0].message") { value("account not found with id 1234") } + } + } + + @Test + fun `should return password recovery link`() { + mockMvc.post("/auth/recoverPassword/${testAccount.id}") + .andExpect { + status { isOk() } + jsonPath("$.recovery_url") { exists() } + } + } + } + @NestedTest @DisplayName("GET /auth/check") inner class CheckToken { From 413af2022cb0c8c76ee1b89214cfb7482d855a8b Mon Sep 17 00:00:00 2001 From: Joaquim Cunha Date: Wed, 3 May 2023 17:30:25 +0100 Subject: [PATCH 04/10] fix documentation --- .../backend/controller/AccountController.kt | 2 +- .../website/backend/service/AccountService.kt | 8 ++-- .../controller/AccountControllerTest.kt | 39 ++++++++++--------- .../backend/controller/AuthControllerTest.kt | 10 ++++- .../payloadschemas/model/PayloadAuth.kt | 6 +++ 5 files changed, 40 insertions(+), 25 deletions(-) diff --git a/src/main/kotlin/pt/up/fe/ni/website/backend/controller/AccountController.kt b/src/main/kotlin/pt/up/fe/ni/website/backend/controller/AccountController.kt index 34cbd1c9..d4e930fb 100644 --- a/src/main/kotlin/pt/up/fe/ni/website/backend/controller/AccountController.kt +++ b/src/main/kotlin/pt/up/fe/ni/website/backend/controller/AccountController.kt @@ -11,9 +11,9 @@ import org.springframework.web.bind.annotation.RequestMapping import org.springframework.web.bind.annotation.RequestParam import org.springframework.web.bind.annotation.RequestPart import org.springframework.web.bind.annotation.RestController -import pt.up.fe.ni.website.backend.dto.auth.PassRecoveryDto 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.entity.account.CreateAccountDto import pt.up.fe.ni.website.backend.dto.entity.account.UpdateAccountDto import pt.up.fe.ni.website.backend.model.Account diff --git a/src/main/kotlin/pt/up/fe/ni/website/backend/service/AccountService.kt b/src/main/kotlin/pt/up/fe/ni/website/backend/service/AccountService.kt index fd8ea298..6b12f9f3 100644 --- a/src/main/kotlin/pt/up/fe/ni/website/backend/service/AccountService.kt +++ b/src/main/kotlin/pt/up/fe/ni/website/backend/service/AccountService.kt @@ -1,19 +1,19 @@ package pt.up.fe.ni.website.backend.service +import java.time.Instant import java.util.UUID import org.springframework.data.repository.findByIdOrNull import org.springframework.security.crypto.password.PasswordEncoder import org.springframework.security.oauth2.jwt.JwtDecoder import org.springframework.security.oauth2.server.resource.InvalidBearerTokenException import org.springframework.stereotype.Service -import pt.up.fe.ni.website.backend.dto.auth.PassRecoveryDto -import pt.up.fe.ni.website.backend.model.Account -import pt.up.fe.ni.website.backend.repository.AccountRepository -import java.time.Instant 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.entity.account.CreateAccountDto import pt.up.fe.ni.website.backend.dto.entity.account.UpdateAccountDto +import pt.up.fe.ni.website.backend.model.Account +import pt.up.fe.ni.website.backend.repository.AccountRepository import pt.up.fe.ni.website.backend.service.upload.FileUploader import pt.up.fe.ni.website.backend.utils.extensions.filenameExtension diff --git a/src/test/kotlin/pt/up/fe/ni/website/backend/controller/AccountControllerTest.kt b/src/test/kotlin/pt/up/fe/ni/website/backend/controller/AccountControllerTest.kt index 067465b8..4f78aeeb 100644 --- a/src/test/kotlin/pt/up/fe/ni/website/backend/controller/AccountControllerTest.kt +++ b/src/test/kotlin/pt/up/fe/ni/website/backend/controller/AccountControllerTest.kt @@ -40,6 +40,7 @@ import pt.up.fe.ni.website.backend.utils.annotations.NestedTest import pt.up.fe.ni.website.backend.utils.documentation.payloadschemas.model.PayloadAccount import pt.up.fe.ni.website.backend.utils.documentation.utils.DocumentedJSONField import pt.up.fe.ni.website.backend.utils.documentation.utils.MockMVCExtension.Companion.andDocument +import pt.up.fe.ni.website.backend.utils.documentation.utils.MockMVCExtension.Companion.andDocumentCustomRequestSchema import pt.up.fe.ni.website.backend.utils.documentation.utils.MockMVCExtension.Companion.andDocumentCustomRequestSchemaEmptyResponse import pt.up.fe.ni.website.backend.utils.documentation.utils.MockMVCExtension.Companion.andDocumentCustomRequestSchemaErrorResponse import pt.up.fe.ni.website.backend.utils.documentation.utils.MockMVCExtension.Companion.andDocumentEmptyObjectResponse @@ -915,7 +916,7 @@ class AccountControllerTest @Autowired constructor( private val passwordRecoveryPayload = PayloadSchema( "password-recover", mutableListOf( - DocumentedJSONField("password", "The new password.", JsonFieldType.STRING), + DocumentedJSONField("password", "The new password.", JsonFieldType.STRING) ) ) @@ -928,20 +929,21 @@ class AccountControllerTest @Autowired constructor( fun `should update the password`() { mockMvc.perform(post("/auth/recoverPassword/${testAccount.id}")) .andReturn().response.let { authResponse -> - val recoveryLink = objectMapper.readTree(authResponse.contentAsString)["recovery_url"].asText() - .removePrefix("localhost:8080") - mockMvc.perform(put(recoveryLink) - .contentType(MediaType.APPLICATION_JSON) - .content( - objectMapper.writeValueAsString( - mapOf( - "password" to newPassword + val recoveryToken = objectMapper.readTree(authResponse.contentAsString)["recovery_url"].asText() + .removePrefix("localhost:8080/accounts/recoverPassword/") + mockMvc.perform( + put("/accounts/recoverPassword/{recoveryToken}", recoveryToken) + .contentType(MediaType.APPLICATION_JSON) + .content( + objectMapper.writeValueAsString( + mapOf( + "password" to newPassword + ) ) ) - ) ).andExpectAll( status().isOk() - ).andDocumentCustomRequestSchemaEmptyResponse( + ).andDocumentCustomRequestSchema( documentation, passwordRecoveryPayload, "Recover password", @@ -954,15 +956,16 @@ class AccountControllerTest @Autowired constructor( @Test fun `should fail when token is invalid`() { - mockMvc.perform(put("/accounts/recoverPassword/invalid_token") - .contentType(MediaType.APPLICATION_JSON) - .content( - objectMapper.writeValueAsString( - mapOf( - "password" to newPassword + mockMvc.perform( + put("/accounts/recoverPassword/invalid_token") + .contentType(MediaType.APPLICATION_JSON) + .content( + objectMapper.writeValueAsString( + mapOf( + "password" to newPassword + ) ) ) - ) ).andExpectAll( status().isUnauthorized(), jsonPath("$.errors.length()").value(1), diff --git a/src/test/kotlin/pt/up/fe/ni/website/backend/controller/AuthControllerTest.kt b/src/test/kotlin/pt/up/fe/ni/website/backend/controller/AuthControllerTest.kt index 34375725..e895cae0 100644 --- a/src/test/kotlin/pt/up/fe/ni/website/backend/controller/AuthControllerTest.kt +++ b/src/test/kotlin/pt/up/fe/ni/website/backend/controller/AuthControllerTest.kt @@ -198,7 +198,7 @@ class AuthControllerTest @Autowired constructor( status().isNotFound(), jsonPath("$.errors.length()").value(1), jsonPath("$.errors[0].message").value("account not found with id 1234") - ).andDocument( + ).andDocumentErrorResponse( documentation, "Recover password", "This endpoint operation allows the recovery of the password of an account, " + @@ -213,7 +213,13 @@ class AuthControllerTest @Autowired constructor( .andExpectAll( status().isOk(), jsonPath("$.recovery_url").exists() - ).andDocumentErrorResponse(documentation) + ).andDocument( + documentation, + "Recover password", + "This endpoint operation allows the recovery of the password of an account, " + + "sending an email with a link to reset the password.", + documentRequestPayload = false + ) } } diff --git a/src/test/kotlin/pt/up/fe/ni/website/backend/utils/documentation/payloadschemas/model/PayloadAuth.kt b/src/test/kotlin/pt/up/fe/ni/website/backend/utils/documentation/payloadschemas/model/PayloadAuth.kt index 4911f1e8..dff39edf 100644 --- a/src/test/kotlin/pt/up/fe/ni/website/backend/utils/documentation/payloadschemas/model/PayloadAuth.kt +++ b/src/test/kotlin/pt/up/fe/ni/website/backend/utils/documentation/payloadschemas/model/PayloadAuth.kt @@ -65,6 +65,12 @@ class PayloadRecoverPassword : ModelDocumentation( "Id of the account", JsonFieldType.NUMBER, isInResponse = false + ), + DocumentedJSONField( + "recovery_url", + "URL to recover the password", + JsonFieldType.STRING, + isInRequest = false ) ) ) From 1be6f37ae7be362507311a379ac7c01428b48c65 Mon Sep 17 00:00:00 2001 From: Joaquim Cunha Date: Wed, 28 Jun 2023 17:22:37 +0100 Subject: [PATCH 05/10] added a property for backend url --- .../up/fe/ni/website/backend/controller/AuthController.kt | 6 +++++- src/main/resources/application.properties | 3 +++ .../ni/website/backend/controller/AccountControllerTest.kt | 6 +++++- 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/main/kotlin/pt/up/fe/ni/website/backend/controller/AuthController.kt b/src/main/kotlin/pt/up/fe/ni/website/backend/controller/AuthController.kt index 00bbceb3..91c3e96c 100644 --- a/src/main/kotlin/pt/up/fe/ni/website/backend/controller/AuthController.kt +++ b/src/main/kotlin/pt/up/fe/ni/website/backend/controller/AuthController.kt @@ -1,5 +1,6 @@ package pt.up.fe.ni.website.backend.controller +import org.springframework.beans.factory.annotation.Value import org.springframework.security.access.prepost.PreAuthorize import org.springframework.web.bind.annotation.GetMapping import org.springframework.web.bind.annotation.PathVariable @@ -15,6 +16,9 @@ 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 + @PostMapping("/new") fun getNewToken(@RequestBody loginDto: LoginDto): Map { val account = authService.authenticate(loginDto.email, loginDto.password) @@ -33,7 +37,7 @@ class AuthController(val authService: AuthService) { fun generateRecoveryToken(@PathVariable id: Long): Map { val recoveryToken = authService.generateRecoveryToken(id) // TODO: Change URL Later - return mapOf("recovery_url" to "localhost:8080/accounts/recoverPassword/$recoveryToken") + return mapOf("recovery_url" to "$backendUrl/accounts/recoverPassword/$recoveryToken") } @GetMapping diff --git a/src/main/resources/application.properties b/src/main/resources/application.properties index feb315ca..f05c3954 100644 --- a/src/main/resources/application.properties +++ b/src/main/resources/application.properties @@ -36,6 +36,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 + # Due to a problem with Hibernate, which is using a deprecated property. This should be removed when fixed # See https://github.com/spring-projects/spring-data-jpa/issues/2717 for more information spring.jpa.properties.jakarta.persistence.sharedCache.mode=UNSPECIFIED diff --git a/src/test/kotlin/pt/up/fe/ni/website/backend/controller/AccountControllerTest.kt b/src/test/kotlin/pt/up/fe/ni/website/backend/controller/AccountControllerTest.kt index 4f78aeeb..f155b689 100644 --- a/src/test/kotlin/pt/up/fe/ni/website/backend/controller/AccountControllerTest.kt +++ b/src/test/kotlin/pt/up/fe/ni/website/backend/controller/AccountControllerTest.kt @@ -13,6 +13,7 @@ import org.junit.jupiter.api.DisplayName import org.junit.jupiter.api.Test import org.mockito.Mockito import org.springframework.beans.factory.annotation.Autowired +import org.springframework.beans.factory.annotation.Value import org.springframework.http.MediaType import org.springframework.mock.web.MockPart import org.springframework.restdocs.mockmvc.RestDocumentationRequestBuilders.delete @@ -907,6 +908,9 @@ class AccountControllerTest @Autowired constructor( @NestedTest @DisplayName("PUT /recoverPassword/{recoveryToken}") inner class RecoverPassword { + @field:Value("\${backend.url}") + private lateinit var backendUrl: String + private val newPassword = "new-password" private val parameters = listOf( @@ -930,7 +934,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("localhost:8080/accounts/recoverPassword/") + .removePrefix("$backendUrl/accounts/recoverPassword/") mockMvc.perform( put("/accounts/recoverPassword/{recoveryToken}", recoveryToken) .contentType(MediaType.APPLICATION_JSON) From 34666f6e01c5ccea9c869ec3454273e1aa4915bc Mon Sep 17 00:00:00 2001 From: Joaquim Cunha Date: Sun, 2 Jul 2023 03:39:55 +0100 Subject: [PATCH 06/10] review changes --- .../backend/controller/AccountController.kt | 15 ++- .../backend/controller/AuthController.kt | 8 +- .../backend/dto/auth/ChangePasswordDto.kt | 6 ++ .../backend/dto/auth/PassRecoveryDto.kt | 5 - .../backend/dto/auth/PasswordRecoveryDto.kt | 9 ++ .../website/backend/service/AccountService.kt | 4 +- src/main/resources/application.properties | 5 +- .../controller/AccountControllerTest.kt | 100 +++++++++++++++++- 8 files changed, 133 insertions(+), 19 deletions(-) delete mode 100644 src/main/kotlin/pt/up/fe/ni/website/backend/dto/auth/PassRecoveryDto.kt create mode 100644 src/main/kotlin/pt/up/fe/ni/website/backend/dto/auth/PasswordRecoveryDto.kt diff --git a/src/main/kotlin/pt/up/fe/ni/website/backend/controller/AccountController.kt b/src/main/kotlin/pt/up/fe/ni/website/backend/controller/AccountController.kt index d4e930fb..cb30e9b4 100644 --- a/src/main/kotlin/pt/up/fe/ni/website/backend/controller/AccountController.kt +++ b/src/main/kotlin/pt/up/fe/ni/website/backend/controller/AccountController.kt @@ -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 @@ -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 @@ -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 { + fun changePassword( + @Valid @RequestBody + dto: ChangePasswordDto, + @PathVariable id: Long + ): Map { service.changePassword(id, dto) return emptyMap() } diff --git a/src/main/kotlin/pt/up/fe/ni/website/backend/controller/AuthController.kt b/src/main/kotlin/pt/up/fe/ni/website/backend/controller/AuthController.kt index 91c3e96c..cddeab57 100644 --- a/src/main/kotlin/pt/up/fe/ni/website/backend/controller/AuthController.kt +++ b/src/main/kotlin/pt/up/fe/ni/website/backend/controller/AuthController.kt @@ -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 @PostMapping("/new") fun getNewToken(@RequestBody loginDto: LoginDto): Map { @@ -36,8 +36,8 @@ class AuthController(val authService: AuthService) { @PostMapping("/recoverPassword/{id}") fun generateRecoveryToken(@PathVariable id: Long): Map { val recoveryToken = authService.generateRecoveryToken(id) - // 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") } @GetMapping diff --git a/src/main/kotlin/pt/up/fe/ni/website/backend/dto/auth/ChangePasswordDto.kt b/src/main/kotlin/pt/up/fe/ni/website/backend/dto/auth/ChangePasswordDto.kt index 706e8c0f..e7810de7 100644 --- a/src/main/kotlin/pt/up/fe/ni/website/backend/dto/auth/ChangePasswordDto.kt +++ b/src/main/kotlin/pt/up/fe/ni/website/backend/dto/auth/ChangePasswordDto.kt @@ -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) val oldPassword: String, + + @field:Size(min = Constants.Password.minSize, max = Constants.Password.maxSize) val newPassword: String ) diff --git a/src/main/kotlin/pt/up/fe/ni/website/backend/dto/auth/PassRecoveryDto.kt b/src/main/kotlin/pt/up/fe/ni/website/backend/dto/auth/PassRecoveryDto.kt deleted file mode 100644 index 155dd619..00000000 --- a/src/main/kotlin/pt/up/fe/ni/website/backend/dto/auth/PassRecoveryDto.kt +++ /dev/null @@ -1,5 +0,0 @@ -package pt.up.fe.ni.website.backend.dto.auth - -data class PassRecoveryDto( - val password: String -) diff --git a/src/main/kotlin/pt/up/fe/ni/website/backend/dto/auth/PasswordRecoveryDto.kt b/src/main/kotlin/pt/up/fe/ni/website/backend/dto/auth/PasswordRecoveryDto.kt new file mode 100644 index 00000000..f1cfd3b5 --- /dev/null +++ b/src/main/kotlin/pt/up/fe/ni/website/backend/dto/auth/PasswordRecoveryDto.kt @@ -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) + val password: String +) diff --git a/src/main/kotlin/pt/up/fe/ni/website/backend/service/AccountService.kt b/src/main/kotlin/pt/up/fe/ni/website/backend/service/AccountService.kt index 6b12f9f3..80738bf0 100644 --- a/src/main/kotlin/pt/up/fe/ni/website/backend/service/AccountService.kt +++ b/src/main/kotlin/pt/up/fe/ni/website/backend/service/AccountService.kt @@ -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 @@ -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) diff --git a/src/main/resources/application.properties b/src/main/resources/application.properties index 37abff47..6e02c379 100644 --- a/src/main/resources/application.properties +++ b/src/main/resources/application.properties @@ -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 diff --git a/src/test/kotlin/pt/up/fe/ni/website/backend/controller/AccountControllerTest.kt b/src/test/kotlin/pt/up/fe/ni/website/backend/controller/AccountControllerTest.kt index f155b689..a507619b 100644 --- a/src/test/kotlin/pt/up/fe/ni/website/backend/controller/AccountControllerTest.kt +++ b/src/test/kotlin/pt/up/fe/ni/website/backend/controller/AccountControllerTest.kt @@ -619,6 +619,57 @@ class AccountControllerTest @Autowired constructor( hasRequestPayload = true ) } + + @NestedTest + @DisplayName("Input Validation") + inner class InputValidation { + private val validationTester = ValidationTester( + req = { params: Map -> + 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 @@ -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" @@ -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) @@ -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 @@ -980,6 +1040,40 @@ class AccountControllerTest @Autowired constructor( hasRequestPayload = true ) } + + @NestedTest + @DisplayName("Input Validation") + inner class InputValidation { + private val validationTester = ValidationTester( + req = { params: Map -> + 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 { From d6991b1409dd4860e877779a86c9505e9d2d4076 Mon Sep 17 00:00:00 2001 From: Joaquim Cunha Date: Sun, 2 Jul 2023 16:54:47 +0100 Subject: [PATCH 07/10] refactor recover password to single use jwt --- .../backend/dto/auth/ChangePasswordDto.kt | 1 - .../website/backend/service/AccountService.kt | 7 ++++++ .../ni/website/backend/service/AuthService.kt | 22 +++++++++++++++---- .../website/backend/service/ErrorMessages.kt | 2 ++ .../controller/AccountControllerTest.kt | 16 -------------- 5 files changed, 27 insertions(+), 21 deletions(-) diff --git a/src/main/kotlin/pt/up/fe/ni/website/backend/dto/auth/ChangePasswordDto.kt b/src/main/kotlin/pt/up/fe/ni/website/backend/dto/auth/ChangePasswordDto.kt index e7810de7..f7da3103 100644 --- a/src/main/kotlin/pt/up/fe/ni/website/backend/dto/auth/ChangePasswordDto.kt +++ b/src/main/kotlin/pt/up/fe/ni/website/backend/dto/auth/ChangePasswordDto.kt @@ -4,7 +4,6 @@ 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) val oldPassword: String, @field:Size(min = Constants.Password.minSize, max = Constants.Password.maxSize) diff --git a/src/main/kotlin/pt/up/fe/ni/website/backend/service/AccountService.kt b/src/main/kotlin/pt/up/fe/ni/website/backend/service/AccountService.kt index 80738bf0..0a4ec273 100644 --- a/src/main/kotlin/pt/up/fe/ni/website/backend/service/AccountService.kt +++ b/src/main/kotlin/pt/up/fe/ni/website/backend/service/AccountService.kt @@ -82,6 +82,13 @@ class AccountService( } val account = getAccountByEmail(jwt.subject) + val tokenPasswordHash = jwt.getClaim("passwordHash") + ?: throw InvalidBearerTokenException(ErrorMessages.missingHashClaim) + + if (account.password != tokenPasswordHash) { + throw InvalidBearerTokenException(ErrorMessages.invalidRecoveryToken) + } + account.password = encoder.encode(dto.password) return repository.save(account) } diff --git a/src/main/kotlin/pt/up/fe/ni/website/backend/service/AuthService.kt b/src/main/kotlin/pt/up/fe/ni/website/backend/service/AuthService.kt index 36df7f9d..ccae70b8 100644 --- a/src/main/kotlin/pt/up/fe/ni/website/backend/service/AuthService.kt +++ b/src/main/kotlin/pt/up/fe/ni/website/backend/service/AuthService.kt @@ -59,7 +59,11 @@ class AuthService( fun generateRecoveryToken(id: Long): String { val account = accountService.getAccountById(id) - return generateToken(account, Duration.ofMinutes(authConfigProperties.jwtRecoveryExpirationMinutes)) + return generateToken( + account, + Duration.ofMinutes(authConfigProperties.jwtRecoveryExpirationMinutes), + usePasswordHash = true + ) } fun getAuthenticatedAccount(): Account { @@ -67,21 +71,31 @@ class AuthService( return accountService.getAccountByEmail(authentication.name) } - private fun generateToken(account: Account, expiration: Duration, isRefresh: Boolean = false): String { + private fun generateToken( + account: Account, + expiration: Duration, + isRefresh: Boolean = false, + usePasswordHash: Boolean = false + ): String { val roles = if (isRefresh) emptyList() else getAuthorities() // TODO: Pass account to getAuthorities() val scope = roles .stream() .map(GrantedAuthority::getAuthority) .collect(Collectors.joining(" ")) val currentInstant = Instant.now() - val claims = JwtClaimsSet + val claimsBuilder = JwtClaimsSet .builder() .issuer("self") .issuedAt(currentInstant) .expiresAt(currentInstant.plus(expiration)) .subject(account.email) .claim("scope", scope) - .build() + + if (usePasswordHash) { + claimsBuilder.claim("passwordHash", account.password) + } + + val claims = claimsBuilder.build() return jwtEncoder.encode(JwtEncoderParameters.from(claims)).tokenValue } diff --git a/src/main/kotlin/pt/up/fe/ni/website/backend/service/ErrorMessages.kt b/src/main/kotlin/pt/up/fe/ni/website/backend/service/ErrorMessages.kt index cd55189d..25f73ff8 100644 --- a/src/main/kotlin/pt/up/fe/ni/website/backend/service/ErrorMessages.kt +++ b/src/main/kotlin/pt/up/fe/ni/website/backend/service/ErrorMessages.kt @@ -15,6 +15,8 @@ object ErrorMessages { const val expiredRecoveryToken = "password recovery token has expired" + const val missingHashClaim = "password hash claim is missing" + const val noGenerations = "no generations created yet" const val noGenerationsToInferYear = "no generations created yet, please specify school year" diff --git a/src/test/kotlin/pt/up/fe/ni/website/backend/controller/AccountControllerTest.kt b/src/test/kotlin/pt/up/fe/ni/website/backend/controller/AccountControllerTest.kt index a507619b..99e0fe2d 100644 --- a/src/test/kotlin/pt/up/fe/ni/website/backend/controller/AccountControllerTest.kt +++ b/src/test/kotlin/pt/up/fe/ni/website/backend/controller/AccountControllerTest.kt @@ -638,22 +638,6 @@ class AccountControllerTest @Autowired constructor( ) ) - @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 { From 723d8c9e94d640afa486728f38719a497b2bd068 Mon Sep 17 00:00:00 2001 From: Joaquim Cunha Date: Tue, 4 Jul 2023 19:36:20 +0100 Subject: [PATCH 08/10] refactored endpoinnts to be email ready --- .../backend/controller/AccountController.kt | 9 - .../backend/controller/AuthController.kt | 18 +- .../backend/dto/auth/PasswordRecoveryDto.kt | 6 +- .../website/backend/service/AccountService.kt | 31 +-- .../ni/website/backend/service/AuthService.kt | 29 +- .../website/backend/service/ErrorMessages.kt | 2 - src/main/resources/application.properties | 2 +- .../controller/AccountControllerTest.kt | 124 --------- .../backend/controller/AuthControllerTest.kt | 250 +++++++++++++++++- .../payloadschemas/model/PayloadAuth.kt | 9 +- 10 files changed, 297 insertions(+), 183 deletions(-) diff --git a/src/main/kotlin/pt/up/fe/ni/website/backend/controller/AccountController.kt b/src/main/kotlin/pt/up/fe/ni/website/backend/controller/AccountController.kt index cb30e9b4..0ef2d06b 100644 --- a/src/main/kotlin/pt/up/fe/ni/website/backend/controller/AccountController.kt +++ b/src/main/kotlin/pt/up/fe/ni/website/backend/controller/AccountController.kt @@ -14,7 +14,6 @@ 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.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 @@ -31,14 +30,6 @@ class AccountController(private val service: AccountService) { @GetMapping("/{id}") fun getAccountById(@PathVariable id: Long) = service.getAccountById(id) - @PutMapping("/recoverPassword/{recoveryToken}") - fun recoverPassword( - @Valid @RequestBody - dto: PasswordRecoveryDto, - @PathVariable recoveryToken: String - ) = - service.recoverPassword(recoveryToken, dto) - @PostMapping("/changePassword/{id}") fun changePassword( @Valid @RequestBody diff --git a/src/main/kotlin/pt/up/fe/ni/website/backend/controller/AuthController.kt b/src/main/kotlin/pt/up/fe/ni/website/backend/controller/AuthController.kt index cddeab57..d3c5766d 100644 --- a/src/main/kotlin/pt/up/fe/ni/website/backend/controller/AuthController.kt +++ b/src/main/kotlin/pt/up/fe/ni/website/backend/controller/AuthController.kt @@ -1,5 +1,6 @@ package pt.up.fe.ni.website.backend.controller +import jakarta.validation.Valid import org.springframework.beans.factory.annotation.Value import org.springframework.security.access.prepost.PreAuthorize import org.springframework.web.bind.annotation.GetMapping @@ -9,6 +10,8 @@ import org.springframework.web.bind.annotation.RequestBody import org.springframework.web.bind.annotation.RequestMapping import org.springframework.web.bind.annotation.RestController import pt.up.fe.ni.website.backend.dto.auth.LoginDto +import pt.up.fe.ni.website.backend.dto.auth.PasswordRecoveryConfirmDto +import pt.up.fe.ni.website.backend.dto.auth.PasswordRecoveryRequestDto import pt.up.fe.ni.website.backend.dto.auth.TokenDto import pt.up.fe.ni.website.backend.model.Account import pt.up.fe.ni.website.backend.service.AuthService @@ -33,13 +36,20 @@ class AuthController(val authService: AuthService) { return mapOf("access_token" to accessToken) } - @PostMapping("/recoverPassword/{id}") - fun generateRecoveryToken(@PathVariable id: Long): Map { - val recoveryToken = authService.generateRecoveryToken(id) + @PostMapping("/password/recovery") + fun generateRecoveryToken(@RequestBody recoveryRequestDto: PasswordRecoveryRequestDto): Map { + val recoveryToken = authService.generateRecoveryToken(recoveryRequestDto.email) // TODO: Change to email service - return mapOf("recovery_url" to "$recoverPasswordPage/$recoveryToken") + return mapOf("recovery_url" to "$recoverPasswordPage/$recoveryToken/confirm") } + @PostMapping("/password/recovery/{token}/confirm") + fun confirmRecoveryToken( + @Valid @RequestBody + dto: PasswordRecoveryConfirmDto, + @PathVariable token: String + ) = authService.confirmRecoveryToken(token, dto) + @GetMapping @PreAuthorize("hasRole('MEMBER')") fun checkAuthentication(): Map { diff --git a/src/main/kotlin/pt/up/fe/ni/website/backend/dto/auth/PasswordRecoveryDto.kt b/src/main/kotlin/pt/up/fe/ni/website/backend/dto/auth/PasswordRecoveryDto.kt index f1cfd3b5..a70352b9 100644 --- a/src/main/kotlin/pt/up/fe/ni/website/backend/dto/auth/PasswordRecoveryDto.kt +++ b/src/main/kotlin/pt/up/fe/ni/website/backend/dto/auth/PasswordRecoveryDto.kt @@ -3,7 +3,11 @@ 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( +data class PasswordRecoveryConfirmDto( @field:Size(min = Constants.Password.minSize, max = Constants.Password.maxSize) val password: String ) + +data class PasswordRecoveryRequestDto( + val email: String +) diff --git a/src/main/kotlin/pt/up/fe/ni/website/backend/service/AccountService.kt b/src/main/kotlin/pt/up/fe/ni/website/backend/service/AccountService.kt index 0a4ec273..383b7876 100644 --- a/src/main/kotlin/pt/up/fe/ni/website/backend/service/AccountService.kt +++ b/src/main/kotlin/pt/up/fe/ni/website/backend/service/AccountService.kt @@ -1,15 +1,13 @@ package pt.up.fe.ni.website.backend.service -import java.time.Instant import java.util.UUID import org.springframework.data.repository.findByIdOrNull import org.springframework.security.crypto.password.PasswordEncoder import org.springframework.security.oauth2.jwt.JwtDecoder -import org.springframework.security.oauth2.server.resource.InvalidBearerTokenException import org.springframework.stereotype.Service import org.springframework.web.multipart.MultipartFile +import pt.up.fe.ni.website.backend.config.Logging import pt.up.fe.ni.website.backend.dto.auth.ChangePasswordDto -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 @@ -23,7 +21,7 @@ class AccountService( private val encoder: PasswordEncoder, private val jwtDecoder: JwtDecoder, private val fileUploader: FileUploader -) { +) : Logging { fun getAllAccounts(): List = repository.findAll().toList() fun createAccount(dto: CreateAccountDto): Account { @@ -67,32 +65,11 @@ class AccountService( return repository.save(newAccount) } + fun updateAccount(account: Account): Account = repository.save(account) + fun getAccountByEmail(email: String): Account = repository.findByEmail(email) ?: throw NoSuchElementException(ErrorMessages.emailNotFound(email)) - fun recoverPassword(recoveryToken: String, dto: PasswordRecoveryDto): Account { - val jwt = - try { - jwtDecoder.decode(recoveryToken) - } catch (e: Exception) { - throw InvalidBearerTokenException(ErrorMessages.invalidRecoveryToken) - } - if (jwt.expiresAt?.isBefore(Instant.now()) != false) { - throw InvalidBearerTokenException(ErrorMessages.expiredRecoveryToken) - } - val account = getAccountByEmail(jwt.subject) - - val tokenPasswordHash = jwt.getClaim("passwordHash") - ?: throw InvalidBearerTokenException(ErrorMessages.missingHashClaim) - - if (account.password != tokenPasswordHash) { - throw InvalidBearerTokenException(ErrorMessages.invalidRecoveryToken) - } - - account.password = encoder.encode(dto.password) - return repository.save(account) - } - fun changePassword(id: Long, dto: ChangePasswordDto) { val account = getAccountById(id) if (!encoder.matches(dto.oldPassword, account.password)) { diff --git a/src/main/kotlin/pt/up/fe/ni/website/backend/service/AuthService.kt b/src/main/kotlin/pt/up/fe/ni/website/backend/service/AuthService.kt index ccae70b8..afc40094 100644 --- a/src/main/kotlin/pt/up/fe/ni/website/backend/service/AuthService.kt +++ b/src/main/kotlin/pt/up/fe/ni/website/backend/service/AuthService.kt @@ -15,6 +15,7 @@ import org.springframework.security.oauth2.jwt.JwtEncoderParameters import org.springframework.security.oauth2.server.resource.InvalidBearerTokenException import org.springframework.stereotype.Service import pt.up.fe.ni.website.backend.config.auth.AuthConfigProperties +import pt.up.fe.ni.website.backend.dto.auth.PasswordRecoveryConfirmDto import pt.up.fe.ni.website.backend.model.Account @Service @@ -57,8 +58,8 @@ class AuthService( return generateAccessToken(account) } - fun generateRecoveryToken(id: Long): String { - val account = accountService.getAccountById(id) + fun generateRecoveryToken(email: String): String? { + val account = accountService.getAccountByEmail(email) return generateToken( account, Duration.ofMinutes(authConfigProperties.jwtRecoveryExpirationMinutes), @@ -66,6 +67,30 @@ class AuthService( ) } + fun confirmRecoveryToken(recoveryToken: String, dto: PasswordRecoveryConfirmDto): Account { + val jwt = + try { + jwtDecoder.decode(recoveryToken) + } catch (e: Exception) { + throw InvalidBearerTokenException(ErrorMessages.invalidRecoveryToken) + } + + if (jwt.expiresAt?.isBefore(Instant.now()) != false) { + throw InvalidBearerTokenException(ErrorMessages.expiredRecoveryToken) + } + val account = accountService.getAccountByEmail(jwt.subject) + + val tokenPasswordHash = jwt.getClaim("passwordHash") + ?: throw InvalidBearerTokenException(ErrorMessages.invalidRecoveryToken) + + if (account.password != tokenPasswordHash) { + throw InvalidBearerTokenException(ErrorMessages.invalidRecoveryToken) + } + + account.password = passwordEncoder.encode(dto.password) + return accountService.updateAccount(account) + } + fun getAuthenticatedAccount(): Account { val authentication = SecurityContextHolder.getContext().authentication return accountService.getAccountByEmail(authentication.name) diff --git a/src/main/kotlin/pt/up/fe/ni/website/backend/service/ErrorMessages.kt b/src/main/kotlin/pt/up/fe/ni/website/backend/service/ErrorMessages.kt index 25f73ff8..cd55189d 100644 --- a/src/main/kotlin/pt/up/fe/ni/website/backend/service/ErrorMessages.kt +++ b/src/main/kotlin/pt/up/fe/ni/website/backend/service/ErrorMessages.kt @@ -15,8 +15,6 @@ object ErrorMessages { const val expiredRecoveryToken = "password recovery token has expired" - const val missingHashClaim = "password hash claim is missing" - const val noGenerations = "no generations created yet" const val noGenerationsToInferYear = "no generations created yet, please specify school year" diff --git a/src/main/resources/application.properties b/src/main/resources/application.properties index 6e02c379..f95e9527 100644 --- a/src/main/resources/application.properties +++ b/src/main/resources/application.properties @@ -39,7 +39,7 @@ upload.static-serve=http://localhost:3000/static # 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 +page.recover-password=http://localhost:8080/auth/password/recovery # Cors Origin cors.allow-origin = http://localhost:3000 diff --git a/src/test/kotlin/pt/up/fe/ni/website/backend/controller/AccountControllerTest.kt b/src/test/kotlin/pt/up/fe/ni/website/backend/controller/AccountControllerTest.kt index 99e0fe2d..089f1dde 100644 --- a/src/test/kotlin/pt/up/fe/ni/website/backend/controller/AccountControllerTest.kt +++ b/src/test/kotlin/pt/up/fe/ni/website/backend/controller/AccountControllerTest.kt @@ -13,19 +13,16 @@ import org.junit.jupiter.api.DisplayName import org.junit.jupiter.api.Test import org.mockito.Mockito import org.springframework.beans.factory.annotation.Autowired -import org.springframework.beans.factory.annotation.Value import org.springframework.http.MediaType import org.springframework.mock.web.MockPart import org.springframework.restdocs.mockmvc.RestDocumentationRequestBuilders.delete import org.springframework.restdocs.mockmvc.RestDocumentationRequestBuilders.get import org.springframework.restdocs.mockmvc.RestDocumentationRequestBuilders.multipart import org.springframework.restdocs.mockmvc.RestDocumentationRequestBuilders.post -import org.springframework.restdocs.mockmvc.RestDocumentationRequestBuilders.put import org.springframework.restdocs.payload.JsonFieldType import org.springframework.security.crypto.password.PasswordEncoder import org.springframework.test.web.servlet.MockMvc import org.springframework.test.web.servlet.post -import org.springframework.test.web.servlet.put import org.springframework.test.web.servlet.result.MockMvcResultMatchers.content import org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath import org.springframework.test.web.servlet.result.MockMvcResultMatchers.status @@ -41,7 +38,6 @@ import pt.up.fe.ni.website.backend.utils.annotations.NestedTest import pt.up.fe.ni.website.backend.utils.documentation.payloadschemas.model.PayloadAccount import pt.up.fe.ni.website.backend.utils.documentation.utils.DocumentedJSONField import pt.up.fe.ni.website.backend.utils.documentation.utils.MockMVCExtension.Companion.andDocument -import pt.up.fe.ni.website.backend.utils.documentation.utils.MockMVCExtension.Companion.andDocumentCustomRequestSchema import pt.up.fe.ni.website.backend.utils.documentation.utils.MockMVCExtension.Companion.andDocumentCustomRequestSchemaEmptyResponse import pt.up.fe.ni.website.backend.utils.documentation.utils.MockMVCExtension.Companion.andDocumentCustomRequestSchemaErrorResponse import pt.up.fe.ni.website.backend.utils.documentation.utils.MockMVCExtension.Companion.andDocumentEmptyObjectResponse @@ -940,126 +936,6 @@ class AccountControllerTest @Autowired constructor( } } - @NestedTest - @DisplayName("PUT /recoverPassword/{recoveryToken}") - inner class RecoverPassword { - @field:Value("\${page.recover-password}") - private lateinit var recoverPasswordPage: String - - private val newPassword = "new-password" - - private val parameters = listOf( - parameterWithName("recoveryToken").description("The recovery token sent to the user's email.") - ) - - private val passwordRecoveryPayload = PayloadSchema( - "password-recover", - mutableListOf( - DocumentedJSONField("password", "The new password.", JsonFieldType.STRING) - ) - ) - - @BeforeEach - fun setup() { - repository.save(testAccount) - } - - @Test - fun `should update the password`() { - mockMvc.perform(post("/auth/recoverPassword/${testAccount.id}")) - .andReturn().response.let { authResponse -> - val recoveryToken = objectMapper.readTree(authResponse.contentAsString)["recovery_url"].asText() - .removePrefix("$recoverPasswordPage/") - mockMvc.perform( - put("/accounts/recoverPassword/{recoveryToken}", recoveryToken) - .contentType(MediaType.APPLICATION_JSON) - .content( - objectMapper.writeValueAsString( - mapOf( - "password" to newPassword - ) - ) - ) - ).andExpectAll( - status().isOk() - ).andDocumentCustomRequestSchema( - documentation, - passwordRecoveryPayload, - "Recover password", - "Update the password of an account using a recovery token.", - urlParameters = parameters, - 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 - fun `should fail when token is invalid`() { - mockMvc.perform( - put("/accounts/recoverPassword/invalid_token") - .contentType(MediaType.APPLICATION_JSON) - .content( - objectMapper.writeValueAsString( - mapOf( - "password" to newPassword - ) - ) - ) - ).andExpectAll( - status().isUnauthorized(), - jsonPath("$.errors.length()").value(1), - jsonPath("$.errors[0].message").value("invalid password recovery token") - ).andDocumentCustomRequestSchemaErrorResponse( - documentation, - passwordRecoveryPayload, - hasRequestPayload = true - ) - } - - @NestedTest - @DisplayName("Input Validation") - inner class InputValidation { - private val validationTester = ValidationTester( - req = { params: Map -> - 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 { val quotedDate = objectMapper.writeValueAsString(this) // objectMapper adds quotes to the date, so remove them diff --git a/src/test/kotlin/pt/up/fe/ni/website/backend/controller/AuthControllerTest.kt b/src/test/kotlin/pt/up/fe/ni/website/backend/controller/AuthControllerTest.kt index e895cae0..a9b499a6 100644 --- a/src/test/kotlin/pt/up/fe/ni/website/backend/controller/AuthControllerTest.kt +++ b/src/test/kotlin/pt/up/fe/ni/website/backend/controller/AuthControllerTest.kt @@ -1,38 +1,50 @@ package pt.up.fe.ni.website.backend.controller import com.epages.restdocs.apispec.HeaderDescriptorWithType +import com.epages.restdocs.apispec.ResourceDocumentation import com.epages.restdocs.apispec.ResourceDocumentation.headerWithName import com.fasterxml.jackson.databind.ObjectMapper import java.util.Calendar import org.hamcrest.Matchers.startsWith +import org.junit.jupiter.api.BeforeAll import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.DisplayName import org.junit.jupiter.api.Test import org.springframework.beans.factory.annotation.Autowired +import org.springframework.beans.factory.annotation.Value import org.springframework.http.HttpHeaders import org.springframework.http.MediaType import org.springframework.restdocs.mockmvc.RestDocumentationRequestBuilders.get import org.springframework.restdocs.mockmvc.RestDocumentationRequestBuilders.post +import org.springframework.restdocs.payload.JsonFieldType import org.springframework.security.crypto.password.PasswordEncoder import org.springframework.test.web.servlet.MockMvc import org.springframework.test.web.servlet.post +import org.springframework.test.web.servlet.result.MockMvcResultMatchers.content import org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath import org.springframework.test.web.servlet.result.MockMvcResultMatchers.status import pt.up.fe.ni.website.backend.dto.auth.LoginDto import pt.up.fe.ni.website.backend.dto.auth.TokenDto import pt.up.fe.ni.website.backend.model.Account import pt.up.fe.ni.website.backend.model.CustomWebsite +import pt.up.fe.ni.website.backend.model.constants.AccountConstants import pt.up.fe.ni.website.backend.repository.AccountRepository import pt.up.fe.ni.website.backend.utils.TestUtils +import pt.up.fe.ni.website.backend.utils.ValidationTester import pt.up.fe.ni.website.backend.utils.annotations.ControllerTest import pt.up.fe.ni.website.backend.utils.annotations.NestedTest +import pt.up.fe.ni.website.backend.utils.documentation.payloadschemas.model.PayloadAccount import pt.up.fe.ni.website.backend.utils.documentation.payloadschemas.model.PayloadAuthCheck import pt.up.fe.ni.website.backend.utils.documentation.payloadschemas.model.PayloadAuthNew import pt.up.fe.ni.website.backend.utils.documentation.payloadschemas.model.PayloadAuthRefresh import pt.up.fe.ni.website.backend.utils.documentation.payloadschemas.model.PayloadRecoverPassword +import pt.up.fe.ni.website.backend.utils.documentation.utils.DocumentedJSONField import pt.up.fe.ni.website.backend.utils.documentation.utils.MockMVCExtension.Companion.andDocument +import pt.up.fe.ni.website.backend.utils.documentation.utils.MockMVCExtension.Companion.andDocumentCustomRequestSchema +import pt.up.fe.ni.website.backend.utils.documentation.utils.MockMVCExtension.Companion.andDocumentCustomRequestSchemaErrorResponse import pt.up.fe.ni.website.backend.utils.documentation.utils.MockMVCExtension.Companion.andDocumentErrorResponse import pt.up.fe.ni.website.backend.utils.documentation.utils.ModelDocumentation +import pt.up.fe.ni.website.backend.utils.documentation.utils.PayloadSchema @ControllerTest class AuthControllerTest @Autowired constructor( @@ -182,8 +194,8 @@ class AuthControllerTest @Autowired constructor( } @NestedTest - @DisplayName("POST /auth/recoverPassword/{id}") - inner class RecoverPassword { + @DisplayName("POST /auth/password/recovery") + inner class RecoverPasswordRequest { var documentation: ModelDocumentation = PayloadRecoverPassword() @BeforeEach @@ -192,24 +204,33 @@ class AuthControllerTest @Autowired constructor( } @Test - fun `should fail if id is not found`() { - mockMvc.perform(post("/auth/recoverPassword/1234")) + fun `should fail if email is not found`() { + mockMvc.perform( + post("/auth/password/recovery") + .contentType(MediaType.APPLICATION_JSON) + .content(objectMapper.writeValueAsString(mapOf("email" to "dont@exist.com"))) + ) .andExpectAll( status().isNotFound(), jsonPath("$.errors.length()").value(1), - jsonPath("$.errors[0].message").value("account not found with id 1234") - ).andDocumentErrorResponse( + jsonPath("$.errors[0].message").value("account not found with email dont@exist.com") + ) + .andDocumentErrorResponse( documentation, "Recover password", "This endpoint operation allows the recovery of the password of an account, " + "sending an email with a link to reset the password.", - documentRequestPayload = false + documentRequestPayload = true ) } @Test fun `should return password recovery link`() { - mockMvc.perform(post("/auth/recoverPassword/${testAccount.id}")) + mockMvc.perform( + post("/auth/password/recovery") + .contentType(MediaType.APPLICATION_JSON) + .content(objectMapper.writeValueAsString(mapOf("email" to testAccount.email))) + ) .andExpectAll( status().isOk(), jsonPath("$.recovery_url").exists() @@ -218,8 +239,219 @@ class AuthControllerTest @Autowired constructor( "Recover password", "This endpoint operation allows the recovery of the password of an account, " + "sending an email with a link to reset the password.", - documentRequestPayload = false + documentRequestPayload = true + ) + } + } + + @NestedTest + @DisplayName("POST /auth/password/recovery/{token}/confirm") + inner class RecoverPasswordConfirm { + @field:Value("\${page.recover-password}") + private lateinit var recoverPasswordPage: String + + private val newPassword = "new-password" + + private val parameters = listOf( + ResourceDocumentation.parameterWithName("token").description("The recovery token sent to the user's email.") + ) + + private val passwordRecoveryPayload = PayloadSchema( + "password-recover", + mutableListOf( + DocumentedJSONField("password", "The new password.", JsonFieldType.STRING) + ) + ) + + private val documentation: ModelDocumentation = PayloadAccount() + + @BeforeEach + fun setup() { + repository.save(testAccount) + } + + @Test + fun `should update the password`() { + mockMvc.perform( + post("/auth/password/recovery") + .contentType(MediaType.APPLICATION_JSON) + .content( + objectMapper.writeValueAsString( + mapOf( + "email" to testAccount.email + ) + ) + ) + ).andReturn().response.let { authResponse -> + val token = objectMapper.readTree(authResponse.contentAsString)["recovery_url"].asText() + .removePrefix("$recoverPasswordPage/") + .removeSuffix("/confirm") + mockMvc.perform( + post("/auth/password/recovery/{token}/confirm", token) + .contentType(MediaType.APPLICATION_JSON) + .content( + objectMapper.writeValueAsString( + mapOf( + "password" to newPassword + ) + ) + ) + ).andExpectAll( + status().isOk(), + content().contentType(MediaType.APPLICATION_JSON), + jsonPath("$.id").value(testAccount.id) + ).andDocumentCustomRequestSchema( + documentation, + passwordRecoveryPayload, + "Recover password", + "Update the password of an account using a recovery token.", + urlParameters = parameters, + 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 + fun `should fail when token is invalid`() { + mockMvc.perform( + post("/auth/password/recovery/{token}/confirm", "invalid-token") + .contentType(MediaType.APPLICATION_JSON) + .content( + objectMapper.writeValueAsString( + mapOf( + "password" to newPassword + ) + ) + ) + ).andExpectAll( + status().isUnauthorized(), + jsonPath("$.errors.length()").value(1), + jsonPath("$.errors[0].message").value("invalid password recovery token") + ).andDocumentCustomRequestSchemaErrorResponse( + documentation, + passwordRecoveryPayload, + "Recover password", + "Update the password of an account using a recovery token.", + urlParameters = parameters, + documentRequestPayload = true + ) + + mockMvc.post("/auth/new") { + contentType = MediaType.APPLICATION_JSON + content = objectMapper.writeValueAsString( + mapOf( + "email" to testAccount.email, + "password" to newPassword + ) + ) + }.andExpect { status { isUnauthorized() } } + } + + @Test + fun `should fail when using recovery token twice`() { + mockMvc.perform( + post("/auth/password/recovery") + .contentType(MediaType.APPLICATION_JSON) + .content( + objectMapper.writeValueAsString( + mapOf( + "email" to testAccount.email + ) + ) + ) + ) + .andReturn().response.let { authResponse -> + val token = objectMapper.readTree(authResponse.contentAsString)["recovery_url"].asText() + .removePrefix("$recoverPasswordPage/") + .removeSuffix("/confirm") + mockMvc.perform( + post("/auth/password/recovery/{token}/confirm", token) + .contentType(MediaType.APPLICATION_JSON) + .content( + objectMapper.writeValueAsString( + mapOf( + "password" to newPassword + ) + ) + ) + ).andExpectAll( + status().isOk(), + content().contentType(MediaType.APPLICATION_JSON), + jsonPath("$.id").value(testAccount.id) + ) + mockMvc.perform( + post("/auth/password/recovery/{token}/confirm", token) + .contentType(MediaType.APPLICATION_JSON) + .content( + objectMapper.writeValueAsString( + mapOf( + "password" to "not using password" + ) + ) + ) + ).andExpectAll( + status().isUnauthorized(), + jsonPath("$.errors.length()").value(1), + jsonPath("$.errors[0].message").value("invalid password recovery token") + ).andDocumentCustomRequestSchemaErrorResponse( + documentation, + passwordRecoveryPayload, + "Recover password", + "Update the password of an account using a recovery token.", + urlParameters = parameters, + documentRequestPayload = true + ) + } + } + + @NestedTest + @DisplayName("Input Validation") + inner class InputValidation { + private val validationTester = ValidationTester( + req = { params: Map -> + mockMvc.perform( + post("/auth/password/recovery/random-token/confirm") + .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 ${AccountConstants.Password.minSize}" + + " and ${AccountConstants.Password.maxSize}()" + ) + fun size() = validationTester.hasSizeBetween( + AccountConstants.Password.minSize, + AccountConstants.Password.maxSize + ) + } } } diff --git a/src/test/kotlin/pt/up/fe/ni/website/backend/utils/documentation/payloadschemas/model/PayloadAuth.kt b/src/test/kotlin/pt/up/fe/ni/website/backend/utils/documentation/payloadschemas/model/PayloadAuth.kt index dff39edf..4dbf8042 100644 --- a/src/test/kotlin/pt/up/fe/ni/website/backend/utils/documentation/payloadschemas/model/PayloadAuth.kt +++ b/src/test/kotlin/pt/up/fe/ni/website/backend/utils/documentation/payloadschemas/model/PayloadAuth.kt @@ -61,16 +61,17 @@ class PayloadRecoverPassword : ModelDocumentation( Tag.AUTH, mutableListOf( DocumentedJSONField( - "id", - "Id of the account", - JsonFieldType.NUMBER, + "email", + "Email of the account", + JsonFieldType.STRING, isInResponse = false ), DocumentedJSONField( "recovery_url", "URL to recover the password", JsonFieldType.STRING, - isInRequest = false + isInRequest = false, + optional = true // change this when email service is implemented ) ) ) From ac49380a4f96493b4a015929c47b4976a7f596da Mon Sep 17 00:00:00 2001 From: Joaquim Cunha Date: Tue, 1 Aug 2023 16:30:10 +0100 Subject: [PATCH 09/10] changed endpoint error handling and minor fixes --- .../ni/website/backend/controller/AuthController.kt | 1 + .../fe/ni/website/backend/service/AccountService.kt | 5 +---- .../up/fe/ni/website/backend/service/AuthService.kt | 8 ++++++-- .../website/backend/controller/AuthControllerTest.kt | 12 ++++++------ .../payloadschemas/model/PayloadAuth.kt | 2 +- 5 files changed, 15 insertions(+), 13 deletions(-) diff --git a/src/main/kotlin/pt/up/fe/ni/website/backend/controller/AuthController.kt b/src/main/kotlin/pt/up/fe/ni/website/backend/controller/AuthController.kt index d3c5766d..aea8ebe8 100644 --- a/src/main/kotlin/pt/up/fe/ni/website/backend/controller/AuthController.kt +++ b/src/main/kotlin/pt/up/fe/ni/website/backend/controller/AuthController.kt @@ -39,6 +39,7 @@ class AuthController(val authService: AuthService) { @PostMapping("/password/recovery") fun generateRecoveryToken(@RequestBody recoveryRequestDto: PasswordRecoveryRequestDto): Map { val recoveryToken = authService.generateRecoveryToken(recoveryRequestDto.email) + ?: return emptyMap() // TODO: Change to email service return mapOf("recovery_url" to "$recoverPasswordPage/$recoveryToken/confirm") } diff --git a/src/main/kotlin/pt/up/fe/ni/website/backend/service/AccountService.kt b/src/main/kotlin/pt/up/fe/ni/website/backend/service/AccountService.kt index 383b7876..1653b4c6 100644 --- a/src/main/kotlin/pt/up/fe/ni/website/backend/service/AccountService.kt +++ b/src/main/kotlin/pt/up/fe/ni/website/backend/service/AccountService.kt @@ -3,10 +3,8 @@ package pt.up.fe.ni.website.backend.service import java.util.UUID import org.springframework.data.repository.findByIdOrNull import org.springframework.security.crypto.password.PasswordEncoder -import org.springframework.security.oauth2.jwt.JwtDecoder import org.springframework.stereotype.Service import org.springframework.web.multipart.MultipartFile -import pt.up.fe.ni.website.backend.config.Logging import pt.up.fe.ni.website.backend.dto.auth.ChangePasswordDto import pt.up.fe.ni.website.backend.dto.entity.account.CreateAccountDto import pt.up.fe.ni.website.backend.dto.entity.account.UpdateAccountDto @@ -19,9 +17,8 @@ import pt.up.fe.ni.website.backend.utils.extensions.filenameExtension class AccountService( private val repository: AccountRepository, private val encoder: PasswordEncoder, - private val jwtDecoder: JwtDecoder, private val fileUploader: FileUploader -) : Logging { +) { fun getAllAccounts(): List = repository.findAll().toList() fun createAccount(dto: CreateAccountDto): Account { diff --git a/src/main/kotlin/pt/up/fe/ni/website/backend/service/AuthService.kt b/src/main/kotlin/pt/up/fe/ni/website/backend/service/AuthService.kt index afc40094..7aa002d8 100644 --- a/src/main/kotlin/pt/up/fe/ni/website/backend/service/AuthService.kt +++ b/src/main/kotlin/pt/up/fe/ni/website/backend/service/AuthService.kt @@ -59,7 +59,11 @@ class AuthService( } fun generateRecoveryToken(email: String): String? { - val account = accountService.getAccountByEmail(email) + val account = try { + accountService.getAccountByEmail(email) + } catch (e: Exception) { + return null + } return generateToken( account, Duration.ofMinutes(authConfigProperties.jwtRecoveryExpirationMinutes), @@ -84,7 +88,7 @@ class AuthService( ?: throw InvalidBearerTokenException(ErrorMessages.invalidRecoveryToken) if (account.password != tokenPasswordHash) { - throw InvalidBearerTokenException(ErrorMessages.invalidRecoveryToken) + throw InvalidBearerTokenException(ErrorMessages.expiredRecoveryToken) } account.password = passwordEncoder.encode(dto.password) diff --git a/src/test/kotlin/pt/up/fe/ni/website/backend/controller/AuthControllerTest.kt b/src/test/kotlin/pt/up/fe/ni/website/backend/controller/AuthControllerTest.kt index a9b499a6..16eb4bc7 100644 --- a/src/test/kotlin/pt/up/fe/ni/website/backend/controller/AuthControllerTest.kt +++ b/src/test/kotlin/pt/up/fe/ni/website/backend/controller/AuthControllerTest.kt @@ -29,6 +29,7 @@ import pt.up.fe.ni.website.backend.model.Account import pt.up.fe.ni.website.backend.model.CustomWebsite import pt.up.fe.ni.website.backend.model.constants.AccountConstants import pt.up.fe.ni.website.backend.repository.AccountRepository +import pt.up.fe.ni.website.backend.service.ErrorMessages import pt.up.fe.ni.website.backend.utils.TestUtils import pt.up.fe.ni.website.backend.utils.ValidationTester import pt.up.fe.ni.website.backend.utils.annotations.ControllerTest @@ -204,18 +205,17 @@ class AuthControllerTest @Autowired constructor( } @Test - fun `should fail if email is not found`() { + fun `should return empty if email is not found`() { mockMvc.perform( post("/auth/password/recovery") .contentType(MediaType.APPLICATION_JSON) .content(objectMapper.writeValueAsString(mapOf("email" to "dont@exist.com"))) ) .andExpectAll( - status().isNotFound(), - jsonPath("$.errors.length()").value(1), - jsonPath("$.errors[0].message").value("account not found with email dont@exist.com") + status().isOk(), + jsonPath("$.recovery_url").doesNotExist() ) - .andDocumentErrorResponse( + .andDocument( documentation, "Recover password", "This endpoint operation allows the recovery of the password of an account, " + @@ -402,7 +402,7 @@ class AuthControllerTest @Autowired constructor( ).andExpectAll( status().isUnauthorized(), jsonPath("$.errors.length()").value(1), - jsonPath("$.errors[0].message").value("invalid password recovery token") + jsonPath("$.errors[0].message").value(ErrorMessages.expiredRecoveryToken) ).andDocumentCustomRequestSchemaErrorResponse( documentation, passwordRecoveryPayload, diff --git a/src/test/kotlin/pt/up/fe/ni/website/backend/utils/documentation/payloadschemas/model/PayloadAuth.kt b/src/test/kotlin/pt/up/fe/ni/website/backend/utils/documentation/payloadschemas/model/PayloadAuth.kt index 4dbf8042..510f0b4f 100644 --- a/src/test/kotlin/pt/up/fe/ni/website/backend/utils/documentation/payloadschemas/model/PayloadAuth.kt +++ b/src/test/kotlin/pt/up/fe/ni/website/backend/utils/documentation/payloadschemas/model/PayloadAuth.kt @@ -71,7 +71,7 @@ class PayloadRecoverPassword : ModelDocumentation( "URL to recover the password", JsonFieldType.STRING, isInRequest = false, - optional = true // change this when email service is implemented + optional = true // TODO change this when email service is implemented ) ) ) From 9a9fa6c0ea13d2bf8b2f962004d7ffa3dbf5403f Mon Sep 17 00:00:00 2001 From: Joaquim Cunha Date: Sat, 12 Aug 2023 07:23:53 +0100 Subject: [PATCH 10/10] improved jwt token error handling --- .../backend/controller/ErrorController.kt | 19 +++ .../ni/website/backend/service/AuthService.kt | 25 +--- .../website/backend/service/ErrorMessages.kt | 8 +- .../backend/controller/AuthControllerTest.kt | 115 +++++++++++++++++- 4 files changed, 136 insertions(+), 31 deletions(-) diff --git a/src/main/kotlin/pt/up/fe/ni/website/backend/controller/ErrorController.kt b/src/main/kotlin/pt/up/fe/ni/website/backend/controller/ErrorController.kt index 1546a9d1..8d11256e 100644 --- a/src/main/kotlin/pt/up/fe/ni/website/backend/controller/ErrorController.kt +++ b/src/main/kotlin/pt/up/fe/ni/website/backend/controller/ErrorController.kt @@ -10,6 +10,8 @@ import org.springframework.http.HttpStatus import org.springframework.http.converter.HttpMessageNotReadableException import org.springframework.security.access.AccessDeniedException import org.springframework.security.core.AuthenticationException +import org.springframework.security.oauth2.jwt.BadJwtException +import org.springframework.security.oauth2.jwt.JwtValidationException import org.springframework.web.bind.MethodArgumentNotValidException import org.springframework.web.bind.annotation.ExceptionHandler import org.springframework.web.bind.annotation.RequestMapping @@ -18,6 +20,7 @@ import org.springframework.web.bind.annotation.RestController import org.springframework.web.bind.annotation.RestControllerAdvice import org.springframework.web.multipart.MaxUploadSizeExceededException import pt.up.fe.ni.website.backend.config.Logging +import pt.up.fe.ni.website.backend.service.ErrorMessages data class SimpleError( val message: String, @@ -134,6 +137,22 @@ class ErrorController(private val objectMapper: ObjectMapper) : ErrorController, return wrapSimpleError(e.message ?: "invalid authentication") } + @ExceptionHandler(JwtValidationException::class) + @ResponseStatus(HttpStatus.UNAUTHORIZED) + fun invalidAuthentication(e: JwtValidationException): CustomError { + return if (e.message?.contains("expired") == true) { + wrapSimpleError(ErrorMessages.expiredToken) + } else { + wrapSimpleError(ErrorMessages.invalidToken) + } + } + + @ExceptionHandler(BadJwtException::class) + @ResponseStatus(HttpStatus.UNAUTHORIZED) + fun invalidAuthentication(e: BadJwtException): CustomError { + return wrapSimpleError(ErrorMessages.invalidToken) + } + fun wrapSimpleError(msg: String, param: String? = null, value: Any? = null) = CustomError( mutableListOf(SimpleError(msg, param, value)) ) diff --git a/src/main/kotlin/pt/up/fe/ni/website/backend/service/AuthService.kt b/src/main/kotlin/pt/up/fe/ni/website/backend/service/AuthService.kt index 7aa002d8..dfef1623 100644 --- a/src/main/kotlin/pt/up/fe/ni/website/backend/service/AuthService.kt +++ b/src/main/kotlin/pt/up/fe/ni/website/backend/service/AuthService.kt @@ -45,15 +45,7 @@ class AuthService( } fun refreshAccessToken(refreshToken: String): String { - val jwt = - try { - jwtDecoder.decode(refreshToken) - } catch (e: Exception) { - throw InvalidBearerTokenException(ErrorMessages.invalidRefreshToken) - } - if (jwt.expiresAt?.isBefore(Instant.now()) != false) { - throw InvalidBearerTokenException(ErrorMessages.expiredRefreshToken) - } + val jwt = jwtDecoder.decode(refreshToken) val account = accountService.getAccountByEmail(jwt.subject) return generateAccessToken(account) } @@ -72,23 +64,14 @@ class AuthService( } fun confirmRecoveryToken(recoveryToken: String, dto: PasswordRecoveryConfirmDto): Account { - val jwt = - try { - jwtDecoder.decode(recoveryToken) - } catch (e: Exception) { - throw InvalidBearerTokenException(ErrorMessages.invalidRecoveryToken) - } - - if (jwt.expiresAt?.isBefore(Instant.now()) != false) { - throw InvalidBearerTokenException(ErrorMessages.expiredRecoveryToken) - } + val jwt = jwtDecoder.decode(recoveryToken) val account = accountService.getAccountByEmail(jwt.subject) val tokenPasswordHash = jwt.getClaim("passwordHash") - ?: throw InvalidBearerTokenException(ErrorMessages.invalidRecoveryToken) + ?: throw InvalidBearerTokenException(ErrorMessages.invalidToken) if (account.password != tokenPasswordHash) { - throw InvalidBearerTokenException(ErrorMessages.expiredRecoveryToken) + throw InvalidBearerTokenException(ErrorMessages.invalidToken) } account.password = passwordEncoder.encode(dto.password) diff --git a/src/main/kotlin/pt/up/fe/ni/website/backend/service/ErrorMessages.kt b/src/main/kotlin/pt/up/fe/ni/website/backend/service/ErrorMessages.kt index cd55189d..ea20ceb8 100644 --- a/src/main/kotlin/pt/up/fe/ni/website/backend/service/ErrorMessages.kt +++ b/src/main/kotlin/pt/up/fe/ni/website/backend/service/ErrorMessages.kt @@ -7,13 +7,9 @@ object ErrorMessages { const val invalidCredentials = "invalid credentials" - const val invalidRefreshToken = "invalid refresh token" + const val invalidToken = "invalid token" - const val expiredRefreshToken = "refresh token has expired" - - const val invalidRecoveryToken = "invalid password recovery token" - - const val expiredRecoveryToken = "password recovery token has expired" + const val expiredToken = "token has expired" const val noGenerations = "no generations created yet" diff --git a/src/test/kotlin/pt/up/fe/ni/website/backend/controller/AuthControllerTest.kt b/src/test/kotlin/pt/up/fe/ni/website/backend/controller/AuthControllerTest.kt index 16eb4bc7..8dabdbcd 100644 --- a/src/test/kotlin/pt/up/fe/ni/website/backend/controller/AuthControllerTest.kt +++ b/src/test/kotlin/pt/up/fe/ni/website/backend/controller/AuthControllerTest.kt @@ -4,6 +4,8 @@ import com.epages.restdocs.apispec.HeaderDescriptorWithType import com.epages.restdocs.apispec.ResourceDocumentation import com.epages.restdocs.apispec.ResourceDocumentation.headerWithName import com.fasterxml.jackson.databind.ObjectMapper +import java.time.Instant +import java.time.temporal.ChronoUnit import java.util.Calendar import org.hamcrest.Matchers.startsWith import org.junit.jupiter.api.BeforeAll @@ -18,6 +20,10 @@ import org.springframework.restdocs.mockmvc.RestDocumentationRequestBuilders.get import org.springframework.restdocs.mockmvc.RestDocumentationRequestBuilders.post import org.springframework.restdocs.payload.JsonFieldType import org.springframework.security.crypto.password.PasswordEncoder +import org.springframework.security.oauth2.jwt.JwtClaimsSet +import org.springframework.security.oauth2.jwt.JwtDecoder +import org.springframework.security.oauth2.jwt.JwtEncoder +import org.springframework.security.oauth2.jwt.JwtEncoderParameters import org.springframework.test.web.servlet.MockMvc import org.springframework.test.web.servlet.post import org.springframework.test.web.servlet.result.MockMvcResultMatchers.content @@ -29,7 +35,6 @@ import pt.up.fe.ni.website.backend.model.Account import pt.up.fe.ni.website.backend.model.CustomWebsite import pt.up.fe.ni.website.backend.model.constants.AccountConstants import pt.up.fe.ni.website.backend.repository.AccountRepository -import pt.up.fe.ni.website.backend.service.ErrorMessages import pt.up.fe.ni.website.backend.utils.TestUtils import pt.up.fe.ni.website.backend.utils.ValidationTester import pt.up.fe.ni.website.backend.utils.annotations.ControllerTest @@ -52,6 +57,8 @@ class AuthControllerTest @Autowired constructor( val repository: AccountRepository, val mockMvc: MockMvc, val objectMapper: ObjectMapper, + val jwtEncoder: JwtEncoder, + val jwtDecoder: JwtDecoder, passwordEncoder: PasswordEncoder ) { final val testPassword = "testPassword" @@ -162,7 +169,7 @@ class AuthControllerTest @Autowired constructor( ) .andExpectAll( status().isUnauthorized, - jsonPath("$.errors[0].message").value("invalid refresh token") + jsonPath("$.errors[0].message").value("invalid token") ) .andDocumentErrorResponse(documentation, hasRequestPayload = true) } @@ -336,7 +343,7 @@ class AuthControllerTest @Autowired constructor( ).andExpectAll( status().isUnauthorized(), jsonPath("$.errors.length()").value(1), - jsonPath("$.errors[0].message").value("invalid password recovery token") + jsonPath("$.errors[0].message").value("invalid token") ).andDocumentCustomRequestSchemaErrorResponse( documentation, passwordRecoveryPayload, @@ -357,6 +364,106 @@ class AuthControllerTest @Autowired constructor( }.andExpect { status { isUnauthorized() } } } + @Test + fun `should fail when token is expired`() { + mockMvc.perform( + post("/auth/password/recovery") + .contentType(MediaType.APPLICATION_JSON) + .content( + objectMapper.writeValueAsString( + mapOf( + "email" to testAccount.email + ) + ) + ) + ) + .andReturn().response.let { authResponse -> + val token = objectMapper.readTree(authResponse.contentAsString)["recovery_url"].asText() + .removePrefix("$recoverPasswordPage/") + .removeSuffix("/confirm") + + val decoded = jwtDecoder.decode(token) + val newClaims = mutableMapOf() + newClaims.putAll(decoded.claims) + + val claimsBuilder = JwtClaimsSet + .builder() + .issuer("self") + .issuedAt(Instant.now().minus(2, ChronoUnit.DAYS)) + .expiresAt(Instant.now().minus(1, ChronoUnit.DAYS)) + .subject(decoded.subject) + .claim("scope", decoded.claims["scope"]) + + val newToken = jwtEncoder.encode(JwtEncoderParameters.from(claimsBuilder.build())).tokenValue + + mockMvc.perform( + post("/auth/password/recovery/{token}/confirm", newToken) + .contentType(MediaType.APPLICATION_JSON) + .content( + objectMapper.writeValueAsString( + mapOf( + "password" to newPassword + ) + ) + ) + ).andExpectAll( + status().isUnauthorized(), + jsonPath("$.errors.length()").value(1), + jsonPath("$.errors[0].message").value("token has expired") + ) + } + } + + @Test + fun `should fail when password hash claim is missing`() { + mockMvc.perform( + post("/auth/password/recovery") + .contentType(MediaType.APPLICATION_JSON) + .content( + objectMapper.writeValueAsString( + mapOf( + "email" to testAccount.email + ) + ) + ) + ) + .andReturn().response.let { authResponse -> + val token = objectMapper.readTree(authResponse.contentAsString)["recovery_url"].asText() + .removePrefix("$recoverPasswordPage/") + .removeSuffix("/confirm") + + val decoded = jwtDecoder.decode(token) + val newClaims = mutableMapOf() + newClaims.putAll(decoded.claims) + + val claimsBuilder = JwtClaimsSet + .builder() + .issuer("self") + .issuedAt(decoded.issuedAt) + .expiresAt(decoded.expiresAt) + .subject(decoded.subject) + .claim("scope", decoded.claims["scope"]) + + val newToken = jwtEncoder.encode(JwtEncoderParameters.from(claimsBuilder.build())).tokenValue + + mockMvc.perform( + post("/auth/password/recovery/{token}/confirm", newToken) + .contentType(MediaType.APPLICATION_JSON) + .content( + objectMapper.writeValueAsString( + mapOf( + "password" to newPassword + ) + ) + ) + ).andExpectAll( + status().isUnauthorized(), + jsonPath("$.errors.length()").value(1), + jsonPath("$.errors[0].message").value("invalid token") + ) + } + } + @Test fun `should fail when using recovery token twice`() { mockMvc.perform( @@ -402,7 +509,7 @@ class AuthControllerTest @Autowired constructor( ).andExpectAll( status().isUnauthorized(), jsonPath("$.errors.length()").value(1), - jsonPath("$.errors[0].message").value(ErrorMessages.expiredRecoveryToken) + jsonPath("$.errors[0].message").value("invalid token") ).andDocumentCustomRequestSchemaErrorResponse( documentation, passwordRecoveryPayload,