Allow Admin to filter users by 2FA status

Closes internal https://dev.gitlab.org/gitlab/gitlabhq/issues/2414

Reassigned to @DouweM

@DouweM Another one for quick review and merge.

Added 1 commit:

• d3ff8c1a - Make default value for otp_required_for_login false instead of null

@rspeicher Great—you've confirmed the migration works?

Thanks @razer6

Without looking at any of the code I'm surprised this needs a db migration @dzaporozhets

@sytses otp_required_for_login is a boolean, but used to default to null, which would function the same as false. We'd rather have it be either false or true all the time.

@rspeicher It will convert all null to false for existing records, right?

Yes, it just kind of improving db structure quality.

@DouweM @dzaporozhets I confirmed the migration works (up and down) and converts existing null records to false on postgres; I'll do a quick check on MySQL.

Added 1 commit:

• aedb5469 - Correct AddDefaultOtpRequiredForLoginValue migration

Yes, it just kind of improving db structure quality.

This was part of it, but without the migration the scope where(otp_required_for_login: false) wouldn't return all users without 2FA enabled. It would only return those who enabled it and then disabled it, which is why I had the non-ARel raw SQL in there before.

@rspeicher you can do where(otp_required_for_login: [false, nil]) and it would work without db migration

But your solution is cleaner

@dzaporozhets Fixed migration for MySQL, should be good to merge.

you can do where(otp_required_for_login: [false, nil]) and it would work without db migration

I avoided doing that because I thought it would use an IN clause, but I just tested and ActiveRecord was smart enough to just use an OR clause, so that's cool. But I think I prefer the migration route for better integrity.

Added 1 commit:

• 087919d4 - Use alias_attribute to define User#two_factor_enabled

Discovered Rails' alias_attribute method, which made this quite a bit cleaner!

Reassigned to @rspeicher