-
Notifications
You must be signed in to change notification settings - Fork 35
Issue 394 restrict edgar api access #570
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
base: development
Are you sure you want to change the base?
Changes from 2 commits
438a153
524e8aa
a771300
bb4517c
52dc581
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 |
|---|---|---|
|
|
@@ -150,8 +150,9 @@ async fn run(settings: LoadedConfig, get_resource_manager_ref: bool) -> anyhow:: | |
| } | ||
| }; | ||
|
|
||
| let mut routes_builder = Routes::builder(); | ||
| let reqwest_client = reqwest_client::oidc::create_from_config(&settings)?; | ||
|
Contributor
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. This line is a duplicate of the same line below. |
||
|
|
||
| let mut routes_builder = Routes::builder(); | ||
| routes_builder | ||
| .add_service(grpc_facades.cluster_manager_facade.into_grpc_service()) | ||
| .add_service(grpc_facades.metadata_provider_facade.into_grpc_service()) | ||
|
|
@@ -166,8 +167,18 @@ async fn run(settings: LoadedConfig, get_resource_manager_ref: bool) -> anyhow:: | |
| routes_builder | ||
| .routes() | ||
| .into_axum_router() | ||
| .layer(async_interceptor(move |request| { | ||
| Clone::clone(&grpc_auth_layer).auth_interceptor(request, reqwest_client.clone()) | ||
| .layer(async_interceptor(move |request: tonic::Request<()>| { | ||
| let is_edge = request | ||
| .extensions() | ||
| .get::<axum::extract::OriginalUri>() | ||
| .map(|u| u.path().contains("PeerMessagingBroker")) | ||
|
Contributor
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. This match is too broad; it should use an exact/prefix match on the fully-qualified gRPC service path (on OriginalUri) to avoid accidental scope misclassification if other paths ever include that substring. |
||
| .unwrap_or(false); | ||
| let required_scope = if is_edge { | ||
| opendut_auth::types::SCOPE_EDGE_API | ||
| } else { | ||
| opendut_auth::types::SCOPE_ADMIN_API | ||
| }; | ||
| Clone::clone(&grpc_auth_layer).auth_interceptor(request, reqwest_client.clone(), required_scope) | ||
| })) | ||
| }; | ||
|
|
||
|
|
||
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.
As written in the comment, there can be multiple scopes encoded. The field should also be named
scopes.