-
Notifications
You must be signed in to change notification settings - Fork 550
Open
Labels
Description
The Untappd provider uses GET requests and sends the token request parameters in both the query string and the request form using formurl-encoding, which is a strong sign something is not right as GET requests are not expected to have a content attached (.NET Core's HttpClient
allows it, but on .NET Framework, the following code would throw an exception:)
AspNet.Security.OAuth.Providers/src/AspNet.Security.OAuth.Untappd/UntappdAuthenticationHandler.cs
Lines 29 to 74 in cbbc7a1
protected override async Task<OAuthTokenResponse> ExchangeCodeAsync([NotNull] OAuthCodeExchangeContext context) | |
{ | |
var tokenRequestParameters = new Dictionary<string, string>() | |
{ | |
["client_id"] = Options.ClientId, | |
["redirect_uri"] = context.RedirectUri, | |
["client_secret"] = Options.ClientSecret, | |
["code"] = context.Code, | |
["grant_type"] = "authorization_code", | |
}; | |
// PKCE https://tools.ietf.org/html/rfc7636#section-4.5, see BuildChallengeUrl | |
if (context.Properties.Items.TryGetValue(OAuthConstants.CodeVerifierKey, out var codeVerifier)) | |
{ | |
tokenRequestParameters.Add(OAuthConstants.CodeVerifierKey, codeVerifier!); | |
context.Properties.Items.Remove(OAuthConstants.CodeVerifierKey); | |
} | |
var parameters = new Dictionary<string, string?> | |
{ | |
["client_id"] = Options.ClientId, | |
["redirect_uri"] = context.RedirectUri, | |
["client_secret"] = Options.ClientSecret, | |
["code"] = context.Code, | |
}; | |
var address = QueryHelpers.AddQueryString(Options.TokenEndpoint, parameters); | |
using var requestContent = new FormUrlEncodedContent(tokenRequestParameters); | |
using var requestMessage = new HttpRequestMessage(HttpMethod.Get, address); | |
requestMessage.Headers.Accept.Add(new MediaTypeWithQualityHeaderValue("application/json")); | |
requestMessage.Content = requestContent; | |
requestMessage.Version = Backchannel.DefaultRequestVersion; | |
using var response = await Backchannel.SendAsync(requestMessage, Context.RequestAborted); | |
if (response.IsSuccessStatusCode) | |
{ | |
var payload = JsonDocument.Parse(await response.Content.ReadAsStringAsync(Context.RequestAborted)); | |
return OAuthTokenResponse.Success(payload); | |
} | |
else | |
{ | |
await Log.ExchangeCodeErrorAsync(Logger, response, Context.RequestAborted); | |
throw new HttpRequestException("An error occurred while exchanging token codes."); | |
} | |
} |
We should determine whether this monstrosity is 100% required or remove the request form part if it's not.
/cc @martincostello
martincostello