mirror of https://github.com/tuskyapp/Tusky.git
Browse Source
This does 2 things: - Removes `AccountSwitchInterceptor`, the main culprit for the bug. APIs can no longer change their base url after they have been created. As a result they are not Singletons anymore. - Additionally, I refactored how MainActivity handles Intents to make it less likely to have multiple instances of it active. Here is how I could reliably reproduce the bug: - Be logged in with account A and B - Write a post with account A, cancel it before it sends (go into flight mode for that) - Switch to account B - Open the "this post failed to send" notification from account A, drafts will open - Go back. You are in the MainActivity of account A, everything seems fine. - Go back again. You are in the old, now broken MainActivity of account B. It uses the database of account B but the network of account A. Refreshing will show posts from A. closes #4567 closes #4554 closes #4402 closes #4148 closes #2663 and possibly #4588pull/4617/head
Konrad Pozniak
2 years ago
committed by
GitHub
9 changed files with 306 additions and 261 deletions
@ -0,0 +1,57 @@
|
||||
package com.keylesspalace.tusky.network |
||||
|
||||
import com.keylesspalace.tusky.db.entity.AccountEntity |
||||
import okhttp3.OkHttpClient |
||||
import retrofit2.Retrofit |
||||
import retrofit2.create |
||||
|
||||
/** |
||||
* Creates an instance of an Api that will only make requests as the provided account. |
||||
* @param account The account to make requests as. |
||||
* When null, request without additional DOMAIN_HEADER will fail. |
||||
* @param httpClient The OkHttpClient to make requests as |
||||
* @param retrofit The Retrofit instance to derive the api from |
||||
* @param scheme The scheme to use. Only used in tests. |
||||
* @param port The port to use. Only used in tests. |
||||
*/ |
||||
inline fun <reified T> apiForAccount( |
||||
account: AccountEntity?, |
||||
httpClient: OkHttpClient, |
||||
retrofit: Retrofit, |
||||
scheme: String = "https://", |
||||
port: Int? = null |
||||
): T { |
||||
return retrofit.newBuilder() |
||||
.apply { |
||||
if (account != null) { |
||||
baseUrl("$scheme${account.domain}${ if (port == null) "" else ":$port"}") |
||||
} |
||||
} |
||||
.callFactory { originalRequest -> |
||||
var request = originalRequest |
||||
|
||||
val domainHeader = originalRequest.header(MastodonApi.DOMAIN_HEADER) |
||||
if (domainHeader != null) { |
||||
request = originalRequest.newBuilder() |
||||
.url( |
||||
originalRequest.url.newBuilder().host(domainHeader).build() |
||||
) |
||||
.removeHeader(MastodonApi.DOMAIN_HEADER) |
||||
.build() |
||||
} |
||||
|
||||
if (account != null && request.url.host == account.domain) { |
||||
request = request.newBuilder() |
||||
.header("Authorization", "Bearer ${account.accessToken}") |
||||
.build() |
||||
} |
||||
|
||||
if (request.url.host == MastodonApi.PLACEHOLDER_DOMAIN) { |
||||
FailingCall(request) |
||||
} else { |
||||
httpClient.newCall(request) |
||||
} |
||||
} |
||||
.build() |
||||
.create() |
||||
} |
||||
@ -0,0 +1,65 @@
|
||||
/* Copyright 2024 Tusky contributors |
||||
* |
||||
* This file is a part of Tusky. |
||||
* |
||||
* This program is free software; you can redistribute it and/or modify it under the terms of the |
||||
* GNU General Public License as published by the Free Software Foundation; either version 3 of the |
||||
* License, or (at your option) any later version. |
||||
* |
||||
* Tusky is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even |
||||
* the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General |
||||
* Public License for more details. |
||||
* |
||||
* You should have received a copy of the GNU General Public License along with Tusky; if not, |
||||
* see <http://www.gnu.org/licenses>. */ |
||||
|
||||
package com.keylesspalace.tusky.network |
||||
|
||||
import okhttp3.Call |
||||
import okhttp3.Callback |
||||
import okhttp3.Protocol |
||||
import okhttp3.Request |
||||
import okhttp3.Response |
||||
import okhttp3.ResponseBody.Companion.toResponseBody |
||||
import okio.Timeout |
||||
|
||||
class FailingCall(private val request: Request) : Call { |
||||
|
||||
private var isExecuted: Boolean = false |
||||
|
||||
override fun cancel() { } |
||||
|
||||
override fun clone(): Call { |
||||
return FailingCall(request()) |
||||
} |
||||
|
||||
override fun enqueue(responseCallback: Callback) { |
||||
isExecuted = true |
||||
responseCallback.onResponse(this, failingResponse()) |
||||
} |
||||
|
||||
override fun execute(): Response { |
||||
isExecuted = true |
||||
return failingResponse() |
||||
} |
||||
|
||||
override fun isCanceled(): Boolean = false |
||||
|
||||
override fun isExecuted(): Boolean = isExecuted |
||||
|
||||
override fun request(): Request = request |
||||
|
||||
override fun timeout(): Timeout { |
||||
return Timeout.NONE |
||||
} |
||||
|
||||
private fun failingResponse(): Response { |
||||
return Response.Builder() |
||||
.request(request) |
||||
.code(400) |
||||
.message("Bad Request") |
||||
.protocol(Protocol.HTTP_1_1) |
||||
.body("".toResponseBody()) |
||||
.build() |
||||
} |
||||
} |
||||
@ -1,84 +0,0 @@
|
||||
/* Copyright 2022 Tusky Contributors |
||||
* |
||||
* This file is a part of Tusky. |
||||
* |
||||
* This program is free software; you can redistribute it and/or modify it under the terms of the |
||||
* GNU General Public License as published by the Free Software Foundation; either version 3 of the |
||||
* License, or (at your option) any later version. |
||||
* |
||||
* Tusky is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even |
||||
* the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General |
||||
* Public License for more details. |
||||
* |
||||
* You should have received a copy of the GNU General Public License along with Tusky; if not, |
||||
* see <http://www.gnu.org/licenses>. */ |
||||
|
||||
package com.keylesspalace.tusky.network |
||||
|
||||
import android.util.Log |
||||
import com.keylesspalace.tusky.db.AccountManager |
||||
import java.io.IOException |
||||
import okhttp3.HttpUrl |
||||
import okhttp3.Interceptor |
||||
import okhttp3.MediaType.Companion.toMediaType |
||||
import okhttp3.Protocol |
||||
import okhttp3.Request |
||||
import okhttp3.Response |
||||
import okhttp3.ResponseBody.Companion.toResponseBody |
||||
|
||||
class InstanceSwitchAuthInterceptor(private val accountManager: AccountManager) : Interceptor { |
||||
|
||||
@Throws(IOException::class) |
||||
override fun intercept(chain: Interceptor.Chain): Response { |
||||
val originalRequest: Request = chain.request() |
||||
|
||||
// only switch domains if the request comes from retrofit |
||||
return if (originalRequest.url.host == MastodonApi.PLACEHOLDER_DOMAIN) { |
||||
val builder: Request.Builder = originalRequest.newBuilder() |
||||
val instanceHeader = originalRequest.header(MastodonApi.DOMAIN_HEADER) |
||||
|
||||
if (instanceHeader != null) { |
||||
// use domain explicitly specified in custom header |
||||
builder.url(swapHost(originalRequest.url, instanceHeader)) |
||||
builder.removeHeader(MastodonApi.DOMAIN_HEADER) |
||||
} else { |
||||
val currentAccount = accountManager.activeAccount |
||||
|
||||
if (currentAccount != null) { |
||||
val accessToken = currentAccount.accessToken |
||||
if (accessToken.isNotEmpty()) { |
||||
// use domain of current account |
||||
builder.url(swapHost(originalRequest.url, currentAccount.domain)) |
||||
.header("Authorization", "Bearer %s".format(accessToken)) |
||||
} |
||||
} |
||||
} |
||||
|
||||
val newRequest: Request = builder.build() |
||||
|
||||
if (MastodonApi.PLACEHOLDER_DOMAIN == newRequest.url.host) { |
||||
Log.w( |
||||
"ISAInterceptor", |
||||
"no user logged in or no domain header specified - can't make request to " + newRequest.url |
||||
) |
||||
return Response.Builder() |
||||
.code(400) |
||||
.message("Bad Request") |
||||
.protocol(Protocol.HTTP_2) |
||||
.body("".toResponseBody("text/plain".toMediaType())) |
||||
.request(chain.request()) |
||||
.build() |
||||
} |
||||
|
||||
chain.proceed(newRequest) |
||||
} else { |
||||
chain.proceed(originalRequest) |
||||
} |
||||
} |
||||
|
||||
companion object { |
||||
private fun swapHost(url: HttpUrl, host: String): HttpUrl { |
||||
return url.newBuilder().host(host).build() |
||||
} |
||||
} |
||||
} |
||||
@ -1,142 +0,0 @@
|
||||
package com.keylesspalace.tusky.network |
||||
|
||||
import com.keylesspalace.tusky.db.AccountManager |
||||
import com.keylesspalace.tusky.db.entity.AccountEntity |
||||
import okhttp3.OkHttpClient |
||||
import okhttp3.Request |
||||
import okhttp3.mockwebserver.MockResponse |
||||
import okhttp3.mockwebserver.MockWebServer |
||||
import org.junit.After |
||||
import org.junit.Assert.assertEquals |
||||
import org.junit.Assert.assertNull |
||||
import org.junit.Before |
||||
import org.junit.Test |
||||
import org.mockito.kotlin.doAnswer |
||||
import org.mockito.kotlin.mock |
||||
|
||||
class InstanceSwitchAuthInterceptorTest { |
||||
|
||||
private val mockWebServer = MockWebServer() |
||||
|
||||
@Before |
||||
fun setup() { |
||||
mockWebServer.start() |
||||
} |
||||
|
||||
@After |
||||
fun teardown() { |
||||
mockWebServer.shutdown() |
||||
} |
||||
|
||||
@Test |
||||
fun `should make regular request when requested`() { |
||||
mockWebServer.enqueue(MockResponse()) |
||||
|
||||
val accountManager: AccountManager = mock { |
||||
on { activeAccount } doAnswer { null } |
||||
} |
||||
|
||||
val okHttpClient = OkHttpClient.Builder() |
||||
.addInterceptor(InstanceSwitchAuthInterceptor(accountManager)) |
||||
.build() |
||||
|
||||
val request = Request.Builder() |
||||
.get() |
||||
.url(mockWebServer.url("/test")) |
||||
.build() |
||||
|
||||
val response = okHttpClient.newCall(request).execute() |
||||
|
||||
assertEquals(200, response.code) |
||||
} |
||||
|
||||
@Test |
||||
fun `should make request to instance requested in special header`() { |
||||
mockWebServer.enqueue(MockResponse()) |
||||
|
||||
val accountManager: AccountManager = mock { |
||||
on { activeAccount } doAnswer { |
||||
AccountEntity( |
||||
id = 1, |
||||
domain = "test.domain", |
||||
accessToken = "fakeToken", |
||||
clientId = "fakeId", |
||||
clientSecret = "fakeSecret", |
||||
isActive = true |
||||
) |
||||
} |
||||
} |
||||
|
||||
val okHttpClient = OkHttpClient.Builder() |
||||
.addInterceptor(InstanceSwitchAuthInterceptor(accountManager)) |
||||
.build() |
||||
|
||||
val request = Request.Builder() |
||||
.get() |
||||
.url("http://" + MastodonApi.PLACEHOLDER_DOMAIN + ":" + mockWebServer.port + "/test") |
||||
.header(MastodonApi.DOMAIN_HEADER, mockWebServer.hostName) |
||||
.build() |
||||
|
||||
val response = okHttpClient.newCall(request).execute() |
||||
|
||||
assertEquals(200, response.code) |
||||
|
||||
assertNull(mockWebServer.takeRequest().getHeader("Authorization")) |
||||
} |
||||
|
||||
@Test |
||||
fun `should make request to current instance when requested and user is logged in`() { |
||||
mockWebServer.enqueue(MockResponse()) |
||||
|
||||
val accountManager: AccountManager = mock { |
||||
on { activeAccount } doAnswer { |
||||
AccountEntity( |
||||
id = 1, |
||||
domain = mockWebServer.hostName, |
||||
accessToken = "fakeToken", |
||||
clientId = "fakeId", |
||||
clientSecret = "fakeSecret", |
||||
isActive = true |
||||
) |
||||
} |
||||
} |
||||
|
||||
val okHttpClient = OkHttpClient.Builder() |
||||
.addInterceptor(InstanceSwitchAuthInterceptor(accountManager)) |
||||
.build() |
||||
|
||||
val request = Request.Builder() |
||||
.get() |
||||
.url("http://" + MastodonApi.PLACEHOLDER_DOMAIN + ":" + mockWebServer.port + "/test") |
||||
.build() |
||||
|
||||
val response = okHttpClient.newCall(request).execute() |
||||
|
||||
assertEquals(200, response.code) |
||||
|
||||
assertEquals("Bearer fakeToken", mockWebServer.takeRequest().getHeader("Authorization")) |
||||
} |
||||
|
||||
@Test |
||||
fun `should fail to make request when request to current instance is requested but no user is logged in`() { |
||||
mockWebServer.enqueue(MockResponse()) |
||||
|
||||
val accountManager: AccountManager = mock { |
||||
on { activeAccount } doAnswer { null } |
||||
} |
||||
|
||||
val okHttpClient = OkHttpClient.Builder() |
||||
.addInterceptor(InstanceSwitchAuthInterceptor(accountManager)) |
||||
.build() |
||||
|
||||
val request = Request.Builder() |
||||
.get() |
||||
.url("http://" + MastodonApi.PLACEHOLDER_DOMAIN + "/test") |
||||
.build() |
||||
|
||||
val response = okHttpClient.newCall(request).execute() |
||||
|
||||
assertEquals(400, response.code) |
||||
assertEquals(0, mockWebServer.requestCount) |
||||
} |
||||
} |
||||
@ -0,0 +1,126 @@
|
||||
package com.keylesspalace.tusky.network |
||||
|
||||
import at.connyduck.calladapter.networkresult.NetworkResultCallAdapterFactory |
||||
import com.keylesspalace.tusky.db.entity.AccountEntity |
||||
import com.keylesspalace.tusky.entity.Instance |
||||
import com.squareup.moshi.Moshi |
||||
import kotlinx.coroutines.test.runTest |
||||
import okhttp3.OkHttpClient |
||||
import okhttp3.mockwebserver.MockResponse |
||||
import okhttp3.mockwebserver.MockWebServer |
||||
import org.junit.After |
||||
import org.junit.Assert.assertEquals |
||||
import org.junit.Assert.assertNull |
||||
import org.junit.Assert.assertTrue |
||||
import org.junit.Before |
||||
import org.junit.Test |
||||
import retrofit2.Retrofit |
||||
import retrofit2.converter.moshi.MoshiConverterFactory |
||||
|
||||
class RetrofitApiTest { |
||||
|
||||
private val mockWebServer = MockWebServer() |
||||
private val okHttpClient = OkHttpClient.Builder().build() |
||||
private val moshi = Moshi.Builder().build() |
||||
|
||||
@Before |
||||
fun setup() { |
||||
mockWebServer.start() |
||||
} |
||||
|
||||
@After |
||||
fun teardown() { |
||||
mockWebServer.shutdown() |
||||
} |
||||
|
||||
private fun retrofit() = Retrofit.Builder() |
||||
.baseUrl("http://${MastodonApi.PLACEHOLDER_DOMAIN}:${mockWebServer.port}") |
||||
.client(okHttpClient) |
||||
.addConverterFactory(MoshiConverterFactory.create(moshi)) |
||||
.addCallAdapterFactory(NetworkResultCallAdapterFactory.create()) |
||||
.build() |
||||
|
||||
@Test |
||||
fun `should make request to the active account's instance`() = runTest { |
||||
mockInstanceResponse() |
||||
|
||||
val account = AccountEntity( |
||||
id = 1, |
||||
domain = mockWebServer.hostName, |
||||
accessToken = "fakeToken", |
||||
clientId = "fakeId", |
||||
clientSecret = "fakeSecret", |
||||
isActive = true |
||||
) |
||||
|
||||
val retrofit = retrofit() |
||||
val api: MastodonApi = apiForAccount(account, okHttpClient, retrofit, "http://", mockWebServer.port) |
||||
|
||||
val instanceResponse = api.getInstance() |
||||
|
||||
assertTrue(instanceResponse.isSuccess) |
||||
assertEquals("Bearer fakeToken", mockWebServer.takeRequest().getHeader("Authorization")) |
||||
} |
||||
|
||||
@Test |
||||
fun `should make request to instance requested in special header when account active`() = runTest { |
||||
mockInstanceResponse() |
||||
|
||||
val account = AccountEntity( |
||||
id = 1, |
||||
domain = "test.domain", |
||||
accessToken = "fakeToken", |
||||
clientId = "fakeId", |
||||
clientSecret = "fakeSecret", |
||||
isActive = true |
||||
) |
||||
|
||||
val retrofit = retrofit() |
||||
val api: MastodonApi = apiForAccount(account, okHttpClient, retrofit, "http://", mockWebServer.port) |
||||
|
||||
val instanceResponse = api.getInstance(domain = mockWebServer.hostName) |
||||
|
||||
assertTrue(instanceResponse.isSuccess) |
||||
assertNull(mockWebServer.takeRequest().getHeader("Authorization")) |
||||
} |
||||
|
||||
@Test |
||||
fun `should make request to instance requested in special header when no account active`() = runTest { |
||||
mockInstanceResponse() |
||||
|
||||
val retrofit = retrofit() |
||||
val api: MastodonApi = apiForAccount(null, okHttpClient, retrofit, "http://", mockWebServer.port) |
||||
|
||||
val instanceResponse = api.getInstance(domain = mockWebServer.hostName) |
||||
|
||||
assertTrue(instanceResponse.isSuccess) |
||||
assertNull(mockWebServer.takeRequest().getHeader("Authorization")) |
||||
} |
||||
|
||||
@Test |
||||
fun `should fail when current instance is requested but no user is logged in`() = runTest { |
||||
mockInstanceResponse() |
||||
|
||||
val retrofit = retrofit() |
||||
val api: MastodonApi = apiForAccount(null, okHttpClient, retrofit, "http://", mockWebServer.port) |
||||
|
||||
val instanceResponse = api.getInstance() |
||||
|
||||
assertTrue(instanceResponse.isFailure) |
||||
assertEquals(0, mockWebServer.requestCount) |
||||
} |
||||
|
||||
private fun mockInstanceResponse() { |
||||
mockWebServer.enqueue( |
||||
MockResponse() |
||||
.setBody( |
||||
moshi.adapter(Instance::class.java).toJson( |
||||
Instance( |
||||
domain = "example.org", |
||||
version = "1.0.0" |
||||
) |
||||
) |
||||
) |
||||
) |
||||
} |
||||
} |
||||
Loading…
Reference in new issue