diff --git a/app/controllers/api/connect/base_controller.rb b/app/controllers/api/connect/base_controller.rb index 1fbeaff07..84de46574 100644 --- a/app/controllers/api/connect/base_controller.rb +++ b/app/controllers/api/connect/base_controller.rb @@ -44,4 +44,14 @@ def authenticate_with_token end end + def system_token_header + headers[SYSTEM_TOKEN_HEADER] = @system.system_token + end + + def refresh_system_token + if system_tokens_enabled? + @system.update(system_token: SecureRandom.uuid) + system_token_header + end + end end diff --git a/app/controllers/api/connect/v3/systems/products_controller.rb b/app/controllers/api/connect/v3/systems/products_controller.rb index 5688cc9b4..648443ecd 100644 --- a/app/controllers/api/connect/v3/systems/products_controller.rb +++ b/app/controllers/api/connect/v3/systems/products_controller.rb @@ -5,6 +5,7 @@ class Api::Connect::V3::Systems::ProductsController < Api::Connect::BaseControll before_action :check_product_service_and_repositories, only: %i[show activate] before_action :load_subscription, only: %i[activate upgrade] before_action :check_base_product_dependencies, only: %i[activate upgrade show] + after_action :refresh_system_token, only: %i[activate upgrade], if: -> { request.headers.key?(SYSTEM_TOKEN_HEADER) } def activate create_product_activation diff --git a/app/controllers/api/connect/v3/systems/systems_controller.rb b/app/controllers/api/connect/v3/systems/systems_controller.rb index 863557279..d9a17154d 100644 --- a/app/controllers/api/connect/v3/systems/systems_controller.rb +++ b/app/controllers/api/connect/v3/systems/systems_controller.rb @@ -1,6 +1,7 @@ class Api::Connect::V3::Systems::SystemsController < Api::Connect::BaseController before_action :authenticate_system + after_action :refresh_system_token, only: [:update], if: -> { request.headers.key?(SYSTEM_TOKEN_HEADER) } def update if params[:online_at].present? diff --git a/app/controllers/api/connect/v4/systems/products_controller.rb b/app/controllers/api/connect/v4/systems/products_controller.rb index 7237e5a80..d4c87adfc 100644 --- a/app/controllers/api/connect/v4/systems/products_controller.rb +++ b/app/controllers/api/connect/v4/systems/products_controller.rb @@ -1,5 +1,6 @@ class Api::Connect::V4::Systems::ProductsController < Api::Connect::V3::Systems::ProductsController + after_action :refresh_system_token, only: %i[activate upgrade synchronize destroy], if: -> { request.headers.key?(SYSTEM_TOKEN_HEADER) } def destroy if @product.base? raise ActionController::TranslatedError.new(N_('The product "%s" is a base product and cannot be deactivated'), @product.name) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 686972895..63458615a 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -21,10 +21,10 @@ def authenticate_system(skip_on_duplicated: false) update_user_agent # If SYSTEM_TOKEN_HEADER is present, RMT assumes the client uses a SUSEConnect version - # that supports this feature. In this case, refresh the token and include it in the response. + # that supports this feature. if system_tokens_enabled? && request.headers.key?(SYSTEM_TOKEN_HEADER) - @system.update(last_seen_at: Time.zone.now, system_token: SecureRandom.uuid) - headers[SYSTEM_TOKEN_HEADER] = @system.system_token + @system.update(last_seen_at: Time.zone.now) + system_token_header # only update last_seen_at each 3 minutes, # so that a system that calls SCC every second doesn't write + lock the database row elsif !@system.last_seen_at || @system.last_seen_at < 3.minutes.ago diff --git a/spec/requests/api/connect/base_controller_spec.rb b/spec/requests/api/connect/base_controller_spec.rb index 70ef9aaff..8832909ea 100644 --- a/spec/requests/api/connect/base_controller_spec.rb +++ b/spec/requests/api/connect/base_controller_spec.rb @@ -55,15 +55,6 @@ def require_product end end - shared_examples 'updates the system token' do - it 'updates the system token' do - allow(SecureRandom).to receive(:uuid).and_return(new_system_token) - - expect { get :service, params: { id: 1 } } - .to change { system.reload.system_token } - .from(current_system_token).to(new_system_token) - end - end shared_examples "does not update the old system's token" do it 'does not update the system token' do @@ -74,8 +65,6 @@ def require_product shared_examples 'creates a duplicate system' do it 'creates a new System (duplicate)' do - allow(SecureRandom).to receive(:uuid).and_return(new_system_token) - expect { get :service, params: { id: 1 } } .to change { System.get_by_credentials(system.login, system.password).count } .by(1) @@ -85,7 +74,6 @@ def require_product expect(duplicate_system).not_to eq(system) expect(duplicate_system.activations.count).to eq(system.activations.count) expect(duplicate_system.system_token).not_to eq(system.system_token) - expect(duplicate_system.system_token).to eq(new_system_token) end end @@ -182,8 +170,7 @@ def require_product let(:system) { create(:system, hostname: 'system') } include_examples 'does not create a duplicate system' - include_examples 'updates the system token' - include_examples 'responds with a new token' + include_examples "does not update the old system's token" end context 'when the system has a token and the header matches it' do @@ -193,8 +180,7 @@ def require_product let(:system) { create(:system, hostname: 'system', system_token: current_system_token) } include_examples 'does not create a duplicate system' - include_examples 'updates the system token' - include_examples 'responds with a new token' + include_examples "does not update the old system's token" end context 'when the system has a token and the header is blank' do @@ -208,7 +194,6 @@ def require_product include_examples "does not update the old system's token" include_examples 'creates a duplicate system' - include_examples 'responds with a new token' end context 'when the system has a token and the header does not match it' do diff --git a/spec/requests/api/connect/v3/systems/activations_controller_spec.rb b/spec/requests/api/connect/v3/systems/activations_controller_spec.rb index a4ff0a562..6a62ee2bd 100644 --- a/spec/requests/api/connect/v3/systems/activations_controller_spec.rb +++ b/spec/requests/api/connect/v3/systems/activations_controller_spec.rb @@ -69,5 +69,28 @@ expect(system.scc_synced_at).to be_nil end end + + context 'system token header' do + context 'when system token header is present in request' do + let(:token_headers) do + authenticated_headers.merge({ 'System-Token' => 'some_token' }) + end + + it 'sets system token in response headers' do + get url, headers: token_headers + expect(response.code).to eq '200' + expect(response.headers).to include('System-Token') + expect(response.headers['System-Token']).not_to be_nil + expect(response.headers['System-Token']).not_to be_empty + end + + it 'does not set system token header if no system token header in request' do + get url, headers: authenticated_headers + + expect(response.code).to eq '200' + expect(response.headers).not_to include('System-Token') + end + end + end end end diff --git a/spec/requests/api/connect/v3/systems/products_controller_spec.rb b/spec/requests/api/connect/v3/systems/products_controller_spec.rb index 457a77d65..9f6c47916 100644 --- a/spec/requests/api/connect/v3/systems/products_controller_spec.rb +++ b/spec/requests/api/connect/v3/systems/products_controller_spec.rb @@ -135,6 +135,26 @@ end end + shared_context 'activate with token in request headers' do + let(:payload) do + { + identifier: product.identifier, + version: product.version, + arch: product.arch, + token: regcode + } + end + + before { post url, headers: { 'System-Token' => 'existing_token' }.merge(headers), params: payload } + subject do + Struct.new(:body, :code, :headers).new( + JSON.parse(response.body, symbolize_names: true), + response.status, + response.headers + ) + end + end + context 'unknown subscription' do include_context 'with subscriptions' let(:regcode) { 'NOT-EXISTING-SUBSCRIPTION' } @@ -173,6 +193,17 @@ expect(activation.product).to eq(product) end end + + context 'token update after activation is success' do + let(:subscription) { create :subscription, :with_products } + let(:product) { subscription.products.first } + let(:regcode) { subscription.regcode } + + include_context 'activate with token in request headers' + its(:code) { is_expected.to eq(201) } + its(:headers) { is_expected.to include('System-Token') } + its(:headers['System-Token']) { is_expected.not_to eq('existing_token') } + end end end @@ -225,6 +256,18 @@ its(:body) { is_expected.to eq(serialized_json) } end + describe 'response header should contain token' do + subject { response } + + let(:token_headers) do + headers.merge({ 'System-Token' => 'some_token' }) + end + + before { get url, headers: token_headers, params: payload } + its(:code) { is_expected.to eq('200') } + its(:headers) { is_expected.to include('System-Token') } + end + describe 'response with "-" in version' do subject { response } @@ -339,6 +382,18 @@ system.reload end + it 'calls refresh_system_token after upgrade action when system token header is present' do + put url, headers: headers.merge('System-Token' => 'test_token'), params: payload + expect(response.code).to eq('201') + expect(response.headers).to include('System-Token') + expect(response.headers['System-Token']).not_to eq('test_token') + end + + it 'No update in token after upgrade action when system token header is absent' do + put url, headers: headers, params: payload + expect(response.code).to eq('201') + expect(response.headers).not_to include('System-Token') + end context 'new product' do its(:code) { is_expected.to eq('201') } its(:body) { is_expected.to eq(serialized_json) } diff --git a/spec/requests/api/connect/v3/systems/systems_controller_spec.rb b/spec/requests/api/connect/v3/systems/systems_controller_spec.rb index 95eede620..91f73d4f4 100644 --- a/spec/requests/api/connect/v3/systems/systems_controller_spec.rb +++ b/spec/requests/api/connect/v3/systems/systems_controller_spec.rb @@ -125,6 +125,25 @@ expect(system.reload.system_information_hash[:user_agent]).to be_nil end end + + context 'response header should contain token' do + let(:headers) { auth_header.merge('System-Token': 'existing-token') } + + it 'contains refreshed token in response' do + update_action + expect(response.headers).to include('System-Token') + expect(response.headers['System-Token']).not_to equal('existing-token') + end + end + + context 'response header should not contain token' do + let(:headers) { auth_header } + + it 'contains refreshed token in response' do + update_action + expect(response.headers).not_to include('System-Token') + end + end end describe '#deregister' do diff --git a/spec/requests/api/connect/v4/systems/products_controller_spec.rb b/spec/requests/api/connect/v4/systems/products_controller_spec.rb index 8bf9a64f4..6f960a941 100644 --- a/spec/requests/api/connect/v4/systems/products_controller_spec.rb +++ b/spec/requests/api/connect/v4/systems/products_controller_spec.rb @@ -140,4 +140,77 @@ end end end + + describe 'system token refresh' do + let(:system) { FactoryBot.create(:system, :with_activated_base_product) } + let(:headers) { auth_header.merge(version_header).merge('System-Token' => 'existing_token') } + + context 'token refresh for destroy action' do + let(:product) { FactoryBot.create(:product, :extension, :with_mirrored_repositories, :activated, system: system) } + let(:payload) { { identifier: product.identifier, version: product.version, arch: product.arch } } + + it 'refreshes system token when System-Token header is present' do + delete connect_systems_products_url, + headers: headers, + params: payload + + expect(response.status).to eq(200) + expect(response.headers).to include('System-Token') + expect(response.headers['System-Token']).not_to eq('existing_token') + end + + it 'does not refresh token when System-Token header is absent' do + headers_without_token = auth_header.merge(version_header) + expect_any_instance_of(described_class).not_to receive(:refresh_system_token) + + delete connect_systems_products_url, + headers: headers_without_token, + params: payload + + expect(response.status).to eq(200) + expect(response.headers).not_to include('System-Token') + end + end + + context 'token refresh for synchronize action' do + let(:path) { '/connect/systems/products/synchronize' } + + it 'refreshes system token when System-Token header is present' do + params = system.products.map do |product| + { + identifier: product.identifier, + version: product.version, + arch: product.arch, + release_type: product.release_type + } + end + post path, + params: { products: params }, + headers: headers + + expect(response.status).to eq(200) + expect(response.headers).to include('System-Token') + expect(response.headers['System-Token']).not_to eq('existing_token') + end + + it 'does not refresh token when System-Token header is absent' do + headers_without_token = auth_header.merge(version_header) + + params = system.products.map do |product| + { + identifier: product.identifier, + version: product.version, + arch: product.arch, + release_type: product.release_type + } + end + post path, + params: { products: params }, + headers: headers_without_token + + expect(response.status).to eq(200) + expect(response.headers).not_to include('System-Token') + end + end + end end