mirror of
https://github.com/ModernRelay/omnigraph.git
synced 2026-06-12 01:45:14 +02:00
mr-668: GET /graphs endpoint + per-graph policy wire-up (PR 6b/10)
PR 6b of the MR-668 multi-graph server work. First management endpoint —
`GET /graphs` lists every graph registered with the server, gated by the
server-level Cedar policy from PR 6a.
New API shapes (in `omnigraph-server::api`):
- `GraphInfo { graph_id, uri }` — one entry per registered graph.
- `GraphListResponse { graphs: Vec<GraphInfo> }` — sorted alphabetically
by `graph_id` for deterministic output.
Handler `server_graphs_list`:
- Mounted at `GET /graphs` in both modes.
- Single mode: returns 405 (resource exists in the API surface, just
not operational without a `graphs:` map). 405 chosen over 404 so
clients see "resource exists, wrong context" rather than "no such
resource".
- Multi mode: requires bearer auth (when configured); Cedar-gated by
`PolicyAction::GraphList` against `Omnigraph::Server::"root"`
(PR 6a's chassis). Returns the sorted registry list.
Cedar gate composition:
- When no `server.policy.file` is configured, the MR-723 default-deny
falls through: `GraphList` is not `Read`, so an authenticated actor
without a server policy gets 403. This is the right default — don't
expose the registry until the operator explicitly authorizes it.
- When a server policy is configured, Cedar evaluates the rule. The
test `get_graphs_with_server_policy_authorizes_per_cedar` pins the
admin-allow / viewer-deny split.
Routing:
- New `management` sub-router holding `/graphs` (auth-required, no
`resolve_graph_handle` middleware — operates on the registry, not
a single graph).
- Single mode merges flat protected routes + management.
- Multi mode merges nested `/graphs/{graph_id}/...` + management.
OpenAPI:
- `server_graphs_list` registered in `ApiDoc::paths(...)`.
- `EXPECTED_PATHS` in `tests/openapi.rs` gains `/graphs`.
- `openapi.json` regenerated (auto-tracked by
`openapi_spec_is_up_to_date` in CI).
Tests: 4 new in `tests/server.rs::multi_graph_startup`:
- `get_graphs_lists_registered_graphs_in_multi_mode`
- `get_graphs_returns_405_in_single_mode`
- `get_graphs_requires_bearer_auth_when_configured`
- `get_graphs_with_server_policy_authorizes_per_cedar`
What's NOT in this PR (deferred):
- Per-graph policy enforcement is wired through `handle.policy`
(PR 4a already did this); PR 6b doesn't add new per-graph
behavior beyond making sure the server policy lookup composes
cleanly alongside it.
- `POST /graphs` (PR 7) and `DELETE /graphs/{id}` (out of scope
for v0.7.0).
- CLI `omnigraph graphs list` (PR 8 will add).
Result: 215 server tests green (74 lib + 66 openapi + 75 integration),
11 policy tests green. MR-731 spoof regression preserved across all
this work.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
0e5aa036f4
commit
94b6346bdd
5 changed files with 392 additions and 6 deletions
|
|
@ -467,3 +467,23 @@ pub fn read_target_output(target: &ReadTarget) -> ReadTargetOutput {
|
|||
},
|
||||
}
|
||||
}
|
||||
|
||||
// ─── MR-668 PR 6b — management endpoint shapes ─────────────────────────────
|
||||
|
||||
/// One entry in the response from `GET /graphs`. Cluster operators
|
||||
/// consume this list to discover which graphs the server is currently
|
||||
/// serving. The shape is intentionally minimal — `graph_id` and `uri`
|
||||
/// are the only fields a routing client needs.
|
||||
#[derive(Debug, Clone, Serialize, Deserialize, ToSchema)]
|
||||
pub struct GraphInfo {
|
||||
pub graph_id: String,
|
||||
pub uri: String,
|
||||
}
|
||||
|
||||
/// Response from `GET /graphs`. Lists every graph registered with the
|
||||
/// server in alphabetical order by `graph_id` (sorted server-side so
|
||||
/// clients get deterministic output across requests).
|
||||
#[derive(Debug, Clone, Serialize, Deserialize, ToSchema)]
|
||||
pub struct GraphListResponse {
|
||||
pub graphs: Vec<GraphInfo>,
|
||||
}
|
||||
|
|
|
|||
|
|
@ -21,9 +21,10 @@ use std::sync::Arc;
|
|||
use api::{
|
||||
BranchCreateOutput, BranchCreateRequest, BranchDeleteOutput, BranchListOutput,
|
||||
BranchMergeOutput, BranchMergeRequest, ChangeOutput, ChangeRequest, CommitListOutput,
|
||||
CommitListQuery, ErrorCode, ErrorOutput, ExportRequest, HealthOutput, IngestOutput,
|
||||
IngestRequest, ReadOutput, ReadRequest, SchemaApplyOutput, SchemaApplyRequest, SchemaOutput,
|
||||
SnapshotQuery, ingest_output, schema_apply_output, snapshot_payload,
|
||||
CommitListQuery, ErrorCode, ErrorOutput, ExportRequest, GraphInfo, GraphListResponse,
|
||||
HealthOutput, IngestOutput, IngestRequest, ReadOutput, ReadRequest, SchemaApplyOutput,
|
||||
SchemaApplyRequest, SchemaOutput, SnapshotQuery, ingest_output, schema_apply_output,
|
||||
snapshot_payload,
|
||||
};
|
||||
pub use auth::{AWS_SECRET_ENV, EnvOrFileTokenSource, TokenSource, resolve_token_source};
|
||||
use axum::body::{Body, Bytes};
|
||||
|
|
@ -79,6 +80,7 @@ fn hash_bearer_token(token: &str) -> BearerTokenHash {
|
|||
),
|
||||
paths(
|
||||
server_health,
|
||||
server_graphs_list,
|
||||
server_snapshot,
|
||||
server_read,
|
||||
server_export,
|
||||
|
|
@ -909,13 +911,27 @@ pub fn build_app(state: AppState) -> Router {
|
|||
require_bearer_auth,
|
||||
));
|
||||
|
||||
// Management endpoints (`GET /graphs`, future `POST /graphs`)
|
||||
// live alongside the per-graph router. They go through bearer auth
|
||||
// but NOT through `resolve_graph_handle` — they operate on the
|
||||
// registry directly. The endpoint is mounted in both modes; in
|
||||
// single mode the handler returns 405 so clients see "resource
|
||||
// exists, wrong context" rather than 404 "no such resource."
|
||||
let management = Router::new()
|
||||
.route("/graphs", get(server_graphs_list))
|
||||
.route_layer(middleware::from_fn_with_state(
|
||||
state.clone(),
|
||||
require_bearer_auth,
|
||||
));
|
||||
|
||||
// Mount the protected routes differently per mode:
|
||||
// * Single → flat routes (legacy: `/snapshot`, `/read`, etc.)
|
||||
// * Multi → nested under `/graphs/{graph_id}/...`
|
||||
// Mode is inferred at startup; the same router code branches once.
|
||||
let protected: Router<AppState> = match state.mode() {
|
||||
ServerMode::Single { .. } => per_graph_protected,
|
||||
ServerMode::Multi { .. } => Router::new().nest("/graphs/{graph_id}", per_graph_protected),
|
||||
ServerMode::Single { .. } => per_graph_protected.merge(management),
|
||||
ServerMode::Multi { .. } => Router::new()
|
||||
.nest("/graphs/{graph_id}", per_graph_protected)
|
||||
.merge(management),
|
||||
};
|
||||
|
||||
Router::new()
|
||||
|
|
@ -1108,6 +1124,78 @@ async fn server_health() -> Json<HealthOutput> {
|
|||
})
|
||||
}
|
||||
|
||||
#[utoipa::path(
|
||||
get,
|
||||
path = "/graphs",
|
||||
tag = "management",
|
||||
operation_id = "listGraphs",
|
||||
responses(
|
||||
(status = 200, description = "List of registered graphs", body = GraphListResponse),
|
||||
(status = 401, description = "Unauthorized", body = ErrorOutput),
|
||||
(status = 403, description = "Forbidden", body = ErrorOutput),
|
||||
(status = 405, description = "Method not allowed (single-graph mode)", body = ErrorOutput),
|
||||
),
|
||||
security(("bearer_token" = [])),
|
||||
)]
|
||||
/// List every graph currently registered with this server (MR-668).
|
||||
///
|
||||
/// Multi-graph mode only. In single mode, the route returns 405 — there's
|
||||
/// no registry to enumerate. Cedar-gated by the server-level policy via
|
||||
/// the `graph_list` action against `Omnigraph::Server::"root"`.
|
||||
///
|
||||
/// Order: alphabetical by `graph_id` (server-sorted so clients see
|
||||
/// deterministic output across requests).
|
||||
async fn server_graphs_list(
|
||||
State(state): State<AppState>,
|
||||
actor: Option<Extension<ResolvedActor>>,
|
||||
) -> std::result::Result<Json<GraphListResponse>, ApiError> {
|
||||
// 405 in single mode — there's no registry to enumerate, and the
|
||||
// legacy URL surface didn't expose this endpoint.
|
||||
if matches!(state.mode(), ServerMode::Single { .. }) {
|
||||
return Err(ApiError {
|
||||
status: StatusCode::METHOD_NOT_ALLOWED,
|
||||
code: ErrorCode::BadRequest,
|
||||
message: "GET /graphs is only available in multi-graph mode".to_string(),
|
||||
merge_conflicts: Vec::new(),
|
||||
manifest_conflict: None,
|
||||
});
|
||||
}
|
||||
|
||||
// Server-level Cedar gate. `state.server_policy` is loaded from
|
||||
// `server.policy.file` in `omnigraph.yaml` at startup. When no
|
||||
// server policy is configured, `authorize_request_server` falls
|
||||
// through to the MR-723 default-deny semantics (every non-Read
|
||||
// action denied for an authenticated actor). `GraphList` is not
|
||||
// `Read`, so without a server policy the request gets 403 — which
|
||||
// is the right default (don't leak the registry until the operator
|
||||
// explicitly authorizes it).
|
||||
authorize_request(
|
||||
actor.as_ref().map(|Extension(actor)| actor),
|
||||
state.server_policy.as_deref(),
|
||||
PolicyRequest {
|
||||
actor_id: actor
|
||||
.as_ref()
|
||||
.map(|Extension(actor)| actor.actor_id.as_ref().to_string())
|
||||
.unwrap_or_default(),
|
||||
action: PolicyAction::GraphList,
|
||||
branch: None,
|
||||
target_branch: None,
|
||||
},
|
||||
)?;
|
||||
|
||||
let mut graphs: Vec<GraphInfo> = state
|
||||
.registry()
|
||||
.list()
|
||||
.into_iter()
|
||||
.map(|handle| GraphInfo {
|
||||
graph_id: handle.key.graph_id.as_str().to_string(),
|
||||
uri: handle.uri.clone(),
|
||||
})
|
||||
.collect();
|
||||
graphs.sort_by(|a, b| a.graph_id.cmp(&b.graph_id));
|
||||
Ok(Json(GraphListResponse { graphs }))
|
||||
}
|
||||
|
||||
async fn server_openapi(State(state): State<AppState>) -> Json<utoipa::openapi::OpenApi> {
|
||||
let mut doc = ApiDoc::openapi();
|
||||
if !state.requires_bearer_auth() {
|
||||
|
|
|
|||
|
|
@ -161,6 +161,7 @@ fn openapi_info_contains_version() {
|
|||
|
||||
const EXPECTED_PATHS: &[&str] = &[
|
||||
"/healthz",
|
||||
"/graphs",
|
||||
"/snapshot",
|
||||
"/read",
|
||||
"/export",
|
||||
|
|
|
|||
|
|
@ -4675,6 +4675,195 @@ graphs:
|
|||
}
|
||||
}
|
||||
|
||||
/// `GET /graphs` lists the registered graphs alphabetically in
|
||||
/// multi mode. No auth or server policy = open mode (allowed).
|
||||
#[tokio::test(flavor = "multi_thread")]
|
||||
async fn get_graphs_lists_registered_graphs_in_multi_mode() {
|
||||
let (_dirs, app) = build_multi_mode_app(&["beta", "alpha"]).await;
|
||||
let resp = app
|
||||
.oneshot(
|
||||
Request::builder()
|
||||
.method(Method::GET)
|
||||
.uri("/graphs")
|
||||
.body(Body::empty())
|
||||
.unwrap(),
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
assert_eq!(resp.status(), StatusCode::OK);
|
||||
let body = to_bytes(resp.into_body(), usize::MAX).await.unwrap();
|
||||
let json: Value = serde_json::from_slice(&body).unwrap();
|
||||
let graphs = json["graphs"].as_array().unwrap();
|
||||
assert_eq!(graphs.len(), 2);
|
||||
// Server-sorted alphabetically.
|
||||
assert_eq!(graphs[0]["graph_id"].as_str().unwrap(), "alpha");
|
||||
assert_eq!(graphs[1]["graph_id"].as_str().unwrap(), "beta");
|
||||
}
|
||||
|
||||
/// `GET /graphs` returns 405 in single mode (resource exists in the
|
||||
/// API surface, just not operational without a `graphs:` map).
|
||||
#[tokio::test(flavor = "multi_thread")]
|
||||
async fn get_graphs_returns_405_in_single_mode() {
|
||||
let temp = init_loaded_graph().await;
|
||||
let graph = graph_path(temp.path());
|
||||
let state = AppState::open(graph.to_string_lossy().to_string())
|
||||
.await
|
||||
.unwrap();
|
||||
let app = build_app(state);
|
||||
let resp = app
|
||||
.oneshot(
|
||||
Request::builder()
|
||||
.method(Method::GET)
|
||||
.uri("/graphs")
|
||||
.body(Body::empty())
|
||||
.unwrap(),
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
assert_eq!(resp.status(), StatusCode::METHOD_NOT_ALLOWED);
|
||||
}
|
||||
|
||||
/// `GET /graphs` requires bearer auth when tokens are configured.
|
||||
#[tokio::test(flavor = "multi_thread")]
|
||||
async fn get_graphs_requires_bearer_auth_when_configured() {
|
||||
use omnigraph_server::{GraphHandle, GraphId, GraphKey};
|
||||
// Build a multi-mode app with bearer tokens configured.
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let graph_uri = dir.path().join("alpha").to_str().unwrap().to_string();
|
||||
let schema = fs::read_to_string(fixture("test.pg")).unwrap();
|
||||
let engine = Omnigraph::init(&graph_uri, &schema).await.unwrap();
|
||||
let handle = Arc::new(GraphHandle {
|
||||
key: GraphKey::cluster(GraphId::try_from("alpha").unwrap()),
|
||||
uri: graph_uri,
|
||||
engine: Arc::new(engine),
|
||||
policy: None,
|
||||
});
|
||||
let tokens = vec![("act-andrew".to_string(), "secret-token".to_string())];
|
||||
let workload = omnigraph_server::workload::WorkloadController::from_env();
|
||||
let state =
|
||||
AppState::new_multi(vec![handle], tokens, None, workload, None).unwrap();
|
||||
let app = build_app(state);
|
||||
|
||||
// No Authorization header → 401.
|
||||
let resp_no_auth = app
|
||||
.clone()
|
||||
.oneshot(
|
||||
Request::builder()
|
||||
.method(Method::GET)
|
||||
.uri("/graphs")
|
||||
.body(Body::empty())
|
||||
.unwrap(),
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
assert_eq!(resp_no_auth.status(), StatusCode::UNAUTHORIZED);
|
||||
|
||||
// With auth but no server policy → 403 (default-deny, since
|
||||
// GraphList is not Read).
|
||||
let resp_authed = app
|
||||
.oneshot(
|
||||
Request::builder()
|
||||
.method(Method::GET)
|
||||
.uri("/graphs")
|
||||
.header("authorization", "Bearer secret-token")
|
||||
.body(Body::empty())
|
||||
.unwrap(),
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
assert_eq!(resp_authed.status(), StatusCode::FORBIDDEN);
|
||||
}
|
||||
|
||||
/// `GET /graphs` with a server policy that allows `graph_list` → 200.
|
||||
/// `GET /graphs` with a server policy that does NOT allow `graph_list` → 403.
|
||||
#[tokio::test(flavor = "multi_thread")]
|
||||
async fn get_graphs_with_server_policy_authorizes_per_cedar() {
|
||||
use omnigraph_policy::PolicyEngine;
|
||||
use omnigraph_server::{GraphHandle, GraphId, GraphKey};
|
||||
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let graph_uri = dir.path().join("alpha").to_str().unwrap().to_string();
|
||||
let schema = fs::read_to_string(fixture("test.pg")).unwrap();
|
||||
let engine = Omnigraph::init(&graph_uri, &schema).await.unwrap();
|
||||
let handle = Arc::new(GraphHandle {
|
||||
key: GraphKey::cluster(GraphId::try_from("alpha").unwrap()),
|
||||
uri: graph_uri,
|
||||
engine: Arc::new(engine),
|
||||
policy: None,
|
||||
});
|
||||
|
||||
// Server policy: admins can graph_list, viewers cannot.
|
||||
let policy_path = dir.path().join("server-policy.yaml");
|
||||
fs::write(
|
||||
&policy_path,
|
||||
r#"
|
||||
version: 1
|
||||
groups:
|
||||
admins: [act-andrew]
|
||||
viewers: [act-bruno]
|
||||
rules:
|
||||
- id: admins-list-graphs
|
||||
allow:
|
||||
actors: { group: admins }
|
||||
actions: [graph_list]
|
||||
"#,
|
||||
)
|
||||
.unwrap();
|
||||
let server_policy = PolicyEngine::load(&policy_path, "server").unwrap();
|
||||
|
||||
let tokens = vec![
|
||||
("act-andrew".to_string(), "andrew-token".to_string()),
|
||||
("act-bruno".to_string(), "bruno-token".to_string()),
|
||||
];
|
||||
let workload = omnigraph_server::workload::WorkloadController::from_env();
|
||||
let state = AppState::new_multi(
|
||||
vec![handle],
|
||||
tokens,
|
||||
Some(server_policy),
|
||||
workload,
|
||||
None,
|
||||
)
|
||||
.unwrap();
|
||||
let app = build_app(state);
|
||||
|
||||
// Admin → 200
|
||||
let resp_admin = app
|
||||
.clone()
|
||||
.oneshot(
|
||||
Request::builder()
|
||||
.method(Method::GET)
|
||||
.uri("/graphs")
|
||||
.header("authorization", "Bearer andrew-token")
|
||||
.body(Body::empty())
|
||||
.unwrap(),
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
assert_eq!(
|
||||
resp_admin.status(),
|
||||
StatusCode::OK,
|
||||
"admin must be allowed graph_list"
|
||||
);
|
||||
|
||||
// Viewer → 403
|
||||
let resp_viewer = app
|
||||
.oneshot(
|
||||
Request::builder()
|
||||
.method(Method::GET)
|
||||
.uri("/graphs")
|
||||
.header("authorization", "Bearer bruno-token")
|
||||
.body(Body::empty())
|
||||
.unwrap(),
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
assert_eq!(
|
||||
resp_viewer.status(),
|
||||
StatusCode::FORBIDDEN,
|
||||
"viewer must be denied graph_list (Cedar gate)"
|
||||
);
|
||||
}
|
||||
|
||||
/// End-to-end: load an `omnigraph.yaml` with two graphs and serve
|
||||
/// them. Both graphs must be queryable via cluster routes.
|
||||
///
|
||||
|
|
|
|||
88
openapi.json
88
openapi.json
|
|
@ -585,6 +585,63 @@
|
|||
]
|
||||
}
|
||||
},
|
||||
"/graphs": {
|
||||
"get": {
|
||||
"tags": [
|
||||
"management"
|
||||
],
|
||||
"summary": "List every graph currently registered with this server (MR-668).",
|
||||
"description": "Multi-graph mode only. In single mode, the route returns 405 — there's\nno registry to enumerate. Cedar-gated by the server-level policy via\nthe `graph_list` action against `Omnigraph::Server::\"root\"`.\n\nOrder: alphabetical by `graph_id` (server-sorted so clients see\ndeterministic output across requests).",
|
||||
"operationId": "listGraphs",
|
||||
"responses": {
|
||||
"200": {
|
||||
"description": "List of registered graphs",
|
||||
"content": {
|
||||
"application/json": {
|
||||
"schema": {
|
||||
"$ref": "#/components/schemas/GraphListResponse"
|
||||
}
|
||||
}
|
||||
}
|
||||
},
|
||||
"401": {
|
||||
"description": "Unauthorized",
|
||||
"content": {
|
||||
"application/json": {
|
||||
"schema": {
|
||||
"$ref": "#/components/schemas/ErrorOutput"
|
||||
}
|
||||
}
|
||||
}
|
||||
},
|
||||
"403": {
|
||||
"description": "Forbidden",
|
||||
"content": {
|
||||
"application/json": {
|
||||
"schema": {
|
||||
"$ref": "#/components/schemas/ErrorOutput"
|
||||
}
|
||||
}
|
||||
}
|
||||
},
|
||||
"405": {
|
||||
"description": "Method not allowed (single-graph mode)",
|
||||
"content": {
|
||||
"application/json": {
|
||||
"schema": {
|
||||
"$ref": "#/components/schemas/ErrorOutput"
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
},
|
||||
"security": [
|
||||
{
|
||||
"bearer_token": []
|
||||
}
|
||||
]
|
||||
}
|
||||
},
|
||||
"/healthz": {
|
||||
"get": {
|
||||
"tags": [
|
||||
|
|
@ -1268,6 +1325,37 @@
|
|||
}
|
||||
}
|
||||
},
|
||||
"GraphInfo": {
|
||||
"type": "object",
|
||||
"description": "One entry in the response from `GET /graphs`. Cluster operators\nconsume this list to discover which graphs the server is currently\nserving. The shape is intentionally minimal — `graph_id` and `uri`\nare the only fields a routing client needs.",
|
||||
"required": [
|
||||
"graph_id",
|
||||
"uri"
|
||||
],
|
||||
"properties": {
|
||||
"graph_id": {
|
||||
"type": "string"
|
||||
},
|
||||
"uri": {
|
||||
"type": "string"
|
||||
}
|
||||
}
|
||||
},
|
||||
"GraphListResponse": {
|
||||
"type": "object",
|
||||
"description": "Response from `GET /graphs`. Lists every graph registered with the\nserver in alphabetical order by `graph_id` (sorted server-side so\nclients get deterministic output across requests).",
|
||||
"required": [
|
||||
"graphs"
|
||||
],
|
||||
"properties": {
|
||||
"graphs": {
|
||||
"type": "array",
|
||||
"items": {
|
||||
"$ref": "#/components/schemas/GraphInfo"
|
||||
}
|
||||
}
|
||||
}
|
||||
},
|
||||
"HealthOutput": {
|
||||
"type": "object",
|
||||
"required": [
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue