Commit 5a670322 authored by Grzegorz Bizon's avatar Grzegorz Bizon Committed by Robert Speicher

Merge branch 'fix/ci-runners-token-persistence' into 'master'

Fix method that ensures authentication token

Until now, `ensure_#{token_filed_name}!` method didn't persist new token in database.

This closes #4235.

See merge request !2185
parent 60066dcb
...@@ -134,4 +134,8 @@ class ApplicationSetting < ActiveRecord::Base ...@@ -134,4 +134,8 @@ class ApplicationSetting < ActiveRecord::Base
/x) /x)
self.restricted_signup_domains.reject! { |d| d.empty? } self.restricted_signup_domains.reject! { |d| d.empty? }
end end
def runners_registration_token
ensure_runners_registration_token!
end
end end
...@@ -18,15 +18,16 @@ module TokenAuthenticatable ...@@ -18,15 +18,16 @@ module TokenAuthenticatable
define_method("ensure_#{token_field}") do define_method("ensure_#{token_field}") do
current_token = read_attribute(token_field) current_token = read_attribute(token_field)
if current_token.blank? current_token.blank? ? write_new_token(token_field) : current_token
write_attribute(token_field, generate_token_for(token_field)) end
else
current_token define_method("ensure_#{token_field}!") do
end send("reset_#{token_field}!") if read_attribute(token_field).blank?
read_attribute(token_field)
end end
define_method("reset_#{token_field}!") do define_method("reset_#{token_field}!") do
write_attribute(token_field, generate_token_for(token_field)) write_new_token(token_field)
save! save!
end end
end end
...@@ -34,7 +35,12 @@ module TokenAuthenticatable ...@@ -34,7 +35,12 @@ module TokenAuthenticatable
private private
def generate_token_for(token_field) def write_new_token(token_field)
new_token = generate_token(token_field)
write_attribute(token_field, new_token)
end
def generate_token(token_field)
loop do loop do
token = Devise.friendly_token token = Devise.friendly_token
break token unless self.class.unscoped.find_by(token_field => token) break token unless self.class.unscoped.find_by(token_field => token)
......
...@@ -3,7 +3,7 @@ ...@@ -3,7 +3,7 @@
To register a new runner you should enter the following registration token. 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. With this token the runner will request a unique runner token and use that for future communication.
Registration token is Registration token is
%code{ id: 'runners-token' } #{current_application_settings.ensure_runners_registration_token} %code{ id: 'runners-token' } #{current_application_settings.runners_registration_token}
.bs-callout.clearfix .bs-callout.clearfix
.pull-left .pull-left
......
...@@ -19,7 +19,7 @@ module Ci ...@@ -19,7 +19,7 @@ module Ci
end end
def runner_registration_token_valid? def runner_registration_token_valid?
params[:token] == current_application_settings.ensure_runners_registration_token params[:token] == current_application_settings.runners_registration_token
end end
def update_runner_last_contact def update_runner_last_contact
......
...@@ -63,7 +63,7 @@ describe "Admin Runners" do ...@@ -63,7 +63,7 @@ describe "Admin Runners" do
end end
describe 'runners registration token' do describe 'runners registration token' do
let!(:token) { current_application_settings.ensure_runners_registration_token } let!(:token) { current_application_settings.runners_registration_token }
before { visit admin_runners_path } before { visit admin_runners_path }
it 'has a registration token' do it 'has a registration token' do
......
...@@ -2,7 +2,8 @@ require 'spec_helper' ...@@ -2,7 +2,8 @@ require 'spec_helper'
shared_examples 'TokenAuthenticatable' do shared_examples 'TokenAuthenticatable' do
describe 'dynamically defined methods' do describe 'dynamically defined methods' do
it { expect(described_class).to be_private_method_defined(:generate_token_for) } it { expect(described_class).to be_private_method_defined(:generate_token) }
it { expect(described_class).to be_private_method_defined(:write_new_token) }
it { expect(described_class).to respond_to("find_by_#{token_field}") } 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("ensure_#{token_field}") }
it { is_expected.to respond_to("reset_#{token_field}!") } it { is_expected.to respond_to("reset_#{token_field}!") }
...@@ -24,11 +25,11 @@ describe ApplicationSetting, 'TokenAuthenticatable' do ...@@ -24,11 +25,11 @@ describe ApplicationSetting, 'TokenAuthenticatable' do
it_behaves_like 'TokenAuthenticatable' it_behaves_like 'TokenAuthenticatable'
describe 'generating new token' do describe 'generating new token' do
subject { described_class.new }
let(:token) { subject.send(token_field) }
context 'token is not generated yet' do context 'token is not generated yet' do
it { expect(token).to be nil } describe 'token field accessor' do
subject { described_class.new.send(token_field) }
it { is_expected.to_not be_blank }
end
describe 'ensured token' do describe 'ensured token' do
subject { described_class.new.send("ensure_#{token_field}") } subject { described_class.new.send("ensure_#{token_field}") }
...@@ -36,11 +37,21 @@ describe ApplicationSetting, 'TokenAuthenticatable' do ...@@ -36,11 +37,21 @@ describe ApplicationSetting, 'TokenAuthenticatable' do
it { is_expected.to be_a String } it { is_expected.to be_a String }
it { is_expected.to_not be_blank } it { is_expected.to_not be_blank }
end end
describe 'ensured! token' do
subject { described_class.new.send("ensure_#{token_field}!") }
it 'should persist new token' do
expect(subject).to eq described_class.current[token_field]
end
end
end end
context 'token is generated' do context 'token is generated' do
before { subject.send("reset_#{token_field}!") } before { subject.send("reset_#{token_field}!") }
it { expect(token).to be_a String } it 'persists a new token 'do
expect(subject.send(:read_attribute, token_field)).to be_a String
end
end end
end end
......
...@@ -8,7 +8,6 @@ describe Ci::API::API do ...@@ -8,7 +8,6 @@ describe Ci::API::API do
before do before do
stub_gitlab_calls stub_gitlab_calls
stub_application_setting(ensure_runners_registration_token: registration_token)
stub_application_setting(runners_registration_token: registration_token) stub_application_setting(runners_registration_token: registration_token)
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment