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