Skip to content

Commit 7c53ced

Browse files
committed
fix(tests): repair CI failures from legacy auth rebuild + cache authorized ids
Move query_registrations import from dojo.authorization.__init__ to apps.py ready() so the per-app auth-filter registry is guaranteed populated by the time any code path imports dojo.authorization.query_filters. The eager package-init import was raising mid-app-load circular ImportErrors that the try/except in each queries.py silently swallowed, leaving callers with the stub get_auth_filter that returns None — every per-model auth filter was bypassed and querysets returned unfiltered results. Cache the authorized product / product-type id sets per request and use set membership in _user_authorized_for instead of repeated authorized_users.filter(pk=user.pk).exists() probes; Alert dispatch skips the user_id FK existence check in clean_fields since the user instance was just fetched from the DB. Unit-test alignments under legacy auth: UserProfileSerializer no longer returns RBAC carriers, UsersTest.deleted_objects is 12 not 13, and the ZAP-import query baselines drop to 1378/1256 (one EXISTS-per-recipient per dispatch goes away, net -7 against pre-Track-B). Integration tests: rewrite product_member / product_type_member flows for the authorized_users add/revoke UI (no per-member role), and delete group_test / product_group_test / product_type_group_test outright — groups don't exist under legacy auth.
1 parent 43e7754 commit 7c53ced

14 files changed

Lines changed: 218 additions & 919 deletions

.github/workflows/integration-tests.yml

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,20 +37,17 @@ jobs:
3737
"tests/finding_extended_test.py",
3838
"tests/finding_group_test.py",
3939
"tests/finding_test.py",
40-
"tests/group_test.py",
4140
"tests/login_test.py",
4241
"tests/metrics_extended_test.py",
4342
"tests/note_type_test.py",
4443
"tests/notes_test.py",
4544
"tests/notification_webhook_test.py",
4645
"tests/notifications_test.py",
4746
"tests/object_test.py",
48-
"tests/product_group_test.py",
4947
"tests/product_member_test.py",
5048
"tests/product_metadata_test.py",
5149
"tests/product_tag_metrics_test.py",
5250
"tests/product_test.py",
53-
"tests/product_type_group_test.py",
5451
"tests/product_type_member_test.py",
5552
"tests/product_type_test.py",
5653
"tests/questionnaire_advanced_test.py",

dojo/apps.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,12 @@ def ready(self):
7272

7373
register_check(check_configuration_deduplication, "dojo")
7474

75+
# Trigger registration of the OS authorization queryset filters.
76+
# query_registrations.py is no longer imported by the package
77+
# __init__ (to avoid circular import edge cases during early model
78+
# loading) — we do it here once all models are ready.
79+
import dojo.authorization.query_registrations # noqa: PLC0415, F401
80+
7581
# Load any signals here that will be ready for runtime
7682
# Importing the signals file is good enough if using the receiver decorator
7783
import dojo.announcement.signals # noqa: PLC0415, F401 raised: AppRegistryNotReady

dojo/authorization/__init__.py

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,7 @@
1-
# Import query_registrations to trigger RBAC filter registration at startup
2-
from dojo.authorization import query_registrations # noqa: F401
3-
from dojo.authorization.authorization import ( # noqa: F401
4-
user_has_configuration_permission,
5-
user_has_global_permission,
6-
user_has_global_permission_or_403,
7-
user_has_permission,
8-
user_has_permission_or_403,
9-
user_is_superuser_or_global_owner,
10-
)
11-
from dojo.authorization.roles_permissions import Permissions, Roles # noqa: F401
1+
# NOTE: do not import query_registrations here. It pulls in dojo.models
2+
# (and dojo.location.models), which can be mid-import when this package
3+
# is loaded transitively (e.g. via `from dojo.authorization.query_filters
4+
# import get_auth_filter`). If that chain raises ImportError, callers
5+
# silently fall back to a stub `get_auth_filter` and the queryset auth
6+
# filters never apply. Registration is now triggered explicitly in
7+
# dojo/apps.py ready() once all models are loaded.

dojo/authorization/authorization.py

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,10 @@
2121
from django.db.models import Model
2222

2323
from dojo.authorization.models import Dojo_Group_Member, Product_Member, Product_Type_Member
24+
from dojo.authorization.query_registrations import (
25+
authorized_product_id_set,
26+
authorized_product_type_id_set,
27+
)
2428
from dojo.authorization.roles_permissions import (
2529
Action,
2630
permission_to_action,
@@ -121,14 +125,14 @@ def _user_authorized_for(user: Dojo_User, obj: Model, action: Action) -> bool:
121125
if isinstance(obj, Product_Type):
122126
if user.is_staff:
123127
return True
124-
return obj.authorized_users.filter(pk=user.pk).exists()
128+
return obj.pk in authorized_product_type_id_set(user.pk)
125129

126130
if isinstance(obj, Product):
127131
if user.is_staff:
128132
return True
129-
if obj.authorized_users.filter(pk=user.pk).exists():
130-
return True
131-
return bool(obj.prod_type_id and obj.prod_type.authorized_users.filter(pk=user.pk).exists())
133+
# authorized_product_id_set folds direct Product membership AND
134+
# inherited membership via prod_type into one cached lookup.
135+
return obj.pk in authorized_product_id_set(user.pk)
132136

133137
if isinstance(obj, Engagement):
134138
return _user_authorized_for(user, obj.product, action)

dojo/authorization/query_registrations.py

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
Tool_Product_Settings,
4141
Vulnerability_Id,
4242
)
43+
from dojo.request_cache import cache_for_request
4344

4445

4546
def _resolve_user(user):
@@ -62,15 +63,57 @@ def _is_unrestricted(user, action):
6263

6364

6465
def _authorized_product_ids(user):
66+
"""
67+
QuerySet of product ids the user can access via authorized_users.
68+
Lazy on purpose — callers that pass this into ``.filter(id__in=...)``
69+
let Postgres collapse it into a single subquery.
70+
"""
6571
return Product.objects.filter(
6672
Q(authorized_users=user) | Q(prod_type__authorized_users=user),
6773
).values("id")
6874

6975

7076
def _authorized_product_type_ids(user):
77+
"""
78+
QuerySet of product_type ids the user can access via authorized_users.
79+
Lazy on purpose (see ``_authorized_product_ids``).
80+
"""
7181
return Product_Type.objects.filter(authorized_users=user).values("id")
7282

7383

84+
@cache_for_request
85+
def authorized_product_id_set(user_pk):
86+
"""
87+
Frozen set of product ids the user can access via authorized_users
88+
(direct or via prod_type). Result is cached for the lifetime of the
89+
current request — repeated per-object permission checks (one per
90+
object, often dozens per request) collapse to a single SELECT.
91+
92+
Returns an empty frozenset for anonymous / missing users so callers
93+
can do ``pid in set`` without a None check.
94+
"""
95+
if not user_pk:
96+
return frozenset()
97+
return frozenset(
98+
Product.objects.filter(
99+
Q(authorized_users=user_pk) | Q(prod_type__authorized_users=user_pk),
100+
).values_list("id", flat=True),
101+
)
102+
103+
104+
@cache_for_request
105+
def authorized_product_type_id_set(user_pk):
106+
"""
107+
Frozen set of product_type ids the user is a direct member of via
108+
authorized_users. Cached per request (see ``authorized_product_id_set``).
109+
"""
110+
if not user_pk:
111+
return frozenset()
112+
return frozenset(
113+
Product_Type.objects.filter(authorized_users=user_pk).values_list("id", flat=True),
114+
)
115+
116+
74117
def _filter_by_authorized_products(queryset, product_path, permission, user=None):
75118
"""
76119
Generic helper: restrict ``queryset`` to rows whose ``product_path`` FK

dojo/notifications/helper.py

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -607,8 +607,13 @@ def send_alert_notification(
607607
icon=icon[:25],
608608
source=source,
609609
)
610-
# relative urls will fail validation
611-
alert.clean_fields(exclude=["url"])
610+
# ``url`` skips validation (relative URLs are valid here but
611+
# URLField.validate rejects them). ``user_id`` skips the FK
612+
# existence probe — the user was just fetched from our own
613+
# DB by the caller, so the ``SELECT 1 FROM auth_user WHERE id=N
614+
# LIMIT 1`` round-trip every ForeignKey.validate would issue
615+
# is pure overhead at fan-out time.
616+
alert.clean_fields(exclude=["url", "user_id"])
612617
alert.save()
613618
except Exception as exception:
614619
logger.exception("Unable to create Alert Notification")

tests/group_test.py

Lines changed: 0 additions & 204 deletions
This file was deleted.

0 commit comments

Comments
 (0)