diff --git a/enterprise/app/builders/saml_user_builder.rb b/enterprise/app/builders/saml_user_builder.rb index 568225201..75272e7cd 100644 --- a/enterprise/app/builders/saml_user_builder.rb +++ b/enterprise/app/builders/saml_user_builder.rb @@ -85,7 +85,7 @@ class SamlUserBuilder if matching_mapping['role'] account_user.update(role: matching_mapping['role']) - elsif matching_mapping['custom_role_id'] + elsif matching_mapping['custom_role_id'] && account.feature_enabled?('custom_roles') account_user.update(custom_role_id: matching_mapping['custom_role_id']) end end diff --git a/enterprise/app/controllers/api/v1/accounts/custom_roles_controller.rb b/enterprise/app/controllers/api/v1/accounts/custom_roles_controller.rb index 2d6823b90..a45fba1d9 100644 --- a/enterprise/app/controllers/api/v1/accounts/custom_roles_controller.rb +++ b/enterprise/app/controllers/api/v1/accounts/custom_roles_controller.rb @@ -1,4 +1,5 @@ class Api::V1::Accounts::CustomRolesController < Api::V1::Accounts::EnterpriseAccountsController + before_action :ensure_custom_roles_feature_enabled before_action :fetch_custom_role, only: [:show, :update, :destroy] before_action :check_authorization @@ -28,4 +29,8 @@ class Api::V1::Accounts::CustomRolesController < Api::V1::Accounts::EnterpriseAc def fetch_custom_role @custom_role = Current.account.custom_roles.find_by(id: params[:id]) end + + def ensure_custom_roles_feature_enabled + raise Pundit::NotAuthorizedError unless Current.account.feature_enabled?('custom_roles') + end end diff --git a/enterprise/app/controllers/enterprise/api/v1/accounts/agents_controller.rb b/enterprise/app/controllers/enterprise/api/v1/accounts/agents_controller.rb index 02d027b58..5652e02e1 100644 --- a/enterprise/app/controllers/enterprise/api/v1/accounts/agents_controller.rb +++ b/enterprise/app/controllers/enterprise/api/v1/accounts/agents_controller.rb @@ -14,6 +14,10 @@ module Enterprise::Api::V1::Accounts::AgentsController private def associate_agent_with_custom_role + # Custom roles are a premium feature; block assigning one when the feature is disabled, + # but still allow clearing a stale custom_role_id left over from before a downgrade. + return if params[:custom_role_id].present? && !Current.account.feature_enabled?('custom_roles') + @agent.current_account_user.update!(custom_role_id: params[:custom_role_id]) end end diff --git a/spec/enterprise/builders/saml_user_builder_spec.rb b/spec/enterprise/builders/saml_user_builder_spec.rb index d3f3cb601..1fabe58d9 100644 --- a/spec/enterprise/builders/saml_user_builder_spec.rb +++ b/spec/enterprise/builders/saml_user_builder_spec.rb @@ -218,11 +218,19 @@ RSpec.describe SamlUserBuilder do before { saml_settings } - it 'applies custom role based on SAML groups' do + it 'applies custom role based on SAML groups when the custom_roles feature is enabled' do + account.enable_features!('custom_roles') + user = builder.perform account_user = AccountUser.find_by(user: user, account: account) expect(account_user.custom_role_id).to eq(custom_role.id) end + + it 'ignores the custom role mapping when the custom_roles feature is disabled' do + user = builder.perform + account_user = AccountUser.find_by(user: user, account: account) + expect(account_user.custom_role_id).to be_nil + end end context 'when user is not in any mapped groups' do diff --git a/spec/enterprise/controllers/api/v1/accounts/custom_roles_controller_spec.rb b/spec/enterprise/controllers/api/v1/accounts/custom_roles_controller_spec.rb index 5c6d96a7b..3c771ce2b 100644 --- a/spec/enterprise/controllers/api/v1/accounts/custom_roles_controller_spec.rb +++ b/spec/enterprise/controllers/api/v1/accounts/custom_roles_controller_spec.rb @@ -6,6 +6,8 @@ RSpec.describe 'Custom Roles API', type: :request do let!(:agent) { create(:user, account: account, role: :agent) } let!(:custom_role) { create(:custom_role, account: account, name: 'Manager') } + before { account.enable_features!('custom_roles') } + describe 'GET #index' do context 'when it is an authenticated administrator' do it 'returns all custom roles in the account' do diff --git a/spec/enterprise/controllers/enterprise/api/v1/accounts/agents_controller_spec.rb b/spec/enterprise/controllers/enterprise/api/v1/accounts/agents_controller_spec.rb index fbffe088e..2331818f0 100644 --- a/spec/enterprise/controllers/enterprise/api/v1/accounts/agents_controller_spec.rb +++ b/spec/enterprise/controllers/enterprise/api/v1/accounts/agents_controller_spec.rb @@ -5,6 +5,8 @@ RSpec.describe 'Enterprise Agents API', type: :request do let(:admin) { create(:user, account: account, role: :administrator) } let!(:custom_role) { create(:custom_role, account: account) } + before { account.enable_features!('custom_roles') } + describe 'POST /api/v1/accounts/{account.id}/agents' do let(:params) { { email: 'test@example.com', name: 'Test User', role: 'agent', custom_role_id: custom_role.id } } @@ -35,5 +37,22 @@ RSpec.describe 'Enterprise Agents API', type: :request do expect(JSON.parse(response.body)['custom_role_id']).to eq(custom_role.id) end end + + context 'when the custom_roles feature is disabled' do + before do + other_agent.account_users.first.update!(custom_role_id: custom_role.id) + account.disable_features!('custom_roles') + end + + it 'ignores assignment but still allows clearing a stale custom role' do + put "/api/v1/accounts/#{account.id}/agents/#{other_agent.id}", + headers: admin.create_new_auth_token, params: { custom_role_id: custom_role.id }, as: :json + expect(other_agent.account_users.first.reload.custom_role_id).to eq(custom_role.id) + + put "/api/v1/accounts/#{account.id}/agents/#{other_agent.id}", + headers: admin.create_new_auth_token, params: { custom_role_id: nil }, as: :json + expect(other_agent.account_users.first.reload.custom_role_id).to be_nil + end + end end end