diff options
42 files changed, 474 insertions, 248 deletions
diff --git a/app/assets/javascripts/pages/snippets/show/index.js b/app/assets/javascripts/pages/snippets/show/index.js index 26936110402..5efcc901fde 100644 --- a/app/assets/javascripts/pages/snippets/show/index.js +++ b/app/assets/javascripts/pages/snippets/show/index.js @@ -5,9 +5,11 @@ import initNotes from '~/init_notes'; import snippetEmbed from '~/snippet/snippet_embed'; document.addEventListener('DOMContentLoaded', () => { - new LineHighlighter(); // eslint-disable-line no-new - new BlobViewer(); // eslint-disable-line no-new - initNotes(); - new ZenMode(); // eslint-disable-line no-new - snippetEmbed(); + if (!gon.features.snippetsVue) { + new LineHighlighter(); // eslint-disable-line no-new + new BlobViewer(); // eslint-disable-line no-new + initNotes(); + new ZenMode(); // eslint-disable-line no-new + snippetEmbed(); + } }); diff --git a/app/assets/stylesheets/components/toast.scss b/app/assets/stylesheets/components/toast.scss deleted file mode 100644 index e27bf282247..00000000000 --- a/app/assets/stylesheets/components/toast.scss +++ /dev/null @@ -1,52 +0,0 @@ -/* -* These styles are specific to the gl-toast component. -* Documentation: https://design.gitlab.com/components/toasts -* Note: Styles below are nested in order to override some of vue-toasted's default styling -*/ -.toasted-container { - - max-width: $toast-max-width; - - @include media-breakpoint-down(xs) { - width: 100%; - padding-right: $toast-padding-right; - } - - .toasted.gl-toast { - border-radius: $border-radius-default; - font-size: $gl-font-size; - padding: $gl-padding-8 $gl-padding $gl-padding-8 $gl-padding-24; - margin-top: $toast-default-margin; - line-height: $gl-line-height; - background-color: rgba($gray-900, $toast-background-opacity); - - span { - padding-right: $gl-padding-8; - } - - @include media-breakpoint-down(xs) { - .action:first-of-type { - // Ensures actions buttons are right aligned on mobile - margin-left: auto; - } - } - - .action { - color: $blue-300; - margin: 0 0 0 $toast-default-margin; - text-transform: none; - font-size: $gl-font-size; - } - - .toast-close { - font-size: $default-icon-size; - margin-left: $toast-default-margin; - } - } -} - -// Overrides the default positioning of toasts -body .toasted-container.bottom-left { - bottom: $toast-offset; - left: $toast-offset; -} diff --git a/app/assets/stylesheets/framework/variables.scss b/app/assets/stylesheets/framework/variables.scss index 0f77c451fac..a833e104b49 100644 --- a/app/assets/stylesheets/framework/variables.scss +++ b/app/assets/stylesheets/framework/variables.scss @@ -529,16 +529,6 @@ $pagination-line-height: 20px; $pagination-disabled-color: #cdcdcd; /* -* Toasts -*/ -$toast-offset: 24px; -$toast-height: 48px; -$toast-max-width: 586px; -$toast-padding-right: 42px; -$toast-default-margin: 8px; -$toast-background-opacity: 0.95; - -/* * Status icons */ $status-icon-size: 22px; diff --git a/app/controllers/projects/pipeline_schedules_controller.rb b/app/controllers/projects/pipeline_schedules_controller.rb index 72e939a3310..6a7e2b69652 100644 --- a/app/controllers/projects/pipeline_schedules_controller.rb +++ b/app/controllers/projects/pipeline_schedules_controller.rb @@ -83,12 +83,14 @@ class Projects::PipelineSchedulesController < Projects::ApplicationController def play_rate_limit return unless current_user - limiter = ::Gitlab::ActionRateLimiter.new(action: :play_pipeline_schedule) - - return unless limiter.throttled?([current_user, schedule], 1) + if rate_limiter.throttled?(:play_pipeline_schedule, scope: [current_user, schedule]) + flash[:alert] = _('You cannot play this scheduled pipeline at the moment. Please wait a minute.') + redirect_to pipeline_schedules_path(@project) + end + end - flash[:alert] = _('You cannot play this scheduled pipeline at the moment. Please wait a minute.') - redirect_to pipeline_schedules_path(@project) + def rate_limiter + ::Gitlab::ApplicationRateLimiter end def schedule diff --git a/app/controllers/projects/raw_controller.rb b/app/controllers/projects/raw_controller.rb index c94fdd9483d..985587268c5 100644 --- a/app/controllers/projects/raw_controller.rb +++ b/app/controllers/projects/raw_controller.rb @@ -19,14 +19,16 @@ class Projects::RawController < Projects::ApplicationController private def show_rate_limit - limiter = ::Gitlab::ActionRateLimiter.new(action: :show_raw_controller) + if rate_limiter.throttled?(:show_raw_controller, scope: [@project, @commit, @path], threshold: raw_blob_request_limit) + rate_limiter.log_request(request, :raw_blob_request_limit, current_user) - return unless limiter.throttled?([@project, @commit, @path], raw_blob_request_limit) - - limiter.log_request(request, :raw_blob_request_limit, current_user) + flash[:alert] = _('You cannot access the raw file. Please wait a minute.') + redirect_to project_blob_path(@project, File.join(@ref, @path)), status: :too_many_requests + end + end - flash[:alert] = _('You cannot access the raw file. Please wait a minute.') - redirect_to project_blob_path(@project, File.join(@ref, @path)), status: :too_many_requests + def rate_limiter + ::Gitlab::ApplicationRateLimiter end def raw_blob_request_limit diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index e5dea031bb5..47d6fb67108 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -32,6 +32,9 @@ class ProjectsController < Projects::ApplicationController before_action :authorize_archive_project!, only: [:archive, :unarchive] before_action :event_filter, only: [:show, :activity] + # Project Export Rate Limit + before_action :export_rate_limit, only: [:export, :download_export, :generate_new_export] + layout :determine_layout def index @@ -465,6 +468,21 @@ class ProjectsController < Projects::ApplicationController def present_project @project = @project.present(current_user: current_user) end + + def export_rate_limit + prefixed_action = "project_#{params[:action]}".to_sym + + if rate_limiter.throttled?(prefixed_action, scope: [current_user, prefixed_action, @project]) + rate_limiter.log_request(request, "#{prefixed_action}_request_limit".to_sym, current_user) + + flash[:alert] = _('This endpoint has been requested too many times. Try again later.') + redirect_to edit_project_path(@project) + end + end + + def rate_limiter + ::Gitlab::ApplicationRateLimiter + end end ProjectsController.prepend_if_ee('EE::ProjectsController') diff --git a/app/views/snippets/show.html.haml b/app/views/snippets/show.html.haml index 36b4e00e8d5..c77b05e3ea8 100644 --- a/app/views/snippets/show.html.haml +++ b/app/views/snippets/show.html.haml @@ -4,13 +4,16 @@ - breadcrumb_title @snippet.to_reference - page_title "#{@snippet.title} (#{@snippet.to_reference})", _("Snippets") -= render 'shared/snippets/header' +- if Feature.enabled?(:snippets_vue) + #js-snippet-view{ 'data-qa-selector': 'snippet_view' } +- else + = render 'shared/snippets/header' -.personal-snippets - %article.file-holder.snippet-file-content - = render 'shared/snippets/blob' + .personal-snippets + %article.file-holder.snippet-file-content + = render 'shared/snippets/blob' - .row-content-block.top-block.content-component-block - = render 'award_emoji/awards_block', awardable: @snippet, inline: true + .row-content-block.top-block.content-component-block + = render 'award_emoji/awards_block', awardable: @snippet, inline: true - #notes.limited-width-notes= render "shared/notes/notes_with_form", :autocomplete => false + #notes.limited-width-notes= render "shared/notes/notes_with_form", :autocomplete => false diff --git a/changelogs/unreleased/georgekoltsov-add-rate-limit-to-exports.yml b/changelogs/unreleased/georgekoltsov-add-rate-limit-to-exports.yml new file mode 100644 index 00000000000..316a4ed46a3 --- /dev/null +++ b/changelogs/unreleased/georgekoltsov-add-rate-limit-to-exports.yml @@ -0,0 +1,5 @@ +--- +title: Add Project Export request/download rate limits +merge_request: 20962 +author: +type: other diff --git a/changelogs/unreleased/remove-redundant-toast-scss.yml b/changelogs/unreleased/remove-redundant-toast-scss.yml new file mode 100644 index 00000000000..6b950465149 --- /dev/null +++ b/changelogs/unreleased/remove-redundant-toast-scss.yml @@ -0,0 +1,5 @@ +--- +title: Remove redundant toast.scss file and variables +merge_request: 21105 +author: +type: fixed diff --git a/doc/administration/monitoring/prometheus/index.md b/doc/administration/monitoring/prometheus/index.md index c0b563bd76e..eb7a2d791c1 100644 --- a/doc/administration/monitoring/prometheus/index.md +++ b/doc/administration/monitoring/prometheus/index.md @@ -288,6 +288,12 @@ The PgBouncer exporter allows you to measure various PgBouncer metrics. [➔ Read more about the PgBouncer exporter.](pgbouncer_exporter.md) +### Registry exporter + +The Registry exporter allows you to measure various Registry metrics. + +[➔ Read more about the Registry exporter.](registry_exporter.md) + ### GitLab exporter The GitLab exporter allows you to measure various GitLab metrics, pulled from Redis and the database. diff --git a/doc/administration/monitoring/prometheus/postgres_exporter.md b/doc/administration/monitoring/prometheus/postgres_exporter.md index 3ad15b65497..044ce64af53 100644 --- a/doc/administration/monitoring/prometheus/postgres_exporter.md +++ b/doc/administration/monitoring/prometheus/postgres_exporter.md @@ -8,20 +8,54 @@ The [postgres exporter] allows you to measure various PostgreSQL metrics. To enable the postgres exporter: -1. [Enable Prometheus](index.md#configuring-prometheus) -1. Edit `/etc/gitlab/gitlab.rb` -1. Add or find and uncomment the following line, making sure it's set to `true`: +1. [Enable Prometheus](index.md#configuring-prometheus). +1. Edit `/etc/gitlab/gitlab.rb` and enable `postgres_exporter`: ```ruby postgres_exporter['enable'] = true ``` +NOTE: **Note:** +If PostgreSQL is configured on a separate node, make sure that the local +address is [listed in `trust_auth_cidr_addresses`](../../high_availability/database.md#network-information) or the +exporter will not be able to connect to the database. + 1. Save the file and [reconfigure GitLab][reconfigure] for the changes to - take effect + take effect. Prometheus will now automatically begin collecting performance data from the postgres exporter exposed under `localhost:9187`. +## Advanced configuration + +In most cases, Postgres exporter will work with the defaults and you should not +need to change anything. + +The following configuration options can be used to further customize the +Postgres exporter: + +1. Edit `/etc/gitlab/gitlab.rb`: + + ```ruby + postgres_exporter['dbname'] = 'pgbouncer' # The name of the database to connect to. + postgres_exporter['user'] = 'gitlab-psql' # The user to sign in as. + postgres_exporter['password'] = '' # The user's password. + postgres_exporter['host'] = 'localhost' # The host to connect to. Values that start with '/' are for unix domain sockets (default is 'localhost'). + postgres_exporter['port'] = 5432 # The port to bind to (default is '5432'). + postgres_exporter['sslmode'] = 'require' # Whether or not to use SSL. Valid options are: + # 'disable' (no SSL), + # 'require' (always use SSL and skip verification, this is the default value), + # 'verify-ca' (always use SSL and verify that the certificate presented by the server was signed by a trusted CA), + # 'verify-full' (always use SSL and verify that the certification presented by the server was signed by a trusted CA and the server host name matches the one in the certificate). + postgres_exporter['fallback_application_name'] = '' # An application_name to fall back to if one isn't provided. + postgres_exporter['connect_timeout'] = '' # Maximum wait for connection, in seconds. Zero or not specified means wait indefinitely. + postgres_exporter['sslcert'] = 'ssl.crt' # Cert file location. The file must contain PEM encoded data. + postgres_exporter['sslkey'] = 'ssl.key' # Key file location. The file must contain PEM encoded data. + postgres_exporter['sslrootcert'] = 'ssl-root.crt' # The location of the root certificate file. The file must contain PEM encoded data. + ``` + +1. Save the file and [reconfigure GitLab][reconfigure] for the changes to take effect. + [← Back to the main Prometheus page](index.md) [1131]: https://gitlab.com/gitlab-org/omnibus-gitlab/merge_requests/1131 diff --git a/doc/administration/monitoring/prometheus/registry_exporter.md b/doc/administration/monitoring/prometheus/registry_exporter.md new file mode 100644 index 00000000000..692e589185e --- /dev/null +++ b/doc/administration/monitoring/prometheus/registry_exporter.md @@ -0,0 +1,21 @@ +# Registry exporter + +> [Introduced](https://gitlab.com/gitlab-org/omnibus-gitlab/merge_requests/2884) in GitLab 11.9. + +The Registry exporter allows you to measure various Registry metrics. +To enable it: + +1. [Enable Prometheus](index.md#configuring-prometheus). +1. Edit `/etc/gitlab/gitlab.rb` and enable [debug mode](https://docs.docker.com/registry/#debug) for the Registry: + + ```ruby + registry['debug_addr'] = "localhost:5001" # localhost:5001/metrics + ``` + +1. Save the file and [reconfigure GitLab](../../restart_gitlab.md#omnibus-gitlab-reconfigure) + for the changes to take effect. + +Prometheus will now automatically begin collecting performance data from +the registry exporter exposed under `localhost:5001/metrics`. + +[← Back to the main Prometheus page](index.md) diff --git a/doc/development/feature_flags/development.md b/doc/development/feature_flags/development.md index c410c7eae41..77795b8f1d7 100644 --- a/doc/development/feature_flags/development.md +++ b/doc/development/feature_flags/development.md @@ -129,3 +129,9 @@ In the rails console (`rails c`), enter the following command to enable your fea ```ruby Feature.enable(:feature_flag_name) ``` + +Similarly, the following command will disable a feature flag: + +```ruby +Feature.disable(:feature_flag_name) +``` diff --git a/doc/user/application_security/security_dashboard/index.md b/doc/user/application_security/security_dashboard/index.md index c3360df4ac6..e3044fccafb 100644 --- a/doc/user/application_security/security_dashboard/index.md +++ b/doc/user/application_security/security_dashboard/index.md @@ -71,7 +71,7 @@ Once you're on the dashboard, at the top you should see a series of filters for: - Report type - Project -To the right of the filters, you should see a **Hide dismissed** toggle button ([available for GitLab.com Gold, planned for GitLab Ultimate 12.6](https://gitlab.com/gitlab-org/gitlab/issues/9102)). +To the right of the filters, you should see a **Hide dismissed** toggle button. NOTE: **Note:** The dashboard only shows projects with [security reports](#supported-reports) enabled in a group. diff --git a/doc/user/gitlab_com/index.md b/doc/user/gitlab_com/index.md index 929a132c4c3..466e4e43bfc 100644 --- a/doc/user/gitlab_com/index.md +++ b/doc/user/gitlab_com/index.md @@ -107,6 +107,8 @@ installed. Instances provide 1 vCPU and 25GB of HDD disk space. The default region of the VMs is US East1. Each instance is used only for one job, this ensures any sensitive data left on the system can't be accessed by other people their CI jobs. +The `gitlab-shared-runners-manager-X.gitlab.com` fleet of Runners are dedicated for GitLab projects as well as community forks of them. They use a slightly larger machine type (n1-standard-2) and have a bigger SSD disk size. They will not run untagged jobs and unlike the general fleet of shared Runners, the instances are re-used up to 40 times. + Jobs handled by the shared Runners on GitLab.com (`shared-runners-manager-X.gitlab.com`), **will be timed out after 3 hours**, regardless of the timeout configured in a project. Check the issues [4010] and [4070] for the reference. diff --git a/doc/user/packages/container_registry/index.md b/doc/user/packages/container_registry/index.md index f6c9a2c9e34..9c1a9d5a41a 100644 --- a/doc/user/packages/container_registry/index.md +++ b/doc/user/packages/container_registry/index.md @@ -144,7 +144,6 @@ project or branch name. Special characters can include: - Leading underscore - Trailing hyphen/dash -- Double hyphen/dash To get around this, you can [change the group path](../../group/index.md#changing-a-groups-path), [change the project path](../../project/settings/index.md#renaming-a-repository) or change the branch diff --git a/doc/user/packages/maven_repository/img/maven_package_view.png b/doc/user/packages/maven_repository/img/maven_package_view.png Binary files differdeleted file mode 100644 index 2eb4b6f76b4..00000000000 --- a/doc/user/packages/maven_repository/img/maven_package_view.png +++ /dev/null diff --git a/doc/user/packages/maven_repository/img/maven_package_view_v12_6.png b/doc/user/packages/maven_repository/img/maven_package_view_v12_6.png Binary files differnew file mode 100644 index 00000000000..92cefc26660 --- /dev/null +++ b/doc/user/packages/maven_repository/img/maven_package_view_v12_6.png diff --git a/doc/user/packages/maven_repository/index.md b/doc/user/packages/maven_repository/index.md index 70ff26b28b2..da5139fcaf9 100644 --- a/doc/user/packages/maven_repository/index.md +++ b/doc/user/packages/maven_repository/index.md @@ -5,7 +5,7 @@ With the GitLab [Maven](https://maven.apache.org) Repository, every project can have its own space to store its Maven artifacts. -![GitLab Maven Repository](img/maven_package_view.png) +![GitLab Maven Repository](img/maven_package_view_v12_6.png) ## Enabling the Maven Repository diff --git a/doc/user/packages/npm_registry/img/npm_package_view.png b/doc/user/packages/npm_registry/img/npm_package_view.png Binary files differdeleted file mode 100644 index e0634718c02..00000000000 --- a/doc/user/packages/npm_registry/img/npm_package_view.png +++ /dev/null diff --git a/doc/user/packages/npm_registry/img/npm_package_view_v12_5.png b/doc/user/packages/npm_registry/img/npm_package_view_v12_5.png Binary files differnew file mode 100644 index 00000000000..a6f823011eb --- /dev/null +++ b/doc/user/packages/npm_registry/img/npm_package_view_v12_5.png diff --git a/doc/user/packages/npm_registry/index.md b/doc/user/packages/npm_registry/index.md index e611e4d99fb..214be0854b6 100644 --- a/doc/user/packages/npm_registry/index.md +++ b/doc/user/packages/npm_registry/index.md @@ -5,7 +5,7 @@ With the GitLab NPM Registry, every project can have its own space to store NPM packages. -![GitLab NPM Registry](img/npm_package_view.png) +![GitLab NPM Registry](img/npm_package_view_v12_5.png) NOTE: **Note:** Only [scoped](https://docs.npmjs.com/misc/scope) packages are supported. @@ -42,6 +42,20 @@ it is not possible due to a naming collision. For example: | `gitlab-org/gitlab` | `@gitlab-org/gitlab` | Yes | | `gitlab-org/gitlab` | `@foo/bar` | No | +The regex that is used for naming is validating all package names from all package managers: + +``` +/\A\@?(([\w\-\.\+]*)\/)*([\w\-\.]+)@?(([\w\-\.\+]*)\/)*([\w\-\.]*)\z/ +``` + +It allows for capital letters, while NPM does not, and allows for almost all of the +characters NPM allows with a few exceptions (`~` is not allowed). + +NOTE: **Note:** Capital letters are needed because the scope is required to be +identical to the top level namespace of the project. So, for example, if your +project path is `My-Group/project-foo`, your package must be named `@My-Group/any-package-name`. +`@my-group/any-package-name` will not work. + CAUTION: **When updating the path of a user/group or transferring a (sub)group/project:** If you update the root namespace of a project with NPM packages, your changes will be rejected. To be allowed to do that, make sure to remove any NPM package first. Don't forget to update your `.npmrc` files to follow the above naming convention and run `npm publish` if necessary. diff --git a/doc/user/project/integrations/webhooks.md b/doc/user/project/integrations/webhooks.md index 1e229db8b2e..f371f2ac288 100644 --- a/doc/user/project/integrations/webhooks.md +++ b/doc/user/project/integrations/webhooks.md @@ -1285,7 +1285,7 @@ Markdown features, like link labels. ## Testing webhooks -You can trigger the webhook manually. Sample data from the project will be used.Sample data will take from the project. +You can trigger the webhook manually. Sample data from the project will be used. Sample data will take from the project. > For example: for triggering `Push Events` your project should have at least one commit. ![Webhook testing](img/webhook_testing.png) diff --git a/lib/api/project_export.rb b/lib/api/project_export.rb index e34ed0bdb44..ef6a8f1a396 100644 --- a/lib/api/project_export.rb +++ b/lib/api/project_export.rb @@ -2,6 +2,15 @@ module API class ProjectExport < Grape::API + helpers do + def throttled?(action) + rate_limiter.throttled?(action, scope: [current_user, action, user_project]) + end + + def rate_limiter + ::Gitlab::ApplicationRateLimiter + end + end before do not_found! unless Gitlab::CurrentSettings.project_export_enabled? authorize_admin_project @@ -23,6 +32,10 @@ module API detail 'This feature was introduced in GitLab 10.6.' end get ':id/export/download' do + if throttled?(:project_download_export) + render_api_error!({ error: 'This endpoint has been requested too many times. Try again later.' }, 429) + end + if user_project.export_file_exists? present_carrierwave_file!(user_project.export_file) else @@ -41,6 +54,10 @@ module API end end post ':id/export' do + if throttled?(:project_export) + render_api_error!({ error: 'This endpoint has been requested too many times. Try again later.' }, 429) + end + project_export_params = declared_params(include_missing: false) after_export_params = project_export_params.delete(:upload) || {} diff --git a/lib/gitlab/action_rate_limiter.rb b/lib/gitlab/action_rate_limiter.rb deleted file mode 100644 index 0e8707af631..00000000000 --- a/lib/gitlab/action_rate_limiter.rb +++ /dev/null @@ -1,81 +0,0 @@ -# frozen_string_literal: true - -module Gitlab - # This class implements a simple rate limiter that can be used to throttle - # certain actions. Unlike Rack Attack and Rack::Throttle, which operate at - # the middleware level, this can be used at the controller level. - class ActionRateLimiter - TIME_TO_EXPIRE = 60 # 1 min - - attr_accessor :action, :expiry_time - - def initialize(action:, expiry_time: TIME_TO_EXPIRE) - @action = action - @expiry_time = expiry_time - end - - # Increments the given cache key and increments the value by 1 with the - # given expiration time. Returns the incremented value. - # - # key - An array of ActiveRecord instances - def increment(key) - value = 0 - - Gitlab::Redis::Cache.with do |redis| - cache_key = action_key(key) - value = redis.incr(cache_key) - redis.expire(cache_key, expiry_time) if value == 1 - end - - value - end - - # Increments the given key and returns true if the action should - # be throttled. - # - # key - An array of ActiveRecord instances or strings - # threshold_value - The maximum number of times this action should occur in the given time interval. If number is zero is considered disabled. - def throttled?(key, threshold_value) - threshold_value > 0 && - self.increment(key) > threshold_value - end - - # Logs request into auth.log - # - # request - Web request to be logged - # type - A symbol key that represents the request. - # current_user - Current user of the request, it can be nil. - def log_request(request, type, current_user) - request_information = { - message: 'Action_Rate_Limiter_Request', - env: type, - remote_ip: request.ip, - request_method: request.request_method, - path: request.fullpath - } - - if current_user - request_information.merge!({ - user_id: current_user.id, - username: current_user.username - }) - end - - Gitlab::AuthLogger.error(request_information) - end - - private - - def action_key(key) - serialized = key.map do |obj| - if obj.is_a?(String) - "#{obj}" - else - "#{obj.class.model_name.to_s.underscore}:#{obj.id}" - end - end.join(":") - - "action_rate_limiter:#{action}:#{serialized}" - end - end -end diff --git a/lib/gitlab/application_rate_limiter.rb b/lib/gitlab/application_rate_limiter.rb new file mode 100644 index 00000000000..629632b744b --- /dev/null +++ b/lib/gitlab/application_rate_limiter.rb @@ -0,0 +1,129 @@ +# frozen_string_literal: true + +module Gitlab + # This class implements a simple rate limiter that can be used to throttle + # certain actions. Unlike Rack Attack and Rack::Throttle, which operate at + # the middleware level, this can be used at the controller or API level. + # + # @example + # if Gitlab::ApplicationRateLimiter.throttled?(:project_export, scope: [@project, @current_user]) + # flash[:alert] = 'error!' + # redirect_to(edit_project_path(@project), status: :too_many_requests) + # end + class ApplicationRateLimiter + class << self + # Application rate limits + # + # Threshold value can be either an Integer or a Proc + # in order to not evaluate it's value every time this method is called + # and only do that when it's needed. + def rate_limits + { + project_export: { threshold: 1, interval: 5.minutes }, + project_download_export: { threshold: 10, interval: 10.minutes }, + project_generate_new_export: { threshold: 1, interval: 5.minutes }, + play_pipeline_schedule: { threshold: 1, interval: 1.minute }, + show_raw_controller: { threshold: -> { Gitlab::CurrentSettings.current_application_settings.raw_blob_request_limit }, interval: 1.minute } + }.freeze + end + + # Increments the given key and returns true if the action should + # be throttled. + # + # @param key [Symbol] Key attribute registered in `.rate_limits` + # @option scope [Array<ActiveRecord>] Array of ActiveRecord models to scope throttling to a specific request (e.g. per user per project) + # @option threshold [Integer] Optional threshold value to override default one registered in `.rate_limits` + # @option interval [Integer] Optional interval value to override default one registered in `.rate_limits` + # + # @return [Boolean] Whether or not a request should be throttled + def throttled?(key, scope: nil, interval: nil, threshold: nil) + return unless rate_limits[key] + + threshold_value = threshold || threshold(key) + + threshold_value > 0 && + increment(key, scope, interval) > threshold_value + end + + # Increments the given cache key and increments the value by 1 with the + # expiration interval defined in `.rate_limits`. + # + # @param key [Symbol] Key attribute registered in `.rate_limits` + # @option scope [Array<ActiveRecord>] Array of ActiveRecord models to scope throttling to a specific request (e.g. per user per project) + # @option interval [Integer] Optional interval value to override default one registered in `.rate_limits` + # + # @return [Integer] incremented value + def increment(key, scope, interval = nil) + value = 0 + interval_value = interval || interval(key) + + Gitlab::Redis::Cache.with do |redis| + cache_key = action_key(key, scope) + value = redis.incr(cache_key) + redis.expire(cache_key, interval_value) if value == 1 + end + + value + end + + # Logs request using provided logger + # + # @param request [Http::Request] - Web request to be logged + # @param type [Symbol] A symbol key that represents the request + # @param current_user [User] Current user of the request, it can be nil + # @param logger [Logger] Logger to log request to a specific log file. Defaults to Gitlab::AuthLogger + def log_request(request, type, current_user, logger = Gitlab::AuthLogger) + request_information = { + message: 'Application_Rate_Limiter_Request', + env: type, + remote_ip: request.ip, + request_method: request.request_method, + path: request.fullpath + } + + if current_user + request_information.merge!({ + user_id: current_user.id, + username: current_user.username + }) + end + + logger.error(request_information) + end + + private + + def threshold(key) + value = rate_limit_value_by_key(key, :threshold) + + return value.call if value.is_a?(Proc) + + value.to_i + end + + def interval(key) + rate_limit_value_by_key(key, :interval).to_i + end + + def rate_limit_value_by_key(key, setting) + action = rate_limits[key] + + action[setting] if action + end + + def action_key(key, scope) + composed_key = [key, scope].flatten.compact + + serialized = composed_key.map do |obj| + if obj.is_a?(String) || obj.is_a?(Symbol) + "#{obj}" + else + "#{obj.class.model_name.to_s.underscore}:#{obj.id}" + end + end.join(":") + + "application_rate_limiter:#{serialized}" + end + end + end +end diff --git a/lib/gitlab/gon_helper.rb b/lib/gitlab/gon_helper.rb index 2616a19fdaa..487dcd58d01 100644 --- a/lib/gitlab/gon_helper.rb +++ b/lib/gitlab/gon_helper.rb @@ -42,6 +42,7 @@ module Gitlab # Initialize gon.features with any flags that should be # made globally available to the frontend push_frontend_feature_flag(:suppress_ajax_navigation_errors, default_enabled: true) + push_frontend_feature_flag(:snippets_vue, default_enabled: false) end # Exposes the state of a feature flag to the frontend code. diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 58b9c7d905a..5a404b8976f 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -17898,6 +17898,9 @@ msgstr "" msgid "This domain is not verified. You will need to verify ownership before access is enabled." msgstr "" +msgid "This endpoint has been requested too many times. Try again later." +msgstr "" + msgid "This environment has no deployments yet." msgstr "" diff --git a/spec/controllers/projects/raw_controller_spec.rb b/spec/controllers/projects/raw_controller_spec.rb index 8b43d1264b2..ae9932174e8 100644 --- a/spec/controllers/projects/raw_controller_spec.rb +++ b/spec/controllers/projects/raw_controller_spec.rb @@ -65,7 +65,7 @@ describe Projects::RawController do it 'logs the event on auth.log' do attributes = { - message: 'Action_Rate_Limiter_Request', + message: 'Application_Rate_Limiter_Request', env: :raw_blob_request_limit, remote_ip: '0.0.0.0', request_method: 'GET', diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb index d16201fff5a..a1f9b98dc2c 100644 --- a/spec/controllers/projects_controller_spec.rb +++ b/spec/controllers/projects_controller_spec.rb @@ -1055,45 +1055,34 @@ describe ProjectsController do end end - describe '#export' do + describe 'project export' do before do sign_in(user) project.add_maintainer(user) end - context 'when project export is enabled' do - it 'returns 302' do - get :export, params: { namespace_id: project.namespace, id: project } - - expect(response).to have_gitlab_http_status(302) - end - end - - context 'when project export is disabled' do + shared_examples 'rate limits project export endpoint' do before do - stub_application_setting(project_export_enabled?: false) + allow(::Gitlab::ApplicationRateLimiter) + .to receive(:throttled?) + .and_return(true) end - it 'returns 404' do - get :export, params: { namespace_id: project.namespace, id: project } + it 'prevents requesting project export' do + get action, params: { namespace_id: project.namespace, id: project } - expect(response).to have_gitlab_http_status(404) + expect(flash[:alert]).to eq('This endpoint has been requested too many times. Try again later.') + expect(response).to have_gitlab_http_status(302) end end - end - describe '#download_export' do - before do - sign_in(user) + describe '#export' do + let(:action) { :export } - project.add_maintainer(user) - end - - context 'object storage enabled' do context 'when project export is enabled' do it 'returns 302' do - get :download_export, params: { namespace_id: project.namespace, id: project } + get action, params: { namespace_id: project.namespace, id: project } expect(response).to have_gitlab_http_status(302) end @@ -1105,66 +1094,96 @@ describe ProjectsController do end it 'returns 404' do - get :download_export, params: { namespace_id: project.namespace, id: project } + get action, params: { namespace_id: project.namespace, id: project } expect(response).to have_gitlab_http_status(404) end end + + context 'when the endpoint receives requests above the limit', :clean_gitlab_redis_cache do + include_examples 'rate limits project export endpoint' + end end - end - describe '#remove_export' do - before do - sign_in(user) + describe '#download_export' do + let(:action) { :download_export } - project.add_maintainer(user) - end + context 'object storage enabled' do + context 'when project export is enabled' do + it 'returns 302' do + get action, params: { namespace_id: project.namespace, id: project } - context 'when project export is enabled' do - it 'returns 302' do - post :remove_export, params: { namespace_id: project.namespace, id: project } + expect(response).to have_gitlab_http_status(302) + end + end - expect(response).to have_gitlab_http_status(302) - end - end + context 'when project export is disabled' do + before do + stub_application_setting(project_export_enabled?: false) + end - context 'when project export is disabled' do - before do - stub_application_setting(project_export_enabled?: false) - end + it 'returns 404' do + get action, params: { namespace_id: project.namespace, id: project } - it 'returns 404' do - post :remove_export, params: { namespace_id: project.namespace, id: project } + expect(response).to have_gitlab_http_status(404) + end + end - expect(response).to have_gitlab_http_status(404) + context 'when the endpoint receives requests above the limit', :clean_gitlab_redis_cache do + include_examples 'rate limits project export endpoint' + end end end - end - describe '#generate_new_export' do - before do - sign_in(user) + describe '#remove_export' do + let(:action) { :remove_export } - project.add_maintainer(user) - end + context 'when project export is enabled' do + it 'returns 302' do + post action, params: { namespace_id: project.namespace, id: project } - context 'when project export is enabled' do - it 'returns 302' do - post :generate_new_export, params: { namespace_id: project.namespace, id: project } + expect(response).to have_gitlab_http_status(302) + end + end - expect(response).to have_gitlab_http_status(302) + context 'when project export is disabled' do + before do + stub_application_setting(project_export_enabled?: false) + end + + it 'returns 404' do + post action, params: { namespace_id: project.namespace, id: project } + + expect(response).to have_gitlab_http_status(404) + end end end - context 'when project export is disabled' do - before do - stub_application_setting(project_export_enabled?: false) + describe '#generate_new_export' do + let(:action) { :generate_new_export } + + context 'when project export is enabled' do + it 'returns 302' do + post action, params: { namespace_id: project.namespace, id: project } + + expect(response).to have_gitlab_http_status(302) + end end - it 'returns 404' do - post :generate_new_export, params: { namespace_id: project.namespace, id: project } + context 'when project export is disabled' do + before do + stub_application_setting(project_export_enabled?: false) + end - expect(response).to have_gitlab_http_status(404) + it 'returns 404' do + post action, params: { namespace_id: project.namespace, id: project } + + expect(response).to have_gitlab_http_status(404) + end + end + + context 'when the endpoint receives requests above the limit', :clean_gitlab_redis_cache do + include_examples 'rate limits project export endpoint' end end end diff --git a/spec/features/snippets/internal_snippet_spec.rb b/spec/features/snippets/internal_snippet_spec.rb index 4ef3b0e5e7a..fd7ef71db15 100644 --- a/spec/features/snippets/internal_snippet_spec.rb +++ b/spec/features/snippets/internal_snippet_spec.rb @@ -5,6 +5,10 @@ require 'spec_helper' describe 'Internal Snippets', :js do let(:internal_snippet) { create(:personal_snippet, :internal) } + before do + stub_feature_flags(snippets_vue: false) + end + describe 'normal user' do before do sign_in(create(:user)) diff --git a/spec/features/snippets/notes_on_personal_snippets_spec.rb b/spec/features/snippets/notes_on_personal_snippets_spec.rb index 2bd01be25e9..57264f97ddc 100644 --- a/spec/features/snippets/notes_on_personal_snippets_spec.rb +++ b/spec/features/snippets/notes_on_personal_snippets_spec.rb @@ -16,6 +16,7 @@ describe 'Comments on personal snippets', :js do let!(:other_note) { create(:note_on_personal_snippet) } before do + stub_feature_flags(snippets_vue: false) sign_in user visit snippet_path(snippet) diff --git a/spec/features/snippets/private_snippets_spec.rb b/spec/features/snippets/private_snippets_spec.rb index 9df4cd01103..37f45f22a27 100644 --- a/spec/features/snippets/private_snippets_spec.rb +++ b/spec/features/snippets/private_snippets_spec.rb @@ -6,6 +6,7 @@ describe 'Private Snippets', :js do let(:user) { create(:user) } before do + stub_feature_flags(snippets_vue: false) sign_in(user) end diff --git a/spec/features/snippets/public_snippets_spec.rb b/spec/features/snippets/public_snippets_spec.rb index 82edda509c2..045afcf1c12 100644 --- a/spec/features/snippets/public_snippets_spec.rb +++ b/spec/features/snippets/public_snippets_spec.rb @@ -3,6 +3,10 @@ require 'spec_helper' describe 'Public Snippets', :js do + before do + stub_feature_flags(snippets_vue: false) + end + it 'Unauthenticated user should see public snippets' do public_snippet = create(:personal_snippet, :public) diff --git a/spec/features/snippets/show_spec.rb b/spec/features/snippets/show_spec.rb index 450e520e293..9c686be012b 100644 --- a/spec/features/snippets/show_spec.rb +++ b/spec/features/snippets/show_spec.rb @@ -6,6 +6,10 @@ describe 'Snippet', :js do let(:project) { create(:project, :repository) } let(:snippet) { create(:personal_snippet, :public, file_name: file_name, content: content) } + before do + stub_feature_flags(snippets_vue: false) + end + context 'Ruby file' do let(:file_name) { 'popen.rb' } let(:content) { project.repository.blob_at('master', 'files/ruby/popen.rb').data } diff --git a/spec/features/snippets/spam_snippets_spec.rb b/spec/features/snippets/spam_snippets_spec.rb index 3e71a4e7879..0c3ca6f17c8 100644 --- a/spec/features/snippets/spam_snippets_spec.rb +++ b/spec/features/snippets/spam_snippets_spec.rb @@ -7,6 +7,7 @@ describe 'User creates snippet', :js do before do stub_feature_flags(allow_possible_spam: false) + stub_feature_flags(snippets_vue: false) stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false') Gitlab::CurrentSettings.update!( diff --git a/spec/features/snippets/user_creates_snippet_spec.rb b/spec/features/snippets/user_creates_snippet_spec.rb index 9a141dd463a..b373264bbe4 100644 --- a/spec/features/snippets/user_creates_snippet_spec.rb +++ b/spec/features/snippets/user_creates_snippet_spec.rb @@ -8,6 +8,7 @@ describe 'User creates snippet', :js do let(:user) { create(:user) } before do + stub_feature_flags(snippets_vue: false) sign_in(user) visit new_snippet_path end diff --git a/spec/features/snippets/user_deletes_snippet_spec.rb b/spec/features/snippets/user_deletes_snippet_spec.rb index 217419a220a..35619b92561 100644 --- a/spec/features/snippets/user_deletes_snippet_spec.rb +++ b/spec/features/snippets/user_deletes_snippet_spec.rb @@ -10,6 +10,8 @@ describe 'User deletes snippet' do before do sign_in(user) + stub_feature_flags(snippets_vue: false) + visit snippet_path(snippet) end diff --git a/spec/features/snippets/user_edits_snippet_spec.rb b/spec/features/snippets/user_edits_snippet_spec.rb index 51d9baf44bc..1d26660a4f6 100644 --- a/spec/features/snippets/user_edits_snippet_spec.rb +++ b/spec/features/snippets/user_edits_snippet_spec.rb @@ -12,6 +12,7 @@ describe 'User edits snippet', :js do let(:snippet) { create(:personal_snippet, :public, file_name: file_name, content: content, author: user) } before do + stub_feature_flags(snippets_vue: false) sign_in(user) visit edit_snippet_path(snippet) diff --git a/spec/features/snippets_spec.rb b/spec/features/snippets_spec.rb index 9df6fe7d16b..bc7fa161e87 100644 --- a/spec/features/snippets_spec.rb +++ b/spec/features/snippets_spec.rb @@ -6,11 +6,38 @@ describe 'Snippets' do context 'when the project has snippets' do let(:project) { create(:project, :public) } let!(:snippets) { create_list(:project_snippet, 2, :public, author: project.owner, project: project) } + before do allow(Snippet).to receive(:default_per_page).and_return(1) - visit snippets_path(username: project.owner.username) + + visit project_snippets_path(project) end it_behaves_like 'paginated snippets' end + + describe 'rendering engine' do + let_it_be(:snippet) { create(:personal_snippet, :public) } + let(:snippets_vue_feature_flag_enabled) { true } + + before do + stub_feature_flags(snippets_vue: snippets_vue_feature_flag_enabled) + + visit snippet_path(snippet) + end + + it 'renders Vue application' do + expect(page).to have_selector('#js-snippet-view') + expect(page).not_to have_selector('.personal-snippets') + end + + context 'when feature flag is disabled' do + let(:snippets_vue_feature_flag_enabled) { false } + + it 'renders HAML application and not Vue' do + expect(page).not_to have_selector('#js-snippet-view') + expect(page).to have_selector('.personal-snippets') + end + end + end end diff --git a/spec/lib/gitlab/action_rate_limiter_spec.rb b/spec/lib/gitlab/application_rate_limiter_spec.rb index 8b510a475d2..f1a0163d91c 100644 --- a/spec/lib/gitlab/action_rate_limiter_spec.rb +++ b/spec/lib/gitlab/application_rate_limiter_spec.rb @@ -2,30 +2,40 @@ require 'spec_helper' -describe Gitlab::ActionRateLimiter, :clean_gitlab_redis_cache do +describe Gitlab::ApplicationRateLimiter, :clean_gitlab_redis_cache do let(:redis) { double('redis') } let(:user) { create(:user) } let(:project) { create(:project) } + let(:rate_limits) do + { + test_action: { + threshold: 1, + interval: 2.minutes + } + } + end + let(:key) { rate_limits.keys[0] } - subject { described_class.new(action: :test_action, expiry_time: 100) } + subject { described_class } before do allow(Gitlab::Redis::Cache).to receive(:with).and_yield(redis) + allow(described_class).to receive(:rate_limits).and_return(rate_limits) end shared_examples 'action rate limiter' do it 'increases the throttle count and sets the expiration time' do expect(redis).to receive(:incr).with(cache_key).and_return(1) - expect(redis).to receive(:expire).with(cache_key, 100) + expect(redis).to receive(:expire).with(cache_key, 120) - expect(subject.throttled?(key, 1)).to be_falsy + expect(subject.throttled?(key, scope: scope)).to be_falsy end it 'returns true if the key is throttled' do expect(redis).to receive(:incr).with(cache_key).and_return(2) expect(redis).not_to receive(:expire) - expect(subject.throttled?(key, 1)).to be_truthy + expect(subject.throttled?(key, scope: scope)).to be_truthy end context 'when throttling is disabled' do @@ -33,16 +43,16 @@ describe Gitlab::ActionRateLimiter, :clean_gitlab_redis_cache do expect(redis).not_to receive(:incr) expect(redis).not_to receive(:expire) - expect(subject.throttled?(key, 0)).to be_falsy + expect(subject.throttled?(key, scope: scope, threshold: 0)).to be_falsy end end end context 'when the key is an array of only ActiveRecord models' do - let(:key) { [user, project] } + let(:scope) { [user, project] } let(:cache_key) do - "action_rate_limiter:test_action:user:#{user.id}:project:#{project.id}" + "application_rate_limiter:test_action:user:#{user.id}:project:#{project.id}" end it_behaves_like 'action rate limiter' @@ -52,10 +62,10 @@ describe Gitlab::ActionRateLimiter, :clean_gitlab_redis_cache do let(:project) { create(:project, :public, :repository) } let(:commit) { project.repository.commit } let(:path) { 'app/controllers/groups_controller.rb' } - let(:key) { [project, commit, path] } + let(:scope) { [project, commit, path] } let(:cache_key) do - "action_rate_limiter:test_action:project:#{project.id}:commit:#{commit.sha}:#{path}" + "application_rate_limiter:test_action:project:#{project.id}:commit:#{commit.sha}:#{path}" end it_behaves_like 'action rate limiter' @@ -72,7 +82,7 @@ describe Gitlab::ActionRateLimiter, :clean_gitlab_redis_cache do let(:base_attributes) do { - message: 'Action_Rate_Limiter_Request', + message: 'Application_Rate_Limiter_Request', env: type, remote_ip: '127.0.0.1', request_method: 'GET', diff --git a/spec/requests/api/project_export_spec.rb b/spec/requests/api/project_export_spec.rb index 605ff888234..37f2cc85a50 100644 --- a/spec/requests/api/project_export_spec.rb +++ b/spec/requests/api/project_export_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe API::ProjectExport do +describe API::ProjectExport, :clean_gitlab_redis_cache do set(:project) { create(:project) } set(:project_none) { create(:project) } set(:project_started) { create(:project) } @@ -47,6 +47,19 @@ describe API::ProjectExport do it_behaves_like '404 response' end + shared_examples_for 'when rate limit is exceeded' do + before do + allow(::Gitlab::ApplicationRateLimiter).to receive(:throttled?).and_return(true) + end + + it 'prevents requesting project export' do + request + + expect(response).to have_gitlab_http_status(429) + expect(json_response['message']['error']).to eq('This endpoint has been requested too many times. Try again later.') + end + end + describe 'GET /projects/:project_id/export' do shared_examples_for 'get project export status not found' do it_behaves_like '404 response' do @@ -219,6 +232,12 @@ describe API::ProjectExport do let(:user) { admin } it_behaves_like 'get project download by strategy' + + context 'when rate limit is exceeded' do + let(:request) { get api(download_path, admin) } + + include_examples 'when rate limit is exceeded' + end end context 'when user is a maintainer' do @@ -329,6 +348,12 @@ describe API::ProjectExport do let(:user) { admin } it_behaves_like 'post project export start' + + context 'when rate limit is exceeded' do + let(:request) { post api(path, admin) } + + include_examples 'when rate limit is exceeded' + end end context 'when user is a maintainer' do |