Merge pull request #6761 from farias-r7/bug/MS-1361/spec-failures-address-validation

Bug/ms 1361/spec failures address validation

app/concerns/mdm/workspace/boundary_range.rb

@@ -38,12 +38,31 @@ module Mdm::Workspace::BoundaryRange
 
     # Validates that {#boundary} is {#valid_ip_or_range? a valid IP address or
     # IP address range}. Due to this not being tested before it was moved here
-    # from Mdm, the default workspace does not validate. We therefore don't
-    # validate boundaries of workspaces that don't use them.
+    # from Mdm, the default workspace does not validate. We always validate boundaries
+    # and a workspace may have a blank default boundary.
     #
     # @return [void]
     def boundary_must_be_ip_range
-      errors.add(:boundary, "must be a valid IP range") unless !limit_to_network || valid_ip_or_range?(boundary)
+      unless boundary.blank?
+        begin
+          boundaries = Shellwords.split(boundary)
+        rescue ArgumentError
+          boundaries = []
+        end
+
+        boundaries.each do |range|
+          unless valid_ip_or_range?(range)
+            errors.add(:boundary, "must be a valid IP range")
+          end
+        end
+      end
+    end
+
+    # Returns an array of addresses ranges
+    #
+    # @return [Array<String>]
+    def addresses
+      (boundary || "").split("\n")
     end
 
     private
@@ -56,5 +75,8 @@ module Mdm::Workspace::BoundaryRange
       range = Rex::Socket::RangeWalker.new(string)
       range && range.ranges && range.ranges.any?
     end
+
   end
+
+
 end

spec/models/mdm/workspace_spec.rb

@@ -20,8 +20,8 @@ RSpec.describe Mdm::Workspace, type: :model do
           workspace.valid?
         end
 
-        it 'should validate using #valid_ip_or_range?' do
-          expect(workspace).to receive(:valid_ip_or_range?).with(boundary).and_return(false)
+        it 'should validate using #boundary_must_be_ip_range' do
+          expect(workspace).to receive(:boundary_must_be_ip_range).and_return(false)
 
           workspace.valid?
         end
@@ -70,4 +70,37 @@ RSpec.describe Mdm::Workspace, type: :model do
       end
     end
   end
+
+  context 'methods' do
+    context '#valid_ip_or_range?' do
+      let(:ip_or_range) do
+        nil
+      end
+
+      subject do
+        -> {workspace.send(:valid_ip_or_range?, ip_or_range)}
+      end
+
+      context 'with exception from Rex::Socket::RangeWalker' do
+        before(:example) do
+          allow(Rex::Socket::RangeWalker).to receive(:new).with(ip_or_range).and_raise(StandardError)
+        end
+
+        it { is_expected.to raise_error(StandardError) }
+      end
+
+      context 'without exception from Rex::Socket::RangeWalker' do
+        context 'with valid IP' do
+          let(:ip_or_range) do
+            '192.168.0.1'
+          end
+
+          it { is_expected.to be_truthy }
+        end
+      end
+    end
+  end
+
+
+  it_should_behave_like 'Mdm::Workspace::Boundary'
 end

spec/support/shared/examples/mdm/workspace/boundary.rb

@@ -0,0 +1,134 @@
+RSpec.shared_examples_for 'Mdm::Workspace::Boundary' do
+  context 'methods' do
+    let(:boundary) do
+      nil
+    end
+
+    before(:example) do
+      workspace.boundary = boundary
+    end
+
+    context '#addresses' do
+      subject(:addresses) do
+        workspace.addresses
+      end
+
+      context 'with boundary' do
+        let(:boundary) do
+          expected_addresses.join("\n")
+        end
+
+        let(:expected_addresses) do
+          [
+            '10,10,10,10',
+            '192.168.0.1'
+          ]
+        end
+
+        it 'should return addresses split on newline' do
+          expect(addresses).to eq(expected_addresses)
+        end
+      end
+
+      context 'without boundary' do
+        let(:boundary) do
+          nil
+        end
+
+        it 'should return an empty Array' do
+          expect(addresses).to eq([])
+        end
+      end
+    end
+
+    context '#boundary_must_be_ip_range' do
+      let(:error) do
+        'must be a valid IP range'
+      end
+
+      context 'with boundary' do
+        let(:boundary) do
+          '192.168.0.1'
+        end
+
+        it 'should split boundary' do
+          expect(Shellwords).to receive(:split).with(boundary).and_call_original
+
+          workspace.valid?
+        end
+
+        context 'with error from Shellwords.split' do
+          before(:example) do
+            allow(Shellwords).to receive(:split).with(boundary).and_raise(ArgumentError)
+          end
+
+          it 'should not raise error' do
+            expect {
+              workspace.valid?
+            }.to_not raise_error
+          end
+
+          it 'should not record an error' do
+            workspace.valid?
+
+            expect(workspace.errors[:boundary]).not_to include(error)
+          end
+        end
+
+        context 'with empty' do
+          let(:boundary) do
+            ''
+          end
+
+          it 'should not record an error' do
+            workspace.valid?
+
+            expect(workspace.errors[:boundary]).not_to include(error)
+          end
+        end
+
+        context 'without empty' do
+          let(:ranges) do
+            [
+              '10.10.10.10',
+              '192.168.0.1'
+            ]
+          end
+
+          let(:boundary) do
+            ranges.join(' ')
+          end
+
+          it 'should validate each range' do
+            ranges.each do |range|
+              expect(workspace).to receive(:valid_ip_or_range?).with(range).and_call_original
+            end
+
+            workspace.valid?
+          end
+
+          context 'with invalid range' do
+            let(:ranges) do
+              [
+                '192.168'
+              ]
+            end
+
+            it 'should record error', :skip => 'https://www.pivotaltracker.com/story/show/43171927' do
+              expect(workspace).not_to be_valid
+              expect(workspace.errors[:boundary]).to include(error)
+            end
+          end
+        end
+      end
+
+      context 'without boundary' do
+        it 'should not record error' do
+          workspace.valid?
+
+          expect(workspace.errors[:boundary]).not_to include(error)
+        end
+      end
+    end
+  end
+end