Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

skip rotation for read Apis #1254

Merged
merged 7 commits into from
Dec 5, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions app/controllers/api/connect/base_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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) }
digitaltom marked this conversation as resolved.
Show resolved Hide resolved

def activate
create_product_activation
Expand Down
Original file line number Diff line number Diff line change
@@ -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?
Expand Down
Original file line number Diff line number Diff line change
@@ -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) }
digitaltom marked this conversation as resolved.
Show resolved Hide resolved
def destroy
if @product.base?
raise ActionController::TranslatedError.new(N_('The product "%s" is a base product and cannot be deactivated'), @product.name)
Expand Down
6 changes: 3 additions & 3 deletions app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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 [email protected]_seen_at || @system.last_seen_at < 3.minutes.ago
Expand Down
19 changes: 2 additions & 17 deletions spec/requests/api/connect/base_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -55,15 +55,6 @@ def require_product
end
end

shared_examples 'updates the system token' do
digitaltom marked this conversation as resolved.
Show resolved Hide resolved
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
Expand All @@ -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)
Expand All @@ -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

Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
55 changes: 55 additions & 0 deletions spec/requests/api/connect/v3/systems/products_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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' }
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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 }

Expand Down Expand Up @@ -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) }
Expand Down
19 changes: 19 additions & 0 deletions spec/requests/api/connect/v3/systems/systems_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
73 changes: 73 additions & 0 deletions spec/requests/api/connect/v4/systems/products_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Loading