summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGitLab Bot <gitlab-bot@gitlab.com>2021-09-29 12:57:45 +0000
committerGitLab Bot <gitlab-bot@gitlab.com>2021-09-29 12:57:45 +0000
commit3c992c78e972b8019ae49015d445524d654e4076 (patch)
treed675ed17caeebc4f9b32c1a84d0db7172bb9d24b
parentef37104bc0f26d2fd509f49d8e16c5a45b392b43 (diff)
downloadgitlab-ce-3c992c78e972b8019ae49015d445524d654e4076.tar.gz
Add latest changes from gitlab-org/security/gitlab@14-1-stable-ee
-rw-r--r--app/assets/javascripts/gfm_auto_complete.js33
-rw-r--r--app/models/user.rb3
-rw-r--r--app/services/projects/destroy_service.rb11
-rw-r--r--app/views/projects/_export.html.haml1
-rw-r--r--db/post_migrate/20210914095310_cleanup_orphan_project_access_tokens.rb56
-rw-r--r--db/schema_migrations/202109140953101
-rw-r--r--doc/user/project/settings/import_export.md1
-rw-r--r--lib/api/import_bitbucket_server.rb4
-rw-r--r--lib/gitlab/auth.rb6
-rw-r--r--lib/gitlab/auth/two_factor_auth_verifier.rb4
-rw-r--r--lib/gitlab/import_export/project/import_export.yml1
-rw-r--r--spec/fixtures/lib/gitlab/import_export/complex/project.json17
-rw-r--r--spec/fixtures/lib/gitlab/import_export/complex/tree/project/triggers.ndjson2
-rw-r--r--spec/frontend/gfm_auto_complete_spec.js37
-rw-r--r--spec/lib/gitlab/auth/two_factor_auth_verifier_spec.rb61
-rw-r--r--spec/lib/gitlab/auth_spec.rb55
-rw-r--r--spec/lib/gitlab/git_access_spec.rb35
-rw-r--r--spec/lib/gitlab/import_export/project/tree_restorer_spec.rb4
-rw-r--r--spec/lib/gitlab/import_export/safe_model_attributes.yml9
-rw-r--r--spec/lib/gitlab/lfs_token_spec.rb14
-rw-r--r--spec/migrations/20210914095310_cleanup_orphan_project_access_tokens_spec.rb47
-rw-r--r--spec/models/user_spec.rb82
-rw-r--r--spec/policies/global_policy_spec.rb18
-rw-r--r--spec/requests/api/import_bitbucket_server_spec.rb14
-rw-r--r--spec/requests/git_http_spec.rb16
-rw-r--r--spec/requests/lfs_http_spec.rb6
-rw-r--r--spec/services/projects/destroy_service_spec.rb10
27 files changed, 360 insertions, 188 deletions
diff --git a/app/assets/javascripts/gfm_auto_complete.js b/app/assets/javascripts/gfm_auto_complete.js
index 470c785f7e4..cb63c86a4fa 100644
--- a/app/assets/javascripts/gfm_auto_complete.js
+++ b/app/assets/javascripts/gfm_auto_complete.js
@@ -1,6 +1,6 @@
import $ from 'jquery';
import '~/lib/utils/jquery_at_who';
-import { escape, sortBy, template } from 'lodash';
+import { escape as lodashEscape, sortBy, template } from 'lodash';
import * as Emoji from '~/emoji';
import axios from '~/lib/utils/axios_utils';
import { s__, __, sprintf } from '~/locale';
@@ -11,8 +11,21 @@ import { spriteIcon } from './lib/utils/common_utils';
import { parsePikadayDate } from './lib/utils/datetime_utility';
import glRegexp from './lib/utils/regexp';
-function sanitize(str) {
- return str.replace(/<(?:.|\n)*?>/gm, '');
+/**
+ * Escapes user input before we pass it to at.js, which
+ * renders it as HTML in the autocomplete dropdown.
+ *
+ * at.js allows you to reference data using `${}` syntax
+ * (e.g. ${search}) which it replaces with the actual data
+ * before rendering it in the autocomplete dropdown.
+ * To prevent user input from executing this `${}` syntax,
+ * we also need to escape the $ character.
+ *
+ * @param string user input
+ * @return {string} escaped user input
+ */
+function escape(string) {
+ return lodashEscape(string).replace(/\$/g, '&dollar;');
}
function createMemberSearchString(member) {
@@ -44,8 +57,8 @@ export function membersBeforeSave(members) {
return {
username: member.username,
avatarTag: autoCompleteAvatar.length === 1 ? txtAvatar : imgAvatar,
- title: sanitize(title),
- search: sanitize(createMemberSearchString(member)),
+ title,
+ search: createMemberSearchString(member),
icon: avatarIcon,
availability: member?.availability,
};
@@ -366,7 +379,7 @@ class GfmAutoComplete {
}
return {
id: i.iid,
- title: sanitize(i.title),
+ title: i.title,
reference: i.reference,
search: `${i.iid} ${i.title}`,
};
@@ -404,7 +417,7 @@ class GfmAutoComplete {
return {
id: m.iid,
- title: sanitize(m.title),
+ title: m.title,
search: m.title,
expired,
dueDate,
@@ -456,7 +469,7 @@ class GfmAutoComplete {
}
return {
id: m.iid,
- title: sanitize(m.title),
+ title: m.title,
reference: m.reference,
search: `${m.iid} ${m.title}`,
};
@@ -492,7 +505,7 @@ class GfmAutoComplete {
beforeSave(merges) {
if (GfmAutoComplete.isLoading(merges)) return merges;
return $.map(merges, (m) => ({
- title: sanitize(m.title),
+ title: m.title,
color: m.color,
search: m.title,
set: m.set,
@@ -586,7 +599,7 @@ class GfmAutoComplete {
}
return {
id: m.id,
- title: sanitize(m.title),
+ title: m.title,
search: `${m.id} ${m.title}`,
};
});
diff --git a/app/models/user.rb b/app/models/user.rb
index 80b8c9173d1..903149a5252 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -1892,7 +1892,8 @@ class User < ApplicationRecord
def password_expired_if_applicable?
return false if bot?
- return false unless password_expired? && password_automatically_set?
+ return false unless password_expired?
+ return false if password_automatically_set?
return false unless allow_password_authentication?
true
diff --git a/app/services/projects/destroy_service.rb b/app/services/projects/destroy_service.rb
index e27ea7c07e5..afa8de04fca 100644
--- a/app/services/projects/destroy_service.rb
+++ b/app/services/projects/destroy_service.rb
@@ -117,6 +117,7 @@ module Projects
trash_relation_repositories!
trash_project_repositories!
destroy_web_hooks!
+ destroy_project_bots!
# Rails attempts to load all related records into memory before
# destroying: https://github.com/rails/rails/issues/22510
@@ -149,6 +150,16 @@ module Projects
end
end
+ # The project can have multiple project bots with personal access tokens generated.
+ # We need to remove them when a project is deleted
+ # rubocop: disable CodeReuse/ActiveRecord
+ def destroy_project_bots!
+ project.members.includes(:user).references(:user).merge(User.project_bot).each do |member|
+ Users::DestroyService.new(current_user).execute(member.user, skip_authorization: true)
+ end
+ end
+ # rubocop: enable CodeReuse/ActiveRecord
+
def remove_registry_tags
return true unless Gitlab.config.registry.enabled
return false unless remove_legacy_registry_tags
diff --git a/app/views/projects/_export.html.haml b/app/views/projects/_export.html.haml
index eb4630b84d5..97f5cdb54e5 100644
--- a/app/views/projects/_export.html.haml
+++ b/app/views/projects/_export.html.haml
@@ -16,6 +16,7 @@
%li= _('Job logs and artifacts')
%li= _('Container registry images')
%li= _('CI variables')
+ %li= _('Pipeline triggers')
%li= _('Webhooks')
%li= _('Any encrypted tokens')
- if project.export_status == :finished
diff --git a/db/post_migrate/20210914095310_cleanup_orphan_project_access_tokens.rb b/db/post_migrate/20210914095310_cleanup_orphan_project_access_tokens.rb
new file mode 100644
index 00000000000..9d981413437
--- /dev/null
+++ b/db/post_migrate/20210914095310_cleanup_orphan_project_access_tokens.rb
@@ -0,0 +1,56 @@
+# frozen_string_literal: true
+
+class CleanupOrphanProjectAccessTokens < ActiveRecord::Migration[6.1]
+ include Gitlab::Database::MigrationHelpers
+
+ disable_ddl_transaction!
+
+ TMP_INDEX_NAME = 'idx_users_on_user_type_project_bots_batched'
+
+ def up
+ users_table = define_batchable_model('users')
+
+ add_concurrent_index(:users, :id, name: TMP_INDEX_NAME, where: 'user_type = 6')
+
+ accumulated_orphans = []
+ users_table.where(user_type: 6).each_batch(of: 500) do |relation|
+ orphan_ids = relation.where("not exists(select 1 from members where members.user_id = users.id)").pluck(:id)
+
+ orphan_ids.each_slice(10) do |ids|
+ users_table.where(id: ids).update_all(state: 'deactivated')
+ end
+
+ accumulated_orphans += orphan_ids
+ end
+
+ schedule_deletion(accumulated_orphans)
+ ensure
+ remove_concurrent_index_by_name(:users, TMP_INDEX_NAME)
+ end
+
+ def down
+ remove_concurrent_index_by_name(:users, TMP_INDEX_NAME) if index_exists_by_name?(:users, TMP_INDEX_NAME)
+ end
+
+ private
+
+ def schedule_deletion(orphan_ids)
+ return unless deletion_worker
+
+ orphan_ids.each_slice(100) do |ids|
+ job_arguments = ids.map do |orphan_id|
+ [orphan_id, orphan_id, { skip_authorization: true }]
+ end
+
+ deletion_worker.bulk_perform_async(job_arguments)
+ end
+ rescue StandardError
+ # Ignore any errors or interface changes since this part of migration is optional
+ end
+
+ def deletion_worker
+ @deletion_worker = "DeleteUserWorker".safe_constantize unless defined?(@deletion_worker)
+
+ @deletion_worker
+ end
+end
diff --git a/db/schema_migrations/20210914095310 b/db/schema_migrations/20210914095310
new file mode 100644
index 00000000000..fee7e0b9719
--- /dev/null
+++ b/db/schema_migrations/20210914095310
@@ -0,0 +1 @@
+6fcf3ff9867df68f5e9603ae0311b29bec33aa5c5b826786b094ab0960ebcd90 \ No newline at end of file
diff --git a/doc/user/project/settings/import_export.md b/doc/user/project/settings/import_export.md
index 7cbc22bef63..22fb2da8038 100644
--- a/doc/user/project/settings/import_export.md
+++ b/doc/user/project/settings/import_export.md
@@ -133,6 +133,7 @@ The following items are **not** exported:
- Build traces and artifacts
- Container registry images
- CI/CD variables
+- Pipeline triggers
- Webhooks
- Any encrypted tokens
- Merge Request Approvers
diff --git a/lib/api/import_bitbucket_server.rb b/lib/api/import_bitbucket_server.rb
index ecd78c6e6db..0f0d62dcbfb 100644
--- a/lib/api/import_bitbucket_server.rb
+++ b/lib/api/import_bitbucket_server.rb
@@ -4,6 +4,10 @@ module API
class ImportBitbucketServer < ::API::Base
feature_category :importers
+ before do
+ forbidden! unless Gitlab::CurrentSettings.import_sources&.include?('bitbucket_server')
+ end
+
helpers do
def client
@client ||= BitbucketServer::Client.new(credentials)
diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb
index 13e78e72175..2848f7c3ef8 100644
--- a/lib/gitlab/auth.rb
+++ b/lib/gitlab/auth.rb
@@ -172,7 +172,11 @@ module Gitlab
user = find_with_user_password(login, password)
return unless user
- raise Gitlab::Auth::MissingPersonalAccessTokenError if user.two_factor_enabled?
+ verifier = TwoFactorAuthVerifier.new(user)
+
+ if user.two_factor_enabled? || verifier.two_factor_authentication_enforced?
+ raise Gitlab::Auth::MissingPersonalAccessTokenError
+ end
Gitlab::Auth::Result.new(user, nil, :gitlab_or_ldap, full_authentication_abilities)
end
diff --git a/lib/gitlab/auth/two_factor_auth_verifier.rb b/lib/gitlab/auth/two_factor_auth_verifier.rb
index 86552ef1267..5a203a1fe9c 100644
--- a/lib/gitlab/auth/two_factor_auth_verifier.rb
+++ b/lib/gitlab/auth/two_factor_auth_verifier.rb
@@ -9,6 +9,10 @@ module Gitlab
@current_user = current_user
end
+ def two_factor_authentication_enforced?
+ two_factor_authentication_required? && two_factor_grace_period_expired?
+ end
+
def two_factor_authentication_required?
Gitlab::CurrentSettings.require_two_factor_authentication? ||
current_user&.require_two_factor_authentication_from_group?
diff --git a/lib/gitlab/import_export/project/import_export.yml b/lib/gitlab/import_export/project/import_export.yml
index e5a39efb798..2772e7ef02b 100644
--- a/lib/gitlab/import_export/project/import_export.yml
+++ b/lib/gitlab/import_export/project/import_export.yml
@@ -88,7 +88,6 @@ tree:
- :external_pull_request
- :merge_request
- :auto_devops
- - :triggers
- :pipeline_schedules
- :container_expiration_policy
- protected_branches:
diff --git a/spec/fixtures/lib/gitlab/import_export/complex/project.json b/spec/fixtures/lib/gitlab/import_export/complex/project.json
index e3aeace6383..1072e63b20b 100644
--- a/spec/fixtures/lib/gitlab/import_export/complex/project.json
+++ b/spec/fixtures/lib/gitlab/import_export/complex/project.json
@@ -7579,23 +7579,6 @@
}
}
],
- "triggers": [
- {
- "id": 123,
- "token": "cdbfasdf44a5958c83654733449e585",
- "project_id": 5,
- "owner_id": 1,
- "created_at": "2017-01-16T15:25:28.637Z",
- "updated_at": "2017-01-16T15:25:28.637Z"
- },
- {
- "id": 456,
- "token": "33a66349b5ad01fc00174af87804e40",
- "project_id": 5,
- "created_at": "2017-01-16T15:25:29.637Z",
- "updated_at": "2017-01-16T15:25:29.637Z"
- }
- ],
"pipeline_schedules": [
{
"id": 1,
diff --git a/spec/fixtures/lib/gitlab/import_export/complex/tree/project/triggers.ndjson b/spec/fixtures/lib/gitlab/import_export/complex/tree/project/triggers.ndjson
index 93619f4fb44..2b5bda687b8 100644
--- a/spec/fixtures/lib/gitlab/import_export/complex/tree/project/triggers.ndjson
+++ b/spec/fixtures/lib/gitlab/import_export/complex/tree/project/triggers.ndjson
@@ -1,2 +1,2 @@
{"id":123,"token":"cdbfasdf44a5958c83654733449e585","project_id":5,"owner_id":1,"created_at":"2017-01-16T15:25:28.637Z","updated_at":"2017-01-16T15:25:28.637Z"}
-{"id":456,"token":"33a66349b5ad01fc00174af87804e40","project_id":5,"created_at":"2017-01-16T15:25:29.637Z","updated_at":"2017-01-16T15:25:29.637Z"}
+{"id":456,"token":"33a66349b5ad01fc00174af87804e40","project_id":5,"created_at":"2017-01-16T15:25:29.637Z","updated_at":"2017-01-16T15:25:29.637Z"} \ No newline at end of file
diff --git a/spec/frontend/gfm_auto_complete_spec.js b/spec/frontend/gfm_auto_complete_spec.js
index 211ed064762..94ad7759110 100644
--- a/spec/frontend/gfm_auto_complete_spec.js
+++ b/spec/frontend/gfm_auto_complete_spec.js
@@ -574,6 +574,15 @@ describe('GfmAutoComplete', () => {
}),
).toBe('<li><small>grp/proj#5</small> Some Issue</li>');
});
+
+ it('escapes title in the template as it is user input', () => {
+ expect(
+ GfmAutoComplete.Issues.templateFunction({
+ id: 5,
+ title: '${search}<script>oh no $', // eslint-disable-line no-template-curly-in-string
+ }),
+ ).toBe('<li><small>5</small> &dollar;{search}&lt;script&gt;oh no &dollar;</li>');
+ });
});
describe('GfmAutoComplete.Members', () => {
@@ -608,16 +617,18 @@ describe('GfmAutoComplete', () => {
).toBe('<li>IMG my-group <small></small> <i class="icon"/></li>');
});
- it('should add escaped title if title is set', () => {
+ it('escapes title in the template as it is user input', () => {
expect(
GfmAutoComplete.Members.templateFunction({
avatarTag: 'IMG',
username: 'my-group',
- title: 'MyGroup+',
+ title: '${search}<script>oh no $', // eslint-disable-line no-template-curly-in-string
icon: '<i class="icon"/>',
availabilityStatus: '',
}),
- ).toBe('<li>IMG my-group <small>MyGroup+</small> <i class="icon"/></li>');
+ ).toBe(
+ '<li>IMG my-group <small>&dollar;{search}&lt;script&gt;oh no &dollar;</small> <i class="icon"/></li>',
+ );
});
it('should add user availability status if availabilityStatus is set', () => {
@@ -782,6 +793,15 @@ describe('GfmAutoComplete', () => {
${'/unlabel ~'} | ${assignedLabels}
`('$input shows $output.length labels', expectLabels);
});
+
+ it('escapes title in the template as it is user input', () => {
+ const color = '#123456';
+ const title = '${search}<script>oh no $'; // eslint-disable-line no-template-curly-in-string
+
+ expect(GfmAutoComplete.Labels.templateFunction(color, title)).toBe(
+ '<li><span class="dropdown-label-box" style="background: #123456"></span> &dollar;{search}&lt;script&gt;oh no &dollar;</li>',
+ );
+ });
});
describe('emoji', () => {
@@ -829,4 +849,15 @@ describe('GfmAutoComplete', () => {
});
});
});
+
+ describe('milestones', () => {
+ it('escapes title in the template as it is user input', () => {
+ const expired = false;
+ const title = '${search}<script>oh no $'; // eslint-disable-line no-template-curly-in-string
+
+ expect(GfmAutoComplete.Milestones.templateFunction(title, expired)).toBe(
+ '<li>&dollar;{search}&lt;script&gt;oh no &dollar;</li>',
+ );
+ });
+ });
});
diff --git a/spec/lib/gitlab/auth/two_factor_auth_verifier_spec.rb b/spec/lib/gitlab/auth/two_factor_auth_verifier_spec.rb
index f906870195a..876c23a91bd 100644
--- a/spec/lib/gitlab/auth/two_factor_auth_verifier_spec.rb
+++ b/spec/lib/gitlab/auth/two_factor_auth_verifier_spec.rb
@@ -3,33 +3,50 @@
require 'spec_helper'
RSpec.describe Gitlab::Auth::TwoFactorAuthVerifier do
- let(:user) { create(:user) }
+ using RSpec::Parameterized::TableSyntax
- subject { described_class.new(user) }
+ subject(:verifier) { described_class.new(user) }
- describe '#two_factor_authentication_required?' do
- describe 'when it is required on application level' do
- it 'returns true' do
- stub_application_setting require_two_factor_authentication: true
+ let(:user) { build_stubbed(:user, otp_grace_period_started_at: Time.zone.now) }
- expect(subject.two_factor_authentication_required?).to be_truthy
- end
- end
+ describe '#two_factor_authentication_enforced?' do
+ subject { verifier.two_factor_authentication_enforced? }
- describe 'when it is required on group level' do
- it 'returns true' do
- allow(user).to receive(:require_two_factor_authentication_from_group?).and_return(true)
+ where(:instance_level_enabled, :group_level_enabled, :grace_period_expired, :should_be_enforced) do
+ false | false | true | false
+ true | false | false | false
+ true | false | true | true
+ false | true | false | false
+ false | true | true | true
+ end
- expect(subject.two_factor_authentication_required?).to be_truthy
+ with_them do
+ before do
+ stub_application_setting(require_two_factor_authentication: instance_level_enabled)
+ allow(user).to receive(:require_two_factor_authentication_from_group?).and_return(group_level_enabled)
+ stub_application_setting(two_factor_grace_period: grace_period_expired ? 0 : 1.month.in_hours)
end
+
+ it { is_expected.to eq(should_be_enforced) }
end
+ end
- describe 'when it is not required' do
- it 'returns false when not required on group level' do
- allow(user).to receive(:require_two_factor_authentication_from_group?).and_return(false)
+ describe '#two_factor_authentication_required?' do
+ subject { verifier.two_factor_authentication_required? }
+
+ where(:instance_level_enabled, :group_level_enabled, :should_be_required) do
+ true | false | true
+ false | true | true
+ false | false | false
+ end
- expect(subject.two_factor_authentication_required?).to be_falsey
+ with_them do
+ before do
+ stub_application_setting(require_two_factor_authentication: instance_level_enabled)
+ allow(user).to receive(:require_two_factor_authentication_from_group?).and_return(group_level_enabled)
end
+
+ it { is_expected.to eq(should_be_required) }
end
end
@@ -85,25 +102,21 @@ RSpec.describe Gitlab::Auth::TwoFactorAuthVerifier do
end
describe '#two_factor_grace_period_expired?' do
- before do
- allow(user).to receive(:otp_grace_period_started_at).and_return(4.hours.ago)
- end
-
it 'returns true if the grace period has expired' do
- allow(subject).to receive(:two_factor_grace_period).and_return(2)
+ stub_application_setting two_factor_grace_period: 0
expect(subject.two_factor_grace_period_expired?).to be_truthy
end
it 'returns false if the grace period has not expired' do
- allow(subject).to receive(:two_factor_grace_period).and_return(6)
+ stub_application_setting two_factor_grace_period: 1.month.in_hours
expect(subject.two_factor_grace_period_expired?).to be_falsey
end
context 'when otp_grace_period_started_at is nil' do
it 'returns false' do
- allow(user).to receive(:otp_grace_period_started_at).and_return(nil)
+ user.otp_grace_period_started_at = nil
expect(subject.two_factor_grace_period_expired?).to be_falsey
end
diff --git a/spec/lib/gitlab/auth_spec.rb b/spec/lib/gitlab/auth_spec.rb
index 1d708b17076..901ef3b64db 100644
--- a/spec/lib/gitlab/auth_spec.rb
+++ b/spec/lib/gitlab/auth_spec.rb
@@ -424,6 +424,41 @@ RSpec.describe Gitlab::Auth, :use_clean_rails_memory_store_caching do
.to eq(Gitlab::Auth::Result.new(nil, nil, nil, nil))
end
+ context 'when 2fa is enabled globally' do
+ let_it_be(:user) do
+ create(:user, username: 'normal_user', password: 'my-secret', otp_grace_period_started_at: 1.day.ago)
+ end
+
+ before do
+ stub_application_setting(require_two_factor_authentication: true)
+ end
+
+ it 'fails if grace period expired' do
+ stub_application_setting(two_factor_grace_period: 0)
+
+ expect { gl_auth.find_for_git_client(user.username, user.password, project: nil, ip: 'ip') }
+ .to raise_error(Gitlab::Auth::MissingPersonalAccessTokenError)
+ end
+
+ it 'goes through if grace period is not expired yet' do
+ stub_application_setting(two_factor_grace_period: 72)
+
+ expect(gl_auth.find_for_git_client(user.username, user.password, project: nil, ip: 'ip'))
+ .to have_attributes(actor: user, project: nil, type: :gitlab_or_ldap, authentication_abilities: described_class.full_authentication_abilities)
+ end
+ end
+
+ context 'when 2fa is enabled personally' do
+ let(:user) do
+ create(:user, :two_factor, username: 'normal_user', password: 'my-secret', otp_grace_period_started_at: 1.day.ago)
+ end
+
+ it 'fails' do
+ expect { gl_auth.find_for_git_client(user.username, user.password, project: nil, ip: 'ip') }
+ .to raise_error(Gitlab::Auth::MissingPersonalAccessTokenError)
+ end
+ end
+
it 'goes through lfs authentication' do
user = create(
:user,
@@ -717,16 +752,16 @@ RSpec.describe Gitlab::Auth, :use_clean_rails_memory_store_caching do
describe 'find_with_user_password' do
let!(:user) do
create(:user,
- username: username,
- password: password,
- password_confirmation: password)
+ username: username,
+ password: password,
+ password_confirmation: password)
end
let(:username) { 'John' } # username isn't lowercase, test this
let(:password) { 'my-secret' }
it "finds user by valid login/password" do
- expect( gl_auth.find_with_user_password(username, password) ).to eql user
+ expect(gl_auth.find_with_user_password(username, password)).to eql user
end
it 'finds user by valid email/password with case-insensitive email' do
@@ -739,12 +774,12 @@ RSpec.describe Gitlab::Auth, :use_clean_rails_memory_store_caching do
it "does not find user with invalid password" do
password = 'wrong'
- expect( gl_auth.find_with_user_password(username, password) ).not_to eql user
+ expect(gl_auth.find_with_user_password(username, password)).not_to eql user
end
it "does not find user with invalid login" do
user = 'wrong'
- expect( gl_auth.find_with_user_password(username, password) ).not_to eql user
+ expect(gl_auth.find_with_user_password(username, password)).not_to eql user
end
include_examples 'user login operation with unique ip limit' do
@@ -756,13 +791,13 @@ RSpec.describe Gitlab::Auth, :use_clean_rails_memory_store_caching do
it 'finds the user in deactivated state' do
user.deactivate!
- expect( gl_auth.find_with_user_password(username, password) ).to eql user
+ expect(gl_auth.find_with_user_password(username, password)).to eql user
end
it "does not find user in blocked state" do
user.block
- expect( gl_auth.find_with_user_password(username, password) ).not_to eql user
+ expect(gl_auth.find_with_user_password(username, password)).not_to eql user
end
it 'does not find user in locked state' do
@@ -774,13 +809,13 @@ RSpec.describe Gitlab::Auth, :use_clean_rails_memory_store_caching do
it "does not find user in ldap_blocked state" do
user.ldap_block
- expect( gl_auth.find_with_user_password(username, password) ).not_to eql user
+ expect(gl_auth.find_with_user_password(username, password)).not_to eql user
end
it 'does not find user in blocked_pending_approval state' do
user.block_pending_approval
- expect( gl_auth.find_with_user_password(username, password) ).not_to eql user
+ expect(gl_auth.find_with_user_password(username, password)).not_to eql user
end
context 'with increment_failed_attempts' do
diff --git a/spec/lib/gitlab/git_access_spec.rb b/spec/lib/gitlab/git_access_spec.rb
index bf682e4e4c6..bf2e3c7f5f8 100644
--- a/spec/lib/gitlab/git_access_spec.rb
+++ b/spec/lib/gitlab/git_access_spec.rb
@@ -435,17 +435,19 @@ RSpec.describe Gitlab::GitAccess do
it 'disallows users with expired password to pull' do
project.add_maintainer(user)
- user.update!(password_expires_at: 2.minutes.ago, password_automatically_set: true)
+ user.update!(password_expires_at: 2.minutes.ago)
expect { pull_access_check }.to raise_forbidden("Your password expired. Please access GitLab from a web browser to update your password.")
end
- it 'allows ldap users with expired password to pull' do
- project.add_maintainer(user)
- user.update!(password_expires_at: 2.minutes.ago)
- allow(user).to receive(:ldap_user?).and_return(true)
+ context 'with an ldap user' do
+ let(:user) { create(:omniauth_user, provider: 'ldap', password_expires_at: 2.minutes.ago) }
- expect { pull_access_check }.not_to raise_error
+ it 'allows ldap users with expired password to pull' do
+ project.add_maintainer(user)
+
+ expect { pull_access_check }.not_to raise_error
+ end
end
context 'when the project repository does not exist' do
@@ -987,24 +989,23 @@ RSpec.describe Gitlab::GitAccess do
end
it 'disallows users with expired password to push' do
- user.update!(password_expires_at: 2.minutes.ago, password_automatically_set: true)
+ user.update!(password_expires_at: 2.minutes.ago)
expect { push_access_check }.to raise_forbidden("Your password expired. Please access GitLab from a web browser to update your password.")
end
- it 'allows ldap users with expired password to push' do
- user.update!(password_expires_at: 2.minutes.ago)
- allow(user).to receive(:ldap_user?).and_return(true)
+ context 'with an ldap user' do
+ let(:user) { create(:omniauth_user, provider: 'ldap', password_expires_at: 2.minutes.ago) }
- expect { push_access_check }.not_to raise_error
- end
+ it 'allows ldap users with expired password to push' do
+ expect { push_access_check }.not_to raise_error
+ end
- it 'disallows blocked ldap users with expired password to push' do
- user.block
- user.update!(password_expires_at: 2.minutes.ago)
- allow(user).to receive(:ldap_user?).and_return(true)
+ it 'disallows blocked ldap users with expired password to push' do
+ user.block
- expect { push_access_check }.to raise_forbidden("Your account has been blocked.")
+ expect { push_access_check }.to raise_forbidden("Your account has been blocked.")
+ end
end
it 'cleans up the files' do
diff --git a/spec/lib/gitlab/import_export/project/tree_restorer_spec.rb b/spec/lib/gitlab/import_export/project/tree_restorer_spec.rb
index 82f465c4f9e..518a9337826 100644
--- a/spec/lib/gitlab/import_export/project/tree_restorer_spec.rb
+++ b/spec/lib/gitlab/import_export/project/tree_restorer_spec.rb
@@ -445,8 +445,8 @@ RSpec.describe Gitlab::ImportExport::Project::TreeRestorer do
expect(@project.merge_requests.size).to eq(9)
end
- it 'only restores valid triggers' do
- expect(@project.triggers.size).to eq(1)
+ it 'does not restore triggers' do
+ expect(@project.triggers.size).to eq(0)
end
it 'has the correct number of pipelines and statuses' do
diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml
index 77d126e012e..0be0da86651 100644
--- a/spec/lib/gitlab/import_export/safe_model_attributes.yml
+++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml
@@ -400,15 +400,6 @@ Ci::Variable:
- encrypted_value
- encrypted_value_salt
- encrypted_value_iv
-Ci::Trigger:
-- id
-- token
-- project_id
-- created_at
-- updated_at
-- owner_id
-- description
-- ref
Ci::PipelineSchedule:
- id
- description
diff --git a/spec/lib/gitlab/lfs_token_spec.rb b/spec/lib/gitlab/lfs_token_spec.rb
index a8472062f03..3bc0bd385a7 100644
--- a/spec/lib/gitlab/lfs_token_spec.rb
+++ b/spec/lib/gitlab/lfs_token_spec.rb
@@ -126,7 +126,7 @@ RSpec.describe Gitlab::LfsToken, :clean_gitlab_redis_shared_state do
end
context 'when the user password is expired' do
- let(:actor) { create(:user, password_expires_at: 1.minute.ago, password_automatically_set: true) }
+ let(:actor) { create(:user, password_expires_at: 1.minute.ago) }
it 'returns false' do
expect(lfs_token.token_valid?(lfs_token.token)).to be false
@@ -135,12 +135,12 @@ RSpec.describe Gitlab::LfsToken, :clean_gitlab_redis_shared_state do
end
context 'when the actor is an ldap user' do
- before do
- allow(actor).to receive(:ldap_user?).and_return(true)
- end
+ let(:actor) { create(:omniauth_user, provider: 'ldap') }
context 'when the user is blocked' do
- let(:actor) { create(:user, :blocked) }
+ before do
+ actor.block!
+ end
it 'returns false' do
expect(lfs_token.token_valid?(lfs_token.token)).to be false
@@ -148,7 +148,9 @@ RSpec.describe Gitlab::LfsToken, :clean_gitlab_redis_shared_state do
end
context 'when the user password is expired' do
- let(:actor) { create(:user, password_expires_at: 1.minute.ago) }
+ before do
+ actor.update!(password_expires_at: 1.minute.ago)
+ end
it 'returns true' do
expect(lfs_token.token_valid?(lfs_token.token)).to be true
diff --git a/spec/migrations/20210914095310_cleanup_orphan_project_access_tokens_spec.rb b/spec/migrations/20210914095310_cleanup_orphan_project_access_tokens_spec.rb
new file mode 100644
index 00000000000..0d0f6a3df67
--- /dev/null
+++ b/spec/migrations/20210914095310_cleanup_orphan_project_access_tokens_spec.rb
@@ -0,0 +1,47 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+require_migration!('cleanup_orphan_project_access_tokens')
+
+RSpec.describe CleanupOrphanProjectAccessTokens, :migration do
+ def create_user(**extra_options)
+ defaults = { state: 'active', projects_limit: 0, email: "#{extra_options[:username]}@example.com" }
+
+ table(:users).create!(defaults.merge(extra_options))
+ end
+
+ def create_membership(**extra_options)
+ defaults = { access_level: 30, notification_level: 0, source_id: 1, source_type: 'Project' }
+
+ table(:members).create!(defaults.merge(extra_options))
+ end
+
+ let!(:regular_user) { create_user(username: 'regular') }
+ let!(:orphan_bot) { create_user(username: 'orphaned_bot', user_type: 6) }
+ let!(:used_bot) do
+ create_user(username: 'used_bot', user_type: 6).tap do |bot|
+ create_membership(user_id: bot.id)
+ end
+ end
+
+ it 'marks all bots without memberships as deactivated' do
+ expect do
+ migrate!
+ regular_user.reload
+ orphan_bot.reload
+ used_bot.reload
+ end.to change {
+ [regular_user.state, orphan_bot.state, used_bot.state]
+ }.from(%w[active active active]).to(%w[active deactivated active])
+ end
+
+ it 'schedules for deletion all bots without memberships' do
+ job_class = 'DeleteUserWorker'.safe_constantize
+
+ if job_class
+ expect(job_class).to receive(:bulk_perform_async).with([[orphan_bot.id, orphan_bot.id, skip_authorization: true]])
+
+ migrate!
+ end
+ end
+end
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 0eb769c65cd..8f2bbc138da 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -5276,43 +5276,11 @@ RSpec.describe User do
end
describe '#password_expired_if_applicable?' do
- let(:user) { build(:user, password_expires_at: password_expires_at, password_automatically_set: set_automatically?) }
+ let(:user) { build(:user, password_expires_at: password_expires_at) }
subject { user.password_expired_if_applicable? }
- context 'when user is not ldap user' do
- context 'when user has password set automatically' do
- let(:set_automatically?) { true }
-
- context 'when password_expires_at is not set' do
- let(:password_expires_at) {}
-
- it 'returns false' do
- is_expected.to be_falsey
- end
- end
-
- context 'when password_expires_at is in the past' do
- let(:password_expires_at) { 1.minute.ago }
-
- it 'returns true' do
- is_expected.to be_truthy
- end
- end
-
- context 'when password_expires_at is in the future' do
- let(:password_expires_at) { 1.minute.from_now }
-
- it 'returns false' do
- is_expected.to be_falsey
- end
- end
- end
- end
-
- context 'when user has password not set automatically' do
- let(:set_automatically?) { false }
-
+ shared_examples 'password expired not applicable' do
context 'when password_expires_at is not set' do
let(:password_expires_at) {}
@@ -5338,13 +5306,7 @@ RSpec.describe User do
end
end
- context 'when user is ldap user' do
- let(:user) { build(:user, password_expires_at: password_expires_at) }
-
- before do
- allow(user).to receive(:ldap_user?).and_return(true)
- end
-
+ context 'with a regular user' do
context 'when password_expires_at is not set' do
let(:password_expires_at) {}
@@ -5356,8 +5318,8 @@ RSpec.describe User do
context 'when password_expires_at is in the past' do
let(:password_expires_at) { 1.minute.ago }
- it 'returns false' do
- is_expected.to be_falsey
+ it 'returns true' do
+ is_expected.to be_truthy
end
end
@@ -5370,32 +5332,26 @@ RSpec.describe User do
end
end
- context 'when user is a project bot' do
- let(:user) { build(:user, :project_bot, password_expires_at: password_expires_at) }
-
- context 'when password_expires_at is not set' do
- let(:password_expires_at) {}
-
- it 'returns false' do
- is_expected.to be_falsey
- end
+ context 'when user is a bot' do
+ before do
+ allow(user).to receive(:bot?).and_return(true)
end
- context 'when password_expires_at is in the past' do
- let(:password_expires_at) { 1.minute.ago }
+ it_behaves_like 'password expired not applicable'
+ end
- it 'returns false' do
- is_expected.to be_falsey
- end
- end
+ context 'when password_automatically_set is true' do
+ let(:user) { create(:omniauth_user, provider: 'ldap')}
- context 'when password_expires_at is in the future' do
- let(:password_expires_at) { 1.minute.from_now }
+ it_behaves_like 'password expired not applicable'
+ end
- it 'returns false' do
- is_expected.to be_falsey
- end
+ context 'when allow_password_authentication is false' do
+ before do
+ allow(user).to receive(:allow_password_authentication?).and_return(false)
end
+
+ it_behaves_like 'password expired not applicable'
end
end
diff --git a/spec/policies/global_policy_spec.rb b/spec/policies/global_policy_spec.rb
index 122612df355..ca9a5b1853c 100644
--- a/spec/policies/global_policy_spec.rb
+++ b/spec/policies/global_policy_spec.rb
@@ -249,15 +249,13 @@ RSpec.describe GlobalPolicy do
context 'user with expired password' do
before do
- current_user.update!(password_expires_at: 2.minutes.ago, password_automatically_set: true)
+ current_user.update!(password_expires_at: 2.minutes.ago)
end
it { is_expected.not_to be_allowed(:access_api) }
context 'when user is using ldap' do
- before do
- allow(current_user).to receive(:ldap_user?).and_return(true)
- end
+ let(:current_user) { create(:omniauth_user, provider: 'ldap', password_expires_at: 2.minutes.ago) }
it { is_expected.to be_allowed(:access_api) }
end
@@ -445,15 +443,13 @@ RSpec.describe GlobalPolicy do
context 'user with expired password' do
before do
- current_user.update!(password_expires_at: 2.minutes.ago, password_automatically_set: true)
+ current_user.update!(password_expires_at: 2.minutes.ago)
end
it { is_expected.not_to be_allowed(:access_git) }
context 'when user is using ldap' do
- before do
- allow(current_user).to receive(:ldap_user?).and_return(true)
- end
+ let(:current_user) { create(:omniauth_user, provider: 'ldap', password_expires_at: 2.minutes.ago) }
it { is_expected.to be_allowed(:access_git) }
end
@@ -537,15 +533,13 @@ RSpec.describe GlobalPolicy do
context 'user with expired password' do
before do
- current_user.update!(password_expires_at: 2.minutes.ago, password_automatically_set: true)
+ current_user.update!(password_expires_at: 2.minutes.ago)
end
it { is_expected.not_to be_allowed(:use_slash_commands) }
context 'when user is using ldap' do
- before do
- allow(current_user).to receive(:ldap_user?).and_return(true)
- end
+ let(:current_user) { create(:omniauth_user, provider: 'ldap', password_expires_at: 2.minutes.ago) }
it { is_expected.to be_allowed(:use_slash_commands) }
end
diff --git a/spec/requests/api/import_bitbucket_server_spec.rb b/spec/requests/api/import_bitbucket_server_spec.rb
index 2225f737f36..970416c7444 100644
--- a/spec/requests/api/import_bitbucket_server_spec.rb
+++ b/spec/requests/api/import_bitbucket_server_spec.rb
@@ -28,6 +28,20 @@ RSpec.describe API::ImportBitbucketServer do
Grape::Endpoint.before_each nil
end
+ it 'rejects requests when Bitbucket Server Importer is disabled' do
+ stub_application_setting(import_sources: nil)
+
+ post api("/import/bitbucket_server", user), params: {
+ bitbucket_server_url: base_uri,
+ bitbucket_server_username: user,
+ personal_access_token: token,
+ bitbucket_server_project: project_key,
+ bitbucket_server_repo: repo_slug
+ }
+
+ expect(response).to have_gitlab_http_status(:forbidden)
+ end
+
it 'returns 201 response when the project is imported successfully' do
allow(Gitlab::BitbucketServerImport::ProjectCreator)
.to receive(:new).with(project_key, repo_slug, anything, repo_slug, user.namespace, user, anything)
diff --git a/spec/requests/git_http_spec.rb b/spec/requests/git_http_spec.rb
index 34f8a479719..15004b798da 100644
--- a/spec/requests/git_http_spec.rb
+++ b/spec/requests/git_http_spec.rb
@@ -61,7 +61,7 @@ RSpec.describe 'Git HTTP requests' do
shared_examples 'operations are not allowed with expired password' do
context "when password is expired" do
it "responds to downloads with status 401 Unauthorized" do
- user.update!(password_expires_at: 2.days.ago, password_automatically_set: true)
+ user.update!(password_expires_at: 2.days.ago)
download(path, user: user.username, password: user.password) do |response|
expect(response).to have_gitlab_http_status(:unauthorized)
@@ -69,7 +69,7 @@ RSpec.describe 'Git HTTP requests' do
end
it "responds to uploads with status 401 Unauthorized" do
- user.update!(password_expires_at: 2.days.ago, password_automatically_set: true)
+ user.update!(password_expires_at: 2.days.ago)
upload(path, user: user.username, password: user.password) do |response|
expect(response).to have_gitlab_http_status(:unauthorized)
@@ -614,7 +614,7 @@ RSpec.describe 'Git HTTP requests' do
context "when password is expired" do
it "responds to downloads with status 401 unauthorized" do
- user.update!(password_expires_at: 2.days.ago, password_automatically_set: true)
+ user.update!(password_expires_at: 2.days.ago)
download(path, **env) do |response|
expect(response).to have_gitlab_http_status(:unauthorized)
@@ -697,7 +697,7 @@ RSpec.describe 'Git HTTP requests' do
context "when password is expired" do
it "responds to uploads with status 401 unauthorized" do
- user.update!(password_expires_at: 2.days.ago, password_automatically_set: true)
+ user.update!(password_expires_at: 2.days.ago)
write_access_token = create(:personal_access_token, user: user, scopes: [:write_repository])
@@ -946,7 +946,7 @@ RSpec.describe 'Git HTTP requests' do
context 'when users password is expired' do
it 'rejects pulls with 401 unauthorized' do
- user.update!(password_expires_at: 2.days.ago, password_automatically_set: true)
+ user.update!(password_expires_at: 2.days.ago)
download(path, user: 'gitlab-ci-token', password: build.token) do |response|
expect(response).to have_gitlab_http_status(:unauthorized)
@@ -1241,7 +1241,7 @@ RSpec.describe 'Git HTTP requests' do
context "when password is expired" do
it "responds to downloads with status 401 unauthorized" do
- user.update!(password_expires_at: 2.days.ago, password_automatically_set: true)
+ user.update!(password_expires_at: 2.days.ago)
download(path, **env) do |response|
expect(response).to have_gitlab_http_status(:unauthorized)
@@ -1324,7 +1324,7 @@ RSpec.describe 'Git HTTP requests' do
context "when password is expired" do
it "responds to uploads with status 401 unauthorized" do
- user.update!(password_expires_at: 2.days.ago, password_automatically_set: true)
+ user.update!(password_expires_at: 2.days.ago)
write_access_token = create(:personal_access_token, user: user, scopes: [:write_repository])
@@ -1547,7 +1547,7 @@ RSpec.describe 'Git HTTP requests' do
context 'when users password is expired' do
it 'rejects pulls with 401 unauthorized' do
- user.update!(password_expires_at: 2.days.ago, password_automatically_set: true)
+ user.update!(password_expires_at: 2.days.ago)
download(path, user: 'gitlab-ci-token', password: build.token) do |response|
expect(response).to have_gitlab_http_status(:unauthorized)
diff --git a/spec/requests/lfs_http_spec.rb b/spec/requests/lfs_http_spec.rb
index 02eb4262690..656ae744ac1 100644
--- a/spec/requests/lfs_http_spec.rb
+++ b/spec/requests/lfs_http_spec.rb
@@ -126,7 +126,7 @@ RSpec.describe 'Git LFS API and storage' do
it_behaves_like 'LFS http 200 blob response'
context 'when user password is expired' do
- let_it_be(:user) { create(:user, password_expires_at: 1.minute.ago, password_automatically_set: true)}
+ let_it_be(:user) { create(:user, password_expires_at: 1.minute.ago)}
it_behaves_like 'LFS http 401 response'
end
@@ -344,7 +344,7 @@ RSpec.describe 'Git LFS API and storage' do
end
context 'when user password is expired' do
- let_it_be(:user) { create(:user, password_expires_at: 1.minute.ago, password_automatically_set: true)}
+ let_it_be(:user) { create(:user, password_expires_at: 1.minute.ago)}
let(:role) { :reporter}
@@ -958,7 +958,7 @@ RSpec.describe 'Git LFS API and storage' do
it_behaves_like 'LFS http 200 workhorse response'
context 'when user password is expired' do
- let_it_be(:user) { create(:user, password_expires_at: 1.minute.ago, password_automatically_set: true) }
+ let_it_be(:user) { create(:user, password_expires_at: 1.minute.ago) }
it_behaves_like 'LFS http 401 response'
end
diff --git a/spec/services/projects/destroy_service_spec.rb b/spec/services/projects/destroy_service_spec.rb
index 4a76347ea45..27688d8c966 100644
--- a/spec/services/projects/destroy_service_spec.rb
+++ b/spec/services/projects/destroy_service_spec.rb
@@ -450,6 +450,16 @@ RSpec.describe Projects::DestroyService, :aggregate_failures do
end
end
+ context 'when project has project bots' do
+ let!(:project_bot) { create(:user, :project_bot).tap { |user| project.add_maintainer(user) } }
+
+ it 'deletes bot user as well' do
+ expect do
+ destroy_project(project, user)
+ end.to change { User.find_by(id: project_bot.id) }.to(nil)
+ end
+ end
+
context 'error while destroying', :sidekiq_inline do
let!(:pipeline) { create(:ci_pipeline, project: project) }
let!(:builds) { create_list(:ci_build, 2, :artifacts, pipeline: pipeline) }