From 4fab8891204be73bb21aa122bc0b2d276af62dbd Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 16 Feb 2026 04:10:49 +0000 Subject: [PATCH 1/4] Initial plan From 3bf14235d2b1243ae0edcf2a21fce3b75dd9e774 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 16 Feb 2026 04:15:38 +0000 Subject: [PATCH 2/4] Refactor OAuth2 configuration retrieval to eliminate code duplication Co-authored-by: mdellison90-stack <230609064+mdellison90-stack@users.noreply.github.com> --- .../Cloud/BitbucketOAuth2Client.cs | 39 +++------ .../DataCenter/BitbucketOAuth2Client.cs | 39 +++------ .../OAuth/OAuth2SettingsHelper.cs | 84 +++++++++++++++++++ src/shared/GitHub/GitHubOAuth2Client.cs | 43 ++++------ src/shared/GitLab/GitLabOAuth2Client.cs | 40 +++------ 5 files changed, 135 insertions(+), 110 deletions(-) create mode 100644 src/shared/Core/Authentication/OAuth/OAuth2SettingsHelper.cs diff --git a/src/shared/Atlassian.Bitbucket/Cloud/BitbucketOAuth2Client.cs b/src/shared/Atlassian.Bitbucket/Cloud/BitbucketOAuth2Client.cs index 4b5edbbf7..dfa34a32f 100644 --- a/src/shared/Atlassian.Bitbucket/Cloud/BitbucketOAuth2Client.cs +++ b/src/shared/Atlassian.Bitbucket/Cloud/BitbucketOAuth2Client.cs @@ -23,44 +23,29 @@ public BitbucketOAuth2Client(HttpClient httpClient, ISettings settings, ITrace2 private static string GetClientId(ISettings settings) { - // Check for developer override value - if (settings.TryGetSetting( + return settings.GetOAuthConfigValue( CloudConstants.EnvironmentVariables.OAuthClientId, - Constants.GitConfiguration.Credential.SectionName, CloudConstants.GitConfiguration.Credential.OAuthClientId, - out string clientId)) - { - return clientId; - } - - return CloudConstants.OAuth2ClientId; + Constants.GitConfiguration.Credential.SectionName, + CloudConstants.GitConfiguration.Credential.OAuthClientId, + CloudConstants.OAuth2ClientId); } private static Uri GetRedirectUri(ISettings settings) { - // Check for developer override value - if (settings.TryGetSetting( + return settings.GetOAuthConfigUri( CloudConstants.EnvironmentVariables.OAuthRedirectUri, - Constants.GitConfiguration.Credential.SectionName, CloudConstants.GitConfiguration.Credential.OAuthRedirectUri, - out string redirectUriStr) && Uri.TryCreate(redirectUriStr, UriKind.Absolute, out Uri redirectUri)) - { - return redirectUri; - } - - return CloudConstants.OAuth2RedirectUri; + Constants.GitConfiguration.Credential.SectionName, + CloudConstants.GitConfiguration.Credential.OAuthRedirectUri, + CloudConstants.OAuth2RedirectUri); } private static string GetClientSecret(ISettings settings) { - // Check for developer override value - if (settings.TryGetSetting( + return settings.GetOAuthConfigValue( CloudConstants.EnvironmentVariables.OAuthClientSecret, - Constants.GitConfiguration.Credential.SectionName, CloudConstants.GitConfiguration.Credential.OAuthClientSecret, - out string clientSecret)) - { - return clientSecret; - } - - return CloudConstants.OAuth2ClientSecret; + Constants.GitConfiguration.Credential.SectionName, + CloudConstants.GitConfiguration.Credential.OAuthClientSecret, + CloudConstants.OAuth2ClientSecret); } private static OAuth2ServerEndpoints GetEndpoints() diff --git a/src/shared/Atlassian.Bitbucket/DataCenter/BitbucketOAuth2Client.cs b/src/shared/Atlassian.Bitbucket/DataCenter/BitbucketOAuth2Client.cs index 97abd533c..d50326cf7 100644 --- a/src/shared/Atlassian.Bitbucket/DataCenter/BitbucketOAuth2Client.cs +++ b/src/shared/Atlassian.Bitbucket/DataCenter/BitbucketOAuth2Client.cs @@ -26,44 +26,29 @@ public BitbucketOAuth2Client(HttpClient httpClient, ISettings settings, ITrace2 private static string GetClientId(ISettings settings) { - // Check for developer override value - if (settings.TryGetSetting( + return settings.GetRequiredOAuthConfigValue( DataCenterConstants.EnvironmentVariables.OAuthClientId, - Constants.GitConfiguration.Credential.SectionName, DataCenterConstants.GitConfiguration.Credential.OAuthClientId, - out string clientId)) - { - return clientId; - } - - throw new ArgumentException("Bitbucket DC OAuth Client ID must be defined"); + Constants.GitConfiguration.Credential.SectionName, + DataCenterConstants.GitConfiguration.Credential.OAuthClientId, + "Bitbucket DC OAuth Client ID must be defined"); } private static Uri GetRedirectUri(ISettings settings) { - // Check for developer override value - if (settings.TryGetSetting( + return settings.GetOAuthConfigUri( DataCenterConstants.EnvironmentVariables.OAuthRedirectUri, - Constants.GitConfiguration.Credential.SectionName, DataCenterConstants.GitConfiguration.Credential.OAuthRedirectUri, - out string redirectUriStr) && Uri.TryCreate(redirectUriStr, UriKind.Absolute, out Uri redirectUri)) - { - return redirectUri; - } - - return DataCenterConstants.OAuth2RedirectUri; + Constants.GitConfiguration.Credential.SectionName, + DataCenterConstants.GitConfiguration.Credential.OAuthRedirectUri, + DataCenterConstants.OAuth2RedirectUri); } private static string GetClientSecret(ISettings settings) { - // Check for developer override value - if (settings.TryGetSetting( + return settings.GetRequiredOAuthConfigValue( DataCenterConstants.EnvironmentVariables.OAuthClientSecret, - Constants.GitConfiguration.Credential.SectionName, DataCenterConstants.GitConfiguration.Credential.OAuthClientSecret, - out string clientSecret)) - { - return clientSecret; - } - - throw new ArgumentException("Bitbucket DC OAuth Client Secret must be defined"); + Constants.GitConfiguration.Credential.SectionName, + DataCenterConstants.GitConfiguration.Credential.OAuthClientSecret, + "Bitbucket DC OAuth Client Secret must be defined"); } private static OAuth2ServerEndpoints GetEndpoints(ISettings settings) diff --git a/src/shared/Core/Authentication/OAuth/OAuth2SettingsHelper.cs b/src/shared/Core/Authentication/OAuth/OAuth2SettingsHelper.cs new file mode 100644 index 000000000..42e78eda2 --- /dev/null +++ b/src/shared/Core/Authentication/OAuth/OAuth2SettingsHelper.cs @@ -0,0 +1,84 @@ +using System; + +namespace GitCredentialManager.Authentication.OAuth +{ + /// + /// Helper class for retrieving OAuth2 configuration settings from environment variables or Git configuration. + /// + public static class OAuth2SettingsHelper + { + /// + /// Retrieves an OAuth configuration value from settings, with fallback to a default value. + /// + /// The settings instance to query. + /// The environment variable name. + /// The Git configuration section name. + /// The Git configuration property name. + /// The default value to return if no setting is found. + /// The configured value if found, otherwise the default value. + public static string GetOAuthConfigValue( + this ISettings settings, + string environmentVariable, + string configSection, + string configProperty, + string defaultValue) + { + if (settings.TryGetSetting(environmentVariable, configSection, configProperty, out string value)) + { + return value; + } + + return defaultValue; + } + + /// + /// Retrieves a required OAuth configuration value from settings, throwing an exception if not found. + /// + /// The settings instance to query. + /// The environment variable name. + /// The Git configuration section name. + /// The Git configuration property name. + /// The error message to include in the exception if the value is not found. + /// The configured value. + /// Thrown when the required value is not found. + public static string GetRequiredOAuthConfigValue( + this ISettings settings, + string environmentVariable, + string configSection, + string configProperty, + string errorMessage) + { + if (settings.TryGetSetting(environmentVariable, configSection, configProperty, out string value)) + { + return value; + } + + throw new ArgumentException(errorMessage); + } + + /// + /// Retrieves an OAuth configuration URI from settings, with fallback to a default value. + /// + /// The settings instance to query. + /// The environment variable name. + /// The Git configuration section name. + /// The Git configuration property name. + /// The default URI to return if no setting is found. + /// The configured URI if found and valid, otherwise the default URI. + public static Uri GetOAuthConfigUri( + this ISettings settings, + string environmentVariable, + string configSection, + string configProperty, + Uri defaultValue) + { + if (settings.TryGetSetting(environmentVariable, configSection, configProperty, out string uriStr) && + Uri.TryCreate(uriStr, UriKind.Absolute, out Uri uri)) + { + return uri; + } + + return defaultValue; + } + } +} diff --git a/src/shared/GitHub/GitHubOAuth2Client.cs b/src/shared/GitHub/GitHubOAuth2Client.cs index 2eb4aae88..26e388493 100644 --- a/src/shared/GitHub/GitHubOAuth2Client.cs +++ b/src/shared/GitHub/GitHubOAuth2Client.cs @@ -28,47 +28,34 @@ private static OAuth2ServerEndpoints CreateEndpoints(Uri uri) private static string GetClientId(ISettings settings) { - // Check for developer override value - if (settings.TryGetSetting( + return settings.GetOAuthConfigValue( GitHubConstants.EnvironmentVariables.DevOAuthClientId, - Constants.GitConfiguration.Credential.SectionName, GitHubConstants.GitConfiguration.Credential.DevOAuthClientId, - out string clientId)) - { - return clientId; - } - - return GitHubConstants.OAuthClientId; + Constants.GitConfiguration.Credential.SectionName, + GitHubConstants.GitConfiguration.Credential.DevOAuthClientId, + GitHubConstants.OAuthClientId); } private static Uri GetRedirectUri(ISettings settings, Uri targetUri) { - // Check for developer override value - if (settings.TryGetSetting( - GitHubConstants.EnvironmentVariables.DevOAuthRedirectUri, - Constants.GitConfiguration.Credential.SectionName, GitHubConstants.GitConfiguration.Credential.DevOAuthRedirectUri, - out string redirectUriStr) && Uri.TryCreate(redirectUriStr, UriKind.Absolute, out Uri redirectUri)) - { - return redirectUri; - } - // Only GitHub.com supports the new OAuth redirect URI today - return GitHubHostProvider.IsGitHubDotCom(targetUri) + Uri defaultUri = GitHubHostProvider.IsGitHubDotCom(targetUri) ? GitHubConstants.OAuthRedirectUri : GitHubConstants.OAuthLegacyRedirectUri; + + return settings.GetOAuthConfigUri( + GitHubConstants.EnvironmentVariables.DevOAuthRedirectUri, + Constants.GitConfiguration.Credential.SectionName, + GitHubConstants.GitConfiguration.Credential.DevOAuthRedirectUri, + defaultUri); } private static string GetClientSecret(ISettings settings) { - // Check for developer override value - if (settings.TryGetSetting( + return settings.GetOAuthConfigValue( GitHubConstants.EnvironmentVariables.DevOAuthClientSecret, - Constants.GitConfiguration.Credential.SectionName, GitHubConstants.GitConfiguration.Credential.DevOAuthClientSecret, - out string clientSecret)) - { - return clientSecret; - } - - return GitHubConstants.OAuthClientSecret; + Constants.GitConfiguration.Credential.SectionName, + GitHubConstants.GitConfiguration.Credential.DevOAuthClientSecret, + GitHubConstants.OAuthClientSecret); } } } diff --git a/src/shared/GitLab/GitLabOAuth2Client.cs b/src/shared/GitLab/GitLabOAuth2Client.cs index ba72f5b41..1b4630a81 100644 --- a/src/shared/GitLab/GitLabOAuth2Client.cs +++ b/src/shared/GitLab/GitLabOAuth2Client.cs @@ -22,45 +22,29 @@ private static OAuth2ServerEndpoints CreateEndpoints(Uri baseUri) private static Uri GetRedirectUri(ISettings settings) { - // Check for developer override value - if (settings.TryGetSetting( + return settings.GetOAuthConfigUri( GitLabConstants.EnvironmentVariables.DevOAuthRedirectUri, - Constants.GitConfiguration.Credential.SectionName, GitLabConstants.GitConfiguration.Credential.DevOAuthRedirectUri, - out string redirectUriStr) && Uri.TryCreate(redirectUriStr, UriKind.Absolute, out Uri redirectUri)) - { - return redirectUri; - } - - return GitLabConstants.OAuthRedirectUri; + Constants.GitConfiguration.Credential.SectionName, + GitLabConstants.GitConfiguration.Credential.DevOAuthRedirectUri, + GitLabConstants.OAuthRedirectUri); } internal static string GetClientId(ISettings settings) { - // Check for developer override value - if (settings.TryGetSetting( + return settings.GetOAuthConfigValue( GitLabConstants.EnvironmentVariables.DevOAuthClientId, - Constants.GitConfiguration.Credential.SectionName, GitLabConstants.GitConfiguration.Credential.DevOAuthClientId, - out string clientId)) - { - return clientId; - } - - return GitLabConstants.OAuthClientId; + Constants.GitConfiguration.Credential.SectionName, + GitLabConstants.GitConfiguration.Credential.DevOAuthClientId, + GitLabConstants.OAuthClientId); } private static string GetClientSecret(ISettings settings) { - // Check for developer override value - if (settings.TryGetSetting( + return settings.GetOAuthConfigValue( GitLabConstants.EnvironmentVariables.DevOAuthClientSecret, - Constants.GitConfiguration.Credential.SectionName, GitLabConstants.GitConfiguration.Credential.DevOAuthClientSecret, - out string clientSecret)) - { - return clientSecret; - } - - // no secret necessary - return null; + Constants.GitConfiguration.Credential.SectionName, + GitLabConstants.GitConfiguration.Credential.DevOAuthClientSecret, + null); // no secret necessary } } } From 8db5b6378287eee55171a503ee946f121ba36854 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 16 Feb 2026 04:18:30 +0000 Subject: [PATCH 3/4] Add JsonHelper to eliminate duplicated JSON serialization options Co-authored-by: mdellison90-stack <230609064+mdellison90-stack@users.noreply.github.com> --- .../BitbucketTokenEndpointResponseJsonTest.cs | 7 ++--- .../Cloud/UserInfoTest.cs | 6 ++-- .../Cloud/BitbucketRestApi.cs | 6 +--- .../DataCenter/BitbucketRestApi.cs | 7 +---- .../TokenEndpointResponseJsonTest.cs | 7 ++--- .../Core/Authentication/OAuth/OAuth2Client.cs | 5 +--- src/shared/Core/JsonHelper.cs | 29 +++++++++++++++++++ 7 files changed, 38 insertions(+), 29 deletions(-) create mode 100644 src/shared/Core/JsonHelper.cs diff --git a/src/shared/Atlassian.Bitbucket.Tests/BitbucketTokenEndpointResponseJsonTest.cs b/src/shared/Atlassian.Bitbucket.Tests/BitbucketTokenEndpointResponseJsonTest.cs index f9ac14b9a..b3a720334 100644 --- a/src/shared/Atlassian.Bitbucket.Tests/BitbucketTokenEndpointResponseJsonTest.cs +++ b/src/shared/Atlassian.Bitbucket.Tests/BitbucketTokenEndpointResponseJsonTest.cs @@ -1,5 +1,6 @@ using System; using System.Text.Json; +using GitCredentialManager; using Xunit; namespace Atlassian.Bitbucket.Tests @@ -17,11 +18,7 @@ public void BitbucketTokenEndpointResponseJson_Deserialize_Uses_Scopes() var json = $"{{\"access_token\": \"{accessToken}\", \"token_type\": \"{tokenType}\", \"expires_in\": {expiresIn}, \"scopes\": \"{scopesString}\", \"scope\": \"{scopeString}\"}}"; - var result = JsonSerializer.Deserialize(json, - new JsonSerializerOptions - { - PropertyNameCaseInsensitive = true - }); + var result = JsonSerializer.Deserialize(json, JsonHelper.CaseInsensitiveOptions); Assert.Equal(accessToken, result.AccessToken); Assert.Equal(tokenType, result.TokenType); diff --git a/src/shared/Atlassian.Bitbucket.Tests/Cloud/UserInfoTest.cs b/src/shared/Atlassian.Bitbucket.Tests/Cloud/UserInfoTest.cs index 1fa039898..9dacf0c5b 100644 --- a/src/shared/Atlassian.Bitbucket.Tests/Cloud/UserInfoTest.cs +++ b/src/shared/Atlassian.Bitbucket.Tests/Cloud/UserInfoTest.cs @@ -3,6 +3,7 @@ using System.Text.Json.Serialization; using System.Threading.Tasks; using Atlassian.Bitbucket.Cloud; +using GitCredentialManager; using Xunit; namespace Atlassian.Bitbucket.Tests.Cloud @@ -29,10 +30,7 @@ public void Deserialize_UserInfo() var json = $"{{\"uuid\": \"{uuid}\", \"has_2fa_enabled\": null, \"username\": \"{userName}\", \"account_id\": \"{accountId}\"}}"; - var result = JsonSerializer.Deserialize(json, new JsonSerializerOptions() - { - PropertyNameCaseInsensitive = true, - }); + var result = JsonSerializer.Deserialize(json, JsonHelper.CaseInsensitiveOptions); Assert.Equal(userName, result.UserName); } diff --git a/src/shared/Atlassian.Bitbucket/Cloud/BitbucketRestApi.cs b/src/shared/Atlassian.Bitbucket/Cloud/BitbucketRestApi.cs index 94021e14d..a3b17a913 100644 --- a/src/shared/Atlassian.Bitbucket/Cloud/BitbucketRestApi.cs +++ b/src/shared/Atlassian.Bitbucket/Cloud/BitbucketRestApi.cs @@ -43,11 +43,7 @@ public async Task> GetUserInformationAsync(string userN if (response.IsSuccessStatusCode) { - var obj = JsonSerializer.Deserialize(json, - new JsonSerializerOptions - { - PropertyNameCaseInsensitive = true, - }); + var obj = JsonSerializer.Deserialize(json, JsonHelper.CaseInsensitiveOptions); return new RestApiResult(response.StatusCode, obj); } diff --git a/src/shared/Atlassian.Bitbucket/DataCenter/BitbucketRestApi.cs b/src/shared/Atlassian.Bitbucket/DataCenter/BitbucketRestApi.cs index 159229885..52d461d57 100644 --- a/src/shared/Atlassian.Bitbucket/DataCenter/BitbucketRestApi.cs +++ b/src/shared/Atlassian.Bitbucket/DataCenter/BitbucketRestApi.cs @@ -107,12 +107,7 @@ public async Task> GetAuthenticationMethodsAsync() if (response.IsSuccessStatusCode) { - var loginOptions = JsonSerializer.Deserialize(json, - new JsonSerializerOptions - { - PropertyNameCaseInsensitive = true, - DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull - }); + var loginOptions = JsonSerializer.Deserialize(json, JsonHelper.CaseInsensitiveIgnoreNullOptions); if (loginOptions.Results.Any(r => "LOGIN_FORM".Equals(r.Type))) { diff --git a/src/shared/Core.Tests/TokenEndpointResponseJsonTest.cs b/src/shared/Core.Tests/TokenEndpointResponseJsonTest.cs index a36395d83..793562b11 100644 --- a/src/shared/Core.Tests/TokenEndpointResponseJsonTest.cs +++ b/src/shared/Core.Tests/TokenEndpointResponseJsonTest.cs @@ -1,5 +1,6 @@ using System; using System.Text.Json; +using GitCredentialManager; using GitCredentialManager.Authentication.OAuth.Json; using Xunit; @@ -17,11 +18,7 @@ public void TokenEndpointResponseJson_Deserialize_Uses_Scope() var scopeString = "a,b,c"; var json = $"{{\"access_token\": \"{accessToken}\", \"token_type\": \"{tokenType}\", \"expires_in\": {expiresIn}, \"scopes\": \"{scopesString}\", \"scope\": \"{scopeString}\"}}"; - var result = JsonSerializer.Deserialize(json, - new JsonSerializerOptions - { - PropertyNameCaseInsensitive = true - }); + var result = JsonSerializer.Deserialize(json, JsonHelper.CaseInsensitiveOptions); Assert.Equal(accessToken, result.AccessToken); Assert.Equal(tokenType, result.TokenType); diff --git a/src/shared/Core/Authentication/OAuth/OAuth2Client.cs b/src/shared/Core/Authentication/OAuth/OAuth2Client.cs index 27834d2aa..93deec314 100644 --- a/src/shared/Core/Authentication/OAuth/OAuth2Client.cs +++ b/src/shared/Core/Authentication/OAuth/OAuth2Client.cs @@ -321,10 +321,7 @@ public async Task GetTokenByDeviceCodeAsync(OAuth2DeviceCodeR return result; } - var error = JsonSerializer.Deserialize(json, new JsonSerializerOptions - { - PropertyNameCaseInsensitive = true - }); + var error = JsonSerializer.Deserialize(json, JsonHelper.CaseInsensitiveOptions); switch (error.Error) { diff --git a/src/shared/Core/JsonHelper.cs b/src/shared/Core/JsonHelper.cs new file mode 100644 index 000000000..f239f0c84 --- /dev/null +++ b/src/shared/Core/JsonHelper.cs @@ -0,0 +1,29 @@ +using System.Text.Json; +using System.Text.Json.Serialization; + +namespace GitCredentialManager +{ + /// + /// Helper class providing common JSON serialization options and utilities. + /// + public static class JsonHelper + { + /// + /// Gets JSON serializer options configured for case-insensitive property names. + /// + public static JsonSerializerOptions CaseInsensitiveOptions { get; } = new JsonSerializerOptions + { + PropertyNameCaseInsensitive = true + }; + + /// + /// Gets JSON serializer options configured for case-insensitive property names + /// and ignoring null values when writing. + /// + public static JsonSerializerOptions CaseInsensitiveIgnoreNullOptions { get; } = new JsonSerializerOptions + { + PropertyNameCaseInsensitive = true, + DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull + }; + } +} From 2fb5fe13158c7281043ed5b0c7f57ea53b95aada Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 16 Feb 2026 04:20:53 +0000 Subject: [PATCH 4/4] Document thread safety for shared JsonSerializerOptions instances Co-authored-by: mdellison90-stack <230609064+mdellison90-stack@users.noreply.github.com> --- src/shared/Core/JsonHelper.cs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/shared/Core/JsonHelper.cs b/src/shared/Core/JsonHelper.cs index f239f0c84..02aa52514 100644 --- a/src/shared/Core/JsonHelper.cs +++ b/src/shared/Core/JsonHelper.cs @@ -6,10 +6,15 @@ namespace GitCredentialManager /// /// Helper class providing common JSON serialization options and utilities. /// + /// + /// The shared JsonSerializerOptions instances should not be modified after initialization + /// to ensure thread safety across the application. + /// public static class JsonHelper { /// /// Gets JSON serializer options configured for case-insensitive property names. + /// Do not modify this instance; create a new instance if different options are needed. /// public static JsonSerializerOptions CaseInsensitiveOptions { get; } = new JsonSerializerOptions { @@ -19,6 +24,7 @@ public static class JsonHelper /// /// Gets JSON serializer options configured for case-insensitive property names /// and ignoring null values when writing. + /// Do not modify this instance; create a new instance if different options are needed. /// public static JsonSerializerOptions CaseInsensitiveIgnoreNullOptions { get; } = new JsonSerializerOptions {