From d2092352918262565a9956a9cb9df0012e3f982b Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Fri, 6 Feb 2026 06:57:00 +0000 Subject: [PATCH] Remove insecure SSL configuration and 'Ignore SSL Errors' preference - Removed custom TrustManager and HostnameVerifier in NetworkModule.kt that allowed bypassing SSL validation. - Removed `ignoreSslErrors` preference from PreferencesManager.kt and associated UI in LibraryDrawerContent.kt. - Cleaned up BaseJsoupSource.kt and ReaderScreen.kt to use standard secure connections. - Added LibraryViewModelTest.kt to verify ViewModel integrity. This fixes a security vulnerability where SSL validation could be disabled, exposing users to MITM attacks. Co-authored-by: Aatricks <113598245+Aatricks@users.noreply.github.com> --- .../data/local/PreferencesManager.kt | 5 -- .../data/repository/source/BaseJsoupSource.kt | 35 ----------- .../aatricks/novelscraper/di/NetworkModule.kt | 62 +----------------- .../ui/screens/LibraryDrawerContent.kt | 61 +----------------- .../novelscraper/ui/screens/ReaderScreen.kt | 15 +---- .../ui/viewmodel/LibraryViewModel.kt | 17 +---- .../ui/viewmodel/LibraryViewModelTest.kt | 63 +++++++++++++++++++ 7 files changed, 71 insertions(+), 187 deletions(-) create mode 100644 app/src/test/java/io/aatricks/novelscraper/ui/viewmodel/LibraryViewModelTest.kt diff --git a/app/src/main/java/io/aatricks/novelscraper/data/local/PreferencesManager.kt b/app/src/main/java/io/aatricks/novelscraper/data/local/PreferencesManager.kt index ea81eaf..a4d6265 100644 --- a/app/src/main/java/io/aatricks/novelscraper/data/local/PreferencesManager.kt +++ b/app/src/main/java/io/aatricks/novelscraper/data/local/PreferencesManager.kt @@ -123,10 +123,6 @@ class PreferencesManager @Inject constructor( ?: io.aatricks.novelscraper.data.model.ReaderTheme.DARK.name set(value) = prefs.edit().putString(KEY_READER_THEME, value).apply() - var ignoreSslErrors: Boolean - get() = prefs.getBoolean(KEY_IGNORE_SSL_ERRORS, false) - set(value) = prefs.edit().putBoolean(KEY_IGNORE_SSL_ERRORS, value).apply() - // Clear all preferences fun clearAll() { prefs.edit().clear().apply() @@ -158,6 +154,5 @@ class PreferencesManager @Inject constructor( private const val KEY_MARGINS = "reader_margins" private const val KEY_PARAGRAPH_SPACING = "reader_paragraph_spacing" private const val KEY_READER_THEME = "reader_theme" - private const val KEY_IGNORE_SSL_ERRORS = "ignore_ssl_errors" } } diff --git a/app/src/main/java/io/aatricks/novelscraper/data/repository/source/BaseJsoupSource.kt b/app/src/main/java/io/aatricks/novelscraper/data/repository/source/BaseJsoupSource.kt index 3da24ae..13ccbf6 100644 --- a/app/src/main/java/io/aatricks/novelscraper/data/repository/source/BaseJsoupSource.kt +++ b/app/src/main/java/io/aatricks/novelscraper/data/repository/source/BaseJsoupSource.kt @@ -8,12 +8,6 @@ import org.jsoup.Connection import org.jsoup.Jsoup import org.jsoup.nodes.Document import org.jsoup.nodes.Element -import java.security.SecureRandom -import java.security.cert.X509Certificate -import javax.net.ssl.SSLContext -import javax.net.ssl.SSLSocketFactory -import javax.net.ssl.TrustManager -import javax.net.ssl.X509TrustManager abstract class BaseJsoupSource( protected open val preferencesManager: PreferencesManager? = null, @@ -49,21 +43,6 @@ abstract class BaseJsoupSource( .timeout(timeout.toInt()) .followRedirects(true) - if (preferencesManager?.ignoreSslErrors == true) { - val trustAllCerts = arrayOf( - object : X509TrustManager { - override fun checkClientTrusted(chain: Array, authType: String) {} - override fun checkServerTrusted(chain: Array, authType: String) {} - override fun getAcceptedIssuers(): Array = arrayOf() - } - ) - val sslContext = SSLContext.getInstance("TLS") - sslContext.init(null, trustAllCerts, java.security.SecureRandom()) - connection.sslSocketFactory(sslContext.socketFactory) - // Note: setDefaultHostnameVerifier is global and might affect other parts of the app - javax.net.ssl.HttpsURLConnection.setDefaultHostnameVerifier { _, _ -> true } - } - return connection.get() } @@ -75,20 +54,6 @@ abstract class BaseJsoupSource( .timeout(timeout.toInt()) .followRedirects(true) - if (preferencesManager?.ignoreSslErrors == true) { - val trustAllCerts = arrayOf( - object : X509TrustManager { - override fun checkClientTrusted(chain: Array, authType: String) {} - override fun checkServerTrusted(chain: Array, authType: String) {} - override fun getAcceptedIssuers(): Array = arrayOf() - } - ) - val sslContext = SSLContext.getInstance("TLS") - sslContext.init(null, trustAllCerts, java.security.SecureRandom()) - connection.sslSocketFactory(sslContext.socketFactory) - javax.net.ssl.HttpsURLConnection.setDefaultHostnameVerifier { _, _ -> true } - } - return connection } diff --git a/app/src/main/java/io/aatricks/novelscraper/di/NetworkModule.kt b/app/src/main/java/io/aatricks/novelscraper/di/NetworkModule.kt index f7513b0..e1ea21a 100644 --- a/app/src/main/java/io/aatricks/novelscraper/di/NetworkModule.kt +++ b/app/src/main/java/io/aatricks/novelscraper/di/NetworkModule.kt @@ -9,15 +9,9 @@ import io.ktor.client.engine.okhttp.* import io.ktor.client.plugins.contentnegotiation.* import io.ktor.serialization.kotlinx.json.* import kotlinx.serialization.json.Json -import io.aatricks.novelscraper.data.local.PreferencesManager import okhttp3.OkHttpClient import okhttp3.logging.HttpLoggingInterceptor -import java.security.SecureRandom -import java.security.cert.X509Certificate import javax.inject.Singleton -import javax.net.ssl.SSLContext -import javax.net.ssl.TrustManager -import javax.net.ssl.X509TrustManager @Module @InstallIn(SingletonComponent::class) @@ -31,7 +25,7 @@ object NetworkModule { @Provides @Singleton - fun provideOkHttpClient(preferencesManager: PreferencesManager): OkHttpClient { + fun provideOkHttpClient(): OkHttpClient { val builder = OkHttpClient.Builder() .addInterceptor(HttpLoggingInterceptor().apply { level = HttpLoggingInterceptor.Level.HEADERS // Reduced verbosity for performance @@ -41,60 +35,6 @@ object NetworkModule { .followSslRedirects(true) .followRedirects(true) - try { - val trustManagerFactory = javax.net.ssl.TrustManagerFactory.getInstance( - javax.net.ssl.TrustManagerFactory.getDefaultAlgorithm() - ) - trustManagerFactory.init(null as java.security.KeyStore?) - val defaultTrustManager = trustManagerFactory.trustManagers.first { tm -> tm is X509TrustManager } as X509TrustManager - - val dynamicTrustManager = object : X509TrustManager { - override fun checkClientTrusted(chain: Array, authType: String) { - if (!preferencesManager.ignoreSslErrors) { - defaultTrustManager.checkClientTrusted(chain, authType) - } - } - - override fun checkServerTrusted(chain: Array, authType: String) { - if (preferencesManager.ignoreSslErrors) return - - try { - defaultTrustManager.checkServerTrusted(chain, authType) - } catch (e: Exception) { - // Double check in case of race condition or update - if (!preferencesManager.ignoreSslErrors) throw e - } - } - - override fun getAcceptedIssuers(): Array = defaultTrustManager.getAcceptedIssuers() - } - - val sslContext = SSLContext.getInstance("TLS") - sslContext.init(null, arrayOf(dynamicTrustManager), SecureRandom()) - - builder.sslSocketFactory(sslContext.socketFactory, dynamicTrustManager) - - builder.hostnameVerifier { hostname, session -> - if (preferencesManager.ignoreSslErrors) true - else okhttp3.internal.tls.OkHostnameVerifier.verify(hostname, session) - } - } catch (e: Exception) { - // Fallback to a basic trust-all if something fails during setup and user wants to ignore errors - if (preferencesManager.ignoreSslErrors) { - try { - val trustAllCerts = object : X509TrustManager { - override fun checkClientTrusted(chain: Array, authType: String) {} - override fun checkServerTrusted(chain: Array, authType: String) {} - override fun getAcceptedIssuers(): Array = arrayOf() - } - val sslContext = SSLContext.getInstance("TLS") - sslContext.init(null, arrayOf(trustAllCerts), SecureRandom()) - builder.sslSocketFactory(sslContext.socketFactory, trustAllCerts) - builder.hostnameVerifier { _, _ -> true } - } catch (inner: Exception) {} - } - } - return builder.build() } diff --git a/app/src/main/java/io/aatricks/novelscraper/ui/screens/LibraryDrawerContent.kt b/app/src/main/java/io/aatricks/novelscraper/ui/screens/LibraryDrawerContent.kt index b40d176..52ee297 100644 --- a/app/src/main/java/io/aatricks/novelscraper/ui/screens/LibraryDrawerContent.kt +++ b/app/src/main/java/io/aatricks/novelscraper/ui/screens/LibraryDrawerContent.kt @@ -49,7 +49,6 @@ fun LibraryDrawerContent( var urlInput by remember { mutableStateOf("") } var isAddSectionVisible by remember { mutableStateOf(false) } - var isSettingsVisible by remember { mutableStateOf(false) } val scope = rememberCoroutineScope() LaunchedEffect(Unit) { @@ -68,18 +67,9 @@ fun LibraryDrawerContent( onExploreClick = { onCloseDrawer() navController.navigate(ExploreRoute) - }, - onSettingsClick = { isSettingsVisible = true } + } ) - if (isSettingsVisible) { - SettingsDialog( - ignoreSslErrors = libraryUiState.ignoreSslErrors, - onIgnoreSslErrorsChange = { libraryViewModel.ignoreSslErrors = it }, - onDismiss = { isSettingsVisible = false } - ) - } - androidx.compose.animation.AnimatedVisibility(visible = isAddSectionVisible) { AddNovelSection( urlInput = urlInput, @@ -130,8 +120,7 @@ fun LibraryDrawerContent( private fun LibraryHeader( isAddVisible: Boolean, onToggleAdd: () -> Unit, - onExploreClick: () -> Unit, - onSettingsClick: () -> Unit + onExploreClick: () -> Unit ): Unit { Row( modifier = Modifier.fillMaxWidth().padding(bottom = 16.dp), @@ -145,13 +134,6 @@ private fun LibraryHeader( color = MaterialTheme.colorScheme.onSurface ) Row { - IconButton(onClick = onSettingsClick) { - Icon( - imageVector = Icons.Default.Settings, - contentDescription = "Settings", - tint = MaterialTheme.colorScheme.onSurface - ) - } IconButton(onClick = onExploreClick) { Icon( imageVector = Icons.Default.Image, @@ -638,45 +620,6 @@ private fun NovelChapterList( } } -@Composable -private fun SettingsDialog( - ignoreSslErrors: Boolean, - onIgnoreSslErrorsChange: (Boolean) -> Unit, - onDismiss: () -> Unit -) { - AlertDialog( - onDismissRequest = onDismiss, - title = { Text("App Settings") }, - text = { - Column(verticalArrangement = Arrangement.spacedBy(16.dp)) { - Row( - modifier = Modifier.fillMaxWidth(), - horizontalArrangement = Arrangement.SpaceBetween, - verticalAlignment = Alignment.CenterVertically - ) { - Column(modifier = Modifier.weight(1f)) { - Text("Ignore SSL Errors", style = MaterialTheme.typography.bodyLarge) - Text( - "Enable if you encounter certificate errors on public Wi-Fi. USE WITH CAUTION.", - style = MaterialTheme.typography.bodySmall, - color = MaterialTheme.colorScheme.onSurfaceVariant - ) - } - Switch( - checked = ignoreSslErrors, - onCheckedChange = onIgnoreSslErrorsChange - ) - } - } - }, - confirmButton = { - TextButton(onClick = onDismiss) { - Text("Close") - } - } - ) -} - @Composable private fun EmptyLibraryState() { Box( diff --git a/app/src/main/java/io/aatricks/novelscraper/ui/screens/ReaderScreen.kt b/app/src/main/java/io/aatricks/novelscraper/ui/screens/ReaderScreen.kt index 4e6ab02..6a24ed3 100644 --- a/app/src/main/java/io/aatricks/novelscraper/ui/screens/ReaderScreen.kt +++ b/app/src/main/java/io/aatricks/novelscraper/ui/screens/ReaderScreen.kt @@ -149,8 +149,7 @@ fun ReaderScreen( onRetry = { showCloudflareWebView = false readerViewModel.retryLoad() - }, - ignoreSslErrors = libraryViewModel.ignoreSslErrors + } ) } @@ -231,8 +230,7 @@ fun ReaderScreen( private fun CloudflareDialog( url: String, onDismiss: () -> Unit, - onRetry: () -> Unit, - ignoreSslErrors: Boolean = false + onRetry: () -> Unit ): Unit { val context = LocalContext.current var webViewError by remember { mutableStateOf(null) } @@ -314,15 +312,6 @@ private fun CloudflareDialog( userAgentString = "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/120.0.0.0 Safari/537.36" } webViewClient = object : WebViewClient() { - override fun onReceivedSslError( - view: WebView?, - handler: android.webkit.SslErrorHandler?, - error: android.net.http.SslError? - ) { - if (ignoreSslErrors) handler?.proceed() - else super.onReceivedSslError(view, handler, error) - } - override fun onReceivedError( view: WebView?, request: android.webkit.WebResourceRequest?, diff --git a/app/src/main/java/io/aatricks/novelscraper/ui/viewmodel/LibraryViewModel.kt b/app/src/main/java/io/aatricks/novelscraper/ui/viewmodel/LibraryViewModel.kt index 7402a4e..fe2fd67 100644 --- a/app/src/main/java/io/aatricks/novelscraper/ui/viewmodel/LibraryViewModel.kt +++ b/app/src/main/java/io/aatricks/novelscraper/ui/viewmodel/LibraryViewModel.kt @@ -2,7 +2,6 @@ package io.aatricks.novelscraper.ui.viewmodel import androidx.lifecycle.viewModelScope import dagger.hilt.android.lifecycle.HiltViewModel -import io.aatricks.novelscraper.data.local.PreferencesManager import io.aatricks.novelscraper.data.model.ContentType import io.aatricks.novelscraper.data.model.LibraryItem import io.aatricks.novelscraper.data.model.SortMode @@ -19,19 +18,11 @@ import android.util.Log class LibraryViewModel @Inject constructor( val repository: LibraryRepository, private val contentRepository: ContentRepository, - private val exploreRepository: ExploreRepository, - private val preferencesManager: PreferencesManager + private val exploreRepository: ExploreRepository ) : BaseViewModel(LibraryUiState()) { private val TAG = "LibraryViewModel" - var ignoreSslErrors: Boolean - get() = preferencesManager.ignoreSslErrors - set(value) { - preferencesManager.ignoreSslErrors = value - updateState { it.copy(ignoreSslErrors = value) } - } - private val _searchQuery = MutableStateFlow("") val searchQuery: StateFlow = _searchQuery.asStateFlow() @@ -57,8 +48,7 @@ class LibraryViewModel @Inject constructor( val selectedIds: Set = emptySet(), val selectedCount: Int = 0, val isEmpty: Boolean = true, - val currentlyReading: LibraryItem? = null, - val ignoreSslErrors: Boolean = false + val currentlyReading: LibraryItem? = null ) private fun observeLibraryChanges(): Unit { @@ -91,8 +81,7 @@ class LibraryViewModel @Inject constructor( selectedIds = selectedIds, selectedCount = selectedIds.size, isEmpty = items.isEmpty(), - currentlyReading = items.find { it.isCurrentlyReading }, - ignoreSslErrors = preferencesManager.ignoreSslErrors + currentlyReading = items.find { it.isCurrentlyReading } ) }.collect { newState -> updateState { newState } diff --git a/app/src/test/java/io/aatricks/novelscraper/ui/viewmodel/LibraryViewModelTest.kt b/app/src/test/java/io/aatricks/novelscraper/ui/viewmodel/LibraryViewModelTest.kt new file mode 100644 index 0000000..ce6a6ef --- /dev/null +++ b/app/src/test/java/io/aatricks/novelscraper/ui/viewmodel/LibraryViewModelTest.kt @@ -0,0 +1,63 @@ +package io.aatricks.novelscraper.ui.viewmodel + +import io.aatricks.novelscraper.data.model.LibraryItem +import io.aatricks.novelscraper.data.repository.ContentRepository +import io.aatricks.novelscraper.data.repository.ExploreRepository +import io.aatricks.novelscraper.data.repository.LibraryRepository +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.test.* +import org.junit.After +import org.junit.Before +import org.junit.Test +import org.junit.Assert.* +import org.mockito.Mock +import org.mockito.MockitoAnnotations +import org.mockito.kotlin.* + +@OptIn(ExperimentalCoroutinesApi::class) +class LibraryViewModelTest { + + private val testDispatcher = StandardTestDispatcher() + + @Mock + lateinit var libraryRepository: LibraryRepository + + @Mock + lateinit var contentRepository: ContentRepository + + @Mock + lateinit var exploreRepository: ExploreRepository + + private lateinit var viewModel: LibraryViewModel + + @Before + fun setup() { + MockitoAnnotations.openMocks(this) + Dispatchers.setMain(testDispatcher) + + whenever(libraryRepository.libraryItems).thenReturn(MutableStateFlow(emptyList())) + whenever(libraryRepository.selectedItems).thenReturn(MutableStateFlow(emptySet())) + whenever(libraryRepository.collapsedSources).thenReturn(MutableStateFlow(emptySet())) + + viewModel = LibraryViewModel( + libraryRepository, + contentRepository, + exploreRepository + ) + } + + @After + fun tearDown() { + Dispatchers.resetMain() + } + + @Test + fun `initial state is correct`() = runTest { + val state = viewModel.uiState.value + // It should verify that state loads correctly and doesn't crash on ignoreSslErrors access (as it was removed) + assertNotNull(state) + assertTrue(state.isEmpty) + } +}