From 7c78fd3087a978f09c3181fb69646b6c9e04c4a7 Mon Sep 17 00:00:00 2001 From: Shivam Mishra Date: Thu, 28 Aug 2025 16:36:59 +0530 Subject: [PATCH] test: update specs to handle removed fields --- .../accounts/saml_settings_controller_spec.rb | 22 ++-- .../models/account_saml_settings_spec.rb | 123 +++--------------- 2 files changed, 27 insertions(+), 118 deletions(-) diff --git a/spec/enterprise/controllers/api/v1/accounts/saml_settings_controller_spec.rb b/spec/enterprise/controllers/api/v1/accounts/saml_settings_controller_spec.rb index d2bc7f8d8..44bf932ed 100644 --- a/spec/enterprise/controllers/api/v1/accounts/saml_settings_controller_spec.rb +++ b/spec/enterprise/controllers/api/v1/accounts/saml_settings_controller_spec.rb @@ -29,9 +29,7 @@ RSpec.describe 'Api::V1::Accounts::SamlSettings', type: :request do let(:saml_settings) do create(:account_saml_settings, account: account, - enabled: true, sso_url: 'https://idp.example.com/saml/sso', - attribute_mappings: { email: 'emailAddress' }, role_mappings: { 'Admins' => { 'role' => 1 } }) end @@ -45,9 +43,13 @@ RSpec.describe 'Api::V1::Accounts::SamlSettings', type: :request do as: :json expect(response).to have_http_status(:success) - expect(json_response[:enabled]).to be(true) expect(json_response[:sso_url]).to eq('https://idp.example.com/saml/sso') expect(json_response[:role_mappings]).to eq({ Admins: { role: 1 } }) + expect(json_response[:attribute_mappings]).to eq({ + email: 'http://schemas.xmlsoap.org/ws/2005/05/identity/claims/emailaddress', + first_name: 'http://schemas.xmlsoap.org/ws/2005/05/identity/claims/givenname', + last_name: 'http://schemas.xmlsoap.org/ws/2005/05/identity/claims/surname' + }) end end @@ -58,8 +60,11 @@ RSpec.describe 'Api::V1::Accounts::SamlSettings', type: :request do as: :json expect(response).to have_http_status(:success) - expect(json_response[:enabled]).to be(false) - expect(json_response[:attribute_mappings]).to eq({}) + expect(json_response[:attribute_mappings]).to eq({ + email: 'http://schemas.xmlsoap.org/ws/2005/05/identity/claims/emailaddress', + first_name: 'http://schemas.xmlsoap.org/ws/2005/05/identity/claims/givenname', + last_name: 'http://schemas.xmlsoap.org/ws/2005/05/identity/claims/surname' + }) expect(json_response[:role_mappings]).to eq({}) end end @@ -105,12 +110,9 @@ RSpec.describe 'Api::V1::Accounts::SamlSettings', type: :request do { saml_settings: { - enabled: true, sso_url: 'https://idp.example.com/saml/sso', certificate: cert.to_pem, sp_entity_id: 'chatwoot-production', - enforced_sso: false, - attribute_mappings: { email: 'emailAddress', name: 'displayName' }, role_mappings: { 'Admins' => { 'role' => 1 }, 'Users' => { 'role' => 0 } } } } @@ -136,7 +138,6 @@ RSpec.describe 'Api::V1::Accounts::SamlSettings', type: :request do expect(response).to have_http_status(:success) saml_settings = AccountSamlSettings.find_by(account: account) - expect(saml_settings.enabled).to be(true) expect(saml_settings.sso_url).to eq('https://idp.example.com/saml/sso') expect(saml_settings.role_mappings).to eq({ 'Admins' => { 'role' => 1 }, 'Users' => { 'role' => 0 } }) end @@ -177,7 +178,6 @@ RSpec.describe 'Api::V1::Accounts::SamlSettings', type: :request do let(:saml_settings) do create(:account_saml_settings, account: account, - enabled: false, sso_url: 'https://old.example.com/saml') end let(:update_params) do @@ -194,7 +194,6 @@ RSpec.describe 'Api::V1::Accounts::SamlSettings', type: :request do { saml_settings: { - enabled: true, sso_url: 'https://new.example.com/saml/sso', certificate: cert.to_pem, role_mappings: { 'NewGroup' => { 'custom_role_id' => 5 } } @@ -223,7 +222,6 @@ RSpec.describe 'Api::V1::Accounts::SamlSettings', type: :request do expect(response).to have_http_status(:success) saml_settings.reload - expect(saml_settings.enabled).to be(true) expect(saml_settings.sso_url).to eq('https://new.example.com/saml/sso') expect(saml_settings.role_mappings).to eq({ 'NewGroup' => { 'custom_role_id' => 5 } }) end diff --git a/spec/enterprise/models/account_saml_settings_spec.rb b/spec/enterprise/models/account_saml_settings_spec.rb index 8c0cce53c..0e931478c 100644 --- a/spec/enterprise/models/account_saml_settings_spec.rb +++ b/spec/enterprise/models/account_saml_settings_spec.rb @@ -11,124 +11,47 @@ RSpec.describe AccountSamlSettings, type: :model do end describe 'validations' do - context 'when enabled is false' do - it 'does not require sso_url, certificate, or sp_entity_id' do - settings = build(:account_saml_settings, account: account, enabled: false) - expect(settings).to be_valid - end + it 'requires sso_url' do + settings = build(:account_saml_settings, account: account, sso_url: nil) + expect(settings).not_to be_valid + expect(settings.errors[:sso_url]).to include("can't be blank") end - context 'when enabled is true' do - it 'requires sso_url' do - settings = build(:account_saml_settings, account: account, enabled: true, sso_url: nil) - expect(settings).not_to be_valid - expect(settings.errors[:sso_url]).to include("can't be blank") - end - - it 'requires certificate' do - settings = build(:account_saml_settings, account: account, enabled: true, certificate: nil) - expect(settings).not_to be_valid - expect(settings.errors[:certificate]).to include("can't be blank") - end - - it 'requires sp_entity_id' do - settings = build(:account_saml_settings, account: account, enabled: true, sp_entity_id: nil) - expect(settings).not_to be_valid - expect(settings.errors[:sp_entity_id]).to include("can't be blank") - end - end - end - - describe '#generate_certificate_fingerprint' do - context 'when certificate is valid' do - it 'generates SHA256 fingerprint automatically on save' do - settings = create(:account_saml_settings, account: account) - expect(settings.certificate_fingerprint).to be_present - expect(settings.certificate_fingerprint).to match(/\A[0-9A-F]{2}(:[0-9A-F]{2}){31}\z/) - end - - it 'regenerates fingerprint when certificate changes' do - settings = create(:account_saml_settings, account: account) - original_fingerprint = settings.certificate_fingerprint - - # Generate a new certificate - key = OpenSSL::PKey::RSA.new(2048) - cert = OpenSSL::X509::Certificate.new - cert.version = 2 - cert.serial = 2 # Different serial number - cert.subject = OpenSSL::X509::Name.parse('/C=US/ST=Test/L=Test/O=Test/CN=different.example.com') - cert.issuer = cert.subject - cert.public_key = key.public_key - cert.not_before = Time.zone.now - cert.not_after = cert.not_before + (365 * 24 * 60 * 60) - cert.sign(key, OpenSSL::Digest.new('SHA256')) - new_cert = cert.to_pem - - settings.update!(certificate: new_cert) - - expect(settings.certificate_fingerprint).not_to eq(original_fingerprint) - expect(settings.certificate_fingerprint).to be_present - end - - it 'does not regenerate fingerprint when other fields change' do - settings = create(:account_saml_settings, account: account) - original_fingerprint = settings.certificate_fingerprint - - settings.update!(sso_url: 'https://new.example.com/sso') - - expect(settings.certificate_fingerprint).to eq(original_fingerprint) - end + it 'requires certificate' do + settings = build(:account_saml_settings, account: account, certificate: nil) + expect(settings).not_to be_valid + expect(settings.errors[:certificate]).to include("can't be blank") end - context 'when certificate is invalid' do - it 'adds error and prevents save' do - settings = build(:account_saml_settings, account: account, certificate: 'invalid certificate') - expect(settings.save).to be_falsey - expect(settings.errors[:certificate]).to include('is not a valid X.509 certificate') - end - - it 'handles empty certificate gracefully' do - settings = build(:account_saml_settings, account: account, certificate: '', certificate_fingerprint: nil) - settings.save(validate: false) - expect(settings.certificate_fingerprint).to be_nil - end + it 'requires sp_entity_id' do + settings = build(:account_saml_settings, account: account, sp_entity_id: nil) + expect(settings).not_to be_valid + expect(settings.errors[:sp_entity_id]).to include("can't be blank") end end describe '#saml_enabled?' do - it 'returns true when enabled and required fields are present' do + it 'returns true when required fields are present' do settings = build(:account_saml_settings, account: account, - enabled: true, sso_url: 'https://example.com/sso', - certificate_fingerprint: 'AA:BB:CC') + certificate: 'valid-certificate') expect(settings.saml_enabled?).to be true end - it 'returns false when not enabled' do - settings = build(:account_saml_settings, - account: account, - enabled: false, - sso_url: 'https://example.com/sso', - certificate_fingerprint: 'AA:BB:CC') - expect(settings.saml_enabled?).to be false - end - it 'returns false when sso_url is missing' do settings = build(:account_saml_settings, account: account, - enabled: true, sso_url: nil, - certificate_fingerprint: 'AA:BB:CC') + certificate: 'valid-certificate') expect(settings.saml_enabled?).to be false end - it 'returns false when certificate_fingerprint is missing' do + it 'returns false when certificate is missing' do settings = build(:account_saml_settings, account: account, - enabled: true, sso_url: 'https://example.com/sso', - certificate_fingerprint: nil) + certificate: nil) expect(settings.saml_enabled?).to be false end end @@ -145,16 +68,4 @@ RSpec.describe AccountSamlSettings, type: :model do expect(settings.sp_entity_id_or_default).to eq(expected) end end - - describe 'scopes' do - describe '.enabled' do - it 'returns only enabled SAML settings' do - enabled_setting = create(:account_saml_settings, :enabled, account: account) - disabled_setting = create(:account_saml_settings, enabled: false) - - expect(described_class.enabled).to include(enabled_setting) - expect(described_class.enabled).not_to include(disabled_setting) - end - end - end end