diff --git a/docs/generic-oauth.md b/docs/generic-oauth.md index 92ad6dc5cc..dbf5c06fbb 100644 --- a/docs/generic-oauth.md +++ b/docs/generic-oauth.md @@ -42,6 +42,7 @@ following values in your Git configuration: - Client Secret (optional) - Redirect URL (optional, defaults to `http://127.0.0.1`) - Scopes (optional) +- Response Mode (optional, defaults to `query`) - OAuth Endpoints - Authorization Endpoint - Token Endpoint @@ -62,6 +63,7 @@ git config --global credential..oauthAuthorizeEndpoint git config --global credential..oauthTokenEndpoint git config --global credential..oauthScopes git config --global credential..oauthDeviceEndpoint +git config --global credential..oauthResponseMode ``` **Example commands:** @@ -83,6 +85,7 @@ git config --global credential..oauthDeviceEndpoint oauthScopes = "code:write profile:read" oauthDefaultUserName = "OAUTH" oauthUseClientAuthHeader = false + oauthResponseMode = "query" ``` ### Additional configuration @@ -90,6 +93,27 @@ git config --global credential..oauthDeviceEndpoint Depending on the specific implementation of OAuth with your Git host you may also need to specify additional behavior. +#### Response mode + +The response mode controls how the authorization server returns the response to +the loopback redirect URI once the user has authenticated. GCM supports the +following values: + +- `query` (default) - parameters are returned in the redirect URI query string. +- `fragment` - parameters are returned in the redirect URI fragment. +- `form_post` - parameters are returned as an auto-submitting HTML form that is + POSTed to the redirect URI, as described by the + [OAuth 2.0 Form Post Response Mode][form-post-spec] specification. + +Most hosts use the default `query` mode. Only set this if your host requires a +specific response mode: + +```shell +git config --global credential..oauthResponseMode +``` + +[form-post-spec]: https://openid.net/specs/oauth-v2-form-post-response-mode-1_0.html + #### Token user name If your Git host requires that you specify a username to use with OAuth tokens diff --git a/src/Atlassian.Bitbucket.Tests/Cloud/BitbucketOAuth2ClientTest.cs b/src/Atlassian.Bitbucket.Tests/Cloud/BitbucketOAuth2ClientTest.cs index fbf6371818..0e889ec302 100644 --- a/src/Atlassian.Bitbucket.Tests/Cloud/BitbucketOAuth2ClientTest.cs +++ b/src/Atlassian.Bitbucket.Tests/Cloud/BitbucketOAuth2ClientTest.cs @@ -36,7 +36,7 @@ public async Task BitbucketOAuth2Client_GetAuthorizationCodeAsync_ReturnsCode() Bitbucket.Cloud.BitbucketOAuth2Client client = GetBitbucketOAuth2Client(); - MockGetAuthenticationCodeAsync(finalCallbackUri, null, client.Scopes); + MockGetAuthenticationResponseAsync(finalCallbackUri, null, client.Scopes); MockCodeGenerator(); @@ -56,7 +56,7 @@ public async Task BitbucketOAuth2Client_GetAuthorizationCodeAsync_RespectsClient Bitbucket.Cloud.BitbucketOAuth2Client client = GetBitbucketOAuth2Client(); - MockGetAuthenticationCodeAsync(finalCallbackUri, clientId, client.Scopes); + MockGetAuthenticationResponseAsync(finalCallbackUri, clientId, client.Scopes); MockCodeGenerator(); @@ -115,7 +115,7 @@ private void MockCodeGenerator() codeGenerator.Setup(c => c.CreatePkceCodeChallenge(OAuth2PkceChallengeMethod.Sha256, pkceCodeVerifier)).Returns(pkceCodeChallenge); } - private void MockGetAuthenticationCodeAsync(Uri finalCallbackUri, string overrideClientId, IEnumerable scopes) + private void MockGetAuthenticationResponseAsync(Uri finalCallbackUri, string overrideClientId, IEnumerable scopes) { var authorizationUri = new UriBuilder(CloudConstants.OAuth2AuthorizationEndpoint) { @@ -128,7 +128,8 @@ private void MockGetAuthenticationCodeAsync(Uri finalCallbackUri, string overrid + "&scope=" + WebUtility.UrlEncode(string.Join(" ", scopes)).ToLower() }.Uri; - browser.Setup(b => b.GetAuthenticationCodeAsync(authorizationUri, rootCallbackUri, ct)).Returns(Task.FromResult(finalCallbackUri)); + browser.Setup(b => b.GetAuthenticationResponseAsync(authorizationUri, rootCallbackUri, OAuth2ResponseMode.Default, ct)) + .Returns(Task.FromResult(finalCallbackUri.GetQueryParameters())); } private Uri MockFinalCallbackUri() diff --git a/src/Atlassian.Bitbucket.Tests/DataCenter/BitbucketOAuth2ClientTest.cs b/src/Atlassian.Bitbucket.Tests/DataCenter/BitbucketOAuth2ClientTest.cs index e2e7225db3..5931a6a0c7 100644 --- a/src/Atlassian.Bitbucket.Tests/DataCenter/BitbucketOAuth2ClientTest.cs +++ b/src/Atlassian.Bitbucket.Tests/DataCenter/BitbucketOAuth2ClientTest.cs @@ -37,7 +37,7 @@ public async Task BitbucketOAuth2Client_GetAuthorizationCodeAsync_ReturnsCode() var client = GetBitbucketOAuth2Client(); - MockGetAuthenticationCodeAsync(remoteUrl, rootCallbackUri, finalCallbackUri, clientId, client.Scopes); + MockGetAuthenticationResponseAsync(remoteUrl, rootCallbackUri, finalCallbackUri, clientId, client.Scopes); MockCodeGenerator(); @@ -58,7 +58,7 @@ public async Task BitbucketOAuth2Client_GetAuthorizationCodeAsync_ReturnsCode_Wh var client = GetBitbucketOAuth2Client(); - MockGetAuthenticationCodeAsync(remoteUrl, new Uri(rootCallbackUrl), finalCallbackUri, clientId, client.Scopes); + MockGetAuthenticationResponseAsync(remoteUrl, new Uri(rootCallbackUrl), finalCallbackUri, clientId, client.Scopes); MockCodeGenerator(); @@ -90,7 +90,7 @@ private void MockCodeGenerator() codeGenerator.Setup(c => c.CreatePkceCodeChallenge(OAuth2PkceChallengeMethod.Sha256, pkceCodeVerifier)).Returns(pkceCodeChallenge); } - private void MockGetAuthenticationCodeAsync(string url, Uri redirectUri, Uri finalCallbackUri, string overrideClientId, IEnumerable scopes) + private void MockGetAuthenticationResponseAsync(string url, Uri redirectUri, Uri finalCallbackUri, string overrideClientId, IEnumerable scopes) { var authorizationUri = new UriBuilder(url + "/rest/oauth2/latest/authorize") { @@ -103,7 +103,8 @@ private void MockGetAuthenticationCodeAsync(string url, Uri redirectUri, Uri fin + "&scope=" + WebUtility.UrlEncode(string.Join(" ", scopes)).ToUpper() }.Uri; - browser.Setup(b => b.GetAuthenticationCodeAsync(authorizationUri, redirectUri, ct)).Returns(Task.FromResult(finalCallbackUri)); + browser.Setup(b => b.GetAuthenticationResponseAsync(authorizationUri, redirectUri, OAuth2ResponseMode.Default, ct)) + .Returns(Task.FromResult(finalCallbackUri.GetQueryParameters())); } private Uri MockFinalCallbackUri(Uri redirectUri) diff --git a/src/Core.Tests/Authentication/OAuth2ClientTests.cs b/src/Core.Tests/Authentication/OAuth2ClientTests.cs index be660b99bb..1ec3eae251 100644 --- a/src/Core.Tests/Authentication/OAuth2ClientTests.cs +++ b/src/Core.Tests/Authentication/OAuth2ClientTests.cs @@ -174,6 +174,89 @@ await Assert.ThrowsAsync(() => client.GetAuthorizationCodeAsync(expectedScopes, browser, extraParams, CancellationToken.None)); } + [Theory] + [InlineData(OAuth2ResponseMode.Query, "query")] + [InlineData(OAuth2ResponseMode.Fragment, "fragment")] + [InlineData(OAuth2ResponseMode.FormPost, "form_post")] + public async Task OAuth2Client_GetAuthorizationCodeAsync_NonDefaultResponseMode_SendsResponseModeParameter( + OAuth2ResponseMode responseMode, string expectedValue) + { + const string expectedAuthCode = "68c39cbd8d"; + + var baseUri = new Uri("https://example.com"); + OAuth2ServerEndpoints endpoints = CreateEndpoints(baseUri); + + var httpHandler = new TestHttpMessageHandler {ThrowOnUnexpectedRequest = true}; + + string[] expectedScopes = {"read", "write", "delete"}; + + OAuth2Application app = CreateTestApplication(); + + var server = new TestOAuth2Server(endpoints); + server.RegisterApplication(app); + server.Bind(httpHandler); + server.TokenGenerator.AuthCodes.Add(expectedAuthCode); + + server.AuthorizationEndpointInvoked += (_, request) => + { + IDictionary actualParams = request.RequestUri.GetQueryParameters(); + Assert.True(actualParams.TryGetValue( + OAuth2Constants.AuthorizationEndpoint.ResponseModeParameter, out string actualMode)); + Assert.Equal(expectedValue, actualMode); + }; + + IOAuth2WebBrowser browser = new TestOAuth2WebBrowser(httpHandler); + + var trace2 = new NullTrace2(); + OAuth2Client client = new OAuth2Client( + new HttpClient(httpHandler), endpoints, TestClientId, trace2, + TestRedirectUri, TestClientSecret, responseMode: responseMode); + + OAuth2AuthorizationCodeResult result = await client.GetAuthorizationCodeAsync( + expectedScopes, browser, null, CancellationToken.None); + + Assert.Equal(expectedAuthCode, result.Code); + } + + [Fact] + public async Task OAuth2Client_GetAuthorizationCodeAsync_DefaultResponseMode_OmitsResponseModeParameter() + { + const string expectedAuthCode = "68c39cbd8d"; + + var baseUri = new Uri("https://example.com"); + OAuth2ServerEndpoints endpoints = CreateEndpoints(baseUri); + + var httpHandler = new TestHttpMessageHandler {ThrowOnUnexpectedRequest = true}; + + string[] expectedScopes = {"read", "write", "delete"}; + + OAuth2Application app = CreateTestApplication(); + + var server = new TestOAuth2Server(endpoints); + server.RegisterApplication(app); + server.Bind(httpHandler); + server.TokenGenerator.AuthCodes.Add(expectedAuthCode); + + server.AuthorizationEndpointInvoked += (_, request) => + { + IDictionary actualParams = request.RequestUri.GetQueryParameters(); + Assert.False(actualParams.ContainsKey( + OAuth2Constants.AuthorizationEndpoint.ResponseModeParameter)); + }; + + IOAuth2WebBrowser browser = new TestOAuth2WebBrowser(httpHandler); + + var trace2 = new NullTrace2(); + OAuth2Client client = new OAuth2Client( + new HttpClient(httpHandler), endpoints, TestClientId, trace2, + TestRedirectUri, TestClientSecret, responseMode: OAuth2ResponseMode.Default); + + OAuth2AuthorizationCodeResult result = await client.GetAuthorizationCodeAsync( + expectedScopes, browser, null, CancellationToken.None); + + Assert.Equal(expectedAuthCode, result.Code); + } + [Fact] public async Task OAuth2Client_GetDeviceCodeAsync() { diff --git a/src/Core.Tests/Authentication/OAuth2ResponseModeTests.cs b/src/Core.Tests/Authentication/OAuth2ResponseModeTests.cs new file mode 100644 index 0000000000..a52bf23508 --- /dev/null +++ b/src/Core.Tests/Authentication/OAuth2ResponseModeTests.cs @@ -0,0 +1,42 @@ +using GitCredentialManager.Authentication.OAuth; +using Xunit; + +namespace GitCredentialManager.Tests.Authentication; + +public class OAuth2ResponseModeTests +{ + [Theory] + [InlineData(OAuth2ResponseMode.Default, null)] + [InlineData(OAuth2ResponseMode.Query, "query")] + [InlineData(OAuth2ResponseMode.Fragment, "fragment")] + [InlineData(OAuth2ResponseMode.FormPost, "form_post")] + public void OAuth2ResponseMode_GetParameterValue(OAuth2ResponseMode mode, string expected) + { + Assert.Equal(expected, mode.GetParameterValue()); + } + + [Theory] + [InlineData("query", OAuth2ResponseMode.Query)] + [InlineData("Query", OAuth2ResponseMode.Query)] + [InlineData("fragment", OAuth2ResponseMode.Fragment)] + [InlineData("FRAGMENT", OAuth2ResponseMode.Fragment)] + [InlineData("form_post", OAuth2ResponseMode.FormPost)] + [InlineData("FORM_POST", OAuth2ResponseMode.FormPost)] + [InlineData("formpost", OAuth2ResponseMode.FormPost)] + public void OAuth2ResponseMode_TryParse_Valid(string value, OAuth2ResponseMode expected) + { + Assert.True(OAuth2ResponseModeExtensions.TryParse(value, out OAuth2ResponseMode actual)); + Assert.Equal(expected, actual); + } + + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData(" ")] + [InlineData("web_message")] + [InlineData("unknown")] + public void OAuth2ResponseMode_TryParse_Invalid_ReturnsFalse(string value) + { + Assert.False(OAuth2ResponseModeExtensions.TryParse(value, out _)); + } +} diff --git a/src/Core.Tests/Authentication/OAuth2SystemWebBrowserTests.cs b/src/Core.Tests/Authentication/OAuth2SystemWebBrowserTests.cs index cea6abe17f..9274845a2f 100644 --- a/src/Core.Tests/Authentication/OAuth2SystemWebBrowserTests.cs +++ b/src/Core.Tests/Authentication/OAuth2SystemWebBrowserTests.cs @@ -63,4 +63,20 @@ public void OAuth2SystemWebBrowser_UpdateRedirectUri_AnyPort(string input) ); Assert.False(actualUri.IsDefaultPort); } + + [Theory] + [InlineData("application/x-www-form-urlencoded", true)] + [InlineData("application/x-www-form-urlencoded; charset=utf-8", true)] + [InlineData("application/x-www-form-urlencoded;charset=UTF-8", true)] + [InlineData("APPLICATION/X-WWW-FORM-URLENCODED", true)] + [InlineData(" application/x-www-form-urlencoded ; charset=utf-8 ", true)] + [InlineData("application/json", false)] + [InlineData("text/plain; charset=utf-8", false)] + [InlineData("multipart/form-data; boundary=----abc", false)] + [InlineData("", false)] + [InlineData(null, false)] + public void OAuth2SystemWebBrowser_IsFormUrlEncoded(string contentType, bool expected) + { + Assert.Equal(expected, OAuth2SystemWebBrowser.IsFormUrlEncoded(contentType)); + } } diff --git a/src/Core.Tests/GenericOAuthConfigTests.cs b/src/Core.Tests/GenericOAuthConfigTests.cs index 8b78984828..5239fd70d6 100644 --- a/src/Core.Tests/GenericOAuthConfigTests.cs +++ b/src/Core.Tests/GenericOAuthConfigTests.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using GitCredentialManager.Authentication.OAuth; using GitCredentialManager.Tests.Objects; using Xunit; @@ -99,5 +100,75 @@ public void GenericOAuthConfig_TryGet_Gitea() Assert.Equal(expectedAuthzEndpoint, config.Endpoints.AuthorizationEndpoint); Assert.Equal(expectedTokenEndpoint, config.Endpoints.TokenEndpoint); } + + [Theory] + [InlineData("query", OAuth2ResponseMode.Query)] + [InlineData("fragment", OAuth2ResponseMode.Fragment)] + [InlineData("form_post", OAuth2ResponseMode.FormPost)] + [InlineData("FORM_POST", OAuth2ResponseMode.FormPost)] + public void GenericOAuthConfig_TryGet_ParsesResponseMode(string value, OAuth2ResponseMode expected) + { + bool result = TryGetWithResponseMode(value, out GenericOAuthConfig config); + + Assert.True(result); + Assert.Equal(expected, config.ResponseMode); + } + + [Fact] + public void GenericOAuthConfig_TryGet_InvalidResponseMode_FallsBackToDefault() + { + bool result = TryGetWithResponseMode("bogus", out GenericOAuthConfig config); + + Assert.True(result); + Assert.Equal(OAuth2ResponseMode.Default, config.ResponseMode); + } + + [Fact] + public void GenericOAuthConfig_TryGet_ResponseModeUnset_UsesDefault() + { + bool result = TryGetWithResponseMode(null, out GenericOAuthConfig config); + + Assert.True(result); + Assert.Equal(OAuth2ResponseMode.Default, config.ResponseMode); + } + + private static bool TryGetWithResponseMode(string responseMode, out GenericOAuthConfig config) + { + const string protocol = "https"; + const string host = "example.com"; + var remoteUri = new Uri($"{protocol}://{host}"); + + string GetKey(string name) => $"{Constants.GitConfiguration.Credential.SectionName}.https://example.com.{name}"; + + var trace = new NullTrace(); + var gitConfig = new TestGitConfiguration + { + Global = + { + [GetKey(Constants.GitConfiguration.Credential.OAuthClientId)] = new[] { "client-id" }, + [GetKey(Constants.GitConfiguration.Credential.OAuthAuthzEndpoint)] = new[] { "/oauth/authorize" }, + [GetKey(Constants.GitConfiguration.Credential.OAuthTokenEndpoint)] = new[] { "/oauth/token" }, + } + }; + + if (responseMode != null) + { + gitConfig.Global[GetKey(Constants.GitConfiguration.Credential.OAuthResponseMode)] = new[] { responseMode }; + } + + var settings = new TestSettings + { + GitConfiguration = gitConfig, + RemoteUri = remoteUri + }; + + var input = new GitRequest(new Dictionary + { + {"protocol", protocol}, + {"host", host}, + }); + + return GenericOAuthConfig.TryGet(trace, settings, input, out config); + } } } diff --git a/src/Core/Authentication/OAuth/IOAuth2WebBrowser.cs b/src/Core/Authentication/OAuth/IOAuth2WebBrowser.cs index a9dbdf519a..72a30de613 100644 --- a/src/Core/Authentication/OAuth/IOAuth2WebBrowser.cs +++ b/src/Core/Authentication/OAuth/IOAuth2WebBrowser.cs @@ -1,5 +1,5 @@ using System; -using System.Net; +using System.Collections.Generic; using System.Threading; using System.Threading.Tasks; @@ -9,6 +9,16 @@ public interface IOAuth2WebBrowser { Uri UpdateRedirectUri(Uri uri); - Task GetAuthenticationCodeAsync(Uri authorizationUri, Uri redirectUri, CancellationToken ct); + /// + /// Drive the user agent through the authorization request and intercept the + /// authorization response delivered to the redirect URI. + /// + /// Authorization request URI to open in the user agent. + /// Redirect URI to intercept the response on. + /// Mechanism the authorization server uses to deliver the response. + /// Token to cancel the operation. + /// The authorization response parameters. + Task> GetAuthenticationResponseAsync( + Uri authorizationUri, Uri redirectUri, OAuth2ResponseMode responseMode, CancellationToken ct); } } diff --git a/src/Core/Authentication/OAuth/OAuth2Client.cs b/src/Core/Authentication/OAuth/OAuth2Client.cs index 199a0c0310..b7fe452cb4 100644 --- a/src/Core/Authentication/OAuth/OAuth2Client.cs +++ b/src/Core/Authentication/OAuth/OAuth2Client.cs @@ -74,6 +74,7 @@ public class OAuth2Client : IOAuth2Client private readonly ITrace2 _trace2; private readonly string _clientSecret; private readonly bool _addAuthHeader; + private readonly OAuth2ResponseMode _responseMode; private IOAuth2CodeGenerator _codeGenerator; @@ -83,7 +84,8 @@ public OAuth2Client(HttpClient httpClient, ITrace2 trace2, Uri redirectUri = null, string clientSecret = null, - bool addAuthHeader = true) + bool addAuthHeader = true, + OAuth2ResponseMode responseMode = OAuth2ResponseMode.Default) { _httpClient = httpClient; _endpoints = endpoints; @@ -92,6 +94,7 @@ public OAuth2Client(HttpClient httpClient, _redirectUri = redirectUri; _clientSecret = clientSecret; _addAuthHeader = addAuthHeader; + _responseMode = responseMode; } public IOAuth2CodeGenerator CodeGenerator @@ -120,6 +123,13 @@ public async Task GetAuthorizationCodeAsync(IEnum [OAuth2Constants.AuthorizationEndpoint.PkceChallengeParameter] = codeChallenge }; + // Only send the parameter when requesting a non-default mode to keep the request unchanged otherwise. + if (_responseMode != OAuth2ResponseMode.Default) + { + queryParams[OAuth2Constants.AuthorizationEndpoint.ResponseModeParameter] = + _responseMode.GetParameterValue(); + } + if (extraQueryParams?.Count > 0) { foreach (var kvp in extraQueryParams) @@ -158,25 +168,27 @@ public async Task GetAuthorizationCodeAsync(IEnum Uri authorizationUri = authorizationUriBuilder.Uri; - // Open the browser at the request URI to start the authorization code grant flow. - Uri finalUri = await browser.GetAuthenticationCodeAsync(authorizationUri, redirectUri, ct); + // Open the browser at the request URI to start the authorization code grant flow, and + // intercept the response parameters delivered to the redirect URI. + IDictionary responseParams = + await browser.GetAuthenticationResponseAsync(authorizationUri, redirectUri, _responseMode, 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 redirectQueryParams = finalUri.GetQueryParameters(); - if (!redirectQueryParams.TryGetValue(OAuth2Constants.AuthorizationGrantResponse.StateParameter, out string replyState)) + if (!responseParams.TryGetValue(OAuth2Constants.AuthorizationGrantResponse.StateParameter, out string replyState)) { - throw new Trace2OAuth2Exception(_trace2, $"Missing '{OAuth2Constants.AuthorizationGrantResponse.StateParameter}' in response."); + throw new Trace2OAuth2Exception(_trace2, + $"Missing '{OAuth2Constants.AuthorizationGrantResponse.StateParameter}' in response."); } if (!StringComparer.Ordinal.Equals(state, replyState)) { throw new Trace2OAuth2Exception(_trace2, - $"Missing '{OAuth2Constants.AuthorizationGrantResponse.StateParameter}' in response."); + $"Invalid '{OAuth2Constants.AuthorizationGrantResponse.StateParameter}' in response; does not match the request."); } // We expect to have the auth code in the response otherwise terminate the flow (we failed authentication for some reason) - if (!redirectQueryParams.TryGetValue(OAuth2Constants.AuthorizationGrantResponse.AuthorizationCodeParameter, out string authCode)) + if (!responseParams.TryGetValue(OAuth2Constants.AuthorizationGrantResponse.AuthorizationCodeParameter, out string authCode)) { throw new Trace2OAuth2Exception(_trace2, $"Missing '{OAuth2Constants.AuthorizationGrantResponse.AuthorizationCodeParameter}' in response."); diff --git a/src/Core/Authentication/OAuth/OAuth2Constants.cs b/src/Core/Authentication/OAuth/OAuth2Constants.cs index 0b96a60476..a1c0ca90a8 100644 --- a/src/Core/Authentication/OAuth/OAuth2Constants.cs +++ b/src/Core/Authentication/OAuth/OAuth2Constants.cs @@ -14,6 +14,10 @@ public static class AuthorizationEndpoint public const string StateParameter = "state"; public const string AuthorizationCodeResponseType = "code"; public const string ResponseTypeParameter = "response_type"; + public const string ResponseModeParameter = "response_mode"; + public const string QueryResponseMode = "query"; + public const string FragmentResponseMode = "fragment"; + public const string FormPostResponseMode = "form_post"; public const string PkceChallengeParameter = "code_challenge"; public const string PkceChallengeMethodParameter = "code_challenge_method"; public const string PkceChallengeMethodPlain = "plain"; diff --git a/src/Core/Authentication/OAuth/OAuth2ResponseMode.cs b/src/Core/Authentication/OAuth/OAuth2ResponseMode.cs new file mode 100644 index 0000000000..2f5ef0f13f --- /dev/null +++ b/src/Core/Authentication/OAuth/OAuth2ResponseMode.cs @@ -0,0 +1,90 @@ +using System; + +namespace GitCredentialManager.Authentication.OAuth; + +/// +/// The mechanism the authorization server uses to return authorization response +/// parameters to the redirect URI. +/// +public enum OAuth2ResponseMode +{ + /// + /// Use the default response mode as determined by the authorization server. + /// + Default = 0, + + /// + /// Parameters are encoded in the query component of the redirect URI. + /// + Query, + + /// + /// Parameters are encoded in the fragment component of the redirect URI. + /// + Fragment, + + /// + /// Parameters are returned as an HTML form that is auto-submitted as an + /// application/x-www-form-urlencoded POST to the redirect URI, as + /// described by the OAuth 2.0 Form Post Response Mode specification. + /// + FormPost, +} + +public static class OAuth2ResponseModeExtensions +{ + /// + /// Get the wire value for the response_mode authorization request parameter. + /// + public static string GetParameterValue(this OAuth2ResponseMode mode) + { + switch (mode) + { + case OAuth2ResponseMode.Default: + return null; + case OAuth2ResponseMode.Query: + return OAuth2Constants.AuthorizationEndpoint.QueryResponseMode; + case OAuth2ResponseMode.Fragment: + return OAuth2Constants.AuthorizationEndpoint.FragmentResponseMode; + case OAuth2ResponseMode.FormPost: + return OAuth2Constants.AuthorizationEndpoint.FormPostResponseMode; + default: + throw new ArgumentOutOfRangeException(nameof(mode), mode, "Unknown OAuth2 response mode."); + } + } + + /// + /// Try to parse a response_mode wire value into an . + /// + public static bool TryParse(string value, out OAuth2ResponseMode mode) + { + mode = OAuth2ResponseMode.Default; + + if (string.IsNullOrWhiteSpace(value)) + { + return false; + } + + if (StringComparer.OrdinalIgnoreCase.Equals(value, OAuth2Constants.AuthorizationEndpoint.QueryResponseMode)) + { + mode = OAuth2ResponseMode.Query; + return true; + } + + if (StringComparer.OrdinalIgnoreCase.Equals(value, OAuth2Constants.AuthorizationEndpoint.FragmentResponseMode)) + { + mode = OAuth2ResponseMode.Fragment; + return true; + } + + // Accept both "form_post" (wire value) and "formpost" for convenience. + if (StringComparer.OrdinalIgnoreCase.Equals(value, OAuth2Constants.AuthorizationEndpoint.FormPostResponseMode) || + StringComparer.OrdinalIgnoreCase.Equals(value, "formpost")) + { + mode = OAuth2ResponseMode.FormPost; + return true; + } + + return false; + } +} diff --git a/src/Core/Authentication/OAuth/OAuth2SystemWebBrowser.cs b/src/Core/Authentication/OAuth/OAuth2SystemWebBrowser.cs index 05843f9df2..4f55072a47 100644 --- a/src/Core/Authentication/OAuth/OAuth2SystemWebBrowser.cs +++ b/src/Core/Authentication/OAuth/OAuth2SystemWebBrowser.cs @@ -1,7 +1,9 @@ using System; using System.Collections.Generic; +using System.IO; using System.Net; using System.Net.Sockets; +using System.Text; using System.Threading; using System.Threading.Tasks; @@ -36,6 +38,34 @@ public class OAuth2WebBrowserOptions public class OAuth2SystemWebBrowser : IOAuth2WebBrowser { + // Served during the fragment response flow. The authorization parameters live in the + // URI fragment, which user agents do not transmit to the server, so we reissue them as + // a form POST to the redirect URI - keeping them out of the URL (and thus out of + // browser history and server logs) and letting the listener read them from the body. + private const string FragmentFormPostHtml = @" +Authenticating... +
"; + private readonly ISessionManager _sessionManager; private readonly OAuth2WebBrowserOptions _options; @@ -65,26 +95,28 @@ public Uri UpdateRedirectUri(Uri uri) return uri; } - public async Task GetAuthenticationCodeAsync(Uri authorizationUri, Uri redirectUri, CancellationToken ct) + public async Task> GetAuthenticationResponseAsync( + Uri authorizationUri, Uri redirectUri, OAuth2ResponseMode responseMode, CancellationToken ct) { if (!redirectUri.IsLoopback) { throw new ArgumentException("Only localhost is supported as a redirect URI.", nameof(redirectUri)); } - Task interceptTask = InterceptRequestsAsync(redirectUri, ct); + Task> interceptTask = InterceptRequestsAsync(redirectUri, responseMode, ct); _sessionManager.OpenBrowser(authorizationUri); return await interceptTask; } - private async Task InterceptRequestsAsync(Uri listenUri, CancellationToken ct) + private async Task> InterceptRequestsAsync( + Uri listenUri, OAuth2ResponseMode responseMode, CancellationToken ct) { // Create a TaskCompletionSource which completes when we're asked to cancel. - // We can then await the this task together with other tasks that don't take a + // We can then await this task together with other tasks that don't take a // CancellationToken and exit the method quickly when cancelled. - var tcs = new TaskCompletionSource(); + var tcs = new TaskCompletionSource>(); ct.Register(() => tcs.SetCanceled()); // Prefixes must end with a '/' @@ -99,25 +131,40 @@ private async Task InterceptRequestsAsync(Uri listenUri, CancellationToken try { - Task contextTask = listener.GetContextAsync(); - Task cancelTask = tcs.Task; + while (true) + { + Task contextTask = listener.GetContextAsync(); + Task> cancelTask = tcs.Task; - Task completedTask = await Task.WhenAny(contextTask, tcs.Task); + Task completedTask = await Task.WhenAny(contextTask, cancelTask); - // Check if we 'completed' the context task or the cancellation task - if (completedTask == cancelTask) - { - // We were cancelled! - return await cancelTask; - } + // Check if we 'completed' the context task or the cancellation task + if (completedTask == cancelTask) + { + // We were cancelled! + return await cancelTask; + } + + // We intercepted a request! + HttpListenerContext context = await contextTask; - // We intercepted a request! - HttpListenerContext context = await contextTask; + IDictionary parameters = await GetResponseParametersAsync(context.Request); - await HandleInterceptedRequestAsync(context.Request, context.Response); + // In fragment mode the authorization parameters are in the URI fragment, which + // user agents do not send to the server. The first leg is therefore a parameterless + // GET; reply with a script that reissues the parameters as a form POST so we can + // read them from the body on the next iteration. + if (responseMode == OAuth2ResponseMode.Fragment && parameters.Count == 0) + { + await context.Response.WriteResponseAsync(FragmentFormPostHtml); + context.Response.Close(); + continue; + } - // Return the final intercepted URI - return context.Request.Url; + await WriteFinalResponseAsync(context.Response, parameters); + + return parameters; + } } finally { @@ -126,14 +173,41 @@ private async Task InterceptRequestsAsync(Uri listenUri, CancellationToken } } - private async Task HandleInterceptedRequestAsync(HttpListenerRequest request, HttpListenerResponse response) + private static async Task> GetResponseParametersAsync(HttpListenerRequest request) + { + // Form post responses - and the form POST used to forward fragment responses - carry + // the authorization parameters in the urlencoded request body. + if (StringComparer.OrdinalIgnoreCase.Equals(request.HttpMethod, Constants.Http.MethodPost) && + IsFormUrlEncoded(request.ContentType)) + { + using var reader = new StreamReader(request.InputStream, request.ContentEncoding ?? Encoding.UTF8); + string body = await reader.ReadToEndAsync(); + return UriExtensions.ParseQueryString(body); + } + + // Query responses carry the parameters in the request query string. + return request.QueryString.ToDictionary(StringComparer.OrdinalIgnoreCase); + } + + internal static bool IsFormUrlEncoded(string contentType) { - IDictionary queryParams = request.QueryString.ToDictionary(StringComparer.OrdinalIgnoreCase); + if (string.IsNullOrEmpty(contentType)) + { + return false; + } + + // Compare only the media type, ignoring any parameters such as "; charset=utf-8". + // The media type is everything up to the first ';'. + string mediaType = contentType.Split(';')[0].Trim(); + return StringComparer.OrdinalIgnoreCase.Equals(mediaType, Constants.Http.MimeTypeFormUrlEncoded); + } + private async Task WriteFinalResponseAsync(HttpListenerResponse response, IDictionary parameters) + { // If we have an error value then the request failed and we should reply with a page containing the error information - bool hasError = queryParams.TryGetValue(OAuth2Constants.AuthorizationGrantResponse.ErrorCodeParameter, out string errorCode); - queryParams.TryGetValue(OAuth2Constants.AuthorizationGrantResponse.ErrorDescriptionParameter, out string errorDescription); - queryParams.TryGetValue(OAuth2Constants.AuthorizationGrantResponse.ErrorUriParameter, out string errorUri); + bool hasError = parameters.TryGetValue(OAuth2Constants.AuthorizationGrantResponse.ErrorCodeParameter, out string errorCode); + parameters.TryGetValue(OAuth2Constants.AuthorizationGrantResponse.ErrorDescriptionParameter, out string errorDescription); + parameters.TryGetValue(OAuth2Constants.AuthorizationGrantResponse.ErrorUriParameter, out string errorUri); if (hasError) { string FormatError(string format) diff --git a/src/Core/Constants.cs b/src/Core/Constants.cs index 12d66ef57a..667ff8b0ba 100644 --- a/src/Core/Constants.cs +++ b/src/Core/Constants.cs @@ -156,6 +156,7 @@ public static class EnvironmentVariables public const string OAuthDeviceEndpoint = "GCM_OAUTH_DEVICE_ENDPOINT"; public const string OAuthClientAuthHeader = "GCM_OAUTH_USE_CLIENT_AUTH_HEADER"; public const string OAuthDefaultUserName = "GCM_OAUTH_DEFAULT_USERNAME"; + public const string OAuthResponseMode = "GCM_OAUTH_RESPONSE_MODE"; public const string GcmDevUseLegacyUiHelpers = "GCM_DEV_USELEGACYUIHELPERS"; public const string GcmGuiSoftwareRendering = "GCM_GUI_SOFTWARE_RENDERING"; public const string GcmAllowUnsafeRemotes = "GCM_ALLOW_UNSAFE_REMOTES"; @@ -172,6 +173,10 @@ public static class Http public const string WwwAuthenticateNtlmScheme = "NTLM"; public const string MimeTypeJson = "application/json"; + public const string MimeTypeFormUrlEncoded = "application/x-www-form-urlencoded"; + + public const string MethodGet = "GET"; + public const string MethodPost = "POST"; } public static class GitConfiguration @@ -218,6 +223,7 @@ public static class Credential public const string OAuthDeviceEndpoint = "oauthDeviceEndpoint"; public const string OAuthClientAuthHeader = "oauthUseClientAuthHeader"; public const string OAuthDefaultUserName = "oauthDefaultUserName"; + public const string OAuthResponseMode = "oauthResponseMode"; } public static class Http diff --git a/src/Core/GenericHostProvider.cs b/src/Core/GenericHostProvider.cs index b5d5012bcb..ab17405b69 100644 --- a/src/Core/GenericHostProvider.cs +++ b/src/Core/GenericHostProvider.cs @@ -276,7 +276,8 @@ private async Task GetOAuthAccessToken(Uri remoteUri, string userNa trace2, config.RedirectUri, config.ClientSecret, - config.UseAuthHeader); + config.UseAuthHeader, + config.ResponseMode); // // Prepend "refresh_token" to the hostname to get a (hopefully) unique service name that diff --git a/src/Core/GenericOAuthConfig.cs b/src/Core/GenericOAuthConfig.cs index ff95a4a0d1..ac40e79b24 100644 --- a/src/Core/GenericOAuthConfig.cs +++ b/src/Core/GenericOAuthConfig.cs @@ -134,6 +134,23 @@ public static bool TryGet(ITrace trace, ISettings settings, GitRequest request, config.UseAuthHeader = true; } + // Response mode is optional and defaults to 'query' + if (settings.TryGetSetting( + Constants.EnvironmentVariables.OAuthResponseMode, + Constants.GitConfiguration.Credential.SectionName, + Constants.GitConfiguration.Credential.OAuthResponseMode, + out string responseModeStr) && !string.IsNullOrWhiteSpace(responseModeStr)) + { + if (OAuth2ResponseModeExtensions.TryParse(responseModeStr, out OAuth2ResponseMode responseMode)) + { + config.ResponseMode = responseMode; + } + else + { + trace.WriteLine($"Invalid OAuth configuration - unknown response mode '{responseModeStr}'; using default"); + } + } + config.DefaultUserName = settings.TryGetSetting( Constants.EnvironmentVariables.OAuthDefaultUserName, Constants.GitConfiguration.Credential.SectionName, @@ -152,6 +169,7 @@ public static bool TryGet(ITrace trace, ISettings settings, GitRequest request, public Uri RedirectUri { get; set; } public string[] Scopes { get; set; } public bool UseAuthHeader { get; set; } + public OAuth2ResponseMode ResponseMode { get; set; } public string DefaultUserName { get; set; } public bool SupportsDeviceCode => Endpoints.DeviceAuthorizationEndpoint != null; diff --git a/src/Core/ISessionManager.cs b/src/Core/ISessionManager.cs index c61394749c..8ee291f300 100644 --- a/src/Core/ISessionManager.cs +++ b/src/Core/ISessionManager.cs @@ -67,6 +67,12 @@ public void OpenBrowser(Uri uri) throw new ArgumentException("Can only open HTTP/HTTPS URIs", nameof(uri)); } + // Important! Use AbsoluteUri to ensure that the URL is properly + // escaped (e.g. spaces are converted to %20). + // The 'shell execute' handler on some operating systems (e.g. macOS) + // will try to validate the URL handed to it and if it sees any + // unescaped characters it will decide that the rest of the query + // parameters also need esacaping leading to double escaping! OpenBrowserInternal(uri.AbsoluteUri); } diff --git a/src/TestInfrastructure/Objects/TestOAuth2WebBrowser.cs b/src/TestInfrastructure/Objects/TestOAuth2WebBrowser.cs index 547aaf360b..86f011cddc 100644 --- a/src/TestInfrastructure/Objects/TestOAuth2WebBrowser.cs +++ b/src/TestInfrastructure/Objects/TestOAuth2WebBrowser.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Generic; using System.Net.Http; using System.Threading; using System.Threading.Tasks; @@ -20,12 +21,13 @@ public Uri UpdateRedirectUri(Uri uri) return uri; } - public async Task GetAuthenticationCodeAsync(Uri authorizationUri, Uri redirectUri, CancellationToken ct) + public async Task> GetAuthenticationResponseAsync( + Uri authorizationUri, Uri redirectUri, OAuth2ResponseMode responseMode, CancellationToken ct) { using (var response = await _httpClient.SendAsync(HttpMethod.Get, authorizationUri)) { response.EnsureSuccessStatusCode(); - return response.Headers.Location; + return response.Headers.Location.GetQueryParameters(); } } }