Merge branch 'housekeeping/refactoring-oauth' into 'develop'

Refactor OAuth code

See merge request funkwhale/funkwhale-android!73

app/src/main/java/audio/funkwhale/ffa/activities/SplashActivity.kt

@@ -18,7 +18,7 @@ class SplashActivity : AppCompatActivity() {
 
     getSharedPreferences(AppContext.PREFS_CREDENTIALS, Context.MODE_PRIVATE)
       .apply {
-        when (oAuth.isAuthorized(this@SplashActivity) || Settings.isAnonymous()) {
+        when (Settings.isAnonymous() || oAuth.isAuthorized(this@SplashActivity)) {
           true -> Intent(this@SplashActivity, MainActivity::class.java)
             .apply {
               Log.i("SplashActivity", "Authorized, redirecting to MainActivity")

app/src/main/java/audio/funkwhale/ffa/utils/OAuth.kt

@@ -50,13 +50,14 @@ class OAuth(private val authorizationServiceFactory: AuthorizationServiceFactory
     }
   }
 
-  fun state(): AuthState =
-    tryState() ?: throw IllegalStateException("Couldn't find saved state")
+  fun state(): AuthState {
+    return tryState() ?: throw IllegalStateException("Couldn't find saved state")
+  }
 
   fun isAuthorized(context: Context): Boolean {
     val state = tryState()
     return if (state != null) {
-      state.isAuthorized || doTryRefreshAccessToken(state, context)
+      state.isAuthorized || refreshAccessToken(context)
     } else {
       false
     }.also {
@@ -66,7 +67,17 @@ class OAuth(private val authorizationServiceFactory: AuthorizationServiceFactory
 
   fun tryRefreshAccessToken(context: Context): Boolean {
     tryState()?.let { state ->
-      return doTryRefreshAccessToken(state, context)
+      return if (state.needsTokenRefresh && state.refreshToken != null) {
+        Log.i(
+          "OAuth",
+          "needsTokenRefresh()=${state.needsTokenRefresh}, refreshToken=${
+            state.refreshToken!!.subSequence(0, 5)
+          }..."
+        )
+        refreshAccessToken(context)
+      } else {
+        state.isAuthorized
+      }.also { it.logInfo("tryRefreshAccessToken()") }
     }
     return false
   }
@@ -74,13 +85,13 @@ class OAuth(private val authorizationServiceFactory: AuthorizationServiceFactory
   fun refreshAccessToken(context: Context): Boolean {
     Log.i("OAuth", "refreshAccessToken()")
     val state = tryState()
-    return if (state != null) {
+    return if (state != null && state.refreshToken != null) {
       val refreshRequest = state.createTokenRefreshRequest()
       val auth = ClientSecretPost(state.clientSecret)
       runBlocking {
         service(context).performTokenRequest(refreshRequest, auth) { response, e ->
           state.apply {
-            Log.i("OAuth", "applying new autState")
+            Log.i("OAuth", "applying new authState")
             update(response, e)
             save()
           }
@@ -92,30 +103,6 @@ class OAuth(private val authorizationServiceFactory: AuthorizationServiceFactory
     }
   }
 
-  private fun doTryRefreshAccessToken(
-    state: AuthState,
-    context: Context
-  ): Boolean {
-    if (state.needsTokenRefresh.also { it.logInfo("needsTokenRefresh()") } &&
-      state.refreshToken != null) {
-      val refreshRequest = state.createTokenRefreshRequest()
-      val auth = ClientSecretPost(state.clientSecret)
-      runBlocking {
-        service(context).performTokenRequest(refreshRequest, auth) { response, e ->
-          state.apply {
-            Log.i("OAuth", "applying new autState")
-            update(response, e)
-            save()
-          }
-        }
-      }
-    }
-    return (state.isAuthorized)
-      .also {
-        it.logInfo("tryRefreshAccessToken()")
-      }
-  }
-
   fun init(hostname: String): AuthState {
     return AuthState(
       AuthorizationServiceConfiguration(

app/src/test/java/audio/funkwhale/ffa/utils/OAuthTest.kt

@@ -114,7 +114,7 @@ class OAuthTest {
   }
 
   @Test
-  fun `isAuthorized() should return false if existing state is not authorized and token is not refreshed`() {
+  fun `isAuthorized() should return false if existing state is not authorized and refreshToken is missing`() {
     mockkStatic(PowerPreference::class)
     mockkStatic(AuthState::class)
 
@@ -122,6 +122,7 @@ class OAuthTest {
     every { AuthState.jsonDeserialize(any<String>()) } returns authState
     every { authState.isAuthorized } returns false
     every { authState.needsTokenRefresh } returns false
+    every { authState.refreshToken } returns null
 
     every { PowerPreference.getFileByName(any()) } returns mockPreference
     every { mockPreference.getString(any()) } returns "{}"