You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@shindig.apache.org by Jesse Ciancetta <jc...@mitre.org> on 2011/12/14 22:13:40 UTC
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-12-14 21:13:40.309607)
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 think the sticking point we had on this last time was a concern that we might not want to be using siteId for the moduleId -- but I'm thinking maybe we could start there and then update later to introduce a specific moduleId property somewhere if we find that it does actually need to be different.
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/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
Re: Review Request: Common container currently doesnt include the siteId
(moduleId) in any of it's security token processing/handling
Posted by Jesse Ciancetta <jc...@mitre.org>.
> On 2011-12-20 22:27:52, Henry Saputra wrote:
> > http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/commoncontainer/assembler.js, line 28
> > <https://reviews.apache.org/r/1632/diff/4/?file=64700#file64700line28>
> >
> > Do you need to take out this line for the common container test example?
Yup -- I've got a comment above it saying it is only there to facilitate testing and shouldn't be included in a final patch.
This patch and review are really more just to have people look at and validate the approach before I spend more time finishing up the final bits -- this definitely isn't ready to commit as is (although it should be close enough to trunk to apply to a local deployment and play around with if desired).
> On 2011-12-20 22:27:52, Henry Saputra wrote:
> > http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/container/gadget_site_test.js, line 76
> > <https://reviews.apache.org/r/1632/diff/4/?file=64705#file64705line76>
> >
> > I dont think we need to add 's' to the end of metadata?
I think I'd added an 's' to the getGadgetToken function (since it returns multiple tokens) so I also added the 's' to getGadgetMetadata too just to keep it consistent (since it returns multiple metadata objects) -- it's not a big deal to me though and would be happy to roll those changes back.
> On 2011-12-20 22:27:52, Henry Saputra wrote:
> > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandler.java, line 573
> > <https://reviews.apache.org/r/1632/diff/4/?file=64707#file64707line573>
> >
> > Where does the code change the common container that adds module id at the end of the gadget url?
The osapi.container.Service.prototype.getGadgetTokens (line 215 of service.js in my patch) takes a request object as one of its parameters which it sends to shindig with the token request -- that request object needs to have the gadgets that we're requesting metadata for specified in the gadgetUrl:moduleId format. You can look at the new osapi.container.Service.prototype.getGadgetToken function (line 313 of service.js in my patch) to see an example of calling getGadgetTokens with the properly formatted request object.
Also -- looking back over this code now reminds me why I renamed the existing getGadgetToken function (which returned multiple tokens) to getGadgetTokens -- it was because I wanted to add a function that actually returned a single gadget token (to be used to fetch a single token during gadget rendering if needed). And since I renamed the getGadgetToken function to getGadgetTokens I also renamed the getGadgetMetadata to getGadgetMetadatas for consistency.
- Jesse
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1632/#review4021
-----------------------------------------------------------
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
>
>
Re: Review Request: Common container currently doesnt include the siteId
(moduleId) in any of it's security token processing/handling
Posted by Henry Saputra <hs...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1632/#review4021
-----------------------------------------------------------
http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/commoncontainer/assembler.js
<https://reviews.apache.org/r/1632/#comment9119>
Do you need to take out this line for the common container test example?
http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/container/gadget_site_test.js
<https://reviews.apache.org/r/1632/#comment9120>
I dont think we need to add 's' to the end of metadata?
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandler.java
<https://reviews.apache.org/r/1632/#comment9127>
Where does the code change the common container that adds module id at the end of the gadget url?
- Henry
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
>
>
Re: Review Request: Common container currently doesnt include the siteId
(moduleId) in any of it's security token processing/handling
Posted by Dan Dumont <dd...@us.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1632/#review4075
-----------------------------------------------------------
http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container/service.js
<https://reviews.apache.org/r/1632/#comment9175>
Per my previous comment, we could change this to function(site) {}
http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container/service.js
<https://reviews.apache.org/r/1632/#comment9176>
This too: function(site){}
http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container/service.js
<https://reviews.apache.org/r/1632/#comment9177>
function(site, callback){}.. though, my changes after the container token refresh might make this unnecessary... not sure yet.
- Dan
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
>
>
Re: Review Request: Common container currently doesnt include the siteId
(moduleId) in any of it's security token processing/handling
Posted by Dan Dumont <dd...@us.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1632/#review4074
-----------------------------------------------------------
http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.gadget/gadget_site.js
<https://reviews.apache.org/r/1632/#comment9173>
Hmm. Ok our conversation at panera is staring to come back to me.
My understanding of the code is that moduleid != siteid
I could be wrong here, but everything in the code down to the expected data types for each within the token tell me they are different. I'm not sure who the original author of this stuff is, so we may never know for sure.
That said. I do think that siteid is a more... container related bit of information, and moduleid is more of a backend-data bit of information.
It would make sense to me that a site's id is the site id, but that a site *could* be associated with a module id (if persisted) but it's not necessary.
I don't think I would want to enforce that the transport for a module id is in the id of the site.
Since I do have plans to realign GadgetSite and UrlSite down the road. Perhaps we can tweak the constructor of a site or something to make this more straight-forward.
At the very least, I would suggest that we not pass in the id to the siteIdExtractor() but instead just pass the currentGadgetEl element.
Perhaps also rename this function to moduleIdExtractor. That way when creating the site, a container could append a data-module-id attribute to the element for specific use as the moduleid
- Dan
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
>
>
Re: Review Request: Common container currently doesnt include the siteId (moduleId) in
any of it's security token processing/handling
Posted by Dan Dumont <dd...@us.ibm.com>.
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" <da...@oclc.org>
To: <de...@shindig.apache.org>,
Cc: "shindig" <de...@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
>>
>>
>
Re: Review Request: Common container currently doesnt include the siteId (moduleId) in any of it's security token processing/handling
Posted by "Davies,Douglas" <da...@oclc.org>.
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
>>
>>
>
Re: Review Request: Common container currently doesnt include the siteId
(moduleId) in any of it's security token processing/handling
Posted by Jesse Ciancetta <jc...@mitre.org>.
> 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
>
>
Re: Review Request: Common container currently doesnt include the siteId
(moduleId) in any of it's security token processing/handling
Posted by Dan Dumont <dd...@us.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1632/#review4085
-----------------------------------------------------------
http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.gadget/gadget_site.js
<https://reviews.apache.org/r/1632/#comment9184>
Do you know why this is done in the loadingGadgetHolder?
- Dan
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
>
>
Re: Review Request: Common container currently doesnt include the siteId
(moduleId) in any of it's security token processing/handling
Posted by Jesse Ciancetta <jc...@mitre.org>.
> On 2011-12-19 20:23:10, Henry Saputra wrote:
> > Jesse, I think I miss the part where the "siteIdExtractor" is being set? Is it need to be done by common container client?
Yes -- it can be set by the client as a property of the configuration object passed to the common container constructor. There is a sample of that in the patch -- it should be in the assembler.js file.
- Jesse
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1632/#review3981
-----------------------------------------------------------
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
>
>
Re: Review Request: Common container currently doesnt include the siteId
(moduleId) in any of it's security token processing/handling
Posted by Henry Saputra <hs...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1632/#review3981
-----------------------------------------------------------
Jesse, I think I miss the part where the "siteIdExtractor" is being set? Is it need to be done by common container client?
- Henry
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
>
>