Credentials / dav4jvm: store passwords as CharArray (#1483)

* Credentials / dav4jvm: store passwords as CharArray

* Fix tests
This commit is contained in:
Ricki Hirner
2025-05-30 17:37:14 +02:00
committed by GitHub
parent 5c485834e9
commit 711543c5f1
16 changed files with 63 additions and 29 deletions

View File

@@ -70,7 +70,7 @@ class DavResourceFinderTest {
start()
}
val credentials = Credentials("mock", "12345")
val credentials = Credentials(username = "mock", password = "12345".toCharArray())
client = httpClientBuilder
.authenticate(host = null, credentials = credentials)
.build()

View File

@@ -1,3 +1,7 @@
/*
* Copyright © All Contributors. See LICENSE and AUTHORS in the root directory for details.
*/
package at.bitfire.davdroid.ui.setup
import android.content.Intent
@@ -17,7 +21,7 @@ class LoginActivityTest {
val loginInfo = LoginActivity.loginInfoFromIntent(intent)
assertEquals("https://example.com/nextcloud", loginInfo.baseUri.toString())
assertEquals("user", loginInfo.credentials!!.username)
assertEquals("password", loginInfo.credentials.password)
assertEquals("password", loginInfo.credentials.password?.concatToString())
}
@Test
@@ -30,7 +34,7 @@ class LoginActivityTest {
val loginInfo = LoginActivity.loginInfoFromIntent(intent)
assertEquals("https://example.com:444/nextcloud", loginInfo.baseUri.toString())
assertEquals("user", loginInfo.credentials!!.username)
assertEquals("password", loginInfo.credentials.password)
assertEquals("password", loginInfo.credentials.password?.concatToString())
}
@Test
@@ -39,7 +43,7 @@ class LoginActivityTest {
val loginInfo = LoginActivity.loginInfoFromIntent(intent)
assertEquals("https://example.com/path", loginInfo.baseUri.toString())
assertEquals("user", loginInfo.credentials!!.username)
assertEquals("password", loginInfo.credentials.password)
assertEquals("password", loginInfo.credentials.password?.concatToString())
}
@Test
@@ -48,7 +52,7 @@ class LoginActivityTest {
val loginInfo = LoginActivity.loginInfoFromIntent(intent)
assertEquals("https://example.com:0/path", loginInfo.baseUri.toString())
assertEquals("user", loginInfo.credentials!!.username)
assertEquals("password", loginInfo.credentials.password)
assertEquals("password", loginInfo.credentials.password?.concatToString())
}
@Test
@@ -57,7 +61,7 @@ class LoginActivityTest {
val loginInfo = LoginActivity.loginInfoFromIntent(intent)
assertEquals(null, loginInfo.baseUri)
assertEquals("user@example.com", loginInfo.credentials!!.username)
assertEquals(null, loginInfo.credentials.password)
assertEquals(null, loginInfo.credentials.password?.concatToString())
}
}

View File

@@ -30,8 +30,8 @@ class CredentialsStoreTest {
@Test
fun testSetGetDelete() {
store.setCredentials(0, Credentials(username = "myname", password = "12345"))
assertEquals(Credentials(username = "myname", password = "12345"), store.getCredentials(0))
store.setCredentials(0, Credentials(username = "myname", password = "12345".toCharArray()))
assertEquals(Credentials(username = "myname", password = "12345".toCharArray()), store.getCredentials(0))
store.setCredentials(0, null)
assertNull(store.getCredentials(0))

View File

@@ -8,7 +8,7 @@ import net.openid.appauth.AuthState
data class Credentials(
val username: String? = null,
val password: String? = null,
val password: CharArray? = null,
val certificateAlias: String? = null,
@@ -32,4 +32,27 @@ data class Credentials(
return "Credentials(" + s.joinToString(", ") + ")"
}
override fun equals(other: Any?): Boolean {
if (this === other) return true
if (javaClass != other?.javaClass) return false
other as Credentials
if (username != other.username) return false
if (!password.contentEquals(other.password)) return false
if (certificateAlias != other.certificateAlias) return false
if (authState != other.authState) return false
return true
}
override fun hashCode(): Int {
var result = username?.hashCode() ?: 0
result = 31 * result + (password?.contentHashCode() ?: 0)
result = 31 * result + (certificateAlias?.hashCode() ?: 0)
result = 31 * result + (authState?.hashCode() ?: 0)
return result
}
}

View File

@@ -107,7 +107,12 @@ class HttpClient(
} else if (credentials.username != null && credentials.password != null) {
// basic/digest auth
val authHandler = BasicDigestAuthHandler(UrlUtils.hostToDomain(host), credentials.username, credentials.password, insecurePreemptive = true)
val authHandler = BasicDigestAuthHandler(
domain = UrlUtils.hostToDomain(host),
username = credentials.username,
password = credentials.password,
insecurePreemptive = true
)
authenticationInterceptor = authHandler
authenticator = authHandler
}

View File

@@ -106,7 +106,7 @@ class NextcloudLoginFlow @Inject constructor(
baseUri = URI(serverUrl).resolve(DAV_PATH),
credentials = Credentials(
username = json.getString("loginName"),
password = json.getString("appPassword")
password = json.getString("appPassword").toCharArray()
),
suggestedGroupMethod = GroupMethod.CATEGORIES
)

View File

@@ -107,7 +107,7 @@ class AccountSettings @AssistedInject constructor(
fun credentials() = Credentials(
accountManager.getUserData(account, KEY_USERNAME),
accountManager.getPassword(account),
accountManager.getPassword(account)?.toCharArray(),
accountManager.getUserData(account, KEY_CERTIFICATE_ALIAS),
@@ -119,7 +119,7 @@ class AccountSettings @AssistedInject constructor(
fun credentials(credentials: Credentials) {
// Basic/Digest auth
accountManager.setAndVerifyUserData(account, KEY_USERNAME, credentials.username)
accountManager.setPassword(account, credentials.password)
accountManager.setPassword(account, credentials.password?.concatToString())
// client certificate
accountManager.setAndVerifyUserData(account, KEY_CERTIFICATE_ALIAS, credentials.certificateAlias)

View File

@@ -18,13 +18,14 @@ object SystemAccountUtils {
* @param context operating context
* @param account account to create
* @param userData user data to set
* @param password password to set
*
* @return whether the account has been created
*
* @throws IllegalArgumentException when user data contains non-String values
* @throws IllegalStateException if user data can't be set
*/
fun createAccount(context: Context, account: Account, userData: Bundle, password: String? = null): Boolean {
fun createAccount(context: Context, account: Account, userData: Bundle, password: CharArray? = null): Boolean {
// validate user data
for (key in userData.keySet()) {
userData.get(key)?.let { entry ->
@@ -35,7 +36,7 @@ object SystemAccountUtils {
// create account
val manager = AccountManager.get(context)
if (!manager.addAccountExplicitly(account, password, userData))
if (!manager.addAccountExplicitly(account, password?.concatToString(), userData))
return false
// Android seems to lose the initial user data sometimes, so make sure that the values are set

View File

@@ -549,7 +549,7 @@ fun AuthenticationSettings(
initialValue = null, // Do not show the existing password
passwordField = true,
onValueEntered = { newValue ->
onUpdateCredentials(credentials.copy(password = newValue))
onUpdateCredentials(credentials.copy(password = newValue.toCharArray()))
},
onDismiss = { showPasswordDialog = false }
)
@@ -740,7 +740,7 @@ fun AccountSettingsScreen_Preview() {
onUpdateIgnoreVpns = {},
// Authentication Settings
credentials = Credentials(username = "test", password = "test"),
credentials = Credentials(username = "test", password = "test".toCharArray()),
onUpdateCredentials = {},
isCredentialsUpdateAllowed = true,

View File

@@ -46,7 +46,7 @@ class AdvancedLoginModel @AssistedInject constructor(
baseUri = uri,
credentials = Credentials(
username = username.trimToNull(),
password = password.trimToNull(),
password = password.trimToNull()?.toCharArray(),
certificateAlias = certAlias.trimToNull()
)
)
@@ -60,7 +60,7 @@ class AdvancedLoginModel @AssistedInject constructor(
uiState = uiState.copy(
url = initialLoginInfo.baseUri?.toString()?.removePrefix("https://") ?: "",
username = initialLoginInfo.credentials?.username ?: "",
password = initialLoginInfo.credentials?.password ?: "",
password = initialLoginInfo.credentials?.password?.concatToString() ?: "",
certAlias = initialLoginInfo.credentials?.certificateAlias ?: ""
)
}

View File

@@ -38,7 +38,7 @@ class EmailLoginModel @AssistedInject constructor(
baseUri = uri,
credentials = Credentials(
username = email,
password = password
password = password.toCharArray()
)
)
}
@@ -50,7 +50,7 @@ class EmailLoginModel @AssistedInject constructor(
init {
uiState = uiState.copy(
email = initialLoginInfo.credentials?.username ?: "",
password = initialLoginInfo.credentials?.password ?: ""
password = initialLoginInfo.credentials?.password?.concatToString() ?: ""
)
}

View File

@@ -139,7 +139,7 @@ class LoginActivity @Inject constructor(): AppCompatActivity() {
},
credentials = Credentials(
username = givenUsername,
password = givenPassword
password = givenPassword?.toCharArray()
)
)
}

View File

@@ -46,7 +46,7 @@ class UrlLoginModel @AssistedInject constructor(
baseUri = uri,
credentials = Credentials(
username = username.trimToNull(),
password = password.trimToNull()
password = password.trimToNull()?.toCharArray()
)
)
@@ -59,7 +59,7 @@ class UrlLoginModel @AssistedInject constructor(
uiState = UiState(
url = initialLoginInfo.baseUri?.toString()?.removePrefix("https://") ?: "",
username = initialLoginInfo.credentials?.username ?: "",
password = initialLoginInfo.credentials?.password ?: ""
password = initialLoginInfo.credentials?.password?.concatToString() ?: ""
)
}

View File

@@ -13,6 +13,7 @@ import androidx.lifecycle.viewModelScope
import at.bitfire.davdroid.R
import at.bitfire.davdroid.db.AppDatabase
import at.bitfire.davdroid.db.Credentials
import at.bitfire.davdroid.util.trimToNull
import at.bitfire.davdroid.webdav.WebDavMountRepository
import dagger.hilt.android.lifecycle.HiltViewModel
import dagger.hilt.android.qualifiers.ApplicationContext
@@ -82,8 +83,8 @@ class AddWebdavMountModel @Inject constructor(
val displayName = uiState.displayName
val credentials = Credentials(
username = uiState.username,
password = uiState.password,
username = uiState.username.trimToNull(),
password = uiState.password.trimToNull()?.toCharArray(),
certificateAlias = uiState.certificateAlias
)

View File

@@ -47,7 +47,7 @@ class CredentialsStore @Inject constructor(
return Credentials(
prefs.getString(keyName(mountId, USER_NAME), null),
prefs.getString(keyName(mountId, PASSWORD), null),
prefs.getString(keyName(mountId, PASSWORD), null)?.toCharArray(),
prefs.getString(keyName(mountId, CERTIFICATE_ALIAS), null)
)
}
@@ -57,7 +57,7 @@ class CredentialsStore @Inject constructor(
if (credentials != null)
putBoolean(keyName(mountId, HAS_CREDENTIALS), true)
.putString(keyName(mountId, USER_NAME), credentials.username)
.putString(keyName(mountId, PASSWORD), credentials.password)
.putString(keyName(mountId, PASSWORD), credentials.password?.concatToString())
.putString(keyName(mountId, CERTIFICATE_ALIAS), credentials.certificateAlias)
else
remove(keyName(mountId, HAS_CREDENTIALS))

View File

@@ -19,7 +19,7 @@ androidx-test-rules = "1.6.1"
androidx-test-junit = "1.2.1"
androidx-work = "2.10.1"
bitfire-cert4android = "b67ba86d31"
bitfire-dav4jvm = "ec6264d427"
bitfire-dav4jvm = "05fb8ecda6"
bitfire-ical4android = "240f756bab"
bitfire-vcard4android = "59eb998f29"
compose-accompanist = "0.37.3"