Fix 500 error on IP restrictions when displaying snippet names
What does this MR do and why?
Related to: Reinstate bypass of IP restriction allowing acc... (#511506 - closed)
My previous MR aimed to prevent users from accessing snippet names when their IP doesn't fall within the group's IP restriction range. However, it was reverted due to 500 server errors occurring when IP ranges didn't conform to PostgreSQL's CIDR format requirements.
This MR uses PostgreSQL's INET type instead of CIDR type for IP range validation
The INET type is more lenient and properly handles all valid IP ranges, including those that don't strictly conform to CIDR format (e.g., 1.1.1.1/22)
The Query Plans
- The first run without a specific current user's IP: 37.977 ms
- The second run with a current user's IP: 21.496 ms
Sort (cost=7762.79..7762.81 rows=7 width=2122) (actual time=13.275..13.294 rows=51 loops=1)
Sort Key: snippets.id DESC
Sort Method: quicksort Memory: 75kB
Buffers: shared hit=1787 read=11
I/O Timings: read=10.967 write=0.000
-> Nested Loop (cost=985.11..7762.69 rows=7 width=2122) (actual time=12.816..13.158 rows=51 loops=1)
Buffers: shared hit=1784 read=11
I/O Timings: read=10.967 write=0.000
-> HashAggregate (cost=467.02..467.16 rows=14 width=4) (actual time=0.691..0.709 rows=51 loops=1)
Group Key: snippets_1.id
Buffers: shared hit=613
I/O Timings: read=0.000 write=0.000
-> Append (cost=0.42..466.99 rows=14 width=4) (actual time=0.014..0.668 rows=65 loops=1)
Buffers: shared hit=613
I/O Timings: read=0.000 write=0.000
-> Index Scan using index_snippets_on_author_id on public.snippets snippets_1 (cost=0.42..95.02 rows=10 width=4) (actual time=0.013..0.130 rows=33 loops=1)
Index Cond: (snippets_1.author_id = 64248)
Filter: ((snippets_1.project_id IS NULL) AND (snippets_1.visibility_level = ANY ('{10,20}'::integer[])))
Rows Removed by Filter: 24
Buffers: shared hit=60
I/O Timings: read=0.000 write=0.000
-> Nested Loop (cost=1.42..132.07 rows=3 width=4) (actual time=0.044..0.217 rows=14 loops=1)
Buffers: shared hit=210
I/O Timings: read=0.000 write=0.000
-> Nested Loop (cost=0.86..129.67 rows=2 width=12) (actual time=0.033..0.133 rows=16 loops=1)
Buffers: shared hit=130
I/O Timings: read=0.000 write=0.000
-> Index Scan using index_snippets_on_author_id on public.snippets snippets_2 (cost=0.42..95.02 rows=17 width=8) (actual time=0.007..0.042 rows=49 loops=1)
Index Cond: (snippets_2.author_id = 64248)
Filter: (snippets_2.visibility_level = ANY ('{10,20}'::integer[]))
Rows Removed by Filter: 8
Buffers: shared hit=60
I/O Timings: read=0.000 write=0.000
-> Index Only Scan using index_projects_on_id_partial_for_visibility on public.projects (cost=0.43..2.04 rows=1 width=4) (actual time=0.001..0.001 rows=0 loops=49)
Index Cond: (projects.id = snippets_2.project_id)
Heap Fetches: 12
Buffers: shared hit=70
I/O Timings: read=0.000 write=0.000
-> Index Scan using index_project_features_on_project_id on public.project_features (cost=0.57..1.20 rows=1 width=4) (actual time=0.005..0.005 rows=1 loops=16)
Index Cond: (project_features.project_id = projects.id)
Filter: (project_features.snippets_access_level = ANY ('{20,30}'::integer[]))
Rows Removed by Filter: 0
Buffers: shared hit=80
I/O Timings: read=0.000 write=0.000
-> Nested Loop (cost=2.13..239.82 rows=1 width=4) (actual time=0.056..0.311 rows=18 loops=1)
Buffers: shared hit=343
I/O Timings: read=0.000 write=0.000
-> Nested Loop (cost=1.57..239.16 rows=1 width=16) (actual time=0.048..0.239 rows=19 loops=1)
Buffers: shared hit=248
I/O Timings: read=0.000 write=0.000
-> Nested Loop Semi Join (cost=1.00..237.10 rows=1 width=12) (actual time=0.027..0.133 rows=19 loops=1)
Buffers: shared hit=144
I/O Timings: read=0.000 write=0.000
-> Index Scan using index_snippets_on_author_id on public.snippets snippets_3 (cost=0.42..94.87 rows=62 width=8) (actual time=0.007..0.040 rows=57 loops=1)
Index Cond: (snippets_3.author_id = 64248)
Buffers: shared hit=60
I/O Timings: read=0.000 write=0.000
-> Index Only Scan using project_authorizations_pkey on public.project_authorizations (cost=0.58..2.27 rows=1 width=4) (actual time=0.001..0.001 rows=0 loops=57)
Index Cond: ((project_authorizations.user_id = 21572755) AND (project_authorizations.project_id = snippets_3.project_id))
Heap Fetches: 1
Buffers: shared hit=84
I/O Timings: read=0.000 write=0.000
-> Index Only Scan using projects_pkey on public.projects projects_1 (cost=0.56..2.05 rows=1 width=4) (actual time=0.005..0.005 rows=1 loops=19)
Index Cond: (projects_1.id = project_authorizations.project_id)
Heap Fetches: 15
Buffers: shared hit=104
I/O Timings: read=0.000 write=0.000
-> Index Scan using index_project_features_on_project_id on public.project_features project_features_1 (cost=0.57..0.66 rows=1 width=4) (actual time=0.003..0.003 rows=1 loops=19)
Index Cond: (project_features_1.project_id = projects_1.id)
Filter: (project_features_1.snippets_access_level = ANY ('{20,30,10}'::integer[]))
Rows Removed by Filter: 0
Buffers: shared hit=95
I/O Timings: read=0.000 write=0.000
-> Index Scan using index_snippets_on_id_and_type on public.snippets (cost=518.09..521.11 rows=1 width=2122) (actual time=0.243..0.243 rows=1 loops=51)
Index Cond: (snippets.id = snippets_1.id)
Filter: (NOT (hashed SubPlan 1))
Rows Removed by Filter: 0
Buffers: shared hit=1171 read=11
I/O Timings: read=10.967 write=0.000
SubPlan 1
-> Nested Loop Anti Join (cost=468.86..517.66 rows=1 width=4) (actual time=12.094..12.103 rows=0 loops=1)
Buffers: shared hit=967 read=11
I/O Timings: read=10.967 write=0.000
-> Nested Loop (cost=468.86..517.65 rows=1 width=4) (actual time=12.093..12.102 rows=0 loops=1)
Buffers: shared hit=967 read=11
I/O Timings: read=10.967 write=0.000
-> Nested Loop (cost=468.58..515.52 rows=1 width=12) (actual time=6.374..12.045 rows=18 loops=1)
Buffers: shared hit=931 read=11
I/O Timings: read=10.967 write=0.000
-> Nested Loop (cost=468.01..511.94 rows=6 width=8) (actual time=0.527..0.880 rows=18 loops=1)
Buffers: shared hit=867
I/O Timings: read=0.000 write=0.000
-> Nested Loop (cost=467.45..495.89 rows=6 width=8) (actual time=0.517..0.782 rows=18 loops=1)
Buffers: shared hit=777
I/O Timings: read=0.000 write=0.000
-> HashAggregate (cost=467.02..467.16 rows=14 width=4) (actual time=0.487..0.508 rows=51 loops=1)
Group Key: snippets_5.id
Buffers: shared hit=613
I/O Timings: read=0.000 write=0.000
-> Append (cost=0.42..466.99 rows=14 width=4) (actual time=0.010..0.471 rows=65 loops=1)
Buffers: shared hit=613
I/O Timings: read=0.000 write=0.000
-> Index Scan using index_snippets_on_author_id on public.snippets snippets_5 (cost=0.42..95.02 rows=10 width=4) (actual time=0.010..0.043 rows=33 loops=1)
Index Cond: (snippets_5.author_id = 64248)
Filter: ((snippets_5.project_id IS NULL) AND (snippets_5.visibility_level = ANY ('{10,20}'::integer[])))
Rows Removed by Filter: 24
Buffers: shared hit=60
I/O Timings: read=0.000 write=0.000
-> Nested Loop (cost=1.42..132.07 rows=3 width=4) (actual time=0.032..0.163 rows=14 loops=1)
Buffers: shared hit=210
I/O Timings: read=0.000 write=0.000
-> Nested Loop (cost=0.86..129.67 rows=2 width=12) (actual time=0.023..0.110 rows=16 loops=1)
Buffers: shared hit=130
I/O Timings: read=0.000 write=0.000
-> Index Scan using index_snippets_on_author_id on public.snippets snippets_6 (cost=0.42..95.02 rows=17 width=8) (actual time=0.012..0.047 rows=49 loops=1)
Index Cond: (snippets_6.author_id = 64248)
Filter: (snippets_6.visibility_level = ANY ('{10,20}'::integer[]))
Rows Removed by Filter: 8
Buffers: shared hit=60
I/O Timings: read=0.000 write=0.000
-> Index Only Scan using index_projects_on_id_partial_for_visibility on public.projects projects_3 (cost=0.43..2.04 rows=1 width=4) (actual time=0.001..0.001 rows=0 loops=49)
Index Cond: (projects_3.id = snippets_6.project_id)
Heap Fetches: 12
Buffers: shared hit=70
I/O Timings: read=0.000 write=0.000
-> Index Scan using index_project_features_on_project_id on public.project_features project_features_2 (cost=0.57..1.20 rows=1 width=4) (actual time=0.003..0.003 rows=1 loops=16)
Index Cond: (project_features_2.project_id = projects_3.id)
Filter: (project_features_2.snippets_access_level = ANY ('{20,30}'::integer[]))
Rows Removed by Filter: 0
Buffers: shared hit=80
I/O Timings: read=0.000 write=0.000
-> Nested Loop (cost=2.13..239.82 rows=1 width=4) (actual time=0.042..0.256 rows=18 loops=1)
Buffers: shared hit=343
I/O Timings: read=0.000 write=0.000
-> Nested Loop (cost=1.57..239.16 rows=1 width=16) (actual time=0.033..0.193 rows=19 loops=1)
Buffers: shared hit=248
I/O Timings: read=0.000 write=0.000
-> Nested Loop Semi Join (cost=1.00..237.10 rows=1 width=12) (actual time=0.021..0.124 rows=19 loops=1)
Buffers: shared hit=144
I/O Timings: read=0.000 write=0.000
-> Index Scan using index_snippets_on_author_id on public.snippets snippets_7 (cost=0.42..94.87 rows=62 width=8) (actual time=0.008..0.039 rows=57 loops=1)
Index Cond: (snippets_7.author_id = 64248)
Buffers: shared hit=60
I/O Timings: read=0.000 write=0.000
-> Index Only Scan using project_authorizations_pkey on public.project_authorizations project_authorizations_1 (cost=0.58..2.27 rows=1 width=4) (actual time=0.001..0.001 rows=0 loops=57)
Index Cond: ((project_authorizations_1.user_id = 21572755) AND (project_authorizations_1.project_id = snippets_7.project_id))
Heap Fetches: 1
Buffers: shared hit=84
I/O Timings: read=0.000 write=0.000
-> Index Only Scan using projects_pkey on public.projects projects_4 (cost=0.56..2.05 rows=1 width=4) (actual time=0.003..0.003 rows=1 loops=19)
Index Cond: (projects_4.id = project_authorizations_1.project_id)
Heap Fetches: 15
Buffers: shared hit=104
I/O Timings: read=0.000 write=0.000
-> Index Scan using index_project_features_on_project_id on public.project_features project_features_3 (cost=0.57..0.66 rows=1 width=4) (actual time=0.003..0.003 rows=1 loops=19)
Index Cond: (project_features_3.project_id = projects_4.id)
Filter: (project_features_3.snippets_access_level = ANY ('{20,30,10}'::integer[]))
Rows Removed by Filter: 0
Buffers: shared hit=95
I/O Timings: read=0.000 write=0.000
-> Index Only Scan using index_snippet_on_id_and_project_id on public.snippets snippets_4 (cost=0.42..2.05 rows=1 width=8) (actual time=0.005..0.005 rows=0 loops=51)
Index Cond: ((snippets_4.id = snippets_5.id) AND (snippets_4.project_id IS NOT NULL))
Heap Fetches: 0
Buffers: shared hit=164
I/O Timings: read=0.000 write=0.000
-> Index Scan using projects_pkey on public.projects projects_2 (cost=0.56..2.67 rows=1 width=8) (actual time=0.005..0.005 rows=1 loops=18)
Index Cond: (projects_2.id = snippets_4.project_id)
Buffers: shared hit=90
I/O Timings: read=0.000 write=0.000
-> Index Only Scan using index_namespaces_on_type_and_id on public.namespaces (cost=0.56..0.60 rows=1 width=4) (actual time=0.619..0.619 rows=1 loops=18)
Index Cond: ((namespaces.type = 'Group'::text) AND (namespaces.id = projects_2.namespace_id))
Heap Fetches: 0
Buffers: shared hit=64 read=11
I/O Timings: read=10.967 write=0.000
-> Index Only Scan using index_ip_restrictions_on_group_id on public.ip_restrictions (cost=0.29..1.46 rows=67 width=4) (actual time=0.003..0.003 rows=0 loops=18)
Index Cond: (ip_restrictions.group_id = namespaces.id)
Heap Fetches: 0
Buffers: shared hit=36
I/O Timings: read=0.000 write=0.000
-> Result (cost=0.00..0.00 rows=0 width=0) (actual time=0.000..0.000 rows=0 loops=0)
I/O Timings: read=0.000 write=0.000
Settings: seq_page_cost = '4', work_mem = '100MB', effective_cache_size = '472585MB', jit = 'off', random_page_cost = '1.5'Time: 37.977 ms
- planning: 24.259 ms
- execution: 13.718 ms
- I/O read: 10.967 ms
- I/O write: 0.000 ms
Shared buffers:
- hits: 1787 (~14.00 MiB) from the buffer pool
- reads: 11 (~88.00 KiB) from the OS file cache, including disk I/O
- dirtied: 0
- writes: 0MR acceptance checklist
Please evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.
The UI testing
Just to clarify: During testing, I referred to one group as the 'invalid group' because its IP range (1.1.1.1/22) isn't valid for PostgreSQL's cidr type. However, the range is actually valid in networking terms and works fine with the inet type. (1.1.1.1/22) triggered 500 server error when the sql query used cidr and I changed it to inet in this MR
**Test scenarios: ** 5 Groups: no restriction group / Invalid group / valid group / multi-restriction group / the specific restriction group
- When the user's IP doesn't fall within any group with restrictions -- It only shows no restriction group snippet.
- Invalid group IP range: 1.1.1.1/22
- valid group IP range: 1.1.0.0/22
- the user's IP: 2.2.2.2
- When the user's IP falls within both valid + invalid group range -- It shows both group snippet + no restriction group snippet.
- Invalid group IP range: 1.1.1.1/22
- valid group IP range: 1.1.0.0/22
- the user's IP: 1.1.1.100
- When the user's IP falls only within the invalid group -- It only shows invalid group snippet + no restriction group snippet.
- Invalid group IP range: 1.1.1.1/22
- valid group IP range: 10.0.0.0/22
- the user's IP: 1.1.1.100
- When the user's IP falls only within the valid group -- It only shows valid group snippet + no restriction group snippet.
- Invalid group IP range: 1.1.1.1/22
- valid group IP range: 10.0.0.0/22
- the user's IP: 10.0.1.100
- When the user's IP falls only within the group with multi-restrictions -- It only shows multi-restriction group snippet + no restriction group snippet.
- Invalid group IP range: 1.1.1.1/22
- valid group IP range: 10.0.0.0/22
- multi-restriction group IP range: [20.0.0.0/22, 30.0.0.0/22, 40.0.0.0/22]
- the user's IP: 30.0.1.100
- When the user's IP falls a specific group restriction -- It only shows the specific group snippet + no restriction group snippet.
- Invalid group IP range: 1.1.1.1/22
- valid group IP range: 10.0.0.0/22
- multi-restriction group IP range: [20.0.0.0/22, 30.0.0.0/22, 40.0.0.0/22]
- the specific restriction group: 50.0.0.1
- the user's IP: 50.0.0.1
| 1. Test scenario 1 | 2. Test scenario 2 |
|---|---|
| test1 | test2 |
| 3. Test scenario 3 | 4. Test scenario 4 |
| ------ | ------ |
| test3 | test4 |
| 5.Test scenario 5 | 6.Test scenario 6 |
| ------ | ------ |
| test5 | test6 |
How to set up and validate locally
Scenario:
1. Actors:
User A - private group A(PoC) owner with Gitlab public profile
User B - private group A(PoC) member e.g. role Guest
2. Steps:
- User A - create the private project AP in PoC group A
- User A - create a private snippet APS_1 in the PoC project AP
- User A - in the PoC group A go to
Settings->General->Permissions and group featuresand setRestrict access by IP addressto specified ip address - User B - try to access to the PoC group A from another ip address than specified - you should see 404 not found - expected behaviour
- User B - go to User A public Gitlab profile then open tab
Snippets- notice that you see there the snippet APS_1 - Bypassing "Restrict access by IP address" to view snippets names of the restricted group projects - User A - now, in the project AP create the new private snippet APS_2
- User B - refresh the tab
Snippetsof the public profile of User A - notice that you also see the snippet APS_2 there - Bypassing "Restrict access by IP address" to view snippets names of the restricted group projects
Edited by Emma Park