goyave icon indicating copy to clipboard operation
goyave copied to clipboard

Permission system (guards)

Open System-Glitch opened this issue 4 years ago • 14 comments

Proposal

Guards are an extension of the authentication system using user-defined fields from the authenticator's user model to allow or deny specific actions to an authenticated user.

For example, a Goyave application will define a guard for forums moderation. A moderator is allowed to modify the posts of other users, but regular users aren't. A guard update-others will be implemented, using the IsModerator field from the user model and the route parameter to check if the user is a moderator or if its own post.

Guards could be used in two ways:

  • As function calls, so they can be used anywhere such as in the middle of a controller handler
  • In middleware to protect routes directly without cluttering controller handlers.

This system could support OAuth in some way too.

The implementation is not defined yet, so feel free to discuss and suggest one.

Possible drawbacks

None.

Additional information

This issue is a feature proposal and is meant to be discussed. It is also a good candidate if you want to contribute to the project.

System-Glitch avatar Mar 06 '20 10:03 System-Glitch

@System-Glitch I have an idea on my mind. I'll write a proposal and we can discuss it.

amit-davidson avatar Mar 13 '20 07:03 amit-davidson

Abstract

I propose a permission system based on RBAC. Every user will have a list of roles with specific permissions saved in the DB. The permissions will specify what a role is allowed to do and on what items. Anytime a validation is required, the validation will compare the permissions of the user passed with the request against the required permissions. The validation will be called from the code for handlers and from a built-in middleware for routes.

Background

RBAC - (Role-based access control) is based on defining a list of roles and adding each user in the system to one or more roles. Permissions and privileges are then granted to each role, and users receive them via their membership in the role.

ACL- (Access-control list) is a list of permissions attached to an object. It specifies which users or system processes are granted access to objects, as well as what operations are allowed on given objects.

RBAC defines permissions from the view of the roles, which means: roleA can read objectA and write to objectB whereas ACL is set from the perspective of the item: objectA allows userA to delete it, and userB to read it.

Proposal

Initialization

A new field named Roles will be added to the user object. I think the easiest way to declare the permissions, for now, is in the DB. The permissions will be fetched alongside the username and password in the authenticate middleware and attached to the user object. In the future, we can add support for loading the permissions from a file or add them from the code.

Permissions Structure

The following JSON depicts the structure of the permissions.

{
  User: {
    Roles: [
      {
        Name: "UserA",
        "Policies": [
          {
            "Name": "PolicyA",
            "Effect": "Allow/Deny",
            "Resource": "ModelA",
            "Permissions": ["A", "B", "C", "D"]
          }
        ]
      }
    ]
  }
}

  • User (Goyave's user) will have a new field containing a list of roles the user can use.

  • Each role will have a unique name and a set of policies. The policies contain the logic of the permissions system.

  • Each policy contains a unique name used to identify itself and the following fields

  • Effect: Whether to allow or deny the following permissions.

  • Resource: A string used to identify the resource. Each resource has a unique syntax, but all of them share a common prefix of resourceType. There might be more syntaxes for different resources in the future.

    • For URIs: resourceType.requestMethod.itemId where resourceType is endpoint, requestMethod is GET/POST/etc, and itemId is a regex expression for identifying of the allowed endpoints. For example:

      • endpoint.GET./product/7(only to product 7 on get requests)
      • endpoint.POST./product/abc+(only products that have ab followed by a c for POST requests)
      • endpoint.DELETE|PUT./category/* (Delete or PUT for alll categories)
    • for Models: resourceType.ownerID.resourceName.itemID where resourceType is model,ownerID is the user who created the resource. resourceName is the model's name, and itemID is the unique identifier of the item.

      • model.John.product.* (Only products owned by John)
      • model..category.clothes (Only model category with id clothes. Ownership might not mean much in the case of category, so it's left empty, and a check against ownership won't be made. That to differ from * where a check is made.)
      • model.self.category.7 (Only category 7 that was also created by self )
  • Permissions: A list of permissions the role can do where each permission is just a string. A permission can be anything the developer wants, as long as the "string" the user has in his permissions and the required permission matches. For example: - ['Read', 'Write'] - ['Create', 'Update', 'Delete'] - ['ListAllProducts', 'ReorganizeCategories'] (An example for non-classic set of permissions)

Few points to notice:

  • self will be converted to the username the policy is assigned to. When a user is loaded, any place self appears will be changed to the user's username.
  • Only the itemIDs will be validated with regex for now (It should match the way Goyave already supports regex). I think that's the easiest to implement now, and regex support for other fields can be added in the future if needed.

How it will be used

A new function called IsAuthorized will be added to Goyave. Its signature is: IsAuthorized(user interface{}, permissions []string, ownerName string, itemId string) where:

  • user - The user requesting access.
  • permissions - A list of needed permissions.
  • ownerName - The name of the owner. If the parameter is not relevant, an empty string will be passed.
  • itemID - the ID of the item. If the parameter is not relevant, an empty string will be passed.
  1. Inside handlers, the function can be called directly. User will be fetched from the request object. Permissions are given according to the use case. ownerName and itemID should be tracked and given by the developer. The developer can choose not to use the itemID and ownerName, for example, if he doesn't want permissions that granular or doesn't wish to track ownership of items.

  2. For the endpoints themselves, a list of permissions will be added on the route when it is defined. The check will run as a middleware after authentication. User is given from the request. The permissions are provided, and ownerName and itemID will be left empty as they have no meaning.

Rationale

The reason I propose an RBAC solution instead of ACL is because the developer will have many resources in the system and not many roles. That way, it will be easier to define a small number of roles in relation to many resources instead of defining permissions on many resources.

Feel free to comment if you have different suggestions or something is not clarified. 😃

After we settle on the proposal, I'll break the design into steps, and we'll start working from there.

amit-davidson avatar Mar 14 '20 08:03 amit-davidson

Great proposal. I do have a few questions and comments though.

I think that the policy should be "Deny" by default, and that only "Allow" policies are needed. However, "Deny" could be useful in a case where the user has multiple roles, one allowing access to a resource and one denying it. In this scenario, which policy has priority?

Roles should be put in a separate model and database table, to avoid redundancy and potential mismatches.

The default User model can be replaced by the developer, how would the framework define a pivot table for the n:n relation between the custom User model and our Role model? Also how should we manage migrations in that case?

User in the request object is an interface{}, so some reflection would be needed to retrieve it as a struct. Then, how would the framework know which struct field to use? A new field tag may be needed, for example: auth:roles.

I like the resource field for URIs very much, but for models, I don't think we can implement that. At the time middleware is executed, we don't know which model or which action the controller handler is going to use. The same goes for permission list.

About IsAuthorized, can we move the permissions list to make it the last argument of the function, and make it variadic?

System-Glitch avatar Mar 14 '20 09:03 System-Glitch

  • I agree with you that by default, roles aren't allowed to do anything. The deny is used to complement. For example, if you have products 1,2,3. You can allow access to products 1,2 by using allow on 1,2 or allow on * and deny for 3. That's a simple case, but when faced with many items, or you want to exclude a specific element like "allow access to all items but self", it's a must. As for the priority, we just "sum" all the permissions from all the policies and get the calculated result. In the example above, allow on * and deny for 3 will evaluate to allow on 1,2.

  • Sure.

  • As you suggested, we'll have a table containing users and a table containing roles. To manage the n:n relation, each user will have a field named roles holding the roles he can use. It's better to have users with field roles and not roles with a field users. First of all, it's easier to think about it that way. Second, and most importantly, we're working with users and roles. When a lookup is made, we retrieve the roles with O(1) instead of searching in all the roles for the ones containing the user with O(N).

  • I agree that a new tag auth:roles is needed. How the framework handles reflection when it retrieves the username and password fields?

  • The way I see this, routes have one set of permissions, and models have another. This way, when the validation middleware is executed (and the IsAuthorized function is called with the permissions given when the route was defined), it only checks if the user has sufficient permissions for accessing the URI. If more permissions are required for different actions inside the handler itself, then the IsAuthorized function will be called again.

    I'll take the example you used in the issue. The route comment/delete is defined with the permissions accessURI,allow,* to access the route itself and listAllComments,allow,* (It needs in all scenarios in the handler, so we can avoid cluttering the handler and put it on the route). Then in the handler, before delete is called, another check is made to make sure the user (regular user/moderator) has sufficient permissions to delete the comment. As you say, in the route, we don't know what models the user is going to use (everyone or his), so it has to be in the controller. This way routes only know about route relevant permissions and handlers about model relevant permissions.

  • I don't see much point in doing this:

    1. I suspect the developer will usually have the permissions required for a particular action defined as a slice and not as a single permission. For example, for removing a product, you may need both: read access to category and read&write access to products. Unpacking the slice of permissions in the function call, just to be repacked again, seems a bit awkward (and also inefficient performance-wise).
    2. It's small, but the first two parameters (user and permissions) will always be non-empty. ownerID and itemID might be passed as empty strings (in the case of routes), and I think it's nicer if the empty values come last.
    3. In the future, if we wish to expand the signature of the function (Not planning to break the API... Just in case we must.), changing a variadic function is more painful.

One last thing, I thought about more use cases for permission checking except using models for accessing the DB or for routes. For example, a user might want to have an option to clear the cache, restart a service, send a notification, etc.). By using resourceType for identifying the resource type, the developers are limited to models and routes. I'll try to think of generalized way resources can be recognized instead of a custom ID for each type. I'll handle this point together with points we decide to change.

amit-davidson avatar Mar 14 '20 19:03 amit-davidson

  • Why not using a pivot table instead of a field in the user model?
  • Column retrieval using the auth:username and auth:password fields is done through the auth.FindColumns()function. Promoted fields are checked recursively as well so it's possible to add a new struct Permissionable to make it easier for developers to add support for guards on their users (and also to make it more flexible if we ever need to change something later on).
  • Making two different permission sets applied on completely different pieces of code is not very intuitive and doesn't make the system feel unified. The solution should be kept as simple as possible to use, with minimal code required from the developer. Needing to call IsAuthorized on top of a middleware is in my opinion not a good approach.
  • Good point about the variadic function. Let's keep it this way.
  • Finding a system that is flexible enough to suit all sorts of needs related to permissions is indeed a must, and it's related to my point above. They should all be defined the same way and if possible, not part of the controller handler.

Note: I took a lot of inspiration from the PHP Laravel framework for certain parts of Goyave. Laravel provides an Authorization feature which may be a good source of inspiration.

System-Glitch avatar Mar 15 '20 09:03 System-Glitch

  • A pivot table is a really good idea. At first, I wasn't sure if it would be easier for a developer to separate the authorization into another source for initialization. Now, I understand it allows more flexibility, as the authorization can be loaded separately from the authentication since these are different tables. (From a file, separate table). It also put less burden on the developer when implementing a custom User.

  • Not sure I understand. Do you suggest calling IsAuthorized in the handler in addition to it being called it in the middleware redundant or the opposite? Besides, Laravel does this as well (With the policies and the can function). Can you explain how to see this? And I agree with you about different sets of permissions for routes and models. It feels a bit off. It's connected to the next point.

  • I agree everything should be defined the same way. In Laravel, they solved it by allowing you to add custom functions instead. This way, you can define functions wherein one function you don't receive ids, and it is suitable for routes, and in the others you do, and it's ideal for models. I think it's a nice solution, but most of the time, the developer won't need custom logic and implement every validation. I'll try to think of a solution that addresses both of these issues. What do you think?

amit-davidson avatar Mar 15 '20 18:03 amit-davidson

Calling IsAuthorized both in the handler and in the middleware is redundant. It should be possible but shouldn't be the primary way to do it, especially if it's only for models. Laravel does it so we can also use the authorization system in artisan commands, scheduled jobs, etc.

In short: authorization should always be defined in the route definition, preferably as a middleware. However, the developer should still be able to check authorization in another context (with the can function for example), such as in a background go routine, a websocket, etc. Like this, the system is flexible enough to suit everyone's need.

I think custom authorization function is indeed a must, and is probably the best way to setup authorization on models and services that are not related to database records. I think we should focus on developing a base for the system and make it expandable, like the Goyave auth system: you can add more authenticators easily. Then we'll add a basic guard covering the most common authorization cases: list, show, create, update, delete. More complex cases such as the moderator being able to edit other users's post may be implemented by the developer.

System-Glitch avatar Mar 15 '20 19:03 System-Glitch

I have most of it on my mind, but there are a few more points I'm not confident about yet. If it's possible for you, I think it's better if we sum it up over an audio call like zoom or even chat.

amit-davidson avatar Mar 16 '20 07:03 amit-davidson

I'll email you to know how we can contact each other. A summary of what's been said will be posted here as well.

System-Glitch avatar Mar 16 '20 08:03 System-Glitch

Summary of yesterday's chat

Roles and policies will be dynamic and persistent: two tables will be created by the framework.

  • user_roles : pivot table for many to many relation between users and roles
  • roles: the table where roles will be stored.

A Permissionable struct will be added. This struct shall be promoted inside a user model.

type Permissionable struct {
  Roles []Role `gorm:"many2many:user_roles;"`
}

This allows us to take advantage of the relation handling in Gorm.


Roles and policies will be manageable through standard functions such as auth.CreateRole, myRole.AddPolicy(), etc. Therefore, the developer will be able to register roles in different ways:

  • Using a startup hook, following the same principle as automatic migrations. New roles will be inserted with INSERT IGNORE.
  • Inside handlers. The developer has the freedom of managing roles manually from his controllers and middleware.
  • A basic CRUD controller for roles will be implemented, letting developers add easy dynamic roles management through API calls with a single line in their route definitions. This controller will cover the typical role management over API, and the developer will still be able to implement one himself if his needs are more specific.

Most likely use-cases:

  • The app has static roles. They are defined in the startup hook. The hook will create them if they don't exist.
  • The app has dynamic roles. No need for a startup hook, they will be created by API calls.
  • The app has mixed (static and dynamic) roles. The default roles are created in the startup hook if they don't exist, and are created and modified by API calls.

This system allows developers to move from use-case to another very easily and would require minimal changes to the application code to extend or shrink its roles functionalities mid-development.


An authorization middleware will be implemented. This middleware will load the user's related roles and check if he has access to the requested route. Once loaded, the roles stay available and can be used inside custom middleware and controller handlers.

Thanks to the Permissionable struct, reflection is not needed, we can use type assertion.


Roles and users won't be loaded at all times into memory for better memory efficiency and scalability, at the cost of very slightly longer response time.


On top of the URI authorization definition, developers will be able to implement custom authorization functions. These functions will be located in a new folder in the recommended directory structure. This directory name is still to be defined.

System-Glitch avatar Mar 18 '20 13:03 System-Glitch

I am still looking for contributors to implement this feature.

System-Glitch avatar Mar 28 '20 13:03 System-Glitch

I am still looking for contributors to implement this feature.

I can help.

dhairya0904 avatar Oct 20 '20 07:10 dhairya0904

Hello @dhairya0904 ! Thank you very much for your help ! Don't hesitate to contact me (via email, telegram or discord) if you have any question or if you need guidance.

Shall I move this issue to "In progress"?

System-Glitch avatar Oct 20 '20 08:10 System-Glitch

Hello @dhairya0904 ! Thank you very much for your help ! Don't hesitate to contact me (via email, telegram or discord) if you have any question or if you need guidance.

Shall I move this issue to "In progress"?

Sure

dhairya0904 avatar Oct 20 '20 11:10 dhairya0904