From 2e35a313fcd42fae3ee224442eb907ba7afefb12 Mon Sep 17 00:00:00 2001 From: Ryan Harg Date: Mon, 2 Aug 2021 13:24:12 +0200 Subject: [PATCH] #7: Add more tests to OAuth component --- app/build.gradle.kts | 7 + .../java/audio/funkwhale/ffa/utils/OAuth.kt | 86 +++--- .../ffa/playback/OAuthDatasourceTest.kt | 12 +- .../funkwhale/ffa/utils/DefaultOAuthTest.kt | 270 ++++++++++++++++++ .../audio/funkwhale/util/MockKJUnitRunner.kt | 58 ---- build.gradle.kts | 2 + 6 files changed, 338 insertions(+), 97 deletions(-) create mode 100644 app/src/test/java/audio/funkwhale/ffa/utils/DefaultOAuthTest.kt delete mode 100644 app/src/test/java/audio/funkwhale/util/MockKJUnitRunner.kt diff --git a/app/build.gradle.kts b/app/build.gradle.kts index 87040e1..b666b00 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -9,6 +9,7 @@ plugins { id("org.jlleitschuh.gradle.ktlint") version "8.1.0" id("com.gladed.androidgitversion") version "0.4.14" id("com.github.triplet.play") version "2.4.2" + id("de.mobilej.unmock") jacoco } @@ -19,6 +20,12 @@ val props = Properties().apply { } } +unMock { + keep = listOf("android.net.Uri") + //keepStartingWith("org.") + //keepStartingWith("libcore.") +} + jacoco { toolVersion = "0.8.7" } diff --git a/app/src/main/java/audio/funkwhale/ffa/utils/OAuth.kt b/app/src/main/java/audio/funkwhale/ffa/utils/OAuth.kt index 4b174ce..811256e 100644 --- a/app/src/main/java/audio/funkwhale/ffa/utils/OAuth.kt +++ b/app/src/main/java/audio/funkwhale/ffa/utils/OAuth.kt @@ -33,15 +33,15 @@ interface OAuth { fun exchange(context: Activity, authorization: Intent, success: () -> Unit, error: () -> Unit) - fun init(hostname: String) + fun init(hostname: String): AuthState - fun register(callback: () -> Unit) + fun register(authState: AuthState? = null, callback: () -> Unit) fun authorize(context: Activity) fun isAuthorized(context: Context): Boolean - fun tryRefreshAccessToken(context: Context, overrideNeedsTokenRefresh: Boolean = false): Boolean + fun tryRefreshAccessToken(context: Context): Boolean fun tryState(): AuthState? @@ -55,13 +55,20 @@ object OAuthFactory { private val oAuth: OAuth init { - oAuth = DefaultOAuth() + oAuth = DefaultOAuth(AuthorizationServiceFactory()) } fun instance() = oAuth } -class DefaultOAuth : OAuth { +class AuthorizationServiceFactory { + + fun create(context: Context): AuthorizationService { + return AuthorizationService(context) + } +} + +class DefaultOAuth(private val authorizationServiceFactory: AuthorizationServiceFactory) : OAuth { companion object { @@ -84,12 +91,13 @@ class DefaultOAuth : OAuth { } } - override fun state(): AuthState = tryState()!! + override fun state(): AuthState = + tryState() ?: throw IllegalStateException("Couldn't find saved state") override fun isAuthorized(context: Context): Boolean { val state = tryState() return if (state != null) { - state.isAuthorized || tryRefreshAccessToken(context) + state.isAuthorized || doTryRefreshAccessToken(state, context) } else { false }.also { @@ -97,41 +105,53 @@ class DefaultOAuth : OAuth { } } - override fun tryRefreshAccessToken( - context: Context, - overrideNeedsTokenRefresh: Boolean - ): Boolean { + override fun tryRefreshAccessToken(context: Context): Boolean { tryState()?.let { state -> - val shouldRefreshAccessToken = overrideNeedsTokenRefresh || state.needsTokenRefresh - if (shouldRefreshAccessToken && state.refreshToken != null) { - val refreshRequest = state.createTokenRefreshRequest() - val auth = ClientSecretPost(state.clientSecret) - runBlocking { - service(context).performTokenRequest(refreshRequest, auth) { response, e -> - state.apply { - update(response, e) - save() - } + return doTryRefreshAccessToken(state, context) + } + return false + } + + private fun doTryRefreshAccessToken( + state: AuthState, + context: Context + ): Boolean { + if (state.needsTokenRefresh && state.refreshToken != null) { + val refreshRequest = state.createTokenRefreshRequest() + val auth = ClientSecretPost(state.clientSecret) + runBlocking { + service(context).performTokenRequest(refreshRequest, auth) { response, e -> + state.apply { + update(response, e) + save() } } } } - - return (tryState()?.isAuthorized ?: false) + return (state.isAuthorized) .also { it.log("tryRefreshAccessToken()") } } - override fun init(hostname: String) { - AuthState(config(hostname)).save() + override fun init(hostname: String): AuthState { + return AuthState( + AuthorizationServiceConfiguration( + Uri.parse("$hostname/authorize"), + Uri.parse("$hostname/api/v1/oauth/token/"), + Uri.parse("$hostname/api/v1/oauth/apps/") + ) + ) + .also { + it.save() + } } - override fun service(context: Context): AuthorizationService = AuthorizationService(context) - - override fun register(callback: () -> Unit) { - state().authorizationServiceConfiguration?.let { config -> + override fun service(context: Context): AuthorizationService = + authorizationServiceFactory.create(context) + override fun register(authState: AuthState?, callback: () -> Unit) { + (authState ?: state()).authorizationServiceConfiguration?.let { config -> runBlocking { val (_, _, result) = Fuel.post(config.registrationEndpoint.toString()) .header("Content-Type", "application/json") @@ -214,12 +234,6 @@ class DefaultOAuth : OAuth { } } - private fun config(hostname: String) = AuthorizationServiceConfiguration( - Uri.parse("$hostname/authorize"), - Uri.parse("$hostname/api/v1/oauth/token/"), - Uri.parse("$hostname/api/v1/oauth/apps/") - ) - private fun registration() = state().authorizationServiceConfiguration?.let { config -> RegistrationRequest.Builder(config, listOf(REDIRECT_URI)).build() @@ -238,3 +252,5 @@ class DefaultOAuth : OAuth { } } } + + diff --git a/app/src/test/java/audio/funkwhale/ffa/playback/OAuthDatasourceTest.kt b/app/src/test/java/audio/funkwhale/ffa/playback/OAuthDatasourceTest.kt index 1dd0b7c..76ffccf 100644 --- a/app/src/test/java/audio/funkwhale/ffa/playback/OAuthDatasourceTest.kt +++ b/app/src/test/java/audio/funkwhale/ffa/playback/OAuthDatasourceTest.kt @@ -3,21 +3,20 @@ package audio.funkwhale.ffa.playback import android.content.Context import android.net.Uri import audio.funkwhale.ffa.utils.OAuth -import audio.funkwhale.util.MockKJUnitRunner import com.google.android.exoplayer2.upstream.DataSpec import com.google.android.exoplayer2.upstream.HttpDataSource import com.google.android.exoplayer2.upstream.TransferListener +import io.mockk.MockKAnnotations import io.mockk.every import io.mockk.impl.annotations.InjectMockKs import io.mockk.impl.annotations.MockK import io.mockk.mockk import io.mockk.verify +import org.junit.Before import org.junit.Test -import org.junit.runner.RunWith import strikt.api.expectThat import strikt.assertions.isEqualTo -@RunWith(MockKJUnitRunner::class) class OAuthDatasourceTest { @InjectMockKs @@ -34,10 +33,15 @@ class OAuthDatasourceTest { private var dataSpec: DataSpec = DataSpec(Uri.EMPTY) + @Before + fun setup(){ + MockKAnnotations.init(this, relaxUnitFun = true) + } + @Test fun `open() should set accessToken and delegate to http dataSource`() { every { http.open(any()) } returns 0 - every { oAuth.tryRefreshAccessToken(any(), any()) } returns true + every { oAuth.tryRefreshAccessToken(any()) } returns true every { oAuth.state().accessToken } returns "accessToken" datasource.open(dataSpec) diff --git a/app/src/test/java/audio/funkwhale/ffa/utils/DefaultOAuthTest.kt b/app/src/test/java/audio/funkwhale/ffa/utils/DefaultOAuthTest.kt new file mode 100644 index 0000000..6923501 --- /dev/null +++ b/app/src/test/java/audio/funkwhale/ffa/utils/DefaultOAuthTest.kt @@ -0,0 +1,270 @@ +package audio.funkwhale.ffa.utils + +import android.content.Context +import com.github.kittinunf.fuel.core.Client +import com.github.kittinunf.fuel.core.FuelManager +import com.github.kittinunf.fuel.core.Request +import com.google.gson.Gson +import com.google.gson.reflect.TypeToken +import com.preference.PowerPreference +import com.preference.Preference +import io.mockk.MockKAnnotations +import io.mockk.coVerify +import io.mockk.every +import io.mockk.impl.annotations.InjectMockKs +import io.mockk.impl.annotations.MockK +import io.mockk.mockk +import io.mockk.mockkStatic +import io.mockk.slot +import io.mockk.verify +import net.openid.appauth.AuthState +import net.openid.appauth.AuthorizationService +import net.openid.appauth.AuthorizationServiceConfiguration +import net.openid.appauth.ClientSecretPost +import org.junit.Before +import org.junit.Test +import strikt.api.expectThat +import strikt.api.expectThrows +import strikt.assertions.isEqualTo +import strikt.assertions.isFalse +import strikt.assertions.isNotNull +import strikt.assertions.isNull +import strikt.assertions.isTrue + + +class DefaultOAuthTest { + + @InjectMockKs + private lateinit var oAuth: DefaultOAuth + + @MockK + private lateinit var authServiceFactory: AuthorizationServiceFactory + + @MockK + private lateinit var authService: AuthorizationService + + @MockK + private lateinit var mockPreference: Preference + + @MockK + private lateinit var context: Context + + @Before + fun setup() { + MockKAnnotations.init(this, relaxUnitFun = true) + } + + @Test + fun `tryState() should return null if saved state is missing`() { + mockkStatic(PowerPreference::class) + every { PowerPreference.getFileByName(any()) } returns mockPreference + every { mockPreference.getString(any()) } returns null + expectThat(oAuth.tryState()).isNull() + } + + @Test + fun `tryState() should return null if saved state is empty`() { + mockkStatic(PowerPreference::class) + every { PowerPreference.getFileByName(any()) } returns mockPreference + every { mockPreference.getString(any()) } returns "" + expectThat(oAuth.tryState()).isNull() + } + + @Test + fun `tryState() should return deserialized object if saved state is present`() { + mockkStatic(PowerPreference::class) + every { PowerPreference.getFileByName(any()) } returns mockPreference + every { mockPreference.getString(any()) } returns "{}" + expectThat(oAuth.tryState()).isNotNull() + } + + @Test + fun `state() should return deserialized object if saved state is present`() { + + mockkStatic(PowerPreference::class) + mockkStatic(AuthState::class) + + val authState = AuthState() + every { AuthState.jsonDeserialize(any()) } returns authState + + every { PowerPreference.getFileByName(any()) } returns mockPreference + every { mockPreference.getString(any()) } returns "{}" + + val result = oAuth.state() + expectThat(result).isEqualTo(authState) + } + + @Test + fun `state() should throw error if saved state is missing`() { + mockkStatic(PowerPreference::class) + every { PowerPreference.getFileByName(any()) } returns mockPreference + every { mockPreference.getString(any()) } returns null + + expectThrows { oAuth.state() } + } + + @Test + fun `isAuthorized() should return false if no state exists`() { + mockkStatic(PowerPreference::class) + every { PowerPreference.getFileByName(any()) } returns mockPreference + every { mockPreference.getString(any()) } returns null + + expectThat(oAuth.isAuthorized(context)).isFalse() + } + + @Test + fun `isAuthorized() should return false if existing state is not authorized and token is not refreshed`() { + mockkStatic(PowerPreference::class) + mockkStatic(AuthState::class) + + val authState = mockk() + every { AuthState.jsonDeserialize(any()) } returns authState + every { authState.isAuthorized } returns false + every { authState.needsTokenRefresh } returns false + + every { PowerPreference.getFileByName(any()) } returns mockPreference + every { mockPreference.getString(any()) } returns "{}" + + expectThat(oAuth.isAuthorized(context)).isFalse() + } + + @Test + fun `isAuthorized() should return true if existing state is authorized`() { + mockkStatic(PowerPreference::class) + mockkStatic(AuthState::class) + + val authState = mockk() + every { AuthState.jsonDeserialize(any()) } returns authState + every { authState.isAuthorized } returns true + + val mockPref = mockk() + every { PowerPreference.getFileByName(any()) } returns mockPref + every { mockPref.getString(any()) } returns "{}" + + expectThat(oAuth.isAuthorized(context)).isTrue() + } + + @Test + fun `tryRefreshAccessToken() should perform token refresh request if accessToken needs refresh and refreshToken exists`() { + mockkStatic(PowerPreference::class) + mockkStatic(AuthState::class) + + val authState = mockk() + every { AuthState.jsonDeserialize(any()) } returns authState + every { authState.isAuthorized } returns false + every { authState.needsTokenRefresh } returns true + every { authState.refreshToken } returns "refreshToken" + every { authState.createTokenRefreshRequest() } returns mockk() + every { authState.clientSecret } returns "clientSecret" + every { authServiceFactory.create(any()) } returns authService + every { authService.performTokenRequest(any(), any(), any()) } returns mockk() + + every { PowerPreference.getFileByName(any()) } returns mockPreference + every { mockPreference.getString(any()) } returns "{}" + + oAuth.tryRefreshAccessToken(context) + + verify { authService.performTokenRequest(any(), any(), any()) } + } + + @Test + fun `tryRefreshAccessToken() should not perform token refresh request if accessToken doesn't need refresh`() { + mockkStatic(PowerPreference::class) + every { PowerPreference.getFileByName(any()) } returns mockPreference + every { mockPreference.getString(any()) } returns "{}" + + mockkStatic(AuthState::class) + + val authState = mockk() + every { AuthState.jsonDeserialize(any()) } returns authState + every { authState.isAuthorized } returns false + every { authState.needsTokenRefresh } returns false + + oAuth.tryRefreshAccessToken(context) + + verify(exactly = 0) { authService.performTokenRequest(any(), any(), any()) } + } + + @Test + fun `init() should setup correct endpoints`() { + mockkStatic(PowerPreference::class) + every { PowerPreference.getFileByName(any()) } returns mockPreference + every { mockPreference.setString(any(), any()) } returns true + + val result = oAuth.init("hostname") + + expectThat(result.authorizationServiceConfiguration?.authorizationEndpoint.toString()) + .isEqualTo("hostname/authorize") + expectThat(result.authorizationServiceConfiguration?.tokenEndpoint.toString()) + .isEqualTo("hostname/api/v1/oauth/token/") + expectThat(result.authorizationServiceConfiguration?.registrationEndpoint.toString()) + .isEqualTo("hostname/api/v1/oauth/apps/") + } + + @Test + fun `register() should not initiate http request if configuration is missing`() { + mockkStatic(PowerPreference::class) + every { PowerPreference.getFileByName(any()) } returns mockPreference + every { mockPreference.getString(any()) } returns "{}" + + mockkStatic(AuthState::class) + val authState = mockk() + every { AuthState.jsonDeserialize(any()) } returns authState + every { authState.authorizationServiceConfiguration } returns null + + val mockkClient = mockk() + FuelManager.instance.client = mockkClient + + oAuth.register {} + + verify(exactly = 0) { mockkClient.executeRequest(any()) } + } + + @Test + fun `register() should initiate correct HTTP request to registration endpoint`() { + mockkStatic(PowerPreference::class) + every { PowerPreference.getFileByName(any()) } returns mockPreference + every { mockPreference.getString(any()) } returns "{}" + every { mockPreference.setString(any(), any()) } returns true + + mockkStatic(AuthState::class) + val authState = mockk() + every { AuthState.jsonDeserialize(any()) } returns authState + val mockConfig = mockk() + every { authState.authorizationServiceConfiguration } returns mockConfig + + val mockkClient = mockk() + + FuelManager.instance.client = mockkClient + + val state = oAuth.init("https://example.com") + oAuth.register(state) { } + + val requestSlot = slot() + + coVerify { mockkClient.awaitRequest(capture(requestSlot)) } + + val capturedRequest = requestSlot.captured + expectThat(capturedRequest.url.toString()) + .isEqualTo("https://example.com/api/v1/oauth/apps/") + + expectThat(deserializeJson>(capturedRequest)).isEqualTo( + mapOf( + "name" to "Funkwhale for Android (null)", + "redirect_uris" to "urn:/audio.funkwhale.funkwhale-android/oauth/callback", + "scopes" to "read write" + ) + ) + } + + private fun deserializeJson( + capturedRequest: Request + ): T { + return Gson().fromJson( + capturedRequest.body.asString("application/json"), + object : TypeToken() {}.type + ) + } + +} + diff --git a/app/src/test/java/audio/funkwhale/util/MockKJUnitRunner.kt b/app/src/test/java/audio/funkwhale/util/MockKJUnitRunner.kt deleted file mode 100644 index 3965737..0000000 --- a/app/src/test/java/audio/funkwhale/util/MockKJUnitRunner.kt +++ /dev/null @@ -1,58 +0,0 @@ -package audio.funkwhale.util - -import io.mockk.MockKAnnotations -import io.mockk.clearAllMocks -import org.junit.Test -import org.junit.runner.Description -import org.junit.runner.Runner -import org.junit.runner.notification.Failure -import org.junit.runner.notification.RunNotifier -import java.lang.reflect.Method - -class MockKJUnitRunner(private val testClass: Class<*>) : Runner() { - - private val methodDescriptions: MutableMap = mutableMapOf() - - init { - // Build method/descriptions map - testClass.methods - .map { method -> - val annotation: Annotation? = method.getAnnotation(Test::class.java) - method to annotation - } - .filter { (_, annotation) -> - annotation != null - } - .map { (method, annotation) -> - val desc = Description.createTestDescription(testClass, method.name, annotation) - method to desc - } - .forEach { (method, desc) -> methodDescriptions[method] = desc } - } - - override fun getDescription(): Description { - val description = Description.createSuiteDescription( - testClass.name, *testClass.annotations - ) - methodDescriptions.values.forEach { description.addChild(it) } - return description - } - - override fun run(notifier: RunNotifier?) { - val testObject = testClass.newInstance() - MockKAnnotations.init(testObject, relaxUnitFun = true) - - methodDescriptions - .onEach { (_, _) -> clearAllMocks() } - .onEach { (_, desc) -> notifier!!.fireTestStarted(desc) } - .forEach { (method, desc) -> - try { - method.invoke(testObject) - } catch (e: Throwable) { - notifier!!.fireTestFailure(Failure(desc, e.cause)) - } finally { - notifier!!.fireTestFinished(desc) - } - } - } -} diff --git a/build.gradle.kts b/build.gradle.kts index 78a0e96..0e1ca98 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -1,12 +1,14 @@ buildscript { repositories { google() + mavenCentral() jcenter() } dependencies { classpath("com.android.tools.build:gradle:4.2.2") classpath("org.jetbrains.kotlin:kotlin-gradle-plugin:1.5.21") + classpath("com.github.bjoernq:unmockplugin:0.7.8") } }