Error if Delete permission is issued without Change permission

[thedoubl3j]

Currently, we have no logic to check if a combination of role permissions that includes delete is allowed which causes some issues when appropriately reflecting things in the UI and user_capabilities field. While at first we thought that this was caused because the permissions were not boiling up correctly, the system was actually working as intended but we were not warning the user of the bad combination of permissions.

This PR addresses that by warning the user of the bad combination and alerting them that they need to include the change permission in order to appropriately assign the delete permission.

[thedoubl3j]

@AlanCoding updated the logic to make check if the default perms has delete and change. this is causing 2 of the tests that you wrote for the view validator to fail since they have only delete_publicdata and or view. Do you want me to update those tests for now so that they don't fail?

[thedoubl3j]

leaving this in the comments so that I can go fix awx test once this merges FAILED awx/main/tests/functional/dab_rbac/test_dab_rbac_api.py::test_assign_custom_delete_role

[AlanCoding]

In the test output I see:

                        raise ValidationError(
>                           {'permissions': f'Permissions for model {role_model._meta.verbose_name} needs to include change, got: {display_perms}'}
                        )
E                       AttributeError: 'NoneType' object has no attribute '_meta'

This looks like a legitimate issue that should be fixed. We don't allow custom system-wide roles for anything that matters for now, so if role_model is None, which indicates a system-wide role, I think it's best to avoid this validation logic.

@AlanCoding updated the logic to make check if the default perms has delete and change. this is causing 2 of the tests that you wrote for the view validator to fail since they have only delete_publicdata and or view. Do you want me to update those tests for now so that they don't fail?

I don't want to get into questioning whether the test would still be meaningful after making that change. For these failing tests, can you instead just wrap them in @override_settings to turn off the validation logic you added? That would be my preference.

[thedoubl3j]

awx PR to fix failing test is up: https://github.com/ansible/awx/pull/15385

[sonarcloud[bot]]

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud