Last Comment Bug 1199090 - add printable recovery 2fa codes
: add printable recovery 2fa codes
Status: RESOLVED FIXED
:
Product: bugzilla.mozilla.org
Classification: Other
Component: General (show other bugs)
: Production
: Unspecified Unspecified
P2 normal (vote)
: ---
Assigned To: Byron Jones ‹:glob›
:
:
Mentors:
Depends on: 1199087
Blocks: 1196628
  Show dependency treegraph
 
Reported: 2015-08-26 23:06 PDT by Byron Jones ‹:glob›
Modified: 2015-09-29 07:57 PDT (History)
8 users (show)
See Also:
Due Date:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
1199090_1.patch (15.54 KB, patch)
2015-09-23 00:31 PDT, Byron Jones ‹:glob›
dylan: review-
Details | Diff | Splinter Review
1199090_2.patch (15.37 KB, patch)
2015-09-23 21:17 PDT, Byron Jones ‹:glob›
dylan: review+
Details | Diff | Splinter Review

Description User image Byron Jones ‹:glob› 2015-08-26 23:06:35 PDT
add printable recovery 2fa codes
Comment 1 User image Byron Jones ‹:glob› 2015-09-23 00:31:09 PDT
Created attachment 8664662 [details] [diff] [review]
1199090_1.patch

- add ability to generate codes
- allows codes to be used anywhere 2fa is required
- minor fixes (uninit warnings, printing tweaks)
Comment 2 User image Dylan Hardison [:dylan] 2015-09-23 19:20:57 PDT
Comment on attachment 8664662 [details] [diff] [review]
1199090_1.patch

Review of attachment 8664662 [details] [diff] [review]:
-----------------------------------------------------------------

r- because inconsistent calling convention of verify_check() -- in one place it is called with no arguments,
and in another with arguments. The behavior seems fine but I'd like that to be fixed before I finish reviewing it.

::: Bugzilla/MFA.pm
@@ +90,5 @@
> +}
> +
> +# methods
> +
> +sub generate_recovery_codes {

nit 1: I don't think we really need three loops here. 
nit 2: the first loop doesn't conform to our usual coding conventions.

::: userprefs.cgi
@@ +675,5 @@
>          clear_settings_cache(Bugzilla->user->id);
>      }
> +
> +    elsif ($action eq 'recovery') {
> +        $user->mfa_provider->verify_check(Bugzilla->input_params);

Argument passed to method is not used.

@@ +686,2 @@
>      else {
> +        $user->mfa_provider->verify_check(Bugzilla->input_params);

Argument passed to method is not used by method.
Comment 3 User image Byron Jones ‹:glob› 2015-09-23 21:17:42 PDT
Created attachment 8665259 [details] [diff] [review]
1199090_2.patch

- actually use the passed param
- simplify code generation
Comment 4 User image Dylan Hardison [:dylan] 2015-09-28 09:39:37 PDT
Comment on attachment 8665259 [details] [diff] [review]
1199090_2.patch

Review of attachment 8665259 [details] [diff] [review]:
-----------------------------------------------------------------

r=dylan
Comment 5 User image Byron Jones ‹:glob› 2015-09-29 07:57:27 PDT
To ssh://gitolite3@git.mozilla.org/webtools/bmo/bugzilla.git
   87c32cb..05fed61  master -> master

Note You need to log in before you can comment on or make changes to this bug.