From 84ffffbbe7fc473ecfa29274dc27703bb40cf409 Mon Sep 17 00:00:00 2001 From: Paul Hawke Date: Sun, 28 Jan 2024 21:36:57 -0600 Subject: [PATCH] Create CsrfTokenClient and LoginClient by injection, along with a little cleanup (#5491) --- .../free/nrw/commons/auth/LoginActivity.java | 8 +-- .../nrw/commons/auth/csrf/CsrfTokenClient.kt | 19 ++--- .../nrw/commons/auth/login/LoginClient.kt | 69 +++++++++---------- .../nrw/commons/auth/login/LoginResponse.kt | 13 ++-- .../nrw/commons/auth/login/LoginResult.kt | 12 +--- .../free/nrw/commons/di/NetworkingModule.java | 25 +++++-- .../commons/auth/csrf/CsrfTokenClientTest.kt | 8 +-- 7 files changed, 73 insertions(+), 81 deletions(-) diff --git a/app/src/main/java/fr/free/nrw/commons/auth/LoginActivity.java b/app/src/main/java/fr/free/nrw/commons/auth/LoginActivity.java index 86e0e033c..bf7183de4 100644 --- a/app/src/main/java/fr/free/nrw/commons/auth/LoginActivity.java +++ b/app/src/main/java/fr/free/nrw/commons/auth/LoginActivity.java @@ -64,10 +64,6 @@ public class LoginActivity extends AccountAuthenticatorActivity { @Inject SessionManager sessionManager; - @Inject - @Named(NAMED_COMMONS_WIKI_SITE) - WikiSite commonsWikiSite; - @Inject @Named("default_preferences") JsonKvStore applicationKvStore; @@ -231,13 +227,13 @@ public class LoginActivity extends AccountAuthenticatorActivity { private void doLogin(String username, String password, String twoFactorCode) { progressDialog.show(); - loginToken = ServiceFactory.get(commonsWikiSite, LoginInterface.class).getLoginToken(); + loginToken = loginClient.getLoginToken(); loginToken.enqueue( new Callback() { @Override public void onResponse(Call call, Response response) { - loginClient.login(commonsWikiSite, username, password, null, twoFactorCode, + loginClient.login(username, password, null, twoFactorCode, response.body().query().loginToken(), Locale.getDefault().getLanguage(), new LoginCallback() { @Override public void success(@NonNull LoginResult result) { diff --git a/app/src/main/java/fr/free/nrw/commons/auth/csrf/CsrfTokenClient.kt b/app/src/main/java/fr/free/nrw/commons/auth/csrf/CsrfTokenClient.kt index 53e7ded0b..a46095441 100644 --- a/app/src/main/java/fr/free/nrw/commons/auth/csrf/CsrfTokenClient.kt +++ b/app/src/main/java/fr/free/nrw/commons/auth/csrf/CsrfTokenClient.kt @@ -3,9 +3,7 @@ package fr.free.nrw.commons.auth.csrf import androidx.annotation.VisibleForTesting import fr.free.nrw.commons.auth.SessionManager import org.wikipedia.AppAdapter -import org.wikipedia.dataclient.ServiceFactory import org.wikipedia.dataclient.SharedPreferenceCookieManager -import org.wikipedia.dataclient.WikiSite import org.wikipedia.dataclient.mwapi.MwQueryResponse import fr.free.nrw.commons.auth.login.LoginClient import fr.free.nrw.commons.auth.login.LoginCallback @@ -19,17 +17,16 @@ import java.util.concurrent.Callable import java.util.concurrent.Executors.newSingleThreadExecutor class CsrfTokenClient( - private val csrfWikiSite: WikiSite, - private val sessionManager: SessionManager + private val sessionManager: SessionManager, + private val csrfTokenInterface: CsrfTokenInterface, + private val loginClient: LoginClient ) { private var retries = 0 private var csrfTokenCall: Call? = null - private val loginClient = LoginClient() @Throws(Throwable::class) fun getTokenBlocking(): String { var token = "" - val service = ServiceFactory.get(csrfWikiSite, CsrfTokenInterface::class.java) val userName = AppAdapter.get().getUserName() val password = AppAdapter.get().getPassword() @@ -37,13 +34,12 @@ class CsrfTokenClient( try { if (retry > 0) { // Log in explicitly - LoginClient() - .loginBlocking(csrfWikiSite, userName, password, "") + loginClient.loginBlocking(userName, password, "") } // Get CSRFToken response off the main thread. val response = newSingleThreadExecutor().submit(Callable { - service.getCsrfTokenCall().execute() + csrfTokenInterface.getCsrfTokenCall().execute() }).get() if (response.body()?.query()?.csrfToken().isNullOrEmpty()) { @@ -114,7 +110,7 @@ class CsrfTokenClient( login(userName, password, callback) { Timber.i("retrying...") cancel() - csrfTokenCall = request(ServiceFactory.get(csrfWikiSite, CsrfTokenInterface::class.java), callback) + csrfTokenCall = request(csrfTokenInterface, callback) } } else { callback.failure(caught()) @@ -126,8 +122,7 @@ class CsrfTokenClient( password: String, callback: Callback, retryCallback: () -> Unit - ) = LoginClient() - .request(csrfWikiSite, username, password, object : LoginCallback { + ) = loginClient.request(username, password, object : LoginCallback { override fun success(loginResult: LoginResult) { if (loginResult.pass) { sessionManager.updateAccount(loginResult) diff --git a/app/src/main/java/fr/free/nrw/commons/auth/login/LoginClient.kt b/app/src/main/java/fr/free/nrw/commons/auth/login/LoginClient.kt index 978215935..b87d9d7ba 100644 --- a/app/src/main/java/fr/free/nrw/commons/auth/login/LoginClient.kt +++ b/app/src/main/java/fr/free/nrw/commons/auth/login/LoginClient.kt @@ -6,8 +6,6 @@ import fr.free.nrw.commons.auth.login.LoginResult.ResetPasswordResult import fr.free.nrw.commons.wikidata.WikidataConstants.WIKIPEDIA_URL import io.reactivex.android.schedulers.AndroidSchedulers import io.reactivex.schedulers.Schedulers -import org.wikipedia.dataclient.ServiceFactory -import org.wikipedia.dataclient.WikiSite import org.wikipedia.dataclient.mwapi.MwQueryResponse import retrofit2.Call import retrofit2.Callback @@ -18,7 +16,7 @@ import java.io.IOException /** * Responsible for making login related requests to the server. */ -class LoginClient { +class LoginClient(private val loginInterface: LoginInterface) { private var tokenCall: Call? = null private var loginCall: Call? = null @@ -30,14 +28,18 @@ class LoginClient { */ private var userLanguage = "" - fun request(wiki: WikiSite, userName: String, password: String, cb: LoginCallback) { + fun getLoginToken() = loginInterface.getLoginToken() + + fun request(userName: String, password: String, cb: LoginCallback) { cancel() - tokenCall = ServiceFactory.get(wiki, LoginInterface::class.java).getLoginToken() + tokenCall = getLoginToken() tokenCall!!.enqueue(object : Callback { override fun onResponse(call: Call, response: Response) { - login(wiki, userName, password, null, null, - response.body()!!.query()!!.loginToken(), userLanguage, cb) + login( + userName, password, null, null, response.body()!!.query()!!.loginToken(), + userLanguage, cb + ) } override fun onFailure(call: Call, caught: Throwable) { @@ -50,16 +52,15 @@ class LoginClient { } fun login( - wiki: WikiSite, userName: String, password: String, retypedPassword: String?, - twoFactorCode: String?, loginToken: String?, userLanguage: String, cb: LoginCallback + userName: String, password: String, retypedPassword: String?, twoFactorCode: String?, + loginToken: String?, userLanguage: String, cb: LoginCallback ) { this.userLanguage = userLanguage loginCall = if (twoFactorCode.isNullOrEmpty() && retypedPassword.isNullOrEmpty()) { - ServiceFactory.get(wiki, LoginInterface::class.java) - .postLogIn(userName, password, loginToken, userLanguage, WIKIPEDIA_URL) + loginInterface.postLogIn(userName, password, loginToken, userLanguage, WIKIPEDIA_URL) } else { - ServiceFactory.get(wiki, LoginInterface::class.java).postLogIn( + loginInterface.postLogIn( userName, password, retypedPassword, twoFactorCode, loginToken, userLanguage, true ) } @@ -69,12 +70,12 @@ class LoginClient { call: Call, response: Response ) { - val loginResult = response.body()?.toLoginResult(wiki, password) + val loginResult = response.body()?.toLoginResult(password) if (loginResult != null) { if (loginResult.pass && !loginResult.userName.isNullOrEmpty()) { // The server could do some transformations on user names, e.g. on some // wikis is uppercases the first letter. - getExtendedInfo(wiki, loginResult.userName, loginResult, cb) + getExtendedInfo(loginResult.userName, loginResult, cb) } else if ("UI" == loginResult.status) { when (loginResult) { is OAuthResult -> cb.twoFactorPrompt( @@ -106,25 +107,24 @@ class LoginClient { } @Throws(Throwable::class) - fun loginBlocking(wiki: WikiSite, userName: String, password: String, twoFactorCode: String?) { - val tokenResponse = ServiceFactory.get(wiki, LoginInterface::class.java).getLoginToken().execute() + fun loginBlocking(userName: String, password: String, twoFactorCode: String?) { + val tokenResponse = getLoginToken().execute() if (tokenResponse.body()?.query()?.loginToken().isNullOrEmpty()) { throw IOException("Unexpected response when getting login token.") } val loginToken = tokenResponse.body()?.query()?.loginToken() val tempLoginCall = if (twoFactorCode.isNullOrEmpty()) { - ServiceFactory.get(wiki, LoginInterface::class.java).postLogIn( - userName, password, loginToken, userLanguage, WIKIPEDIA_URL) + loginInterface.postLogIn(userName, password, loginToken, userLanguage, WIKIPEDIA_URL) } else { - ServiceFactory.get(wiki, LoginInterface::class.java).postLogIn( + loginInterface.postLogIn( userName, password, null, twoFactorCode, loginToken, userLanguage, true ) } val response = tempLoginCall.execute() val loginResponse = response.body() ?: throw IOException("Unexpected response when logging in.") - val loginResult = loginResponse.toLoginResult(wiki, password) ?: throw IOException("Unexpected response when logging in.") + val loginResult = loginResponse.toLoginResult(password) ?: throw IOException("Unexpected response when logging in.") if ("UI" == loginResult.status) { if (loginResult is OAuthResult) { @@ -139,23 +139,18 @@ class LoginClient { } } - private fun getExtendedInfo( - wiki: WikiSite, userName: String, loginResult: LoginResult, cb: LoginCallback - ) = ServiceFactory.get(wiki, LoginInterface::class.java).getUserInfo(userName) - .subscribeOn(Schedulers.io()).observeOn(AndroidSchedulers.mainThread()) - .subscribe({ response: MwQueryResponse? -> - loginResult.userId = response?.query()?.userInfo()?.id() ?: 0 - loginResult.groups = response?.query()?.getUserResponse(userName)?.groups ?: emptySet() - cb.success(loginResult) - Timber.v( - "Found user ID %s for %s", - response?.query()?.userInfo()?.id(), - wiki.subdomain() - ) - }, { caught: Throwable -> - Timber.e(caught, "Login succeeded but getting group information failed. ") - cb.error(caught) - }) + private fun getExtendedInfo(userName: String, loginResult: LoginResult, cb: LoginCallback) = + loginInterface.getUserInfo(userName) + .subscribeOn(Schedulers.io()).observeOn(AndroidSchedulers.mainThread()) + .subscribe({ response: MwQueryResponse? -> + loginResult.userId = response?.query()?.userInfo()?.id() ?: 0 + loginResult.groups = + response?.query()?.getUserResponse(userName)?.groups ?: emptySet() + cb.success(loginResult) + }, { caught: Throwable -> + Timber.e(caught, "Login succeeded but getting group information failed. ") + cb.error(caught) + }) fun cancel() { tokenCall?.let { diff --git a/app/src/main/java/fr/free/nrw/commons/auth/login/LoginResponse.kt b/app/src/main/java/fr/free/nrw/commons/auth/login/LoginResponse.kt index 01bf1160c..fc4d9b206 100644 --- a/app/src/main/java/fr/free/nrw/commons/auth/login/LoginResponse.kt +++ b/app/src/main/java/fr/free/nrw/commons/auth/login/LoginResponse.kt @@ -4,7 +4,6 @@ import com.google.gson.annotations.SerializedName import fr.free.nrw.commons.auth.login.LoginResult.OAuthResult import fr.free.nrw.commons.auth.login.LoginResult.ResetPasswordResult import fr.free.nrw.commons.auth.login.LoginResult.Result -import org.wikipedia.dataclient.WikiSite import org.wikipedia.dataclient.mwapi.MwServiceError class LoginResponse { @@ -14,8 +13,8 @@ class LoginResponse { @SerializedName("clientlogin") private val clientLogin: ClientLogin? = null - fun toLoginResult(site: WikiSite, password: String): LoginResult? { - return clientLogin?.toLoginResult(site, password) + fun toLoginResult(password: String): LoginResult? { + return clientLogin?.toLoginResult(password) } } @@ -27,15 +26,15 @@ internal class ClientLogin { @SerializedName("username") private val userName: String? = null - fun toLoginResult(site: WikiSite, password: String): LoginResult { + fun toLoginResult(password: String): LoginResult { var userMessage = message if ("UI" == status) { if (requests != null) { for (req in requests) { if ("MediaWiki\\Extension\\OATHAuth\\Auth\\TOTPAuthenticationRequest" == req.id()) { - return OAuthResult(site, status, userName, password, message) + return OAuthResult(status, userName, password, message) } else if ("MediaWiki\\Auth\\PasswordAuthenticationRequest" == req.id()) { - return ResetPasswordResult(site, status, userName, password, message) + return ResetPasswordResult(status, userName, password, message) } } } @@ -43,7 +42,7 @@ internal class ClientLogin { //TODO: String resource -- Looks like needed for others in this class too userMessage = "An unknown error occurred." } - return Result(site, status ?: "", userName, password, userMessage) + return Result(status ?: "", userName, password, userMessage) } } diff --git a/app/src/main/java/fr/free/nrw/commons/auth/login/LoginResult.kt b/app/src/main/java/fr/free/nrw/commons/auth/login/LoginResult.kt index f0bbb2107..69e4a7f89 100644 --- a/app/src/main/java/fr/free/nrw/commons/auth/login/LoginResult.kt +++ b/app/src/main/java/fr/free/nrw/commons/auth/login/LoginResult.kt @@ -1,9 +1,6 @@ package fr.free.nrw.commons.auth.login -import org.wikipedia.dataclient.WikiSite - sealed class LoginResult( - val site: WikiSite, val status: String, val userName: String?, val password: String?, @@ -14,26 +11,23 @@ sealed class LoginResult( val pass: Boolean get() = "PASS" == status class Result( - site: WikiSite, status: String, userName: String?, password: String?, message: String? - ): LoginResult(site, status, userName, password, message) + ): LoginResult(status, userName, password, message) class OAuthResult( - site: WikiSite, status: String, userName: String?, password: String?, message: String? - ) : LoginResult(site, status, userName, password, message) + ) : LoginResult(status, userName, password, message) class ResetPasswordResult( - site: WikiSite, status: String, userName: String?, password: String?, message: String? - ) : LoginResult(site, status, userName, password, message) + ) : LoginResult(status, userName, password, message) } diff --git a/app/src/main/java/fr/free/nrw/commons/di/NetworkingModule.java b/app/src/main/java/fr/free/nrw/commons/di/NetworkingModule.java index e2425c6b1..0828eaf88 100644 --- a/app/src/main/java/fr/free/nrw/commons/di/NetworkingModule.java +++ b/app/src/main/java/fr/free/nrw/commons/di/NetworkingModule.java @@ -11,6 +11,8 @@ import fr.free.nrw.commons.actions.PageEditClient; import fr.free.nrw.commons.actions.PageEditInterface; import fr.free.nrw.commons.actions.ThanksInterface; import fr.free.nrw.commons.auth.SessionManager; +import fr.free.nrw.commons.auth.csrf.CsrfTokenInterface; +import fr.free.nrw.commons.auth.login.LoginInterface; import fr.free.nrw.commons.category.CategoryInterface; import fr.free.nrw.commons.explore.depictions.DepictsClient; import fr.free.nrw.commons.kvstore.JsonKvStore; @@ -37,7 +39,6 @@ import okhttp3.OkHttpClient; import okhttp3.logging.HttpLoggingInterceptor; import okhttp3.logging.HttpLoggingInterceptor.Level; import fr.free.nrw.commons.auth.csrf.CsrfTokenClient; -import org.wikipedia.dataclient.Service; import org.wikipedia.dataclient.ServiceFactory; import org.wikipedia.dataclient.WikiSite; import org.wikipedia.json.GsonUtil; @@ -106,15 +107,27 @@ public class NetworkingModule { @Named(NAMED_COMMONS_CSRF) @Provides @Singleton - public CsrfTokenClient provideCommonsCsrfTokenClient( - @Named(NAMED_COMMONS_WIKI_SITE) WikiSite commonsWikiSite, SessionManager sessionManager) { - return new CsrfTokenClient(commonsWikiSite, sessionManager); + public CsrfTokenClient provideCommonsCsrfTokenClient(SessionManager sessionManager, + CsrfTokenInterface tokenInterface, LoginClient loginClient) { + return new CsrfTokenClient(sessionManager, tokenInterface, loginClient); } @Provides @Singleton - public LoginClient provideLoginClient() { - return new LoginClient(); + public CsrfTokenInterface provideCsrfTokenInterface(@Named(NAMED_COMMONS_WIKI_SITE) WikiSite commonsWikiSite) { + return ServiceFactory.get(commonsWikiSite, BuildConfig.COMMONS_URL, CsrfTokenInterface.class); + } + + @Provides + @Singleton + public LoginInterface provideLoginInterface(@Named(NAMED_COMMONS_WIKI_SITE) WikiSite commonsWikiSite) { + return ServiceFactory.get(commonsWikiSite, BuildConfig.COMMONS_URL, LoginInterface.class); + } + + @Provides + @Singleton + public LoginClient provideLoginClient(LoginInterface loginInterface) { + return new LoginClient(loginInterface); } @Provides diff --git a/app/src/test/kotlin/fr/free/nrw/commons/auth/csrf/CsrfTokenClientTest.kt b/app/src/test/kotlin/fr/free/nrw/commons/auth/csrf/CsrfTokenClientTest.kt index ad434649a..e2f4a7773 100644 --- a/app/src/test/kotlin/fr/free/nrw/commons/auth/csrf/CsrfTokenClientTest.kt +++ b/app/src/test/kotlin/fr/free/nrw/commons/auth/csrf/CsrfTokenClientTest.kt @@ -3,6 +3,7 @@ package fr.free.nrw.commons.auth.csrf import com.google.gson.stream.MalformedJsonException import fr.free.nrw.commons.MockWebServerTest import fr.free.nrw.commons.auth.SessionManager +import fr.free.nrw.commons.auth.login.LoginClient import org.junit.Test import org.mockito.ArgumentMatchers.any import org.mockito.ArgumentMatchers.eq @@ -10,16 +11,15 @@ import org.mockito.ArgumentMatchers.isA import org.mockito.Mockito.mock import org.mockito.Mockito.never import org.mockito.Mockito.verify -import org.wikipedia.dataclient.Service -import org.wikipedia.dataclient.WikiSite import org.wikipedia.dataclient.mwapi.MwException import org.wikipedia.dataclient.okhttp.HttpStatusException class CsrfTokenClientTest : MockWebServerTest() { - private val wikiSite = WikiSite("test.wikipedia.org") private val cb = mock(CsrfTokenClient.Callback::class.java) private val sessionManager = mock(SessionManager::class.java) - private val subject = CsrfTokenClient(wikiSite, sessionManager) + private val tokenInterface = mock(CsrfTokenInterface::class.java) + private val loginClient = mock(LoginClient::class.java) + private val subject = CsrfTokenClient(sessionManager, tokenInterface, loginClient) @Test @Throws(Throwable::class)