You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@shindig.apache.org by Stanton Sievers <si...@gmail.com> on 2012/02/21 22:40:32 UTC
Review Request: OAuth2 access tokens being removed from OAuth2Store when
request returns any 4xx response
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3987/
-----------------------------------------------------------
Review request for shindig, li xu and Adam Clarke.
Summary
-------
>From JIRA:
If the url to which a gadget is doing a makeRequest doesn't exist, i.e., returns a 404 to the Shindig server, the access token is being removed from the OAuth2 Store. This functionality is implemented here: org.apache.shindig.gadgets.oauth2.BasicOAuth2Request.fetchFromServer(OAuth2Accessor, HttpRequest)
fetchFromServer is checking only if the response code is 4xx, and if so, it is removing the access token from the store. This seems right for 401 or 403 return codes, perhaps, but not for 404. The behavior for an end user would then be that they have to do the OAuth dance again next time the gadget tries to access a resource.
The proposal is to change the current implementation to look explicitly for 401 or 403 response codes in fetchFromServer instead of looking for any 4xx.
Any other recommendations on what the behavior should be are welcome.
This addresses bug SHINDIG-1711.
https://issues.apache.org/jira/browse/SHINDIG-1711
Diffs
-----
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2Request.java 1292022
Diff: https://reviews.apache.org/r/3987/diff
Testing
-------
Built and ran existing JUnits.
Thanks,
Stanton
Re: Review Request: OAuth2 access tokens being removed from OAuth2Store when
request returns any 4xx response
Posted by Stanton Sievers <si...@gmail.com>.
> On 2012-02-22 00:39:19, Ryan Baxter wrote:
> > LGTM. This seems reasonable. I took a quick peak at the OAuth 2 spec and it didnt seem to go into any details on this. You might want to post something on the OAuth mailing list to see if anyone there has any thoughts.
The only details I found were these: http://tools.ietf.org/html/draft-ietf-oauth-v2-bearer-16#section-3.1
It talks about services typically returning 400, 401, 403, and 405 when a request fails. However, the information it provides there also seems like we should really only remove the token if there's a 401. Removing the token from the store in that case would be fulfilling this part of the spec: "The client MAY request a new access token and retry the protected resource request."
403 indicates that the request requires higher privileges than that provided by the access token and that it's not necessarily a problem with the access token itself. I'm not sure if it would be possible to request a new access token with higher privileges since the scope is defined in the gadget from what I've seen.
And now the more I think about it, we should really be looking for those error codes like "invalid_token" or "insufficient scope" in the response headers... if those are even available in the flow that I'm looking at in Shindig... I'll investigate more.
- Stanton
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3987/#review5255
-----------------------------------------------------------
On 2012-02-21 21:40:31, Stanton Sievers wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3987/
> -----------------------------------------------------------
>
> (Updated 2012-02-21 21:40:31)
>
>
> Review request for shindig, li xu and Adam Clarke.
>
>
> Summary
> -------
>
> From JIRA:
> If the url to which a gadget is doing a makeRequest doesn't exist, i.e., returns a 404 to the Shindig server, the access token is being removed from the OAuth2 Store. This functionality is implemented here: org.apache.shindig.gadgets.oauth2.BasicOAuth2Request.fetchFromServer(OAuth2Accessor, HttpRequest)
>
> fetchFromServer is checking only if the response code is 4xx, and if so, it is removing the access token from the store. This seems right for 401 or 403 return codes, perhaps, but not for 404. The behavior for an end user would then be that they have to do the OAuth dance again next time the gadget tries to access a resource.
>
> The proposal is to change the current implementation to look explicitly for 401 or 403 response codes in fetchFromServer instead of looking for any 4xx.
>
> Any other recommendations on what the behavior should be are welcome.
>
>
> This addresses bug SHINDIG-1711.
> https://issues.apache.org/jira/browse/SHINDIG-1711
>
>
> Diffs
> -----
>
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2Request.java 1292022
>
> Diff: https://reviews.apache.org/r/3987/diff
>
>
> Testing
> -------
>
> Built and ran existing JUnits.
>
>
> Thanks,
>
> Stanton
>
>
Re: Review Request: OAuth2 access tokens being removed from OAuth2Store when
request returns any 4xx response
Posted by Ryan Baxter <rb...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3987/#review5255
-----------------------------------------------------------
Ship it!
LGTM. This seems reasonable. I took a quick peak at the OAuth 2 spec and it didnt seem to go into any details on this. You might want to post something on the OAuth mailing list to see if anyone there has any thoughts.
- Ryan
On 2012-02-21 21:40:31, Stanton Sievers wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3987/
> -----------------------------------------------------------
>
> (Updated 2012-02-21 21:40:31)
>
>
> Review request for shindig, li xu and Adam Clarke.
>
>
> Summary
> -------
>
> From JIRA:
> If the url to which a gadget is doing a makeRequest doesn't exist, i.e., returns a 404 to the Shindig server, the access token is being removed from the OAuth2 Store. This functionality is implemented here: org.apache.shindig.gadgets.oauth2.BasicOAuth2Request.fetchFromServer(OAuth2Accessor, HttpRequest)
>
> fetchFromServer is checking only if the response code is 4xx, and if so, it is removing the access token from the store. This seems right for 401 or 403 return codes, perhaps, but not for 404. The behavior for an end user would then be that they have to do the OAuth dance again next time the gadget tries to access a resource.
>
> The proposal is to change the current implementation to look explicitly for 401 or 403 response codes in fetchFromServer instead of looking for any 4xx.
>
> Any other recommendations on what the behavior should be are welcome.
>
>
> This addresses bug SHINDIG-1711.
> https://issues.apache.org/jira/browse/SHINDIG-1711
>
>
> Diffs
> -----
>
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2Request.java 1292022
>
> Diff: https://reviews.apache.org/r/3987/diff
>
>
> Testing
> -------
>
> Built and ran existing JUnits.
>
>
> Thanks,
>
> Stanton
>
>
Re: Review Request: OAuth2 access tokens being removed from OAuth2Store when
request returns any 4xx response
Posted by Adam Clarke <cl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3987/#review5466
-----------------------------------------------------------
Ship it!
OAuth 2.0 spec says response codes from the resource server are being the scope of the spec. I think that only removing the token on a 401 is more reasonable than what we have now.
- Adam
On 2012-02-21 21:40:31, Stanton Sievers wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3987/
> -----------------------------------------------------------
>
> (Updated 2012-02-21 21:40:31)
>
>
> Review request for shindig, li xu and Adam Clarke.
>
>
> Summary
> -------
>
> From JIRA:
> If the url to which a gadget is doing a makeRequest doesn't exist, i.e., returns a 404 to the Shindig server, the access token is being removed from the OAuth2 Store. This functionality is implemented here: org.apache.shindig.gadgets.oauth2.BasicOAuth2Request.fetchFromServer(OAuth2Accessor, HttpRequest)
>
> fetchFromServer is checking only if the response code is 4xx, and if so, it is removing the access token from the store. This seems right for 401 or 403 return codes, perhaps, but not for 404. The behavior for an end user would then be that they have to do the OAuth dance again next time the gadget tries to access a resource.
>
> The proposal is to change the current implementation to look explicitly for 401 or 403 response codes in fetchFromServer instead of looking for any 4xx.
>
> Any other recommendations on what the behavior should be are welcome.
>
>
> This addresses bug SHINDIG-1711.
> https://issues.apache.org/jira/browse/SHINDIG-1711
>
>
> Diffs
> -----
>
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2Request.java 1292022
>
> Diff: https://reviews.apache.org/r/3987/diff
>
>
> Testing
> -------
>
> Built and ran existing JUnits.
>
>
> Thanks,
>
> Stanton
>
>
Re: Review Request: OAuth2 access tokens being removed from OAuth2Store when
request returns any 4xx response
Posted by li xu <le...@gmail.com>.
> On 2012-02-22 03:03:25, li xu wrote:
> > LGTM. Thanks!
Sorry,missed previous comments. Second thought after reading it and checking a few web links, I agree more thoughts on "403" will be needed. The reason about "403" may not be token related...it's common to return "403" with a reason like "No directory Browsing" etc.
- li
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3987/#review5259
-----------------------------------------------------------
On 2012-02-21 21:40:31, Stanton Sievers wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3987/
> -----------------------------------------------------------
>
> (Updated 2012-02-21 21:40:31)
>
>
> Review request for shindig, li xu and Adam Clarke.
>
>
> Summary
> -------
>
> From JIRA:
> If the url to which a gadget is doing a makeRequest doesn't exist, i.e., returns a 404 to the Shindig server, the access token is being removed from the OAuth2 Store. This functionality is implemented here: org.apache.shindig.gadgets.oauth2.BasicOAuth2Request.fetchFromServer(OAuth2Accessor, HttpRequest)
>
> fetchFromServer is checking only if the response code is 4xx, and if so, it is removing the access token from the store. This seems right for 401 or 403 return codes, perhaps, but not for 404. The behavior for an end user would then be that they have to do the OAuth dance again next time the gadget tries to access a resource.
>
> The proposal is to change the current implementation to look explicitly for 401 or 403 response codes in fetchFromServer instead of looking for any 4xx.
>
> Any other recommendations on what the behavior should be are welcome.
>
>
> This addresses bug SHINDIG-1711.
> https://issues.apache.org/jira/browse/SHINDIG-1711
>
>
> Diffs
> -----
>
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2Request.java 1292022
>
> Diff: https://reviews.apache.org/r/3987/diff
>
>
> Testing
> -------
>
> Built and ran existing JUnits.
>
>
> Thanks,
>
> Stanton
>
>
Re: Review Request: OAuth2 access tokens being removed from OAuth2Store when
request returns any 4xx response
Posted by li xu <le...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3987/#review5259
-----------------------------------------------------------
Ship it!
LGTM. Thanks!
- li
On 2012-02-21 21:40:31, Stanton Sievers wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3987/
> -----------------------------------------------------------
>
> (Updated 2012-02-21 21:40:31)
>
>
> Review request for shindig, li xu and Adam Clarke.
>
>
> Summary
> -------
>
> From JIRA:
> If the url to which a gadget is doing a makeRequest doesn't exist, i.e., returns a 404 to the Shindig server, the access token is being removed from the OAuth2 Store. This functionality is implemented here: org.apache.shindig.gadgets.oauth2.BasicOAuth2Request.fetchFromServer(OAuth2Accessor, HttpRequest)
>
> fetchFromServer is checking only if the response code is 4xx, and if so, it is removing the access token from the store. This seems right for 401 or 403 return codes, perhaps, but not for 404. The behavior for an end user would then be that they have to do the OAuth dance again next time the gadget tries to access a resource.
>
> The proposal is to change the current implementation to look explicitly for 401 or 403 response codes in fetchFromServer instead of looking for any 4xx.
>
> Any other recommendations on what the behavior should be are welcome.
>
>
> This addresses bug SHINDIG-1711.
> https://issues.apache.org/jira/browse/SHINDIG-1711
>
>
> Diffs
> -----
>
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2Request.java 1292022
>
> Diff: https://reviews.apache.org/r/3987/diff
>
>
> Testing
> -------
>
> Built and ran existing JUnits.
>
>
> Thanks,
>
> Stanton
>
>
Re: Review Request: OAuth2 access tokens being removed from OAuth2Store when
request returns any 4xx response
Posted by Henry Saputra <hs...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3987/#review5467
-----------------------------------------------------------
Ship it!
+1
- Henry
On 2012-02-21 21:40:31, Stanton Sievers wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3987/
> -----------------------------------------------------------
>
> (Updated 2012-02-21 21:40:31)
>
>
> Review request for shindig, li xu and Adam Clarke.
>
>
> Summary
> -------
>
> From JIRA:
> If the url to which a gadget is doing a makeRequest doesn't exist, i.e., returns a 404 to the Shindig server, the access token is being removed from the OAuth2 Store. This functionality is implemented here: org.apache.shindig.gadgets.oauth2.BasicOAuth2Request.fetchFromServer(OAuth2Accessor, HttpRequest)
>
> fetchFromServer is checking only if the response code is 4xx, and if so, it is removing the access token from the store. This seems right for 401 or 403 return codes, perhaps, but not for 404. The behavior for an end user would then be that they have to do the OAuth dance again next time the gadget tries to access a resource.
>
> The proposal is to change the current implementation to look explicitly for 401 or 403 response codes in fetchFromServer instead of looking for any 4xx.
>
> Any other recommendations on what the behavior should be are welcome.
>
>
> This addresses bug SHINDIG-1711.
> https://issues.apache.org/jira/browse/SHINDIG-1711
>
>
> Diffs
> -----
>
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2Request.java 1292022
>
> Diff: https://reviews.apache.org/r/3987/diff
>
>
> Testing
> -------
>
> Built and ran existing JUnits.
>
>
> Thanks,
>
> Stanton
>
>
Re: Review Request: OAuth2 access tokens being removed from OAuth2Store when
request returns any 4xx response
Posted by Stanton Sievers <si...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3987/#review5475
-----------------------------------------------------------
If there are no other questions/issues I'll go ahead and commit this tomorrow.
- Stanton
On 2012-02-29 21:30:23, Stanton Sievers wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3987/
> -----------------------------------------------------------
>
> (Updated 2012-02-29 21:30:23)
>
>
> Review request for shindig, li xu and Adam Clarke.
>
>
> Summary
> -------
>
> From JIRA:
> If the url to which a gadget is doing a makeRequest doesn't exist, i.e., returns a 404 to the Shindig server, the access token is being removed from the OAuth2 Store. This functionality is implemented here: org.apache.shindig.gadgets.oauth2.BasicOAuth2Request.fetchFromServer(OAuth2Accessor, HttpRequest)
>
> fetchFromServer is checking only if the response code is 4xx, and if so, it is removing the access token from the store. This seems right for 401 or 403 return codes, perhaps, but not for 404. The behavior for an end user would then be that they have to do the OAuth dance again next time the gadget tries to access a resource.
>
> The proposal is to change the current implementation to look explicitly for 401 or 403 response codes in fetchFromServer instead of looking for any 4xx.
>
> Any other recommendations on what the behavior should be are welcome.
>
>
> This addresses bug SHINDIG-1711.
> https://issues.apache.org/jira/browse/SHINDIG-1711
>
>
> Diffs
> -----
>
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2Request.java 1295256
>
> Diff: https://reviews.apache.org/r/3987/diff
>
>
> Testing
> -------
>
> Built and ran existing JUnits.
>
>
> Thanks,
>
> Stanton
>
>
Re: Review Request: OAuth2 access tokens being removed from OAuth2Store when
request returns any 4xx response
Posted by Stanton Sievers <si...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3987/#review5526
-----------------------------------------------------------
Ship it!
Committed revision 1295877.
- Stanton
On 2012-02-29 21:30:23, Stanton Sievers wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3987/
> -----------------------------------------------------------
>
> (Updated 2012-02-29 21:30:23)
>
>
> Review request for shindig, li xu and Adam Clarke.
>
>
> Summary
> -------
>
> From JIRA:
> If the url to which a gadget is doing a makeRequest doesn't exist, i.e., returns a 404 to the Shindig server, the access token is being removed from the OAuth2 Store. This functionality is implemented here: org.apache.shindig.gadgets.oauth2.BasicOAuth2Request.fetchFromServer(OAuth2Accessor, HttpRequest)
>
> fetchFromServer is checking only if the response code is 4xx, and if so, it is removing the access token from the store. This seems right for 401 or 403 return codes, perhaps, but not for 404. The behavior for an end user would then be that they have to do the OAuth dance again next time the gadget tries to access a resource.
>
> The proposal is to change the current implementation to look explicitly for 401 or 403 response codes in fetchFromServer instead of looking for any 4xx.
>
> Any other recommendations on what the behavior should be are welcome.
>
>
> This addresses bug SHINDIG-1711.
> https://issues.apache.org/jira/browse/SHINDIG-1711
>
>
> Diffs
> -----
>
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2Request.java 1295256
>
> Diff: https://reviews.apache.org/r/3987/diff
>
>
> Testing
> -------
>
> Built and ran existing JUnits.
>
>
> Thanks,
>
> Stanton
>
>
Re: Review Request: OAuth2 access tokens being removed from OAuth2Store when
request returns any 4xx response
Posted by Stanton Sievers <si...@gmail.com>.
> On 2012-02-29 21:43:25, Henry Saputra wrote:
> > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2Request.java, line 563
> > <https://reviews.apache.org/r/3987/diff/2/?file=86671#file86671line563>
> >
> > Wouldnt we want to check for forbidden?
>From talking to Adam and reading the spec, no. http://tools.ietf.org/html/draft-ietf-oauth-v2-bearer-16#section-3.1
insufficient_scope
The request requires higher privileges than provided by the
access token. The resource server SHOULD respond with the HTTP
403 (Forbidden) status code and MAY include the "scope"
attribute with the scope necessary to access the protected
resource.
Basically this means that even if we got a new token we still wouldn't have permissions. It's not that the token is bad (in fact it's a valid access token), we simply don't have permissions to access the resource. In the 403 case the user could get a new token but the request would still fail.
- Stanton
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3987/#review5469
-----------------------------------------------------------
On 2012-02-29 21:30:23, Stanton Sievers wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3987/
> -----------------------------------------------------------
>
> (Updated 2012-02-29 21:30:23)
>
>
> Review request for shindig, li xu and Adam Clarke.
>
>
> Summary
> -------
>
> From JIRA:
> If the url to which a gadget is doing a makeRequest doesn't exist, i.e., returns a 404 to the Shindig server, the access token is being removed from the OAuth2 Store. This functionality is implemented here: org.apache.shindig.gadgets.oauth2.BasicOAuth2Request.fetchFromServer(OAuth2Accessor, HttpRequest)
>
> fetchFromServer is checking only if the response code is 4xx, and if so, it is removing the access token from the store. This seems right for 401 or 403 return codes, perhaps, but not for 404. The behavior for an end user would then be that they have to do the OAuth dance again next time the gadget tries to access a resource.
>
> The proposal is to change the current implementation to look explicitly for 401 or 403 response codes in fetchFromServer instead of looking for any 4xx.
>
> Any other recommendations on what the behavior should be are welcome.
>
>
> This addresses bug SHINDIG-1711.
> https://issues.apache.org/jira/browse/SHINDIG-1711
>
>
> Diffs
> -----
>
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2Request.java 1295256
>
> Diff: https://reviews.apache.org/r/3987/diff
>
>
> Testing
> -------
>
> Built and ran existing JUnits.
>
>
> Thanks,
>
> Stanton
>
>
Re: Review Request: OAuth2 access tokens being removed from OAuth2Store when
request returns any 4xx response
Posted by Henry Saputra <hs...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3987/#review5469
-----------------------------------------------------------
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2Request.java
<https://reviews.apache.org/r/3987/#comment11870>
Wouldnt we want to check for forbidden?
- Henry
On 2012-02-29 21:30:23, Stanton Sievers wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3987/
> -----------------------------------------------------------
>
> (Updated 2012-02-29 21:30:23)
>
>
> Review request for shindig, li xu and Adam Clarke.
>
>
> Summary
> -------
>
> From JIRA:
> If the url to which a gadget is doing a makeRequest doesn't exist, i.e., returns a 404 to the Shindig server, the access token is being removed from the OAuth2 Store. This functionality is implemented here: org.apache.shindig.gadgets.oauth2.BasicOAuth2Request.fetchFromServer(OAuth2Accessor, HttpRequest)
>
> fetchFromServer is checking only if the response code is 4xx, and if so, it is removing the access token from the store. This seems right for 401 or 403 return codes, perhaps, but not for 404. The behavior for an end user would then be that they have to do the OAuth dance again next time the gadget tries to access a resource.
>
> The proposal is to change the current implementation to look explicitly for 401 or 403 response codes in fetchFromServer instead of looking for any 4xx.
>
> Any other recommendations on what the behavior should be are welcome.
>
>
> This addresses bug SHINDIG-1711.
> https://issues.apache.org/jira/browse/SHINDIG-1711
>
>
> Diffs
> -----
>
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2Request.java 1295256
>
> Diff: https://reviews.apache.org/r/3987/diff
>
>
> Testing
> -------
>
> Built and ran existing JUnits.
>
>
> Thanks,
>
> Stanton
>
>
Re: Review Request: OAuth2 access tokens being removed from OAuth2Store when
request returns any 4xx response
Posted by Stanton Sievers <si...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3987/
-----------------------------------------------------------
(Updated 2012-02-29 21:30:23.106625)
Review request for shindig, li xu and Adam Clarke.
Changes
-------
Updated to only remove the token based on 401. Getting a new token in the 403 case will not help since it indicates the user does not have the right permissions.
Summary
-------
>From JIRA:
If the url to which a gadget is doing a makeRequest doesn't exist, i.e., returns a 404 to the Shindig server, the access token is being removed from the OAuth2 Store. This functionality is implemented here: org.apache.shindig.gadgets.oauth2.BasicOAuth2Request.fetchFromServer(OAuth2Accessor, HttpRequest)
fetchFromServer is checking only if the response code is 4xx, and if so, it is removing the access token from the store. This seems right for 401 or 403 return codes, perhaps, but not for 404. The behavior for an end user would then be that they have to do the OAuth dance again next time the gadget tries to access a resource.
The proposal is to change the current implementation to look explicitly for 401 or 403 response codes in fetchFromServer instead of looking for any 4xx.
Any other recommendations on what the behavior should be are welcome.
This addresses bug SHINDIG-1711.
https://issues.apache.org/jira/browse/SHINDIG-1711
Diffs (updated)
-----
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2Request.java 1295256
Diff: https://reviews.apache.org/r/3987/diff
Testing
-------
Built and ran existing JUnits.
Thanks,
Stanton