Api v2 get paginated customers #965
Conversation
…n_alert_filter_is_not_found
…a very simple tests, to show it is hard to execute them (because of the app.__init__)
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (1)
Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis PR restructures database initialization by creating a dedicated app/db.py module, refactors access control logic into centralized helpers (ac_current_user_has_permission, ac_current_user_has_customer_access), adds pagination support for customers search, and updates over 60 files to import db from the new location. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Route as Route Handler
participant AC as ac_current_user_has_customer_access
participant Session
participant DB as Access DB
Client->>Route: Request
Route->>AC: ac_current_user_has_customer_access(customer_id)
AC->>Session: Check current_user permissions
alt Has Server Admin Permission
AC->>AC: Grant Access
else Check Customer Access
AC->>DB: access_controls_user_has_customer_access(user, customer_id)
DB-->>AC: Return access result
end
AC-->>Route: Return True/False
alt Access Granted
Route->>Route: Process Request
Route-->>Client: Success Response
else Access Denied
Route-->>Client: Access Denied Error
end
sequenceDiagram
participant App
participant app_init as source/app/__init__.py
participant db_module as source/app/db.py
participant Other as Other Modules
App->>app_init: Initialize
app_init->>db_module: from app.db import db
db_module->>db_module: SQLAlchemy(engine_options=...)
db_module-->>app_init: db instance
app_init-->>App: Initialization complete
Other->>db_module: from app.db import db
db_module-->>Other: db instance
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Areas requiring extra attention:
Possibly related issues
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
source/app/blueprints/rest/v2/alerts.py (1)
100-124: Fix positional arguments when callingalerts_search.
get_filtered_alerts(and the business-layeralerts_search) now expect the pagination tuple(page, per_page, sort)beforeuser_identifier, followed bysource_referenceandcustom_conditions. This call still uses the old order (source_reference,custom_conditions,user_identifier, then pagination), sopageends up receiving the source reference string,per_pagegets the JSON payload,sortreceives the user id, and the actual pagination values are shifted into the access-control slot. The result is broken pagination and the customer-access filter no longer working. Please reorder the arguments (or switch to keyword arguments) so the call matches the updated signature.- filtered_alerts = alerts_search( + filtered_alerts = alerts_search( request.args.get('creation_start_date'), request.args.get('creation_end_date'), request.args.get('source_start_date'), request.args.get('source_end_date'), request.args.get('alert_title'), request.args.get('alert_description'), request.args.get('alert_status_id', type=int), request.args.get('alert_severity_id', type=int), request.args.get('alert_owner_id', type=int), request.args.get('alert_source'), request.args.get('alert_tags'), request.args.get('case_id', type=int), request.args.get('alert_customer_id'), request.args.get('alert_classification_id', type=int), alert_ids, alert_assets, alert_iocs, request.args.get('alert_resolution_id', type=int), - request.args.get('source_reference'), - request.args.get('custom_conditions'), - user_identifier_filter, - page, - per_page, - request.args.get('sort') + page, + per_page, + request.args.get('sort'), + user_identifier_filter, + request.args.get('source_reference'), + request.args.get('custom_conditions') )
🧹 Nitpick comments (2)
source/app/blueprints/pages/manage/manage_users.py (1)
105-105: Consider using keyword arguments for clarity.While positional arguments work correctly, keyword arguments would improve readability given the descriptive parameter names in
get_client_list.- groups = get_client_list(iris_current_user.id, user_is_server_administrator) + groups = get_client_list(current_user_id=iris_current_user.id, is_server_administrator=user_is_server_administrator)source/app/blueprints/pages/manage/manage_cases_routes.py (1)
89-89: Consider using keyword arguments for clarity.The change to positional arguments in
get_client_listcalls reduces readability. Given the descriptive parameter names (current_user_id,is_server_administrator), keyword arguments would better document the intent.Apply this diff to improve readability:
- customers = get_client_list(iris_current_user.id, user_is_server_administrator) + customers = get_client_list(current_user_id=iris_current_user.id, is_server_administrator=user_is_server_administrator)And:
- client_list = get_client_list(iris_current_user.id, ac_current_user_has_permission(Permissions.server_administrator)) + client_list = get_client_list( + current_user_id=iris_current_user.id, + is_server_administrator=ac_current_user_has_permission(Permissions.server_administrator) + )Also applies to: 122-122
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (97)
.vulture.ignore(0 hunks)pyproject.toml(1 hunks)source/app/__init__.py(3 hunks)source/app/blueprints/access_controls.py(4 hunks)source/app/blueprints/graphql/cases.py(2 hunks)source/app/blueprints/pages/alerts/alerts_routes.py(2 hunks)source/app/blueprints/pages/login/login_routes.py(1 hunks)source/app/blueprints/pages/manage/manage_cases_routes.py(3 hunks)source/app/blueprints/pages/manage/manage_users.py(2 hunks)source/app/blueprints/rest/alerts_routes.py(20 hunks)source/app/blueprints/rest/case/case_assets_routes.py(1 hunks)source/app/blueprints/rest/case/case_evidences_routes.py(1 hunks)source/app/blueprints/rest/case/case_ioc_routes.py(1 hunks)source/app/blueprints/rest/case/case_notes_routes.py(1 hunks)source/app/blueprints/rest/case/case_routes.py(1 hunks)source/app/blueprints/rest/case/case_tasks_routes.py(1 hunks)source/app/blueprints/rest/case/case_timeline_routes.py(1 hunks)source/app/blueprints/rest/context_routes.py(1 hunks)source/app/blueprints/rest/dashboard_routes.py(1 hunks)source/app/blueprints/rest/datastore_routes.py(1 hunks)source/app/blueprints/rest/filters_routes.py(1 hunks)source/app/blueprints/rest/manage/manage_assets_type_routes.py(1 hunks)source/app/blueprints/rest/manage/manage_attributes_routes.py(1 hunks)source/app/blueprints/rest/manage/manage_case_classifications_routes.py(1 hunks)source/app/blueprints/rest/manage/manage_case_state.py(1 hunks)source/app/blueprints/rest/manage/manage_case_templates_routes.py(1 hunks)source/app/blueprints/rest/manage/manage_cases_routes.py(3 hunks)source/app/blueprints/rest/manage/manage_customers_routes.py(1 hunks)source/app/blueprints/rest/manage/manage_evidence_types_routes.py(1 hunks)source/app/blueprints/rest/manage/manage_groups.py(1 hunks)source/app/blueprints/rest/manage/manage_ioc_types_routes.py(1 hunks)source/app/blueprints/rest/manage/manage_server_settings_routes.py(1 hunks)source/app/blueprints/rest/manage/manage_templates_routes.py(1 hunks)source/app/blueprints/rest/manage/manage_users.py(1 hunks)source/app/blueprints/rest/profile_routes.py(2 hunks)source/app/blueprints/rest/v2/alerts.py(4 hunks)source/app/blueprints/rest/v2/auth.py(1 hunks)source/app/blueprints/rest/v2/case_routes/evidences.py(1 hunks)source/app/blueprints/rest/v2/cases.py(2 hunks)source/app/blueprints/rest/v2/manage_routes/customers.py(2 hunks)source/app/business/access_controls.py(1 hunks)source/app/business/alerts.py(3 hunks)source/app/business/alerts_filters.py(1 hunks)source/app/business/assets.py(1 hunks)source/app/business/auth.py(1 hunks)source/app/business/cases.py(1 hunks)source/app/business/comments.py(1 hunks)source/app/business/customers.py(1 hunks)source/app/business/events.py(1 hunks)source/app/business/iocs.py(1 hunks)source/app/business/notes.py(1 hunks)source/app/business/notes_directories.py(1 hunks)source/app/business/tasks.py(1 hunks)source/app/business/users.py(1 hunks)source/app/datamgmt/alerts/alerts_db.py(5 hunks)source/app/datamgmt/case/case_assets_db.py(1 hunks)source/app/datamgmt/case/case_db.py(1 hunks)source/app/datamgmt/case/case_events_db.py(1 hunks)source/app/datamgmt/case/case_iocs_db.py(1 hunks)source/app/datamgmt/case/case_notes_db.py(1 hunks)source/app/datamgmt/case/case_rfiles_db.py(1 hunks)source/app/datamgmt/case/case_tasks_db.py(1 hunks)source/app/datamgmt/client/client_db.py(3 hunks)source/app/datamgmt/comments.py(1 hunks)source/app/datamgmt/dashboard/dashboard_db.py(1 hunks)source/app/datamgmt/datastore/datastore_db.py(1 hunks)source/app/datamgmt/iris_engine/modules_db.py(1 hunks)source/app/datamgmt/manage/manage_access_control_db.py(1 hunks)source/app/datamgmt/manage/manage_attribute_db.py(1 hunks)source/app/datamgmt/manage/manage_case_templates_db.py(1 hunks)source/app/datamgmt/manage/manage_cases_db.py(1 hunks)source/app/datamgmt/manage/manage_common.py(1 hunks)source/app/datamgmt/manage/manage_groups_db.py(1 hunks)source/app/datamgmt/manage/manage_srv_settings_db.py(1 hunks)source/app/datamgmt/manage/manage_tags_db.py(1 hunks)source/app/datamgmt/manage/manage_users_db.py(1 hunks)source/app/datamgmt/states.py(1 hunks)source/app/db.py(2 hunks)source/app/iris_engine/access_control/utils.py(1 hunks)source/app/iris_engine/demo_builder.py(1 hunks)source/app/iris_engine/module_handler/module_handler.py(1 hunks)source/app/iris_engine/tasker/tasks.py(1 hunks)source/app/iris_engine/updater/updater.py(1 hunks)source/app/iris_engine/utils/tracker.py(1 hunks)source/app/models/alerts.py(1 hunks)source/app/models/authorization.py(2 hunks)source/app/models/cases.py(1 hunks)source/app/models/comments.py(1 hunks)source/app/models/iocs.py(1 hunks)source/app/models/models.py(1 hunks)source/app/post_init.py(1 hunks)source/app/schema/marshables.py(1 hunks)source/app/util.py(1 hunks)source/tests/app/blueprints/rest/test_parsing.py(2 hunks)source/tests/test_helper.py(0 hunks)source/tests/unit/blueprints/manage/test_manage_modules_routes.py(0 hunks)tests/tests_rest_customers.py(1 hunks)
💤 Files with no reviewable changes (3)
- .vulture.ignore
- source/tests/test_helper.py
- source/tests/unit/blueprints/manage/test_manage_modules_routes.py
🧰 Additional context used
🧬 Code graph analysis (20)
source/app/blueprints/rest/v2/cases.py (1)
source/app/blueprints/access_controls.py (1)
ac_current_user_has_customer_access(595-599)
source/tests/app/blueprints/rest/test_parsing.py (1)
source/app/blueprints/rest/parsing.py (1)
parse_comma_separated_identifiers(23-24)
source/app/blueprints/rest/alerts_routes.py (3)
source/app/blueprints/access_controls.py (2)
ac_current_user_has_customer_access(595-599)ac_current_user_has_permission(588-592)source/app/models/authorization.py (1)
Permissions(49-66)source/app/datamgmt/alerts/alerts_db.py (1)
get_filtered_alerts(98-239)
source/app/blueprints/rest/profile_routes.py (1)
source/app/blueprints/access_controls.py (1)
ac_current_user_has_permission(588-592)
source/app/datamgmt/alerts/alerts_db.py (2)
source/app/datamgmt/manage/manage_access_control_db.py (1)
get_user_clients_id(142-157)source/app/datamgmt/filtering.py (1)
combine_conditions(90-105)
source/app/blueprints/graphql/cases.py (1)
source/app/blueprints/access_controls.py (1)
ac_current_user_has_customer_access(595-599)
tests/tests_rest_customers.py (1)
tests/iris.py (1)
get(63-64)
source/app/blueprints/rest/manage/manage_customers_routes.py (4)
source/app/blueprints/rest/endpoints.py (1)
endpoint_deprecated(76-86)source/app/blueprints/access_controls.py (2)
ac_api_requires(380-384)ac_current_user_has_permission(588-592)source/app/models/authorization.py (1)
Permissions(49-66)source/app/datamgmt/client/client_db.py (1)
get_client_list(52-74)
source/app/blueprints/access_controls.py (3)
source/app/business/access_controls.py (1)
access_controls_user_has_customer_access(97-98)source/app/models/authorization.py (2)
Permissions(49-66)ac_flag_match_mask(259-260)source/app/__init__.py (1)
ac_current_user_has_permission(68-81)
source/app/blueprints/rest/v2/alerts.py (2)
source/app/blueprints/access_controls.py (2)
ac_current_user_has_customer_access(595-599)ac_current_user_has_permission(588-592)source/app/models/authorization.py (1)
Permissions(49-66)
source/app/blueprints/pages/manage/manage_users.py (3)
source/app/blueprints/access_controls.py (2)
ac_requires(327-342)ac_current_user_has_permission(588-592)source/app/__init__.py (1)
ac_current_user_has_permission(68-81)source/app/datamgmt/client/client_db.py (1)
get_client_list(52-74)
source/app/__init__.py (1)
source/app/models/authorization.py (1)
ac_flag_match_mask(259-260)
source/app/business/customers.py (2)
source/app/models/models.py (1)
Client(120-132)source/app/datamgmt/client/client_db.py (1)
get_paginated_customers(77-86)
source/app/blueprints/pages/alerts/alerts_routes.py (1)
source/app/blueprints/access_controls.py (2)
ac_requires(327-342)ac_current_user_has_customer_access(595-599)
source/app/blueprints/rest/v2/case_routes/evidences.py (1)
source/app/blueprints/rest/v2/manage_routes/customers.py (1)
search_evidences(106-107)
source/app/datamgmt/client/client_db.py (3)
source/app/models/models.py (1)
Client(120-132)source/app/models/authorization.py (1)
UserClient(185-197)source/app/datamgmt/filtering.py (1)
paginate(215-228)
source/app/blueprints/pages/manage/manage_cases_routes.py (6)
source/app/blueprints/access_controls.py (3)
ac_api_return_access_denied(424-434)ac_fast_check_current_user_has_case_access(584-585)ac_current_user_has_permission(588-592)source/app/__init__.py (1)
ac_current_user_has_permission(68-81)source/app/datamgmt/client/client_db.py (1)
get_client_list(52-74)source/app/datamgmt/manage/manage_common.py (1)
get_severities_list(24-31)source/app/datamgmt/manage/manage_cases_db.py (1)
get_case_protagonists(244-257)source/app/models/authorization.py (1)
Permissions(49-66)
source/app/blueprints/rest/manage/manage_cases_routes.py (1)
source/app/blueprints/access_controls.py (3)
ac_requires_case_identifier(252-273)ac_fast_check_current_user_has_case_access(584-585)ac_current_user_has_customer_access(595-599)
source/app/blueprints/rest/v2/manage_routes/customers.py (5)
source/app/blueprints/rest/endpoints.py (1)
response_api_paginated(40-52)source/app/blueprints/access_controls.py (1)
ac_current_user_has_permission(588-592)source/app/models/authorization.py (1)
Permissions(49-66)source/app/business/customers.py (2)
customers_filter(34-35)customers_get(49-53)source/app/blueprints/rest/parsing.py (1)
parse_pagination_parameters(46-53)
source/app/business/alerts.py (2)
source/app/blueprints/access_controls.py (1)
ac_current_user_has_customer_access(595-599)source/app/datamgmt/alerts/alerts_db.py (1)
get_filtered_alerts(98-239)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: End to end tests
- GitHub Check: Database migration tests
- GitHub Check: Test API
🔇 Additional comments (93)
source/app/datamgmt/case/case_rfiles_db.py (1)
23-23: Import path updated correctly.The import path change aligns with the PR's refactoring objective to centralize database initialization in the new
app.dbmodule. All existing usages ofdbthroughout the file will continue to work identically with the new import source.source/app/models/iocs.py (1)
19-19: LGTM! Clean import refactoring.The import path update correctly aligns with the PR's database module reorganization. All subsequent uses of
dbin the model definitions remain compatible.source/app/datamgmt/manage/manage_srv_settings_db.py (1)
3-3: LGTM! Import path updated correctly.The import change aligns with the PR objective to centralize database-related code in a dedicated
app.dbmodule. The refactoring improves modularity and helps avoid circular import issues.source/app/datamgmt/manage/manage_common.py (1)
20-20: LGTM! Import path updated correctly.The import path change from
from app import dbtofrom app.db import dbis consistent with the broader refactoring effort to move database initialization into a dedicated module. This improves code organization and reduces potential circular import issues.source/app/business/events.py (1)
21-21: LGTM! Clean refactoring to dedicated database module.The import path update is correct and aligns with the architectural improvement of moving database initialization to a dedicated module. This helps avoid circular import issues.
source/app/datamgmt/manage/manage_users_db.py (1)
29-29: LGTM! Clean refactoring.The import path update to use the dedicated
app.dbmodule improves code organization and aligns with the PR's goal of consolidating database-related code.source/app/post_init.py (1)
39-39: LGTM! Import path refactored correctly.The database import has been successfully updated to use the new dedicated
app.dbmodule, aligning with the PR's goal of consolidating database initialization.source/app/business/users.py (1)
19-19: LGTM! Import path updated correctly.The import has been properly refactored to use the new
app.dbmodule.source/app/iris_engine/tasker/tasks.py (1)
23-23: LGTM! Database import path updated.The change aligns with the project-wide refactoring to centralize database imports.
source/app/business/cases.py (1)
22-22: LGTM! Import refactored correctly.The database import has been updated to use the dedicated
app.dbmodule.source/app/business/auth.py (1)
29-29: LGTM! Import path updated correctly.The change is consistent with the project-wide refactoring to consolidate database imports.
source/app/business/tasks.py (1)
23-23: LGTM! Database import updated.The import path has been correctly refactored to use
app.db.source/app/models/comments.py (1)
30-30: LGTM! Import path refactored.The database import has been successfully updated to the new module structure.
source/app/datamgmt/states.py (1)
22-22: LGTM! Import path updated correctly.The change aligns with the broader refactoring effort to centralize database imports in
app.db.source/app/business/iocs.py (1)
20-20: LGTM! Clean import refactoring.The database import has been correctly updated to use the new centralized
app.dbmodule, improving modularity and helping avoid potential circular import issues.source/app/datamgmt/case/case_notes_db.py (1)
25-25: LGTM! Import path correctly updated.The change aligns with the project-wide refactoring to centralize database imports through the
app.dbmodule.source/app/models/authorization.py (1)
23-24: LGTM! Clean import restructuring.The separation of
BigIntegerandJSONimports into individual lines improves readability, and the database import has been correctly updated to use the centralizedapp.dbmodule.Also applies to: 36-36
source/app/datamgmt/manage/manage_attribute_db.py (1)
23-23: LGTM! Import path correctly updated.The database import has been successfully migrated to the new
app.dbmodule, consistent with the project-wide refactoring.source/app/iris_engine/updater/updater.py (1)
35-35: LGTM! Database import correctly refactored.The change to import
dbfromapp.dbaligns with the PR's goal of centralizing database-related code in a dedicated module.source/app/business/notes_directories.py (1)
19-19: LGTM! Import refactoring applied correctly.The database import has been successfully updated to use the centralized
app.dbmodule.source/app/datamgmt/comments.py (1)
22-22: LGTM! Import path correctly updated.The database import change is consistent with the project-wide refactoring to use the
app.dbmodule.source/app/datamgmt/iris_engine/modules_db.py (1)
21-22: LGTM! Imports correctly restructured.The separation of
dbandappimports is correct. Thedbobject now comes from the centralizedapp.dbmodule, whileappis still imported separately for logger access (line 29), maintaining the necessary functionality.source/app/blueprints/pages/manage/manage_users.py (1)
34-34: LGTM: Import consolidation.The consolidation of access control imports on a single line is a minor readability improvement.
source/app/iris_engine/demo_builder.py (1)
23-23: LGTM: Database import refactoring.The import path update to
from app.db import dbis part of a systematic refactoring to centralize database initialization in a dedicated module, improving separation of concerns.pyproject.toml (1)
46-54: LGTM: Strengthened architectural boundaries.These import linter contract updates improve the separation between persistence and API layers by:
- Broadening the restriction from
app.blueprints.iris_userto all ofapp.blueprints- Adding a specific constraint to prevent
app.datamgmt.alerts.alerts_dbfrom importingapp.blueprints.access_controlsBoth changes help prevent circular dependencies and maintain clean architectural boundaries.
source/app/blueprints/pages/manage/manage_cases_routes.py (1)
40-43: LGTM: Import consolidation.Consolidating access control imports improves code organization and reduces line count.
source/app/db.py (1)
19-31: LGTM: Clean database module initialization.The new dedicated
dbmodule improves separation of concerns. The engine options are well-chosen:
pool_pre_ping: Trueensures connection health- Custom JSON deserializer with
OrderedDictpreserves key order (note: Python 3.7+ dicts already maintain insertion order, but this ensures compatibility)source/app/business/comments.py (1)
23-23: LGTM: Database import refactoring.The import path update is consistent with the systematic refactoring to centralize database initialization in
app.db.source/app/datamgmt/case/case_events_db.py (1)
21-21: LGTM: Database import refactoring.The import path update is consistent with the systematic refactoring to move database initialization to a dedicated module.
source/app/datamgmt/manage/manage_tags_db.py (1)
24-24: LGTM - Import path refactored to use centralized db module.The import path change aligns with the PR's goal of moving database-related code into a dedicated
app.dbmodule. This refactor improves code organization without changing functionality.source/app/business/notes.py (1)
21-21: LGTM - Import path refactored to use centralized db module.The import path change aligns with the repository-wide refactor to centralize database imports.
source/app/datamgmt/case/case_tasks_db.py (1)
25-25: LGTM - Import path refactored to use centralized db module.The import path change is consistent with the repository-wide refactor to centralize database imports.
source/app/util.py (1)
32-32: LGTM - Import path refactored to use centralized db module.The import path change is consistent with the repository-wide refactor to centralize database imports.
source/app/datamgmt/manage/manage_cases_db.py (1)
28-28: LGTM - Import path refactored to use centralized db module.The import path change is consistent with the repository-wide refactor to centralize database imports.
source/app/iris_engine/utils/tracker.py (1)
23-23: LGTM - Import path refactored to use centralized db module.The import path change is consistent with the repository-wide refactor to centralize database imports.
source/app/datamgmt/manage/manage_case_templates_db.py (1)
24-24: LGTM - Import path refactored to use centralized db module.The import path change is consistent with the repository-wide refactor to centralize database imports.
source/app/datamgmt/case/case_assets_db.py (1)
26-26: LGTM - Import path refactored to use centralized db module.The import path change is consistent with the repository-wide refactor to centralize database imports.
source/app/business/assets.py (1)
21-21: LGTM - Clean import refactor.The import path update aligns with the project-wide refactor to centralize database imports in
app.db.source/app/iris_engine/module_handler/module_handler.py (1)
33-33: LGTM - Consistent import refactor.The import path change aligns with the standardized database import location.
source/app/blueprints/rest/manage/manage_evidence_types_routes.py (1)
25-25: LGTM - Import path standardization.The import change is consistent with the project-wide refactor.
source/app/blueprints/pages/login/login_routes.py (1)
37-37: LGTM - Import standardization.The import path update is consistent with the centralized database module pattern.
source/app/models/alerts.py (1)
34-34: LGTM - Database import refactor.The import change maintains the same functionality while following the new module structure.
source/app/datamgmt/client/client_db.py (3)
23-33: LGTM - Imports updated for pagination support.The new imports support the pagination feature being added.
77-86: LGTM - New pagination function looks correct.The
get_paginated_customersfunction properly filters results for non-administrators by checking UserClient associations. The logic is sound and consistent withget_client_list.
52-74: Breaking signature change verified - all call sites updated correctly.All 4 callers of
get_client_list()pass both required arguments explicitly:
manage_customers_routes.py:62✓manage_cases_routes.py:89✓manage_cases_routes.py:122✓manage_users.py:105✓No call sites rely on default parameters. The migration from optional parameters with defaults to required parameters is complete and safe.
source/app/blueprints/rest/manage/manage_customers_routes.py (2)
58-58: LGTM - Proper API deprecation.The deprecation decorator correctly signals users to migrate to the new v2 endpoint.
62-62: LGTM - Updated call matches new signature.The call now uses positional arguments, correctly matching the updated
get_client_listsignature.source/app/datamgmt/manage/manage_groups_db.py (1)
19-19: LGTM - Import refactor.The import path change aligns with the centralized database module structure.
source/app/blueprints/rest/datastore_routes.py (1)
30-30: LGTM - Import refactor applied consistently.source/app/blueprints/rest/manage/manage_attributes_routes.py (1)
22-22: LGTM - Import refactor applied consistently.source/app/blueprints/rest/case/case_timeline_routes.py (1)
29-29: LGTM - Import refactor applied correctly.Note that
from app import appon Line 30 correctly remains unchanged, as only the database object was moved toapp.db.source/app/business/alerts_filters.py (1)
19-19: LGTM - Import refactor applied consistently.source/app/blueprints/rest/manage/manage_case_classifications_routes.py (1)
25-25: LGTM - Import refactor applied consistently.source/app/blueprints/rest/case/case_notes_routes.py (1)
24-24: LGTM - Import refactor applied correctly.source/app/datamgmt/dashboard/dashboard_db.py (1)
21-21: LGTM - Import refactor applied consistently across all layers.The refactor successfully extends to the data management layer, maintaining consistency throughout the codebase.
source/app/blueprints/rest/manage/manage_users.py (1)
26-26: LGTM! Clean import path refactoring.The database import has been correctly updated to use the new centralized
app.dbmodule.source/app/blueprints/rest/manage/manage_assets_type_routes.py (1)
27-27: LGTM! Import path correctly updated.Consistent with the repository-wide refactoring to centralize the database import.
source/app/models/models.py (1)
47-47: LGTM! Database import path updated correctly.The change aligns with the PR's objective to move database-related code into a dedicated
app.dbmodule.source/app/blueprints/rest/case/case_ioc_routes.py (1)
28-28: LGTM! Import refactoring applied correctly.The database import now uses the centralized module path.
source/app/datamgmt/case/case_iocs_db.py (1)
21-21: LGTM! Database import correctly updated.Consistent with the project-wide import path standardization.
source/app/datamgmt/case/case_db.py (1)
27-27: LGTM! Import path refactoring completed correctly.The database import has been updated to use the centralized
app.dbmodule.source/app/blueprints/rest/context_routes.py (1)
25-25: LGTM! Database import path updated.Aligns with the broader refactoring to centralize database imports in
app.db.source/app/blueprints/rest/manage/manage_ioc_types_routes.py (1)
23-23: LGTM! Import refactoring correctly applied.The database import path has been successfully updated to use the centralized module.
source/app/business/access_controls.py (1)
19-19: LGTM: Import path updated to use dedicated db module.The import has been correctly updated to use the new
app.dbmodule, aligning with the PR's objective to centralize database imports.source/app/blueprints/rest/filters_routes.py (1)
22-22: LGTM: Import path updated consistently.The database import has been correctly updated to use the centralized
app.dbmodule.source/app/models/cases.py (1)
39-39: LGTM: Import path updated in models.The database import has been correctly updated to use
app.db, maintaining consistency across the codebase.source/app/datamgmt/manage/manage_access_control_db.py (2)
20-20: LGTM: Import path updated.The database import has been correctly updated to use the centralized
app.dbmodule.
160-175: No issues found. Admin bypass properly centralized.The verification confirms all concerns from the review comment have been addressed correctly:
- ✓
user_has_client_accessis only called fromaccess_controls_user_has_customer_access(business layer), which is itself only called fromac_current_user_has_customer_access(blueprints layer)- ✓ Server administrators retain appropriate access via the admin bypass in
ac_current_user_has_customer_access(lines 596-597: checksPermissions.server_administratorand returnsTrue)- ✓ The access control flow correctly centralizes admin checks before delegating to the database-level permission check
The refactored code properly moved the admin bypass to a centralized location, maintaining security while simplifying the lower-level function.
source/app/blueprints/rest/v2/cases.py (2)
49-49: LGTM: Centralized access control helper imported.The import of
ac_current_user_has_customer_accessaligns with the PR's objective to use centralized access control helpers.
138-139: LGTM: Access check centralized through helper function.The customer access validation now uses the centralized
ac_current_user_has_customer_accesshelper, which provides consistent access control logic across the codebase. This helper properly checks for server administrator permissions before delegating to lower-level checks.source/app/blueprints/rest/manage/manage_case_templates_routes.py (1)
24-24: LGTM: Import path updated.The database import has been correctly updated to use the centralized
app.dbmodule.tests/tests_rest_customers.py (1)
159-161: LGTM: Basic test for customer search endpoint.The test verifies the new GET
/api/v2/manage/customersendpoint returns a successful response. While the test coverage is minimal (only checking status code), this aligns with the PR's stated objective to keep testing simple for this endpoint.source/app/datamgmt/datastore/datastore_db.py (1)
28-28: LGTM: Import path updated.The database import has been correctly updated to use the centralized
app.dbmodule, maintaining consistency with the broader refactoring effort.source/app/blueprints/rest/manage/manage_groups.py (1)
25-25: LGTM! Import path refactor aligns with architectural improvement.The change to import
dbfromapp.dbinstead ofappis part of the broader refactor to centralize database initialization in a dedicated module, reducing circular import issues.source/app/schema/marshables.py (1)
46-46: LGTM! Import path update is consistent with the codebase-wide refactor.source/app/blueprints/rest/dashboard_routes.py (1)
30-30: LGTM! Consistent import path refactor.source/app/blueprints/rest/manage/manage_templates_routes.py (1)
31-31: LGTM! Import path refactor is correct.source/app/blueprints/rest/v2/auth.py (1)
29-29: LGTM! Import update aligns with the architectural refactor.source/app/blueprints/rest/case/case_routes.py (1)
27-27: LGTM! Import path refactor is consistent.source/app/blueprints/rest/case/case_assets_routes.py (1)
25-25: LGTM! Import path update is correct.source/app/blueprints/rest/case/case_evidences_routes.py (1)
25-25: LGTM! Import path refactor is complete.source/app/blueprints/rest/v2/case_routes/evidences.py (1)
162-163: LGTM!The function rename from
get_evidencestosearch_evidencesaligns with the broader naming conventions adopted across the v2 REST routes.source/app/blueprints/rest/manage/manage_server_settings_routes.py (1)
25-25: LGTM!The import path update aligns with the repository-wide refactor to centralize database imports in
app.db.source/app/blueprints/rest/profile_routes.py (2)
25-25: LGTM!The database import path update is consistent with the repository-wide refactor to centralize database imports.
38-38: LGTM!The import of
ac_current_user_has_permissionfrom the centralized access control module aligns with the PR's goal to consolidate access control helpers.source/app/blueprints/rest/case/case_tasks_routes.py (1)
25-25: LGTM!The database import path update is consistent with the repository-wide refactor.
source/app/blueprints/graphql/cases.py (2)
30-30: LGTM!The import of the centralized customer access control helper is consistent with the PR's refactoring objectives.
187-188: LGTM!The replacement of
user_has_client_accesswithac_current_user_has_customer_accesscentralizes the customer access validation logic. The new helper internally handles the current user context and server administrator permissions.source/app/business/customers.py (1)
19-35: LGTM!The new
customers_filterfunction correctly implements pagination support for customer searches. The parameter passing toget_paginated_customersmatches the expected signature.source/app/iris_engine/access_control/utils.py (1)
3-3: LGTM!The database import path update is consistent with the repository-wide refactor to centralize database imports in
app.db.source/app/blueprints/rest/manage/manage_cases_routes.py (3)
29-29: LGTM!The database import path update is consistent with the repository-wide refactor.
53-56: LGTM!The import updates for access control functions centralize these utilities under
app.blueprints.access_controls, improving code organization.
285-286: LGTM!The use of
ac_current_user_has_customer_accesscentralizes the customer access validation logic, consistent with the refactoring pattern applied across the codebase.
| def ac_current_user_has_permission(permission): | ||
| """ | ||
| Return True if current user has permission | ||
| """ | ||
| return ac_flag_match_mask(session['permissions'], permission.value) | ||
|
|
||
|
|
||
| def ac_current_user_has_customer_access(customer_identifier): | ||
| if ac_current_user_has_permission(Permissions.server_administrator): | ||
| return True | ||
|
|
There was a problem hiding this comment.
Prevent KeyError when checking permissions outside session auth
Calling ac_current_user_has_permission during token-authenticated requests blows up because session['permissions'] is never populated in that flow—wrap_with_permission_checks caches permissions on g, not in the session cookie. This raises a KeyError, causing 500 responses for routes that use the new helper (e.g., GET /api/v2/manage/customers with a Bearer token). We need to reuse the same guards as _user_has_at_least_a_required_permission, falling back to g.auth_user_permissions and lazily loading the session flag. (search.google.com)
def ac_current_user_has_permission(permission):
"""
Return True if current user has permission
"""
- return ac_flag_match_mask(session['permissions'], permission.value)
+ if hasattr(g, 'auth_token_user_id'):
+ if hasattr(g, 'auth_user_permissions'):
+ permissions_flag = g.auth_user_permissions
+ else:
+ user = get_user(g.auth_token_user_id)
+ if not user:
+ return False
+ permissions_flag = ac_get_effective_permissions_of_user(user)
+ g.auth_user_permissions = permissions_flag
+ return ac_flag_match_mask(permissions_flag, permission.value)
+
+ if 'permissions' not in session:
+ session['permissions'] = ac_get_effective_permissions_of_user(current_user)
+
+ return ac_flag_match_mask(session['permissions'], permission.value)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def ac_current_user_has_permission(permission): | |
| """ | |
| Return True if current user has permission | |
| """ | |
| return ac_flag_match_mask(session['permissions'], permission.value) | |
| def ac_current_user_has_customer_access(customer_identifier): | |
| if ac_current_user_has_permission(Permissions.server_administrator): | |
| return True | |
| def ac_current_user_has_permission(permission): | |
| """ | |
| Return True if current user has permission | |
| """ | |
| if hasattr(g, 'auth_token_user_id'): | |
| if hasattr(g, 'auth_user_permissions'): | |
| permissions_flag = g.auth_user_permissions | |
| else: | |
| user = get_user(g.auth_token_user_id) | |
| if not user: | |
| return False | |
| permissions_flag = ac_get_effective_permissions_of_user(user) | |
| g.auth_user_permissions = permissions_flag | |
| return ac_flag_match_mask(permissions_flag, permission.value) | |
| if 'permissions' not in session: | |
| session['permissions'] = ac_get_effective_permissions_of_user(current_user) | |
| return ac_flag_match_mask(session['permissions'], permission.value) |
🤖 Prompt for AI Agents
In source/app/blueprints/access_controls.py around lines 588–598,
ac_current_user_has_permission currently accesses session['permissions']
directly which raises KeyError for token-authenticated requests; change the
function to first try session.get('permissions') (safely) and if missing fall
back to g.auth_user_permissions (or call the same guards used by
_user_has_at_least_a_required_permission) and return False if neither are
present, avoiding direct session indexing and ensuring token-authenticated flows
use g-cached permissions; keep return semantics the same by passing the resolved
permission mask into ac_flag_match_mask.
| from flask import request | ||
|
|
||
| from app import db | ||
| from app.db import db |
There was a problem hiding this comment.
🧩 Analysis chain
LGTM - Clean import refactor.
The database import path has been successfully updated as part of the architectural improvement to centralize database access in app.db.
To ensure completeness of this repository-wide refactor, verify that all imports have been consistently updated:
🏁 Script executed:
#!/bin/bash
# Verify no remaining old-style db imports from app
rg -n "from app import.*\bdb\b" --type=py
# Verify the new import pattern is used
rg -n "from app\.db import db" --type=py -c | head -20Length of output: 805
Database import refactor is incomplete—one file still uses the old pattern.
Verification revealed that source/tests/clean_database.py:22 still imports using the deprecated from app import db pattern. Update this file to use from app.db import db to align with the rest of the codebase.
🤖 Prompt for AI Agents
In source/tests/clean_database.py around line 22, the test still uses the
deprecated import pattern "from app import db"; replace that with "from app.db
import db" so it matches the refactor used elsewhere, then run the test suite to
confirm no import errors remain.
| def test_parse_comma_separated_identifiers_should_return_identifiers(self): | ||
| result = parse_comma_separated_identifiers('a,b') | ||
| self.assertEqual(['a', 'c'], result) |
There was a problem hiding this comment.
Critical: Test has multiple errors and will fail.
The test has several issues:
- Input/output mismatch: Input is
'a,b'but expected output is['a', 'c']- 'b' and 'c' don't match - Type mismatch:
parse_comma_separated_identifiersreturns a list of integers (see line 23 of source/app/blueprints/rest/parsing.py:return [int(identifier) for identifier in identifiers.split(',')]), but the assertion expects strings['a', 'c'] - Runtime error: The function will raise
ValueErrorwhen trying to convert 'a' and 'b' to integers
Apply this diff to fix the test:
def test_parse_comma_separated_identifiers_should_return_identifiers(self):
- result = parse_comma_separated_identifiers('a,b')
- self.assertEqual(['a', 'c'], result)
+ result = parse_comma_separated_identifiers('1,2')
+ self.assertEqual([1, 2], result)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def test_parse_comma_separated_identifiers_should_return_identifiers(self): | |
| result = parse_comma_separated_identifiers('a,b') | |
| self.assertEqual(['a', 'c'], result) | |
| def test_parse_comma_separated_identifiers_should_return_identifiers(self): | |
| result = parse_comma_separated_identifiers('1,2') | |
| self.assertEqual([1, 2], result) |
🤖 Prompt for AI Agents
In source/tests/app/blueprints/rest/test_parsing.py around lines 26 to 28, the
test currently mismatches input and expected output and the types: it passes
'a,b' and expects ['a','c'] while the parser converts identifiers to integers;
update the test to use a numeric comma-separated string like '1,2' and assert
the returned list equals [1, 2] (or alternatively assert a ValueError for
non-numeric input if you want to test error handling) so the input, expected
output, and types align with parse_comma_separated_identifiers.
Implementation of endpoint
GET /api/v2/manage/customersdbin its own moduleapp.dbto avoid importingappin some placesac_user_has_permissionThis PR goes with documentation PR#93
Summary by CodeRabbit
New Features
Deprecations
Chores