Skip to content

Commit

Permalink
Fix auth issues in the app (#3064)
Browse files Browse the repository at this point in the history
  • Loading branch information
maskaravivek authored Jul 11, 2019
1 parent 5d64cac commit cd57552
Show file tree
Hide file tree
Showing 8 changed files with 26 additions and 109 deletions.
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

0 comments on commit cd57552

Please sign in to comment.