Merge branch 'security-mirror-urls' into 'master'

Return only safe urls for mirrors

Closes #101

See merge request gitlab-org/security/gitlab!389

app/serializers/remote_mirror_entity.rb

@@ -2,7 +2,7 @@
 
 class RemoteMirrorEntity < Grape::Entity
   expose :id
-  expose :url
+  expose :safe_url, as: :url
   expose :enabled
 
   expose :auth_method

changelogs/unreleased/security-mirror-urls.yml

+---
+title: Return only safe urls for mirrors
+merge_request:
+author:
+type: security

ee/app/serializers/ee/project_mirror_entity.rb

@@ -6,7 +6,7 @@ module EE
 
     prepended do
       expose :mirror
-      expose :import_url
+      expose :safe_import_url, as: :import_url
       expose :username_only_import_url
       expose :mirror_user_id
       expose :mirror_trigger_builds

ee/spec/serializers/project_mirror_entity_spec.rb

@@ -18,7 +18,7 @@ describe ProjectMirrorEntity do
         is_expected.to eq(
           id: project.id,
           mirror: true,
-          import_url: project.import_url,
+          import_url: project.safe_import_url,
           username_only_import_url: project.username_only_import_url,
           mirror_user_id: project.mirror_user_id,
           mirror_trigger_builds: project.mirror_trigger_builds,
@@ -36,6 +36,10 @@ describe ProjectMirrorEntity do
           remote_mirrors_attributes: []
         )
       end
+
+      it 'excludes password information' do
+        expect(subject[:import_url]).not_to include('password')
+      end
     end
 
     context 'SSH public-key authentication' do

spec/serializers/remote_mirror_entity_spec.rb

@@ -3,7 +3,7 @@
 require 'spec_helper'
 
 describe RemoteMirrorEntity do
-  let(:project) { create(:project, :repository, :remote_mirror) }
+  let(:project) { create(:project, :repository, :remote_mirror, url: "https://test:password@gitlab.com") }
   let(:remote_mirror) { project.remote_mirrors.first }
   let(:entity) { described_class.new(remote_mirror) }
 
@@ -15,4 +15,9 @@ describe RemoteMirrorEntity do
       :ssh_known_hosts, :ssh_public_key, :ssh_known_hosts_fingerprints
     )
   end
+
+  it 'does not expose password information' do
+    expect(subject[:url]).not_to include('password')
+    expect(subject[:url]).to eq(remote_mirror.safe_url)
+  end
 end