diff options
-rw-r--r-- | app/controllers/admin/application_settings_controller.rb | 6 | ||||
-rw-r--r-- | app/models/application_setting.rb | 7 | ||||
-rw-r--r-- | app/models/concerns/token_authenticatable.rb | 44 | ||||
-rw-r--r-- | app/models/user.rb | 4 | ||||
-rw-r--r-- | app/views/admin/runners/index.html.haml | 18 | ||||
-rw-r--r-- | config/initializers/4_ci_app.rb | 2 | ||||
-rw-r--r-- | config/routes.rb | 1 | ||||
-rw-r--r-- | db/migrate/20151210072243_add_runners_registration_token_to_application_settings.rb | 5 | ||||
-rw-r--r-- | db/schema.rb | 1 | ||||
-rw-r--r-- | lib/ci/api/helpers.rb | 6 | ||||
-rw-r--r-- | lib/ci/api/runners.rb | 2 | ||||
-rw-r--r-- | spec/features/atom/runners_spec.rb | 22 | ||||
-rw-r--r-- | spec/models/concerns/token_authenticatable_spec.rb | 57 | ||||
-rw-r--r-- | spec/requests/ci/api/runners_spec.rb | 10 |
14 files changed, 158 insertions, 27 deletions
diff --git a/app/controllers/admin/application_settings_controller.rb b/app/controllers/admin/application_settings_controller.rb index a9bcfc7456a..48040359389 100644 --- a/app/controllers/admin/application_settings_controller.rb +++ b/app/controllers/admin/application_settings_controller.rb @@ -13,6 +13,12 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController end end + def reset_runners_token + @application_setting.reset_runners_registration_token! + flash[:notice] = 'New runners registration token has been generated!' + redirect_to ci_admin_runners_path + end + private def set_application_setting diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index 1880ad9f33c..faa0bdf840b 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -27,9 +27,13 @@ # admin_notification_email :string(255) # shared_runners_enabled :boolean default(TRUE), not null # max_artifacts_size :integer default(100), not null +# runners_registration_token :string(255) # class ApplicationSetting < ActiveRecord::Base + include TokenAuthenticatable + add_authentication_token_field :runners_registration_token + CACHE_KEY = 'application_setting.last' serialize :restricted_visibility_levels @@ -74,6 +78,8 @@ class ApplicationSetting < ActiveRecord::Base end end + before_save :ensure_runners_registration_token + after_commit do Rails.cache.write(CACHE_KEY, self) end @@ -128,5 +134,4 @@ class ApplicationSetting < ActiveRecord::Base /x) self.restricted_signup_domains.reject! { |d| d.empty? } end - end diff --git a/app/models/concerns/token_authenticatable.rb b/app/models/concerns/token_authenticatable.rb index 9b88ec1cc38..56d38fe8250 100644 --- a/app/models/concerns/token_authenticatable.rb +++ b/app/models/concerns/token_authenticatable.rb @@ -1,31 +1,43 @@ module TokenAuthenticatable extend ActiveSupport::Concern - module ClassMethods - def find_by_authentication_token(authentication_token = nil) - if authentication_token - where(authentication_token: authentication_token).first - end + class_methods do + def authentication_token_fields + @token_fields || [] end - end - def ensure_authentication_token - if authentication_token.blank? - self.authentication_token = generate_authentication_token - end - end + private + + def add_authentication_token_field(token_field) + @token_fields = [] unless @token_fields + @token_fields << token_field - def reset_authentication_token! - self.authentication_token = generate_authentication_token - save + define_singleton_method("find_by_#{token_field}") do |token| + where(token_field => token).first if token + end + + define_method("ensure_#{token_field}") do + current_token = read_attribute(token_field) + if current_token.blank? + write_attribute(token_field, generate_token_for(token_field)) + else + current_token + end + end + + define_method("reset_#{token_field}!") do + write_attribute(token_field, generate_token_for(token_field)) + save! + end + end end private - def generate_authentication_token + def generate_token_for(token_field) loop do token = Devise.friendly_token - break token unless self.class.unscoped.where(authentication_token: token).first + break token unless self.class.unscoped.where(token_field => token).first end end end diff --git a/app/models/user.rb b/app/models/user.rb index 87d46a49430..fdd14f4571d 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -69,8 +69,10 @@ class User < ActiveRecord::Base include Gitlab::CurrentSettings include Referable include Sortable - include TokenAuthenticatable include CaseSensitivity + include TokenAuthenticatable + + add_authentication_token_field :authentication_token default_value_for :admin, false default_value_for :can_create_group, gitlab_config.default_can_create_group diff --git a/app/views/admin/runners/index.html.haml b/app/views/admin/runners/index.html.haml index 26b180b4052..c5fb3c95506 100644 --- a/app/views/admin/runners/index.html.haml +++ b/app/views/admin/runners/index.html.haml @@ -1,6 +1,20 @@ %p.lead - %span To register a new runner you should enter the following registration token. With this token the runner will request a unique runner token and use that for future communication. - %code #{GitlabCi::REGISTRATION_TOKEN} + %span + To register a new runner you should enter the following registration token. + With this token the runner will request a unique runner token and use that for future communication. + Registration token is + %code{ id: 'runners-token' } #{current_application_settings.ensure_runners_registration_token} + +.bs-callout.clearfix + .pull-left + %p + You can reset runners registration token by pressing a button below. + %p + = button_to reset_runners_token_admin_application_settings_path, + method: :put, class: 'btn btn-default', + data: { confirm: 'Are you sure you want to reset registration token?' } do + = icon('refresh') + Reset runners registration token .bs-callout %p diff --git a/config/initializers/4_ci_app.rb b/config/initializers/4_ci_app.rb index cac8edb32bf..d252e403102 100644 --- a/config/initializers/4_ci_app.rb +++ b/config/initializers/4_ci_app.rb @@ -1,8 +1,6 @@ module GitlabCi VERSION = Gitlab::VERSION REVISION = Gitlab::REVISION - - REGISTRATION_TOKEN = SecureRandom.hex(10) def self.config Settings diff --git a/config/routes.rb b/config/routes.rb index 50836d63fa5..e2d4fcb65a8 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -245,6 +245,7 @@ Rails.application.routes.draw do resource :application_settings, only: [:show, :update] do resources :services + put :reset_runners_token end resources :labels diff --git a/db/migrate/20151210072243_add_runners_registration_token_to_application_settings.rb b/db/migrate/20151210072243_add_runners_registration_token_to_application_settings.rb new file mode 100644 index 00000000000..00f88180e46 --- /dev/null +++ b/db/migrate/20151210072243_add_runners_registration_token_to_application_settings.rb @@ -0,0 +1,5 @@ +class AddRunnersRegistrationTokenToApplicationSettings < ActiveRecord::Migration + def change + add_column :application_settings, :runners_registration_token, :string + end +end diff --git a/db/schema.rb b/db/schema.rb index d17930b84b0..0167e30ff8b 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -49,6 +49,7 @@ ActiveRecord::Schema.define(version: 20151210125932) do t.string "admin_notification_email" t.boolean "shared_runners_enabled", default: true, null: false t.integer "max_artifacts_size", default: 100, null: false + t.string "runners_registration_token" end create_table "audit_events", force: :cascade do |t| diff --git a/lib/ci/api/helpers.rb b/lib/ci/api/helpers.rb index 9891b5e38ea..443563c2e4a 100644 --- a/lib/ci/api/helpers.rb +++ b/lib/ci/api/helpers.rb @@ -6,7 +6,7 @@ module Ci UPDATE_RUNNER_EVERY = 60 def authenticate_runners! - forbidden! unless params[:token] == GitlabCi::REGISTRATION_TOKEN + forbidden! unless runner_registration_token_valid? end def authenticate_runner! @@ -18,6 +18,10 @@ module Ci forbidden! unless token && build.valid_token?(token) end + def runner_registration_token_valid? + params[:token] == current_application_settings.ensure_runners_registration_token + end + def update_runner_last_contact # Use a random threshold to prevent beating DB updates contacted_at_max_age = UPDATE_RUNNER_EVERY + Random.rand(UPDATE_RUNNER_EVERY) diff --git a/lib/ci/api/runners.rb b/lib/ci/api/runners.rb index 8704917b9e4..bfc14fe7a6b 100644 --- a/lib/ci/api/runners.rb +++ b/lib/ci/api/runners.rb @@ -29,7 +29,7 @@ module Ci required_attributes! [:token] runner = - if params[:token] == GitlabCi::REGISTRATION_TOKEN + if runner_registration_token_valid? # Create shared runner. Requires admin access Ci::Runner.create( description: params[:description], diff --git a/spec/features/atom/runners_spec.rb b/spec/features/atom/runners_spec.rb index b1f2d401042..d5dd11a01d3 100644 --- a/spec/features/atom/runners_spec.rb +++ b/spec/features/atom/runners_spec.rb @@ -61,4 +61,26 @@ describe "Admin Runners" do it { expect(page).not_to have_content(@project2.name_with_namespace) } end end + + describe 'runners registration token' do + let!(:token) { current_application_settings.ensure_runners_registration_token } + before { visit ci_admin_runners_path } + + it 'has a registration token' do + expect(page).to have_content("Registration token is #{token}") + expect(page).to have_selector('#runners-token', text: token) + end + + describe 'reload registration token' do + let(:page_token) { find('#runners-token').text } + + before do + click_button 'Reset runners registration token' + end + + it 'changes registration token' do + expect(page_token).to_not eq token + end + end + end end diff --git a/spec/models/concerns/token_authenticatable_spec.rb b/spec/models/concerns/token_authenticatable_spec.rb new file mode 100644 index 00000000000..a9b0b64e5de --- /dev/null +++ b/spec/models/concerns/token_authenticatable_spec.rb @@ -0,0 +1,57 @@ +require 'spec_helper' + +shared_examples 'TokenAuthenticatable' do + describe 'dynamically defined methods' do + it { expect(described_class).to be_private_method_defined(:generate_token_for) } + it { expect(described_class).to respond_to("find_by_#{token_field}") } + it { is_expected.to respond_to("ensure_#{token_field}") } + it { is_expected.to respond_to("reset_#{token_field}!") } + end +end + +describe User, 'TokenAuthenticatable' do + let(:token_field) { :authentication_token } + it_behaves_like 'TokenAuthenticatable' + + describe 'ensures authentication token' do + subject { create(:user).send(token_field) } + it { is_expected.to be_a String } + end +end + +describe ApplicationSetting, 'TokenAuthenticatable' do + let(:token_field) { :runners_registration_token } + it_behaves_like 'TokenAuthenticatable' + + describe 'generating new token' do + subject { described_class.new } + let(:token) { subject.send(token_field) } + + context 'token is not generated yet' do + it { expect(token).to be nil } + + describe 'ensured token' do + subject { described_class.new.send("ensure_#{token_field}") } + + it { is_expected.to be_a String } + it { is_expected.to_not be_blank } + end + end + + context 'token is generated' do + before { subject.send("reset_#{token_field}!") } + it { expect(token).to be_a String } + end + end + + describe 'multiple token fields' do + before do + described_class.send(:add_authentication_token_field, :yet_another_token) + end + + describe '.token_fields' do + subject { described_class.authentication_token_fields } + it { is_expected.to include(:runners_registration_token, :yet_another_token) } + end + end +end diff --git a/spec/requests/ci/api/runners_spec.rb b/spec/requests/ci/api/runners_spec.rb index b98c4d506c7..567da013e6f 100644 --- a/spec/requests/ci/api/runners_spec.rb +++ b/spec/requests/ci/api/runners_spec.rb @@ -4,26 +4,30 @@ describe Ci::API::API do include ApiHelpers include StubGitlabCalls + let(:registration_token) { 'abcdefg123456' } + before do stub_gitlab_calls + stub_application_setting(ensure_runners_registration_token: registration_token) + stub_application_setting(runners_registration_token: registration_token) end describe "POST /runners/register" do describe "should create a runner if token provided" do - before { post ci_api("/runners/register"), token: GitlabCi::REGISTRATION_TOKEN } + before { post ci_api("/runners/register"), token: registration_token } it { expect(response.status).to eq(201) } end describe "should create a runner with description" do - before { post ci_api("/runners/register"), token: GitlabCi::REGISTRATION_TOKEN, description: "server.hostname" } + before { post ci_api("/runners/register"), token: registration_token, description: "server.hostname" } it { expect(response.status).to eq(201) } it { expect(Ci::Runner.first.description).to eq("server.hostname") } end describe "should create a runner with tags" do - before { post ci_api("/runners/register"), token: GitlabCi::REGISTRATION_TOKEN, tag_list: "tag1, tag2" } + before { post ci_api("/runners/register"), token: registration_token, tag_list: "tag1, tag2" } it { expect(response.status).to eq(201) } it { expect(Ci::Runner.first.tag_list.sort).to eq(["tag1", "tag2"]) } |