Stateless (Flyweight) and current user

Dmitri Dolguikh dmitri at redhat.com
Thu Apr 1 16:28:24 UTC 2010


On 04/01/2010 12:35 PM, Adam Young wrote:
> On 04/01/2010 11:20 AM, Dmitri Dolguikh wrote:
>    
>> On 04/01/2010 12:16 PM, Justin Harris wrote:
>>
>>      
>>> ----- "Bryan Kearney"<bkearney at redhat.com>    wrote:
>>>
>>>
>>>
>>>        
>>>> On 04/01/2010 10:55 AM, Justin Harris wrote:
>>>>
>>>>
>>>>          
>>>>> ----- "Bryan Kearney"<bkearney at redhat.com>     wrote:
>>>>>
>>>>>
>>>>>
>>>>>            
>>>>>> On 04/01/2010 10:40 AM, Adam Young wrote:
>>>>>>
>>>>>>
>>>>>>              
>>>>>>> The current implementation of candlepin is complete oblivious to
>>>>>>>
>>>>>>>
>>>>>>>                
>>>>>> the
>>>>>>
>>>>>>
>>>>>>              
>>>>>>> caller.  We can authenticate, but once we do, we do not tailor
>>>>>>>
>>>>>>>
>>>>>>>                
>>>> the
>>>>
>>>>
>>>>          
>>>>>>> output of the calls based on who logged in.
>>>>>>>
>>>>>>> If we are to implement the multi-org functionality, we need to
>>>>>>>
>>>>>>>
>>>>>>>                
>>>>>> rpovide a
>>>>>>
>>>>>>
>>>>>>              
>>>>>>> mechanism for checking "who asked this."
>>>>>>>
>>>>>>>
>>>>>>> For example, on  ConsumerResource.list is implemented as:
>>>>>>>
>>>>>>>        public List<Consumer>      list() {
>>>>>>>               return consumerCurator.findAll();
>>>>>>>           }
>>>>>>>
>>>>>>>
>>>>>>> To filter this via user, we have three choices.  I'm going to use
>>>>>>> rincipal for the user, as we don't havea first calss user object
>>>>>>>
>>>>>>>
>>>>>>>                
>>>>>> yet,
>>>>>>
>>>>>>
>>>>>>              
>>>>>>> but we can change that later.
>>>>>>>
>>>>>>> 1.  Change the method signature
>>>>>>>
>>>>>>>
>>>>>>>        public List<Consumer>      list(Principal p) {
>>>>>>>               return consumerCurator.findByOwner(getOwner(p));
>>>>>>>           }
>>>>>>>
>>>>>>>
>>>>>>> 2.  Add it to the ConsumerResource constructor via dependency
>>>>>>>
>>>>>>>
>>>>>>>                
>>>>>> injection.
>>>>>>
>>>>>>
>>>>>>              
>>>>>>> (Note that I wouldn't really implement getOwner this way, just to
>>>>>>> demostrate)
>>>>>>>
>>>>>>>
>>>>>>>        public List<Consumer>      list() {
>>>>>>>               return
>>>>>>> consumerCurator.findByOwner(this.getOwner(this.getPrinciapl()));
>>>>>>>           }
>>>>>>>
>>>>>>> 3.  Fetch it from a context like JNDI
>>>>>>>
>>>>>>>        public List<Consumer>      list() {
>>>>>>>               Principal p = (Principal)new
>>>>>>> InitialContext().resolve(Principal.class.getName());
>>>>>>>               return consumerCurator.findByOwner(this.getOwner(p));
>>>>>>>           }
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Option 1 is best for cases where this is going to be only
>>>>>>>
>>>>>>>
>>>>>>>                
>>>> required
>>>>
>>>>
>>>>          
>>>>>> for
>>>>>>
>>>>>>
>>>>>>              
>>>>>>> one or two methods.  THe Principal will be needed for just about
>>>>>>>
>>>>>>>
>>>>>>>                
>>>>>> every
>>>>>>
>>>>>>
>>>>>>              
>>>>>>> method.
>>>>>>>
>>>>>>> Option 2 means that the Resource Objects will no longer be
>>>>>>>
>>>>>>>
>>>>>>>                
>>>>>> stateless,
>>>>>>
>>>>>>
>>>>>>              
>>>>>>> but instead will have to be created either on each request, or
>>>>>>>
>>>>>>>
>>>>>>>                
>>>> once
>>>>
>>>>
>>>>          
>>>>>> per
>>>>>>
>>>>>>
>>>>>>              
>>>>>>> session
>>>>>>>
>>>>>>> Option 3 menas we have non-explicit dependencies.
>>>>>>>
>>>>>>>
>>>>>>> Do we need a meeting for this?
>>>>>>>
>>>>>>>
>>>>>>>                
>>>>>> I would say no, and I would be +1 for the Option 3. I would suggest
>>>>>>
>>>>>>
>>>>>>              
>>>> we
>>>>
>>>>
>>>>          
>>>>>> use a Context object which could hide me from JNDI, Guce, or name
>>>>>>
>>>>>>
>>>>>>              
>>>> your
>>>>
>>>>
>>>>          
>>>>>> magic thread local pattern..
>>>>>>
>>>>>>
>>>>>>              
>>>>> I think that this is a losing argument for me, but I am going to
>>>>>
>>>>>
>>>>>            
>>>> echo that I think having a resource look different depending on who is
>>>> looking at it violates RESTiness and also sets us up for a tough road
>>>> wrt maintenance.  I really would like to explore organizing our URLs
>>>> to specify the collections more accurately, and using the user/roles
>>>> to either allow or deny access - and that is all.
>>>>
>>>>
>>>>          
>>>>>      - Justin
>>>>>
>>>>>
>>>>>
>>>>>            
>>>> This is a red herring a bit in the discussion. Think of it how do you
>>>>
>>>> get data from the resource into the curator.
>>>>
>>>> FWIW.. I dont think it does violate restfullness. I am seeing eaxmples
>>>>
>>>> of people using media to coerce what comes back. User and security is
>>>>
>>>> not much different.
>>>>
>>>>
>>>>          
>>> Yes I see your point now.  Sorry about that. :)   Has anyone looked into if Guice can do a lot of this for us?  I'm not sure about the component lifecycles for Guice, but I know that for example Seam will do dependency injection per request, so that we can have our auth filters set up the current Owner, Roles, etc. and just pass those in as parameters to the resource (or curators).  Relying on the DI framework can also simplify unit testing because you just pass in what you want and don't have to worry about mocking out JNDI contexts which would make me want to punch myself in the face.
>>>
>>>
>>>        
>> Yup. we can have http-request-scoped objects. And a custom 'provider' to
>> inject those.
>> -d
>>
>>      
>
>
> Yep.  The reason to not do that is to limit the amount of DI overhead
> per request call.    We aren't doing that thus far, but we might want to
> do so in the future.
>    
Majority of DI overhead comes from guice creating proxies, executing aop 
interceptors and such. We use guice already and are paying this price. 
Guice is also one of the faster di frameworks out there.
>
> Of course, then you have the question about making all object references
> into instance variables.
>
> What my gut is telling me is that stuff that survives over multple
> requests should be member variables of the Resources, and stuff that is
> per-request should be parameters to functions.  I'd hold off on
> createing a general purpose "context" until we are certain we need it,
> and instead add the Principal or User object to the method calls that
> require it.
>    
I must admit, i don't quite understand the point of passing a user 
principal object as a parameter on method calls. Since it's a security 
thing (cross-cutting concern), it may end up being used on a lot of 
calls to the db.

Will this make api easier to understand? How are you going to use the 
principal in those calls? Are you planning on making an explicit method 
call to verify authorization?

Personally, I prefer a declarative approach to handle roles-based 
authorization, and guice-aop makes that quite feasible (and less 
intrusive compared to explicit calls to handle security). Bryan posted a 
link to a toy interceptor that I used to verify the above approach, if 
that's of any help.

> I missed the JAAS discussion, as I think it happened prior to me joining
> the team.  Why did we remove JAAS, and why do we think we might want it
> back now?
>
>    
JAAS was removed because we can't mix non-JAAS and JAAS authentication 
in the way we need, and it's rather inconvenient to work with (jaas 
module has to be deployed at the container level, and therefore must be 
independent of the rest of the application.) The only reason JAAS was 
looked at was to support SSL-cert authentication, but it can be handled 
with a servlet filter.
>>>
>>>        
>>>> -- bk
>>>>
>>>>
>>>>          
>>> _______________________________________________
>>> candlepin mailing list
>>> candlepin at lists.fedorahosted.org
>>> https://fedorahosted.org/mailman/listinfo/candlepin
>>>
>>>
>>>        
>> _______________________________________________
>> candlepin mailing list
>> candlepin at lists.fedorahosted.org
>> https://fedorahosted.org/mailman/listinfo/candlepin
>>
>>      
> _______________________________________________
> candlepin mailing list
> candlepin at lists.fedorahosted.org
> https://fedorahosted.org/mailman/listinfo/candlepin
>    
-d



More information about the candlepin mailing list