fix: Ensure cookies authentication prior to webview loading (openedx#312

) Ensure cookies' expiry time is verified before loading a webview. Additionally, addressed a race condition within the `clearWebViewCookie` method. This race condition caused the premature reset of `authSessionCookieExpiration` to -1 due to delays in the callback execution. Fixes: LEARNER-9891
touchapp · May 6, 2024 · 223fc43 · 223fc43
1 parent 4d782ee
commit 223fc43
Show file tree

Hide file tree

Showing 5 changed files with 65 additions and 30 deletions.
diff --git a/core/src/main/java/org/openedx/core/extension/ViewExt.kt b/core/src/main/java/org/openedx/core/extension/ViewExt.kt
@@ -6,8 +6,12 @@ import android.graphics.Rect
 import android.util.DisplayMetrics
 import android.view.View
 import android.view.ViewGroup
+import android.webkit.WebView
 import android.widget.Toast
 import androidx.fragment.app.DialogFragment
+import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.launch
+import org.openedx.core.system.AppCookieManager
 
 fun Context.dpToPixel(dp: Int): Float {
     return dp * (resources.displayMetrics.densityDpi.toFloat() / DisplayMetrics.DENSITY_DEFAULT)
@@ -46,3 +50,14 @@ fun DialogFragment.setWidthPercent(percentage: Int) {
 fun Context.toastMessage(message: String) {
     Toast.makeText(this, message, Toast.LENGTH_SHORT).show()
 }
+
+fun WebView.loadUrl(url: String, scope: CoroutineScope, cookieManager: AppCookieManager) {
+    if (cookieManager.isSessionCookieMissingOrExpired()) {
+        scope.launch {
+            cookieManager.tryToRefreshSessionCookie()
+            loadUrl(url)
+        }
+    } else {
+        loadUrl(url)
+    }
+}
diff --git a/core/src/main/java/org/openedx/core/system/AppCookieManager.kt b/core/src/main/java/org/openedx/core/system/AppCookieManager.kt
@@ -11,8 +11,6 @@ import java.util.concurrent.TimeUnit
 class AppCookieManager(private val config: Config, private val api: CookiesApi) {
 
     companion object {
-        private const val REV_934_COOKIE =
-            "REV_934=mobile; expires=Tue, 31 Dec 2021 12:00:20 GMT; domain=.edx.org;"
         private val FRESHNESS_INTERVAL = TimeUnit.HOURS.toMillis(1)
     }
 
@@ -34,19 +32,11 @@ class AppCookieManager(private val config: Config, private val api: CookiesApi)
     }
 
     fun clearWebViewCookie() {
-        CookieManager.getInstance().removeAllCookies { result ->
-            if (result) {
-                authSessionCookieExpiration = -1
-            }
-        }
+        CookieManager.getInstance().removeAllCookies(null)
+        authSessionCookieExpiration = -1
     }
 
     fun isSessionCookieMissingOrExpired(): Boolean {
         return authSessionCookieExpiration < System.currentTimeMillis()
     }
-
-    fun setMobileCookie() {
-        CookieManager.getInstance().setCookie(config.getApiHostURL(), REV_934_COOKIE)
-    }
-
 }
diff --git a/course/src/main/java/org/openedx/course/presentation/unit/html/HtmlUnitFragment.kt b/course/src/main/java/org/openedx/course/presentation/unit/html/HtmlUnitFragment.kt
@@ -9,13 +9,30 @@ import android.os.Bundle
 import android.util.Log
 import android.view.LayoutInflater
 import android.view.ViewGroup
-import android.webkit.*
+import android.webkit.JavascriptInterface
+import android.webkit.WebResourceRequest
+import android.webkit.WebResourceResponse
+import android.webkit.WebView
+import android.webkit.WebViewClient
 import androidx.compose.foundation.background
 import androidx.compose.foundation.isSystemInDarkTheme
-import androidx.compose.foundation.layout.*
+import androidx.compose.foundation.layout.Box
+import androidx.compose.foundation.layout.fillMaxHeight
+import androidx.compose.foundation.layout.fillMaxSize
+import androidx.compose.foundation.layout.fillMaxWidth
+import androidx.compose.foundation.layout.padding
+import androidx.compose.foundation.layout.widthIn
 import androidx.compose.foundation.shape.RoundedCornerShape
-import androidx.compose.material.*
-import androidx.compose.runtime.*
+import androidx.compose.material.CircularProgressIndicator
+import androidx.compose.material.MaterialTheme
+import androidx.compose.material.Surface
+import androidx.compose.runtime.Composable
+import androidx.compose.runtime.collectAsState
+import androidx.compose.runtime.getValue
+import androidx.compose.runtime.mutableStateOf
+import androidx.compose.runtime.remember
+import androidx.compose.runtime.rememberCoroutineScope
+import androidx.compose.runtime.setValue
 import androidx.compose.ui.Alignment
 import androidx.compose.ui.Modifier
 import androidx.compose.ui.draw.clip
@@ -33,10 +50,15 @@ import androidx.fragment.app.Fragment
 import kotlinx.coroutines.launch
 import org.koin.androidx.viewmodel.ext.android.viewModel
 import org.openedx.core.extension.isEmailValid
+import org.openedx.core.extension.loadUrl
 import org.openedx.core.system.AppCookieManager
-import org.openedx.core.ui.*
+import org.openedx.core.ui.ConnectionErrorView
+import org.openedx.core.ui.WindowSize
+import org.openedx.core.ui.rememberWindowSize
+import org.openedx.core.ui.roundBorderWithoutBottom
 import org.openedx.core.ui.theme.OpenEdXTheme
 import org.openedx.core.ui.theme.appColors
+import org.openedx.core.ui.windowSizeValue
 import org.openedx.core.utils.EmailUtil
 
 class HtmlUnitFragment : Fragment() {
@@ -268,13 +290,15 @@ private fun HTMLContentView(
                 }
                 isVerticalScrollBarEnabled = false
                 isHorizontalScrollBarEnabled = false
-                loadUrl(url)
+
+                loadUrl(url, coroutineScope, cookieManager)
             }
         },
         update = { webView ->
             if (!isLoading && injectJSList.isNotEmpty()) {
                 injectJSList.forEach { webView.evaluateJavascript(it, null) }
             }
-        })
+        }
+    )
 }
 
diff --git a/discovery/src/main/java/org/openedx/discovery/presentation/program/ProgramFragment.kt b/discovery/src/main/java/org/openedx/discovery/presentation/program/ProgramFragment.kt
@@ -23,6 +23,7 @@ import androidx.compose.runtime.collectAsState
 import androidx.compose.runtime.getValue
 import androidx.compose.runtime.mutableStateOf
 import androidx.compose.runtime.remember
+import androidx.compose.runtime.rememberCoroutineScope
 import androidx.compose.runtime.setValue
 import androidx.compose.ui.Alignment
 import androidx.compose.ui.ExperimentalComposeUiApi
@@ -41,10 +42,14 @@ import androidx.compose.ui.viewinterop.AndroidView
 import androidx.compose.ui.zIndex
 import androidx.core.os.bundleOf
 import androidx.fragment.app.Fragment
+import kotlinx.coroutines.launch
+import org.koin.androidx.compose.koinViewModel
 import org.koin.androidx.viewmodel.ext.android.viewModel
+import org.openedx.core.extension.loadUrl
 import org.openedx.core.extension.toastMessage
 import org.openedx.core.presentation.dialog.alert.ActionDialogFragment
 import org.openedx.core.presentation.dialog.alert.InfoDialogFragment
+import org.openedx.core.system.AppCookieManager
 import org.openedx.core.ui.ConnectionErrorView
 import org.openedx.core.ui.HandleUIMessage
 import org.openedx.core.ui.Toolbar
@@ -119,6 +124,7 @@ class ProgramFragment(private val myPrograms: Boolean = false) : Fragment() {
                     windowSize = windowSize,
                     uiState = uiState,
                     contentUrl = getInitialUrl(),
+                    cookieManager = viewModel.cookieManager,
                     canShowBackBtn = arguments?.getString(ARG_PATH_ID, "")
                         ?.isNotEmpty() == true,
                     uriScheme = viewModel.uriScheme,
@@ -182,15 +188,11 @@ class ProgramFragment(private val myPrograms: Boolean = false) : Fragment() {
                     onSettingsClick = {
                         viewModel.navigateToSettings(requireActivity().supportFragmentManager)
                     },
-                    refreshSessionCookie = {
-                        viewModel.refreshCookie()
-                    },
                 )
             }
         }
     }
 
-
     private fun getInitialUrl(): String {
         return arguments?.let { args ->
             val pathId = args.getString(ARG_PATH_ID) ?: ""
@@ -219,6 +221,7 @@ private fun ProgramInfoScreen(
     windowSize: WindowSize,
     uiState: ProgramUIState?,
     contentUrl: String,
+    cookieManager: AppCookieManager,
     uriScheme: String,
     canShowBackBtn: Boolean,
     hasInternetConnection: Boolean,
@@ -227,10 +230,10 @@ private fun ProgramInfoScreen(
     onSettingsClick: () -> Unit,
     onBackClick: () -> Unit,
     onUriClick: (String, WebViewLink.Authority) -> Unit,
-    refreshSessionCookie: () -> Unit = {},
 ) {
     val scaffoldState = rememberScaffoldState()
     val configuration = LocalConfiguration.current
+    val coroutineScope = rememberCoroutineScope()
     val isLoading = uiState is ProgramUIState.Loading
 
     when (uiState) {
@@ -290,7 +293,11 @@ private fun ProgramInfoScreen(
                             uriScheme = uriScheme,
                             isAllLinksExternal = true,
                             onWebPageLoaded = onWebPageLoaded,
-                            refreshSessionCookie = refreshSessionCookie,
+                            refreshSessionCookie = {
+                                coroutineScope.launch {
+                                    cookieManager.tryToRefreshSessionCookie()
+                                }
+                            },
                             onUriClick = onUriClick,
                         )
 
@@ -301,7 +308,7 @@ private fun ProgramInfoScreen(
                                 webView
                             },
                             update = {
-                                webView.loadUrl(contentUrl)
+                                webView.loadUrl(contentUrl, coroutineScope, cookieManager)
                             }
                         )
                     } else {
@@ -339,6 +346,7 @@ fun MyProgramsPreview() {
             windowSize = WindowSize(WindowType.Compact, WindowType.Compact),
             uiState = ProgramUIState.Loading,
             contentUrl = "https://www.example.com/",
+            cookieManager = koinViewModel<ProgramViewModel>().cookieManager,
             uriScheme = "",
             canShowBackBtn = false,
             hasInternetConnection = false,

diff --git a/discovery/src/main/java/org/openedx/discovery/presentation/program/ProgramViewModel.kt b/discovery/src/main/java/org/openedx/discovery/presentation/program/ProgramViewModel.kt
@@ -34,6 +34,8 @@ class ProgramViewModel(
 
     val programConfig get() = config.getProgramConfig().webViewConfig
 
+    val cookieManager get() = edxCookieManager
+
     val hasInternetConnection: Boolean get() = networkConnection.isOnline()
 
     private val _uiState = MutableSharedFlow<ProgramUIState>(
@@ -104,8 +106,4 @@ class ProgramViewModel(
     fun navigateToSettings(fragmentManager: FragmentManager) {
         router.navigateToSettings(fragmentManager)
     }
-
-    fun refreshCookie() {
-        viewModelScope.launch { edxCookieManager.tryToRefreshSessionCookie() }
-    }
 }