From 79c90794adc8fd0f81caf27d2e9a5d48cde0f20d Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Tue, 25 Mar 2025 17:57:26 +0000 Subject: [PATCH] api: Don't restrict unknown querystring parameters yet We will address this in a new API version. For now, such a change would be a breaking one. No release note is included since these changes haven't been released yet. Change-Id: I1e862cb1c5e9c218cea59800ff759a1b094b5906 Signed-off-by: Stephen Finucane Closes-Bug: #2104185 (cherry picked from commit 05cc3d1903c1bfe669a1032746aa9e7b197238e9) --- keystone/application_credential/schema.py | 12 +++++++++--- keystone/assignment/schema.py | 8 ++++++-- keystone/catalog/schema.py | 5 ++++- keystone/credential/schema.py | 3 +++ keystone/federation/schema.py | 4 +++- keystone/identity/schema.py | 2 ++ keystone/limit/schema.py | 10 ++++++++-- keystone/resource/schema.py | 7 ++++++- .../tests/unit/test_v3_application_credential.py | 15 ++++++++++++++- keystone/trust/schema.py | 9 +++++++-- 10 files changed, 62 insertions(+), 13 deletions(-) diff --git a/keystone/application_credential/schema.py b/keystone/application_credential/schema.py index 886d567b52..b4bb2da0a1 100644 --- a/keystone/application_credential/schema.py +++ b/keystone/application_credential/schema.py @@ -70,7 +70,9 @@ access_rule_schema: dict[str, Any] = { index_request_query: dict[str, Any] = { "type": "object", "properties": {}, - "additionalProperties": False, + # TODO(stephenfin): Change this to False once we have schemas for all + # resources. Doing so will remove comparator (name__icontains) support. + "additionalProperties": True, } # Response of the `/access_rules` API @@ -92,7 +94,9 @@ rule_index_response_body: dict[str, Any] = { rule_show_request_query: dict[str, Any] = { "type": "object", "properties": {}, - "additionalProperties": False, + # TODO(stephenfin): Change this to False once we have schemas for all + # resources. + "additionalProperties": True, } # Response of `/access_rules/{access_rule_id}` API returning @@ -216,7 +220,9 @@ application_credential_index_request_query: dict[str, Any] = { ), } }, - "additionalProperties": False, + # TODO(stephenfin): Change this to False once we have schemas for all + # resources. Doing so will remove comparator (name__icontains) support. + "additionalProperties": True, } # Response of the `/application_credentials` API diff --git a/keystone/assignment/schema.py b/keystone/assignment/schema.py index 55b9f41015..8479b8c463 100644 --- a/keystone/assignment/schema.py +++ b/keystone/assignment/schema.py @@ -61,7 +61,9 @@ roles_index_request_query: dict[str, Any] = { "name": parameter_types.name, "domain_id": parameter_types.domain_id, }, - "additionalProperties": False, + # TODO(stephenfin): Change this to False once we have schemas for all + # resources. Doing so will remove comparator (name__icontains) support. + "additionalProperties": True, } # Response body of the `GET /roles` API operation @@ -399,7 +401,9 @@ role_assignments_index_request_query: dict[str, Any] = { ] }, "not": {"required": ["effective", "group"]}, - "additionalProperties": False, + # TODO(stephenfin): Change this to False once we have schemas for all + # resources. Doing so will remove comparator (user.id__icontains) support. + "additionalProperties": True, } # Response body of the `GET /role_assignments` API operation diff --git a/keystone/catalog/schema.py b/keystone/catalog/schema.py index 418ecfe443..d1d083125c 100644 --- a/keystone/catalog/schema.py +++ b/keystone/catalog/schema.py @@ -254,7 +254,10 @@ endpoint_index_request_query: dict[str, Any] = { "endpoint belongs", }, }, - "additionalProperties": False, + # TODO(stephenfin): Change this to False once we have schemas for all + # resources. Doing so will remove comparator (interface__icontains) + # support. + "additionalProperties": True, } # Response of the `/endpoints` API diff --git a/keystone/credential/schema.py b/keystone/credential/schema.py index 274fa57cda..0b95c3258a 100644 --- a/keystone/credential/schema.py +++ b/keystone/credential/schema.py @@ -70,6 +70,9 @@ index_request_query: dict[str, Any] = { ), }, }, + # TODO(stephenfin): Change this to False once we have schemas for all + # resources. Doing so will remove comparator (type__icontains) support. + "additionalProperties": True, } # Response of the `/credentials` API diff --git a/keystone/federation/schema.py b/keystone/federation/schema.py index 3a47e134f7..546024c7b8 100644 --- a/keystone/federation/schema.py +++ b/keystone/federation/schema.py @@ -96,7 +96,9 @@ service_provider_index_request_query: dict[str, Any] = { "description": "Whether the service provider is enabled or not", }, }, - "additionalProperties": False, + # TODO(stephenfin): Change this to False once we have schemas for all + # resources. Doing so will remove comparator (id__icontains) support. + "additionalProperties": True, } service_provider_index_response_body: dict[str, Any] = { diff --git a/keystone/identity/schema.py b/keystone/identity/schema.py index 3edb238da2..440d0f833a 100644 --- a/keystone/identity/schema.py +++ b/keystone/identity/schema.py @@ -81,6 +81,8 @@ user_index_request_query: dict[str, Any] = { "sort_key": parameter_types.sort_key, "sort_dir": parameter_types.sort_dir, }, + # TODO(stephenfin): Change this to False once we have schemas for all + # resources. Doing so will remove comparator (name__icontains) support. "additionalProperties": True, } diff --git a/keystone/limit/schema.py b/keystone/limit/schema.py index 6bb0eeb8c5..5d51abd558 100644 --- a/keystone/limit/schema.py +++ b/keystone/limit/schema.py @@ -93,7 +93,10 @@ registered_limits_index_request_query: dict[str, Any] = { **parameter_types.name, }, }, - "additionalProperties": False, + # TODO(stephenfin): Change this to False once we have schemas for all + # resources. Doing so will remove comparator (service_id__icontains) + # support. + "additionalProperties": True, } # Response body of the `GET /registered_limits` API operation @@ -257,7 +260,10 @@ limits_index_request_query: dict[str, Any] = { **parameter_types.domain_id, }, }, - "additionalProperties": False, + # TODO(stephenfin): Change this to False once we have schemas for all + # resources. Doing so will remove comparator (project_id__icontains) + # support. + "additionalProperties": True, } # Response body of the `GET /limits` API operation diff --git a/keystone/resource/schema.py b/keystone/resource/schema.py index c8a7ab3680..f19c4742c1 100644 --- a/keystone/resource/schema.py +++ b/keystone/resource/schema.py @@ -100,6 +100,9 @@ project_index_request_query = { }, "limit": {"type": ["integer", "string"]}, }, + # TODO(stephenfin): Change this to False once we have schemas for all + # resources. Doing so will remove comparator (name__icontains) support. + "additionalProperties": True, } project_schema: dict[str, Any] = { @@ -208,7 +211,9 @@ domain_index_request_query: dict[str, Any] = { }, "limit": {"type": ["integer", "string"]}, }, - "additionalProperties": "False", + # TODO(stephenfin): Change this to False once we have schemas for all + # resources. Doing so will remove comparator (name__icontains) support. + "additionalProperties": True, } domain_index_response_body: dict[str, Any] = { diff --git a/keystone/tests/unit/test_v3_application_credential.py b/keystone/tests/unit/test_v3_application_credential.py index 0ca075807c..df0c12095d 100644 --- a/keystone/tests/unit/test_v3_application_credential.py +++ b/keystone/tests/unit/test_v3_application_credential.py @@ -12,6 +12,7 @@ import datetime import http.client +import unittest import uuid from oslo_utils import timeutils @@ -742,7 +743,10 @@ class ApplicationCredentialTestCase(test_v3.RestfulTestCase): ar = r.json["access_rules"] self.assertEqual(access_rules[0]["method"], ar[0]["method"]) - def test_list_access_rules_wrong_qp(self): + # TODO(stephenfin): This will pass once we increase strictness of the query + # string validation + @unittest.expectedFailure + def test_list_access_rules_invalid_qs(self): with self.test_client() as c: token = self.get_scoped_token() # Invoke GET access_rules with unsupported query parameters and @@ -778,6 +782,15 @@ class ApplicationCredentialTestCase(test_v3.RestfulTestCase): expected_status_code=http.client.OK, headers={"X-Auth-Token": token}, ) + + # TODO(stephenfin): This will pass once we increase strictness of the query + # string validation + @unittest.expectedFailure + def test_show_access_rule_invalid_qs(self): + with self.test_client() as c: + token = self.get_scoped_token() + # Invoke GET access_rules/{id} with unsupported query parameters and + # trigger internal validation c.get( f"/v3/users/{self.user_id}/access_rules/{access_rule_id}" "?foo=bar", diff --git a/keystone/trust/schema.py b/keystone/trust/schema.py index 3d34987857..20c85b3907 100644 --- a/keystone/trust/schema.py +++ b/keystone/trust/schema.py @@ -170,7 +170,10 @@ trust_index_request_query: dict[str, Any] = { "the trust.", }, }, - "additionalProperties": False, + # TODO(stephenfin): Change this to False once we have schemas for all + # resources. Doing so will remove comparator (trustor_user_id__icontains) + # support. + "additionalProperties": True, } trust_index_response_body: dict[str, Any] = { @@ -190,7 +193,9 @@ trust_index_response_body: dict[str, Any] = { trust_request_query: dict[str, Any] = { "type": "object", "properties": {}, - "additionalProperties": False, + # TODO(stephenfin): Change this to False once we have schemas for all + # resources. + "additionalProperties": True, } trust_response_body: dict[str, Any] = {