feat: enhance password validation and add auth backoff mechanism in sync process
This commit is contained in:
@@ -27,13 +27,13 @@ object ApiClient {
|
|||||||
val request = chain.request()
|
val request = chain.request()
|
||||||
val password =
|
val password =
|
||||||
context?.let {
|
context?.let {
|
||||||
PreferencesManager.getInstance(it).getServerPassword()
|
PreferencesManager.getInstance(it).getServerPassword()?.trim()
|
||||||
}
|
}
|
||||||
|
|
||||||
val newRequest =
|
val newRequest =
|
||||||
if (!password.isNullOrBlank()) {
|
if (!password.isNullOrBlank() && password != "\$password") {
|
||||||
request.newBuilder()
|
request.newBuilder()
|
||||||
.header("Authorization", "Bearer ${'$'}password")
|
.header("Authorization", "Bearer $password")
|
||||||
.build()
|
.build()
|
||||||
} else {
|
} else {
|
||||||
request
|
request
|
||||||
|
|||||||
+26
-6
@@ -5,6 +5,7 @@ import androidx.room.withTransaction
|
|||||||
import com.collabtable.app.data.api.ApiClient
|
import com.collabtable.app.data.api.ApiClient
|
||||||
import com.collabtable.app.data.api.SyncRequest
|
import com.collabtable.app.data.api.SyncRequest
|
||||||
import com.collabtable.app.data.database.CollabTableDatabase
|
import com.collabtable.app.data.database.CollabTableDatabase
|
||||||
|
import com.collabtable.app.data.preferences.PreferencesManager
|
||||||
import com.collabtable.app.utils.Logger
|
import com.collabtable.app.utils.Logger
|
||||||
import kotlinx.coroutines.Dispatchers
|
import kotlinx.coroutines.Dispatchers
|
||||||
import kotlinx.coroutines.sync.Mutex
|
import kotlinx.coroutines.sync.Mutex
|
||||||
@@ -17,6 +18,13 @@ class SyncRepository(context: Context) {
|
|||||||
private val api = ApiClient.api
|
private val api = ApiClient.api
|
||||||
private val prefs = appContext.getSharedPreferences("collab_table_prefs", Context.MODE_PRIVATE)
|
private val prefs = appContext.getSharedPreferences("collab_table_prefs", Context.MODE_PRIVATE)
|
||||||
|
|
||||||
|
@Volatile private var authBackoffUntil: Long = 0L
|
||||||
|
@Volatile private var authBackoffMs: Long = 0L
|
||||||
|
|
||||||
|
fun resetAuthBackoff() {
|
||||||
|
authBackoffMs = 0L
|
||||||
|
authBackoffUntil = 0L
|
||||||
|
}
|
||||||
// We keep separate watermarks to avoid clock-skew issues:
|
// We keep separate watermarks to avoid clock-skew issues:
|
||||||
// - server watermark: server-assigned timestamp for fetching server changes
|
// - server watermark: server-assigned timestamp for fetching server changes
|
||||||
// - local watermark: device local time for selecting local changes to upload
|
// - local watermark: device local time for selecting local changes to upload
|
||||||
@@ -53,6 +61,18 @@ class SyncRepository(context: Context) {
|
|||||||
// Prevent overlapping syncs across screens/viewmodels
|
// Prevent overlapping syncs across screens/viewmodels
|
||||||
syncMutex.withLock {
|
syncMutex.withLock {
|
||||||
try {
|
try {
|
||||||
|
// If not authenticated, skip making network calls (e.g., after leaving server)
|
||||||
|
val prefMgr = PreferencesManager.getInstance(appContext)
|
||||||
|
val currentPassword = prefMgr.getServerPassword()?.trim()
|
||||||
|
if (currentPassword.isNullOrBlank() || currentPassword == "\$password") {
|
||||||
|
// Quietly skip to avoid spamming server with missing Authorization
|
||||||
|
return@withContext Result.success(Unit)
|
||||||
|
}
|
||||||
|
// Respect auth backoff window to avoid spamming the server on repeated 401s
|
||||||
|
val now = System.currentTimeMillis()
|
||||||
|
if (authBackoffUntil > now) {
|
||||||
|
return@withContext Result.success(Unit)
|
||||||
|
}
|
||||||
val lastServerTs = getLastServerSyncTs()
|
val lastServerTs = getLastServerSyncTs()
|
||||||
val lastLocalTs = getLastLocalSyncTs()
|
val lastLocalTs = getLastLocalSyncTs()
|
||||||
// Capture a snapshot timestamp BEFORE reading local changes to avoid missing
|
// Capture a snapshot timestamp BEFORE reading local changes to avoid missing
|
||||||
@@ -100,12 +120,13 @@ class SyncRepository(context: Context) {
|
|||||||
if (!response.isSuccessful) {
|
if (!response.isSuccessful) {
|
||||||
if (response.code() == 401) {
|
if (response.code() == 401) {
|
||||||
// Unauthorized: likely bad/missing password. Reset sync baseline and surface a clear error.
|
// Unauthorized: likely bad/missing password. Reset sync baseline and surface a clear error.
|
||||||
Logger.e("Sync", "[ERROR] Unauthorized (401). Check server password in Settings.")
|
|
||||||
setLastServerSyncTs(0)
|
setLastServerSyncTs(0)
|
||||||
setLastLocalSyncTs(0)
|
setLastLocalSyncTs(0)
|
||||||
|
// Apply exponential backoff on repeated auth failures
|
||||||
|
authBackoffMs = if (authBackoffMs == 0L) 2_000L else (authBackoffMs * 2).coerceAtMost(300_000L)
|
||||||
|
authBackoffUntil = System.currentTimeMillis() + authBackoffMs
|
||||||
return@withContext Result.failure(Exception("Unauthorized (401). Please verify server password."))
|
return@withContext Result.failure(Exception("Unauthorized (401). Please verify server password."))
|
||||||
}
|
}
|
||||||
Logger.e("Sync", "[ERROR] Sync failed: HTTP ${response.code()}")
|
|
||||||
return@withContext Result.failure(Exception("Sync failed: ${response.code()}"))
|
return@withContext Result.failure(Exception("Sync failed: ${response.code()}"))
|
||||||
}
|
}
|
||||||
val syncResponse = response.body()!!
|
val syncResponse = response.body()!!
|
||||||
@@ -195,17 +216,16 @@ class SyncRepository(context: Context) {
|
|||||||
Logger.i("Sync", "[SYNC] Initial sync completed")
|
Logger.i("Sync", "[SYNC] Initial sync completed")
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Successful sync clears any previous auth backoff
|
||||||
|
resetAuthBackoff()
|
||||||
|
|
||||||
return@withContext Result.success(Unit)
|
return@withContext Result.success(Unit)
|
||||||
} catch (e: Exception) {
|
} catch (e: Exception) {
|
||||||
if (e is java.net.UnknownHostException) {
|
|
||||||
Logger.e("Sync", "[ERROR] Unknown host. If you're running the server on the host machine, use 10.0.2.2 on the Android emulator or a LAN IP on a physical device.")
|
|
||||||
}
|
|
||||||
// If WS indicated unauthorized, align behavior with HTTP path
|
// If WS indicated unauthorized, align behavior with HTTP path
|
||||||
if (e.message?.contains("Unauthorized") == true) {
|
if (e.message?.contains("Unauthorized") == true) {
|
||||||
setLastServerSyncTs(0)
|
setLastServerSyncTs(0)
|
||||||
setLastLocalSyncTs(0)
|
setLastLocalSyncTs(0)
|
||||||
}
|
}
|
||||||
Logger.e("Sync", "[ERROR] Sync error: ${e.message}")
|
|
||||||
return@withContext Result.failure(e)
|
return@withContext Result.failure(e)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
+7
-2
@@ -85,13 +85,18 @@ class ServerSetupViewModel(
|
|||||||
// Ensure it ends with /
|
// Ensure it ends with /
|
||||||
normalizedUrl = if (normalizedUrl.endsWith("/")) normalizedUrl else "$normalizedUrl/"
|
normalizedUrl = if (normalizedUrl.endsWith("/")) normalizedUrl else "$normalizedUrl/"
|
||||||
|
|
||||||
// Validate password is not empty (trim leading/trailing spaces)
|
// Validate password is not empty (trim leading/trailing spaces) and not a placeholder
|
||||||
val trimmedPassword = password.trim()
|
val trimmedPassword = password.trim()
|
||||||
if (trimmedPassword.isBlank()) {
|
if (trimmedPassword.isBlank()) {
|
||||||
_validationError.value = "Password cannot be empty"
|
_validationError.value = "Password cannot be empty"
|
||||||
_isValidating.value = false
|
_isValidating.value = false
|
||||||
return@launch
|
return@launch
|
||||||
}
|
}
|
||||||
|
if (trimmedPassword == "\$password") {
|
||||||
|
_validationError.value = "Enter the actual server password, not the placeholder $password"
|
||||||
|
_isValidating.value = false
|
||||||
|
return@launch
|
||||||
|
}
|
||||||
|
|
||||||
// On physical devices, block local-only hosts that won't resolve
|
// On physical devices, block local-only hosts that won't resolve
|
||||||
val isEmulator = run {
|
val isEmulator = run {
|
||||||
@@ -179,7 +184,7 @@ class ServerSetupViewModel(
|
|||||||
val authRequest =
|
val authRequest =
|
||||||
Request.Builder()
|
Request.Builder()
|
||||||
.url(testUrl)
|
.url(testUrl)
|
||||||
.header("Authorization", "Bearer ${'$'}trimmedPassword")
|
.header("Authorization", "Bearer $trimmedPassword")
|
||||||
.get()
|
.get()
|
||||||
.build()
|
.build()
|
||||||
|
|
||||||
|
|||||||
+8
-1
@@ -46,6 +46,8 @@ import com.collabtable.app.ui.components.ConnectionStatusAction
|
|||||||
import androidx.compose.ui.Alignment
|
import androidx.compose.ui.Alignment
|
||||||
import androidx.compose.ui.Modifier
|
import androidx.compose.ui.Modifier
|
||||||
import androidx.compose.ui.draw.clip
|
import androidx.compose.ui.draw.clip
|
||||||
|
import androidx.compose.foundation.rememberScrollState
|
||||||
|
import androidx.compose.foundation.verticalScroll
|
||||||
import androidx.compose.ui.platform.LocalContext
|
import androidx.compose.ui.platform.LocalContext
|
||||||
import androidx.compose.ui.res.stringResource
|
import androidx.compose.ui.res.stringResource
|
||||||
import androidx.compose.ui.unit.dp
|
import androidx.compose.ui.unit.dp
|
||||||
@@ -115,7 +117,8 @@ fun SettingsScreen(
|
|||||||
Modifier
|
Modifier
|
||||||
.fillMaxSize()
|
.fillMaxSize()
|
||||||
.padding(padding)
|
.padding(padding)
|
||||||
.padding(16.dp),
|
.padding(16.dp)
|
||||||
|
.verticalScroll(rememberScrollState()),
|
||||||
verticalArrangement = Arrangement.spacedBy(16.dp),
|
verticalArrangement = Arrangement.spacedBy(16.dp),
|
||||||
) {
|
) {
|
||||||
Text(
|
Text(
|
||||||
@@ -188,11 +191,15 @@ fun SettingsScreen(
|
|||||||
val trimmed = passwordInput.trim()
|
val trimmed = passwordInput.trim()
|
||||||
if (trimmed.isEmpty()) {
|
if (trimmed.isEmpty()) {
|
||||||
passwordError = "Password cannot be empty"
|
passwordError = "Password cannot be empty"
|
||||||
|
} else if (trimmed == "\$password") {
|
||||||
|
passwordError = "Enter the actual server password, not the placeholder \$password"
|
||||||
} else {
|
} else {
|
||||||
preferencesManager.setServerPassword(trimmed)
|
preferencesManager.setServerPassword(trimmed)
|
||||||
// Trigger a quick sync in background to validate
|
// Trigger a quick sync in background to validate
|
||||||
coroutineScope.launch(Dispatchers.IO) {
|
coroutineScope.launch(Dispatchers.IO) {
|
||||||
try {
|
try {
|
||||||
|
// Clear any auth backoff immediately when updating password
|
||||||
|
syncRepository.resetAuthBackoff()
|
||||||
syncRepository.performSync()
|
syncRepository.performSync()
|
||||||
} catch (_: Exception) { }
|
} catch (_: Exception) { }
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -33,29 +33,33 @@ object Logger {
|
|||||||
val logs: StateFlow<List<LogEntry>> = _logs.asStateFlow()
|
val logs: StateFlow<List<LogEntry>> = _logs.asStateFlow()
|
||||||
|
|
||||||
private const val MAX_LOGS = 500
|
private const val MAX_LOGS = 500
|
||||||
|
private const val DEDUPE_WINDOW_MS = 1_000L
|
||||||
|
|
||||||
fun d(
|
fun d(
|
||||||
tag: String,
|
tag: String,
|
||||||
message: String,
|
message: String,
|
||||||
) {
|
) {
|
||||||
log(LogLevel.DEBUG, tag, message)
|
if (log(LogLevel.DEBUG, tag, message)) {
|
||||||
Log.d(tag, message)
|
Log.d(tag, message)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
fun i(
|
fun i(
|
||||||
tag: String,
|
tag: String,
|
||||||
message: String,
|
message: String,
|
||||||
) {
|
) {
|
||||||
log(LogLevel.INFO, tag, message)
|
if (log(LogLevel.INFO, tag, message)) {
|
||||||
Log.i(tag, message)
|
Log.i(tag, message)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
fun w(
|
fun w(
|
||||||
tag: String,
|
tag: String,
|
||||||
message: String,
|
message: String,
|
||||||
) {
|
) {
|
||||||
log(LogLevel.WARN, tag, message)
|
if (log(LogLevel.WARN, tag, message)) {
|
||||||
Log.w(tag, message)
|
Log.w(tag, message)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
fun e(
|
fun e(
|
||||||
@@ -64,11 +68,12 @@ object Logger {
|
|||||||
throwable: Throwable? = null,
|
throwable: Throwable? = null,
|
||||||
) {
|
) {
|
||||||
val msg = if (throwable != null) "$message: ${throwable.message}" else message
|
val msg = if (throwable != null) "$message: ${throwable.message}" else message
|
||||||
log(LogLevel.ERROR, tag, msg)
|
if (log(LogLevel.ERROR, tag, msg)) {
|
||||||
if (throwable != null) {
|
if (throwable != null) {
|
||||||
Log.e(tag, message, throwable)
|
Log.e(tag, message, throwable)
|
||||||
} else {
|
} else {
|
||||||
Log.e(tag, message)
|
Log.e(tag, message)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -76,16 +81,16 @@ object Logger {
|
|||||||
level: LogLevel,
|
level: LogLevel,
|
||||||
tag: String,
|
tag: String,
|
||||||
message: String,
|
message: String,
|
||||||
) {
|
): Boolean {
|
||||||
val entry =
|
val now = System.currentTimeMillis()
|
||||||
LogEntry(
|
val last = _logs.value.lastOrNull()
|
||||||
timestamp = System.currentTimeMillis(),
|
// Suppress exact duplicate consecutive logs within a short window
|
||||||
level = level,
|
if (last != null && last.level == level && last.tag == tag && last.message == message && (now - last.timestamp) <= DEDUPE_WINDOW_MS) {
|
||||||
tag = tag,
|
return false
|
||||||
message = message,
|
}
|
||||||
)
|
val entry = LogEntry(timestamp = now, level = level, tag = tag, message = message)
|
||||||
|
|
||||||
_logs.value = (_logs.value + entry).takeLast(MAX_LOGS)
|
_logs.value = (_logs.value + entry).takeLast(MAX_LOGS)
|
||||||
|
return true
|
||||||
}
|
}
|
||||||
|
|
||||||
fun clear() {
|
fun clear() {
|
||||||
|
|||||||
Reference in New Issue
Block a user