From 7b16a1f0e736bd936db3093e12bac1caa275904d Mon Sep 17 00:00:00 2001 From: Matthew John Cheetham Date: Wed, 17 Jun 2026 17:17:00 +0100 Subject: [PATCH 1/3] oauth: support non-query response modes The authorization code flow only handled the default 'query' response mode, where the loopback browser reads the response from the request query string and returns a URI for the client to parse. The 'fragment' and 'form_post' modes deliver the response over channels a URI cannot represent - the fragment is never transmitted to the server, and form_post arrives as a urlencoded POST body - so hosts that mandate those modes could not be used. Have the browser return the parsed response parameters regardless of transport and tell it which mode to expect. The system browser reads the POST body for form_post, and for fragment serves a small page that re-submits the parameters as a form POST to the loopback redirect - keeping the authorization code out of the URL, browser history, and server logs. The client sends 'response_mode' only when it is not the default, so existing query-mode requests are unchanged. Assisted-by: Claude Opus 4.8 Signed-off-by: Matthew John Cheetham --- .../Cloud/BitbucketOAuth2ClientTest.cs | 9 +- .../DataCenter/BitbucketOAuth2ClientTest.cs | 9 +- .../Authentication/OAuth2ClientTests.cs | 83 ++++++++++++ .../Authentication/OAuth2ResponseModeTests.cs | 42 ++++++ .../OAuth2SystemWebBrowserTests.cs | 16 +++ .../Authentication/OAuth/IOAuth2WebBrowser.cs | 14 +- src/Core/Authentication/OAuth/OAuth2Client.cs | 28 ++-- .../Authentication/OAuth/OAuth2Constants.cs | 4 + .../OAuth/OAuth2ResponseMode.cs | 90 +++++++++++++ .../OAuth/OAuth2SystemWebBrowser.cs | 122 ++++++++++++++---- src/Core/Constants.cs | 4 + .../Objects/TestOAuth2WebBrowser.cs | 6 +- 12 files changed, 383 insertions(+), 44 deletions(-) create mode 100644 src/Core.Tests/Authentication/OAuth2ResponseModeTests.cs create mode 100644 src/Core/Authentication/OAuth/OAuth2ResponseMode.cs 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/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..8054597ab8 100644 --- a/src/Core/Constants.cs +++ b/src/Core/Constants.cs @@ -172,6 +172,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 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(); } } } From 1c0b9109546c5e98fb5430efc891f7f95002faa2 Mon Sep 17 00:00:00 2001 From: Matthew John Cheetham Date: Thu, 18 Jun 2026 08:37:18 +0100 Subject: [PATCH 2/3] generic-oauth: add response mode setting Now that the OAuth client can request non-query response modes, expose the choice to generic host configurations through a new optional setting (credential..oauthResponseMode, or the GCM_OAUTH_RESPONSE_MODE environment variable). The built-in providers target known hosts that use 'query', so the generic provider is the only place an arbitrary host's response mode needs to be configurable. The setting is optional and defaults to 'query', so existing configurations are unaffected. An unrecognised value is traced and falls back to the default rather than failing configuration outright. Assisted-by: Claude Opus 4.8 Signed-off-by: Matthew John Cheetham --- docs/generic-oauth.md | 24 ++++++++ src/Core.Tests/GenericOAuthConfigTests.cs | 71 +++++++++++++++++++++++ src/Core/Constants.cs | 2 + src/Core/GenericHostProvider.cs | 3 +- src/Core/GenericOAuthConfig.cs | 18 ++++++ 5 files changed, 117 insertions(+), 1 deletion(-) 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/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/Constants.cs b/src/Core/Constants.cs index 8054597ab8..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"; @@ -222,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; From 329da7f830993d24f4f2fcfeddbbc779f1214b4e Mon Sep 17 00:00:00 2001 From: Matthew John Cheetham Date: Mon, 6 Jul 2026 18:48:36 +0100 Subject: [PATCH 3/3] browser: open AbsoluteUri to avoid double-escaping GCM launches the system browser for interactive OAuth by handing the authorization URL to the OS "shell execute" handler. On macOS that is /usr/bin/open, which validates the URL and, on finding any character that is not legal in a fully percent-encoded URL, re-encodes the whole query string. That step double-escapes parameters we had already encoded -- redirect_uri=http%3A%2F%2F... becomes redirect_uri=http%253A%252F%252F... -- and the authorization server rejects the redirect. Windows ShellExecuteEx forwards the string verbatim, so only macOS is affected. The trigger was a raw space in the query. Uri.ToString() is a display form that unescapes %20 back to a literal space (while leaving %2F alone), so building the launch string that way reintroduced spaces, most easily via the space-delimited scope parameter. This surfaced after MSAL began encoding spaces[1] as %20 rather than +; a literal + is left untouched by ToString(), which had masked the problem. Uri.AbsoluteUri keeps the query fully percent-encoded, so %20 stays %20 and macOS open accepts the URL unchanged. [1]: https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/pull/5128 Assisted-by: Claude Opus 4.8 Signed-off-by: Matthew John Cheetham --- src/Core/ISessionManager.cs | 6 ++++++ 1 file changed, 6 insertions(+) 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); }