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.

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

-Ryan

Email: [email protected]
Phone: 978-899-3041
developerWorks Profile



From:   "Ciancetta, Jesse E." <[email protected]>
To:     "[email protected]" <[email protected]>, 
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:[email protected]]
>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(HttpReque
>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/ex
>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/ex
>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
>>
>>




Reply via email to