Open System-Glitch opened 4 years ago
@System-Glitch I have an idea on my mind. I'll write a proposal and we can discuss it.
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.
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.
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.
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 )['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. 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.A new function called IsAuthorized will be added to Goyave. Its signature is:
IsAuthorized(user interface{}, permissions []string, ownerName string, itemId string)
where:
itemID - the ID of the item. If the parameter is not relevant, an empty string will be passed.
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.User
is given from the request. The permissions are provided, and ownerName
and itemID
will be left empty as they have no meaning.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.
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?
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:
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.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.
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).IsAuthorized
on top of a middleware is in my opinion not a good approach.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.
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?
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.
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.
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.
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 rolesroles
: 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:
INSERT IGNORE
.Most likely use-cases:
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.
I am still looking for contributors to implement this feature.
I am still looking for contributors to implement this feature.
I can help.
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"?
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
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 theIsModerator
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:
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.