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

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

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

app/models/concerns/token_authenticatable.rb

 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
+
+    private
+
+    def add_authentication_token_field(token_field)
+      @token_fields = [] unless @token_fields
+      @token_fields << token_field
+
+      define_singleton_method("find_by_#{token_field}") do |token|
+        where(token_field => token).first if token
       end
 
-  def ensure_authentication_token
-    if authentication_token.blank?
-      self.authentication_token = generate_authentication_token
+      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
 
-  def reset_authentication_token!
-    self.authentication_token = generate_authentication_token
-    save
+      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

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

app/views/admin/runners/index.html.haml

 %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

config/initializers/4_ci_app.rb

@@ -2,8 +2,6 @@ module GitlabCi
   VERSION = Gitlab::VERSION
   REVISION = Gitlab::REVISION
 
-  REGISTRATION_TOKEN = SecureRandom.hex(10)
-
   def self.config
     Settings
   end

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

db/migrate/20151210072243_add_runners_registration_token_to_application_settings.rb

+class AddRunnersRegistrationTokenToApplicationSettings < ActiveRecord::Migration
+  def change
+    add_column :application_settings, :runners_registration_token, :string
+  end
+end

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|

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)

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],

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

spec/models/concerns/token_authenticatable_spec.rb

+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

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"]) }