Skip to content
This repository was archived by the owner on May 1, 2024. It is now read-only.

Added flag for URL-escaping of scopes parameter #48

Closed
wants to merge 2 commits into from
Closed

Added flag for URL-escaping of scopes parameter #48

wants to merge 2 commits into from

Conversation

jsauve
Copy link

@jsauve jsauve commented Dec 14, 2013

URL-encoding the scope parameter of an OAuth request to Instagram (and possibly other services as well) causes a failure. If the scope parameter is “scope=basic+likes+comments+relationships”, it is URL-escaped by the Xamarin.Auth OAuth2Authenticator to "scope=basic%2Blikes%2Bcomments%2Brelationships”. The auth endpoint at Instagram does not like this and replies with {"code": 400, "error_type": "OAuthException", "error_message": "Invalid scope field(s): basic+likes+”comments+relationships}. A misleading message for sure, because as the error message rendered to the browser, it's not obvious that the parameter has been encoded. I imagine that other services work similarly and also barf if the scope param is URL-encoded. So, I have added a flag to the OAuth2Authenticator constructors to enable/disable the URL-encoding of the scope parameter. I added a flag instead of just removing the URL-encoding altogether because I am unsure of the behavior of other OAuth services when it comes to this matter. By default, the original behavior will remain, but provides the option to NOT encode the scope parameter.

URL-escaping the scopes parameter of an OAuth request to Instagram (and
possibly other services as well) causes a failure. If the scopes
parmeter is “basic+likes+comments+relationships”, it is URL-escaped to
"basic%2Blikes%2Bcomments%2Brelationships”. The auth endppint at
Instagram does not like this and replies with {"code": 400,
"error_type": "OAuthException", "error_message": "Invalid scope
field(s): basic+likes+”comments+relationships}. I imagine that other
services work similarly and also barf if the scopes param is
URL-encoded. So, I have added a flag to the OAuth2Authenticator
constructrs to enable/disable the URL-escaping of the scopes parameter.
In my previous pull request, I named this param ambiguously. It should
be named encodeScope, not escapeScope; because the bool flag is
referring to the URL-encoding of the scope parameter. “Escape” is the
wrong terminology.
@ermau
Copy link
Contributor

ermau commented Apr 21, 2014

Adding the parameter where you have is a breaking change for anyone who supplied getUsernameAsync. We should move the param to the end or make it a property. I'm thinking it should become a property instead, if we have to add more toggles like this in the future, the ctor is going to become polluted very fast.

@jsauve
Copy link
Author

jsauve commented Apr 21, 2014

Good point. Property is better. Thanks for considering my request.
On Apr 21, 2014 8:14 AM, "Eric Maupin" notifications@github.com wrote:

Adding the parameter where you have is a breaking change for anyone who
supplied getUsernameAsync. We should move the param to the end or make it
a property. I'm thinking it should become a property instead, if we have to
add more toggles like this in the future, the ctor is going to become
polluted very fast.


Reply to this email directly or view it on GitHubhttps://github.com//pull/48#issuecomment-40934319
.

@ermau ermau added this to the v1.3 milestone Apr 21, 2014
@jsauve
Copy link
Author

jsauve commented Apr 28, 2014

Re-worked as a better solution by using a property instead. See pull request #62 .

@jsauve jsauve closed this Apr 28, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants