update user auth for list filters

[nikhilsinhaparseable]

current: server checks if user is authorized to a dataset based on query string
this logic does not work for saved searches because there is no query string
as searches have key:value pair syntax

update: server fetches the filter_type
if filter_type is sql or filter,
server checks if user is authorized to a dataset based on query string if filter_type is search,
server checks if user is authorized to a dataset based on the dataset_name

Summary by CodeRabbit

• New Features

  • Enhanced filter functionality now categorizes filters into distinct types (e.g., SQL, Search) for clearer processing.
  • Improved asynchronous query handling provides a more responsive experience.

• Bug Fixes

  • Addressed inconsistencies in user authorization logic by shifting from query-specific checks to dataset-level permissions.

• Refactor

  • Updated user authorization mechanisms across features to enforce streamlined, dataset-level permissions.
  • Reorganized and consolidated internal dependencies for greater system clarity and reliability.

[coderabbitai]

Walkthrough

The changes refactor the mechanism for user authorization and query handling. Functions and imports related to table scanning and query-specific authorization have been removed from the alerts utility, with a shift to dataset-level authorization across several modules. Imports for user_auth_for_query have been redirected from the alerts utility module to the utils module. In addition, the filter functionality now leverages a strongly typed enum for filter types. The utils module introduces an asynchronous function for extracting tables from a query and adjusts the signature of user_auth_for_query accordingly.

Changes

File(s)	Changes Summary
src/alerts/alerts_utils.rs	Removed functions get_tables_from_query and user_auth_for_query along with associated rbac imports, eliminating query table extraction and query-specific auth logic.
src/alerts/mod.rs, src/handlers/http/alerts.rs, src/users/dashboards.rs	Updated imports to retrieve user_auth_for_query from crate::utils instead of alerts_utils, consolidating utility access.
src/correlation.rs, src/handlers/airplane.rs, src/handlers/http/correlation.rs, src/handlers/http/query.rs	Replaced function calls to user_auth_for_query with user_auth_for_datasets to change authorization checks from query-level to dataset-level.
src/users/filters.rs	Modified FilterQuery.filter_type: changed from String to a new FilterType enum (variants: Filter, SQL, Search), added a to_str method, and updated auth logic based on filter type.
src/utils/mod.rs	Introduced new async function get_tables_from_query and updated user_auth_for_query to be asynchronous, integrating table extraction with dataset-level authorization.

Possibly related PRs

• hotfix for alerts #1283: Updates the usage of user_auth_for_query to user_auth_for_datasets, directly related to the refactoring of authorization logic.

Suggested labels

for next release

Suggested reviewers

• de-sh

Poem

I'm a bouncy rabbit, with whiskers so neat,
Hopping through code changes on nimble feet.
Out with the old queries and tangled imports away,
In with sleek datasets leading the way.
My hops sing of refactors under the soft moonlight,
A joyful chorus in the realm of code tonight! 🐰

Tip

⚡💬 Agentic Chat (Pro Plan, General Availability)

• We're introducing multi-step agentic chat in review comments and issue comments, within and outside of PR's. This feature enhances review and issue discussions with the CodeRabbit agentic chat by enabling advanced interactions, including the ability to create pull requests directly from comments and add commits to existing pull requests.

---

📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 461bbdd and 3851b0f.

📒 Files selected for processing (10)

• src/alerts/alerts_utils.rs (1 hunks)
• src/alerts/mod.rs (1 hunks)
• src/correlation.rs (3 hunks)
• src/handlers/airplane.rs (2 hunks)
• src/handlers/http/alerts.rs (2 hunks)
• src/handlers/http/correlation.rs (2 hunks)
• src/handlers/http/query.rs (3 hunks)
• src/users/dashboards.rs (1 hunks)
• src/users/filters.rs (3 hunks)
• src/utils/mod.rs (2 hunks)

🚧 Files skipped from review as they are similar to previous changes (6)

• src/users/dashboards.rs
• src/handlers/airplane.rs
• src/handlers/http/alerts.rs
• src/handlers/http/query.rs
• src/alerts/mod.rs
• src/handlers/http/correlation.rs

🧰 Additional context used

🧬 Code Graph Analysis (3)

src/correlation.rs (1)

src/utils/mod.rs (1)

• user_auth_for_datasets (110-142)

src/alerts/alerts_utils.rs (1)

src/handlers/http/query.rs (1)

• query (69-155)

src/users/filters.rs (3)

src/handlers/http/rbac.rs (1)

• Users (63-63)

src/utils/mod.rs (2)

• user_auth_for_datasets (110-142)
• user_auth_for_query (100-108)

src/handlers/http/query.rs (1)

• query (69-155)

⏰ Context from checks skipped due to timeout of 90000ms (10)

• GitHub Check: Build Default x86_64-pc-windows-msvc
• GitHub Check: Build Default x86_64-apple-darwin
• GitHub Check: Build Default aarch64-apple-darwin
• GitHub Check: Build Default aarch64-unknown-linux-gnu
• GitHub Check: Build Default x86_64-unknown-linux-gnu
• GitHub Check: Build Kafka x86_64-unknown-linux-gnu
• GitHub Check: Build Kafka aarch64-apple-darwin
• GitHub Check: Quest Smoke and Load Tests for Distributed deployments
• GitHub Check: Quest Smoke and Load Tests for Standalone deployments
• GitHub Check: coverage

🔇 Additional comments (12)

src/alerts/alerts_utils.rs (1)

33-33: Update to imports reflects refactored authorization

The import statement has been updated to remove references to RBAC and TableScanVisitor modules, which aligns with the removal of query-specific authorization functions from this file.

src/correlation.rs (3)

43-43: Updated import to use dataset-based authorization

The import has been updated to use user_auth_for_datasets instead of user_auth_for_query, which aligns with the broader shift in the codebase toward dataset-level authorization.

---

90-90: Updated authorization check for list_correlations

Function call updated to use user_auth_for_datasets instead of user_auth_for_query, maintaining the same functionality while using the more appropriate dataset-level authorization model.

---

284-284: Updated authorization check in validate method

Consistently updating the authorization check to use user_auth_for_datasets instead of user_auth_for_query. This maintains the same validation behavior while leveraging the dataset-level authorization model.

src/users/filters.rs (4)

48-48: Enhanced type safety with FilterType enum

Replacing the String type with a strongly typed enum improves type safety and prevents potential errors from typos or invalid filter types.

---

53-59: Well-defined FilterType enum for improved clarity

The introduction of this enum properly categorizes the three filter types (Filter, SQL, Search) with appropriate serialization annotations. This enhances code readability and maintainability.

---

61-69: Useful helper method for string conversion

The to_str() method provides a clean way to convert enum variants to their string representations, which is particularly useful for logging, debugging, and API integration.

---

187-202: Implemented authorization logic based on filter type

This is the core implementation of the PR objective - differentiating authorization based on filter type. The code now:

1. Uses query-based authorization for SQL and Filter types
2. Uses dataset-based authorization for Search type

This change ensures proper authorization regardless of the filter syntax being used, addressing the issue with saved searches.

src/utils/mod.rs (4)

29-30: Added necessary imports for table extraction

These imports support the new get_tables_from_query function, providing access to the query session state and table scan visitor functionality.

---

36-36: Added TreeNode import for visitor pattern

This import is required for the visitor pattern used in the get_tables_from_query function to extract tables from the logical plan.

---

88-98: Implemented table extraction from queries

This utility function creates a logical plan from a query string and uses a visitor pattern to extract table names referenced in the query. It properly handles errors by converting them to HTTP-friendly error responses.

---

100-108: Refactored query authorization to use dataset-level checks

The user_auth_for_query function has been redesigned to:

1. Accept a session key instead of permissions
2. Extract tables from the query asynchronously
3. Get permissions for the session key
4. Delegate to user_auth_for_datasets for the actual authorization check

This change creates a more consistent authorization flow while maintaining backward compatibility with existing code.

✨ Finishing Touches

• 📝 Generate Docstrings

---

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>, please review it.
  • Generate unit testing code for this file.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
  • @coderabbitai generate unit testing code for this file.
  • @coderabbitai modularize this function.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
  • @coderabbitai read src/utils.ts and generate unit testing code.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
  • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai full review to do a full review from scratch and review all the files again.
• @coderabbitai summary to regenerate the summary of the PR.
• @coderabbitai generate docstrings to generate docstrings for this PR.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai plan to trigger planning for file edits and PR creation.
• @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
• @coderabbitai help to get help.

Other keywords and placeholders

• Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
• Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
• Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• Please see the configuration documentation for more information.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

• Visit our Documentation for detailed information on how to use CodeRabbit.
• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

src/correlation.rs (1)

90-90: Consider logging skipped correlations for easier debugging.
Here, if user_auth_for_datasets fails, the correlation is silently skipped. While this behavior may be intentional, adding a log statement could help identify when and why certain correlations are excluded.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cec4ab8 and 461bbdd.

📒 Files selected for processing (10)

• src/alerts/alerts_utils.rs (1 hunks)
• src/alerts/mod.rs (1 hunks)
• src/correlation.rs (3 hunks)
• src/handlers/airplane.rs (2 hunks)
• src/handlers/http/alerts.rs (2 hunks)
• src/handlers/http/correlation.rs (2 hunks)
• src/handlers/http/query.rs (3 hunks)
• src/users/dashboards.rs (1 hunks)
• src/users/filters.rs (3 hunks)
• src/utils/mod.rs (2 hunks)

🧰 Additional context used

🧬 Code Graph Analysis (8)

src/handlers/http/alerts.rs (2)

src/utils/actix.rs (1)

• extract_session_key_from_req (51-71)

src/utils/mod.rs (1)

• user_auth_for_query (100-108)

src/handlers/airplane.rs (1)

src/utils/mod.rs (1)

• user_auth_for_datasets (110-142)

src/handlers/http/query.rs (1)

src/utils/mod.rs (1)

• user_auth_for_datasets (110-142)

src/alerts/alerts_utils.rs (1)

src/handlers/http/query.rs (1)

• query (69-155)

src/utils/mod.rs (1)

src/handlers/http/query.rs (1)

• query (69-155)

src/alerts/mod.rs (1)

src/utils/mod.rs (1)

• user_auth_for_query (100-108)

src/users/filters.rs (1)

src/utils/mod.rs (2)

• user_auth_for_datasets (110-142)
• user_auth_for_query (100-108)

src/correlation.rs (1)

src/utils/mod.rs (1)

• user_auth_for_datasets (110-142)

⏰ Context from checks skipped due to timeout of 90000ms (9)

• GitHub Check: Build Default x86_64-pc-windows-msvc
• GitHub Check: Build Default aarch64-apple-darwin
• GitHub Check: Build Default x86_64-unknown-linux-gnu
• GitHub Check: Build Default aarch64-unknown-linux-gnu
• GitHub Check: Build Kafka aarch64-apple-darwin
• GitHub Check: Quest Smoke and Load Tests for Distributed deployments
• GitHub Check: Build Kafka x86_64-unknown-linux-gnu
• GitHub Check: Quest Smoke and Load Tests for Standalone deployments
• GitHub Check: coverage

🔇 Additional comments (18)

src/alerts/mod.rs (1)

45-45: Acknowledge updated import path for user authorization.

The import for user_auth_for_query has been moved from alerts_utils to crate::utils, aligning with the broader refactoring of the authorization system to better handle different filter types.

src/users/dashboards.rs (1)

29-29: Import consolidation looks good.

The import statement has been refactored to include user_auth_for_query in the grouped import from utils, improving code organization while maintaining the same functionality.

src/handlers/http/alerts.rs (1)

25-25: Refactored import path for user authentication utilities.

The import for user_auth_for_query has been successfully moved from a module-specific location to the centralized utils module, which aligns with the PR's goal of improving the user authorization process.

src/handlers/airplane.rs (1)

214-216: Function call updated to use dataset-based authorization.

The change from user_auth_for_query to user_auth_for_datasets reflects the architecture shift from query-based to dataset-based authorization. This is a key part of the PR's objective to enhance authorization for list filters, ensuring users can be properly authorized regardless of the filter type being used.

src/correlation.rs (2)

43-43: New import aligns with dataset-level authorization refactor.
Importing user_auth_for_datasets in place of user_auth_for_query seems consistent with the new approach to dataset-based permissions.

---

284-284: Consistent dataset-level check for user authorization.
Replacing the old query-based check with a dataset-focused authorization is clear and consistent. This approach improves readability and unifies the authorization flow.

src/handlers/http/correlation.rs (2)

26-26: Updated import reflects shift to dataset-based checks.
Importing user_auth_for_datasets here aligns with the broader refactor and centralizes authorization logic.

---

57-57: Dataset-level authorization ensures correctness.
This replacement of the old function properly returns an error if the user lacks permissions for any target dataset. The approach is straightforward and consistent with the rest of the codebase.

src/alerts/alerts_utils.rs (1)

33-33: Cleaned up imports to remove the old query auth references.
This import change reflects the removal of query-based authorization in the alerts utility. No further issues noted.

src/handlers/http/query.rs (3)

52-52: Updated import to use dataset-based authorization instead of query-based

The import has been updated to use user_auth_for_datasets which is consistent with the PR objectives to enhance user authorization for datasets based on different filter types.

---

102-102: Updated authorization check to use dataset-level permissions

The function call has been updated from user_auth_for_query to user_auth_for_datasets, which aligns with the PR objective of modifying the server's authorization logic. This change ensures consistent authorization checking across different filter types.

---

165-165: Updated counts request authorization to use dataset-level permissions

Similar to the main query function, the authorization check in the get_counts function has been updated to use user_auth_for_datasets, ensuring consistent application of the new authorization approach.

src/utils/mod.rs (3)

29-30: Added necessary imports for table scanning and session management

The imports have been appropriately updated to support the new functionality for extracting tables from queries and handling user sessions. These changes are required for the new async get_tables_from_query function.

Also applies to: 36-37

---

88-98: Added new async function to extract tables from queries

This new helper function encapsulates the logic to extract table names from a query string, which is a crucial step in dataset-based authorization. The function properly handles error cases and returns a structured TableScanVisitor that contains the table names.

---

100-108: Refactored user_auth_for_query to use dataset-based authorization

The user_auth_for_query function has been updated to:

1. Be asynchronous to support the async table extraction
2. Accept a session key instead of permissions
3. Leverage the new get_tables_from_query function
4. Use the dataset-level authorization via user_auth_for_datasets

This maintains backward compatibility while implementing the PR's objective to shift to dataset-level authorization.

src/users/filters.rs (3)

48-48: Replaced string type with strongly typed enum for filter_type

Using a strongly typed enum instead of a string for filter_type improves type safety and makes the code more maintainable by preventing typos and invalid filter types.

---

53-69: Added FilterType enum with conversion method

The new FilterType enum provides a structured way to represent the different filter types (Filter, SQL, and Search) with proper serialization annotations. The to_str method is a helpful utility for converting the enum variants to their string representations when needed.

---

188-198: Implemented conditional authorization based on filter type

This is the core change that fulfills the PR objective. The code now:

1. Checks the filter type to determine the appropriate authorization method
2. For SQL and Filter types, it uses query-based authorization (user_auth_for_query)
3. For Search type, it uses dataset-based authorization (user_auth_for_datasets)

This change ensures that saved searches using key:value pair syntax can be properly authorized using the dataset name, addressing the limitation mentioned in the PR description.

[nitisht]

Merging because tests passed once and we just added comments now.