RedHatInsights / insights-api-common-rails

Header, Encryption, RBAC, Serialization, Pagination and other common behavior for Insights microservices built with Rails
Apache License 2.0
3 stars 25 forks source link

RBAC - Added support for scopes in permissions #171

Closed mkanoor closed 4 years ago

mkanoor commented 4 years ago

In the RBAC resource definition we are adding scope to differentiate between admin, groups and users

Sample Usage by Approval

x = Insights::API::Common::RBAC::Access.new
my_scopes = x.scopes('workflows','read')
if my_scopes.include?('admin')
    # handle admin scope 
    # Worfklows.all
elsif my_scopes.includes?('group')
   # Handle Group Scope
elsif my_scopes.includes?('user')
   # Handle User Scope
else
   # Nothing Accessible
end

Sample Usage by Catalog API

x = Insights::API::Common::RBAC::Access.new('catalog, approval')
my_scopes = x.scopes('portfolios','read')
if my_scopes.include?('admin')
    # handle admin scope 
    Portfolios.all
elsif my_scopes.includes?('group')
   # Handle Group Scope
elsif my_scopes.includes?('user')
   # Handle User Scope
else
   # Nothing Accessible
end

# Somewhere else when doing Set Approval
my_scopes = x.scopes('workflows', 'read',  'approval')
my_tags_scopes = x.scopes('taglinks', 'write',  'approval')
if my_scopes.count > 0 && my_tags_scopes.count > 0
    # User can set approval on Portfolio
end
mkanoor commented 4 years ago

@bzwei @hsong-rh Please review removed old methods and cleaned up specs.

mkanoor commented 4 years ago

@bzwei We have a ticket opened with the RBAC team https://projects.engineering.redhat.com/browse/RHCLOUD-5269 Which might give us more clarity on what the app_name filter would look like

'catalog,approval' '*' ''

or something else

mkanoor commented 4 years ago

@gmcculloug I am not sure adding an extra parameter helps anything here for a normal use case say for approval which only ever works with only its objects the code would look like

x = Insights::API::Common::RBAC::Access.new
x.accessible?('workflows','read')
x.admin_scope?('workflows','read')

For a case like catalog which needs to get access from catalog and approval and maybe topology down the road

x = Insights::API::Common::RBAC::Access.new('catalog, approval')
x.accessible?('portfolio','read')
x.admin_scope?('portfolio','read')

x.accessible?('workflow','read','approval')

You need to pass in the extra argument only when you are trying to get permission check for objects which are not in your app, When you need to check for permissions in your app you dont have to pass in the extra argument. It couldn't be much simpler than that. Adding extra argument in the initializer has no value since the app name is needed when you make the check. Only when you need to check for permissions for objects not in your own app.

mkanoor commented 4 years ago

I changed this to WIP temporarily because the caller might see a big performance improvement if it collected all the scopes in a single call and then did some logic locally.

e.g

x = Insights::API::Common::RBAC::Access.new
if x.admin_scope?('workflows', 'update')
   # Do Admin Stuff
else if x.group_scope?('workflows','update')
  # Do Group Stuff
else if x.user_scope?('workflows,'update')
  # Do User Stuff
else
     puts "Not Accessible"
end

versus Collecting all scopes in a single call so you dont have to traverse the array multiple times.

x = Insights::API::Common::RBAC::Access.new
current_scopes = x.scopes('workflows', 'update')
if current_scope.includes?('admin')
   # Do Admin Stuff
else if current_scope.includes?('group')
   # Do Group Stuff
else if current_scope.includes?('user')
   # Do User Stuff
else
   puts "Not Accessible"
end
mkanoor commented 4 years ago

@hsong-rh @bzwei Please review the example usages. I have added a new method called scopes since it will make it easier to make a single pass and collect all the scopes for an object.

The use case is

A user can be part of all these groups at the same time

Or the user could be part of 2 groups

So the scopes method allows you to collect all the scopes with a single call and then have the caller decide the order of precedence. Admin will be checked first then group and then user.

gmcculloug commented 4 years ago

@mkanoor I like the new approach as it does not require this code to know the scope values (admin, group, user) and the app has more control over how it accesses the data. 👍

mkanoor commented 4 years ago

@bzwei Please review