Skip to content

Commit

Permalink
oauth: ensure auth grant redirect URI matches token req
Browse files Browse the repository at this point in the history
  • Loading branch information
@mjcheetham
mjcheetham committed Apr 22, 2020
1 parent dfc2868 commit 5c88b02
Show file tree
Hide file tree
Showing 7 changed files with 74 additions and 54 deletions.
2 changes: 1 addition & 1 deletion src/shared/Microsoft.Git.CredentialManager.Tests/Authentication/OAuth2ClientTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ public async Task OAuth2Client_GetTokenByAuthorizationCodeAsync()

OAuth2Client client = CreateClient(httpHandler, endpoints);

var authCodeResult = new OAuth2AuthorizationCodeResult(authCode);
var authCodeResult = new OAuth2AuthorizationCodeResult(authCode, TestRedirectUri);
OAuth2TokenResult result = await client.GetTokenByAuthorizationCodeAsync(authCodeResult, CancellationToken.None);

Assert.NotNull(result);
Expand Down
2 changes: 2 additions & 0 deletions src/shared/Microsoft.Git.CredentialManager/Authentication/OAuth/IOAuth2WebBrowser.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ namespace Microsoft.Git.CredentialManager.Authentication.OAuth
{
public interface IOAuth2WebBrowser
{
Uri UpdateRedirectUri(Uri uri);

Task<Uri> GetAuthenticationCodeAsync(Uri authorizationUri, Uri redirectUri, CancellationToken ct);
}
}
6 changes: 4 additions & 2 deletions ...red/Microsoft.Git.CredentialManager/Authentication/OAuth/OAuth2AuthorizationCodeResult.cs
View file
Original file line number Diff line number Diff line change
@@ -1,18 +1,20 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT license.
using System;

namespace Microsoft.Git.CredentialManager.Authentication.OAuth
{
public class OAuth2AuthorizationCodeResult
{
public OAuth2AuthorizationCodeResult(string code, string codeVerifier = null)
public OAuth2AuthorizationCodeResult(string code, Uri redirectUri = null, string codeVerifier = null)
{
Code = code;
RedirectUri = redirectUri;
CodeVerifier = codeVerifier;
}

public string Code { get; }

public Uri RedirectUri { get; }
public string CodeVerifier { get; }
}
}
18 changes: 10 additions & 8 deletions src/shared/Microsoft.Git.CredentialManager/Authentication/OAuth/OAuth2Client.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,9 +104,11 @@ public async Task<OAuth2AuthorizationCodeResult> GetAuthorizationCodeAsync(IEnum
[OAuth2Constants.AuthorizationEndpoint.PkceChallengeParameter] = codeChallenge
};

Uri redirectUri = null;
if (_redirectUri != null)
{
queryParams[OAuth2Constants.RedirectUriParameter] = _redirectUri.ToString();
redirectUri = browser.UpdateRedirectUri(_redirectUri);
queryParams[OAuth2Constants.RedirectUriParameter] = redirectUri.ToString();
}

string scopesStr = string.Join(" ", scopes);
Expand All @@ -123,12 +125,12 @@ public async Task<OAuth2AuthorizationCodeResult> GetAuthorizationCodeAsync(IEnum
Uri authorizationUri = authorizationUriBuilder.Uri;

// Open the browser at the request URI to start the authorization code grant flow.
Uri redirectUri = await browser.GetAuthenticationCodeAsync(authorizationUri, _redirectUri, ct);
Uri finalUri = await browser.GetAuthenticationCodeAsync(authorizationUri, redirectUri, ct);

// Check for errors serious enough we should terminate the flow, such as if the state value returned does
// not match the one we passed. This indicates a badly implemented Authorization Server, or worse, some
// form of failed MITM or replay attack.
IDictionary<string, string> redirectQueryParams = redirectUri.GetQueryParameters();
IDictionary<string, string> redirectQueryParams = finalUri.GetQueryParameters();
if (!redirectQueryParams.TryGetValue(OAuth2Constants.AuthorizationGrantResponse.StateParameter, out string replyState))
{
throw new OAuth2Exception($"Missing '{OAuth2Constants.AuthorizationGrantResponse.StateParameter}' in response.");
Expand All @@ -144,7 +146,7 @@ public async Task<OAuth2AuthorizationCodeResult> GetAuthorizationCodeAsync(IEnum
throw new OAuth2Exception($"Missing '{OAuth2Constants.AuthorizationGrantResponse.AuthorizationCodeParameter}' in response.");
}

return new OAuth2AuthorizationCodeResult(authCode, codeVerifier);
return new OAuth2AuthorizationCodeResult(authCode, redirectUri, codeVerifier);
}

public async Task<OAuth2DeviceCodeResult> GetDeviceCodeAsync(IEnumerable<string> scopes, CancellationToken ct)
Expand Down Expand Up @@ -191,14 +193,14 @@ public async Task<OAuth2TokenResult> GetTokenByAuthorizationCodeAsync(OAuth2Auth
[OAuth2Constants.ClientIdParameter] = _clientId
};

if (authorizationCodeResult.CodeVerifier != null)
if (authorizationCodeResult.RedirectUri != null)
{
formData[OAuth2Constants.TokenEndpoint.PkceVerifierParameter] = authorizationCodeResult.CodeVerifier;
formData[OAuth2Constants.RedirectUriParameter] = authorizationCodeResult.RedirectUri.ToString();
}

if (_redirectUri != null)
if (authorizationCodeResult.CodeVerifier != null)
{
formData[OAuth2Constants.RedirectUriParameter] = _redirectUri.ToString();
formData[OAuth2Constants.TokenEndpoint.PkceVerifierParameter] = authorizationCodeResult.CodeVerifier;
}

using (HttpContent requestContent = new FormUrlEncodedContent(formData))
Expand Down
64 changes: 30 additions & 34 deletions src/shared/Microsoft.Git.CredentialManager/Authentication/OAuth/OAuth2SystemWebBrowser.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,55 +42,35 @@ public OAuth2SystemWebBrowser(OAuth2WebBrowserOptions options)
_options = options;
}

public async Task<Uri> GetAuthenticationCodeAsync(Uri authorizationUri, Uri redirectUri, CancellationToken ct)
public Uri UpdateRedirectUri(Uri uri)
{
if (!redirectUri.IsLoopback)
if (!uri.IsLoopback)
{
throw new ArgumentException("Only localhost is supported as a redirect URI.", nameof(redirectUri));
throw new ArgumentException("Only localhost is supported as a redirect URI.", nameof(uri));
}

// If a port has been specified use it, otherwise find a free one
if (redirectUri.IsDefaultPort)
if (uri.IsDefaultPort)
{
int port = GetFreeTcpPort();

UpdateLoopbackRedirectPort(port, ref authorizationUri, ref redirectUri);
return new UriBuilder(uri) {Port = port}.Uri;
}

Task<Uri> interceptTask = InterceptRequestsAsync(redirectUri, ct);

OpenDefaultBrowser(authorizationUri);

return await interceptTask;
return uri;
}

private void UpdateLoopbackRedirectPort(int port, ref Uri authorizationUri, ref Uri redirectUri)
public async Task<Uri> GetAuthenticationCodeAsync(Uri authorizationUri, Uri redirectUri, CancellationToken ct)
{
IDictionary<string, string> authUriQuery = authorizationUri.GetQueryParameters();

Uri newRedirectUri = new UriBuilder(redirectUri) {Port = port}.Uri;

authUriQuery[OAuth2Constants.RedirectUriParameter] = newRedirectUri.ToString();

Uri newAuthorizationUri = new UriBuilder(authorizationUri){Query = authUriQuery.ToQueryString()}.Uri;
if (!redirectUri.IsLoopback)
{
throw new ArgumentException("Only localhost is supported as a redirect URI.", nameof(redirectUri));
}

authorizationUri = newAuthorizationUri;
redirectUri = newRedirectUri;
}
Task<Uri> interceptTask = InterceptRequestsAsync(redirectUri, ct);

private static int GetFreeTcpPort()
{
var listener = new TcpListener(IPAddress.Loopback, 0);
OpenDefaultBrowser(authorizationUri);

try
{
listener.Start();
return ((IPEndPoint) listener.LocalEndpoint).Port;
}
finally
{
listener.Stop();
}
return await interceptTask;
}

private void OpenDefaultBrowser(Uri uri)
Expand Down Expand Up @@ -205,5 +185,21 @@ string FormatError(string format)
}
}
}

private static int GetFreeTcpPort()
{
var listener = new TcpListener(IPAddress.Loopback, 0);

try
{
listener.Start();
return ((IPEndPoint) listener.LocalEndpoint).Port;
}
finally
{
listener.Stop();
}
}

}
}
31 changes: 22 additions & 9 deletions src/shared/TestInfrastructure/Objects/TestOAuth2Server.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,8 @@ private Task<HttpResponseMessage> OnAuthorizationEndpointAsync(HttpRequestMessag
}

// Redirect is optional, but if it is specified it must match a registered URI
Uri redirectUri = app.ValidateRedirect(reqQuery[OAuth2Constants.RedirectUriParameter]);
reqQuery.TryGetValue(OAuth2Constants.RedirectUriParameter, out string redirectUriStr);
Uri redirectUri = app.ValidateRedirect(redirectUriStr);

// Scope is optional
reqQuery.TryGetValue(OAuth2Constants.ScopeParameter, out string scopeStr);
Expand Down Expand Up @@ -99,7 +100,7 @@ private Task<HttpResponseMessage> OnAuthorizationEndpointAsync(HttpRequestMessag

// Create the auth code grant
OAuth2Application.AuthCodeGrant grant = app.CreateAuthorizationCodeGrant(
TokenGenerator, scopes, codeChallenge, codeChallengeMethod);
TokenGenerator, scopes, redirectUriStr, codeChallenge, codeChallengeMethod);

var respQuery = new Dictionary<string, string>
{
Expand Down Expand Up @@ -189,10 +190,12 @@ private async Task<HttpResponseMessage> OnTokenEndpointAsync(HttpRequestMessage
}

formData.TryGetValue(OAuth2Constants.TokenEndpoint.PkceVerifierParameter, out string codeVerifier);
if (formData.TryGetValue(OAuth2Constants.RedirectUriParameter, out string redirectUriStr))
{
app.ValidateRedirect(redirectUriStr);
}

app.ValidateRedirect(formData[OAuth2Constants.RedirectUriParameter]);

tokenResp = app.CreateTokenByAuthorizationGrant(TokenGenerator, authCode, codeVerifier);
tokenResp = app.CreateTokenByAuthorizationGrant(TokenGenerator, authCode, codeVerifier, redirectUriStr);
}
else if (StringComparer.OrdinalIgnoreCase.Equals(grantType, OAuth2Constants.TokenEndpoint.RefreshTokenGrantType))
{
Expand Down Expand Up @@ -287,16 +290,18 @@ public class OAuth2Application
{
public class AuthCodeGrant
{
public AuthCodeGrant(string code, string[] scopes,
public AuthCodeGrant(string code, string[] scopes, string redirectUri = null,
string codeChallenge = null, OAuth2PkceChallengeMethod codeChallengeMethod = OAuth2PkceChallengeMethod.Plain)
{
Code = code;
Scopes = scopes;
RedirectUri = redirectUri;
CodeChallenge = codeChallenge;
CodeChallengeMethod = codeChallengeMethod;
}
public string Code { get; }
public string[] Scopes { get; }
public string RedirectUri { get; }
public string CodeChallenge { get; }
public OAuth2PkceChallengeMethod CodeChallengeMethod { get; }
}
Expand Down Expand Up @@ -336,11 +341,11 @@ public OAuth2Application(string id)
public IDictionary<string, string[]> RefreshTokens { get; } = new Dictionary<string, string[]>();

public AuthCodeGrant CreateAuthorizationCodeGrant(TestOAuth2ServerTokenGenerator generator,
string[] scopes, string codeChallenge, OAuth2PkceChallengeMethod codeChallengeMethod)
string[] scopes, string redirectUri, string codeChallenge, OAuth2PkceChallengeMethod codeChallengeMethod)
{
string code = generator.CreateAuthorizationCode();

var grant = new AuthCodeGrant(code, scopes, codeChallenge, codeChallengeMethod);
var grant = new AuthCodeGrant(code, scopes, redirectUri, codeChallenge, codeChallengeMethod);
AuthGrants.Add(grant);

return grant;
Expand Down Expand Up @@ -387,14 +392,15 @@ public bool IsDeviceCodeGrantApproved(string deviceCode)
}

public TokenEndpointResponseJson CreateTokenByAuthorizationGrant(
TestOAuth2ServerTokenGenerator generator, string authCode, string codeVerifier)
TestOAuth2ServerTokenGenerator generator, string authCode, string codeVerifier, string redirectUri)
{
var grant = AuthGrants.FirstOrDefault(x => x.Code == authCode);
if (grant is null)
{
throw new Exception($"Invalid authorization code '{authCode}'");
}

// Validate the grant's code challenge was constructed from the given code verifier
if (!string.IsNullOrWhiteSpace(grant.CodeChallenge))
{
if (string.IsNullOrWhiteSpace(codeVerifier))
Expand Down Expand Up @@ -431,6 +437,13 @@ public TokenEndpointResponseJson CreateTokenByAuthorizationGrant(
}
}

// If an explicit redirect URI was used as part of the authorization request then
// the redirect URI used for the token call must match exactly.
if (!string.IsNullOrWhiteSpace(grant.RedirectUri) && !StringComparer.Ordinal.Equals(grant.RedirectUri, redirectUri))
{
throw new Exception("Redirect URI must match exactly the one used when requesting the authorization code.");
}

string accessToken = generator.CreateAccessToken();
string refreshToken = generator.CreateRefreshToken();

Expand Down
5 changes: 5 additions & 0 deletions src/shared/TestInfrastructure/Objects/TestOAuth2WebBrowser.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@ public TestOAuth2WebBrowser(HttpMessageHandler httpHandler)
_httpClient = new HttpClient(httpHandler);
}

public Uri UpdateRedirectUri(Uri uri)
{
return uri;
}

public async Task<Uri> GetAuthenticationCodeAsync(Uri authorizationUri, Uri redirectUri, CancellationToken ct)
{
using (var response = await _httpClient.SendAsync(HttpMethod.Get, authorizationUri))
Expand Down

0 comments on commit 5c88b02

Please sign in to comment.