Improve ajax js errors and solve ACL problems #53

andrepereiradasilva · 2016-06-09T13:27:23Z

Pull Request for Several ACL issues.

Summary of Changes

This PR makes several changes in the permissions panel and also replicate js improvements in sendtest mail in order to solves bugs and improve php/js code.

Bugs/Improvements

1: Ajax js errors are not catched in permission.js
After this PR uses the same checks as sendtestmail.js that where now moved to core.js

2: Session token is not being checked in permission.js
After this PR the session token is checked.

3: When a "Denied" permission is inherited the calculated permission is "Not Allowed (Locked)"
After this PR the text was changed to "Not Allowed (Inhreited)"

4: Permision js ajax call is returned in html mimetype
After this PR the ajax call is returned in json mimetype.

5: Some errored of permision and sentestmil js ajax calls are returned in HTML
After this PR the all messages are returned in Json.

6: The js messages in sendtestmail and permissions.js are not cleaned on change.
After this PR they are cleaned everytime you change.

7: Permission js uses a custom html message system
After this PR all message use the Joomla API message system.

8: A super user cannot remove super user permission from other groups that his not into.
After this PR this is possible.

9: When a Super permission is changed or a a group with child groups permission is changed the user is not informed that we needs to refresh the page to recalculate permissions.
After this PR the user is notified.

10: A user that is not Super User can change a Super User group permissions.
After this PR this is not possible.

11: A user that is not Super User can change change its own's groups ot it's parent permissions.
After this PR this is not possible.

12: We don't have calulate permissions in com_config public group.
After this PR we have the calculated permission here too.

13: Changing a permission with "Not Allowed (Inhreited)" to "Alloewd" or "Denied" changes it's calculate permission.
After this PR the calculated permissions always stays "Not Allowed (Inhreited)" in this cases.

14: If you change a "Inherited" permission with a calculated permission of "Not Allowed" to "Allowed" and then back to "Inherited", the calculated status is changed to "Allowed" (it should be "Not Allowed").
This PR solves that.

15: There are some "Conflict" messages when you have a parent group with "Denied" and some child group with "Allowed". This should be only "Not Allowed (Inhreited)"
This PR solves that.

There can be more bugs/improvements that i don't remember...

Scenarios

Calculated Permissions Scenarios

If only "Inherited" across the group tree:

"Inherited" -> "Not Allowed" (red).
"Allowed" -> "Allowed" (green).
"Denied" -> "Not Allowed" (red).

If any "Denied" across the group tree:

"Inherited" -> "Not Allowed (Inherited)" (gray).
"Allowed" -> "Not Allowed (Inherited)" (gray).
"Denied" -> "Not Allowed (Inherited)" (gray).

If no "Denied" and a "Allowed" across the group tree:

"Inherited" -> "Allowed" (green).
"Allowed" -> "Allowed" (green).
"Denied" -> "Not Allowed" (red).

Special case: If group has super User permissions all values in that group and child groups should be "Allowed (Super User)" (green).

Error/Notices Scenarios

Can't change permissions scenarios:

Session token is not valid.
Database query/update errors.
Any error while checking or storing asset.
Current user does not have permissions to change ACL for the component.
Current user is Super User and is trying to remove his own Super User permissions.
Current user is not Super User and is trying to change a Super User group permissions.
Current user is not Super User and is trying to change its own's groups permissions.
Current user is not Super User and is trying to change its own's groups parent permissions.

Ask user to reload the page scenarios:

User changed permissions of a group with child groups.
User changed group permissions to Super User.

Testing Instructions

Use latest staging.
Check all "Bugs/Improvements".
Apply patch
Go to global configuration
Clean browser cache.
Check all "Bugs/Improvements" are solved.
Check all scenarios described in the "Scenarios".
Do the tests two times. With your super user and other user with less permissions (easier if you open a session for that user in a private browser window).

andrepereiradasilva · 2016-06-09T13:32:13Z

@roland-d i make all the changes needed to have always a js result.
Also improved and centralized some thing since they are used in to different js to not repeat code.

Didn't have test all yet, but it seems the work reggarding js is all done here.

See https://github.com/andrepereiradasilva/joomla-cms/pull/53/files?w=1 to check the changes without whitespaces.

I will further test this when i have time.
I also whant to have the execute to cehck token.

You can try:

Apply patch
Go to configuration, clear browser cache and do permission, sendmail operation that return js messages.
You can even try a syntax error in storepermission js and see the result.

andrepereiradasilva · 2016-06-09T13:41:21Z

BTW this alreasy includes your 10764 PR

roland-d · 2016-06-09T19:54:38Z

@andrepereiradasilva I have checked the two issues as you requested and I have not found any issues other than the known issues.

andrepereiradasilva · 2016-06-10T01:11:48Z

i think i solved all issues now. hope ...

infograf768 · 2016-06-10T08:08:30Z

@andrepereiradasilva
Conflict with present core staging for store.php. Can't apply.

…ve-ajax-js-errors

andrepereiradasilva · 2016-06-10T08:42:22Z

conflicts fixed

infograf768 · 2016-06-10T10:08:03Z

Looks like working here.
remains the major security issues:
A user can change its own's permissions and parent permissions.
And another one: see joomla#10775

andrepereiradasilva · 2016-06-11T08:54:02Z

Done since last comments:

Comments and messages review
Move hardcoded messages to languages variables
Add calculated permissioncolumns also in Root group (Public) in com_config
Removed "Conflict" message, there is no need now since it's always Not Allowed (inhreited)
A user (not super user) cannot change its own's groups permissions
Check token in ajax call.

Still TO DO:

A user (not super user) cannot change its own's groups parent permissions

infograf768 · 2016-06-11T09:57:40Z

Looks good. remains only the TODO

… it's a super user)

andrepereiradasilva · 2016-06-11T10:20:33Z

A user (not super user) cannot change its own's groups parent permissions

DONE!
I think now it's all solved. Please retest everything with attention.

infograf768 · 2016-06-11T10:42:39Z

First series of tests look promessing

@andrepereiradasilva

* Codestyle * indent * order * oops * changes requested by @andrepereiradasilva * cs * changes requested by @wojsmol * Update mod_logged.xml (#43) * Update templateDetails.xml (#45) i need to fix my ide!! * Update mod_popular.xml (#44) * Update templateDetails.xml (#57) * Update mod_version.xml (#56) * Update mod_toolbar.xml (#55) * Update mod_title.xml (#54) * Update mod_submenu.xml (#53) * Update mod_status.xml (#52) * Update mod_stats_admin.xml (#51) * Update mod_quickicon.xml (#50) * Update mod_menu.xml (#49) * Update mod_login.xml (#48) * Update mod_latest.xml (#47) * Update mod_feed.xml (#46)

Export Data from Request List

andrepereiradasilva added 8 commits June 9, 2016 11:50

ajax js error messages

ac2d33c

add the js error messages to views

7ae9758

send test mail should send always json

177d2cf

simplify sendmailjs

c61bd27

centralzie ajax errors message sin core

9a9819e

message, not sucess

45cad98

don't render messages if null

fd87424

better json/javascript handling

0d5f209

andrepereiradasilva added 2 commits June 9, 2016 14:37

remove extra debug

4dd345d

corrrect admin check by component

3eb91c9

andrepereiradasilva added 12 commits June 9, 2016 14:42

app not this->app

1510488

ups

9f13208

ups ... again

9893e10

remove messages if they exist

9246cd1

remov messages in sendmail too

74832a5

to do and superuser doesn't change

b3f16d2

a super user can demote other groups from being a super user

a89c953

check if current group has super user permissions

67bbad9

language var doesn't exist

92a4340

isUserSuperUser

81940d0

we can have child groups ...

0a00a9f

remvoe extra spaces

37d5be2

andrepereiradasilva added 5 commits June 9, 2016 21:10

if user is not is superuser group, cannot change a superuser group

e4ef8fb

a super user can change other groups

407f64d

improve comments

42f3559

improve messages

5d35504

not Locked, should be Inherited

996949a

andrepereiradasilva added 2 commits June 10, 2016 00:54

better permissions

7ad4776

alert recalculate

b234ecb

andrepereiradasilva added 3 commits June 10, 2016 09:41

solves conflicts 1

126f797

Merge remote-tracking branch 'refs/remotes/joomla/staging' into impro…

05df1ba

…ve-ajax-js-errors

solve conflicts final

9b00a3c

infograf768 mentioned this pull request Jun 10, 2016

Fixed users with permissions not allowed to change ACL joomla/joomla-cms#10764

Closed

andrepereiradasilva changed the title ~~Improve ajax js errors~~ Improve ajax js errors and solve ACL problems Jun 10, 2016

andrepereiradasilva added 7 commits June 11, 2016 00:39

a user (not super user) can't change it's own group(s) permissions

fec324a

better comments and messages

a9db8a5

add language vars

378115d

show calculated permissions also on Public group in global config!

241108b

ups

6ebc49e

check session token!

3729bfc

do not mark as confict.

c56b735

replace conflict by not allowed (inhreited)

01f3ff8

don't allow to change permissions of his own parent groups (except if…

f0890e6

… it's a super user)

andrepereiradasilva closed this Jun 11, 2016

andrepereiradasilva pushed a commit that referenced this pull request Dec 24, 2018

Merge pull request #53 from joomla-projects/feature/export-data

99b106c

Export Data from Request List

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve ajax js errors and solve ACL problems #53

Improve ajax js errors and solve ACL problems #53

andrepereiradasilva commented Jun 9, 2016 •

edited

Loading

andrepereiradasilva commented Jun 9, 2016

andrepereiradasilva commented Jun 9, 2016

roland-d commented Jun 9, 2016

andrepereiradasilva commented Jun 10, 2016

infograf768 commented Jun 10, 2016

andrepereiradasilva commented Jun 10, 2016

infograf768 commented Jun 10, 2016

andrepereiradasilva commented Jun 11, 2016

infograf768 commented Jun 11, 2016

andrepereiradasilva commented Jun 11, 2016

infograf768 commented Jun 11, 2016

Improve ajax js errors and solve ACL problems #53

Improve ajax js errors and solve ACL problems #53

Conversation

andrepereiradasilva commented Jun 9, 2016 • edited Loading

Summary of Changes

Bugs/Improvements

Scenarios

Calculated Permissions Scenarios

Error/Notices Scenarios

Testing Instructions

andrepereiradasilva commented Jun 9, 2016

andrepereiradasilva commented Jun 9, 2016

roland-d commented Jun 9, 2016

andrepereiradasilva commented Jun 10, 2016

infograf768 commented Jun 10, 2016

andrepereiradasilva commented Jun 10, 2016

infograf768 commented Jun 10, 2016

andrepereiradasilva commented Jun 11, 2016

infograf768 commented Jun 11, 2016

andrepereiradasilva commented Jun 11, 2016

infograf768 commented Jun 11, 2016

andrepereiradasilva commented Jun 9, 2016 •

edited

Loading