Skip to content
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

Clear auth cookie while logging out #1875

Merged
merged 1 commit into from
Sep 5, 2018

Conversation

maskaravivek
Copy link
Member

Title (required)

Fixes issue where the uploads were attributed to a different account on logging out

Description (required)

As mentioned in #1866 on logging out, the next upload was being attributed to a different account. I investigated and found out that the authCookie wasn't getting cleared on logging out. This PR handles the log out more gracefully.

Tests performed (required)

I didn't upload pictures from both the accounts but have verified that the authCookie is up to date when a fresh login occurs. I switched between Testmaskara and Maskaravivek in the same session and found that the cookies were set correctly.

@misaochan With this fix can you also try making a wiki data edit with a 2FA enabled account. I am hoping that the failures would be less frequent(There's one scenario that I mentioned in #1874 that is still not handled).

@codecov-io
Copy link

codecov-io commented Sep 2, 2018

Codecov Report

Merging #1875 into 2.8-release will decrease coverage by <.01%.
The diff coverage is 0%.

Impacted file tree graph

@@              Coverage Diff               @@
##           2.8-release   #1875      +/-   ##
==============================================
- Coverage         3.67%   3.66%   -0.01%     
==============================================
  Files              188     188              
  Lines             9555    9565      +10     
  Branches           846     846              
==============================================
  Hits               351     351              
- Misses            9180    9190      +10     
  Partials            24      24
Impacted Files Coverage Δ
...n/java/fr/free/nrw/commons/CommonsApplication.java 33.33% <0%> (ø) ⬆️
...in/java/fr/free/nrw/commons/mwapi/CustomMwApi.java 7.86% <0%> (-0.19%) ⬇️
...rw/commons/mwapi/ApacheHttpClientMediaWikiApi.java 3.52% <0%> (-0.04%) ⬇️
.../java/fr/free/nrw/commons/auth/SessionManager.java 9.52% <0%> (-0.48%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b584942...33610d7. Read the comment docs.

@misaochan
Copy link
Member

I merged #1874 . I think it would be helpful if you could rebase on 2.8-release so that when we test this, we can make use of the error logging in that PR.

@maskaravivek
Copy link
Member Author

@misaochan I have rebased the PR.

@misaochan
Copy link
Member

misaochan commented Sep 3, 2018

Test 1: Logged out of Misaochan (2FA) into Misaochan2 (non-2FA). Wikidata edit succeeds as usual.

Test 2: Logged out of Misaochan2 and into Misaochan.

  • First time I tried to load Nearby, it crashed (normal after logout, due to an error which we haven't fixed)
  • 2nd time I loaded Nearby succesfully, but after selecting an item in Nearby List and when I was trying to select my photo, it crashed due to Error 1 below. (Edit: I looked through the stack trace and Error 1 doesn't appear to be relevant to your PR, but rather caused by fetching user deletion rate. Will create a separate issue)
  • 3rd try, loaded Nearby successfully, didn't crash when I was selecting my photo, and the Wikidata edit succeeded! https://www.wikidata.org/wiki/Q5355155
  • 4th try, going to NearbyActivity again from my Contributions gives me the toast "Error loading Nearby Places!" and Error 2 below. This has not happened to me before. (Edit: this seems to be caused by a connection error, so not relevant either I think)
  • 5th try, works again! Successfully added image to https://www.wikidata.org/wiki/Q19879858 but removed it because it's not a proper image and I'm out of Nearby images, lol.

Conclusion:

  • This works! Whenever I manage to submit an image to Nearby, it edits p18 and isn't misattributed.
  • But 2/4 of my attempts to load Nearby crashed, not including the known bug with the first load post-login. We need to figure out if the crashes are caused by this PR or not.
  • Edit: I think the errors are not related. @maskaravivek please upload to Nearby with your 2FA enabled. Once you report the results of your tests, we can go ahead and merge.

All tests on Android 8.0 on Samsung Galaxy s7 device.

Error 1:

09-03 19:16:22.906 27648-27648/fr.free.nrw.commons W/System.err: io.reactivex.exceptions.OnErrorNotImplementedException: timeout
09-03 19:16:22.907 27648-27648/fr.free.nrw.commons W/System.err:     at io.reactivex.internal.functions.Functions$OnErrorMissingConsumer.accept(Functions.java:704)
        at io.reactivex.internal.functions.Functions$OnErrorMissingConsumer.accept(Functions.java:701)
        at io.reactivex.internal.observers.ConsumerSingleObserver.onError(ConsumerSingleObserver.java:45)
09-03 19:16:22.908 27648-27648/fr.free.nrw.commons W/System.err:     at com.tspoon.traceur.SingleOnAssembly$OnAssemblySingleObserver.onError(SingleOnAssembly.java:75)
        at io.reactivex.internal.operators.single.SingleObserveOn$ObserveOnSingleObserver.run(SingleObserveOn.java:79)
        at io.reactivex.android.schedulers.HandlerScheduler$ScheduledRunnable.run(HandlerScheduler.java:109)
        at android.os.Handler.handleCallback(Handler.java:789)
09-03 19:16:22.909 27648-27648/fr.free.nrw.commons W/System.err:     at android.os.Handler.dispatchMessage(Handler.java:98)
        at android.os.Looper.loop(Looper.java:164)
        at android.app.ActivityThread.main(ActivityThread.java:6944)
09-03 19:16:22.910 27648-27648/fr.free.nrw.commons W/System.err:     at java.lang.reflect.Method.invoke(Native Method)
        at com.android.internal.os.Zygote$MethodAndArgsCaller.run(Zygote.java:327)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1374)
09-03 19:16:22.913 27648-27648/fr.free.nrw.commons W/System.err: Caused by: java.net.SocketTimeoutException: timeout
        at okhttp3.internal.http2.Http2Stream$StreamTimeout.newTimeoutException(Http2Stream.java:593)
        at okhttp3.internal.http2.Http2Stream$StreamTimeout.exitAndThrowIfTimedOut(Http2Stream.java:601)
09-03 19:16:22.914 27648-27648/fr.free.nrw.commons W/System.err:     at okhttp3.internal.http2.Http2Stream.takeResponseHeaders(Http2Stream.java:146)
        at okhttp3.internal.http2.Http2Codec.readResponseHeaders(Http2Codec.java:125)
        at okhttp3.internal.http.CallServerInterceptor.intercept(CallServerInterceptor.java:88)
        at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:147)
09-03 19:16:22.915 27648-27648/fr.free.nrw.commons W/System.err:     at okhttp3.internal.connection.ConnectInterceptor.intercept(ConnectInterceptor.java:45)
        at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:147)
        at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:121)
09-03 19:16:22.916 27648-27648/fr.free.nrw.commons W/System.err:     at okhttp3.internal.cache.CacheInterceptor.intercept(CacheInterceptor.java:93)
        at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:147)
        at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:121)
        at okhttp3.internal.http.BridgeInterceptor.intercept(BridgeInterceptor.java:93)
09-03 19:16:22.917 27648-27648/fr.free.nrw.commons W/System.err:     at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:147)
        at okhttp3.internal.http.RetryAndFollowUpInterceptor.intercept(RetryAndFollowUpInterceptor.java:126)
09-03 19:16:22.918 27648-27648/fr.free.nrw.commons W/System.err:     at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:147)
        at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:121)
        at okhttp3.RealCall.getResponseWithInterceptorChain(RealCall.java:200)
        at okhttp3.RealCall.execute(RealCall.java:77)
09-03 19:16:22.919 27648-27648/fr.free.nrw.commons W/System.err:     at fr.free.nrw.commons.mwapi.ApacheHttpClientMediaWikiApi.lambda$getRevertRespObjectSingle$5$ApacheHttpClientMediaWikiApi(ApacheHttpClientMediaWikiApi.java:1018)
        at fr.free.nrw.commons.mwapi.ApacheHttpClientMediaWikiApi$$Lambda$9.call(Unknown Source:4)
        at io.reactivex.internal.operators.single.SingleFromCallable.subscribeActual(SingleFromCallable.java:35)
09-03 19:16:22.920 27648-27648/fr.free.nrw.commons W/System.err:     at io.reactivex.Single.subscribe(Single.java:2700)
        at com.tspoon.traceur.SingleOnAssembly.subscribeActual(SingleOnAssembly.java:43)
        at io.reactivex.Single.subscribe(Single.java:2700)
        at io.reactivex.internal.operators.single.SingleSubscribeOn$SubscribeOnObserver.run(SingleSubscribeOn.java:89)
09-03 19:16:22.921 27648-27648/fr.free.nrw.commons W/System.err:     at io.reactivex.Scheduler$DisposeTask.run(Scheduler.java:452)
        at io.reactivex.internal.schedulers.ScheduledRunnable.run(ScheduledRunnable.java:61)
09-03 19:16:22.922 27648-27648/fr.free.nrw.commons W/System.err:     at io.reactivex.internal.schedulers.ScheduledRunnable.call(ScheduledRunnable.java:52)
        at java.util.concurrent.FutureTask.run(FutureTask.java:266)
        at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:301)
09-03 19:16:22.923 27648-27648/fr.free.nrw.commons W/System.err:     at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1162)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:636)
        at java.lang.Thread.run(Thread.java:764)
09-03 19:16:22.924 27648-27648/fr.free.nrw.commons W/System.err: Caused by: com.tspoon.traceur.TraceurException: Debug Exception generated at call site
        at dalvik.system.VMStack.getThreadStackTrace(Native Method)
        at java.lang.Thread.getStackTrace(Thread.java:1536)
        at io.reactivex.Single.fromCallable(Single.java:448)
09-03 19:16:22.925 27648-27648/fr.free.nrw.commons W/System.err:     at fr.free.nrw.commons.mwapi.ApacheHttpClientMediaWikiApi.getRevertRespObjectSingle(ApacheHttpClientMediaWikiApi.java:1005)
        at fr.free.nrw.commons.quiz.QuizChecker.setRevertCount(QuizChecker.java:92)
        at fr.free.nrw.commons.quiz.QuizChecker.<init>(QuizChecker.java:55)
09-03 19:16:22.926 27648-27648/fr.free.nrw.commons W/System.err:     at fr.free.nrw.commons.contributions.ContributionsActivity.onCreate(ContributionsActivity.java:152)
        at android.app.Activity.performCreate(Activity.java:7174)
        at android.app.Instrumentation.callActivityOnCreate(Instrumentation.java:1220)
09-03 19:16:22.927 27648-27648/fr.free.nrw.commons W/System.err:     at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:2910)
        at android.app.ActivityThread.handleLaunchActivity(ActivityThread.java:3032)
        at android.app.ActivityThread.-wrap11(Unknown Source:0)
        at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1696)
09-03 19:16:22.928 27648-27648/fr.free.nrw.commons W/System.err:     at android.os.Handler.dispatchMessage(Handler.java:105)
        at android.os.Looper.loop(Looper.java:164)
        at android.app.ActivityThread.main(ActivityThread.java:6944)
        at java.lang.reflect.Method.invoke(Native Method)
        at com.android.internal.os.Zygote$MethodAndArgsCaller.run(Zygote.java:327)
09-03 19:16:22.929 27648-27648/fr.free.nrw.commons W/System.err:     at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1374)
09-03 19:16:22.930 27648-27648/fr.free.nrw.commons W/System.err: Caused by: com.tspoon.traceur.TraceurException: Debug Exception generated at call site
        at dalvik.system.VMStack.getThreadStackTrace(Native Method)
        at java.lang.Thread.getStackTrace(Thread.java:1536)
09-03 19:16:22.931 27648-27648/fr.free.nrw.commons W/System.err:     at io.reactivex.Single.subscribeOn(Single.java:2768)
        at fr.free.nrw.commons.quiz.QuizChecker.setRevertCount(QuizChecker.java:93)
09-03 19:16:22.932 27648-27648/fr.free.nrw.commons W/System.err: 	... 14 more
    Caused by: com.tspoon.traceur.TraceurException: Debug Exception generated at call site
09-03 19:16:22.933 27648-27648/fr.free.nrw.commons W/System.err:     at dalvik.system.VMStack.getThreadStackTrace(Native Method)
        at java.lang.Thread.getStackTrace(Thread.java:1536)
        at io.reactivex.Single.observeOn(Single.java:2296)
        at fr.free.nrw.commons.quiz.QuizChecker.setRevertCount(QuizChecker.java:94)
09-03 19:16:22.934 27648-27648/fr.free.nrw.commons W/System.err: 	... 14 more
09-03 19:16:22.946 3646-8649/? W/ActivityManager: crash : fr.free.nrw.commons,0
09-03 19:16:22.948 3646-8649/? W/ActivityManager:   Force finishing activity fr.free.nrw.commons/.nearby.NearbyActivity

Error 2:

09-03 19:23:45.306 28233-28233/fr.free.nrw.commons D/NearbyActivity: java.net.UnknownHostException: Unable to resolve host "query.wikidata.org": No address associated with hostname
        at java.net.Inet6AddressImpl.lookupHostByName(Inet6AddressImpl.java:141)
09-03 19:23:45.307 28233-28233/fr.free.nrw.commons D/NearbyActivity:     at java.net.Inet6AddressImpl.lookupAllHostAddr(Inet6AddressImpl.java:90)
        at java.net.InetAddress.getAllByName(InetAddress.java:787)
        at com.android.okhttp.Dns$1.lookup(Dns.java:39)
        at com.android.okhttp.internal.http.RouteSelector.resetNextInetSocketAddress(RouteSelector.java:200)
        at com.android.okhttp.internal.http.RouteSelector.nextProxy(RouteSelector.java:148)
        at com.android.okhttp.internal.http.RouteSelector.next(RouteSelector.java:90)
        at com.android.okhttp.internal.http.StreamAllocation.findConnection(StreamAllocation.java:190)
        at com.android.okhttp.internal.http.StreamAllocation.findHealthyConnection(StreamAllocation.java:142)
        at com.android.okhttp.internal.http.StreamAllocation.newStream(StreamAllocation.java:104)
        at com.android.okhttp.internal.http.HttpEngine.connect(HttpEngine.java:410)
        at com.android.okhttp.internal.http.HttpEngine.sendRequest(HttpEngine.java:343)
        at com.android.okhttp.internal.huc.HttpURLConnectionImpl.execute(HttpURLConnectionImpl.java:489)
        at com.android.okhttp.internal.huc.HttpURLConnectionImpl.getResponse(HttpURLConnectionImpl.java:435)
        at com.android.okhttp.internal.huc.HttpURLConnectionImpl.getInputStream(HttpURLConnectionImpl.java:248)
        at com.android.okhttp.internal.huc.DelegatingHttpsURLConnection.getInputStream(DelegatingHttpsURLConnection.java:210)
        at com.android.okhttp.internal.huc.HttpsURLConnectionImpl.getInputStream(Unknown Source:0)
        at fr.free.nrw.commons.nearby.NearbyPlaces.getFromWikidataQuery(NearbyPlaces.java:84)
        at fr.free.nrw.commons.nearby.NearbyPlaces.getFromWikidataQuery(NearbyPlaces.java:48)
        at fr.free.nrw.commons.nearby.NearbyController.loadAttractionsFromLocation(NearbyController.java:56)
        at fr.free.nrw.commons.nearby.NearbyActivity.lambda$refreshView$7$NearbyActivity(NearbyActivity.java:437)
        at fr.free.nrw.commons.nearby.NearbyActivity$$Lambda$7.call(Unknown Source:2)
09-03 19:23:45.308 28233-28233/fr.free.nrw.commons D/NearbyActivity:     at io.reactivex.internal.operators.observable.ObservableFromCallable.subscribeActual(ObservableFromCallable.java:42)
        at io.reactivex.Observable.subscribe(Observable.java:10901)
        at com.tspoon.traceur.ObservableOnAssemblyCallable.subscribeActual(ObservableOnAssemblyCallable.java:42)
        at io.reactivex.Observable.subscribe(Observable.java:10901)
        at io.reactivex.internal.operators.observable.ObservableSubscribeOn$SubscribeTask.run(ObservableSubscribeOn.java:96)
        at io.reactivex.Scheduler$DisposeTask.run(Scheduler.java:452)
        at io.reactivex.internal.schedulers.ScheduledRunnable.run(ScheduledRunnable.java:61)
        at io.reactivex.internal.schedulers.ScheduledRunnable.call(ScheduledRunnable.java:52)
        at java.util.concurrent.FutureTask.run(FutureTask.java:266)
        at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:301)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1162)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:636)
        at java.lang.Thread.run(Thread.java:764)
    Caused by: android.system.GaiException: android_getaddrinfo failed: EAI_NODATA (No address associated with hostname)
        at libcore.io.Linux.android_getaddrinfo(Native Method)
        at libcore.io.ForwardingOs.android_getaddrinfo(ForwardingOs.java:58)
        at java.net.Inet6AddressImpl.lookupHostByName(Inet6AddressImpl.java:122)
    	... 34 more
    Caused by: com.tspoon.traceur.TraceurException: Debug Exception generated at call site
        at dalvik.system.VMStack.getThreadStackTrace(Native Method)
        at java.lang.Thread.getStackTrace(Thread.java:1536)
        at io.reactivex.Observable.fromCallable(Observable.java:1724)
        at fr.free.nrw.commons.nearby.NearbyActivity.refreshView(NearbyActivity.java:436)
        at fr.free.nrw.commons.nearby.NearbyActivity.checkLocationPermission(NearbyActivity.java:285)
        at fr.free.nrw.commons.nearby.NearbyActivity.checkGps(NearbyActivity.java:278)
        at fr.free.nrw.commons.nearby.NearbyActivity.onResume(NearbyActivity.java:348)
09-03 19:23:45.309 28233-28233/fr.free.nrw.commons D/NearbyActivity:     at android.app.Instrumentation.callActivityOnResume(Instrumentation.java:1361)
        at android.app.Activity.performResume(Activity.java:7352)
        at android.app.ActivityThread.performResumeActivity(ActivityThread.java:3765)
        at android.app.ActivityThread.handleResumeActivity(ActivityThread.java:3830)
        at android.app.ActivityThread.handleLaunchActivity(ActivityThread.java:3038)
        at android.app.ActivityThread.-wrap11(Unknown Source:0)
09-03 19:23:45.310 28233-28233/fr.free.nrw.commons D/NearbyActivity:     at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1696)
        at android.os.Handler.dispatchMessage(Handler.java:105)
        at android.os.Looper.loop(Looper.java:164)
        at android.app.ActivityThread.main(ActivityThread.java:6944)
        at java.lang.reflect.Method.invoke(Native Method)
09-03 19:23:45.311 28233-28233/fr.free.nrw.commons D/NearbyActivity:     at com.android.internal.os.Zygote$MethodAndArgsCaller.run(Zygote.java:327)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1374)
    Caused by: com.tspoon.traceur.TraceurException: Debug Exception generated at call site
        at dalvik.system.VMStack.getThreadStackTrace(Native Method)
        at java.lang.Thread.getStackTrace(Thread.java:1536)
        at io.reactivex.Observable.subscribeOn(Observable.java:10977)
        at fr.free.nrw.commons.nearby.NearbyActivity.refreshView(NearbyActivity.java:438)
    	... 16 more
    Caused by: com.tspoon.traceur.TraceurException: Debug Exception generated at call site
        at dalvik.system.VMStack.getThreadStackTrace(Native Method)
        at java.lang.Thread.getStackTrace(Thread.java:1536)
        at io.reactivex.Observable.observeOn(Observable.java:8693)
        at io.reactivex.Observable.observeOn(Observable.java:8626)
        at fr.free.nrw.commons.nearby.NearbyActivity.refreshView(NearbyActivity.java:439)
    	... 16 more

@maskaravivek
Copy link
Member Author

@misaochan Thanks for testing the PR. :)

Both the error look to be related to flaky or no internet. We should surely handle these cases and I am sure there are other cases that needs handling too. For eg. if I launch the app with flight mode enabled, the app immediately crashes instead of handling it gracefully.

@misaochan
Copy link
Member

I agree, the crashes look unrelated. Will merge this as soon as you get global OATH permissions and can verify the edits work for you too.

Copy link
Collaborator

@neslihanturan neslihanturan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @maskaravivek , it works with no crashes for me.

@misaochan
Copy link
Member

@neslihanturan We need you to test that your Nearby uploads actually edit Wikidata p18 with this, after logging out and in. Not for crashes.

@misaochan
Copy link
Member

Edit: Talked to Neslihan, unavoidable issues with her VPN. I will merge, and pray really hard that it works. ;)

@misaochan misaochan merged commit 54caad2 into commons-app:2.8-release Sep 5, 2018
maskaravivek added a commit that referenced this pull request Sep 7, 2018
* Add Traceur for getting meaningful RxJava stack traces (#1832)

* Hotfix for overwrite issue in 2.8.0  (#1838)

* This solution is an hotfix for overrite issue came back on 2.8.0 version. What I did is checking the extension, and if it is null, adding .jpg suffix. Because commons files always have suffixes, and we should compare file names after adding suffixes. Othervise overrides are possible.

* Check if file title includes an extension already, by checking if is there any dot in it.

* Fix logic error

* Add uncovered tests

* Remove unecessary line breaks

* Make Javadocs more explicit

* Versioning and changelog for v2.8.2 (#1842)

* Versioning for v2.8.2

* Changelog for v2.8.2

* Add logs in wiki data edit and session refresh flow (#1874)

* Fix logout (#1875)

* [WIP] Refactor feedback and quiz to reduce possibility of NPE (#1881)

* Refactor feedback and quiz to reduce possibility of NPE

* Handle throwables in quiz checker

* Minor refactoring

* Set Traceur to only work in DEBUG mode (#1884)

* Bug fix for uploaded images count in achievements activity (#1885)

* Versioning and changelog for v2.8.3 (#1886)

* Update changelog.md

* Versioning for v2.8.3
@maskaravivek maskaravivek deleted the fixLogout branch September 12, 2018 20:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants