Serenity: [Design Note] Extending Row Permission System

Created on 17 Jan 2020  路  5Comments  路  Source: serenity-is/Serenity

This is basically a note to myself, about a few extensions i'm planning to do on row permission system. You are welcomed to comment with your ideas if any.

Current Permission System

  • Currently we can add [ReadPermission("Something")] on a row to make it readable by users who has "Something" permission.
  • This information is also delegated to the service endpoint, unless it's explicitly overridden in the service itself or the service method.
  • This information is also used by the navigation system, again unless overridden while defining the navigation item, to determine if a row's related page should be shown to the end user.
  • We also have field level permissions. For example, by adding [ReadPermission("SecretFields")] to a property, we ensure that field can only be read by users who has "SecretFields" permission.

Problem Description

  • We may override a row permission in service level by using ServiceAuthorize("Another") on controller or an action, but user still requires "Something" permission in addition to "Another", as the list/retrieve service handler has no idea about which method was used to call itself. This might prevent a security hole in case a developer forgots to add ServiceAuthorize to the controller or action, so even if we could somehow find the caller and its permission, might not be safe to implement it. For such cases, i recommend defining an [ImplicitPermission] so that any user who has "Another" permission also has "Something" permission automatically.
  • Similarly, we could override a navigation permission while defining the navigation item so that a user with "Another" permission can see the page for the row, but as the page will still call the List service for the Row, he still needs "Something" permission. Again recommended solution is to define [ImplicitPermission].
  • One problem with defining permission at navigation item level is, as the Row Page Controller still takes the permission from Row.cs ReadPermission, a user with "Something" permission can open the page (if they know the url) even if he doesn't have the Another permission, even though they can't see the page in the navigation. Thus, if you set a navigation items permission explicitly, you should also set the same in the Page.cs. But one might forget it. That's why we read permission for navigation items, pages, and services from the rows read permission by default. If you override something, you gotta be careful.
  • Although setting a fields permission explicitly is useful, and allows only a user with SecretFields permission to read it, what if all fields except a few is secret? Adding [ReadPermission("SecretFields")] to all fields except a few is possible, but tiring and error prone. You could set [ReadPermission("SecretFields")] on the row itself and override it at field level, but in this case "SecretFields" permission would also apply to the service, thus only users with "SecretFields" permission could access List service for the row. There should be a way to say all fields of this row requires "SecretFields" except these few i mark.

Solution Idea

  • Define a new [NavigationPermission] attribute. If the row has this attribute, it takes precedence over [ReadPermission] attribute only for determining permission for the page and the navigation item. This [NavigationPermission] does not affect List/Retrieve services.
  • Define [FieldReadPermission], [FieldModifyPermission], [FieldInsertPermission], [FieldUpdatePermission] etc which can only be used on the row itself. When such an attribute exists on the row, it sets the default Field level permission for all fields, and takes precedence over [ReadPermission], [ModifyPermission], [InsertPermission] etc which controls the service access. This way, you could just add [ReadPermission("Public")] to a few fields and someone with Public permission could access read fields only. Setting field level permissions this way might also be useful later for replacing lookup editors with service based async editors that can only read a subset of fields through a Lookup permission. Although lookups are useful, they are not so good for big tables, and works through sync requests by default which is obsolete, so we need a better alternative.

All 5 comments

Define a new [NavigationPermission] attribute. If the row has this attribute, it takes precedence over [ReadPermission] attribute only for determining permission for the page and the navigation item. This [NavigationPermission] does not affect List/Retrieve services.

Yes, boss. Currently, we are managing this kind of situation by logic operator like the following
[ReadPermission("Module:SomeRow:Read|Module:SomeRow:Navigation")]
Your solution will be best.

Although lookups are useful, they are not so good for big tables, and works through sync requests by default which is obsolete, so we need a better alternative.

We are facing this kind of problem. Please save us from manually converting a sync lookup to async lookup. It will be really amazing if we have the option to declare [AsyncLookupScript] to a Row or something similar. Paging enabled lookup also an option that will perform similar as the grid.

Implemented with fa008535d0b0df476bae026ad3b66383886e0d13, see the comment in commit for a sample.

@dfaruque After applying this to your big data row, you may remove LookupScript attribute and replace LookupEditor's with ServiceLookupEditor's and should theorically work same. Of course there will be some differences as everything works async through the list service and there is no lookup script so you can't access items by their id (sync through Row.getLookup() etc)

Ah also there is now LookupEditor("Something", Async = true), or AsyncLookupEditor that you can use with ordinary lookups, loading the lookup in async mode just like service lookup editor.

Hi @volkanceylan
Is possible used this in Controller:
[Authorize(Roles="Credit Card Admins")]
Best regards

We don't use roles for permissions, Role:RoleKey might work

Was this page helpful?
0 / 5 - 0 ratings

Related issues

dudeman972 picture dudeman972  路  3Comments

Pinellus picture Pinellus  路  3Comments

dkontod picture dkontod  路  3Comments

StefanTheiner picture StefanTheiner  路  3Comments

gfo2007 picture gfo2007  路  3Comments