[
https://issues.apache.org/jira/browse/ATLAS-497?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15264200#comment-15264200
]
Saqeeb Shaikh commented on ATLAS-497:
-------------------------------------
Thanks [~yhemanth] for reviewing the patch. Please find my comments inline.
* Do we have a requirement for separating creates and updates? Can we merge
them into one write operation? In fact many operations in Atlas backend are a
create or update kind of operation. Merging into one may be better, IMHO.
** *Can we please finalize the requirement for this and track it as part of
another JIRA?*
* In AtlasAccessRequest and PolicyUtil there are many unused methods. Please
remove them.
** *Some of the unused methods from AtlasAccessRequest, will be used when I add
support for RangerPlugin. I will remove the setter methods from the
PolicyUtils.*
* In the authorization code path where AtlasException is thrown due to
authorization problems, maybe it is better to throw a custom
AtlasAuthorizationException. This could have information about what was
attempted to be accessed etc.
** *Will make this change*
* For requests that require access to multiple resource types (e.g. paths like
entities/traits - which requires access to both entities & traits), access
should be granted only if all of them are allowed, no? Currently, even if one
matches we are allowing access, as far as I can tell.
** *Yes, you are right, the access to such resources is granted only if the
user or group have access to both the resource types. In
SimpleAtlasAuthorizer.checkAccess() method, I am iterating through each of the
resourceTypes that the user has access to and if he has access to all of them
only then he is granted access.*
* Currently, since we don't have resource specific match, in
SimpleAtlasAuthorizer, can we simplify the resource check logic and just check
for access to resourcetypes for now?
** *Yes, I can change it for now, however I had added this keeping in mind that
down the line we will support resource filtering as well.*
* Without the above, there are some important issues: for e.g. since
SimpleAtlasAuthorizer is a singleton object, the value isMatchAny is being
accessed in a non-thread safe manner.
** *Yes, I will fix this*
* In a later JIRA, we'll need to figure how principal information like user
name / groups will be got in Kerberos authentication case. This is because
currently we are picking these up from Spring security context.
** *Yes, I will raise another JIRA for this.*
* Can we please add a merge test in PolicyUtilTest - one that has > 1 policies
with different (possibly conflicting) rules and see how the end result works
out?
* Please add some tests for AtlasAuthorizationFilter.
** *Yes, I am adding more test cases for this feature.*
> Simple Authorization
> --------------------
>
> Key: ATLAS-497
> URL: https://issues.apache.org/jira/browse/ATLAS-497
> Project: Atlas
> Issue Type: New Feature
> Affects Versions: 0.7-incubating
> Reporter: Erik Bergenholtz
> Assignee: Saqeeb Shaikh
> Fix For: 0.7-incubating
>
> Attachments: ATLAS-497.1.patch, ATLAS-497.2.patch, ATLAS-497.patch
>
>
> Atlas needs to support a simple (out of box) authorization mechanism.
> Defined Roles:
> - Data Scientist: provides a read only view (GET)
> - Data Steward: provides a read/edit view (PUT, POST, DELETE)
> - Admin (can do anything)
> All can comment on entity
> Requirements
> - Atlas will implement a simple file based store for providing user to role
> mapping
> - The out of box experience will be this file based mechanism for
> authorization
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
