Merge pull request #2315 from digma-ai/auth-improvements

shalom938 · web-flow · commit eee6aba34e7c · 2024-07-25T11:45:03.000+03:00
auth improvements
diff --git a/build.gradle.kts b/build.gradle.kts
@@ -290,6 +290,7 @@ tasks {
             "idea.log.limit" to "999999999",
             "idea.trace.stub.index.update" to "true",
             "org.digma.plugin.enable.devtools" to "true",
+            "kotlinx.coroutines.debug" to "",
 
 //            "idea.ProcessCanceledException" to "disabled"
 
diff --git a/ide-common/src/main/kotlin/org/digma/intellij/plugin/analytics/AnalyticsUrlProvider.kt b/ide-common/src/main/kotlin/org/digma/intellij/plugin/analytics/AnalyticsUrlProvider.kt
@@ -39,6 +39,7 @@ class AnalyticsUrlProvider : DisposableAdaptor, BaseUrlProvider {
 
                 if (state.apiUrl != myApiUrl) {
                     Log.log(logger::trace, "api url changed to {}, replacing myApiUrl", state.apiUrl)
+                    AuthManager.getInstance().stopAutoRefresh("on api url changed")
                     AuthManager.getInstance().logoutSynchronously()
                     ThreadPoolProviderService.getInstance().interruptAll()
                     val oldUrl = myApiUrl
@@ -48,6 +49,7 @@ class AnalyticsUrlProvider : DisposableAdaptor, BaseUrlProvider {
                     fireUrlChangedEvent(oldUrl, myApiUrl)
                     //after clients replaced do loginOrRefresh
                     AuthManager.getInstance().loginOrRefreshAsync()
+                    AuthManager.getInstance().startAutoRefresh()
                     doForAllProjects { project ->
                         project.messageBus.syncPublisher(ApiClientChangedEvent.API_CLIENT_CHANGED_TOPIC).apiClientChanged(myApiUrl)
                     }
diff --git a/ide-common/src/main/kotlin/org/digma/intellij/plugin/analytics/ApiErrorHandler.kt b/ide-common/src/main/kotlin/org/digma/intellij/plugin/analytics/ApiErrorHandler.kt
@@ -8,6 +8,7 @@ import com.intellij.openapi.project.ProjectManager
 import com.intellij.util.Alarm
 import org.digma.intellij.plugin.common.DisposableAdaptor
 import org.digma.intellij.plugin.common.EDT
+import org.digma.intellij.plugin.common.ExceptionUtils
 import org.digma.intellij.plugin.common.ExceptionUtils.findConnectException
 import org.digma.intellij.plugin.common.ExceptionUtils.findFirstRealExceptionCause
 import org.digma.intellij.plugin.common.ExceptionUtils.findSslException
@@ -196,7 +197,7 @@ class ApiErrorHandler : DisposableAdaptor {
 
 
     //AuthManager doesn't always have a project, only on startup when calling withAuth
-    fun handleAuthManagerError(throwable: Throwable, project: Project?) {
+    fun handleAuthManagerCantConnectError(throwable: Throwable, project: Project?) {
         try {
             myLock.lock()
             handleAuthManagerErrorImpl(throwable, project)
@@ -210,6 +211,26 @@ class ApiErrorHandler : DisposableAdaptor {
         }
     }
 
+    fun handleAuthManagerCantRefreshError(throwable: Throwable, project: Project?) {
+        //todo: currently only reporting
+        Log.warnWithException(logger, throwable, "error in AuthManager {}", throwable)
+
+        val message = if (throwable is AuthenticationException) {
+            throwable.message
+        } else {
+            ExceptionUtils.getNonEmptyMessage(throwable)
+        }
+
+        doForAllProjects { p ->
+            EDT.ensureEDT {
+                NotificationUtil.notifyWarning(
+                    project, "<html>Error with Digma backend " + message + ".<br> "
+                            + message + ".<br> See logs for details."
+                )
+            }
+        }
+    }
+
 
     //called from auth manager when it can't create a login handler which is a critical error.
     // we want to show no connection anyway to notify the user that we can't login or refresh the token. there is no way for
diff --git a/ide-common/src/main/kotlin/org/digma/intellij/plugin/auth/AuthManager.kt b/ide-common/src/main/kotlin/org/digma/intellij/plugin/auth/AuthManager.kt
diff --git a/ide-common/src/main/kotlin/org/digma/intellij/plugin/auth/account/AbstractLoginHandler.kt b/ide-common/src/main/kotlin/org/digma/intellij/plugin/auth/account/AbstractLoginHandler.kt
@@ -92,7 +92,7 @@ abstract class AbstractLoginHandler(protected val analyticsProvider: RestAnalyti
                 reportPosthogEvent("refresh failed", mapOf("error" to errorMessage))
             }
 
-            false
+            throw e
         }
     }
 
diff --git a/ide-common/src/main/kotlin/org/digma/intellij/plugin/auth/account/CentralizedLoginHandler.kt b/ide-common/src/main/kotlin/org/digma/intellij/plugin/auth/account/CentralizedLoginHandler.kt
@@ -90,11 +90,10 @@ class CentralizedLoginHandler(analyticsProvider: RestAnalyticsProvider) : Abstra
             val errorMessage = ExceptionUtils.getNonEmptyMessage(e)
             reportPosthogEvent("loginOrRefresh failed", mapOf("error" to errorMessage))
 
-            //if got exception here it may be from refresh or login, in both cases delete the current account,
-            //and user will be redirected to log in again
+            //if got exception here then we probably can't refresh,logout, user will be redirected to login,
+            // throw the exception to report it
             logout()
-
-            false
+            throw e
         }
 
     }
diff --git a/ide-common/src/main/kotlin/org/digma/intellij/plugin/auth/account/LocalLoginHandler.kt b/ide-common/src/main/kotlin/org/digma/intellij/plugin/auth/account/LocalLoginHandler.kt
@@ -116,9 +116,10 @@ class LocalLoginHandler(analyticsProvider: RestAnalyticsProvider) : AbstractLogi
             val errorMessage = ExceptionUtils.getNonEmptyMessage(e)
             reportPosthogEvent("loginOrRefresh failed", mapOf("error" to errorMessage))
 
-            //if got exception here it may be from refresh or login, in both cases delete the current account,
-            //we'll do a silent login again
+            //if got exception here it may be from refresh or login, in both cases delete the current account
+            //and login again
             logout()
+            login(SILENT_LOGIN_USER, SILENT_LOGIN_PASSWORD)
 
             false
         }
diff --git a/ide-common/src/main/kotlin/org/digma/intellij/plugin/auth/account/LoginHandler.kt b/ide-common/src/main/kotlin/org/digma/intellij/plugin/auth/account/LoginHandler.kt
@@ -3,6 +3,8 @@ package org.digma.intellij.plugin.auth.account
 import com.intellij.openapi.diagnostic.Logger
 import com.intellij.openapi.project.Project
 import kotlinx.coroutines.CoroutineName
+import kotlinx.coroutines.sync.Mutex
+import kotlinx.coroutines.sync.withLock
 import org.digma.intellij.plugin.analytics.ApiErrorHandler
 import org.digma.intellij.plugin.analytics.ReplacingClientException
 import org.digma.intellij.plugin.analytics.RestAnalyticsProvider
@@ -22,6 +24,8 @@ interface LoginHandler {
 
         private val logger: Logger = Logger.getInstance(this::class.java)
 
+        private val mutex = Mutex()
+
         fun createLoginHandler(analyticsProvider: RestAnalyticsProvider): LoginHandler {
             return createLoginHandler(null, analyticsProvider)
         }
@@ -51,7 +55,7 @@ interface LoginHandler {
 
                 //if we can't create a login handler we assume there is a connection issue, it may be a real connect issue,
                 // or any other issue were we can't get analyticsProvider.about
-                ApiErrorHandler.getInstance().handleAuthManagerError(e, project)
+                ApiErrorHandler.getInstance().handleAuthManagerCantConnectError(e, project)
 
                 Log.warnWithException(logger, e, "Exception in createLoginHandler {}, url {}", e, analyticsProvider.apiUrl)
                 ErrorReporter.getInstance().reportError("AuthManager.createLoginHandler", e)
@@ -65,15 +69,22 @@ interface LoginHandler {
 
     suspend fun login(user: String, password: String): LoginResult
 
-    //does login or refresh if necessary, return true if did successful login or successful refresh,
-    // or when no need to do anything. return false if failed.
-    //we don't really rely on the returned value, mostly for logging
-    //todo: can be true always because its called only from onAuthenticationException
+    /**
+     * does login or refresh if necessary, return true if did successful login or successful refresh,or did nothing because credentials are valid.
+     * false otherwise.
+     * don't rely on the result for authentication purposes, its mainly for logging
+     */
     suspend fun loginOrRefresh(onAuthenticationError: Boolean = false): Boolean
 
+    /**
+     * return true if refresh was successful
+     */
     suspend fun refresh(account: DigmaAccount, credentials: DigmaCredentials): Boolean
 
 
+    /**
+     * return true only if an account was really deleted
+     */
     suspend fun logout(): Boolean {
         return try {
             trace("logout called")
@@ -104,21 +115,25 @@ interface LoginHandler {
     suspend fun updateAccount(digmaAccount: DigmaAccount, digmaCredentials: DigmaCredentials) {
         trace("updating account {}", digmaAccount)
         //this is the only place we update the account and credentials.
-        DigmaAccountManager.getInstance().updateAccount(digmaAccount, digmaCredentials)
-        DigmaDefaultAccountHolder.getInstance().account = digmaAccount
-        CredentialsHolder.digmaCredentials = digmaCredentials
+        mutex.withLock {
+            DigmaAccountManager.getInstance().updateAccount(digmaAccount, digmaCredentials)
+            DigmaDefaultAccountHolder.getInstance().account = digmaAccount
+            CredentialsHolder.digmaCredentials = digmaCredentials
+        }
     }
 
 
     suspend fun deleteAccount(digmaAccount: DigmaAccount) {
         trace("deleting account {}", digmaAccount)
         //this is the only place we delete the account.
-        try {
-            DigmaAccountManager.getInstance().removeAccount(digmaAccount)
-        } finally {
-            //it's in finally because even if removeAccount failed we want to delete the account and nullify CredentialsHolder.digmaCredentials
-            DigmaDefaultAccountHolder.getInstance().account = null
-            CredentialsHolder.digmaCredentials = null
+        mutex.withLock {
+            try {
+                DigmaAccountManager.getInstance().removeAccount(digmaAccount)
+            } finally {
+                //it's in finally because even if removeAccount failed we want to delete the account and nullify CredentialsHolder.digmaCredentials
+                DigmaDefaultAccountHolder.getInstance().account = null
+                CredentialsHolder.digmaCredentials = null
+            }
         }
     }
 

Original file line number	Diff line number	Diff line change
`@@ -92,7 +92,7 @@ abstract class AbstractLoginHandler(protected val analyticsProvider: RestAnalyti`
`92`	`92`	`reportPosthogEvent("refresh failed", mapOf("error" to errorMessage))`
`93`	`93`	`}`
`94`	`94`
`95`		`- false`
	`95`	`+ throw e`
`96`	`96`	`}`
`97`	`97`	`}`
`98`	`98`
Original file line number	Diff line number	Diff line change
`@@ -90,11 +90,10 @@ class CentralizedLoginHandler(analyticsProvider: RestAnalyticsProvider) : Abstra`
`90`	`90`	`val errorMessage = ExceptionUtils.getNonEmptyMessage(e)`
`91`	`91`	`reportPosthogEvent("loginOrRefresh failed", mapOf("error" to errorMessage))`
`92`	`92`
`93`		`- //if got exception here it may be from refresh or login, in both cases delete the current account,`
`94`		`- //and user will be redirected to log in again`
	`93`	`+ //if got exception here then we probably can't refresh,logout, user will be redirected to login,`
	`94`	`+ // throw the exception to report it`
`95`	`95`	`logout()`
`96`		`-`
`97`		`- false`
	`96`	`+ throw e`
`98`	`97`	`}`
`99`	`98`
`100`	`99`	`}`