[Enhancement] Giving security attributes better names

[nagmat84]

Intro

In #1443 (comment) @ildyria asked why there is this seemingly duplication of sending certain security attributes to the front-end. The short answer is: because they only happen to have the same name (unfortunately), but have different semantical meanings.

This reminds me of something which has been bothering me for quite a while. The issue has some overlapping with #1462 and #1403, but it is worth to be considered on its own. I have been planning to give better names to some security attributes for a long time, but as I have been unable to come up with some better names on my own, I will put it to discussion.

Problem

The problem are code lines like

lychee.may_upload = data.admin || data.may_upload

This is from the frontend, but it illustrates the problem. Here a user has the right to upload into an album, if the user has the admin capability or upload capability. The problem is that the term "upload" is used twice in two different meanings: on the left hand-side of the assignment it denotes the currently effective right in a specific use-case context, on the right hand-side it denotes a capability of a user. This bears some risk. In particular, it may lead to defective code if the wrong kind of some equivocally named attribute is used in the wrong place.

I also had a quick look at our new Laravel policies below app/Policies and my heart sunk, when I found some mix-up. At the moment, this is still a peace of cake, because we only have very limited multi-user support and hence the impact is manageable, if something gets wrong, but it will become a nightmare, if we get more and more fine-grained multi-user support.

Definitions

Let me first get some terms and definitions straight, before I get back to Lychee. (Sorry, I am a scholar.)

• A permission is something which is based on a relationship between subjects (i.e. user in our case) and objects (e.g. albums, photos, etc.). Permissions may be illustrated by an access control matrix which shows which specific subject has which permission on which specific object.
• A security attribute is directly attached to a subject or an object, i.e. it functionally depends on the subject or the object only, but not on the relation between them. In case of subjects security attributes are also called capabilities sometimes.
• A right is the effective outcome of a decision making policy based on permissions and security attributes in a specific context.

In summary: The security attributes of object O_i, the capabilities (=security attributes) of subject S_j and the permissions of S_j for O_i are input to a policy which determines whether S_j has the right to perform a certain operation on O_i.

Situation for Lychee

(I will only consider albums for now).

• Albums (Security attributes)
  • requires_link
  • password
• Users (Capabilites)
  • is_locked
  • is_admin
  • may_upload
• User-Album Relation (Permissions)
  • is_downloadable
  • grants_full_photo
• User-Album Rights
  • is_downloadable
  • grants_full_photo
  • is_downloadable

I have written the list above with #1462 in mind (that is is_downloadable and grants_full_resolution appear as permissions), but I have left out write permissions and rights to not make it more complicate than it is already.

As you can see, some terms appear twice in the list above and that is bad. For example grants_full_photo appears as a permission and as a right. However, a user does not necessarily require the grants_full_photo permission to have the right grants_full_photo, because the user could also own the is_admin capability.

So, I would really badly like to have distinct names for the attributes above to ensure that is always unambiguously clear what an attribute means. The easy solution is to simply add the designation for the kind to the name. I believe this is what most system do. This would give something like

• Albums (Security attributes)
  • is_link_required_secattr
  • password_secattr
• Users (Capabilites)
  • is_locked_cap
  • is_admin_cap
  • may_upload_cap
• User-Album Relation (Permissions)
  • is_downloadable_perm
  • is_full_photo_granted_perm
• User-Album Rights
  • is_downloadable_right
  • is_full_photo_granted_right
  • is_downloadable_right

But maybe someone can come up with something better? Any ideas @LycheeOrg/reviewers? I think we should get this straight soon and have a consistent system before things become unfixable.

PS: There is a slightly other inconvenience. Typically, the things above are expressed "positively", because this makes reasoning easier. However, is_link_required_attr and is_locked_cap are "negative" in the sense that they restrict rights. It would be nicer, if they were named is_browseable_attr and may_change_pwd_cap with the exact inverse meaning.

[ildyria]

I had some thoughts which would clarify this but without having to depend on a prefix _cap/_attr/_right.

• Albums (Security attributes)

  • is_link_required
  • password -> which derives has_password

• Users (Capabilites)

  • may_selfedit
  • may_admin
  • may_upload

• User-Album Relation (Permissions)

  • grant_download
  • grant_access_full_photo

  and later:

  • grant_edit
  • grant_upload
  • grant_delete
  • ...

• User-Album Rights

  • can_download
  • can_access_full_photo
  • can_download

By using grant_ may_ and can_ it makes it easier to do the difference between the permissions, the capabilities and the rights while remaining easily readable. :)

@nagmat84 opinion ?

[nagmat84]

In principle, I am fine with using verbs instead of nouns to distinguish the different categories.

can for rights and grants for permission works for me.

I am only struggling with may. may_admin is not even a proper phrase. Also the security attributes for albums are missing a consistent naming pattern.

[ildyria]

I am only struggling with may. may_admin is not even a proper phrase.

User may admin[istrate] :)
I considered may_manage but then we lose the meaning of administrator . :)

Also the security attributes for albums are missing a consistent naming pattern.

Indeed, I just left them as is because I assumed there would not be many.

[nagmat84]

I am only struggling with may. may_admin is not even a proper phrase.

User may admin[istrate] :) I considered may_manage but then we lose the meaning of administrator . :)

Works for me.

Also the security attributes for albums are missing a consistent naming pattern.

Indeed, I just left them as is because I assumed there would not be many.

Maybe, we should try to start all of them with is_. As in

• is_link_required
• is_nsfw
• password -> which derives is_password_required

First I considered to start everything with requires_ which would work for requires_link and requires_password, but requires_nsfw make no sense and I guess that is_ will turn out to be more versatile in the future.

[kamil4]

Sorry for jumping late into this discussion; it's just that these days I'm rarely in a frame of mind where I can read through a longer message like this and give it the thought it deserves.

Anyway, maybe I didn't give it enough thought, because I still don't understand half of the points that are being discussed here.

For example, why is requires_link a security attribute of an album but is_downloadable a user-album permission (as well as right)? Both are fields of base_albums, both are editable by the user, and neither determines the right on its own (i.e., both are ignored if the current user is the owner of the album in question or is the admin). I fail to see the distinction here. Or is this about the future, where is_downloadable would be moved out of base_albums to user_base_album?

I think that I do understand though the larger point that is being made here. Currently we primarily transfer the attributes between the server and the front end, and then both the server and the front end have to implement the policies to translate these attributes into rights. This is error-prone on the front end side, which may not have some of the necessary information, especially for mixed-ownership results (such as smart albums and search results), where the attributes reside with the actual parent album, not with the objects (photos) themselves or the "virtual" (smart/search) albums.

In my opinion, we should eliminate as many policies from the front end as possible. Instead, every object sent by the server should include rights such as can_download and, importantly, can_edit (currently missing), and that should be done not just for albums but also duplicated for every photo inside the album (this will let us fix the current mixed-ownership issues). Albums also contain the corresponding attributes (downloadable/requires_link/grants_full_photo) and those need to be included as well because the owner/admin needs to be able to inspect/edit them via the GUI. So yes, I do see a need for disambiguating the names so that the "downloadable" attribute can be distinguished from the "downloadable" right.

I like the can_ prefix for rights, but I would be equally OK with may_. In fact, I believe the latter would be strictly speaking more correct from the linguistic point of view, but the distinction is subtle enough that native speakers frequently mix them up ("Can I go to the bathroom?"). So I would be extremely hesitant to use both, with different meanings, because that will be very confusing for future readers of such code. Maybe better to prefix attributes with attr_ or something? Though I do agree that mixing verbs and nouns in prefixes is at least inelegant...

OK, that's my two cents for now.

[ildyria]

For example, why is requires_link a security attribute of an album but is_downloadable a user-album permission (as well as right)?

Because requires_link is only applicable to Guest users. while is_downloadable can be different for different user.

In my opinion, we should eliminate as many policies from the front end as possible.

That is the end goal. :)

Instead, every object sent by the server should include rights such as can_download and, importantly, can_edit (currently missing), and that should be done not just for albums but also duplicated for every photo inside the album (this will let us fix the current mixed-ownership issues).

That would be is a very big change, but yes I would go incrementally in that direction. :)

I like the can_ prefix for rights, but I would be equally OK with may_. In fact, I believe the latter would be strictly speaking more correct from the linguistic point of view

I see what you mean with may_ and can_. However my English book also says:

May and Might are not used to discuss a permission already granted or refused. In such cases, can, could or be allowed to are to be used.

[nagmat84]

Instead, every object sent by the server should include rights such as can_download and, importantly, can_edit (currently missing), and that should be done not just for albums but also duplicated for every photo inside the album (this will let us fix the current mixed-ownership issues).

That would be is a very big change, but yes I would go incrementally in that direction. :)

We have to go this way, we have no other chance. Especially when we allow to share albums with users and different permissions on a per-share basis, we must send the currently effective rights alongside the album as otherwise the frontend cannot show or hide the appropriate action buttons.

I like the can_ prefix for rights, but I would be equally OK with may_. In fact, I believe the latter would be strictly speaking more correct from the linguistic point of view

I see what you mean with may_ and can_. However my English book also says:

Yes, that is somewhat problematic. I guess that is why most systems make the category explicit part of the attribute's name like I have suggested initially as in

• Users (Capabilites)

  • is_admin_cap
  • [...]

• User-Album Relation (Permissions)

  • is_downloadable_perm
  • [...]

• User-Album Rights

  • is_downloadable_right
  • [...]

However, I also see the advantages in the naming scheme suggested by @ildyria and use prefixes grants_ , may_ and can_. It is far less clumsy as in

• Users (Capabilites)

  • may_administrate
  • [...]

• User-Album Relation (Permissions)

  • grants_download
  • [...]

• User-Album Rights

  • can_download
  • [...]

Sure, the syntactical and semantic distance between may_ and can_ is rather small and gives leeway to confusion. We only need to be very, very persnickety about using the correct terms in the correct places. The first attempt in PR has already failed on that 😝 But I am still open to give it a try.

[ildyria]

Done !