Summary: | Two-factor authentication code should be valid longer | ||
---|---|---|---|
Product: | Koha | Reporter: | Jonathan Druart <jonathan.druart> |
Component: | Authentication | Assignee: | Jonathan Druart <jonathan.druart> |
Status: | CLOSED FIXED | QA Contact: | Marcel de Rooy <m.de.rooy> |
Severity: | normal | ||
Priority: | P5 - low | CC: | arthur.suzuki, david, dcook, dpavlin, lucas, m.de.rooy, tomascohen |
Version: | unspecified | ||
Hardware: | All | ||
OS: | All | ||
See Also: | https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28787 | ||
Change sponsored?: | --- | Patch complexity: | Trivial patch |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: |
This extends the time a two-factor authentication code is valid for, in case it is not entered quickly enough. (Example: wait for the code to change, then enter the previous code - this should still work, but will not work when the code changes again.)
|
Version(s) released in: |
22.11.00, 22.05.01
|
Circulation function: | |||
Bug Depends on: | 28786 | ||
Bug Blocks: | 30843 | ||
Attachments: |
Bug 30842: 2FA - Allow at least one old TOTP
Bug 30842: 2FA - Allow at least one old TOTP Bug 30842: 2FA - Allow at least one old TOTP |
Description
Jonathan Druart
2022-05-25 08:03:39 UTC
Created attachment 135333 [details] [review] Bug 30842: 2FA - Allow at least one old TOTP We allow one old token when we are setting the two-factor auth, we should reuse the same settings when validation the authentication itself. Test plan: Setup 2FA for your logged-in user Logout/Login Have a look at the code and wait for 30 sec before using it (< 1min however) Created attachment 135414 [details] [review] Bug 30842: 2FA - Allow at least one old TOTP We allow one old token when we are setting the two-factor auth, we should reuse the same settings when validation the authentication itself. Test plan: Setup 2FA for your logged-in user Logout/Login Have a look at the code and wait for 30 sec before using it (< 1min however) Signed-off-by: David Nind <david@davidnind.com> Testing notes (koha-testing-docker). Enabling two-factor authentication (from test plan for Bug 28786): 1. Enable two faction authentication: TwoFactorAuthentication system preference. 2. Go to your account, click 'More' > 'Manage Two-Factor authentication'. 3. Click Enable, scan the QR code with the app, insert the pin code and register. 4. Your account now requires 2FA to login! Notes: - Used andOTP to test. - Waited for code to change, then entered old code: success! - Waited for code to change twice, and then entered old code: fail (as expected). - Entered code before it changed: success (as expected)! - Disabled 2FA and setup again: worked as expected. From my experience with 2FA, I've never had any problems with the short period of time (for other services, not Koha). I dont think that this oneliner patch should go on its own. It is just hardcoding a 1 in the verify call in Auth. This 1 should be a parameter from koha-conf (imo). We should merge this with 30843 and add a parameter too for the interval that GoogleAuth gets from us in ->code and ->verify. It's fixing a bug, an inconsistency. Bug 30843 is an enhancement. IMO it makes sense to have it pushed now and backported. (In reply to Jonathan Druart from comment #5) > It's fixing a bug, an inconsistency. Bug 30843 is an enhancement. IMO it > makes sense to have it pushed now and backported. The backport argument wins. The title of this report could be more specific btw. Created attachment 135417 [details] [review] Bug 30842: 2FA - Allow at least one old TOTP We allow one old token when we are setting the two-factor auth, we should reuse the same settings when validation the authentication itself. Test plan: Setup 2FA for your logged-in user Logout/Login Have a look at the code and wait for 30 sec before using it (< 1min however) Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Pushed to master for 22.11. Nice work everyone, thanks! Pushed to 22.05.x for 22.05.01 depends on 28786 which is an enhancement. not backporting to 21.11. |