Create CsrfTokenClient and LoginClient by injection, along with a little cleanup (commons-app#5491)

psh · web-flow · commit 84ffffbbe7fc · 2024-01-29T12:36:57.000+09:00
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
@@ -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 void performLogin() {
 
     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<MwQueryResponse>() {
                     @Override
                     public void onResponse(Call<MwQueryResponse> call,
                                            Response<MwQueryResponse> 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
@@ -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,31 +17,29 @@ 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<MwQueryResponse?>? = 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()
 
         for (retry in 0 until MAX_RETRIES_OF_LOGIN_BLOCKING) {
             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
@@ -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<MwQueryResponse?>? = null
     private var loginCall: Call<LoginResponse?>? = 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<MwQueryResponse?> {
             override fun onResponse(call: Call<MwQueryResponse?>, response: Response<MwQueryResponse?>) {
-                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<MwQueryResponse?>, 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<LoginResponse?>,
                 response: Response<LoginResponse?>
             ) {
-                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
@@ -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,23 +26,23 @@ 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)
                     }
                 }
             }
         } else if ("PASS" != status && "FAIL" != status) {
             //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
@@ -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
@@ -11,6 +11,8 @@
 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.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 OkHttpJsonApiClient provideOkHttpJsonApiClient(OkHttpClient okHttpClient,
     @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

Original file line number	Diff line number	Diff line change
`@@ -4,7 +4,6 @@ import com.google.gson.annotations.SerializedName`
`4`	`4`	`import fr.free.nrw.commons.auth.login.LoginResult.OAuthResult`
`5`	`5`	`import fr.free.nrw.commons.auth.login.LoginResult.ResetPasswordResult`
`6`	`6`	`import fr.free.nrw.commons.auth.login.LoginResult.Result`
`7`		`-import org.wikipedia.dataclient.WikiSite`
`8`	`7`	`import org.wikipedia.dataclient.mwapi.MwServiceError`
`9`	`8`
`10`	`9`	`class LoginResponse {`
`@@ -14,8 +13,8 @@ class LoginResponse {`
`14`	`13`	`@SerializedName("clientlogin")`
`15`	`14`	`private val clientLogin: ClientLogin? = null`
`16`	`15`
`17`		`- fun toLoginResult(site: WikiSite, password: String): LoginResult? {`
`18`		`- return clientLogin?.toLoginResult(site, password)`
	`16`	`+ fun toLoginResult(password: String): LoginResult? {`
	`17`	`+ return clientLogin?.toLoginResult(password)`
`19`	`18`	`}`
`20`	`19`	`}`
`21`	`20`
`@@ -27,23 +26,23 @@ internal class ClientLogin {`
`27`	`26`	`@SerializedName("username")`
`28`	`27`	`private val userName: String? = null`
`29`	`28`
`30`		`- fun toLoginResult(site: WikiSite, password: String): LoginResult {`
	`29`	`+ fun toLoginResult(password: String): LoginResult {`
`31`	`30`	`var userMessage = message`
`32`	`31`	`if ("UI" == status) {`
`33`	`32`	`if (requests != null) {`
`34`	`33`	`for (req in requests) {`
`35`	`34`	`if ("MediaWiki\\Extension\\OATHAuth\\Auth\\TOTPAuthenticationRequest" == req.id()) {`
`36`		`- return OAuthResult(site, status, userName, password, message)`
	`35`	`+ return OAuthResult(status, userName, password, message)`
`37`	`36`	`} else if ("MediaWiki\\Auth\\PasswordAuthenticationRequest" == req.id()) {`
`38`		`- return ResetPasswordResult(site, status, userName, password, message)`
	`37`	`+ return ResetPasswordResult(status, userName, password, message)`
`39`	`38`	`}`
`40`	`39`	`}`
`41`	`40`	`}`
`42`	`41`	`} else if ("PASS" != status && "FAIL" != status) {`
`43`	`42`	`//TODO: String resource -- Looks like needed for others in this class too`
`44`	`43`	`userMessage = "An unknown error occurred."`
`45`	`44`	`}`
`46`		`- return Result(site, status ?: "", userName, password, userMessage)`
	`45`	`+ return Result(status ?: "", userName, password, userMessage)`
`47`	`46`	`}`
`48`	`47`	`}`
`49`	`48`