Require email verification

What does this MR do and why?

What When trying to log in from an unknown IP address or after 3 or more failed login attempts, require email verification.

Why In order to reduce the amount of account takeovers

Specifications:

• require email verification after signing in:
  • when 3 or more failed login attempts are made within the last 24 hours, or
  • when logging in from an unknown ip address, unless it is the first time the user signs in
• do not require email verification if two factor authentication is enabled
• an email containing a six digit code is send to the user
• the verification code expires after 60 minutes or when successfully used
• the user can request a new verification code
• a successfully verified page is shown with an automatic redirect after 3 seconds to the requested page

This is different from the current Devise Lockable implementation in the following ways:

• reduced number of maximum failed login attempts from 5 to 3
• timespan for counting failed login attempts increased from 10 minutes to 24 hours
• additionally locks the account when logging in from an unknown ip address, unless its the user's first login
• send an email with a memorizable verification code, instead of an email with an unlock link
• automatically log the user in, instead of letting them log in manually after unlocking
• add an auto-redirecting successfully verified page, instead of displaying an alert on the sign-in page

Because this feature adapts Devise's behavior, it can be expected to leave users in an inconsistent state when toggling the feature flag:

1. when the FF is toggled on after Devise sent unlock instructions:

• good: when clicking the unlock link from the email it will work as expected and the user will notice nothing
• bad: when the user tries to log in with correct credentials, before clicking the unlock link:
  • they are prompted for a token which they were never sent
  • after then clicking the unlock link and signing in, they are redirected to the successfully verified page

2. when the FF is toggled off after being prompted for a verification token:

• good: when staying on the same page, the token will be accepted and the user will notice nothing
• bad: when they close the page and try to login again with correct credentials, there is no way to enter the code from the email and they will keep getting an invalid login message for 10 minutes, at which point the account will unlock automatically.

Issue: https://gitlab.com/gitlab-org/modelops/anti-abuse/pipeline-validation-service/-/issues/100/

Screenshots or screen recordings

Screen recording

required_email_verification

Screenshots

Pages

Verification form	Successfully verified page

Error messages

Input validation error	Rate limited	Code incorrect	Code expired

Emails

E-mail (HTML)	E-mail (TEXT)

How to set up and validate locally

1. Enable the feature flag in rails console

   Feature.enable(:require_email_verification)

2. Sign out from dev environment
3. Sign in 3 times with a wrong password
4. Sign in with your correct password
5. Copy the code from the verification email (check http://localhost:3000/rails/letter_opener/)
6. Paste the code in the verification form

Database

Queries

2 new queries introduced in AuthenticationEvent.initial_login_or_known_ip_address?(user, ip_address):

1. AuthenticationEvent.where(user_id: user).exists? (Postgres.ai)

Raw SQL

SELECT
  1 AS one
FROM
  "authentication_events"
WHERE
  "authentication_events"."user_id" = 4018056
LIMIT 1

Plan

 Limit  (cost=0.57..0.61 rows=1 width=4) (actual time=2.008..2.010 rows=1 loops=1)
   Buffers: shared hit=7 read=1
   I/O Timings: read=1.923 write=0.000
   ->  Index Only Scan using index_authentication_events_on_user_id on public.authentication_events  (cost=0.57..5.42 rows=106 width=4) (actual time=2.006..2.006 rows=1 loops=1)
         Index Cond: (authentication_events.user_id = 4018056)
         Heap Fetches: 0
         Buffers: shared hit=7 read=1
         I/O Timings: read=1.923 write=0.000

Timing

Time: 2.273 ms
  - planning: 0.230 ms
  - execution: 2.043 ms
    - I/O read: 1.923 ms
    - I/O write: 0.000 ms

Shared buffers:
  - hits: 7 (~56.00 KiB) from the buffer pool
  - reads: 1 (~8.00 KiB) from the OS file cache, including disk I/O
  - dirtied: 0
  - writes: 0

Conclusion: no index needed.

2. AuthenticationEvent.where(user_id: user, ip_address: ip_address).success.exists? (Postgres.ai)

Raw SQL

SELECT
  1 AS one
FROM
  "authentication_events"
WHERE
  "authentication_events"."user_id" = 4018056
  AND "authentication_events"."ip_address" = '85.144.207.147'
  AND "authentication_events"."result" = 1
LIMIT 1

Plan

 Limit  (cost=0.57..143.36 rows=1 width=4) (actual time=11.258..11.260 rows=1 loops=1)
   Buffers: shared hit=3 read=6
   I/O Timings: read=11.127 write=0.000
   ->  Index Scan using index_authentication_events_on_user_id on public.authentication_events  (cost=0.57..143.36 rows=1 width=4) (actual time=11.254..11.255 rows=1 loops=1)
         Index Cond: (authentication_events.user_id = 4018056)
         Filter: ((authentication_events.ip_address = '85.144.207.147'::inet) AND (authentication_events.result = 1))
         Rows Removed by Filter: 1
         Buffers: shared hit=3 read=6
         I/O Timings: read=11.127 write=0.000

Timing

Time: 12.739 ms
  - planning: 1.448 ms
  - execution: 11.291 ms
    - I/O read: 11.127 ms
    - I/O write: 0.000 ms

Shared buffers:
  - hits: 3 (~24.00 KiB) from the buffer pool
  - reads: 6 (~48.00 KiB) from the OS file cache, including disk I/O
  - dirtied: 0
  - writes: 0

Conclusion: index needed.

Plan and Timing after index creation:

Plan

 Limit  (cost=0.57..3.59 rows=1 width=4) (actual time=0.475..0.476 rows=1 loops=1)
   Buffers: shared hit=7 read=4
   I/O Timings: read=0.341 write=0.000
   ->  Index Only Scan using index_authentication_events_on_user_and_ip_address_and_result on public.authentication_events  (cost=0.57..3.59 rows=1 width=4) (actual time=0.472..0.473 rows=1 loops=1)
         Index Cond: ((authentication_events.user_id = 4018056) AND (authentication_events.ip_address = '85.144.207.147'::inet) AND (authentication_events.result = 1))
         Heap Fetches: 0
         Buffers: shared hit=7 read=4
         I/O Timings: read=0.341 write=0.000

Timing

Time: 2.039 ms
  - planning: 1.529 ms
  - execution: 0.510 ms
    - I/O read: 0.341 ms
    - I/O write: 0.000 ms

Shared buffers:
  - hits: 7 (~56.00 KiB) from the buffer pool
  - reads: 4 (~32.00 KiB) from the OS file cache, including disk I/O
  - dirtied: 0
  - writes: 0

Migration

Timing: < 10 minutes for concurrent index creation, see here.

rails db:migrate:up

main: == 20220601152916 AddUserIdAndIpAddressSuccessIndexToAuthenticationEvents: migrating
main: -- transaction_open?()
main:    -> 0.0000s
main: -- index_exists?(:authentication_events, [:user_id, :ip_address, :result], {:name=>"index_authentication_events_on_user_and_ip_address_and_result", :algorithm=>:concurrently})
main:    -> 0.0063s
main: -- execute("SET statement_timeout TO 0")
main:    -> 0.0004s
main: -- add_index(:authentication_events, [:user_id, :ip_address, :result], {:name=>"index_authentication_events_on_user_and_ip_address_and_result", :algorithm=>:concurrently})
main:    -> 0.0026s
main: -- execute("RESET statement_timeout")
main:    -> 0.0003s
main: -- transaction_open?()
main:    -> 0.0000s
main: -- indexes(:authentication_events)
main:    -> 0.0088s
main: -- remove_index(:authentication_events, {:algorithm=>:concurrently, :name=>"index_authentication_events_on_user_id"})
main:    -> 0.0045s
main: == 20220601152916 AddUserIdAndIpAddressSuccessIndexToAuthenticationEvents: migrated (0.0312s)

rails db:migrate:down

main: == 20220601152916 AddUserIdAndIpAddressSuccessIndexToAuthenticationEvents: reverting
main: -- transaction_open?()
main:    -> 0.0000s
main: -- index_exists?(:authentication_events, :user_id, {:name=>"index_authentication_events_on_user_id", :algorithm=>:concurrently})
main:    -> 0.0065s
main: -- execute("SET statement_timeout TO 0")
main:    -> 0.0003s
main: -- add_index(:authentication_events, :user_id, {:name=>"index_authentication_events_on_user_id", :algorithm=>:concurrently})
main:    -> 0.0032s
main: -- execute("RESET statement_timeout")
main:    -> 0.0004s
main: -- transaction_open?()
main:    -> 0.0000s
main: -- indexes(:authentication_events)
main:    -> 0.0035s
main: -- remove_index(:authentication_events, {:algorithm=>:concurrently, :name=>"index_authentication_events_on_user_and_ip_address_and_result"})
main:    -> 0.0026s
main: == 20220601152916 AddUserIdAndIpAddressSuccessIndexToAuthenticationEvents: reverted (0.0260s)

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

• I have evaluated the MR acceptance checklist for this MR.