authz: pure-SQL approach
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 oppositerestricted
?- 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.
- Repository permissions are not always enforced based on visibility. For example, when there is no
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
-
Remove authzFilter
, its metric and tests #15731 (closed)
Fixes #11767 (closed)