Issue 394 restrict edgar api access - #570
Conversation
| .add_service(grpc_facades.cluster_manager_facade.into_grpc_service()) | ||
| .add_service(grpc_facades.metadata_provider_facade.into_grpc_service()) | ||
| .add_service(grpc_facades.peer_manager_facade.into_grpc_service()) | ||
| .add_service(grpc_facades.peer_messaging_broker_facade.into_grpc_service()) |
There was a problem hiding this comment.
Is there a reason to move this line?
If yes, it should be documented...
There was a problem hiding this comment.
No, I think I messed it up a bit. I'll update it.
There was a problem hiding this comment.
Great work on this PR — thanks for driving #394 forward so thoroughly. The overall direction (introducing dedicated scopes, assigning them during client setup, and enforcing them in CARL) is exactly what we needed and it’s a strong security improvement.
I only see a few follow-ups before merge:
- In opendut-carl/src/lib.rs, please remove the duplicate reqwest_client initialization (the first one is currently unused).
- For scope routing, replace u.path().contains("PeerMessagingBroker") with an exact/prefix match on the fully-qualified gRPC service path to avoid accidental misclassification.
- In opendut-auth/src/types.rs, since multiple values can be present, consider renaming the claim field to scopes (or otherwise making that intent explicit).
Thanks again — this is a really valuable contribution and very close to done.
| pub groups: Vec<String>, | ||
| /// OAuth scopes (space-separated string in the JWT) | ||
| #[serde(default)] | ||
| pub scope: String, |
There was a problem hiding this comment.
As written in the comment, there can be multiple scopes encoded. The field should also be named scopes.
| }; | ||
|
|
||
| let mut routes_builder = Routes::builder(); | ||
| let reqwest_client = reqwest_client::oidc::create_from_config(&settings)?; |
There was a problem hiding this comment.
This line is a duplicate of the same line below.
| let is_edge = request | ||
| .extensions() | ||
| .get::<axum::extract::OriginalUri>() | ||
| .map(|u| u.path().contains("PeerMessagingBroker")) |
There was a problem hiding this comment.
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.
…matching, rename scope to scopes
- EDGAR requests edge-api scope for peer messaging - CLEO/LEA request admin-api scope for admin operations - metadata_provider accepts both scopes for version checks - gRPC auth layer validates scope-based access control Fixes eclipse-opendut#394
Restrict EDGAR API access via OAuth scopes (#394)
Implements scope-based authorization so EDGAR clients can only access the edge API (PeerMessagingBroker) and are blocked from all management API endpoints.
Tasks completed:
Create client scopes in Keycloak during provisioning (opendut-admin-api, opendut-edge-api)
Assign scopes to clients after registration, add OAuth scope to default scopes to indicate which API it is allowed to use:
Peer messaging broker (Edge API) checks required scope opendut-edge-api
Administrative API checks required scope opendut-admin-api
I have tested my changes manually.