You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@shindig.apache.org by Doug Davies <da...@oclc.org> on 2011/12/07 20:25:23 UTC
Review Request: The HttpRequest object is not available to
GrantRequestHandlers
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3064/
-----------------------------------------------------------
Review request for shindig.
Summary
-------
It would be nice if the GrantRequestHandler had access to the original request object so that it could use values like the security token to pass along additional params to the authorization code request. In our implementation we set values within the security token's trusted json field that we'd like to pass along to our GrantRequestHandler implementation.
This addresses bug SHINDIG-1672.
https://issues.apache.org/jira/browse/SHINDIG-1672
Diffs
-----
/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/ClientCredentialsGrantTypeHandler.java 1211517
/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/CodeGrantTypeHandler.java 1211517
/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/GrantRequestHandler.java 1211517
/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2Request.java 1211517
Diff: https://reviews.apache.org/r/3064/diff
Testing
-------
Thanks,
Doug
Re: Review Request: The HttpRequest object is not available to
GrantRequestHandlers
Posted by li xu <le...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3064/#review3850
-----------------------------------------------------------
Ship it!
It looks pretty good to me, thanks for the patch! I can help you commit.
Please clean up following minor things.
1. please fill in the test field and indicate what tests you have done.
2. please clean up the use about Maps. please use com.google.common.collect.Maps.
3. please fix up the spacing in java, if you use Eclipse IDE, you can find all the coding style templates under Shindig-project/etc/eclipse
thanks!
li
- li
On 2011-12-12 16:48:18, Doug Davies wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3064/
> -----------------------------------------------------------
>
> (Updated 2011-12-12 16:48:18)
>
>
> Review request for shindig.
>
>
> Summary
> -------
>
> It would be nice if the GrantRequestHandler had access to the original request object so that it could use values like the security token to pass along additional params to the authorization code request. In our implementation we set values within the security token's trusted json field that we'd like to pass along to our GrantRequestHandler implementation.
>
>
> This addresses bug SHINDIG-1672.
> https://issues.apache.org/jira/browse/SHINDIG-1672
>
>
> Diffs
> -----
>
> /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2Accessor.java 1212126
> /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2Request.java 1212126
> /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2RequestParameterGenerator.java PRE-CREATION
> /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2Accessor.java 1212126
> /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2Module.java 1212126
> /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2RequestParameterGenerator.java PRE-CREATION
> /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/BasicAuthenticationHandler.java 1212126
> /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/ClientCredentialsGrantTypeHandler.java 1212126
> /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/CodeGrantTypeHandler.java 1212126
> /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/StandardAuthenticationHandler.java 1212126
> /trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/DefaultRequestPipelineTest.java 1212126
>
> Diff: https://reviews.apache.org/r/3064/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Doug
>
>
Re: Review Request: The HttpRequest object is not available to
GrantRequestHandlers
Posted by Doug Davies <da...@oclc.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3064/
-----------------------------------------------------------
(Updated 2012-02-02 13:42:11.546105)
Review request for shindig.
Changes
-------
Eclipse version of the patch file.
Summary
-------
It would be nice if the GrantRequestHandler had access to the original request object so that it could use values like the security token to pass along additional params to the authorization code request. In our implementation we set values within the security token's trusted json field that we'd like to pass along to our GrantRequestHandler implementation.
This addresses bug SHINDIG-1672.
https://issues.apache.org/jira/browse/SHINDIG-1672
Diffs (updated)
-----
/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/BasicAuthenticationHandler.java 1238728
/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/ClientCredentialsGrantTypeHandler.java 1238728
/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/CodeAuthorizationResponseHandler.java 1238728
/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/StandardAuthenticationHandler.java 1238728
Diff: https://reviews.apache.org/r/3064/diff
Testing
-------
Added unit test in CodeGrantTypeHandlerTest (testGetCompleteUrl_5). It tests that the complete url sent to the authentication server has the parameter set in the accessor.
Thanks,
Doug
Re: Review Request: The HttpRequest object is not available to
GrantRequestHandlers
Posted by li xu <le...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3064/#review4752
-----------------------------------------------------------
Ship it!
Looks good. Thanks. This is the better place to flow additional authorization parameters into access token request.
Would you please send an Eclipse based patch? thanks.
- li
On 2012-02-01 14:54:12, Doug Davies wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3064/
> -----------------------------------------------------------
>
> (Updated 2012-02-01 14:54:12)
>
>
> Review request for shindig.
>
>
> Summary
> -------
>
> It would be nice if the GrantRequestHandler had access to the original request object so that it could use values like the security token to pass along additional params to the authorization code request. In our implementation we set values within the security token's trusted json field that we'd like to pass along to our GrantRequestHandler implementation.
>
>
> This addresses bug SHINDIG-1672.
> https://issues.apache.org/jira/browse/SHINDIG-1672
>
>
> Diffs
> -----
>
> /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/BasicAuthenticationHandler.java 1238728
> /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/ClientCredentialsGrantTypeHandler.java 1238728
> /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/CodeAuthorizationResponseHandler.java 1238728
> /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/StandardAuthenticationHandler.java 1238728
>
> Diff: https://reviews.apache.org/r/3064/diff
>
>
> Testing
> -------
>
> Added unit test in CodeGrantTypeHandlerTest (testGetCompleteUrl_5). It tests that the complete url sent to the authentication server has the parameter set in the accessor.
>
>
> Thanks,
>
> Doug
>
>
Re: Review Request: The HttpRequest object is not available to
GrantRequestHandlers
Posted by Doug Davies <da...@oclc.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3064/
-----------------------------------------------------------
(Updated 2012-02-01 14:54:12.796989)
Review request for shindig.
Changes
-------
Reimplemented to just focus on the auth flow (auth_code -> access_token). The previous implementation passed the parameters along fine for the auth code (browser url) but for the access code they were not passed along. Found a better plugin point for that flow.
Summary
-------
It would be nice if the GrantRequestHandler had access to the original request object so that it could use values like the security token to pass along additional params to the authorization code request. In our implementation we set values within the security token's trusted json field that we'd like to pass along to our GrantRequestHandler implementation.
This addresses bug SHINDIG-1672.
https://issues.apache.org/jira/browse/SHINDIG-1672
Diffs (updated)
-----
/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/BasicAuthenticationHandler.java 1238728
/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/ClientCredentialsGrantTypeHandler.java 1238728
/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/CodeAuthorizationResponseHandler.java 1238728
/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/StandardAuthenticationHandler.java 1238728
Diff: https://reviews.apache.org/r/3064/diff
Testing
-------
Added unit test in CodeGrantTypeHandlerTest (testGetCompleteUrl_5). It tests that the complete url sent to the authentication server has the parameter set in the accessor.
Thanks,
Doug
Re: Review Request: The HttpRequest object is not available to
GrantRequestHandlers
Posted by Paul Lindner <li...@inuus.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3064/#review4719
-----------------------------------------------------------
seems okay. I wish I had the time to use Request-Scoped guice injection instead of all this manual wiring...
- Paul
On 2011-12-12 21:22:49, Doug Davies wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3064/
> -----------------------------------------------------------
>
> (Updated 2011-12-12 21:22:49)
>
>
> Review request for shindig.
>
>
> Summary
> -------
>
> It would be nice if the GrantRequestHandler had access to the original request object so that it could use values like the security token to pass along additional params to the authorization code request. In our implementation we set values within the security token's trusted json field that we'd like to pass along to our GrantRequestHandler implementation.
>
>
> This addresses bug SHINDIG-1672.
> https://issues.apache.org/jira/browse/SHINDIG-1672
>
>
> Diffs
> -----
>
> /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2Accessor.java 1212126
> /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2Request.java 1212126
> /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2RequestParameterGenerator.java PRE-CREATION
> /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2Accessor.java 1212126
> /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2Module.java 1212126
> /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2RequestParameterGenerator.java PRE-CREATION
> /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/BasicAuthenticationHandler.java 1212126
> /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/ClientCredentialsGrantTypeHandler.java 1212126
> /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/CodeGrantTypeHandler.java 1212126
> /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/StandardAuthenticationHandler.java 1212126
> /trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/DefaultRequestPipelineTest.java 1212126
> /trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/oauth2/handler/CodeGrantTypeHandlerTest.java 1212126
>
> Diff: https://reviews.apache.org/r/3064/diff
>
>
> Testing
> -------
>
> Added unit test in CodeGrantTypeHandlerTest (testGetCompleteUrl_5). It tests that the complete url sent to the authentication server has the parameter set in the accessor.
>
>
> Thanks,
>
> Doug
>
>
Re: Review Request: The HttpRequest object is not available to
GrantRequestHandlers
Posted by Doug Davies <da...@oclc.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3064/
-----------------------------------------------------------
(Updated 2011-12-12 21:22:49.348470)
Review request for shindig.
Changes
-------
Used google Maps implementation and used Eclipse code formatters. Add junit test for testing that new parameters get passed along.
Summary
-------
It would be nice if the GrantRequestHandler had access to the original request object so that it could use values like the security token to pass along additional params to the authorization code request. In our implementation we set values within the security token's trusted json field that we'd like to pass along to our GrantRequestHandler implementation.
This addresses bug SHINDIG-1672.
https://issues.apache.org/jira/browse/SHINDIG-1672
Diffs (updated)
-----
/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2Accessor.java 1212126
/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2Request.java 1212126
/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2RequestParameterGenerator.java PRE-CREATION
/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2Accessor.java 1212126
/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2Module.java 1212126
/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2RequestParameterGenerator.java PRE-CREATION
/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/BasicAuthenticationHandler.java 1212126
/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/ClientCredentialsGrantTypeHandler.java 1212126
/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/CodeGrantTypeHandler.java 1212126
/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/StandardAuthenticationHandler.java 1212126
/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/DefaultRequestPipelineTest.java 1212126
/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/oauth2/handler/CodeGrantTypeHandlerTest.java 1212126
Diff: https://reviews.apache.org/r/3064/diff
Testing (updated)
-------
Added unit test in CodeGrantTypeHandlerTest (testGetCompleteUrl_5). It tests that the complete url sent to the authentication server has the parameter set in the accessor.
Thanks,
Doug
Re: Review Request: The HttpRequest object is not available to
GrantRequestHandlers
Posted by li xu <le...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3064/#review3849
-----------------------------------------------------------
Ship it!
It looks pretty good to me, thanks for the patch! I can help you commit.
Please clean up following minor things.
1. please fill in the test field and indicate what tests you have done.
2. please clean up the use about Maps. please use com.google.common.collect.Maps.
3. please fix up the spacing in java, if you use Eclipse IDE, you can find all the coding style templates under Shindig-project/etc/eclipse
thanks!
li
- li
On 2011-12-12 16:48:18, Doug Davies wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3064/
> -----------------------------------------------------------
>
> (Updated 2011-12-12 16:48:18)
>
>
> Review request for shindig.
>
>
> Summary
> -------
>
> It would be nice if the GrantRequestHandler had access to the original request object so that it could use values like the security token to pass along additional params to the authorization code request. In our implementation we set values within the security token's trusted json field that we'd like to pass along to our GrantRequestHandler implementation.
>
>
> This addresses bug SHINDIG-1672.
> https://issues.apache.org/jira/browse/SHINDIG-1672
>
>
> Diffs
> -----
>
> /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2Accessor.java 1212126
> /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2Request.java 1212126
> /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2RequestParameterGenerator.java PRE-CREATION
> /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2Accessor.java 1212126
> /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2Module.java 1212126
> /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2RequestParameterGenerator.java PRE-CREATION
> /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/BasicAuthenticationHandler.java 1212126
> /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/ClientCredentialsGrantTypeHandler.java 1212126
> /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/CodeGrantTypeHandler.java 1212126
> /trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/StandardAuthenticationHandler.java 1212126
> /trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/DefaultRequestPipelineTest.java 1212126
>
> Diff: https://reviews.apache.org/r/3064/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Doug
>
>
Re: Review Request: The HttpRequest object is not available to
GrantRequestHandlers
Posted by Doug Davies <da...@oclc.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3064/
-----------------------------------------------------------
(Updated 2011-12-12 16:48:18.875271)
Review request for shindig.
Changes
-------
Trying this again. My 2 new files didn't get included.
Summary
-------
It would be nice if the GrantRequestHandler had access to the original request object so that it could use values like the security token to pass along additional params to the authorization code request. In our implementation we set values within the security token's trusted json field that we'd like to pass along to our GrantRequestHandler implementation.
This addresses bug SHINDIG-1672.
https://issues.apache.org/jira/browse/SHINDIG-1672
Diffs (updated)
-----
/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2Accessor.java 1212126
/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2Request.java 1212126
/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2RequestParameterGenerator.java PRE-CREATION
/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2Accessor.java 1212126
/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2Module.java 1212126
/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2RequestParameterGenerator.java PRE-CREATION
/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/BasicAuthenticationHandler.java 1212126
/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/ClientCredentialsGrantTypeHandler.java 1212126
/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/CodeGrantTypeHandler.java 1212126
/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/StandardAuthenticationHandler.java 1212126
/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/DefaultRequestPipelineTest.java 1212126
Diff: https://reviews.apache.org/r/3064/diff
Testing
-------
Thanks,
Doug
Re: Review Request: The HttpRequest object is not available to
GrantRequestHandlers
Posted by Doug Davies <da...@oclc.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3064/
-----------------------------------------------------------
(Updated 2011-12-12 16:14:04.550741)
Review request for shindig.
Changes
-------
Reimplemented this to be more generalized. Provides a new OAuth2RequestParameterGenerator class that implementors can override to add additional parameters to authorization server requests.
Summary
-------
It would be nice if the GrantRequestHandler had access to the original request object so that it could use values like the security token to pass along additional params to the authorization code request. In our implementation we set values within the security token's trusted json field that we'd like to pass along to our GrantRequestHandler implementation.
This addresses bug SHINDIG-1672.
https://issues.apache.org/jira/browse/SHINDIG-1672
Diffs (updated)
-----
/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2Accessor.java 1212126
/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2Request.java 1212126
/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2Accessor.java 1212126
/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/OAuth2Module.java 1212126
/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/BasicAuthenticationHandler.java 1212126
/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/ClientCredentialsGrantTypeHandler.java 1212126
/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/CodeGrantTypeHandler.java 1212126
/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/handler/StandardAuthenticationHandler.java 1212126
/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/DefaultRequestPipelineTest.java 1212126
Diff: https://reviews.apache.org/r/3064/diff
Testing
-------
Thanks,
Doug