Merge branch 'container-registry-token-ttl' into 'master'

Add Application Setting to configure Container Registry token expire delay (default 5min)

This adds an option to configure Container Registry token expire delay. The default is set to 5mins (something that is also used by Docker Hub).

What is left:
* [x] Write test to check the expire_delay

Fixes: https://gitlab.com/gitlab-org/gitlab-ce/issues/17890

@stanhu I think that this should land in patch release of 8.8.

See merge request !4364

11 files changed, 80 insertions, 27 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 98e24dc63b1..acd42353f46 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -17,6 +17,7 @@ v 8.9.0 (unreleased)
   - Projects pending deletion will render a 404 page
   - Measure queue duration between gitlab-workhorse and Rails
   - Make authentication service for Container Registry to be compatible with < Docker 1.11
+  - Add Application Setting to configure Container Registry token expire delay (default 5min)
 
 v 8.8.3
   - Fix gitlab importer failing to import new projects due to missing credentials
diff --git a/app/controllers/admin/application_settings_controller.rb b/app/controllers/admin/application_settings_controller.rb
index ff7a5cad2fb..0a34a12e2a7 100644
--- a/app/controllers/admin/application_settings_controller.rb
+++ b/app/controllers/admin/application_settings_controller.rb
@@ -107,6 +107,7 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController
       :repository_checks_enabled,
       :metrics_packet_size,
       :send_user_confirmation_email,
+      :container_registry_token_expire_delay,
       restricted_visibility_levels: [],
       import_sources: [],
       disabled_oauth_sign_in_sources: []
diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb
index 9a14954b4a7..fc48307b75d 100644
--- a/app/models/application_setting.rb
+++ b/app/models/application_setting.rb
@@ -51,6 +51,10 @@ class ApplicationSetting < ActiveRecord::Base
             presence: true,
             numericality: { only_integer: true, greater_than: 0 }
 
+  validates :container_registry_token_expire_delay,
+            presence: true,
+            numericality: { only_integer: true, greater_than: 0 }
+
   validates_each :restricted_visibility_levels do |record, attr, value|
     unless value.nil?
       value.each do |level|
@@ -121,7 +125,8 @@ class ApplicationSetting < ActiveRecord::Base
       akismet_enabled: false,
       repository_checks_enabled: true,
       disabled_oauth_sign_in_sources: [],
-      send_user_confirmation_email: false
+      send_user_confirmation_email: false,
+      container_registry_token_expire_delay: 5,
     )
   end
 
diff --git a/app/services/auth/container_registry_authentication_service.rb b/app/services/auth/container_registry_authentication_service.rb
index 5090bd8f6e6..e57b95f21ec 100644
--- a/app/services/auth/container_registry_authentication_service.rb
+++ b/app/services/auth/container_registry_authentication_service.rb
@@ -1,5 +1,7 @@
 module Auth
   class ContainerRegistryAuthenticationService < BaseService
+    include Gitlab::CurrentSettings
+
     AUDIENCE = 'container_registry'
 
     def execute
@@ -17,6 +19,7 @@ module Auth
       token = JSONWebToken::RSAToken.new(registry.key)
       token.issuer = registry.issuer
       token.audience = AUDIENCE
+      token.expire_time = token_expire_at
       token[:access] = names.map do |name|
         { type: 'repository', name: name, actions: %w(*) }
       end
@@ -30,6 +33,7 @@ module Auth
       token.issuer = registry.issuer
       token.audience = params[:service]
       token.subject = current_user.try(:username)
+      token.expire_time = ContainerRegistryAuthenticationService.token_expire_at
       token[:access] = accesses.compact
       token
     end
@@ -75,5 +79,9 @@ module Auth
     def registry
       Gitlab.config.registry
     end
+
+    def self.token_expire_at
+      Time.now + current_application_settings.container_registry_token_expire_delay.minutes
+    end
   end
 end
diff --git a/app/views/admin/application_settings/_form.html.haml b/app/views/admin/application_settings/_form.html.haml
index df286852b97..f149f9eb431 100644
--- a/app/views/admin/application_settings/_form.html.haml
+++ b/app/views/admin/application_settings/_form.html.haml
@@ -178,6 +178,14 @@
       .col-sm-10
         = f.number_field :max_artifacts_size, class: 'form-control'
 
+  - if Gitlab.config.registry.enabled
+    %fieldset
+      %legend Container Registry
+      .form-group
+        = f.label :container_registry_token_expire_delay, 'Authorization token duration (minutes)', class: 'control-label col-sm-2'
+        .col-sm-10
+          = f.number_field :container_registry_token_expire_delay, class: 'form-control'
+
   %fieldset
     %legend Metrics
     %p
diff --git a/db/migrate/20160530150109_add_container_registry_token_expire_delay_to_application_settings.rb b/db/migrate/20160530150109_add_container_registry_token_expire_delay_to_application_settings.rb
new file mode 100644
index 00000000000..e21376bd571
--- /dev/null
+++ b/db/migrate/20160530150109_add_container_registry_token_expire_delay_to_application_settings.rb
@@ -0,0 +1,9 @@
+# This is ONLINE migration
+
+class AddContainerRegistryTokenExpireDelayToApplicationSettings < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  def change
+    add_column :application_settings, :container_registry_token_expire_delay, :integer, default: 5
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index c7eeaadb131..b2af810f600 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20160528043124) do
+ActiveRecord::Schema.define(version: 20160530150109) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -43,46 +43,47 @@ ActiveRecord::Schema.define(version: 20160528043124) do
     t.datetime "created_at"
     t.datetime "updated_at"
     t.string   "home_page_url"
-    t.integer  "default_branch_protection",         default: 2
+    t.integer  "default_branch_protection",             default: 2
     t.text     "restricted_visibility_levels"
-    t.boolean  "version_check_enabled",             default: true
-    t.integer  "max_attachment_size",               default: 10,          null: false
+    t.boolean  "version_check_enabled",                 default: true
+    t.integer  "max_attachment_size",                   default: 10,          null: false
     t.integer  "default_project_visibility"
     t.integer  "default_snippet_visibility"
     t.text     "restricted_signup_domains"
-    t.boolean  "user_oauth_applications",           default: true
+    t.boolean  "user_oauth_applications",               default: true
     t.string   "after_sign_out_path"
-    t.integer  "session_expire_delay",              default: 10080,       null: false
+    t.integer  "session_expire_delay",                  default: 10080,       null: false
     t.text     "import_sources"
     t.text     "help_page_text"
     t.string   "admin_notification_email"
-    t.boolean  "shared_runners_enabled",            default: true,        null: false
-    t.integer  "max_artifacts_size",                default: 100,         null: false
+    t.boolean  "shared_runners_enabled",                default: true,        null: false
+    t.integer  "max_artifacts_size",                    default: 100,         null: false
     t.string   "runners_registration_token"
-    t.boolean  "require_two_factor_authentication", default: false
-    t.integer  "two_factor_grace_period",           default: 48
-    t.boolean  "metrics_enabled",                   default: false
-    t.string   "metrics_host",                      default: "localhost"
-    t.integer  "metrics_pool_size",                 default: 16
-    t.integer  "metrics_timeout",                   default: 10
-    t.integer  "metrics_method_call_threshold",     default: 10
-    t.boolean  "recaptcha_enabled",                 default: false
+    t.boolean  "require_two_factor_authentication",     default: false
+    t.integer  "two_factor_grace_period",               default: 48
+    t.boolean  "metrics_enabled",                       default: false
+    t.string   "metrics_host",                          default: "localhost"
+    t.integer  "metrics_pool_size",                     default: 16
+    t.integer  "metrics_timeout",                       default: 10
+    t.integer  "metrics_method_call_threshold",         default: 10
+    t.boolean  "recaptcha_enabled",                     default: false
     t.string   "recaptcha_site_key"
     t.string   "recaptcha_private_key"
-    t.integer  "metrics_port",                      default: 8089
-    t.boolean  "akismet_enabled",                   default: false
+    t.integer  "metrics_port",                          default: 8089
+    t.boolean  "akismet_enabled",                       default: false
     t.string   "akismet_api_key"
-    t.integer  "metrics_sample_interval",           default: 15
-    t.boolean  "sentry_enabled",                    default: false
+    t.integer  "metrics_sample_interval",               default: 15
+    t.boolean  "sentry_enabled",                        default: false
     t.string   "sentry_dsn"
-    t.boolean  "email_author_in_body",              default: false
+    t.boolean  "email_author_in_body",                  default: false
     t.integer  "default_group_visibility"
-    t.boolean  "repository_checks_enabled",         default: false
+    t.boolean  "repository_checks_enabled",             default: false
     t.text     "shared_runners_text"
-    t.integer  "metrics_packet_size",               default: 1
+    t.integer  "metrics_packet_size",                   default: 1
     t.text     "disabled_oauth_sign_in_sources"
     t.string   "health_check_access_token"
-    t.boolean  "send_user_confirmation_email",      default: false
+    t.boolean  "send_user_confirmation_email",          default: false
+    t.integer  "container_registry_token_expire_delay", default: 5
   end
 
   create_table "audit_events", force: :cascade do |t|
diff --git a/doc/api/settings.md b/doc/api/settings.md
index 1e745115dc8..43a0fe35e42 100644
--- a/doc/api/settings.md
+++ b/doc/api/settings.md
@@ -37,7 +37,8 @@ Example response:
    "created_at" : "2016-01-04T15:44:55.176Z",
    "default_project_visibility" : 0,
    "gravatar_enabled" : true,
-   "sign_in_text" : null
+   "sign_in_text" : null,
+   "container_registry_token_expire_delay": 5
 }
 ```
 
@@ -64,6 +65,7 @@ PUT /application/settings
 | `restricted_signup_domains` | array of strings | no | Force people to use only corporate emails for sign-up. Default is null, meaning there is no restriction. |
 | `user_oauth_applications` | boolean | no | Allow users to register any application to use GitLab as an OAuth provider |
 | `after_sign_out_path` | string | no | Where to redirect users after logout |
+| `container_registry_token_expire_delay` | integer | no | Container Registry token duration in minutes |
 
 ```bash
 curl -X PUT -H "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/application/settings?signup_enabled=false&default_project_visibility=1
@@ -90,6 +92,7 @@ Example response:
   "default_snippet_visibility": 0,
   "restricted_signup_domains": [],
   "user_oauth_applications": true,
-  "after_sign_out_path": ""
+  "after_sign_out_path": "",
+  "container_registry_token_expire_delay": 5
 }
 ```
diff --git a/lib/api/entities.rb b/lib/api/entities.rb
index 31491cf31dd..790a1869f73 100644
--- a/lib/api/entities.rb
+++ b/lib/api/entities.rb
@@ -362,6 +362,7 @@ module API
       expose :restricted_signup_domains
       expose :user_oauth_applications
       expose :after_sign_out_path
+      expose :container_registry_token_expire_delay
     end
 
     class Release < Grape::Entity
diff --git a/lib/gitlab/current_settings.rb b/lib/gitlab/current_settings.rb
index f44d1b3a44e..29c4e221dd4 100644
--- a/lib/gitlab/current_settings.rb
+++ b/lib/gitlab/current_settings.rb
@@ -36,6 +36,7 @@ module Gitlab
         two_factor_grace_period: 48,
         akismet_enabled: false,
         repository_checks_enabled: true,
+        container_registry_token_expire_delay: 5,
       )
     end
 
diff --git a/spec/services/auth/container_registry_authentication_service_spec.rb b/spec/services/auth/container_registry_authentication_service_spec.rb
index 2d114f59ca4..67777ad48bc 100644
--- a/spec/services/auth/container_registry_authentication_service_spec.rb
+++ b/spec/services/auth/container_registry_authentication_service_spec.rb
@@ -17,6 +17,21 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do
   shared_examples 'a valid token' do
     it { is_expected.to include(:token) }
     it { expect(payload).to include('access') }
+
+    context 'a expirable' do
+      let(:expires_at) { Time.at(payload['exp']) }
+      let(:expire_delay) { 10 }
+
+      context 'for default configuration' do
+        it { expect(expires_at).not_to be_within(2.seconds).of(Time.now + expire_delay.minutes) }
+      end
+
+      context 'for changed configuration' do
+        before { stub_application_setting(container_registry_token_expire_delay: expire_delay) }
+
+        it { expect(expires_at).to be_within(2.seconds).of(Time.now + expire_delay.minutes) }
+      end
+    end
   end
 
   shared_examples 'a accessible' do