diff --git a/app/controllers/graphql_controller.rb b/app/controllers/graphql_controller.rb index 123102bf793..b004851587a 100644 --- a/app/controllers/graphql_controller.rb +++ b/app/controllers/graphql_controller.rb @@ -81,7 +81,7 @@ class GraphqlController < ApplicationController end def context - @context ||= { current_user: current_user, is_sessionless_user: !!sessionless_user? } + @context ||= { current_user: current_user, is_sessionless_user: !!sessionless_user?, request: request } end def build_variables(variable_info) diff --git a/app/graphql/mutations/concerns/mutations/spammable_mutation_fields.rb b/app/graphql/mutations/concerns/mutations/spammable_mutation_fields.rb new file mode 100644 index 00000000000..7aef55f8011 --- /dev/null +++ b/app/graphql/mutations/concerns/mutations/spammable_mutation_fields.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +module Mutations + module SpammableMutationFields + extend ActiveSupport::Concern + + included do + field :spam, + GraphQL::BOOLEAN_TYPE, + null: true, + description: 'Indicates whether the operation returns a record detected as spam' + end + + def with_spam_params(&block) + request = Feature.enabled?(:snippet_spam) ? context[:request] : nil + + yield.merge({ api: true, request: request }) + end + + def with_spam_fields(spammable, &block) + { spam: spammable.spam? }.merge!(yield) + end + end +end diff --git a/app/graphql/mutations/issues/update.rb b/app/graphql/mutations/issues/update.rb index cc03d32731b..5000d2a90fb 100644 --- a/app/graphql/mutations/issues/update.rb +++ b/app/graphql/mutations/issues/update.rb @@ -46,6 +46,10 @@ module Mutations required: false, description: 'The ID of the milestone to be assigned, milestone will be removed if set to null.' + argument :state_event, Types::IssueStateEventEnum, + description: 'Close or reopen an issue.', + required: false + def resolve(project_path:, iid:, **args) issue = authorized_find!(project_path: project_path, iid: iid) project = issue.project diff --git a/app/graphql/mutations/snippets/create.rb b/app/graphql/mutations/snippets/create.rb index a8aeb15afcd..37c0f80310c 100644 --- a/app/graphql/mutations/snippets/create.rb +++ b/app/graphql/mutations/snippets/create.rb @@ -3,6 +3,7 @@ module Mutations module Snippets class Create < BaseMutation + include SpammableMutationFields include ResolvesProject graphql_name 'CreateSnippet' @@ -56,10 +57,12 @@ module Mutations ::Gitlab::UsageDataCounters::EditorUniqueCounter.track_snippet_editor_edit_action(author: current_user) end - { - snippet: service_response.success? ? snippet : nil, - errors: errors_on_object(snippet) - } + with_spam_fields(snippet) do + { + snippet: service_response.success? ? snippet : nil, + errors: errors_on_object(snippet) + } + end end private @@ -81,14 +84,16 @@ module Mutations end def create_params(args) - args.tap do |create_args| - # We need to rename `blob_actions` into `snippet_actions` because - # it's the expected key param - create_args[:snippet_actions] = create_args.delete(:blob_actions)&.map(&:to_h) + with_spam_params do + args.tap do |create_args| + # We need to rename `blob_actions` into `snippet_actions` because + # it's the expected key param + create_args[:snippet_actions] = create_args.delete(:blob_actions)&.map(&:to_h) - # We need to rename `uploaded_files` into `files` because - # it's the expected key param - create_args[:files] = create_args.delete(:uploaded_files) + # We need to rename `uploaded_files` into `files` because + # it's the expected key param + create_args[:files] = create_args.delete(:uploaded_files) + end end end end diff --git a/app/graphql/mutations/snippets/update.rb b/app/graphql/mutations/snippets/update.rb index d0db5fa2eb9..74266880806 100644 --- a/app/graphql/mutations/snippets/update.rb +++ b/app/graphql/mutations/snippets/update.rb @@ -3,6 +3,8 @@ module Mutations module Snippets class Update < Base + include SpammableMutationFields + graphql_name 'UpdateSnippet' argument :id, @@ -39,10 +41,12 @@ module Mutations ::Gitlab::UsageDataCounters::EditorUniqueCounter.track_snippet_editor_edit_action(author: current_user) end - { - snippet: result.success? ? snippet : snippet.reset, - errors: errors_on_object(snippet) - } + with_spam_fields(snippet) do + { + snippet: result.success? ? snippet : snippet.reset, + errors: errors_on_object(snippet) + } + end end private @@ -52,10 +56,12 @@ module Mutations end def update_params(args) - args.tap do |update_args| - # We need to rename `blob_actions` into `snippet_actions` because - # it's the expected key param - update_args[:snippet_actions] = update_args.delete(:blob_actions)&.map(&:to_h) + with_spam_params do + args.tap do |update_args| + # We need to rename `blob_actions` into `snippet_actions` because + # it's the expected key param + update_args[:snippet_actions] = update_args.delete(:blob_actions)&.map(&:to_h) + end end end end diff --git a/app/graphql/types/issue_state_event_enum.rb b/app/graphql/types/issue_state_event_enum.rb new file mode 100644 index 00000000000..6a9d840831d --- /dev/null +++ b/app/graphql/types/issue_state_event_enum.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +module Types + class IssueStateEventEnum < BaseEnum + graphql_name 'IssueStateEvent' + description 'Values for issue state events' + + value 'REOPEN', 'Reopens the issue', value: 'reopen' + value 'CLOSE', 'Closes the issue', value: 'close' + end +end diff --git a/app/helpers/search_helper.rb b/app/helpers/search_helper.rb index 8d99d760613..e0dbd33b5d2 100644 --- a/app/helpers/search_helper.rb +++ b/app/helpers/search_helper.rb @@ -131,8 +131,7 @@ module SearchHelper { category: "Help", label: _("Rake Tasks Help"), url: help_page_path("raketasks/README") }, { category: "Help", label: _("SSH Keys Help"), url: help_page_path("ssh/README") }, { category: "Help", label: _("System Hooks Help"), url: help_page_path("system_hooks/system_hooks") }, - { category: "Help", label: _("Webhooks Help"), url: help_page_path("user/project/integrations/webhooks") }, - { category: "Help", label: _("Workflow Help"), url: help_page_path("workflow/README") } + { category: "Help", label: _("Webhooks Help"), url: help_page_path("user/project/integrations/webhooks") } ] end diff --git a/app/models/ci/build_trace_chunk.rb b/app/models/ci/build_trace_chunk.rb index 55bcb8adaca..f8b419727ae 100644 --- a/app/models/ci/build_trace_chunk.rb +++ b/app/models/ci/build_trace_chunk.rb @@ -126,12 +126,18 @@ module Ci Ci::BuildTraceChunkFlushWorker.perform_async(id) end - def persisted? - !redis? - end - - def live? - redis? + ## + # It is possible that we run into two concurrent migrations. It might + # happen that a chunk gets migrated after being loaded by another worker + # but before the worker acquires a lock to perform the migration. + # + # We want to reset a chunk in that case and retry migration. If it fails + # again, we want to re-raise the exception. + # + def flush! + persist_data! + rescue FailedToPersistDataError + self.reset.persist_data! end ## @@ -143,6 +149,14 @@ module Ci build.pending_state.present? && chunks_max_index == chunk_index end + def persisted? + !redis? + end + + def live? + redis? + end + def <=>(other) return unless self.build_id == other.build_id diff --git a/app/workers/ci/build_trace_chunk_flush_worker.rb b/app/workers/ci/build_trace_chunk_flush_worker.rb index 2908c7c2d0b..dfa1417f18e 100644 --- a/app/workers/ci/build_trace_chunk_flush_worker.rb +++ b/app/workers/ci/build_trace_chunk_flush_worker.rb @@ -9,9 +9,7 @@ module Ci # rubocop: disable CodeReuse/ActiveRecord def perform(chunk_id) - ::Ci::BuildTraceChunk.find_by(id: chunk_id).try do |chunk| - chunk.persist_data! - end + ::Ci::BuildTraceChunk.find_by(id: chunk_id).try(&:flush!) end # rubocop: enable CodeReuse/ActiveRecord end diff --git a/changelogs/unreleased/217722-fj-add-spam-mechanism-to-snippet-mutations.yml b/changelogs/unreleased/217722-fj-add-spam-mechanism-to-snippet-mutations.yml new file mode 100644 index 00000000000..70e1099e7d8 --- /dev/null +++ b/changelogs/unreleased/217722-fj-add-spam-mechanism-to-snippet-mutations.yml @@ -0,0 +1,5 @@ +--- +title: Add spam flag to snippet create/update mutations +merge_request: 44010 +author: +type: changed diff --git a/changelogs/unreleased/add_options_to_dast_scanner_profile_254626.yml b/changelogs/unreleased/add_options_to_dast_scanner_profile_254626.yml new file mode 100644 index 00000000000..207fe49525a --- /dev/null +++ b/changelogs/unreleased/add_options_to_dast_scanner_profile_254626.yml @@ -0,0 +1,6 @@ +--- +title: Add on-demand DAST scan options (scanType, showDebugMessages, useAjaxSpider) + ajax spider and set the scan type +merge_request: 43240 +author: +type: added diff --git a/changelogs/unreleased/issue_233479-allow-graphql-to-update-issue-state.yml b/changelogs/unreleased/issue_233479-allow-graphql-to-update-issue-state.yml new file mode 100644 index 00000000000..43bf32ce87f --- /dev/null +++ b/changelogs/unreleased/issue_233479-allow-graphql-to-update-issue-state.yml @@ -0,0 +1,5 @@ +--- +title: Allow to update issue state on GraphQL +merge_request: 44061 +author: +type: added diff --git a/changelogs/unreleased/vij-public-snippet-access.yml b/changelogs/unreleased/vij-public-snippet-access.yml new file mode 100644 index 00000000000..8a6bb324e75 --- /dev/null +++ b/changelogs/unreleased/vij-public-snippet-access.yml @@ -0,0 +1,5 @@ +--- +title: Allow unauthenticated users access to public Personal Snippets via the REST API +merge_request: 44135 +author: +type: fixed diff --git a/config/feature_flags/development/snippet_spam.yml b/config/feature_flags/development/snippet_spam.yml new file mode 100644 index 00000000000..8215766fd3f --- /dev/null +++ b/config/feature_flags/development/snippet_spam.yml @@ -0,0 +1,7 @@ +--- +name: snippet_spam +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/44010 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/262013 +type: development +group: group::editor +default_enabled: false diff --git a/db/migrate/20200123090839_remove_analytics_repository_table_fks_on_projects.rb b/db/migrate/20200123090839_remove_analytics_repository_table_fks_on_projects.rb index 94fafe214c2..18fd349df2a 100644 --- a/db/migrate/20200123090839_remove_analytics_repository_table_fks_on_projects.rb +++ b/db/migrate/20200123090839_remove_analytics_repository_table_fks_on_projects.rb @@ -5,22 +5,26 @@ class RemoveAnalyticsRepositoryTableFksOnProjects < ActiveRecord::Migration[5.2] DOWNTIME = false + disable_ddl_transaction! + def up + # Requires ExclusiveLock on all tables. analytics_* tables are empty with_lock_retries do - # Requires ExclusiveLock on all tables. analytics_* tables are empty - remove_foreign_key :analytics_repository_files, :projects - remove_foreign_key :analytics_repository_file_edits, :projects if table_exists?(:analytics_repository_file_edits) # this table might be already dropped on development environment - remove_foreign_key :analytics_repository_file_commits, :projects + remove_foreign_key_if_exists(:analytics_repository_files, :projects) + end + + with_lock_retries do + remove_foreign_key_if_exists(:analytics_repository_file_edits, :projects) if table_exists?(:analytics_repository_file_edits) # this table might be already dropped on development environment + end + + with_lock_retries do + remove_foreign_key_if_exists(:analytics_repository_file_commits, :projects) end end def down - with_lock_retries do - # rubocop:disable Migration/AddConcurrentForeignKey - add_foreign_key :analytics_repository_files, :projects, on_delete: :cascade - add_foreign_key :analytics_repository_file_edits, :projects, on_delete: :cascade - add_foreign_key :analytics_repository_file_commits, :projects, on_delete: :cascade - # rubocop:enable Migration/AddConcurrentForeignKey - end + add_concurrent_foreign_key(:analytics_repository_files, :projects, column: :project_id, on_delete: :cascade) + add_concurrent_foreign_key(:analytics_repository_file_edits, :projects, column: :project_id, on_delete: :cascade) + add_concurrent_foreign_key(:analytics_repository_file_commits, :projects, column: :project_id, on_delete: :cascade) end end diff --git a/db/migrate/20200123091422_remove_analytics_repository_files_fk_on_other_analytics_tables.rb b/db/migrate/20200123091422_remove_analytics_repository_files_fk_on_other_analytics_tables.rb index e3e415dd0ad..a436ad8d529 100644 --- a/db/migrate/20200123091422_remove_analytics_repository_files_fk_on_other_analytics_tables.rb +++ b/db/migrate/20200123091422_remove_analytics_repository_files_fk_on_other_analytics_tables.rb @@ -5,20 +5,21 @@ class RemoveAnalyticsRepositoryFilesFkOnOtherAnalyticsTables < ActiveRecord::Mig DOWNTIME = false + disable_ddl_transaction! + def up + # Requires ExclusiveLock on all tables. analytics_* tables are empty with_lock_retries do - # Requires ExclusiveLock on all tables. analytics_* tables are empty - remove_foreign_key :analytics_repository_file_edits, :analytics_repository_files if table_exists?(:analytics_repository_file_edits) # this table might be already dropped on development environment - remove_foreign_key :analytics_repository_file_commits, :analytics_repository_files + remove_foreign_key_if_exists(:analytics_repository_file_edits, :analytics_repository_files) if table_exists?(:analytics_repository_file_edits) # this table might be already dropped on development environment + end + + with_lock_retries do + remove_foreign_key_if_exists(:analytics_repository_file_commits, :analytics_repository_files) end end def down - with_lock_retries do - # rubocop:disable Migration/AddConcurrentForeignKey - add_foreign_key :analytics_repository_file_edits, :analytics_repository_files, on_delete: :cascade - add_foreign_key :analytics_repository_file_commits, :analytics_repository_files, on_delete: :cascade - # rubocop:enable Migration/AddConcurrentForeignKey - end + add_concurrent_foreign_key(:analytics_repository_file_edits, :analytics_repository_files, column: :analytics_repository_file_id, on_delete: :cascade) + add_concurrent_foreign_key(:analytics_repository_file_commits, :analytics_repository_files, column: :analytics_repository_file_id, on_delete: :cascade) end end diff --git a/db/migrate/20200924035825_add_options_to_dast_scanner_profile.rb b/db/migrate/20200924035825_add_options_to_dast_scanner_profile.rb new file mode 100644 index 00000000000..588ce8fada7 --- /dev/null +++ b/db/migrate/20200924035825_add_options_to_dast_scanner_profile.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +class AddOptionsToDastScannerProfile < ActiveRecord::Migration[6.0] + DOWNTIME = false + + PASSIVE_SCAN_ENUM_VALUE = 1 + + def change + add_column :dast_scanner_profiles, :scan_type, :integer, limit: 2, default: PASSIVE_SCAN_ENUM_VALUE, null: false + add_column :dast_scanner_profiles, :use_ajax_spider, :boolean, default: false, null: false + add_column :dast_scanner_profiles, :show_debug_messages, :boolean, default: false, null: false + end +end diff --git a/db/post_migrate/20201002175953_add_index_for_merged_merge_requests.rb b/db/post_migrate/20201002175953_add_index_for_merged_merge_requests.rb new file mode 100644 index 00000000000..cd663f3da89 --- /dev/null +++ b/db/post_migrate/20201002175953_add_index_for_merged_merge_requests.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +class AddIndexForMergedMergeRequests < ActiveRecord::Migration[6.0] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + INDEX_NAME = 'idx_merge_requests_on_merged_state' + + disable_ddl_transaction! + + def up + add_concurrent_index :merge_requests, + :id, + where: 'state_id = 3', + name: INDEX_NAME + end + + def down + remove_concurrent_index_by_name :merge_requests, INDEX_NAME + end +end diff --git a/db/schema_migrations/20200924035825 b/db/schema_migrations/20200924035825 new file mode 100644 index 00000000000..0c27ed47732 --- /dev/null +++ b/db/schema_migrations/20200924035825 @@ -0,0 +1 @@ +d413e19c8ddaba4556cf36a38e03b1c7c46ee7edf7e56692028e066f97605784 \ No newline at end of file diff --git a/db/schema_migrations/20201002175953 b/db/schema_migrations/20201002175953 new file mode 100644 index 00000000000..df5f64c0deb --- /dev/null +++ b/db/schema_migrations/20201002175953 @@ -0,0 +1 @@ +619e788f8868a2ff602a06e0025154b0b10b17cd69edcce70378e11750d5e686 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 0ab6b6f1849..fbd33ecc19c 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -11216,6 +11216,9 @@ CREATE TABLE dast_scanner_profiles ( spider_timeout smallint, target_timeout smallint, name text NOT NULL, + scan_type smallint DEFAULT 1 NOT NULL, + use_ajax_spider boolean DEFAULT false NOT NULL, + show_debug_messages boolean DEFAULT false NOT NULL, CONSTRAINT check_568568fabf CHECK ((char_length(name) <= 255)) ); @@ -19476,6 +19479,8 @@ CREATE INDEX idx_members_created_at_user_id_invite_token ON members USING btree CREATE INDEX idx_merge_requests_on_id_and_merge_jid ON merge_requests USING btree (id, merge_jid) WHERE ((merge_jid IS NOT NULL) AND (state_id = 4)); +CREATE INDEX idx_merge_requests_on_merged_state ON merge_requests USING btree (id) WHERE (state_id = 3); + CREATE INDEX idx_merge_requests_on_source_project_and_branch_state_opened ON merge_requests USING btree (source_project_id, source_branch) WHERE (state_id = 1); CREATE INDEX idx_merge_requests_on_state_id_and_merge_status ON merge_requests USING btree (state_id, merge_status) WHERE ((state_id = 1) AND ((merge_status)::text = 'can_be_merged'::text)); diff --git a/doc/README.md b/doc/README.md index 03ecbef56ea..47f6535e80e 100644 --- a/doc/README.md +++ b/doc/README.md @@ -218,7 +218,7 @@ The following documentation relates to the DevOps **Create** stage: | [GitLab GraphQL API](api/graphql/index.md) | Integrate with GitLab using our GraphQL API. | | [GitLab Integration](integration/README.md) | Integrate with multiple third-party services with GitLab to allow external issue trackers and external authentication. | | [GitLab Webhooks](user/project/integrations/webhooks.md) | Let GitLab notify you when new code has been pushed to your project. | -| [Jira Development Panel](integration/jira_development_panel.md) **(PREMIUM)** | See GitLab information in the Jira Development Panel. | +| [Jira Development Panel](integration/jira_development_panel.md) | See GitLab information in the Jira Development Panel. | | [Integrations](user/project/integrations/overview.md) | Integrate a project with external services, such as CI and chat. | | [Trello Power-Up](integration/trello_power_up.md) | Integrate with GitLab's Trello Power-Up. | diff --git a/doc/administration/operations/fast_ssh_key_lookup.md b/doc/administration/operations/fast_ssh_key_lookup.md index 6cd393be330..a521dd2c194 100644 --- a/doc/administration/operations/fast_ssh_key_lookup.md +++ b/doc/administration/operations/fast_ssh_key_lookup.md @@ -114,7 +114,6 @@ This is a brief overview. Please refer to the above instructions for more contex 1. Enable writes to the `authorized_keys` file in Application Settings 1. Remove the `AuthorizedKeysCommand` lines from `/etc/ssh/sshd_config` or from `/assets/sshd_config` if you are using Omnibus Docker. 1. Reload `sshd`: `sudo service sshd reload` -1. Remove the `/opt/gitlab-shell/authorized_keys` file ## Compiling a custom version of OpenSSH for CentOS 6 diff --git a/doc/api/graphql/reference/gitlab_schema.graphql b/doc/api/graphql/reference/gitlab_schema.graphql index 6c60b2d26c2..3622501a227 100644 --- a/doc/api/graphql/reference/gitlab_schema.graphql +++ b/doc/api/graphql/reference/gitlab_schema.graphql @@ -3504,6 +3504,11 @@ type CreateSnippetPayload { The snippet after mutation """ snippet: Snippet + + """ + Indicates whether the operation returns a record detected as spam + """ + spam: Boolean } """ @@ -3634,6 +3639,11 @@ type DastOnDemandScanCreatePayload { } enum DastScanTypeEnum { + """ + Active DAST scan. This scan will make active attacks against the target site. + """ + ACTIVE + """ Passive DAST scan. This scan will not make active attacks against the target site. """ @@ -3664,6 +3674,16 @@ type DastScannerProfile { """ profileName: String + """ + Indicates the type of DAST scan that will run. Either a Passive Scan or an Active Scan. + """ + scanType: DastScanTypeEnum + + """ + Indicates if debug messages should be included in DAST console output. True to include the debug messages. + """ + showDebugMessages: Boolean! + """ The maximum number of minutes allowed for the spider to traverse the site """ @@ -3673,6 +3693,13 @@ type DastScannerProfile { The maximum number of seconds allowed for the site under test to respond to a request """ targetTimeout: Int + + """ + Indicates if the AJAX spider should be used to crawl the target site. True to + run the AJAX spider in addition to the traditional spider, and false to run + only the traditional spider. + """ + useAjaxSpider: Boolean! } """ @@ -3714,6 +3741,16 @@ input DastScannerProfileCreateInput { """ profileName: String! + """ + Indicates the type of DAST scan that will run. Either a Passive Scan or an Active Scan. + """ + scanType: DastScanTypeEnum = PASSIVE + + """ + Indicates if debug messages should be included in DAST console output. True to include the debug messages. + """ + showDebugMessages: Boolean = false + """ The maximum number of minutes allowed for the spider to traverse the site. """ @@ -3723,6 +3760,13 @@ input DastScannerProfileCreateInput { The maximum number of seconds allowed for the site under test to respond to a request. """ targetTimeout: Int + + """ + Indicates if the AJAX spider should be used to crawl the target site. True to + run the AJAX spider in addition to the traditional spider, and false to run + only the traditional spider. + """ + useAjaxSpider: Boolean = false } """ @@ -3829,6 +3873,16 @@ input DastScannerProfileUpdateInput { """ profileName: String! + """ + Indicates the type of DAST scan that will run. Either a Passive Scan or an Active Scan. + """ + scanType: DastScanTypeEnum + + """ + Indicates if debug messages should be included in DAST console output. True to include the debug messages. + """ + showDebugMessages: Boolean + """ The maximum number of minutes allowed for the spider to traverse the site. """ @@ -3838,6 +3892,13 @@ input DastScannerProfileUpdateInput { The maximum number of seconds allowed for the site under test to respond to a request. """ targetTimeout: Int! + + """ + Indicates if the AJAX spider should be used to crawl the target site. True to + run the AJAX spider in addition to the traditional spider, and false to run + only the traditional spider. + """ + useAjaxSpider: Boolean } """ @@ -9429,6 +9490,21 @@ enum IssueState { opened } +""" +Values for issue state events +""" +enum IssueStateEvent { + """ + Closes the issue + """ + CLOSE + + """ + Reopens the issue + """ + REOPEN +} + """ Represents total number of issues for the represented statuses """ @@ -19012,6 +19088,11 @@ input UpdateIssueInput { """ removeLabelIds: [ID!] + """ + Close or reopen an issue. + """ + stateEvent: IssueStateEvent + """ Title of the issue """ @@ -19256,6 +19337,11 @@ type UpdateSnippetPayload { The snippet after mutation """ snippet: Snippet + + """ + Indicates whether the operation returns a record detected as spam + """ + spam: Boolean } scalar Upload diff --git a/doc/api/graphql/reference/gitlab_schema.json b/doc/api/graphql/reference/gitlab_schema.json index 47cbca51eed..4ca34d124e6 100644 --- a/doc/api/graphql/reference/gitlab_schema.json +++ b/doc/api/graphql/reference/gitlab_schema.json @@ -9438,6 +9438,20 @@ }, "isDeprecated": false, "deprecationReason": null + }, + { + "name": "spam", + "description": "Indicates whether the operation returns a record detected as spam", + "args": [ + + ], + "type": { + "kind": "SCALAR", + "name": "Boolean", + "ofType": null + }, + "isDeprecated": false, + "deprecationReason": null } ], "inputFields": null, @@ -9839,6 +9853,12 @@ "description": "Passive DAST scan. This scan will not make active attacks against the target site.", "isDeprecated": false, "deprecationReason": null + }, + { + "name": "ACTIVE", + "description": "Active DAST scan. This scan will make active attacks against the target site.", + "isDeprecated": false, + "deprecationReason": null } ], "possibleTypes": null @@ -9912,6 +9932,38 @@ "isDeprecated": false, "deprecationReason": null }, + { + "name": "scanType", + "description": "Indicates the type of DAST scan that will run. Either a Passive Scan or an Active Scan.", + "args": [ + + ], + "type": { + "kind": "ENUM", + "name": "DastScanTypeEnum", + "ofType": null + }, + "isDeprecated": false, + "deprecationReason": null + }, + { + "name": "showDebugMessages", + "description": "Indicates if debug messages should be included in DAST console output. True to include the debug messages.", + "args": [ + + ], + "type": { + "kind": "NON_NULL", + "name": null, + "ofType": { + "kind": "SCALAR", + "name": "Boolean", + "ofType": null + } + }, + "isDeprecated": false, + "deprecationReason": null + }, { "name": "spiderTimeout", "description": "The maximum number of minutes allowed for the spider to traverse the site", @@ -9939,6 +9991,24 @@ }, "isDeprecated": false, "deprecationReason": null + }, + { + "name": "useAjaxSpider", + "description": "Indicates if the AJAX spider should be used to crawl the target site. True to run the AJAX spider in addition to the traditional spider, and false to run only the traditional spider.", + "args": [ + + ], + "type": { + "kind": "NON_NULL", + "name": null, + "ofType": { + "kind": "SCALAR", + "name": "Boolean", + "ofType": null + } + }, + "isDeprecated": false, + "deprecationReason": null } ], "inputFields": null, @@ -10069,6 +10139,36 @@ }, "defaultValue": null }, + { + "name": "scanType", + "description": "Indicates the type of DAST scan that will run. Either a Passive Scan or an Active Scan.", + "type": { + "kind": "ENUM", + "name": "DastScanTypeEnum", + "ofType": null + }, + "defaultValue": "PASSIVE" + }, + { + "name": "useAjaxSpider", + "description": "Indicates if the AJAX spider should be used to crawl the target site. True to run the AJAX spider in addition to the traditional spider, and false to run only the traditional spider.", + "type": { + "kind": "SCALAR", + "name": "Boolean", + "ofType": null + }, + "defaultValue": "false" + }, + { + "name": "showDebugMessages", + "description": "Indicates if debug messages should be included in DAST console output. True to include the debug messages.", + "type": { + "kind": "SCALAR", + "name": "Boolean", + "ofType": null + }, + "defaultValue": "false" + }, { "name": "clientMutationId", "description": "A unique identifier for the client performing the mutation.", @@ -10398,6 +10498,36 @@ }, "defaultValue": null }, + { + "name": "scanType", + "description": "Indicates the type of DAST scan that will run. Either a Passive Scan or an Active Scan.", + "type": { + "kind": "ENUM", + "name": "DastScanTypeEnum", + "ofType": null + }, + "defaultValue": null + }, + { + "name": "useAjaxSpider", + "description": "Indicates if the AJAX spider should be used to crawl the target site. True to run the AJAX spider in addition to the traditional spider, and false to run only the traditional spider.", + "type": { + "kind": "SCALAR", + "name": "Boolean", + "ofType": null + }, + "defaultValue": null + }, + { + "name": "showDebugMessages", + "description": "Indicates if debug messages should be included in DAST console output. True to include the debug messages.", + "type": { + "kind": "SCALAR", + "name": "Boolean", + "ofType": null + }, + "defaultValue": null + }, { "name": "clientMutationId", "description": "A unique identifier for the client performing the mutation.", @@ -25789,6 +25919,29 @@ ], "possibleTypes": null }, + { + "kind": "ENUM", + "name": "IssueStateEvent", + "description": "Values for issue state events", + "fields": null, + "inputFields": null, + "interfaces": null, + "enumValues": [ + { + "name": "REOPEN", + "description": "Reopens the issue", + "isDeprecated": false, + "deprecationReason": null + }, + { + "name": "CLOSE", + "description": "Closes the issue", + "isDeprecated": false, + "deprecationReason": null + } + ], + "possibleTypes": null + }, { "kind": "OBJECT", "name": "IssueStatusCountsType", @@ -55396,6 +55549,16 @@ }, "defaultValue": null }, + { + "name": "stateEvent", + "description": "Close or reopen an issue.", + "type": { + "kind": "ENUM", + "name": "IssueStateEvent", + "ofType": null + }, + "defaultValue": null + }, { "name": "healthStatus", "description": "The desired health status", @@ -56073,6 +56236,20 @@ }, "isDeprecated": false, "deprecationReason": null + }, + { + "name": "spam", + "description": "Indicates whether the operation returns a record detected as spam", + "args": [ + + ], + "type": { + "kind": "SCALAR", + "name": "Boolean", + "ofType": null + }, + "isDeprecated": false, + "deprecationReason": null } ], "inputFields": null, diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 350a1b6c0f3..13dc69d679c 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -561,6 +561,7 @@ Autogenerated return type of CreateSnippet. | `clientMutationId` | String | A unique identifier for the client performing the mutation. | | `errors` | String! => Array | Errors encountered during execution of the mutation. | | `snippet` | Snippet | The snippet after mutation | +| `spam` | Boolean | Indicates whether the operation returns a record detected as spam | ### CreateTestCasePayload @@ -592,8 +593,11 @@ Represents a DAST scanner profile. | `globalId` | DastScannerProfileID! | ID of the DAST scanner profile | | `id` **{warning-solid}** | ID! | **Deprecated:** Use `global_id`. Deprecated in 13.4 | | `profileName` | String | Name of the DAST scanner profile | +| `scanType` | DastScanTypeEnum | Indicates the type of DAST scan that will run. Either a Passive Scan or an Active Scan. | +| `showDebugMessages` | Boolean! | Indicates if debug messages should be included in DAST console output. True to include the debug messages. | | `spiderTimeout` | Int | The maximum number of minutes allowed for the spider to traverse the site | | `targetTimeout` | Int | The maximum number of seconds allowed for the site under test to respond to a request | +| `useAjaxSpider` | Boolean! | Indicates if the AJAX spider should be used to crawl the target site. True to run the AJAX spider in addition to the traditional spider, and false to run only the traditional spider. | ### DastScannerProfileCreatePayload @@ -2732,6 +2736,7 @@ Autogenerated return type of UpdateSnippet. | `clientMutationId` | String | A unique identifier for the client performing the mutation. | | `errors` | String! => Array | Errors encountered during execution of the mutation. | | `snippet` | Snippet | The snippet after mutation | +| `spam` | Boolean | Indicates whether the operation returns a record detected as spam | ### User @@ -3140,6 +3145,7 @@ Mode of a commit action. | Value | Description | | ----- | ----------- | +| `ACTIVE` | Active DAST scan. This scan will make active attacks against the target site. | | `PASSIVE` | Passive DAST scan. This scan will not make active attacks against the target site. | ### DastSiteProfileValidationStatusEnum @@ -3304,6 +3310,15 @@ State of a GitLab issue. | `locked` | | | `opened` | | +### IssueStateEvent + +Values for issue state events. + +| Value | Description | +| ----- | ----------- | +| `CLOSE` | Closes the issue | +| `REOPEN` | Reopens the issue | + ### IssueType Issue type. diff --git a/doc/development/migration_style_guide.md b/doc/development/migration_style_guide.md index 740ac54c415..48de6e53a85 100644 --- a/doc/development/migration_style_guide.md +++ b/doc/development/migration_style_guide.md @@ -331,7 +331,7 @@ end **Usage with `disable_ddl_transaction!`** -Generally the `with_lock_retries` helper should work with `disabled_ddl_transaction!`. A custom RuboCop rule ensures that only allowed methods can be placed within the lock retries block. +Generally the `with_lock_retries` helper should work with `disable_ddl_transaction!`. A custom RuboCop rule ensures that only allowed methods can be placed within the lock retries block. ```ruby disable_ddl_transaction! @@ -348,7 +348,7 @@ end The RuboCop rule generally allows standard Rails migration methods, listed below. This example will cause a Rubocop offense: ```ruby -disabled_ddl_transaction! +disable_ddl_transaction! def up with_lock_retries do diff --git a/doc/user/project/integrations/jira.md b/doc/user/project/integrations/jira.md index 3e0b6492477..2bfc929ba56 100644 --- a/doc/user/project/integrations/jira.md +++ b/doc/user/project/integrations/jira.md @@ -23,7 +23,7 @@ Features include: - **View a list of Jira issues directly in GitLab** **(PREMIUM)** For additional features, you can install the -[Jira Development Panel integration](../../../integration/jira_development_panel.md) **(PREMIUM)**. +[Jira Development Panel integration](../../../integration/jira_development_panel.md). This enables you to: - In a Jira issue, display relevant GitLab information in the [development panel](https://support.atlassian.com/jira-software-cloud/docs/view-development-information-for-an-issue/), including related branches, commits, and merge requests. diff --git a/lib/api/snippets.rb b/lib/api/snippets.rb index c6ef35875fc..3acf8b2e944 100644 --- a/lib/api/snippets.rb +++ b/lib/api/snippets.rb @@ -5,8 +5,6 @@ module API class Snippets < Grape::API::Instance include PaginationParams - before { authenticate! } - resource :snippets do helpers Helpers::SnippetsHelpers helpers do @@ -23,7 +21,7 @@ module API end end - desc 'Get a snippets list for authenticated user' do + desc 'Get a snippets list for an authenticated user' do detail 'This feature was introduced in GitLab 8.15.' success Entities::Snippet end @@ -31,6 +29,8 @@ module API use :pagination end get do + authenticate! + present paginate(snippets_for_current_user), with: Entities::Snippet, current_user: current_user end @@ -42,6 +42,8 @@ module API use :pagination end get 'public' do + authenticate! + present paginate(public_snippets), with: Entities::PersonalSnippet, current_user: current_user end @@ -74,6 +76,8 @@ module API use :create_file_params end post do + authenticate! + authorize! :create_snippet attrs = process_create_params(declared_params(include_missing: false)) @@ -109,6 +113,8 @@ module API use :minimum_update_params end put ':id' do + authenticate! + snippet = snippets_for_current_user.find_by_id(params.delete(:id)) break not_found!('Snippet') unless snippet @@ -139,6 +145,8 @@ module API requires :id, type: Integer, desc: 'The ID of a snippet' end delete ':id' do + authenticate! + snippet = snippets_for_current_user.find_by_id(params.delete(:id)) break not_found!('Snippet') unless snippet diff --git a/lib/gitlab/usage_data.rb b/lib/gitlab/usage_data.rb index fbfc7beed9a..b9b9164d746 100644 --- a/lib/gitlab/usage_data.rb +++ b/lib/gitlab/usage_data.rb @@ -816,8 +816,6 @@ module Gitlab clear_memoization(:unique_visit_service) clear_memoization(:deployment_minimum_id) clear_memoization(:deployment_maximum_id) - clear_memoization(:approval_merge_request_rule_minimum_id) - clear_memoization(:approval_merge_request_rule_maximum_id) clear_memoization(:project_minimum_id) clear_memoization(:project_maximum_id) clear_memoization(:auth_providers) diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 5c52c17fee2..5f83f93e7cc 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -29237,9 +29237,6 @@ msgstr "" msgid "Work in progress Limit" msgstr "" -msgid "Workflow Help" -msgstr "" - msgid "Would you like to create a new branch?" msgstr "" diff --git a/rubocop/cop/migration/with_lock_retries_disallowed_method.rb b/rubocop/cop/migration/with_lock_retries_disallowed_method.rb index a89c33c298b..aef19517a9d 100644 --- a/rubocop/cop/migration/with_lock_retries_disallowed_method.rb +++ b/rubocop/cop/migration/with_lock_retries_disallowed_method.rb @@ -29,9 +29,14 @@ module RuboCop ].sort.freeze MSG = "The method is not allowed to be called within the `with_lock_retries` block, the only allowed methods are: #{ALLOWED_MIGRATION_METHODS.join(', ')}" + MSG_ONLY_ONE_FK_ALLOWED = "Avoid adding more than one foreign key within the `with_lock_retries`. See https://docs.gitlab.com/ee/development/migration_style_guide.html#examples" def_node_matcher :send_node?, <<~PATTERN - send + send + PATTERN + + def_node_matcher :add_foreign_key?, <<~PATTERN + (send nil? :add_foreign_key ...) PATTERN def on_block(node) @@ -53,6 +58,17 @@ module RuboCop name = node.children[1] add_offense(node, location: :expression) unless ALLOWED_MIGRATION_METHODS.include?(name) + add_offense(node, location: :selector, message: MSG_ONLY_ONE_FK_ALLOWED) if multiple_fks?(node) + end + + def multiple_fks?(node) + return unless add_foreign_key?(node) + + count = node.parent.each_descendant(:send).count do |node| + add_foreign_key?(node) + end + + count > 1 end end end diff --git a/spec/controllers/graphql_controller_spec.rb b/spec/controllers/graphql_controller_spec.rb index 405f1eae482..b0aa4b016d9 100644 --- a/spec/controllers/graphql_controller_spec.rb +++ b/spec/controllers/graphql_controller_spec.rb @@ -98,6 +98,12 @@ RSpec.describe GraphqlController do expect(assigns(:context)[:is_sessionless_user]).to be false end end + + it 'includes request object in context' do + post :execute + + expect(assigns(:context)[:request]).to eq request + end end describe 'Admin Mode' do diff --git a/spec/graphql/mutations/issues/update_spec.rb b/spec/graphql/mutations/issues/update_spec.rb index 15c15afd9b7..f9f4bdeb6fa 100644 --- a/spec/graphql/mutations/issues/update_spec.rb +++ b/spec/graphql/mutations/issues/update_spec.rb @@ -70,6 +70,23 @@ RSpec.describe Mutations::Issues::Update do end end + context 'when changing state' do + let_it_be_with_refind(:issue) { create(:issue, project: project, state: :opened) } + + it 'closes issue' do + mutation_params[:state_event] = 'close' + + expect { subject }.to change { issue.reload.state }.from('opened').to('closed') + end + + it 'reopens issue' do + issue.close + mutation_params[:state_event] = 'reopen' + + expect { subject }.to change { issue.reload.state }.from('closed').to('opened') + end + end + context 'when changing labels' do let_it_be(:label_1) { create(:label, project: project) } let_it_be(:label_2) { create(:label, project: project) } diff --git a/spec/helpers/search_helper_spec.rb b/spec/helpers/search_helper_spec.rb index 8ef37642db9..0c982116103 100644 --- a/spec/helpers/search_helper_spec.rb +++ b/spec/helpers/search_helper_spec.rb @@ -29,7 +29,7 @@ RSpec.describe SearchHelper do end it "includes Help sections" do - expect(search_autocomplete_opts("hel").size).to eq(9) + expect(search_autocomplete_opts("hel").size).to eq(8) end it "includes default sections" do diff --git a/spec/lib/gitlab/usage_data_spec.rb b/spec/lib/gitlab/usage_data_spec.rb index 09728539499..27b7394cc15 100644 --- a/spec/lib/gitlab/usage_data_spec.rb +++ b/spec/lib/gitlab/usage_data_spec.rb @@ -28,9 +28,16 @@ RSpec.describe Gitlab::UsageData, :aggregate_failures do project_minimum_id project_maximum_id user_minimum_id user_maximum_id unique_visit_service deployment_minimum_id deployment_maximum_id - approval_merge_request_rule_minimum_id - approval_merge_request_rule_maximum_id auth_providers) + + if Gitlab.ee? + values << %i(approval_merge_request_rule_minimum_id + approval_merge_request_rule_maximum_id + merge_request_minimum_id + merge_request_maximum_id) + values.flatten! + end + values.each do |key| expect(described_class).to receive(:clear_memoization).with(key) end diff --git a/spec/models/ci/build_trace_chunk_spec.rb b/spec/models/ci/build_trace_chunk_spec.rb index 57e58fe494f..a2f6bc1412c 100644 --- a/spec/models/ci/build_trace_chunk_spec.rb +++ b/spec/models/ci/build_trace_chunk_spec.rb @@ -780,6 +780,51 @@ RSpec.describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do end end + describe '#flush!' do + context 'when chunk can be flushed without problems' do + before do + allow(build_trace_chunk).to receive(:persist_data!) + end + + it 'completes migration successfully' do + expect { build_trace_chunk.flush! }.not_to raise_error + end + end + + context 'when the flush operation fails at first' do + it 'retries reloads the chunk' do + expect(build_trace_chunk) + .to receive(:persist_data!) + .and_raise(described_class::FailedToPersistDataError) + .ordered + expect(build_trace_chunk).to receive(:reset) + .and_return(build_trace_chunk) + .ordered + expect(build_trace_chunk) + .to receive(:persist_data!) + .ordered + + build_trace_chunk.flush! + end + end + + context 'when the flush constatly fails' do + before do + allow(build_trace_chunk) + .to receive(:persist_data!) + .and_raise(described_class::FailedToPersistDataError) + end + + it 'attems to reset the chunk but eventually fails too' do + expect(build_trace_chunk).to receive(:reset) + .and_return(build_trace_chunk) + + expect { build_trace_chunk.flush! } + .to raise_error(described_class::FailedToPersistDataError) + end + end + end + describe 'comparable build trace chunks' do describe '#<=>' do context 'when chunks are associated with different builds' do diff --git a/spec/requests/api/graphql/mutations/snippets/create_spec.rb b/spec/requests/api/graphql/mutations/snippets/create_spec.rb index a708c3fdf1f..d2fa3cfc24f 100644 --- a/spec/requests/api/graphql/mutations/snippets/create_spec.rb +++ b/spec/requests/api/graphql/mutations/snippets/create_spec.rb @@ -76,6 +76,8 @@ RSpec.describe 'Creating a Snippet' do expect(mutation_response['snippet']).to be_nil end + + it_behaves_like 'spam flag is present' end shared_examples 'creates snippet' do @@ -103,6 +105,10 @@ RSpec.describe 'Creating a Snippet' do end it_behaves_like 'snippet edit usage data counters' + it_behaves_like 'spam flag is present' + it_behaves_like 'can raise spam flag' do + let(:service) { Snippets::CreateService } + end end context 'with PersonalSnippet' do @@ -142,6 +148,9 @@ RSpec.describe 'Creating a Snippet' do it_behaves_like 'a mutation that returns errors in the response', errors: ["Title can't be blank"] it_behaves_like 'does not create snippet' + it_behaves_like 'can raise spam flag' do + let(:service) { Snippets::CreateService } + end end context 'when there non ActiveRecord errors' do diff --git a/spec/requests/api/graphql/mutations/snippets/update_spec.rb b/spec/requests/api/graphql/mutations/snippets/update_spec.rb index 67a9869c001..21d403c6f73 100644 --- a/spec/requests/api/graphql/mutations/snippets/update_spec.rb +++ b/spec/requests/api/graphql/mutations/snippets/update_spec.rb @@ -37,6 +37,8 @@ RSpec.describe 'Updating a Snippet' do graphql_mutation_response(:update_snippet) end + subject { post_graphql_mutation(mutation, current_user: current_user) } + shared_examples 'graphql update actions' do context 'when the user does not have permission' do let(:current_user) { create(:user) } @@ -46,14 +48,14 @@ RSpec.describe 'Updating a Snippet' do it 'does not update the Snippet' do expect do - post_graphql_mutation(mutation, current_user: current_user) + subject end.not_to change { snippet.reload } end end context 'when the user has permission' do it 'updates the snippet record' do - post_graphql_mutation(mutation, current_user: current_user) + subject expect(snippet.reload.title).to eq(updated_title) end @@ -65,7 +67,7 @@ RSpec.describe 'Updating a Snippet' do expect(blob_to_update.data).not_to eq updated_content expect(blob_to_delete).to be_present - post_graphql_mutation(mutation, current_user: current_user) + subject blob_to_update = blob_at(updated_file) blob_to_delete = blob_at(deleted_file) @@ -79,13 +81,19 @@ RSpec.describe 'Updating a Snippet' do end end + it_behaves_like 'can raise spam flag' do + let(:service) { Snippets::UpdateService } + end + + it_behaves_like 'spam flag is present' + context 'when there are ActiveRecord validation errors' do let(:updated_title) { '' } it_behaves_like 'a mutation that returns errors in the response', errors: ["Title can't be blank"] it 'does not update the Snippet' do - post_graphql_mutation(mutation, current_user: current_user) + subject expect(snippet.reload.title).to eq(original_title) end @@ -94,7 +102,7 @@ RSpec.describe 'Updating a Snippet' do blob_to_update = blob_at(updated_file) blob_to_delete = blob_at(deleted_file) - post_graphql_mutation(mutation, current_user: current_user) + subject aggregate_failures do expect(blob_at(updated_file).data).to eq blob_to_update.data @@ -104,6 +112,11 @@ RSpec.describe 'Updating a Snippet' do expect(mutation_response['snippet']['visibilityLevel']).to eq('private') end end + + it_behaves_like 'spam flag is present' + it_behaves_like 'can raise spam flag' do + let(:service) { Snippets::UpdateService } + end end def blob_at(filename) @@ -144,7 +157,7 @@ RSpec.describe 'Updating a Snippet' do context 'when the author is not a member of the project' do it 'returns an an error' do - post_graphql_mutation(mutation, current_user: current_user) + subject errors = json_response['errors'] expect(errors.first['message']).to eq(Gitlab::Graphql::Authorize::AuthorizeResource::RESOURCE_ACCESS_ERROR) @@ -162,7 +175,7 @@ RSpec.describe 'Updating a Snippet' do it 'returns an an error' do project.project_feature.update_attribute(:snippets_access_level, ProjectFeature::DISABLED) - post_graphql_mutation(mutation, current_user: current_user) + subject errors = json_response['errors'] expect(errors.first['message']).to eq(Gitlab::Graphql::Authorize::AuthorizeResource::RESOURCE_ACCESS_ERROR) diff --git a/spec/requests/api/project_snippets_spec.rb b/spec/requests/api/project_snippets_spec.rb index 08c88873078..c285f18f8b3 100644 --- a/spec/requests/api/project_snippets_spec.rb +++ b/spec/requests/api/project_snippets_spec.rb @@ -432,6 +432,14 @@ RSpec.describe API::ProjectSnippets do describe 'GET /projects/:project_id/snippets/:id/files/:ref/:file_path/raw' do let_it_be(:snippet) { create(:project_snippet, :repository, author: admin, project: project) } + context 'with no user' do + it 'requires authentication' do + get api("/projects/#{snippet.project.id}/snippets/#{snippet.id}/files/master/%2Egitattributes/raw", nil) + + expect(response).to have_gitlab_http_status(:unauthorized) + end + end + it_behaves_like 'raw snippet files' do let(:api_path) { "/projects/#{snippet.project.id}/snippets/#{snippet_id}/files/#{ref}/#{file_path}/raw" } end diff --git a/spec/requests/api/snippets_spec.rb b/spec/requests/api/snippets_spec.rb index 8d77026d26c..9c4902774f5 100644 --- a/spec/requests/api/snippets_spec.rb +++ b/spec/requests/api/snippets_spec.rb @@ -5,14 +5,15 @@ require 'spec_helper' RSpec.describe API::Snippets do include SnippetHelpers - let_it_be(:user) { create(:user) } + let_it_be(:admin) { create(:user, :admin) } + let_it_be(:user) { create(:user) } + let_it_be(:other_user) { create(:user) } + let_it_be(:public_snippet) { create(:personal_snippet, :repository, :public, author: user) } + let_it_be(:private_snippet) { create(:personal_snippet, :repository, :private, author: user) } + let_it_be(:internal_snippet) { create(:personal_snippet, :repository, :internal, author: user) } describe 'GET /snippets/' do - it 'returns snippets available' do - public_snippet = create(:personal_snippet, :repository, :public, author: user) - private_snippet = create(:personal_snippet, :repository, :private, author: user) - internal_snippet = create(:personal_snippet, :repository, :internal, author: user) - + it 'returns snippets available for user' do get api("/snippets/", user) expect(response).to have_gitlab_http_status(:ok) @@ -29,9 +30,7 @@ RSpec.describe API::Snippets do end it 'hides private snippets from regular user' do - create(:personal_snippet, :private) - - get api("/snippets/", user) + get api("/snippets/", other_user) expect(response).to have_gitlab_http_status(:ok) expect(response).to include_pagination_headers @@ -39,9 +38,7 @@ RSpec.describe API::Snippets do expect(json_response.size).to eq(0) end - it 'returns 404 for non-authenticated' do - create(:personal_snippet, :internal) - + it 'returns 401 for non-authenticated' do get api("/snippets/") expect(response).to have_gitlab_http_status(:unauthorized) @@ -62,10 +59,6 @@ RSpec.describe API::Snippets do end describe 'GET /snippets/public' do - let_it_be(:other_user) { create(:user) } - let_it_be(:public_snippet) { create(:personal_snippet, :repository, :public, author: user) } - let_it_be(:private_snippet) { create(:personal_snippet, :repository, :private, author: user) } - let_it_be(:internal_snippet) { create(:personal_snippet, :repository, :internal, author: user) } let_it_be(:public_snippet_other) { create(:personal_snippet, :repository, :public, author: other_user) } let_it_be(:private_snippet_other) { create(:personal_snippet, :repository, :private, author: other_user) } let_it_be(:internal_snippet_other) { create(:personal_snippet, :repository, :internal, author: other_user) } @@ -73,8 +66,10 @@ RSpec.describe API::Snippets do let_it_be(:private_snippet_project) { create(:project_snippet, :repository, :private, author: user) } let_it_be(:internal_snippet_project) { create(:project_snippet, :repository, :internal, author: user) } - it 'returns all snippets with public visibility from all users' do - get api("/snippets/public", user) + let(:path) { "/snippets/public" } + + it 'returns only public snippets from all users when authenticated' do + get api(path, user) aggregate_failures do expect(response).to have_gitlab_http_status(:ok) @@ -90,20 +85,23 @@ RSpec.describe API::Snippets do expect(json_response[1]['files'].first).to eq snippet_blob_file(public_snippet.blobs.first) end end - end - - describe 'GET /snippets/:id/raw' do - let_it_be(:author) { create(:user) } - let_it_be(:snippet) { create(:personal_snippet, :repository, :private, author: author) } it 'requires authentication' do - get api("/snippets/#{snippet.id}", nil) + get api(path, nil) expect(response).to have_gitlab_http_status(:unauthorized) end + end + + describe 'GET /snippets/:id/raw' do + let(:snippet) { private_snippet } + + it_behaves_like 'snippet access with different users' do + let(:path) { "/snippets/#{snippet.id}/raw" } + end it 'returns raw text' do - get api("/snippets/#{snippet.id}/raw", author) + get api("/snippets/#{snippet.id}/raw", user) expect(response).to have_gitlab_http_status(:ok) expect(response.media_type).to eq 'text/plain' @@ -113,28 +111,14 @@ RSpec.describe API::Snippets do it 'returns 404 for invalid snippet id' do snippet.destroy! - get api("/snippets/#{snippet.id}/raw", author) + get api("/snippets/#{snippet.id}/raw", user) expect(response).to have_gitlab_http_status(:not_found) expect(json_response['message']).to eq('404 Snippet Not Found') end - it 'hides private snippets from ordinary users' do - get api("/snippets/#{snippet.id}/raw", user) - - expect(response).to have_gitlab_http_status(:not_found) - end - - it 'shows internal snippets to ordinary users' do - internal_snippet = create(:personal_snippet, :internal, author: author) - - get api("/snippets/#{internal_snippet.id}/raw", user) - - expect(response).to have_gitlab_http_status(:ok) - end - it_behaves_like 'snippet blob content' do - let_it_be(:snippet_with_empty_repo) { create(:personal_snippet, :empty_repo, :private, author: author) } + let_it_be(:snippet_with_empty_repo) { create(:personal_snippet, :empty_repo, :private, author: user) } subject { get api("/snippets/#{snippet.id}/raw", snippet.author) } end @@ -149,33 +133,11 @@ RSpec.describe API::Snippets do end describe 'GET /snippets/:id' do - let_it_be(:admin) { create(:user, :admin) } - let_it_be(:author) { create(:user) } - let_it_be(:private_snippet) { create(:personal_snippet, :repository, :private, author: author) } - let_it_be(:internal_snippet) { create(:personal_snippet, :repository, :internal, author: author) } - let(:snippet) { private_snippet } + let(:snippet_id) { private_snippet.id } - subject { get api("/snippets/#{snippet.id}", user) } - - it 'hides private snippets from an ordinary user' do - subject - - expect(response).to have_gitlab_http_status(:not_found) - end - - context 'without a user' do - let(:user) { nil } - - it 'requires authentication' do - subject - - expect(response).to have_gitlab_http_status(:unauthorized) - end - end + subject { get api("/snippets/#{snippet_id}", user) } context 'with the author' do - let(:user) { author } - it 'returns snippet json' do subject @@ -191,18 +153,10 @@ RSpec.describe API::Snippets do end end - context 'with an admin' do - let(:user) { admin } - - it 'shows private snippets to an admin' do - subject - - expect(response).to have_gitlab_http_status(:ok) - end - - it 'returns 404 for invalid snippet id' do - private_snippet.destroy! + context 'with a non-existent snippet ID' do + let(:snippet_id) { 0 } + it 'returns 404' do subject expect(response).to have_gitlab_http_status(:not_found) @@ -210,19 +164,11 @@ RSpec.describe API::Snippets do end end - context 'with an internal snippet' do - let(:snippet) { internal_snippet } - - it 'shows internal snippets to an ordinary user' do - subject - - expect(response).to have_gitlab_http_status(:ok) - end + it_behaves_like 'snippet access with different users' do + let(:path) { "/snippets/#{snippet.id}" } end - it_behaves_like 'snippet_multiple_files feature disabled' do - let(:user) { author } - end + it_behaves_like 'snippet_multiple_files feature disabled' end describe 'POST /snippets/' do diff --git a/spec/routing/routing_spec.rb b/spec/routing/routing_spec.rb index 85a4c0e4bca..6150a8b26cc 100644 --- a/spec/routing/routing_spec.rb +++ b/spec/routing/routing_spec.rb @@ -119,9 +119,9 @@ RSpec.describe HelpController, "routing" do path: 'user/markdown', format: 'md') - path = '/help/workflow/protected_branches/protected_branches1.png' + path = '/help/user/markdown/markdown_logo.png' expect(get(path)).to route_to('help#show', - path: 'workflow/protected_branches/protected_branches1', + path: 'user/markdown/markdown_logo', format: 'png') end end diff --git a/spec/rubocop/cop/migration/with_lock_retries_disallowed_method_spec.rb b/spec/rubocop/cop/migration/with_lock_retries_disallowed_method_spec.rb index 11e4d784617..607daf0c9f0 100644 --- a/spec/rubocop/cop/migration/with_lock_retries_disallowed_method_spec.rb +++ b/spec/rubocop/cop/migration/with_lock_retries_disallowed_method_spec.rb @@ -53,6 +53,22 @@ RSpec.describe RuboCop::Cop::Migration::WithLockRetriesDisallowedMethod, type: : expect(cop.offenses.size).to eq(0) end + + describe 'for `add_foreign_key`' do + it 'registers an offense when more than two FKs are added' do + message = described_class::MSG_ONLY_ONE_FK_ALLOWED + + expect_offense <<~RUBY + with_lock_retries do + add_foreign_key :imports, :projects, column: :project_id, on_delete: :cascade + ^^^^^^^^^^^^^^^ #{message} + add_column :projects, :name, :text + add_foreign_key :imports, :users, column: :user_id, on_delete: :cascade + ^^^^^^^^^^^^^^^ #{message} + end + RUBY + end + end end context 'outside of migration' do diff --git a/spec/support/helpers/snippet_helpers.rb b/spec/support/helpers/snippet_helpers.rb index de64ad7d3e2..1ec50bce070 100644 --- a/spec/support/helpers/snippet_helpers.rb +++ b/spec/support/helpers/snippet_helpers.rb @@ -8,7 +8,7 @@ module SnippetHelpers def snippet_blob_file(blob) { "path" => blob.path, - "raw_url" => gitlab_raw_snippet_blob_url(blob.container, blob.path) + "raw_url" => gitlab_raw_snippet_blob_url(blob.container, blob.path, host: 'localhost') } end end diff --git a/spec/support/shared_examples/graphql/mutations/spammable_mutation_fields_examples.rb b/spec/support/shared_examples/graphql/mutations/spammable_mutation_fields_examples.rb new file mode 100644 index 00000000000..54b3f84a6e6 --- /dev/null +++ b/spec/support/shared_examples/graphql/mutations/spammable_mutation_fields_examples.rb @@ -0,0 +1,47 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.shared_examples 'spam flag is present' do + specify :aggregate_failures do + subject + + expect(mutation_response).to have_key('spam') + expect(mutation_response['spam']).to be_falsey + end +end + +RSpec.shared_examples 'can raise spam flag' do + it 'spam parameters are passed to the service' do + expect(service).to receive(:new).with(anything, anything, hash_including(api: true, request: instance_of(ActionDispatch::Request))) + + subject + end + + context 'when the snippet is detected as spam' do + it 'raises spam flag' do + allow_next_instance_of(service) do |instance| + allow(instance).to receive(:spam_check) do |snippet, user, _| + snippet.spam! + end + end + + subject + + expect(mutation_response['spam']).to be true + expect(mutation_response['errors']).to include("Your snippet has been recognized as spam and has been discarded.") + end + end + + context 'when :snippet_spam flag is disabled' do + before do + stub_feature_flags(snippet_spam: false) + end + + it 'request parameter is not passed to the service' do + expect(service).to receive(:new).with(anything, anything, hash_not_including(request: instance_of(ActionDispatch::Request))) + + subject + end + end +end diff --git a/spec/support/shared_examples/requests/api/snippets_shared_examples.rb b/spec/support/shared_examples/requests/api/snippets_shared_examples.rb index 051367fbe96..1a8acf67176 100644 --- a/spec/support/shared_examples/requests/api/snippets_shared_examples.rb +++ b/spec/support/shared_examples/requests/api/snippets_shared_examples.rb @@ -7,14 +7,6 @@ RSpec.shared_examples 'raw snippet files' do let(:file_path) { '%2Egitattributes' } let(:ref) { 'master' } - context 'with no user' do - it 'requires authentication' do - get api(api_path) - - expect(response).to have_gitlab_http_status(:unauthorized) - end - end - shared_examples 'not found' do it 'returns 404' do get api(api_path, user) @@ -216,3 +208,58 @@ RSpec.shared_examples 'invalid snippet updates' do expect(json_response['error']).to eq 'title is empty' end end + +RSpec.shared_examples 'snippet access with different users' do + using RSpec::Parameterized::TableSyntax + + where(:requester, :visibility, :status) do + :admin | :public | :ok + :admin | :private | :ok + :admin | :internal | :ok + :author | :public | :ok + :author | :private | :ok + :author | :internal | :ok + :other | :public | :ok + :other | :private | :not_found + :other | :internal | :ok + nil | :public | :ok + nil | :private | :not_found + nil | :internal | :not_found + end + + with_them do + let(:snippet) { snippet_for(visibility) } + + it 'returns the correct response' do + request_user = user_for(requester) + + get api(path, request_user) + + expect(response).to have_gitlab_http_status(status) + end + end + + def user_for(user_type) + case user_type + when :author + user + when :other + other_user + when :admin + admin + else + nil + end + end + + def snippet_for(snippet_type) + case snippet_type + when :private + private_snippet + when :internal + internal_snippet + when :public + public_snippet + end + end +end