Merge pull request #481 from dylanegan/security_group_fixes

Security group fixes

lib/fog/compute/requests/aws/authorize_security_group_ingress.rb

@@ -62,10 +62,10 @@ module Fog
 
           if group
             group['ipPermissions'] ||= []
-            if group_name && options['SourceSecurityGroupName'] && options['SourceSecurityGroupOwnerId']
+            if group_name && source_group_name = options['SourceSecurityGroupName']
               ['tcp', 'udp'].each do |protocol|
                 group['ipPermissions'] << {
-                  'groups'      => [{'groupName' => group_name, 'userId' => self.data[:owner_id]}],
+                  'groups'      => [{'groupName' => source_group_name, 'userId' => (options['SourceSecurityGroupOwnerId'] || self.data[:owner_id]) }],
                   'fromPort'    => 1,
                   'ipRanges'    => [],
                   'ipProtocol'  => protocol,
@@ -73,7 +73,7 @@ module Fog
                 }
               end
               group['ipPermissions'] << {
-                'groups'      => [{'groupName' => group_name, 'userId' => self.data[:owner_id]}],
+                'groups'      => [{'groupName' => source_group_name, 'userId' => (options['SourceSecurityGroupOwnerId'] || self.data[:owner_id]) }],
                 'fromPort'    => -1,
                 'ipRanges'    => [],
                 'ipProtocol'  => 'icmp',

lib/fog/compute/requests/aws/revoke_security_group_ingress.rb

@@ -58,10 +58,14 @@ module Fog
           response = Excon::Response.new
           group = self.data[:security_groups][group_name]
           if group
-            if options['SourceSecurityGroupName'] && options['SourceSecurityGroupOwnerId']
+            if source_group_name = options['SourceSecurityGroupName']
-              group['ipPermissions'].delete_if {|permission|
+              group['ipPermissions'].delete_if do |permission|
-                permission['groups'].first['groupName'] == group_name
+                if source_owner_id = options['SourceSecurityGroupOwnerId']
-              }
+                  permission['groups'].first['groupName'] == source_group_name && permission['groups'].first['userId'] == source_owner_id
+                else
+                  permission['groups'].first['groupName'] == source_group_name
+                end
+              end
             else
               ingress = group['ipPermissions'].select {|permission|
                 permission['fromPort']    == options['FromPort'] &&

tests/compute/models/aws/security_group_tests.rb

@@ -12,6 +12,21 @@ Shindo.tests("Fog::Compute[:aws] | security_group", ['aws']) do
       @group.description == "   fog group desc   "
     end
 
+    @other_group = Fog::Compute[:aws].security_groups.create(:name => 'other group', :description => 'another group')
+
+    test("authorize access by another security group") do
+      @group.authorize_group_and_owner(@other_group.name, @other_group.owner_id)
+      @group.reload
+      @group.ip_permissions.size == 3
+    end
+
+    test("revoke access from another security group") do
+      @group.revoke_group_and_owner(@other_group.name, @other_group.owner_id)
+      @group.reload
+      @group.ip_permissions.empty?
+    end
+
+    @other_group.destroy
     @group.destroy
   end
 end

tests/compute/requests/aws/security_group_tests.rb

@@ -12,6 +12,7 @@ Shindo.tests('Fog::Compute[:aws] | security group requests', ['aws']) do
         'ipRanges'    => [],
         'toPort'      => Integer,
       }],
+      'ipPermissionsEgress' => [],
       'ownerId'           => String
     }]
   }