Skip to content
This repository has been archived by the owner on Jan 14, 2024. It is now read-only.

Commit

Permalink
feat(rohan): store hashed version of user password
Browse files Browse the repository at this point in the history
  • Loading branch information
PascalHonegger committed May 7, 2021
1 parent fcc87fc commit 1462492
Show file tree
Hide file tree
Showing 6 changed files with 90 additions and 14 deletions.
1 change: 1 addition & 0 deletions backend/rohan/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ dependencies {
implementation(libs.kotlinx.serialization.json)
implementation(libs.grpc.kotlin)
implementation(libs.jjwt.api)
implementation(libs.argon2.jvm)
runtimeOnly(libs.logback)
runtimeOnly(libs.jjwt.impl)
runtimeOnly(libs.jjwt.jackson)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,5 +24,5 @@ data class TemporaryUser(
@Serializable
data class UserCredentials(
val name: String,
val password: String
val passwordHash: String
)
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,19 @@ private val logger: Logger =
private fun createUniqueIdentifier() =
UUID.randomUUID().toString()

private fun UserScore.removeSensitiveInformation() = copy(
user = user.removeSensitiveInformation()
)

private fun User.removeSensitiveInformation() = when (this) {
is RegularUser -> removeSensitiveInformation()
is TemporaryUser -> removeSensitiveInformation()
}

private fun TemporaryUser.removeSensitiveInformation() = this
private fun RegularUser.removeSensitiveInformation() =
copy(credentials = credentials.copy(passwordHash = ""))

@ExperimentalPathApi
@ExperimentalStdlibApi
@Singleton
Expand All @@ -48,7 +61,7 @@ class StorageServiceImpl(config: StorageConfig) :
val newUser = processor(identifier)
requireUsernameUnique(newUser)
dataStore[identifier] = UserScore(user = newUser, score = 0)
return newUser
return newUser.removeSensitiveInformation()
}

override fun updateUser(
Expand All @@ -63,7 +76,7 @@ class StorageServiceImpl(config: StorageConfig) :
val updatedUser = processor(entry.user)
requireUsernameUnique(updatedUser)
dataStore[identifier] = entry.copy(user = updatedUser)
return updatedUser
return updatedUser.removeSensitiveInformation()
}

override fun updateScore(
Expand All @@ -77,11 +90,11 @@ class StorageServiceImpl(config: StorageConfig) :
}
val updatedUser = entry.copy(score = processor(entry.score))
dataStore[identifier] = updatedUser
return updatedUser
return updatedUser.removeSensitiveInformation()
}

override fun getUser(identifier: String): UserScore? =
lock.read { dataStore[identifier] }
lock.read { dataStore[identifier]?.removeSensitiveInformation() }

override fun findUser(
username: String,
Expand All @@ -92,10 +105,9 @@ class StorageServiceImpl(config: StorageConfig) :
.map { it.user }
.filterIsInstance<RegularUser>()
.find {
it.credentials.name.lowercase() == username.lowercase() && additionalFilter(
it
)
}
it.credentials.name.lowercase() == username.lowercase()
&& additionalFilter(it)
}?.removeSensitiveInformation()
}

private fun requireUsernameUnique(changedUser: User): Unit =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import ch.chaosconnect.rohan.model.RegularUser
import ch.chaosconnect.rohan.model.TemporaryUser
import ch.chaosconnect.rohan.model.User
import ch.chaosconnect.rohan.model.UserCredentials
import de.mkammerer.argon2.Argon2Factory
import javax.inject.Singleton
import javax.security.auth.login.AccountException
import javax.security.auth.login.FailedLoginException
Expand All @@ -16,6 +17,8 @@ private fun requireNotBlank(value: CharSequence, name: String) =

private fun getCurrentIdentifier() = userIdentifierContextKey.get()

private val argon2 = Argon2Factory.create()

@Singleton
class UserServiceImpl(private val storageService: StorageService) :
UserService {
Expand All @@ -28,12 +31,15 @@ class UserServiceImpl(private val storageService: StorageService) :
requireNotBlank(password, "Password")
requireNotBlank(displayName, "Display name")

val credentials =
UserCredentials(name = name, passwordHash = hashPassword(password))

when (val currentIdentifier = getCurrentIdentifier()) {
null -> storageService.addUser {
RegularUser(
identifier = it,
displayName = displayName,
credentials = UserCredentials(name, password)
credentials = credentials
)
}
else -> storageService.updateUser(currentIdentifier) {
Expand All @@ -42,7 +48,7 @@ class UserServiceImpl(private val storageService: StorageService) :
is TemporaryUser -> RegularUser(
identifier = it.identifier,
displayName = it.displayName,
credentials = UserCredentials(name, password)
credentials = credentials
)
}
}
Expand All @@ -59,7 +65,12 @@ class UserServiceImpl(private val storageService: StorageService) :
name: String,
password: String
): String = signIn {
storageService.findUser(name) { it.credentials.password == password }
storageService.findUser(name) {
verifyPassword(
hash = it.credentials.passwordHash,
password = password
)
}
?: throw FailedLoginException("Sign-in failed")
}

Expand All @@ -70,7 +81,7 @@ class UserServiceImpl(private val storageService: StorageService) :
when (it) {
is RegularUser -> it.copy(
credentials = it.credentials.copy(
password = password
passwordHash = hashPassword(password)
)
)
is TemporaryUser -> throw AccountException("Cannot set password for temporary user")
Expand Down Expand Up @@ -100,4 +111,10 @@ class UserServiceImpl(private val storageService: StorageService) :

private fun update(userProvider: () -> User): String =
userProvider().identifier

private fun hashPassword(password: String): String =
argon2.hash(10, 65536, 1, password.toCharArray())

private fun verifyPassword(hash: String, password: String): Boolean =
argon2.verify(hash, password.toCharArray())
}
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,48 @@ internal class StorageServiceImplTest {
assertEquals(updatedUser, service.getUser(updatedUser.identifier)?.user)
}

@Test
fun `get user does not include sensitive information`() {
val addedUser = service.addUser {
RegularUser(
identifier = it,
displayName = "Dummy User",
credentials = UserCredentials("dummy", "some-password-hash")
)
}
val loadedUser = service.getUser(addedUser.identifier)
assertEquals(
"",
(loadedUser?.user as RegularUser).credentials.passwordHash
)
}

@Test
fun `update user does not include sensitive information`() {
val addedUser = service.addUser {
RegularUser(
identifier = it,
displayName = "Dummy User",
credentials = UserCredentials("dummy", "some-password-hash")
)
}
val updatedUser = service.updateUser(addedUser.identifier) { it }
assertEquals("", (updatedUser as RegularUser).credentials.passwordHash)
}

@Test
fun `update score does not include sensitive information`() {
val addedUser = service.addUser {
RegularUser(
identifier = it,
displayName = "Dummy User",
credentials = UserCredentials("dummy", "some-password-hash")
)
}
val updatedUser = service.updateScore(addedUser.identifier) { it }
assertEquals("", (updatedUser.user as RegularUser).credentials.passwordHash)
}

@Test
fun `can update a temporary to a regular user`() {
val addedUser = service.addUser { TemporaryUser(it, "Dummy User") }
Expand Down Expand Up @@ -107,7 +149,7 @@ internal class StorageServiceImplTest {
}
assertEquals(addedUser, service.findUser("myuser"))
assertEquals(addedUser, service.findUser("myuser") {
it.credentials.password == "123"
it.credentials.passwordHash == "123"
})
assertNull(service.findUser("myuser") { false })
assertNull(service.findUser("OtherUser"))
Expand Down
4 changes: 4 additions & 0 deletions backend/settings.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ dependencyResolutionManagement {
version("jjwt", "0.11.2")
version("mockk", "1.11.0")
version("turbine", "0.4.1")
version("argon2-jvm", "2.10.1")

alias("kotlin-stdlib").to(
"org.jetbrains.kotlin",
Expand Down Expand Up @@ -114,6 +115,9 @@ dependencyResolutionManagement {
alias("jjwt-jackson").to("io.jsonwebtoken", "jjwt-jackson")
.versionRef("jjwt")

alias("argon2-jvm").to("de.mkammerer", "argon2-jvm")
.versionRef("argon2-jvm")

alias("logback").to("ch.qos.logback", "logback-classic")
.withoutVersion()

Expand Down

0 comments on commit 1462492

Please sign in to comment.