From e9e09a45dc2846077d7f554cdbae5f1e1565e4c1 Mon Sep 17 00:00:00 2001 From: Joel Kanyi Date: Tue, 23 Dec 2025 18:39:40 +0300 Subject: [PATCH 1/2] `refactor:` Extract URL validation and normalization to `AuthUtils` - Introduce `AuthUtils.kt` to centralize URL handling logic. - Move URL validation (`validInputUrl`) and create a new URL normalization (`normalizeUrl`) function within `AuthUtils`. - The `normalizeUrl` function now automatically prepends "https://" to base URLs that are missing a scheme. - Update `AuthViewModel` to use these new utility functions, ensuring the normalized URL is reflected back in the UI. - Add comprehensive unit tests for `AuthUtils` to cover various URL scenarios. - Add JUnit as a test dependency in the `auth` module's `build.gradle.kts`. --- auth/build.gradle.kts | 3 + .../auth/presentation/util/AuthUtils.kt | 16 ++++ .../presentation/viewmodel/AuthViewModel.kt | 13 ++-- .../auth/presentation/util/AuthUtilsTest.kt | 76 +++++++++++++++++++ 4 files changed, 102 insertions(+), 6 deletions(-) create mode 100644 auth/src/main/java/com/android/swingmusic/auth/presentation/util/AuthUtils.kt create mode 100644 auth/src/test/java/com/android/swingmusic/auth/presentation/util/AuthUtilsTest.kt diff --git a/auth/build.gradle.kts b/auth/build.gradle.kts index 07b4cf4c..2ef1abdd 100644 --- a/auth/build.gradle.kts +++ b/auth/build.gradle.kts @@ -98,6 +98,9 @@ dependencies { implementation(libs.kotlinx.coroutines.android) implementation(libs.androidx.lifecycle.runtime.ktx) implementation(libs.androidx.lifecycle.viewmodel.compose) + + // Unit tests + testImplementation(libs.junit) } kotlin { diff --git a/auth/src/main/java/com/android/swingmusic/auth/presentation/util/AuthUtils.kt b/auth/src/main/java/com/android/swingmusic/auth/presentation/util/AuthUtils.kt new file mode 100644 index 00000000..027439a2 --- /dev/null +++ b/auth/src/main/java/com/android/swingmusic/auth/presentation/util/AuthUtils.kt @@ -0,0 +1,16 @@ +package com.android.swingmusic.auth.presentation.util + +object AuthUtils { + fun normalizeUrl(url: String?): String? { + val trimmed = url?.trim() + if (trimmed.isNullOrEmpty()) return null + // If it already has a scheme like http:// or https:// (or any scheme), leave it as-is + val hasScheme = Regex("^[a-zA-Z][a-zA-Z0-9+.-]*://").containsMatchIn(trimmed) + return if (hasScheme) trimmed else "https://$trimmed" + } + + fun validInputUrl(url: String?): Boolean { + val urlRegex = Regex("^(https?|ftp)://[^\\s/$.?#].\\S*$") + return url?.matches(urlRegex) == true + } +} \ No newline at end of file diff --git a/auth/src/main/java/com/android/swingmusic/auth/presentation/viewmodel/AuthViewModel.kt b/auth/src/main/java/com/android/swingmusic/auth/presentation/viewmodel/AuthViewModel.kt index 0bd6b668..43144fa0 100644 --- a/auth/src/main/java/com/android/swingmusic/auth/presentation/viewmodel/AuthViewModel.kt +++ b/auth/src/main/java/com/android/swingmusic/auth/presentation/viewmodel/AuthViewModel.kt @@ -14,6 +14,8 @@ import com.android.swingmusic.auth.presentation.event.AuthUiEvent.OnUsernameChan import com.android.swingmusic.auth.presentation.state.AuthState import com.android.swingmusic.auth.presentation.state.AuthUiState import com.android.swingmusic.auth.presentation.util.AuthError +import com.android.swingmusic.auth.presentation.util.AuthUtils.normalizeUrl +import com.android.swingmusic.auth.presentation.util.AuthUtils.validInputUrl import com.android.swingmusic.core.data.util.Resource import dagger.hilt.android.lifecycle.HiltViewModel import kotlinx.coroutines.channels.Channel @@ -93,11 +95,15 @@ class AuthViewModel @Inject constructor( } private fun logInWithUsernameAndPassword() { - val baseUrl = _authUiState.value.baseUrl + val inputBaseUrl = _authUiState.value.baseUrl + val baseUrl = normalizeUrl(inputBaseUrl) val username = _authUiState.value.username val password = _authUiState.value.password viewModelScope.launch { + // Persist the normalized URL back to UI state so the user sees the auto-prepended scheme + _authUiState.value = _authUiState.value.copy(baseUrl = baseUrl) + if (baseUrl.isNullOrEmpty() || !validInputUrl(baseUrl)) { _authUiState.value = _authUiState.value.copy( authState = AuthState.LOGGED_OUT, @@ -225,11 +231,6 @@ class AuthViewModel @Inject constructor( } } - private fun validInputUrl(url: String?): Boolean { - val urlRegex = Regex("^(https?|ftp)://[^\\s/$.?#].\\S*$") - return url?.matches(urlRegex) == true - } - fun onAuthUiEvent(event: AuthUiEvent) { when (event) { is LogInWithQrCode -> { diff --git a/auth/src/test/java/com/android/swingmusic/auth/presentation/util/AuthUtilsTest.kt b/auth/src/test/java/com/android/swingmusic/auth/presentation/util/AuthUtilsTest.kt new file mode 100644 index 00000000..1347f7db --- /dev/null +++ b/auth/src/test/java/com/android/swingmusic/auth/presentation/util/AuthUtilsTest.kt @@ -0,0 +1,76 @@ +package com.android.swingmusic.auth.presentation.util + +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 AuthUtilsTest { + + // normalizeUrl + @Test + fun `normalizeUrl returns null for null input`() { + assertNull(AuthUtils.normalizeUrl(null)) + } + + @Test + fun `normalizeUrl returns null for blank input`() { + assertNull(AuthUtils.normalizeUrl(" \t \n ")) + } + + @Test + fun `normalizeUrl trims whitespace`() { + assertEquals("https://example.com", AuthUtils.normalizeUrl(" example.com ")) + } + + @Test + fun `normalizeUrl prepends https when scheme is missing`() { + assertEquals("https://example.com", AuthUtils.normalizeUrl("example.com")) + assertEquals("https://sub.domain.com", AuthUtils.normalizeUrl("sub.domain.com")) + } + + @Test + fun `normalizeUrl leaves http and https unchanged`() { + assertEquals("http://example.com", AuthUtils.normalizeUrl("http://example.com")) + assertEquals("https://example.com", AuthUtils.normalizeUrl("https://example.com")) + } + + @Test + fun `normalizeUrl leaves other schemes unchanged`() { + assertEquals("ftp://example.com", AuthUtils.normalizeUrl("ftp://example.com")) + assertEquals("custom+scheme://host/path", AuthUtils.normalizeUrl("custom+scheme://host/path")) + } + + // validInputUrl + @Test + fun `validInputUrl accepts http, https, and ftp`() { + assertTrue(AuthUtils.validInputUrl("http://example.com")) + assertTrue(AuthUtils.validInputUrl("https://example.com")) + assertTrue(AuthUtils.validInputUrl("ftp://example.com")) + } + + @Test + fun `validInputUrl rejects missing scheme`() { + assertFalse(AuthUtils.validInputUrl("example.com")) + assertFalse(AuthUtils.validInputUrl("www.example.com")) + } + + @Test + fun `validInputUrl rejects blank or null`() { + assertFalse(AuthUtils.validInputUrl(null)) + assertFalse(AuthUtils.validInputUrl(" ")) + } + + @Test + fun `validInputUrl accepts typical paths and query`() { + assertTrue(AuthUtils.validInputUrl("https://example.com/path?query=1#frag")) + } + + @Test + fun `validInputUrl rejects obvious invalid urls`() { + assertFalse(AuthUtils.validInputUrl("https:///example.com")) + assertFalse(AuthUtils.validInputUrl("https://")) + assertFalse(AuthUtils.validInputUrl("not a url")) + } +} From 8e13813eb5c4167d5a04d396fb6112222cdfed04 Mon Sep 17 00:00:00 2001 From: Joel Kanyi Date: Tue, 23 Dec 2025 18:56:20 +0300 Subject: [PATCH 2/2] refactor: Enhance URL validation and normalization in AuthUtils --- .../auth/presentation/util/AuthUtils.kt | 18 ++++++++- .../presentation/viewmodel/AuthViewModel.kt | 7 ++-- .../auth/presentation/util/AuthUtilsTest.kt | 38 ++++++++++++++++++- 3 files changed, 57 insertions(+), 6 deletions(-) diff --git a/auth/src/main/java/com/android/swingmusic/auth/presentation/util/AuthUtils.kt b/auth/src/main/java/com/android/swingmusic/auth/presentation/util/AuthUtils.kt index 027439a2..55b781b7 100644 --- a/auth/src/main/java/com/android/swingmusic/auth/presentation/util/AuthUtils.kt +++ b/auth/src/main/java/com/android/swingmusic/auth/presentation/util/AuthUtils.kt @@ -9,8 +9,24 @@ object AuthUtils { return if (hasScheme) trimmed else "https://$trimmed" } + /** + * Validates URLs for login input. + * + * Rules: + * - Allowed schemes: http, https, ftp + * - Host: domain (with subdomains), localhost, or IPv4 address + * - Optional port + * - Optional path/query/fragment + */ fun validInputUrl(url: String?): Boolean { - val urlRegex = Regex("^(https?|ftp)://[^\\s/$.?#].\\S*$") + val urlRegex = Regex( + pattern = "^(https?|ftp)://(" + + "localhost|" + + "(?:[a-zA-Z0-9-]+\\.)+[a-zA-Z]{2,}|" + + "(?:(?:25[0-5]|2[0-4]\\d|1\\d{2}|[1-9]?\\d)(?:\\.(?:25[0-5]|2[0-4]\\d|1\\d{2}|[1-9]?\\d)){3})" + + ")(?::\\d{1,5})?(?:/\\S*)?$", + options = setOf(RegexOption.IGNORE_CASE) + ) return url?.matches(urlRegex) == true } } \ No newline at end of file diff --git a/auth/src/main/java/com/android/swingmusic/auth/presentation/viewmodel/AuthViewModel.kt b/auth/src/main/java/com/android/swingmusic/auth/presentation/viewmodel/AuthViewModel.kt index 43144fa0..e3d23010 100644 --- a/auth/src/main/java/com/android/swingmusic/auth/presentation/viewmodel/AuthViewModel.kt +++ b/auth/src/main/java/com/android/swingmusic/auth/presentation/viewmodel/AuthViewModel.kt @@ -101,9 +101,7 @@ class AuthViewModel @Inject constructor( val password = _authUiState.value.password viewModelScope.launch { - // Persist the normalized URL back to UI state so the user sees the auto-prepended scheme - _authUiState.value = _authUiState.value.copy(baseUrl = baseUrl) - + // Validate the normalized URL first; only persist back to UI if it's valid if (baseUrl.isNullOrEmpty() || !validInputUrl(baseUrl)) { _authUiState.value = _authUiState.value.copy( authState = AuthState.LOGGED_OUT, @@ -113,6 +111,9 @@ class AuthViewModel @Inject constructor( return@launch } + // Persist the normalized, validated URL back to UI so the user sees the auto-prepended scheme + _authUiState.value = _authUiState.value.copy(baseUrl = baseUrl) + if (username.isNullOrEmpty() || password.isNullOrEmpty()) { _authUiState.value = _authUiState.value.copy( authState = AuthState.LOGGED_OUT, diff --git a/auth/src/test/java/com/android/swingmusic/auth/presentation/util/AuthUtilsTest.kt b/auth/src/test/java/com/android/swingmusic/auth/presentation/util/AuthUtilsTest.kt index 1347f7db..65a87242 100644 --- a/auth/src/test/java/com/android/swingmusic/auth/presentation/util/AuthUtilsTest.kt +++ b/auth/src/test/java/com/android/swingmusic/auth/presentation/util/AuthUtilsTest.kt @@ -20,16 +20,29 @@ class AuthUtilsTest { } @Test - fun `normalizeUrl trims whitespace`() { + fun `normalizeUrl trims whitespace without scheme`() { assertEquals("https://example.com", AuthUtils.normalizeUrl(" example.com ")) } + @Test + fun `normalizeUrl trims whitespace with scheme`() { + assertEquals("https://example.com", AuthUtils.normalizeUrl(" https://example.com ")) + } + @Test fun `normalizeUrl prepends https when scheme is missing`() { assertEquals("https://example.com", AuthUtils.normalizeUrl("example.com")) assertEquals("https://sub.domain.com", AuthUtils.normalizeUrl("sub.domain.com")) } + @Test + fun `normalizeUrl handles ports and IPs when missing scheme`() { + assertEquals("https://example.com:8080", AuthUtils.normalizeUrl("example.com:8080")) + assertEquals("https://192.168.1.1", AuthUtils.normalizeUrl("192.168.1.1")) + assertEquals("https://192.168.1.1:8443", AuthUtils.normalizeUrl("192.168.1.1:8443")) + assertEquals("https://localhost:3000", AuthUtils.normalizeUrl("localhost:3000")) + } + @Test fun `normalizeUrl leaves http and https unchanged`() { assertEquals("http://example.com", AuthUtils.normalizeUrl("http://example.com")) @@ -50,6 +63,26 @@ class AuthUtilsTest { assertTrue(AuthUtils.validInputUrl("ftp://example.com")) } + @Test + fun `validInputUrl accepts domains with ports`() { + assertTrue(AuthUtils.validInputUrl("https://example.com:8080")) + assertTrue(AuthUtils.validInputUrl("http://sub.example.co.uk:3000/path")) + } + + @Test + fun `validInputUrl accepts localhost and IPv4 with optional ports`() { + assertTrue(AuthUtils.validInputUrl("http://localhost")) + assertTrue(AuthUtils.validInputUrl("http://localhost:3000/health")) + assertTrue(AuthUtils.validInputUrl("https://192.168.1.1")) + assertTrue(AuthUtils.validInputUrl("https://192.168.1.1:8443/api")) + } + + @Test + fun `validInputUrl rejects malformed IPv4`() { + assertFalse(AuthUtils.validInputUrl("https://999.1.1.1")) + assertFalse(AuthUtils.validInputUrl("http://256.256.256.256")) + } + @Test fun `validInputUrl rejects missing scheme`() { assertFalse(AuthUtils.validInputUrl("example.com")) @@ -68,9 +101,10 @@ class AuthUtilsTest { } @Test - fun `validInputUrl rejects obvious invalid urls`() { + fun `validInputUrl rejects obvious invalid urls and custom schemes`() { assertFalse(AuthUtils.validInputUrl("https:///example.com")) assertFalse(AuthUtils.validInputUrl("https://")) assertFalse(AuthUtils.validInputUrl("not a url")) + assertFalse(AuthUtils.validInputUrl("custom+scheme://host/path")) } }