service_id and plan_id checks

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

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

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."))

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."
+ ))

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."))

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."))

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())