From fd376b3ed4de310f8e599cbbeb5c18e5a31c188c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Tue, 30 May 2017 14:11:58 +0200 Subject: Revert "Merge branch '1937-https-clone-url-username' into 'master' " MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit c425f366bfa84efab92b5d5e1d0721f16a2890bc, reversing changes made to 82f6c0f5ac4ed29390ed90592d2c431f3494d93f. Signed-off-by: Rémy Coutable --- app/helpers/button_helper.rb | 2 +- app/helpers/projects_helper.rb | 2 +- app/models/project.rb | 6 ++---- features/steps/explore/projects.rb | 2 +- .../admin/admin_disables_git_access_protocol_spec.rb | 2 +- .../developer_views_empty_project_instructions_spec.rb | 12 +++--------- spec/models/project_spec.rb | 16 +++------------- 7 files changed, 12 insertions(+), 30 deletions(-) diff --git a/app/helpers/button_helper.rb b/app/helpers/button_helper.rb index 206d0753f08..0081bbd92b3 100644 --- a/app/helpers/button_helper.rb +++ b/app/helpers/button_helper.rb @@ -56,7 +56,7 @@ module ButtonHelper content_tag (append_link ? :a : :span), protocol, class: klass, - href: (project.http_url_to_repo(current_user) if append_link), + href: (project.http_url_to_repo if append_link), data: { html: true, placement: placement, diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb index 654aa1a6533..7b0584c42a2 100644 --- a/app/helpers/projects_helper.rb +++ b/app/helpers/projects_helper.rb @@ -276,7 +276,7 @@ module ProjectsHelper when 'ssh' project.ssh_url_to_repo else - project.http_url_to_repo(current_user) + project.http_url_to_repo end end diff --git a/app/models/project.rb b/app/models/project.rb index 29af57d7664..990db73f0d7 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -873,10 +873,8 @@ class Project < ActiveRecord::Base url_to_repo end - def http_url_to_repo(user = nil) - credentials = Gitlab::UrlSanitizer.http_credentials_for_user(user) - - Gitlab::UrlSanitizer.new("#{web_url}.git", credentials: credentials).full_url + def http_url_to_repo + "#{web_url}.git" end def user_can_push_to_empty_repo?(user) diff --git a/features/steps/explore/projects.rb b/features/steps/explore/projects.rb index b2194275751..1a55f40abb9 100644 --- a/features/steps/explore/projects.rb +++ b/features/steps/explore/projects.rb @@ -49,7 +49,7 @@ class Spinach::Features::ExploreProjects < Spinach::FeatureSteps step 'I should see an http link to the repository' do project = Project.find_by(name: 'Community') - expect(page).to have_field('project_clone', with: project.http_url_to_repo(@user)) + expect(page).to have_field('project_clone', with: project.http_url_to_repo) end step 'I should see an ssh link to the repository' do diff --git a/spec/features/admin/admin_disables_git_access_protocol_spec.rb b/spec/features/admin/admin_disables_git_access_protocol_spec.rb index 273cacd82cd..e8e080ce3e2 100644 --- a/spec/features/admin/admin_disables_git_access_protocol_spec.rb +++ b/spec/features/admin/admin_disables_git_access_protocol_spec.rb @@ -32,7 +32,7 @@ feature 'Admin disables Git access protocol', feature: true do scenario 'shows only HTTP url' do visit_project - expect(page).to have_content("git clone #{project.http_url_to_repo(admin)}") + expect(page).to have_content("git clone #{project.http_url_to_repo}") expect(page).not_to have_selector('#clone-dropdown') end end diff --git a/spec/features/projects/developer_views_empty_project_instructions_spec.rb b/spec/features/projects/developer_views_empty_project_instructions_spec.rb index 2352329d58c..0c51fe72ca4 100644 --- a/spec/features/projects/developer_views_empty_project_instructions_spec.rb +++ b/spec/features/projects/developer_views_empty_project_instructions_spec.rb @@ -56,14 +56,8 @@ feature 'Developer views empty project instructions', feature: true do end def expect_instructions_for(protocol) - url = - case protocol - when 'ssh' - project.ssh_url_to_repo - when 'http' - project.http_url_to_repo(developer) - end - - expect(page).to have_content("git clone #{url}") + msg = :"#{protocol.downcase}_url_to_repo" + + expect(page).to have_content("git clone #{project.send(msg)}") end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 36575acf671..2ef3d5654d5 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -1909,19 +1909,9 @@ describe Project, models: true do describe '#http_url_to_repo' do let(:project) { create :empty_project } - context 'when no user is given' do - it 'returns the url to the repo without a username' do - expect(project.http_url_to_repo).to eq("#{project.web_url}.git") - expect(project.http_url_to_repo).not_to include('@') - end - end - - context 'when user is given' do - it 'returns the url to the repo with the username' do - user = build_stubbed(:user) - - expect(project.http_url_to_repo(user)).to start_with("http://#{user.username}@") - end + it 'returns the url to the repo without a username' do + expect(project.http_url_to_repo).to eq("#{project.web_url}.git") + expect(project.http_url_to_repo).not_to include('@') end end -- cgit v1.2.1 From bf4cc9e1f3ca6e4d828eb2bc1ca22d4f350c9dc6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Tue, 30 May 2017 14:18:58 +0200 Subject: Don't allow to pass a user to ProjectWiki#http_url_to_repo MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This partially reverts be25bbc4d2c7e3d5cf3da6f51cb7f7355295ef52. Signed-off-by: Rémy Coutable --- app/models/project_wiki.rb | 7 ++----- lib/gitlab/url_sanitizer.rb | 6 ------ .../projects/wiki/user_git_access_wiki_page_spec.rb | 2 +- spec/lib/gitlab/url_sanitizer_spec.rb | 9 ++------- spec/models/project_wiki_spec.rb | 18 ++++-------------- 5 files changed, 9 insertions(+), 33 deletions(-) diff --git a/app/models/project_wiki.rb b/app/models/project_wiki.rb index 189c106b70b..f38fbda7839 100644 --- a/app/models/project_wiki.rb +++ b/app/models/project_wiki.rb @@ -42,11 +42,8 @@ class ProjectWiki url_to_repo end - def http_url_to_repo(user = nil) - url = "#{Gitlab.config.gitlab.url}/#{path_with_namespace}.git" - credentials = Gitlab::UrlSanitizer.http_credentials_for_user(user) - - Gitlab::UrlSanitizer.new(url, credentials: credentials).full_url + def http_url_to_repo + "#{Gitlab.config.gitlab.url}/#{path_with_namespace}.git" end def wiki_base_path diff --git a/lib/gitlab/url_sanitizer.rb b/lib/gitlab/url_sanitizer.rb index 9ce13feb79a..c81dc7e30d0 100644 --- a/lib/gitlab/url_sanitizer.rb +++ b/lib/gitlab/url_sanitizer.rb @@ -18,12 +18,6 @@ module Gitlab false end - def self.http_credentials_for_user(user) - return {} unless user.respond_to?(:username) - - { user: user.username } - end - def initialize(url, credentials: nil) @url = Addressable::URI.parse(url.strip) @credentials = credentials diff --git a/spec/features/projects/wiki/user_git_access_wiki_page_spec.rb b/spec/features/projects/wiki/user_git_access_wiki_page_spec.rb index 6825b95c8aa..95826e7e5be 100644 --- a/spec/features/projects/wiki/user_git_access_wiki_page_spec.rb +++ b/spec/features/projects/wiki/user_git_access_wiki_page_spec.rb @@ -21,6 +21,6 @@ describe 'Projects > Wiki > User views Git access wiki page', :feature do click_link 'Clone repository' expect(page).to have_text("Clone repository #{project.wiki.path_with_namespace}") - expect(page).to have_text(project.wiki.http_url_to_repo(user)) + expect(page).to have_text(project.wiki.http_url_to_repo) end end diff --git a/spec/lib/gitlab/url_sanitizer_spec.rb b/spec/lib/gitlab/url_sanitizer_spec.rb index fc144a2556a..6bce724a3f6 100644 --- a/spec/lib/gitlab/url_sanitizer_spec.rb +++ b/spec/lib/gitlab/url_sanitizer_spec.rb @@ -62,11 +62,6 @@ describe Gitlab::UrlSanitizer, lib: true do end end - describe '.http_credentials_for_user' do - it { expect(described_class.http_credentials_for_user(user)).to eq({ user: 'john.doe' }) } - it { expect(described_class.http_credentials_for_user('foo')).to eq({}) } - end - describe '#sanitized_url' do it { expect(url_sanitizer.sanitized_url).to eq("https://github.com/me/project.git") } end @@ -76,7 +71,7 @@ describe Gitlab::UrlSanitizer, lib: true do context 'when user is given to #initialize' do let(:url_sanitizer) do - described_class.new("https://github.com/me/project.git", credentials: described_class.http_credentials_for_user(user)) + described_class.new("https://github.com/me/project.git", credentials: { user: user.username }) end it { expect(url_sanitizer.credentials).to eq({ user: 'john.doe' }) } @@ -94,7 +89,7 @@ describe Gitlab::UrlSanitizer, lib: true do context 'when user is given to #initialize' do let(:url_sanitizer) do - described_class.new("https://github.com/me/project.git", credentials: described_class.http_credentials_for_user(user)) + described_class.new("https://github.com/me/project.git", credentials: { user: user.username }) end it { expect(url_sanitizer.full_url).to eq("https://john.doe@github.com/me/project.git") } diff --git a/spec/models/project_wiki_spec.rb b/spec/models/project_wiki_spec.rb index 969e9f7a130..224067f58dd 100644 --- a/spec/models/project_wiki_spec.rb +++ b/spec/models/project_wiki_spec.rb @@ -37,21 +37,11 @@ describe ProjectWiki, models: true do describe "#http_url_to_repo" do let(:project) { create :empty_project } - context 'when no user is given' do - it 'returns the url to the repo without a username' do - expected_url = "#{Gitlab.config.gitlab.url}/#{subject.path_with_namespace}.git" + it 'returns the full http url to the repo' do + expected_url = "#{Gitlab.config.gitlab.url}/#{subject.path_with_namespace}.git" - expect(project_wiki.http_url_to_repo).to eq(expected_url) - expect(project_wiki.http_url_to_repo).not_to include('@') - end - end - - context 'when user is given' do - it 'returns the url to the repo with the username' do - user = build_stubbed(:user) - - expect(project_wiki.http_url_to_repo(user)).to start_with("http://#{user.username}@") - end + expect(project_wiki.http_url_to_repo).to eq(expected_url) + expect(project_wiki.http_url_to_repo).not_to include('@') end end -- cgit v1.2.1 From 08134ad2909c4497dc5ef0e4b144ccc6330a9de8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Tue, 30 May 2017 14:23:14 +0200 Subject: Add changelog MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémy Coutable --- changelogs/unreleased/30410-revert-9347-and-10079.yml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelogs/unreleased/30410-revert-9347-and-10079.yml diff --git a/changelogs/unreleased/30410-revert-9347-and-10079.yml b/changelogs/unreleased/30410-revert-9347-and-10079.yml new file mode 100644 index 00000000000..0149209caf2 --- /dev/null +++ b/changelogs/unreleased/30410-revert-9347-and-10079.yml @@ -0,0 +1,5 @@ +--- +title: Revert the feature that would include the current user's username in the HTTP + clone URL +merge_request: 11792 +author: -- cgit v1.2.1