Skip to content

Commit adccfaf

Browse files
mjcheethammjcheetham
committed
oauth: ensure auth grant redirect URI matches token req
1 parent 5f13ab2 commit adccfaf

File tree

7 files changed

+74
-54
lines changed

7 files changed

+74
-54
lines changed

‎src/shared/Microsoft.Git.CredentialManager.Tests/Authentication/OAuth2ClientTests.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ public async Task OAuth2Client_GetTokenByAuthorizationCodeAsync()
9898

9999
OAuth2Client client = CreateClient(httpHandler, endpoints);
100100

101-
var authCodeResult = new OAuth2AuthorizationCodeResult(authCode);
101+
var authCodeResult = new OAuth2AuthorizationCodeResult(authCode, TestRedirectUri);
102102
OAuth2TokenResult result = await client.GetTokenByAuthorizationCodeAsync(authCodeResult, CancellationToken.None);
103103

104104
Assert.NotNull(result);

‎src/shared/Microsoft.Git.CredentialManager/Authentication/OAuth/IOAuth2WebBrowser.cs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ namespace Microsoft.Git.CredentialManager.Authentication.OAuth
99
{
1010
public interface IOAuth2WebBrowser
1111
{
12+
Uri UpdateRedirectUri(Uri uri);
13+
1214
Task<Uri> GetAuthenticationCodeAsync(Uri authorizationUri, Uri redirectUri, CancellationToken ct);
1315
}
1416
}

‎src/shared/Microsoft.Git.CredentialManager/Authentication/OAuth/OAuth2AuthorizationCodeResult.cs‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,20 @@
11
// Copyright (c) Microsoft Corporation. All rights reserved.
22
// Licensed under the MIT license.
3+
using System;
34

45
namespace Microsoft.Git.CredentialManager.Authentication.OAuth
56
{
67
public class OAuth2AuthorizationCodeResult
78
{
8-
public OAuth2AuthorizationCodeResult(string code, string codeVerifier = null)
9+
public OAuth2AuthorizationCodeResult(string code, Uri redirectUri = null, string codeVerifier = null)
910
{
1011
Code = code;
12+
RedirectUri = redirectUri;
1113
CodeVerifier = codeVerifier;
1214
}
1315

1416
public string Code { get; }
15-
17+
public Uri RedirectUri { get; }
1618
public string CodeVerifier { get; }
1719
}
1820
}

‎src/shared/Microsoft.Git.CredentialManager/Authentication/OAuth/OAuth2Client.cs‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -104,9 +104,11 @@ public async Task<OAuth2AuthorizationCodeResult> GetAuthorizationCodeAsync(IEnum
104104
[OAuth2Constants.AuthorizationEndpoint.PkceChallengeParameter] = codeChallenge
105105
};
106106

107+
Uri redirectUri = null;
107108
if (_redirectUri != null)
108109
{
109-
queryParams[OAuth2Constants.RedirectUriParameter] = _redirectUri.ToString();
110+
redirectUri = browser.UpdateRedirectUri(_redirectUri);
111+
queryParams[OAuth2Constants.RedirectUriParameter] = redirectUri.ToString();
110112
}
111113

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

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

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

147-
return new OAuth2AuthorizationCodeResult(authCode, codeVerifier);
149+
return new OAuth2AuthorizationCodeResult(authCode, redirectUri, codeVerifier);
148150
}
149151

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

194-
if (authorizationCodeResult.CodeVerifier != null)
196+
if (authorizationCodeResult.RedirectUri != null)
195197
{
196-
formData[OAuth2Constants.TokenEndpoint.PkceVerifierParameter] = authorizationCodeResult.CodeVerifier;
198+
formData[OAuth2Constants.RedirectUriParameter] = authorizationCodeResult.RedirectUri.ToString();
197199
}
198200

199-
if (_redirectUri != null)
201+
if (authorizationCodeResult.CodeVerifier != null)
200202
{
201-
formData[OAuth2Constants.RedirectUriParameter] = _redirectUri.ToString();
203+
formData[OAuth2Constants.TokenEndpoint.PkceVerifierParameter] = authorizationCodeResult.CodeVerifier;
202204
}
203205

204206
using (HttpContent requestContent = new FormUrlEncodedContent(formData))

‎src/shared/Microsoft.Git.CredentialManager/Authentication/OAuth/OAuth2SystemWebBrowser.cs‎

Lines changed: 30 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -42,55 +42,35 @@ public OAuth2SystemWebBrowser(OAuth2WebBrowserOptions options)
4242
_options = options;
4343
}
4444

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

5252
// If a port has been specified use it, otherwise find a free one
53-
if (redirectUri.IsDefaultPort)
53+
if (uri.IsDefaultPort)
5454
{
5555
int port = GetFreeTcpPort();
56-
57-
UpdateLoopbackRedirectPort(port, ref authorizationUri, ref redirectUri);
56+
return new UriBuilder(uri) {Port = port}.Uri;
5857
}
5958

60-
Task<Uri> interceptTask = InterceptRequestsAsync(redirectUri, ct);
61-
62-
OpenDefaultBrowser(authorizationUri);
63-
64-
return await interceptTask;
59+
return uri;
6560
}
6661

67-
private void UpdateLoopbackRedirectPort(int port, ref Uri authorizationUri, ref Uri redirectUri)
62+
public async Task<Uri> GetAuthenticationCodeAsync(Uri authorizationUri, Uri redirectUri, CancellationToken ct)
6863
{
69-
IDictionary<string, string> authUriQuery = authorizationUri.GetQueryParameters();
70-
71-
Uri newRedirectUri = new UriBuilder(redirectUri) {Port = port}.Uri;
72-
73-
authUriQuery[OAuth2Constants.RedirectUriParameter] = newRedirectUri.ToString();
74-
75-
Uri newAuthorizationUri = new UriBuilder(authorizationUri){Query = authUriQuery.ToQueryString()}.Uri;
64+
if (!redirectUri.IsLoopback)
65+
{
66+
throw new ArgumentException("Only localhost is supported as a redirect URI.", nameof(redirectUri));
67+
}
7668

77-
authorizationUri = newAuthorizationUri;
78-
redirectUri = newRedirectUri;
79-
}
69+
Task<Uri> interceptTask = InterceptRequestsAsync(redirectUri, ct);
8070

81-
private static int GetFreeTcpPort()
82-
{
83-
var listener = new TcpListener(IPAddress.Loopback, 0);
71+
OpenDefaultBrowser(authorizationUri);
8472

85-
try
86-
{
87-
listener.Start();
88-
return ((IPEndPoint) listener.LocalEndpoint).Port;
89-
}
90-
finally
91-
{
92-
listener.Stop();
93-
}
73+
return await interceptTask;
9474
}
9575

9676
private void OpenDefaultBrowser(Uri uri)
@@ -205,5 +185,21 @@ string FormatError(string format)
205185
}
206186
}
207187
}
188+
189+
private static int GetFreeTcpPort()
190+
{
191+
var listener = new TcpListener(IPAddress.Loopback, 0);
192+
193+
try
194+
{
195+
listener.Start();
196+
return ((IPEndPoint) listener.LocalEndpoint).Port;
197+
}
198+
finally
199+
{
200+
listener.Stop();
201+
}
202+
}
203+
208204
}
209205
}

‎src/shared/TestInfrastructure/Objects/TestOAuth2Server.cs‎

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,8 @@ private Task<HttpResponseMessage> OnAuthorizationEndpointAsync(HttpRequestMessag
6868
}
6969

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

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

100101
// Create the auth code grant
101102
OAuth2Application.AuthCodeGrant grant = app.CreateAuthorizationCodeGrant(
102-
TokenGenerator, scopes, codeChallenge, codeChallengeMethod);
103+
TokenGenerator, scopes, redirectUriStr, codeChallenge, codeChallengeMethod);
103104

104105
var respQuery = new Dictionary<string, string>
105106
{
@@ -189,10 +190,12 @@ private async Task<HttpResponseMessage> OnTokenEndpointAsync(HttpRequestMessage
189190
}
190191

191192
formData.TryGetValue(OAuth2Constants.TokenEndpoint.PkceVerifierParameter, out string codeVerifier);
193+
if (formData.TryGetValue(OAuth2Constants.RedirectUriParameter, out string redirectUriStr))
194+
{
195+
app.ValidateRedirect(redirectUriStr);
196+
}
192197

193-
app.ValidateRedirect(formData[OAuth2Constants.RedirectUriParameter]);
194-
195-
tokenResp = app.CreateTokenByAuthorizationGrant(TokenGenerator, authCode, codeVerifier);
198+
tokenResp = app.CreateTokenByAuthorizationGrant(TokenGenerator, authCode, codeVerifier, redirectUriStr);
196199
}
197200
else if (StringComparer.OrdinalIgnoreCase.Equals(grantType, OAuth2Constants.TokenEndpoint.RefreshTokenGrantType))
198201
{
@@ -287,16 +290,18 @@ public class OAuth2Application
287290
{
288291
public class AuthCodeGrant
289292
{
290-
public AuthCodeGrant(string code, string[] scopes,
293+
public AuthCodeGrant(string code, string[] scopes, string redirectUri = null,
291294
string codeChallenge = null, OAuth2PkceChallengeMethod codeChallengeMethod = OAuth2PkceChallengeMethod.Plain)
292295
{
293296
Code = code;
294297
Scopes = scopes;
298+
RedirectUri = redirectUri;
295299
CodeChallenge = codeChallenge;
296300
CodeChallengeMethod = codeChallengeMethod;
297301
}
298302
public string Code { get; }
299303
public string[] Scopes { get; }
304+
public string RedirectUri { get; }
300305
public string CodeChallenge { get; }
301306
public OAuth2PkceChallengeMethod CodeChallengeMethod { get; }
302307
}
@@ -336,11 +341,11 @@ public OAuth2Application(string id)
336341
public IDictionary<string, string[]> RefreshTokens { get; } = new Dictionary<string, string[]>();
337342

338343
public AuthCodeGrant CreateAuthorizationCodeGrant(TestOAuth2ServerTokenGenerator generator,
339-
string[] scopes, string codeChallenge, OAuth2PkceChallengeMethod codeChallengeMethod)
344+
string[] scopes, string redirectUri, string codeChallenge, OAuth2PkceChallengeMethod codeChallengeMethod)
340345
{
341346
string code = generator.CreateAuthorizationCode();
342347

343-
var grant = new AuthCodeGrant(code, scopes, codeChallenge, codeChallengeMethod);
348+
var grant = new AuthCodeGrant(code, scopes, redirectUri, codeChallenge, codeChallengeMethod);
344349
AuthGrants.Add(grant);
345350

346351
return grant;
@@ -387,14 +392,15 @@ public bool IsDeviceCodeGrantApproved(string deviceCode)
387392
}
388393

389394
public TokenEndpointResponseJson CreateTokenByAuthorizationGrant(
390-
TestOAuth2ServerTokenGenerator generator, string authCode, string codeVerifier)
395+
TestOAuth2ServerTokenGenerator generator, string authCode, string codeVerifier, string redirectUri)
391396
{
392397
var grant = AuthGrants.FirstOrDefault(x => x.Code == authCode);
393398
if (grant is null)
394399
{
395400
throw new Exception($"Invalid authorization code '{authCode}'");
396401
}
397402

403+
// Validate the grant's code challenge was constructed from the given code verifier
398404
if (!string.IsNullOrWhiteSpace(grant.CodeChallenge))
399405
{
400406
if (string.IsNullOrWhiteSpace(codeVerifier))
@@ -431,6 +437,13 @@ public TokenEndpointResponseJson CreateTokenByAuthorizationGrant(
431437
}
432438
}
433439

440+
// If an explicit redirect URI was used as part of the authorization request then
441+
// the redirect URI used for the token call must match exactly.
442+
if (!string.IsNullOrWhiteSpace(grant.RedirectUri) && !StringComparer.Ordinal.Equals(grant.RedirectUri, redirectUri))
443+
{
444+
throw new Exception("Redirect URI must match exactly the one used when requesting the authorization code.");
445+
}
446+
434447
string accessToken = generator.CreateAccessToken();
435448
string refreshToken = generator.CreateRefreshToken();
436449

‎src/shared/TestInfrastructure/Objects/TestOAuth2WebBrowser.cs‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,11 @@ public TestOAuth2WebBrowser(HttpMessageHandler httpHandler)
1717
_httpClient = new HttpClient(httpHandler);
1818
}
1919

20+
public Uri UpdateRedirectUri(Uri uri)
21+
{
22+
return uri;
23+
}
24+
2025
public async Task<Uri> GetAuthenticationCodeAsync(Uri authorizationUri, Uri redirectUri, CancellationToken ct)
2126
{
2227
using (var response = await _httpClient.SendAsync(HttpMethod.Get, authorizationUri))

0 commit comments

Comments
 (0)