-
Notifications
You must be signed in to change notification settings - Fork 1.3k
On Logout, fetch the CSRF token #3173
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
On Logout, fetch the CSRF token #3173
Conversation
* use the CSRF token to make the logout post call * once both of these succeed, clear the local data
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please add tests to make sure this change works as expected?
Codecov Report
@@ Coverage Diff @@
## backend-overhaul #3173 +/- ##
===================================================
- Coverage 5.79% 5.77% -0.02%
===================================================
Files 257 257
Lines 11270 11308 +38
Branches 895 897 +2
===================================================
Hits 653 653
- Misses 10562 10600 +38
Partials 55 55
Continue to review full report at Codecov.
|
* Fetch the CSRF token to logout | ||
*/ | ||
private void handleLogout() { | ||
service = ServiceFactory.get(commonsWikiSite); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move this to a separate class. ie LogoutClient
|
||
progressDialog.show(); | ||
disposable.add( | ||
service.getCsrfToken() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use @Named(NAMED_COMMONS_CSRF)
/** | ||
* Make the logout post call with the CSRF token fetched | ||
*/ | ||
private void postLogoutApi(MwQueryResponse response) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move to LogoutClient
or AuthClient
@ashishkumar468 have you opened a new PR for the same? |
@maskaravivek Yes, this branch somehow got very much away from backend-overhaul, after we recently synced backend-overhaul with master, so as the changes were small, I thought making a new one would be quicker :-) |
Fixes #3150 (comment)
Description (required)
Tests performed (required)
Tested prodDebug on Samsung S7 API 27