Stateless (Flyweight) and current user

Justin Harris jharris at redhat.com
Thu Apr 1 16:36:30 UTC 2010


----- "Dmitri Dolguikh" <dmitri at redhat.com> wrote:

> 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.

Yeah +1 on this.  When we start to single out Guice as bottleneck for 
performance problems, I think we have other issues.


> >
> > 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
> _______________________________________________
> candlepin mailing list
> candlepin at lists.fedorahosted.org
> https://fedorahosted.org/mailman/listinfo/candlepin



More information about the candlepin mailing list