diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2023-06-21 12:09:11 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2023-06-21 12:09:11 +0300 |
commit | 49abdb108a4d3c3f2ef9b27c7c4dcde43da1016a (patch) | |
tree | 31dee66af9f14c3bd320c349810d877d96fd66cf /spec | |
parent | 760dc7721406a82bbea06f3512a4e270d0bb3f0a (diff) |
Add latest changes from gitlab-org/gitlab@master
Diffstat (limited to 'spec')
35 files changed, 692 insertions, 501 deletions
diff --git a/spec/controllers/groups/children_controller_spec.rb b/spec/controllers/groups/children_controller_spec.rb index ee8b2dce298..82dd8c18cfd 100644 --- a/spec/controllers/groups/children_controller_spec.rb +++ b/spec/controllers/groups/children_controller_spec.rb @@ -222,13 +222,13 @@ RSpec.describe Groups::ChildrenController, feature_category: :groups_and_project control = ActiveRecord::QueryRecorder.new { get_list } _new_project = create(:project, :public, namespace: group) - expect { get_list }.not_to exceed_query_limit(control).with_threshold(expected_queries_per_project) + expect { get_list }.not_to exceed_query_limit(control).with_threshold(expected_queries_per_project + 1) end context 'when rendering hierarchies' do # When loading hierarchies we load the all the ancestors for matched projects - # in 2 separate queries - let(:extra_queries_for_hierarchies) { 2 } + # in 3 separate queries + let(:extra_queries_for_hierarchies) { 3 } def get_filtered_list get :index, params: { group_id: group.to_param, filter: 'filter' }, format: :json diff --git a/spec/controllers/projects/design_management/designs/resized_image_controller_spec.rb b/spec/controllers/projects/design_management/designs/resized_image_controller_spec.rb index 5ed670a4f0b..b4667b4568f 100644 --- a/spec/controllers/projects/design_management/designs/resized_image_controller_spec.rb +++ b/spec/controllers/projects/design_management/designs/resized_image_controller_spec.rb @@ -59,7 +59,7 @@ RSpec.describe Projects::DesignManagement::Designs::ResizedImageController, feat end it 'sets appropriate caching headers' do - expect(response.header['Cache-Control']).to eq('private') + expect(response.header['Cache-Control']).to eq('max-age=0, private, must-revalidate') expect(response.header['ETag']).to be_present end end diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb index 5e9135c00e3..f9ce77a44ba 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/spec/controllers/projects/issues_controller_spec.rb @@ -1809,7 +1809,7 @@ RSpec.describe Projects::IssuesController, :request_store, feature_category: :te create(:user_status, user: second_discussion.author) expect { get :discussions, params: { namespace_id: project.namespace, project_id: project, id: issue.iid } } - .not_to exceed_query_limit(control) + .not_to exceed_query_limit(control).with_threshold(9) end context 'when user is setting notes filters' do diff --git a/spec/controllers/projects/pipelines_controller_spec.rb b/spec/controllers/projects/pipelines_controller_spec.rb index 8c5f8fc6259..a5542a2b825 100644 --- a/spec/controllers/projects/pipelines_controller_spec.rb +++ b/spec/controllers/projects/pipelines_controller_spec.rb @@ -328,7 +328,7 @@ RSpec.describe Projects::PipelinesController, feature_category: :continuous_inte expect do get_pipeline_html expect(response).to have_gitlab_http_status(:ok) - end.not_to exceed_all_query_limit(control) + end.not_to exceed_all_query_limit(control).with_threshold(3) end end diff --git a/spec/features/dashboard/issues_filter_spec.rb b/spec/features/dashboard/issues_filter_spec.rb index 964ac2f714d..ab3aa29a3aa 100644 --- a/spec/features/dashboard/issues_filter_spec.rb +++ b/spec/features/dashboard/issues_filter_spec.rb @@ -61,10 +61,15 @@ RSpec.describe 'Dashboard Issues filtering', :js, feature_category: :team_planni auto_discovery_link = find('link[type="application/atom+xml"]', visible: false) auto_discovery_params = CGI.parse(URI.parse(auto_discovery_link[:href]).query) - expect(params).to include('feed_token' => [user.feed_token]) + feed_token_param = params['feed_token'] + expect(feed_token_param).to match([Gitlab::Auth::AuthFinders::PATH_DEPENDENT_FEED_TOKEN_REGEX]) + expect(feed_token_param.first).to end_with(user.id.to_s) expect(params).to include('milestone_title' => ['']) expect(params).to include('assignee_username' => [user.username.to_s]) - expect(auto_discovery_params).to include('feed_token' => [user.feed_token]) + + feed_token_param = auto_discovery_params['feed_token'] + expect(feed_token_param).to match([Gitlab::Auth::AuthFinders::PATH_DEPENDENT_FEED_TOKEN_REGEX]) + expect(feed_token_param.first).to end_with(user.id.to_s) expect(auto_discovery_params).to include('milestone_title' => ['']) expect(auto_discovery_params).to include('assignee_username' => [user.username.to_s]) end diff --git a/spec/finders/clusters/agent_tokens_finder_spec.rb b/spec/finders/clusters/agent_tokens_finder_spec.rb index 1f5bfd58e85..16fdbc1b669 100644 --- a/spec/finders/clusters/agent_tokens_finder_spec.rb +++ b/spec/finders/clusters/agent_tokens_finder_spec.rb @@ -47,10 +47,7 @@ RSpec.describe Clusters::AgentTokensFinder do context 'when filtering by an unrecognised status' do subject(:execute) { described_class.new(agent, user, status: 'dummy').execute } - it 'raises an error' do - # 'dummy' is not a valid status as defined in the AgentToken status enum - expect { execute.count }.to raise_error(ActiveRecord::StatementInvalid) - end + it { is_expected.to be_empty } end context 'when user does not have permission' do diff --git a/spec/frontend/vue_shared/components/markdown/non_gfm_markdown_spec.js b/spec/frontend/vue_shared/components/markdown/non_gfm_markdown_spec.js new file mode 100644 index 00000000000..cd73ef6892a --- /dev/null +++ b/spec/frontend/vue_shared/components/markdown/non_gfm_markdown_spec.js @@ -0,0 +1,157 @@ +import { nextTick } from 'vue'; +import Markdown from '~/vue_shared/components/markdown/non_gfm_markdown.vue'; +import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; +import CodeBlockHighlighted from '~/vue_shared/components/code_block_highlighted.vue'; +import ModalCopyButton from '~/vue_shared/components/modal_copy_button.vue'; + +describe('NonGitlabMarkdown', () => { + let wrapper; + + const createComponent = ({ propsData = {} } = {}) => { + wrapper = shallowMountExtended(Markdown, { + propsData, + }); + }; + + const codeBlockContent = 'stages:\n - build\n - test\n - deploy\n'; + const codeBlockLanguage = 'yaml'; + const nonCodeContent = + "Certainly! Here's an updated GitLab CI/CD configuration in YAML format that includes Kubernetes deployment:"; + const testMarkdownWithCodeBlock = `${nonCodeContent}\n\n\`\`\`${codeBlockLanguage}\n${codeBlockContent}\n\`\`\`\n\nIn this updated configuration, we have added a \`deploy\` job that deploys the Python app to a Kubernetes cluster. The \`script\` section of the job includes commands to authenticate with GCP, set the project and zone, configure kubectl to use the GKE cluster, and deploy the application using a deployment.yaml file.\n\nNote that you will need to modify this configuration to fit your specific deployment needs, including replacing the placeholders (\`<PROJECT_ID>\`, \`<COMPUTE_ZONE>\`, \`<CLUSTER_NAME>\`, and \`<COMPUTE_REGION>\`) with your GCP and Kubernetes deployment information, and creating the deployment.yaml file with your Kubernetes deployment configuration.`; + const codeOnlyMarkdown = `\`\`\`${codeBlockLanguage}\n${codeBlockContent}\n\`\`\``; + const markdownWithMultipleCodeSnippets = `${testMarkdownWithCodeBlock}\n${testMarkdownWithCodeBlock}`; + const codeBlockNoLanguage = ` + \`\`\` + const foo = 'bar'; + \`\`\` + `; + + const findCodeBlock = () => wrapper.findComponent(CodeBlockHighlighted); + const findCopyCodeButton = () => wrapper.findComponent(ModalCopyButton); + const findCodeBlockWrapper = () => wrapper.findByTestId('code-block-wrapper'); + const findMarkdownBlock = () => wrapper.findByTestId('non-code-markdown'); + + describe('rendering markdown without code snippet', () => { + beforeEach(() => { + createComponent({ propsData: { markdown: nonCodeContent } }); + }); + it('should render non-code content', () => { + const markdownBlock = findMarkdownBlock(); + expect(markdownBlock.exists()).toBe(true); + expect(markdownBlock.text()).toBe(nonCodeContent); + }); + it('should not render code block', () => { + const codeBlock = findCodeBlock(); + expect(codeBlock.exists()).toBe(false); + }); + }); + + describe('rendering code snippet without other markdown', () => { + beforeEach(() => { + createComponent({ propsData: { markdown: codeOnlyMarkdown } }); + }); + it('should not render non-code content', () => { + const markdownBlock = findMarkdownBlock(); + expect(markdownBlock.exists()).toBe(false); + }); + it('should render code block', () => { + const codeBlock = findCodeBlock(); + expect(codeBlock.exists()).toBe(true); + }); + }); + + describe('rendering code snippet with no language specified', () => { + beforeEach(() => { + createComponent({ propsData: { markdown: codeBlockNoLanguage } }); + }); + + it('should render code block', () => { + const codeBlock = findCodeBlock(); + expect(codeBlock.exists()).toBe(true); + expect(codeBlock.props('language')).toBe('text'); + }); + }); + + describe.each` + markdown | codeBlocksCount | markdownBlocksCount + ${testMarkdownWithCodeBlock} | ${1} | ${2} + ${markdownWithMultipleCodeSnippets} | ${2} | ${3} + ${codeOnlyMarkdown} | ${1} | ${0} + ${nonCodeContent} | ${0} | ${1} + `( + 'extracting tokens in markdownBlocks computed', + ({ markdown, codeBlocksCount, markdownBlocksCount }) => { + beforeEach(() => { + createComponent({ propsData: { markdown } }); + }); + + it('should create correct number of tokens', () => { + const findAllCodeBlocks = () => wrapper.findAllByTestId('code-block-wrapper'); + const findAllMarkdownBlocks = () => wrapper.findAllByTestId('non-code-markdown'); + + expect(findAllCodeBlocks()).toHaveLength(codeBlocksCount); + expect(findAllMarkdownBlocks()).toHaveLength(markdownBlocksCount); + }); + }, + ); + + describe('rendering markdown with multiple code snippets', () => { + beforeEach(() => { + createComponent({ propsData: { markdown: markdownWithMultipleCodeSnippets } }); + }); + + it('should render code block with correct props', () => { + const codeBlock = findCodeBlock(); + expect(codeBlock.exists()).toBe(true); + expect(codeBlock.props()).toEqual( + expect.objectContaining({ + language: codeBlockLanguage, + code: codeBlockContent, + }), + ); + expect(wrapper.findAllComponents(CodeBlockHighlighted)).toHaveLength(2); + }); + + it('should not show copy code button', () => { + const copyCodeButton = findCopyCodeButton(); + expect(copyCodeButton.exists()).toBe(false); + }); + + it('should render non-code content', () => { + const markdownBlock = findMarkdownBlock(); + expect(markdownBlock.exists()).toBe(true); + expect(markdownBlock.text()).toContain(nonCodeContent); + }); + + describe('copy code button', () => { + beforeEach(() => { + const codeBlock = findCodeBlockWrapper(); + codeBlock.trigger('mouseenter'); + }); + + it('should render only one copy button per code block', () => { + const copyCodeButtons = wrapper.findAllComponents(ModalCopyButton); + expect(copyCodeButtons).toHaveLength(1); + }); + + it('should render code block button with correct props', () => { + const copyCodeButton = findCopyCodeButton(); + expect(copyCodeButton.exists()).toBe(true); + expect(copyCodeButton.props()).toEqual( + expect.objectContaining({ + text: codeBlockContent, + title: 'Copy code', + }), + ); + }); + + it('should hide code block button on mouseleave', async () => { + const codeBlock = findCodeBlockWrapper(); + codeBlock.trigger('mouseleave'); + await nextTick(); + const copyCodeButton = findCopyCodeButton(); + expect(copyCodeButton.exists()).toBe(false); + }); + }); + }); +}); diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb index 01be083b506..8ff36280dbc 100644 --- a/spec/helpers/application_helper_spec.rb +++ b/spec/helpers/application_helper_spec.rb @@ -590,12 +590,13 @@ RSpec.describe ApplicationHelper do it 'adds custom form builder to options and calls `form_for`' do options = { html: { class: 'foo-bar' } } - expected_options = options.merge({ builder: ::Gitlab::FormBuilders::GitlabUiFormBuilder, url: '/root' }) + expected_options = options.merge({ builder: ::Gitlab::FormBuilders::GitlabUiFormBuilder }) expect do |b| helper.gitlab_ui_form_for(user, options, &b) end.to yield_with_args(::Gitlab::FormBuilders::GitlabUiFormBuilder) - expect(helper).to have_received(:form_for).with(user, expected_options) + + expect(helper).to have_received(:form_for).with(user, a_hash_including(expected_options)) end end @@ -722,19 +723,19 @@ RSpec.describe ApplicationHelper do it 'uses print stylesheet when feature flag disabled' do stub_feature_flags(remove_startup_css: false) - expect(helper.stylesheet_link_tag_defer('test')).to eq( '<link rel="stylesheet" media="print" href="/stylesheets/test.css" />') + expect(helper.stylesheet_link_tag_defer('test')).to eq( '<link rel="stylesheet" href="/stylesheets/test.css" media="print" />') end it 'uses regular stylesheet when feature flag enabled' do stub_feature_flags(remove_startup_css: true) - expect(helper.stylesheet_link_tag_defer('test')).to eq( '<link rel="stylesheet" media="all" href="/stylesheets/test.css" />') + expect(helper.stylesheet_link_tag_defer('test')).to eq( '<link rel="stylesheet" href="/stylesheets/test.css" media="all" />') end it 'uses regular stylesheet when no_startup_css param present' do allow(helper.controller).to receive(:params).and_return({ no_startup_css: '' }) - expect(helper.stylesheet_link_tag_defer('test')).to eq( '<link rel="stylesheet" media="all" href="/stylesheets/test.css" />') + expect(helper.stylesheet_link_tag_defer('test')).to eq( '<link rel="stylesheet" href="/stylesheets/test.css" media="all" />') end end diff --git a/spec/helpers/calendar_helper_spec.rb b/spec/helpers/calendar_helper_spec.rb index 08993dd1dd0..a18ed479465 100644 --- a/spec/helpers/calendar_helper_spec.rb +++ b/spec/helpers/calendar_helper_spec.rb @@ -8,7 +8,10 @@ RSpec.describe CalendarHelper do it "includes the current_user's feed_token" do current_user = create(:user) allow(helper).to receive(:current_user).and_return(current_user) - expect(helper.calendar_url_options).to include feed_token: current_user.feed_token + + feed_token = helper.calendar_url_options[:feed_token] + expect(feed_token).to match(Gitlab::Auth::AuthFinders::PATH_DEPENDENT_FEED_TOKEN_REGEX) + expect(feed_token).to end_with(current_user.id.to_s) end end diff --git a/spec/helpers/feed_token_helper_spec.rb b/spec/helpers/feed_token_helper_spec.rb new file mode 100644 index 00000000000..4382758965c --- /dev/null +++ b/spec/helpers/feed_token_helper_spec.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe FeedTokenHelper, feature_category: :system_access do + describe '#generate_feed_token' do + context 'with type :atom' do + let(:current_user) { build(:user, feed_token: 'KNOWN VALUE') } + + it "returns the current_user's atom feed_token" do + allow(helper).to receive(:current_user).and_return(current_user) + allow(helper).to receive(:current_request).and_return(instance_double(ActionDispatch::Request, path: 'url')) + + expect(helper.generate_feed_token(:atom)) + # The middle part is the output of OpenSSL::HMAC.hexdigest("SHA256", 'KNOWN VALUE', 'url.atom') + .to eq("glft-a8cc74ccb0de004d09a968705ba49099229b288b3de43f26c473a9d8d7fb7693-#{current_user.id}") + end + end + + context 'when signed out' do + it "returns nil" do + allow(helper).to receive(:current_user).and_return(nil) + + expect(helper.generate_feed_token(:atom)).to be_nil + end + end + end +end diff --git a/spec/helpers/rss_helper_spec.rb b/spec/helpers/rss_helper_spec.rb index 05f6ebb6c1b..f99a1f6d547 100644 --- a/spec/helpers/rss_helper_spec.rb +++ b/spec/helpers/rss_helper_spec.rb @@ -8,7 +8,10 @@ RSpec.describe RssHelper do it "includes the current_user's feed_token" do current_user = create(:user) allow(helper).to receive(:current_user).and_return(current_user) - expect(helper.rss_url_options).to include feed_token: current_user.feed_token + + feed_token = helper.rss_url_options[:feed_token] + expect(feed_token).to match(Gitlab::Auth::AuthFinders::PATH_DEPENDENT_FEED_TOKEN_REGEX) + expect(feed_token).to end_with(current_user.id.to_s) end end diff --git a/spec/initializers/00_rails_disable_joins_spec.rb b/spec/initializers/00_rails_disable_joins_spec.rb deleted file mode 100644 index 3b390f1ef17..00000000000 --- a/spec/initializers/00_rails_disable_joins_spec.rb +++ /dev/null @@ -1,288 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe 'DisableJoins' do - let(:primary_model) do - Class.new(ApplicationRecord) do - self.table_name = '_test_primary_records' - - def self.name - 'TestPrimary' - end - end - end - - let(:bridge_model) do - Class.new(ApplicationRecord) do - self.table_name = '_test_bridge_records' - - def self.name - 'TestBridge' - end - end - end - - let(:secondary_model) do - Class.new(ApplicationRecord) do - self.table_name = '_test_secondary_records' - - def self.name - 'TestSecondary' - end - end - end - - context 'passing disable_joins as an association option' do - context 'when the association is a bare has_one' do - it 'disallows the disable_joins option' do - expect do - primary_model.has_one :test_bridge, disable_joins: true - end.to raise_error(ArgumentError, /Unknown key: :disable_joins/) - end - end - - context 'when the association is a belongs_to' do - it 'disallows the disable_joins option' do - expect do - bridge_model.belongs_to :test_secondary, disable_joins: true - end.to raise_error(ArgumentError, /Unknown key: :disable_joins/) - end - end - - context 'when the association is has_one :through' do - it 'allows the disable_joins option' do - primary_model.has_one :test_bridge - bridge_model.belongs_to :test_secondary - - expect do - primary_model.has_one :test_secondary, through: :test_bridge, disable_joins: true - end.not_to raise_error - end - end - - context 'when the association is a bare has_many' do - it 'disallows the disable_joins option' do - expect do - primary_model.has_many :test_bridges, disable_joins: true - end.to raise_error(ArgumentError, /Unknown key: :disable_joins/) - end - end - - context 'when the association is a has_many :through' do - it 'allows the disable_joins option' do - primary_model.has_many :test_bridges - bridge_model.belongs_to :test_secondary - - expect do - primary_model.has_many :test_secondaries, through: :test_bridges, disable_joins: true - end.not_to raise_error - end - end - end - - context 'querying has_one :through when disable_joins is set' do - before do - create_tables(<<~SQL) - CREATE TABLE _test_primary_records ( - id serial NOT NULL PRIMARY KEY); - - CREATE TABLE _test_bridge_records ( - id serial NOT NULL PRIMARY KEY, - primary_record_id int NOT NULL, - secondary_record_id int NOT NULL); - - CREATE TABLE _test_secondary_records ( - id serial NOT NULL PRIMARY KEY); - SQL - - primary_model.has_one :test_bridge, anonymous_class: bridge_model, foreign_key: :primary_record_id - bridge_model.belongs_to :test_secondary, anonymous_class: secondary_model, foreign_key: :secondary_record_id - primary_model.has_one :test_secondary, - through: :test_bridge, anonymous_class: secondary_model, disable_joins: -> { joins_disabled_flag } - - primary_record = primary_model.create! - secondary_record = secondary_model.create! - bridge_model.create!(primary_record_id: primary_record.id, secondary_record_id: secondary_record.id) - end - - context 'when disable_joins evaluates to true' do - let(:joins_disabled_flag) { true } - - it 'executes separate queries' do - primary_record = primary_model.first - - query_count = ActiveRecord::QueryRecorder.new { primary_record.test_secondary }.count - - expect(query_count).to eq(2) - end - end - - context 'when disable_joins evalutes to false' do - let(:joins_disabled_flag) { false } - - it 'executes a single query' do - primary_record = primary_model.first - - query_count = ActiveRecord::QueryRecorder.new { primary_record.test_secondary }.count - - expect(query_count).to eq(1) - end - end - end - - context 'querying has_many :through when disable_joins is set' do - before do - create_tables(<<~SQL) - CREATE TABLE _test_primary_records ( - id serial NOT NULL PRIMARY KEY); - - CREATE TABLE _test_bridge_records ( - id serial NOT NULL PRIMARY KEY, - primary_record_id int NOT NULL); - - CREATE TABLE _test_secondary_records ( - id serial NOT NULL PRIMARY KEY, - bridge_record_id int NOT NULL); - SQL - - primary_model.has_many :test_bridges, anonymous_class: bridge_model, foreign_key: :primary_record_id - bridge_model.has_many :test_secondaries, anonymous_class: secondary_model, foreign_key: :bridge_record_id - primary_model.has_many :test_secondaries, through: :test_bridges, anonymous_class: secondary_model, - disable_joins: -> { disabled_join_flag } - - primary_record = primary_model.create! - bridge_record = bridge_model.create!(primary_record_id: primary_record.id) - secondary_model.create!(bridge_record_id: bridge_record.id) - end - - context 'when disable_joins evaluates to true' do - let(:disabled_join_flag) { true } - - it 'executes separate queries' do - primary_record = primary_model.first - - query_count = ActiveRecord::QueryRecorder.new { primary_record.test_secondaries.first }.count - - expect(query_count).to eq(2) - end - end - - context 'when disable_joins evalutes to false' do - let(:disabled_join_flag) { false } - - it 'executes a single query' do - primary_record = primary_model.first - - query_count = ActiveRecord::QueryRecorder.new { primary_record.test_secondaries.first }.count - - expect(query_count).to eq(1) - end - end - end - - context 'querying STI relationships' do - let(:child_bridge_model) do - Class.new(bridge_model) do - def self.name - 'ChildBridge' - end - end - end - - let(:child_secondary_model) do - Class.new(secondary_model) do - def self.name - 'ChildSecondary' - end - end - end - - before do - create_tables(<<~SQL) - CREATE TABLE _test_primary_records ( - id serial NOT NULL PRIMARY KEY); - - CREATE TABLE _test_bridge_records ( - id serial NOT NULL PRIMARY KEY, - primary_record_id int NOT NULL, - type text); - - CREATE TABLE _test_secondary_records ( - id serial NOT NULL PRIMARY KEY, - bridge_record_id int NOT NULL, - type text); - SQL - - primary_model.has_many :child_bridges, anonymous_class: child_bridge_model, foreign_key: :primary_record_id - child_bridge_model.has_one :child_secondary, anonymous_class: child_secondary_model, foreign_key: :bridge_record_id - primary_model.has_many :child_secondaries, through: :child_bridges, anonymous_class: child_secondary_model, disable_joins: true - - primary_record = primary_model.create! - parent_bridge_record = bridge_model.create!(primary_record_id: primary_record.id) - child_bridge_record = child_bridge_model.create!(primary_record_id: primary_record.id) - - secondary_model.create!(bridge_record_id: child_bridge_record.id) - child_secondary_model.create!(bridge_record_id: parent_bridge_record.id) - child_secondary_model.create!(bridge_record_id: child_bridge_record.id) - end - - it 'filters correctly by the STI type across multiple queries' do - primary_record = primary_model.first - - query_recorder = ActiveRecord::QueryRecorder.new do - expect(primary_record.child_secondaries.count).to eq(1) - end - - expect(query_recorder.count).to eq(2) - end - end - - context 'querying polymorphic relationships' do - before do - create_tables(<<~SQL) - CREATE TABLE _test_primary_records ( - id serial NOT NULL PRIMARY KEY); - - CREATE TABLE _test_bridge_records ( - id serial NOT NULL PRIMARY KEY, - primaryable_id int NOT NULL, - primaryable_type text NOT NULL); - - CREATE TABLE _test_secondary_records ( - id serial NOT NULL PRIMARY KEY, - bridgeable_id int NOT NULL, - bridgeable_type text NOT NULL); - SQL - - primary_model.has_many :test_bridges, anonymous_class: bridge_model, foreign_key: :primaryable_id, as: :primaryable - bridge_model.has_one :test_secondaries, anonymous_class: secondary_model, foreign_key: :bridgeable_id, as: :bridgeable - primary_model.has_many :test_secondaries, through: :test_bridges, anonymous_class: secondary_model, disable_joins: true - - primary_record = primary_model.create! - primary_bridge_record = bridge_model.create!(primaryable_id: primary_record.id, primaryable_type: 'TestPrimary') - nonprimary_bridge_record = bridge_model.create!(primaryable_id: primary_record.id, primaryable_type: 'NonPrimary') - - secondary_model.create!(bridgeable_id: primary_bridge_record.id, bridgeable_type: 'TestBridge') - secondary_model.create!(bridgeable_id: nonprimary_bridge_record.id, bridgeable_type: 'TestBridge') - secondary_model.create!(bridgeable_id: primary_bridge_record.id, bridgeable_type: 'NonBridge') - end - - it 'filters correctly by the polymorphic type across multiple queries' do - primary_record = primary_model.first - - query_recorder = ActiveRecord::QueryRecorder.new do - expect(primary_record.test_secondaries.count).to eq(1) - end - - expect(query_recorder.count).to eq(2) - end - end - - def create_tables(table_sql) - ApplicationRecord.connection.execute(table_sql) - - bridge_model.reset_column_information - secondary_model.reset_column_information - end -end diff --git a/spec/initializers/100_patch_omniauth_saml_spec.rb b/spec/initializers/100_patch_omniauth_saml_spec.rb index de556cfa1e5..886f350ca88 100644 --- a/spec/initializers/100_patch_omniauth_saml_spec.rb +++ b/spec/initializers/100_patch_omniauth_saml_spec.rb @@ -6,6 +6,15 @@ RSpec.describe 'OmniAuth::Strategies::SAML', type: :strategy do let(:idp_sso_target_url) { 'https://login.example.com/idp' } let(:strategy) { [OmniAuth::Strategies::SAML, { idp_sso_target_url: idp_sso_target_url }] } + before do + mock_session = {} + + allow(mock_session).to receive(:enabled?).and_return(true) + allow(mock_session).to receive(:loaded?).and_return(true) + + env('rack.session', mock_session) + end + describe 'POST /users/auth/saml' do it 'redirects to the provider login page', :aggregate_failures do post '/users/auth/saml' diff --git a/spec/initializers/action_dispatch_journey_router_spec.rb b/spec/initializers/action_dispatch_journey_router_spec.rb new file mode 100644 index 00000000000..641a8c6d11f --- /dev/null +++ b/spec/initializers/action_dispatch_journey_router_spec.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +require 'spec_helper' + +# Adds a missing test to provide full coverage for the patch +RSpec.describe 'ActionDispatch::Journey::Router Patch', feature_category: :database do + before do + load Rails.root.join('config/initializers/action_dispatch_journey_router.rb') + end + + describe '#find_routes' do + context 'when a route has additional constrains' do + it 'does not raise an error' do + stub_const('PagesController', Class.new(ApplicationController)) + + set = ActionDispatch::Routing::RouteSet.new + + set.draw do + get "*namespace_id/:project_id/bar", + to: "pages#show", + constraints: { + namespace_id: %r{(?!api/)[a-zA-Z0-9_\\]+}, + project_id: /[a-zA-Z0-9]+/ + } + + get "/api/foo/bar", to: "pages#index" + end + + params = set.recognize_path("/api/foo/bar", method: :get) + + expect(params[:controller]).to eq('pages') + expect(params[:action]).to eq('index') + end + end + end +end diff --git a/spec/initializers/activerecord_postgresql_timestamp_with_timezone_patches_spec.rb b/spec/initializers/activerecord_postgresql_timestamp_with_timezone_patches_spec.rb new file mode 100644 index 00000000000..8ad73a0b890 --- /dev/null +++ b/spec/initializers/activerecord_postgresql_timestamp_with_timezone_patches_spec.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +require 'spec_helper' + +# A missing test to provide full coverage for the patch +RSpec.describe 'ActiveRecord PostgreSQL Timespamp With Timezone', feature_category: :database do + before do + load Rails.root.join('config/initializers/activerecord_postgresql_timestamp_with_timezone_patches.rb') + end + + describe '#cast_value' do + it 'returns local time' do + timestamp = ActiveRecord::ConnectionAdapters::PostgreSQL::OID::TimestampWithTimeZone.new + + allow(ActiveRecord).to receive(:default_timezone).and_return(:local) + + expect(timestamp.cast_value(DateTime.now)).not_to be_utc + end + end +end diff --git a/spec/lib/gitlab/auth/auth_finders_spec.rb b/spec/lib/gitlab/auth/auth_finders_spec.rb index 4498e369695..1a8a2ec2980 100644 --- a/spec/lib/gitlab/auth/auth_finders_spec.rb +++ b/spec/lib/gitlab/auth/auth_finders_spec.rb @@ -181,7 +181,7 @@ RSpec.describe Gitlab::Auth::AuthFinders, feature_category: :system_access do set_header('HTTP_ACCEPT', 'application/atom+xml') end - context 'when feed_token param is provided' do + context 'when old format feed_token param is provided' do it 'returns user if valid feed_token' do set_param(:feed_token, user.feed_token) @@ -206,7 +206,44 @@ RSpec.describe Gitlab::Auth::AuthFinders, feature_category: :system_access do end end - context 'when rss_token param is provided' do + context 'when path-dependent format feed_token param is provided' do + let_it_be(:feed_user, freeze: true) { create(:user, feed_token: 'KNOWN VALUE').tap(&:feed_token) } + # The middle part is the output of OpenSSL::HMAC.hexdigest("SHA256", 'KNOWN VALUE', 'url.atom') + let(:feed_token) { "glft-a8cc74ccb0de004d09a968705ba49099229b288b3de43f26c473a9d8d7fb7693-#{feed_user.id}" } + + it 'returns user if valid feed_token' do + set_param(:feed_token, feed_token) + + expect(find_user_from_feed_token(:rss)).to eq feed_user + end + + it 'returns nil if valid feed_token and disabled' do + allow(Gitlab::CurrentSettings).to receive_messages(disable_feed_token: true) + set_param(:feed_token, feed_token) + + expect(find_user_from_feed_token(:rss)).to be_nil + end + + it 'returns exception if token has same HMAC but different user ID' do + set_param(:feed_token, "glft-a8cc74ccb0de004d09a968705ba49099229b288b3de43f26c473a9d8d7fb7693-#{user.id}") + + expect { find_user_from_feed_token(:rss) }.to raise_error(Gitlab::Auth::UnauthorizedError) + end + + it 'returns exception if token has wrong HMAC but same user ID' do + set_param(:feed_token, "glft-aaaaaaaaaade004d09a968705ba49099229b288b3de43f26c473a9d8d7fb7693-#{feed_user.id}") + + expect { find_user_from_feed_token(:rss) }.to raise_error(Gitlab::Auth::UnauthorizedError) + end + + it 'returns exception if user does not exist' do + set_param(:feed_token, "glft-a8cc74ccb0de004d09a968705ba49099229b288b3de43f26c473a9d8d7fb7693-#{non_existing_record_id}") + + expect { find_user_from_feed_token(:rss) }.to raise_error(Gitlab::Auth::UnauthorizedError) + end + end + + context 'when old format rss_token param is provided' do it 'returns user if valid rss_token' do set_param(:rss_token, user.feed_token) diff --git a/spec/lib/gitlab/database/postgresql_adapter/empty_query_ping_spec.rb b/spec/lib/gitlab/database/postgresql_adapter/empty_query_ping_spec.rb deleted file mode 100644 index 6e1e53e0e41..00000000000 --- a/spec/lib/gitlab/database/postgresql_adapter/empty_query_ping_spec.rb +++ /dev/null @@ -1,43 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Gitlab::Database::PostgresqlAdapter::EmptyQueryPing do - describe '#active?' do - let(:adapter_class) do - Class.new do - include Gitlab::Database::PostgresqlAdapter::EmptyQueryPing - - def initialize(connection, lock) - @connection = connection - @lock = lock - end - end - end - - subject { adapter_class.new(connection, lock).active? } - - let(:connection) { double(query: nil) } - let(:lock) { double } - - before do - allow(lock).to receive(:synchronize).and_yield - end - - it 'uses an empty query to check liveness' do - expect(connection).to receive(:query).with(';') - - subject - end - - it 'returns true if no error was signaled' do - expect(subject).to be_truthy - end - - it 'returns false when an error occurs' do - expect(lock).to receive(:synchronize).and_raise(PG::Error) - - expect(subject).to be_falsey - end - end -end diff --git a/spec/lib/gitlab/database/postgresql_adapter/type_map_cache_spec.rb b/spec/lib/gitlab/database/postgresql_adapter/type_map_cache_spec.rb index c6542aa2adb..75c3a3650d7 100644 --- a/spec/lib/gitlab/database/postgresql_adapter/type_map_cache_spec.rb +++ b/spec/lib/gitlab/database/postgresql_adapter/type_map_cache_spec.rb @@ -13,7 +13,7 @@ RSpec.describe Gitlab::Database::PostgresqlAdapter::TypeMapCache do describe '#initialize_type_map' do it 'caches loading of types in memory' do recorder_without_cache = ActiveRecord::QueryRecorder.new(skip_schema_queries: false) { initialize_connection.disconnect! } - expect(recorder_without_cache.log).to include(a_string_matching(/FROM pg_type/)).twice + expect(recorder_without_cache.log).to include(a_string_matching(/FROM pg_type/)).exactly(4).times recorder_with_cache = ActiveRecord::QueryRecorder.new(skip_schema_queries: false) { initialize_connection.disconnect! } @@ -33,7 +33,7 @@ RSpec.describe Gitlab::Database::PostgresqlAdapter::TypeMapCache do recorder = ActiveRecord::QueryRecorder.new(skip_schema_queries: false) { initialize_connection(other_config).disconnect! } - expect(recorder.log).to include(a_string_matching(/FROM pg_type/)).twice + expect(recorder.log).to include(a_string_matching(/FROM pg_type/)).exactly(4).times end end @@ -44,7 +44,7 @@ RSpec.describe Gitlab::Database::PostgresqlAdapter::TypeMapCache do connection = initialize_connection recorder = ActiveRecord::QueryRecorder.new(skip_schema_queries: false) { connection.reload_type_map } - expect(recorder.log).to include(a_string_matching(/FROM pg_type/)).once + expect(recorder.log).to include(a_string_matching(/FROM pg_type/)).exactly(3).times end end diff --git a/spec/lib/gitlab/database_spec.rb b/spec/lib/gitlab/database_spec.rb index ab3cd8fa5e6..789ae348dc1 100644 --- a/spec/lib/gitlab/database_spec.rb +++ b/spec/lib/gitlab/database_spec.rb @@ -103,7 +103,7 @@ RSpec.describe Gitlab::Database, feature_category: :database do before do # CI config might not be configured allow(ActiveRecord::Base.configurations).to receive(:configs_for) - .with(env_name: 'test', name: 'ci', include_replicas: true) + .with(env_name: 'test', name: 'ci', include_hidden: true) .and_return(ci_db_config) end diff --git a/spec/lib/gitlab/kas/client_spec.rb b/spec/lib/gitlab/kas/client_spec.rb index 5668c265611..811f07a6bec 100644 --- a/spec/lib/gitlab/kas/client_spec.rb +++ b/spec/lib/gitlab/kas/client_spec.rb @@ -77,8 +77,8 @@ RSpec.describe Gitlab::Kas::Client do let(:request) { instance_double(Gitlab::Agent::ConfigurationProject::Rpc::ListAgentConfigFilesRequest) } let(:response) { double(Gitlab::Agent::ConfigurationProject::Rpc::ListAgentConfigFilesResponse, config_files: agent_configurations) } - let(:repository) { instance_double(Gitlab::Agent::Modserver::Repository) } - let(:gitaly_address) { instance_double(Gitlab::Agent::Modserver::GitalyAddress) } + let(:repository) { instance_double(Gitlab::Agent::Entity::GitalyRepository) } + let(:gitaly_info) { instance_double(Gitlab::Agent::Entity::GitalyInfo) } let(:agent_configurations) { [double] } @@ -89,16 +89,16 @@ RSpec.describe Gitlab::Kas::Client do .with('example.kas.internal', :this_channel_is_insecure, timeout: described_class::TIMEOUT) .and_return(stub) - expect(Gitlab::Agent::Modserver::Repository).to receive(:new) + expect(Gitlab::Agent::Entity::GitalyRepository).to receive(:new) .with(project.repository.gitaly_repository.to_h) .and_return(repository) - expect(Gitlab::Agent::Modserver::GitalyAddress).to receive(:new) + expect(Gitlab::Agent::Entity::GitalyInfo).to receive(:new) .with(Gitlab::GitalyClient.connection_data(project.repository_storage)) - .and_return(gitaly_address) + .and_return(gitaly_info) expect(Gitlab::Agent::ConfigurationProject::Rpc::ListAgentConfigFilesRequest).to receive(:new) - .with(repository: repository, gitaly_address: gitaly_address) + .with(repository: repository, gitaly_info: gitaly_info) .and_return(request) expect(stub).to receive(:list_agent_config_files) @@ -112,7 +112,8 @@ RSpec.describe Gitlab::Kas::Client do describe '#send_git_push_event' do let(:stub) { instance_double(Gitlab::Agent::Notifications::Rpc::Notifications::Stub) } let(:request) { instance_double(Gitlab::Agent::Notifications::Rpc::GitPushEventRequest) } - let(:project_param) { instance_double(Gitlab::Agent::Notifications::Rpc::Project) } + let(:event_param) { instance_double(Gitlab::Agent::Event::GitPushEvent) } + let(:project_param) { instance_double(Gitlab::Agent::Event::Project) } let(:response) { double(Gitlab::Agent::Notifications::Rpc::GitPushEventResponse) } subject { described_class.new.send_git_push_event(project: project) } @@ -122,12 +123,16 @@ RSpec.describe Gitlab::Kas::Client do .with('example.kas.internal', :this_channel_is_insecure, timeout: described_class::TIMEOUT) .and_return(stub) - expect(Gitlab::Agent::Notifications::Rpc::Project).to receive(:new) + expect(Gitlab::Agent::Event::Project).to receive(:new) .with(id: project.id, full_path: project.full_path) .and_return(project_param) - expect(Gitlab::Agent::Notifications::Rpc::GitPushEventRequest).to receive(:new) + expect(Gitlab::Agent::Event::GitPushEvent).to receive(:new) .with(project: project_param) + .and_return(event_param) + + expect(Gitlab::Agent::Notifications::Rpc::GitPushEventRequest).to receive(:new) + .with(event: event_param) .and_return(request) expect(stub).to receive(:git_push_event) diff --git a/spec/lib/gitlab/rack_attack/request_spec.rb b/spec/lib/gitlab/rack_attack/request_spec.rb index ae0abfd0bc5..e8433d99d15 100644 --- a/spec/lib/gitlab/rack_attack/request_spec.rb +++ b/spec/lib/gitlab/rack_attack/request_spec.rb @@ -258,6 +258,11 @@ RSpec.describe Gitlab::RackAttack::Request do valid_token = SecureRandom.base64(ActionController::RequestForgeryProtection::AUTHENTICITY_TOKEN_LENGTH) other_token = SecureRandom.base64(ActionController::RequestForgeryProtection::AUTHENTICITY_TOKEN_LENGTH) + before do + allow(session).to receive(:enabled?).and_return(true) + allow(session).to receive(:loaded?).and_return(true) + end + where(:session, :env, :expected) do {} | {} | false {} | { 'HTTP_X_CSRF_TOKEN' => valid_token } | false diff --git a/spec/lib/gitlab/request_forgery_protection_spec.rb b/spec/lib/gitlab/request_forgery_protection_spec.rb index 10842173365..dbf9f295706 100644 --- a/spec/lib/gitlab/request_forgery_protection_spec.rb +++ b/spec/lib/gitlab/request_forgery_protection_spec.rb @@ -13,6 +13,11 @@ RSpec.describe Gitlab::RequestForgeryProtection, :allow_forgery_protection do } end + before do + allow(env['rack.session']).to receive(:enabled?).and_return(true) + allow(env['rack.session']).to receive(:loaded?).and_return(true) + end + it 'logs to /dev/null' do expect(ActiveSupport::Logger).to receive(:new).with(File::NULL) diff --git a/spec/lib/gitlab_settings/options_spec.rb b/spec/lib/gitlab_settings/options_spec.rb index 23cb2180edd..cd5a0adeecf 100644 --- a/spec/lib/gitlab_settings/options_spec.rb +++ b/spec/lib/gitlab_settings/options_spec.rb @@ -7,6 +7,32 @@ RSpec.describe GitlabSettings::Options, :aggregate_failures, feature_category: : subject(:options) { described_class.build(config) } + shared_examples 'do not mutate' do |method| + context 'when in production env' do + it 'returns the unchanged internal hash' do + stub_rails_env('production') + + expect(Gitlab::AppLogger) + .to receive(:warn) + .with('Warning: Do not mutate GitlabSettings::Options objects', method: method) + + expect(options.send(method)).to be_truthy + end + end + + context 'when not in production env' do + it 'raises an exception to avoid changing the internal keys' do + exception = "Warning: Do not mutate GitlabSettings::Options objects: `#{method}`" + + stub_rails_env('development') + expect { options.send(method) }.to raise_error(exception) + + stub_rails_env('test') + expect { options.send(method) }.to raise_error(exception) + end + end + end + describe '.build' do context 'when argument is a hash' do it 'creates a new GitlabSettings::Options instance' do @@ -19,6 +45,16 @@ RSpec.describe GitlabSettings::Options, :aggregate_failures, feature_category: : end end + describe '#default' do + it 'returns the option value' do + expect(options.default).to be_nil + + options['default'] = 'The default value' + + expect(options.default).to eq('The default value') + end + end + describe '#[]' do it 'accesses the configuration key as string' do expect(options['foo']).to be_a described_class @@ -96,7 +132,7 @@ RSpec.describe GitlabSettings::Options, :aggregate_failures, feature_category: : end describe '#merge' do - it 'merges a hash to the existing options' do + it 'returns a new object with the options merged' do expect(options.merge(more: 'configs').to_hash).to eq( 'foo' => { 'bar' => 'baz' }, 'more' => 'configs' @@ -104,14 +140,33 @@ RSpec.describe GitlabSettings::Options, :aggregate_failures, feature_category: : end context 'when the merge hash replaces existing configs' do - it 'merges a hash to the existing options' do + it 'returns a new object with the duplicated options replaced' do expect(options.merge(foo: 'configs').to_hash).to eq('foo' => 'configs') end end end + describe '#merge!' do + it 'merges in place with the existing options' do + options.merge!(more: 'configs') # rubocop: disable Performance/RedundantMerge + + expect(options.to_hash).to eq( + 'foo' => { 'bar' => 'baz' }, + 'more' => 'configs' + ) + end + + context 'when the merge hash replaces existing configs' do + it 'merges in place with the duplicated options replaced' do + options.merge!(foo: 'configs') # rubocop: disable Performance/RedundantMerge + + expect(options.to_hash).to eq('foo' => 'configs') + end + end + end + describe '#deep_merge' do - it 'merges a hash to the existing options' do + it 'returns a new object with the options merged' do expect(options.deep_merge(foo: { more: 'configs' }).to_hash).to eq('foo' => { 'bar' => 'baz', 'more' => 'configs' @@ -119,7 +174,24 @@ RSpec.describe GitlabSettings::Options, :aggregate_failures, feature_category: : end context 'when the merge hash replaces existing configs' do - it 'merges a hash to the existing options' do + it 'returns a new object with the duplicated options replaced' do + expect(options.deep_merge(foo: { bar: 'configs' }).to_hash).to eq('foo' => { + 'bar' => 'configs' + }) + end + end + end + + describe '#deep_merge!' do + it 'merges in place with the existing options' do + expect(options.deep_merge(foo: { more: 'configs' }).to_hash).to eq('foo' => { + 'bar' => 'baz', + 'more' => 'configs' + }) + end + + context 'when the merge hash replaces existing configs' do + it 'merges in place with the duplicated options replaced' do expect(options.deep_merge(foo: { bar: 'configs' }).to_hash).to eq('foo' => { 'bar' => 'configs' }) @@ -135,6 +207,14 @@ RSpec.describe GitlabSettings::Options, :aggregate_failures, feature_category: : end end + describe '#symbolize_keys!' do + it_behaves_like 'do not mutate', :symbolize_keys! + end + + describe '#stringify_keys!' do + it_behaves_like 'do not mutate', :stringify_keys! + end + describe '#method_missing' do context 'when method is an option' do it 'delegates methods to options keys' do @@ -149,10 +229,30 @@ RSpec.describe GitlabSettings::Options, :aggregate_failures, feature_category: : end context 'when method is not an option' do - it 'delegates the method to the internal options hash' do - expect { options.foo.delete('bar') } - .to change { options.to_hash } - .to({ 'foo' => {} }) + context 'when in production env' do + it 'delegates the method to the internal options hash' do + stub_rails_env('production') + + expect(Gitlab::AppLogger) + .to receive(:warn) + .with('Calling a hash method on GitlabSettings::Options', method: :delete) + + expect { options.foo.delete('bar') } + .to change { options.to_hash } + .to({ 'foo' => {} }) + end + end + + context 'when not in production env' do + it 'delegates the method to the internal options hash' do + exception = 'Calling a hash method on GitlabSettings::Options: `delete`' + + stub_rails_env('development') + expect { options.foo.delete('bar') }.to raise_error(exception) + + stub_rails_env('test') + expect { options.foo.delete('bar') }.to raise_error(exception) + end end end diff --git a/spec/models/abuse/user_trust_score_spec.rb b/spec/models/abuse/user_trust_score_spec.rb new file mode 100644 index 00000000000..81cf761bd24 --- /dev/null +++ b/spec/models/abuse/user_trust_score_spec.rb @@ -0,0 +1,113 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Abuse::UserTrustScore, feature_category: :instance_resiliency do + let_it_be(:user1) { create(:user) } + let_it_be(:user2) { create(:user) } + let(:user_1_scores) { described_class.new(user1) } + let(:user_2_scores) { described_class.new(user2) } + + describe '#spammer?' do + context 'when the user is a spammer' do + before do + allow(user_1_scores).to receive(:spam_score).and_return(0.9) + end + + it 'classifies the user as a spammer' do + expect(user_1_scores).to be_spammer + end + end + + context 'when the user is not a spammer' do + before do + allow(user_1_scores).to receive(:spam_score).and_return(0.1) + end + + it 'does not classify the user as a spammer' do + expect(user_1_scores).not_to be_spammer + end + end + end + + describe '#spam_score' do + context 'when the user is a spammer' do + before do + create(:abuse_trust_score, user: user1, score: 0.8) + create(:abuse_trust_score, user: user1, score: 0.9) + end + + it 'returns the expected score' do + expect(user_1_scores.spam_score).to be_within(0.01).of(0.85) + end + end + + context 'when the user is not a spammer' do + before do + create(:abuse_trust_score, user: user1, score: 0.1) + create(:abuse_trust_score, user: user1, score: 0.0) + end + + it 'returns the expected score' do + expect(user_1_scores.spam_score).to be_within(0.01).of(0.05) + end + end + end + + describe '#telesign_score' do + context 'when the user has a telesign risk score' do + before do + create(:abuse_trust_score, user: user1, score: 12.0, source: :telesign) + create(:abuse_trust_score, user: user1, score: 24.0, source: :telesign) + end + + it 'returns the latest score' do + expect(user_1_scores.telesign_score).to be(24.0) + end + end + + context 'when the user does not have a telesign risk score' do + it 'defaults to zero' do + expect(user_2_scores.telesign_score).to be(0.0) + end + end + end + + describe '#arkose_global_score' do + context 'when the user has an arkose global risk score' do + before do + create(:abuse_trust_score, user: user1, score: 12.0, source: :arkose_global_score) + create(:abuse_trust_score, user: user1, score: 24.0, source: :arkose_global_score) + end + + it 'returns the latest score' do + expect(user_1_scores.arkose_global_score).to be(24.0) + end + end + + context 'when the user does not have an arkose global risk score' do + it 'defaults to zero' do + expect(user_2_scores.arkose_global_score).to be(0.0) + end + end + end + + describe '#arkose_custom_score' do + context 'when the user has an arkose custom risk score' do + before do + create(:abuse_trust_score, user: user1, score: 12.0, source: :arkose_custom_score) + create(:abuse_trust_score, user: user1, score: 24.0, source: :arkose_custom_score) + end + + it 'returns the latest score' do + expect(user_1_scores.arkose_custom_score).to be(24.0) + end + end + + context 'when the user does not have an arkose custom risk score' do + it 'defaults to zero' do + expect(user_2_scores.arkose_custom_score).to be(0.0) + end + end + end +end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 690c0be3b7a..4fc4341673c 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -2950,56 +2950,6 @@ RSpec.describe User, feature_category: :user_profile do end end - describe '#spammer?' do - let_it_be(:user) { create(:user) } - - context 'when the user is a spammer' do - before do - allow(user).to receive(:spam_score).and_return(0.9) - end - - it 'classifies the user as a spammer' do - expect(user).to be_spammer - end - end - - context 'when the user is not a spammer' do - before do - allow(user).to receive(:spam_score).and_return(0.1) - end - - it 'does not classify the user as a spammer' do - expect(user).not_to be_spammer - end - end - end - - describe '#spam_score' do - let_it_be(:user) { create(:user) } - - context 'when the user is a spammer' do - before do - create(:abuse_trust_score, user: user, score: 0.8) - create(:abuse_trust_score, user: user, score: 0.9) - end - - it 'returns the expected score' do - expect(user.spam_score).to be_within(0.01).of(0.85) - end - end - - context 'when the user is not a spammer' do - before do - create(:abuse_trust_score, user: user, score: 0.1) - create(:abuse_trust_score, user: user, score: 0.0) - end - - it 'returns the expected score' do - expect(user.spam_score).to be_within(0.01).of(0.05) - end - end - end - describe '.find_for_database_authentication' do it 'strips whitespace from login' do user = create(:user) @@ -6145,7 +6095,9 @@ RSpec.describe User, feature_category: :user_profile do context 'when the user is a spammer' do before do - allow(user).to receive(:spammer?).and_return(true) + user_scores = Abuse::UserTrustScore.new(user) + allow(Abuse::UserTrustScore).to receive(:new).and_return(user_scores) + allow(user_scores).to receive(:spammer?).and_return(true) end context 'when the user account is less than 7 days old' do @@ -8110,70 +8062,4 @@ RSpec.describe User, feature_category: :user_profile do end end end - - describe '#telesign_score' do - let_it_be(:user1) { create(:user) } - let_it_be(:user2) { create(:user) } - - context 'when the user has a telesign risk score' do - before do - create(:abuse_trust_score, user: user1, score: 12.0, source: :telesign) - create(:abuse_trust_score, user: user1, score: 24.0, source: :telesign) - end - - it 'returns the latest score' do - expect(user1.telesign_score).to be(24.0) - end - end - - context 'when the user does not have a telesign risk score' do - it 'defaults to zero' do - expect(user2.telesign_score).to be(0.0) - end - end - end - - describe '#arkose_global_score' do - let_it_be(:user1) { create(:user) } - let_it_be(:user2) { create(:user) } - - context 'when the user has an arkose global risk score' do - before do - create(:abuse_trust_score, user: user1, score: 12.0, source: :arkose_global_score) - create(:abuse_trust_score, user: user1, score: 24.0, source: :arkose_global_score) - end - - it 'returns the latest score' do - expect(user1.arkose_global_score).to be(24.0) - end - end - - context 'when the user does not have an arkose global risk score' do - it 'defaults to zero' do - expect(user2.arkose_global_score).to be(0.0) - end - end - end - - describe '#arkose_custom_score' do - let_it_be(:user1) { create(:user) } - let_it_be(:user2) { create(:user) } - - context 'when the user has an arkose custom risk score' do - before do - create(:abuse_trust_score, user: user1, score: 12.0, source: :arkose_custom_score) - create(:abuse_trust_score, user: user1, score: 24.0, source: :arkose_custom_score) - end - - it 'returns the latest score' do - expect(user1.arkose_custom_score).to be(24.0) - end - end - - context 'when the user does not have an arkose custom risk score' do - it 'defaults to zero' do - expect(user2.arkose_custom_score).to be(0.0) - end - end - end end diff --git a/spec/requests/api/helpers_spec.rb b/spec/requests/api/helpers_spec.rb index 0be9df41e8f..7304437bc42 100644 --- a/spec/requests/api/helpers_spec.rb +++ b/spec/requests/api/helpers_spec.rb @@ -32,6 +32,9 @@ RSpec.describe API::Helpers, :enable_admin_mode, feature_category: :system_acces before do allow_any_instance_of(self.class).to receive(:options).and_return({}) + + allow(env['rack.session']).to receive(:enabled?).and_return(true) + allow(env['rack.session']).to receive(:loaded?).and_return(true) end def warden_authenticate_returns(value) @@ -567,6 +570,9 @@ RSpec.describe API::Helpers, :enable_admin_mode, feature_category: :system_acces context 'using warden authentication' do before do + allow(session).to receive(:enabled?).and_return(true) + allow(session).to receive(:loaded?).and_return(true) + warden_authenticate_returns admin env[API::Helpers::SUDO_HEADER] = user.username end diff --git a/spec/requests/api/npm_group_packages_spec.rb b/spec/requests/api/npm_group_packages_spec.rb index d97c7682b4b..efb606b23d7 100644 --- a/spec/requests/api/npm_group_packages_spec.rb +++ b/spec/requests/api/npm_group_packages_spec.rb @@ -152,6 +152,14 @@ RSpec.describe API::NpmGroupPackages, feature_category: :package_registry do it_behaves_like 'returning response status', params[:expected_status] end end + + context 'when metadata cache exists' do + let_it_be(:npm_metadata_cache) { create(:npm_metadata_cache, package_name: package.name, project_id: project.id) } + + subject { get(url) } + + it_behaves_like 'generates metadata response "on-the-fly"' + end end describe 'GET /api/v4/packages/npm/-/package/*package_name/dist-tags' do diff --git a/spec/requests/api/npm_instance_packages_spec.rb b/spec/requests/api/npm_instance_packages_spec.rb index 97de7fa9e52..63a50e7f511 100644 --- a/spec/requests/api/npm_instance_packages_spec.rb +++ b/spec/requests/api/npm_instance_packages_spec.rb @@ -45,6 +45,14 @@ RSpec.describe API::NpmInstancePackages, feature_category: :package_registry do end end end + + context 'when metadata cache exists' do + let_it_be(:npm_metadata_cache) { create(:npm_metadata_cache, package_name: package.name, project_id: project.id) } + + subject { get(url) } + + it_behaves_like 'generates metadata response "on-the-fly"' + end end describe 'GET /api/v4/packages/npm/-/package/*package_name/dist-tags' do diff --git a/spec/requests/api/npm_project_packages_spec.rb b/spec/requests/api/npm_project_packages_spec.rb index 60d4bddc502..33418e85601 100644 --- a/spec/requests/api/npm_project_packages_spec.rb +++ b/spec/requests/api/npm_project_packages_spec.rb @@ -26,6 +26,48 @@ RSpec.describe API::NpmProjectPackages, feature_category: :package_registry do it_behaves_like 'rejects invalid package names' do subject { get(url) } end + + context 'when metadata cache exists', :aggregate_failures do + let!(:npm_metadata_cache) { create(:npm_metadata_cache, package_name: package.name, project_id: project.id) } + let(:metadata) { Gitlab::Json.parse(npm_metadata_cache.file.read.gsub('dist_tags', 'dist-tags')) } + + subject { get(url) } + + before do + project.add_developer(user) + end + + it 'returns response from metadata cache' do + expect(Packages::Npm::GenerateMetadataService).not_to receive(:new) + expect(Packages::Npm::MetadataCache).to receive(:find_by_package_name_and_project_id) + .with(package.name, project.id).and_call_original + + subject + + expect(json_response).to eq(metadata) + end + + it 'bumps last_downloaded_at of metadata cache' do + expect { subject } + .to change { npm_metadata_cache.reload.last_downloaded_at }.from(nil).to(instance_of(ActiveSupport::TimeWithZone)) + end + + context 'when npm_metadata_cache disabled' do + before do + stub_feature_flags(npm_metadata_cache: false) + end + + it_behaves_like 'generates metadata response "on-the-fly"' + end + + context 'when metadata cache file does not exist' do + before do + FileUtils.rm_rf(npm_metadata_cache.file.path) + end + + it_behaves_like 'generates metadata response "on-the-fly"' + end + end end describe 'GET /api/v4/projects/:id/packages/npm/-/package/*package_name/dist-tags' do diff --git a/spec/services/packages/npm/create_metadata_cache_service_spec.rb b/spec/services/packages/npm/create_metadata_cache_service_spec.rb index 02f29dd94df..f4010a7d548 100644 --- a/spec/services/packages/npm/create_metadata_cache_service_spec.rb +++ b/spec/services/packages/npm/create_metadata_cache_service_spec.rb @@ -46,7 +46,7 @@ RSpec.describe Packages::Npm::CreateMetadataCacheService, :clean_gitlab_redis_sh new_metadata = Gitlab::Json.parse(npm_metadata_cache.file.read) expect(new_metadata).not_to eq(metadata) - expect(new_metadata['dist_tags'].keys).to include(tag_name) + expect(new_metadata['dist-tags'].keys).to include(tag_name) expect(npm_metadata_cache.reload.size).not_to eq(metadata_size) end end diff --git a/spec/services/spam/spam_verdict_service_spec.rb b/spec/services/spam/spam_verdict_service_spec.rb index 6b14cf33041..5baa476a23e 100644 --- a/spec/services/spam/spam_verdict_service_spec.rb +++ b/spec/services/spam/spam_verdict_service_spec.rb @@ -237,6 +237,8 @@ RSpec.describe Spam::SpamVerdictService, feature_category: :instance_resiliency end context 'if the endpoint is accessible' do + let(:user_scores) { Abuse::UserTrustScore.new(user) } + before do allow(service).to receive(:spamcheck_client).and_return(spam_client) allow(spam_client).to receive(:spam?).and_return(spam_client_result) @@ -248,7 +250,7 @@ RSpec.describe Spam::SpamVerdictService, feature_category: :instance_resiliency it 'returns the verdict' do is_expected.to eq(NOOP) - expect(user.spam_score).to eq(0.0) + expect(user_scores.spam_score).to eq(0.0) end end @@ -259,7 +261,7 @@ RSpec.describe Spam::SpamVerdictService, feature_category: :instance_resiliency context 'the result was evaluated' do it 'returns the verdict and updates the spam score' do is_expected.to eq(ALLOW) - expect(user.spam_score).to be_within(0.000001).of(verdict_score) + expect(user_scores.spam_score).to be_within(0.000001).of(verdict_score) end end @@ -268,7 +270,7 @@ RSpec.describe Spam::SpamVerdictService, feature_category: :instance_resiliency it 'returns the verdict and does not update the spam score' do expect(subject).to eq(ALLOW) - expect(user.spam_score).to eq(0.0) + expect(user_scores.spam_score).to eq(0.0) end end end @@ -290,7 +292,7 @@ RSpec.describe Spam::SpamVerdictService, feature_category: :instance_resiliency with_them do it "returns expected spam constant and updates the spam score" do is_expected.to eq(expected) - expect(user.spam_score).to be_within(0.000001).of(verdict_score) + expect(user_scores.spam_score).to be_within(0.000001).of(verdict_score) end end end diff --git a/spec/support/helpers/emails_helper_test_helper.rb b/spec/support/helpers/emails_helper_test_helper.rb index ea7dbc89ebd..572b2f6853d 100644 --- a/spec/support/helpers/emails_helper_test_helper.rb +++ b/spec/support/helpers/emails_helper_test_helper.rb @@ -2,7 +2,7 @@ module EmailsHelperTestHelper def default_header_logo - %r{<img alt="GitLab" src="/images/mailers/gitlab_logo\.(?:gif|png)" width="\d+" height="\d+" />} + %r{<img alt="GitLab" src="http://test.host/images/mailers/gitlab_logo\.(?:gif|png)" width="\d+" height="\d+" />} end end diff --git a/spec/support/shared_contexts/requests/api/npm_packages_metadata_shared_examples.rb b/spec/support/shared_contexts/requests/api/npm_packages_metadata_shared_examples.rb new file mode 100644 index 00000000000..7faf7cb32ba --- /dev/null +++ b/spec/support/shared_contexts/requests/api/npm_packages_metadata_shared_examples.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +RSpec.shared_examples 'generates metadata response "on-the-fly"' do + let(:metadata) do + { + 'dist-tags' => { + 'latest' => package.version + }, + 'name' => package.name, + 'versions' => { + package.version => { + 'dist' => { + 'shasum' => 'be93151dc23ac34a82752444556fe79b32c7a1ad', + 'tarball' => "http://localhost/api/v4/projects/#{project.id}/packages/npm/#{package.name}/-/foo-1.0.1.tgz" + }, + 'name' => package.name, + 'version' => package.version + } + } + } + end + + before do + Grape::Endpoint.before_each do |endpoint| + expect(endpoint).not_to receive(:present_carrierwave_file!) # rubocop:disable RSpec/ExpectInHook + end + end + + after do + Grape::Endpoint.before_each nil + end + + it 'generates metadata response "on-the-fly"', :aggregate_failures do + expect(Packages::Npm::GenerateMetadataService).to receive(:new).and_call_original + + subject + + expect(json_response).to eq(metadata) + end +end diff --git a/spec/support/shared_examples/features/rss_shared_examples.rb b/spec/support/shared_examples/features/rss_shared_examples.rb index f6566214e32..a6b9c98923a 100644 --- a/spec/support/shared_examples/features/rss_shared_examples.rb +++ b/spec/support/shared_examples/features/rss_shared_examples.rb @@ -2,20 +2,20 @@ RSpec.shared_examples "an autodiscoverable RSS feed with current_user's feed token" do it "has an RSS autodiscovery link tag with current_user's feed token" do - expect(page).to have_css("link[type*='atom+xml'][href*='feed_token=#{user.feed_token}']", visible: false) + expect(page).to have_css("link[type*='atom+xml'][href*='feed_token=glft-'][href*='-#{user.id}']", visible: false) end end RSpec.shared_examples "it has an RSS button with current_user's feed token" do it "shows the RSS button with current_user's feed token" do expect(page) - .to have_css("a:has([data-testid='rss-icon'])[href*='feed_token=#{user.feed_token}']") + .to have_css("a:has([data-testid='rss-icon'])[href*='feed_token=glft-'][href*='-#{user.id}']") end end RSpec.shared_examples "it has an RSS link with current_user's feed token" do it "shows the RSS link with current_user's feed token" do - expect(page).to have_link 'Subscribe to RSS feed', href: /feed_token=#{user.feed_token}/ + expect(page).to have_link 'Subscribe to RSS feed', href: /feed_token=glft-.*-#{user.id}/ end end @@ -51,11 +51,17 @@ RSpec.shared_examples "updates atom feed link" do |type| auto_discovery_params = CGI.parse(URI.parse(auto_discovery_link[:href]).query) expected = { - 'feed_token' => [user.feed_token], 'assignee_id' => [user.id.to_s] } expect(params).to include(expected) + feed_token_param = params['feed_token'] + expect(feed_token_param).to match([Gitlab::Auth::AuthFinders::PATH_DEPENDENT_FEED_TOKEN_REGEX]) + expect(feed_token_param.first).to end_with(user.id.to_s) + expect(auto_discovery_params).to include(expected) + feed_token_param = auto_discovery_params['feed_token'] + expect(feed_token_param).to match([Gitlab::Auth::AuthFinders::PATH_DEPENDENT_FEED_TOKEN_REGEX]) + expect(feed_token_param.first).to end_with(user.id.to_s) end end diff --git a/spec/views/layouts/_head.html.haml_spec.rb b/spec/views/layouts/_head.html.haml_spec.rb index 9aec8f2d759..504a9492d7a 100644 --- a/spec/views/layouts/_head.html.haml_spec.rb +++ b/spec/views/layouts/_head.html.haml_spec.rb @@ -59,7 +59,7 @@ RSpec.describe 'layouts/_head' do render - expect(rendered).to match('<link rel="stylesheet" media="all" href="/stylesheets/highlight/themes/solarised-light.css" />') + expect(rendered).to match('<link rel="stylesheet" href="/stylesheets/highlight/themes/solarised-light.css" media="all" />') end context 'when an asset_host is set and snowplow url is set', quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/346542' do |