From 30b2bd973025a727e66de5f3aeb61caac7de9c0b Mon Sep 17 00:00:00 2001 From: Thomas Schmidt Date: Tue, 12 Mar 2024 17:26:24 +0100 Subject: [PATCH 01/21] skip tests that don't work with sqlite --- spec/lib/rmt/cli/smt_importer_spec.rb | 2 +- spec/lib/rmt/lockfile_spec.rb | 2 +- spec/lib/rmt/scc_spec.rb | 2 +- spec/models/repository_spec.rb | 6 +++--- spec/rails_helper.rb | 6 ++++++ spec/services/repository_service_spec.rb | 2 +- 6 files changed, 13 insertions(+), 7 deletions(-) diff --git a/spec/lib/rmt/cli/smt_importer_spec.rb b/spec/lib/rmt/cli/smt_importer_spec.rb index d77404a52..17063bbc0 100644 --- a/spec/lib/rmt/cli/smt_importer_spec.rb +++ b/spec/lib/rmt/cli/smt_importer_spec.rb @@ -1,7 +1,7 @@ require 'rails_helper' require 'rmt/cli/smt_importer' -describe RMT::CLI::SMTImporter do +describe RMT::CLI::SMTImporter, :skip_sqlite do let(:data_dir) { File.join(Dir.pwd, 'spec/fixtures/files/csv') } let(:no_systems) { false } let(:importer) { described_class.new(data_dir, no_systems) } diff --git a/spec/lib/rmt/lockfile_spec.rb b/spec/lib/rmt/lockfile_spec.rb index d1b86f968..c06644bc3 100644 --- a/spec/lib/rmt/lockfile_spec.rb +++ b/spec/lib/rmt/lockfile_spec.rb @@ -3,7 +3,7 @@ RSpec.describe RMT::Lockfile do let(:lock_name) { nil } - describe '#lock' do + describe '#lock', :skip_sqlite do subject(:lock) { described_class.lock(lock_name) { nil } } context 'with an unnamed lock' do diff --git a/spec/lib/rmt/scc_spec.rb b/spec/lib/rmt/scc_spec.rb index a4732b0cd..94b2a739d 100644 --- a/spec/lib/rmt/scc_spec.rb +++ b/spec/lib/rmt/scc_spec.rb @@ -238,7 +238,7 @@ described_class.new.sync end - it 'removes existing predecessor associations' do + it 'removes existing predecessor associations', :skip_sqlite do expect { ProductPredecessorAssociation.find(existing_association.id) }.to raise_error(ActiveRecord::RecordNotFound) end end diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 3c958ac15..3c49cd6fa 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -146,7 +146,7 @@ expect(friendly_id).to eq('my-repo-100000') end - it 'appends with unicode within the chain' do + it 'appends with unicode within the chain', :skip_sqlite do create(:repository, friendly_id: 'my-repo') create(:repository, friendly_id: 'my-repö-1') expect(friendly_id).to eq('my-repo-2') @@ -165,13 +165,13 @@ expect(friendly_id).to eq('dümmy-repö') end - it 'appends with unicode within the chain' do + it 'appends with unicode within the chain', :skip_sqlite do create(:repository, friendly_id: 'dummy-repo') create(:repository, friendly_id: 'dümmy-repö-1') expect(friendly_id).to eq('dümmy-repö-2') end - it 'correctly appends' do + it 'correctly appends', :skip_sqlite do create(:repository, friendly_id: 'dummy-repo') create(:repository, friendly_id: 'dummy-repo-1') expect(friendly_id).to eq('dümmy-repö-2') diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index 5d71fea9f..f5b47b1e6 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -58,6 +58,12 @@ config.filter_rails_from_backtrace! # arbitrary gems may also be filtered via: # config.filter_gems_from_backtrace("gem name") + + # Skipping some tests when running with (experimental) sqlite backend. + # Some tests / code parts are using specific mysql behavior + if ActiveRecord::Base.connection.adapter_name == 'SQLite' + config.filter_run_excluding :skip_sqlite => true + end end Shoulda::Matchers.configure do |config| diff --git a/spec/services/repository_service_spec.rb b/spec/services/repository_service_spec.rb index 1387ea3b0..0035395b5 100644 --- a/spec/services/repository_service_spec.rb +++ b/spec/services/repository_service_spec.rb @@ -88,7 +88,7 @@ it('is not custom') { expect(repository.custom?).to eq(true) } - context 'already existing repositories with changing URL' do + context 'already existing repositories with changing URL', :skip_sqlite do subject(:repository) do service.create_repository!(product, url, attributes, custom: custom).reload url = 'https://foo.bar.com/bar/foo' From d380414ae3bd537c464ed06ffaaf5fd439bd4f1a Mon Sep 17 00:00:00 2001 From: Thomas Schmidt Date: Tue, 12 Mar 2024 17:28:16 +0100 Subject: [PATCH 02/21] add step to ci to run tests with sqlite --- .github/workflows/lint-unit.yml | 5 +++++ spec/rails_helper.rb | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/.github/workflows/lint-unit.yml b/.github/workflows/lint-unit.yml index 9ac44b6eb..66bf3f77c 100644 --- a/.github/workflows/lint-unit.yml +++ b/.github/workflows/lint-unit.yml @@ -83,6 +83,11 @@ jobs: run: | bundle exec rspec --format documentation + - name: Run core tests with sqlite + run: | + sed -i 's/adapter: mysql2/adapter: sqlite3/' config/rmt.yml + bundle exec rspec --format documentation + - name: Run PubCloud engines tests run: | bundle exec rake test:engines diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index f5b47b1e6..9c8bffe57 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -62,7 +62,7 @@ # Skipping some tests when running with (experimental) sqlite backend. # Some tests / code parts are using specific mysql behavior if ActiveRecord::Base.connection.adapter_name == 'SQLite' - config.filter_run_excluding :skip_sqlite => true + config.filter_run_excluding skip_sqlite: true end end From 8d99b17075e8545dd44428b811edc03d7c65fe91 Mon Sep 17 00:00:00 2001 From: Jesus Bermudez Velazquez Date: Thu, 7 Nov 2024 18:01:51 +0000 Subject: [PATCH 03/21] Add request to billing check The verification code will check the headers of @request object in order to access the content of SUMA headers to grant access Currently @request is nil coming from this code flow This add the request to the call --- .../instance_verification/billing_check_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/engines/instance_verification/app/controllers/instance_verification/billing_check_controller.rb b/engines/instance_verification/app/controllers/instance_verification/billing_check_controller.rb index 9d00eee26..0217e54d7 100644 --- a/engines/instance_verification/app/controllers/instance_verification/billing_check_controller.rb +++ b/engines/instance_verification/app/controllers/instance_verification/billing_check_controller.rb @@ -5,7 +5,7 @@ def check # belongs to a PAYG or BYOS instance verification_provider = InstanceVerification.provider.new( logger, - nil, + request, nil, params[:metadata] ) From a4697936fe2f02dc004468dd4f67a7975a1136ed Mon Sep 17 00:00:00 2001 From: Jesus Bermudez Velazquez Date: Fri, 29 Nov 2024 10:59:54 +0000 Subject: [PATCH 04/21] Add changes for v2.20 for pubcloud --- package/obs/rmt-server.changes | 3 +++ 1 file changed, 3 insertions(+) diff --git a/package/obs/rmt-server.changes b/package/obs/rmt-server.changes index ed80834ce..cb1e13732 100644 --- a/package/obs/rmt-server.changes +++ b/package/obs/rmt-server.changes @@ -8,6 +8,9 @@ Wed Oct 30 09:01:32 UTC 2024 - Natnael Getahun * rmt-server-pubcloud: * Fix LTSS product verification (bsc#1230154) * Fix activations check when no product info is available (bsc#1230157) + * Fix Azure SCC connection (bsc#1233314) + * Deny access to Azure Basic type images to LTSS + * Allow SLE Micro system to access free SLES repositories (bsc#1230419) ------------------------------------------------------------------- Wed Aug 21 15:28:43 UTC 2024 - Jesús Bermúdez Velázquez From bef95d6ce357a2da9f37ac4049b77655941dc64d Mon Sep 17 00:00:00 2001 From: Natnael Getahun Date: Fri, 29 Nov 2024 14:47:38 +0100 Subject: [PATCH 05/21] Update SCC host configuration to use environment variable --- config/rmt.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/rmt.yml b/config/rmt.yml index 811e44ff2..b1f7e6c6e 100644 --- a/config/rmt.yml +++ b/config/rmt.yml @@ -20,7 +20,7 @@ database_test: database: rmt_test scc: - host: https://scc.suse.com/connect + host: <%= ENV.fetch('SCC_HOST'){ 'https://scc.suse.com/connect' } username: <%= ENV['SCC_USERNAME'] %> password: <%= ENV['SCC_PASSWORD'] %> sync_systems: true From 608defaf0f4d45675219c118aa3a13aa0207850b Mon Sep 17 00:00:00 2001 From: Natnael Getahun Date: Fri, 29 Nov 2024 14:47:38 +0100 Subject: [PATCH 06/21] Update SCC host configuration to use environment variable --- config/rmt.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/rmt.yml b/config/rmt.yml index b1f7e6c6e..6a2df5b5c 100644 --- a/config/rmt.yml +++ b/config/rmt.yml @@ -20,7 +20,7 @@ database_test: database: rmt_test scc: - host: <%= ENV.fetch('SCC_HOST'){ 'https://scc.suse.com/connect' } + host: <%= ENV.fetch('SCC_HOST'){ 'https://scc.suse.com/connect' } %> username: <%= ENV['SCC_USERNAME'] %> password: <%= ENV['SCC_PASSWORD'] %> sync_systems: true From 2728fc73333596f9711e9862d71ab0a900d523f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jes=C3=BAs=20Berm=C3=BAdez=20Vel=C3=A1zquez?= Date: Mon, 2 Dec 2024 08:33:36 +0000 Subject: [PATCH 07/21] Update package/obs/rmt-server.changes Co-authored-by: Thomas Schmidt --- package/obs/rmt-server.changes | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package/obs/rmt-server.changes b/package/obs/rmt-server.changes index cb1e13732..cbee6e2ff 100644 --- a/package/obs/rmt-server.changes +++ b/package/obs/rmt-server.changes @@ -9,7 +9,7 @@ Wed Oct 30 09:01:32 UTC 2024 - Natnael Getahun * Fix LTSS product verification (bsc#1230154) * Fix activations check when no product info is available (bsc#1230157) * Fix Azure SCC connection (bsc#1233314) - * Deny access to Azure Basic type images to LTSS + * Deny access of Azure Basic type images to LTSS * Allow SLE Micro system to access free SLES repositories (bsc#1230419) ------------------------------------------------------------------- From acde3707577044a65b12f5910a7bd0a58498bc86 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jes=C3=BAs=20Berm=C3=BAdez=20Vel=C3=A1zquez?= Date: Mon, 2 Dec 2024 08:33:47 +0000 Subject: [PATCH 08/21] Update package/obs/rmt-server.changes Co-authored-by: Thomas Schmidt --- package/obs/rmt-server.changes | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package/obs/rmt-server.changes b/package/obs/rmt-server.changes index cbee6e2ff..16e598617 100644 --- a/package/obs/rmt-server.changes +++ b/package/obs/rmt-server.changes @@ -10,7 +10,7 @@ Wed Oct 30 09:01:32 UTC 2024 - Natnael Getahun * Fix activations check when no product info is available (bsc#1230157) * Fix Azure SCC connection (bsc#1233314) * Deny access of Azure Basic type images to LTSS - * Allow SLE Micro system to access free SLES repositories (bsc#1230419) + * Allow SLE Micro system to access SLES repositories (bsc#1230419) ------------------------------------------------------------------- Wed Aug 21 15:28:43 UTC 2024 - Jesús Bermúdez Velázquez From 0e430ea67dbf4eba08648a56734b7edabfbc6385 Mon Sep 17 00:00:00 2001 From: Jesus Bermudez Velazquez Date: Mon, 2 Dec 2024 10:34:30 +0000 Subject: [PATCH 09/21] Fix typo in comment --- .../lib/instance_verification/providers/example.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/engines/instance_verification/lib/instance_verification/providers/example.rb b/engines/instance_verification/lib/instance_verification/providers/example.rb index 393236e4b..ac1523135 100644 --- a/engines/instance_verification/lib/instance_verification/providers/example.rb +++ b/engines/instance_verification/lib/instance_verification/providers/example.rb @@ -52,7 +52,7 @@ def instance_identifier def allowed_extension? # method to check if a product (extension) meet the criteria - # to be acivated on SCC or not, i.e. LTSS in Azure Basic VM + # to be activated on SCC or not, i.e. LTSS in Azure Basic VM true end end From 7002ee6124d5c93c6bbe72c141ba1f35c2551fd3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jes=C3=BAs=20Berm=C3=BAdez=20Vel=C3=A1zquez?= Date: Mon, 2 Dec 2024 12:18:47 +0000 Subject: [PATCH 10/21] Update engines/instance_verification/lib/instance_verification/providers/example.rb Co-authored-by: Thomas Schmidt --- .../lib/instance_verification/providers/example.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/engines/instance_verification/lib/instance_verification/providers/example.rb b/engines/instance_verification/lib/instance_verification/providers/example.rb index ac1523135..e4379c39b 100644 --- a/engines/instance_verification/lib/instance_verification/providers/example.rb +++ b/engines/instance_verification/lib/instance_verification/providers/example.rb @@ -51,7 +51,7 @@ def instance_identifier end def allowed_extension? - # method to check if a product (extension) meet the criteria + # method to check if a product (extension) meets the criteria # to be activated on SCC or not, i.e. LTSS in Azure Basic VM true end From 798ecf9f02d5c2a2c28216f7a4dd7c060a1107f7 Mon Sep 17 00:00:00 2001 From: "parag.jain" Date: Mon, 2 Dec 2024 20:13:17 +0530 Subject: [PATCH 11/21] skip rotation for read Apis --- .../api/connect/base_controller.rb | 8 ++++++++ .../connect/v3/systems/products_controller.rb | 1 + .../connect/v3/systems/systems_controller.rb | 1 + .../connect/v4/systems/products_controller.rb | 1 + app/controllers/application_controller.rb | 6 +++--- .../api/connect/base_controller_spec.rb | 19 +++---------------- 6 files changed, 17 insertions(+), 19 deletions(-) diff --git a/app/controllers/api/connect/base_controller.rb b/app/controllers/api/connect/base_controller.rb index 1fbeaff07..1983fb267 100644 --- a/app/controllers/api/connect/base_controller.rb +++ b/app/controllers/api/connect/base_controller.rb @@ -44,4 +44,12 @@ def authenticate_with_token end end + def system_token_header + headers[SYSTEM_TOKEN_HEADER] = @system.system_token + end + + def refresh_system_token + @system.update(system_token: SecureRandom.uuid) + system_token_header + 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 2545a9e92..fcad5de58 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..c92df74d5 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[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..ba11c3a75 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -21,11 +21,11 @@ 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) + @system.update(last_seen_at: Time.zone.now) headers[SYSTEM_TOKEN_HEADER] = @system.system_token - # only update last_seen_at each 3 minutes, + # 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 @system.touch(:last_seen_at) diff --git a/spec/requests/api/connect/base_controller_spec.rb b/spec/requests/api/connect/base_controller_spec.rb index 70ef9aaff..76b1f4b6b 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,7 +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 } @@ -85,7 +75,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 +171,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 +181,8 @@ 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 +196,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 From 3f5a6e8d3d8fcf2b8ff7503e26a37652718642dd Mon Sep 17 00:00:00 2001 From: Thomas Schmidt Date: Wed, 4 Dec 2024 00:09:08 +0100 Subject: [PATCH 12/21] fix rubocop --- app/controllers/application_controller.rb | 2 +- spec/requests/api/connect/base_controller_spec.rb | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index ba11c3a75..bb7504888 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -25,7 +25,7 @@ def authenticate_system(skip_on_duplicated: false) if system_tokens_enabled? && request.headers.key?(SYSTEM_TOKEN_HEADER) @system.update(last_seen_at: Time.zone.now) headers[SYSTEM_TOKEN_HEADER] = @system.system_token - # only update last_seen_at each 3 minutes, + # 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 @system.touch(:last_seen_at) diff --git a/spec/requests/api/connect/base_controller_spec.rb b/spec/requests/api/connect/base_controller_spec.rb index 76b1f4b6b..8832909ea 100644 --- a/spec/requests/api/connect/base_controller_spec.rb +++ b/spec/requests/api/connect/base_controller_spec.rb @@ -65,7 +65,6 @@ def require_product shared_examples 'creates a duplicate system' do it 'creates a new System (duplicate)' do - expect { get :service, params: { id: 1 } } .to change { System.get_by_credentials(system.login, system.password).count } .by(1) @@ -182,7 +181,6 @@ def require_product include_examples 'does not create a duplicate system' include_examples "does not update the old system's token" - end context 'when the system has a token and the header is blank' do From 59131b6353fd1b2c29a17a4f158e34306dbfcf8f Mon Sep 17 00:00:00 2001 From: "parag.jain" Date: Wed, 4 Dec 2024 12:22:50 +0530 Subject: [PATCH 13/21] add testcases supporting token rotation for write api; add testcases for header token addition in read api --- app/controllers/application_controller.rb | 2 +- .../v3/systems/activations_controller_spec.rb | 23 ++++++ .../v3/systems/products_controller_spec.rb | 55 ++++++++++++++ .../v3/systems/systems_controller_spec.rb | 19 +++++ .../v4/systems/products_controller_spec.rb | 73 +++++++++++++++++++ 5 files changed, 171 insertions(+), 1 deletion(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index bb7504888..63458615a 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -24,7 +24,7 @@ def authenticate_system(skip_on_duplicated: false) # that supports this feature. if system_tokens_enabled? && request.headers.key?(SYSTEM_TOKEN_HEADER) @system.update(last_seen_at: Time.zone.now) - headers[SYSTEM_TOKEN_HEADER] = @system.system_token + 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/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 From c0be53cd3e758fa7ba9b026d6d5e64c332f81c31 Mon Sep 17 00:00:00 2001 From: "parag.jain" Date: Wed, 4 Dec 2024 15:41:57 +0530 Subject: [PATCH 14/21] add system token enable check --- app/controllers/api/connect/base_controller.rb | 6 ++++-- .../api/connect/v4/systems/products_controller.rb | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/app/controllers/api/connect/base_controller.rb b/app/controllers/api/connect/base_controller.rb index 1983fb267..84de46574 100644 --- a/app/controllers/api/connect/base_controller.rb +++ b/app/controllers/api/connect/base_controller.rb @@ -49,7 +49,9 @@ def system_token_header end def refresh_system_token - @system.update(system_token: SecureRandom.uuid) - system_token_header + if system_tokens_enabled? + @system.update(system_token: SecureRandom.uuid) + system_token_header + end end end diff --git a/app/controllers/api/connect/v4/systems/products_controller.rb b/app/controllers/api/connect/v4/systems/products_controller.rb index c92df74d5..d4c87adfc 100644 --- a/app/controllers/api/connect/v4/systems/products_controller.rb +++ b/app/controllers/api/connect/v4/systems/products_controller.rb @@ -1,6 +1,6 @@ class Api::Connect::V4::Systems::ProductsController < Api::Connect::V3::Systems::ProductsController - after_action :refresh_system_token, only: %i[synchronize destroy], if: -> { request.headers.key?(SYSTEM_TOKEN_HEADER) } + 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) From 07961c69f578ae92e5b5163d2a8a1221753d65cf Mon Sep 17 00:00:00 2001 From: "parag.jain" Date: Thu, 5 Dec 2024 18:26:05 +0530 Subject: [PATCH 15/21] Add entry to .changes file for skipping system token rotation in read-only APIs --- package/obs/rmt-server.changes | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package/obs/rmt-server.changes b/package/obs/rmt-server.changes index ed80834ce..6e048aba3 100644 --- a/package/obs/rmt-server.changes +++ b/package/obs/rmt-server.changes @@ -8,7 +8,7 @@ Wed Oct 30 09:01:32 UTC 2024 - Natnael Getahun * rmt-server-pubcloud: * Fix LTSS product verification (bsc#1230154) * Fix activations check when no product info is available (bsc#1230157) - + * Skip system token rotation in read-only APIs ------------------------------------------------------------------- Wed Aug 21 15:28:43 UTC 2024 - Jesús Bermúdez Velázquez From 992c0a0b23121a70f82af8b26ffebdd7cb797ac2 Mon Sep 17 00:00:00 2001 From: "parag.jain" Date: Wed, 11 Dec 2024 13:02:53 +0530 Subject: [PATCH 16/21] Refactor SUSE repo cleanup and local path generation logic supporting akamai url --- app/models/repository.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/app/models/repository.rb b/app/models/repository.rb index 6a7719aa2..6fedd7945 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -23,14 +23,16 @@ class Repository < ApplicationRecord class << self def remove_suse_repos_without_tokens! - where(auth_token: nil).where('external_url LIKE ?', 'https://updates.suse.com%').delete_all + where(auth_token: nil) + .where("external_url LIKE 'https://updates.suse.com%' OR external_url LIKE 'https://dl.suse.com%'") + .delete_all end # Mangles remote repo URL to make a nicer local path, see specs for examples def make_local_path(url) uri = URI(url) path = uri.path.to_s - path.gsub!(%r{^/repo}, '') if (uri.hostname == 'updates.suse.com') + path.gsub!(%r{^/repo}, '') if (uri.hostname == 'updates.suse.com' || uri.hostname == 'dl.suse.com') (path == '') ? '/' : path end From caa983103853f35b895d95dd7290c8b0bbc41834 Mon Sep 17 00:00:00 2001 From: "parag.jain" Date: Thu, 12 Dec 2024 17:50:37 +0530 Subject: [PATCH 17/21] remove function remove_suse_repos_without_tokens!; --- app/models/repository.rb | 10 ++-------- lib/rmt/scc.rb | 6 ------ spec/lib/rmt/scc_spec.rb | 4 ---- 3 files changed, 2 insertions(+), 18 deletions(-) diff --git a/app/models/repository.rb b/app/models/repository.rb index 6fedd7945..ff09674e0 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -21,18 +21,12 @@ class Repository < ApplicationRecord before_destroy :ensure_destroy_possible class << self - - def remove_suse_repos_without_tokens! - where(auth_token: nil) - .where("external_url LIKE 'https://updates.suse.com%' OR external_url LIKE 'https://dl.suse.com%'") - .delete_all - end - # Mangles remote repo URL to make a nicer local path, see specs for examples def make_local_path(url) uri = URI(url) path = uri.path.to_s - path.gsub!(%r{^/repo}, '') if (uri.hostname == 'updates.suse.com' || uri.hostname == 'dl.suse.com') + # drop '/repo' from SLE11 paths, to avoid double /repo/repo in local storage path. + path.gsub!(%r{^/repo/\$RCE/}, '/$RCE/') (path == '') ? '/' : path end diff --git a/lib/rmt/scc.rb b/lib/rmt/scc.rb index 59fccdf2a..63cbe5106 100644 --- a/lib/rmt/scc.rb +++ b/lib/rmt/scc.rb @@ -24,9 +24,6 @@ def sync data.each { |item| migration_paths(item) } update_repositories(scc_api_client.list_repositories) - - Repository.remove_suse_repos_without_tokens! - update_subscriptions(scc_api_client.list_subscriptions) end @@ -68,9 +65,6 @@ def import(path) data.each { |item| migration_paths(item) } update_repositories(JSON.parse(File.read(File.join(path, 'organizations_repositories.json')), symbolize_names: true)) - - Repository.remove_suse_repos_without_tokens! - update_subscriptions(JSON.parse(File.read(File.join(path, 'organizations_subscriptions.json')), symbolize_names: true)) end diff --git a/spec/lib/rmt/scc_spec.rb b/spec/lib/rmt/scc_spec.rb index 802cf1c8a..4b62370fb 100644 --- a/spec/lib/rmt/scc_spec.rb +++ b/spec/lib/rmt/scc_spec.rb @@ -290,10 +290,6 @@ def scc expect { suse_repo_with_token.reload }.not_to raise_error end - it 'SUSE repos without auth_tokens are removed' do - expect { suse_repo_without_token.reload }.to raise_error(ActiveRecord::RecordNotFound) - end - it 'other repos without auth_tokens persist' do expect { other_repo_without_token.reload }.not_to raise_error end From d81ee4b82a8591e2f56203236418116524a6fb74 Mon Sep 17 00:00:00 2001 From: "parag.jain" Date: Mon, 16 Dec 2024 14:08:21 +0530 Subject: [PATCH 18/21] Refactor SUSE repo cleanup and local path generation logic supporting akamai url --- app/models/repository.rb | 5 +++++ lib/rmt/scc.rb | 6 ++++++ spec/lib/rmt/scc_spec.rb | 7 ++++++- 3 files changed, 17 insertions(+), 1 deletion(-) diff --git a/app/models/repository.rb b/app/models/repository.rb index ff09674e0..347ec5548 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -21,6 +21,11 @@ class Repository < ApplicationRecord before_destroy :ensure_destroy_possible class << self + + def remove_suse_repos_without_tokens! + where(auth_token: nil).where("external_url LIKE '%.suse.com%'").where(installer_updates: 0).delete_all + end + # Mangles remote repo URL to make a nicer local path, see specs for examples def make_local_path(url) uri = URI(url) diff --git a/lib/rmt/scc.rb b/lib/rmt/scc.rb index 63cbe5106..59fccdf2a 100644 --- a/lib/rmt/scc.rb +++ b/lib/rmt/scc.rb @@ -24,6 +24,9 @@ def sync data.each { |item| migration_paths(item) } update_repositories(scc_api_client.list_repositories) + + Repository.remove_suse_repos_without_tokens! + update_subscriptions(scc_api_client.list_subscriptions) end @@ -65,6 +68,9 @@ def import(path) data.each { |item| migration_paths(item) } update_repositories(JSON.parse(File.read(File.join(path, 'organizations_repositories.json')), symbolize_names: true)) + + Repository.remove_suse_repos_without_tokens! + update_subscriptions(JSON.parse(File.read(File.join(path, 'organizations_subscriptions.json')), symbolize_names: true)) end diff --git a/spec/lib/rmt/scc_spec.rb b/spec/lib/rmt/scc_spec.rb index 496730e56..a62077d00 100644 --- a/spec/lib/rmt/scc_spec.rb +++ b/spec/lib/rmt/scc_spec.rb @@ -261,7 +261,8 @@ :repository, :with_products, auth_token: nil, - external_url: 'https://example.com/repos/not/updates.suse.com/' + external_url: 'https://installer-updates.suse.com/repos/not/updates', + installer_updates: true ) end @@ -290,6 +291,10 @@ def scc expect { suse_repo_with_token.reload }.not_to raise_error end + it 'SUSE repos without auth_tokens are removed' do + expect { suse_repo_without_token.reload }.to raise_error(ActiveRecord::RecordNotFound) + end + it 'other repos without auth_tokens persist' do expect { other_repo_without_token.reload }.not_to raise_error end From 3503590a1b8e7128b707fd32e15dc1eba985d687 Mon Sep 17 00:00:00 2001 From: "parag.jain" Date: Mon, 16 Dec 2024 16:41:35 +0530 Subject: [PATCH 19/21] add check for custom repos --- app/models/repository.rb | 2 +- spec/lib/rmt/scc_spec.rb | 14 ++++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/app/models/repository.rb b/app/models/repository.rb index 347ec5548..c65c602cd 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -23,7 +23,7 @@ class Repository < ApplicationRecord class << self def remove_suse_repos_without_tokens! - where(auth_token: nil).where("external_url LIKE '%.suse.com%'").where(installer_updates: 0).delete_all + where(auth_token: nil).where("external_url LIKE '%.suse.com%'").where(installer_updates: 0).where.not(scc_id: nil).delete_all end # Mangles remote repo URL to make a nicer local path, see specs for examples diff --git a/spec/lib/rmt/scc_spec.rb b/spec/lib/rmt/scc_spec.rb index a62077d00..f1e3b9d61 100644 --- a/spec/lib/rmt/scc_spec.rb +++ b/spec/lib/rmt/scc_spec.rb @@ -265,6 +265,16 @@ installer_updates: true ) end + let!(:custom_repo) do + FactoryBot.create( + :repository, + :with_products, + auth_token: nil, + external_url: 'http://customer.com/stuff.suse.com/x86', + installer_updates: false, + scc_id: nil + ) + end before do # to prevent 'does not implement' verifying doubles error @@ -298,6 +308,10 @@ def scc it 'other repos without auth_tokens persist' do expect { other_repo_without_token.reload }.not_to raise_error end + + it 'custom repos without auth_tokens persist' do + expect { custom_repo.reload }.not_to raise_error + end end describe '#export' do From 4aa1e0e1151a1939430e781adfb82c1872ff3de2 Mon Sep 17 00:00:00 2001 From: "parag.jain" Date: Mon, 16 Dec 2024 18:48:16 +0530 Subject: [PATCH 20/21] Enable RMT to handle the new dl.suse.com CDN domain --- package/obs/rmt-server.changes | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/package/obs/rmt-server.changes b/package/obs/rmt-server.changes index f587e6685..2cc735f92 100644 --- a/package/obs/rmt-server.changes +++ b/package/obs/rmt-server.changes @@ -11,7 +11,8 @@ Wed Oct 30 09:01:32 UTC 2024 - Natnael Getahun * Fix Azure SCC connection (bsc#1233314) * Deny access of Azure Basic type images to LTSS * Allow SLE Micro system to access SLES repositories (bsc#1230419) - * Skip system token rotation in read-only APIs + * Skip system token rotation in read-only APIs + * Enable RMT to handle the new dl.suse.com CDN domain. ------------------------------------------------------------------- Wed Aug 21 15:28:43 UTC 2024 - Jesús Bermúdez Velázquez From 192969892576986b04353fffa0909ce87cc2225c Mon Sep 17 00:00:00 2001 From: Thomas Schmidt Date: Mon, 16 Dec 2024 16:24:07 +0100 Subject: [PATCH 21/21] add bugzilla reference --- package/obs/rmt-server.changes | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package/obs/rmt-server.changes b/package/obs/rmt-server.changes index 2cc735f92..c84e3dbf6 100644 --- a/package/obs/rmt-server.changes +++ b/package/obs/rmt-server.changes @@ -12,7 +12,7 @@ Wed Oct 30 09:01:32 UTC 2024 - Natnael Getahun * Deny access of Azure Basic type images to LTSS * Allow SLE Micro system to access SLES repositories (bsc#1230419) * Skip system token rotation in read-only APIs - * Enable RMT to handle the new dl.suse.com CDN domain. + * Enable RMT to handle the new dl.suse.com CDN domain (bsc#1234641) ------------------------------------------------------------------- Wed Aug 21 15:28:43 UTC 2024 - Jesús Bermúdez Velázquez