From a2cd694e76804aae1d83541a0aa521e97abe72f0 Mon Sep 17 00:00:00 2001 From: Sihao Jiang Date: Tue, 28 Jul 2026 17:35:17 -0400 Subject: [PATCH 1/2] feat(policy): Implement legacy Policy API (/v3/policies) Add the deprecated `/v3/policies` document store as a new `policy_store` domain, closing one row of the python compatibility matrix (#1035). The API stores opaque policy documents that remote services fetch and interpret; it is unrelated to this service's OPA-based authorization, hence `policy_store` rather than `policy` (`core::policy` is the OPA enforcement client, and `[api_policy]` its config section). Routes mirror python keystone exactly: GET/POST on the collection and GET/PATCH/DELETE on the member, with no PUT. Compatibility details carried over from upstream: `blob` is accepted as a string only and returned as the stored JSON value; `type` is required but, matching upstream's `maxLength`-only schema, may be empty; `id` on create is an unconstrained additional property that is accepted in any JSON shape and discarded; a PATCH `id` must equal the path; an empty patch document is a 400; extras round-trip and are merged on PATCH; and additional-property names are normalized on create only. Security: `list` re-checks every item with the per-item show policy and propagates non-Forbidden policy errors (I8). OPA input is built from an allowlist of `id`/`type` rather than by filtering, because `blob` and `extra` are arbitrary caller-supplied data with an open key space (I7). Handlers and the SQL backend use `skip_all` spans so those fields never reach a debug log. The service layer is the sole source of the policy id, assigning it before the audited operation so the event carries the final identifier; the backend rejects a missing id rather than minting a second one. A malformed stored `extra` is an error rather than an empty map, since `update` writes the decoded extras back and would otherwise destroy a row's properties on an ordinary PATCH. The new Rego reads `input.credentials.system`, the key `Credentials` actually emits; several older policies compare against a `system_scope` key that is never emitted, which is recorded as a known gap. Co-Authored-By: Claude Opus 5 Signed-off-by: Sihao Jiang --- Cargo.lock | 16 + Cargo.toml | 2 + crates/api-types/src/error_conv.rs | 15 + crates/api-types/src/v3.rs | 3 + crates/api-types/src/v3/policy.rs | 394 ++++++++++ crates/api-types/src/v3/policy_conv.rs | 144 ++++ crates/config/src/lib.rs | 9 + crates/config/src/policy_store.rs | 46 ++ crates/core-types/src/error.rs | 9 + crates/core-types/src/events.rs | 9 + crates/core-types/src/lib.rs | 1 + crates/core-types/src/policy_store.rs | 29 + crates/core-types/src/policy_store/error.rs | 58 ++ crates/core-types/src/policy_store/policy.rs | 129 ++++ crates/core/src/api.rs | 28 + crates/core/src/cadf_hook.rs | 1 + crates/core/src/lib.rs | 1 + crates/core/src/mocks.rs | 47 ++ crates/core/src/plugin_manager.rs | 27 + crates/core/src/policy_store/backend.rs | 101 +++ crates/core/src/policy_store/error.rs | 30 + crates/core/src/policy_store/mod.rs | 44 ++ crates/core/src/policy_store/provider_api.rs | 104 +++ crates/core/src/policy_store/service.rs | 242 ++++++ crates/core/src/provider.rs | 24 + crates/keystone/Cargo.toml | 1 + crates/keystone/src/api/mod.rs | 42 +- crates/keystone/src/api/v3/mod.rs | 2 + crates/keystone/src/api/v3/policy/create.rs | 399 ++++++++++ crates/keystone/src/api/v3/policy/delete.rs | 286 +++++++ crates/keystone/src/api/v3/policy/list.rs | 712 ++++++++++++++++++ crates/keystone/src/api/v3/policy/mod.rs | 390 ++++++++++ crates/keystone/src/api/v3/policy/show.rs | 278 +++++++ crates/keystone/src/api/v3/policy/types.rs | 21 + crates/keystone/src/api/v3/policy/update.rs | 407 ++++++++++ crates/keystone/src/lib.rs | 1 + crates/keystone/src/plugin_manager.rs | 37 + crates/keystone/src/policy_store/mod.rs | 21 + crates/policy-store-driver-sql/Cargo.toml | 30 + crates/policy-store-driver-sql/src/entity.rs | 19 + .../src/entity/policy.rs | 52 ++ .../src/entity/prelude.rs | 18 + crates/policy-store-driver-sql/src/lib.rs | 212 ++++++ crates/policy-store-driver-sql/src/policy.rs | 230 ++++++ .../src/policy/create.rs | 114 +++ .../src/policy/delete.rs | 90 +++ .../policy-store-driver-sql/src/policy/get.rs | 86 +++ .../src/policy/list.rs | 185 +++++ .../src/policy/update.rs | 192 +++++ doc/src/SUMMARY.md | 1 + doc/src/contributor/python-compatibility.md | 125 +++ doc/src/contributor/security-model.md | 43 +- policy/policy/create.rego | 34 + policy/policy/create_test.rego | 24 + policy/policy/delete.rego | 33 + policy/policy/delete_test.rego | 22 + policy/policy/list.rego | 53 ++ policy/policy/list_test.rego | 26 + policy/policy/show.rego | 52 ++ policy/policy/show_test.rego | 25 + policy/policy/update.rego | 34 + policy/policy/update_test.rego | 23 + tests/api/src/lib.rs | 1 + tests/api/src/policy.rs | 246 ++++++ tests/api/tests/api_v3.rs | 1 + tests/api/tests/api_v3/policy.rs | 18 + tests/api/tests/api_v3/policy/create.rs | 102 +++ tests/api/tests/api_v3/policy/delete.rs | 63 ++ tests/api/tests/api_v3/policy/list.rs | 105 +++ tests/api/tests/api_v3/policy/show.rs | 61 ++ tests/api/tests/api_v3/policy/update.rs | 170 +++++ tests/integration/src/integration.rs | 1 + tests/integration/src/policy_store.rs | 45 ++ tests/integration/src/policy_store/crud.rs | 312 ++++++++ 74 files changed, 6954 insertions(+), 4 deletions(-) create mode 100644 crates/api-types/src/v3/policy.rs create mode 100644 crates/api-types/src/v3/policy_conv.rs create mode 100644 crates/config/src/policy_store.rs create mode 100644 crates/core-types/src/policy_store.rs create mode 100644 crates/core-types/src/policy_store/error.rs create mode 100644 crates/core-types/src/policy_store/policy.rs create mode 100644 crates/core/src/policy_store/backend.rs create mode 100644 crates/core/src/policy_store/error.rs create mode 100644 crates/core/src/policy_store/mod.rs create mode 100644 crates/core/src/policy_store/provider_api.rs create mode 100644 crates/core/src/policy_store/service.rs create mode 100644 crates/keystone/src/api/v3/policy/create.rs create mode 100644 crates/keystone/src/api/v3/policy/delete.rs create mode 100644 crates/keystone/src/api/v3/policy/list.rs create mode 100644 crates/keystone/src/api/v3/policy/mod.rs create mode 100644 crates/keystone/src/api/v3/policy/show.rs create mode 100644 crates/keystone/src/api/v3/policy/types.rs create mode 100644 crates/keystone/src/api/v3/policy/update.rs create mode 100644 crates/keystone/src/policy_store/mod.rs create mode 100644 crates/policy-store-driver-sql/Cargo.toml create mode 100644 crates/policy-store-driver-sql/src/entity.rs create mode 100644 crates/policy-store-driver-sql/src/entity/policy.rs create mode 100644 crates/policy-store-driver-sql/src/entity/prelude.rs create mode 100644 crates/policy-store-driver-sql/src/lib.rs create mode 100644 crates/policy-store-driver-sql/src/policy.rs create mode 100644 crates/policy-store-driver-sql/src/policy/create.rs create mode 100644 crates/policy-store-driver-sql/src/policy/delete.rs create mode 100644 crates/policy-store-driver-sql/src/policy/get.rs create mode 100644 crates/policy-store-driver-sql/src/policy/list.rs create mode 100644 crates/policy-store-driver-sql/src/policy/update.rs create mode 100644 doc/src/contributor/python-compatibility.md create mode 100644 policy/policy/create.rego create mode 100644 policy/policy/create_test.rego create mode 100644 policy/policy/delete.rego create mode 100644 policy/policy/delete_test.rego create mode 100644 policy/policy/list.rego create mode 100644 policy/policy/list_test.rego create mode 100644 policy/policy/show.rego create mode 100644 policy/policy/show_test.rego create mode 100644 policy/policy/update.rego create mode 100644 policy/policy/update_test.rego create mode 100644 tests/api/src/policy.rs create mode 100644 tests/api/tests/api_v3/policy.rs create mode 100644 tests/api/tests/api_v3/policy/create.rs create mode 100644 tests/api/tests/api_v3/policy/delete.rs create mode 100644 tests/api/tests/api_v3/policy/list.rs create mode 100644 tests/api/tests/api_v3/policy/show.rs create mode 100644 tests/api/tests/api_v3/policy/update.rs create mode 100644 tests/integration/src/policy_store.rs create mode 100644 tests/integration/src/policy_store/crud.rs diff --git a/Cargo.lock b/Cargo.lock index 993edde0f..2d7272bbf 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4874,6 +4874,7 @@ dependencies = [ "openstack-keystone-oauth2-key-driver-raft", "openstack-keystone-oauth2-session-driver-raft", "openstack-keystone-password-hashing", + "openstack-keystone-policy-store-driver-sql", "openstack-keystone-resource-driver-sql", "openstack-keystone-revoke-driver-sql", "openstack-keystone-role-driver-sql", @@ -5515,6 +5516,21 @@ dependencies = [ "tracing-test", ] +[[package]] +name = "openstack-keystone-policy-store-driver-sql" +version = "0.1.0" +dependencies = [ + "async-trait", + "inventory", + "openstack-keystone-core", + "openstack-keystone-core-types", + "sea-orm", + "serde_json", + "tokio", + "tracing", + "uuid", +] + [[package]] name = "openstack-keystone-resource-driver-sql" version = "0.1.0" diff --git a/Cargo.toml b/Cargo.toml index 3976f69f7..87683eeb7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -26,6 +26,7 @@ members = [ "crates/oauth2-key-driver-raft", "crates/oauth2-session-driver-raft", "crates/password-hashing", + "crates/policy-store-driver-sql", "crates/resource-driver-sql", "crates/role-driver-sql", "crates/revoke-driver-sql", @@ -146,6 +147,7 @@ openstack-keystone-password-hashing = { version = "0.1", path = "crates/password openstack-keystone-identity-driver-ldap = { version = "0.1", path = "crates/identity-driver-ldap" } openstack-keystone-identity-driver-sql = { version = "0.1", path = "crates/identity-driver-sql" } openstack-keystone-idmapping-driver-sql = { version = "0.1", path = "crates/idmapping-driver-sql/" } +openstack-keystone-policy-store-driver-sql = { version = "0.1", path = "crates/policy-store-driver-sql/" } openstack-keystone-resource-driver-sql = { version = "0.1", path = "crates/resource-driver-sql/" } openstack-keystone-revoke-driver-sql = { version = "0.1", path = "crates/revoke-driver-sql/" } openstack-keystone-role-driver-sql = { version = "0.1", path = "crates/role-driver-sql/" } diff --git a/crates/api-types/src/error_conv.rs b/crates/api-types/src/error_conv.rs index 0e8e54449..6a215c74d 100644 --- a/crates/api-types/src/error_conv.rs +++ b/crates/api-types/src/error_conv.rs @@ -35,6 +35,7 @@ use openstack_keystone_core_types::identity::IdentityProviderError; use openstack_keystone_core_types::mapping::MappingProviderError; use openstack_keystone_core_types::oauth2_client::Oauth2ClientProviderError; use openstack_keystone_core_types::oauth2_key::Oauth2KeyProviderError; +use openstack_keystone_core_types::policy_store::PolicyStoreProviderError; use openstack_keystone_core_types::resource::ResourceProviderError; use openstack_keystone_core_types::revoke::RevokeProviderError; use openstack_keystone_core_types::role::RoleProviderError; @@ -230,6 +231,20 @@ impl From for KeystoneApiError { } } +impl From for KeystoneApiError { + fn from(value: PolicyStoreProviderError) -> Self { + match value { + ref err @ PolicyStoreProviderError::Conflict(..) => Self::Conflict(err.to_string()), + PolicyStoreProviderError::PolicyNotFound(x) => Self::NotFound { + resource: "policy".into(), + identifier: x, + }, + PolicyStoreProviderError::Validation { source } => Self::BadRequest(source.to_string()), + other => Self::InternalError(other.to_string()), + } + } +} + impl From for KeystoneApiError { fn from(value: CatalogProviderError) -> Self { match value { diff --git a/crates/api-types/src/v3.rs b/crates/api-types/src/v3.rs index 606868fe7..6ececef5d 100644 --- a/crates/api-types/src/v3.rs +++ b/crates/api-types/src/v3.rs @@ -19,6 +19,7 @@ pub mod ec2tokens; pub mod endpoint; pub mod group; pub mod os_ec2_credential; +pub mod policy; pub mod project; pub mod region; pub mod role; @@ -38,6 +39,8 @@ mod endpoint_conv; #[cfg(feature = "conv")] mod group_conv; #[cfg(feature = "conv")] +mod policy_conv; +#[cfg(feature = "conv")] mod project_conv; #[cfg(feature = "conv")] mod region_conv; diff --git a/crates/api-types/src/v3/policy.rs b/crates/api-types/src/v3/policy.rs new file mode 100644 index 000000000..ac0fbfd32 --- /dev/null +++ b/crates/api-types/src/v3/policy.rs @@ -0,0 +1,394 @@ +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 +//! # Legacy policy (`/v3/policies`) API types. +//! +//! The `blob` asymmetry between request and response types is deliberate and +//! matches python keystone: +//! +//! * **Requests** accept a `String` only, per `keystone/policy/schema.py` +//! (`{'blob': {'type': 'string'}}`). An object-valued `blob` is a 400. +//! * **Responses** carry an arbitrary JSON value, because the stored column +//! round-trips whatever was written and rows created by older python +//! keystone releases may hold objects. + +use std::collections::HashMap; + +use serde::{Deserialize, Serialize}; +use serde_json::Value; +#[cfg(feature = "validate")] +use validator::Validate; + +/// Build the OPA policy input for a policy resource from an explicit +/// allowlist of non-sensitive fields. +/// +/// # Security Note +/// +/// Policy documents are opaque caller-supplied data: `blob` holds the +/// document itself and `extra` holds arbitrary unknown properties, either of +/// which may contain secrets a caller chose to put there. No `.rego` rule in +/// `policy/policy/` reads them, and an external OPA can persist policy input +/// through decision logging, so the input is **constructed** from known-safe +/// fields rather than filtered — a denylist would have to be extended every +/// time a field is added, and could never cover `extra`'s open key space. +fn policy_input(id: Option<&str>, r#type: Option<&str>) -> Value { + let mut input = serde_json::Map::new(); + input.insert("id".to_string(), serde_json::json!(id)); + input.insert("type".to_string(), serde_json::json!(r#type)); + Value::Object(input) +} + +/// The policy data. +#[derive(Clone, Debug, Deserialize, PartialEq, Serialize)] +#[cfg_attr(feature = "openapi", derive(utoipa::ToSchema))] +#[cfg_attr(feature = "validate", derive(validator::Validate))] +pub struct Policy { + /// Policy ID. + #[cfg_attr(feature = "validate", validate(length(min = 1, max = 64)))] + pub id: String, + + /// The MIME media type of the serialized policy blob. + #[cfg_attr(feature = "validate", validate(length(max = 255)))] + pub r#type: String, + + /// The policy rule set itself, as a serialized blob. + /// + /// Any JSON value: normally the string the caller supplied on create, but + /// an object for rows written by older python keystone releases. No + /// `value_type` override here on purpose — `serde_json::Value`'s native + /// utoipa mapping is an unconstrained `AnyValue`, whereas `Object` would + /// document `type: object` and make a generated client reject the ordinary + /// string case. + pub blob: Value, + + #[cfg_attr(feature = "openapi", schema(inline, additional_properties))] + #[serde(flatten)] + pub extra: HashMap, +} + +impl Policy { + /// Non-sensitive policy-input projection. See [`policy_input`]. + #[must_use] + pub fn to_policy_input(&self) -> Value { + policy_input(Some(&self.id), Some(&self.r#type)) + } +} + +#[derive(Clone, Debug, Deserialize, PartialEq, Serialize)] +#[cfg_attr(feature = "openapi", derive(utoipa::ToSchema))] +#[cfg_attr(feature = "validate", derive(validator::Validate))] +pub struct PolicyResponse { + /// Policy object. + #[cfg_attr(feature = "validate", validate(nested))] + pub policy: Policy, +} + +/// Policies. +#[derive(Clone, Debug, Deserialize, PartialEq, Serialize)] +#[cfg_attr(feature = "openapi", derive(utoipa::ToSchema))] +#[cfg_attr(feature = "validate", derive(validator::Validate))] +pub struct PolicyList { + /// Collection of policy objects. + #[cfg_attr(feature = "validate", validate(nested))] + pub policies: Vec, + + /// Pagination links. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub links: Option>, +} + +#[derive(Clone, Debug, Default, Deserialize, Serialize)] +#[cfg_attr(feature = "openapi", derive(utoipa::IntoParams))] +#[cfg_attr(feature = "validate", derive(validator::Validate))] +pub struct PolicyListParameters { + /// Filters the response by a MIME media type for the serialized policy + /// blob. For example, `application/json`. Exact match. + #[cfg_attr(feature = "validate", validate(length(max = 255)))] + pub r#type: Option, +} + +impl PolicyListParameters { + /// Non-sensitive policy-input projection. See [`policy_input`]. + #[must_use] + pub fn to_policy_input(&self) -> Value { + policy_input(None, self.r#type.as_deref()) + } +} + +/// Policy create request body. +#[derive(Clone, Debug, Deserialize, PartialEq, Serialize)] +#[cfg_attr( + feature = "builder", + derive(derive_builder::Builder), + builder( + build_fn(error = "crate::error::BuilderError"), + setter(strip_option, into) + ) +)] +#[cfg_attr(feature = "openapi", derive(utoipa::ToSchema))] +#[cfg_attr(feature = "validate", derive(validator::Validate))] +pub struct PolicyCreate { + /// The MIME media type of the serialized policy blob. + /// + /// Required, but only length-capped: python keystone's schema is + /// `{'type': 'string', 'maxLength': 255}`, so the empty string is valid. + #[cfg_attr(feature = "validate", validate(length(max = 255)))] + pub r#type: String, + + /// The policy rule set itself, as a serialized blob. Required, and a + /// string only — see the module docs. + pub blob: String, + + /// Ignored: a caller-supplied `id` on create is discarded and a fresh + /// UUID assigned, matching python keystone's `_assign_unique_id`. + /// + /// Declared explicitly (rather than left to `extra`'s `#[serde(flatten)]`) + /// so it is *consumed* here instead of being silently persisted as an + /// extra property. + /// + /// Typed as an arbitrary `Value`, not `String`: upstream never validates + /// it — `id` is not in `_policy_properties`, so it arrives as an + /// unconstrained `additionalProperties` entry and is then overwritten. A + /// `{"id": 123}` body is therefore a 201 upstream, and must not be a 400 + /// here. + #[cfg_attr(feature = "builder", builder(default))] + #[cfg_attr(feature = "openapi", schema(value_type = Option))] + #[serde(default, skip_serializing_if = "Option::is_none")] + pub id: Option, + + /// Extra attributes for the policy. + #[cfg_attr(feature = "builder", builder(default))] + #[cfg_attr(feature = "openapi", schema(inline, additional_properties))] + #[serde(flatten)] + pub extra: HashMap, +} + +impl PolicyCreate { + /// Non-sensitive policy-input projection. See [`policy_input`]. + /// + /// `id` is deliberately absent: it does not exist yet at create time. + #[must_use] + pub fn to_policy_input(&self) -> Value { + policy_input(None, Some(&self.r#type)) + } +} + +/// New policy creation request. +#[derive(Clone, Debug, Deserialize, PartialEq, Serialize)] +#[cfg_attr(feature = "openapi", derive(utoipa::ToSchema))] +#[cfg_attr(feature = "validate", derive(validator::Validate))] +pub struct PolicyCreateRequest { + /// Policy object. + #[cfg_attr(feature = "validate", validate(nested))] + pub policy: PolicyCreate, +} + +/// Update policy data. +#[derive(Clone, Debug, Default, Deserialize, PartialEq, Serialize)] +#[cfg_attr( + feature = "builder", + derive(derive_builder::Builder), + builder( + build_fn(error = "crate::error::BuilderError"), + setter(strip_option, into) + ) +)] +#[cfg_attr(feature = "openapi", derive(utoipa::ToSchema))] +#[cfg_attr(feature = "validate", derive(validator::Validate))] +pub struct PolicyUpdate { + /// New MIME media type of the serialized policy blob. + #[cfg_attr(feature = "builder", builder(default))] + #[serde(default, skip_serializing_if = "Option::is_none")] + #[cfg_attr(feature = "validate", validate(length(max = 255)))] + pub r#type: Option, + + /// New policy blob. String only — see the module docs. + #[cfg_attr(feature = "builder", builder(default))] + #[serde(default, skip_serializing_if = "Option::is_none")] + pub blob: Option, + + /// Only accepted when equal to the `{policy_id}` path segment; a + /// different value is a 400 ("Cannot change policy ID"), matching python + /// keystone's `Manager.update_policy`. Never persisted. + /// + /// Declared explicitly so it is consumed here rather than landing in + /// `extra`. + #[cfg_attr(feature = "builder", builder(default))] + #[serde(default, skip_serializing_if = "Option::is_none")] + pub id: Option, + + /// Extra attributes for the policy, merged into the stored properties. + #[cfg_attr(feature = "builder", builder(default))] + #[cfg_attr(feature = "openapi", schema(inline, additional_properties))] + #[serde(flatten)] + pub extra: HashMap, +} + +impl PolicyUpdate { + /// Non-sensitive policy-input projection. See [`policy_input`]. + #[must_use] + pub fn to_policy_input(&self) -> Value { + policy_input(self.id.as_deref(), self.r#type.as_deref()) + } + + /// Whether the request body carries no change at all. + /// + /// Python keystone's `policy_update` schema sets `minProperties: 1`, so an + /// empty `{"policy": {}}` document is a 400 rather than a no-op. + #[must_use] + pub fn is_empty(&self) -> bool { + self.r#type.is_none() && self.blob.is_none() && self.id.is_none() && self.extra.is_empty() + } +} + +/// Policy update request. +#[derive(Clone, Debug, Deserialize, PartialEq, Serialize)] +#[cfg_attr(feature = "openapi", derive(utoipa::ToSchema))] +#[cfg_attr(feature = "validate", derive(validator::Validate))] +pub struct PolicyUpdateRequest { + /// Policy object. + #[cfg_attr(feature = "validate", validate(nested))] + pub policy: PolicyUpdate, +} + +#[cfg(test)] +mod tests { + use super::*; + + /// The OPA projection is an allowlist: neither the blob nor any extra + /// property — including deliberately secret-shaped and *nested* ones — + /// can appear in it. + #[test] + fn test_policy_input_is_allowlisted() { + let policy = Policy { + id: "pid".into(), + r#type: "application/json".into(), + blob: serde_json::json!("s3cr3t-blob"), + extra: HashMap::from([ + ("password".into(), serde_json::json!("hunter2")), + ( + "nested".into(), + serde_json::json!({"totp_seed": "AAAA", "deep": {"secret": "x"}}), + ), + ]), + }; + + let input = policy.to_policy_input(); + assert_eq!( + input, + serde_json::json!({"id": "pid", "type": "application/json"}) + ); + let rendered = input.to_string(); + for needle in ["s3cr3t-blob", "hunter2", "AAAA", "password", "totp_seed"] { + assert!( + !rendered.contains(needle), + "{needle} leaked into {rendered}" + ); + } + } + + #[test] + fn test_create_policy_input_has_no_id() { + let create = PolicyCreate { + r#type: "text/plain".into(), + blob: "b".into(), + id: Some("ignored".into()), + extra: HashMap::from([("blob".into(), serde_json::json!("sneaky"))]), + }; + assert_eq!( + create.to_policy_input(), + serde_json::json!({"id": null, "type": "text/plain"}) + ); + } + + #[test] + fn test_update_is_empty() { + assert!(PolicyUpdate::default().is_empty()); + assert!( + !PolicyUpdate { + r#type: Some("x".into()), + ..Default::default() + } + .is_empty() + ); + assert!( + !PolicyUpdate { + extra: HashMap::from([("k".into(), serde_json::json!(1))]), + ..Default::default() + } + .is_empty() + ); + } + + /// A caller-supplied `id` must bind to the explicit field, not be + /// swallowed by `extra`'s flatten. + #[test] + fn test_create_id_is_not_captured_by_extra() { + let req: PolicyCreateRequest = serde_json::from_value(serde_json::json!({ + "policy": {"type": "t", "blob": "b", "id": "caller-supplied", "other": 1} + })) + .unwrap(); + + assert_eq!(req.policy.id, Some(serde_json::json!("caller-supplied"))); + assert!(!req.policy.extra.contains_key("id")); + assert_eq!(req.policy.extra.get("other"), Some(&serde_json::json!(1))); + } + + /// Upstream never validates `id` on create — it is an unconstrained + /// additional property that `_assign_unique_id` overwrites — so a + /// non-string `id` must deserialize rather than 400. + #[test] + fn test_create_accepts_non_string_id() { + for id in [ + serde_json::json!(123), + serde_json::json!({"nested": true}), + serde_json::json!(null), + ] { + let req: PolicyCreateRequest = serde_json::from_value(serde_json::json!({ + "policy": {"type": "t", "blob": "b", "id": id} + })) + .unwrap_or_else(|e| panic!("id {id} must be accepted on create: {e}")); + assert!(!req.policy.extra.contains_key("id")); + } + } + + /// `type` is required but may be empty: upstream's schema is + /// `{'type': 'string', 'maxLength': 255}`, with no `minLength`. + #[test] + fn test_empty_type_is_valid() { + let req: PolicyCreateRequest = serde_json::from_value(serde_json::json!({ + "policy": {"type": "", "blob": "b"} + })) + .unwrap(); + assert_eq!(req.policy.r#type, ""); + + #[cfg(feature = "validate")] + { + use validator::Validate; + assert!( + req.validate().is_ok(), + "an empty `type` must pass validation" + ); + } + } + + /// An object-valued `blob` fails deserialization, which the router turns + /// into a 400. + #[test] + fn test_object_blob_is_rejected() { + let err = serde_json::from_value::(serde_json::json!({ + "policy": {"type": "t", "blob": {"foobar_user": ["role:compute-user"]}} + })); + assert!(err.is_err(), "object-valued blob must not deserialize"); + } +} diff --git a/crates/api-types/src/v3/policy_conv.rs b/crates/api-types/src/v3/policy_conv.rs new file mode 100644 index 000000000..bc8992487 --- /dev/null +++ b/crates/api-types/src/v3/policy_conv.rs @@ -0,0 +1,144 @@ +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 + +use std::collections::HashMap; + +use serde_json::Value; + +use openstack_keystone_core_types::policy_store as provider_types; + +use crate::v3::policy as api_types; + +/// Normalize an additional-property name the way python keystone's +/// `ResourceBase._normalize_arg` does: `:` and `-` both become `_`. +/// +/// Applied on create only. Python keystone runs `_normalize_dict` over the +/// `POST` body but not over the `PATCH` body, so this asymmetry is faithful, +/// not an oversight. +fn normalize_extra_keys(extra: HashMap) -> HashMap { + extra + .into_iter() + .map(|(k, v)| (k.replace([':', '-'], "_"), v)) + .collect() +} + +impl From for api_types::Policy { + fn from(value: provider_types::Policy) -> Self { + Self { + id: value.id, + r#type: value.r#type, + blob: value.blob, + extra: value.extra, + } + } +} + +impl From for provider_types::PolicyListParameters { + fn from(value: api_types::PolicyListParameters) -> Self { + Self { + r#type: value.r#type, + pagination: Default::default(), + } + } +} + +impl From for provider_types::PolicyCreate { + fn from(value: api_types::PolicyCreateRequest) -> Self { + Self { + // `id` is intentionally dropped: python keystone's + // `_assign_unique_id` overwrites any caller-supplied value, and + // the service layer assigns the real one. + id: None, + r#type: value.policy.r#type, + blob: Value::String(value.policy.blob), + extra: normalize_extra_keys(value.policy.extra), + } + } +} + +impl From for provider_types::PolicyUpdate { + fn from(value: api_types::PolicyUpdateRequest) -> Self { + Self { + r#type: value.policy.r#type, + blob: value.policy.blob.map(Value::String), + // `id` is validated against the path by the handler and never + // persisted. + extra: value.policy.extra, + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_create_conv_drops_id_and_wraps_blob() { + let create: provider_types::PolicyCreate = api_types::PolicyCreateRequest { + policy: api_types::PolicyCreate { + r#type: "application/json".into(), + blob: "raw".into(), + id: Some(serde_json::json!("caller-supplied")), + extra: HashMap::new(), + }, + } + .into(); + + assert_eq!(create.id, None, "caller-supplied id must be discarded"); + assert_eq!(create.blob, Value::String("raw".into())); + assert_eq!(create.r#type, "application/json"); + } + + /// Python keystone normalizes `:`/`-` in additional-property names on + /// create. + #[test] + fn test_create_conv_normalizes_extra_keys() { + let create: provider_types::PolicyCreate = api_types::PolicyCreateRequest { + policy: api_types::PolicyCreate { + r#type: "t".into(), + blob: "b".into(), + id: None, + extra: HashMap::from([ + ("x-dash".into(), serde_json::json!(1)), + ("x:colon".into(), serde_json::json!(2)), + ("plain".into(), serde_json::json!(3)), + ]), + }, + } + .into(); + + assert_eq!(create.extra.get("x_dash"), Some(&serde_json::json!(1))); + assert_eq!(create.extra.get("x_colon"), Some(&serde_json::json!(2))); + assert_eq!(create.extra.get("plain"), Some(&serde_json::json!(3))); + assert!(!create.extra.contains_key("x-dash")); + } + + /// PATCH does not normalize, matching python keystone. + #[test] + fn test_update_conv_preserves_extra_keys_and_drops_id() { + let update: provider_types::PolicyUpdate = api_types::PolicyUpdateRequest { + policy: api_types::PolicyUpdate { + r#type: None, + blob: Some("nb".into()), + id: Some("pid".into()), + extra: HashMap::from([("x-dash".into(), serde_json::json!(1))]), + }, + } + .into(); + + assert_eq!(update.blob, Some(Value::String("nb".into()))); + assert!(update.extra.contains_key("x-dash")); + assert!(!update.extra.contains_key("id")); + } +} diff --git a/crates/config/src/lib.rs b/crates/config/src/lib.rs index 290e09c22..e534c5881 100644 --- a/crates/config/src/lib.rs +++ b/crates/config/src/lib.rs @@ -92,6 +92,7 @@ mod oauth2; mod oslo_middleware; mod pagination; mod policy; +mod policy_store; mod rate_limit; mod resource; mod revoke; @@ -134,6 +135,7 @@ pub use oauth2::*; pub use oslo_middleware::*; pub use pagination::*; pub use policy::*; +pub use policy_store::*; pub use rate_limit::*; pub use resource::*; pub use revoke::*; @@ -296,6 +298,13 @@ pub struct Config { #[serde(rename = "rate_limit_user_auth", default)] pub rate_limit_user_auth: RateLimitSection, + /// Policy store provider configuration (legacy `/v3/policies`). + /// + /// Bound to `[policy]`, matching python keystone. The OPA authorization + /// configuration is a different section, `[api_policy]` above. + #[serde(default)] + pub policy: PolicyStoreProvider, + /// Resource provider configuration. #[serde(default)] pub resource: ResourceProvider, diff --git a/crates/config/src/policy_store.rs b/crates/config/src/policy_store.rs new file mode 100644 index 000000000..ab7ad4332 --- /dev/null +++ b/crates/config/src/policy_store.rs @@ -0,0 +1,46 @@ +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 +//! # Keystone configuration +//! +//! Parsing of the Keystone configuration file implementation. +use serde::Deserialize; + +use crate::common::default_sql_driver; +use crate::pagination::ListLimitConfig; + +/// Policy store provider (legacy `/v3/policies` document storage). +/// +/// Bound to the `[policy]` section, matching python keystone's `[policy] +/// driver`. Not to be confused with `[api_policy]` +/// ([`crate::policy::PolicyProvider`]), which configures this service's +/// OPA-based authorization. +#[derive(Debug, Deserialize, Clone)] +pub struct PolicyStoreProvider { + /// Policy store provider driver. + #[serde(default = "default_sql_driver")] + pub driver: String, + + /// `GET /v3/policies` pagination limits. + #[serde(default)] + pub list_limit: ListLimitConfig, +} + +impl Default for PolicyStoreProvider { + fn default() -> Self { + Self { + driver: default_sql_driver(), + list_limit: ListLimitConfig::default(), + } + } +} diff --git a/crates/core-types/src/error.rs b/crates/core-types/src/error.rs index 9fe65dc9f..a85f274f5 100644 --- a/crates/core-types/src/error.rs +++ b/crates/core-types/src/error.rs @@ -31,6 +31,7 @@ use crate::mapping::MappingProviderError; use crate::oauth2_client::Oauth2ClientProviderError; use crate::oauth2_key::Oauth2KeyProviderError; use crate::oauth2_session::Oauth2SessionProviderError; +use crate::policy_store::PolicyStoreProviderError; use crate::resource::ResourceProviderError; use crate::revoke::RevokeProviderError; use crate::role::RoleProviderError; @@ -218,6 +219,14 @@ pub enum KeystoneError { #[error("invalid rate limit configuration: {0}")] RateLimitConfig(String), + /// Policy store provider (legacy `/v3/policies`). + #[error(transparent)] + PolicyStoreProvider { + /// The source of the error. + #[from] + source: PolicyStoreProviderError, + }, + /// Resource provider. #[error(transparent)] ResourceProvider { diff --git a/crates/core-types/src/events.rs b/crates/core-types/src/events.rs index b56dc0d40..76fef2820 100644 --- a/crates/core-types/src/events.rs +++ b/crates/core-types/src/events.rs @@ -178,6 +178,15 @@ pub enum EventPayload { Trust { id: String, }, + + // Policy blob store (legacy `/v3/policies`) + // + // Carries the ID only. The policy `blob` is arbitrary caller-supplied + // data and `blob` is on the audit denylist enforced by + // `tools/check_event_payload_no_secret_fields.py`. + Policy { + id: String, + }, } impl Event { diff --git a/crates/core-types/src/lib.rs b/crates/core-types/src/lib.rs index 8d48ae8c7..b1571f9a7 100644 --- a/crates/core-types/src/lib.rs +++ b/crates/core-types/src/lib.rs @@ -36,6 +36,7 @@ pub mod mapping; pub mod oauth2_client; pub mod oauth2_key; pub mod oauth2_session; +pub mod policy_store; pub mod resource; pub mod revoke; pub mod role; diff --git a/crates/core-types/src/policy_store.rs b/crates/core-types/src/policy_store.rs new file mode 100644 index 000000000..ec3c1515e --- /dev/null +++ b/crates/core-types/src/policy_store.rs @@ -0,0 +1,29 @@ +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 +//! # Policy store provider +//! +//! Storage for the legacy `/v3/policies` API: opaque, arbitrarily serialized +//! policy documents that *remote* services fetch and interpret themselves. +//! +//! This has nothing to do with authorization in this service. Keystone's own +//! access control is decided by OPA/Rego (`crate::policy` in the `core` +//! crate, `policy/**/*.rego` in the repository root); a `Policy` here is +//! inert data Keystone never reads. Python keystone's api-ref marks the API +//! deprecated ("Keystone is not a policy management service") and it is +//! implemented purely for API compatibility. +mod error; +mod policy; + +pub use error::*; +pub use policy::*; diff --git a/crates/core-types/src/policy_store/error.rs b/crates/core-types/src/policy_store/error.rs new file mode 100644 index 000000000..8fb27a2ea --- /dev/null +++ b/crates/core-types/src/policy_store/error.rs @@ -0,0 +1,58 @@ +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 + +use thiserror::Error; + +use crate::error::BuilderError; + +#[derive(Error, Debug)] +pub enum PolicyStoreProviderError { + /// Conflict. + #[error("conflict: {0}")] + Conflict(String), + + /// Driver error. + #[error("backend driver error: {0}")] + Driver(String), + + /// The policy has not been found. + #[error("policy {0} not found")] + PolicyNotFound(String), + + #[error("data serialization error")] + Serde { + #[from] + source: serde_json::Error, + }, + + /// Structures builder error. + #[error(transparent)] + StructBuilder { + /// The source of the error. + #[from] + source: BuilderError, + }, + + /// Unsupported driver. + #[error("unsupported driver `{0}` for the policy store provider.")] + UnsupportedDriver(String), + + /// Validation error. + #[error("request validation error: {}", source)] + Validation { + /// The source of the error. + #[from] + source: validator::ValidationErrors, + }, +} diff --git a/crates/core-types/src/policy_store/policy.rs b/crates/core-types/src/policy_store/policy.rs new file mode 100644 index 000000000..6152c98b3 --- /dev/null +++ b/crates/core-types/src/policy_store/policy.rs @@ -0,0 +1,129 @@ +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 + +use std::collections::HashMap; + +use derive_builder::Builder; +use serde_json::Value; +use validator::Validate; + +use crate::error::BuilderError; + +/// A stored policy document. +/// +/// # Note on `Serialize` +/// +/// This type deliberately does **not** derive `Serialize`. Handlers build the +/// OPA policy input from an explicit non-sensitive allowlist +/// (`policy_input()` in the `v3::policy` API module) rather than by +/// serializing this struct, and `blob`/`extra` hold arbitrary caller-supplied +/// JSON that must never reach the policy engine or its decision log. Not +/// having `Serialize` makes an accidental `json!({"policy": policy})` a +/// compile error instead of a silent leak. +#[derive(Builder, Clone, Debug, Default, PartialEq, Validate)] +#[builder(build_fn(error = "BuilderError"))] +#[builder(setter(strip_option, into))] +pub struct Policy { + /// Additional policy properties. + #[builder(default)] + pub extra: HashMap, + + /// The ID of the policy. + #[validate(length(min = 1, max = 64))] + pub id: String, + + /// The policy rule set itself, as an opaque serialized document. + /// + /// Modelled as an arbitrary JSON value rather than a `String` so rows + /// written by python keystone before its schema constrained the API input + /// to a string (object-valued blobs) still deserialize. The HTTP layer + /// only ever accepts a string. + #[builder(default)] + pub blob: Value, + + /// The MIME media type of the serialized policy blob. + /// + /// Only length-capped, deliberately not `min = 1`: python keystone's + /// schema is `{'type': 'string', 'maxLength': 255}`, so `type` is + /// *required* but may legitimately be the empty string. + #[validate(length(max = 255))] + pub r#type: String, +} + +/// Parameters for creating a new policy. +#[derive(Clone, Debug, Default, PartialEq, Validate)] +pub struct PolicyCreate { + /// Additional policy properties. + pub extra: HashMap, + + /// The ID of the policy. + /// + /// Always populated by the service layer before the backend is called, so + /// the audit event carries the final identifier. The API layer never sets + /// it: python keystone's `_assign_unique_id` discards any caller-supplied + /// `id` on create, and so do we. + #[validate(length(min = 1, max = 64))] + pub id: Option, + + /// The policy rule set itself. + pub blob: Value, + + /// The MIME media type of the serialized policy blob. + /// + /// Only length-capped, deliberately not `min = 1`: python keystone's + /// schema is `{'type': 'string', 'maxLength': 255}`, so `type` is + /// *required* but may legitimately be the empty string. + #[validate(length(max = 255))] + pub r#type: String, +} + +/// Parameters for listing policies. +#[derive(Builder, Clone, Debug, Default, PartialEq, Validate)] +#[builder(build_fn(error = "BuilderError"))] +#[builder(setter(strip_option, into))] +pub struct PolicyListParameters { + /// Pagination controls (limit/marker/page_reverse). + #[builder(default)] + pub pagination: crate::ListPagination, + + /// Filters the response by a MIME media type of the policy blob. + #[builder(default)] + #[validate(length(max = 255))] + pub r#type: Option, +} + +/// Fields that can be changed when updating a policy. +/// +/// Each field is `None` when the caller did not provide it (leave unchanged) +/// and `Some(..)` to set a new value. +#[derive(Clone, Debug, Default, PartialEq, Validate)] +pub struct PolicyUpdate { + /// Additional policy properties, *merged* into the stored `extra` (keys + /// present here win; keys only present in the stored object survive). + /// + /// This mirrors python keystone's `update_policy`, which patches the old + /// dict (`old_dict.update(policy)`) before writing it back. It is + /// deliberately different from e.g. `RegionUpdate.extra`, which this + /// codebase overwrites wholesale. + pub extra: HashMap, + + /// New policy blob. + pub blob: Option, + + /// New MIME media type of the serialized policy blob. + /// + /// See [`Policy::type`] on the absence of a minimum length. + #[validate(length(max = 255))] + pub r#type: Option, +} diff --git a/crates/core/src/api.rs b/crates/core/src/api.rs index d185b43c4..d292dc56b 100644 --- a/crates/core/src/api.rs +++ b/crates/core/src/api.rs @@ -300,6 +300,34 @@ pub mod tests { } } + /// Like [`get_mocked_state`], but wired to a caller-supplied + /// [`crate::policy::PolicyEnforcer`]. + /// + /// [`get_mocked_state`] decides uniformly (allow-all or deny-all) and + /// [`get_capturing_state`] always allows, so neither can express + /// "allow the collection check, deny *this* item" — which is exactly + /// what a list handler's per-item re-check (security model I8) needs to + /// be tested for, along with propagation of non-`Forbidden` policy + /// errors. + pub async fn get_state_with_policy( + provider_builder: ProviderBuilder, + policy: Arc, + ) -> ServiceState { + let provider = provider_builder.build().unwrap(); + Arc::new( + Service::new( + ConfigManager::not_watched(Config::default()), + DatabaseConnection::default(), + provider, + policy, + AuditDispatcher::noop(), + None, + ) + .await + .unwrap(), + ) + } + /// Like [`get_mocked_state`], but wired to a [`CapturingPolicy`] instead /// of an allow/deny-only mock, so the test can inspect exactly what /// `(policy_name, target, existing)` the handler under test sent. diff --git a/crates/core/src/cadf_hook.rs b/crates/core/src/cadf_hook.rs index cbe653b11..b5babadd0 100644 --- a/crates/core/src/cadf_hook.rs +++ b/crates/core/src/cadf_hook.rs @@ -97,6 +97,7 @@ fn build_target_from_event(event: &Event) -> Target { EventPayload::Region { id } => (id, "data/compute/catalog/region"), EventPayload::Service { id } => (id, "data/compute/catalog/service"), EventPayload::Trust { id } => (id, "data/security/identity/trust"), + EventPayload::Policy { id } => (id, "data/security/identity/policy"), EventPayload::ServiceAccount { id } => (id, "data/security/identity/service-account"), EventPayload::TokenRestriction { id } => (id, "data/security/identity/token-restriction"), EventPayload::IdentityProvider { id } => (id, "data/security/identity/identity-provider"), diff --git a/crates/core/src/lib.rs b/crates/core/src/lib.rs index 93bb070ec..8a167dab5 100644 --- a/crates/core/src/lib.rs +++ b/crates/core/src/lib.rs @@ -106,6 +106,7 @@ pub mod oauth2_key; pub mod oauth2_session; pub mod plugin_manager; pub mod policy; +pub mod policy_store; pub mod provider; pub mod rate_limit; pub mod request_cache; diff --git a/crates/core/src/mocks.rs b/crates/core/src/mocks.rs index d405f037f..d7e985c7b 100644 --- a/crates/core/src/mocks.rs +++ b/crates/core/src/mocks.rs @@ -1709,6 +1709,53 @@ mod scim_resource { } pub use scim_resource::MockScimResourceProvider; +mod policy_store { + use super::*; + + use openstack_keystone_core_types::policy_store::*; + + use crate::policy_store::{PolicyStoreApi, PolicyStoreProviderError}; + + mock! { + pub PolicyStoreProvider {} + + #[async_trait] + impl PolicyStoreApi for PolicyStoreProvider { + async fn create_policy<'a>( + &self, + ctx: &ExecutionContext<'a>, + policy: PolicyCreate, + ) -> Result; + + async fn delete_policy<'a>( + &self, + ctx: &ExecutionContext<'a>, + id: &'a str, + ) -> Result<(), PolicyStoreProviderError>; + + async fn get_policy<'a>( + &self, + ctx: &ExecutionContext<'a>, + id: &'a str, + ) -> Result, PolicyStoreProviderError>; + + async fn list_policies<'a>( + &self, + ctx: &ExecutionContext<'a>, + params: &PolicyListParameters, + ) -> Result, PolicyStoreProviderError>; + + async fn update_policy<'a>( + &self, + ctx: &ExecutionContext<'a>, + id: &'a str, + policy: PolicyUpdate, + ) -> Result; + } + } +} +pub use policy_store::MockPolicyStoreProvider; + mod trust { use super::*; diff --git a/crates/core/src/plugin_manager.rs b/crates/core/src/plugin_manager.rs index cf2edf8ac..39c3cf30d 100644 --- a/crates/core/src/plugin_manager.rs +++ b/crates/core/src/plugin_manager.rs @@ -56,6 +56,8 @@ use crate::oauth2_key::Oauth2KeyProviderError; use crate::oauth2_key::backend::Oauth2KeyBackend; use crate::oauth2_session::Oauth2SessionProviderError; use crate::oauth2_session::backend::Oauth2SessionBackend; +use crate::policy_store::PolicyStoreProviderError; +use crate::policy_store::backend::PolicyStoreBackend; use crate::resource::backend::ResourceBackend; use crate::resource::error::ResourceProviderError; use crate::revoke::RevokeProviderError; @@ -121,6 +123,7 @@ declare_backend_registry!( dyn Oauth2KeyBackend, dyn Oauth2SessionBackend, dyn K8sAuthBackend, + dyn PolicyStoreBackend, dyn ResourceBackend, dyn RevokeBackend, dyn RoleBackend, @@ -445,6 +448,19 @@ pub trait PluginManagerApi { name: S, ) -> Result<&Arc, TrustProviderError>; + /// Get registered policy store backend (legacy `/v3/policies`). + /// + /// # Parameters + /// - `name`: The name of the backend to retrieve. + /// + /// # Returns + /// - `Ok(&Arc)` if found, otherwise + /// `Err(PolicyStoreProviderError)`. + fn get_policy_store_backend>( + &self, + name: S, + ) -> Result<&Arc, PolicyStoreProviderError>; + /// Register API Key backend. /// /// # Parameters @@ -658,4 +674,15 @@ pub trait PluginManagerApi { /// - `name`: The name to register the backend under. /// - `plugin`: The backend implementation. fn register_trust_backend>(&mut self, name: S, plugin: Arc); + + /// Register policy store backend (legacy `/v3/policies`). + /// + /// # Parameters + /// - `name`: The name to register the backend under. + /// - `plugin`: The backend implementation. + fn register_policy_store_backend>( + &mut self, + name: S, + plugin: Arc, + ); } diff --git a/crates/core/src/policy_store/backend.rs b/crates/core/src/policy_store/backend.rs new file mode 100644 index 000000000..5dce31df8 --- /dev/null +++ b/crates/core/src/policy_store/backend.rs @@ -0,0 +1,101 @@ +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 + +use async_trait::async_trait; + +use openstack_keystone_core_types::policy_store::*; + +use crate::keystone::ServiceState; +use crate::policy_store::error::PolicyStoreProviderError; + +#[cfg_attr(test, mockall::automock)] +#[async_trait] +pub trait PolicyStoreBackend: Send + Sync { + /// Create a new policy. + /// + /// # Parameters + /// - `state`: The current service state. + /// - `policy`: The policy creation parameters. `policy.id` is always + /// populated by the service layer. + /// + /// # Returns + /// A `Result` containing the created `Policy`, or a + /// `PolicyStoreProviderError`. + async fn create_policy( + &self, + state: &ServiceState, + policy: PolicyCreate, + ) -> Result; + + /// Delete a policy by ID. + /// + /// # Parameters + /// - `state`: The current service state. + /// - `id`: The unique identifier of the policy. + /// + /// # Returns + /// A `Result` indicating success or a `PolicyStoreProviderError`. + async fn delete_policy( + &self, + state: &ServiceState, + id: &str, + ) -> Result<(), PolicyStoreProviderError>; + + /// Get a single policy by ID. + /// + /// # Parameters + /// - `state`: The current service state. + /// - `id`: The unique identifier of the policy. + /// + /// # Returns + /// A `Result` containing an `Option` with the `Policy` if found, or a + /// `PolicyStoreProviderError`. + async fn get_policy( + &self, + state: &ServiceState, + id: &str, + ) -> Result, PolicyStoreProviderError>; + + /// List policies. + /// + /// # Parameters + /// - `state`: The current service state. + /// - `params`: Parameters for filtering the policy list. + /// + /// # Returns + /// A `Result` containing a vector of `Policy` objects or a + /// `PolicyStoreProviderError`. + async fn list_policies( + &self, + state: &ServiceState, + params: &PolicyListParameters, + ) -> Result, PolicyStoreProviderError>; + + /// Update an existing policy. + /// + /// # Parameters + /// - `state`: The current service state. + /// - `id`: The unique identifier of the policy. + /// - `policy`: The fields to change. + /// + /// # Returns + /// A `Result` containing the updated `Policy`, or a + /// `PolicyStoreProviderError`. + async fn update_policy( + &self, + state: &ServiceState, + id: &str, + policy: PolicyUpdate, + ) -> Result; +} diff --git a/crates/core/src/policy_store/error.rs b/crates/core/src/policy_store/error.rs new file mode 100644 index 000000000..88be289b2 --- /dev/null +++ b/crates/core/src/policy_store/error.rs @@ -0,0 +1,30 @@ +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 +pub use openstack_keystone_core_types::policy_store::PolicyStoreProviderError; + +impl From for PolicyStoreProviderError { + /// Converts a `DatabaseError` into a `PolicyStoreProviderError`. + /// + /// # Parameters + /// - `source`: The `DatabaseError` to convert. + /// + /// # Returns + /// A `PolicyStoreProviderError` equivalent to the source database error. + fn from(source: crate::error::DatabaseError) -> Self { + match source { + cfl @ crate::error::DatabaseError::Conflict { .. } => Self::Conflict(cfl.to_string()), + other => Self::Driver(other.to_string()), + } + } +} diff --git a/crates/core/src/policy_store/mod.rs b/crates/core/src/policy_store/mod.rs new file mode 100644 index 000000000..bb2de5b2a --- /dev/null +++ b/crates/core/src/policy_store/mod.rs @@ -0,0 +1,44 @@ +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 +//! # Policy store provider +//! +//! Backs the legacy `/v3/policies` API: CRUD over opaque, arbitrarily +//! serialized policy documents that *remote* services fetch and interpret +//! themselves. +//! +//! ## This is not authorization +//! +//! Keystone's own access control is decided by OPA/Rego — see +//! [`crate::policy`] for the enforcement client and `policy/**/*.rego` for +//! the rules. A stored [`Policy`] is inert data this service never reads or +//! evaluates. The two are unrelated despite the shared word, which is why +//! this domain is `policy_store` and not `policy`. +//! +//! Python keystone's api-ref marks the API deprecated ("Keystone is not a +//! policy management service"); it exists here only for API compatibility +//! with the python implementation (issue #1035). +//! +//! [`Policy`]: openstack_keystone_core_types::policy_store::Policy + +pub mod backend; +pub mod error; +mod provider_api; +pub mod service; + +pub use error::PolicyStoreProviderError; +pub use provider_api::PolicyStoreApi; +pub use service::PolicyStoreService; + +#[cfg(any(test, feature = "mock"))] +pub use crate::mocks::MockPolicyStoreProvider; diff --git a/crates/core/src/policy_store/provider_api.rs b/crates/core/src/policy_store/provider_api.rs new file mode 100644 index 000000000..564121979 --- /dev/null +++ b/crates/core/src/policy_store/provider_api.rs @@ -0,0 +1,104 @@ +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 + +use async_trait::async_trait; + +use openstack_keystone_core_types::policy_store::*; + +use crate::auth::ExecutionContext; +use crate::policy_store::PolicyStoreProviderError; + +#[async_trait] +pub trait PolicyStoreApi: Send + Sync { + /// Create a new policy. + /// + /// This layer is the single source of the identifier: it **always** mints + /// a fresh one, discarding any `policy.id` the caller set, so the audit + /// event raised for the operation carries the final ID. The backend + /// rejects a create whose `id` is unset. + /// + /// # Parameters + /// - `exec`: The current execution context. + /// - `policy`: The policy creation parameters. + /// + /// # Returns + /// A `Result` containing the created `Policy`, or a + /// `PolicyStoreProviderError`. + async fn create_policy<'a>( + &self, + exec: &ExecutionContext<'a>, + policy: PolicyCreate, + ) -> Result; + + /// Delete a policy by ID. + /// + /// # Parameters + /// - `exec`: The current execution context. + /// - `id`: The unique identifier of the policy. + /// + /// # Returns + /// A `Result` indicating success or a `PolicyStoreProviderError`. + async fn delete_policy<'a>( + &self, + exec: &ExecutionContext<'a>, + id: &'a str, + ) -> Result<(), PolicyStoreProviderError>; + + /// Get a single policy by ID. + /// + /// # Parameters + /// - `exec`: The current execution context. + /// - `id`: The unique identifier of the policy. + /// + /// # Returns + /// A `Result` containing an `Option` with the `Policy` if found, or a + /// `PolicyStoreProviderError`. + async fn get_policy<'a>( + &self, + exec: &ExecutionContext<'a>, + id: &'a str, + ) -> Result, PolicyStoreProviderError>; + + /// List policies. + /// + /// # Parameters + /// - `exec`: The current execution context. + /// - `params`: Parameters for filtering the policy list. + /// + /// # Returns + /// A `Result` containing a vector of `Policy` objects or a + /// `PolicyStoreProviderError`. + async fn list_policies<'a>( + &self, + exec: &ExecutionContext<'a>, + params: &PolicyListParameters, + ) -> Result, PolicyStoreProviderError>; + + /// Update an existing policy. + /// + /// # Parameters + /// - `exec`: The current execution context. + /// - `id`: The unique identifier of the policy. + /// - `policy`: The fields to change. + /// + /// # Returns + /// A `Result` containing the updated `Policy`, or a + /// `PolicyStoreProviderError`. + async fn update_policy<'a>( + &self, + exec: &ExecutionContext<'a>, + id: &'a str, + policy: PolicyUpdate, + ) -> Result; +} diff --git a/crates/core/src/policy_store/service.rs b/crates/core/src/policy_store/service.rs new file mode 100644 index 000000000..981a33755 --- /dev/null +++ b/crates/core/src/policy_store/service.rs @@ -0,0 +1,242 @@ +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 +//! # Policy store provider +use async_trait::async_trait; +use std::sync::Arc; +use uuid::Uuid; +use validator::Validate; + +use openstack_keystone_config::Config; +use openstack_keystone_core_types::events::{Event, EventPayload, Operation}; +use openstack_keystone_core_types::policy_store::*; + +use crate::auth::ExecutionContext; +use crate::events::AuditDispatchError; +use crate::plugin_manager::PluginManagerApi; +use crate::policy_store::{PolicyStoreApi, PolicyStoreProviderError, backend::PolicyStoreBackend}; + +pub struct PolicyStoreService { + backend_driver: Arc, +} + +impl PolicyStoreService { + /// Creates a new `PolicyStoreService`. + /// + /// # Parameters + /// - `config`: The configuration for the policy store provider. + /// - `plugin_manager`: The plugin manager used to load the backend. + /// + /// # Returns + /// A `Result` containing the `PolicyStoreService` instance or a + /// `PolicyStoreProviderError`. + pub fn new( + config: &Config, + plugin_manager: &P, + ) -> Result { + let backend_driver = plugin_manager + .get_policy_store_backend(config.policy.driver.clone())? + .clone(); + Ok(Self { backend_driver }) + } +} + +#[async_trait] +impl PolicyStoreApi for PolicyStoreService { + /// Create a new policy. + /// + /// # Parameters + /// - `exec`: The current execution context. + /// - `policy`: The policy creation parameters. + /// + /// # Returns + /// A `Result` containing the created `Policy`, or a + /// `PolicyStoreProviderError`. + async fn create_policy<'a>( + &self, + exec: &ExecutionContext<'a>, + mut policy: PolicyCreate, + ) -> Result { + // This layer is the single source of the identifier, and it always + // mints a fresh one — matching python keystone's `_assign_unique_id`, + // which overwrites whatever the caller sent. Assigning it *here*, + // before the audited operation is constructed, means the event raised + // around the write already carries the final resource ID; doing it in + // the backend would leave the pre-operation audit record without one. + let id = Uuid::new_v4().simple().to_string(); + policy.id = Some(id.clone()); + policy.validate()?; + + let policy = if let Some(vsc) = exec.ctx() { + let backend_driver = &self.backend_driver; + crate::audited_op! { + dispatcher: &exec.state().event_dispatcher, + ctx: vsc, + event: Event::new( + Operation::Create, + EventPayload::Policy { id: id.clone() }, + ), + operation: async { + backend_driver.create_policy(exec.state(), policy).await + }, + on_audit_error: |_: AuditDispatchError| PolicyStoreProviderError::Driver("audit dispatch failed".into()), + }? + } else { + let policy = self + .backend_driver + .create_policy(exec.state(), policy) + .await?; + + exec.state() + .event_dispatcher + .emit(Event::new( + Operation::Create, + EventPayload::Policy { + id: policy.id.clone(), + }, + )) + .await; + + policy + }; + + Ok(policy) + } + + /// Delete a policy by ID. + /// + /// # Parameters + /// - `exec`: The current execution context. + /// - `id`: The unique identifier of the policy. + /// + /// # Returns + /// A `Result` indicating success or a `PolicyStoreProviderError`. + async fn delete_policy<'a>( + &self, + exec: &ExecutionContext<'a>, + id: &'a str, + ) -> Result<(), PolicyStoreProviderError> { + if let Some(vsc) = exec.ctx() { + crate::audited_op! { + dispatcher: &exec.state().event_dispatcher, + ctx: vsc, + event: Event::new( + Operation::Delete, + EventPayload::Policy { id: id.to_string() }, + ), + operation: async { + self.backend_driver.delete_policy(exec.state(), id).await?; + Ok::<(), PolicyStoreProviderError>(()) + }, + on_audit_error: |_: AuditDispatchError| PolicyStoreProviderError::Driver("audit dispatch failed".into()), + }?; + } else { + self.backend_driver.delete_policy(exec.state(), id).await?; + + exec.state() + .event_dispatcher + .emit(Event::new( + Operation::Delete, + EventPayload::Policy { id: id.to_string() }, + )) + .await; + } + + Ok(()) + } + + /// Get a single policy by ID. + /// + /// # Parameters + /// - `exec`: The current execution context. + /// - `id`: The unique identifier of the policy. + /// + /// # Returns + /// A `Result` containing an `Option` with the policy if found, or a + /// `PolicyStoreProviderError`. + async fn get_policy<'a>( + &self, + exec: &ExecutionContext<'a>, + id: &'a str, + ) -> Result, PolicyStoreProviderError> { + self.backend_driver.get_policy(exec.state(), id).await + } + + /// List policies. + /// + /// # Parameters + /// - `exec`: The current execution context. + /// - `params`: Parameters for filtering the policy list. + /// + /// # Returns + /// A `Result` containing a vector of `Policy` objects or a + /// `PolicyStoreProviderError`. + async fn list_policies<'a>( + &self, + exec: &ExecutionContext<'a>, + params: &PolicyListParameters, + ) -> Result, PolicyStoreProviderError> { + params.validate()?; + self.backend_driver + .list_policies(exec.state(), params) + .await + } + + /// Update an existing policy. + /// + /// # Parameters + /// - `exec`: The current execution context. + /// - `id`: The unique identifier of the policy. + /// - `policy`: The fields to change. + /// + /// # Returns + /// A `Result` containing the updated `Policy`, or a + /// `PolicyStoreProviderError`. + async fn update_policy<'a>( + &self, + exec: &ExecutionContext<'a>, + id: &'a str, + policy: PolicyUpdate, + ) -> Result { + policy.validate()?; + let updated = if let Some(vsc) = exec.ctx() { + let backend_driver = &self.backend_driver; + crate::audited_op! { + dispatcher: &exec.state().event_dispatcher, + ctx: vsc, + event: Event::new( + Operation::Update, + EventPayload::Policy { id: id.to_string() }, + ), + operation: async { + backend_driver.update_policy(exec.state(), id, policy).await + }, + on_audit_error: |_: AuditDispatchError| PolicyStoreProviderError::Driver("audit dispatch failed".into()), + }? + } else { + let updated = self + .backend_driver + .update_policy(exec.state(), id, policy) + .await?; + exec.state() + .event_dispatcher + .emit(Event::new( + Operation::Update, + EventPayload::Policy { id: id.to_string() }, + )) + .await; + updated + }; + Ok(updated) + } +} diff --git a/crates/core/src/provider.rs b/crates/core/src/provider.rs index eec9a8101..371e3e2d7 100644 --- a/crates/core/src/provider.rs +++ b/crates/core/src/provider.rs @@ -68,6 +68,9 @@ use crate::oauth2_session::MockOauth2SessionProvider; use crate::oauth2_session::Oauth2SessionApi; use crate::plugin_manager::PluginManagerApi; #[cfg(any(test, feature = "mock"))] +use crate::policy_store::MockPolicyStoreProvider; +use crate::policy_store::PolicyStoreApi; +#[cfg(any(test, feature = "mock"))] use crate::resource::MockResourceProvider; use crate::resource::ResourceApi; #[cfg(any(test, feature = "mock"))] @@ -121,6 +124,8 @@ pub struct Provider { oauth2_session: Box, /// K8s auth provider. k8s_auth: Box, + /// Policy store provider (legacy `/v3/policies`). + policy_store: Box, /// Resource provider. resource: Box, /// Revoke provider. @@ -256,6 +261,12 @@ impl ProviderBuilder { new } + pub fn mock_policy_store(self, value: impl PolicyStoreApi + 'static) -> Self { + let mut new = self; + new.policy_store = Some(Box::new(value)); + new + } + pub fn mock_token(self, value: impl TokenApi + 'static) -> Self { let mut new = self; new.token = Some(Box::new(value)); @@ -318,6 +329,10 @@ impl Provider { crate::k8s_auth::K8sAuthService::new(cfg, plugin_manager, k8s_http_client) .map_err(|e| KeystoneError::K8sAuthProvider { source: e })?, ); + let policy_store = Box::new(crate::policy_store::PolicyStoreService::new( + cfg, + plugin_manager, + )?); let resource = Box::new(crate::resource::ResourceService::new(cfg, plugin_manager)?); let revoke = Box::new(crate::revoke::RevokeService::new(cfg, plugin_manager)?); let role = Box::new(crate::role::RoleService::new(cfg, plugin_manager)?); @@ -347,6 +362,7 @@ impl Provider { oauth2_key, oauth2_session, k8s_auth, + policy_store, resource, revoke, role, @@ -375,6 +391,7 @@ impl Provider { .mock_oauth2_session(MockOauth2SessionProvider::default()) .mock_federation(MockFederationProvider::default()) .mock_k8s_auth(MockK8sAuthProvider::default()) + .mock_policy_store(MockPolicyStoreProvider::default()) .mock_resource(MockResourceProvider::default()) .mock_revoke(MockRevokeProvider::default()) .mock_role(MockRoleProvider::default()) @@ -484,6 +501,13 @@ impl Provider { &*self.token } + /// Get the policy store provider (legacy `/v3/policies`). + /// + /// Unrelated to [`crate::policy`], which is the OPA authorization client. + pub fn get_policy_store_provider(&self) -> &dyn PolicyStoreApi { + &*self.policy_store + } + /// Get the trust provider. pub fn get_trust_provider(&self) -> &dyn TrustApi { &*self.trust diff --git a/crates/keystone/Cargo.toml b/crates/keystone/Cargo.toml index 8298a2639..dff10f68a 100644 --- a/crates/keystone/Cargo.toml +++ b/crates/keystone/Cargo.toml @@ -62,6 +62,7 @@ openstack-keystone-identity-driver-ldap.workspace = true openstack-keystone-identity-driver-sql.workspace = true openstack-keystone-idmapping-driver-sql.workspace = true openstack-keystone-password-hashing.workspace = true +openstack-keystone-policy-store-driver-sql.workspace = true openstack-keystone-resource-driver-sql.workspace = true openstack-keystone-revoke-driver-sql.workspace = true openstack-keystone-role-driver-sql.workspace = true diff --git a/crates/keystone/src/api/mod.rs b/crates/keystone/src/api/mod.rs index 670cee528..aa029ed21 100644 --- a/crates/keystone/src/api/mod.rs +++ b/crates/keystone/src/api/mod.rs @@ -122,7 +122,8 @@ async fn version( pub(crate) mod tests { pub use openstack_keystone_core::api::policy_contract; pub use openstack_keystone_core::api::tests::{ - get_capturing_state, get_mocked_state, get_mocked_state_with_config, test_fixture_scoped, + get_capturing_state, get_mocked_state, get_mocked_state_with_config, get_state_with_policy, + test_fixture_scoped, }; use std::net::SocketAddr; @@ -389,6 +390,45 @@ pub(crate) mod tests { .build(); ValidatedSecurityContext::test_new(sc) } + + /// A plain password-authenticated caller in the **system** scope with + /// `roles` — the `RULE_ADMIN_OR_SYSTEM_READER` shape python keystone + /// uses for read verbs, which the project-scoped [`member_vsc`] + /// cannot express. + /// + /// `scope` is the raw system-scope value; the literal `"system"` is + /// aliased to `"all"` by `Credentials`, which is what + /// `input.credentials.system` compares against in Rego. + pub fn system_scoped_vsc( + user_id: &str, + scope: &str, + roles: &[&str], + ) -> ValidatedSecurityContext { + let authz = AuthzInfoBuilder::default() + .scope(ScopeInfo::System(scope.to_string())) + .roles( + roles + .iter() + .enumerate() + .map(|(i, name)| RoleRef { + domain_id: None, + id: format!("role-{i}"), + name: Some((*name).to_string()), + }) + .collect::>(), + ) + .build() + .unwrap(); + + let sc = SecurityContext::test_build() + .authentication_context(AuthenticationContext::Password) + .principal(PrincipalInfo { + identity: user_identity(user_id), + }) + .authorization(authz) + .build(); + ValidatedSecurityContext::test_new(sc) + } } /// A request to a route with a trailing slash must be served by the same diff --git a/crates/keystone/src/api/v3/mod.rs b/crates/keystone/src/api/v3/mod.rs index 543bad433..1ad839102 100644 --- a/crates/keystone/src/api/v3/mod.rs +++ b/crates/keystone/src/api/v3/mod.rs @@ -34,6 +34,7 @@ pub mod ec2tokens; pub mod endpoint; pub mod group; pub mod os_trust; +pub mod policy; pub mod project; pub mod region; pub mod role; @@ -62,6 +63,7 @@ pub(super) fn openapi_router() -> OpenApiRouter { .nest("/endpoints", endpoint::openapi_router()) .nest("/groups", group::openapi_router()) .nest("/OS-TRUST", os_trust::openapi_router()) + .nest("/policies", policy::openapi_router()) .nest("/projects", project::openapi_router()) .nest("/regions", region::openapi_router()) .nest("/roles", role::openapi_router()) diff --git a/crates/keystone/src/api/v3/policy/create.rs b/crates/keystone/src/api/v3/policy/create.rs new file mode 100644 index 000000000..6a77137ab --- /dev/null +++ b/crates/keystone/src/api/v3/policy/create.rs @@ -0,0 +1,399 @@ +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 +//! # Create policy API + +use axum::{ + extract::{Json, State}, + http::StatusCode, + response::IntoResponse, +}; +use validator::Validate; + +use super::types::{Policy, PolicyCreateRequest, PolicyResponse}; +use crate::api::auth::Auth; +use crate::api::error::KeystoneApiError; +use crate::keystone::ServiceState; +use openstack_keystone_core::auth::ExecutionContext; + +/// Create a new policy. +/// +/// Any caller-supplied `id` in the body is discarded and a fresh UUID +/// assigned, matching python keystone's `_assign_unique_id`. +#[utoipa::path( + post, + path = "/", + responses( + (status = CREATED, description = "Policy created", body = PolicyResponse), + (status = 400, description = "Invalid input"), + (status = 500, description = "Internal error") + ), + tag="policies" +)] +// `skip_all`, not `skip(state)`: the request body carries the policy +// document (`blob`) and an open-ended `extra` map of caller-supplied JSON, +// both of which `tracing::instrument` would otherwise render into the span +// via `Debug`. Only the non-sensitive media type is recorded. +#[tracing::instrument( + name = "api::v3::policy_create", + level = "debug", + skip_all, + fields(policy_type = %payload.policy.r#type) +)] +pub(super) async fn create( + Auth(user_auth): Auth, + State(state): State, + Json(payload): Json, +) -> Result { + payload.validate()?; + + state + .policy_enforcer + .enforce( + "identity/policy/create", + &user_auth, + super::policy_input(payload.policy.to_policy_input()), + None, + ) + .await?; + + let created = state + .provider + .get_policy_store_provider() + .create_policy( + &ExecutionContext::from_auth(&state, &user_auth), + payload.into(), + ) + .await?; + + Ok(( + StatusCode::CREATED, + Json(PolicyResponse { + policy: Policy::from(created), + }), + ) + .into_response()) +} + +#[cfg(test)] +mod tests { + use axum::{ + body::Body, + http::{Request, StatusCode}, + }; + use http_body_util::BodyExt; + use tower::ServiceExt; + use tower_http::trace::TraceLayer; + + use openstack_keystone_core_types::policy_store::{PolicyBuilder, PolicyCreate}; + + use super::super::openapi_router; + use crate::api::tests::{ + get_capturing_state, get_mocked_state, policy_contract, test_fixture_scoped, + }; + use crate::api::v3::policy::types::PolicyResponse; + use crate::policy_store::MockPolicyStoreProvider; + use crate::provider::Provider; + + fn stored( + id: &str, + r#type: &str, + blob: &str, + ) -> openstack_keystone_core_types::policy_store::Policy { + PolicyBuilder::default() + .id(id) + .r#type(r#type) + .blob(serde_json::Value::String(blob.into())) + .build() + .unwrap() + } + + #[tokio::test] + async fn test_create() { + let mut mock = MockPolicyStoreProvider::default(); + mock.expect_create_policy() + .withf(|_, req: &PolicyCreate| { + req.r#type == "application/json" + && req.blob == serde_json::Value::String("blob text".into()) + }) + .returning(|_, _| Ok(stored("pid", "application/json", "blob text"))); + + let vsc = test_fixture_scoped(); + let state = get_mocked_state( + Provider::mocked_builder().mock_policy_store(mock), + true, + None, + ) + .await; + + let mut api = openapi_router() + .layer(TraceLayer::new_for_http()) + .with_state(state); + + let response = api + .as_service() + .oneshot( + Request::builder() + .method("POST") + .uri("/") + .header("content-type", "application/json") + .extension(vsc) + .body(Body::from( + r#"{"policy": {"type": "application/json", "blob": "blob text"}}"#, + )) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::CREATED); + let body = response.into_body().collect().await.unwrap().to_bytes(); + let res: PolicyResponse = serde_json::from_slice(&body).unwrap(); + assert_eq!(res.policy.id, "pid"); + assert_eq!(res.policy.blob, serde_json::json!("blob text")); + } + + /// A caller-supplied `id` must be replaced by a server-generated one and + /// must not be smuggled into `extra` (python keystone's + /// `_assign_unique_id` behaviour). + #[tokio::test] + async fn test_create_discards_caller_supplied_id() { + let mut mock = MockPolicyStoreProvider::default(); + mock.expect_create_policy() + .withf(|_, req: &PolicyCreate| req.id.is_none() && !req.extra.contains_key("id")) + .returning(|_, _| Ok(stored("server-generated", "t", "b"))); + + let vsc = test_fixture_scoped(); + let state = get_mocked_state( + Provider::mocked_builder().mock_policy_store(mock), + true, + None, + ) + .await; + + let mut api = openapi_router() + .layer(TraceLayer::new_for_http()) + .with_state(state); + + let response = api + .as_service() + .oneshot( + Request::builder() + .method("POST") + .uri("/") + .header("content-type", "application/json") + .extension(vsc) + .body(Body::from( + r#"{"policy": {"type": "t", "blob": "b", "id": "caller-supplied"}}"#, + )) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::CREATED); + let body = response.into_body().collect().await.unwrap().to_bytes(); + let res: PolicyResponse = serde_json::from_slice(&body).unwrap(); + assert_eq!(res.policy.id, "server-generated"); + } + + /// Unknown properties round-trip through `extra`. + #[tokio::test] + async fn test_create_unknown_properties_round_trip() { + let mut mock = MockPolicyStoreProvider::default(); + mock.expect_create_policy() + .withf(|_, req: &PolicyCreate| { + req.extra.get("custom") == Some(&serde_json::json!("value")) + }) + .returning(|_, _| { + let mut policy = stored("pid", "t", "b"); + policy + .extra + .insert("custom".into(), serde_json::json!("value")); + Ok(policy) + }); + + let vsc = test_fixture_scoped(); + let state = get_mocked_state( + Provider::mocked_builder().mock_policy_store(mock), + true, + None, + ) + .await; + + let mut api = openapi_router() + .layer(TraceLayer::new_for_http()) + .with_state(state); + + let response = api + .as_service() + .oneshot( + Request::builder() + .method("POST") + .uri("/") + .header("content-type", "application/json") + .extension(vsc) + .body(Body::from( + r#"{"policy": {"type": "t", "blob": "b", "custom": "value"}}"#, + )) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::CREATED); + let body = response.into_body().collect().await.unwrap().to_bytes(); + let res: PolicyResponse = serde_json::from_slice(&body).unwrap(); + assert_eq!( + res.policy.extra.get("custom"), + Some(&serde_json::json!("value")) + ); + } + + /// The HTTP contract accepts a string `blob` only, mirroring python + /// keystone's `{'blob': {'type': 'string'}}` jsonschema. + /// + /// Mounts the production `normalize_json_rejection` layer (applied on the + /// real router in `crate::api::openapi_router`), so this asserts the 400 + /// the service actually returns rather than axum's raw 422. + #[tokio::test] + async fn test_create_rejects_object_blob() { + let vsc = test_fixture_scoped(); + let state = get_mocked_state(Provider::mocked_builder(), true, None).await; + + let mut api = openapi_router() + .layer(axum::middleware::map_response( + crate::api::json_rejection::normalize_json_rejection, + )) + .layer(TraceLayer::new_for_http()) + .with_state(state); + + let response = api + .as_service() + .oneshot( + Request::builder() + .method("POST") + .uri("/") + .header("content-type", "application/json") + .extension(vsc) + .body(Body::from( + r#"{"policy": {"type": "t", "blob": {"foobar_user": ["role:compute-user"]}}}"#, + )) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::BAD_REQUEST); + } + + #[tokio::test] + async fn test_create_forbidden() { + let vsc = test_fixture_scoped(); + let state = get_mocked_state(Provider::mocked_builder(), false, None).await; + + let mut api = openapi_router() + .layer(TraceLayer::new_for_http()) + .with_state(state); + + let response = api + .as_service() + .oneshot( + Request::builder() + .method("POST") + .uri("/") + .header("content-type", "application/json") + .extension(vsc) + .body(Body::from(r#"{"policy": {"type": "t", "blob": "b"}}"#)) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::FORBIDDEN); + } + + #[tokio::test] + async fn test_create_unauth() { + let state = get_mocked_state(Provider::mocked_builder(), false, None).await; + + let mut api = openapi_router() + .layer(TraceLayer::new_for_http()) + .with_state(state); + + let response = api + .as_service() + .oneshot( + Request::builder() + .method("POST") + .uri("/") + .header("content-type", "application/json") + .body(Body::from(r#"{"policy": {"type": "t", "blob": "b"}}"#)) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::UNAUTHORIZED); + } + + /// Gate B2 (issue #978): the handler must feed `enforce()` exactly the + /// contract `identity/policy/create.rego` documents, with no secret or + /// caller-controlled payload anywhere in it. + #[tokio::test] + async fn test_create_policy_input_contract() { + let mut mock = MockPolicyStoreProvider::default(); + mock.expect_create_policy() + .returning(|_, _| Ok(stored("pid", "application/json", "b"))); + + let vsc = test_fixture_scoped(); + let (state, policy) = + get_capturing_state(Provider::mocked_builder().mock_policy_store(mock)).await; + + let mut api = openapi_router() + .layer(TraceLayer::new_for_http()) + .with_state(state); + + let response = api + .as_service() + .oneshot( + Request::builder() + .method("POST") + .uri("/") + .header("content-type", "application/json") + .extension(vsc) + .body(Body::from( + r#"{"policy": {"type": "application/json", "blob": "s3cr3t-doc", "password": "hunter2", "nested": {"totp_seed": "AAAA"}}}"#, + )) + .unwrap(), + ) + .await + .unwrap(); + assert_eq!(response.status(), StatusCode::CREATED); + + let calls = policy.calls(); + assert_eq!(calls.len(), 1); + assert_eq!(calls[0].policy_name, "identity/policy/create"); + policy_contract::assert_object_keys(&calls[0].target, &["policy"]); + policy_contract::assert_existing_presence(&calls[0].existing, false); + policy_contract::assert_no_secrets(&calls[0].target); + + let rendered = calls[0].target.to_string(); + for needle in ["s3cr3t-doc", "hunter2", "AAAA"] { + assert!( + !rendered.contains(needle), + "{needle} leaked into policy input: {rendered}" + ); + } + } +} diff --git a/crates/keystone/src/api/v3/policy/delete.rs b/crates/keystone/src/api/v3/policy/delete.rs new file mode 100644 index 000000000..93377be22 --- /dev/null +++ b/crates/keystone/src/api/v3/policy/delete.rs @@ -0,0 +1,286 @@ +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 + +//! Delete policy API. + +use axum::{ + extract::{Path, State}, + http::StatusCode, + response::IntoResponse, +}; + +use super::types::Policy; +use crate::api::auth::Auth; +use crate::api::error::KeystoneApiError; +use crate::keystone::ServiceState; +use openstack_keystone_core::auth::ExecutionContext; + +/// Delete a policy. +#[utoipa::path( + delete, + path = "/{policy_id}", + description = "Delete policy by ID", + params(), + responses( + (status = NO_CONTENT, description = "Policy deleted"), + (status = 404, description = "Policy not found") + ), + tag="policies" +)] +#[tracing::instrument( + name = "api::policy_delete", + level = "debug", + skip_all, + fields(policy_id = %policy_id) +)] +pub(super) async fn delete( + Auth(user_auth): Auth, + Path(policy_id): Path, + State(state): State, +) -> Result { + let exec = ExecutionContext::from_auth(&state, &user_auth); + let current = state + .provider + .get_policy_store_provider() + .get_policy(&exec, &policy_id) + .await? + .map(Policy::from); + + state + .policy_enforcer + .enforce( + "identity/policy/delete", + &user_auth, + serde_json::Value::Null, + Some(super::policy_input( + current + .as_ref() + .map(Policy::to_policy_input) + .unwrap_or(serde_json::Value::Null), + )), + ) + .await?; + + match current { + Some(_) => { + state + .provider + .get_policy_store_provider() + .delete_policy(&exec, &policy_id) + .await?; + + Ok(StatusCode::NO_CONTENT.into_response()) + } + None => Err(KeystoneApiError::NotFound { + resource: "policy".into(), + identifier: policy_id, + }), + } +} + +#[cfg(test)] +mod tests { + use axum::{ + body::Body, + http::{Request, StatusCode}, + }; + use tower::ServiceExt; + use tower_http::trace::TraceLayer; + + use openstack_keystone_core_types::policy_store::PolicyBuilder; + + use super::super::openapi_router; + use crate::api::tests::{ + get_capturing_state, get_mocked_state, policy_contract, test_fixture_scoped, + }; + use crate::policy_store::MockPolicyStoreProvider; + use crate::provider::Provider; + + fn stored() -> openstack_keystone_core_types::policy_store::Policy { + PolicyBuilder::default() + .id("pid") + .r#type("application/json") + .blob(serde_json::Value::String("s3cr3t-doc".into())) + .build() + .unwrap() + } + + #[tokio::test] + async fn test_delete() { + let mut mock = MockPolicyStoreProvider::default(); + mock.expect_get_policy() + .returning(|_, _| Ok(Some(stored()))); + mock.expect_delete_policy() + .withf(|_, id: &str| id == "pid") + .returning(|_, _| Ok(())); + + let vsc = test_fixture_scoped(); + let state = get_mocked_state( + Provider::mocked_builder().mock_policy_store(mock), + true, + None, + ) + .await; + + let mut api = openapi_router() + .layer(TraceLayer::new_for_http()) + .with_state(state); + + let response = api + .as_service() + .oneshot( + Request::builder() + .method("DELETE") + .uri("/pid") + .extension(vsc) + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::NO_CONTENT); + } + + #[tokio::test] + async fn test_delete_not_found() { + let mut mock = MockPolicyStoreProvider::default(); + mock.expect_get_policy().returning(|_, _| Ok(None)); + + let vsc = test_fixture_scoped(); + let state = get_mocked_state( + Provider::mocked_builder().mock_policy_store(mock), + true, + None, + ) + .await; + + let mut api = openapi_router() + .layer(TraceLayer::new_for_http()) + .with_state(state); + + let response = api + .as_service() + .oneshot( + Request::builder() + .method("DELETE") + .uri("/missing") + .extension(vsc) + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::NOT_FOUND); + } + + #[tokio::test] + async fn test_delete_forbidden() { + let mut mock = MockPolicyStoreProvider::default(); + mock.expect_get_policy() + .returning(|_, _| Ok(Some(stored()))); + + let vsc = test_fixture_scoped(); + let state = get_mocked_state( + Provider::mocked_builder().mock_policy_store(mock), + false, + None, + ) + .await; + + let mut api = openapi_router() + .layer(TraceLayer::new_for_http()) + .with_state(state); + + let response = api + .as_service() + .oneshot( + Request::builder() + .method("DELETE") + .uri("/pid") + .extension(vsc) + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::FORBIDDEN); + } + + #[tokio::test] + async fn test_delete_unauth() { + let state = get_mocked_state(Provider::mocked_builder(), false, None).await; + + let mut api = openapi_router() + .layer(TraceLayer::new_for_http()) + .with_state(state); + + let response = api + .as_service() + .oneshot( + Request::builder() + .method("DELETE") + .uri("/pid") + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::UNAUTHORIZED); + } + + /// Gate B2 (issue #978). + #[tokio::test] + async fn test_delete_policy_input_contract() { + let mut mock = MockPolicyStoreProvider::default(); + mock.expect_get_policy() + .returning(|_, _| Ok(Some(stored()))); + mock.expect_delete_policy().returning(|_, _| Ok(())); + + let vsc = test_fixture_scoped(); + let (state, policy) = + get_capturing_state(Provider::mocked_builder().mock_policy_store(mock)).await; + + let mut api = openapi_router() + .layer(TraceLayer::new_for_http()) + .with_state(state); + + let response = api + .as_service() + .oneshot( + Request::builder() + .method("DELETE") + .uri("/pid") + .extension(vsc) + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + assert_eq!(response.status(), StatusCode::NO_CONTENT); + + let calls = policy.calls(); + assert_eq!(calls.len(), 1); + assert_eq!(calls[0].policy_name, "identity/policy/delete"); + assert_eq!(calls[0].target, serde_json::Value::Null); + policy_contract::assert_existing_presence(&calls[0].existing, true); + let existing = calls[0].existing.as_ref().unwrap(); + policy_contract::assert_object_keys(existing, &["policy"]); + policy_contract::assert_no_secrets(existing); + assert!(!existing.to_string().contains("s3cr3t-doc")); + } +} diff --git a/crates/keystone/src/api/v3/policy/list.rs b/crates/keystone/src/api/v3/policy/list.rs new file mode 100644 index 000000000..fba502de8 --- /dev/null +++ b/crates/keystone/src/api/v3/policy/list.rs @@ -0,0 +1,712 @@ +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 + +use axum::{ + Json, + extract::{OriginalUri, Query, State}, + http::StatusCode, + response::IntoResponse, +}; + +use openstack_keystone_api_types::PaginationQuery; +use openstack_keystone_core_types::ListPagination; + +use super::types::{Policy, PolicyList, PolicyListParameters}; +use crate::api::auth::Auth; +use crate::api::common::{collect_authorized_page, paginate_forward_filtered}; +use crate::api::error::KeystoneApiError; +use crate::keystone::ServiceState; +use openstack_keystone_core::auth::ExecutionContext; +use openstack_keystone_core::policy::PolicyError; + +/// List policies. +/// +/// Two-phase policy check (security model I8): `identity/policy/list` is +/// enforced first against the query filter, then **every** returned record is +/// individually re-checked against `identity/policy/show` using that record's +/// own identifiers, dropping the ones the caller may not read. A policy error +/// that is not `Forbidden` (OPA unreachable, malformed decision, ...) is +/// propagated rather than silently filtering the item out — failing closed on +/// the request instead of returning a quietly truncated collection. +/// +/// Pagination follows this repository's ADR 0029 `limit`/`marker` + `links` +/// model. Python keystone instead truncates to `[DEFAULT] list_limit` and sets +/// a `truncated` flag; that is a deliberate, documented deviation, consistent +/// with every other collection endpoint here. +#[utoipa::path( + get, + path = "/", + params(PolicyListParameters, PaginationQuery), + description = "List policies", + responses( + (status = OK, description = "List of policies", body = PolicyList), + (status = 500, description = "Internal error") + ), + tag="policies" +)] +#[tracing::instrument( + name = "api::policy_list", + level = "debug", + skip_all, + fields(policy_type = query.r#type.as_deref().unwrap_or("*")) +)] +pub(super) async fn list( + Auth(user_auth): Auth, + OriginalUri(original_url): OriginalUri, + Query(query): Query, + Query(pagination): Query, + State(state): State, +) -> Result { + state + .policy_enforcer + .enforce( + "identity/policy/list", + &user_auth, + super::policy_input(query.to_policy_input()), + None, + ) + .await?; + + let config = state.config_manager.config.read().await; + let base_params = + openstack_keystone_core_types::policy_store::PolicyListParameters::from(query); + let limit = config.resolve_list_limit(&config.policy.list_limit, pagination.limit); + + let provider = state.provider.get_policy_store_provider(); + // Bind by reference: `collect_authorized_page` takes `FnMut`, so the + // per-batch future must capture a `Copy` handle rather than move the + // context out of the closure. + let exec_ctx = ExecutionContext::from_auth(&state, &user_auth); + let exec = &exec_ctx; + let enforcer = &state.policy_enforcer; + let auth = &user_auth; + + let page = collect_authorized_page( + limit, + pagination.marker.clone(), + |marker| { + let mut params = base_params.clone(); + params.pagination = ListPagination { + limit, + marker, + page_reverse: false, + }; + async move { + Ok(provider + .list_policies(exec, ¶ms) + .await? + .into_iter() + .map(Policy::from) + .collect()) + } + }, + |item: Policy| async move { + match enforcer + .enforce( + "identity/policy/show", + auth, + serde_json::Value::Null, + Some(super::policy_input(item.to_policy_input())), + ) + .await + { + Ok(_) => Ok(Some(item)), + Err(PolicyError::Forbidden(_)) => Ok(None), + Err(err) => Err(err.into()), + } + }, + ) + .await?; + + let (policies, links) = paginate_forward_filtered( + &config, + &config.policy.list_limit, + page, + &pagination, + &original_url, + )?; + + Ok((StatusCode::OK, Json(PolicyList { policies, links })).into_response()) +} + +#[cfg(test)] +mod tests { + use std::sync::Arc; + + use axum::{ + body::Body, + http::{Request, StatusCode}, + }; + use http_body_util::BodyExt; + use tower::ServiceExt; + use tower_http::trace::TraceLayer; + + use openstack_keystone_core::auth::ValidatedSecurityContext; + use openstack_keystone_core::policy::{PolicyEnforcer, PolicyError, PolicyEvaluationResult}; + use openstack_keystone_core_types::policy_store::{PolicyBuilder, PolicyListParameters}; + + use super::super::openapi_router; + use crate::api::tests::{ + get_capturing_state, get_mocked_state, get_state_with_policy, policy_contract, + test_fixture_scoped, + }; + use crate::api::v3::policy::types::PolicyList; + use crate::policy_store::MockPolicyStoreProvider; + use crate::provider::Provider; + + fn stored(id: &str) -> openstack_keystone_core_types::policy_store::Policy { + PolicyBuilder::default() + .id(id) + .r#type("application/json") + .blob(serde_json::Value::String(format!("blob-{id}"))) + .build() + .unwrap() + } + + /// A `PolicyEnforcer` that allows the collection check but decides each + /// per-item `show` check by policy id, so the I8 re-check can be tested + /// for both filtering and error propagation. + struct SelectivePolicy { + /// Ids whose `show` check is denied with `Forbidden`. + forbidden: Vec, + /// Ids whose `show` check fails with a non-`Forbidden` error. + erroring: Vec, + } + + #[async_trait::async_trait] + impl PolicyEnforcer for SelectivePolicy { + async fn enforce( + &self, + policy_name: &'static str, + _credentials: &ValidatedSecurityContext, + _target: serde_json::Value, + existing: Option, + ) -> Result { + if policy_name == "identity/policy/show" { + let id = existing + .as_ref() + .and_then(|e| e["policy"]["id"].as_str()) + .unwrap_or_default() + .to_string(); + if self.erroring.contains(&id) { + return Err(PolicyError::IO(std::io::Error::other("opa unreachable"))); + } + if self.forbidden.contains(&id) { + return Err(PolicyError::Forbidden(PolicyEvaluationResult::forbidden())); + } + } + Ok(PolicyEvaluationResult::allowed_admin()) + } + + async fn health_check(&self) -> Result<(), PolicyError> { + Ok(()) + } + } + + async fn list_with( + policy: SelectivePolicy, + stored_ids: &'static [&'static str], + ) -> axum::response::Response { + let mut mock = MockPolicyStoreProvider::default(); + mock.expect_list_policies() + .returning(|_, _| Ok(stored_ids.iter().map(|id| stored(id)).collect())); + + let vsc = test_fixture_scoped(); + let state = get_state_with_policy( + Provider::mocked_builder().mock_policy_store(mock), + Arc::new(policy), + ) + .await; + + let mut api = openapi_router() + .layer(TraceLayer::new_for_http()) + .with_state(state); + + api.as_service() + .oneshot( + Request::builder() + .uri("/") + .extension(vsc) + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap() + } + + #[tokio::test] + async fn test_list() { + let mut mock = MockPolicyStoreProvider::default(); + mock.expect_list_policies() + .withf(|_, _: &PolicyListParameters| true) + .returning(|_, _| Ok(vec![stored("1")])); + + let vsc = test_fixture_scoped(); + let state = get_mocked_state( + Provider::mocked_builder().mock_policy_store(mock), + true, + None, + ) + .await; + + let mut api = openapi_router() + .layer(TraceLayer::new_for_http()) + .with_state(state); + + let response = api + .as_service() + .oneshot( + Request::builder() + .uri("/") + .extension(vsc) + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::OK); + let body = response.into_body().collect().await.unwrap().to_bytes(); + let res: PolicyList = serde_json::from_slice(&body).unwrap(); + assert_eq!(res.policies.len(), 1); + assert_eq!(res.policies[0].id, "1"); + assert_eq!(res.policies[0].blob, serde_json::json!("blob-1")); + } + + /// `?type=` must reach the provider as an exact-match filter. + #[tokio::test] + async fn test_list_type_filter_reaches_provider() { + let mut mock = MockPolicyStoreProvider::default(); + mock.expect_list_policies() + .withf(|_, qp: &PolicyListParameters| qp.r#type == Some("application/json".into())) + .returning(|_, _| Ok(Vec::new())); + + let vsc = test_fixture_scoped(); + let state = get_mocked_state( + Provider::mocked_builder().mock_policy_store(mock), + true, + None, + ) + .await; + + let mut api = openapi_router() + .layer(TraceLayer::new_for_http()) + .with_state(state); + + let response = api + .as_service() + .oneshot( + Request::builder() + .uri("/?type=application%2Fjson") + .extension(vsc) + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::OK); + } + + #[tokio::test] + async fn test_list_unauth() { + let state = get_mocked_state(Provider::mocked_builder(), false, None).await; + + let mut api = openapi_router() + .layer(TraceLayer::new_for_http()) + .with_state(state); + + let response = api + .as_service() + .oneshot(Request::builder().uri("/").body(Body::empty()).unwrap()) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::UNAUTHORIZED); + } + + #[tokio::test] + async fn test_list_forbidden() { + let state = get_mocked_state(Provider::mocked_builder(), false, None).await; + + let mut api = openapi_router() + .layer(TraceLayer::new_for_http()) + .with_state(state); + + let response = api + .as_service() + .oneshot( + Request::builder() + .uri("/") + .extension(test_fixture_scoped()) + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::FORBIDDEN); + } + + /// Security model I8: one collection check, then exactly one per-item + /// `show` check for every candidate, each keyed on that record's own + /// identifiers under `existing`. + #[tokio::test] + async fn test_list_runs_one_show_check_per_item() { + let mut mock = MockPolicyStoreProvider::default(); + mock.expect_list_policies() + .returning(|_, _| Ok(vec![stored("1"), stored("2"), stored("3")])); + + let vsc = test_fixture_scoped(); + let (state, policy) = + get_capturing_state(Provider::mocked_builder().mock_policy_store(mock)).await; + + let mut api = openapi_router() + .layer(TraceLayer::new_for_http()) + .with_state(state); + + let response = api + .as_service() + .oneshot( + Request::builder() + .uri("/") + .extension(vsc) + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + assert_eq!(response.status(), StatusCode::OK); + + let calls = policy.calls(); + assert_eq!(calls.len(), 4, "1 collection check + 3 per-item checks"); + + assert_eq!(calls[0].policy_name, "identity/policy/list"); + policy_contract::assert_object_keys(&calls[0].target, &["policy"]); + policy_contract::assert_existing_presence(&calls[0].existing, false); + policy_contract::assert_no_secrets(&calls[0].target); + + for (i, expected_id) in ["1", "2", "3"].iter().enumerate() { + let call = &calls[i + 1]; + assert_eq!(call.policy_name, "identity/policy/show"); + assert_eq!(call.target, serde_json::Value::Null); + policy_contract::assert_existing_presence(&call.existing, true); + let existing = call.existing.as_ref().unwrap(); + policy_contract::assert_object_keys(existing, &["policy"]); + policy_contract::assert_no_secrets(existing); + assert_eq!(existing["policy"]["id"], *expected_id); + assert!( + !existing.to_string().contains("blob"), + "per-item input must not carry the document: {existing}" + ); + } + } + + /// Items the per-item `show` policy denies are omitted from the + /// collection (CVE-2019-19687 class). + #[tokio::test] + async fn test_list_omits_items_denied_by_show_policy() { + let response = list_with( + SelectivePolicy { + forbidden: vec!["2".into()], + erroring: Vec::new(), + }, + &["1", "2", "3"], + ) + .await; + + assert_eq!(response.status(), StatusCode::OK); + let body = response.into_body().collect().await.unwrap().to_bytes(); + let res: PolicyList = serde_json::from_slice(&body).unwrap(); + let ids: Vec<&str> = res.policies.iter().map(|p| p.id.as_str()).collect(); + assert_eq!(ids, vec!["1", "3"], "denied item must be omitted"); + } + + /// A non-`Forbidden` policy failure must not be mistaken for "deny this + /// item": it fails the whole request instead of silently truncating. + #[tokio::test] + async fn test_list_propagates_non_forbidden_policy_errors() { + let response = list_with( + SelectivePolicy { + forbidden: Vec::new(), + erroring: vec!["2".into()], + }, + &["1", "2", "3"], + ) + .await; + + assert_eq!(response.status(), StatusCode::INTERNAL_SERVER_ERROR); + } + + #[tokio::test] + async fn test_list_pagination_link() { + let mut mock = MockPolicyStoreProvider::default(); + mock.expect_list_policies() + .withf(|_, qp: &PolicyListParameters| qp.pagination.limit == Some(1)) + .returning(|_, _| Ok(vec![stored("1"), stored("2")])); + + let vsc = test_fixture_scoped(); + let state = get_mocked_state( + Provider::mocked_builder().mock_policy_store(mock), + true, + None, + ) + .await; + + let mut api = openapi_router() + .layer(TraceLayer::new_for_http()) + .with_state(state); + + let response = api + .as_service() + .oneshot( + Request::builder() + .uri("/?limit=1") + .extension(vsc) + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::OK); + let body = response.into_body().collect().await.unwrap().to_bytes(); + let res: PolicyList = serde_json::from_slice(&body).unwrap(); + assert_eq!(res.policies.len(), 1); + assert_eq!(res.policies[0].id, "1"); + assert!(res.links.is_some()); + } + + /// The per-item filter must not truncate pagination: with `limit=2` and a + /// denied row in the first batch, the handler keeps pulling batches until + /// it has `limit + 1` *visible* items, so a full page and a `next` link + /// are still produced. Before the fill loop this returned a short page + /// with no `next`, stranding every remaining visible policy. + #[tokio::test] + async fn test_list_refills_page_when_items_are_filtered_out() { + // Backend pages of 3 (limit + 1). + let mut mock = MockPolicyStoreProvider::default(); + mock.expect_list_policies().returning(|_, params| { + let after = params.pagination.marker.clone(); + let all = ["1", "2", "3", "4", "5", "6", "7"]; + let start = match after { + None => 0, + Some(m) => all.iter().position(|id| *id == m).map_or(0, |i| i + 1), + }; + Ok(all + .iter() + .skip(start) + .take(3) + .map(|id| stored(id)) + .collect()) + }); + + let vsc = test_fixture_scoped(); + let state = get_state_with_policy( + Provider::mocked_builder().mock_policy_store(mock), + Arc::new(SelectivePolicy { + forbidden: vec!["1".into()], + erroring: Vec::new(), + }), + ) + .await; + + let mut api = openapi_router() + .layer(TraceLayer::new_for_http()) + .with_state(state); + + let response = api + .as_service() + .oneshot( + Request::builder() + .uri("/?limit=2") + .extension(vsc) + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::OK); + let body = response.into_body().collect().await.unwrap().to_bytes(); + let res: PolicyList = serde_json::from_slice(&body).unwrap(); + + let ids: Vec<&str> = res.policies.iter().map(|p| p.id.as_str()).collect(); + assert_eq!( + ids, + vec!["2", "3"], + "the page must be filled with visible items past the denied row" + ); + assert!( + res.links.is_some(), + "more visible policies remain, so a next link is required" + ); + } + + /// When denied rows exhaust the `limit * AUTHORIZED_PAGE_SCAN_FACTOR` + /// examine budget before the page fills, the response is deliberately + /// short — but it must still carry a `next` link, or the caller would read + /// the short page as the end of the collection and lose the tail. The + /// budget is what stops a sparse-visibility caller from forcing a full + /// table scan. + #[tokio::test] + async fn test_list_short_page_still_links_when_scan_budget_exhausted() { + let mut mock = MockPolicyStoreProvider::default(); + mock.expect_list_policies().returning(|_, params| { + let after = params.pagination.marker.clone(); + let all = ["1", "2", "3", "4", "5", "6", "7"]; + let start = match after { + None => 0, + Some(m) => all.iter().position(|id| *id == m).map_or(0, |i| i + 1), + }; + Ok(all + .iter() + .skip(start) + .take(3) + .map(|id| stored(id)) + .collect()) + }); + + let vsc = test_fixture_scoped(); + let state = get_state_with_policy( + Provider::mocked_builder().mock_policy_store(mock), + Arc::new(SelectivePolicy { + // limit=2 gives a budget of 4 examined rows; three denials + // leave room for only one authorized item. + forbidden: vec!["1".into(), "2".into(), "3".into()], + erroring: Vec::new(), + }), + ) + .await; + + let mut api = openapi_router() + .layer(TraceLayer::new_for_http()) + .with_state(state); + + let response = api + .as_service() + .oneshot( + Request::builder() + .uri("/?limit=2") + .extension(vsc) + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::OK); + let body = response.into_body().collect().await.unwrap().to_bytes(); + let res: PolicyList = serde_json::from_slice(&body).unwrap(); + + let ids: Vec<&str> = res.policies.iter().map(|p| p.id.as_str()).collect(); + assert_eq!(ids, vec!["4"], "the budget cut the scan short"); + let links = res + .links + .expect("a truncated scan must still advertise a next link"); + assert!( + links[0].href.contains("marker=4"), + "keyed on the last authorized item: {}", + links[0].href + ); + } + + /// A `next` link must keep the collection's resource filters: dropping + /// `?type=` would silently widen page 2 from the filtered collection to + /// the unfiltered one. + #[tokio::test] + async fn test_list_pagination_link_preserves_type_filter() { + let mut mock = MockPolicyStoreProvider::default(); + mock.expect_list_policies() + .returning(|_, _| Ok(vec![stored("1"), stored("2")])); + + let vsc = test_fixture_scoped(); + let state = get_mocked_state( + Provider::mocked_builder().mock_policy_store(mock), + true, + None, + ) + .await; + + let mut api = openapi_router() + .layer(TraceLayer::new_for_http()) + .with_state(state); + + let response = api + .as_service() + .oneshot( + Request::builder() + .uri("/?type=application%2Fjson&limit=1") + .extension(vsc) + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::OK); + let body = response.into_body().collect().await.unwrap().to_bytes(); + let res: PolicyList = serde_json::from_slice(&body).unwrap(); + + let links = res.links.expect("a next link is produced"); + let next = links + .iter() + .find(|l| l.rel == "next") + .expect("next link present"); + assert!( + next.href.contains("type=application%2Fjson"), + "next link must keep the type filter: {}", + next.href + ); + assert!(next.href.contains("marker=1"), "next link: {}", next.href); + } + + #[tokio::test] + async fn test_list_pagination_no_false_positive_next() { + let mut mock = MockPolicyStoreProvider::default(); + mock.expect_list_policies() + .withf(|_, qp: &PolicyListParameters| qp.pagination.limit == Some(1)) + .returning(|_, _| Ok(vec![stored("1")])); + + let vsc = test_fixture_scoped(); + let state = get_mocked_state( + Provider::mocked_builder().mock_policy_store(mock), + true, + None, + ) + .await; + + let mut api = openapi_router() + .layer(TraceLayer::new_for_http()) + .with_state(state); + + let response = api + .as_service() + .oneshot( + Request::builder() + .uri("/?limit=1") + .extension(vsc) + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::OK); + let body = response.into_body().collect().await.unwrap().to_bytes(); + let res: PolicyList = serde_json::from_slice(&body).unwrap(); + assert_eq!(res.policies.len(), 1); + assert_eq!(res.links, None); + } +} diff --git a/crates/keystone/src/api/v3/policy/mod.rs b/crates/keystone/src/api/v3/policy/mod.rs new file mode 100644 index 000000000..157b976d8 --- /dev/null +++ b/crates/keystone/src/api/v3/policy/mod.rs @@ -0,0 +1,390 @@ +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 +//! `/v3/policies` API — legacy policy document storage (issue #1035). +//! +//! CRUD over opaque, arbitrarily serialized policy documents that *remote* +//! services fetch and interpret. Deprecated upstream ("Keystone is not a +//! policy management service"), implemented for python API compatibility. +//! +//! Not to be confused with this service's own authorization, which is decided +//! by OPA over `policy/**/*.rego`. +//! +//! Routes mirror python keystone exactly: `GET`/`POST` on the collection and +//! `GET`/`PATCH`/`DELETE` on the member. There is deliberately **no `PUT`** — +//! `PolicyResource` upstream defines only those five verbs, so +//! `PUT /v3/policies/{id}` must stay a 405. + +use serde_json::{Value, json}; +use utoipa_axum::{router::OpenApiRouter, routes}; + +use crate::keystone::ServiceState; + +mod create; +mod delete; +mod list; +mod show; +pub mod types; +mod update; + +/// Wrap a non-sensitive field projection in the `{"policy": …}` envelope that +/// every `policy/policy/*.rego` rule reads (ADR 0002). +/// +/// # Security Note +/// +/// The projection itself is built by `to_policy_input()` on the API types, +/// which *constructs* the document from an allowlist (`id`, `type`) rather +/// than filtering a denylist out of it. A policy `blob` is arbitrary +/// caller-supplied data and `extra` has an open key space, so either could +/// carry secrets; no `.rego` rule reads them, and an external OPA can persist +/// policy input via decision logging. +pub(super) fn policy_input(projection: Value) -> Value { + json!({ "policy": projection }) +} + +pub(super) fn openapi_router() -> OpenApiRouter { + OpenApiRouter::new() + .routes(routes!(list::list, create::create)) + .routes(routes!(show::show, update::update, delete::delete)) +} + +/// Gate B3 (security review V3a, issue #979): the five handlers driven +/// against a real `opa run` subprocess evaluating the repository's actual +/// `policy/policy/*.rego`, instead of a mock's canned allow/deny. +/// +/// The mocked tests prove each handler *builds* the documented input; only +/// these prove the input the handler actually sends is decided the way +/// python keystone's defaults decide it — +/// `RULE_ADMIN_OR_SYSTEM_READER` for `list`/`show`, `RULE_ADMIN_REQUIRED` +/// for `create`/`update`/`delete`. +#[cfg(test)] +mod real_policy_decision { + use axum::{ + body::Body, + http::{Request, StatusCode}, + }; + use tower::ServiceExt; + use tower_http::trace::TraceLayer; + + use openstack_keystone_core::auth::ValidatedSecurityContext; + use openstack_keystone_core_types::policy_store::PolicyBuilder; + + use super::openapi_router; + use crate::api::tests::get_state_with_real_policy; + use crate::api::tests::real_policy_fixtures::{member_vsc, system_scoped_vsc}; + use crate::policy_store::MockPolicyStoreProvider; + use crate::provider::Provider; + + fn stored() -> openstack_keystone_core_types::policy_store::Policy { + PolicyBuilder::default() + .id("pid") + .r#type("application/json") + .blob(serde_json::Value::String("doc".into())) + .build() + .unwrap() + } + + fn provider_with_everything() -> crate::provider::ProviderBuilder { + let mut mock = MockPolicyStoreProvider::default(); + mock.expect_list_policies() + .returning(|_, _| Ok(vec![stored()])); + mock.expect_get_policy() + .returning(|_, _| Ok(Some(stored()))); + mock.expect_create_policy().returning(|_, _| Ok(stored())); + mock.expect_update_policy() + .returning(|_, _, _| Ok(stored())); + mock.expect_delete_policy().returning(|_, _| Ok(())); + Provider::mocked_builder().mock_policy_store(mock) + } + + /// Drive one request through the real Rego and return its status. + async fn status_for( + vsc: ValidatedSecurityContext, + method: &str, + uri: &str, + body: Option<&'static str>, + ) -> StatusCode { + let (state, _opa) = get_state_with_real_policy(provider_with_everything()).await; + + let mut api = openapi_router() + .layer(TraceLayer::new_for_http()) + .with_state(state); + + let mut request = Request::builder().method(method).uri(uri).extension(vsc); + if body.is_some() { + request = request.header("content-type", "application/json"); + } + let request = request + .body(body.map_or_else(Body::empty, Body::from)) + .unwrap(); + + api.as_service().oneshot(request).await.unwrap().status() + } + + /// `admin` passes every verb. + #[tokio::test] + async fn test_real_policy_admin_allowed_everywhere() { + let admin = || member_vsc("uid", "pid", &["admin"]); + + assert_eq!(status_for(admin(), "GET", "/", None).await, StatusCode::OK); + assert_eq!( + status_for(admin(), "GET", "/pid", None).await, + StatusCode::OK + ); + assert_eq!( + status_for( + admin(), + "POST", + "/", + Some(r#"{"policy": {"type": "application/json", "blob": "b"}}"#) + ) + .await, + StatusCode::CREATED + ); + assert_eq!( + status_for( + admin(), + "PATCH", + "/pid", + Some(r#"{"policy": {"type": "text/plain"}}"#) + ) + .await, + StatusCode::OK + ); + assert_eq!( + status_for(admin(), "DELETE", "/pid", None).await, + StatusCode::NO_CONTENT + ); + } + + /// A system-scoped `reader` may read (`list`, `show`) — the + /// `RULE_ADMIN_OR_SYSTEM_READER` path, exercised through the real + /// `input.credentials.system` projection rather than a + /// hand-written Rego input. + #[tokio::test] + async fn test_real_policy_system_reader_may_read() { + assert_eq!( + status_for( + system_scoped_vsc("uid", "system", &["reader"]), + "GET", + "/", + None + ) + .await, + StatusCode::OK + ); + assert_eq!( + status_for( + system_scoped_vsc("uid", "system", &["reader"]), + "GET", + "/pid", + None + ) + .await, + StatusCode::OK + ); + } + + /// ... but must not write: `create`/`update`/`delete` are + /// `RULE_ADMIN_REQUIRED` upstream, with no system-reader path. + #[tokio::test] + async fn test_real_policy_system_reader_may_not_write() { + assert_eq!( + status_for( + system_scoped_vsc("uid", "system", &["reader"]), + "POST", + "/", + Some(r#"{"policy": {"type": "application/json", "blob": "b"}}"#) + ) + .await, + StatusCode::FORBIDDEN + ); + assert_eq!( + status_for( + system_scoped_vsc("uid", "system", &["reader"]), + "PATCH", + "/pid", + Some(r#"{"policy": {"type": "text/plain"}}"#) + ) + .await, + StatusCode::FORBIDDEN + ); + assert_eq!( + status_for( + system_scoped_vsc("uid", "system", &["reader"]), + "DELETE", + "/pid", + None + ) + .await, + StatusCode::FORBIDDEN + ); + } + + /// A project-scoped `reader` is not a *system* reader: it may not even + /// read. Catches dropping the `system_scope` conjunct from the read + /// rules. + #[tokio::test] + async fn test_real_policy_project_reader_denied() { + assert_eq!( + status_for(member_vsc("uid", "pid", &["reader"]), "GET", "/", None).await, + StatusCode::FORBIDDEN + ); + assert_eq!( + status_for(member_vsc("uid", "pid", &["reader"]), "GET", "/pid", None).await, + StatusCode::FORBIDDEN + ); + } + + /// A domain-scoped system value is not `"all"`. + #[tokio::test] + async fn test_real_policy_non_all_system_scope_denied() { + assert_eq!( + status_for( + system_scoped_vsc("uid", "other_system", &["reader"]), + "GET", + "/", + None + ) + .await, + StatusCode::FORBIDDEN + ); + } + + /// Security model I8 against the real Rego: a system reader whose + /// per-item `show` check passes sees the item; a plain `member` gets + /// the collection check itself refused, so no item can leak. + #[tokio::test] + async fn test_real_policy_member_cannot_list() { + assert_eq!( + status_for(member_vsc("uid", "pid", &["member"]), "GET", "/", None).await, + StatusCode::FORBIDDEN + ); + } +} + +#[cfg(test)] +mod tests { + use std::collections::HashMap; + + use axum::body::Body; + use axum::http::{Request, StatusCode}; + use tower::ServiceExt; + + use super::{openapi_router, policy_input}; + use crate::api::tests::{get_mocked_state, policy_contract}; + use crate::api::v3::policy::types::Policy; + use crate::provider::Provider; + + /// Gate I (security review V9, issue #987): a direct structural test on + /// the projection itself, independent of any handler round-trip. A future + /// field addition to the API `Policy` cannot leak into policy input, + /// because the projection is an allowlist rather than a filter. + #[test] + fn test_policy_input_never_leaks_blob_or_extra() { + let policy = Policy { + id: "pid".into(), + r#type: "application/json".into(), + blob: serde_json::json!("{\"secret\": \"s3cr3t\"}"), + extra: HashMap::from([ + ("password".into(), serde_json::json!("hunter2")), + ( + "deeply".into(), + serde_json::json!({"nested": {"totp_seed": "AAAA"}}), + ), + ]), + }; + + let input = policy_input(policy.to_policy_input()); + policy_contract::assert_object_keys(&input, &["policy"]); + policy_contract::assert_no_secrets(&input); + + let rendered = input.to_string(); + for needle in ["s3cr3t", "hunter2", "AAAA"] { + assert!( + !rendered.contains(needle), + "policy input must not contain {needle}: {rendered}" + ); + } + } + + /// The envelope key must be exactly `policy`: a mis-keyed resource makes + /// every `input.target.policy.*` lookup `undefined` in Rego, which an + /// `undefined`-driven rule can silently treat as allow. + #[test] + fn test_policy_input_envelope_key() { + let input = policy_input(serde_json::json!({"id": "x", "type": "t"})); + policy_contract::assert_object_keys(&input, &["policy"]); + assert_eq!(input["policy"]["id"], "x"); + } + + /// Python keystone's `PolicyResource` exposes only `GET`/`POST` on the + /// collection and `GET`/`PATCH`/`DELETE` on the member. `PUT + /// /v3/policies/{id}` must therefore be a 405, not a create-or-update + /// (unlike `PUT /v3/regions/{id}`, which python keystone *does* define). + #[tokio::test] + async fn test_put_member_is_method_not_allowed() { + let state = get_mocked_state(Provider::mocked_builder(), true, None).await; + + let mut api = openapi_router().with_state(state); + + let response = api + .as_service() + .oneshot( + Request::builder() + .method("PUT") + .uri("/pid") + .header("content-type", "application/json") + .body(Body::from(r#"{"policy": {"type": "t", "blob": "b"}}"#)) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::METHOD_NOT_ALLOWED); + } + + /// The generated OpenAPI document must publish exactly the five upstream + /// operations and no `put`. + #[test] + fn test_openapi_publishes_five_operations_and_no_put() { + let (_, api) = openapi_router().split_for_parts(); + + let collection = api.paths.paths.get("/").expect("collection path published"); + assert!(collection.get.is_some(), "GET /v3/policies"); + assert!(collection.post.is_some(), "POST /v3/policies"); + assert!(collection.put.is_none(), "no PUT on the collection"); + assert!(collection.patch.is_none(), "no PATCH on the collection"); + assert!(collection.delete.is_none(), "no DELETE on the collection"); + + let member = api + .paths + .paths + .get("/{policy_id}") + .expect("member path published"); + assert!(member.get.is_some(), "GET /v3/policies/{{policy_id}}"); + assert!(member.patch.is_some(), "PATCH /v3/policies/{{policy_id}}"); + assert!(member.delete.is_some(), "DELETE /v3/policies/{{policy_id}}"); + assert!( + member.put.is_none(), + "PUT /v3/policies/{{policy_id}} does not exist upstream" + ); + assert!(member.post.is_none(), "no POST on the member"); + + assert_eq!( + api.paths.paths.len(), + 2, + "only the collection and member paths are published" + ); + } +} diff --git a/crates/keystone/src/api/v3/policy/show.rs b/crates/keystone/src/api/v3/policy/show.rs new file mode 100644 index 000000000..672010cff --- /dev/null +++ b/crates/keystone/src/api/v3/policy/show.rs @@ -0,0 +1,278 @@ +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 +//! # Show policy API + +use axum::{ + Json, + extract::{Path, State}, + http::StatusCode, + response::IntoResponse, +}; + +use super::types::{Policy, PolicyResponse}; +use crate::api::auth::Auth; +use crate::api::error::KeystoneApiError; +use crate::keystone::ServiceState; +use openstack_keystone_core::auth::ExecutionContext; + +/// Get a single policy. +#[utoipa::path( + get, + path = "/{policy_id}", + description = "Get policy by ID", + params(), + responses( + (status = OK, description = "Policy object", body = PolicyResponse), + (status = 404, description = "Policy not found") + ), + tag="policies" +)] +#[tracing::instrument( + name = "api::policy_show", + level = "debug", + skip_all, + fields(policy_id = %policy_id) +)] +pub(super) async fn show( + Auth(user_auth): Auth, + Path(policy_id): Path, + State(state): State, +) -> Result { + let current = state + .provider + .get_policy_store_provider() + .get_policy(&ExecutionContext::from_auth(&state, &user_auth), &policy_id) + .await? + .map(Policy::from); + + state + .policy_enforcer + .enforce( + "identity/policy/show", + &user_auth, + serde_json::Value::Null, + Some(super::policy_input( + current + .as_ref() + .map(Policy::to_policy_input) + .unwrap_or(serde_json::Value::Null), + )), + ) + .await?; + + match current { + Some(policy) => Ok((StatusCode::OK, Json(PolicyResponse { policy })).into_response()), + None => Err(KeystoneApiError::NotFound { + resource: "policy".into(), + identifier: policy_id, + }), + } +} + +#[cfg(test)] +mod tests { + use axum::{ + body::Body, + http::{Request, StatusCode}, + }; + use http_body_util::BodyExt; + use tower::ServiceExt; + use tower_http::trace::TraceLayer; + + use openstack_keystone_core_types::policy_store::PolicyBuilder; + + use super::super::openapi_router; + use crate::api::tests::{ + get_capturing_state, get_mocked_state, policy_contract, test_fixture_scoped, + }; + use crate::api::v3::policy::types::PolicyResponse; + use crate::policy_store::MockPolicyStoreProvider; + use crate::provider::Provider; + + fn stored() -> openstack_keystone_core_types::policy_store::Policy { + PolicyBuilder::default() + .id("pid") + .r#type("application/json") + .blob(serde_json::Value::String("s3cr3t-doc".into())) + .build() + .unwrap() + } + + #[tokio::test] + async fn test_show() { + let mut mock = MockPolicyStoreProvider::default(); + mock.expect_get_policy() + .withf(|_, id: &str| id == "pid") + .returning(|_, _| Ok(Some(stored()))); + + let vsc = test_fixture_scoped(); + let state = get_mocked_state( + Provider::mocked_builder().mock_policy_store(mock), + true, + None, + ) + .await; + + let mut api = openapi_router() + .layer(TraceLayer::new_for_http()) + .with_state(state); + + let response = api + .as_service() + .oneshot( + Request::builder() + .uri("/pid") + .extension(vsc) + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::OK); + let body = response.into_body().collect().await.unwrap().to_bytes(); + let res: PolicyResponse = serde_json::from_slice(&body).unwrap(); + assert_eq!(res.policy.id, "pid"); + assert_eq!(res.policy.blob, serde_json::json!("s3cr3t-doc")); + } + + #[tokio::test] + async fn test_show_not_found() { + let mut mock = MockPolicyStoreProvider::default(); + mock.expect_get_policy().returning(|_, _| Ok(None)); + + let vsc = test_fixture_scoped(); + let state = get_mocked_state( + Provider::mocked_builder().mock_policy_store(mock), + true, + None, + ) + .await; + + let mut api = openapi_router() + .layer(TraceLayer::new_for_http()) + .with_state(state); + + let response = api + .as_service() + .oneshot( + Request::builder() + .uri("/missing") + .extension(vsc) + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::NOT_FOUND); + } + + #[tokio::test] + async fn test_show_forbidden() { + let mut mock = MockPolicyStoreProvider::default(); + mock.expect_get_policy() + .returning(|_, _| Ok(Some(stored()))); + + let vsc = test_fixture_scoped(); + let state = get_mocked_state( + Provider::mocked_builder().mock_policy_store(mock), + false, + None, + ) + .await; + + let mut api = openapi_router() + .layer(TraceLayer::new_for_http()) + .with_state(state); + + let response = api + .as_service() + .oneshot( + Request::builder() + .uri("/pid") + .extension(vsc) + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::FORBIDDEN); + } + + #[tokio::test] + async fn test_show_unauth() { + let state = get_mocked_state(Provider::mocked_builder(), false, None).await; + + let mut api = openapi_router() + .layer(TraceLayer::new_for_http()) + .with_state(state); + + let response = api + .as_service() + .oneshot(Request::builder().uri("/pid").body(Body::empty()).unwrap()) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::UNAUTHORIZED); + } + + /// Gate B2 (issue #978): the stored record is keyed under `existing`, the + /// target is null, and the document never reaches OPA. + #[tokio::test] + async fn test_show_policy_input_contract() { + let mut mock = MockPolicyStoreProvider::default(); + mock.expect_get_policy().returning(|_, _| { + let mut policy = stored(); + policy + .extra + .insert("password".into(), serde_json::json!("hunter2")); + Ok(Some(policy)) + }); + + let vsc = test_fixture_scoped(); + let (state, policy) = + get_capturing_state(Provider::mocked_builder().mock_policy_store(mock)).await; + + let mut api = openapi_router() + .layer(TraceLayer::new_for_http()) + .with_state(state); + + let response = api + .as_service() + .oneshot( + Request::builder() + .uri("/pid") + .extension(vsc) + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + assert_eq!(response.status(), StatusCode::OK); + + let calls = policy.calls(); + assert_eq!(calls.len(), 1); + assert_eq!(calls[0].policy_name, "identity/policy/show"); + assert_eq!(calls[0].target, serde_json::Value::Null); + policy_contract::assert_existing_presence(&calls[0].existing, true); + let existing = calls[0].existing.as_ref().unwrap(); + policy_contract::assert_object_keys(existing, &["policy"]); + policy_contract::assert_no_secrets(existing); + let rendered = existing.to_string(); + assert!(!rendered.contains("s3cr3t-doc")); + assert!(!rendered.contains("hunter2")); + } +} diff --git a/crates/keystone/src/api/v3/policy/types.rs b/crates/keystone/src/api/v3/policy/types.rs new file mode 100644 index 000000000..c81ade6bf --- /dev/null +++ b/crates/keystone/src/api/v3/policy/types.rs @@ -0,0 +1,21 @@ +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 + +pub use openstack_keystone_api_types::v3::policy::*; + +impl crate::api::common::ResourceIdentifier for Policy { + fn get_id(&self) -> String { + self.id.clone() + } +} diff --git a/crates/keystone/src/api/v3/policy/update.rs b/crates/keystone/src/api/v3/policy/update.rs new file mode 100644 index 000000000..8c7137af3 --- /dev/null +++ b/crates/keystone/src/api/v3/policy/update.rs @@ -0,0 +1,407 @@ +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 +//! # Update policy API + +use axum::{ + Json, + extract::{Path, State}, + http::StatusCode, + response::IntoResponse, +}; +use validator::Validate; + +use super::types::{Policy, PolicyResponse, PolicyUpdateRequest}; +use crate::api::auth::Auth; +use crate::api::error::KeystoneApiError; +use crate::keystone::ServiceState; +use openstack_keystone_core::auth::ExecutionContext; + +/// Update an existing policy. +/// +/// Only the supplied fields change; `extra` properties are merged into the +/// stored ones. Two request-shape rules are inherited from python keystone: +/// +/// * an empty `{"policy": {}}` document is a 400 (`minProperties: 1` in +/// `keystone/policy/schema.py`), and +/// * an `id` in the body is accepted only when it equals `{policy_id}`, else +/// 400 ("Cannot change policy ID" in `Manager.update_policy`). It is never +/// persisted either way. +#[utoipa::path( + patch, + path = "/{policy_id}", + description = "Update policy by ID", + params(), + responses( + (status = OK, description = "Updated policy", body = PolicyResponse), + (status = 400, description = "Invalid input"), + (status = 404, description = "Policy not found") + ), + tag="policies" +)] +// `skip_all` for the same reason as `create`: the patch body may carry a new +// `blob` and arbitrary `extra` properties. +#[tracing::instrument( + name = "api::policy_update", + level = "debug", + skip_all, + fields(policy_id = %policy_id) +)] +pub(super) async fn update( + Auth(user_auth): Auth, + Path(policy_id): Path, + State(state): State, + Json(req): Json, +) -> Result { + req.validate()?; + + if req.policy.is_empty() { + return Err(KeystoneApiError::BadRequest( + "at least one policy property must be supplied".into(), + )); + } + + if let Some(body_id) = &req.policy.id + && *body_id != policy_id + { + return Err(KeystoneApiError::BadRequest( + "Cannot change policy ID".into(), + )); + } + + let exec = ExecutionContext::from_auth(&state, &user_auth); + let current = state + .provider + .get_policy_store_provider() + .get_policy(&exec, &policy_id) + .await? + .map(Policy::from); + + state + .policy_enforcer + .enforce( + "identity/policy/update", + &user_auth, + super::policy_input(req.policy.to_policy_input()), + Some(super::policy_input( + current + .as_ref() + .map(Policy::to_policy_input) + .unwrap_or(serde_json::Value::Null), + )), + ) + .await?; + + match current { + Some(_) => { + let updated = state + .provider + .get_policy_store_provider() + .update_policy(&exec, &policy_id, req.into()) + .await?; + Ok(( + StatusCode::OK, + Json(PolicyResponse { + policy: Policy::from(updated), + }), + ) + .into_response()) + } + None => Err(KeystoneApiError::NotFound { + resource: "policy".into(), + identifier: policy_id, + }), + } +} + +#[cfg(test)] +mod tests { + use axum::{ + body::Body, + http::{Request, StatusCode}, + }; + use http_body_util::BodyExt; + use tower::ServiceExt; + use tower_http::trace::TraceLayer; + + use openstack_keystone_core_types::policy_store::{PolicyBuilder, PolicyUpdate}; + + use super::super::openapi_router; + use crate::api::tests::{ + get_capturing_state, get_mocked_state, policy_contract, test_fixture_scoped, + }; + use crate::api::v3::policy::types::PolicyResponse; + use crate::policy_store::MockPolicyStoreProvider; + use crate::provider::Provider; + + fn stored() -> openstack_keystone_core_types::policy_store::Policy { + PolicyBuilder::default() + .id("pid") + .r#type("application/json") + .blob(serde_json::Value::String("original blob".into())) + .build() + .unwrap() + } + + async fn patch(body: &'static str, mock: MockPolicyStoreProvider) -> axum::response::Response { + let vsc = test_fixture_scoped(); + let state = get_mocked_state( + Provider::mocked_builder().mock_policy_store(mock), + true, + None, + ) + .await; + + // Mount the production `normalize_json_rejection` layer so + // type-mismatch bodies surface as the 400 the service actually + // returns rather than axum's raw 422. + let mut api = openapi_router() + .layer(axum::middleware::map_response( + crate::api::json_rejection::normalize_json_rejection, + )) + .layer(TraceLayer::new_for_http()) + .with_state(state); + + api.as_service() + .oneshot( + Request::builder() + .method("PATCH") + .uri("/pid") + .header("content-type", "application/json") + .extension(vsc) + .body(Body::from(body)) + .unwrap(), + ) + .await + .unwrap() + } + + /// A type-only PATCH must not touch the blob — the exact assertion in + /// tempest's `test_create_update_delete_policy`. + #[tokio::test] + async fn test_update_type_only() { + let mut mock = MockPolicyStoreProvider::default(); + mock.expect_get_policy() + .returning(|_, _| Ok(Some(stored()))); + mock.expect_update_policy() + .withf(|_, id: &str, upd: &PolicyUpdate| { + id == "pid" && upd.r#type == Some("text/plain".into()) && upd.blob.is_none() + }) + .returning(|_, _, _| { + let mut policy = stored(); + policy.r#type = "text/plain".into(); + Ok(policy) + }); + + let response = patch(r#"{"policy": {"type": "text/plain"}}"#, mock).await; + + assert_eq!(response.status(), StatusCode::OK); + let body = response.into_body().collect().await.unwrap().to_bytes(); + let res: PolicyResponse = serde_json::from_slice(&body).unwrap(); + assert_eq!(res.policy.r#type, "text/plain"); + assert_eq!(res.policy.blob, serde_json::json!("original blob")); + } + + /// An `id` equal to the path is accepted and not persisted. + #[tokio::test] + async fn test_update_accepts_matching_id_without_persisting_it() { + let mut mock = MockPolicyStoreProvider::default(); + mock.expect_get_policy() + .returning(|_, _| Ok(Some(stored()))); + mock.expect_update_policy() + .withf(|_, _: &str, upd: &PolicyUpdate| !upd.extra.contains_key("id")) + .returning(|_, _, _| Ok(stored())); + + let response = patch(r#"{"policy": {"id": "pid", "type": "text/plain"}}"#, mock).await; + assert_eq!(response.status(), StatusCode::OK); + } + + /// An `id` different from the path is a 400, matching python keystone's + /// "Cannot change policy ID". + #[tokio::test] + async fn test_update_rejects_mismatched_id() { + let response = patch( + r#"{"policy": {"id": "other", "type": "text/plain"}}"#, + MockPolicyStoreProvider::default(), + ) + .await; + assert_eq!(response.status(), StatusCode::BAD_REQUEST); + } + + /// `minProperties: 1`: an empty patch document is a 400. + #[tokio::test] + async fn test_update_rejects_empty_document() { + let response = patch(r#"{"policy": {}}"#, MockPolicyStoreProvider::default()).await; + assert_eq!(response.status(), StatusCode::BAD_REQUEST); + } + + /// Requests carry a string blob only. + #[tokio::test] + async fn test_update_rejects_object_blob() { + let response = patch( + r#"{"policy": {"blob": {"foobar_user": ["role:compute-user"]}}}"#, + MockPolicyStoreProvider::default(), + ) + .await; + assert_eq!(response.status(), StatusCode::BAD_REQUEST); + } + + /// Supplied extras are forwarded for merging; the driver preserves the + /// omitted stored ones. + #[tokio::test] + async fn test_update_forwards_extra_for_merge() { + let mut mock = MockPolicyStoreProvider::default(); + mock.expect_get_policy() + .returning(|_, _| Ok(Some(stored()))); + mock.expect_update_policy() + .withf(|_, _: &str, upd: &PolicyUpdate| { + upd.extra.get("added") == Some(&serde_json::json!("x")) + }) + .returning(|_, _, _| Ok(stored())); + + let response = patch(r#"{"policy": {"added": "x"}}"#, mock).await; + assert_eq!(response.status(), StatusCode::OK); + } + + #[tokio::test] + async fn test_update_not_found() { + let mut mock = MockPolicyStoreProvider::default(); + mock.expect_get_policy().returning(|_, _| Ok(None)); + + let response = patch(r#"{"policy": {"type": "text/plain"}}"#, mock).await; + assert_eq!(response.status(), StatusCode::NOT_FOUND); + } + + #[tokio::test] + async fn test_update_forbidden() { + let mut mock = MockPolicyStoreProvider::default(); + mock.expect_get_policy() + .returning(|_, _| Ok(Some(stored()))); + + let vsc = test_fixture_scoped(); + let state = get_mocked_state( + Provider::mocked_builder().mock_policy_store(mock), + false, + None, + ) + .await; + + let mut api = openapi_router() + .layer(TraceLayer::new_for_http()) + .with_state(state); + + let response = api + .as_service() + .oneshot( + Request::builder() + .method("PATCH") + .uri("/pid") + .header("content-type", "application/json") + .extension(vsc) + .body(Body::from(r#"{"policy": {"type": "text/plain"}}"#)) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::FORBIDDEN); + } + + #[tokio::test] + async fn test_update_unauth() { + let state = get_mocked_state(Provider::mocked_builder(), false, None).await; + + let mut api = openapi_router() + .layer(TraceLayer::new_for_http()) + .with_state(state); + + let response = api + .as_service() + .oneshot( + Request::builder() + .method("PATCH") + .uri("/pid") + .header("content-type", "application/json") + .body(Body::from(r#"{"policy": {"type": "text/plain"}}"#)) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::UNAUTHORIZED); + } + + /// Gate B2 (issue #978): the requested change goes to `target`, the + /// stored record to `existing` (not swapped), and neither carries the + /// document or arbitrary extras. + #[tokio::test] + async fn test_update_policy_input_contract() { + let mut mock = MockPolicyStoreProvider::default(); + mock.expect_get_policy() + .returning(|_, _| Ok(Some(stored()))); + mock.expect_update_policy() + .returning(|_, _, _| Ok(stored())); + + let vsc = test_fixture_scoped(); + let (state, policy) = + get_capturing_state(Provider::mocked_builder().mock_policy_store(mock)).await; + + let mut api = openapi_router() + .layer(TraceLayer::new_for_http()) + .with_state(state); + + let response = api + .as_service() + .oneshot( + Request::builder() + .method("PATCH") + .uri("/pid") + .header("content-type", "application/json") + .extension(vsc) + .body(Body::from( + r#"{"policy": {"type": "text/plain", "blob": "new-s3cr3t", "totp_seed": "AAAA"}}"#, + )) + .unwrap(), + ) + .await + .unwrap(); + assert_eq!(response.status(), StatusCode::OK); + + let calls = policy.calls(); + assert_eq!(calls.len(), 1); + assert_eq!(calls[0].policy_name, "identity/policy/update"); + + policy_contract::assert_object_keys(&calls[0].target, &["policy"]); + policy_contract::assert_no_secrets(&calls[0].target); + assert_eq!(calls[0].target["policy"]["type"], "text/plain"); + + policy_contract::assert_existing_presence(&calls[0].existing, true); + let existing = calls[0].existing.as_ref().unwrap(); + policy_contract::assert_object_keys(existing, &["policy"]); + policy_contract::assert_no_secrets(existing); + assert_eq!( + existing["policy"]["type"], "application/json", + "existing must be the stored record, not the request" + ); + + for value in [&calls[0].target, existing] { + let rendered = value.to_string(); + for needle in ["new-s3cr3t", "original blob", "AAAA"] { + assert!( + !rendered.contains(needle), + "{needle} leaked into policy input: {rendered}" + ); + } + } + } +} diff --git a/crates/keystone/src/lib.rs b/crates/keystone/src/lib.rs index c4990daaf..03bf1227c 100644 --- a/crates/keystone/src/lib.rs +++ b/crates/keystone/src/lib.rs @@ -104,6 +104,7 @@ include!(concat!(env!("OUT_DIR"), "/inventory_anchors.rs")); pub mod plugin_manager; pub mod policy; +pub mod policy_store; pub mod provider; pub mod resource; pub mod revoke; diff --git a/crates/keystone/src/plugin_manager.rs b/crates/keystone/src/plugin_manager.rs index 85ef72f58..f386d36f7 100644 --- a/crates/keystone/src/plugin_manager.rs +++ b/crates/keystone/src/plugin_manager.rs @@ -53,6 +53,8 @@ use openstack_keystone_core::oauth2_key::Oauth2KeyProviderError; use openstack_keystone_core::oauth2_key::backend::Oauth2KeyBackend; use openstack_keystone_core::oauth2_session::Oauth2SessionProviderError; use openstack_keystone_core::oauth2_session::backend::Oauth2SessionBackend; +use openstack_keystone_core::policy_store::PolicyStoreProviderError; +use openstack_keystone_core::policy_store::backend::PolicyStoreBackend; use openstack_keystone_core::resource::backend::ResourceBackend; use openstack_keystone_core::resource::error::ResourceProviderError; use openstack_keystone_core::revoke::RevokeProviderError; @@ -118,6 +120,7 @@ pub struct PluginManager { token_restriction_backends: HashMap>, /// Trust backend plugins. trust_backends: HashMap>, + policy_store_backends: HashMap>, } impl PluginManagerApi for PluginManager { @@ -542,6 +545,24 @@ impl PluginManagerApi for PluginManager { )) } + /// Get registered policy store backend (legacy `/v3/policies`). + /// + /// # Parameters + /// * `name` - The name of the backend to retrieve. + /// + /// # Returns + /// A `Result` containing a reference to the `PolicyStoreBackend` if found, + /// or a `PolicyStoreProviderError`. + #[allow(clippy::borrowed_box)] + fn get_policy_store_backend>( + &self, + name: S, + ) -> Result<&Arc, PolicyStoreProviderError> { + self.policy_store_backends.get(name.as_ref()).ok_or( + PolicyStoreProviderError::UnsupportedDriver(name.as_ref().to_string()), + ) + } + /// Register API Key backend. /// /// # Parameters @@ -828,6 +849,20 @@ impl PluginManagerApi for PluginManager { self.trust_backends .insert(name.as_ref().to_string(), plugin); } + + /// Register policy store backend (legacy `/v3/policies`). + /// + /// # Parameters + /// * `name` - The name of the backend to register. + /// * `plugin` - The backend implementation to register. + fn register_policy_store_backend>( + &mut self, + name: S, + plugin: Arc, + ) { + self.policy_store_backends + .insert(name.as_ref().to_string(), plugin); + } } impl PluginManager { @@ -870,6 +905,7 @@ impl PluginManager { token_backends: HashMap::new(), token_restriction_backends: HashMap::new(), trust_backends: HashMap::new(), + policy_store_backends: HashMap::new(), }; register_backends(config, &mut slf.api_key_backends).await?; register_backends(config, &mut slf.application_credential_backends).await?; @@ -893,6 +929,7 @@ impl PluginManager { register_backends(config, &mut slf.token_backends).await?; register_backends(config, &mut slf.token_restriction_backends).await?; register_backends(config, &mut slf.trust_backends).await?; + register_backends(config, &mut slf.policy_store_backends).await?; Ok(slf) } } diff --git a/crates/keystone/src/policy_store/mod.rs b/crates/keystone/src/policy_store/mod.rs new file mode 100644 index 000000000..981a9820c --- /dev/null +++ b/crates/keystone/src/policy_store/mod.rs @@ -0,0 +1,21 @@ +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 +//! # Policy store provider +//! +//! Storage for the legacy `/v3/policies` documents. +//! +//! Not to be confused with [`crate::policy`], which is the OPA authorization +//! enforcement client. A stored policy is inert data that remote services +//! fetch and interpret; this service never evaluates it. +pub use openstack_keystone_core::policy_store::*; diff --git a/crates/policy-store-driver-sql/Cargo.toml b/crates/policy-store-driver-sql/Cargo.toml new file mode 100644 index 000000000..576c8e5e9 --- /dev/null +++ b/crates/policy-store-driver-sql/Cargo.toml @@ -0,0 +1,30 @@ +[package] +name = "openstack-keystone-policy-store-driver-sql" +description = "OpenStack Keystone SQL driver for the policy store provider" +version = "0.1.0" +edition.workspace = true +license.workspace = true +authors.workspace = true +rust-version.workspace = true +repository.workspace = true +homepage.workspace = true +exclude.workspace = true + +[dependencies] +async-trait.workspace = true +inventory.workspace = true +openstack-keystone-core.workspace = true +openstack-keystone-core-types.workspace = true +sea-orm = { workspace = true, features = ["default"] } +serde_json.workspace = true +tracing.workspace = true +uuid.workspace = true + +[dev-dependencies] +openstack-keystone-core = { workspace = true, features = ["mock"] } +sea-orm = { workspace = true, features = ["mock", "sqlx-sqlite" ]} +serde_json.workspace = true +tokio = { workspace = true, features = ["macros"] } + +[lints] +workspace = true diff --git a/crates/policy-store-driver-sql/src/entity.rs b/crates/policy-store-driver-sql/src/entity.rs new file mode 100644 index 000000000..f6ca204db --- /dev/null +++ b/crates/policy-store-driver-sql/src/entity.rs @@ -0,0 +1,19 @@ +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 +#![allow(clippy::doc_paragraphs_missing_punctuation)] +//! `SeaORM` entities for the policy store. + +pub mod prelude; + +pub mod policy; diff --git a/crates/policy-store-driver-sql/src/entity/policy.rs b/crates/policy-store-driver-sql/src/entity/policy.rs new file mode 100644 index 000000000..84291a83c --- /dev/null +++ b/crates/policy-store-driver-sql/src/entity/policy.rs @@ -0,0 +1,52 @@ +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 + +//! `SeaORM` Entity for the `policy` table. +//! +//! Column types mirror python keystone's initial migration exactly, so +//! `SqlDriver::setup` produces the same schema an existing python-managed +//! database already has: +//! +//! ```text +//! id String(64) primary key +//! type String(255) not null +//! blob Text not null (ks_sql.JsonBlob) +//! extra Text null (ks_sql.JsonBlob) +//! ``` + +use sea_orm::entity::prelude::*; + +#[derive(Clone, Debug, PartialEq, DeriveEntityModel, Eq)] +#[sea_orm(table_name = "policy")] +pub struct Model { + #[sea_orm( + primary_key, + auto_increment = false, + column_type = "String(StringLen::N(64))" + )] + pub id: String, + #[sea_orm(column_name = "type", column_type = "String(StringLen::N(255))")] + pub r#type: String, + /// JSON-encoded policy document (python keystone's `JsonBlob`). + #[sea_orm(column_type = "Text")] + pub blob: String, + /// JSON-encoded additional properties. + #[sea_orm(column_type = "Text", nullable)] + pub extra: Option, +} + +#[derive(Copy, Clone, Debug, EnumIter, DeriveRelation)] +pub enum Relation {} + +impl ActiveModelBehavior for ActiveModel {} diff --git a/crates/policy-store-driver-sql/src/entity/prelude.rs b/crates/policy-store-driver-sql/src/entity/prelude.rs new file mode 100644 index 000000000..57a66f509 --- /dev/null +++ b/crates/policy-store-driver-sql/src/entity/prelude.rs @@ -0,0 +1,18 @@ +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 + +//! `SeaORM` Entity prelude. +#![allow(unused_imports)] + +pub use super::policy::Entity as Policy; diff --git a/crates/policy-store-driver-sql/src/lib.rs b/crates/policy-store-driver-sql/src/lib.rs new file mode 100644 index 000000000..8c4bbce25 --- /dev/null +++ b/crates/policy-store-driver-sql/src/lib.rs @@ -0,0 +1,212 @@ +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 +//! # OpenStack Keystone SQL driver for the policy store provider +//! +//! Persists the legacy `/v3/policies` documents in the `policy` table, whose +//! schema matches python keystone's so an existing database works unchanged. +//! This has nothing to do with OPA authorization — see +//! `openstack_keystone_core::policy_store` for the distinction. + +use std::sync::Arc; + +use async_trait::async_trait; +use sea_orm::{DatabaseConnection, Schema}; + +use openstack_keystone_core::error::DatabaseError; +use openstack_keystone_core::keystone::ServiceState; +use openstack_keystone_core::plugin_manager::BackendRegistration; +use openstack_keystone_core::policy_store::PolicyStoreProviderError; +use openstack_keystone_core::policy_store::backend::PolicyStoreBackend; +use openstack_keystone_core::{SqlDriver, SqlDriverRegistration, db::create_table}; +use openstack_keystone_core_types::policy_store::*; + +pub mod entity; +mod policy; + +#[derive(Default)] +pub struct SqlBackend {} + +/// Linkage anchor — see ADR-0018. Referenced by the `keystone` crate's +/// `build.rs`-generated `_ANCHORS` static so the linker extracts `.rlib` +/// members, keeping `inventory::submit!` sections visible at runtime. +#[allow(dead_code)] +pub fn anchor() {} + +// Submit the plugin to the registry at compile-time +static PLUGIN: SqlBackend = SqlBackend {}; +inventory::submit! { + SqlDriverRegistration { driver: &PLUGIN } +} +inventory::submit! { + BackendRegistration:: { + name: "sql", + selected: |_| true, + build: |_cfg| Box::pin(async { + Ok(Arc::new(SqlBackend::default()) as Arc) + }), + } +} + +#[async_trait] +impl PolicyStoreBackend for SqlBackend { + /// Create a new policy. + /// + /// # Parameters + /// - `state`: The service state containing the database connection. + /// - `policy`: The policy creation parameters. + /// + /// # Returns + /// A `Result` containing the created `Policy`, or a + /// `PolicyStoreProviderError`. + // `skip_all`: `policy` carries the document (`blob`) and arbitrary + // caller-supplied `extra` properties, which `Debug` would otherwise render + // into the span. + #[tracing::instrument(level = "debug", skip_all, fields(policy_type = %policy.r#type))] + async fn create_policy( + &self, + state: &ServiceState, + policy: PolicyCreate, + ) -> Result { + policy::create(&state.db, policy).await + } + + /// Delete a policy by ID. + /// + /// # Parameters + /// - `state`: The service state containing the database connection. + /// - `id`: The unique identifier of the policy. + /// + /// # Returns + /// A `Result` indicating success or a `PolicyStoreProviderError`. + #[tracing::instrument(level = "debug", skip_all, fields(policy_id = %id))] + async fn delete_policy( + &self, + state: &ServiceState, + id: &str, + ) -> Result<(), PolicyStoreProviderError> { + policy::delete(&state.db, id).await + } + + /// Get a single policy by ID. + /// + /// # Parameters + /// - `state`: The service state containing the database connection. + /// - `id`: The unique identifier of the policy. + /// + /// # Returns + /// A `Result` containing an `Option` with the `Policy` if found, or a + /// `PolicyStoreProviderError`. + #[tracing::instrument(level = "debug", skip_all, fields(policy_id = %id))] + async fn get_policy( + &self, + state: &ServiceState, + id: &str, + ) -> Result, PolicyStoreProviderError> { + policy::get(&state.db, id).await + } + + /// List policies. + /// + /// # Parameters + /// - `state`: The service state containing the database connection. + /// - `params`: Parameters for filtering the policy list. + /// + /// # Returns + /// A `Result` containing a vector of `Policy` objects or a + /// `PolicyStoreProviderError`. + #[tracing::instrument(level = "debug", skip_all)] + async fn list_policies( + &self, + state: &ServiceState, + params: &PolicyListParameters, + ) -> Result, PolicyStoreProviderError> { + policy::list(&state.db, params).await + } + + /// Update an existing policy. + /// + /// # Parameters + /// - `state`: The service state containing the database connection. + /// - `id`: The unique identifier of the policy. + /// - `policy`: The fields to change. + /// + /// # Returns + /// A `Result` containing the updated `Policy`, or a + /// `PolicyStoreProviderError`. + // `skip_all` for the same reason as `create_policy`. + #[tracing::instrument(level = "debug", skip_all, fields(policy_id = %id))] + async fn update_policy( + &self, + state: &ServiceState, + id: &str, + policy: PolicyUpdate, + ) -> Result { + policy::update(&state.db, id, policy).await + } +} + +#[async_trait] +impl SqlDriver for SqlBackend { + /// Sets up the database tables for the policy store. + /// + /// # Parameters + /// - `connection`: The database connection. + /// - `schema`: The database schema. + /// + /// # Returns + /// A `Result` indicating success or a `DatabaseError`. + async fn setup( + &self, + connection: &DatabaseConnection, + schema: &Schema, + ) -> Result<(), DatabaseError> { + create_table(connection, schema, crate::entity::prelude::Policy).await?; + Ok(()) + } +} + +#[cfg(test)] +mod tests { + use sea_orm::sea_query::PostgresQueryBuilder; + use sea_orm::{DatabaseBackend, Schema}; + + /// The generated schema must match python keystone's `policy` table: + /// `id VARCHAR(64)` primary key and `type VARCHAR(255)`, not sea-orm's + /// unconstrained default. A wider column silently diverges from an + /// existing python-managed database. + #[test] + fn test_schema_column_widths_match_python_keystone() { + let schema = Schema::new(DatabaseBackend::Postgres); + let sql = schema + .create_table_from_entity(crate::entity::prelude::Policy) + .to_string(PostgresQueryBuilder); + + assert!( + sql.contains(r#""id" varchar(64)"#), + "expected id VARCHAR(64), got: {sql}" + ); + assert!( + sql.contains(r#""type" varchar(255)"#), + "expected type VARCHAR(255), got: {sql}" + ); + assert!( + sql.contains(r#""blob" text NOT NULL"#), + "expected blob TEXT NOT NULL, got: {sql}" + ); + assert!( + sql.contains(r#""extra" text"#), + "expected extra TEXT, got: {sql}" + ); + } +} diff --git a/crates/policy-store-driver-sql/src/policy.rs b/crates/policy-store-driver-sql/src/policy.rs new file mode 100644 index 000000000..ab29130c9 --- /dev/null +++ b/crates/policy-store-driver-sql/src/policy.rs @@ -0,0 +1,230 @@ +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 + +use std::collections::HashMap; + +use sea_orm::entity::*; +use serde_json::Value; +use tracing::error; + +use openstack_keystone_core::policy_store::PolicyStoreProviderError; +use openstack_keystone_core_types::policy_store::*; + +use crate::entity::policy as db_policy; + +mod create; +mod delete; +mod get; +mod list; +mod update; + +pub use create::create; +pub use delete::delete; +pub use get::get; +pub use list::list; +pub use update::update; + +/// Decode a `JsonBlob`-style column into a map of extra properties. +/// +/// A malformed value is an **error**, not an empty map. The catalog driver +/// logs-and-drops for `region.extra`, but that is unsafe here: `update()` +/// reads the stored extras, merges the patch over them, and writes the result +/// back, so swallowing a decode failure would silently replace a row's +/// existing properties with `{}` — permanent data loss on an ordinary PATCH. +/// Python keystone's `JsonBlob.process_result_value` is a bare `json.loads` +/// and raises here too. +fn decode_extra(raw: Option<&String>) -> Result, PolicyStoreProviderError> { + match raw { + Some(extra) if extra != "{}" => serde_json::from_str::>(extra) + .map_err(|e| { + error!("failed to deserialize policy extra: {e}"); + PolicyStoreProviderError::Serde { source: e } + }), + _ => Ok(HashMap::new()), + } +} + +impl TryFrom for Policy { + type Error = PolicyStoreProviderError; + + /// Tries to convert a database policy model into a domain policy. + /// + /// # Parameters + /// - `value`: The database policy model. + /// + /// # Returns + /// A `Result` containing the `Policy`, or a `PolicyStoreProviderError`. + fn try_from(value: db_policy::Model) -> Result { + // `blob` is `NOT NULL` and always JSON-encoded by whoever wrote it + // (python keystone's `JsonBlob.process_bind_param` does + // `json.dumps`). A row that fails to decode is corrupt data, and + // python keystone raises there too, so propagate rather than guess. + let blob: Value = serde_json::from_str(&value.blob)?; + + let mut builder = PolicyBuilder::default(); + builder.id(value.id.clone()); + builder.r#type(value.r#type.clone()); + builder.blob(blob); + builder.extra(decode_extra(value.extra.as_ref())?); + + Ok(builder.build()?) + } +} + +impl TryFrom for db_policy::ActiveModel { + type Error = PolicyStoreProviderError; + + /// Tries to convert policy creation parameters into a database active + /// model. + /// + /// # Parameters + /// - `value`: The policy creation parameters. + /// + /// # Returns + /// A `Result` containing the `ActiveModel`, or a + /// `PolicyStoreProviderError`. + fn try_from(value: PolicyCreate) -> Result { + Ok(Self { + // The service layer always assigns the ID before calling the + // backend, so that the audit event carries it. No fallback here: + // a second generation site would silently diverge from the ID the + // audit record already announced. + id: Set(value.id.ok_or_else(|| { + PolicyStoreProviderError::Driver( + "policy id must be assigned by the service layer".into(), + ) + })?), + r#type: Set(value.r#type), + blob: Set(serde_json::to_string(&value.blob)?), + extra: Set(Some(serde_json::to_string(&value.extra)?)), + }) + } +} + +#[cfg(test)] +pub(crate) mod tests { + use super::*; + use crate::entity::policy; + + /// A stored policy whose `blob` is a JSON *string*, which is what the API + /// accepts today. + pub fn get_policy_mock>(id: I) -> policy::Model { + policy::Model { + id: id.into(), + r#type: "application/json".into(), + blob: r#""{'foobar_user': 'role:compute-user'}""#.to_string(), + extra: Some(r#"{"key": "value"}"#.to_string()), + } + } + + /// A stored policy whose `blob` is a JSON *object*, as written by older + /// python keystone releases before the create schema was narrowed to a + /// string. Such rows must keep deserializing. + pub fn get_legacy_object_blob_policy_mock>(id: I) -> policy::Model { + policy::Model { + id: id.into(), + r#type: "application/json".into(), + blob: r#"{"foobar_user":["role:compute-user"]}"#.to_string(), + extra: None, + } + } + + #[test] + fn test_string_blob_round_trips() { + let policy: Policy = get_policy_mock("1").try_into().unwrap(); + assert_eq!(policy.id, "1"); + assert_eq!(policy.r#type, "application/json"); + assert_eq!( + policy.blob, + Value::String("{'foobar_user': 'role:compute-user'}".into()) + ); + assert_eq!( + policy.extra, + HashMap::from([("key".to_string(), serde_json::json!("value"))]) + ); + } + + #[test] + fn test_legacy_object_blob_round_trips() { + let policy: Policy = get_legacy_object_blob_policy_mock("2").try_into().unwrap(); + assert_eq!( + policy.blob, + serde_json::json!({"foobar_user":["role:compute-user"]}) + ); + assert!(policy.extra.is_empty()); + } + + /// `blob` is JSON-encoded on write, exactly as python keystone's + /// `JsonBlob` type decorator does, so a string blob is stored quoted. + #[test] + fn test_create_encodes_blob_as_json() { + let model: db_policy::ActiveModel = PolicyCreate { + id: Some("pid".into()), + r#type: "application/json".into(), + blob: Value::String("raw text".into()), + extra: HashMap::new(), + } + .try_into() + .unwrap(); + + // Match the typed `sea_orm::Value` rather than its `to_string()` + // rendering: the `Display` form adds its own quoting/escaping, so + // asserting on it tests sea-orm's formatting instead of what this + // driver actually writes. + let stored = match model.blob.into_value() { + Some(sea_orm::Value::String(Some(s))) => s.to_string(), + other => panic!("blob must bind as a non-null string, got {other:?}"), + }; + + // python keystone's `JsonBlob.process_bind_param` is `json.dumps`, so + // a string blob is stored as a *quoted* JSON string. + assert_eq!(stored, r#""raw text""#); + assert_eq!( + serde_json::from_str::(&stored).unwrap(), + Value::String("raw text".into()), + "the stored bytes must decode back to the original value" + ); + } + + /// A corrupt (non-JSON) `blob` column is an error, not a silent + /// misrepresentation. + #[test] + fn test_malformed_blob_is_error() { + let model = policy::Model { + id: "3".into(), + r#type: "application/json".into(), + blob: "not json at all".into(), + extra: None, + }; + assert!(Policy::try_from(model).is_err()); + } + + /// A malformed `extra` must surface as an error rather than decoding to an + /// empty map: `update()` writes the decoded extras back, so silently + /// yielding `{}` would destroy the row's stored properties on the next + /// PATCH. + #[test] + fn test_malformed_extra_is_error() { + let model = policy::Model { + id: "4".into(), + r#type: "application/json".into(), + blob: r#""x""#.into(), + extra: Some("{not json".into()), + }; + assert!(matches!( + Policy::try_from(model), + Err(PolicyStoreProviderError::Serde { .. }) + )); + } +} diff --git a/crates/policy-store-driver-sql/src/policy/create.rs b/crates/policy-store-driver-sql/src/policy/create.rs new file mode 100644 index 000000000..503e5389d --- /dev/null +++ b/crates/policy-store-driver-sql/src/policy/create.rs @@ -0,0 +1,114 @@ +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 +//! # Create policy + +use sea_orm::DatabaseConnection; +use sea_orm::entity::*; + +use openstack_keystone_core::error::DbContextExt; +use openstack_keystone_core::policy_store::PolicyStoreProviderError; +use openstack_keystone_core_types::policy_store::{Policy, PolicyCreate}; + +use crate::entity::policy as db_policy; + +/// Creates a new policy. +/// +/// # Parameters +/// - `db`: The database connection. +/// - `policy`: The policy creation parameters. +/// +/// # Returns +/// A `Result` containing the created `Policy`, or an `Error`. +pub async fn create( + db: &DatabaseConnection, + policy: PolicyCreate, +) -> Result { + TryInto::::try_into(policy)? + .insert(db) + .await + .context("creating policy")? + .try_into() +} + +#[cfg(test)] +mod tests { + use std::collections::HashMap; + + use sea_orm::{DatabaseBackend, MockDatabase}; + use serde_json::{Value, json}; + + use super::*; + + #[tokio::test] + async fn test_create() { + let db = MockDatabase::new(DatabaseBackend::Postgres) + .append_query_results([vec![db_policy::Model { + id: "policy-1".into(), + r#type: "application/json".into(), + blob: r#""blob text""#.into(), + extra: Some(r#"{"key":"value"}"#.into()), + }]]) + .into_connection(); + + let created = create( + &db, + PolicyCreate { + id: Some("policy-1".into()), + r#type: "application/json".into(), + blob: Value::String("blob text".into()), + extra: HashMap::from([("key".into(), json!("value"))]), + }, + ) + .await + .unwrap(); + + assert_eq!(created.id, "policy-1"); + assert_eq!(created.r#type, "application/json"); + assert_eq!(created.blob, Value::String("blob text".into())); + assert_eq!( + created.extra, + HashMap::from([("key".to_string(), json!("value"))]) + ); + } + + /// A missing ID is a programming error, not something the driver papers + /// over: the service layer has already announced the ID in the audit + /// event by the time the backend is called. + #[tokio::test] + async fn test_create_without_id_is_rejected() { + let db = MockDatabase::new(DatabaseBackend::Postgres) + .append_query_results([Vec::::new()]) + .into_connection(); + + let result = create( + &db, + PolicyCreate { + id: None, + r#type: "text/plain".into(), + blob: Value::String("x".into()), + extra: HashMap::new(), + }, + ) + .await; + + assert!(matches!( + result, + Err(openstack_keystone_core::policy_store::PolicyStoreProviderError::Driver(_)) + )); + assert!( + db.into_transaction_log().is_empty(), + "nothing may be written without a service-assigned ID" + ); + } +} diff --git a/crates/policy-store-driver-sql/src/policy/delete.rs b/crates/policy-store-driver-sql/src/policy/delete.rs new file mode 100644 index 000000000..b0c4eed7d --- /dev/null +++ b/crates/policy-store-driver-sql/src/policy/delete.rs @@ -0,0 +1,90 @@ +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 +//! # Delete policy + +use sea_orm::DatabaseConnection; +use sea_orm::entity::*; + +use openstack_keystone_core::error::DbContextExt; +use openstack_keystone_core::policy_store::PolicyStoreProviderError; + +use crate::entity::prelude::Policy as DbPolicy; + +/// Deletes a policy by ID. +/// +/// # Parameters +/// - `db`: The database connection. +/// - `id`: The ID of the policy to delete. +/// +/// # Returns +/// A `Result` indicating success, or `PolicyNotFound` when no row matched. +pub async fn delete>( + db: &DatabaseConnection, + id: I, +) -> Result<(), PolicyStoreProviderError> { + let res = DbPolicy::delete_by_id(id.as_ref()) + .exec(db) + .await + .context("deleting policy")?; + + if res.rows_affected == 0 { + return Err(PolicyStoreProviderError::PolicyNotFound( + id.as_ref().to_string(), + )); + } + Ok(()) +} + +#[cfg(test)] +mod tests { + use sea_orm::{DatabaseBackend, MockDatabase, MockExecResult, Transaction}; + + use super::*; + + #[tokio::test] + async fn test_delete() { + let db = MockDatabase::new(DatabaseBackend::Postgres) + .append_exec_results([MockExecResult { + last_insert_id: 0, + rows_affected: 1, + }]) + .into_connection(); + + assert!(delete(&db, "1").await.is_ok()); + + assert_eq!( + db.into_transaction_log(), + [Transaction::from_sql_and_values( + DatabaseBackend::Postgres, + r#"DELETE FROM "policy" WHERE "policy"."id" = $1"#, + ["1".into()] + )] + ); + } + + #[tokio::test] + async fn test_delete_missing_is_not_found() { + let db = MockDatabase::new(DatabaseBackend::Postgres) + .append_exec_results([MockExecResult { + last_insert_id: 0, + rows_affected: 0, + }]) + .into_connection(); + + assert!(matches!( + delete(&db, "missing").await, + Err(PolicyStoreProviderError::PolicyNotFound(_)) + )); + } +} diff --git a/crates/policy-store-driver-sql/src/policy/get.rs b/crates/policy-store-driver-sql/src/policy/get.rs new file mode 100644 index 000000000..12767e008 --- /dev/null +++ b/crates/policy-store-driver-sql/src/policy/get.rs @@ -0,0 +1,86 @@ +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 +//! # Get policy + +use sea_orm::DatabaseConnection; +use sea_orm::entity::*; + +use openstack_keystone_core::error::DbContextExt; +use openstack_keystone_core::policy_store::PolicyStoreProviderError; +use openstack_keystone_core_types::policy_store::Policy; + +use crate::entity::prelude::Policy as DbPolicy; + +/// Fetches a single policy by ID. +/// +/// # Parameters +/// - `db`: The database connection. +/// - `id`: The ID of the policy. +/// +/// # Returns +/// A `Result` containing an `Option` with the `Policy` if found, or an +/// `Error`. +pub async fn get>( + db: &DatabaseConnection, + id: I, +) -> Result, PolicyStoreProviderError> { + DbPolicy::find_by_id(id.as_ref()) + .one(db) + .await + .context("fetching policy")? + .map(TryInto::try_into) + .transpose() +} + +#[cfg(test)] +mod tests { + use sea_orm::{DatabaseBackend, MockDatabase, Transaction}; + use serde_json::Value; + + use super::super::tests::get_policy_mock; + use super::*; + + #[tokio::test] + async fn test_get() { + let db = MockDatabase::new(DatabaseBackend::Postgres) + .append_query_results([vec![get_policy_mock("1")]]) + .append_query_results([Vec::::new()]) + .into_connection(); + + let found = get(&db, "1").await.unwrap().unwrap(); + assert_eq!(found.id, "1"); + assert_eq!( + found.blob, + Value::String("{'foobar_user': 'role:compute-user'}".into()) + ); + + assert!(get(&db, "missing").await.unwrap().is_none()); + + assert_eq!( + db.into_transaction_log(), + [ + Transaction::from_sql_and_values( + DatabaseBackend::Postgres, + r#"SELECT "policy"."id", "policy"."type", "policy"."blob", "policy"."extra" FROM "policy" WHERE "policy"."id" = $1 LIMIT $2"#, + ["1".into(), 1u64.into()] + ), + Transaction::from_sql_and_values( + DatabaseBackend::Postgres, + r#"SELECT "policy"."id", "policy"."type", "policy"."blob", "policy"."extra" FROM "policy" WHERE "policy"."id" = $1 LIMIT $2"#, + ["missing".into(), 1u64.into()] + ), + ] + ); + } +} diff --git a/crates/policy-store-driver-sql/src/policy/list.rs b/crates/policy-store-driver-sql/src/policy/list.rs new file mode 100644 index 000000000..5333ca917 --- /dev/null +++ b/crates/policy-store-driver-sql/src/policy/list.rs @@ -0,0 +1,185 @@ +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 +//! # List policies + +use sea_orm::DatabaseConnection; +use sea_orm::entity::*; +use sea_orm::query::*; +use sea_orm::{Cursor, SelectModel}; + +use openstack_keystone_core::error::DbContextExt; +use openstack_keystone_core::policy_store::PolicyStoreProviderError; +use openstack_keystone_core_types::policy_store::*; + +use crate::entity::{policy as db_policy, prelude::Policy as DbPolicy}; + +/// Prepare the paginated query for listing policies. +/// +/// The `type` filter is an exact match, matching python keystone's +/// `filter_by_attributes` behaviour for the `type` hint (which it applies in +/// its API layer; pushing it into SQL here is observably identical). +/// +/// # Parameters +/// - `params`: The parameters for listing policies. +/// +/// # Returns +/// A `Result` containing a `Cursor` for the select model. +fn get_list_query( + params: &PolicyListParameters, +) -> Result>, PolicyStoreProviderError> { + let mut select = DbPolicy::find(); + + if let Some(r#type) = ¶ms.r#type { + select = select.filter(db_policy::Column::Type.eq(r#type)); + } + + let mut cursor = select.cursor_by(db_policy::Column::Id); + if let Some(marker) = ¶ms.pagination.marker { + if params.pagination.page_reverse { + cursor.before(marker); + } else { + cursor.after(marker); + } + } + // Over-fetch by one row so the API layer can tell "there is a + // next/previous page" exactly instead of guessing from + // `returned == limit` (ADR 0029). + if let Some(limit) = params.pagination.limit { + if params.pagination.page_reverse { + cursor.last(limit + 1); + } else { + cursor.first(limit + 1); + } + } + Ok(cursor) +} + +/// Lists policies. +/// +/// # Parameters +/// - `db`: The database connection. +/// - `params`: The parameters for listing policies. +/// +/// # Returns +/// A `Result` containing a vector of `Policy`s, or a +/// `PolicyStoreProviderError`. +pub async fn list( + db: &DatabaseConnection, + params: &PolicyListParameters, +) -> Result, PolicyStoreProviderError> { + get_list_query(params)? + .all(db) + .await + .context("fetching policies")? + .into_iter() + .map(TryInto::::try_into) + .collect::>() +} + +#[cfg(test)] +mod tests { + use sea_orm::{DatabaseBackend, MockDatabase, QuerySelect, Transaction, sea_query::*}; + + use super::super::tests::get_policy_mock; + use super::*; + + #[tokio::test] + async fn test_query_all() { + assert_eq!( + r#"SELECT "policy"."id", "policy"."type", "policy"."blob", "policy"."extra" FROM "policy""#, + QuerySelect::query(&mut get_list_query(&PolicyListParameters::default()).unwrap()) + .to_string(PostgresQueryBuilder) + ); + } + + /// `?type=` is an exact-match filter, not a prefix or `LIKE` match. + #[tokio::test] + async fn test_query_type_is_exact_match() { + let sql = QuerySelect::query( + &mut get_list_query(&PolicyListParameters { + r#type: Some("application/json".into()), + ..Default::default() + }) + .unwrap(), + ) + .to_string(PostgresQueryBuilder); + + assert!(sql.contains(r#""policy"."type" = 'application/json'"#)); + assert!(!sql.to_uppercase().contains("LIKE")); + } + + #[tokio::test] + async fn test_list() { + let db = MockDatabase::new(DatabaseBackend::Postgres) + .append_query_results([vec![get_policy_mock("1")]]) + .append_query_results([vec![get_policy_mock("1")]]) + .into_connection(); + + assert!(list(&db, &PolicyListParameters::default()).await.is_ok()); + let filtered = list( + &db, + &PolicyListParameters { + r#type: Some("application/json".into()), + ..Default::default() + }, + ) + .await + .unwrap(); + assert_eq!(filtered.len(), 1); + assert_eq!(filtered[0].id, "1"); + + assert_eq!( + db.into_transaction_log(), + [ + Transaction::from_sql_and_values( + DatabaseBackend::Postgres, + r#"SELECT "policy"."id", "policy"."type", "policy"."blob", "policy"."extra" FROM "policy" ORDER BY "policy"."id" ASC"#, + [] + ), + Transaction::from_sql_and_values( + DatabaseBackend::Postgres, + r#"SELECT "policy"."id", "policy"."type", "policy"."blob", "policy"."extra" FROM "policy" WHERE "policy"."type" = $1 ORDER BY "policy"."id" ASC"#, + ["application/json".into()] + ), + ] + ); + } + + #[tokio::test] + async fn test_list_pagination_over_fetches_and_uses_marker() { + let db = MockDatabase::new(DatabaseBackend::Postgres) + .append_query_results([vec![get_policy_mock("1"), get_policy_mock("2")]]) + .into_connection(); + + let policies = list( + &db, + &PolicyListParameters { + pagination: openstack_keystone_core_types::ListPagination { + limit: Some(1), + marker: Some("0".into()), + page_reverse: false, + }, + ..Default::default() + }, + ) + .await + .unwrap(); + assert_eq!(policies.len(), 2, "backend over-fetched limit+1 rows"); + + let txns = db.into_transaction_log(); + let sql = &txns[0].statements()[0].sql; + assert!(sql.contains(r#""policy"."id" >"#)); + assert!(sql.contains("LIMIT")); + } +} diff --git a/crates/policy-store-driver-sql/src/policy/update.rs b/crates/policy-store-driver-sql/src/policy/update.rs new file mode 100644 index 000000000..4c00256d7 --- /dev/null +++ b/crates/policy-store-driver-sql/src/policy/update.rs @@ -0,0 +1,192 @@ +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 +//! # Update policy + +use sea_orm::DatabaseConnection; +use sea_orm::entity::*; + +use openstack_keystone_core::error::DbContextExt; +use openstack_keystone_core::policy_store::PolicyStoreProviderError; +use openstack_keystone_core_types::policy_store::{Policy, PolicyUpdate}; + +use crate::entity::{policy as db_policy, prelude::Policy as DbPolicy}; + +/// Updates an existing policy. +/// +/// Only the fields set in `policy` are changed; the rest are left as-is. +/// `extra` is **merged** into the stored value rather than replacing it, +/// mirroring python keystone's `update_policy` (which patches the old dict +/// with the request body before writing it back). +/// +/// # Parameters +/// - `db`: The database connection. +/// - `id`: The ID of the policy to update. +/// - `policy`: The fields to change. +/// +/// # Returns +/// A `Result` containing the updated `Policy`, or an `Error` (including +/// `PolicyNotFound` if no policy with that ID exists). +pub async fn update>( + db: &DatabaseConnection, + id: I, + policy: PolicyUpdate, +) -> Result { + let existing = DbPolicy::find_by_id(id.as_ref()) + .one(db) + .await + .context("fetching policy for update")? + .ok_or_else(|| PolicyStoreProviderError::PolicyNotFound(id.as_ref().to_string()))?; + + // Merge the supplied extras over the stored ones (new keys win, omitted + // stored keys survive). + let mut extra = super::decode_extra(existing.extra.as_ref())?; + extra.extend(policy.extra); + + let mut update_model: db_policy::ActiveModel = existing.into(); + + if let Some(r#type) = policy.r#type { + update_model.r#type = Set(r#type); + } + if let Some(blob) = policy.blob { + update_model.blob = Set(serde_json::to_string(&blob)?); + } + update_model.extra = Set(Some(serde_json::to_string(&extra)?)); + + update_model + .update(db) + .await + .context("updating policy")? + .try_into() +} + +#[cfg(test)] +mod tests { + use std::collections::HashMap; + + use sea_orm::{DatabaseBackend, MockDatabase}; + use serde_json::{Value, json}; + + use super::super::tests::get_policy_mock; + use super::*; + + /// PATCHing only `type` must leave `blob` untouched — this is exactly + /// what tempest's `test_create_update_delete_policy` asserts. + #[tokio::test] + async fn test_update_type_only_preserves_blob() { + let db = MockDatabase::new(DatabaseBackend::Postgres) + .append_query_results([vec![get_policy_mock("1")]]) + .append_query_results([vec![db_policy::Model { + id: "1".into(), + r#type: "text/plain".into(), + blob: r#""{'foobar_user': 'role:compute-user'}""#.into(), + extra: Some(r#"{"key":"value"}"#.into()), + }]]) + .into_connection(); + + let updated = update( + &db, + "1", + PolicyUpdate { + r#type: Some("text/plain".into()), + blob: None, + extra: HashMap::new(), + }, + ) + .await + .unwrap(); + + assert_eq!(updated.r#type, "text/plain"); + assert_eq!( + updated.blob, + Value::String("{'foobar_user': 'role:compute-user'}".into()), + "blob must survive a type-only PATCH" + ); + + let txns = db.into_transaction_log(); + let sql = &txns[1].statements()[0].sql; + assert!(sql.contains(r#"UPDATE "policy""#)); + } + + /// Stored extras omitted from the PATCH body survive; supplied keys win. + #[tokio::test] + async fn test_update_merges_extra() { + let db = MockDatabase::new(DatabaseBackend::Postgres) + .append_query_results([vec![db_policy::Model { + id: "1".into(), + r#type: "application/json".into(), + blob: r#""b""#.into(), + extra: Some(r#"{"keep":"me","over":"old"}"#.into()), + }]]) + .append_query_results([vec![db_policy::Model { + id: "1".into(), + r#type: "application/json".into(), + blob: r#""b""#.into(), + extra: Some(r#"{"keep":"me","over":"new","add":"x"}"#.into()), + }]]) + .into_connection(); + + update( + &db, + "1", + PolicyUpdate { + r#type: None, + blob: None, + extra: HashMap::from([("over".into(), json!("new")), ("add".into(), json!("x"))]), + }, + ) + .await + .unwrap(); + + // Pull the bound `extra` parameter out of the UPDATE as a typed + // `sea_orm::Value`. Rendering the values with `to_string()` and then + // splitting on single quotes would break on any extra value that + // itself contains a quote, and asserts on sea-orm's `Display` impl + // rather than on the bytes this driver binds. + let txns = db.into_transaction_log(); + let stmt = &txns[1].statements()[0]; + let values = stmt + .values + .as_ref() + .expect("the UPDATE statement binds parameters"); + let extra_json = values + .0 + .iter() + .find_map(|v| match v { + sea_orm::Value::String(Some(s)) if s.starts_with('{') => Some(s.as_str()), + _ => None, + }) + .expect("serialized extra is bound as a JSON object string"); + let extra: HashMap = serde_json::from_str(extra_json).unwrap(); + + assert_eq!( + extra.get("keep"), + Some(&json!("me")), + "omitted key survives" + ); + assert_eq!(extra.get("over"), Some(&json!("new")), "supplied key wins"); + assert_eq!(extra.get("add"), Some(&json!("x")), "new key added"); + } + + #[tokio::test] + async fn test_update_missing_is_not_found() { + let db = MockDatabase::new(DatabaseBackend::Postgres) + .append_query_results([Vec::::new()]) + .into_connection(); + + assert!(matches!( + update(&db, "missing", PolicyUpdate::default()).await, + Err(PolicyStoreProviderError::PolicyNotFound(_)) + )); + } +} diff --git a/doc/src/SUMMARY.md b/doc/src/SUMMARY.md index 56207d065..063592e66 100644 --- a/doc/src/SUMMARY.md +++ b/doc/src/SUMMARY.md @@ -84,6 +84,7 @@ - [Local development](contributor/development.md) - [Testing](contributor/testing.md) - [API development](contributor/api-development.md) + - [Python API compatibility](contributor/python-compatibility.md) - [Authentication plugin development](contributor/auth-plugins.md) - [Security model](contributor/security-model.md) - [Security architecture review](contributor/security-review.md) diff --git a/doc/src/contributor/python-compatibility.md b/doc/src/contributor/python-compatibility.md new file mode 100644 index 000000000..923069d02 --- /dev/null +++ b/doc/src/contributor/python-compatibility.md @@ -0,0 +1,125 @@ +# Python API compatibility + +Tracks where this implementation's HTTP surface differs from python Keystone's. +Started as the documentation deliverable requested by +[#938](https://github.com/openstack-experimental/keystone/issues/938); it is +seeded from that issue's body table and grows as rows are closed. #938's table +is itself stale in places, so rows here are checked against the code and the +[Identity API v3 reference](https://docs.openstack.org/api-ref/identity/v3/) +rather than copied verbatim. + +Verify with the tempest identity suite: + +```console +tools/run-tempest-local.sh # full suite +TEMPEST_REGEX='tempest\.api\.identity\.admin\.v3\.test_policies' \ + tools/run-tempest-local.sh # one module +``` + +Claims in this document are backed by a measured before/after pair on the same +revision and host, not by a remembered score. The most recent run, against +`8fc78201`: + +| | Baseline | With the policy API | +| --- | --- | --- | +| Ran | 133 | 133 | +| Passed | 61 | **63** | +| Failed | 68 | **66** | +| Skipped | 4 | 4 | + +Diffing the two failing-test lists shows exactly two fixed — +`test_create_update_delete_policy` and `test_list_policies` — and **zero** +regressions. + +## Endpoint coverage + +| Feature | Endpoints | Status | +| --- | --- | --- | +| Service catalog | `/v3/services` (CRUD) | Done | +| Endpoints | `/v3/endpoints` (CRUD) | Done — legacy `region` attribute auto-vivifies a Region and is mirrored back | +| Regions | `/v3/regions` (CRUD, plus `PUT /v3/regions/{id}`) | Done (#1078) | +| Policies (legacy blob store) | `/v3/policies` (CRUD) | Done (#1035) — tempest verified, see below | +| Limits | `/v3/limits`, `/v3/limits-model` | Missing | +| Project tags | `/v3/projects/{project_id}/tags` | Missing | +| Application credentials (v3.10) | `/v3/users/{user_id}/application_credentials` | Missing | +| Access rules (v3.13) | `/v3/users/{user_id}/access_rules` | Missing | +| System role assignments | `/v3/system/users/{user_id}/roles`, `.../roles/{role_id}` | Partial — user grants (`GET`/`HEAD`/`PUT`/`DELETE`) implemented; the `/v3/system/groups/{group_id}/roles*` variants are missing | +| Project hierarchy | `/v3/projects/{project_id}/parents`, `.../subtree` | Missing | +| OS-INHERIT role inference list | `/v3/role_inferences` | Done | +| OS-TRUST | `/v3/OS-TRUST/trusts` | Partial — create/get/list/delete; no `PATCH` (python has none either). Missing `/{id}/roles` sub-resource and response `links.self` (#1082) | +| OAuth2 | `/v3/OS-OAUTH2/*` | Missing | +| OS-ENDPOINT-POLICY | `/v3/policies/{id}/OS-ENDPOINT-POLICY/*`, `/v3/endpoints/{id}/OS-ENDPOINT-POLICY/policy` | Missing — needs the `policy_association` table; deliberately out of scope for #1035, follow-up issue to be filed | + +## Cross-cutting behaviours + +- **Malformed JSON bodies** answer `400` with an `{"error": ...}` document. + Axum's `Json` extractor natively answers a type-mismatch body with `422 + text/plain`; `crates/keystone/src/api/json_rejection.rs` normalizes it. +- **Collection pagination** follows [ADR 0029](../adr/0029-pagination.md): + `?limit=`/`?marker=` with `links`. Python Keystone instead truncates to + `[DEFAULT] list_limit` and sets a `truncated` flag on the collection. This is + a deliberate, uniform deviation across every collection endpoint here, not a + per-resource gap. +- **Per-member `links.self`** is not emitted on any resource yet (#1082). + +## Policies (`/v3/policies`) + +Storage for opaque policy documents that *remote* services fetch and interpret. +Deprecated upstream ("Keystone is not a policy management service") and +unrelated to this service's own OPA-based authorization — see +`openstack_keystone_core::policy_store`. + +Implemented verbs, matching python Keystone's `PolicyResource` exactly: + +| Verb | Path | Success | +| --- | --- | --- | +| `POST` | `/v3/policies` | 201 | +| `GET` | `/v3/policies` (`?type=` exact match) | 200 | +| `GET` | `/v3/policies/{policy_id}` | 200 | +| `PATCH` | `/v3/policies/{policy_id}` | 200 | +| `DELETE` | `/v3/policies/{policy_id}` | 204 | + +There is deliberately **no `PUT /v3/policies/{policy_id}`** — python Keystone +does not define one, so it answers `405`. (Contrast `/v3/regions/{id}`, which +*does* have an upstream `PUT`.) + +Request/response details carried over from upstream: + +- `blob` is accepted as a **string only** on `POST`/`PATCH`, per + `keystone/policy/schema.py`. An object-valued `blob` is a `400`. Responses + carry whatever JSON value is stored, so rows written by older python releases + (object blobs) still read back correctly. +- The stored column is JSON-encoded exactly as python's `JsonBlob` type + decorator does. Round-tripping is **JSON-semantic**, not byte-identical: + whitespace, string escaping, and object-key ordering may differ between + `json.dumps` and `serde_json`. +- `id` is always server-assigned on create; a caller-supplied `id` is + discarded (`_assign_unique_id`). On `PATCH` an `id` is accepted only when it + equals the path segment, else `400` ("Cannot change policy ID"), and is never + persisted. +- An empty `{"policy": {}}` patch document is a `400` (`minProperties: 1`). +- Unknown properties round-trip through the `extra` column. On **create** their + names are normalized (`:` and `-` become `_`), matching upstream's + `_normalize_dict`; `PATCH` does not normalize, also matching upstream. +- `PATCH` **merges** `extra` into the stored properties (supplied keys win, + omitted stored keys survive), matching upstream's `update_policy`. Note this + differs from e.g. `RegionUpdate.extra`, which this codebase overwrites + wholesale. + +Authorization mirrors `keystone/common/policies/policy.py`: +`list`/`show` are admin-or-system-reader, `create`/`update`/`delete` are +admin-only. + +## Follow-ups + +Known gaps deliberately left out of #1035. **These issues still need to be +filed** — no issue numbers exist yet, so nothing here links to one: + +- **`OS-ENDPOINT-POLICY`** — `/v3/policies/{id}/OS-ENDPOINT-POLICY/*` and + `/v3/endpoints/{id}/OS-ENDPOINT-POLICY/policy`, backed by the + `policy_association` table. Tempest's `test_policies` does not exercise it. +- **`input.credentials.system_scope` audit** — several policies compare + against a key `Credentials` never emits. The affected rules are not uniform + and `region`'s python default is *broader* than the rego, so this needs an + endpoint-by-endpoint audit rather than a rename. See + [security model](security-model.md) §9 for the per-rule breakdown. diff --git a/doc/src/contributor/security-model.md b/doc/src/contributor/security-model.md index 489343fc1..c42365eda 100644 --- a/doc/src/contributor/security-model.md +++ b/doc/src/contributor/security-model.md @@ -260,13 +260,24 @@ shipping it leaks EC2 secret keys / TOTP seeds into OPA decision logs. **Where:** `credential_policy_input()` in `crates/keystone/src/api/v3/credential/mod.rs`, used by all credential handlers. +A stronger variant exists for `/v3/policies`, whose stored document (`blob`) +*and* open-ended `extra` map are both arbitrary caller-supplied data: the +`to_policy_input()` methods in `crates/api-types/src/v3/policy.rs` **construct** +the policy input from an allowlist (`id`, `type`) instead of removing known-bad +keys. Prefer that shape for any resource with an open key space — a denylist +cannot cover `extra`. + ### I8 — List re-checks every item individually **What:** list endpoints run the collection-level policy first, then re-enforce the per-item `show` policy against **each record's own** identifiers, dropping -unreadable rows. **Why:** a permissive list filter must not leak individual -objects the caller cannot read (CVE-2019-19687). **Where:** -`crates/keystone/src/api/v3/credential/list.rs`. +unreadable rows. A policy failure that is *not* `Forbidden` (OPA unreachable, +malformed decision) must be propagated, not treated as "drop this item" — +otherwise an infrastructure fault silently returns a truncated collection. +**Why:** a permissive list filter must not leak individual objects the caller +cannot read (CVE-2019-19687). **Where:** +`crates/keystone/src/api/v3/credential/list.rs`, +`crates/keystone/src/api/v3/policy/list.rs`. ## 5. Delegated-auth specifics @@ -392,3 +403,29 @@ carries it, ticked by the reviewer, not just this prose reference. (security review V5): creation warns unconditionally on a non-empty `access_rules` list, and `application_credential.reject_unenforced_access_rules` (default `false`) can be set to fail loud instead. +- **`input.credentials.system_scope` is a dead key in several policies.** + `Credentials.system` (`crates/core/src/policy.rs`) serializes the system + scope under the field name `system`; nothing ever emits `system_scope`, so + any rule comparing against it is unreachable. It fails *closed* (access is + denied, never granted), so this is a functional gap rather than a + vulnerability. `policy/policy/*.rego` uses the correct + `input.credentials.system`. + + The affected rules are **not** uniform, and a global key rename would be + wrong. Each needs checking against the python default it is meant to mirror: + + | Rego | Python default | Effect of the dead key | + | --- | --- | --- | + | `assignment/list` | `RULE_ADMIN_OR_SYSTEM_READER` | Real: system reader is denied | + | `auth/token/show` | admin or token owner, plus system reader | Real: system reader is denied | + | `endpoint/{list,show}` | `RULE_ADMIN_OR_SYSTEM_READER` | Real: system reader is denied | + | `service/{list,show}` | `RULE_ADMIN_OR_SYSTEM_READER` | Real: system reader is denied | + | `auth/project/list` | any valid token | None — the rego is `default allow := true`, so nothing is denied | + | `auth/token/revoke` | admin or token owner (no system reader) | None — the `system_scope` lines are commented out | + | `region/{list,show}` | `check_str=''` (any valid token) | Different bug: the rego is `default allow := false`, so it is *stricter* than python regardless of the key; renaming would not fix it | + + Needs a follow-up issue for an endpoint-by-endpoint audit against the + [Identity API policy reference](https://docs.openstack.org/keystone/latest/configuration/policy.html), + not a blanket search-and-replace. This class of bug is exactly what the Gate + B3 real-`opa run` handler tests catch and hand-written Rego unit tests + cannot. diff --git a/policy/policy/create.rego b/policy/policy/create.rego new file mode 100644 index 000000000..2c5106cbf --- /dev/null +++ b/policy/policy/create.rego @@ -0,0 +1,34 @@ +# METADATA +# title: Create policy +# description: Policy for creating a legacy policy document +package identity.policy.create + +import data.identity + +# Create policy (legacy `POST /v3/policies`). +# +# The `input.target.policy` is the new policy, projected onto the +# non-sensitive allowlist: +# id: null Always null; the server assigns the ID. +# type: string The MIME media type of the serialized blob. +# +# The document `blob` and any additional properties are deliberately NOT part +# of the input: they are arbitrary caller-supplied data that may contain +# secrets, and no rule here reads them. +# +# The `input.existing` is null +# +# Mirrors python keystone's `identity:create_policy` (RULE_ADMIN_REQUIRED): +# admin only, no system-reader path. +# +default allow := false + +# METADATA +# description: "`Admin` is allowed by default" +allow if { + "admin" in input.credentials.roles +} + +allow if { + input.credentials.is_admin +} diff --git a/policy/policy/create_test.rego b/policy/policy/create_test.rego new file mode 100644 index 000000000..d8505d610 --- /dev/null +++ b/policy/policy/create_test.rego @@ -0,0 +1,24 @@ +package test_policy_create + +import data.identity.policy.create + +test_allowed if { + create.allow with input as {"credentials": {"roles": ["admin"]}} + create.allow with input as {"credentials": {"is_admin": true}} +} + +# Admin-only: unlike show/list there is no system-reader path +# (python keystone uses RULE_ADMIN_REQUIRED here). +test_forbidden if { + not create.allow with input as {"credentials": {"roles": []}} + not create.allow with input as {"credentials": {"roles": ["reader"], "system": "all"}} + not create.allow with input as {"credentials": {"roles": ["member"]}} + not create.allow with input as {"credentials": {"roles": ["manager"]}} +} + +test_target_content_does_not_grant if { + not create.allow with input as { + "credentials": {"roles": ["member"]}, + "target": {"policy": {"id": null, "type": "application/json"}}, + } +} diff --git a/policy/policy/delete.rego b/policy/policy/delete.rego new file mode 100644 index 000000000..0dcf324ca --- /dev/null +++ b/policy/policy/delete.rego @@ -0,0 +1,33 @@ +# METADATA +# title: Delete policy +# description: Policy for deleting a legacy policy document +package identity.policy.delete + +import data.identity + +# Delete policy (legacy `DELETE /v3/policies/{policy_id}`). +# +# The `input.existing.policy` is the stored policy, projected onto the +# non-sensitive allowlist: +# id: string Policy ID. +# type: string The MIME media type of the serialized blob. +# +# The document `blob` and any additional properties are deliberately NOT part +# of the input. +# +# The `input.target` is null +# +# Mirrors python keystone's `identity:delete_policy` (RULE_ADMIN_REQUIRED): +# admin only, no system-reader path. +# +default allow := false + +# METADATA +# description: "`Admin` is allowed by default" +allow if { + "admin" in input.credentials.roles +} + +allow if { + input.credentials.is_admin +} diff --git a/policy/policy/delete_test.rego b/policy/policy/delete_test.rego new file mode 100644 index 000000000..770702163 --- /dev/null +++ b/policy/policy/delete_test.rego @@ -0,0 +1,22 @@ +package test_policy_delete + +import data.identity.policy.delete + +test_allowed if { + delete.allow with input as {"credentials": {"roles": ["admin"]}} + delete.allow with input as {"credentials": {"is_admin": true}} +} + +test_forbidden if { + not delete.allow with input as {"credentials": {"roles": []}} + not delete.allow with input as {"credentials": {"roles": ["reader"], "system": "all"}} + not delete.allow with input as {"credentials": {"roles": ["member"]}} + not delete.allow with input as {"credentials": {"roles": ["manager"]}} +} + +test_existing_content_does_not_grant if { + not delete.allow with input as { + "credentials": {"roles": ["member"]}, + "existing": {"policy": {"id": "pid", "type": "application/json"}}, + } +} diff --git a/policy/policy/list.rego b/policy/policy/list.rego new file mode 100644 index 000000000..4826245ff --- /dev/null +++ b/policy/policy/list.rego @@ -0,0 +1,53 @@ +# METADATA +# title: List policies +# description: Policy for listing legacy policy documents +package identity.policy.list + +import data.identity + +# List policies (legacy `GET /v3/policies`). +# +# NOTE: this governs the deprecated `/v3/policies` *document store*, not this +# service's own authorization. A stored "policy" is opaque data a remote +# service fetches; Keystone never evaluates it. +# +# The `input.target.policy` contains the query parameters, projected onto the +# non-sensitive allowlist (see `crates/api-types/src/v3/policy.rs`): +# id: null Always null when listing. +# type: string or null (optional) Exact-match filter on the blob media type. +# +# The document `blob` and any additional properties are deliberately NOT part +# of the input: they are arbitrary caller-supplied data no rule here reads. +# +# The `input.existing` is null +# +# Mirrors python keystone's `identity:list_policies` +# (RULE_ADMIN_OR_SYSTEM_READER). +# +# NOTE on the credentials key: the system scope arrives as +# `input.credentials.system` (`Credentials.system` in +# `crates/core/src/policy.rs`, serialized under its own field name). Several +# older policies in this tree compare against `input.credentials.system_scope`, +# a key `Credentials` never emits, which makes their system-reader branches +# unreachable against real input — their Rego unit tests only pass because they +# hand-write that key. Do not copy that spelling here. +# +default allow := false + +# METADATA +# description: "`Admin` is allowed by default" +allow if { + "admin" in input.credentials.roles +} + +allow if { + input.credentials.is_admin +} + +# METADATA +# description: "'reader' in the system scope can list any policies." +allow if { + "reader" in input.credentials.roles + input.credentials.system != null + "all" == input.credentials.system +} diff --git a/policy/policy/list_test.rego b/policy/policy/list_test.rego new file mode 100644 index 000000000..ca6b98db2 --- /dev/null +++ b/policy/policy/list_test.rego @@ -0,0 +1,26 @@ +package test_policy_list + +import data.identity.policy.list + +test_allowed if { + list.allow with input as {"credentials": {"roles": ["admin"]}} + list.allow with input as {"credentials": {"is_admin": true}} + list.allow with input as {"credentials": {"roles": ["reader"], "system": "all"}} +} + +test_forbidden if { + not list.allow with input as {"credentials": {"roles": []}} + not list.allow with input as {"credentials": {"roles": ["reader"]}} + not list.allow with input as {"credentials": {"roles": ["reader"], "system": "domain"}} + not list.allow with input as {"credentials": {"roles": ["member"]}} + not list.allow with input as {"credentials": {"roles": ["manager"]}} +} + +# The decision must not depend on the target document at all: a caller who is +# not admin/system-reader stays denied whatever the query carries. +test_target_content_does_not_grant if { + not list.allow with input as { + "credentials": {"roles": ["member"]}, + "target": {"policy": {"id": null, "type": "application/json"}}, + } +} diff --git a/policy/policy/show.rego b/policy/policy/show.rego new file mode 100644 index 000000000..e8649bc5a --- /dev/null +++ b/policy/policy/show.rego @@ -0,0 +1,52 @@ +# METADATA +# title: Show policy +# description: Policy for fetching a single legacy policy document +package identity.policy.show + +import data.identity + +# Show policy (legacy `GET /v3/policies/{policy_id}`). +# +# Also re-checked per item by the list handler (security model I8), so a +# caller only ever sees collection members this rule admits. +# +# The `input.existing.policy` is the stored policy, projected onto the +# non-sensitive allowlist: +# id: string Policy ID. +# type: string The MIME media type of the serialized blob. +# +# The document `blob` and any additional properties are deliberately NOT part +# of the input. +# +# The `input.target` is null +# +# Mirrors python keystone's `identity:get_policy` +# (RULE_ADMIN_OR_SYSTEM_READER). +# +# NOTE on the credentials key: the system scope arrives as +# `input.credentials.system` (`Credentials.system` in +# `crates/core/src/policy.rs`, serialized under its own field name). Several +# older policies in this tree compare against `input.credentials.system_scope`, +# a key `Credentials` never emits, which makes their system-reader branches +# unreachable against real input — their Rego unit tests only pass because they +# hand-write that key. Do not copy that spelling here. +# +default allow := false + +# METADATA +# description: "`Admin` is allowed by default" +allow if { + "admin" in input.credentials.roles +} + +allow if { + input.credentials.is_admin +} + +# METADATA +# description: "'reader' in the system scope can show any policy." +allow if { + "reader" in input.credentials.roles + input.credentials.system != null + "all" == input.credentials.system +} diff --git a/policy/policy/show_test.rego b/policy/policy/show_test.rego new file mode 100644 index 000000000..44236b053 --- /dev/null +++ b/policy/policy/show_test.rego @@ -0,0 +1,25 @@ +package test_policy_show + +import data.identity.policy.show + +test_allowed if { + show.allow with input as {"credentials": {"roles": ["admin"]}} + show.allow with input as {"credentials": {"is_admin": true}} + show.allow with input as {"credentials": {"roles": ["reader"], "system": "all"}} +} + +test_forbidden if { + not show.allow with input as {"credentials": {"roles": []}} + not show.allow with input as {"credentials": {"roles": ["reader"]}} + not show.allow with input as {"credentials": {"roles": ["reader"], "system": "domain"}} + not show.allow with input as {"credentials": {"roles": ["member"]}} +} + +# Used per-item by the list handler (security model I8): a member must not be +# able to read a stored policy just because it exists. +test_existing_content_does_not_grant if { + not show.allow with input as { + "credentials": {"roles": ["member"]}, + "existing": {"policy": {"id": "pid", "type": "application/json"}}, + } +} diff --git a/policy/policy/update.rego b/policy/policy/update.rego new file mode 100644 index 000000000..44bbb8732 --- /dev/null +++ b/policy/policy/update.rego @@ -0,0 +1,34 @@ +# METADATA +# title: Update policy +# description: Policy for updating a legacy policy document +package identity.policy.update + +import data.identity + +# Update policy (legacy `PATCH /v3/policies/{policy_id}`). +# +# The `input.target.policy` contains the requested changes, projected onto the +# non-sensitive allowlist: +# id: string or null (optional) Only accepted when equal to the path ID. +# type: string or null (optional) New MIME media type. +# +# The document `blob` and any additional properties are deliberately NOT part +# of the input. +# +# The `input.existing.policy` is the stored policy under the same projection, +# or null if it does not exist. +# +# Mirrors python keystone's `identity:update_policy` (RULE_ADMIN_REQUIRED): +# admin only, no system-reader path. +# +default allow := false + +# METADATA +# description: "`Admin` is allowed by default" +allow if { + "admin" in input.credentials.roles +} + +allow if { + input.credentials.is_admin +} diff --git a/policy/policy/update_test.rego b/policy/policy/update_test.rego new file mode 100644 index 000000000..3a58808c0 --- /dev/null +++ b/policy/policy/update_test.rego @@ -0,0 +1,23 @@ +package test_policy_update + +import data.identity.policy.update + +test_allowed if { + update.allow with input as {"credentials": {"roles": ["admin"]}} + update.allow with input as {"credentials": {"is_admin": true}} +} + +test_forbidden if { + not update.allow with input as {"credentials": {"roles": []}} + not update.allow with input as {"credentials": {"roles": ["reader"], "system": "all"}} + not update.allow with input as {"credentials": {"roles": ["member"]}} + not update.allow with input as {"credentials": {"roles": ["manager"]}} +} + +test_target_content_does_not_grant if { + not update.allow with input as { + "credentials": {"roles": ["member"]}, + "target": {"policy": {"id": "pid", "type": "text/plain"}}, + "existing": {"policy": {"id": "pid", "type": "application/json"}}, + } +} diff --git a/tests/api/src/lib.rs b/tests/api/src/lib.rs index ab62766c7..7795f9965 100644 --- a/tests/api/src/lib.rs +++ b/tests/api/src/lib.rs @@ -24,6 +24,7 @@ pub mod identity; pub mod macros; pub mod mapping; pub mod oauth2; +pub mod policy; pub mod region; pub mod resource; pub mod role; diff --git a/tests/api/src/policy.rs b/tests/api/src/policy.rs new file mode 100644 index 000000000..066d45319 --- /dev/null +++ b/tests/api/src/policy.rs @@ -0,0 +1,246 @@ +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 +//! Legacy policy document store (`/v3/policies`) test client. +use std::borrow::Cow; +use std::sync::Arc; + +use eyre::Result; + +use openstack_keystone_api_types::v3::policy::*; +use openstack_sdk::api::rest_endpoint_prelude::*; +use openstack_sdk::{AsyncOpenStack, api::QueryAsync}; + +use crate::guard::*; + +#[derive(Clone, Debug)] +struct PolicyCreateRequestInternal { + policy: PolicyCreate, +} + +impl RestEndpoint for PolicyCreateRequestInternal { + fn method(&self) -> http::Method { + http::Method::POST + } + + fn endpoint(&self) -> Cow<'static, str> { + "policies".to_string().into() + } + + fn body(&self) -> Result)>, BodyError> { + let mut params = JsonBodyParams::default(); + params.push("policy", serde_json::to_value(&self.policy)?); + params.into_body() + } + + fn service_type(&self) -> ServiceType { + ServiceType::Identity + } + + fn response_key(&self) -> Option> { + Some("policy".into()) + } + + fn api_version(&self) -> Option { + Some(ApiVersion::new(3, 0)) + } +} + +#[derive(Clone, Debug)] +struct PolicyShowRequest<'a> { + id: Cow<'a, str>, +} + +impl RestEndpoint for PolicyShowRequest<'_> { + fn method(&self) -> http::Method { + http::Method::GET + } + + fn endpoint(&self) -> Cow<'static, str> { + format!("policies/{id}", id = self.id).into() + } + + fn service_type(&self) -> ServiceType { + ServiceType::Identity + } + + fn response_key(&self) -> Option> { + Some("policy".into()) + } + + fn api_version(&self) -> Option { + Some(ApiVersion::new(3, 0)) + } +} + +#[derive(Default, Clone, Debug)] +struct PolicyListRequest { + r#type: Option, +} + +impl RestEndpoint for PolicyListRequest { + fn method(&self) -> http::Method { + http::Method::GET + } + + fn endpoint(&self) -> Cow<'static, str> { + "policies".into() + } + + fn parameters(&self) -> QueryParams<'_> { + let mut params = QueryParams::default(); + params.push_opt("type", self.r#type.as_ref()); + params + } + + fn response_key(&self) -> Option> { + Some("policies".into()) + } + + fn service_type(&self) -> ServiceType { + ServiceType::Identity + } + + fn api_version(&self) -> Option { + Some(ApiVersion::new(3, 0)) + } +} + +#[derive(Clone, Debug)] +struct PolicyUpdateRequestInternal<'a> { + id: Cow<'a, str>, + policy: PolicyUpdate, +} + +impl RestEndpoint for PolicyUpdateRequestInternal<'_> { + fn method(&self) -> http::Method { + http::Method::PATCH + } + + fn endpoint(&self) -> Cow<'static, str> { + format!("policies/{id}", id = self.id).into() + } + + fn body(&self) -> Result)>, BodyError> { + let mut params = JsonBodyParams::default(); + params.push("policy", serde_json::to_value(&self.policy)?); + params.into_body() + } + + fn service_type(&self) -> ServiceType { + ServiceType::Identity + } + + fn response_key(&self) -> Option> { + Some("policy".into()) + } + + fn api_version(&self) -> Option { + Some(ApiVersion::new(3, 0)) + } +} + +struct PolicyDeleteRequest<'a> { + id: Cow<'a, str>, +} + +impl RestEndpoint for PolicyDeleteRequest<'_> { + fn method(&self) -> http::Method { + http::Method::DELETE + } + + fn endpoint(&self) -> Cow<'static, str> { + format!("policies/{id}", id = self.id).into() + } + + fn service_type(&self) -> ServiceType { + ServiceType::Identity + } + + fn api_version(&self) -> Option { + Some(ApiVersion::new(3, 0)) + } +} + +/// Create a policy. +pub async fn create_policy( + tc: &Arc, + policy: PolicyCreate, +) -> Result> { + let obj: Policy = PolicyCreateRequestInternal { policy } + .query_async(tc.as_ref()) + .await?; + Ok(AsyncResourceGuard::new(obj, tc.clone())) +} + +/// Get a policy by ID. +pub async fn show_policy>(tc: &Arc, id: I) -> Result { + Ok(PolicyShowRequest { + id: id.as_ref().into(), + } + .query_async(tc.as_ref()) + .await?) +} + +/// List policies. +pub async fn list_policies(tc: &Arc) -> Result> { + Ok(PolicyListRequest::default() + .query_async(tc.as_ref()) + .await?) +} + +/// List policies filtered by blob media type. +pub async fn list_policies_by_type>( + tc: &Arc, + r#type: I, +) -> Result> { + Ok(PolicyListRequest { + r#type: Some(r#type.into()), + } + .query_async(tc.as_ref()) + .await?) +} + +/// Update a policy. +pub async fn update_policy>( + tc: &Arc, + id: I, + policy: PolicyUpdate, +) -> Result { + Ok(PolicyUpdateRequestInternal { + id: id.as_ref().into(), + policy, + } + .query_async(tc.as_ref()) + .await?) +} + +/// Delete a policy. +pub async fn delete_policy>(tc: &Arc, id: I) -> Result<()> { + Ok(openstack_sdk::api::ignore(PolicyDeleteRequest { + id: id.as_ref().into(), + }) + .query_async(tc.as_ref()) + .await?) +} + +#[async_trait::async_trait] +impl DeletableResource for Policy { + async fn delete(&self, state: &Arc) -> Result<()> { + Ok(openstack_sdk::api::ignore(PolicyDeleteRequest { + id: self.id.clone().into(), + }) + .query_async(state.as_ref()) + .await?) + } +} diff --git a/tests/api/tests/api_v3.rs b/tests/api/tests/api_v3.rs index ef0aefc2e..5b00036ca 100644 --- a/tests/api/tests/api_v3.rs +++ b/tests/api/tests/api_v3.rs @@ -22,6 +22,7 @@ mod api_v3 { mod credential; mod endpoint; mod identity; + mod policy; mod region; mod resource; mod role; diff --git a/tests/api/tests/api_v3/policy.rs b/tests/api/tests/api_v3/policy.rs new file mode 100644 index 000000000..c4b957a8d --- /dev/null +++ b/tests/api/tests/api_v3/policy.rs @@ -0,0 +1,18 @@ +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 +mod create; +mod delete; +mod list; +mod show; +mod update; diff --git a/tests/api/tests/api_v3/policy/create.rs b/tests/api/tests/api_v3/policy/create.rs new file mode 100644 index 000000000..a15f87e86 --- /dev/null +++ b/tests/api/tests/api_v3/policy/create.rs @@ -0,0 +1,102 @@ +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 + +use std::collections::HashMap; +use std::sync::Arc; + +use eyre::Result; +use tracing_test::traced_test; +use uuid::Uuid; + +use openstack_keystone_api_types::v3::policy::*; +use openstack_sdk::{AsyncOpenStack, config::CloudConfig}; + +use test_api::guard::ResourceGuard; +use test_api::policy::create_policy; + +/// The blob goes in as a string and comes back as the same string — the +/// assertion tempest's `test_create_update_delete_policy` makes. +#[tokio::test] +#[traced_test] +async fn test_create_policy() -> Result<()> { + let tc = Arc::new(AsyncOpenStack::new(&CloudConfig::from_env()?).await?); + let blob = format!("BlobName-{}", Uuid::new_v4().simple()); + let policy_type = format!("PolicyType-{}", Uuid::new_v4().simple()); + + let guard = create_policy( + &tc, + PolicyCreateBuilder::default() + .blob(blob.clone()) + .r#type(policy_type.clone()) + .build()?, + ) + .await?; + + assert!(!guard.id.is_empty(), "the server must assign an ID"); + assert_eq!(guard.blob, serde_json::json!(blob)); + assert_eq!(guard.r#type, policy_type); + + guard.delete().await?; + Ok(()) +} + +/// A caller-supplied `id` is discarded (python keystone's +/// `_assign_unique_id`) and must not resurface as an extra property. +#[tokio::test] +#[traced_test] +async fn test_create_policy_ignores_caller_supplied_id() -> Result<()> { + let tc = Arc::new(AsyncOpenStack::new(&CloudConfig::from_env()?).await?); + let requested = format!("caller{}", Uuid::new_v4().simple()); + + let guard = create_policy( + &tc, + PolicyCreateBuilder::default() + .blob("blob") + .r#type("application/json") + .id(requested.clone()) + .build()?, + ) + .await?; + + assert_ne!(guard.id, requested, "caller-supplied id must be replaced"); + assert!(!guard.extra.contains_key("id")); + + guard.delete().await?; + Ok(()) +} + +/// Unknown properties round-trip through `extra`. +#[tokio::test] +#[traced_test] +async fn test_create_policy_round_trips_extra() -> Result<()> { + let tc = Arc::new(AsyncOpenStack::new(&CloudConfig::from_env()?).await?); + + let guard = create_policy( + &tc, + PolicyCreateBuilder::default() + .blob("blob") + .r#type("application/json") + .extra(HashMap::from([( + "custom".to_string(), + serde_json::json!("value"), + )])) + .build()?, + ) + .await?; + + assert_eq!(guard.extra.get("custom"), Some(&serde_json::json!("value"))); + + guard.delete().await?; + Ok(()) +} diff --git a/tests/api/tests/api_v3/policy/delete.rs b/tests/api/tests/api_v3/policy/delete.rs new file mode 100644 index 000000000..b03491830 --- /dev/null +++ b/tests/api/tests/api_v3/policy/delete.rs @@ -0,0 +1,63 @@ +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 + +use std::sync::Arc; + +use eyre::Result; +use tracing_test::traced_test; +use uuid::Uuid; + +use openstack_keystone_api_types::v3::policy::*; +use openstack_sdk::{AsyncOpenStack, config::CloudConfig}; + +use test_api::guard::ResourceGuard; +use test_api::policy::{create_policy, delete_policy, show_policy}; + +#[tokio::test] +#[traced_test] +async fn test_delete_policy() -> Result<()> { + let tc = Arc::new(AsyncOpenStack::new(&CloudConfig::from_env()?).await?); + + let guard = create_policy( + &tc, + PolicyCreateBuilder::default() + .blob("blob") + .r#type("application/json") + .build()?, + ) + .await?; + let id = guard.id.clone(); + + // The guard's consuming `delete()` *is* the call under test, so no second + // DELETE is issued and the leak detection stays satisfied. + guard.delete().await?; + + assert!( + show_policy(&tc, &id).await.is_err(), + "a deleted policy must no longer be readable" + ); + Ok(()) +} + +#[tokio::test] +#[traced_test] +async fn test_delete_missing_policy_is_404() -> Result<()> { + let tc = Arc::new(AsyncOpenStack::new(&CloudConfig::from_env()?).await?); + assert!( + delete_policy(&tc, format!("missing{}", Uuid::new_v4().simple())) + .await + .is_err() + ); + Ok(()) +} diff --git a/tests/api/tests/api_v3/policy/list.rs b/tests/api/tests/api_v3/policy/list.rs new file mode 100644 index 000000000..a2daa9cbb --- /dev/null +++ b/tests/api/tests/api_v3/policy/list.rs @@ -0,0 +1,105 @@ +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 + +use std::sync::Arc; + +use eyre::Result; +use tracing_test::traced_test; +use uuid::Uuid; + +use openstack_keystone_api_types::v3::policy::*; +use openstack_sdk::{AsyncOpenStack, config::CloudConfig}; + +use test_api::guard::ResourceGuard; +use test_api::policy::{create_policy, list_policies, list_policies_by_type}; + +/// Mirrors tempest's `test_list_policies`: several created policies must all +/// appear in the unfiltered collection. +#[tokio::test] +#[traced_test] +async fn test_list_includes_created_policies() -> Result<()> { + let tc = Arc::new(AsyncOpenStack::new(&CloudConfig::from_env()?).await?); + + let mut guards = Vec::new(); + for _ in 0..3 { + guards.push( + create_policy( + &tc, + PolicyCreateBuilder::default() + .blob(format!("BlobName-{}", Uuid::new_v4().simple())) + .r#type(format!("PolicyType-{}", Uuid::new_v4().simple())) + .build()?, + ) + .await?, + ); + } + + let all = list_policies(&tc).await?; + for guard in &guards { + assert!( + all.iter().any(|p| p.id == guard.id), + "created policy {} must appear in the unfiltered list", + guard.id + ); + } + + for guard in guards { + guard.delete().await?; + } + Ok(()) +} + +/// `?type=` is an exact-match filter. +#[tokio::test] +#[traced_test] +async fn test_list_filters_by_type() -> Result<()> { + let tc = Arc::new(AsyncOpenStack::new(&CloudConfig::from_env()?).await?); + let unique_type = format!("application/{}", Uuid::new_v4().simple()); + + let wanted = create_policy( + &tc, + PolicyCreateBuilder::default() + .blob("wanted") + .r#type(unique_type.clone()) + .build()?, + ) + .await?; + let other = create_policy( + &tc, + PolicyCreateBuilder::default() + .blob("other") + .r#type("text/plain") + .build()?, + ) + .await?; + + let filtered = list_policies_by_type(&tc, unique_type.clone()).await?; + assert!(filtered.iter().any(|p| p.id == wanted.id)); + assert!( + filtered.iter().all(|p| p.r#type == unique_type), + "only the requested media type may be returned" + ); + assert!(!filtered.iter().any(|p| p.id == other.id)); + + // A prefix must not match. + let prefix = list_policies_by_type(&tc, "application").await?; + assert!( + !prefix.iter().any(|p| p.id == wanted.id), + "type filtering must be exact, not a prefix match" + ); + + wanted.delete().await?; + other.delete().await?; + Ok(()) +} diff --git a/tests/api/tests/api_v3/policy/show.rs b/tests/api/tests/api_v3/policy/show.rs new file mode 100644 index 000000000..275ed9ced --- /dev/null +++ b/tests/api/tests/api_v3/policy/show.rs @@ -0,0 +1,61 @@ +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 + +use std::sync::Arc; + +use eyre::Result; +use tracing_test::traced_test; +use uuid::Uuid; + +use openstack_keystone_api_types::v3::policy::*; +use openstack_sdk::{AsyncOpenStack, config::CloudConfig}; + +use test_api::guard::ResourceGuard; +use test_api::policy::{create_policy, show_policy}; + +#[tokio::test] +#[traced_test] +async fn test_show_policy() -> Result<()> { + let tc = Arc::new(AsyncOpenStack::new(&CloudConfig::from_env()?).await?); + let blob = format!("BlobName-{}", Uuid::new_v4().simple()); + + let guard = create_policy( + &tc, + PolicyCreateBuilder::default() + .blob(blob.clone()) + .r#type("application/json") + .build()?, + ) + .await?; + + let fetched = show_policy(&tc, &guard.id).await?; + assert_eq!(fetched.id, guard.id); + assert_eq!(fetched.blob, serde_json::json!(blob)); + assert_eq!(fetched.r#type, "application/json"); + + guard.delete().await?; + Ok(()) +} + +#[tokio::test] +#[traced_test] +async fn test_show_missing_policy_is_404() -> Result<()> { + let tc = Arc::new(AsyncOpenStack::new(&CloudConfig::from_env()?).await?); + assert!( + show_policy(&tc, format!("missing{}", Uuid::new_v4().simple())) + .await + .is_err() + ); + Ok(()) +} diff --git a/tests/api/tests/api_v3/policy/update.rs b/tests/api/tests/api_v3/policy/update.rs new file mode 100644 index 000000000..b11411084 --- /dev/null +++ b/tests/api/tests/api_v3/policy/update.rs @@ -0,0 +1,170 @@ +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 + +use std::collections::HashMap; +use std::sync::Arc; + +use eyre::Result; +use tracing_test::traced_test; +use uuid::Uuid; + +use openstack_keystone_api_types::v3::policy::*; +use openstack_sdk::{AsyncOpenStack, config::CloudConfig}; + +use test_api::guard::ResourceGuard; +use test_api::policy::{create_policy, show_policy, update_policy}; + +/// Mirrors tempest's `test_create_update_delete_policy`: PATCHing only +/// `type` must leave the blob untouched, as observed through a fresh GET. +#[tokio::test] +#[traced_test] +async fn test_update_type_preserves_blob() -> Result<()> { + let tc = Arc::new(AsyncOpenStack::new(&CloudConfig::from_env()?).await?); + let blob = format!("BlobName-{}", Uuid::new_v4().simple()); + + let guard = create_policy( + &tc, + PolicyCreateBuilder::default() + .blob(blob.clone()) + .r#type(format!("PolicyType-{}", Uuid::new_v4().simple())) + .build()?, + ) + .await?; + + let updated_type = format!("UpdatedPolicyType-{}", Uuid::new_v4().simple()); + let updated = update_policy( + &tc, + &guard.id, + PolicyUpdateBuilder::default() + .r#type(updated_type.clone()) + .build()?, + ) + .await?; + assert_eq!(updated.r#type, updated_type); + + let fetched = show_policy(&tc, &guard.id).await?; + assert_eq!(fetched.id, guard.id); + assert_eq!(fetched.r#type, updated_type); + assert_eq!( + fetched.blob, + serde_json::json!(blob), + "a type-only PATCH must not change the blob" + ); + + guard.delete().await?; + Ok(()) +} + +/// Extras supplied on PATCH are merged into the stored ones; omitted stored +/// keys survive (python keystone's `update_policy` semantics). +#[tokio::test] +#[traced_test] +async fn test_update_merges_extra() -> Result<()> { + let tc = Arc::new(AsyncOpenStack::new(&CloudConfig::from_env()?).await?); + + let guard = create_policy( + &tc, + PolicyCreateBuilder::default() + .blob("blob") + .r#type("application/json") + .extra(HashMap::from([ + ("keep".to_string(), serde_json::json!("me")), + ("over".to_string(), serde_json::json!("old")), + ])) + .build()?, + ) + .await?; + + update_policy( + &tc, + &guard.id, + PolicyUpdateBuilder::default() + .extra(HashMap::from([( + "over".to_string(), + serde_json::json!("new"), + )])) + .build()?, + ) + .await?; + + let fetched = show_policy(&tc, &guard.id).await?; + assert_eq!( + fetched.extra.get("keep"), + Some(&serde_json::json!("me")), + "an omitted stored extra must survive the PATCH" + ); + assert_eq!(fetched.extra.get("over"), Some(&serde_json::json!("new"))); + + guard.delete().await?; + Ok(()) +} + +/// An `id` different from the path is rejected ("Cannot change policy ID"). +#[tokio::test] +#[traced_test] +async fn test_update_rejects_mismatched_id() -> Result<()> { + let tc = Arc::new(AsyncOpenStack::new(&CloudConfig::from_env()?).await?); + + let guard = create_policy( + &tc, + PolicyCreateBuilder::default() + .blob("blob") + .r#type("application/json") + .build()?, + ) + .await?; + + assert!( + update_policy( + &tc, + &guard.id, + PolicyUpdateBuilder::default() + .id(format!("other{}", Uuid::new_v4().simple())) + .r#type("text/plain") + .build()?, + ) + .await + .is_err(), + "an id that differs from the path must be refused" + ); + + guard.delete().await?; + Ok(()) +} + +/// An empty patch document is refused (`minProperties: 1`). +#[tokio::test] +#[traced_test] +async fn test_update_rejects_empty_document() -> Result<()> { + let tc = Arc::new(AsyncOpenStack::new(&CloudConfig::from_env()?).await?); + + let guard = create_policy( + &tc, + PolicyCreateBuilder::default() + .blob("blob") + .r#type("application/json") + .build()?, + ) + .await?; + + assert!( + update_policy(&tc, &guard.id, PolicyUpdate::default()) + .await + .is_err(), + "an empty patch document must be refused" + ); + + guard.delete().await?; + Ok(()) +} diff --git a/tests/integration/src/integration.rs b/tests/integration/src/integration.rs index bc40e2fac..bbe7d3a4c 100644 --- a/tests/integration/src/integration.rs +++ b/tests/integration/src/integration.rs @@ -32,6 +32,7 @@ mod oauth2_key_janitor; mod oauth2_session; mod oauth2_token_exchange; mod oauth2_token_verify; +mod policy_store; mod resource; mod revoke; mod role; diff --git a/tests/integration/src/policy_store.rs b/tests/integration/src/policy_store.rs new file mode 100644 index 000000000..03d5fcfb7 --- /dev/null +++ b/tests/integration/src/policy_store.rs @@ -0,0 +1,45 @@ +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 +//! Legacy policy document store (`/v3/policies`) provider tests. + +mod crud; + +use std::pin::Pin; +use std::sync::Arc; + +use eyre::Result; + +use openstack_keystone::keystone::Service; +use openstack_keystone::keystone::ServiceState; +use openstack_keystone_core::auth::ExecutionContext; +use openstack_keystone_core_types::policy_store::*; + +use crate::common::*; +use crate::impl_deleter; + +impl_deleter!(Service, Policy, get_policy_store_provider, delete_policy); + +/// Create a policy through the policy store provider, returning a guard that +/// deletes it again when dropped. +pub async fn create_policy( + state: &ServiceState, + data: PolicyCreate, +) -> Result> { + let res = state + .provider + .get_policy_store_provider() + .create_policy(&ExecutionContext::internal(state), data) + .await?; + Ok(AsyncResourceGuard::new(res, state.clone())) +} diff --git a/tests/integration/src/policy_store/crud.rs b/tests/integration/src/policy_store/crud.rs new file mode 100644 index 000000000..3f7fb692e --- /dev/null +++ b/tests/integration/src/policy_store/crud.rs @@ -0,0 +1,312 @@ +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 +//! End-to-end policy store CRUD against a real database. + +use std::collections::HashMap; + +use eyre::Result; +use serde_json::{Value, json}; +use tracing_test::traced_test; + +use openstack_keystone_core::auth::ExecutionContext; +use openstack_keystone_core_types::policy_store::*; + +use crate::common::get_state; +use crate::policy_store::create_policy; + +fn create_params(r#type: &str, blob: &str) -> PolicyCreate { + PolicyCreate { + id: None, + r#type: r#type.to_string(), + blob: Value::String(blob.to_string()), + extra: HashMap::new(), + } +} + +/// The service layer assigns the ID, so the created record always carries one. +#[traced_test] +#[tokio::test] +async fn test_create_assigns_id() -> Result<()> { + let (state, _tmp) = get_state().await?; + let policy = create_policy(&state, create_params("application/json", "doc")).await?; + + assert!(!policy.id.is_empty(), "service layer must assign an ID"); + assert_eq!(policy.r#type, "application/json"); + assert_eq!(policy.blob, json!("doc")); + Ok(()) +} + +/// A string blob survives the JSON encode/decode round-trip through the +/// `JsonBlob`-compatible `blob` column. +#[traced_test] +#[tokio::test] +async fn test_string_blob_round_trip() -> Result<()> { + let (state, _tmp) = get_state().await?; + let raw = "{'foobar_user': 'role:compute-user'}"; + let policy = create_policy(&state, create_params("application/json", raw)).await?; + + let fetched = state + .provider + .get_policy_store_provider() + .get_policy(&ExecutionContext::internal(&state), &policy.id) + .await? + .expect("policy must be readable after creation"); + + assert_eq!(fetched.blob, json!(raw)); + Ok(()) +} + +/// Unknown properties round-trip through the `extra` column. +#[traced_test] +#[tokio::test] +async fn test_extra_round_trip() -> Result<()> { + let (state, _tmp) = get_state().await?; + let policy = create_policy( + &state, + PolicyCreate { + id: None, + r#type: "text/plain".into(), + blob: json!("b"), + extra: HashMap::from([("custom".into(), json!("value"))]), + }, + ) + .await?; + + let fetched = state + .provider + .get_policy_store_provider() + .get_policy(&ExecutionContext::internal(&state), &policy.id) + .await? + .expect("policy must be readable"); + + assert_eq!(fetched.extra.get("custom"), Some(&json!("value"))); + Ok(()) +} + +/// `?type=` filtering is an exact match applied in SQL. +#[traced_test] +#[tokio::test] +async fn test_list_filters_by_type_exactly() -> Result<()> { + let (state, _tmp) = get_state().await?; + let json_policy = create_policy(&state, create_params("application/json", "a")).await?; + let _text_policy = create_policy(&state, create_params("text/plain", "b")).await?; + + let exec = ExecutionContext::internal(&state); + let provider = state.provider.get_policy_store_provider(); + + let all = provider + .list_policies(&exec, &PolicyListParameters::default()) + .await?; + assert!(all.len() >= 2); + + let filtered = provider + .list_policies( + &exec, + &PolicyListParameters { + r#type: Some("application/json".into()), + ..Default::default() + }, + ) + .await?; + assert!( + filtered.iter().any(|p| p.id == json_policy.id), + "matching policy must be listed" + ); + assert!( + filtered.iter().all(|p| p.r#type == "application/json"), + "no other media type may appear" + ); + + // A prefix of a stored type must not match. + let prefix = provider + .list_policies( + &exec, + &PolicyListParameters { + r#type: Some("application".into()), + ..Default::default() + }, + ) + .await?; + assert!(prefix.is_empty(), "type filtering must be an exact match"); + Ok(()) +} + +/// A type-only update leaves the blob alone and merges `extra`, matching +/// python keystone's `update_policy`. +#[traced_test] +#[tokio::test] +async fn test_update_preserves_blob_and_merges_extra() -> Result<()> { + let (state, _tmp) = get_state().await?; + let policy = create_policy( + &state, + PolicyCreate { + id: None, + r#type: "application/json".into(), + blob: json!("original"), + extra: HashMap::from([("keep".into(), json!("me")), ("over".into(), json!("old"))]), + }, + ) + .await?; + + let exec = ExecutionContext::internal(&state); + let updated = state + .provider + .get_policy_store_provider() + .update_policy( + &exec, + &policy.id, + PolicyUpdate { + r#type: Some("text/plain".into()), + blob: None, + extra: HashMap::from([("over".into(), json!("new")), ("add".into(), json!(1))]), + }, + ) + .await?; + + assert_eq!(updated.r#type, "text/plain"); + assert_eq!(updated.blob, json!("original"), "blob must be preserved"); + assert_eq!(updated.extra.get("keep"), Some(&json!("me")), "omitted key"); + assert_eq!(updated.extra.get("over"), Some(&json!("new")), "new wins"); + assert_eq!(updated.extra.get("add"), Some(&json!(1)), "added key"); + Ok(()) +} + +/// Updating the blob replaces it, and it still decodes as the same JSON value. +#[traced_test] +#[tokio::test] +async fn test_update_blob() -> Result<()> { + let (state, _tmp) = get_state().await?; + let policy = create_policy(&state, create_params("application/json", "original")).await?; + + let updated = state + .provider + .get_policy_store_provider() + .update_policy( + &ExecutionContext::internal(&state), + &policy.id, + PolicyUpdate { + r#type: None, + blob: Some(json!("replaced")), + extra: HashMap::new(), + }, + ) + .await?; + + assert_eq!(updated.blob, json!("replaced")); + assert_eq!(updated.r#type, "application/json"); + Ok(()) +} + +#[traced_test] +#[tokio::test] +async fn test_delete_then_get_is_none() -> Result<()> { + let (state, _tmp) = get_state().await?; + let policy = create_policy(&state, create_params("application/json", "doc")).await?; + let id = policy.id.clone(); + + let exec = ExecutionContext::internal(&state); + state + .provider + .get_policy_store_provider() + .delete_policy(&exec, &id) + .await?; + + assert!( + state + .provider + .get_policy_store_provider() + .get_policy(&exec, &id) + .await? + .is_none() + ); + Ok(()) +} + +#[traced_test] +#[tokio::test] +async fn test_get_missing_is_none() -> Result<()> { + let (state, _tmp) = get_state().await?; + assert!( + state + .provider + .get_policy_store_provider() + .get_policy(&ExecutionContext::internal(&state), "does-not-exist") + .await? + .is_none() + ); + Ok(()) +} + +#[traced_test] +#[tokio::test] +async fn test_update_missing_is_not_found() -> Result<()> { + let (state, _tmp) = get_state().await?; + let result = state + .provider + .get_policy_store_provider() + .update_policy( + &ExecutionContext::internal(&state), + "does-not-exist", + PolicyUpdate { + r#type: Some("text/plain".into()), + ..Default::default() + }, + ) + .await; + + assert!(matches!( + result, + Err(PolicyStoreProviderError::PolicyNotFound(_)) + )); + Ok(()) +} + +#[traced_test] +#[tokio::test] +async fn test_delete_missing_is_not_found() -> Result<()> { + let (state, _tmp) = get_state().await?; + let result = state + .provider + .get_policy_store_provider() + .delete_policy(&ExecutionContext::internal(&state), "does-not-exist") + .await; + + assert!(matches!( + result, + Err(PolicyStoreProviderError::PolicyNotFound(_)) + )); + Ok(()) +} + +/// `type` is capped at 255 characters by the validator, which must reject the +/// request before it reaches the database. +#[traced_test] +#[tokio::test] +async fn test_create_type_too_long_is_rejected() -> Result<()> { + let (state, _tmp) = get_state().await?; + let result = state + .provider + .get_policy_store_provider() + .create_policy( + &ExecutionContext::internal(&state), + create_params(&"x".repeat(256), "doc"), + ) + .await; + + assert!( + result.is_err(), + "expected a validation error for a type longer than 255 characters" + ); + Ok(()) +} From 750a679065b1c42d306ad6888baaec20e9875a30 Mon Sep 17 00:00:00 2001 From: ShJ-code Date: Sun, 2 Aug 2026 05:33:36 +0000 Subject: [PATCH 2/2] chore(deps): Document temporary Wasmtime advisory exception Extism 1.30 still requires unsupported Wasmtime 43. Its upstream upgrade remains a draft. Keystone never exposes or mixes Wasmtime engine, store, or type-index handles through the Extism API. Keep the advisory visible as a narrow exception tied to upstream issue extism/extism#905, and remove it when a patched Extism release is available. Signed-off-by: ShJ-code --- deny.toml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/deny.toml b/deny.toml index c62b6aa35..a6d2043d6 100644 --- a/deny.toml +++ b/deny.toml @@ -74,7 +74,11 @@ ignore = [ "RUSTSEC-2023-0071", "RUSTSEC-2024-0436", # { id = "RUSTSEC-2026-0097", reason = "log feature is not enabled, deps need to update"}, - { id = "RUSTSEC-2026-0173", reason = "wait for deps to update"} + { id = "RUSTSEC-2026-0173", reason = "wait for deps to update"}, + # Extism 1.30 requires Wasmtime 43, which has no patched release. Keystone + # only uses Extism's high-level API: Engine, Store, and type indices are + # never exposed or mixed. Remove this after extism/extism#905 is released. + { id = "RUSTSEC-2026-0222", reason = "not reachable through the Extism API; track extism/extism#905" }, #{ id = "RUSTSEC-0000-0000", reason = "you can specify a reason the advisory is ignored" }, #"a-crate-that-is-yanked@0.1.1", # you can also ignore yanked crate versions if you wish #{ crate = "a-crate-that-is-yanked@0.1.1", reason = "you can specify why you are ignoring the yanked crate" },