Skip to content

Logging in to app with two-factor authentication #328

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.

Sign up for GitHub

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

Jump to bottom
Closed
misaochan opened this issue Dec 6, 2016 · 21 comments
Closed

Logging in to app with two-factor authentication #328

misaochan opened this issue Dec 6, 2016 · 21 comments
Assignees
@maskaravivek
Labels
enhancement high priority IEG

Comments

@misaochan
Copy link
Member

Comment on our review page:

I can't login after enabling 2FA I've activated the Two Factor Authentication and the app shows "Incorrect login"

Has anyone tried this before? Also, is there a need to support 2FA in our app?
@nicolas-raoul
Copy link
Member

nicolas-raoul commented Dec 8, 2016 via email

@whym
Copy link
Collaborator

whym commented Jan 8, 2017 •
edited
Loading

Wikipedia App seems to have the same issue and has a pending change.

It might be limited to power users, but there is a need. All sysops were asked to enable 2FA in late November if I recall it correctly.

@VojtechDostal
Copy link
Collaborator

Confirm that logging in with two-factor authentication on is not possible.
Once I was logged in it sort of worked, but maybe this was a cause for some of the problems I have been having with the app.

@misaochan
Copy link
Member Author

What problems were you experiencing with the app @VojtechDostal ?

@VojtechDostal
Copy link
Collaborator

The app failed to start or crashed while searching for Nearby things.
I reinstalled and now it runs smoothly

@whym whym mentioned this issue Apr 16, 2017
Closed
addshore added a commit that referenced this issue May 12, 2017
@addshore
Use clientlogin API module
9453466 
This is the first step to allowing 2 factor authentication #328.
This uses the new API module clientlogin instead of the login module.

We still report the same set of errors in a 'nice' way with real
error messages, how ever there are lots more that can probably be
handled, for example #507.
@addshore addshore mentioned this issue May 12, 2017
Merged
addshore added a commit that referenced this issue May 12, 2017
@addshore
Use clientlogin API module
9ed8162 
This is the first step to allowing 2 factor authentication #328.
This uses the new API module clientlogin instead of the login module.

We still report the same set of errors in a 'nice' way with real
error messages, how ever there are lots more that can probably be
handled, for example #507.
addshore added a commit that referenced this issue May 12, 2017
@addshore
Use clientlogin API module
5cf87a7 
This is the first step to allowing 2 factor authentication #328.
This uses the new API module clientlogin instead of the login module.

We still report the same set of errors in a 'nice' way with real
error messages, how ever there are lots more that can probably be
handled, for example #507.
addshore added a commit that referenced this issue May 12, 2017
@addshore
Use clientlogin API module
ff018a0 
This is the first step to allowing 2 factor authentication #328.
This uses the new API module clientlogin instead of the login module.

We still report the same set of errors in a 'nice' way with real
error messages, how ever there are lots more that can probably be
handled, for example #507.
addshore added a commit that referenced this issue May 12, 2017
@addshore
Use clientlogin API module
3be1af4 
This is the first step to allowing 2 factor authentication #328.
This uses the new API module clientlogin instead of the login module.

We still report the same set of errors in a 'nice' way with real
error messages, how ever there are lots more that can probably be
handled, for example #507.
addshore added a commit that referenced this issue May 12, 2017
@addshore
Use clientlogin API module
f39b5f9 
This is the first step to allowing 2 factor authentication #328.
This uses the new API module clientlogin instead of the login module.

We still report the same set of errors in a 'nice' way with real
error messages, how ever there are lots more that can probably be
handled, for example #507.
addshore added a commit that referenced this issue May 12, 2017
@addshore
Use clientlogin API module
d38dde0 
This is the first step to allowing 2 factor authentication #328.
This uses the new API module clientlogin instead of the login module.

We still report the same set of errors in a 'nice' way with real
error messages, how ever there are lots more that can probably be
handled, for example #507.
sandarumk pushed a commit to sandarumk/apps-android-commons that referenced this issue May 13, 2017
@addshore @sandarumk
Use clientlogin API module
bace89b 
This is the first step to allowing 2 factor authentication commons-app#328.
This uses the new API module clientlogin instead of the login module.

We still report the same set of errors in a 'nice' way with real
error messages, how ever there are lots more that can probably be
handled, for example commons-app#507.
sandarumk pushed a commit to sandarumk/apps-android-commons that referenced this issue May 13, 2017
@addshore @sandarumk
Use clientlogin API module
9c30af5 
This is the first step to allowing 2 factor authentication commons-app#328.
This uses the new API module clientlogin instead of the login module.

We still report the same set of errors in a 'nice' way with real
error messages, how ever there are lots more that can probably be
handled, for example commons-app#507.
sandarumk pushed a commit to sandarumk/apps-android-commons that referenced this issue May 13, 2017
@addshore @sandarumk
Use clientlogin API module
6e5e75b 
This is the first step to allowing 2 factor authentication commons-app#328.
This uses the new API module clientlogin instead of the login module.

We still report the same set of errors in a 'nice' way with real
error messages, how ever there are lots more that can probably be
handled, for example commons-app#507.
addshore added a commit that referenced this issue May 13, 2017
@addshore
WIP 2fa
db19c20 
Fixes #328
addshore added a commit that referenced this issue May 14, 2017
@addshore
Add 2fa to login activity
6bb7de8 
Fixes #328
@addshore
Copy link
Contributor

Not quite done yet

@addshore addshore reopened this May 15, 2017
@misaochan misaochan mentioned this issue Jun 5, 2017
Closed
@misaochan
Copy link
Member Author

Hi @addshore , any chance you might have the time in the near future to complete this? If not, should we add it to the IEG renewal proposal?

@misaochan misaochan mentioned this issue Jun 8, 2017
Closed
@addshore
Copy link
Contributor

I might have time to look at it over the coming weeks but adding it to the IEG grant sounds like a good idea!

@misaochan misaochan mentioned this issue Aug 3, 2017
Closed
@sivaraam
Copy link
Member

sivaraam commented Sep 5, 2017 •
edited
Loading

Just for the note the 2FA has been successfully implemented in the Wikipedia android app. I guess their implementation could help you finish this quickly i.e., you could use their implementation as reference.

@maskaravivek
Copy link
Member

@sivaraam Does wikipedia app use OAuth for authentication? I have added the 2FA permission in OAuth Consumer registration for Commons app. Ref #819

@ragesoss
Copy link
Contributor

ragesoss commented Nov 1, 2017

The Wikipedia app does not use OAuth. It has an in-app login flow that handles 2FA.

@misaochan
Copy link
Member Author

This can be unblocked now that we have confirmed that OAuth is not possible and not needed.

@maskaravivek
Copy link
Member

Our app already has most of the code in place needed for enabling 2FA. I am just not able to test it out as i am not a power user and as the article mentions only power users can enable it for their accounts.

@strainu
Copy link

strainu commented Dec 17, 2017

@maskaravivek , you can ask the stewards to add you to the "oath testing group". I believe such requests are done at https://meta.wikimedia.org/wiki/Steward_requests/Global_permissions#Requests_for_other_global_permissions

@maskaravivek
Copy link
Member

Thanks @strainu for the link. Got the permission by creating a phab ticket. https://phabricator.wikimedia.org/T183079

@maskaravivek maskaravivek mentioned this issue Jan 7, 2018
Merged
@maskaravivek
Copy link
Member

What what i understand,

The HttpClient implements some sort of cookie store which is in turn by AccountManager when a request to retrieve auth cookie is made [blockingGetAuthToken]. I observed that after enabling 2FA, this call is not return any cookie.

Read a few posts regarding handling of cookies for 2FA enabled authentication and finally decided to create and store a cookie in such cases. Ref

Wikimedia APIs require these cookies for all subsequent requests.

@psh @misaochan Your insights here would be useful. :)

@dbrant Does this sound good? I tried to follow the approach Wikimedia app takes around authentication, but then Wikimedia app never makes a call to getAuthToken or blockingGetAuthToken.

@misaochan
Copy link
Member Author

@maskaravivek I just requested global OATH permissions for the production servers - https://meta.wikimedia.org/wiki/Steward_requests/Global_permissions#Requests_for_other_global_permissions . Hopefully we will be able to test your PR soon. :)

Re: creating a cookie, your approach looks reasonable to me, however it would be best to wait for @dbrant 's input due to potential privacy concerns.

@VojtechDostal
Copy link
Collaborator

It's already done, @misaochan :) took them only 4 minutes.

@misaochan
Copy link
Member Author

@VojtechDostal Wow, cool! :) That was fast, haha. Will set my 2FA up and test ASAP.

@dbrant
Copy link
Collaborator

dbrant commented Jan 8, 2018

@maskaravivek The Wikipedia app basically stores all cookies in SharedPreferences, since some of our cookies need to be persisted regardless of having an account. If you want to persist the authentication cookie in AccountManager, that's fine too.

Implementing 2FA simply means that instead of returning a successful reply, the API will return a special status message that should cause your app to ask for the 2FA code, and then pass the code back to the API in your next request.

@maskaravivek
Copy link
Member

Fixed in #1048

@misaochan misaochan mentioned this issue Aug 1, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@maskaravivek maskaravivek

Labels
enhancement high priority IEG
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests