diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2023-06-15 18:09:53 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2023-06-15 18:09:53 +0300 |
commit | 977720d7565377672df302ecb82b1e7a221cfba6 (patch) | |
tree | f258b65ed376a3075e0a76971a9360083ee6a059 /spec | |
parent | 717436a767395d0ed850a16d07f19cd51c3d4551 (diff) |
Add latest changes from gitlab-org/gitlab@master
Diffstat (limited to 'spec')
17 files changed, 463 insertions, 122 deletions
diff --git a/spec/controllers/search_controller_spec.rb b/spec/controllers/search_controller_spec.rb index 6c48962210d..7fce39950e5 100644 --- a/spec/controllers/search_controller_spec.rb +++ b/spec/controllers/search_controller_spec.rb @@ -58,19 +58,6 @@ RSpec.describe SearchController, feature_category: :global_search do expect(::Gitlab::ApplicationRateLimiter).to receive(:throttled?).with(:search_rate_limit, scope: [user]) get :show, params: { search: 'hello', scope: 'blobs' * 1000 } end - - context 'when search_rate_limited_scopes feature flag is disabled' do - before do - stub_feature_flags(search_rate_limited_scopes: false) - end - - it 'uses just current_user' do - %w[projects blobs users issues merge_requests].each do |scope| - expect(::Gitlab::ApplicationRateLimiter).to receive(:throttled?).with(:search_rate_limit, scope: [user]) - get :show, params: { search: 'hello', scope: scope } - end - end - end end context 'uses the right partials depending on scope' do @@ -395,19 +382,6 @@ RSpec.describe SearchController, feature_category: :global_search do expect(::Gitlab::ApplicationRateLimiter).to receive(:throttled?).with(:search_rate_limit, scope: [user]) get :count, params: { search: 'hello', scope: 'blobs' * 1000 } end - - context 'when search_rate_limited_scopes feature flag is disabled' do - before do - stub_feature_flags(search_rate_limited_scopes: false) - end - - it 'uses just current_user' do - %w[projects blobs users issues merge_requests].each do |scope| - expect(::Gitlab::ApplicationRateLimiter).to receive(:throttled?).with(:search_rate_limit, scope: [user]) - get :count, params: { search: 'hello', scope: scope } - end - end - end end it 'raises an error if search term is missing' do @@ -486,19 +460,6 @@ RSpec.describe SearchController, feature_category: :global_search do expect(::Gitlab::ApplicationRateLimiter).to receive(:throttled?).with(:search_rate_limit, scope: [user]) get :autocomplete, params: { term: 'hello', scope: 'blobs' * 1000 } end - - context 'when search_rate_limited_scopes feature flag is disabled' do - before do - stub_feature_flags(search_rate_limited_scopes: false) - end - - it 'uses just current_user' do - %w[projects blobs users issues merge_requests].each do |scope| - expect(::Gitlab::ApplicationRateLimiter).to receive(:throttled?).with(:search_rate_limit, scope: [user]) - get :autocomplete, params: { term: 'hello', scope: scope } - end - end - end end it_behaves_like 'rate limited endpoint', rate_limit_key: :search_rate_limit do diff --git a/spec/frontend/fixtures/project.rb b/spec/frontend/fixtures/project.rb new file mode 100644 index 00000000000..6100248d0a5 --- /dev/null +++ b/spec/frontend/fixtures/project.rb @@ -0,0 +1,51 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Project (GraphQL fixtures)', feature_category: :groups_and_projects do + describe GraphQL::Query, type: :request do + include ApiHelpers + include GraphqlHelpers + include JavaScriptFixturesHelpers + include ProjectForksHelper + + let_it_be(:project) { create(:project, :repository) } + let_it_be(:current_user) { create(:user) } + + describe 'writable forks' do + writeable_forks_query_path = 'vue_shared/components/web_ide/get_writable_forks.query.graphql' + + let(:query) { get_graphql_query_as_string(writeable_forks_query_path) } + + subject { post_graphql(query, current_user: current_user, variables: { projectPath: project.full_path }) } + + before do + project.add_developer(current_user) + end + + context 'with none' do + it "graphql/#{writeable_forks_query_path}_none.json" do + subject + + expect_graphql_errors_to_be_empty + end + end + + context 'with some' do + let_it_be(:fork1) { fork_project(project, nil, repository: true) } + let_it_be(:fork2) { fork_project(project, nil, repository: true) } + + before_all do + fork1.add_developer(current_user) + fork2.add_developer(current_user) + end + + it "graphql/#{writeable_forks_query_path}_some.json" do + subject + + expect_graphql_errors_to_be_empty + end + end + end + end +end diff --git a/spec/frontend/vue_merge_request_widget/components/widget/app_spec.js b/spec/frontend/vue_merge_request_widget/components/widget/app_spec.js index 8dbee9b370c..bf318cd6b88 100644 --- a/spec/frontend/vue_merge_request_widget/components/widget/app_spec.js +++ b/spec/frontend/vue_merge_request_widget/components/widget/app_spec.js @@ -12,8 +12,8 @@ describe('MR Widget App', () => { }); }; - it('does not mount if widgets array is empty', () => { + it('renders widget container', () => { createComponent(); - expect(wrapper.findByTestId('mr-widget-app').exists()).toBe(false); + expect(wrapper.findByTestId('mr-widget-app').exists()).toBe(true); }); }); diff --git a/spec/frontend/vue_merge_request_widget/mr_widget_options_spec.js b/spec/frontend/vue_merge_request_widget/mr_widget_options_spec.js index daa45a9e876..4161d51526f 100644 --- a/spec/frontend/vue_merge_request_widget/mr_widget_options_spec.js +++ b/spec/frontend/vue_merge_request_widget/mr_widget_options_spec.js @@ -10,7 +10,6 @@ import getStateQueryResponse from 'test_fixtures/graphql/merge_requests/get_stat import readyToMergeResponse from 'test_fixtures/graphql/merge_requests/states/ready_to_merge.query.graphql.json'; import createMockApollo from 'helpers/mock_apollo_helper'; import waitForPromises from 'helpers/wait_for_promises'; -import { securityReportMergeRequestDownloadPathsQueryResponse } from 'jest/vue_shared/security_reports/mock_data'; import api from '~/api'; import axios from '~/lib/utils/axios_utils'; import { HTTP_STATUS_OK, HTTP_STATUS_NO_CONTENT } from '~/lib/utils/http_status'; @@ -30,7 +29,6 @@ import Approvals from '~/vue_merge_request_widget/components/approvals/approvals import Preparing from '~/vue_merge_request_widget/components/states/mr_widget_preparing.vue'; import WidgetContainer from '~/vue_merge_request_widget/components/widget/app.vue'; import StatusIcon from '~/vue_merge_request_widget/components/extensions/status_icon.vue'; -import securityReportMergeRequestDownloadPathsQuery from '~/vue_shared/security_reports/graphql/queries/security_report_merge_request_download_paths.query.graphql'; import getStateQuery from '~/vue_merge_request_widget/queries/get_state.query.graphql'; import getStateSubscription from '~/vue_merge_request_widget/queries/get_state.subscription.graphql'; import readyToMergeQuery from 'ee_else_ce/vue_merge_request_widget/queries/states/ready_to_merge.query.graphql'; @@ -76,11 +74,6 @@ describe('MrWidgetOptions', () => { const COLLABORATION_MESSAGE = 'Members who can merge are allowed to add commits'; const findApprovalsWidget = () => wrapper.findComponent(Approvals); const findPreparingWidget = () => wrapper.findComponent(Preparing); - const findWidgetContainer = () => wrapper.findComponent(WidgetContainer); - const findExtensionToggleButton = () => - wrapper.find('[data-testid="widget-extension"] [data-testid="toggle-button"]'); - const findExtensionLink = (linkHref) => - wrapper.find(`[data-testid="widget-extension"] [href="${linkHref}"]`); beforeEach(() => { gl.mrWidgetData = { ...mockData }; @@ -163,9 +156,13 @@ describe('MrWidgetOptions', () => { return axios.waitForAll(); }; + const findExtensionToggleButton = () => + wrapper.find('[data-testid="widget-extension"] [data-testid="toggle-button"]'); + const findExtensionLink = (linkHref) => + wrapper.find(`[data-testid="widget-extension"] [href="${linkHref}"]`); const findSuggestPipeline = () => wrapper.find('[data-testid="mr-suggest-pipeline"]'); const findSuggestPipelineButton = () => findSuggestPipeline().find('button'); - const findSecurityMrWidget = () => wrapper.find('[data-testid="security-mr-widget"]'); + const findWidgetContainer = () => wrapper.findComponent(WidgetContainer); describe('default', () => { beforeEach(() => { @@ -870,47 +867,6 @@ describe('MrWidgetOptions', () => { }); }); - describe('security widget', () => { - const setup = (hasPipeline) => { - const mrData = { - ...mockData, - ...(hasPipeline ? {} : { pipeline: null }), - }; - - // Override top-level mocked requests, which always use a fresh copy of - // mockData, which always includes the full pipeline object. - mock.onGet(mockData.merge_request_widget_path).reply(() => [HTTP_STATUS_OK, mrData]); - mock.onGet(mockData.merge_request_cached_widget_path).reply(() => [HTTP_STATUS_OK, mrData]); - - return createComponent(mrData, { - apolloMock: [ - [ - securityReportMergeRequestDownloadPathsQuery, - jest - .fn() - .mockResolvedValue({ data: securityReportMergeRequestDownloadPathsQueryResponse }), - ], - ], - }); - }; - - describe('with a pipeline', () => { - it('renders the security widget', async () => { - await setup(true); - - expect(findSecurityMrWidget().exists()).toBe(true); - }); - }); - - describe('with no pipeline', () => { - it('does not render the security widget', async () => { - await setup(false); - - expect(findSecurityMrWidget().exists()).toBe(false); - }); - }); - }); - describe('suggestPipeline', () => { beforeEach(() => { mock.onAny().reply(HTTP_STATUS_OK); @@ -1179,7 +1135,7 @@ describe('MrWidgetOptions', () => { await nextTick(); await waitForPromises(); - expect(Sentry.captureException).toHaveBeenCalledTimes(2); + expect(Sentry.captureException).toHaveBeenCalledTimes(1); expect(Sentry.captureException).toHaveBeenCalledWith(new Error('Fetch error')); expect(wrapper.findComponent(StatusIcon).props('iconName')).toBe('failed'); }); @@ -1271,18 +1227,12 @@ describe('MrWidgetOptions', () => { expect(api.trackRedisCounterEvent).not.toHaveBeenCalled(); }); }); + }); - describe('widget container', () => { - it('should not be displayed when the refactor_security_extension feature flag is turned off', () => { - createComponent(); - expect(findWidgetContainer().exists()).toBe(false); - }); - - it('should be displayed when the refactor_security_extension feature flag is turned on', () => { - window.gon.features.refactorSecurityExtension = true; - createComponent(); - expect(findWidgetContainer().exists()).toBe(true); - }); + describe('widget container', () => { + it('renders the widget container when there is MR data', async () => { + await createComponent(mockData); + expect(findWidgetContainer().props('mr')).not.toBeUndefined(); }); }); diff --git a/spec/frontend/vue_shared/components/confirm_fork_modal_spec.js b/spec/frontend/vue_shared/components/confirm_fork_modal_spec.js index fbfef5cbe46..97c48a4db74 100644 --- a/spec/frontend/vue_shared/components/confirm_fork_modal_spec.js +++ b/spec/frontend/vue_shared/components/confirm_fork_modal_spec.js @@ -1,8 +1,17 @@ -import { GlModal } from '@gitlab/ui'; +import { GlLoadingIcon, GlModal } from '@gitlab/ui'; +import Vue from 'vue'; +import VueApollo from 'vue-apollo'; +import getNoWritableForksResponse from 'test_fixtures/graphql/vue_shared/components/web_ide/get_writable_forks.query.graphql_none.json'; +import getSomeWritableForksResponse from 'test_fixtures/graphql/vue_shared/components/web_ide/get_writable_forks.query.graphql_some.json'; import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; -import ConfirmForkModal, { i18n } from '~/vue_shared/components/confirm_fork_modal.vue'; +import ConfirmForkModal, { i18n } from '~/vue_shared/components/web_ide/confirm_fork_modal.vue'; +import createMockApollo from 'helpers/mock_apollo_helper'; +import getWritableForksQuery from '~/vue_shared/components/web_ide/get_writable_forks.query.graphql'; +import waitForPromises from 'helpers/wait_for_promises'; describe('vue_shared/components/confirm_fork_modal', () => { + Vue.use(VueApollo); + let wrapper = null; const forkPath = '/fake/fork/path'; @@ -13,13 +22,18 @@ describe('vue_shared/components/confirm_fork_modal', () => { const findModalProp = (prop) => findModal().props(prop); const findModalActionProps = () => findModalProp('actionPrimary'); - const createComponent = (props = {}) => - shallowMountExtended(ConfirmForkModal, { + const createComponent = (props = {}, getWritableForksResponse = getNoWritableForksResponse) => { + const fakeApollo = createMockApollo([ + [getWritableForksQuery, jest.fn().mockResolvedValue(getWritableForksResponse)], + ]); + return shallowMountExtended(ConfirmForkModal, { propsData: { ...defaultProps, ...props, }, + apolloProvider: fakeApollo, }); + }; describe('visible = false', () => { beforeEach(() => { @@ -73,4 +87,45 @@ describe('vue_shared/components/confirm_fork_modal', () => { expect(wrapper.emitted('change')).toEqual([[false]]); }); }); + + describe('writable forks', () => { + describe('when loading', () => { + it('shows loading spinner', () => { + wrapper = createComponent(); + + expect(wrapper.findComponent(GlLoadingIcon).exists()).toBe(true); + }); + }); + + describe('with no writable forks', () => { + it('contains `newForkMessage`', async () => { + wrapper = createComponent(); + + await waitForPromises(); + + expect(wrapper.text()).toContain(i18n.newForkMessage); + }); + }); + + describe('with writable forks', () => { + it('contains `existingForksMessage`', async () => { + wrapper = createComponent(null, getSomeWritableForksResponse); + + await waitForPromises(); + + expect(wrapper.text()).toContain(i18n.existingForksMessage); + }); + + it('renders links to the forks', async () => { + wrapper = createComponent(null, getSomeWritableForksResponse); + + await waitForPromises(); + + const forks = getSomeWritableForksResponse.data.project.visibleForks.nodes; + + expect(wrapper.findByText(forks[0].fullPath).attributes('href')).toBe(forks[0].webUrl); + expect(wrapper.findByText(forks[1].fullPath).attributes('href')).toBe(forks[1].webUrl); + }); + }); + }); }); diff --git a/spec/frontend/vue_shared/components/web_ide_link_spec.js b/spec/frontend/vue_shared/components/web_ide_link_spec.js index 26557c63a77..e54de25dc0d 100644 --- a/spec/frontend/vue_shared/components/web_ide_link_spec.js +++ b/spec/frontend/vue_shared/components/web_ide_link_spec.js @@ -1,14 +1,18 @@ import { GlModal } from '@gitlab/ui'; -import { nextTick } from 'vue'; +import Vue, { nextTick } from 'vue'; +import VueApollo from 'vue-apollo'; +import getWritableForksResponse from 'test_fixtures/graphql/vue_shared/components/web_ide/get_writable_forks.query.graphql_none.json'; import ActionsButton from '~/vue_shared/components/actions_button.vue'; import WebIdeLink, { i18n } from '~/vue_shared/components/web_ide_link.vue'; -import ConfirmForkModal from '~/vue_shared/components/confirm_fork_modal.vue'; +import ConfirmForkModal from '~/vue_shared/components/web_ide/confirm_fork_modal.vue'; import { stubComponent } from 'helpers/stub_component'; import { shallowMountExtended, mountExtended } from 'helpers/vue_test_utils_helper'; +import createMockApollo from 'helpers/mock_apollo_helper'; import { visitUrl } from '~/lib/utils/url_utility'; +import getWritableForksQuery from '~/vue_shared/components/web_ide/get_writable_forks.query.graphql'; jest.mock('~/lib/utils/url_utility'); @@ -77,9 +81,14 @@ const ACTION_PIPELINE_EDITOR = { }; describe('vue_shared/components/web_ide_link', () => { + Vue.use(VueApollo); + let wrapper; function createComponent(props, { mountFn = shallowMountExtended, glFeatures = {} } = {}) { + const fakeApollo = createMockApollo([ + [getWritableForksQuery, jest.fn().mockResolvedValue(getWritableForksResponse)], + ]); wrapper = mountFn(WebIdeLink, { propsData: { editUrl: TEST_EDIT_URL, @@ -102,6 +111,7 @@ describe('vue_shared/components/web_ide_link', () => { </div>`, }), }, + apolloProvider: fakeApollo, }); } diff --git a/spec/graphql/types/user_type_spec.rb b/spec/graphql/types/user_type_spec.rb index 0b0dcf2fb6a..777972df88b 100644 --- a/spec/graphql/types/user_type_spec.rb +++ b/spec/graphql/types/user_type_spec.rb @@ -47,7 +47,14 @@ RSpec.describe GitlabSchema.types['User'], feature_category: :user_profile do profileEnableGitpodPath savedReplies savedReply - user_achievements + userAchievements + bio + linkedin + twitter + discord + organization + jobTitle + createdAt ] expect(described_class).to include_graphql_fields(*expected_fields) diff --git a/spec/helpers/tree_helper_spec.rb b/spec/helpers/tree_helper_spec.rb index 01dacf5fcad..e13b83feefd 100644 --- a/spec/helpers/tree_helper_spec.rb +++ b/spec/helpers/tree_helper_spec.rb @@ -271,4 +271,42 @@ RSpec.describe TreeHelper do end end end + + describe '.fork_modal_options' do + let_it_be(:blob) { project.repository.blob_at('refs/heads/master', @path) } + + before do + allow(helper).to receive(:current_user).and_return(user) + end + + subject { helper.fork_modal_options(project, blob) } + + it 'returns correct fork path' do + expect(subject).to match a_hash_including(fork_path: '/namespace1/project-1/-/forks/new', fork_modal_id: nil) + end + + context 'when show_edit_button true' do + before do + allow(helper).to receive(:show_edit_button?).and_return(true) + end + + it 'returns correct fork path and modal id' do + expect(subject).to match a_hash_including( + fork_path: '/namespace1/project-1/-/forks/new', + fork_modal_id: 'modal-confirm-fork-edit') + end + end + + context 'when show_web_ide_button true' do + before do + allow(helper).to receive(:show_web_ide_button?).and_return(true) + end + + it 'returns correct fork path and modal id' do + expect(subject).to match a_hash_including( + fork_path: '/namespace1/project-1/-/forks/new', + fork_modal_id: 'modal-confirm-fork-webide') + end + end + end end diff --git a/spec/lib/gitlab/data_builder/pipeline_spec.rb b/spec/lib/gitlab/data_builder/pipeline_spec.rb index a379af7483b..351872ffbc5 100644 --- a/spec/lib/gitlab/data_builder/pipeline_spec.rb +++ b/spec/lib/gitlab/data_builder/pipeline_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Gitlab::DataBuilder::Pipeline do +RSpec.describe Gitlab::DataBuilder::Pipeline, feature_category: :continuous_integration do let_it_be(:user) { create(:user, :public_email) } let_it_be(:project) { create(:project, :repository) } @@ -26,6 +26,7 @@ RSpec.describe Gitlab::DataBuilder::Pipeline do it 'has correct attributes', :aggregate_failures do expect(attributes).to be_a(Hash) + expect(attributes[:name]).to be_nil expect(attributes[:ref]).to eq(pipeline.ref) expect(attributes[:sha]).to eq(pipeline.sha) expect(attributes[:tag]).to eq(pipeline.tag) @@ -54,6 +55,16 @@ RSpec.describe Gitlab::DataBuilder::Pipeline do expect(data[:source_pipeline]).to be_nil end + context 'pipeline with metadata' do + let_it_be_with_reload(:pipeline_metadata) do + create(:ci_pipeline_metadata, pipeline: pipeline, name: "My Pipeline") + end + + it 'has pipeline name', :aggregate_failures do + expect(attributes[:name]).to eq("My Pipeline") + end + end + context 'build with runner' do let_it_be(:tag_names) { %w(tag-1 tag-2) } let_it_be(:ci_runner) { create(:ci_runner, tag_list: tag_names.map { |n| ActsAsTaggableOn::Tag.create!(name: n) }) } diff --git a/spec/lib/gitlab/database/gitlab_schema_spec.rb b/spec/lib/gitlab/database/gitlab_schema_spec.rb index 3cb2d91e227..48f5cdb995b 100644 --- a/spec/lib/gitlab/database/gitlab_schema_spec.rb +++ b/spec/lib/gitlab/database/gitlab_schema_spec.rb @@ -140,7 +140,7 @@ RSpec.describe Gitlab::Database::GitlabSchema, feature_category: :database do end describe '.table_schemas!' do - let(:tables) { %w[namespaces projects ci_builds] } + let(:tables) { %w[projects issues ci_builds] } subject { described_class.table_schemas!(tables) } @@ -199,4 +199,82 @@ RSpec.describe Gitlab::Database::GitlabSchema, feature_category: :database do end end end + + context 'when testing cross schema access' do + using RSpec::Parameterized::TableSyntax + + before do + allow(Gitlab::Database).to receive(:all_gitlab_schemas).and_return( + [ + Gitlab::Database::GitlabSchemaInfo.new( + name: "gitlab_main_clusterwide", + allow_cross_joins: %i[gitlab_shared gitlab_main], + allow_cross_transactions: %i[gitlab_internal gitlab_shared gitlab_main], + allow_cross_foreign_keys: %i[gitlab_main] + ), + Gitlab::Database::GitlabSchemaInfo.new( + name: "gitlab_main", + allow_cross_joins: %i[gitlab_shared], + allow_cross_transactions: %i[gitlab_internal gitlab_shared], + allow_cross_foreign_keys: %i[] + ), + Gitlab::Database::GitlabSchemaInfo.new( + name: "gitlab_ci", + allow_cross_joins: %i[gitlab_shared], + allow_cross_transactions: %i[gitlab_internal gitlab_shared], + allow_cross_foreign_keys: %i[] + ) + ].index_by(&:name) + ) + end + + describe '.cross_joins_allowed?' do + where(:schemas, :result) do + %i[] | true + %i[gitlab_main_clusterwide gitlab_main] | true + %i[gitlab_main_clusterwide gitlab_ci] | false + %i[gitlab_main_clusterwide gitlab_main gitlab_ci] | false + %i[gitlab_main_clusterwide gitlab_internal] | false + %i[gitlab_main gitlab_ci] | false + %i[gitlab_main_clusterwide gitlab_main gitlab_shared] | true + %i[gitlab_main_clusterwide gitlab_shared] | true + end + + with_them do + it { expect(described_class.cross_joins_allowed?(schemas)).to eq(result) } + end + end + + describe '.cross_transactions_allowed?' do + where(:schemas, :result) do + %i[] | true + %i[gitlab_main_clusterwide gitlab_main] | true + %i[gitlab_main_clusterwide gitlab_ci] | false + %i[gitlab_main_clusterwide gitlab_main gitlab_ci] | false + %i[gitlab_main_clusterwide gitlab_internal] | true + %i[gitlab_main gitlab_ci] | false + %i[gitlab_main_clusterwide gitlab_main gitlab_shared] | true + %i[gitlab_main_clusterwide gitlab_shared] | true + end + + with_them do + it { expect(described_class.cross_transactions_allowed?(schemas)).to eq(result) } + end + end + + describe '.cross_foreign_key_allowed?' do + where(:schemas, :result) do + %i[] | false + %i[gitlab_main_clusterwide gitlab_main] | true + %i[gitlab_main_clusterwide gitlab_ci] | false + %i[gitlab_main_clusterwide gitlab_internal] | false + %i[gitlab_main gitlab_ci] | false + %i[gitlab_main_clusterwide gitlab_shared] | false + end + + with_them do + it { expect(described_class.cross_foreign_key_allowed?(schemas)).to eq(result) } + end + end + end end diff --git a/spec/lib/gitlab/database/no_cross_db_foreign_keys_spec.rb b/spec/lib/gitlab/database/no_cross_db_foreign_keys_spec.rb index e48937037fa..7899c1588b2 100644 --- a/spec/lib/gitlab/database/no_cross_db_foreign_keys_spec.rb +++ b/spec/lib/gitlab/database/no_cross_db_foreign_keys_spec.rb @@ -16,7 +16,9 @@ RSpec.describe 'cross-database foreign keys' do end def is_cross_db?(fk_record) - Gitlab::Database::GitlabSchema.table_schemas!([fk_record.from_table, fk_record.to_table]).many? + table_schemas = Gitlab::Database::GitlabSchema.table_schemas!([fk_record.from_table, fk_record.to_table]) + + !Gitlab::Database::GitlabSchema.cross_foreign_key_allowed?(table_schemas) end it 'onlies have allowed list of cross-database foreign keys', :aggregate_failures do diff --git a/spec/lib/gitlab/database/query_analyzers/restrict_allowed_schemas_spec.rb b/spec/lib/gitlab/database/query_analyzers/restrict_allowed_schemas_spec.rb index 261bef58bb6..b90f60e0301 100644 --- a/spec/lib/gitlab/database/query_analyzers/restrict_allowed_schemas_spec.rb +++ b/spec/lib/gitlab/database/query_analyzers/restrict_allowed_schemas_spec.rb @@ -2,7 +2,8 @@ require 'spec_helper' -RSpec.describe Gitlab::Database::QueryAnalyzers::RestrictAllowedSchemas, query_analyzers: false do +RSpec.describe Gitlab::Database::QueryAnalyzers::RestrictAllowedSchemas, + query_analyzers: false, feature_category: :database do let(:analyzer) { described_class } context 'properly analyzes queries' do @@ -15,14 +16,38 @@ RSpec.describe Gitlab::Database::QueryAnalyzers::RestrictAllowedSchemas, query_a expected_allowed_gitlab_schemas: { no_schema: :dml_not_allowed, gitlab_main: :success, + gitlab_main_clusterwide: :success, + gitlab_main_cell: :success, gitlab_ci: :dml_access_denied # cross-schema access } }, - "for INSERT" => { + "for SELECT on namespaces" => { + sql: "SELECT 1 FROM namespaces", + expected_allowed_gitlab_schemas: { + no_schema: :dml_not_allowed, + gitlab_main: :success, + gitlab_main_clusterwide: :success, + gitlab_main_cell: :success, + gitlab_ci: :dml_access_denied # cross-schema access + } + }, + "for INSERT on projects" => { sql: "INSERT INTO projects VALUES (1)", expected_allowed_gitlab_schemas: { no_schema: :dml_not_allowed, gitlab_main: :success, + gitlab_main_clusterwide: :success, + gitlab_main_cell: :success, + gitlab_ci: :dml_access_denied # cross-schema access + } + }, + "for INSERT on namespaces" => { + sql: "INSERT INTO namespaces VALUES (1)", + expected_allowed_gitlab_schemas: { + no_schema: :dml_not_allowed, + gitlab_main: :success, + gitlab_main_clusterwide: :success, + gitlab_main_cell: :success, gitlab_ci: :dml_access_denied # cross-schema access } }, diff --git a/spec/lib/gitlab/path_traversal_spec.rb b/spec/lib/gitlab/path_traversal_spec.rb index 8ae1330c203..bba6f8293c2 100644 --- a/spec/lib/gitlab/path_traversal_spec.rb +++ b/spec/lib/gitlab/path_traversal_spec.rb @@ -44,6 +44,18 @@ RSpec.describe Gitlab::PathTraversal, feature_category: :shared do expect { check_path_traversal!('foo\\..') }.to raise_error(/Invalid path/) end + it 'detects path traversal in string with encoded chars' do + expect { check_path_traversal!('foo%2F..%2Fbar') }.to raise_error(/Invalid path/) + expect { check_path_traversal!('foo%2F%2E%2E%2Fbar') }.to raise_error(/Invalid path/) + end + + it 'detects double encoded chars' do + expect { check_path_traversal!('foo%252F..%2Fbar') } + .to raise_error(Gitlab::Utils::DoubleEncodingError, /is not allowed/) + expect { check_path_traversal!('foo%252F%2E%2E%2Fbar') } + .to raise_error(Gitlab::Utils::DoubleEncodingError, /is not allowed/) + end + it 'does nothing for a safe string' do expect(check_path_traversal!('./foo')).to eq('./foo') expect(check_path_traversal!('.test/foo')).to eq('.test/foo') diff --git a/spec/lib/gitlab/usage/service_ping_report_spec.rb b/spec/lib/gitlab/usage/service_ping_report_spec.rb index f1ce48468fe..a848c286fa9 100644 --- a/spec/lib/gitlab/usage/service_ping_report_spec.rb +++ b/spec/lib/gitlab/usage/service_ping_report_spec.rb @@ -120,9 +120,9 @@ RSpec.describe Gitlab::Usage::ServicePingReport, :use_clean_rails_memory_store_c # Because test cases are run inside a transaction, if any query raise and error all queries that follows # it are automatically canceled by PostgreSQL, to avoid that problem, and to provide exhaustive information # about every metric, queries are wrapped explicitly in sub transactions. - table = PgQuery.parse(query).tables.first - gitlab_schema = Gitlab::Database::GitlabSchema.tables_to_schema[table] - base_model = gitlab_schema == :gitlab_main ? ApplicationRecord : Ci::ApplicationRecord + table_name = PgQuery.parse(query).tables.first + gitlab_schema = Gitlab::Database::GitlabSchema.table_schema!(table_name) + base_model = Gitlab::Database.schemas_to_base_models.fetch(gitlab_schema).first base_model.transaction do base_model.connection.execute(query)&.first&.values&.first diff --git a/spec/models/plan_limits_spec.rb b/spec/models/plan_limits_spec.rb index 58abd153d28..d211499e9e9 100644 --- a/spec/models/plan_limits_spec.rb +++ b/spec/models/plan_limits_spec.rb @@ -15,6 +15,46 @@ RSpec.describe PlanLimits do describe 'validations' do it { is_expected.to validate_numericality_of(:notification_limit).only_integer.is_greater_than_or_equal_to(0) } it { is_expected.to validate_numericality_of(:enforcement_limit).only_integer.is_greater_than_or_equal_to(0) } + + describe 'limits_history' do + context 'when does not match the JSON schema' do + it 'does not allow invalid json' do + expect(subject).not_to allow_value({ + invalid_key: { + enforcement_limit: [ + { + username: 'mhamda', + timestamp: 1686140606000, + value: 5000 + } + ], + another_invalid: [ + { + username: 'mhamda', + timestamp: 1686140606000, + value: 5000 + } + ] + } + }).for(:limits_history) + end + end + + context 'when matches the JSON schema' do + it 'allows valid json' do + expect(subject).to allow_value({ + enforcement_limit: [ + { + user_id: 1, + username: 'mhamda', + timestamp: 1686140606000, + value: 5000 + } + ] + }).for(:limits_history) + end + end + end end describe '#exceeded?' do @@ -233,12 +273,17 @@ RSpec.describe PlanLimits do %w[dashboard_limit_enabled_at] end - it "has positive values for enabled limits" do + let(:history_columns) do + %w[limits_history] + end + + it 'has positive values for enabled limits' do attributes = plan_limits.attributes attributes = attributes.except(described_class.primary_key) attributes = attributes.except(described_class.reflections.values.map(&:foreign_key)) attributes = attributes.except(*columns_with_zero) attributes = attributes.except(*datetime_columns) + attributes = attributes.except(*history_columns) expect(attributes).to all(include(be_positive)) end @@ -256,4 +301,95 @@ RSpec.describe PlanLimits do expect(plan_limits.dashboard_storage_limit_enabled?).to be false end end + + describe '#log_limits_changes', :freeze_time do + let(:user) { create(:user) } + let(:plan_limits) { create(:plan_limits) } + let(:current_timestamp) { Time.current.utc.to_i } + let(:history) { plan_limits.limits_history } + + it 'logs a single attribute change' do + plan_limits.log_limits_changes(user, enforcement_limit: 5_000) + + expect(history).to eq( + { 'enforcement_limit' => [{ 'user_id' => user.id, 'username' => user.username, + 'timestamp' => current_timestamp, 'value' => 5_000 }] } + ) + end + + it 'logs multiple attribute changes' do + plan_limits.log_limits_changes(user, enforcement_limit: 10_000, notification_limit: 20_000) + + expect(history).to eq( + { 'enforcement_limit' => [{ 'user_id' => user.id, 'username' => user.username, + 'timestamp' => current_timestamp, 'value' => 10_000 }], + 'notification_limit' => [{ 'user_id' => user.id, 'username' => user.username, + 'timestamp' => current_timestamp, + 'value' => 20_000 }] } + ) + end + + it 'allows logging dashboard_limit_enabled_at from console (without user)' do + plan_limits.log_limits_changes(nil, dashboard_limit_enabled_at: current_timestamp) + + expect(history).to eq( + { 'dashboard_limit_enabled_at' => [{ 'user_id' => nil, 'username' => nil, 'timestamp' => current_timestamp, + 'value' => current_timestamp }] } + ) + end + + context 'with previous history avilable' do + let(:plan_limits) do + create(:plan_limits, + limits_history: { 'enforcement_limit' => [{ user_id: user.id, username: user.username, + timestamp: current_timestamp, + value: 20_000 }, + { user_id: user.id, username: user.username, timestamp: current_timestamp, + value: 50_000 }] }) + end + + it 'appends to it' do + plan_limits.log_limits_changes(user, enforcement_limit: 60_000) + expect(history).to eq( + { + 'enforcement_limit' => [ + { 'user_id' => user.id, 'username' => user.username, 'timestamp' => current_timestamp, + 'value' => 20_000 }, + { 'user_id' => user.id, 'username' => user.username, 'timestamp' => current_timestamp, + 'value' => 50_000 }, + { 'user_id' => user.id, 'username' => user.username, 'timestamp' => current_timestamp, 'value' => 60_000 } + ] + } + ) + end + end + end + + describe '#limit_attribute_changes', :freeze_time do + let(:user) { create(:user) } + let(:current_timestamp) { Time.current.utc.to_i } + let(:plan_limits) do + create(:plan_limits, + limits_history: { 'enforcement_limit' => [ + { user_id: user.id, username: user.username, timestamp: current_timestamp, + value: 20_000 }, { user_id: user.id, username: user.username, timestamp: current_timestamp, + value: 50_000 } + ] }) + end + + it 'returns an empty array for attribute with no changes' do + changes = plan_limits.limit_attribute_changes(:notification_limit) + + expect(changes).to eq([]) + end + + it 'returns the changes for a specific attribute' do + changes = plan_limits.limit_attribute_changes(:enforcement_limit) + + expect(changes).to eq( + [{ timestamp: current_timestamp, value: 20_000, username: user.username, user_id: user.id }, + { timestamp: current_timestamp, value: 50_000, username: user.username, user_id: user.id }] + ) + end + end end diff --git a/spec/support/database/prevent_cross_joins.rb b/spec/support/database/prevent_cross_joins.rb index c44bf96a268..540c287bdad 100644 --- a/spec/support/database/prevent_cross_joins.rb +++ b/spec/support/database/prevent_cross_joins.rb @@ -23,7 +23,6 @@ module Database ALLOW_THREAD_KEY = :allow_cross_joins_across_databases ALLOW_ANNOTATE_KEY = ALLOW_THREAD_KEY.to_s.freeze - IGNORED_SCHEMAS = %i[gitlab_shared gitlab_internal].freeze def self.validate_cross_joins!(sql) return if Thread.current[ALLOW_THREAD_KEY] || sql.include?(ALLOW_ANNOTATE_KEY) @@ -41,9 +40,8 @@ module Database end schemas = ::Gitlab::Database::GitlabSchema.table_schemas!(tables) - schemas.subtract(IGNORED_SCHEMAS) - if schemas.many? + unless ::Gitlab::Database::GitlabSchema.cross_joins_allowed?(schemas) Thread.current[:has_cross_join_exception] = true raise CrossJoinAcrossUnsupportedTablesError, "Unsupported cross-join across '#{tables.join(", ")}' querying '#{schemas.to_a.join(", ")}' discovered " \ diff --git a/spec/support/shared_examples/graphql/types/merge_request_interactions_type_shared_examples.rb b/spec/support/shared_examples/graphql/types/merge_request_interactions_type_shared_examples.rb index 3dffc2066ae..d8cc6f697d7 100644 --- a/spec/support/shared_examples/graphql/types/merge_request_interactions_type_shared_examples.rb +++ b/spec/support/shared_examples/graphql/types/merge_request_interactions_type_shared_examples.rb @@ -42,7 +42,14 @@ RSpec.shared_examples "a user type with merge request interaction type" do profileEnableGitpodPath savedReplies savedReply - user_achievements + userAchievements + bio + linkedin + twitter + discord + organization + jobTitle + createdAt ] # TODO: 'workspaces' needs to be included, but only when this spec is run in EE context, to account for the |