Shindig
  1. Shindig
  2. SHINDIG-1773

Content proxy needs to support proxy for OAuth protected resources

    Details

    • Type: New Feature New Feature
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.5.0, 3.0.0
    • Fix Version/s: 2.5.0-beta2
    • Component/s: Java
    • Labels:
      None

      Description

      There are many cases that some resource are protected by OAuth and they are desired to be accessed via content proxy instead of makeRequest calls, for example, images in user's album. User should be able to specify OAuth parameter in getProxyUrl call and get a proxy url to access the protected resources.

        Activity

        Hide
        jiraposter@reviews.apache.org added a comment -

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

        Review request for 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 1338171
        http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/core.io/io.js 1338232
        http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java 1338123
        http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyServlet.java 1335359
        http://svn.apache.org/repos/asf/shindig/trunk/java/server-resources/src/main/webapp/WEB-INF/web.xml 1335359

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

        Testing
        -------

        Thanks,

        Xiao Feng

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5112/ ----------------------------------------------------------- Review request for 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 1338171 http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/core.io/io.js 1338232 http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java 1338123 http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyServlet.java 1335359 http://svn.apache.org/repos/asf/shindig/trunk/java/server-resources/src/main/webapp/WEB-INF/web.xml 1335359 Diff: https://reviews.apache.org/r/5112/diff Testing ------- Thanks, Xiao Feng
        Hide
        jiraposter@reviews.apache.org added a comment -

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

        Initial pass looks OK for the most part. A sample gadget that makes use of these new APIs would be very helpful in demonstrating the concept.

        http://svn.apache.org/repos/asf/shindig/trunk/config/container.js
        <https://reviews.apache.org/r/5112/#comment17177>

        It looks like there is an extra "%" here between url and authz.

        http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/core.io/io.js
        <https://reviews.apache.org/r/5112/#comment17178>

        Use the "st" var here since you defined it above.

        http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/core.io/io.js
        <https://reviews.apache.org/r/5112/#comment17179>

        Use the "st" var here since you defined it above.

        http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java
        <https://reviews.apache.org/r/5112/#comment17173>

        It appears that request could be null here.

        http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java
        <https://reviews.apache.org/r/5112/#comment17174>

        small nit on trailing whitespace

        http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java
        <https://reviews.apache.org/r/5112/#comment17175>

        small nit on whitespace

        http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java
        <https://reviews.apache.org/r/5112/#comment17176>

        The formatting seems weird here. Can you make sure you are using 2 spaces instead of tabs and 4 spaces for line continuations per the Shindig style guidelines?

        • Stanton

        On 2012-05-14 15:53:08, Xiao Feng Yu wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/5112/

        -----------------------------------------------------------

        (Updated 2012-05-14 15:53:08)

        Review request for 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 1338171

        http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/core.io/io.js 1338232

        http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java 1338123

        http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyServlet.java 1335359

        http://svn.apache.org/repos/asf/shindig/trunk/java/server-resources/src/main/webapp/WEB-INF/web.xml 1335359

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

        Testing

        -------

        Thanks,

        Xiao Feng

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5112/#review7844 ----------------------------------------------------------- Initial pass looks OK for the most part. A sample gadget that makes use of these new APIs would be very helpful in demonstrating the concept. http://svn.apache.org/repos/asf/shindig/trunk/config/container.js < https://reviews.apache.org/r/5112/#comment17177 > It looks like there is an extra "%" here between url and authz. http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/core.io/io.js < https://reviews.apache.org/r/5112/#comment17178 > Use the "st" var here since you defined it above. http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/core.io/io.js < https://reviews.apache.org/r/5112/#comment17179 > Use the "st" var here since you defined it above. http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java < https://reviews.apache.org/r/5112/#comment17173 > It appears that request could be null here. http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java < https://reviews.apache.org/r/5112/#comment17174 > small nit on trailing whitespace http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java < https://reviews.apache.org/r/5112/#comment17175 > small nit on whitespace http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java < https://reviews.apache.org/r/5112/#comment17176 > The formatting seems weird here. Can you make sure you are using 2 spaces instead of tabs and 4 spaces for line continuations per the Shindig style guidelines? Stanton On 2012-05-14 15:53:08, Xiao Feng Yu wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5112/ ----------------------------------------------------------- (Updated 2012-05-14 15:53:08) Review request for 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 1338171 http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/core.io/io.js 1338232 http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java 1338123 http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyServlet.java 1335359 http://svn.apache.org/repos/asf/shindig/trunk/java/server-resources/src/main/webapp/WEB-INF/web.xml 1335359 Diff: https://reviews.apache.org/r/5112/diff Testing ------- Thanks, Xiao Feng
        Hide
        jiraposter@reviews.apache.org added a comment -

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

        Please fix these tests before we do a more in depth review.

        testDoGetNormal(org.apache.shindig.gadgets.servlet.ProxyServletTest)
        testDoGetHttpError(org.apache.shindig.gadgets.servlet.ProxyServletTest)
        testDoGetException(org.apache.shindig.gadgets.servlet.ProxyServletTest)
        testDoPostNormal(org.apache.shindig.gadgets.servlet.ProxyServletTest)
        testDoPostHttpError(org.apache.shindig.gadgets.servlet.ProxyServletTest)
        testDoPostException(org.apache.shindig.gadgets.servlet.ProxyServletTest)
        testNonWhitelistedGadget(org.apache.shindig.gadgets.servlet.ProxyHandlerTest)
        testInvalidHeaderDropped(org.apache.shindig.gadgets.servlet.ProxyHandlerTest)
        testLockedDomainEmbed(org.apache.shindig.gadgets.servlet.ProxyHandlerTest)
        testHttpRequestFillsParentAndContainer(org.apache.shindig.gadgets.servlet.ProxyHandlerTest)
        testHeadersPreserved(org.apache.shindig.gadgets.servlet.ProxyHandlerTest)
        testOctetSetOnNullContentType(org.apache.shindig.gadgets.servlet.ProxyHandlerTest)
        testNoContentDispositionForFlash(org.apache.shindig.gadgets.servlet.ProxyHandlerTest)
        testNoContentDispositionForFlashUtf8(org.apache.shindig.gadgets.servlet.ProxyHandlerTest)
        testGetFallback(org.apache.shindig.gadgets.servlet.ProxyHandlerTest)
        testNoCache(org.apache.shindig.gadgets.servlet.ProxyHandlerTest)
        testRecoverableRewritingException(org.apache.shindig.gadgets.servlet.ProxyHandlerTest)
        testThrowExceptionIfReturnOriginalContentOnErrorNotSet(org.apache.shindig.gadgets.servlet.ProxyHandlerTest)
        testWithCache(org.apache.shindig.gadgets.servlet.ProxyHandlerTest)
        testWithBadTtl(org.apache.shindig.gadgets.servlet.ProxyHandlerTest)
        testMimeMatchPass(org.apache.shindig.gadgets.servlet.ProxyHandlerTest)
        testMimeMatchPassWithAdditionalAttributes(org.apache.shindig.gadgets.servlet.ProxyHandlerTest)
        testMimeMatchOverrideNonMatch(org.apache.shindig.gadgets.servlet.ProxyHandlerTest)
        testMimeMatchVarySupport(org.apache.shindig.gadgets.servlet.ProxyHandlerTest)

        http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/core.io/io.js
        <https://reviews.apache.org/r/5112/#comment17199>

        There is no authorization type of SSO in shingid

        • Ryan

        On 2012-05-14 15:53:08, Xiao Feng Yu wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/5112/

        -----------------------------------------------------------

        (Updated 2012-05-14 15:53:08)

        Review request for 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 1338171

        http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/core.io/io.js 1338232

        http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java 1338123

        http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyServlet.java 1335359

        http://svn.apache.org/repos/asf/shindig/trunk/java/server-resources/src/main/webapp/WEB-INF/web.xml 1335359

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

        Testing

        -------

        Thanks,

        Xiao Feng

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5112/#review7870 ----------------------------------------------------------- Please fix these tests before we do a more in depth review. testDoGetNormal(org.apache.shindig.gadgets.servlet.ProxyServletTest) testDoGetHttpError(org.apache.shindig.gadgets.servlet.ProxyServletTest) testDoGetException(org.apache.shindig.gadgets.servlet.ProxyServletTest) testDoPostNormal(org.apache.shindig.gadgets.servlet.ProxyServletTest) testDoPostHttpError(org.apache.shindig.gadgets.servlet.ProxyServletTest) testDoPostException(org.apache.shindig.gadgets.servlet.ProxyServletTest) testNonWhitelistedGadget(org.apache.shindig.gadgets.servlet.ProxyHandlerTest) testInvalidHeaderDropped(org.apache.shindig.gadgets.servlet.ProxyHandlerTest) testLockedDomainEmbed(org.apache.shindig.gadgets.servlet.ProxyHandlerTest) testHttpRequestFillsParentAndContainer(org.apache.shindig.gadgets.servlet.ProxyHandlerTest) testHeadersPreserved(org.apache.shindig.gadgets.servlet.ProxyHandlerTest) testOctetSetOnNullContentType(org.apache.shindig.gadgets.servlet.ProxyHandlerTest) testNoContentDispositionForFlash(org.apache.shindig.gadgets.servlet.ProxyHandlerTest) testNoContentDispositionForFlashUtf8(org.apache.shindig.gadgets.servlet.ProxyHandlerTest) testGetFallback(org.apache.shindig.gadgets.servlet.ProxyHandlerTest) testNoCache(org.apache.shindig.gadgets.servlet.ProxyHandlerTest) testRecoverableRewritingException(org.apache.shindig.gadgets.servlet.ProxyHandlerTest) testThrowExceptionIfReturnOriginalContentOnErrorNotSet(org.apache.shindig.gadgets.servlet.ProxyHandlerTest) testWithCache(org.apache.shindig.gadgets.servlet.ProxyHandlerTest) testWithBadTtl(org.apache.shindig.gadgets.servlet.ProxyHandlerTest) testMimeMatchPass(org.apache.shindig.gadgets.servlet.ProxyHandlerTest) testMimeMatchPassWithAdditionalAttributes(org.apache.shindig.gadgets.servlet.ProxyHandlerTest) testMimeMatchOverrideNonMatch(org.apache.shindig.gadgets.servlet.ProxyHandlerTest) testMimeMatchVarySupport(org.apache.shindig.gadgets.servlet.ProxyHandlerTest) http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/core.io/io.js < https://reviews.apache.org/r/5112/#comment17199 > There is no authorization type of SSO in shingid Ryan On 2012-05-14 15:53:08, Xiao Feng Yu wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5112/ ----------------------------------------------------------- (Updated 2012-05-14 15:53:08) Review request for 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 1338171 http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/core.io/io.js 1338232 http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java 1338123 http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyServlet.java 1335359 http://svn.apache.org/repos/asf/shindig/trunk/java/server-resources/src/main/webapp/WEB-INF/web.xml 1335359 Diff: https://reviews.apache.org/r/5112/diff Testing ------- Thanks, Xiao Feng
        Hide
        jiraposter@reviews.apache.org added a comment -

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

        (Updated 2012-05-15 11:41:23.700650)

        Review request for Ryan Baxter, Dan Dumont, Stanton Sievers, and Brian Lillie.

        Changes
        -------

        Fixed the unit test cases in Java and added new test case in JavaScript. Handling of null HttpServletRequest in ProxyHander. Some format work.

        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 (updated)


        http://svn.apache.org/repos/asf/shindig/trunk/config/container.js 1338565
        http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/core.io/io.js 1338565
        http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/core.io/iotest.js 1338595
        http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java 1338565
        http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyServlet.java 1338565
        http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ProxyServletTest.java 1338565
        http://svn.apache.org/repos/asf/shindig/trunk/java/server-resources/src/main/webapp/WEB-INF/web.xml 1338565

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

        Testing
        -------

        Thanks,

        Xiao Feng

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5112/ ----------------------------------------------------------- (Updated 2012-05-15 11:41:23.700650) Review request for Ryan Baxter, Dan Dumont, Stanton Sievers, and Brian Lillie. Changes ------- Fixed the unit test cases in Java and added new test case in JavaScript. Handling of null HttpServletRequest in ProxyHander. Some format work. 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 (updated) http://svn.apache.org/repos/asf/shindig/trunk/config/container.js 1338565 http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/core.io/io.js 1338565 http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/core.io/iotest.js 1338595 http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java 1338565 http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyServlet.java 1338565 http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ProxyServletTest.java 1338565 http://svn.apache.org/repos/asf/shindig/trunk/java/server-resources/src/main/webapp/WEB-INF/web.xml 1338565 Diff: https://reviews.apache.org/r/5112/diff Testing ------- Thanks, Xiao Feng
        Hide
        jiraposter@reviews.apache.org added a comment -

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

        (Updated 2012-05-15 12:04:39.044381)

        Review request for Ryan Baxter, Dan Dumont, Stanton Sievers, and Brian Lillie.

        Changes
        -------

        remove heading whitespaces

        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 (updated)


        http://svn.apache.org/repos/asf/shindig/trunk/config/container.js 1338565
        http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/core.io/io.js 1338565
        http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/core.io/iotest.js 1338595
        http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java 1338565
        http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyServlet.java 1338565
        http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ProxyServletTest.java 1338565
        http://svn.apache.org/repos/asf/shindig/trunk/java/server-resources/src/main/webapp/WEB-INF/web.xml 1338565

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

        Testing
        -------

        Thanks,

        Xiao Feng

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5112/ ----------------------------------------------------------- (Updated 2012-05-15 12:04:39.044381) Review request for Ryan Baxter, Dan Dumont, Stanton Sievers, and Brian Lillie. Changes ------- remove heading whitespaces 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 (updated) http://svn.apache.org/repos/asf/shindig/trunk/config/container.js 1338565 http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/core.io/io.js 1338565 http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/core.io/iotest.js 1338595 http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java 1338565 http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyServlet.java 1338565 http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ProxyServletTest.java 1338565 http://svn.apache.org/repos/asf/shindig/trunk/java/server-resources/src/main/webapp/WEB-INF/web.xml 1338565 Diff: https://reviews.apache.org/r/5112/diff Testing ------- Thanks, Xiao Feng
        Hide
        jiraposter@reviews.apache.org added a comment -

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

        Since this functionality is not in the spec, would it be possible to create a sample gadget as an example of how to use this functionality?

        • Ryan

        On 2012-05-15 12:04:39, Xiao Feng Yu wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/5112/

        -----------------------------------------------------------

        (Updated 2012-05-15 12:04:39)

        Review request for 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 1338565

        http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/core.io/io.js 1338565

        http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/core.io/iotest.js 1338595

        http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java 1338565

        http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyServlet.java 1338565

        http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ProxyServletTest.java 1338565

        http://svn.apache.org/repos/asf/shindig/trunk/java/server-resources/src/main/webapp/WEB-INF/web.xml 1338565

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

        Testing

        -------

        Thanks,

        Xiao Feng

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5112/#review7913 ----------------------------------------------------------- Since this functionality is not in the spec, would it be possible to create a sample gadget as an example of how to use this functionality? Ryan On 2012-05-15 12:04:39, Xiao Feng Yu wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5112/ ----------------------------------------------------------- (Updated 2012-05-15 12:04:39) Review request for 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 1338565 http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/core.io/io.js 1338565 http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/core.io/iotest.js 1338595 http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java 1338565 http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyServlet.java 1338565 http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ProxyServletTest.java 1338565 http://svn.apache.org/repos/asf/shindig/trunk/java/server-resources/src/main/webapp/WEB-INF/web.xml 1338565 Diff: https://reviews.apache.org/r/5112/diff Testing ------- Thanks, Xiao Feng
        Hide
        Rich Thompson added a comment -

        I agree that a sample gadget using this functionality would be helpful, but I think the lead in statement for why is too strong. My read of section 12 of the gadget spec (e.g. http://opensocial-resources.googlecode.com/svn/spec/2.0.1/Core-Gadget.xml#OAuth) requires more OAuth support than the rest of the specs explicitly call out. Specifically:

        "As mentioned previously in this specification and other OpenSocial 2.0 specifications, OpenSocial containers MUST be able to proxy OAuth requests for gadgets. The main mechanism used to accomplish proxy requests is the gadgets.io.makeRequest (Section 13.2.1.3) method."

        While makeRequest is expected to be the main mechanism (and certainly is the focus throughout the specs), but the MUST statement relative to support is much broader and certainly applies to the use case driving this JIRA; namely, supporting accessing protected resources through the content proxy.

        Show
        Rich Thompson added a comment - I agree that a sample gadget using this functionality would be helpful, but I think the lead in statement for why is too strong. My read of section 12 of the gadget spec (e.g. http://opensocial-resources.googlecode.com/svn/spec/2.0.1/Core-Gadget.xml#OAuth ) requires more OAuth support than the rest of the specs explicitly call out. Specifically: "As mentioned previously in this specification and other OpenSocial 2.0 specifications, OpenSocial containers MUST be able to proxy OAuth requests for gadgets. The main mechanism used to accomplish proxy requests is the gadgets.io.makeRequest (Section 13.2.1.3) method." While makeRequest is expected to be the main mechanism (and certainly is the focus throughout the specs), but the MUST statement relative to support is much broader and certainly applies to the use case driving this JIRA; namely, supporting accessing protected resources through the content proxy.
        Hide
        jiraposter@reviews.apache.org added a comment -

        On 2012-05-15 21:46:05, Ryan Baxter wrote:

        > Since this functionality is not in the spec, would it be possible to create a sample gadget as an example of how to use this functionality?

        I would also like to see a test gadget accompany this change.

        • Dan

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

        On 2012-05-15 12:04:39, Xiao Feng Yu wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/5112/

        -----------------------------------------------------------

        (Updated 2012-05-15 12:04:39)

        Review request for 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 1338565

        http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/core.io/io.js 1338565

        http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/core.io/iotest.js 1338595

        http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java 1338565

        http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyServlet.java 1338565

        http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ProxyServletTest.java 1338565

        http://svn.apache.org/repos/asf/shindig/trunk/java/server-resources/src/main/webapp/WEB-INF/web.xml 1338565

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

        Testing

        -------

        Thanks,

        Xiao Feng

        Show
        jiraposter@reviews.apache.org added a comment - On 2012-05-15 21:46:05, Ryan Baxter wrote: > Since this functionality is not in the spec, would it be possible to create a sample gadget as an example of how to use this functionality? I would also like to see a test gadget accompany this change. Dan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5112/#review7913 ----------------------------------------------------------- On 2012-05-15 12:04:39, Xiao Feng Yu wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5112/ ----------------------------------------------------------- (Updated 2012-05-15 12:04:39) Review request for 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 1338565 http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/core.io/io.js 1338565 http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/core.io/iotest.js 1338595 http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java 1338565 http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyServlet.java 1338565 http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ProxyServletTest.java 1338565 http://svn.apache.org/repos/asf/shindig/trunk/java/server-resources/src/main/webapp/WEB-INF/web.xml 1338565 Diff: https://reviews.apache.org/r/5112/diff Testing ------- Thanks, Xiao Feng
        Hide
        jiraposter@reviews.apache.org added a comment -

        On 2012-05-15 21:46:05, Ryan Baxter wrote:

        > Since this functionality is not in the spec, would it be possible to create a sample gadget as an example of how to use this functionality?

        Dan Dumont wrote:

        I would also like to see a test gadget accompany this change.

        Hi reviewers, it may take some time to build the sample gadget, I've tried Picasa API, but the access to the photos with the link in ATOM feed doesn't need an access token, if you have any idea on the sample gadget, please let me know.
        The unit tests and format are all clean in the latest submission, maybe we can start review on the code and defer the sample gadget as the use of the new function is also in JS unit test case.

        • Xiao Feng

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

        On 2012-05-15 12:04:39, Xiao Feng Yu wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/5112/

        -----------------------------------------------------------

        (Updated 2012-05-15 12:04:39)

        Review request for 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 1338565

        http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/core.io/io.js 1338565

        http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/core.io/iotest.js 1338595

        http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java 1338565

        http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyServlet.java 1338565

        http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ProxyServletTest.java 1338565

        http://svn.apache.org/repos/asf/shindig/trunk/java/server-resources/src/main/webapp/WEB-INF/web.xml 1338565

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

        Testing

        -------

        Thanks,

        Xiao Feng

        Show
        jiraposter@reviews.apache.org added a comment - On 2012-05-15 21:46:05, Ryan Baxter wrote: > Since this functionality is not in the spec, would it be possible to create a sample gadget as an example of how to use this functionality? Dan Dumont wrote: I would also like to see a test gadget accompany this change. Hi reviewers, it may take some time to build the sample gadget, I've tried Picasa API, but the access to the photos with the link in ATOM feed doesn't need an access token, if you have any idea on the sample gadget, please let me know. The unit tests and format are all clean in the latest submission, maybe we can start review on the code and defer the sample gadget as the use of the new function is also in JS unit test case. Xiao Feng ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5112/#review7913 ----------------------------------------------------------- On 2012-05-15 12:04:39, Xiao Feng Yu wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5112/ ----------------------------------------------------------- (Updated 2012-05-15 12:04:39) Review request for 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 1338565 http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/core.io/io.js 1338565 http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/core.io/iotest.js 1338595 http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java 1338565 http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyServlet.java 1338565 http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ProxyServletTest.java 1338565 http://svn.apache.org/repos/asf/shindig/trunk/java/server-resources/src/main/webapp/WEB-INF/web.xml 1338565 Diff: https://reviews.apache.org/r/5112/diff Testing ------- Thanks, Xiao Feng
        Hide
        jiraposter@reviews.apache.org added a comment -

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

        Instead of passing the request along into ProxyHandler, I think the normal flow is to ensure the information you need is in the ProxyUri that gets passed along. For instance, org.apache.shindig.gadgets.uri.ProxyUriBase.setFromUri(Uri) sets things like rewrite MIME, the gadget, and the container for the proxy uri today. The intent would be that ProxyServlet.processRequest() creates the ProxyUri via the ProxyUriManager and then that Uri would be used in ProxyHandler to do the type of checking you want, instead of having to pass the request along.

        Also see org.apache.shindig.gadgets.uri.UriCommon.Param for the other params that are defined for Uris.

        • Stanton

        On 2012-05-15 12:04:39, Xiao Feng Yu wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/5112/

        -----------------------------------------------------------

        (Updated 2012-05-15 12:04:39)

        Review request for 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 1338565

        http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/core.io/io.js 1338565

        http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/core.io/iotest.js 1338595

        http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java 1338565

        http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyServlet.java 1338565

        http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ProxyServletTest.java 1338565

        http://svn.apache.org/repos/asf/shindig/trunk/java/server-resources/src/main/webapp/WEB-INF/web.xml 1338565

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

        Testing

        -------

        Thanks,

        Xiao Feng

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5112/#review7956 ----------------------------------------------------------- Instead of passing the request along into ProxyHandler, I think the normal flow is to ensure the information you need is in the ProxyUri that gets passed along. For instance, org.apache.shindig.gadgets.uri.ProxyUriBase.setFromUri(Uri) sets things like rewrite MIME, the gadget, and the container for the proxy uri today. The intent would be that ProxyServlet.processRequest() creates the ProxyUri via the ProxyUriManager and then that Uri would be used in ProxyHandler to do the type of checking you want, instead of having to pass the request along. Also see org.apache.shindig.gadgets.uri.UriCommon.Param for the other params that are defined for Uris. Stanton On 2012-05-15 12:04:39, Xiao Feng Yu wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5112/ ----------------------------------------------------------- (Updated 2012-05-15 12:04:39) Review request for 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 1338565 http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/core.io/io.js 1338565 http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/core.io/iotest.js 1338595 http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java 1338565 http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyServlet.java 1338565 http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ProxyServletTest.java 1338565 http://svn.apache.org/repos/asf/shindig/trunk/java/server-resources/src/main/webapp/WEB-INF/web.xml 1338565 Diff: https://reviews.apache.org/r/5112/diff Testing ------- Thanks, Xiao Feng
        Hide
        jiraposter@reviews.apache.org added a comment -

        On 2012-05-15 21:46:05, Ryan Baxter wrote:

        > Since this functionality is not in the spec, would it be possible to create a sample gadget as an example of how to use this functionality?

        Dan Dumont wrote:

        I would also like to see a test gadget accompany this change.

        Xiao Feng Yu wrote:

        Hi reviewers, it may take some time to build the sample gadget, I've tried Picasa API, but the access to the photos with the link in ATOM feed doesn't need an access token, if you have any idea on the sample gadget, please let me know.

        The unit tests and format are all clean in the latest submission, maybe we can start review on the code and defer the sample gadget as the use of the new function is also in JS unit test case.

        I think an example is critical and usually required for these kind of changes. I took a brief look as well and could not find a service that had protected images (not that it has to be just images). One idea I had was to use the the OAuth provider in Shindig. Host a set of resources in a directory on the server configure Shindig / the provider in Shindig so that they are protected and have a sample gadget that fetches them.

        • Ryan

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

        On 2012-05-15 12:04:39, Xiao Feng Yu wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/5112/

        -----------------------------------------------------------

        (Updated 2012-05-15 12:04:39)

        Review request for 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 1338565

        http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/core.io/io.js 1338565

        http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/core.io/iotest.js 1338595

        http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java 1338565

        http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyServlet.java 1338565

        http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ProxyServletTest.java 1338565

        http://svn.apache.org/repos/asf/shindig/trunk/java/server-resources/src/main/webapp/WEB-INF/web.xml 1338565

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

        Testing

        -------

        Thanks,

        Xiao Feng

        Show
        jiraposter@reviews.apache.org added a comment - On 2012-05-15 21:46:05, Ryan Baxter wrote: > Since this functionality is not in the spec, would it be possible to create a sample gadget as an example of how to use this functionality? Dan Dumont wrote: I would also like to see a test gadget accompany this change. Xiao Feng Yu wrote: Hi reviewers, it may take some time to build the sample gadget, I've tried Picasa API, but the access to the photos with the link in ATOM feed doesn't need an access token, if you have any idea on the sample gadget, please let me know. The unit tests and format are all clean in the latest submission, maybe we can start review on the code and defer the sample gadget as the use of the new function is also in JS unit test case. I think an example is critical and usually required for these kind of changes. I took a brief look as well and could not find a service that had protected images (not that it has to be just images). One idea I had was to use the the OAuth provider in Shindig. Host a set of resources in a directory on the server configure Shindig / the provider in Shindig so that they are protected and have a sample gadget that fetches them. Ryan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5112/#review7913 ----------------------------------------------------------- On 2012-05-15 12:04:39, Xiao Feng Yu wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5112/ ----------------------------------------------------------- (Updated 2012-05-15 12:04:39) Review request for 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 1338565 http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/core.io/io.js 1338565 http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/core.io/iotest.js 1338595 http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java 1338565 http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyServlet.java 1338565 http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ProxyServletTest.java 1338565 http://svn.apache.org/repos/asf/shindig/trunk/java/server-resources/src/main/webapp/WEB-INF/web.xml 1338565 Diff: https://reviews.apache.org/r/5112/diff Testing ------- Thanks, Xiao Feng
        Hide
        Stanton Sievers added a comment -

        Resolved: Committed revision 1342085

        Show
        Stanton Sievers added a comment - Resolved: Committed revision 1342085

          People

          • Assignee:
            Unassigned
            Reporter:
            Xiao Feng Yu
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Time Tracking

              Estimated:
              Original Estimate - 48h
              48h
              Remaining:
              Remaining Estimate - 48h
              48h
              Logged:
              Time Spent - Not Specified
              Not Specified

                Development