Merge branch 'ci/persist-registration-token' into 'master'

Persist CI runners registration token

This MR adds feature of persisting CI runners registration token.
User will be able to generate and then reset (if necessary to revoke) this token.

This closes #3703 

cc @ayufan 

See merge request !2039

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..9dd16f8c735 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 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/admin/admin_runners_spec.rb b/spec/features/admin/admin_runners_spec.rb
index b1f2d401042..66a2cc0c157 100644
--- a/spec/features/admin/admin_runners_spec.rb
+++ b/spec/features/admin/admin_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 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"]) }