From 95f42a8d48851d252dfe61b6dda7eebb6682b6dd Mon Sep 17 00:00:00 2001 From: Artem Goncharov Date: Thu, 10 Jul 2025 18:26:26 +0200 Subject: [PATCH] feat: Introduce policy enforcement for federation Add policies and enforce them for the federation idp and mapping resources. --- .github/workflows/functional.yml | 5 +- justfile | 12 +- .../idp/identity_provider_create.rego | 31 ++ .../idp/identity_provider_create_test.rego | 16 + .../idp/identity_provider_delete.rego | 31 ++ .../idp/identity_provider_delete_test.rego | 16 + .../idp/identity_provider_list.rego | 36 +++ .../idp/identity_provider_list_test.rego | 15 + .../idp/identity_provider_show.rego | 36 +++ .../idp/identity_provider_show_test.rego | 15 + .../idp/identity_provider_update.rego | 31 ++ .../idp/identity_provider_update_test.rego | 16 + policy/federation/mapping/mapping_create.rego | 31 ++ .../mapping/mapping_create_test.rego | 16 + policy/federation/mapping/mapping_delete.rego | 31 ++ .../mapping/mapping_delete_test.rego | 16 + policy/federation/mapping/mapping_list.rego | 36 +++ .../federation/mapping/mapping_list_test.rego | 15 + policy/federation/mapping/mapping_show.rego | 36 +++ .../federation/mapping/mapping_show_test.rego | 15 + policy/federation/mapping/mapping_update.rego | 31 ++ .../mapping/mapping_update_test.rego | 16 + policy/identity.rego | 37 +++ policy/identity_provider_list.rego | 30 -- src/api/error.rs | 1 + src/api/mod.rs | 29 +- src/api/v3/federation/error.rs | 10 +- src/api/v3/federation/identity_provider.rs | 300 +++++++++++++++--- src/api/v3/federation/mapping.rs | 220 ++++++++++--- src/api/v3/federation/oidc.rs | 5 +- src/api/v3/mod.rs | 2 +- src/bin/keystone.rs | 2 +- src/policy.rs | 33 +- src/tests/api.rs | 2 +- src/token/federation_domain_scoped.rs | 2 +- src/token/federation_project_scoped.rs | 2 +- src/token/federation_unscoped.rs | 2 +- src/token/fernet_utils.rs | 4 +- 38 files changed, 1045 insertions(+), 139 deletions(-) create mode 100644 policy/federation/idp/identity_provider_create.rego create mode 100644 policy/federation/idp/identity_provider_create_test.rego create mode 100644 policy/federation/idp/identity_provider_delete.rego create mode 100644 policy/federation/idp/identity_provider_delete_test.rego create mode 100644 policy/federation/idp/identity_provider_list.rego create mode 100644 policy/federation/idp/identity_provider_list_test.rego create mode 100644 policy/federation/idp/identity_provider_show.rego create mode 100644 policy/federation/idp/identity_provider_show_test.rego create mode 100644 policy/federation/idp/identity_provider_update.rego create mode 100644 policy/federation/idp/identity_provider_update_test.rego create mode 100644 policy/federation/mapping/mapping_create.rego create mode 100644 policy/federation/mapping/mapping_create_test.rego create mode 100644 policy/federation/mapping/mapping_delete.rego create mode 100644 policy/federation/mapping/mapping_delete_test.rego create mode 100644 policy/federation/mapping/mapping_list.rego create mode 100644 policy/federation/mapping/mapping_list_test.rego create mode 100644 policy/federation/mapping/mapping_show.rego create mode 100644 policy/federation/mapping/mapping_show_test.rego create mode 100644 policy/federation/mapping/mapping_update.rego create mode 100644 policy/federation/mapping/mapping_update_test.rego create mode 100644 policy/identity.rego delete mode 100644 policy/identity_provider_list.rego diff --git a/.github/workflows/functional.yml b/.github/workflows/functional.yml index 02c415a5..79449ea8 100644 --- a/.github/workflows/functional.yml +++ b/.github/workflows/functional.yml @@ -8,8 +8,9 @@ on: - 'Cargo.toml' - 'Cargo.lock' - '.github/workflows/functional.yml' - - 'tests/' - - 'src/' + - 'tests/**' + - 'src/**' + - 'policy/**' env: DATABASE_URL: postgresql://keystone:1234@127.0.0.1:5432/keystone KEYSTONE_URL: http://localhost:8080 diff --git a/justfile b/justfile index c72edb84..163e9f5d 100644 --- a/justfile +++ b/justfile @@ -1,4 +1,14 @@ -POLICY_ENTRY_POINTS := "-e identity/identity_provider_list" +POLICY_ENTRY_POINTS := \ +" -e identity/identity_provider_list" +\ +" -e identity/identity_provider_show" +\ +" -e identity/identity_provider_create" +\ +" -e identity/identity_provider_update" +\ +" -e identity/identity_provider_delete" +\ +" -e identity/mapping_list" +\ +" -e identity/mapping_show" +\ +" -e identity/mapping_create" +\ +" -e identity/mapping_update" +\ +" -e identity/mapping_delete" [working-directory: 'policy'] @build-policy: diff --git a/policy/federation/idp/identity_provider_create.rego b/policy/federation/idp/identity_provider_create.rego new file mode 100644 index 00000000..af63cb63 --- /dev/null +++ b/policy/federation/idp/identity_provider_create.rego @@ -0,0 +1,31 @@ +package identity.identity_provider_create + +import data.identity + +# Create identity provider. + +default allow := false + +allow if { + "admin" in input.credentials.roles +} + +allow if { + identity.own_idp + "manager" in input.credentials.roles +} + +violation contains {"field": "domain_id", "msg": "creating identity provider for other domain requires `admin` role."} if { + identity.foreign_idp + not "admin" in input.credentials.roles +} + +violation contains {"field": "role", "msg": "creating global identity provider requires `admin` role."} if { + identity.global_idp + not "admin" in input.credentials.roles +} + +violation contains {"field": "role", "msg": "creating identity provider requires `manager` role."} if { + identity.own_idp + not "member" in input.credentials.roles +} diff --git a/policy/federation/idp/identity_provider_create_test.rego b/policy/federation/idp/identity_provider_create_test.rego new file mode 100644 index 00000000..1fb86c6a --- /dev/null +++ b/policy/federation/idp/identity_provider_create_test.rego @@ -0,0 +1,16 @@ +package test_identity_provider_create + +import data.identity.identity_provider_create + +test_allowed if { + identity_provider_create.allow with input as {"credentials": {"roles": ["admin"]}} + identity_provider_create.allow with input as {"credentials": {"roles": ["manager"], "domain_id": "domain"}, "target": {"domain_id": "domain"}} + identity_provider_create.allow with input as {"credentials": {"roles": ["admin"]}, "target": {"domain_id": null}} +} + +test_forbidden if { + not identity_provider_create.allow with input as {"credentials": {"roles": []}} + not identity_provider_create.allow with input as {"credentials": {"roles": ["reader"], "domain_id": "domain"}, "target": {"domain_id": "domain"}} + not identity_provider_create.allow with input as {"credentials": {"roles": ["manager"], "domain_id": "domain"}, "target": {"domain_id": "other_domain"}} + not identity_provider_create.allow with input as {"credentials": {"roles": ["manager"], "domain_id": "domain"}, "target": {"domain_id": null}} +} diff --git a/policy/federation/idp/identity_provider_delete.rego b/policy/federation/idp/identity_provider_delete.rego new file mode 100644 index 00000000..1ae03ba5 --- /dev/null +++ b/policy/federation/idp/identity_provider_delete.rego @@ -0,0 +1,31 @@ +package identity.identity_provider_delete + +import data.identity + +# Show identity provider. + +default allow := false + +allow if { + "admin" in input.credentials.roles +} + +allow if { + identity.own_idp + "manager" in input.credentials.roles +} + +violation contains {"field": "domain_id", "msg": "deleting the global identity provider requires `admin` role."} if { + identity.global_idp + not "admin" in input.credentials.roles +} + +violation contains {"field": "role", "msg": "deleting the identity provider owned by the other domain requires `admin` role."} if { + identity.foreign_idp + not "admin" in input.credentials.roles +} + +violation contains {"field": "role", "msg": "deleting the identity provider requires `manager` role."} if { + identity.own_idp + not "manager" in input.credentials.roles +} diff --git a/policy/federation/idp/identity_provider_delete_test.rego b/policy/federation/idp/identity_provider_delete_test.rego new file mode 100644 index 00000000..fb1b9b35 --- /dev/null +++ b/policy/federation/idp/identity_provider_delete_test.rego @@ -0,0 +1,16 @@ +package test_identity_provider_delete + +import data.identity.identity_provider_delete + +test_allowed if { + identity_provider_delete.allow with input as {"credentials": {"roles": ["admin"]}} + identity_provider_delete.allow with input as {"credentials": {"roles": ["manager"], "domain_id": "domain"}, "target": {"domain_id": "domain"}} + identity_provider_delete.allow with input as {"credentials": {"roles": ["admin"]}, "target": {"domain_id": null}} +} + +test_forbidden if { + not identity_provider_delete.allow with input as {"credentials": {"roles": []}} + not identity_provider_delete.allow with input as {"credentials": {"roles": ["reader"], "domain_id": "domain"}, "target": {"domain_id": "domain"}} + not identity_provider_delete.allow with input as {"credentials": {"roles": ["manager"], "domain_id": "domain"}, "target": {"domain_id": "other_domain"}} + not identity_provider_delete.allow with input as {"credentials": {"roles": ["manager"], "domain_id": "domain"}, "target": {"domain_id": null}} +} diff --git a/policy/federation/idp/identity_provider_list.rego b/policy/federation/idp/identity_provider_list.rego new file mode 100644 index 00000000..dfa1830e --- /dev/null +++ b/policy/federation/idp/identity_provider_list.rego @@ -0,0 +1,36 @@ +package identity.identity_provider_list + +import data.identity + +# List identity providers + +default allow := false + +allow if { + identity.own_idp + "reader" in input.credentials.roles +} + +allow if { + identity.global_idp + "reader" in input.credentials.roles +} + +allow if { + "admin" in input.credentials.roles +} + +violation contains {"field": "domain_id", "msg": "listing federated identity providers owned by other domain requires `admin` role."} if { + identity.foreign_identity_provider + not "admin" in input.credentials.roles +} + +violation contains {"field": "role", "msg": "listing federated identity providers owned by the domain requires `reader` role."} if { + identity.own_idp + not "reader" in input.credentials.roles +} + +violation contains {"field": "role", "msg": "listing global federated identity providers requires `reader` role."} if { + identity.global_idp + not "reader" in input.credentials.roles +} diff --git a/policy/federation/idp/identity_provider_list_test.rego b/policy/federation/idp/identity_provider_list_test.rego new file mode 100644 index 00000000..dbe682f3 --- /dev/null +++ b/policy/federation/idp/identity_provider_list_test.rego @@ -0,0 +1,15 @@ +package test_identity_provider_list + +import data.identity.identity_provider_list + +test_allowed if { + identity_provider_list.allow with input as {"credentials": {"roles": ["admin"]}} + identity_provider_list.allow with input as {"credentials": {"roles": ["reader"], "domain_id": "domain"}, "target": {"domain_id": "domain"}} + identity_provider_list.allow with input as {"credentials": {"roles": ["reader"], "domain_id": "domain"}, "target": {"domain_id": null}} +} + +test_forbidden if { + not identity_provider_list.allow with input as {"credentials": {"roles": []}} + not identity_provider_list.allow with input as {"credentials": {"roles": ["reader"], "domain_id": "domain"}, "target": {"domain_id": "other_domain"}} + not identity_provider_list.allow with input as {"credentials": {"roles": ["member"], "domain_id": "domain"}, "target": {"domain_id": "other_domain"}} +} diff --git a/policy/federation/idp/identity_provider_show.rego b/policy/federation/idp/identity_provider_show.rego new file mode 100644 index 00000000..cd17d748 --- /dev/null +++ b/policy/federation/idp/identity_provider_show.rego @@ -0,0 +1,36 @@ +package identity.identity_provider_show + +import data.identity + +# Show identity provider. + +default allow := false + +allow if { + "admin" in input.credentials.roles +} + +allow if { + identity.own_idp + "reader" in input.credentials.roles +} + +allow if { + identity.global_idp + "reader" in input.credentials.roles +} + +violation contains {"field": "domain_id", "msg": "fetching identity provider details owned by other domain requires `admin` role."} if { + identity.foreign_idp + not "admin" in input.credentials.roles +} + +violation contains {"field": "role", "msg": "fetching own identity provider details requires `reader`."} if { + identity.own_idp + not "reader" in input.credentials.roles +} + +violation contains {"field": "role", "msg": "fetching global identity provider details requires `reader`."} if { + identity.global_idp + not "reader" in input.credentials.roles +} diff --git a/policy/federation/idp/identity_provider_show_test.rego b/policy/federation/idp/identity_provider_show_test.rego new file mode 100644 index 00000000..b4bbd21b --- /dev/null +++ b/policy/federation/idp/identity_provider_show_test.rego @@ -0,0 +1,15 @@ +package test_identity_provider_show + +import data.identity.identity_provider_show + +test_allowed if { + identity_provider_show.allow with input as {"credentials": {"roles": ["admin"]}} + identity_provider_show.allow with input as {"credentials": {"roles": ["reader"], "domain_id": "domain"}, "target": {"domain_id": "domain"}} + identity_provider_show.allow with input as {"credentials": {"roles": ["reader"]}, "target": {"domain_id": null}} +} + +test_forbidden if { + not identity_provider_show.allow with input as {"credentials": {"roles": []}} + not identity_provider_show.allow with input as {"credentials": {"roles": ["reader"], "domain_id": "domain"}, "target": {"domain_id": "other_domain"}} + not identity_provider_show.allow with input as {"credentials": {"roles": ["member"], "domain_id": "domain"}, "target": {"domain_id": "other_domain"}} +} diff --git a/policy/federation/idp/identity_provider_update.rego b/policy/federation/idp/identity_provider_update.rego new file mode 100644 index 00000000..c93d445f --- /dev/null +++ b/policy/federation/idp/identity_provider_update.rego @@ -0,0 +1,31 @@ +package identity.identity_provider_update + +import data.identity + +# Update identity provider. + +default allow := false + +allow if { + "admin" in input.credentials.roles +} + +allow if { + identity.own_idp + "manager" in input.credentials.roles +} + +violation contains {"field": "domain_id", "msg": "updating identity provider for other domain requires `admin` role."} if { + identity.foreign_idp + not "admin" in input.credentials.roles +} + +violation contains {"field": "role", "msg": "updating global identity provider requires `admin` role."} if { + identity.global_idp + not "admin" in input.credentials.roles +} + +violation contains {"field": "role", "msg": "updating identity provider requires `manager` role."} if { + identity.own_idp + not "member" in input.credentials.roles +} diff --git a/policy/federation/idp/identity_provider_update_test.rego b/policy/federation/idp/identity_provider_update_test.rego new file mode 100644 index 00000000..3e31f222 --- /dev/null +++ b/policy/federation/idp/identity_provider_update_test.rego @@ -0,0 +1,16 @@ +package test_identity_provider_update + +import data.identity.identity_provider_update + +test_allowed if { + identity_provider_update.allow with input as {"credentials": {"roles": ["admin"]}} + identity_provider_update.allow with input as {"credentials": {"roles": ["manager"], "domain_id": "domain"}, "target": {"domain_id": "domain"}} + identity_provider_update.allow with input as {"credentials": {"roles": ["admin"]}, "target": {"domain_id": null}} +} + +test_forbidden if { + not identity_provider_update.allow with input as {"credentials": {"roles": []}} + not identity_provider_update.allow with input as {"credentials": {"roles": ["reader"], "domain_id": "domain"}, "target": {"domain_id": "domain"}} + not identity_provider_update.allow with input as {"credentials": {"roles": ["manager"], "domain_id": "domain"}, "target": {"domain_id": "other_domain"}} + not identity_provider_update.allow with input as {"credentials": {"roles": ["manager"], "domain_id": "domain"}, "target": {"domain_id": null}} +} diff --git a/policy/federation/mapping/mapping_create.rego b/policy/federation/mapping/mapping_create.rego new file mode 100644 index 00000000..e65256f0 --- /dev/null +++ b/policy/federation/mapping/mapping_create.rego @@ -0,0 +1,31 @@ +package identity.mapping_create + +import data.identity + +# Create mapping. + +default allow := false + +allow if { + "admin" in input.credentials.roles +} + +allow if { + identity.own_mapping + "manager" in input.credentials.roles +} + +violation contains {"field": "domain_id", "msg": "creating mapping for other domain requires `admin` role."} if { + identity.foreign_mapping + not "admin" in input.credentials.roles +} + +violation contains {"field": "role", "msg": "creating global mapping requires `admin` role."} if { + identity.global_mapping + not "admin" in input.credentials.roles +} + +violation contains {"field": "role", "msg": "creating mapping requires `manager` role."} if { + identity.own_mapping + not "member" in input.credentials.roles +} diff --git a/policy/federation/mapping/mapping_create_test.rego b/policy/federation/mapping/mapping_create_test.rego new file mode 100644 index 00000000..40696fc9 --- /dev/null +++ b/policy/federation/mapping/mapping_create_test.rego @@ -0,0 +1,16 @@ +package test_mapping_create + +import data.identity.mapping_create + +test_allowed if { + mapping_create.allow with input as {"credentials": {"roles": ["admin"]}} + mapping_create.allow with input as {"credentials": {"roles": ["manager"], "domain_id": "domain"}, "target": {"domain_id": "domain"}} + mapping_create.allow with input as {"credentials": {"roles": ["admin"]}, "target": {"domain_id": null}} +} + +test_forbidden if { + not mapping_create.allow with input as {"credentials": {"roles": []}} + not mapping_create.allow with input as {"credentials": {"roles": ["reader"], "domain_id": "domain"}, "target": {"domain_id": "domain"}} + not mapping_create.allow with input as {"credentials": {"roles": ["manager"], "domain_id": "domain"}, "target": {"domain_id": "other_domain"}} + not mapping_create.allow with input as {"credentials": {"roles": ["manager"], "domain_id": "domain"}, "target": {"domain_id": null}} +} diff --git a/policy/federation/mapping/mapping_delete.rego b/policy/federation/mapping/mapping_delete.rego new file mode 100644 index 00000000..50f5ea57 --- /dev/null +++ b/policy/federation/mapping/mapping_delete.rego @@ -0,0 +1,31 @@ +package identity.mapping_delete + +import data.identity + +# Show mapping. + +default allow := false + +allow if { + "admin" in input.credentials.roles +} + +allow if { + identity.own_mapping + "manager" in input.credentials.roles +} + +violation contains {"field": "domain_id", "msg": "deleting the global mapping requires `admin` role."} if { + identity.global_mapping + not "admin" in input.credentials.roles +} + +violation contains {"field": "role", "msg": "deleting the mapping owned by the other domain requires `admin` role."} if { + identity.foreign_mapping + not "admin" in input.credentials.roles +} + +violation contains {"field": "role", "msg": "deleting the mapping requires `manager` role."} if { + identity.own_mapping + not "manager" in input.credentials.roles +} diff --git a/policy/federation/mapping/mapping_delete_test.rego b/policy/federation/mapping/mapping_delete_test.rego new file mode 100644 index 00000000..95f8f327 --- /dev/null +++ b/policy/federation/mapping/mapping_delete_test.rego @@ -0,0 +1,16 @@ +package test_mapping_delete + +import data.identity.mapping_delete + +test_allowed if { + mapping_delete.allow with input as {"credentials": {"roles": ["admin"]}} + mapping_delete.allow with input as {"credentials": {"roles": ["manager"], "domain_id": "domain"}, "target": {"domain_id": "domain"}} + mapping_delete.allow with input as {"credentials": {"roles": ["admin"]}, "target": {"domain_id": null}} +} + +test_forbidden if { + not mapping_delete.allow with input as {"credentials": {"roles": []}} + not mapping_delete.allow with input as {"credentials": {"roles": ["reader"], "domain_id": "domain"}, "target": {"domain_id": "domain"}} + not mapping_delete.allow with input as {"credentials": {"roles": ["manager"], "domain_id": "domain"}, "target": {"domain_id": "other_domain"}} + not mapping_delete.allow with input as {"credentials": {"roles": ["manager"], "domain_id": "domain"}, "target": {"domain_id": null}} +} diff --git a/policy/federation/mapping/mapping_list.rego b/policy/federation/mapping/mapping_list.rego new file mode 100644 index 00000000..a0fa4686 --- /dev/null +++ b/policy/federation/mapping/mapping_list.rego @@ -0,0 +1,36 @@ +package identity.mapping_list + +import data.identity + +# List mappings. + +default allow := false + +allow if { + identity.own_mapping + "reader" in input.credentials.roles +} + +allow if { + identity.global_mapping + "reader" in input.credentials.roles +} + +allow if { + "admin" in input.credentials.roles +} + +violation contains {"field": "domain_id", "msg": "listing federated mappings owned by other domain requires `admin` role."} if { + identity.foreign_mapping + not "admin" in input.credentials.roles +} + +violation contains {"field": "role", "msg": "listing federated mappings owned by the domain requires `reader` role."} if { + identity.own_mapping + not "reader" in input.credentials.roles +} + +violation contains {"field": "role", "msg": "listing global federated mappings requires `reader` role."} if { + identity.global_mapping + not "reader" in input.credentials.roles +} diff --git a/policy/federation/mapping/mapping_list_test.rego b/policy/federation/mapping/mapping_list_test.rego new file mode 100644 index 00000000..4c0f5c0a --- /dev/null +++ b/policy/federation/mapping/mapping_list_test.rego @@ -0,0 +1,15 @@ +package test_mapping_list + +import data.identity.mapping_list + +test_allowed if { + mapping_list.allow with input as {"credentials": {"roles": ["admin"]}} + mapping_list.allow with input as {"credentials": {"roles": ["reader"], "domain_id": "domain"}, "target": {"domain_id": "domain"}} + mapping_list.allow with input as {"credentials": {"roles": ["reader"], "domain_id": "domain"}, "target": {"domain_id": null}} +} + +test_forbidden if { + not mapping_list.allow with input as {"credentials": {"roles": []}} + not mapping_list.allow with input as {"credentials": {"roles": ["reader"], "domain_id": "domain"}, "target": {"domain_id": "other_domain"}} + not mapping_list.allow with input as {"credentials": {"roles": ["member"], "domain_id": "domain"}, "target": {"domain_id": "other_domain"}} +} diff --git a/policy/federation/mapping/mapping_show.rego b/policy/federation/mapping/mapping_show.rego new file mode 100644 index 00000000..1ec5b988 --- /dev/null +++ b/policy/federation/mapping/mapping_show.rego @@ -0,0 +1,36 @@ +package identity.mapping_show + +import data.identity + +# Show identity provider. + +default allow := false + +allow if { + "admin" in input.credentials.roles +} + +allow if { + identity.own_mapping + "reader" in input.credentials.roles +} + +allow if { + identity.global_mapping + "reader" in input.credentials.roles +} + +violation contains {"field": "domain_id", "msg": "fetching mapping details owned by other domain requires `admin` role."} if { + identity.foreign_mapping + not "admin" in input.credentials.roles +} + +violation contains {"field": "role", "msg": "fetching own mapping details requires `reader`."} if { + identity.own_mapping + not "reader" in input.credentials.roles +} + +violation contains {"field": "role", "msg": "fetching global mappingdetails requires `reader`."} if { + identity.global_mapping + not "reader" in input.credentials.roles +} diff --git a/policy/federation/mapping/mapping_show_test.rego b/policy/federation/mapping/mapping_show_test.rego new file mode 100644 index 00000000..8964d57d --- /dev/null +++ b/policy/federation/mapping/mapping_show_test.rego @@ -0,0 +1,15 @@ +package test_mapping_show + +import data.identity.mapping_show + +test_allowed if { + mapping_show.allow with input as {"credentials": {"roles": ["admin"]}} + mapping_show.allow with input as {"credentials": {"roles": ["reader"], "domain_id": "domain"}, "target": {"domain_id": "domain"}} + mapping_show.allow with input as {"credentials": {"roles": ["reader"]}, "target": {"domain_id": null}} +} + +test_forbidden if { + not mapping_show.allow with input as {"credentials": {"roles": []}} + not mapping_show.allow with input as {"credentials": {"roles": ["reader"], "domain_id": "domain"}, "target": {"domain_id": "other_domain"}} + not mapping_show.allow with input as {"credentials": {"roles": ["member"], "domain_id": "domain"}, "target": {"domain_id": "other_domain"}} +} diff --git a/policy/federation/mapping/mapping_update.rego b/policy/federation/mapping/mapping_update.rego new file mode 100644 index 00000000..1ea84484 --- /dev/null +++ b/policy/federation/mapping/mapping_update.rego @@ -0,0 +1,31 @@ +package identity.mapping_update + +import data.identity + +# Update mapping. + +default allow := false + +allow if { + "admin" in input.credentials.roles +} + +allow if { + identity.own_mapping + "manager" in input.credentials.roles +} + +violation contains {"field": "domain_id", "msg": "updating mapping for other domain requires `admin` role."} if { + identity.foreign_mapping + not "admin" in input.credentials.roles +} + +violation contains {"field": "role", "msg": "updating global mapping requires `admin` role."} if { + identity.global_mapping + not "admin" in input.credentials.roles +} + +violation contains {"field": "role", "msg": "updating mapping requires `manager` role."} if { + identity.own_mapping + not "member" in input.credentials.roles +} diff --git a/policy/federation/mapping/mapping_update_test.rego b/policy/federation/mapping/mapping_update_test.rego new file mode 100644 index 00000000..87a0bd25 --- /dev/null +++ b/policy/federation/mapping/mapping_update_test.rego @@ -0,0 +1,16 @@ +package test_mapping_update + +import data.identity.mapping_update + +test_allowed if { + mapping_update.allow with input as {"credentials": {"roles": ["admin"]}} + mapping_update.allow with input as {"credentials": {"roles": ["manager"], "domain_id": "domain"}, "target": {"domain_id": "domain"}} + mapping_update.allow with input as {"credentials": {"roles": ["admin"]}, "target": {"domain_id": null}} +} + +test_forbidden if { + not mapping_update.allow with input as {"credentials": {"roles": []}} + not mapping_update.allow with input as {"credentials": {"roles": ["reader"], "domain_id": "domain"}, "target": {"domain_id": "domain"}} + not mapping_update.allow with input as {"credentials": {"roles": ["manager"], "domain_id": "domain"}, "target": {"domain_id": "other_domain"}} + not mapping_update.allow with input as {"credentials": {"roles": ["manager"], "domain_id": "domain"}, "target": {"domain_id": null}} +} diff --git a/policy/identity.rego b/policy/identity.rego new file mode 100644 index 00000000..f7149fc8 --- /dev/null +++ b/policy/identity.rego @@ -0,0 +1,37 @@ +package identity + +global_idp if { + not input.target.domain_id +} + +global_idp if { + input.target.domain_id == null +} + +own_idp if { + input.target.domain_id != null + input.target.domain_id == input.credentials.domain_id +} + +foreign_idp if { + input.target.domain_id != null + input.target.domain_id != input.credentials.domain_id +} + +global_mapping if { + not input.target.domain_id +} + +global_mapping if { + input.target.domain_id == null +} + +own_mapping if { + input.target.domain_id != null + input.target.domain_id == input.credentials.domain_id +} + +foreign_mapping if { + input.target.domain_id != null + input.target.domain_id != input.credentials.domain_id +} diff --git a/policy/identity_provider_list.rego b/policy/identity_provider_list.rego deleted file mode 100644 index 3d9f3f4e..00000000 --- a/policy/identity_provider_list.rego +++ /dev/null @@ -1,30 +0,0 @@ -package identity.identity_provider_list - -# List identity providers. - -default allow := false - -allow if { - count(violation) == 0 -} - -violation contains {"field": "domain_id", "msg": "only admin user is allowed to list identity providers not owned by the domain in scope."} if { - not "admin" in input.credentials.roles - not global_or_local_idp -} - -global_idp if { - not input.target.domain_id -} - -local_idp if { - input.target.domain_id == input.credentials.domain_id -} - -global_or_local_idp if { - global_idp -} - -global_or_local_idp if { - local_idp -} diff --git a/src/api/error.rs b/src/api/error.rs index 7790f649..e7ad498a 100644 --- a/src/api/error.rs +++ b/src/api/error.rs @@ -175,6 +175,7 @@ impl IntoResponse for KeystoneApiError { KeystoneApiError::Unauthorized => StatusCode::UNAUTHORIZED, // KeystoneApiError::AuthenticationInfo { .. } => StatusCode::UNAUTHORIZED, KeystoneApiError::Forbidden => StatusCode::FORBIDDEN, + KeystoneApiError::Policy { .. } => StatusCode::FORBIDDEN, KeystoneApiError::InternalError(_) | KeystoneApiError::IdentityError { .. } | KeystoneApiError::ResourceError { .. } diff --git a/src/api/mod.rs b/src/api/mod.rs index 0e4dae8c..ac06f492 100644 --- a/src/api/mod.rs +++ b/src/api/mod.rs @@ -17,7 +17,10 @@ use axum::{ http::{HeaderMap, header}, response::IntoResponse, }; -use utoipa::OpenApi; +use utoipa::{ + Modify, OpenApi, + openapi::security::{ApiKey, ApiKeyValue, SecurityScheme}, +}; use utoipa_axum::{router::OpenApiRouter, routes}; use crate::api::error::KeystoneApiError; @@ -32,9 +35,29 @@ pub mod v3; use crate::api::types::*; #[derive(OpenApi)] -#[openapi(info(version = "3.14.0"))] +#[openapi( + info(version = "3.14.0"), + modifiers(&SecurityAddon), + tags( + (name="identity_providers", description=v3::federation::identity_provider::DESCRIPTION), + (name="mappings", description=v3::federation::mapping::DESCRIPTION) + ) +)] pub struct ApiDoc; +struct SecurityAddon; + +impl Modify for SecurityAddon { + fn modify(&self, openapi: &mut utoipa::openapi::OpenApi) { + if let Some(components) = openapi.components.as_mut() { + components.add_security_scheme( + "x-auth", + SecurityScheme::ApiKey(ApiKey::Header(ApiKeyValue::new("x-auth-token"))), + ) + } + } +} + pub fn openapi_router() -> OpenApiRouter { OpenApiRouter::new() .nest("/v3", v3::openapi_router()) @@ -59,7 +82,7 @@ async fn version(headers: HeaderMap) -> Result for KeystoneApiError { KeystoneApiError::BadRequest(format!("Error exchanging authorization code for the authorization token: {msg}")) } OidcError::ClaimVerification { source } => { - KeystoneApiError::BadRequest(format!("Error in claims verification: {}", source)) + KeystoneApiError::BadRequest(format!("Error in claims verification: {source}")) } OidcError::OpenIdConnectReqwest { source } => { - KeystoneApiError::InternalError(format!("Error in OpenIDConnect logic: {}", source)) + KeystoneApiError::InternalError(format!("Error in OpenIDConnect logic: {source}")) } OidcError::OpenIdConnectConfiguration { source } => { - KeystoneApiError::InternalError(format!("Error in OpenIDConnect logic: {}", source)) + KeystoneApiError::InternalError(format!("Error in OpenIDConnect logic: {source}")) } OidcError::UrlParse { source } => { - KeystoneApiError::BadRequest(format!("Error in OpenIDConnect logic: {}", source)) + KeystoneApiError::BadRequest(format!("Error in OpenIDConnect logic: {source}")) } e @ OidcError::NoToken => { - KeystoneApiError::InternalError(format!("Error in OpenIDConnect logic: {}", e)) + KeystoneApiError::InternalError(format!("Error in OpenIDConnect logic: {e}")) } OidcError::ClientIdRequired => { KeystoneApiError::BadRequest("Identity Provider mut set `client_id`.".to_string()) diff --git a/src/api/v3/federation/identity_provider.rs b/src/api/v3/federation/identity_provider.rs index 6abf3c6d..7be8fde9 100644 --- a/src/api/v3/federation/identity_provider.rs +++ b/src/api/v3/federation/identity_provider.rs @@ -12,6 +12,7 @@ // // SPDX-License-Identifier: Apache-2.0 +//! Identity providers API use axum::{ Json, debug_handler, extract::{Path, Query, State}, @@ -19,6 +20,7 @@ use axum::{ response::IntoResponse, }; use mockall_double::double; +use serde_json::to_value; use utoipa_axum::{router::OpenApiRouter, routes}; use crate::api::auth::Auth; @@ -29,32 +31,50 @@ use crate::keystone::ServiceState; #[double] use crate::policy::Policy; +pub(crate) static DESCRIPTION: &str = r#"Identity providers API. + +Identity provider resource allows to federate users from an external Identity Provider (i.e. +Keycloak, Azure AD, etc.). + +Using the Identity provider requires creation of the mapping, which describes how to map attributes +of the remote Idp to local users. + +Identity provider with an empty domain_id are considered globals and every domain may use it with +appropriate mapping. +"#; + pub(super) fn openapi_router() -> OpenApiRouter { OpenApiRouter::new() .routes(routes!(list, create)) .routes(routes!(show, update, remove)) } -/// List identity providers +/// List identity providers. +/// +/// List identity providers. Without any filters only global identity providers are returned. +/// With the `domain_id` identity providers owned by the specified identity provider are returned. +/// +/// It is expected that only global or owned identity providers can be returned, while an admin +/// user is able to list all providers. #[utoipa::path( get, path = "/", params(IdentityProviderListParameters), - description = "List Identity Providers", responses( (status = OK, description = "List of identity providers", body = IdentityProviderList), (status = 500, description = "Internal error", example = json!(KeystoneApiError::InternalError(String::from("id = 1")))) ), + security(("x-auth" = [])), tag="identity_providers" )] #[tracing::instrument( name = "api::identity_provider_list", level = "debug", - skip(state, _user_auth, policy), + skip(state, user_auth, policy), err(Debug) )] async fn list( - Auth(_user_auth): Auth, + Auth(user_auth): Auth, mut policy: Policy, Query(query): Query, State(state): State, @@ -62,10 +82,12 @@ async fn list( policy .enforce( "identity/identity_provider_list", - &_user_auth, - serde_json::to_value(&query)?, + &user_auth, + to_value(&query)?, + None, ) .await?; + let identity_providers: Vec = state .provider .get_federation_provider() @@ -78,30 +100,32 @@ async fn list( Ok(IdentityProviderList { identity_providers }) } -/// Get single identity provider +/// Get single identity provider. +/// +/// Shows details of the existing identity provider. #[utoipa::path( get, path = "/{idp_id}", - description = "Get IDP by ID", - params(), responses( (status = OK, description = "IDP object", body = IdentityProviderResponse), (status = 404, description = "IDP not found", example = json!(KeystoneApiError::NotFound(String::from("id = 1")))) ), + security(("x-auth" = [])), tag="identity_providers" )] #[tracing::instrument( name = "api::identity_provider_get", level = "debug", - skip(state), + skip(state, user_auth, policy), err(Debug) )] async fn show( Auth(user_auth): Auth, + mut policy: Policy, Path(idp_id): Path, State(state): State, ) -> Result { - state + let current = state .provider .get_federation_provider() .get_identity_provider(&state.db, &idp_id) @@ -111,31 +135,55 @@ async fn show( resource: "identity provider".into(), identifier: idp_id, }) - })? + })??; + + policy + .enforce( + "identity/identity_provider_show", + &user_auth, + serde_json::to_value(¤t)?, + None, + ) + .await?; + Ok(current) } -/// Create identity provider +/// Create the identity provider. +/// +/// Create the identity provider with the specified properties. +/// +/// It is expected that only admin user is able to create global identity providers. #[utoipa::path( post, path = "/", - description = "Create new identity provider", responses( (status = CREATED, description = "identity provider object", body = IdentityProviderResponse), ), + security(("x-auth" = [])), tag="identity_providers" )] #[tracing::instrument( name = "api::identity_provider_create", level = "debug", - skip(state), + skip(state, user_auth, policy), err(Debug) )] #[debug_handler] async fn create( Auth(user_auth): Auth, + mut policy: Policy, State(state): State, Json(req): Json, ) -> Result { + policy + .enforce( + "identity/identity_provider_create", + &user_auth, + serde_json::to_value(&req.identity_provider)?, + None, + ) + .await?; + let res = state .provider .get_federation_provider() @@ -145,30 +193,49 @@ async fn create( Ok((StatusCode::CREATED, res).into_response()) } -/// Update single identity provider +/// Update single identity provider. +/// +/// Updates the existing identity provider. #[utoipa::path( put, path = "/{idp_id}", - description = "Update Identity Provider", params(), responses( (status = OK, description = "IDP object", body = IdentityProviderResponse), (status = 404, description = "IDP not found", example = json!(KeystoneApiError::NotFound(String::from("id = 1")))) ), + security(("x-auth" = [])), tag="identity_providers" )] #[tracing::instrument( name = "api::identity_provider_update", level = "debug", - skip(state), + skip(state, user_auth, policy), err(Debug) )] async fn update( Auth(user_auth): Auth, + mut policy: Policy, Path(idp_id): Path, State(state): State, Json(req): Json, ) -> Result { + // Fetch the current resource to pass current object into the policy evaluation + let current = state + .provider + .get_federation_provider() + .get_identity_provider(&state.db, &idp_id) + .await?; + + policy + .enforce( + "identity/identity_provider_update", + &user_auth, + serde_json::to_value(¤t)?, + Some(serde_json::to_value(&req.identity_provider)?), + ) + .await?; + let res = state .provider .get_federation_provider() @@ -178,35 +245,64 @@ async fn update( Ok(res.into_response()) } -/// Delete Identity provider +/// Delete Identity provider. +/// +/// Deletes the existing identity provider. +/// +/// It is expected that only admin user is allowed to delete the global identity provider #[utoipa::path( delete, path = "/{idp_id}", - description = "Delete identity provider by ID", params(), responses( (status = 204, description = "Deleted"), (status = 404, description = "identity provider not found", example = json!(KeystoneApiError::NotFound(String::from("id = 1")))) ), + security(("x-auth" = [])), tag="identity_providers" )] #[tracing::instrument( name = "api::identity_provider_delete", level = "debug", - skip(state), + skip(state, user_auth, policy), err(Debug) )] async fn remove( Auth(user_auth): Auth, + mut policy: Policy, Path(id): Path, State(state): State, ) -> Result { - state + let current = state .provider .get_federation_provider() - .delete_identity_provider(&state.db, &id) - .await - .map_err(KeystoneApiError::federation)?; + .get_identity_provider(&state.db, &id) + .await?; + + policy + .enforce( + "identity/identity_provider_delete", + &user_auth, + serde_json::to_value(¤t)?, + None, + ) + .await?; + + // TODO: decide what to do with the users provisioned using this IDP, mappings, ... + + if current.is_some() { + state + .provider + .get_federation_provider() + .delete_identity_provider(&state.db, &id) + .await + .map_err(KeystoneApiError::federation)?; + } else { + return Err(KeystoneApiError::NotFound { + resource: "identity_provider".to_string(), + identifier: id.clone(), + }); + } Ok((StatusCode::NO_CONTENT).into_response()) } @@ -218,10 +314,10 @@ mod tests { }; use http_body_util::BodyExt; // for `collect` use sea_orm::DatabaseConnection; - use std::sync::Arc; use tower::ServiceExt; // for `call`, `oneshot`, and `ready` use tower_http::trace::TraceLayer; + use tracing_test::traced_test; use super::*; use crate::config::Config; @@ -229,11 +325,14 @@ mod tests { MockFederationProvider, error::FederationProviderError, types as provider_types, }; use crate::keystone::{Service, ServiceState}; - use crate::policy::{MockPolicy, MockPolicyFactory, PolicyEvaluationResult}; + use crate::policy::{MockPolicy, MockPolicyFactory, PolicyError, PolicyEvaluationResult}; use crate::provider::Provider; use crate::token::{MockTokenProvider, Token, UnscopedPayload}; - fn get_mocked_state(federation_mock: MockFederationProvider) -> ServiceState { + fn get_mocked_state( + federation_mock: MockFederationProvider, + policy_allowed: bool, + ) -> ServiceState { let mut token_mock = MockTokenProvider::default(); token_mock.expect_validate_token().returning(|_, _, _| { Ok(Token::Unscoped(UnscopedPayload { @@ -257,13 +356,23 @@ mod tests { .unwrap(); let mut policy_factory_mock = MockPolicyFactory::default(); - policy_factory_mock.expect_instantiate().returning(|| { - let mut policy_mock = MockPolicy::default(); - policy_mock - .expect_enforce() - .returning(|_, _, _| Ok(PolicyEvaluationResult::allowed())); - Ok(policy_mock) - }); + if policy_allowed { + policy_factory_mock.expect_instantiate().returning(|| { + let mut policy_mock = MockPolicy::default(); + policy_mock + .expect_enforce() + .returning(|_, _, _, _| Ok(PolicyEvaluationResult::allowed())); + Ok(policy_mock) + }); + } else { + policy_factory_mock.expect_instantiate().returning(|| { + let mut policy_mock = MockPolicy::default(); + policy_mock.expect_enforce().returning(|_, _, _, _| { + Err(PolicyError::Forbidden(PolicyEvaluationResult::forbidden())) + }); + Ok(policy_mock) + }); + } Arc::new( Service::new( Config::default(), @@ -276,6 +385,7 @@ mod tests { } #[tokio::test] + #[traced_test] async fn test_list() { let mut federation_mock = MockFederationProvider::default(); federation_mock @@ -292,7 +402,7 @@ mod tests { ..Default::default() }]) }); - let state = get_mocked_state(federation_mock); + let state = get_mocked_state(federation_mock, true); let mut api = openapi_router() .layer(TraceLayer::new_for_http()) @@ -333,6 +443,7 @@ mod tests { } #[tokio::test] + #[traced_test] async fn test_list_qp() { let mut federation_mock = MockFederationProvider::default(); federation_mock @@ -354,7 +465,7 @@ mod tests { }]) }); - let state = get_mocked_state(federation_mock); + let state = get_mocked_state(federation_mock, true); let mut api = openapi_router() .layer(TraceLayer::new_for_http()) @@ -379,6 +490,46 @@ mod tests { } #[tokio::test] + #[traced_test] + async fn test_list_forbidden() { + let mut federation_mock = MockFederationProvider::default(); + federation_mock + .expect_list_identity_providers() + .withf( + |_: &DatabaseConnection, _: &provider_types::IdentityProviderListParameters| true, + ) + .returning(|_, _| { + Ok(vec![provider_types::IdentityProvider { + id: "id".into(), + name: "name".into(), + domain_id: Some("did".into()), + default_mapping_name: Some("dummy".into()), + ..Default::default() + }]) + }); + let state = get_mocked_state(federation_mock, false); + + let mut api = openapi_router() + .layer(TraceLayer::new_for_http()) + .with_state(state); + + let response = api + .as_service() + .oneshot( + Request::builder() + .uri("/") + .header("x-auth-token", "foo") + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::FORBIDDEN); + } + + #[tokio::test] + #[traced_test] async fn test_get() { let mut federation_mock = MockFederationProvider::default(); federation_mock @@ -399,7 +550,7 @@ mod tests { })) }); - let state = get_mocked_state(federation_mock); + let state = get_mocked_state(federation_mock, true); let mut api = openapi_router() .layer(TraceLayer::new_for_http()) @@ -454,6 +605,45 @@ mod tests { } #[tokio::test] + #[traced_test] + async fn test_get_forbidden() { + let mut federation_mock = MockFederationProvider::default(); + federation_mock + .expect_get_identity_provider() + .withf(|_: &DatabaseConnection, id: &'_ str| id == "bar") + .returning(|_, _| { + Ok(Some(provider_types::IdentityProvider { + id: "bar".into(), + name: "name".into(), + domain_id: Some("did".into()), + default_mapping_name: Some("dummy".into()), + ..Default::default() + })) + }); + + let state = get_mocked_state(federation_mock, false); + + let mut api = openapi_router() + .layer(TraceLayer::new_for_http()) + .with_state(state.clone()); + + let response = api + .as_service() + .oneshot( + Request::builder() + .uri("/bar") + .header("x-auth-token", "foo") + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::FORBIDDEN); + } + + #[tokio::test] + #[traced_test] async fn test_create() { let mut federation_mock = MockFederationProvider::default(); federation_mock @@ -470,7 +660,7 @@ mod tests { }) }); - let state = get_mocked_state(federation_mock); + let state = get_mocked_state(federation_mock, true); let mut api = openapi_router() .layer(TraceLayer::new_for_http()) @@ -510,8 +700,20 @@ mod tests { } #[tokio::test] + #[traced_test] async fn test_update() { let mut federation_mock = MockFederationProvider::default(); + federation_mock + .expect_get_identity_provider() + .withf(|_: &DatabaseConnection, id: &'_ str| id == "1") + .returning(|_, _| { + Ok(Some(provider_types::IdentityProvider { + id: "bar".into(), + name: "name".into(), + domain_id: Some("did".into()), + ..Default::default() + })) + }); federation_mock .expect_update_identity_provider() .withf( @@ -530,7 +732,7 @@ mod tests { }) }); - let state = get_mocked_state(federation_mock); + let state = get_mocked_state(federation_mock, true); let mut api = openapi_router() .layer(TraceLayer::new_for_http()) @@ -565,8 +767,24 @@ mod tests { } #[tokio::test] + #[traced_test] async fn test_delete() { let mut federation_mock = MockFederationProvider::default(); + federation_mock + .expect_get_identity_provider() + .withf(|_: &DatabaseConnection, id: &'_ str| id == "foo") + .returning(|_, _| Ok(None)); + federation_mock + .expect_get_identity_provider() + .withf(|_: &DatabaseConnection, id: &'_ str| id == "bar") + .returning(|_, _| { + Ok(Some(provider_types::IdentityProvider { + id: "bar".into(), + name: "name".into(), + domain_id: Some("did".into()), + ..Default::default() + })) + }); federation_mock .expect_delete_identity_provider() .withf(|_: &DatabaseConnection, id: &'_ str| id == "foo") @@ -581,7 +799,7 @@ mod tests { .withf(|_: &DatabaseConnection, id: &'_ str| id == "bar") .returning(|_, _| Ok(())); - let state = get_mocked_state(federation_mock); + let state = get_mocked_state(federation_mock, true); let mut api = openapi_router() .layer(TraceLayer::new_for_http()) diff --git a/src/api/v3/federation/mapping.rs b/src/api/v3/federation/mapping.rs index e7f08ea7..23af280a 100644 --- a/src/api/v3/federation/mapping.rs +++ b/src/api/v3/federation/mapping.rs @@ -12,12 +12,15 @@ // // SPDX-License-Identifier: Apache-2.0 +//! Federation mappings API use axum::{ Json, debug_handler, extract::{Path, Query, State}, http::StatusCode, response::IntoResponse, }; +use mockall_double::double; +use serde_json::to_value; use utoipa_axum::{router::OpenApiRouter, routes}; use crate::api::auth::Auth; @@ -25,6 +28,17 @@ use crate::api::error::KeystoneApiError; use crate::api::v3::federation::types::*; use crate::federation::FederationApi; use crate::keystone::ServiceState; +#[double] +use crate::policy::Policy; + +pub(crate) static DESCRIPTION: &str = r#"Federation mappings API. + +Mappings define how the user attributes on the remote IDP are mapped to the local user. + +Mappings with an empty domain_id are considered globals and every domain may use it. Such mappings +require the `domain_id_claim` attribute to be set to identify the domain_id for the respective +user. +"#; pub(super) fn openapi_router() -> OpenApiRouter { OpenApiRouter::new() @@ -32,29 +46,41 @@ pub(super) fn openapi_router() -> OpenApiRouter { .routes(routes!(show, update, remove)) } -/// List mappings +/// List federation mappings. +/// +/// List available federation mappings. +/// +/// Without `domain_id` specified global mappings are returned. +/// +/// It is expected that listing mappings belonging to the other domain is only allowed to the admin +/// user. #[utoipa::path( get, path = "/", params(MappingListParameters), - description = "List federation mappings", responses( (status = OK, description = "List of mappings", body = MappingList), (status = 500, description = "Internal error", example = json!(KeystoneApiError::InternalError(String::from("id = 1")))) ), + security(("x-auth" = [])), tag="mappings" )] #[tracing::instrument( name = "api::mapping_list", level = "debug", - skip(state, _user_auth), + skip(state, user_auth, policy), err(Debug) )] async fn list( - Auth(_user_auth): Auth, + Auth(user_auth): Auth, + mut policy: Policy, Query(query): Query, State(state): State, ) -> Result { + policy + .enforce("identity/mapping_list", &user_auth, to_value(&query)?, None) + .await?; + let mappings: Vec = state .provider .get_federation_provider() @@ -72,36 +98,47 @@ async fn list( get, path = "/{idp_id}", description = "Get mapping by ID", - params(), responses( (status = OK, description = "mapping object", body = MappingResponse), (status = 404, description = "mapping not found", example = json!(KeystoneApiError::NotFound(String::from("id = 1")))) ), + security(("x-auth" = [])), tag="mappings" )] #[tracing::instrument( name = "api::mapping_get", level = "debug", - skip(state), + skip(state, user_auth, policy), err(Debug), err(Debug) )] async fn show( Auth(user_auth): Auth, + mut policy: Policy, Path(idp_id): Path, State(state): State, ) -> Result { - state + let current = state .provider .get_federation_provider() .get_mapping(&state.db, &idp_id) .await .map(|x| { x.ok_or_else(|| KeystoneApiError::NotFound { - resource: "identity provider".into(), + resource: "mapping".into(), identifier: idp_id, }) - })? + })??; + + policy + .enforce( + "identity/mapping_show", + &user_auth, + serde_json::to_value(¤t)?, + None, + ) + .await?; + Ok(current) } /// Create mapping @@ -112,15 +149,30 @@ async fn show( responses( (status = CREATED, description = "mapping object", body = MappingResponse), ), + security(("x-auth" = [])), tag="mappings" )] -#[tracing::instrument(name = "api::mapping_create", level = "debug", skip(state))] +#[tracing::instrument( + name = "api::mapping_create", + level = "debug", + skip(state, user_auth, policy) +)] #[debug_handler] async fn create( Auth(user_auth): Auth, + mut policy: Policy, State(state): State, Json(req): Json, ) -> Result { + policy + .enforce( + "identity/mapping_create", + &user_auth, + serde_json::to_value(&req.mapping)?, + None, + ) + .await?; + let res = state .provider .get_federation_provider() @@ -135,20 +187,41 @@ async fn create( put, path = "/{idp_id}", description = "Update existing mapping", - params(), responses( (status = OK, description = "mapping object", body = MappingResponse), (status = 404, description = "mapping not found", example = json!(KeystoneApiError::NotFound(String::from("id = 1")))) ), + security(("x-auth" = [])), tag="mappings" )] -#[tracing::instrument(name = "api::mapping_update", level = "debug", skip(state), err(Debug))] +#[tracing::instrument( + name = "api::mapping_update", + level = "debug", + skip(state, user_auth, policy), + err(Debug) +)] async fn update( Auth(user_auth): Auth, + mut policy: Policy, Path(idp_id): Path, State(state): State, Json(req): Json, ) -> Result { + let current = state + .provider + .get_federation_provider() + .get_mapping(&state.db, &idp_id) + .await?; + + policy + .enforce( + "identity/mapping_update", + &user_auth, + serde_json::to_value(¤t)?, + Some(serde_json::to_value(&req.mapping)?), + ) + .await?; + let res = state .provider .get_federation_provider() @@ -163,25 +236,52 @@ async fn update( delete, path = "/{idp_id}", description = "Delete mapping by ID", - params(), responses( (status = 204, description = "Deleted"), (status = 404, description = "mapping not found", example = json!(KeystoneApiError::NotFound(String::from("id = 1")))) ), + security(("x-auth" = [])), tag="mappings" )] -#[tracing::instrument(name = "api::mapping_delete", level = "debug", skip(state), err(Debug))] +#[tracing::instrument( + name = "api::mapping_delete", + level = "debug", + skip(state, user_auth, policy), + err(Debug) +)] async fn remove( Auth(user_auth): Auth, + mut policy: Policy, Path(id): Path, State(state): State, ) -> Result { - state + let current = state .provider .get_federation_provider() - .delete_mapping(&state.db, &id) - .await - .map_err(KeystoneApiError::federation)?; + .get_mapping(&state.db, &id) + .await?; + + policy + .enforce( + "identity/mapping_delete", + &user_auth, + serde_json::to_value(¤t)?, + None, + ) + .await?; + if current.is_some() { + state + .provider + .get_federation_provider() + .delete_mapping(&state.db, &id) + .await + .map_err(KeystoneApiError::federation)?; + } else { + return Err(KeystoneApiError::NotFound { + resource: "mapping".to_string(), + identifier: id.clone(), + }); + } Ok((StatusCode::NO_CONTENT).into_response()) } @@ -193,22 +293,23 @@ mod tests { }; use http_body_util::BodyExt; // for `collect` use sea_orm::DatabaseConnection; - use std::sync::Arc; use tower::ServiceExt; // for `call`, `oneshot`, and `ready` use tower_http::trace::TraceLayer; + use tracing_test::traced_test; use super::*; use crate::config::Config; - use crate::federation::{ - MockFederationProvider, error::FederationProviderError, types as provider_types, - }; + use crate::federation::{MockFederationProvider, types as provider_types}; use crate::keystone::{Service, ServiceState}; - use crate::policy::{MockPolicy, MockPolicyFactory, PolicyEvaluationResult}; + use crate::policy::{MockPolicy, MockPolicyFactory, PolicyError, PolicyEvaluationResult}; use crate::provider::Provider; use crate::token::{MockTokenProvider, Token, UnscopedPayload}; - fn get_mocked_state(federation_mock: MockFederationProvider) -> ServiceState { + fn get_mocked_state( + federation_mock: MockFederationProvider, + policy_allowed: bool, + ) -> ServiceState { let mut token_mock = MockTokenProvider::default(); token_mock.expect_validate_token().returning(|_, _, _| { Ok(Token::Unscoped(UnscopedPayload { @@ -232,14 +333,23 @@ mod tests { .unwrap(); let mut policy_factory_mock = MockPolicyFactory::default(); - policy_factory_mock.expect_instantiate().returning(|| { - let mut policy_mock = MockPolicy::default(); - policy_mock - .expect_enforce() - .returning(|_, _, _| Ok(PolicyEvaluationResult::allowed())); - Ok(policy_mock) - }); - + if policy_allowed { + policy_factory_mock.expect_instantiate().returning(|| { + let mut policy_mock = MockPolicy::default(); + policy_mock + .expect_enforce() + .returning(|_, _, _, _| Ok(PolicyEvaluationResult::allowed())); + Ok(policy_mock) + }); + } else { + policy_factory_mock.expect_instantiate().returning(|| { + let mut policy_mock = MockPolicy::default(); + policy_mock.expect_enforce().returning(|_, _, _, _| { + Err(PolicyError::Forbidden(PolicyEvaluationResult::forbidden())) + }); + Ok(policy_mock) + }); + } Arc::new( Service::new( Config::default(), @@ -252,6 +362,7 @@ mod tests { } #[tokio::test] + #[traced_test] async fn test_list() { let mut federation_mock = MockFederationProvider::default(); federation_mock @@ -270,7 +381,7 @@ mod tests { }]) }); - let state = get_mocked_state(federation_mock); + let state = get_mocked_state(federation_mock, true); let mut api = openapi_router() .layer(TraceLayer::new_for_http()) @@ -316,6 +427,7 @@ mod tests { } #[tokio::test] + #[traced_test] async fn test_list_qp() { let mut federation_mock = MockFederationProvider::default(); federation_mock @@ -350,7 +462,7 @@ mod tests { }]) }); - let state = get_mocked_state(federation_mock); + let state = get_mocked_state(federation_mock, true); let mut api = openapi_router() .layer(TraceLayer::new_for_http()) @@ -375,6 +487,7 @@ mod tests { } #[tokio::test] + #[traced_test] async fn test_get() { let mut federation_mock = MockFederationProvider::default(); federation_mock @@ -398,7 +511,7 @@ mod tests { })) }); - let state = get_mocked_state(federation_mock); + let state = get_mocked_state(federation_mock, true); let mut api = openapi_router() .layer(TraceLayer::new_for_http()) @@ -458,6 +571,7 @@ mod tests { } #[tokio::test] + #[traced_test] async fn test_create() { let mut federation_mock = MockFederationProvider::default(); federation_mock @@ -472,7 +586,7 @@ mod tests { }) }); - let state = get_mocked_state(federation_mock); + let state = get_mocked_state(federation_mock, true); let mut api = openapi_router() .layer(TraceLayer::new_for_http()) @@ -509,8 +623,21 @@ mod tests { } #[tokio::test] + #[traced_test] async fn test_update() { let mut federation_mock = MockFederationProvider::default(); + federation_mock + .expect_get_mapping() + .withf(|_: &DatabaseConnection, id: &'_ str| id == "1") + .returning(|_, _| { + Ok(Some(provider_types::Mapping { + id: "bar".into(), + name: "name".into(), + domain_id: Some("did".into()), + ..Default::default() + })) + }); + federation_mock .expect_update_mapping() .withf( @@ -527,7 +654,7 @@ mod tests { }) }); - let state = get_mocked_state(federation_mock); + let state = get_mocked_state(federation_mock, true); let mut api = openapi_router() .layer(TraceLayer::new_for_http()) @@ -561,19 +688,30 @@ mod tests { } #[tokio::test] + #[traced_test] async fn test_delete() { let mut federation_mock = MockFederationProvider::default(); federation_mock - .expect_delete_mapping() + .expect_get_mapping() .withf(|_: &DatabaseConnection, id: &'_ str| id == "foo") - .returning(|_, _| Err(FederationProviderError::MappingNotFound("foo".into()))); - + .returning(|_, _| Ok(None)); + federation_mock + .expect_get_mapping() + .withf(|_: &DatabaseConnection, id: &'_ str| id == "bar") + .returning(|_, _| { + Ok(Some(provider_types::Mapping { + id: "bar".into(), + name: "name".into(), + domain_id: Some("did".into()), + ..Default::default() + })) + }); federation_mock .expect_delete_mapping() .withf(|_: &DatabaseConnection, id: &'_ str| id == "bar") .returning(|_, _| Ok(())); - let state = get_mocked_state(federation_mock); + let state = get_mocked_state(federation_mock, true); let mut api = openapi_router() .layer(TraceLayer::new_for_http()) diff --git a/src/api/v3/federation/oidc.rs b/src/api/v3/federation/oidc.rs index 736eddf7..0bb2e566 100644 --- a/src/api/v3/federation/oidc.rs +++ b/src/api/v3/federation/oidc.rs @@ -195,10 +195,7 @@ pub async fn callback( if Url::parse(bound_issuer) .map_err(OidcError::from) .wrap_err_with(|| { - format!( - "while parsing the mapping bound_issuer url: {}", - bound_issuer - ) + format!("while parsing the mapping bound_issuer url: {bound_issuer}") })? == *claims.issuer().url() {} diff --git a/src/api/v3/mod.rs b/src/api/v3/mod.rs index dcecd34a..7a4ba25f 100644 --- a/src/api/v3/mod.rs +++ b/src/api/v3/mod.rs @@ -59,7 +59,7 @@ async fn version( OriginalUri(uri): OriginalUri, req: Request, ) -> Result { - println!("Request: {:?}, uri: {:?}", req, uri); + println!("Request: {req:?}, uri: {uri:?}"); let host = headers .get(header::HOST) .and_then(|header| header.to_str().ok()) diff --git a/src/bin/keystone.rs b/src/bin/keystone.rs index 229ec440..0d967e71 100644 --- a/src/bin/keystone.rs +++ b/src/bin/keystone.rs @@ -66,7 +66,7 @@ impl MakeRequestId for OpenStackRequestId { let req_id = Uuid::new_v4().simple().to_string(); Some(RequestId::new( - http::HeaderValue::from_str(format!("req-{}", req_id).as_str()).unwrap(), + http::HeaderValue::from_str(format!("req-{req_id}").as_str()).unwrap(), )) } } diff --git a/src/policy.rs b/src/policy.rs index 67dad237..73481314 100644 --- a/src/policy.rs +++ b/src/policy.rs @@ -25,12 +25,19 @@ use std::path::Path; use thiserror::Error; use tokio::io::AsyncRead; use tokio::io::AsyncReadExt; -use tracing::debug; +use tracing::{debug, trace}; use crate::token::Token; #[derive(Debug, Error)] pub enum PolicyError { + #[error("{}", .0.violations.as_ref().map( + |v| v.iter().cloned().map(|x| x.msg) + .reduce(|acc, s| format!("{acc}, {s}")) + .unwrap_or_default() + ).unwrap_or("The request you made requires authentication.".into()))] + Forbidden(PolicyEvaluationResult), + #[error("module compilation task crashed")] Compilation(#[from] eyre::Report), @@ -124,6 +131,7 @@ mock! { policy_name: &str, credentials: &Token, target: Value, + current: Option ) -> Result; } } @@ -184,17 +192,26 @@ impl Policy { policy_name: P, credentials: &Token, target: Value, + update: Option, ) -> Result { let input = json!({ "credentials": Credentials::from(credentials), - "target": target + "target": target, + "update": update, }); if let (Some(store), Some(instance)) = (&mut self.store, &self.instance) { + trace!( + "enforcing policy {} with target {target}", + policy_name.as_ref() + ); let [res]: [OpaResponse; 1] = instance .evaluate(store, policy_name.as_ref(), &input) .await?; debug!("Res is {:?}", res); + if !res.result.allow() { + return Err(PolicyError::Forbidden(res.result)); + } Ok(res.result) } else { @@ -208,7 +225,7 @@ impl Policy { } /// A single violation of a policy. -#[derive(Deserialize, Debug, JsonSchema)] +#[derive(Clone, Deserialize, Debug, JsonSchema)] pub struct Violation { pub msg: String, pub field: Option, @@ -221,7 +238,7 @@ pub struct OpaResponse { } /// The result of a policy evaluation. -#[derive(Deserialize, Debug)] +#[derive(Clone, Deserialize, Debug)] pub struct PolicyEvaluationResult { pub allow: bool, #[serde(rename = "violation")] @@ -267,4 +284,12 @@ impl PolicyEvaluationResult { violations: None, } } + + #[cfg(test)] + pub fn forbidden() -> Self { + Self { + allow: false, + violations: None, + } + } } diff --git a/src/tests/api.rs b/src/tests/api.rs index f30b5d63..bc495a1c 100644 --- a/src/tests/api.rs +++ b/src/tests/api.rs @@ -72,7 +72,7 @@ pub(crate) fn get_mocked_state(identity_mock: MockIdentityProvider) -> ServiceSt let mut policy_mock = MockPolicy::default(); policy_mock .expect_enforce() - .returning(|_, _, _| Ok(PolicyEvaluationResult::allowed())); + .returning(|_, _, _, _| Ok(PolicyEvaluationResult::allowed())); Ok(policy_mock) }); diff --git a/src/token/federation_domain_scoped.rs b/src/token/federation_domain_scoped.rs index 108b5c7c..391a33ea 100644 --- a/src/token/federation_domain_scoped.rs +++ b/src/token/federation_domain_scoped.rs @@ -111,7 +111,7 @@ impl MsgPackToken for FederationDomainScopePayload { ) -> Result { // Order of reading is important let user_id = fernet_utils::read_uuid(rd)?; - println!("u: {:?}", user_id); + println!("u: {user_id:?}"); let methods: Vec = fernet::decode_auth_methods(read_pfix(rd)?.into(), auth_map)? .into_iter() .collect(); diff --git a/src/token/federation_project_scoped.rs b/src/token/federation_project_scoped.rs index 9487b096..835bd81b 100644 --- a/src/token/federation_project_scoped.rs +++ b/src/token/federation_project_scoped.rs @@ -111,7 +111,7 @@ impl MsgPackToken for FederationProjectScopePayload { ) -> Result { // Order of reading is important let user_id = fernet_utils::read_uuid(rd)?; - println!("u: {:?}", user_id); + println!("u: {user_id:?}"); let methods: Vec = fernet::decode_auth_methods(read_pfix(rd)?.into(), auth_map)? .into_iter() .collect(); diff --git a/src/token/federation_unscoped.rs b/src/token/federation_unscoped.rs index 4d11b1c1..5bbd598c 100644 --- a/src/token/federation_unscoped.rs +++ b/src/token/federation_unscoped.rs @@ -103,7 +103,7 @@ impl MsgPackToken for FederationUnscopedPayload { ) -> Result { // Order of reading is important let user_id = fernet_utils::read_uuid(rd)?; - println!("u: {:?}", user_id); + println!("u: {user_id:?}"); let methods: Vec = fernet::decode_auth_methods(read_pfix(rd)?.into(), auth_map)? .into_iter() .collect(); diff --git a/src/token/fernet_utils.rs b/src/token/fernet_utils.rs index 41dc4342..9ea3c017 100644 --- a/src/token/fernet_utils.rs +++ b/src/token/fernet_utils.rs @@ -260,9 +260,9 @@ mod tests { async fn test_load_keys() { let tmp_dir = tempdir().unwrap(); for i in 0..5 { - let file_path = tmp_dir.path().join(format!("{}", i)); + let file_path = tmp_dir.path().join(format!("{i}")); let mut tmp_file = File::create(file_path).unwrap(); - write!(tmp_file, "{}", i).unwrap(); + write!(tmp_file, "{i}").unwrap(); } // write dummy file to check it is ignored let file_path = tmp_dir.path().join("dummy");