laravel-enso / enso

Laravel Vue SPA, Bulma themed. For demo login use `admin@laravel-enso.com` & `password` -
https://www.laravel-enso.com
MIT License
1.08k stars 277 forks source link

Permissions per field (on field level) #376

Closed robbykrlos closed 3 years ago

robbykrlos commented 3 years ago

This is a feature request.

We have a request to allow only some users to be able to reset passwords. But not only this scenario. Some other cases too like only some users will be able to change a specific model attribute.

We were wondering if you considered implementing this feature?

We looked at some possibilities, maybe not in full depth, but form JSON may contain a field property "permission" that is mapping an existing permission. Now, if all permissions in Enso are always related to a route, this will be a deviation from this rule and will probably not be something you'll agree with. Further it can be controlled in UI with

v-if="canAccess('administration.users.changePassword')">

Some other tweaks on validations needed too.

Finally it would be a way to hide fields inside forms, based a configurable permission that is or not used for your role.

At the moment this can be achieved without any customizations by rejecting on Store or Update controllers the form request data in some cases. Of course this is not elegant at all.

Store.php

if(!auth()->user()->can('access-route', 'models.groups.changePassword'){
    unset($request['model-attribute-not-allowed-to-change']);
    //change success message...
}

OR

Create slightly different form with - ex: administration.users.edit and administration.users.editWithPassword that will be controlled by 2 separate permissions (and 2 different routes) which will show password fields in the latest form.

Your thoughts are welcomed. In case you see this working for you we can also try a PR to help in this direction.

Thank you,

adithewonderboy commented 3 years ago

+1

vmcvlad commented 3 years ago

Hello!

Regarding your specifc case, I guess you could always extend the core User policy, override the changePassword() method to you own liking which will result in both the password fields being hidden, as well as properly handling the authorization.

As for the password reset button in the login form, you can extend and bind the core ResetPasswordController, and check that the inputted email belongs to a user which can reset his password.

As such, the customizations should be minimal.

A generic solution to hiding fields based on configured permissions may not cover every specific situation and scenario and I fail to see what extra value it can bring since you can very easily hide and show fields when needed from the Form class...but that's just my two cents.

robbykrlos commented 3 years ago

Hi @vmcvlad,

Thanks for your two cents, any idea is highly appreciated.

I'm curious what you meant by "you can very easily hide and show fields when needed from the Form class".

Are you still considering permission based hiding/showing in this idea? Can you give an example?

I think you took my example too literal, imagine a more complex situation where you have a form with, let's say, signatures, and each role (manager, supervisor, medic, judge etc) has to fill in a signature field, which will be only available to that specific role. Imagine having 10 fields which are dependent on the role current user has. Of course these specific situations can be easily implemented from scratch. I was just seeing a relative small improvement that could be easy to implement(at least from initial check)

But sure, I can see that maybe it's a too specific case and it brings no value at the moment for you.

vmcvlad commented 3 years ago

Take a look at the core UserForm. The password and password_confirmation fields are hidden in template but we can call show([field 1...field n]) on the form object to display them conditionally. You can also call hide(['field 1...field n])for the opposite effect.

As for your latest example, I'd approach it differently. Instead of adding a field in the DB for every role and keep adding a new one whenever a new role is required, I'd use a pivot table and only show the field in the form if there it wasn't previously filled by someone with the same role as the authenticated user (or whatever logic) . Of course, I might not get the big picture here, however, based on the provided examples I honestly think there won't be much value in implementing built-in permission based field visibility toggling.

But, of course, it's not up to me to decide :)

image

image

robbykrlos commented 3 years ago

Hi @vmcvlad,

Thank you for your ideas.

robbykrlos commented 3 years ago

As an additional detail on this ticket: as the form buttons or table buttons accept routeSuffix which conditions the visibility of that button in the form/table, would have been nice to have the same option for form fields.

aocneanu commented 3 years ago

Some other cases too like only some users will be able to change a specific model attribute.

@robbykrlos, I believe it should never be the responsibility of the form to prevent users to change specific attributes. This logic should reside in the request validation / controller authorization layers. The form should atmost make some fields readonly/disabled/hidden, which it can already do, but a form would be always exposed to hacking.

Adding to the form logic that will manage fields based on permissions would add a new level of complexity for what I believe is a small benefit. The real work should be put in the validation / authorization layers anyway.

Something that I'm looking into is adding the ability to customize forms by adding the ability to set listeners for the form lifecycle and customize this way the template. This would allow manipulating forms from different packages while keepng things decoupled. We would not need to extends forms anymore and rewrite json templates. The principle applies to tables as well.

Maybe when that will be done it will help at least a bit for your described use case..