Merge pull request #1254 from SUSE/skip_token_rotation

skip rotation for read Apis

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

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

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?

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)

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

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

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

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

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

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