Merge pull request #1360 from cloudfoundry-incubator/v3-create-servic…

…e-broker-support-additional-auths-166266507

Make POST /v3/service_brokers extensible to support different authentication types

app/actions/service_broker_create.rb

@@ -6,12 +6,12 @@ def initialize(service_event_repository, service_manager)
         @service_manager = service_manager
       end
 
-      def create(credentials)
+      def create(message)
         broker = VCAP::CloudController::ServiceBroker.create(
-          name: credentials[:name],
-          broker_url: credentials[:url],
-          auth_username: credentials[:username],
-          auth_password: credentials[:password],
+          name: message.name,
+          broker_url: message.url,
+          auth_username: message.credentials_data.username,
+          auth_password: message.credentials_data.password,
         )
 
         registration = VCAP::Services::ServiceBrokers::ServiceBrokerRegistration.new(

app/controllers/v3/service_brokers_controller.rb

@@ -1,4 +1,5 @@
 require 'messages/service_brokers_list_message'
+require 'messages/service_broker_create_message'
 require 'presenters/v3/service_broker_presenter'
 require 'fetchers/service_broker_list_fetcher'
 require 'actions/service_broker_create'
@@ -35,14 +36,15 @@ def show
   end
 
   def create
+    message = ServiceBrokerCreateMessage.new(hashed_params[:body])
+    unprocessable!(message.errors.full_messages) unless message.valid?
     unauthorized! unless permission_queryer.can_write_service_broker?
 
     service_event_repository = VCAP::CloudController::Repositories::ServiceEventRepository::WithBrokerActor.new
     service_manager = VCAP::Services::ServiceBrokers::ServiceManager.new(service_event_repository)
     service_broker_create = VCAP::CloudController::V3::ServiceBrokerCreate.new(service_event_repository, service_manager)
 
-    credentials = params[:body].permit(:name, :url, :username, :password)
-    result = service_broker_create.create(credentials)
+    result = service_broker_create.create(message)
 
     add_warning_headers(result[:warnings])
     render status: :created, json: {}

app/messages/service_broker_create_message.rb

@@ -0,0 +1,45 @@
+require 'messages/base_message'
+require 'utils/hash_utils'
+
+module VCAP::CloudController
+  class ServiceBrokerCreateMessage < BaseMessage
+    register_allowed_keys [:name, :url, :credentials]
+    ALLOWED_CREDENTIAL_TYPES = ['basic'].freeze
+
+    validates_with NoAdditionalKeysValidator
+
+    validates :name, string: true
+    validates :url, string: true
+    validates :credentials, hash: true
+    validates_inclusion_of :credentials_type, in: ALLOWED_CREDENTIAL_TYPES,
+      message: "credentials.type must be one of #{ALLOWED_CREDENTIAL_TYPES}"
+
+    validate :validate_credentials_data
+
+    def credentials_type
+      HashUtils.dig(credentials, :type)
+    end
+
+    def credentials_data
+      @credentials_data ||= BasicCredentialsMessage.new(HashUtils.dig(credentials, :data))
+    end
+
+    def validate_credentials_data
+      unless credentials_data.valid?
+        errors.add(
+          :credentials_data,
+          "Field(s) #{credentials_data.errors.keys.map(&:to_s)} must be valid: #{credentials_data.errors.full_messages}"
+        )
+      end
+    end
+
+    class BasicCredentialsMessage < BaseMessage
+      register_allowed_keys [:username, :password]
+
+      validates_with NoAdditionalKeysValidator
+
+      validates :username, string: true
+      validates :password, string: true
+    end
+  end
+end

docs/v3/source/includes/experimental_resources/service_brokers/_create.md.erb

@@ -0,0 +1,69 @@
+### Create a service broker
+
+```
+Example Request
+```
+
+```shell
+curl "https://api.example.org/v3/service_brokers" \
+  -X POST \
+  -H "Authorization: bearer [token]" \
+  -H "Content-type: application/json" \
+  -d '{
+    "name": "my_service_broker",
+    "url": "https://example.service-broker.com",
+    "credentials": {
+      "type": "basic",
+      "data": {
+        "username": "us3rn4me",
+        "password": "p4ssw0rd"
+      }
+    },
+    "relationships": {
+      "space": {
+        "data": {
+          "guid": "2f35885d-0c9d-4423-83ad-fd05066f8576"
+        }
+      }
+    }
+  }'
+```
+
+```
+Example Response
+```
+
+```http
+HTTP/1.1 201 Created
+Content-Type: application/json
+
+<%= yield_content :single_service_broker %>
+```
+
+This endpoint creates a new service broker.
+
+#### Definition
+`POST /v3/service_brokers`
+
+#### Required Parameters
+
+Name | Type | Description
+---- | ---- | -----------
+**name** | _string_ | Name of the service broker.
+**url** | _string_ | URL of the service broker.
+**credentials** | [_credentials_](#the-credentials-object) | Credentials used to authenticate against the service broker.
+
+#### Optional Parameters
+
+Name | Type | Description | Default
+---- | ---- | ----------- | -------
+**space** | [_to-one relationship_](#to-one-relationships) | If set, restricts broker visibility to the specified space. | `null`
+
+<%= yield_content :service_broker_credentials_object %>
+
+#### Permitted Roles
+ |
+--- | ---
+Space Developer |
+Admin |
+

docs/v3/source/index.md

@@ -238,6 +238,7 @@ includes:
   - experimental_resources/service_brokers/object
   - experimental_resources/service_brokers/get
   - experimental_resources/service_brokers/list
+  - experimental_resources/service_brokers/create
   - experimental_resources/sidecars/header
   - experimental_resources/sidecars/object
   - experimental_resources/sidecars/create_from_app

spec/lightweight_spec_helper.rb

@@ -0,0 +1,5 @@
+$LOAD_PATH.push(File.expand_path(File.join(__dir__, '..', 'app')))
+$LOAD_PATH.push(File.expand_path(File.join(__dir__, '..', 'lib')))
+
+# So that specs using this helper don't fail with undefined constant error
+module VCAP; end

spec/request/service_brokers_spec.rb

@@ -38,6 +38,20 @@ def catalog
     }
   end
 
+  let(:valid_service_broker_create_body) do
+    {
+      name: 'broker name',
+      url: 'http://example.org/broker-url',
+      credentials: {
+        type: 'basic',
+        data: {
+          username: 'admin',
+          password: 'welcome',
+        }
+      }
+    }
+  end
+
   context 'as an admin user' do
     describe 'getting a single service broker' do
       context 'when there are no service brokers' do
@@ -355,13 +369,10 @@ def catalog
     end
 
     describe 'registering a global service broker' do
+      let(:request_body) { valid_service_broker_create_body }
+
       subject do
-        post('/v3/service_brokers', {
-          name: 'broker name',
-          url: 'http://example.org/broker-url',
-          username: 'admin',
-          password: 'welcome',
-        }.to_json, admin_headers)
+        post('/v3/service_brokers', request_body.to_json, admin_headers)
       end
 
       before do
@@ -451,6 +462,22 @@ def catalog
           end
         end
       end
+
+      context 'when user provides a malformed request' do
+        let(:request_body) do
+          {
+            whatever: 'oopsie'
+          }
+        end
+
+        it 'responds with a helpful error message' do
+          subject
+
+          expect(last_response).to have_status_code(422)
+          expect(last_response.body).to include('UnprocessableEntity')
+          expect(last_response.body).to include('Name must be a string')
+        end
+      end
     end
   end
 
@@ -517,12 +544,7 @@ def catalog
 
     describe 'registering a global service broker' do
       it 'fails authorization' do
-        response = post('/v3/service_brokers', {
-          name: 'broker name',
-          url: 'http://example.org/broker-url',
-          username: 'admin',
-          password: 'welcome',
-        }.to_json, headers_for(user))
+        response = post('/v3/service_brokers', valid_service_broker_create_body.to_json, headers_for(user))
 
         expect(response).to have_status_code(403)
       end
@@ -583,12 +605,7 @@ def catalog
 
     describe 'registering a global service broker' do
       it 'fails authorization' do
-        response = post('/v3/service_brokers', {
-          name: 'broker name',
-          url: 'http://example.org/broker-url',
-          username: 'admin',
-          password: 'welcome',
-        }.to_json, headers_for(user))
+        response = post('/v3/service_brokers', valid_service_broker_create_body.to_json, headers_for(user))
 
         expect(response).to have_status_code(403)
       end

spec/unit/actions/service_broker_create_spec.rb

@@ -7,13 +7,23 @@ module VCAP::CloudController
     let(:service_manager) { double }
     let(:registration) { instance_double(VCAP::Services::ServiceBrokers::ServiceBrokerRegistration) }
     let(:warnings) { [] }
-    let(:result) { V3::ServiceBrokerCreate.new(service_event_repository, service_manager).create(
-      name: 'broker name',
-      url: 'http://example.org/broker-url',
-      username: 'broker username',
-      password: 'broker password',
-    )
-    }
+
+    let(:message) do
+      ServiceBrokerCreateMessage.new(
+        name: 'broker name',
+        url: 'http://example.org/broker-url',
+        credentials: {
+          type: 'basic',
+          data: {
+            username: 'broker username',
+            password: 'broker password',
+          }
+        }
+      )
+    end
+
+    let(:result) { V3::ServiceBrokerCreate.new(service_event_repository, service_manager).create(message) }
+
     let(:service_broker) { ServiceBroker.last }
 
     before do