I think we would certainly want to make sure that all container-proxied requests ferry along the moduleId.
I'll keep that in mind as I make these changes. Thank you for pointing it out. From: "Davies,Douglas" <davi...@oclc.org> To: <dev@shindig.apache.org>, Cc: "shindig" <dev@shindig.apache.org>, Dan Dumont/Westford/IBM@Lotus, "Jesse Ciancetta" <jc...@mitre.org> Date: 12/23/2011 10:24 AM Subject: Re: Review Request: Common container currently doesnt include the siteId (moduleId) in any of it's security token processing/handling Not to throw a monkey wrench into things, but maybe this will help set direction too. I reported bug a few months back https://issues.apache.org/jira/browse/SHINDIG-1557 I discovered that the security token used for rpc request (appdata, userPrefs) is the container security token, not the gadget's. So even if you get the siteId/moduleId in the token you might find you don't have access to it. Doug On Dec 23, 2011, at 7:45 AM, "Jesse Ciancetta" <jc...@mitre.org> wrote: > > >> On 2011-12-22 20:34:52, Dan Dumont wrote: >>> http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.gadget/gadget_site.js , line 419 >>> <https://reviews.apache.org/r/1632/diff/4/?file=64701#file64701line419 > >>> >>> Do you know why this is done in the loadingGadgetHolder? > > Yeah -- it ends up getting used further down the line during osapi.container.GadgetHolder.prototype.render -- specifically in osapi.container.GadgetHolder.prototype.getIframeUrl_ (line 346 -- 348). > > Although I just noticed something else interesting that I hadn't noticed before -- it looks like the original authors of CC are actually already using the siteId as the moduleId... They just (apparently) forgot to also use it in the security tokens. > > If you look at lines 350 and 351 in gadget_holder.js you'll see that the siteId is used as the 'mid' parameter in the iframe url -- which means that the gadgets JS API code that deals with moduleId's will end up using the siteId as the moduleId (because the gadgets JS API parses the moduleId from the 'mid' parameter in the iframe url). However as we've been discussing the moduleId in the security token gets set to 0 -- so anything on the server side that pulls the moduleId from the security token will get the wrong value. > > You can actually see all this play out right now in shindig head by messing around with the common container sample page and adding a few gadgets. To see what I'm taking about -- using shindig head -- browse to the CC sample container, add the todo and horoscope gadgets (to use up a couple of siteId's) and then add the activities stream sample (to get a gadget that needs a security token) -- then using firefox right click the whitespace in the activities gadget and load it into its own tab (so you can easily see the requests it makes in firebug and execute code against the gadgets API in the context of that gadget). Then -- if you reload that tab, watch in firebug and look at the security token passed with the soacial API calls -- you'll see the moduleId in it is 0 -- but if you execute this code in the console: > > var prefs = new gadgets.Prefs(); > console.log(prefs.getModuleId()); > > You'll get back 'gadget-site-2' > > So I guess this gives some precedent for using the siteId as the moduleId. As I've said before I personally think this would work out fine (using siteId as moduleId) and might be worth trying to start with and then changing later if we run into issues -- but in the end I dont think it makes much difference to me. There's also precedent for something similar elsewhere too (in both igoogle and rave and likely other places as well). If you look at igoogle for example you'll see that they wrap all their gadgets in a div which includes their moduleId (and in CC we use that wrapper div ID as the siteId). Ryan and I had a discussion on this a while back which might be worth looking over again -- here is the relevant piece: > > http://markmail.org/message/6fqyufoynv2q5q6a > > > - Jesse > > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/1632/#review4085 > ----------------------------------------------------------- > > > On 2011-12-14 21:13:40, Jesse Ciancetta wrote: >> >> ----------------------------------------------------------- >> This is an automatically generated e-mail. To reply, visit: >> https://reviews.apache.org/r/1632/ >> ----------------------------------------------------------- >> >> (Updated 2011-12-14 21:13:40) >> >> >> 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/content/samplecontainer/examples/commoncontainer/assembler.js 1214246 >> http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.gadget/gadget_site.js 1214246 >> http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.util/util.js 1214246 >> http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container/container.js 1214246 >> http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container/service.js 1214246 >> http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/container/gadget_site_test.js 1214246 >> http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/container/service_test.js 1214246 >> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandler.java 1214246 >> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerApi.java 1214246 >> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerService.java 1214246 >> >> Diff: https://reviews.apache.org/r/1632/diff >> >> >> Testing >> ------- >> >> >> Thanks, >> >> Jesse >> >> >
