AmpersandTarski / Ampersand

Build database applications faster than anyone else, and keep your data pollution free as a bonus.
http://ampersandtarski.github.io/
GNU General Public License v3.0
40 stars 8 forks source link

RESTRICT statement #1375

Open stefjoosten opened 1 year ago

stefjoosten commented 1 year ago

Problem

When I define an information system, I spend serious time making INTERFACEs. Especially getting CRUD restrictions right is time-consuming because I have to inspect every field of every interface separately. Besides, I make mistakes when I restrict a particular role (say Operator) from doing CRUD-actions to a specific relation (say address). Especially when this relation occurs all over the place, mistakes lure. The more I have been doing this work, the more I noticed that I am placing the same restrictions on the same relation for a particular role over and over again.

Proposal

For this purpose, I would like the following statement

RESTRICT <relationref> <crud> FOR <roles>

For example:

RESTRICT address cRud FOR Operator, "Technical Maintainer"

The meaning of this statement is that anywhere in any interface for these two roles where the relation address occurs, the restrictions c, u, and d apply.

To be more precise:

I can still put CRUD restrictions in the interfaces, which just adds more restrictions to the ones in place due to RESTRICT statements. So, this feature alters nothing to the programs that are already running.

Impact

This change is upward compatible because we are merely adding a statement type to the compiler. It is syntactic sugar for writing CRUD restrictions in interfaces.

There is an impact on the parser, which needs to accommodate an extra statement. We also need a function that computes the resulting CRUD restriction for every field in an interface.

Michiel-s commented 1 year ago

I like this feature request. There are a few things that require mentioning here.

CRUD semantics

To be more precise:

  • Restriction c means that the designated roles cannot create a new pair in the relation.
  • Restriction r means that the designated roles cannot read in the relation.
  • Restriction u means that the designated roles cannot create or delete any pair in the relation.
  • Restriction d means that the designated roles cannot delete a pair in the relation.

For c and d this is not consistent with our current CRUD semantics. Read about it in our documentation on CRUD.

Impact

The impact is quite large I think; both on compiler side as well on prototype backend and frontend side. Let me explain.

Currently our crud rights are role-agnostic. They are specified globally (using --crud-defaults config) and/or specified at interface expressions. Those apply to all roles that have access to the interface. There is a 1-1 mapping between interface expression and crud-rights.

The proposal you do to restrict crud rights for certain roles, results in need for role-specific interface expression crud rights. E.g. the script below give cRUd-rights to address expression in the interface to Operator and SomeOtherRole. For the Operator this is restricted to read-only (cRud) by the RESTRICT statement. The SomeOtherRole however still has update rights.

RESTRICT address cRud FOR Operator, "Technical Maintainer"

INTERFACE "Address" FOR Operator, SomeOtherRole : I[Location] BOX
[ "address" : address cRUd
]

This new role-interfaceExpression-crud combination need to be captured and adapted in the generator output of the compiler. The backend implementation must be refactored to included for this combination and include the active roles of the user performing an interface request. Finally the frontend (and templates) must also be adapted, because crud rights and the way the components are presented change from static/fixed crud rights to dynamic depending on the active roles of the user.

Therefore, I think this is a large request with complications for all components in the Ampersand stack.

hanjoosten commented 1 year ago

A suggestion to implement this would be to let the generator do the heave lifing: For every Interface in the script, we could output multiple Interface definitions in the .json file generated, one for every role with access to that interface. The CRUD parts could be calculated by the generator. Also, when an interface is referenced as subinterface, the generator could keep track that the references are done to the correct subinterface, depending on the role. For the back-end, that would hardly cause any trouble, I guess.

stefjoosten commented 1 year ago

I can see @Michiel-s 's point of view. Currently, we switch roles on and off dynamically. Consequently, the INTERFACE must support every combination of roles. This proposal adds complexity to that requirement.

In a way @hanjoosten challenges this requirement by talking about generating different code for different roles. That assumes static roles, i.e. roles on compile time. Static roles allow us to implement the security measure of minimal code per role. Dynamic roles (as currently implemented) on the other hand, require that every service contains the code for all roles that can be switched on dynamically. This blocks minimal code per role implementation, but it also complicates the role graph proposal. In practice, dynamic switching between roles is only nice in demos, but I don't want that in production.

Summarizing, my comment is that we must implement one role per interface and give up to combine and switch roles dynamically.