mattjohnsonpint / DotNetOpenAuth.GoogleOAuth2

DotNetOpenAuth OAuth2 Client for Google
23 stars 18 forks source link

When callback URL has its Query part empty, GoogleOAuth2Client throws #4

Closed agibalov closed 11 years ago

agibalov commented 11 years ago

Here's the code:

private Uri GetGoogleCallbackUrl()
{
  var uriBuilder = new UriBuilder(Request.Url)
    {
      Query = string.Empty, // note, it's valid value for Query
      Fragment = string.Empty,
      Path = Url.Action("GoogleAuthCallback")
    };
  return uriBuilder.Uri;//.AbsoluteUri;
}

...
var authentication = googleClient.VerifyAuthentication(
  HttpContext, 
  GetGoogleCallbackUrl());

The last statement throws:

Parameter name: startIndex]
   System.String.InternalSubStringWithChecks(Int32 startIndex, Int32 length, Boolean fAlwaysCopy) +10613715
   System.String.Substring(Int32 startIndex) +12
   DotNetOpenAuth.GoogleOAuth2.GoogleOAuth2Client.GetServiceLoginUrl(Uri returnUrl) +395
   DotNetOpenAuth.AspNet.Clients.OAuth2Client.RequestAuthentication(HttpContextBase context, Uri returnUrl) +59
   AspNetMvcGoogleFacebookTwitterAuthExperiment.Controllers.HomeController.SignInWithGoogle() in c:\dev\AspNetMvcGoogleFacebookTwitterAuthExperiment\AspNetMvcGoogleFacebookTwitterAuthExperiment\Controllers\HomeController.cs:28

This happens because this piece of code (https://github.com/mj1856/DotNetOpenAuth.GoogleOAuth2/blob/master/DotNetOpenAuth.GoogleOAuth2/GoogleOAuth2Client.cs):

protected override Uri GetServiceLoginUrl(Uri returnUrl)
{
    var scopes = _requestedScopes.Select(x => !x.StartsWith("http", StringComparison.OrdinalIgnoreCase) ? ScopeBaseUri + x : x);

    return BuildUri(AuthorizationEndpoint, new NameValueCollection
        {
            { "response_type", "code" },
            { "client_id", _clientId },
            { "scope", string.Join(" ", scopes) },
            { "redirect_uri", returnUrl.GetLeftPart(UriPartial.Path) },
            { "state", returnUrl.Query.Substring(1) },
        });
}

expects Query to always be at least 1 character long (I suppose, it's for ? in ?a=1).

mattjohnsonpint commented 11 years ago

Merged your PR and fixed in release 1.1.1. Thanks!

agibalov commented 11 years ago

Thanks for 1.1.1!