From 4ae19c09c25adaca03301df1833e9e0d58e26ad8 Mon Sep 17 00:00:00 2001 From: William Brawner Date: Sat, 9 Mar 2024 09:00:56 -0700 Subject: [PATCH] Fix failures in UserRouteTest --- .../kotlin/com/wbrawner/twigs/UserRoutes.kt | 142 +++++++++--------- .../server/api/PasswordResetRouteTest.kt | 26 +--- .../twigs/server/api/UserRouteTest.kt | 99 ++++-------- .../helpers/repository/FakeUserRepository.kt | 29 ++-- 4 files changed, 126 insertions(+), 170 deletions(-) diff --git a/api/src/main/kotlin/com/wbrawner/twigs/UserRoutes.kt b/api/src/main/kotlin/com/wbrawner/twigs/UserRoutes.kt index a95c217..c4ea8e6 100644 --- a/api/src/main/kotlin/com/wbrawner/twigs/UserRoutes.kt +++ b/api/src/main/kotlin/com/wbrawner/twigs/UserRoutes.kt @@ -13,26 +13,32 @@ import io.ktor.server.routing.* import java.time.Instant fun Application.userRoutes( - emailService: EmailService, - passwordResetRepository: PasswordResetRepository, - permissionRepository: PermissionRepository, - sessionRepository: SessionRepository, - userRepository: UserRepository, - passwordHasher: PasswordHasher + emailService: EmailService, + passwordResetRepository: PasswordResetRepository, + permissionRepository: PermissionRepository, + sessionRepository: SessionRepository, + userRepository: UserRepository, + passwordHasher: PasswordHasher ) { routing { route("/api/users") { post("/login") { val request = call.receive() val user = - userRepository.findAll(nameOrEmail = request.username, password = passwordHasher.hash(request.password)) - .firstOrNull() - ?: userRepository.findAll(nameOrEmail = request.username, password = passwordHasher.hash(request.password)) - .firstOrNull() - ?: run { - errorResponse(HttpStatusCode.Unauthorized, "Invalid credentials") - return@post - } + userRepository.findAll( + nameOrEmail = request.username, + password = passwordHasher.hash(request.password) + ) + .firstOrNull() + ?: userRepository.findAll( + nameOrEmail = request.username, + password = passwordHasher.hash(request.password) + ) + .firstOrNull() + ?: run { + errorResponse(HttpStatusCode.Unauthorized, "Invalid credentials") + return@post + } val session = sessionRepository.save(Session(userId = user.id)) call.respond(session.asResponse()) } @@ -48,53 +54,55 @@ fun Application.userRoutes( return@post } val existingUser = userRepository.findAll(nameOrEmail = request.username).firstOrNull() - ?: request.email?.let { - return@let if (it.isBlank()) { - null - } else { - userRepository.findAll(nameOrEmail = it).firstOrNull() - } + ?: request.email?.let { + return@let if (it.isBlank()) { + null + } else { + userRepository.findAll(nameOrEmail = it).firstOrNull() } + } existingUser?.let { errorResponse(HttpStatusCode.BadRequest, "Username or email already taken") return@post } call.respond( - userRepository.save( - User( - name = request.username, - password = passwordHasher.hash(request.password), - email = if (request.email.isNullOrBlank()) "" else request.email - ) - ).asResponse() + userRepository.save( + User( + name = request.username, + password = passwordHasher.hash(request.password), + email = if (request.email.isNullOrBlank()) "" else request.email + ) + ).asResponse() ) } authenticate(optional = false) { get { val query = call.request.queryParameters["query"] + val budgetIds = call.request.queryParameters.getAll("budgetId") if (query != null) { if (query.isBlank()) { errorResponse(HttpStatusCode.BadRequest, "query cannot be empty") } call.respond(userRepository.findAll(nameLike = query).map { it.asResponse() }) return@get + } else if (budgetIds == null || budgetIds.all { it.isBlank() }) { + errorResponse(HttpStatusCode.BadRequest, "query or budgetId required but absent") } - permissionRepository.findAll( - budgetIds = call.request.queryParameters.getAll("budgetId") - ).mapNotNull { - userRepository.findAll(ids = listOf(it.userId)) + permissionRepository.findAll(budgetIds = budgetIds) + .mapNotNull { + userRepository.findAll(ids = listOf(it.userId)) .firstOrNull() ?.asResponse() - }.run { call.respond(this) } + }.run { call.respond(this) } } get("/{id}") { userRepository.findAll(ids = call.parameters.getAll("id")) - .firstOrNull() - ?.asResponse() - ?.let { call.respond(it) } - ?: errorResponse(HttpStatusCode.NotFound) + .firstOrNull() + ?.asResponse() + ?.let { call.respond(it) } + ?: errorResponse(HttpStatusCode.NotFound) } put("/{id}") { @@ -106,17 +114,17 @@ fun Application.userRoutes( return@put } call.respond( - userRepository.save( - userRepository.findAll(ids = call.parameters.getAll("id")) - .first() - .run { - copy( - name = request.username ?: name, - password = request.password?.let { passwordHasher.hash(it) } ?: password, - email = request.email ?: email - ) - } - ).asResponse() + userRepository.save( + userRepository.findAll(ids = call.parameters.getAll("id")) + .first() + .run { + copy( + name = request.username ?: name, + password = request.password?.let { passwordHasher.hash(it) } ?: password, + email = request.email ?: email + ) + } + ).asResponse() ) } @@ -142,12 +150,12 @@ fun Application.userRoutes( post { val request = call.receive() userRepository.findAll(nameOrEmail = request.username) - .firstOrNull() - ?.let { - val email = it.email - val passwordResetToken = passwordResetRepository.save(PasswordResetToken(userId = it.id)) - emailService.sendPasswordResetEmail(passwordResetToken, email) - } + .firstOrNull() + ?.let { + val email = it.email + val passwordResetToken = passwordResetRepository.save(PasswordResetToken(userId = it.id)) + emailService.sendPasswordResetEmail(passwordResetToken, email) + } call.respond(HttpStatusCode.Accepted) } } @@ -156,25 +164,25 @@ fun Application.userRoutes( post { val request = call.receive() val passwordResetToken = passwordResetRepository.findAll(listOf(request.token)) - .firstOrNull() - ?: run { - errorResponse(HttpStatusCode.Unauthorized, "Invalid token") - return@post - } + .firstOrNull() + ?: run { + errorResponse(HttpStatusCode.Unauthorized, "Invalid token") + return@post + } if (passwordResetToken.expiration.isBefore(Instant.now())) { errorResponse(HttpStatusCode.Unauthorized, "Token expired") return@post } userRepository.findAll(listOf(passwordResetToken.userId)) - .firstOrNull() - ?.let { - userRepository.save(it.copy(password = passwordHasher.hash(request.password))) - passwordResetRepository.delete(passwordResetToken) - } - ?: run { - errorResponse(HttpStatusCode.InternalServerError, "Invalid token") - return@post - } + .firstOrNull() + ?.let { + userRepository.save(it.copy(password = passwordHasher.hash(request.password))) + passwordResetRepository.delete(passwordResetToken) + } + ?: run { + errorResponse(HttpStatusCode.InternalServerError, "Invalid token") + return@post + } call.respond(HttpStatusCode.NoContent) } } diff --git a/app/src/test/kotlin/com/wbrawner/twigs/server/api/PasswordResetRouteTest.kt b/app/src/test/kotlin/com/wbrawner/twigs/server/api/PasswordResetRouteTest.kt index bec458b..811d53a 100644 --- a/app/src/test/kotlin/com/wbrawner/twigs/server/api/PasswordResetRouteTest.kt +++ b/app/src/test/kotlin/com/wbrawner/twigs/server/api/PasswordResetRouteTest.kt @@ -4,8 +4,8 @@ import com.wbrawner.twigs.ErrorResponse import com.wbrawner.twigs.PasswordResetRequest import com.wbrawner.twigs.ResetPasswordRequest import com.wbrawner.twigs.model.PasswordResetToken -import com.wbrawner.twigs.model.User import com.wbrawner.twigs.randomString +import com.wbrawner.twigs.test.helpers.repository.FakeUserRepository.Companion.TEST_USER import io.ktor.client.call.* import io.ktor.client.request.* import io.ktor.http.* @@ -17,7 +17,7 @@ import java.util.* class PasswordResetRouteTest : ApiTest() { @Test fun `reset password with invalid username returns 202`() = apiTest { client -> - val request = ResetPasswordRequest(username = "testuser") + val request = ResetPasswordRequest(username = "invaliduser") val response = client.post("/api/resetpassword") { header("Content-Type", "application/json") setBody(request) @@ -28,14 +28,6 @@ class PasswordResetRouteTest : ApiTest() { @Test fun `reset password with valid username returns 202`() = apiTest { client -> - val users = listOf( - User( - name = "testuser", - email = "test@example.com", - password = "\$2a\$10\$bETxbFPja1PyXVLybETxb.CWBYzyYdZpmCcA7NSIN8dkdzidt1Xv2" - ), - ) - users.forEach { userRepository.save(it) } val request = ResetPasswordRequest(username = "testuser") val response = client.post("/api/resetpassword") { header("Content-Type", "application/json") @@ -44,10 +36,10 @@ class PasswordResetRouteTest : ApiTest() { assertEquals(HttpStatusCode.Accepted, response.status) assertEquals(1, emailService.emails.size) val email = emailService.emails.first() - assertEquals(users.first().email, email.to) + assertEquals(TEST_USER.email, email.to) assertEquals(1, passwordResetRepository.entities.size) val passwordReset = passwordResetRepository.entities.first() - assertEquals(users.first().id, passwordReset.userId) + assertEquals(TEST_USER.id, passwordReset.userId) } @Test @@ -77,11 +69,7 @@ class PasswordResetRouteTest : ApiTest() { @Test fun `password reset with valid token returns 200`() = apiTest { client -> - val users = listOf( - User(name = "testuser", password = "\$2a\$10\$bETxbFPja1PyXVLybETxb.CWBYzyYdZpmCcA7NSIN8dkdzidt1Xv2"), - ) - users.forEach { userRepository.save(it) } - val token = passwordResetRepository.save(PasswordResetToken(userId = users.first().id)) + val token = passwordResetRepository.save(PasswordResetToken(userId = userRepository.findAll("testuser").first().id)) val request = PasswordResetRequest(token = token.id, password = "newpass") val response = client.post("/api/passwordreset") { header("Content-Type", "application/json") @@ -89,8 +77,8 @@ class PasswordResetRouteTest : ApiTest() { } assertEquals(HttpStatusCode.NoContent, response.status) assertEquals( - "\$2a\$10\$bETxbFPja1PyXVLybETxb.E7dYGWCalFjrgd3ofAfKD8MqR0Ukua6", - userRepository.entities.first().password + "newpass", + userRepository.findAll(TEST_USER.name).first().password ) assert(passwordResetRepository.entities.isEmpty()) } diff --git a/app/src/test/kotlin/com/wbrawner/twigs/server/api/UserRouteTest.kt b/app/src/test/kotlin/com/wbrawner/twigs/server/api/UserRouteTest.kt index 4ba200a..38902e4 100644 --- a/app/src/test/kotlin/com/wbrawner/twigs/server/api/UserRouteTest.kt +++ b/app/src/test/kotlin/com/wbrawner/twigs/server/api/UserRouteTest.kt @@ -3,10 +3,13 @@ package com.wbrawner.twigs.server.api import com.wbrawner.twigs.* import com.wbrawner.twigs.model.Session import com.wbrawner.twigs.model.User +import com.wbrawner.twigs.test.helpers.repository.FakeUserRepository.Companion.OTHER_USER +import com.wbrawner.twigs.test.helpers.repository.FakeUserRepository.Companion.TEST_USER import io.ktor.client.call.* import io.ktor.client.request.* import io.ktor.http.* import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertNotNull import org.junit.jupiter.api.Test class UserRouteTest : ApiTest() { @@ -36,11 +39,7 @@ class UserRouteTest : ApiTest() { @Test fun `login with invalid password returns 401`() = apiTest { client -> - val users = listOf( - User(name = "testuser", password = "\$2a\$10\$bETxbFPja1PyXVLybETxb.CWBYzyYdZpmCcA7NSIN8dkdzidt1Xv2"), - ) - users.forEach { userRepository.save(it) } - val request = LoginRequest("testuser", "pass") + val request = LoginRequest(TEST_USER.name, "pass") val response = client.post("/api/users/login") { header("Content-Type", "application/json") setBody(request) @@ -52,11 +51,7 @@ class UserRouteTest : ApiTest() { @Test fun `login with empty password returns 401`() = apiTest { client -> - val users = listOf( - User(name = "testuser", password = "\$2a\$10\$bETxbFPja1PyXVLybETxb.CWBYzyYdZpmCcA7NSIN8dkdzidt1Xv2"), - ) - users.forEach { userRepository.save(it) } - val request = LoginRequest("testuser", "") + val request = LoginRequest(TEST_USER.name, "") val response = client.post("/api/users/login") { header("Content-Type", "application/json") setBody(request) @@ -68,41 +63,27 @@ class UserRouteTest : ApiTest() { @Test fun `login with valid username and password returns 200`() = apiTest { client -> - val users = listOf( - User(name = "testuser", password = "\$2a\$10\$bETxbFPja1PyXVLybETxb.CWBYzyYdZpmCcA7NSIN8dkdzidt1Xv2"), - User(name = "otheruser", password = "\$2a\$10\$bETxbFPja1PyXVLybETxb..rhfIeOkP4qil1Drj29LDUhBxVkm6fS"), - ) - users.forEach { userRepository.save(it) } - val request = LoginRequest("testuser", "testpassword") + val request = LoginRequest(TEST_USER.name, TEST_USER.password) val response = client.post("/api/users/login") { header("Content-Type", "application/json") setBody(request) } assertEquals(HttpStatusCode.OK, response.status) val session = response.body() - assertEquals(users.first().id, session.userId) + assertEquals(TEST_USER.id, session.userId) assert(session.token.isNotBlank()) } @Test fun `login with valid email and password returns 200`() = apiTest { client -> - val users = listOf( - User( - name = "testuser", - email = "test@example.com", - password = "\$2a\$10\$bETxbFPja1PyXVLybETxb.CWBYzyYdZpmCcA7NSIN8dkdzidt1Xv2" - ), - User(name = "otheruser", password = "\$2a\$10\$bETxbFPja1PyXVLybETxb..rhfIeOkP4qil1Drj29LDUhBxVkm6fS"), - ) - users.forEach { userRepository.save(it) } - val request = LoginRequest("test@example.com", "testpassword") + val request = LoginRequest(TEST_USER.email, TEST_USER.password) val response = client.post("/api/users/login") { header("Content-Type", "application/json") setBody(request) } assertEquals(HttpStatusCode.OK, response.status) val session = response.body() - assertEquals(users.first().id, session.userId) + assertEquals(TEST_USER.id, session.userId) assert(session.token.isNotBlank()) } @@ -156,15 +137,7 @@ class UserRouteTest : ApiTest() { @Test fun `register with existing username returns 400`() = apiTest { client -> - val users = listOf( - User( - name = "testuser", - email = "test@example.com", - password = "\$2a\$10\$bETxbFPja1PyXVLybETxb.CWBYzyYdZpmCcA7NSIN8dkdzidt1Xv2" - ), - ) - users.forEach { userRepository.save(it) } - val request = UserRequest(username = "testuser", password = "password") + val request = UserRequest(username = TEST_USER.name, password = "password") val response = client.post("/api/users/register") { header("Content-Type", "application/json") setBody(request) @@ -176,15 +149,7 @@ class UserRouteTest : ApiTest() { @Test fun `register with existing email returns 400`() = apiTest { client -> - val users = listOf( - User( - name = "testuser", - email = "test@example.com", - password = "\$2a\$10\$bETxbFPja1PyXVLybETxb.CWBYzyYdZpmCcA7NSIN8dkdzidt1Xv2" - ), - ) - users.forEach { userRepository.save(it) } - val request = UserRequest(username = "testuser2", email = "test@example.com", password = "password") + val request = UserRequest(username = "testuser2", email = TEST_USER.email, password = "password") val response = client.post("/api/users/register") { header("Content-Type", "application/json") setBody(request) @@ -196,7 +161,8 @@ class UserRouteTest : ApiTest() { @Test fun `register with valid username and password returns 200`() = apiTest { client -> - val request = UserRequest("testuser", "testpassword") + val initialUserCount = userRepository.entities.size + val request = UserRequest("newuser", "newpass") val response = client.post("/api/users/register") { header("Content-Type", "application/json") setBody(request) @@ -206,12 +172,14 @@ class UserRouteTest : ApiTest() { assert(userResponse.id.isNotBlank()) assertEquals(request.username, userResponse.username) assertEquals("", userResponse.email) - assertEquals(1, userRepository.entities.size) - val savedUser: User = userRepository.entities.first() + assertEquals(initialUserCount + 1, userRepository.entities.size) + val savedUser: User? = userRepository.findAll("newuser").firstOrNull() + assertNotNull(savedUser) + requireNotNull(savedUser) assertEquals(userResponse.id, savedUser.id) assertEquals(request.username, savedUser.name) assertEquals("", savedUser.email) - assertEquals("\$2a\$10\$bETxbFPja1PyXVLybETxb.CWBYzyYdZpmCcA7NSIN8dkdzidt1Xv2", savedUser.password) + assertEquals("newpass", savedUser.password) } @Test @@ -256,12 +224,7 @@ class UserRouteTest : ApiTest() { @Test fun `get users with valid query and matches returns list`() = apiTest { client -> - val users = listOf( - User(name = "testuser", password = "testpassword"), - User(name = "otheruser", password = "otherpassword"), - ) - users.forEach { userRepository.save(it) } - val session = Session(userId = users.first().id) + val session = Session(userId = TEST_USER.id) sessionRepository.save(session) val response = client.get("/api/users?query=user") { header("Authorization", "Bearer ${session.token}") @@ -269,25 +232,17 @@ class UserRouteTest : ApiTest() { assertEquals(HttpStatusCode.OK, response.status) val userQueryResponse: List = response.body() assertEquals(2, userQueryResponse.size) - repeat(2) { i -> - assertEquals(users[i].id, userQueryResponse[i].id, "User IDs at index $i don't match") - assertEquals(users[i].name, userQueryResponse[i].username, "Usernames at index $i don't match") - assertEquals(users[i].email, userQueryResponse[i].email, "User emails at index $i don't match") - } + assertEquals(TEST_USER.asResponse(), userQueryResponse[0]) + assertEquals(OTHER_USER.asResponse(), userQueryResponse[1]) } @Test fun `get users with empty budgetId returns 400`() = apiTest { client -> - + val session = Session(userId = TEST_USER.id) + sessionRepository.save(session) + val response = client.get("/api/users?budgetId=") { + header("Authorization", "Bearer ${session.token}") + } + assertEquals(HttpStatusCode.BadRequest, response.status) } - -// @Test -// fun ``() = apiTest { client -> -// -// } - -// @Test -// fun ``() = apiTest { client -> -// -// } } \ No newline at end of file diff --git a/testhelpers/src/main/kotlin/com/wbrawner/twigs/test/helpers/repository/FakeUserRepository.kt b/testhelpers/src/main/kotlin/com/wbrawner/twigs/test/helpers/repository/FakeUserRepository.kt index 86a5524..05d62f7 100644 --- a/testhelpers/src/main/kotlin/com/wbrawner/twigs/test/helpers/repository/FakeUserRepository.kt +++ b/testhelpers/src/main/kotlin/com/wbrawner/twigs/test/helpers/repository/FakeUserRepository.kt @@ -4,18 +4,7 @@ import com.wbrawner.twigs.model.User import com.wbrawner.twigs.storage.UserRepository class FakeUserRepository : FakeRepository(), UserRepository { - override val entities: MutableList = mutableListOf( - User( - name = "testuser", - email = "test@example.com", - password = "\$2a\$10\$bETxbFPja1PyXVLybETxb.CWBYzyYdZpmCcA7NSIN8dkdzidt1Xv2" // testpass - ), - User( - name = "otheruser", - email = "other@example.com", - password = "\$2a\$10\$bETxbFPja1PyXVLybETxb..rhfIeOkP4qil1Drj29LDUhBxVkm6fS" - ), - ) + override val entities: MutableList = mutableListOf(TEST_USER, OTHER_USER) override fun findAll(nameOrEmail: String, password: String?): List { return entities.filter { user -> @@ -29,4 +18,20 @@ class FakeUserRepository : FakeRepository(), UserRepository { override fun findAll(nameLike: String): List { return entities.filter { it.name.contains(nameLike, ignoreCase = true) } } + + companion object { + val TEST_USER = User( + id = "id-test-user", + name = "testuser", + email = "test@example.com", + password = "testpass" + ) + + val OTHER_USER = User( + id = "id-other-user", + name = "otheruser", + email = "other@example.com", + password = "otherpass" + ) + } } \ No newline at end of file