Jesse, I know Stanton has been looking at some of this code lately and might be a good person to take a look at your suggested changes. He is out on vaca until next Tuesday but I can see if he can take a look when he gets back.
-Ryan Email: rjbax...@us.ibm.com Phone: 978-899-3041 developerWorks Profile From: "Jesse Ciancetta" <jc...@mitre.org> To: "shindig" <dev@shindig.apache.org>, "Jesse Ciancetta" <jc...@mitre.org>, Date: 09/28/2011 03:39 PM Subject: Re: Review Request: Common container currently doesnt include the siteId (moduleId) in any of it's security token processing/handling ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1632/ ----------------------------------------------------------- (Updated 2011-09-28 19:36:42.867006) Review request for shindig. Changes ------- I've attached an updated patch which has been realigned with the current shindig trunk. More details can be found in the review description and previous comments. I would very much appreciate some feedback on this patch. Thanks! 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 (updated) ----- 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/examples/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/features/container/gadget_site_test.js 1176946 http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/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
