From 5b468e0f9a26c248ddfb1cc89ece280927bb2d55 Mon Sep 17 00:00:00 2001 From: Tristan Moreno Date: Tue, 4 May 2021 10:20:46 +0200 Subject: [PATCH] service_id and plan_id checks --- CHANGELOG.rst | 2 ++ openbrokerapi/api.py | 52 ++++++++++++++++----------- tests/test_bind.py | 34 ++++++++++++++++++ tests/test_deprovisioning.py | 26 ++++++++++++++ tests/test_provisioning.py | 48 +++++++++++++++++++++++-- tests/test_unbind.py | 24 +++++++++++++ tests/test_update.py | 68 ++++++++++++++++++++++++++++++++++++ 7 files changed, 231 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 4395593..84f9360 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -1,6 +1,8 @@ Changelog ============= + - check service_id + **v4.1** - Drop Python 3.5 support - Fix ´bind´ in multi broker setup (#117); thx @vaxvms diff --git a/openbrokerapi/api.py b/openbrokerapi/api.py index f6e515f..ff8decc 100644 --- a/openbrokerapi/api.py +++ b/openbrokerapi/api.py @@ -1,7 +1,7 @@ import logging from http import HTTPStatus from json.decoder import JSONDecodeError -from typing import List, Union +from typing import List, Optional, Union from flask import Blueprint, Request from flask import json, request @@ -52,16 +52,24 @@ def __init__(self, username: str, password: str): self.password = password -def _check_plan_id(broker: ServiceBroker, plan_id) -> bool: +def _check_service_plan_ids(service_broker: ServiceBroker, service_id: str, plan_id: Optional[str]) -> str: """ - Checks that the plan_id exists in the catalog - :return: boolean + Checks that the service_id and plan_id exist in the catalog of the supplied service broker. + + :param ServiceBroker service_broker: Services that this broker exposes + :param str service_id: service identifier + :param Optional[str] plan_id: plan identifier + :return str: Empty string on success, or "service_id" if supplied service identifier was not found, or "plan_id" if supplied plan identifier was not found. """ - for service in ensure_list(broker.catalog()): - for plan in service.plans: - if plan.id == plan_id: - return True - return False + for service in ensure_list(service_broker.catalog()): + if service.id == service_id: + if plan_id is None: + return "" + for plan in service.plans: + if plan.id == plan_id: + return "" + return "plan_id" + return "service_id" def get_blueprint(service_broker: ServiceBroker, @@ -142,8 +150,9 @@ def provision(instance_id): provision_details.originating_identity = request.originating_identity provision_details.authorization_username = extract_authorization_username(request) - if not _check_plan_id(service_broker, provision_details.plan_id): - raise TypeError('plan_id not found in this service.') + check = _check_service_plan_ids(service_broker, provision_details.service_id, provision_details.plan_id) + if check: + raise TypeError(f'{check} not found in this service broker.') except (TypeError, KeyError, JSONDecodeError) as e: logger.exception(e) return to_json_response(ErrorResponse(description=str(e))), HTTPStatus.BAD_REQUEST @@ -180,9 +189,9 @@ def update(instance_id): update_details = UpdateDetails(**json.loads(request.data)) update_details.originating_identity = request.originating_identity update_details.authorization_username = extract_authorization_username(request) - plan_id = update_details.plan_id - if plan_id and not _check_plan_id(service_broker, plan_id): - raise TypeError('plan_id not found in this service.') + check = _check_service_plan_ids(service_broker, update_details.service_id, update_details.plan_id) + if check: + raise TypeError(f'{check} not found in this service broker.') except (TypeError, KeyError, JSONDecodeError) as e: logger.exception(e) return to_json_response(ErrorResponse(description=str(e))), HTTPStatus.BAD_REQUEST @@ -212,8 +221,9 @@ def bind(instance_id, binding_id): binding_details = BindDetails(**json.loads(request.data)) binding_details.originating_identity = request.originating_identity binding_details.authorization_username = extract_authorization_username(request) - if not _check_plan_id(service_broker, binding_details.plan_id): - raise TypeError('plan_id not found in this service.') + check = _check_service_plan_ids(service_broker, binding_details.service_id, binding_details.plan_id) + if check: + raise TypeError(f'{check} not found in this service broker.') except (TypeError, KeyError, JSONDecodeError) as e: logger.exception(e) return to_json_response(ErrorResponse(description=str(e))), HTTPStatus.BAD_REQUEST @@ -256,8 +266,9 @@ def unbind(instance_id, binding_id): unbind_details = UnbindDetails(service_id=service_id, plan_id=plan_id) unbind_details.originating_identity = request.originating_identity unbind_details.authorization_username = extract_authorization_username(request) - if not _check_plan_id(service_broker, unbind_details.plan_id): - raise TypeError('plan_id not found in this service.') + check = _check_service_plan_ids(service_broker, unbind_details.service_id, unbind_details.plan_id) + if check: + raise TypeError(f'{check} not found in this service broker.') except (TypeError, KeyError) as e: logger.exception(e) return to_json_response(ErrorResponse(description=str(e))), HTTPStatus.BAD_REQUEST @@ -283,8 +294,9 @@ def deprovision(instance_id): deprovision_details = DeprovisionDetails(service_id=service_id, plan_id=plan_id) deprovision_details.originating_identity = request.originating_identity deprovision_details.authorization_username = extract_authorization_username(request) - if not _check_plan_id(service_broker, deprovision_details.plan_id): - raise TypeError('plan_id not found in this service.') + check = _check_service_plan_ids(service_broker, deprovision_details.service_id, deprovision_details.plan_id) + if check: + raise TypeError(f'{check} not found in this service broker.') except (TypeError, KeyError) as e: logger.exception(e) return to_json_response(ErrorResponse(description=str(e))), HTTPStatus.BAD_REQUEST diff --git a/tests/test_bind.py b/tests/test_bind.py index a178f60..bc19504 100644 --- a/tests/test_bind.py +++ b/tests/test_bind.py @@ -326,3 +326,37 @@ def test_returns_200_if_identical_binding_already_exists(self): self.assertEqual(response.status_code, http.HTTPStatus.OK) self.assertEqual(response.json, dict()) + + def test_returns_400_wrong_service_id(self): + response = self.client.put( + "/v2/service_instances/here-instance_id/service_bindings/here-binding_id", + data=json.dumps({ + "service_id": "wrong-service-guid-here", + "plan_id": "plan-guid-here", + "bind_resource": {} + }), + headers={ + 'X-Broker-Api-Version': '2.13', + 'Content-Type': 'application/json', + 'Authorization': self.auth_header + }) + + self.assertEqual(response.status_code, http.HTTPStatus.BAD_REQUEST) + self.assertEqual(response.json, dict(description="service_id not found in this service broker.")) + + def test_returns_400_wrong_plan_id(self): + response = self.client.put( + "/v2/service_instances/here-instance_id/service_bindings/here-binding_id", + data=json.dumps({ + "service_id": "service-guid-here", + "plan_id": "wrong-plan-guid-here", + "bind_resource": {} + }), + headers={ + 'X-Broker-Api-Version': '2.13', + 'Content-Type': 'application/json', + 'Authorization': self.auth_header + }) + + self.assertEqual(response.status_code, http.HTTPStatus.BAD_REQUEST) + self.assertEqual(response.json, dict(description="plan_id not found in this service broker.")) diff --git a/tests/test_deprovisioning.py b/tests/test_deprovisioning.py index c9ca208..77760a8 100644 --- a/tests/test_deprovisioning.py +++ b/tests/test_deprovisioning.py @@ -119,3 +119,29 @@ def test_returns_401_if_request_not_contain_auth_header(self): }) self.assertEqual(response.status_code, http.HTTPStatus.UNAUTHORIZED) + + def test_returns_400_with_wrong_service_id(self): + response = self.client.delete( + "/v2/service_instances/abc?service_id=wrong-service-guid-here&plan_id=plan-guid-here", + headers={ + 'X-Broker-Api-Version': '2.13', + 'Authorization': self.auth_header + }) + + self.assertEqual(response.status_code, http.HTTPStatus.BAD_REQUEST) + self.assertEqual(response.json, dict( + description="service_id not found in this service broker." + )) + + def test_returns_400_with_wrong_plan_id(self): + response = self.client.delete( + "/v2/service_instances/abc?service_id=service-guid-here&plan_id=wrong-plan-guid-here", + headers={ + 'X-Broker-Api-Version': '2.13', + 'Authorization': self.auth_header + }) + + self.assertEqual(response.status_code, http.HTTPStatus.BAD_REQUEST) + self.assertEqual(response.json, dict( + description="plan_id not found in this service broker." + )) diff --git a/tests/test_provisioning.py b/tests/test_provisioning.py index e4374cc..d148804 100644 --- a/tests/test_provisioning.py +++ b/tests/test_provisioning.py @@ -57,7 +57,7 @@ def test_provisioning_called_with_the_right_values(self): self.assertEqual(actual_details.context["organization_guid"], "org-guid-here") self.assertEqual(actual_details.context["space_guid"], "space-guid-here") - def test_provisining_called_just_with_required_fields(self): + def test_provisioning_called_just_with_required_fields(self): self.broker.provision.return_value = ProvisionedServiceSpec(dashboard_url="dash_url", operation="operation_str") self.client.put( @@ -88,7 +88,7 @@ def test_provisining_called_just_with_required_fields(self): self.assertIsNone(actual_details.parameters) - def test_provisining_optional_org_and_space_if_available_in_context(self): + def test_provisioning_optional_org_and_space_if_available_in_context(self): self.broker.provision.return_value = ProvisionedServiceSpec(dashboard_url="dash_url", operation="operation_str") self.client.put( @@ -117,7 +117,7 @@ def test_provisining_optional_org_and_space_if_available_in_context(self): self.assertIsNone(actual_details.parameters) - def test_provisining_ignores_unknown_parameters(self): + def test_provisioning_ignores_unknown_parameters(self): self.broker.provision.return_value = ProvisionedServiceSpec(dashboard_url="dash_url", operation="operation_str") self.client.put( @@ -421,3 +421,45 @@ def test_returns_400_if_context_space_guid_mismatch(self): self.assertEqual(response.status_code, http.HTTPStatus.BAD_REQUEST) self.assertEqual(response.json, dict(description="space_guid does not match with context.space_guid")) + + def test_returns_400_with_wrong_service_id(self): + response = self.client.put( + "/v2/service_instances/here-instance-id?accepts_incomplete=true", + data=json.dumps({ + "service_id": "wrong-service-guid-here", + "plan_id": "plan-guid-here", + "organization_guid": "org-guid-here", + "space_guid": "space-guid-here", + "parameters": { + "parameter1": 1 + }, + }), + headers={ + 'X-Broker-Api-Version': '2.13', + 'Content-Type': 'application/json', + 'Authorization': self.auth_header + }) + + self.assertEqual(response.status_code, http.HTTPStatus.BAD_REQUEST) + self.assertEqual(response.json, dict(description="service_id not found in this service broker.")) + + def test_returns_400_with_wrong_plan_id(self): + response = self.client.put( + "/v2/service_instances/here-instance-id?accepts_incomplete=true", + data=json.dumps({ + "service_id": "service-guid-here", + "plan_id": "wrong-plan-guid-here", + "organization_guid": "org-guid-here", + "space_guid": "space-guid-here", + "parameters": { + "parameter1": 1 + }, + }), + headers={ + 'X-Broker-Api-Version': '2.13', + 'Content-Type': 'application/json', + 'Authorization': self.auth_header + }) + + self.assertEqual(response.status_code, http.HTTPStatus.BAD_REQUEST) + self.assertEqual(response.json, dict(description="plan_id not found in this service broker.")) diff --git a/tests/test_unbind.py b/tests/test_unbind.py index af7315e..b2239fe 100644 --- a/tests/test_unbind.py +++ b/tests/test_unbind.py @@ -91,3 +91,27 @@ def test_returns_400_if_request_not_contain_auth_header(self): }) self.assertEqual(response.status_code, http.HTTPStatus.UNAUTHORIZED) + + def test_returns_400_wrong_service_id(self): + query = "service_id=wrong-service-guid-here&plan_id=plan-guid-here" + response = self.client.delete( + "/v2/service_instances/here_instance_id/service_bindings/here_binding_id?%s" % query, + headers={ + 'X-Broker-Api-Version': '2.13', + 'Authorization': self.auth_header + }) + + self.assertEqual(response.status_code, http.HTTPStatus.BAD_REQUEST) + self.assertEqual(response.json, dict(description="service_id not found in this service broker.")) + + def test_returns_400_wrong_plan_id(self): + query = "service_id=service-guid-here&plan_id=wrong-plan-guid-here" + response = self.client.delete( + "/v2/service_instances/here_instance_id/service_bindings/here_binding_id?%s" % query, + headers={ + 'X-Broker-Api-Version': '2.13', + 'Authorization': self.auth_header + }) + + self.assertEqual(response.status_code, http.HTTPStatus.BAD_REQUEST) + self.assertEqual(response.json, dict(description="plan_id not found in this service broker.")) diff --git a/tests/test_update.py b/tests/test_update.py index d6f39c8..09a96ee 100644 --- a/tests/test_update.py +++ b/tests/test_update.py @@ -286,3 +286,71 @@ def test_returns_400_if_request_does_not_contain_valid_json_body(self): self.assertEqual(response.status_code, http.HTTPStatus.BAD_REQUEST) self.assertEqual(response.json, dict(description="Improper Content-Type header. Expecting \"application/json\"")) + + def test_returns_400_if_unknown_service_id(self): + response = self.client.patch( + "/v2/service_instances/abc", + data=json.dumps({ + "service_id": "wrong-service-guid-here", + "plan_id": "plan-guid-here", + "previous_values": { + "plan_id": "old-plan-guid-here", + "service_id": "service-guid-here", + "organization_id": "org-guid-here", + "space_id": "space-guid-here" + } + }), + headers={ + 'X-Broker-Api-Version': '2.13', + 'Content-Type': 'application/json', + 'Authorization': self.auth_header + }) + + self.assertEqual(response.status_code, http.HTTPStatus.BAD_REQUEST) + self.assertEqual(response.json, + dict(description="service_id not found in this service broker.")) + + def test_returns_400_if_unknown_plan_id(self): + response = self.client.patch( + "/v2/service_instances/abc", + data=json.dumps({ + "service_id": "service-guid-here", + "plan_id": "wrong-plan-guid-here", + "previous_values": { + "plan_id": "old-plan-guid-here", + "service_id": "service-guid-here", + "organization_id": "org-guid-here", + "space_id": "space-guid-here" + } + }), + headers={ + 'X-Broker-Api-Version': '2.13', + 'Content-Type': 'application/json', + 'Authorization': self.auth_header + }) + + self.assertEqual(response.status_code, http.HTTPStatus.BAD_REQUEST) + self.assertEqual(response.json, + dict(description="plan_id not found in this service broker.")) + + def test_returns_200_without_plan_id(self): + self.broker.update.return_value = UpdateServiceSpec(False, "operation") + + response = self.client.patch( + "/v2/service_instances/abc", + data=json.dumps({ + "service_id": "service-guid-here", + "previous_values": { + "service_id": "service-guid-here", + "organization_id": "org-guid-here", + "space_id": "space-guid-here" + } + }), + headers={ + 'X-Broker-Api-Version': '2.13', + 'Content-Type': 'application/json', + 'Authorization': self.auth_header + }) + + self.assertEqual(response.status_code, http.HTTPStatus.OK) + self.assertEqual(response.json, dict())