Skip to content

authz: pure-SQL approach

Warren Gifford requested to merge jc/perms-pure-SQL-authz into main

Created by: unknwon

Goal

Completely remove the application-level calls of authzFilter, and enforce repository permissions at SQL level.

Design

The basic idea is very simple, adds an extra column external_services.unrestricted to indicate whether access to repositories belong to the external service is unrestricted (i.e. everyone has access), and if not unrestricted, then check permissions stored in the user_permissions table (which contains a list of repo IDs a given user has access to).

When permissions user mapping is enabled, we ignore the value of external_services.unrestricted and only check the user_permissions table.

Note that we do not enforce permissions for non-private repositories, and site admins and internal actors bypass the permissions check entirely.

FAQs

  • Why use unrestricted not the opposite restricted?
    • It is not always safe to assume a repository can be read by everyone, would rather blocking access by default.
  • Why not just use the private column?
    • Repository permissions are not always enforced based on visibility. For example, when there is no "authorization" field in the external service config, all repositories added by the external service do not enforce permissions from the code host.

Performance

(This is a SQL generated by campaigns code by plugging in the new authz query conditions)

SET enable_seqscan = OFF;

EXPLAIN ANALYZE

-- source: enterprise/internal/campaigns/store_changeset_specs.go:ListChangesetSpecs
SELECT changeset_specs.id, changeset_specs.rand_id, changeset_specs.raw_spec, changeset_specs.spec, changeset_specs.campaign_spec_id, changeset_specs.repo_id, changeset_specs.user_id, changeset_specs.diff_stat_added, changeset_specs.diff_stat_changed, changeset_specs.diff_stat_deleted, changeset_specs.created_at, changeset_specs.updated_at
FROM changeset_specs
INNER JOIN repo ON repo.id = changeset_specs.repo_id
WHERE changeset_specs.id >= 0
 AND  repo.deleted_at IS NULL
 AND  (
	false									-- TRUE or FALSE to indicate whether to bypass the check
OR	(
		NOT	false							-- Disregard unrestricted state when permissions user mapping is enabled
		AND (
				NOT repo.private            -- Happy path of non-private repositories
				OR(                         -- Each external service defines if repositories are unrestricted
					SELECT COUNT(*) FROM external_services AS es
					JOIN external_service_repos AS esr ON esr.external_service_id = es.id
					WHERE
						esr.repo_id = repo.id
					AND	es.unrestricted = TRUE
					AND	es.deleted_at IS NULL
				) > 0
			)
	)
OR	(										-- Restricted repositories require checking permissions
		SELECT object_ids_ints
		FROM user_permissions
		WHERE
			user_id = 1
		AND	permission = 'read'
		AND	object_type = 'repos'
	) @> INTSET(repo.id)
)
 AND  changeset_specs.campaign_spec_id = 2
ORDER BY changeset_specs.id ASC;

Query Plan:

Nested Loop  (cost=8.44..653.95 rows=1 width=207) (actual time=0.013..0.077 rows=20 loops=1)
  InitPlan 2 (returns $1)
    ->  Index Scan using user_permissions_perm_object_unique on user_permissions  (cost=0.13..8.15 rows=1 width=13) (actual time=0.006..0.006 rows=1 loops=1)
"          Index Cond: ((user_id = 1) AND (permission = 'read'::text) AND (object_type = 'repos'::text))"
  ->  Index Scan using changeset_specs_pkey on changeset_specs  (cost=0.14..13.68 rows=21 width=207) (actual time=0.007..0.013 rows=21 loops=1)
        Index Cond: (id >= 0)
        Filter: (campaign_spec_id = 2)
  ->  Index Scan using repo_pkey on repo  (cost=0.14..29.75 rows=1 width=4) (actual time=0.003..0.003 rows=1 loops=21)
        Index Cond: (id = changeset_specs.repo_id)
        Filter: ((deleted_at IS NULL) AND ((NOT private) OR ((SubPlan 1) > 0) OR ($1 @> intset(id))))
        Rows Removed by Filter: 0
        SubPlan 1
          ->  Aggregate  (cost=26.14..26.15 rows=1 width=8) (actual time=0.025..0.026 rows=1 loops=1)
                ->  Nested Loop  (cost=4.33..26.14 rows=1 width=0) (actual time=0.024..0.024 rows=0 loops=1)
                      Join Filter: (es.id = esr.external_service_id)
                      Rows Removed by Join Filter: 4
                      ->  Bitmap Heap Scan on external_service_repos esr  (cost=4.20..13.67 rows=6 width=8) (actual time=0.011..0.011 rows=1 loops=1)
                            Recheck Cond: (repo_id = repo.id)
                            Heap Blocks: exact=1
                            ->  Bitmap Index Scan on external_service_repos_repo_id  (cost=0.00..4.20 rows=6 width=0) (actual time=0.006..0.006 rows=4 loops=1)
                                  Index Cond: (repo_id = repo.id)
                      ->  Materialize  (cost=0.14..12.29 rows=2 width=8) (actual time=0.007..0.010 rows=4 loops=1)
                            ->  Index Scan using external_services_pkey on external_services es  (cost=0.14..12.29 rows=2 width=8) (actual time=0.005..0.007 rows=4 loops=1)
                                  Filter: (unrestricted AND (deleted_at IS NULL))
                                  Rows Removed by Filter: 6
Planning Time: 0.279 ms
Execution Time: 0.121 ms

Before merge

  • We have phased out binary format.

Post merge

Fixes #11767 (closed)

Merge request reports

Loading