dexterity42 / SharedProjectTimesheetsBundle

Shared Project Timesheets Bundle - Kimai 2 Plugin
MIT License
9 stars 14 forks source link

Permissions for non-admin users #24

Open BeckeBauer opened 2 years ago

BeckeBauer commented 2 years ago

It would be great if non admin users could use the plugin as well. I have contacted the developer of kimai and his response was as follow:

'The decision was made by the developer to hard-wire it to the Super-Admin, see eg. these lines:
https://github.com/dexterity42/SharedProjectTimesheetsBundle/blob/master/EventSubscriber/MenuSubscriber.php#L49
https://github.com/dexterity42/SharedProjectTimesheetsBundle/blob/master/Controller/ManageController.php#L28

While the CustomCSSBundle ships its own permissions:
https://github.com/Keleo/CustomCSSBundle/blob/master/DependencyInjection/CustomCSSExtension.php#L39
And then make use of them:
https://github.com/Keleo/CustomCSSBundle/blob/master/Controller/CustomCssController.php#L23
https://github.com/Keleo/CustomCSSBundle/blob/master/EventSubscriber/MenuSubscriber.php#L56

With these links, it should be easy for @dexterity42 to introduce a set of permissions.'

Maybe it helps to amend the plugin?

dexterity42 commented 2 years ago

Hi @BeckeBauer, thanks for your feature request. Should be no problem to implement a set of permissions and roles.

I've thought about permissions when implementing this plugin, but for simplicity reasons I used the super admin role.

I think there should be more than one permission:

  1. View the managed shared project timesheets
  2. Create new shared project timesheets
  3. Update shared project timesheets
  4. Delete shared project timesheets

Maybe 2 + 4 can be merged into one permission.

If any of these permissions are given, the user will see the navigation/menu button.

A new role ROLE_SHARED_PROJECT_TIMESHEETS_ADMIN could be added to combine all permissions. Subsets of the permissions can be created using https://www.kimai.org/documentation/permissions.html

Is the proposed solution sufficient for your needs? Do you agree with the permissions and roles? Any other ideas or improvements? 🙂

Edit: by default the ROLE_SUPER_ADMIN should be assigned to all the permissions as well (backward compatibility).

BeckeBauer commented 2 years ago

Sounds great 👍 Maybe, permission 2 and 3 could be merged although I am not sure what 'update' exactly does.

As regards the introduction of a new role: not sure if it were necessary if the 3(or 4) permissions can be be assigned to the existing roles (a new column for a role which is used only for the plugin might reduce the visibility of role's overview which is already not very clear - I hope I'm not missing the point here)