Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions auth/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
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)
Comment on lines +7 to +8
Copy link

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a mismatch between normalizeUrl and validInputUrl. The normalizeUrl function allows any scheme matching the pattern [a-zA-Z][a-zA-Z0-9+.-]*://, which includes schemes like ftp:// and custom+scheme://. However, validInputUrl only accepts http://, https://, and ftp://. This means if a user enters a URL like custom://example.com, normalizeUrl will preserve it, but validInputUrl will reject it. While this may be intentional, it creates an inconsistency where normalizeUrl accepts more schemes than validInputUrl validates. Consider either restricting normalizeUrl to only preserve http/https/ftp schemes, or documenting this intentional behavior.

Suggested change
// 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)
// If it already has a supported scheme like http://, https://, or ftp://, leave it as-is
val hasScheme = Regex("^(https?|ftp)://").containsMatchIn(trimmed)

Copilot uses AI. Check for mistakes.
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(
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
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -93,11 +95,13 @@ 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 {
// 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,
Expand All @@ -107,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,
Expand Down Expand Up @@ -225,11 +232,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 -> {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
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 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"))
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 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"))
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 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"))
}
}