Skip to content

Refactor 2FA query methods on `User`

Manoj M J requested to merge mmj-missing-refactor into master

What does this MR do and why?

This MR makes a minor refactor in queries that are used to retrieve users with and without 2FA enabled.

  • Rails 6.1+ supports finding records with missing associated records natively using .where.missing(...), which is being made use of in refactoring User.without_two_factor. (Details on where.missing(...) method is here)
  • In User.with_two_factor, I noticed that we have a chance to rewrite the query such that we do not have to write much of raw SQL.

The queries generated after the change are the same, so there is no change in the query plans or timing, but I have included details of the query plan before & after the change anyway.

No changelog added as this is only a refactor.

Change in queries

  User.without_two_factor

Before

Query plan

SELECT
   "users".* 
FROM
   "users" 
   LEFT OUTER JOIN
      u2f_registrations AS u2f 
      ON u2f.user_id = users.id 
   LEFT OUTER JOIN
      webauthn_registrations AS webauthn 
      ON webauthn.user_id = users.id 
WHERE
   (
      u2f.id IS NULL 
      AND webauthn.id IS NULL 
      AND users.otp_required_for_login = FALSE
   )

After

Query plan

SELECT
   "users".* 
FROM
   "users" 
   LEFT OUTER JOIN
      "u2f_registrations" 
      ON "u2f_registrations"."user_id" = "users"."id" 
   LEFT OUTER JOIN
      "webauthn_registrations" 
      ON "webauthn_registrations"."user_id" = "users"."id" 
WHERE
   "u2f_registrations"."id" IS NULL 
   AND "webauthn_registrations"."id" IS NULL 
   AND "users"."otp_required_for_login" = FALSE

Diff is available here

  User.with_two_factor

Before

Query plan

SELECT
   "users".* 
FROM
   "users" 
WHERE
   (
      EXISTS 
      (
         SELECT
            * 
         FROM
            u2f_registrations AS u2f 
         WHERE
            u2f.user_id = users.id 
      )
      OR users.otp_required_for_login = TRUE 
      OR EXISTS 
      (
         SELECT
            * 
         FROM
            webauthn_registrations AS webauthn 
         WHERE
            webauthn.user_id = users.id 
      )
   )

After

Query plan

SELECT
   "users".* 
FROM
   "users" 
WHERE
   (
      "users"."otp_required_for_login" = TRUE 
      OR EXISTS 
      (
         SELECT
            1 
         FROM
            "u2f_registrations" 
         WHERE
            "u2f_registrations"."user_id" = "users"."id"
      )
      OR EXISTS 
      (
         SELECT
            1 
         FROM
            "webauthn_registrations" 
         WHERE
            "webauthn_registrations"."user_id" = "users"."id"
      )
   )

Diff is available here

Screenshots or screen recordings

These are strongly recommended to assist reviewers and reduce the time to merge your change.

How to set up and validate locally

Numbered steps to set up and validate the change are strongly suggested.

MR acceptance checklist

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

Edited by Manoj M J

Merge request reports

Loading