changed endpoint error handling and minor fixes

NIAEFEUP · Aug 26, 2023 · ac49380 · ac49380
1 parent 723d8c9
commit ac49380
Show file tree

Hide file tree

Showing 5 changed files with 15 additions and 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
@@ -39,6 +39,7 @@ class AuthController(val authService: AuthService) {
     @PostMapping("/password/recovery")
     fun generateRecoveryToken(@RequestBody recoveryRequestDto: PasswordRecoveryRequestDto): Map<String, String> {
         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
@@ -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<Account> = 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
@@ -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
@@ -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 "[email protected]")))
             )
                 .andExpectAll(
-                    status().isNotFound(),
-                    jsonPath("$.errors.length()").value(1),
-                    jsonPath("$.errors[0].message").value("account not found with email [email protected]")
+                    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/...otlin/pt/up/fe/ni/website/backend/utils/documentation/payloadschemas/model/PayloadAuth.kt b/...otlin/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
         )
     )
 )