Napraw obsługę kodów OTP podczas logowania

app/src/main/java/pl/firmatpp/kierowca/data/ApiErrorMapper.kt

@@ -50,7 +50,10 @@ object ApiErrorMapper {
         map(throwable).kind != ApiErrorKind.Network
 
     fun mapHttpStatus(statusCode: Int, body: String?): ApiError {
-        val code = body?.let { """"code"\s*:\s*"([^"]+)"""".toRegex().find(it)?.groupValues?.getOrNull(1) }
+        val code = body?.let {
+            """"code"\s*:\s*"([^"]+)"""".toRegex().find(it)?.groupValues?.getOrNull(1)
+                ?: """"reason"\s*:\s*"([^"]+)"""".toRegex().find(it)?.groupValues?.getOrNull(1)
+        }
         val message = body?.let { """"message"\s*:\s*"([^"]+)"""".toRegex().find(it)?.groupValues?.getOrNull(1) }
         val retryable = body?.let { """"retryable"\s*:\s*(true|false)""".toRegex().find(it)?.groupValues?.getOrNull(1)?.toBooleanStrictOrNull() }
 
@@ -81,16 +84,21 @@ object ApiErrorMapper {
         }
         val defaultRetryable = statusCode == 429 || statusCode in 500..599
         val resolvedRetryable = retryable ?: defaultRetryable
-        val resolvedMessage = message?.takeIf { it.isNotBlank() } ?: when (kind) {
+        val resolvedMessage = when {
+            code == "INVALID_OTP" -> "Kod jest nieprawidłowy lub wygasł. Użyj kodu z najnowszego SMS-a."
+            kind == ApiErrorKind.RateLimited -> "Za dużo prób. Odczekaj chwilę i spróbuj ponownie."
+            !message.isNullOrBlank() -> message
+            else -> when (kind) {
                 ApiErrorKind.Auth -> "Sesja wygasła. Zaloguj się ponownie."
                 ApiErrorKind.Forbidden -> "Brak uprawnień do tej operacji."
                 ApiErrorKind.Validation -> "Serwer odrzucił operację. Nie została zapisana."
                 ApiErrorKind.Conflict -> "Operacja jest w konflikcie z aktualnym stanem danych."
-            ApiErrorKind.RateLimited -> "Za dużo prób. Aplikacja spróbuje ponownie później."
+                ApiErrorKind.RateLimited -> "Za dużo prób. Odczekaj chwilę i spróbuj ponownie."
                 ApiErrorKind.Server -> "Serwer nie potwierdził operacji. Aplikacja spróbuje ponownie."
                 ApiErrorKind.Network -> "Nie udało się połączyć z serwerem. Operacja nie została potwierdzona."
                 ApiErrorKind.Unknown -> "Wystąpił błąd. Operacja nie została potwierdzona."
             }
+        }
 
         return ApiError(
             kind = kind,
@@ -103,7 +111,11 @@ object ApiErrorMapper {
     }
 
     private fun mapHttpException(exception: HttpException): ApiError {
-        val body = runCatching { exception.response()?.errorBody()?.string() }.getOrNull()
+        val body = runCatching {
+            val source = exception.response()?.errorBody()?.source() ?: return@runCatching null
+            source.request(Long.MAX_VALUE)
+            source.buffer.clone().readUtf8()
+        }.getOrNull()
         return mapHttpStatus(exception.code(), body)
     }
 }

app/src/main/java/pl/firmatpp/kierowca/ui/DriverApp.kt

@@ -247,8 +247,10 @@ fun DriverApp(
         }
     }
     SmsUserConsentEffect(
-        enabled = state.screen == DriverScreen.Phone || state.screen == DriverScreen.Otp,
+        enabled = state.otpRequestPending ||
-        onCode = viewModel::updateOtpCode,
+            (state.screen == DriverScreen.Otp && !state.loading),
+        requestId = state.otpConsentRequestId,
+        onCode = viewModel::updateOtpCodeFromSms,
     )
     LaunchedEffect(initialRouteId, initialRouteTarget) {
         viewModel.openRouteFromNotification(initialRouteId, initialRouteTarget)
@@ -282,7 +284,13 @@ fun DriverApp(
                 onUseOfflineNow = viewModel::useOfflineStartupDataNow,
             )
             DriverScreen.Phone -> PhoneScreen(state, viewModel::requestOtp)
-            DriverScreen.Otp -> OtpScreen(state, viewModel::updateOtpCode, viewModel::verifyOtp, viewModel::back)
+            DriverScreen.Otp -> OtpScreen(
+                state = state,
+                onCodeChange = viewModel::updateOtpCode,
+                onSubmit = viewModel::verifyOtp,
+                onResend = { viewModel.requestOtp(state.phone) },
+                onBack = viewModel::back,
+            )
             DriverScreen.Routes -> RoutesScreen(
                 state = state,
                 onRefresh = viewModel::refreshRoutesSilently,
@@ -840,8 +848,19 @@ private fun OtpScreen(
     state: DriverUiState,
     onCodeChange: (String) -> Unit,
     onSubmit: (String) -> Unit,
+    onResend: () -> Unit,
     onBack: () -> Unit,
 ) {
+    var nowEpochMillis by remember(state.otpResendAvailableAtEpochMillis) { mutableStateOf(System.currentTimeMillis()) }
+    val resendDelaySeconds = otpResendDelaySeconds(state.otpResendAvailableAtEpochMillis, nowEpochMillis)
+
+    LaunchedEffect(state.otpResendAvailableAtEpochMillis) {
+        while (nowEpochMillis < state.otpResendAvailableAtEpochMillis) {
+            delay(1_000L)
+            nowEpochMillis = System.currentTimeMillis()
+        }
+    }
+
     AuthShell(title = "Kod SMS", subtitle = "Wpisz kod wyslany na ${state.maskedPhone}.", onBack = onBack) {
         OutlinedTextField(
             value = state.otpCode,
@@ -853,7 +872,20 @@ private fun OtpScreen(
         )
         ErrorText(state.error)
         Spacer(Modifier.height(if (state.error.isNullOrBlank()) 18.dp else 8.dp))
-        PrimaryButton("Zaloguj") { onSubmit(state.otpCode) }
+        PrimaryButton(
+            label = "Zaloguj",
+            enabled = isRetrievedOtpCode(state.otpCode) && !state.loading,
+        ) { onSubmit(state.otpCode) }
+        TextButton(
+            onClick = onResend,
+            enabled = resendDelaySeconds == 0 && !state.loading,
+            modifier = Modifier.align(Alignment.CenterHorizontally),
+        ) {
+            Text(
+                if (resendDelaySeconds > 0) "Wyślij kod ponownie za $resendDelaySeconds s"
+                else "Wyślij kod ponownie",
+            )
+        }
     }
 }
 
@@ -4390,9 +4422,10 @@ private fun PhotoPreviewErrorOverlay() {
 }
 
 @Composable
-private fun PrimaryButton(label: String, onClick: () -> Unit) {
+private fun PrimaryButton(label: String, enabled: Boolean = true, onClick: () -> Unit) {
     Button(
         onClick = onClick,
+        enabled = enabled,
         modifier = Modifier.fillMaxWidth().height(56.dp),
         colors = ButtonDefaults.buttonColors(containerColor = TppTheme.colors.forest),
         shape = MaterialTheme.shapes.small,
@@ -4544,28 +4577,48 @@ private fun bestLastKnownLocation(context: Context): Location? {
 }
 
 @Composable
-private fun SmsUserConsentEffect(enabled: Boolean, onCode: (String) -> Unit) {
+private fun SmsUserConsentEffect(enabled: Boolean, requestId: Long, onCode: (Long, String) -> Unit) {
     val context = LocalContext.current
+    var listenGeneration by remember(requestId) { mutableStateOf(0) }
+    var launchedRequestId by remember { mutableStateOf<Long?>(null) }
     val launcher = rememberLauncherForActivityResult(ActivityResultContracts.StartActivityForResult()) { result ->
+        val completedRequestId = launchedRequestId
+        var codeDelivered = false
         if (result.resultCode == Activity.RESULT_OK) {
             val message = result.data?.getStringExtra(SmsRetriever.EXTRA_SMS_MESSAGE).orEmpty()
-            OtpCodeExtractor.extract(message)?.let(onCode)
+            if (completedRequestId != null) {
+                OtpCodeExtractor.extract(message)?.let { code ->
+                    codeDelivered = true
+                    onCode(completedRequestId, code)
+                }
+            }
+        }
+        if (!codeDelivered && enabled && completedRequestId == requestId) {
+            listenGeneration += 1
         }
     }
 
-    DisposableEffect(enabled, context) {
+    LaunchedEffect(enabled, requestId, listenGeneration) {
-        if (!enabled) return@DisposableEffect onDispose { }
+        if (enabled && requestId > 0L) {
-
             SmsRetriever.getClient(context).startSmsUserConsent(null)
+        }
+    }
+
+    DisposableEffect(enabled, context, requestId) {
+        if (!enabled) return@DisposableEffect onDispose { }
 
         val receiver = object : BroadcastReceiver() {
             override fun onReceive(receiverContext: Context?, intent: Intent?) {
                 if (intent?.action != SmsRetriever.SMS_RETRIEVED_ACTION) return
 
                 val status = intent.smsRetrieverStatus() ?: return
-                if (status.statusCode != CommonStatusCodes.SUCCESS) return
+                when (status.statusCode) {
-
+                    CommonStatusCodes.SUCCESS -> intent.smsConsentIntent()?.let { consentIntent ->
-                intent.smsConsentIntent()?.let(launcher::launch)
+                        launchedRequestId = requestId
+                        launcher.launch(consentIntent)
+                    }
+                    CommonStatusCodes.TIMEOUT -> listenGeneration += 1
+                }
             }
         }
 

app/src/main/java/pl/firmatpp/kierowca/ui/DriverViewModel.kt

@@ -68,6 +68,9 @@ data class DriverUiState(
     val startupOfflineAvailable: Boolean = false,
     val phone: String = "",
     val otpCode: String = "",
+    val otpConsentRequestId: Long = 0L,
+    val otpRequestPending: Boolean = false,
+    val otpResendAvailableAtEpochMillis: Long = 0L,
     val maskedPhone: String = "",
     val driver: DriverDto? = null,
     val routes: List<DriverRouteDto> = emptyList(),
@@ -203,6 +206,7 @@ class DriverViewModel(application: Application) : AndroidViewModel(application)
     private val diagnosticSnapshotProvider = DiagnosticSnapshotProvider(application)
     private val _state = MutableStateFlow(DriverUiState(loading = true))
     private val otpAutoSubmitPolicy = OtpAutoSubmitPolicy()
+    private val otpAttemptCoordinator = OtpAttemptCoordinator()
     private val liveSyncClient = DriverLiveSyncClient(
         repository = repository,
         onConnected = { viewModelScope.launch { checkRemoteSyncState() } },
@@ -312,19 +316,64 @@ class DriverViewModel(application: Application) : AndroidViewModel(application)
         }
     }
 
-    fun requestOtp(phone: String) = runLoading("request_otp") {
+    fun requestOtp(phone: String) {
-        if (!_state.value.isOnline) throw IOException("Logowanie wymaga połączenia z internetem.")
+        val requestSnapshot = _state.value
-        val response = repository.requestOtp(phone)
+        val now = System.currentTimeMillis()
+        val resendDelay = if (requestSnapshot.phone == phone) {
+            otpResendDelaySeconds(requestSnapshot.otpResendAvailableAtEpochMillis, now)
+        } else {
+            0
+        }
+        if (resendDelay > 0) {
+            _state.update { it.copy(error = "Nowy SMS możesz wysłać za $resendDelay s.") }
+            return
+        }
+
+        val isResend = requestSnapshot.screen == DriverScreen.Otp && requestSnapshot.phone == phone
+        val retainedCode = requestSnapshot.otpCode.takeIf { isResend }.orEmpty()
+        val attemptId = otpAttemptCoordinator.beginAttempt()
         otpAutoSubmitPolicy.reset()
+
+        viewModelScope.launch {
+            _state.update {
+                it.copy(
+                    loading = true,
+                    phone = phone,
+                    otpCode = retainedCode,
+                    otpConsentRequestId = attemptId,
+                    otpRequestPending = true,
+                    error = null,
+                )
+            }
+            kotlinx.coroutines.yield()
+
+            val result = runCatching {
+                if (!_state.value.isOnline) throw IOException("Logowanie wymaga połączenia z internetem.")
+                repository.requestOtp(phone)
+            }
+
+            if (!otpAttemptCoordinator.isCurrent(attemptId)) return@launch
+
+            result.onSuccess { response ->
+                val bufferedCode = otpAttemptCoordinator.takeBufferedCode(attemptId)
                 _state.update {
                     it.copy(
                         screen = DriverScreen.Otp,
-                phone = phone,
+                        loading = false,
-                otpCode = "",
+                        otpCode = bufferedCode ?: retainedCode,
                         maskedPhone = response.phoneMasked.orEmpty(),
+                        otpRequestPending = false,
+                        otpResendAvailableAtEpochMillis = System.currentTimeMillis() + OTP_RESEND_COOLDOWN_MILLIS,
                         error = null,
                     )
                 }
+                bufferedCode?.let { applyOtpCode(it, retrievedFromSms = true) }
+            }.onFailure { throwable ->
+                otpAttemptCoordinator.discard(attemptId)
+                reportHandledException("request_otp", throwable)
+                _state.update { it.withApiError(throwable).copy(loading = false, otpRequestPending = false) }
+            }
+        }
     }
 
     fun verifyOtp(code: String) = runLoading("verify_otp") {
@@ -336,11 +385,35 @@ class DriverViewModel(application: Application) : AndroidViewModel(application)
     }
 
     fun updateOtpCode(code: String) {
-        val sanitized = code.filter(Char::isDigit).take(6)
+        applyOtpCode(sanitizeOtpCode(code), retrievedFromSms = false)
-        _state.update { it.copy(otpCode = sanitized) }
+    }
 
-        if (otpAutoSubmitPolicy.shouldSubmit(sanitized, _state.value.loading)) {
+    fun updateOtpCodeFromSms(attemptId: Long, code: String) {
-            verifyOtp(sanitized)
+        val sanitized = sanitizeOtpCode(code)
+        if (!isRetrievedOtpCode(sanitized)) return
+
+        val snapshot = _state.value
+        if (attemptId != snapshot.otpConsentRequestId || !otpAttemptCoordinator.isCurrent(attemptId)) return
+
+        if (snapshot.otpRequestPending) {
+            otpAttemptCoordinator.bufferRetrievedCode(attemptId, sanitized)
+            return
+        }
+        if (snapshot.screen != DriverScreen.Otp) return
+
+        applyOtpCode(sanitized, retrievedFromSms = true)
+    }
+
+    private fun applyOtpCode(code: String, retrievedFromSms: Boolean) {
+        _state.update { it.copy(otpCode = code) }
+
+        val shouldSubmit = if (retrievedFromSms) {
+            otpAutoSubmitPolicy.shouldSubmitRetrieved(code, _state.value.loading)
+        } else {
+            otpAutoSubmitPolicy.shouldSubmit(code, _state.value.loading)
+        }
+        if (shouldSubmit) {
+            verifyOtp(code)
         }
     }
 

app/src/main/java/pl/firmatpp/kierowca/ui/OtpAttemptCoordinator.kt

@@ -0,0 +1,45 @@
+package pl.firmatpp.kierowca.ui
+
+class OtpAttemptCoordinator {
+    private var currentAttemptId: Long = 0L
+    private var bufferedCode: String? = null
+
+    fun beginAttempt(): Long {
+        currentAttemptId += 1L
+        bufferedCode = null
+        return currentAttemptId
+    }
+
+    fun isCurrent(attemptId: Long): Boolean = attemptId == currentAttemptId
+
+    fun bufferRetrievedCode(attemptId: Long, code: String): Boolean {
+        if (!isCurrent(attemptId) || !isRetrievedOtpCode(code)) return false
+        bufferedCode = code
+        return true
+    }
+
+    fun takeBufferedCode(attemptId: Long): String? {
+        if (!isCurrent(attemptId)) return null
+        return bufferedCode.also { bufferedCode = null }
+    }
+
+    fun discard(attemptId: Long) {
+        if (isCurrent(attemptId)) bufferedCode = null
+    }
+}
+
+internal fun sanitizeOtpCode(code: String): String =
+    code.filter(Char::isDigit).take(OTP_MAX_LENGTH)
+
+internal fun isRetrievedOtpCode(code: String): Boolean =
+    code.length in OTP_MIN_LENGTH..OTP_MAX_LENGTH && code.all(Char::isDigit)
+
+internal fun otpResendDelaySeconds(availableAtEpochMillis: Long, nowEpochMillis: Long): Int {
+    val remainingMillis = (availableAtEpochMillis - nowEpochMillis).coerceAtLeast(0L)
+    return ((remainingMillis + 999L) / 1_000L).toInt()
+}
+
+internal const val OTP_MIN_LENGTH = 4
+internal const val OTP_DEFAULT_LENGTH = 6
+internal const val OTP_MAX_LENGTH = 10
+internal const val OTP_RESEND_COOLDOWN_MILLIS = 30_000L

app/src/main/java/pl/firmatpp/kierowca/ui/OtpAutoSubmitPolicy.kt

@@ -4,14 +4,20 @@ class OtpAutoSubmitPolicy {
     private var lastSubmittedCode: String? = null
 
     fun shouldSubmit(code: String, loading: Boolean): Boolean {
-        if (code.length < OTP_LENGTH) {
+        return shouldSubmitCompleteCode(code, loading, code.length == OTP_DEFAULT_LENGTH)
+    }
+
+    fun shouldSubmitRetrieved(code: String, loading: Boolean): Boolean {
+        return shouldSubmitCompleteCode(code, loading, isRetrievedOtpCode(code))
+    }
+
+    private fun shouldSubmitCompleteCode(code: String, loading: Boolean, complete: Boolean): Boolean {
+        if (!complete) {
             lastSubmittedCode = null
             return false
         }
 
-        if (loading || code.length != OTP_LENGTH || code == lastSubmittedCode) {
+        if (loading || code == lastSubmittedCode) return false
-            return false
-        }
 
         lastSubmittedCode = code
         return true
@@ -20,8 +26,4 @@ class OtpAutoSubmitPolicy {
     fun reset() {
         lastSubmittedCode = null
     }
-
-    private companion object {
-        const val OTP_LENGTH = 6
-    }
 }

app/src/test/java/pl/firmatpp/kierowca/data/ApiErrorMapperTest.kt

@@ -3,12 +3,16 @@ package pl.firmatpp.kierowca.data
 import java.io.IOException
 import java.net.UnknownHostException
 import kotlin.coroutines.cancellation.CancellationException
+import okhttp3.MediaType.Companion.toMediaType
+import okhttp3.ResponseBody.Companion.toResponseBody
 import org.junit.Assert.assertEquals
 import org.junit.Assert.assertFalse
 import org.junit.Assert.assertSame
 import org.junit.Assert.assertTrue
 import org.junit.Assert.fail
 import org.junit.Test
+import retrofit2.HttpException
+import retrofit2.Response
 
 class ApiErrorMapperTest {
     @Test
@@ -84,4 +88,30 @@ class ApiErrorMapperTest {
         assertEquals("Storage timeout", error.message)
         assertEquals(body, error.responseBody)
     }
+
+    @Test
+    fun mapsInvalidOtpReasonToActionableMessage() {
+        val error = ApiErrorMapper.mapHttpStatus(422, """{"ok":false,"reason":"INVALID_OTP"}""")
+
+        assertEquals("INVALID_OTP", error.code)
+        assertEquals("Kod jest nieprawidłowy lub wygasł. Użyj kodu z najnowszego SMS-a.", error.message)
+    }
+
+    @Test
+    fun translatesRateLimitResponses() {
+        val error = ApiErrorMapper.mapHttpStatus(429, """{"message":"Too Many Attempts."}""")
+
+        assertEquals(ApiErrorKind.RateLimited, error.kind)
+        assertEquals("Za dużo prób. Odczekaj chwilę i spróbuj ponownie.", error.message)
+    }
+
+    @Test
+    fun preservesApiProblemWhenSameExceptionIsMappedMoreThanOnce() {
+        val body = """{"ok":false,"reason":"INVALID_OTP"}"""
+            .toResponseBody("application/json".toMediaType())
+        val exception = HttpException(Response.error<Any>(422, body))
+
+        assertEquals("INVALID_OTP", ApiErrorMapper.map(exception).code)
+        assertEquals("INVALID_OTP", ApiErrorMapper.map(exception).code)
+    }
 }

app/src/test/java/pl/firmatpp/kierowca/ui/OtpAttemptCoordinatorTest.kt

@@ -0,0 +1,48 @@
+package pl.firmatpp.kierowca.ui
+
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertFalse
+import org.junit.Assert.assertNull
+import org.junit.Assert.assertTrue
+import org.junit.Test
+
+class OtpAttemptCoordinatorTest {
+    @Test
+    fun buffersSmsReceivedBeforeOtpRequestCompletes() {
+        val coordinator = OtpAttemptCoordinator()
+        val attemptId = coordinator.beginAttempt()
+
+        assertTrue(coordinator.bufferRetrievedCode(attemptId, "123456"))
+        assertEquals("123456", coordinator.takeBufferedCode(attemptId))
+        assertNull(coordinator.takeBufferedCode(attemptId))
+    }
+
+    @Test
+    fun ignoresConsentResultFromPreviousRequest() {
+        val coordinator = OtpAttemptCoordinator()
+        val previousAttempt = coordinator.beginAttempt()
+        val currentAttempt = coordinator.beginAttempt()
+
+        assertFalse(coordinator.bufferRetrievedCode(previousAttempt, "111111"))
+        assertTrue(coordinator.bufferRetrievedCode(currentAttempt, "222222"))
+        assertEquals("222222", coordinator.takeBufferedCode(currentAttempt))
+    }
+
+    @Test
+    fun acceptsOnlyOtpLengthsSupportedByApi() {
+        val coordinator = OtpAttemptCoordinator()
+        val attemptId = coordinator.beginAttempt()
+
+        assertFalse(coordinator.bufferRetrievedCode(attemptId, "123"))
+        assertTrue(coordinator.bufferRetrievedCode(attemptId, "1234"))
+        assertTrue(coordinator.bufferRetrievedCode(attemptId, "1234567890"))
+        assertFalse(coordinator.bufferRetrievedCode(attemptId, "12345678901"))
+    }
+
+    @Test
+    fun roundsResendCooldownUpToFullSeconds() {
+        assertEquals(30, otpResendDelaySeconds(40_000L, 10_000L))
+        assertEquals(1, otpResendDelaySeconds(10_001L, 10_000L))
+        assertEquals(0, otpResendDelaySeconds(10_000L, 10_001L))
+    }
+}

app/src/test/java/pl/firmatpp/kierowca/ui/OtpAutoSubmitPolicyTest.kt

@@ -38,4 +38,13 @@ class OtpAutoSubmitPolicyTest {
         assertFalse(policy.shouldSubmit("123456", loading = true))
         assertFalse(policy.shouldSubmit("1234567", loading = false))
     }
+
+    @Test
+    fun submitsCompleteRetrievedCodesAcceptedByApi() {
+        val policy = OtpAutoSubmitPolicy()
+
+        assertTrue(policy.shouldSubmitRetrieved("1234", loading = false))
+        assertTrue(policy.shouldSubmitRetrieved("1234567890", loading = false))
+        assertFalse(policy.shouldSubmitRetrieved("123", loading = false))
+    }
 }