Topic: Security Role Clone / Addition errors on edit / save

Moved discussion from @pilotspelman's post here as it is a different topic.

Verified that this error exists:
Clone a Role, change the role name and description, make role changes and insert as new role.

The role is inserted - but - Elements not selected in existing selected groups will remain unselected as it should be. If any new Access group was chosen, then all elements in them will get selected even if some of them were not chosen which is wrong.

Now if the newly created role is chosen and edited, then any group with any elements currently selected or remained selected, will get all elements of those groups selected even if they were unselected when saved which is certainly wrong.

Attached are the screenshots.

Post's attachments

AccessRole_Errors.zip 907.5 kb, 2 downloads since 2015-04-01 

You don't have the permssions to download the attachments of this post.

2 (edited by apmuthu 04/01/2015 02:53:00 pm)

Re: Security Role Clone / Addition errors on edit / save

Further analysis on attempting to clone the simplest role: Salesman

sections: 768;3072;5632;8192;15872
areas: 773;774;3073;3075;3081;5633;8194;15873

into a new role SalesWoman with extra permissions:

section: Inventory analytics    
area: Reorder levels

section: Banking & GL transactions
area: GL postings view     
area: Exchange rate table changes

should have resulted in:

sections: 768;3072;5632;8192;8448;15872
areas: 773;774;3073;3075;3081;5633;8194;8449;15873;15874;15875

but in actuality resulted in the same section field but with extra areas as in:

sections: 768;3072;5632;8192;8448;15872
areas: 773;774;3073;3075;3081;5633;8194;8449;15873;15874;15875;513;514;515;516;517;518;519;520;521;522;523;524;525;526;2817;2818;2819;2820;2821;2822;2823;3329;3330;3331;3332;3333;3334;3335;5377;5889;5890;5891;7937;7938;7939;7940;8450;8451;10497;10753;10754;10755;10756;10757;11009;11010;11011;11012;13057;13313;13314;13315;15617;15618;15619;15620;15621;15622;15623;15624;15628;15625;15626;15627;15629;16129;16130;16131;16132

It is clear that the routine parsing the areas is wrong as it takes all areas for the given sections instead of the chosen ones alone, appending the rest to the proper ones.

This may be alleviated if the next area to be inserted is greater than those already in the area list.

As usual, functions are interspersed with sequential code in admin/security_roles.php where all this presumably occurs or triggers.

Re: Security Role Clone / Addition errors on edit / save

Yes, indeed, the problem exists. The regression was introduced with latest fixes before 2.3.23 release. I have just pushed fix to the problem into stable branch. The most easy fix for the issue is changing lines 47-49 to:

if (list_updated('role')) {
    $Ajax->activate('_page_body');
}

Until  that changing security roles setup should be avoided. We plan to make  bugfix FA release fixing the problem in next hours.

Janusz

Re: Security Role Clone / Addition errors on edit / save

What a nice way to Ajax the whole page temporarily! Thanks for the real fix - that was fast - will test and revert.

When you release the FA v2.3.24, please make the now unused field 0_sys_types.type_no = 1 for all records in both CoAs in the insert statements.

Also "stay in old page when customer is updated fix" can also be committed.

To prevent uneditable item descriptions in "purchasing" files from being unnecessarily being updated with same content (and mangling special characters), my commits to the files:
FAMods/purchasing/includes/po_class.inc
FAMods/purchasing/includes/ui/po_ui.inc
FAMods/purchasing/po_entry_items.php
may be taken into the new release as well.

Post's attachments

Purchasing_Description_Diffs.jpg 153 kb, file has never been downloaded. 

You don't have the permssions to download the attachments of this post.

5 (edited by apmuthu 04/02/2015 04:57:30 am)

Re: Security Role Clone / Addition errors on edit / save

The fix needs to be corrected since the database storage symptoms are still there although the GUI shows it correctly:

Salesman Role:

sections: 768;3072;5632;8192;15872
areas: 773;774;3073;3075;3081;5633;8194;15873

Cloning Salesman as SalesWoman and adding in 2 areas to existing section and 1 area to new section results in:

sections: 768;3072;5632;5888;8192;15872
areas: 773;774;3073;3075;3081;5633;5889;8194;15873;15874;15875;513;514;515;516;517;518;519;520;521;522;523;524;525;526;2817;2818;2819;2820;2821;2822;2823;3329;3330;3331;3332;3333;3334;3335;5377;7937;7938;7939;7940;8449;8450;8451;10497;10753;10754;10755;10756;10757;11009;11010;11011;11012;13057;13313;13314;13315;15617;15618;15619;15620;15621;15622;15623;15624;15628;15625;15626;15627;15629;16129;16130;16131;16132

but it should be:

sections: 768;3072;5632;5888;8192;15872
areas: 773;774;3073;3075;3081;5633;5889;8194;15873;15874;15875

Eliminating all blank areas is better than choosing all selected areas if a mere presence is checked instead of the value!

This persists even in FA 2,4 where an extra permission 775 (Edit other users transactions) is given to Salesman role - is this intended? If so, would it not be better to keep the 775 along with 773 and 774 instead of at the end in the insert statement statements in the CoAs? This (775) seems to be so for practically every role!

6 (edited by apmuthu 04/02/2015 05:59:22 am)

Re: Security Role Clone / Addition errors on edit / save

Finally fixed it - it was failing since the value of the checked element was not being tested:

--- admin/security_roles.php    Sat Mar 14 20:35:36 2015
+++ admin/security_roles.php    Thu Apr 02 11:09:29 2015
@@ -85,8 +85,8 @@
         $sections = array();
         $areas = array();
         foreach($_POST as $p =>$val) {
-            if (substr($p,0,4) == 'Area') {
-                $a = substr($p, 4);
+            if (substr($p,0,4) == 'Area' && $val == 1) {
+                $a = (int)substr($p, 4);
                 if (($a&~0xffff) && (($a&0xff00)<(99<<8))) {
                     $sections[] = $a&~0xff;    // add extended section for plugins
                 }

The devs can now port it to FA 2.4 as well using the same patch file.

Post's attachments

security_role_patch.zip 6.1 kb, 1 downloads since 2015-04-02 

You don't have the permssions to download the attachments of this post.

Re: Security Role Clone / Addition errors on edit / save

Thanks for your efforts!

I may have misunderstood how your patch (fix) should be implemented, but I downloaded the attachment in the last replay. From the attachment, I extracted the new/updated file "security_roles.php", and replaced the previous file on the server (in the directory "admin") with this updated file.

However, after this procedure the problem now seems to be reversed. I.e. all the elements in each group will become unselected instead of the previous problem with all elements being selected.

Here is a link to screenshots that illustrate the new problem: http://shares.jansson-fam.se/s#27218b90-96ce-4e18-91b5-2a8547eea422/bo.clouddrive.jansson-fam.se/FA_Errors/

In addition, I have discovered another problem compared with the previous version (FA 2.3.22) regarding editing of customers, but I will describe this in a new post (new topic).

Re: Security Role Clone / Addition errors on edit / save

The problem you are getting is because all areas have been stored in the security_roles tables for all roles that got edited with the erroneous file. Start afresh for the security_roles table and see how it goes.

Check out the latest FA v2.3.24 and see if that solves your problems.

Re: Security Role Clone / Addition errors on edit / save

Thanks for the help and tips, now it seems to work as expected!
I have updated to version 2.3.24 by downloading the archive file from sourceforge.net, and followed the instructions in your Update Guide.
Please note that the updated file "ui_controls.inc" is currently not included in the archive file available on sourceforge.net. Refer to the topic "Report Bugs here » Problems with editing customer contacts".

Best regards

Re: Security Role Clone / Addition errors on edit / save

The ui_input.inc fix was committed after the release of FA v2.3.24.

Thanks for your diligence in ferreting out this bug and testing it through to it's logical end!