Skip to content

Fix auth issues in the app #3064

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -12,48 +12,19 @@
import io.reactivex.android.schedulers.AndroidSchedulers;
import io.reactivex.schedulers.Schedulers;

import static fr.free.nrw.commons.auth.AccountUtil.AUTH_COOKIE;

public abstract class AuthenticatedActivity extends NavigationBaseActivity {

@Inject
protected SessionManager sessionManager;
@Inject
MediaWikiApi mediaWikiApi;
private String authCookie;

protected void requestAuthToken() {
if (authCookie != null) {
onAuthCookieAcquired(authCookie);
return;
}
authCookie = sessionManager.getAuthCookie();
if (authCookie != null) {
onAuthCookieAcquired(authCookie);
}
}

@Override
protected void onCreate(Bundle savedInstanceState) {
super.onCreate(savedInstanceState);

if (savedInstanceState != null) {
authCookie = savedInstanceState.getString(AUTH_COOKIE);
}

showBlockStatus();
}

@Override
protected void onSaveInstanceState(Bundle outState) {
super.onSaveInstanceState(outState);
outState.putString(AUTH_COOKIE, authCookie);
}

protected abstract void onAuthCookieAcquired(String authCookie);

protected abstract void onAuthFailure();

/**
* Makes API call to check if user is blocked from Commons. If the user is blocked, a snackbar
* is created to notify the user
Expand Down
4 changes: 2 additions & 2 deletions app/src/main/java/fr/free/nrw/commons/auth/LoginActivity.java
Original file line number Diff line number Diff line change
Expand Up @@ -212,8 +212,7 @@ protected void onResume() {
}

if (sessionManager.getCurrentAccount() != null
&& sessionManager.isUserLoggedIn()
&& sessionManager.getCachedAuthCookie() != null) {
&& sessionManager.isUserLoggedIn()) {
applicationKvStore.putBoolean("login_skipped", false);
startMainActivity();
}
Expand Down Expand Up @@ -308,6 +307,7 @@ private void onLoginSuccess(String username, String password) {
// no longer attached to activity!
return;
}
sessionManager.setUserLoggedIn(true);
LoginResult loginResult = new LoginResult(commonsWikiSite, "PASS", username, password, "");
AppAdapter.get().updateAccount(loginResult);
progressDialog.dismiss();
Expand Down
26 changes: 6 additions & 20 deletions app/src/main/java/fr/free/nrw/commons/auth/SessionManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
import fr.free.nrw.commons.mwapi.MediaWikiApi;
import io.reactivex.Completable;
import io.reactivex.Observable;
import timber.log.Timber;

/**
* Manage the current logged in user session.
Expand Down Expand Up @@ -53,7 +52,7 @@ private boolean createAccount(@NonNull String userName, @NonNull String password
return true;
}

public void removeAccount() {
private void removeAccount() {
Account account = getCurrentAccount();
if (account != null) {
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP_MR1) {
Expand Down Expand Up @@ -101,7 +100,7 @@ public String getUserName() {
}

@Nullable
public String getRawUserName() {
private String getRawUserName() {
Account account = getCurrentAccount();
return account == null ? null : accountManager().getUserData(account, KEY_RAWUSERNAME);
}
Expand All @@ -121,27 +120,14 @@ private AccountManager accountManager() {
return AccountManager.get(context);
}

public String getAuthCookie() {
if (!isUserLoggedIn()) {
Timber.e("User is not logged in");
return null;
} else {
String authCookie = getCachedAuthCookie();
if (authCookie == null) {
Timber.e("Auth cookie is null even after login");
}
return authCookie;
}
}

public String getCachedAuthCookie() {
return defaultKvStore.getString("getAuthCookie", null);
}

public boolean isUserLoggedIn() {
return defaultKvStore.getBoolean("isUserLoggedIn", false);
}

void setUserLoggedIn(boolean isLoggedIn) {
defaultKvStore.putBoolean("isUserLoggedIn", isLoggedIn);
}

public void forceLogin(Context context) {
if (context != null) {
LoginActivity.startYourself(context);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,19 @@

import android.content.Context;
import android.content.Intent;
import android.database.DataSetObserver;
import android.os.Bundle;
import androidx.fragment.app.FragmentManager;
import androidx.fragment.app.FragmentTransaction;
import android.view.Menu;
import android.view.MenuInflater;
import android.view.MenuItem;
import android.view.View;
import android.widget.AdapterView;

import androidx.fragment.app.FragmentManager;
import androidx.fragment.app.FragmentTransaction;

import butterknife.ButterKnife;
import fr.free.nrw.commons.Media;
import fr.free.nrw.commons.R;
import fr.free.nrw.commons.auth.AuthenticatedActivity;
import fr.free.nrw.commons.explore.SearchActivity;
import fr.free.nrw.commons.media.MediaDetailPagerFragment;
import fr.free.nrw.commons.theme.NavigationBaseActivity;
Expand All @@ -28,7 +27,7 @@
*/

public class CategoryImagesActivity
extends AuthenticatedActivity
extends NavigationBaseActivity
implements FragmentManager.OnBackStackChangedListener,
MediaDetailPagerFragment.MediaDetailProvider,
AdapterView.OnItemClickListener{
Expand All @@ -38,16 +37,6 @@ public class CategoryImagesActivity
private CategoryImagesListFragment categoryImagesListFragment;
private MediaDetailPagerFragment mediaDetails;

@Override
protected void onAuthCookieAcquired(String authCookie) {

}

@Override
protected void onAuthFailure() {

}

/**
* This method is called on backPressed of anyFragment in the activity.
* We are changing the icon here from back to hamburger icon.
Expand All @@ -69,7 +58,6 @@ protected void onCreate(Bundle savedInstanceState) {
supportFragmentManager = getSupportFragmentManager();
setCategoryImagesFragment();
supportFragmentManager.addOnBackStackChangedListener(this);
requestAuthToken();
initDrawer();
setPageTitle();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,8 @@ public class ContributionsFragment

private ContributionsListFragment contributionsListFragment;
private MediaDetailPagerFragment mediaDetailPagerFragment;
public static final String CONTRIBUTION_LIST_FRAGMENT_TAG = "ContributionListFragmentTag";
public static final String MEDIA_DETAIL_PAGER_FRAGMENT_TAG = "MediaDetailFragmentTag";
private static final String CONTRIBUTION_LIST_FRAGMENT_TAG = "ContributionListFragmentTag";
static final String MEDIA_DETAIL_PAGER_FRAGMENT_TAG = "MediaDetailFragmentTag";

@BindView(R.id.card_view_nearby) public NearbyNotificationCardView nearbyNotificationCardView;
@BindView(R.id.campaigns_view) CampaignView campaignView;
Expand Down Expand Up @@ -257,7 +257,7 @@ public void onAttach(Context context) {
operations on first time fragment attached to an activity. Then they will be retained
until fragment life time ends.
*/
if (((MainActivity)getActivity()).isAuthCookieAcquired && !isFragmentAttachedBefore) {
if (!isFragmentAttachedBefore) {
onAuthCookieAcquired(((MainActivity)getActivity()).uploadServiceIntent);
isFragmentAttachedBefore = true;

Expand All @@ -268,7 +268,7 @@ public void onAttach(Context context) {
* Replace FrameLayout with ContributionsListFragment, user will see contributions list. Creates
* new one if null.
*/
public void showContributionsListFragment() {
private void showContributionsListFragment() {
// show tabs on contribution list is visible
((MainActivity) getActivity()).showTabs();
// show nearby card view on contributions list is visible
Expand All @@ -289,7 +289,7 @@ public void showContributionsListFragment() {
* Replace FrameLayout with MediaDetailPagerFragment, user will see details of selected media.
* Creates new one if null.
*/
public void showMediaDetailPagerFragment() {
private void showMediaDetailPagerFragment() {
// hide tabs on media detail view is visible
((MainActivity)getActivity()).hideTabs();
// hide nearby card view on media detail is visible
Expand All @@ -308,7 +308,7 @@ public void onBackStackChanged() {
* Called when onAuthCookieAcquired is called on authenticated parent activity
* @param uploadServiceIntent
*/
public void onAuthCookieAcquired(Intent uploadServiceIntent) {
void onAuthCookieAcquired(Intent uploadServiceIntent) {
// Since we call onAuthCookieAcquired method from onAttach, isAdded is still false. So don't use it

if (getActivity() != null) { // If fragment is attached to parent activity
Expand All @@ -324,7 +324,7 @@ public void onAuthCookieAcquired(Intent uploadServiceIntent) {
* mediaDetailPagerFragment, and preserve previous state in back stack.
* Called when user selects a contribution.
*/
public void showDetail(int i) {
private void showDetail(int i) {
if (mediaDetailPagerFragment == null || !mediaDetailPagerFragment.isVisible()) {
mediaDetailPagerFragment = new MediaDetailPagerFragment();
showMediaDetailPagerFragment();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
import butterknife.ButterKnife;
import fr.free.nrw.commons.BuildConfig;
import fr.free.nrw.commons.R;
import fr.free.nrw.commons.auth.AuthenticatedActivity;
import fr.free.nrw.commons.auth.SessionManager;
import fr.free.nrw.commons.location.LocationServiceManager;
import fr.free.nrw.commons.nearby.NearbyFragment;
Expand All @@ -38,14 +37,15 @@
import fr.free.nrw.commons.notification.NotificationActivity;
import fr.free.nrw.commons.notification.NotificationController;
import fr.free.nrw.commons.quiz.QuizChecker;
import fr.free.nrw.commons.theme.NavigationBaseActivity;
import fr.free.nrw.commons.upload.UploadService;
import io.reactivex.android.schedulers.AndroidSchedulers;
import io.reactivex.schedulers.Schedulers;
import timber.log.Timber;

import static android.content.ContentResolver.requestSync;

public class MainActivity extends AuthenticatedActivity implements FragmentManager.OnBackStackChangedListener {
public class MainActivity extends NavigationBaseActivity implements FragmentManager.OnBackStackChangedListener {

@Inject
SessionManager sessionManager;
Expand All @@ -63,7 +63,6 @@ public class MainActivity extends AuthenticatedActivity implements FragmentManag


public Intent uploadServiceIntent;
public boolean isAuthCookieAcquired = false;

public ContributionsActivityPagerAdapter contributionsActivityPagerAdapter;
public static final int CONTRIBUTIONS_TAB_POSITION = 0;
Expand All @@ -82,10 +81,10 @@ public void onCreate(Bundle savedInstanceState) {
setContentView(R.layout.activity_contributions);
ButterKnife.bind(this);

requestAuthToken();
initDrawer();
setTitle(getString(R.string.navigation_item_home)); // Should I create a new string variable with another name instead?

initMain();

if (savedInstanceState != null ) {
onOrientationChanged = true; // Will be used in nearby fragment to determine significant update of map
Expand All @@ -103,16 +102,13 @@ protected void onSaveInstanceState(Bundle outState) {
outState.putInt("viewPagerCurrentItem", viewPager.getCurrentItem());
}

@Override
protected void onAuthCookieAcquired(String authCookie) {
// Do a sync everytime we get here!
private void initMain() {
requestSync(sessionManager.getCurrentAccount(), BuildConfig.CONTRIBUTION_AUTHORITY, new Bundle());
uploadServiceIntent = new Intent(this, UploadService.class);
uploadServiceIntent.setAction(UploadService.ACTION_START_SERVICE);
startService(uploadServiceIntent);

addTabsAndFragments();
isAuthCookieAcquired = true;
if (contributionsActivityPagerAdapter.getItem(0) != null) {
((ContributionsFragment)contributionsActivityPagerAdapter.getItem(0)).onAuthCookieAcquired(uploadServiceIntent);
}
Expand Down Expand Up @@ -232,14 +228,9 @@ public void showTabs() {
}
}

@Override
protected void onAuthFailure() {

}

@Override
public void onBackPressed() {
DrawerLayout drawer = (DrawerLayout) findViewById(R.id.drawer_layout);
DrawerLayout drawer = findViewById(R.id.drawer_layout);
String contributionsFragmentTag = ((ContributionsActivityPagerAdapter) viewPager.getAdapter()).makeFragmentName(R.id.pager, 0);
String nearbyFragmentTag = ((ContributionsActivityPagerAdapter) viewPager.getAdapter()).makeFragmentName(R.id.pager, 1);
if (drawer.isDrawerOpen(GravityCompat.START)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,13 +62,6 @@ public void onPerformSync(Account account, Bundle bundle, String s, ContentProvi
return;
}

String authCookie = sessionManager.getAuthCookie();
if (isNullOrWhiteSpace(authCookie)) {
Timber.d("Could not authenticate :(");
return;
}


allModifications.moveToFirst();

Timber.d("Found %d modifications to execute", allModifications.getCount());
Expand Down Expand Up @@ -130,7 +123,4 @@ public void onPerformSync(Account account, Bundle bundle, String s, ContentProvi
}
}

private boolean isNullOrWhiteSpace(String value) {
return value == null || value.trim().isEmpty();
}
}
13 changes: 2 additions & 11 deletions app/src/main/java/fr/free/nrw/commons/review/ReviewActivity.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,16 +26,16 @@
import butterknife.ButterKnife;
import fr.free.nrw.commons.Media;
import fr.free.nrw.commons.R;
import fr.free.nrw.commons.auth.AuthenticatedActivity;
import fr.free.nrw.commons.delete.DeleteHelper;
import fr.free.nrw.commons.mwapi.MediaWikiApi;
import fr.free.nrw.commons.theme.NavigationBaseActivity;
import fr.free.nrw.commons.utils.DialogUtil;
import fr.free.nrw.commons.utils.ViewUtil;
import io.reactivex.android.schedulers.AndroidSchedulers;
import io.reactivex.disposables.CompositeDisposable;
import io.reactivex.schedulers.Schedulers;

public class ReviewActivity extends AuthenticatedActivity {
public class ReviewActivity extends NavigationBaseActivity {

@BindView(R.id.pager_indicator_review)
public CirclePageIndicator pagerIndicator;
Expand Down Expand Up @@ -94,15 +94,6 @@ public Media getMedia() {
return media;
}

@Override
protected void onAuthCookieAcquired(String authCookie) {

}

@Override
protected void onAuthFailure() {
}

@Override
protected void onCreate(Bundle savedInstanceState) {
super.onCreate(savedInstanceState);
Expand Down