You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@shindig.apache.org by Dan Dumont <dd...@us.ibm.com> on 2013/01/24 15:50:41 UTC

Review Request: Shared token logic confuses shared oauth identity with gadget uri in cache and persistence layers.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9099/
-----------------------------------------------------------

Review request for shindig.


Description
-------

After debugging the problem for a while it has become clear that when dealing with a shared token, any interaction with the cache or the persistence layers should always have the shared id (clientid:servicename) instead of the gadgeturi.

The ideal solution would be to move the logic to handle this into the cache and persistence layers, unfortunately that would require a fair bit of interface refactoring and I don't want to break anyone at the moment.  This patch should guard against the issues that I've found, namely, that the gadgeturi was being confused with the sharedtokenid in logic that needed the gadgeturi.

The sync blocks in the patch <i>should</i> protect against concurrency in the current implementation, however it's not the best fix.  I think the only good fix would be to refactor the cache and persistence layers to generate the appropriate cache keys themselves and not overload the gadgetUri field in the token.

We can use this review to discuss the options we have here.


Diffs
-----

  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2Store.java 1437127 

Diff: https://reviews.apache.org/r/9099/diff/


Testing
-------


Thanks,

Dan Dumont


Re: Review Request: Shared token logic confuses shared oauth identity with gadget uri in cache and persistence layers.

Posted by Dan Dumont <dd...@us.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9099/
-----------------------------------------------------------

(Updated Jan. 24, 2013, 8:56 p.m.)


Review request for shindig.


Changes
-------

Wrap the handling of an "existing token" in the same gadgeturi swap out trick.


Description
-------

After debugging the problem for a while it has become clear that when dealing with a shared token, any interaction with the cache or the persistence layers should always have the shared id (clientid:servicename) instead of the gadgeturi.

The ideal solution would be to move the logic to handle this into the cache and persistence layers, unfortunately that would require a fair bit of interface refactoring and I don't want to break anyone at the moment.  This patch should guard against the issues that I've found, namely, that the gadgeturi was being confused with the sharedtokenid in logic that needed the gadgeturi.

The sync blocks in the patch <i>should</i> protect against concurrency in the current implementation, however it's not the best fix.  I think the only good fix would be to refactor the cache and persistence layers to generate the appropriate cache keys themselves and not overload the gadgetUri field in the token.

We can use this review to discuss the options we have here.


Diffs (updated)
-----

  http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2Store.java 1437127 

Diff: https://reviews.apache.org/r/9099/diff/


Testing
-------


Thanks,

Dan Dumont