> On 2012-05-25 07:11:50, Henry Saputra wrote: > > Sorry for late reply but this patch basically change the open proxy > > mechanism to have authentication check. I believe there was a discussion in > > dev thread before about why it was intended as open proxy. > > > > I believe you can achieve this via makeRequest with OAuth2? Why would you > > need OAuth for open proxy endpoint? > > Stanton Sievers wrote: > I don't recall the exact discussion Henry, but I do recall talking about > how to access protected images, which is what this patch is solving. > > The issue with makeRequest is that you cannot take the response and stick > it in an img tag, for instance. There are limitations on putting base64 > encoded data directly into an img, mostly with IE, I think. > > This patch is only changing the behavior of the content proxy if the > extra parameters are added, otherwise, there is no impact. > > Henry Saputra wrote: > Apparently the discussion was about protecting the open proxy itself. > > I was just worry about adding default /gadgets/proxy to the auth servlet > filter that could add impression that this endpoint is auth protected. > > > Stanton Sievers wrote: > Well, it technically is auth protected just by adding the endpoint to the > filter mapping, right? The auth filter just looks for a security token. Is > that overly concerning? > > The concern I remember hearing the most was that adding a security token > to the content proxy was that you lose cacheability. That's only an issue in > this one case, otherwise, the requests look like they used to and will be > cached the same way.
The reason I ask bc adding the proxy filter to the auth filter means that it's trying to auth access to the Shindig auth servlet itself rather than the target proxied content, which I think is the intention of this patch? - Henry ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5112/#review8097 ----------------------------------------------------------- On 2012-05-22 18:20:43, Xiao Feng Yu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/5112/ > ----------------------------------------------------------- > > (Updated 2012-05-22 18:20:43) > > > Review request for shindig, Ryan Baxter, Dan Dumont, Stanton Sievers, and > Brian Lillie. > > > Summary > ------- > > Couple of changes are included in this patch. > 1) On the client side, the getProxyUrl is updated to add auth parameter to > specify the auth scheme used, also check for the AUTHORIZATION and > OAUTH_SERVICE setting and add them in proxy url. > 2) On the server side, proxy servlet will pass additional HttpServletRequest > to ProxyHandler to build the HttpRequest object, in the ProxyHandler > additional information as security token, auth type, oauth service and gadget > will be used to construct a HttpRequest to pass to the DefaultRequestPipeline > for handling. > > > This addresses bug Shindig-1773. > https://issues.apache.org/jira/browse/Shindig-1773 > > > Diffs > ----- > > http://svn.apache.org/repos/asf/shindig/trunk/config/container.js 1341572 > http://svn.apache.org/repos/asf/shindig/trunk/config/oauth2.json 1341572 > > http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/commoncontainer/gadgetCollections.json > 1340019 > > http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/oauth2/oauth2_spring_proxy.xml > PRE-CREATION > > http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/core.io/io.js > 1341572 > > http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/core.io/iotest.js > 1341572 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyServlet.java > 1341572 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/ProxyUriBase.java > 1341572 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/ProxyUriManager.java > 1341572 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/UriCommon.java > 1341572 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ProxyHandlerTest.java > 1341572 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ProxyServletTest.java > 1341572 > > http://svn.apache.org/repos/asf/shindig/trunk/java/server-resources/src/main/webapp/WEB-INF/web.xml > 1341572 > > Diff: https://reviews.apache.org/r/5112/diff > > > Testing > ------- > > > Thanks, > > Xiao Feng > >
