Merge branch 'enforce-runner-token-expires-at' into 'master'

CI Runners: Enforce token expiration

See merge request gitlab-org/gitlab!78557

app/models/ci/runner.rb

@@ -13,7 +13,7 @@ module Ci
     include TaggableQueries
     include Presentable
 
-    add_authentication_token_field :token, encrypted: :optional
+    add_authentication_token_field :token, encrypted: :optional, expires_at: :compute_token_expiration, expiration_enforced?: :token_expiration_enforced?
 
     enum access_level: {
       not_protected: 0,
@@ -479,6 +479,21 @@ module Ci
       end
     end
 
+    def compute_token_expiration
+      case runner_type
+      when 'instance_type'
+        compute_token_expiration_instance
+      when 'group_type'
+        compute_token_expiration_group
+      when 'project_type'
+        compute_token_expiration_project
+      end
+    end
+
+    def self.token_expiration_enforced?
+      Feature.enabled?(:enforce_runner_token_expires_at, default_enabled: :yaml)
+    end
+
     private
 
     EXECUTOR_NAME_TO_TYPES = {
@@ -498,6 +513,20 @@ module Ci
 
     EXECUTOR_TYPE_TO_NAMES = EXECUTOR_NAME_TO_TYPES.invert.freeze
 
+    def compute_token_expiration_instance
+      return unless expiration_interval = Gitlab::CurrentSettings.runner_token_expiration_interval
+
+      expiration_interval.seconds.from_now
+    end
+
+    def compute_token_expiration_group
+      ::Group.where(id: runner_namespaces.map(&:namespace_id)).map(&:effective_runner_token_expiration_interval).compact.min&.from_now
+    end
+
+    def compute_token_expiration_project
+      Project.where(id: runner_projects.map(&:project_id)).map(&:effective_runner_token_expiration_interval).compact.min&.from_now
+    end
+
     def cleanup_runner_queue
       Gitlab::Redis::SharedState.with do |redis|
         redis.del(runner_queue_key)

app/models/concerns/token_authenticatable.rb

@@ -64,6 +64,18 @@ module TokenAuthenticatable
       mod.define_method("format_#{token_field}") do |token|
         token
       end
+
+      mod.define_method("#{token_field}_expires_at") do
+        strategy.expires_at(self)
+      end
+
+      mod.define_method("#{token_field}_expired?") do
+        strategy.expired?(self)
+      end
+
+      mod.define_method("#{token_field}_with_expiration") do
+        strategy.token_with_expiration(self)
+      end
     end
 
     def token_authenticatable_module

app/models/concerns/token_authenticatable_strategies/base.rb

@@ -7,6 +7,7 @@ module TokenAuthenticatableStrategies
     def initialize(klass, token_field, options)
       @klass = klass
       @token_field = token_field
+      @expires_at_field = "#{token_field}_expires_at"
       @options = options
     end
 
@@ -44,6 +45,25 @@ module TokenAuthenticatableStrategies
       instance.save! if Gitlab::Database.read_write?
     end
 
+    def expires_at(instance)
+      instance.read_attribute(@expires_at_field)
+    end
+
+    def expired?(instance)
+      return false unless expirable? && token_expiration_enforced?
+
+      exp = expires_at(instance)
+      !!exp && Time.current > exp
+    end
+
+    def expirable?
+      !!@options[:expires_at]
+    end
+
+    def token_with_expiration(instance)
+      API::Support::TokenWithExpiration.new(self, instance)
+    end
+
     def self.fabricate(model, field, options)
       if options[:digest] && options[:encrypted]
         raise ArgumentError, _('Incompatible options set!')
@@ -64,6 +84,10 @@ module TokenAuthenticatableStrategies
       new_token = generate_available_token
       formatted_token = format_token(instance, new_token)
       set_token(instance, formatted_token)
+
+      if expirable?
+        instance[@expires_at_field] = @options[:expires_at].to_proc.call(instance)
+      end
     end
 
     def unique
@@ -82,11 +106,21 @@ module TokenAuthenticatableStrategies
     end
 
     def relation(unscoped)
-      unscoped ? @klass.unscoped : @klass
+      unscoped ? @klass.unscoped : @klass.where(not_expired)
     end
 
     def token_set?(instance)
       raise NotImplementedError
     end
+
+    def token_expiration_enforced?
+      return true unless @options[:expiration_enforced?]
+
+      @options[:expiration_enforced?].to_proc.call(@klass)
+    end
+
+    def not_expired
+      Arel.sql("#{@expires_at_field} IS NULL OR #{@expires_at_field} >= NOW()") if expirable? && token_expiration_enforced?
+    end
   end
 end

config/feature_flags/development/enforce_runner_token_expires_at.yml

+---
+name: enforce_runner_token_expires_at
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/78557
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/352008
+milestone: '14.8'
+type: development
+group: group::runner
+default_enabled: false

lib/api/support/token_with_expiration.rb

+# frozen_string_literal: true
+
+module API
+  module Support
+    class TokenWithExpiration
+      def initialize(strategy, instance)
+        @strategy = strategy
+        @instance = instance
+      end
+
+      def token
+        @strategy.get_token(@instance)
+      end
+
+      def token_expires_at
+        @strategy.expires_at(@instance)
+      end
+
+      def expirable?
+        @strategy.expirable?
+      end
+    end
+  end
+end

spec/models/ci/runner_spec.rb

@@ -3,6 +3,8 @@
 require 'spec_helper'
 
 RSpec.describe Ci::Runner do
+  include StubGitlabCalls
+
   it_behaves_like 'having unique enum values'
 
   it_behaves_like 'it has loose foreign keys' do
@@ -1301,9 +1303,11 @@ RSpec.describe Ci::Runner do
     end
 
     it 'supports ordering by the token expiration' do
-      runner1 = create(:ci_runner, token_expires_at: 1.year.from_now)
+      runner1 = create(:ci_runner)
+      runner1.update!(token_expires_at: 1.year.from_now)
       runner2 = create(:ci_runner)
-      runner3 = create(:ci_runner, token_expires_at: 1.month.from_now)
+      runner3 = create(:ci_runner)
+      runner3.update!(token_expires_at: 1.month.from_now)
 
       runners = described_class.order_by('token_expires_at_asc')
       expect(runners).to eq([runner3, runner1, runner2])
@@ -1522,4 +1526,182 @@ RSpec.describe Ci::Runner do
       )
     end
   end
+
+  describe '#token_expires_at', :freeze_time do
+    shared_examples 'expiring token' do |interval:|
+      it 'expires' do
+        expect(runner.token_expires_at).to eq(interval.from_now)
+      end
+    end
+
+    shared_examples 'non-expiring token' do
+      it 'does not expire' do
+        expect(runner.token_expires_at).to be_nil
+      end
+    end
+
+    context 'no expiration' do
+      let(:runner) { create(:ci_runner) }
+
+      it_behaves_like 'non-expiring token'
+    end
+
+    context 'system-wide shared expiration' do
+      before do
+        stub_application_setting(runner_token_expiration_interval: 5.days.to_i)
+      end
+
+      let(:runner) { create(:ci_runner) }
+
+      it_behaves_like 'expiring token', interval: 5.days
+    end
+
+    context 'system-wide group expiration' do
+      before do
+        stub_application_setting(group_runner_token_expiration_interval: 5.days.to_i)
+      end
+
+      let(:runner) { create(:ci_runner) }
+
+      it_behaves_like 'non-expiring token'
+    end
+
+    context 'system-wide project expiration' do
+      before do
+        stub_application_setting(project_runner_token_expiration_interval: 5.days.to_i)
+      end
+
+      let(:runner) { create(:ci_runner) }
+
+      it_behaves_like 'non-expiring token'
+    end
+
+    context 'group expiration' do
+      let(:group_settings) { create(:namespace_settings, runner_token_expiration_interval: 6.days.to_i) }
+      let(:group) { create(:group, namespace_settings: group_settings) }
+      let(:runner) { create(:ci_runner, :group, groups: [group]) }
+
+      it_behaves_like 'expiring token', interval: 6.days
+    end
+
+    context 'human-readable group expiration' do
+      let(:group_settings) { create(:namespace_settings, runner_token_expiration_interval_human_readable: '7 days') }
+      let(:group) { create(:group, namespace_settings: group_settings) }
+      let(:runner) { create(:ci_runner, :group, groups: [group]) }
+
+      it_behaves_like 'expiring token', interval: 7.days
+    end
+
+    context 'project expiration' do
+      let(:project) { create(:project, runner_token_expiration_interval: 4.days.to_i).tap(&:save!) }
+      let(:runner) { create(:ci_runner, :project, projects: [project]) }
+
+      it_behaves_like 'expiring token', interval: 4.days
+    end
+
+    context 'human-readable project expiration' do
+      let(:project) { create(:project, runner_token_expiration_interval_human_readable: '5 days').tap(&:save!) }
+      let(:runner) { create(:ci_runner, :project, projects: [project]) }
+
+      it_behaves_like 'expiring token', interval: 5.days
+    end
+
+    context 'multiple projects' do
+      let(:project1) { create(:project, runner_token_expiration_interval: 8.days.to_i).tap(&:save!) }
+      let(:project2) { create(:project, runner_token_expiration_interval: 7.days.to_i).tap(&:save!) }
+      let(:project3) { create(:project, runner_token_expiration_interval: 9.days.to_i).tap(&:save!) }
+      let(:runner) { create(:ci_runner, :project, projects: [project1, project2, project3]) }
+
+      it_behaves_like 'expiring token', interval: 7.days
+    end
+
+    context 'with project runner token expiring' do
+      let_it_be(:project) { create(:project, runner_token_expiration_interval: 4.days.to_i).tap(&:save!) }
+
+      context 'project overrides system' do
+        before do
+          stub_application_setting(project_runner_token_expiration_interval: 5.days.to_i)
+        end
+
+        let(:runner) { create(:ci_runner, :project, projects: [project]) }
+
+        it_behaves_like 'expiring token', interval: 4.days
+      end
+
+      context 'system overrides project' do
+        before do
+          stub_application_setting(project_runner_token_expiration_interval: 3.days.to_i)
+        end
+
+        let(:runner) { create(:ci_runner, :project, projects: [project]) }
+
+        it_behaves_like 'expiring token', interval: 3.days
+      end
+    end
+
+    context 'with group runner token expiring' do
+      let_it_be(:group_settings) { create(:namespace_settings, runner_token_expiration_interval: 4.days.to_i) }
+      let_it_be(:group) { create(:group, namespace_settings: group_settings) }
+
+      context 'group overrides system' do
+        before do
+          stub_application_setting(group_runner_token_expiration_interval: 5.days.to_i)
+        end
+
+        let(:runner) { create(:ci_runner, :group, groups: [group]) }
+
+        it_behaves_like 'expiring token', interval: 4.days
+      end
+
+      context 'system overrides group' do
+        before do
+          stub_application_setting(group_runner_token_expiration_interval: 3.days.to_i)
+        end
+
+        let(:runner) { create(:ci_runner, :group, groups: [group]) }
+
+        it_behaves_like 'expiring token', interval: 3.days
+      end
+    end
+
+    context "with group's project runner token expiring" do
+      let_it_be(:parent_group_settings) { create(:namespace_settings, subgroup_runner_token_expiration_interval: 2.days.to_i) }
+      let_it_be(:parent_group) { create(:group, namespace_settings: parent_group_settings) }
+
+      context 'parent group overrides subgroup' do
+        let(:group_settings) { create(:namespace_settings, runner_token_expiration_interval: 3.days.to_i) }
+        let(:group) { create(:group, parent: parent_group, namespace_settings: group_settings) }
+        let(:runner) { create(:ci_runner, :group, groups: [group]) }
+
+        it_behaves_like 'expiring token', interval: 2.days
+      end
+
+      context 'subgroup overrides parent group' do
+        let(:group_settings) { create(:namespace_settings, runner_token_expiration_interval: 1.day.to_i) }
+        let(:group) { create(:group, parent: parent_group, namespace_settings: group_settings) }
+        let(:runner) { create(:ci_runner, :group, groups: [group]) }
+
+        it_behaves_like 'expiring token', interval: 1.day
+      end
+    end
+
+    context "with group's project runner token expiring" do
+      let_it_be(:group_settings) { create(:namespace_settings, project_runner_token_expiration_interval: 2.days.to_i) }
+      let_it_be(:group) { create(:group, namespace_settings: group_settings) }
+
+      context 'group overrides project' do
+        let(:project) { create(:project, group: group, runner_token_expiration_interval: 3.days.to_i).tap(&:save!) }
+        let(:runner) { create(:ci_runner, :project, projects: [project]) }
+
+        it_behaves_like 'expiring token', interval: 2.days
+      end
+
+      context 'project overrides group' do
+        let(:project) { create(:project, group: group, runner_token_expiration_interval: 1.day.to_i).tap(&:save!) }
+        let(:runner) { create(:ci_runner, :project, projects: [project]) }
+
+        it_behaves_like 'expiring token', interval: 1.day
+      end
+    end
+  end
 end

spec/models/concerns/token_authenticatable_spec.rb

@@ -289,4 +289,142 @@ RSpec.describe Ci::Build, 'TokenAuthenticatable' do
       expect(build.read_attribute('token')).to be_nil
     end
   end
+
+  describe '#token_with_expiration' do
+    describe '#expirable?' do
+      subject { build.token_with_expiration.expirable? }
+
+      it { is_expected.to eq(false) }
+    end
+  end
+end
+
+RSpec.describe Ci::Runner, 'TokenAuthenticatable', :freeze_time do
+  let_it_be(:non_expirable_runner) { create(:ci_runner) }
+  let_it_be(:non_expired_runner) { create(:ci_runner).tap { |r| r.update!(token_expires_at: 5.seconds.from_now) } }
+  let_it_be(:expired_runner) { create(:ci_runner).tap { |r| r.update!(token_expires_at: 5.seconds.ago) } }
+
+  describe '#token_expired?' do
+    subject { runner.token_expired? }
+
+    context 'when enforce_runner_token_expires_at feature flag is disabled' do
+      before do
+        stub_feature_flags(enforce_runner_token_expires_at: false)
+      end
+
+      context 'when runner has no token expiration' do
+        let(:runner) { non_expirable_runner }
+
+        it { is_expected.to eq(false) }
+      end
+
+      context 'when runner token is not expired' do
+        let(:runner) { non_expired_runner }
+
+        it { is_expected.to eq(false) }
+      end
+
+      context 'when runner token is expired' do
+        let(:runner) { expired_runner }
+
+        it { is_expected.to eq(false) }
+      end
+    end
+
+    context 'when enforce_runner_token_expires_at feature flag is enabled' do
+      before do
+        stub_feature_flags(enforce_runner_token_expires_at: true)
+      end
+
+      context 'when runner has no token expiration' do
+        let(:runner) { non_expirable_runner }
+
+        it { is_expected.to eq(false) }
+      end
+
+      context 'when runner token is not expired' do
+        let(:runner) { non_expired_runner }
+
+        it { is_expected.to eq(false) }
+      end
+
+      context 'when runner token is expired' do
+        let(:runner) { expired_runner }
+
+        it { is_expected.to eq(true) }
+      end
+    end
+  end
+
+  describe '#token_with_expiration' do
+    describe '#token' do
+      subject { non_expired_runner.token_with_expiration.token }
+
+      it { is_expected.to eq(non_expired_runner.token) }
+    end
+
+    describe '#token_expires_at' do
+      subject { non_expired_runner.token_with_expiration.token_expires_at }
+
+      it { is_expected.to eq(non_expired_runner.token_expires_at) }
+    end
+
+    describe '#expirable?' do
+      subject { non_expired_runner.token_with_expiration.expirable? }
+
+      it { is_expected.to eq(true) }
+    end
+  end
+
+  describe '.find_by_token' do
+    subject { Ci::Runner.find_by_token(runner.token) }
+
+    context 'when enforce_runner_token_expires_at feature flag is disabled' do
+      before do
+        stub_feature_flags(enforce_runner_token_expires_at: false)
+      end
+
+      context 'when runner has no token expiration' do
+        let(:runner) { non_expirable_runner }
+
+        it { is_expected.to eq(non_expirable_runner) }
+      end
+
+      context 'when runner token is not expired' do
+        let(:runner) { non_expired_runner }
+
+        it { is_expected.to eq(non_expired_runner) }
+      end
+
+      context 'when runner token is expired' do
+        let(:runner) { expired_runner }
+
+        it { is_expected.to eq(expired_runner) }
+      end
+    end
+
+    context 'when enforce_runner_token_expires_at feature flag is enabled' do
+      before do
+        stub_feature_flags(enforce_runner_token_expires_at: true)
+      end
+
+      context 'when runner has no token expiration' do
+        let(:runner) { non_expirable_runner }
+
+        it { is_expected.to eq(non_expirable_runner) }
+      end
+
+      context 'when runner token is not expired' do
+        let(:runner) { non_expired_runner }
+
+        it { is_expected.to eq(non_expired_runner) }
+      end
+
+      context 'when runner token is expired' do
+        let(:runner) { expired_runner }
+
+        it { is_expected.to be_nil }
+      end
+    end
+  end
 end

spec/models/concerns/token_authenticatable_strategies/encrypted_spec.rb

@@ -23,6 +23,8 @@ RSpec.describe TokenAuthenticatableStrategies::Encrypted do
       let(:options) { { encrypted: :required } }
 
       it 'finds the encrypted resource by cleartext' do
+        allow(model).to receive(:where)
+          .and_return(model)
         allow(model).to receive(:find_by)
           .with('some_field_encrypted' => [encrypted, encrypted_with_static_iv])
           .and_return('encrypted resource')
@@ -36,6 +38,8 @@ RSpec.describe TokenAuthenticatableStrategies::Encrypted do
       let(:options) { { encrypted: :optional } }
 
       it 'finds the encrypted resource by cleartext' do
+        allow(model).to receive(:where)
+          .and_return(model)
         allow(model).to receive(:find_by)
           .with('some_field_encrypted' => [encrypted, encrypted_with_static_iv])
           .and_return('encrypted resource')
@@ -49,6 +53,8 @@ RSpec.describe TokenAuthenticatableStrategies::Encrypted do
           .to receive(:find_token_authenticatable)
           .and_return('plaintext resource')
 
+        allow(model).to receive(:where)
+          .and_return(model)
         allow(model).to receive(:find_by)
           .with('some_field_encrypted' => [encrypted, encrypted_with_static_iv])
           .and_return(nil)
@@ -62,6 +68,8 @@ RSpec.describe TokenAuthenticatableStrategies::Encrypted do
       let(:options) { { encrypted: :migrating } }
 
       it 'finds the cleartext resource by cleartext' do
+        allow(model).to receive(:where)
+          .and_return(model)
         allow(model).to receive(:find_by)
           .with('some_field' => 'my-value')
           .and_return('cleartext resource')
@@ -71,6 +79,8 @@ RSpec.describe TokenAuthenticatableStrategies::Encrypted do
       end
 
       it 'returns nil if resource cannot be found' do
+        allow(model).to receive(:where)
+          .and_return(model)
         allow(model).to receive(:find_by)
           .with('some_field' => 'my-value')
           .and_return(nil)

spec/requests/api/ci/runner/runners_verify_post_spec.rb

@@ -49,6 +49,30 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do
           let(:expected_params) { { client_id: "runner/#{runner.id}" } }
         end
       end
+
+      context 'when non-expired token is provided' do
+        subject { post api('/runners/verify'), params: { token: runner.token } }
+
+        it 'verifies Runner credentials' do
+          runner["token_expires_at"] = 10.days.from_now
+          runner.save!
+          subject
+
+          expect(response).to have_gitlab_http_status(:ok)
+        end
+      end
+
+      context 'when expired token is provided' do
+        subject { post api('/runners/verify'), params: { token: runner.token } }
+
+        it 'does not verify Runner credentials' do
+          runner["token_expires_at"] = 10.days.ago
+          runner.save!
+          subject
+
+          expect(response).to have_gitlab_http_status(:forbidden)
+        end
+      end
     end
   end
 end