From 0252b339514c6dbf45a039a4125117d6bad9ad6f Mon Sep 17 00:00:00 2001 From: Joe Rafaniello Date: Tue, 22 Jul 2025 16:12:44 -0400 Subject: [PATCH 1/4] Bypass feature validation for dynamic tenant features We expect a one to one feature assignment with existing product features. This doesn't work with dynamic tenant features. This dynamic tenant feature was only ever implemented for tenant quotas. Related to: https://github.com/ManageIQ/manageiq-ui-classic/pull/5123 https://github.com/ManageIQ/manageiq-ui-classic/pull/5129 https://github.com/ManageIQ/manageiq-ui-classic/pull/5142 Fixes: https://github.com/ManageIQ/manageiq-ui-classic/issues/9512 --- app/controllers/ops_controller/ops_rbac.rb | 2 ++ app/helpers/application_helper.rb | 16 +++++++++++----- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/app/controllers/ops_controller/ops_rbac.rb b/app/controllers/ops_controller/ops_rbac.rb index 21137282e5b..fb1a370aaff 100644 --- a/app/controllers/ops_controller/ops_rbac.rb +++ b/app/controllers/ops_controller/ops_rbac.rb @@ -17,6 +17,8 @@ def role_allows?(**options) end options[:feature] = MiqProductFeature.tenant_identifier(options[:feature], id) + # dynamic tenant feature identifiers need to bypass feature validation + options[:skip_feature_validation] = true end super(**options) diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index f2961b5c1cd..71fbf779994 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -104,6 +104,16 @@ def role_allows?(**options) return false end + # ops_rbac role_allows's dynamic tenant features are supported in rbac but not + # with direct lookup in validate_features so we skip it. + validate_features(features) unless !!options.delete(:skip_feature_validation) + + Rbac.role_allows?(:user => User.current_user, **options) rescue false + end + module_function :role_allows? + public :role_allows? + + def validate_features(features) # Detect if queried features are missing from the database and possibly invalid if !Rails.env.production? && features.detect { |feature| !MiqProductFeature.feature_exists?(feature) } message = "#{__method__} no feature was found with identifier: #{features.inspect}. Correct the identifier or add it to miq_product_features.yml." @@ -114,12 +124,8 @@ def role_allows?(**options) raise("#{message} Note: detected features: #{identifiers.inspect}") end end - - Rbac.role_allows?(:user => User.current_user, **options) rescue false end - - module_function :role_allows? - public :role_allows? + module_function :validate_features # NB: This differs from controller_for_model; until they're unified, # make sure you have the right one. From 8b7c0949243eb65750eb3c5fa7d7f28ba5736cc6 Mon Sep 17 00:00:00 2001 From: Joe Rafaniello Date: Wed, 13 Aug 2025 14:05:34 -0400 Subject: [PATCH 2/4] Add a test for Settings, Access Control, Tenant Add and Delete This recreates the issue found in https://github.com/ManageIQ/manageiq-ui-classic/issues/9512 --- .../settings_access_control.cy.js | 70 +++++++++++++++++++ 1 file changed, 70 insertions(+) create mode 100644 cypress/e2e/ui/Settings/Application-Settings/settings_access_control.cy.js diff --git a/cypress/e2e/ui/Settings/Application-Settings/settings_access_control.cy.js b/cypress/e2e/ui/Settings/Application-Settings/settings_access_control.cy.js new file mode 100644 index 00000000000..90ea65de2c5 --- /dev/null +++ b/cypress/e2e/ui/Settings/Application-Settings/settings_access_control.cy.js @@ -0,0 +1,70 @@ +/* eslint-disable no-undef */ + +const textConstants = { + // Menu options + settingsMenuOption: 'Settings', + appSettingsMenuOption: 'Application Settings', + toolBarConfigMenu: 'Configuration', + + // added item information + initialTenantName: 'Test-name', + initialTenantDescription: 'test description', + + // List items + accessControlAccordion: 'Access Control', + + // flash message assertions + flashMessageOperationAdded: 'added', + flashMessageOperationDeleted: 'delete', + flashTypeSuccess: 'success', + + // Configuration menu options and browser alert text snippets + deleteItem: 'Delete this item', +} + +const { + accessControlAccordion, + appSettingsMenuOption, + deleteItem, + flashMessageOperationAdded, + flashMessageOperationDeleted, + flashTypeSuccess, + initialTenantDescription, + initialTenantName, + settingsMenuOption, + toolBarConfigMenu, +} = textConstants; + +describe('Settings > Application Settings > Access Control', () => { + beforeEach(() => { + cy.login(); + cy.menu(settingsMenuOption, appSettingsMenuOption); + cy.accordion(accessControlAccordion); + }); + + it('should be able to create and delete a tenant', () => { + cy.selectAccordionItem([ + /^ManageIQ Region/, + 'Tenants', + 'My Company', + ]); + + cy.toolbar(toolBarConfigMenu, 'Add child Tenant to this Tenant'); + cy.getFormInputFieldById('name').type(initialTenantName); + cy.getFormInputFieldById('description').type(initialTenantDescription); + cy.getFormFooterButtonByType('Add', 'submit').click(); + cy.expect_flash(flashTypeSuccess, flashMessageOperationAdded); + cy.selectAccordionItem([ + /^ManageIQ Region/, + 'Tenants', + 'My Company', + initialTenantName + ]); + + cy.expect_browser_confirm_with_text({ + confirmTriggerFn: () => cy.toolbar(toolBarConfigMenu, deleteItem), + containsText: deleteItem, + }); + cy.expect_flash(flashTypeSuccess, flashMessageOperationDeleted); + }); +}); From 4b749f2dd153c98490a6b6d9c8456b190b0b77ea Mon Sep 17 00:00:00 2001 From: Joe Rafaniello Date: Thu, 14 Aug 2025 16:40:59 -0400 Subject: [PATCH 3/4] Reuse flashClassMap from just merged #9554 --- .../Application-Settings/settings_access_control.cy.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/cypress/e2e/ui/Settings/Application-Settings/settings_access_control.cy.js b/cypress/e2e/ui/Settings/Application-Settings/settings_access_control.cy.js index 90ea65de2c5..340d1d6d642 100644 --- a/cypress/e2e/ui/Settings/Application-Settings/settings_access_control.cy.js +++ b/cypress/e2e/ui/Settings/Application-Settings/settings_access_control.cy.js @@ -1,4 +1,5 @@ /* eslint-disable no-undef */ +import { flashClassMap } from '../../../../support/assertions/assertion_constants'; const textConstants = { // Menu options @@ -16,7 +17,6 @@ const textConstants = { // flash message assertions flashMessageOperationAdded: 'added', flashMessageOperationDeleted: 'delete', - flashTypeSuccess: 'success', // Configuration menu options and browser alert text snippets deleteItem: 'Delete this item', @@ -28,7 +28,6 @@ const { deleteItem, flashMessageOperationAdded, flashMessageOperationDeleted, - flashTypeSuccess, initialTenantDescription, initialTenantName, settingsMenuOption, @@ -53,7 +52,7 @@ describe('Settings > Application Settings > Access Control', () => { cy.getFormInputFieldById('name').type(initialTenantName); cy.getFormInputFieldById('description').type(initialTenantDescription); cy.getFormFooterButtonByType('Add', 'submit').click(); - cy.expect_flash(flashTypeSuccess, flashMessageOperationAdded); + cy.expect_flash(flashClassMap.success, flashMessageOperationAdded); cy.selectAccordionItem([ /^ManageIQ Region/, 'Tenants', @@ -65,6 +64,6 @@ describe('Settings > Application Settings > Access Control', () => { confirmTriggerFn: () => cy.toolbar(toolBarConfigMenu, deleteItem), containsText: deleteItem, }); - cy.expect_flash(flashTypeSuccess, flashMessageOperationDeleted); + cy.expect_flash(flashClassMap.success, flashMessageOperationDeleted); }); }); From 6f119252d39e1090f0b4d3c8d3166831643da6ce Mon Sep 17 00:00:00 2001 From: Joe Rafaniello Date: Thu, 14 Aug 2025 17:17:48 -0400 Subject: [PATCH 4/4] Simplify the defining of constants and access * Drop repetive constant definition and assignment * Generalize the names of constants * Capitalize constants as per conventions --- .../settings_access_control.cy.js | 64 +++++++------------ 1 file changed, 23 insertions(+), 41 deletions(-) diff --git a/cypress/e2e/ui/Settings/Application-Settings/settings_access_control.cy.js b/cypress/e2e/ui/Settings/Application-Settings/settings_access_control.cy.js index 340d1d6d642..33ea04a4458 100644 --- a/cypress/e2e/ui/Settings/Application-Settings/settings_access_control.cy.js +++ b/cypress/e2e/ui/Settings/Application-Settings/settings_access_control.cy.js @@ -1,44 +1,26 @@ /* eslint-disable no-undef */ import { flashClassMap } from '../../../../support/assertions/assertion_constants'; -const textConstants = { - // Menu options - settingsMenuOption: 'Settings', - appSettingsMenuOption: 'Application Settings', - toolBarConfigMenu: 'Configuration', - - // added item information - initialTenantName: 'Test-name', - initialTenantDescription: 'test description', - - // List items - accessControlAccordion: 'Access Control', - - // flash message assertions - flashMessageOperationAdded: 'added', - flashMessageOperationDeleted: 'delete', +describe('Settings > Application Settings > Access Control', () => { + // Navigation + const PRIMARY_MENU_OPTION = 'Settings'; + const SECONDARY_MENU_OPTION = 'Application Settings'; + const ACCORDION = 'Access Control'; + const TOOLBAR_MENU = 'Configuration'; - // Configuration menu options and browser alert text snippets - deleteItem: 'Delete this item', -} + // Created item information + const INITIAL_TENANT_NAME = 'Test-name'; + const INITIAL_TENANT_DESCRIPTION = 'test description'; -const { - accessControlAccordion, - appSettingsMenuOption, - deleteItem, - flashMessageOperationAdded, - flashMessageOperationDeleted, - initialTenantDescription, - initialTenantName, - settingsMenuOption, - toolBarConfigMenu, -} = textConstants; + // CRUD actions + const FLASH_MESSAGE_OPERATION_ADDED = 'added'; + const FLASH_MESSAGE_OPERATION_DELETED = 'delete'; + const DELETE_ITEM = 'Delete this item'; -describe('Settings > Application Settings > Access Control', () => { beforeEach(() => { cy.login(); - cy.menu(settingsMenuOption, appSettingsMenuOption); - cy.accordion(accessControlAccordion); + cy.menu(PRIMARY_MENU_OPTION, SECONDARY_MENU_OPTION); + cy.accordion(ACCORDION); }); it('should be able to create and delete a tenant', () => { @@ -48,22 +30,22 @@ describe('Settings > Application Settings > Access Control', () => { 'My Company', ]); - cy.toolbar(toolBarConfigMenu, 'Add child Tenant to this Tenant'); - cy.getFormInputFieldById('name').type(initialTenantName); - cy.getFormInputFieldById('description').type(initialTenantDescription); + cy.toolbar(TOOLBAR_MENU, 'Add child Tenant to this Tenant'); + cy.getFormInputFieldById('name').type(INITIAL_TENANT_NAME); + cy.getFormInputFieldById('description').type(INITIAL_TENANT_DESCRIPTION); cy.getFormFooterButtonByType('Add', 'submit').click(); - cy.expect_flash(flashClassMap.success, flashMessageOperationAdded); + cy.expect_flash(flashClassMap.success, FLASH_MESSAGE_OPERATION_ADDED); cy.selectAccordionItem([ /^ManageIQ Region/, 'Tenants', 'My Company', - initialTenantName + INITIAL_TENANT_NAME ]); cy.expect_browser_confirm_with_text({ - confirmTriggerFn: () => cy.toolbar(toolBarConfigMenu, deleteItem), - containsText: deleteItem, + confirmTriggerFn: () => cy.toolbar(TOOLBAR_MENU, DELETE_ITEM), + containsText: DELETE_ITEM, }); - cy.expect_flash(flashClassMap.success, flashMessageOperationDeleted); + cy.expect_flash(flashClassMap.success, FLASH_MESSAGE_OPERATION_DELETED); }); });