Denis Chenu

Results 401 comments of Denis Chenu

I don't like to add a new Service since Permission is already here. I think we can just start with a minimal Permission system : 2 permission, 'user' and 'permission'...

https://github.com/LimeSurvey/LimeSurvey/blob/master/application/models/Interfaces/PermissionInterface.php https://github.com/LimeSurvey/LimeSurvey/blob/master/application/models/Traits/PermissionTrait.php See https://github.com/LimeSurvey/LimeSurvey/blob/e4f3ede06d502a176e9d02d3e9d4ac55bb3b76e4/application/models/Traits/PermissionTrait.php#L41 for usage in User Then your function are - canAssignPermissions : `$user->hasPermission('permission', 'update');` - canAssignRole : `$user->hasPermission('role', 'update');` - canEdit : `$user->hasPermission('user', 'update');` See : https://github.com/LimeSurvey/LimeSurvey/blob/e4f3ede06d502a176e9d02d3e9d4ac55bb3b76e4/application/models/Survey.php#L2250...

And when we create the Permission manage page : a lot is ready :)

> Well the logic about child-parent relationship is not there. ? see [getOwnerId](https://github.com/LimeSurvey/LimeSurvey/blob/e4f3ede06d502a176e9d02d3e9d4ac55bb3b76e4/application/models/Traits/PermissionTrait.php#L10) … done for this … You add it in user : where is the issue here ?...

> Well the logic about child-parent relationship is not there. What's wrong in having a logic layer for that? It's here : https://github.com/LimeSurvey/LimeSurvey/blob/e4f3ede06d502a176e9d02d3e9d4ac55bb3b76e4/application/models/Permission.php#L576 exactly.

My opinion on this commit : 2 solution 1. Just fix the issue 18356 : use `Permission::model()->hasGlobalPermission('superadmin', 'read')` in actionSaveRole and actionAddRole (light commit) 2. Do the whole access system...

> It is not definite. Just trying to solve a problem decently in a short period of time. Then : do only the 1st point : > 1. Just fix...

> So we have similar logics when Editing users, applying permissions, applying roles, theme permissions, ... That's why a class to handle it centrally is usefull, as a starter. Later,...

> > Currently : owner_id is tested in Permission model, but you can test in User->hasPermission andnever use Permission model. > > How would you write `$user->hasPermission()` using the permission...

For me : the commit are really complex against a `if (!Permission::model()->hasGlobalPermission('users', 'update')) {` replaced by a `if (!Permission::model()->hasGlobalPermission('superadmin', 'read')) {`