Page MenuHomePhabricator

2FA logon doesn’t work on the iOS app
Closed, ResolvedPublic

Description

The box to enter the 2FA code doesn’t seem to actually appear... just the error message (will attach a screenshot)

Version: 6.3.0 (1643) - TestFlight Beta

Event Timeline

Reedy created this task.Jul 21 2019, 1:09 PM
Restricted Application added a project: Wikipedia-iOS-App-Backlog. · View Herald TranscriptJul 21 2019, 1:09 PM
Restricted Application added a subscriber: Aklapper. · View Herald Transcript
Reedy updated the task description. (Show Details)Jul 21 2019, 1:09 PM
Reedy updated the task description. (Show Details)
JMinor triaged this task as High priority.Jul 22 2019, 6:35 PM
JMinor moved this task from Needs Triage to Bug Backlog on the Wikipedia-iOS-App-Backlog board.
Reedy added a comment.Jul 28 2019, 9:22 AM

Is there something we can do to add (automated) testing for this, or making sure part of the QA of the releases, that this actually works in both versions?

JoeWalsh added a subscriber: JoeWalsh.EditedAug 2 2019, 5:52 PM

@Reedy yes, we can add automated testing for this. Is there a way to create accounts with 2FA enabled on the beta cluster? nevermind, found out how to do this

@Reedy it looks like the iOS and Android apps are both looking for a request with id equal to TOTPAuthenticationRequest in the response and that string changed to MediaWiki\Extension\OATHAuth\Auth\TOTPAuthenticationRequest - is this expected?

Mhurd added a comment.EditedAug 2 2019, 7:40 PM

For future reference, here's the original 2FA ticket:

T150699

ItSpiderman added a subscriber: ItSpiderman.EditedAug 3 2019, 7:03 AM

@JoeWalsh Due to refactoring and moving towards namespaced classes

MediaWiki\Extension\OATHAuth\Auth\TOTPAuthenticationRequest

class name is expected.
No refactoring was done (from our side) on Wikipedia apps

Please let me know if i can help in any way

Reedy added a comment.Aug 3 2019, 7:59 AM

@JoeWalsh Due to refactoring and moving towards namespaced classes

MediaWiki\Extension\OATHAuth\Auth\TOTPAuthenticationRequest

class name is expected.
No refactoring was done (from our side) on Wikipedia apps
Please let me know if i can help in any way

Ugh. Partially my fault then too.

@ItSpiderman I think this is definitely worthy of a post to both wikitech-l and mediawiki-l to inform people of the breaking change in output for the extension

ItSpiderman added a comment.EditedAug 3 2019, 9:17 AM

Sent a mail to both lists
Edit: well I guess i cannot send mails right after subscribing to the lists. I will try again later, or someone else can send the mail

@Reedy in the future, should the apps rely on the fields in the response rather than the id? Could there be a separate field type (instead of string) for 2FA to allow for only numerical input?

Osnard added a comment.Aug 5 2019, 2:44 PM

I am wondering why the "id" is an actual PHP class name, rather than a "real" id, like totp, which chould just be provided by the PHP class and therefore would not change in such cases.

Reedy added a comment.Aug 7 2019, 2:05 PM

I am wondering why the "id" is an actual PHP class name, rather than a "real" id, like totp, which chould just be provided by the PHP class and therefore would not change in such cases.

Does this code come from AuthManager rather than the OATHAuth extension?

Osnard added a comment.EditedAug 7 2019, 2:30 PM

As far as I can tell, yes. It is part of the response of a call to the clientlogin action API (https://github.com/wikimedia/apps-android-wikipedia/blob/91db452cd50960dc4b1146492dddf23d651bfdcb/app/src/main/java/org/wikipedia/login/LoginClient.java#L212). And that again just outputs stuff from AuthManager. I am not aware of any code inside Extension:OATHAuth that would publish this information directly.

Reedy added a comment.Aug 7 2019, 3:23 PM
[08:03:04] <Reedy> tgr: If you're bored... https://phabricator.wikimedia.org/T228588 class refactoring in OATHAuth broke 2FA in both our mobile apps
[08:03:31] <Reedy> Being questioned about whether the way/stuff we're exposing in clientlogin from authmanager is "right" or we could/should do soemthing else
[08:04:17] <tgr> you can provide a custom ID if you want
[08:04:47] <Reedy> which I guess is the answer to
[08:04:47] <Reedy> >I am wondering why the "id" is an actual PHP class name, rather than a "real" id, like totp, which chould just be provided by the PHP class and therefore would not change in such cases.
[08:05:01] <tgr> we haven't really though about clients wanting to depend on a specific ID, the vision was more about clients using the API to get the definitions for some form builder
[08:06:46] <tgr> there's AuthenticationRequest::getUniqueId(), although the original intent was to support use cases where you can have multiple instances of the same class (e.g. GoogleLogin where there are multiple Google accounts connected to the user) but there is no harm in using it to provide a fixed string
[08:06:54] <tgr> or fake the old value or whatever
JMinor closed this task as Resolved.Tue, Aug 27, 8:42 PM