diff options
author | Douwe Maan <douwe@gitlab.com> | 2016-05-31 15:23:54 +0000 |
---|---|---|
committer | Yorick Peterse <yorickpeterse@gmail.com> | 2016-06-02 13:39:56 +0200 |
commit | 1d94757c1a0305a7cd9a053d5c9ed21bd5d056dc (patch) | |
tree | c8a1629fb596f11a9a2da73f804df187cc867b1d | |
parent | 0df4fa78e7e9a41f3d057eb13b8a9bd014a6e9ff (diff) | |
download | gitlab-ce-1d94757c1a0305a7cd9a053d5c9ed21bd5d056dc.tar.gz |
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
-rw-r--r-- | CHANGELOG | 1 | ||||
-rw-r--r-- | app/controllers/admin/application_settings_controller.rb | 1 | ||||
-rw-r--r-- | app/models/application_setting.rb | 7 | ||||
-rw-r--r-- | app/services/auth/container_registry_authentication_service.rb | 8 | ||||
-rw-r--r-- | app/views/admin/application_settings/_form.html.haml | 8 | ||||
-rw-r--r-- | db/migrate/20160530150109_add_container_registry_token_expire_delay_to_application_settings.rb | 9 | ||||
-rw-r--r-- | db/schema.rb | 48 | ||||
-rw-r--r-- | doc/api/settings.md | 7 | ||||
-rw-r--r-- | lib/api/entities.rb | 1 | ||||
-rw-r--r-- | lib/gitlab/current_settings.rb | 1 | ||||
-rw-r--r-- | spec/services/auth/container_registry_authentication_service_spec.rb | 15 |
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 |