-
-
Notifications
You must be signed in to change notification settings - Fork 147
Feat: Prism APIs #1188
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feat: Prism APIs #1188
Changes from all commits
bb2b08d
8149862
89b5a6a
9d4f25f
25b18da
ed4c1e0
8db824b
3e5077a
6397b71
4cb0073
2d3f7da
c683b96
3cf6197
28fed09
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -21,9 +21,9 @@ use std::thread; | |||||||||||||||||||||
use crate::alerts::ALERTS; | ||||||||||||||||||||||
use crate::correlation::CORRELATIONS; | ||||||||||||||||||||||
use crate::handlers::airplane; | ||||||||||||||||||||||
use crate::handlers::http::base_path; | ||||||||||||||||||||||
use crate::handlers::http::cluster::{self, init_cluster_metrics_schedular}; | ||||||||||||||||||||||
use crate::handlers::http::middleware::{DisAllowRootUser, RouteExt}; | ||||||||||||||||||||||
use crate::handlers::http::{base_path, prism_base_path}; | ||||||||||||||||||||||
use crate::handlers::http::{logstream, MAX_EVENT_PAYLOAD_SIZE}; | ||||||||||||||||||||||
use crate::handlers::http::{rbac, role}; | ||||||||||||||||||||||
use crate::hottier::HotTierManager; | ||||||||||||||||||||||
|
@@ -61,16 +61,23 @@ impl ParseableServer for QueryServer { | |||||||||||||||||||||
.service(Server::get_about_factory()) | ||||||||||||||||||||||
.service(Self::get_logstream_webscope()) | ||||||||||||||||||||||
.service(Self::get_user_webscope()) | ||||||||||||||||||||||
.service(Server::get_users_webscope()) | ||||||||||||||||||||||
.service(Server::get_dashboards_webscope()) | ||||||||||||||||||||||
.service(Server::get_filters_webscope()) | ||||||||||||||||||||||
.service(Server::get_llm_webscope()) | ||||||||||||||||||||||
.service(Server::get_oauth_webscope(oidc_client)) | ||||||||||||||||||||||
.service(Self::get_user_role_webscope()) | ||||||||||||||||||||||
.service(Server::get_roles_webscope()) | ||||||||||||||||||||||
.service(Server::get_counts_webscope()) | ||||||||||||||||||||||
.service(Server::get_metrics_webscope()) | ||||||||||||||||||||||
.service(Server::get_alerts_webscope()) | ||||||||||||||||||||||
.service(Self::get_cluster_web_scope()), | ||||||||||||||||||||||
) | ||||||||||||||||||||||
.service( | ||||||||||||||||||||||
web::scope(&prism_base_path()) | ||||||||||||||||||||||
.service(Server::get_prism_home()) | ||||||||||||||||||||||
.service(Server::get_prism_logstream()), | ||||||||||||||||||||||
) | ||||||||||||||||||||||
Comment on lines
+76
to
+80
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add authorization middleware to Prism endpoints. The new Prism endpoints are missing authorization middleware, which could lead to unauthorized access to sensitive data. Consider adding appropriate authorization checks using the Apply this diff to add authorization: .service(
web::scope(&prism_base_path())
- .service(Server::get_prism_home())
- .service(Server::get_prism_logstream()),
+ .service(Server::get_prism_home().authorize(Action::ListStream))
+ .service(Server::get_prism_logstream().authorize_for_stream(Action::GetStreamInfo)),
) 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||
.service(Server::get_generated()); | ||||||||||||||||||||||
} | ||||||||||||||||||||||
|
||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -26,6 +26,7 @@ use crate::handlers::http::about; | |||||||||||||
use crate::handlers::http::alerts; | ||||||||||||||
use crate::handlers::http::base_path; | ||||||||||||||
use crate::handlers::http::health_check; | ||||||||||||||
use crate::handlers::http::prism_base_path; | ||||||||||||||
use crate::handlers::http::query; | ||||||||||||||
use crate::handlers::http::users::dashboards; | ||||||||||||||
use crate::handlers::http::users::filters; | ||||||||||||||
|
@@ -80,15 +81,22 @@ impl ParseableServer for Server { | |||||||||||||
.service(Self::get_about_factory()) | ||||||||||||||
.service(Self::get_logstream_webscope()) | ||||||||||||||
.service(Self::get_user_webscope()) | ||||||||||||||
.service(Self::get_users_webscope()) | ||||||||||||||
.service(Self::get_dashboards_webscope()) | ||||||||||||||
.service(Self::get_filters_webscope()) | ||||||||||||||
.service(Self::get_llm_webscope()) | ||||||||||||||
.service(Self::get_oauth_webscope(oidc_client)) | ||||||||||||||
.service(Self::get_user_role_webscope()) | ||||||||||||||
.service(Self::get_roles_webscope()) | ||||||||||||||
.service(Self::get_counts_webscope()) | ||||||||||||||
.service(Self::get_alerts_webscope()) | ||||||||||||||
.service(Self::get_metrics_webscope()), | ||||||||||||||
) | ||||||||||||||
.service( | ||||||||||||||
web::scope(&prism_base_path()) | ||||||||||||||
.service(Server::get_prism_home()) | ||||||||||||||
.service(Server::get_prism_logstream()), | ||||||||||||||
) | ||||||||||||||
.service(Self::get_ingest_otel_factory()) | ||||||||||||||
.service(Self::get_generated()); | ||||||||||||||
} | ||||||||||||||
|
@@ -154,6 +162,24 @@ impl ParseableServer for Server { | |||||||||||||
} | ||||||||||||||
|
||||||||||||||
impl Server { | ||||||||||||||
pub fn get_prism_home() -> Resource { | ||||||||||||||
web::resource("/home").route(web::get().to(http::prism_home::home_api)) | ||||||||||||||
} | ||||||||||||||
Comment on lines
+165
to
+167
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Add authorization middleware to the Prism home endpoint. The implementation follows the routing pattern, but lacks authorization middleware that is present in other similar endpoints (like alerts, metrics, etc.). This is needed to maintain consistent security across all API endpoints. Apply this diff to add authorization: pub fn get_prism_home() -> Resource {
- web::resource("/home").route(web::get().to(http::prism_home::home_api))
+ web::resource("/home").route(web::get().to(http::prism_home::home_api).authorize(Action::GetStreamInfo))
} 📝 Committable suggestion
Suggested change
|
||||||||||||||
|
||||||||||||||
pub fn get_prism_logstream() -> Scope { | ||||||||||||||
web::scope("/logstream").service( | ||||||||||||||
web::scope("/{logstream}").service( | ||||||||||||||
web::resource("/info").route( | ||||||||||||||
web::get() | ||||||||||||||
.to(http::prism_logstream::get_info) | ||||||||||||||
.authorize_for_stream(Action::GetStreamInfo) | ||||||||||||||
.authorize_for_stream(Action::GetStats) | ||||||||||||||
.authorize_for_stream(Action::GetRetention), | ||||||||||||||
), | ||||||||||||||
), | ||||||||||||||
) | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
pub fn get_metrics_webscope() -> Scope { | ||||||||||||||
web::scope("/metrics").service( | ||||||||||||||
web::resource("").route(web::get().to(metrics::get).authorize(Action::Metrics)), | ||||||||||||||
|
@@ -455,6 +481,13 @@ impl Server { | |||||||||||||
} | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
// get list of roles | ||||||||||||||
pub fn get_roles_webscope() -> Scope { | ||||||||||||||
web::scope("/roles").service( | ||||||||||||||
web::resource("").route(web::get().to(role::list_roles).authorize(Action::ListRole)), | ||||||||||||||
) | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
// get the role webscope | ||||||||||||||
pub fn get_user_role_webscope() -> Scope { | ||||||||||||||
web::scope("/role") | ||||||||||||||
|
@@ -475,6 +508,27 @@ impl Server { | |||||||||||||
) | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
// get the users webscope (for Prism only) | ||||||||||||||
pub fn get_users_webscope() -> Scope { | ||||||||||||||
web::scope("/users") | ||||||||||||||
.service( | ||||||||||||||
web::resource("") | ||||||||||||||
// GET /users => List all users | ||||||||||||||
.route( | ||||||||||||||
web::get() | ||||||||||||||
.to(http::rbac::list_users_prism) | ||||||||||||||
.authorize(Action::ListUser), | ||||||||||||||
), | ||||||||||||||
) | ||||||||||||||
.service( | ||||||||||||||
web::resource("/{username}").route( | ||||||||||||||
web::get() | ||||||||||||||
.to(http::rbac::get_prism_user) | ||||||||||||||
.authorize_for_user(Action::GetUserRoles), | ||||||||||||||
), | ||||||||||||||
) | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
// get the user webscope | ||||||||||||||
pub fn get_user_webscope() -> Scope { | ||||||||||||||
web::scope("/user") | ||||||||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❓ Verification inconclusive
Add RBAC implementation as noted in TODO comment.
The function needs Role-Based Access Control implementation for security.
Let's verify the RBAC implementation in other similar functions:
Additionally:
Critical
severity case explicitly instead of silently ignoring it.Apply this diff to improve the implementation:
🏁 Script executed:
Length of output: 57
Action Required: Verify RBAC Integration and Critical Severity Handling
The suggested changes introduce an RBAC check via a new
SessionKey
parameter and update the alerts counting logic to account for a newcritical
field in theAlertsInfo
structure. However, the initial search for RBAC patterns in similar functions did not produce any output. Please manually verify that the RBAC approach (i.e. callinguser_auth_for_query
to filter alerts based on permissions) is consistent with the rest of the codebase and that adding acritical
counter aligns with the existing definition ofAlertsInfo
.session: SessionKey
is appropriate for accessing and validating alerts.user_auth_for_query
exists and is used elsewhere for RBAC enforcement.AlertsInfo
struct definition is updated to include acritical
field.Please ensure these modifications are in line with your security model and that the alerts API is thoroughly tested before merging.
📝 Committable suggestion