From 69cd9c94d190fa156b3ede51eb931932f701dc2b Mon Sep 17 00:00:00 2001 From: Kaartic Sivaraam Date: Wed, 19 Jun 2024 02:01:34 +0530 Subject: [PATCH] Make sure to clear cookies on logout (#5727) * Ensure to clear the cookies when logging out It turns out that we failed to clear the cookies from the cookie JAR when logging the user out. As a consequence, the cookie were retained and it was possible to edit depictions as the previous user even without logging in to the app (using the retained cookies). Make sure we properly clear the cookies when we log the user out. As an aside, the fact that the edit button shouldn't have been shown is a different issue being tracked in #5726 * session: reuse removeAccount method for log out The removeAccount method takes care of invoking the non-deprecated API in applicable API levels. The logout method did not do such a thing. Avoid redundancy, and reuse the removeAccount method for logging out. --- .../free/nrw/commons/CommonsApplication.java | 1 + .../free/nrw/commons/auth/SessionManager.java | 20 +++++++++---------- .../nrw/commons/auth/csrf/CsrfTokenClient.kt | 1 - .../wikidata/cookies/CommonsCookieJar.kt | 5 +++++ 4 files changed, 16 insertions(+), 11 deletions(-) diff --git a/app/src/main/java/fr/free/nrw/commons/CommonsApplication.java b/app/src/main/java/fr/free/nrw/commons/CommonsApplication.java index 93413213d..09e34100c 100644 --- a/app/src/main/java/fr/free/nrw/commons/CommonsApplication.java +++ b/app/src/main/java/fr/free/nrw/commons/CommonsApplication.java @@ -294,6 +294,7 @@ public class CommonsApplication extends MultiDexApplication { } sessionManager.logout() + .andThen(Completable.fromAction(() -> cookieJar.clear())) .andThen(Completable.fromAction(() -> { Timber.d("All accounts have been removed"); clearImageCache(); diff --git a/app/src/main/java/fr/free/nrw/commons/auth/SessionManager.java b/app/src/main/java/fr/free/nrw/commons/auth/SessionManager.java index f5395ceda..7c2f4a334 100644 --- a/app/src/main/java/fr/free/nrw/commons/auth/SessionManager.java +++ b/app/src/main/java/fr/free/nrw/commons/auth/SessionManager.java @@ -122,18 +122,18 @@ public class SessionManager { } /** - * 1. Clears existing accounts from account manager - * 2. Calls MediaWikiApi's logout function to clear cookies - * @return + * Returns a Completable that clears existing accounts from account manager */ public Completable logout() { - AccountManager accountManager = AccountManager.get(context); - Account[] allAccounts = accountManager.getAccountsByType(BuildConfig.ACCOUNT_TYPE); - return Completable.fromObservable(Observable.fromArray(allAccounts) - .map(a -> accountManager.removeAccount(a, null, null).getResult())) - .doOnComplete(() -> { - currentAccount = null; - }); + return Completable.fromObservable( + Observable.empty() + .doOnComplete( + () -> { + removeAccount(); + currentAccount = null; + } + ) + ); } /** 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 9e3136237..88cc6b953 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 @@ -10,7 +10,6 @@ import fr.free.nrw.commons.auth.login.LoginResult import retrofit2.Call import retrofit2.Response import timber.log.Timber -import java.io.IOException import java.util.concurrent.Callable import java.util.concurrent.Executors.newSingleThreadExecutor diff --git a/app/src/main/java/fr/free/nrw/commons/wikidata/cookies/CommonsCookieJar.kt b/app/src/main/java/fr/free/nrw/commons/wikidata/cookies/CommonsCookieJar.kt index 34b38ab80..fbc88f55a 100644 --- a/app/src/main/java/fr/free/nrw/commons/wikidata/cookies/CommonsCookieJar.kt +++ b/app/src/main/java/fr/free/nrw/commons/wikidata/cookies/CommonsCookieJar.kt @@ -95,4 +95,9 @@ class CommonsCookieJar(private val cookieStorage: CommonsCookieStorage) : Cookie private fun Cookie.domainSpec(url: HttpUrl): String = domain.ifEmpty { url.toUri().getAuthority() } + + fun clear() { + cookieStorage.clear() + } + }