Developer notes

• Hi, I'm Olav, and who are you? Let me know what brought you here! Use any of the contact details given on my website.
• I currently maintain this app (originally created by Roeland) and hence this GitHub repo. I once studied computer science, but I never was a developer. I'm more a generalist focussed on security and privacy. Please advise if you know better how to do dev things here or in the app, or even contribute (ideally via pull request, but I'm grateful to anything that helps).
• BTW: Are you – as "any GitHub user" – able to edit this wiki? (please notify me if you can't, it's the first time I try this wiki).

ToDo / Help appreciated

• Translation/Localization of the app: For a long time, the app was available in German and English only. Recently, I received some first translations. It's easy, and while it's the easiest way for me to receive them as PR, I also accept them as plain text file e.g. by email. Just contact me :)

• Software engineering, code quality, naming conventions

  • Development environment: Move to proper CI environment, automating builds as much as possible, implement consistency checks
  • Develop module and UI tests – I have zero experience how to do that – But manual checking is tiring and error-prone
  • Improve code quality; honor dependency injection / composition root: e.g. constructor in PR-345/Provider/Email.php should inject AtLoginProvider instead of its parameters
  • Avoid SRP (single responsibility principle) violations, such as the use of occonfig in PR-345/Provider/AtLoginProvider.php? Split Provider/Email.php due to it's internal complexity / many use cases, see SRP?
  • Naming conventions: Improve consistency (use of "_", "-" and CamelCase; similar naming of corresponding files, distinguishable filenames (Service/Email → Service/EmailService and Provider/Email → Provider/EmailProvider)?
  • What exactly is a Provider and what is a Service? When to use which? Do present files correspond to that?

• Fields of improvement I see

  • Currently different 2FA apps differ in the way they make sure that the setting is usable for the user prior to enabling it: While twofactor_notification (NC App) may just be activated (whithout checking that the notification arrives at the user's 2FA device), twofactor_totp, twofactor_webauthn and twofactor_email have an additional INTERNAL state that requires a user to prove that she/he can use it. While this reduces support calls, it also requires the user to do additional steps. It may be discussed whether this verification might be omitted.
  • The state machine logic is dispersed in separate files. It may be refactured to have it in one place only. While doing this, namespace could be enhanced (clearification and cleanup). Note to self: Use the return values, don't set them separately.
  • Improve exception handling (for all the App's namespace calls). Currently, there's "only" expected (functional) exceptions; runtime exceptions should be handled gracefully aswell. Note to self: Introduce "RuntimeException"?

• AFTER solving the issues, I'd like to add localization (currently, there's only german, and it's manual work)

  • First step then would be to decide on which tool to use
  • Only after that start asking for volunteers to translate
  • PR-345/AtLoginProvider/$mySecret should not be part of the constructor but be computed/persisted using a service when needed. As it is, I'm unclear what would happen if, say TOTP and Email 2FA apps are active, 2FA is enforced, a new user logs in for the first time, that user chooses TOTP. Would twofactor_email compute/persist a verification code in the database?

• Independently, DEPRECATED code should be replaced, e.g. ILogger → PSR-3 Logger \Psr\Log\LoggerInterface

Open questions

• Shall the App NAME be localized?
  • Pro: Easier to read for native speakers (whereas I doubt that there are may admins using NC that can't speak English)
  • Con: The app is then no longer listed next to it's companion apps such as "Two-Factor TOTP" or "Two-Factor U2F"
• I adopted the login dialog style (no "Login button") to the challenge dialog. Shall there be a "Press Enter" hint? #262 stongly indicates that users do expect even a button…further discussion in that issue.

Notes

• What is the ideal Screenshot / Thumbnail image size?

  • I deducted these sizes from the store by measuring how big images are displayed:
    • Screenshot: 855x479 px
    • Thumbnail: 357x200 px It seems to fit, but is it really not resized? (Needs verification, either by looking into HTML code or by visual pixel-comparison)
  • Yet I noticed that the screenshot is cropped when displayed in a Nextcloud instance's /settings/apps/installed/twofactor_email (and seems to have a different size there, too.

  So, rather reduce size a bit (is it stretched then, or just centered?)?