>-----Original Message----- >From: Ryan J Baxter [mailto:rjbax...@us.ibm.com] >Sent: Wednesday, October 05, 2011 9:13 AM >To: dev@shindig.apache.org >Subject: RE: Review Request: Common container currently doesnt include the >siteId (moduleId) in any of it's security token processing/handling > >Jesse, to answer your question, I am not sure it is so much a bug as >opposed to it being one of those things that just needs to be implemented >by the container. In either case if we can provide a production level >implementation for it in Shindig I am all for it.
I was thinking of it as a bug because having a moduleId of 0 for all security tokens seems broken to me -- but I don’t really care what we call it. I'd be just as happy to call it an enhancement. >Stanton and I had a short discussion about this yesterday because we have >the situation I described in my easier email where we are reusing the site >for many different gadgets. I am still neutral on the code at this point >because I am not convinced yet that it is not going to cause some >unforseen problems. Sure -- I understand that and I'm happy to accommodate any changes that are needed to get the patch in order, but that wasn’t really what I was getting at. I'm not trying to get community support for the patch so that I can try to push the patch as its currently implemented -- I was more just trying to get a sanity check on doing this at all since the community has been so silent on it. I think you and Stanton have been the only ones to provide me any feedback at all (which I very much appreciate, btw) but it seemed to me that you guys were still questioning whether or not this should be done at all, which of course is perfectly fine and healthy to do. So I was sort of expected that someone else would think moduleId's were important too and would have spoken up over the last month in support for doing this, and since no one had I was/am concerned that I'm somehow missing something obvious which is why I was looking for someone else to agree that we should be doing this at all. >We are in the process of making the security tokens >"real" in our implementation, so we will be looking into this over the >coming weeks. Maybe this can be one of the topics we discuss next week in >person... Sounds good! >-Ryan > >Email: rjbax...@us.ibm.com >Phone: 978-899-3041 >developerWorks Profile > > > >From: "Ciancetta, Jesse E." <jc...@mitre.org> >To: "dev@shindig.apache.org" <dev@shindig.apache.org>, >Date: 10/05/2011 08:04 AM >Subject: RE: Review Request: Common container currently doesnt >include the siteId (moduleId) in any of it's security token >processing/handling > > > >Stanton has given this an initial nod code-wise, but it seems we're still >struggling for consensus on the need to/use cases for making this change >in the first place -- so before I continue working on this it would be >very nice if at least one other person could confirm that we should indeed >be making this change. > >I'm as sure as I can be myself that this is a bug that should be fixed, >but the fact that I've been working on this and sending mail to this list >for over a month now and haven’t received anything from the community to >confirm that this is something that should be fixed has me a little >concerned that I'm somehow missing something. > >So just at the very highest level -- if I could get someone else to please >confirm that common container using 0 for the moduleId in the security >token for all gadgets is a bug that needs to be resolved I would very much >appreciate it. > >Here is a link to the review for more detail: > >https://reviews.apache.org/r/1632/ > >Thanks! > >>-----Original Message----- >>From: Jesse Ciancetta [mailto:jc...@mitre.org] >>Sent: Tuesday, October 04, 2011 1:28 PM >>To: shindig; Ciancetta, Jesse E.; Stanton Sievers >>Subject: Re: Review Request: Common container currently doesnt include >the >>siteId (moduleId) in any of it's security token processing/handling >> >> >> >>> On 2011-10-04 16:28:14, Stanton Sievers wrote: >>> > Overall this looks good. It does lead me to wonder what your use >cases >>are for the module ID. In the Shindig Java code I only ever see >>org.apache.shindig.auth.SecurityToken.getModuleId() used in >>org.apache.shindig.gadgets.http.AbstractHttpCache.getInstanceId(HttpRequ >e >>st). What are the intentions of the module ID and do you have new use >cases >>for its use? >> >>Thanks for taking a look Stanton! >> >>Here are the use cases I am aware of: >> >>-- Within Shindig oAuth tokens are stored on a per user/per gadget >instance >>basis, and moduleId is used as the gadget instance identifier -- see >>BasicOAuthStore.makeBasicOAuthStoreTokenIndex(...) >> >>-- I've seen samples (although I cant seem to find them any longer) of >using >>the moduleId to scope appdata to a particular instance of a gadget. Since >>appdata is stored on a per application basis as opposed to a per >application >>instance basis, if you want to store appdata for a particular application >>instance you need some persistent identifier to use in the schema of the >>appdata that you store -- and moduleId is how I think people typically do >it. If >>anyone our there is doing this or can find the sample showing this usage >>please reply. >> >>-- Third party sites interested in how many users are using their gadget. > I >>believe that the moduleId is one of the parameters passed to third >parties >>when shindig sends a signed makeRequest, so I could envision gadgets that >>wanted to track how many instances are being used in the wild using >signed >>makeRequests to fetch their data -- then on their side the third party >could >>keep track of how many unique userId/moduleId combinations they see >>coming across the wire to get a pretty good idea of how many gadget >>instances are being used. >> >>If anyone has any other use cases please feel free to share. >> >> >>> On 2011-10-04 16:28:14, Stanton Sievers wrote: >>> > >>http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/e >x >>amples/commoncontainer/assembler.js, line 28 >>> > <https://reviews.apache.org/r/1632/diff/3/?file=46287#file46287line28 >> >>> > >>> > I'd rather we not set this by default and if we do, I'd rather it >be a bit >>bigger, e.g., 60 seconds or more. >> >>Agreed -- I've got a comment above this saying it is only there to >facilitate >>testing and shouldn't be included in a final patch. >> >> >>- Jesse >> >> >>----------------------------------------------------------- >>This is an automatically generated e-mail. To reply, visit: >>https://reviews.apache.org/r/1632/#review2310 >>----------------------------------------------------------- >> >> >>On 2011-09-28 19:36:42, Jesse Ciancetta wrote: >>> >>> ----------------------------------------------------------- >>> This is an automatically generated e-mail. To reply, visit: >>> https://reviews.apache.org/r/1632/ >>> ----------------------------------------------------------- >>> >>> (Updated 2011-09-28 19:36:42) >>> >>> >>> Review request for shindig. >>> >>> >>> Summary >>> ------- >>> >>> Common container currently doesn't include the siteId (moduleId) in any >of >>it's security token processing/handling (all security tokens in common >>container currently get minted with a moduleId of 0). >>> >>> This patch is a first (rough) cut at getting moduleId's into security >tokens. I >>am posting it somewhat prematurely to solicit feedback before I invest >any >>more time in finishing up the last bits. Here are the things that I know >are still >>left to do: >>> >>> -- Update unit tests on both the JS and Java side -- currently I've >been >>building and deploying with skipTests=true... >>> -- Figure out a strategy for dealing with preloaded gadgets. The >current >>auth-refresh process maintains tokens for preloaded gadgets, however the >>preoad JS functions just take a gadgetUrl so there is no concept of a >siteId >>(moduleId) for them at this time. >>> -- Figure out how to get the token that is included in the original >iframe to >>include a moduleId. I think the token in the iframe likely comes back in >the >>metadata request (although I haven't looked yet to verify), which means >that >>the call to the metadata service would likely need to include the >moduleId as >>well. >>> >>> I'd greatly appreciate any comments people have on the patch and >>strategies for dealing with the outstanding issues noted above. >>> >>> Thanks! >>> >>> >>> This addresses bugs RAVE-198 and SHINDIG-1611. >>> https://issues.apache.org/jira/browse/RAVE-198 >>> https://issues.apache.org/jira/browse/SHINDIG-1611 >>> >>> >>> Diffs >>> ----- >>> >>> >> >http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/ > >>features/container/service.js 1176946 >>> >> >http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/ > >>features/container.util/util.js 1176946 >>> >> >http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/ > >>features/container/container.js 1176946 >>> >>http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/e >x >>amples/commoncontainer/assembler.js 1176946 >>> >> >http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/ > >>features/container.gadget/gadget_site.js 1176946 >>> >> >http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/f > >>eatures/container/gadget_site_test.js 1176946 >>> >> >http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/f > >>eatures/container/service_test.js 1176946 >>> >>http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java >/ >>org/apache/shindig/gadgets/servlet/GadgetsHandler.java 1176946 >>> >>http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java >/ >>org/apache/shindig/gadgets/servlet/GadgetsHandlerApi.java 1176946 >>> >>http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java >/ >>org/apache/shindig/gadgets/servlet/GadgetsHandlerService.java 1176946 >>> >>> Diff: https://reviews.apache.org/r/1632/diff >>> >>> >>> Testing >>> ------- >>> >>> >>> Thanks, >>> >>> Jesse >>> >>> > > >
