Real-Dev-Squad / website-backend

The backend code for all our website-related apps
https://api.realdevsquad.com/
MIT License
55 stars 257 forks source link

Create levels of access in the recently added data access layer #1314

Closed mailmesriza98 closed 1 year ago

mailmesriza98 commented 1 year ago

Parent issue:

In the current data access layer we are removing all the sensitive informational details like phone, email, chaincode, tokens. while fetching users data. However, going forward we would be providing an option to provide different levels of access in the dataAccess layer so that the module utilizing the data access layer can pass on its requirement (level 1 or 2 etc) for the granularity of information needed in the sensitive user data.

The levels will be categorized as below:

The module using the dataAccess layer can pass on the levels required as arguments depending on the requirements. Also a privileged role like superusers can be able to pass on a certain level at a time if required to gather sensitive information.

Please let me know thoughts and comments on this issue.

heyrandhir commented 1 year ago

This approach looks good to me, However I have some doubts

Will we have some restriction on the layered authorized access. Will layer 0 data be accessible to all user or only super user. How will we be managing access levels authorization. Implementing robust access control mechanisms rules for user privileges will be essential to avoid data breaches and unauthorized access.

Additionally, providing detailed documentation on access level guidelines and enforcing best practices will help users and developers understand and adhere to the defined data access policies effectively.

vikhyat187 commented 1 year ago

Hi @mailmesriza98 if you have any rough approach in mind, can you just create a diagram for the approach that you will follow to implement this?

mailmesriza98 commented 1 year ago

thanks @heyrandhir and @vikhyat187 for the comments. So there are different aspects to approaching this issue as per as I could gather from my research:

Access Control Approach:

RBAC: RBAC is based on the concept of assigning roles to users, and then associating permissions with these roles. Users are assigned to one or more roles, and each role has specific privileges associated with it. For e,g we have the roles like super_user, member, restricted, archived, in_discord ABAC: ABAC uses attributes as the basis for access control decisions. Attributes can be characteristics or properties of users, objects, or the environment. Access control policies are defined using rules that consider these attributes, allowing for more fine-grained and flexible control over access. However in our case we either have to consider the roles as some of our attributes, other than that as per the data model we have isMember and userType

Granularity:

RBAC: RBAC provides coarse-grained access control, as permissions are associated with roles rather than individual users or resources. This can lead to situations where users might have more privileges than necessary for their specific tasks for e.g if in firestore someone can change their status to super_user to perform certain privileged actions like assigning tasks or badges ABAC: ABAC offers fine-grained access control, as access decisions are based on attributes and policies can be tailored to specific combinations of attributes, allowing for more precise control over access to resources. However for this we might need to introduce more attributes in our users data model to specifically quantify our distinction for the levels of access

Dynamic vs. Static:

RBAC: RBAC is often more static in nature, where the assignment of roles to users is typically defined in advance and may require administrative intervention to modify. ABAC: ABAC can be more dynamic since access decisions are based on attributes that can change over time or based on different conditions. This adaptability makes ABAC more suitable for complex and evolving access control requirements.

Complexity:

RBAC: RBAC tends to be less complex and easier to implement, especially in smaller systems with a limited number of roles and permissions. For example in our case with most of the use cases in the backend currently being restricted to only super users and for all other operations we are doing it for general users ABAC: ABAC can be more complex to design and manage, especially in larger systems with a diverse set of attributes and access control policies. However, it provides greater flexibility and precision in access control.

From this comparison, if we add certain attributes in our data models based on roles etc it will help separate the concern and levels of privilege for example for a superuser a role policy could be like:

{
    "Effect": "Allow",
    "Action": "assignBadge, postBadge, createBadge, postItems, deleteItems",
} ,

a restricted user on the other hand should have deny for all the actions that is performable via post or patch, only get methods are allowed(read-only) for a util or service or model could have a policy as below in case we need service specific roles

{
    "Effect": "Allow",
    "Action": "assignBadge",
    "Resource": "badgeQuery.createBadge"
} ,

For the above we may need to update our existing users, tasks, badges and other data models to include the access level. Please let me know suggestions on this approach and we can further discuss upon how much data should be accessible at each level and how we can enumerate it. Attached a rough diagram to explain the process, levelsOfAccess

reference : https://docs.aws.amazon.com/IAM/latest/UserGuide/access_policies.html https://docs.aws.amazon.com/IAM/latest/UserGuide/using-service-linked-roles.html

vikhyat187 commented 1 year ago

I think granular level based permissions should be implemented, as this can help to protect the system, say a malicious actor tries to cause harm to the resources or misuse the data, we can put the restriction on user level, in case of the role based access we will have to create a new role to restrict him / her.

@heyrandhir @prakashchoudhary07 what are your thoughts

prakashchoudhary07 commented 1 year ago

I think, we should go ahead with role based approach. We are already having role based system in place.

mailmesriza98 commented 1 year ago

thanks @prakashchoudhary07 and @vikhyat187 for the comments. So in order to go ahead with role based approach, will we need to create additional roles/attributes for users and attach any policies for them? How should we be going with the design in this case?

The roles that we currently have are:

"roles": {
    "app_owner": false,
    "archived": false,
    "member" : true,
    "restricted": false,
    "super_user": false,
    "in_discord" : true,
  }

1 approach that I felt after revisiting the current data models is drafted here :

{ role: superuser allowedActions:{ tasks:{ get: [listOfRoutes], post: [listOfRoutes], put: [listOfRoutes], patch: [listOfRoutes], delete:[listOfRoutes]}, skills:{ get: [listOfRoutes], post: [listOfRoutes], put: [listOfRoutes], patch: [listOfRoutes], delete:[listOfRoutes]}, }

however this will require a complete redesign of authorizeRoles and authenticate middlewares as to how the flow currently happens with respect to the routes, since at the moment we are only checking for superUser roles and restricted roles, Also the data model will need to be constantly updated every time a new route is added. And since the level of access is wrt the user data which might not be required in every route, the above approach might not be the most optimal one.

@prakashchoudhary07 @vikhyat187 @heyrandhir kindly let me know your thoughts on the above approaches in order to have a final design for this issue

heyrandhir commented 1 year ago

Hey @mailmesriza98 , excellent job on documenting the distinction between Role-Based and Attribute-Based Access Control. I completely agree with @prakashchoudhary07 and @vikhyat187 that the role-based approach is the way to go. It provides a straightforward and manageable way to handle access control for our use case. We should add additional middleware as you mentioned in the 2nd approach in the previous comment.

mailmesriza98 commented 1 year ago

thank you @heyrandhir for the comment. @prakashchoudhary07 could you confirm if this method on middleware will be the final design? We can have a new middleware for levels of access after authorizeRoles or we could have a single middleware that checks the role of the user instead of authorizeRoles and grant the level of access accordingly. This would require creating a new middleware which can then be used by the different routes and it will not have any dependency on any new routes etc So initially the authenticate middleware will be returning the unfiltered userdata which would then be passed on to the new middleware to filter sensitive info on the level based on the role of the user. In either case would we be keeping the same roles on routes that we have currently? like only superuser, restricted user and normal user that we have at present or should we include all the possible roles in the middleware and depending on it specify level?

Should we create any enum/constants repo like we were discussing once to have the levels enumerated?

Let me know your thoughts @prakashchoudhary07 @heyrandhir thanks again

mailmesriza98 commented 1 year ago

Hi so as per our discussion we will be storing the levels -> roles relation in the data access layer determining which roles is allowed to how much level. So a general user would have level 0, a superuser can have level 1 and level 2 and have level 0 by default, and restricted users would have no access to any information. Instead of creating a new middleware, once the dataAccessLayer is passed on the required level, it will check if the calling user's data roles match upto the level specified and if it is apt, then the required level of data is returned , else a 403 error is returned. Level 0,1,2,3 could be renamed to Public, Internal, Restricted, Confidential for better readability.

@prakashchoudhary07 , @heyrandhir , @vikhyat187 let me knoe your thoughts on the same.

heyrandhir commented 1 year ago

Hi so as per our discussion we will be storing the levels -> roles relation in the data access layer determining which roles is allowed to how much level. So a general user would have level 0, a superuser can have level 1 and level 2 and have level 0 by default, and restricted users would have no access to any information. Instead of creating a new middleware, once the dataAccessLayer is passed on the required level, it will check if the calling user's data roles match upto the level specified and if it is apt, then the required level of data is returned , else a 403 error is returned. Level 0,1,2,3 could be renamed to Public, Internal, Restricted, Confidential for better readability.

@prakashchoudhary07 , @heyrandhir , @vikhyat187 let me knoe your thoughts on the same.

Yeah this approach sounds good to me.

vikhyat187 commented 1 year ago

Hi so as per our discussion we will be storing the levels -> roles relation in the data access layer determining which roles is allowed to how much level. So a general user would have level 0, a superuser can have level 1 and level 2 and have level 0 by default, and restricted users would have no access to any information. Instead of creating a new middleware, once the dataAccessLayer is passed on the required level, it will check if the calling user's data roles match upto the level specified and if it is apt, then the required level of data is returned , else a 403 error is returned. Level 0,1,2,3 could be renamed to Public, Internal, Restricted, Confidential for better readability.

@prakashchoudhary07 , @heyrandhir , @vikhyat187 let me knoe your thoughts on the same.

Yeah agreed this sounds good to me

mailmesriza98 commented 1 year ago

thank you @heyrandhir and @vikhyat187 , will be implementing the same