Skip to content

Resolve "User stuck in 2FA setup page even if group disable 2FA enforce"

Gosia Ksionek requested to merge 220433-user-stuck-in-2fa-setup-page-even-if-group-disable-2fa-enforce-migration into master

What does this MR do?

In #220433 (closed) was shown a problem, that was fixed in !46432 (merged), but we need also to take care of records in the database that needs fixing. Here is prepared migration, that picks users which have a field require_two_factor_authentication_from_group set to true, even though they don't belong to any group that requires it, and updates require_two_factor_authentication_from_group field on this records.

Screenshots (strongly suggested)

== 20201030121314 ScheduleUpdateExistingUsersThatRequireTwoFactorAuth: migrating 
-- transaction_open?()
   -> 0.0000s
-- index_exists?(:users, :require_two_factor_authentication_from_group, {:where=>"require_two_factor_authentication_from_group = TRUE", :name=>"index_users_on_require_two_factor_authentication_from_group", :algorithm=>:concurrently})
   -> 0.0094s
== 20201030121314 ScheduleUpdateExistingUsersThatRequireTwoFactorAuth: migrated (0.0807s) 
explain SELECT users.* FROM users WHERE users.require_two_factor_authentication_from_group = TRUE

Index Scan using index_users_on_require_two_factor_authentication_from_group on public.users  (cost=0.43..23950.81 rows=83475 width=4) (actual time=3.949..31893.991 rows=84127 loops=1)
   Index Cond: (users.require_two_factor_authentication_from_group = true)
   Filter: (users.require_two_factor_authentication_from_group IS TRUE)
   Rows Removed by Filter: 0
   Buffers: shared hit=12387 read=62987
   I/O Timings: read=31206.404
UPDATE
          users
        SET
          require_two_factor_authentication_from_group = FALSE
        WHERE
          users.id BETWEEN 1
          AND 1000
          AND users.require_two_factor_authentication_from_group = TRUE
          AND users.id NOT IN ( SELECT DISTINCT
              users_groups_query.user_id
            FROM (
              SELECT
                users.id AS user_id,
                members.source_id AS group_ids
              FROM
                users
              LEFT JOIN members ON members.source_type = 'Namespace'
                AND members.requested_at IS NULL
                AND members.user_id = users.id
                AND members.type = 'GroupMember'
            WHERE
              users.require_two_factor_authentication_from_group = TRUE
              AND users.id BETWEEN 1
              AND 1000) AS users_groups_query
            INNER JOIN LATERAL ( WITH RECURSIVE "base_and_ancestors" AS (
        (
                  SELECT
                    "namespaces"."type",
                    "namespaces"."id",
                    "namespaces"."parent_id",
                    "namespaces"."require_two_factor_authentication"
                  FROM
                    "namespaces"
                  WHERE
                    "namespaces"."type" = 'Group'
                    AND "namespaces"."id" = users_groups_query.group_ids)
                UNION (
                  SELECT
                    "namespaces"."type",
                    "namespaces"."id",
                    "namespaces"."parent_id",
                    "namespaces"."require_two_factor_authentication"
                  FROM
                    "namespaces",
                    "base_and_ancestors"
                  WHERE
                    "namespaces"."type" = 'Group'
                    AND "namespaces"."id" = "base_and_ancestors"."parent_id")),
                "base_and_descendants" AS (
        (
                    SELECT
                      "namespaces"."type",
                      "namespaces"."id",
                      "namespaces"."parent_id",
                      "namespaces"."require_two_factor_authentication"
                    FROM
                      "namespaces"
                    WHERE
                      "namespaces"."type" = 'Group'
                      AND "namespaces"."id" = users_groups_query.group_ids)
                  UNION (
                    SELECT
                      "namespaces"."type",
                      "namespaces"."id",
                      "namespaces"."parent_id",
                      "namespaces"."require_two_factor_authentication"
                    FROM
                      "namespaces",
                      "base_and_descendants"
                    WHERE
                      "namespaces"."type" = 'Group'
                      AND "namespaces"."parent_id" = "base_and_descendants"."id"))
                  SELECT
                    "namespaces".*
                  FROM ((
                      SELECT
                        "namespaces"."type",
                        "namespaces"."id",
                        "namespaces"."parent_id",
                        "namespaces"."require_two_factor_authentication"
                      FROM
                        "base_and_ancestors" AS "namespaces"
                      WHERE
                        "namespaces"."type" = 'Group')
                    UNION (
                      SELECT
                        "namespaces"."type",
                        "namespaces"."id",
                        "namespaces"."parent_id",
                        "namespaces"."require_two_factor_authentication"
                      FROM
                        "base_and_descendants" AS "namespaces"
                      WHERE
                        "namespaces"."type" = 'Group')) namespaces
                  WHERE
                    "namespaces"."type" = 'Group'
                    AND "namespaces"."require_two_factor_authentication" = TRUE) AS hierarchy_tree ON TRUE);

https://explain.depesz.com/s/Yjfh

Time: 108.377 ms
  - planning: 2.623 ms
  - execution: 105.754 ms
    - I/O read: 23.515 ms
    - I/O write: 0.000 ms

Shared buffers:
  - hits: 16727 (~130.70 MiB) from the buffer pool
  - reads: 233 (~1.80 MiB) from the OS file cache, including disk I/O
  - dirtied: 0
  - writes: 0

Background Migration Details:

84127 items to iterate batch size = 1000 84127 / 1000 = 85 loops

Estimated times per batch:

  • 109ms for update statement with 1000 items Total: ~110msec per batch

2 mins delay per loop (safe for the given total time per batch)

85 * 2 min = 170 min (2h 50 min) to run all the scheduled jobs

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • Label as security and @ mention @gitlab-com/gl-security/appsec
  • The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • Security reports checked/validated by a reviewer from the AppSec team

Related to #220433 (closed)

Edited by Gosia Ksionek

Merge request reports