Update rake-attack docs. Handle case when relative path is set.

README.md

@@ -52,10 +52,6 @@ See [doc/settings/configuration.md](doc/settings/configuration.md#storing-git-da
 See [doc/settings/configuration.md](doc/settings/configuration.md#changing-the-name-of-the-git-user-group).
-##### Configuring Rake attack
-
-See [doc/settings/configuration.md](doc/settings/configuration.md#enablingdisabling-rake-attack-and-setting-up-basic-auth-throttling).
-
 ##### Setting up LDAP sign-in
 See [doc/settings/ldap.md](doc/settings/ldap.md).

doc/README.md

@@ -27,6 +27,7 @@ Omnibus is a way to package different services and tools required to run GitLab,
 - [Only start omnibus-gitlab services after a given filesystem is mounted](settings/configuration.md#only-start-omnibus-gitlab-services-after-a-given-filesystem-is-mounted)
 - [Disable user and group account management](settings/configuration.html#disable-user-and-group-account-management)
 - [Disable storage directory management](settings/configuration.html#disable-storage-directories-management)
+- [Configuring Rake attack](doc/settings/configuration.md#configuring-rake-attack)
 - [SMTP](settings/smtp.md)
 - [NGINX](settings/nginx.md)
 - [LDAP](settings/ldap.md)

doc/settings/configuration.md

@@ -370,15 +370,16 @@ gitlab_rails['rack_attack_git_basic_auth'] = {
 }
 ```
-### Setting up paths to be protected
-If you want to change default paths to be protected
-set `gitlab_rails['rack_attack_paths_to_be_protected']` in config file.
-Default list is:
+### Setting up paths to be protected by rake attack
+If you want to change default protected paths 
+set `gitlab_rails['rack_attack_protected_paths']` in config file. 
+**Warning** This action will overwrite 
+list provided by omnibus-gitlab:
 ```ruby
-gitlab_rails['rack_attack_paths_to_be_protected'] = [
+gitlab_rails['rack_attack_protected_paths'] = [
   '/users/password',
   '/users/sign_in',
   '/api/#{API::API.version}/session.json',
@@ -391,10 +392,13 @@ gitlab_rails['rack_attack_paths_to_be_protected'] = [
 ```
 _**Note:** All paths are relative to the gitlab url._
-**Warning** If path contains variable/s which need to be
-interpolated by rails(ex "#{API::API.version}")
-then you need to escape curly brackets or use single quated string.
+Do not include [relative URL](configuration.md#configuring-a-relative-url-for-gitlab) if you set it up.
+
+**Warning** If path contains variables which need to be
+interpolated by rails(ex. `#{API::API.version}`)
+then you need to escape curly brackets or use single quoted string.
+For example `"/api/#\{API::API.version\}/session.json"` or `'/api/#{API::API.version}/session.json'`
 ### Setting up throttling for 'paths to be protected' 
 Use next options to control throttling 'limit' and 'period':

files/gitlab-config-template/gitlab.rb.template

@@ -247,7 +247,7 @@ external_url 'GENERATED_EXTERNAL_URL'
 #   'bantime' => 3600
 # }
-# gitlab_rails['rack_attack_paths_to_be_protected'] = [
+# gitlab_rails['rack_attack_protected_paths'] = [
 #   '/users/password',
 #   '/users/sign_in',
 #   '/api/#{API::API.version}/session.json',

files/gitlab-cookbooks/gitlab/attributes/default.rb

@@ -208,7 +208,7 @@ default['gitlab']['gitlab-rails']['extra_google_analytics_id'] = nil
 default['gitlab']['gitlab-rails']['extra_piwik_url'] = nil
 default['gitlab']['gitlab-rails']['extra_piwik_site_id'] = nil
 default['gitlab']['gitlab-rails']['rack_attack_git_basic_auth'] = nil
-default['gitlab']['gitlab-rails']['rack_attack_paths_to_be_protected'] = [
+default['gitlab']['gitlab-rails']['rack_attack_protected_paths'] = [
   '/users/password',
   '/users/sign_in',
   '/api/#{API::API.version}/session.json',

files/gitlab-cookbooks/gitlab/libraries/gitlab_rails.rb

@@ -23,7 +23,7 @@ module GitlabRails
       parse_external_url
       parse_directories
       parse_gitlab_trusted_proxies
-      parse_rack_attack_paths_to_be_protected
+      parse_rack_attack_protected_paths
     end
     def parse_directories
@@ -92,11 +92,27 @@ module GitlabRails
       Gitlab['gitlab_rails']['trusted_proxies'] ||= Gitlab['nginx']['real_ip_trusted_addresses']
     end
-    def parse_rack_attack_paths_to_be_protected
-      return unless Gitlab['gitlab_rails']['rack_attack_paths_to_be_protected']
-      Gitlab['gitlab_rails']['rack_attack_paths_to_be_protected'].map! do |path|
+    def parse_rack_attack_protected_paths
+      # Fixing common user's input mistakes for rake attack protected paths
+      return unless Gitlab['gitlab_rails']['rack_attack_protected_paths']
+
+      # append leading slash if missing
+      Gitlab['gitlab_rails']['rack_attack_protected_paths'].map! do |path|
         path.start_with?('/') ? path : '/' + path
       end
+
+      # append urls to the list but without relative_url
+      if Gitlab['gitlab_rails']['gitlab_relative_url']
+        paths_without_relative_url = []
+        Gitlab['gitlab_rails']['rack_attack_protected_paths'].each do |path| 
+          if path.start_with?(Gitlab['gitlab_rails']['gitlab_relative_url'] + '/')
+            stripped_path = path.sub(Gitlab['gitlab_rails']['gitlab_relative_url'], '')
+            paths_without_relative_url.push(stripped_path)
+          end
+        end
+        Gitlab['gitlab_rails']['rack_attack_protected_paths'].concat(paths_without_relative_url)
+      end
+
     end
     def disable_gitlab_rails_services

files/gitlab-cookbooks/gitlab/templates/default/rack_attack.rb.erb

@@ -7,7 +7,7 @@
 #
 paths_to_be_protected = [
-  <% @rack_attack_paths_to_be_protected.each do |path| %>
+  <% @rack_attack_protected_paths.each do |path| %>
   "#{Rails.application.config.relative_url_root}<%= path %>",
   <% end %>
 ]

spec/chef/recipes/rake-attack_spec.rb

@@ -5,22 +5,28 @@ describe 'rake-attack' do
   before { allow(Gitlab).to receive(:[]).and_call_original }
-  context 'when rack_attack_paths_to_be_protected is set' do
+  context 'when rack_attack_protected_paths is set' do
     it 'adds leading slashes' do
-      stub_gitlab_rb(gitlab_rails: { rack_attack_paths_to_be_protected: ['admin/', 'users/password'] })
-      expect(chef_run.node['gitlab']['gitlab-rails']['rack_attack_paths_to_be_protected'])
+      stub_gitlab_rb(gitlab_rails: { rack_attack_protected_paths: ['admin/', 'users/password'] })
+      expect(chef_run.node['gitlab']['gitlab-rails']['rack_attack_protected_paths'])
         .to eql(['/admin/', '/users/password'])
     end
     it 'does not add additional slashes' do
-      stub_gitlab_rb(gitlab_rails: { rack_attack_paths_to_be_protected: ['/admin/', '/users/password'] })
-      expect(chef_run.node['gitlab']['gitlab-rails']['rack_attack_paths_to_be_protected'])
+      stub_gitlab_rb(gitlab_rails: { rack_attack_protected_paths: ['/admin/', '/users/password'] })
+      expect(chef_run.node['gitlab']['gitlab-rails']['rack_attack_protected_paths'])
         .to eql(['/admin/', '/users/password'])
     end
+    it 'can contain variables in path' do
+      stub_gitlab_rb(gitlab_rails: { rack_attack_protected_paths: ['/api/#{API::API.version}/session', "/api/#\{API::API.version\}/session.json"] })
+      expect(chef_run.node['gitlab']['gitlab-rails']['rack_attack_protected_paths'])
+        .to eql(['/api/#{API::API.version}/session', '/api/#{API::API.version}/session.json'])
+    end
+
     it 'creates rack_attack config file with user defined list' do
       rack_attack_config = '/var/opt/gitlab/gitlab-rails/etc/rack_attack.rb'
-      stub_gitlab_rb(gitlab_rails: { rack_attack_paths_to_be_protected: ['/admin/', '/users/password'] })
+      stub_gitlab_rb(gitlab_rails: { rack_attack_protected_paths: ['/admin/', '/users/password'] })
       expect(chef_run).to create_template(rack_attack_config)
@@ -32,25 +38,55 @@ describe 'rake-attack' do
   end
-  context 'when rack_attack_paths_to_be_protected is not set' do
-    default_paths_to_be_protected = ['/users/password',
-                                     '/users/sign_in',
-                                     '/api/#{API::API.version}/session.json',
-                                     '/api/#{API::API.version}/session',
-                                     '/users',
-                                     '/users/confirmation',
-                                     '/unsubscribes/',
-                                     '/import/github/personal_access_token'
-                                    ]
+  context 'when rack_attack_protected_paths and relative_url_root are set' do
+    it 'adds paths without relative_url' do
+      stub_gitlab_rb(gitlab_rails: { rack_attack_protected_paths: ['/profile/keys', '/profile/users/password'] }, 
+                     external_url: 'https://example.com/profile' # crazy idea for relative url
+                    )
+      expect(chef_run.node['gitlab']['gitlab-rails']['rack_attack_protected_paths'])
+        .to eql(['/profile/keys', '/profile/users/password', '/keys', '/users/password'])
+    end
+
+    it 'does not add additional paths' do
+      stub_gitlab_rb(gitlab_rails: { rack_attack_protected_paths: ['/admin/', '/users/password'] }, 
+                     external_url: 'https://example.com/gitlab'
+                    )
+      expect(chef_run.node['gitlab']['gitlab-rails']['rack_attack_protected_paths'])
+        .to eql(['/admin/', '/users/password'])
+    end
+
+    it 'adds paths without relative_url for multi-level relative_url' do
+      stub_gitlab_rb(gitlab_rails: { rack_attack_protected_paths: ['/hosting/admin/', '/hosting/gitlab/admin/'] }, 
+                     external_url: 'https://example.com/hosting/gitlab'
+                    )
+      expect(chef_run.node['gitlab']['gitlab-rails']['rack_attack_protected_paths'])
+        .to eql(['/hosting/admin/', '/hosting/gitlab/admin/', '/admin/'])
+    end
+
+
+  end
+
+  context 'when rack_attack_protected_paths is not set' do
+    default_protected_paths = ['/users/password',
+                               '/users/sign_in',
+                               '/api/#{API::API.version}/session.json',
+                               '/api/#{API::API.version}/session',
+                               '/users',
+                               '/users/confirmation',
+                               '/unsubscribes/',
+                               '/import/github/personal_access_token'
+                              ]
     it 'uses default list' do
-      expect(chef_run.node['gitlab']['gitlab-rails']['rack_attack_paths_to_be_protected'])
-        .to eql(default_paths_to_be_protected)
+      stub_gitlab_rb(gitlab_rails: { rack_attack_protected_paths: nil })
+      expect(chef_run.node['gitlab']['gitlab-rails']['rack_attack_protected_paths'])
+        .to eql(default_protected_paths)
     end
     it 'creates rack_attack config file with default list' do
+      stub_gitlab_rb(gitlab_rails: { rack_attack_protected_paths: nil })
       rack_attack_config = '/var/opt/gitlab/gitlab-rails/etc/rack_attack.rb'
       expect(chef_run).to create_template(rack_attack_config)
-      default_paths_to_be_protected.each do |path|
+      default_protected_paths.each do |path|
         expect(chef_run).to render_file(rack_attack_config)
           .with_content(/#\{Rails.application.config.relative_url_root\}#{path}/)
@@ -59,4 +95,6 @@ describe 'rake-attack' do
     end
   end
+  
+
 end