ambari-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Robert Levas" <rle...@hortonworks.com>
Subject Re: Review Request 40606: Enforce granular role-based access control for user functions
Date Mon, 23 Nov 2015 22:45:37 GMT


> On Nov. 23, 2015, 3:47 p.m., Jonathan Hurley wrote:
> > What about using AOP for this kind of stuff? Instead of trying to find and sprinkle
the code with a bunch of tightly coupled calls, you could easily intercept multiple join point
matches. Kind of prevents problems with placing the checks in resource providers vs impls.
I did notice that there were some checks added to AMCImpl - just seems like it's going to
be hard to know what's covered and what isn't.
> 
> Robert Levas wrote:
>     I guess we could create our own annotations, but it seems liked more work than my
current approach.  In many caes, we need to look at the request to determine if the user can
perform the operation. For example, some fields can only be updated based on role... or you
can view/edit resources that you _own_ but cannot have access or know about other resources
of the same type - for example, I shouldn't be able to _know_ whether a user with some username
exists.
> 
> Robert Levas wrote:
>     I think if the API was RPC-based, it would be a different story and we would be able
to annotate the interfaces rather than need to perform logic on the request data before determing
authorization.
> 
> Jonathan Hurley wrote:
>     You still have access to all of the parameters being passed into the join points;
it's not really annotation-based, but advice-based. It's just a thought. Typically when you
have cross cutting concerns like logging and security you'd use AOP to decouple your code.
It just feels very brute-force-ish to add it directly into each method. There's no single
piece of advice that's being applied to multiple places.
>     
>     With that said, I have no real issues with the patch; just thought that this would
be a great opportunity to decouple security from the providers.

Unless I am missing something, if we took the approach you are suggesting, we would need to
parse the request and predicate for each call before we allow the call to execute. I think
this is a good idea, however a request may contain multiple requests and we will need to have
intimate knowlege of what the request might be so we could analyze it.  If this was a one-off
thing, I might be ok with that, but since it is likely for all end points, it just seemed
to make sense to do it inline since the logic to parse the requests and predicates already
works.


> On Nov. 23, 2015, 3:47 p.m., Jonathan Hurley wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/security/authorization/AmbariAuthorizationFilter.java,
lines 61-67
> > <https://reviews.apache.org/r/40606/diff/1/?file=1137525#file1137525line61>
> >
> >     I think we're missing one for /alert_targets ... that's outside the scope of
a cluster and might be missed.
> 
> Robert Levas wrote:
>     I am not sure I follow this. The current patch is for the user and privilege resources.
Alerts will be handled later. 
>     
>     Maybe this is a current security flaw that will be fixed ones the rest of the RBAC
patches are created/applied?
> 
> Jonathan Hurley wrote:
>     I think you're right - it's a current flaw that needs to be covered later. I'll drop
it for now since it's not part of the scope (or you could just add it :) )

I was going to do the whole API in one patch, but then the patch would be HUGE! So I am breaking
it up into peices.


- Robert


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40606/#review107642
-----------------------------------------------------------


On Nov. 23, 2015, 2:53 p.m., Robert Levas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40606/
> -----------------------------------------------------------
> 
> (Updated Nov. 23, 2015, 2:53 p.m.)
> 
> 
> Review request for Ambari, Jonathan Hurley, Nate Cole, Robert Nettleton, and Sumit Mohanty.
> 
> 
> Bugs: AMBARI-13977
>     https://issues.apache.org/jira/browse/AMBARI-13977
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Enforce granular role-based access control for user functions:
> 
>                               | Cluster | Service  | Service       | Cluster  | Cluster
      |
> 							  | User    | Operator | Administrator | Operator | Administrator | Administrator
> ------------------------------|---------|----------|---------------|----------|---------------|--------------
> Create new clusters           |         |          |               |          |     
         | (+)           
> Manage users                  |         |          |               |          |     
         | (+)           
> Assign permissions/roles      |         |          |               |          |     
         | (+)
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementController.java
ea7603f 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java
443c715 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AbstractResourceProvider.java
3464c19 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ActiveWidgetLayoutResourceProvider.java
52b0d56 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AmbariPrivilegeResourceProvider.java
3670775 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterPrivilegeResourceProvider.java
bbcd4a1 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/PrivilegeResourceProvider.java
88e9906 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UserAuthorizationResourceProvider.java
15aa0ec 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UserPrivilegeResourceProvider.java
a8a9909 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UserResourceProvider.java
b993450 
>   ambari-server/src/main/java/org/apache/ambari/server/security/authorization/AmbariAuthorizationFilter.java
81794d8 
>   ambari-server/src/main/java/org/apache/ambari/server/security/authorization/AuthorizationHelper.java
198e209 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerImplTest.java
1d9e53d 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerTest.java
385e3f7 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ActiveWidgetLayoutResourceProviderTest.java
e74520e 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AmbariPrivilegeResourceProviderTest.java
68f1467 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClusterPrivilegeResourceProviderTest.java
1412470 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/UserAuthorizationResourceProviderTest.java
e71c219 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/UserPrivilegeResourceProviderTest.java
e65786b 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/UserResourceProviderTest.java
94f6fd7 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ViewPrivilegeResourceProviderTest.java
8400efd 
>   ambari-server/src/test/java/org/apache/ambari/server/security/TestAuthenticationFactory.java
PRE-CREATION 
>   ambari-server/src/test/java/org/apache/ambari/server/security/authorization/AmbariAuthorizationFilterTest.java
2efab89 
> 
> Diff: https://reviews.apache.org/r/40606/diff/
> 
> 
> Testing
> -------
> 
> Manually tested
> 
> # Local test results: 
> 
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 57:31.344s
> [INFO] Finished at: Mon Nov 23 14:52:50 EST 2015
> [INFO] Final Memory: 67M/1255M
> [INFO] ------------------------------------------------------------------------
> 
> # Jenkins test results: PENDING
> 
> 
> Thanks,
> 
> Robert Levas
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message