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, 26 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 96dda6dffb0..23118035840 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -10,6 +10,7 @@ v 8.8.3
   - Fix import URL migration not rescuing with the correct Error. !4321
   - Fix health check access token changing due to old application settings being used. !4332
   - Make authentication service for Container Registry to be compatible with < Docker 1.11. !4363
+  - Add Application Setting to configure Container Registry token expire delay (default 5min). !4364
 
 v 8.8.2
   - Added remove due date button. !4209
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 33069d66d78..42f908aa344 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|
@@ -125,7 +129,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 2e154b98b34..ee99782e108 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: 20160509201028) do
+ActiveRecord::Schema.define(version: 20160530150109) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -43,45 +43,47 @@ ActiveRecord::Schema.define(version: 20160509201028) 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.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 1a4ddd38513..92c7e8b9d88 100644
--- a/lib/gitlab/current_settings.rb
+++ b/lib/gitlab/current_settings.rb
@@ -40,6 +40,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 11858301844..4d4f5a0a303 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