Commit 60edec1f authored by GitLab Release Tools Bot's avatar GitLab Release Tools Bot

Merge branch 'security-id-leaked-password-in-import-url-frontend-11-11' into '11-11-stable'

Handling password on import by url page

See merge request gitlab/gitlabhq!3109
parents 2d0c3178 731ec33a
# frozen_string_literal: true
module ImportUrlParams
def import_url_params
import_params =
params
.require(:project)
.permit(:import_url, :import_url_user, :import_url_password)
{ import_url: import_params_to_full_url(import_params) }
end
def import_params_to_full_url(params)
Gitlab::UrlSanitizer.new(
params[:import_url],
credentials: {
user: params[:import_url_user],
password: params[:import_url_password]
}
).full_url
end
end
...@@ -2,6 +2,7 @@ ...@@ -2,6 +2,7 @@
class Projects::ImportsController < Projects::ApplicationController class Projects::ImportsController < Projects::ApplicationController
include ContinueParams include ContinueParams
include ImportUrlParams
# Authorize # Authorize
before_action :authorize_admin_project! before_action :authorize_admin_project!
...@@ -13,7 +14,7 @@ class Projects::ImportsController < Projects::ApplicationController ...@@ -13,7 +14,7 @@ class Projects::ImportsController < Projects::ApplicationController
end end
def create def create
if @project.update(import_params) if @project.update(import_url_params)
@project.import_state.reset.schedule @project.import_state.reset.schedule
end end
...@@ -65,12 +66,4 @@ class Projects::ImportsController < Projects::ApplicationController ...@@ -65,12 +66,4 @@ class Projects::ImportsController < Projects::ApplicationController
redirect_to project_path(@project) redirect_to project_path(@project)
end end
end end
def import_params_attributes
[:import_url]
end
def import_params
params.require(:project).permit(import_params_attributes)
end
end end
...@@ -7,6 +7,7 @@ class ProjectsController < Projects::ApplicationController ...@@ -7,6 +7,7 @@ class ProjectsController < Projects::ApplicationController
include PreviewMarkdown include PreviewMarkdown
include SendFileUpload include SendFileUpload
include RecordUserLastActivity include RecordUserLastActivity
include ImportUrlParams
prepend_before_action(only: [:show]) { authenticate_sessionless_user!(:rss) } prepend_before_action(only: [:show]) { authenticate_sessionless_user!(:rss) }
...@@ -333,6 +334,7 @@ class ProjectsController < Projects::ApplicationController ...@@ -333,6 +334,7 @@ class ProjectsController < Projects::ApplicationController
def project_params(attributes: []) def project_params(attributes: [])
params.require(:project) params.require(:project)
.permit(project_params_attributes + attributes) .permit(project_params_attributes + attributes)
.merge(import_url_params)
end end
def project_params_attributes def project_params_attributes
......
- ci_cd_only = local_assigns.fetch(:ci_cd_only, false) - ci_cd_only = local_assigns.fetch(:ci_cd_only, false)
- import_url = Gitlab::UrlSanitizer.new(f.object.import_url)
.form-group.import-url-data .import-url-data
.form-group
= f.label :import_url, class: 'label-bold' do = f.label :import_url, class: 'label-bold' do
%span %span
= _('Git repository URL') = _('Git repository URL')
= f.text_field :import_url, value: import_url.sanitized_url,
autocomplete: 'off', class: 'form-control', placeholder: 'https://gitlab.company.com/group/project.git', required: true
= f.text_field :import_url, autocomplete: 'off', class: 'form-control', placeholder: 'https://username:password@gitlab.company.com/group/project.git', required: true .row
.form-group.col-md-6
= f.label :import_url_user, class: 'label-bold' do
%span
= _('Username (optional)')
= f.text_field :import_url_user, value: import_url.user, class: 'form-control', required: false, autocomplete: 'new-password'
.form-group.col-md-6
= f.label :import_url_password, class: 'label-bold' do
%span
= _('Password (optional)')
= f.password_field :import_url_password, class: 'form-control', required: false, autocomplete: 'new-password'
.info-well.prepend-top-20 .info-well.prepend-top-20
.well-segment .well-segment
...@@ -13,7 +28,7 @@ ...@@ -13,7 +28,7 @@
%li %li
= _('The repository must be accessible over <code>http://</code>, <code>https://</code> or <code>git://</code>.').html_safe = _('The repository must be accessible over <code>http://</code>, <code>https://</code> or <code>git://</code>.').html_safe
%li %li
= _('If your HTTP repository is not publicly accessible, add authentication information to the URL: <code>https://username:password@gitlab.company.com/group/project.git</code>.').html_safe = _('If your HTTP repository is not publicly accessible, add your credentials.')
%li %li
= import_will_timeout_message(ci_cd_only) = import_will_timeout_message(ci_cd_only)
%li %li
......
---
title: Add extra fields for handling basic auth on import by url page
merge_request:
author:
type: security
...@@ -47,6 +47,10 @@ module Gitlab ...@@ -47,6 +47,10 @@ module Gitlab
@credentials ||= { user: @url.user.presence, password: @url.password.presence } @credentials ||= { user: @url.user.presence, password: @url.password.presence }
end end
def user
credentials[:user]
end
def full_url def full_url
@full_url ||= generate_full_url.to_s @full_url ||= generate_full_url.to_s
end end
......
...@@ -4941,7 +4941,7 @@ msgstr "" ...@@ -4941,7 +4941,7 @@ msgstr ""
msgid "If this was a mistake you can leave the %{source_type}." msgid "If this was a mistake you can leave the %{source_type}."
msgstr "" msgstr ""
msgid "If your HTTP repository is not publicly accessible, add authentication information to the URL: <code>https://username:password@gitlab.company.com/group/project.git</code>." msgid "If your HTTP repository is not publicly accessible, add your credentials."
msgstr "" msgstr ""
msgid "ImageDiffViewer|2-up" msgid "ImageDiffViewer|2-up"
...@@ -6641,6 +6641,9 @@ msgstr "" ...@@ -6641,6 +6641,9 @@ msgstr ""
msgid "Password" msgid "Password"
msgstr "" msgstr ""
msgid "Password (optional)"
msgstr ""
msgid "Password authentication is unavailable." msgid "Password authentication is unavailable."
msgstr "" msgstr ""
...@@ -10574,6 +10577,9 @@ msgstr "" ...@@ -10574,6 +10577,9 @@ msgstr ""
msgid "UserProfile|Your projects can be available publicly, internally, or privately, at your choice." msgid "UserProfile|Your projects can be available publicly, internally, or privately, at your choice."
msgstr "" msgstr ""
msgid "Username (optional)"
msgstr ""
msgid "Users" msgid "Users"
msgstr "" msgstr ""
......
# frozen_string_literal: true
require 'spec_helper'
describe ImportUrlParams do
let(:import_url_params) do
controller = OpenStruct.new(params: params).extend(described_class)
controller.import_url_params
end
context 'url and password separately provided' do
let(:params) do
ActionController::Parameters.new(project: {
import_url: 'https://url.com',
import_url_user: 'user', import_url_password: 'password'
})
end
describe '#import_url_params' do
it 'returns hash with import_url' do
expect(import_url_params).to eq(
import_url: "https://user:password@url.com"
)
end
end
end
context 'url with provided empty credentials' do
let(:params) do
ActionController::Parameters.new(project: {
import_url: 'https://user:password@url.com',
import_url_user: '', import_url_password: ''
})
end
describe '#import_url_params' do
it 'does not change the url' do
expect(import_url_params).to eq(
import_url: "https://user:password@url.com"
)
end
end
end
end
...@@ -122,4 +122,19 @@ describe Projects::ImportsController do ...@@ -122,4 +122,19 @@ describe Projects::ImportsController do
end end
end end
end end
describe 'POST #create' do
let(:params) { { import_url: 'https://github.com/vim/vim.git', import_url_user: 'user', import_url_password: 'password' } }
let(:project) { create(:project) }
before do
allow(RepositoryImportWorker).to receive(:perform_async)
post :create, params: { project: params, namespace_id: project.namespace.to_param, project_id: project }
end
it 'sets import_url to the project' do
expect(project.reload.import_url).to eq('https://user:password@github.com/vim/vim.git')
end
end
end end
...@@ -10,7 +10,17 @@ describe('New Project', () => { ...@@ -10,7 +10,17 @@ describe('New Project', () => {
setFixtures(` setFixtures(`
<div class='toggle-import-form'> <div class='toggle-import-form'>
<div class='import-url-data'> <div class='import-url-data'>
<div class="form-group">
<input id="project_import_url" /> <input id="project_import_url" />
</div>
<div id="import-url-auth-method">
<div class="form-group">
<input id="project-import-url-user" />
</div>
<div class="form-group">
<input id="project_import_url_password" />
</div>
</div>
<input id="project_name" /> <input id="project_name" />
<input id="project_path" /> <input id="project_path" />
</div> </div>
...@@ -119,7 +129,7 @@ describe('New Project', () => { ...@@ -119,7 +129,7 @@ describe('New Project', () => {
}); });
it('changes project path for HTTPS URL in $projectImportUrl', () => { it('changes project path for HTTPS URL in $projectImportUrl', () => {
$projectImportUrl.val('https://username:password@gitlab.company.com/group/project.git'); $projectImportUrl.val('https://gitlab.company.com/group/project.git');
projectNew.deriveProjectPathFromUrl($projectImportUrl); projectNew.deriveProjectPathFromUrl($projectImportUrl);
......
...@@ -115,6 +115,40 @@ describe Gitlab::UrlSanitizer do ...@@ -115,6 +115,40 @@ describe Gitlab::UrlSanitizer do
end end
end end
describe '#user' do
context 'credentials in hash' do
it 'overrides URL-provided user' do
sanitizer = described_class.new('http://a:b@example.com', credentials: { user: 'c', password: 'd' })
expect(sanitizer.user).to eq('c')
end
end
context 'credentials in URL' do
where(:url, :user) do
'http://foo:bar@example.com' | 'foo'
'http://foo:bar:baz@example.com' | 'foo'
'http://:bar@example.com' | nil
'http://foo:@example.com' | 'foo'
'http://foo@example.com' | 'foo'
'http://:@example.com' | nil
'http://@example.com' | nil
'http://example.com' | nil
# Other invalid URLs
nil | nil
'' | nil
'no' | nil
end
with_them do
subject { described_class.new(url).user }
it { is_expected.to eq(user) }
end
end
end
describe '#full_url' do describe '#full_url' do
context 'credentials in hash' do context 'credentials in hash' do
where(:credentials, :userinfo) do where(:credentials, :userinfo) do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment