diff options
| -rw-r--r-- | CHANGELOG | 1 | ||||
| -rw-r--r-- | CONTRIBUTING.md | 5 | ||||
| -rw-r--r-- | Gemfile | 2 | ||||
| -rw-r--r-- | Gemfile.lock | 4 | ||||
| -rw-r--r-- | doc_styleguide.md | 22 | ||||
| -rw-r--r-- | lib/gitlab/backend/grack_auth.rb | 1 | ||||
| -rw-r--r-- | lib/gitlab/backend/rack_attack_helpers.rb | 31 | ||||
| -rw-r--r-- | spec/lib/gitlab/backend/grack_auth_spec.rb | 2 | ||||
| -rw-r--r-- | spec/lib/gitlab/backend/rack_attack_helpers_spec.rb | 35 |
9 files changed, 31 insertions, 72 deletions
diff --git a/CHANGELOG b/CHANGELOG index d1ba75a0dc5..a168342fceb 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -4,6 +4,7 @@ v 7.12.0 (unreleased) - Refactor permission checks with issues and merge requests project settings (Stan Hu) - Add web hook support for note events (Stan Hu) - Disable "New Issue" and "New Merge Request" buttons when features are disabled in project settings (Stan Hu) + - Remove Rack Attack monkey patches and bump to version 4.3.0 (Stan Hu) - Allow to configure location of the `.gitlab_shell_secret` file. (Jakub Jirutka) - Disabled expansion of top/bottom blobs for new file diffs - Update Asciidoctor gem to version 1.5.2. (Jakub Jirutka) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 949493a930d..8059b95609a 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -86,7 +86,9 @@ If you can, please submit a merge request with the fix or improvements including 1. If your MR touches code that executes shell commands, make sure it adheres to the [shell command guidelines]( doc/development/shell_commands.md). 1. Also have a look at the [shell command guidelines](doc/development/shell_commands.md) if your code reads or opens files, or handles paths to files on disk. -The **official merge window** is in the beginning of the month from the 1st to the 7th day of the month. The best time to submit a MR and get feedback fast. Before this time the GitLab B.V. team is still dealing with work that is created by the monthly release such as assisting subscribers with upgrade issues, the release of Enterprise Edition and the upgrade of GitLab Cloud. After the 7th it is already getting closer to the release date of the next version. This means there is less time to fix the issues created by merging large new features. +The **official merge window** is in the beginning of the month from the 1st to the 7th day of the month. The best time to submit a MR and get feedback fast. +Before this time the GitLab B.V. team is still dealing with work that is created by the monthly release such as regressions requiring patch releases. +After the 7th it is already getting closer to the release date of the next version. This means there is less time to fix the issues created by merging large new features. Please keep the change in a single MR **as small as possible**. If you want to contribute a large feature think very hard what the minimum viable change is. Can you split functionality? Can you only submit the backend/API code? Can you start with a very simple UI? Can you do part of the refactor? The increased reviewability of small MR's that leads to higher code quality is more important to us than having a minimal commit log. The smaller a MR is the more likely it is it will be merged (quickly), after that you can send more MR's to enhance it. @@ -161,6 +163,7 @@ If you add a dependency in GitLab (such as an operating system package) please c 1. [Shell commands](doc/development/shell_commands.md) created by GitLab contributors to enhance security 1. [Markdown](http://www.cirosantilli.com/markdown-styleguide) 1. [Database Migrations](doc/development/migration_style_guide.md) +1. [Documentation styleguide](doc_styleguide.md) 1. Interface text should be written subjectively instead of objectively. It should be the gitlab core team addressing a person. It should be written in present time and never use past tense (has been/was). For example instead of "prohibited this user from being saved due to the following errors:" the text should be "sorry, we could not create your account because:". Also these [excellent writing guidelines](https://github.com/NARKOZ/guides#writing). This is also the style used by linting tools such as [RuboCop](https://github.com/bbatsov/rubocop), [PullReview](https://www.pullreview.com/) and [Hound CI](https://houndci.com). @@ -172,7 +172,7 @@ gem "underscore-rails", "~> 1.4.4" gem "sanitize", '~> 2.0' # Protect against bruteforcing -gem "rack-attack" +gem "rack-attack", '~> 4.3.0' # Ace editor gem 'ace-rails-ap' diff --git a/Gemfile.lock b/Gemfile.lock index 529131f09b0..4aa56cc7a9a 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -421,7 +421,7 @@ GEM rack (1.5.2) rack-accept (0.4.5) rack (>= 0.4) - rack-attack (4.2.0) + rack-attack (4.3.0) rack rack-cors (0.2.9) rack-mini-profiler (0.9.0) @@ -764,7 +764,7 @@ DEPENDENCIES poltergeist (~> 1.5.1) pry-rails quiet_assets (~> 1.0.1) - rack-attack + rack-attack (~> 4.3.0) rack-cors rack-mini-profiler rack-oauth2 (~> 1.0.5) diff --git a/doc_styleguide.md b/doc_styleguide.md new file mode 100644 index 00000000000..670af765f3a --- /dev/null +++ b/doc_styleguide.md @@ -0,0 +1,22 @@ +# Documentation styleguide + +This styleguide recommends best practices to improve documentation and to keep it organized and easy to find. + +## Text + +* Make sure that the documentation is added in the correct directory and that there's a link to it somewhere useful. + +* Add only one H1 or title in each document, by adding '#' at the begining of it (when using markdown). For subtitles, use '##', '###' and so on. + +* Do not duplicate information. + +* Be brief and clear. + + +## When adding images to a document + +* Create a directory to store the images with the specific name of the document where the images belong. It could be in the same directory where the .md document that you're working on is located. + +* Images should have a specific, non-generic name that will differentiate them. + +* Keep all file names in lower case.
\ No newline at end of file diff --git a/lib/gitlab/backend/grack_auth.rb b/lib/gitlab/backend/grack_auth.rb index 050b5ba29dd..03cef30c97d 100644 --- a/lib/gitlab/backend/grack_auth.rb +++ b/lib/gitlab/backend/grack_auth.rb @@ -1,4 +1,3 @@ -require_relative 'rack_attack_helpers' require_relative 'shell_env' module Grack diff --git a/lib/gitlab/backend/rack_attack_helpers.rb b/lib/gitlab/backend/rack_attack_helpers.rb deleted file mode 100644 index 8538f3f6eca..00000000000 --- a/lib/gitlab/backend/rack_attack_helpers.rb +++ /dev/null @@ -1,31 +0,0 @@ -# rack-attack v4.2.0 doesn't yet support clearing of keys. -# Taken from https://github.com/kickstarter/rack-attack/issues/113 -class Rack::Attack::Allow2Ban - def self.reset(discriminator, options) - findtime = options[:findtime] or raise ArgumentError, "Must pass findtime option" - - cache.reset_count("#{key_prefix}:count:#{discriminator}", findtime) - cache.delete("#{key_prefix}:ban:#{discriminator}") - end -end - -class Rack::Attack::Cache - def reset_count(unprefixed_key, period) - epoch_time = Time.now.to_i - # Add 1 to expires_in to avoid timing error: http://git.io/i1PHXA - expires_in = period - (epoch_time % period) + 1 - key = "#{(epoch_time / period).to_i}:#{unprefixed_key}" - delete(key) - end - - def delete(unprefixed_key) - store.delete("#{prefix}:#{unprefixed_key}") - end -end - -class Rack::Attack::StoreProxy::RedisStoreProxy - def delete(key, options={}) - self.del(key) - rescue Redis::BaseError - end -end diff --git a/spec/lib/gitlab/backend/grack_auth_spec.rb b/spec/lib/gitlab/backend/grack_auth_spec.rb index d0aad54f677..42c9946d2a9 100644 --- a/spec/lib/gitlab/backend/grack_auth_spec.rb +++ b/spec/lib/gitlab/backend/grack_auth_spec.rb @@ -156,7 +156,7 @@ describe Grack::Auth do end expect(attempt_login(true)).to eq(200) - expect(Rack::Attack::Allow2Ban.send(:banned?, ip)).to eq(nil) + expect(Rack::Attack::Allow2Ban.banned?(ip)).to be_falsey for n in 0..maxretry do expect(attempt_login(false)).to eq(401) diff --git a/spec/lib/gitlab/backend/rack_attack_helpers_spec.rb b/spec/lib/gitlab/backend/rack_attack_helpers_spec.rb deleted file mode 100644 index 2ac496fd669..00000000000 --- a/spec/lib/gitlab/backend/rack_attack_helpers_spec.rb +++ /dev/null @@ -1,35 +0,0 @@ -require "spec_helper" - -describe 'RackAttackHelpers' do - describe 'reset' do - let(:discriminator) { 'test-key'} - let(:maxretry) { 5 } - let(:period) { 1.minute } - let(:options) { { findtime: period, bantime: 60, maxretry: maxretry } } - - def do_filter - for i in 1..maxretry - 1 do - status = Rack::Attack::Allow2Ban.filter(discriminator, options) { true } - expect(status).to eq(false) - end - end - - def do_reset - Rack::Attack::Allow2Ban.reset(discriminator, options) - end - - before do - do_reset - end - - after do - do_reset - end - - it 'user is not banned after n - 1 retries' do - do_filter - do_reset - do_filter - end - end -end |