diff --git a/.azure-pipelines/nuget.config b/.azure-pipelines/nuget.config new file mode 100644 index 0000000000..0cdfa50d8e --- /dev/null +++ b/.azure-pipelines/nuget.config @@ -0,0 +1,11 @@ + + + + + + + + diff --git a/.azure-pipelines/release.yml b/.azure-pipelines/release.yml index a957609d89..7b93478284 100644 --- a/.azure-pipelines/release.yml +++ b/.azure-pipelines/release.yml @@ -134,6 +134,15 @@ extends: artifactName: '${{ dim.runtime }}' steps: - checkout: self + - task: CopyFiles@2 + displayName: 'Use Central Feed Services (CFS)' + inputs: + SourceFolder: '$(Build.SourcesDirectory)\.azure-pipelines' + Contents: 'nuget.config' + TargetFolder: '$(Build.SourcesDirectory)' + Overwrite: true + - task: NuGetAuthenticate@1 + displayName: 'Authenticate to NuGet feeds' - task: PowerShell@2 displayName: 'Read version file' inputs: @@ -295,6 +304,15 @@ extends: artifactName: '${{ dim.runtime }}' steps: - checkout: self + - task: CopyFiles@2 + displayName: 'Use Central Feed Services (CFS)' + inputs: + SourceFolder: '$(Build.SourcesDirectory)/.azure-pipelines' + Contents: 'nuget.config' + TargetFolder: '$(Build.SourcesDirectory)' + Overwrite: true + - task: NuGetAuthenticate@1 + displayName: 'Authenticate to NuGet feeds' - task: Bash@3 displayName: 'Read version file' inputs: @@ -570,6 +588,15 @@ extends: artifactName: '${{ dim.runtime }}' steps: - checkout: self + - task: CopyFiles@2 + displayName: 'Use Central Feed Services (CFS)' + inputs: + SourceFolder: '$(Build.SourcesDirectory)/.azure-pipelines' + Contents: 'nuget.config' + TargetFolder: '$(Build.SourcesDirectory)' + Overwrite: true + - task: NuGetAuthenticate@1 + displayName: 'Authenticate to NuGet feeds' - task: Bash@3 displayName: 'Read version file' inputs: @@ -639,7 +666,7 @@ extends: inlineOperation: | [ { - "KeyCode": "CP-453387-Pgp", + "KeyCode": "CP-500207-Pgp", "OperationCode": "LinuxSign", "ToolName": "sign", "ToolVersion": "1.0", @@ -673,6 +700,15 @@ extends: artifactName: 'dotnet-tool' steps: - checkout: self + - task: CopyFiles@2 + displayName: 'Use Central Feed Services (CFS)' + inputs: + SourceFolder: '$(Build.SourcesDirectory)\.azure-pipelines' + Contents: 'nuget.config' + TargetFolder: '$(Build.SourcesDirectory)' + Overwrite: true + - task: NuGetAuthenticate@1 + displayName: 'Authenticate to NuGet feeds' - task: PowerShell@2 displayName: 'Read version file' inputs: diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml index 943e7c465b..de584f174f 100644 --- a/.github/workflows/codeql-analysis.yml +++ b/.github/workflows/codeql-analysis.yml @@ -22,16 +22,16 @@ jobs: language: [ 'csharp' ] steps: - - uses: actions/checkout@v6 + - uses: actions/checkout@v7 - name: Setup .NET - uses: actions/setup-dotnet@v5.2.0 + uses: actions/setup-dotnet@v5.4.0 with: dotnet-version: 10.0.x # Initializes the CodeQL tools for scanning. - name: Initialize CodeQL - uses: github/codeql-action/init@v4 + uses: github/codeql-action/init@v4.37.6 with: languages: ${{ matrix.language }} @@ -39,4 +39,4 @@ jobs: dotnet build - name: Perform CodeQL Analysis - uses: github/codeql-action/analyze@v4 + uses: github/codeql-action/analyze@v4.37.6 diff --git a/.github/workflows/continuous-integration.yml b/.github/workflows/continuous-integration.yml index 1d4488405e..9997aa42ae 100644 --- a/.github/workflows/continuous-integration.yml +++ b/.github/workflows/continuous-integration.yml @@ -25,10 +25,10 @@ jobs: os: windows-11-arm steps: - - uses: actions/checkout@v6 + - uses: actions/checkout@v7 - name: Setup .NET - uses: actions/setup-dotnet@v5.2.0 + uses: actions/setup-dotnet@v5.4.0 with: dotnet-version: 10.0.x @@ -82,10 +82,10 @@ jobs: runtime: [ linux-x64, linux-arm64, linux-arm ] steps: - - uses: actions/checkout@v6 + - uses: actions/checkout@v7 - name: Setup .NET - uses: actions/setup-dotnet@v5.2.0 + uses: actions/setup-dotnet@v5.4.0 with: dotnet-version: 10.0.x @@ -126,10 +126,10 @@ jobs: runtime: [ osx-x64, osx-arm64 ] steps: - - uses: actions/checkout@v6 + - uses: actions/checkout@v7 - name: Setup .NET - uses: actions/setup-dotnet@v5.2.0 + uses: actions/setup-dotnet@v5.4.0 with: dotnet-version: 10.0.x diff --git a/.github/workflows/lint-docs.yml b/.github/workflows/lint-docs.yml index bfbd2bbfaf..aee1daa26e 100644 --- a/.github/workflows/lint-docs.yml +++ b/.github/workflows/lint-docs.yml @@ -13,14 +13,17 @@ on: - '**.md' - '.github/workflows/lint-docs.yml' +permissions: + contents: read + jobs: lint-markdown: name: Lint markdown files runs-on: ubuntu-latest steps: - - uses: actions/checkout@v6 + - uses: actions/checkout@v7 - - uses: DavidAnson/markdownlint-cli2-action@ce4853d43830c74c1753b39f3cf40f71c2031eb9 + - uses: DavidAnson/markdownlint-cli2-action@21c1be1b93ad9ed58fa840aacc3f279cde2a72ff with: globs: | "**/*.md" @@ -30,12 +33,12 @@ jobs: name: Check for broken links runs-on: ubuntu-latest steps: - - uses: actions/checkout@v6 + - uses: actions/checkout@v7 - name: Run link checker # For any troubleshooting, see: # https://github.com/lycheeverse/lychee/blob/master/docs/TROUBLESHOOTING.md - uses: lycheeverse/lychee-action@8646ba30535128ac92d33dfc9133794bfdd9b411 + uses: lycheeverse/lychee-action@e7477775783ea5526144ba13e8db5eec57747ce8 with: # user-agent: if a user agent is not specified, some websites (e.g. # GitHub Docs) return HTTP errors which Lychee will interpret as diff --git a/.github/workflows/validate-install-from-source.yml b/.github/workflows/validate-install-from-source.yml index 85c821eea4..dca6f56b46 100644 --- a/.github/workflows/validate-install-from-source.yml +++ b/.github/workflows/validate-install-from-source.yml @@ -45,7 +45,7 @@ jobs: GNUPGHOME=/root/.gnupg tdnf install tar -y # needed for `actions/checkout` fi - - uses: actions/checkout@v6 + - uses: actions/checkout@v7 - run: | sh "${GITHUB_WORKSPACE}/src/linux/Packaging.Linux/install-from-source.sh" -y diff --git a/Directory.Packages.props b/Directory.Packages.props index d1e002d856..3e71e110a1 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -14,9 +14,9 @@ - - - + + + diff --git a/VERSION b/VERSION index 0ab902011a..111f6e3a6e 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -2.8.0.0 +2.9.1.0 diff --git a/docs/credstores.md b/docs/credstores.md index ca76f56926..4d54059e60 100644 --- a/docs/credstores.md +++ b/docs/credstores.md @@ -277,7 +277,7 @@ Note that you'll want to ensure that another credential helper is placed before GCM in the `credential.helper` Git configuration or else you will be prompted to enter your credentials every time you interact with a remote repository. -[access-windows-credential-manager]: https://support.microsoft.com/en-us/windows/accessing-credential-manager-1b5c916a-6a16-889f-8581-fc16e8165ac0 +[access-windows-credential-manager]: https://support.microsoft.com/en-US/Windows/Security/credential-manager-in-windows [aws-cloudshell]: https://aws.amazon.com/cloudshell/ [azure-cloudshell]: https://docs.microsoft.com/azure/cloud-shell/overview [cmdkey]: https://docs.microsoft.com/en-us/windows-server/administration/windows-commands/cmdkey 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/docs/github-apideprecation.md b/docs/github-apideprecation.md index 6a54a7a401..7075085d29 100644 --- a/docs/github-apideprecation.md +++ b/docs/github-apideprecation.md @@ -143,6 +143,6 @@ the new token-based authentication requirements **DO NOT** apply to GHES: [windows-cli-save-pat-image]: img/windows-cli-save-pat.png [vs-2019]: https://docs.microsoft.com/en-us/visualstudio/install/update-visual-studio?view=vs-2019 [vs-2017]: https://docs.microsoft.com/en-us/visualstudio/install/update-visual-studio?view=vs-2017 -[windows-credential-manager]: https://support.microsoft.com/en-us/windows/accessing-credential-manager-1b5c916a-6a16-889f-8581-fc16e8165ac0 +[windows-credential-manager]: https://support.microsoft.com/en-US/Windows/Security/credential-manager-in-windows [windows-gui-add-pat-image]: img/windows-gui-add-pat.png [windows-gui-credentials-image]: img/windows-gui-credentials.png diff --git a/docs/linux-validate-gpg.md b/docs/linux-validate-gpg.md index 49150c1e59..d19caae96e 100644 --- a/docs/linux-validate-gpg.md +++ b/docs/linux-validate-gpg.md @@ -10,46 +10,42 @@ the latest Debian package and/or tarball signature. apt-get install -y curl debsig-verify # Download public key signature file -curl -s https://api.github.com/repos/git-ecosystem/git-credential-manager/releases/latest \ -| grep -E 'browser_download_url.*gcm-public.asc' \ -| cut -d : -f 2,3 \ -| tr -d \" \ -| xargs -I 'url' curl -L -o gcm-public.asc 'url' +curl -Os https://packages.microsoft.com/keys/microsoft-2025.asc # De-armor public key signature file -gpg --output gcm-public.gpg --dearmor gcm-public.asc +gpg --output microsoft-2025.gpg --dearmor microsoft-2025.asc -# Note that the fingerprint of this key is "3C853823978B07FA", which you can +# Note that the fingerprint of this key is "EE4D7792F748182B", which you can # determine by running: -gpg --show-keys gcm-public.asc | head -n 2 | tail -n 1 | tail -c 17 +gpg --show-keys microsoft-2025.asc | head -n 2 | tail -n 1 | tail -c 17 # Copy de-armored public key to debsig keyring folder -mkdir /usr/share/debsig/keyrings/3C853823978B07FA -mv gcm-public.gpg /usr/share/debsig/keyrings/3C853823978B07FA/ +mkdir /usr/share/debsig/keyrings/EE4D7792F748182B +mv microsoft-2025.gpg /usr/share/debsig/keyrings/EE4D7792F748182B/ # Create an appropriate policy file -mkdir /etc/debsig/policies/3C853823978B07FA -cat > /etc/debsig/policies/3C853823978B07FA/generic.pol << EOL +mkdir /etc/debsig/policies/EE4D7792F748182B +cat > /etc/debsig/policies/EE4D7792F748182B/generic.pol << EOL - + - + - + EOL -# Download Debian package +# Download Debian package (substitute `x64` with `arm64` on ARM machines) curl -s https://api.github.com/repos/git-ecosystem/git-credential-manager/releases/latest \ -| grep "browser_download_url.*deb" \ +| grep "browser_download_url.*-x64-.*deb" \ | cut -d : -f 2,3 \ | tr -d \" \ | xargs -I 'url' curl -L -o gcm.deb 'url' @@ -61,24 +57,20 @@ debsig-verify gcm.deb ## Tarball ```shell # Download the public key signature file -curl -s https://api.github.com/repos/git-ecosystem/git-credential-manager/releases/latest \ -| grep -E 'browser_download_url.*gcm-public.asc' \ -| cut -d : -f 2,3 \ -| tr -d \" \ -| xargs -I 'url' curl -L -o gcm-public.asc 'url' +curl -Os https://packages.microsoft.com/keys/microsoft-2025.asc # Import the public key -gpg --import gcm-public.asc +gpg --import microsoft-2025.asc # Download the tarball and its signature file -curl -s https://api.github.com/repos/ldennington/git-credential-manager/releases/latest \ +curl -s https://api.github.com/repos/git-ecosystem/git-credential-manager/releases/latest \ | grep -E 'browser_download_url.*gcm-linux.*[0-9].[0-9].[0-9].tar.gz' \ | cut -d : -f 2,3 \ | tr -d \" \ | xargs -I 'url' curl -LO 'url' # Trust the public key -echo -e "5\ny\n" | gpg --command-fd 0 --expert --edit-key 3C853823978B07FA trust +echo -e "5\ny\n" | gpg --command-fd 0 --expert --edit-key EE4D7792F748182B trust # Verify the signature gpg --verify gcm-linux_amd64*.tar.gz.asc gcm-linux*.tar.gz diff --git a/global.json b/global.json index 5cc6b13a63..d9483139eb 100644 --- a/global.json +++ b/global.json @@ -1,7 +1,7 @@ { "sdk": { "rollForward": "latestMajor", - "version": "8.0" + "version": "8.0.100" } } diff --git a/src/shared/Atlassian.Bitbucket.Tests/Cloud/BitbucketOAuth2ClientTest.cs b/src/shared/Atlassian.Bitbucket.Tests/Cloud/BitbucketOAuth2ClientTest.cs index 1a6866fb63..e57caf6931 100644 --- a/src/shared/Atlassian.Bitbucket.Tests/Cloud/BitbucketOAuth2ClientTest.cs +++ b/src/shared/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/shared/Atlassian.Bitbucket.Tests/DataCenter/BitbucketOAuth2ClientTest.cs b/src/shared/Atlassian.Bitbucket.Tests/DataCenter/BitbucketOAuth2ClientTest.cs index e2e7225db3..5931a6a0c7 100644 --- a/src/shared/Atlassian.Bitbucket.Tests/DataCenter/BitbucketOAuth2ClientTest.cs +++ b/src/shared/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/shared/Core.Tests/Authentication/MicrosoftAuthenticationTests.cs b/src/shared/Core.Tests/Authentication/MicrosoftAuthenticationTests.cs index 0e1a70659e..f3caf40741 100644 --- a/src/shared/Core.Tests/Authentication/MicrosoftAuthenticationTests.cs +++ b/src/shared/Core.Tests/Authentication/MicrosoftAuthenticationTests.cs @@ -1,8 +1,10 @@ using System; +using System.IO; using System.Threading.Tasks; using GitCredentialManager.Authentication; using GitCredentialManager.Tests.Objects; using Microsoft.Identity.Client.AppConfig; +using Microsoft.Identity.Client.Extensions.Msal; using Xunit; namespace GitCredentialManager.Tests.Authentication @@ -29,6 +31,51 @@ await Assert.ThrowsAsync( () => msAuth.GetTokenForUserAsync(authority, clientId, redirectUri, scopes, userName, false)); } + [MacOSFact] + public void MicrosoftAuthentication_CreateUserTokenCacheProps_OnMacOS_UsesGcmKeychain() + { + var context = new TestCommandContext(); + var msAuth = new MicrosoftAuthentication(context); + + StorageCreationProperties actual = msAuth.CreateUserTokenCacheProps(useLinuxFallback: false); + + Assert.Equal("user.cache", actual.CacheFileName); + Assert.Equal(Path.Combine(context.FileSystem.UserDataDirectoryPath, "msal"), actual.CacheDirectory); + Assert.Equal("GitCredentialManager.MSAL", actual.MacKeyChainServiceName); + Assert.Equal("UserCache", actual.MacKeyChainAccountName); + } + + [WindowsFact] + public void MicrosoftAuthentication_CreateUserTokenCacheProps_OnWindows_UsesSharedCache() + { + var context = new TestCommandContext(); + var msAuth = new MicrosoftAuthentication(context); + + StorageCreationProperties actual = msAuth.CreateUserTokenCacheProps(useLinuxFallback: false); + + Assert.Equal("msal.cache", actual.CacheFileName); + Assert.Equal( + Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.LocalApplicationData), ".IdentityService"), + actual.CacheDirectory); + } + + [LinuxFact] + public void MicrosoftAuthentication_CreateUserTokenCacheProps_OnLinux_UsesSharedCache() + { + var context = new TestCommandContext(); + var msAuth = new MicrosoftAuthentication(context); + + StorageCreationProperties actual = msAuth.CreateUserTokenCacheProps(useLinuxFallback: false); + + Assert.Equal("msal.cache", actual.CacheFileName); + Assert.Equal( + Path.Combine(context.FileSystem.UserHomePath, ".local", ".IdentityService"), + actual.CacheDirectory); + Assert.Equal("msal.cache", actual.KeyringSchemaName); + Assert.Equal("default", actual.KeyringCollection); + Assert.Equal("MSALCache", actual.KeyringSecretLabel); + } + [Theory] [InlineData(null)] [InlineData("")] diff --git a/src/shared/Core.Tests/Authentication/OAuth2ClientTests.cs b/src/shared/Core.Tests/Authentication/OAuth2ClientTests.cs index be660b99bb..1ec3eae251 100644 --- a/src/shared/Core.Tests/Authentication/OAuth2ClientTests.cs +++ b/src/shared/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/shared/Core.Tests/Authentication/OAuth2ResponseModeTests.cs b/src/shared/Core.Tests/Authentication/OAuth2ResponseModeTests.cs new file mode 100644 index 0000000000..a52bf23508 --- /dev/null +++ b/src/shared/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/shared/Core.Tests/Authentication/OAuth2SystemWebBrowserTests.cs b/src/shared/Core.Tests/Authentication/OAuth2SystemWebBrowserTests.cs index cea6abe17f..9274845a2f 100644 --- a/src/shared/Core.Tests/Authentication/OAuth2SystemWebBrowserTests.cs +++ b/src/shared/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/shared/Core.Tests/GenericOAuthConfigTests.cs b/src/shared/Core.Tests/GenericOAuthConfigTests.cs index b05ae2e8b3..cd1f1573fe 100644 --- a/src/shared/Core.Tests/GenericOAuthConfigTests.cs +++ b/src/shared/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 InputArguments(new Dictionary + { + {"protocol", protocol}, + {"host", host}, + }); + + return GenericOAuthConfig.TryGet(trace, settings, input, out config); + } } } diff --git a/src/shared/Core/Authentication/MicrosoftAuthentication.cs b/src/shared/Core/Authentication/MicrosoftAuthentication.cs index 5d65fa9823..eae5437334 100644 --- a/src/shared/Core/Authentication/MicrosoftAuthentication.cs +++ b/src/shared/Core/Authentication/MicrosoftAuthentication.cs @@ -30,7 +30,7 @@ public interface IMicrosoftAuthentication /// /// Azure authority. /// Client ID. - /// Redirect URI for the client. + /// Redirect URI for the client. Use null for the default redirect URI. /// Set of scopes to request. /// Optional user name for an existing account. /// Use MSA-Passthrough behavior when authenticating. @@ -116,14 +116,17 @@ public interface IMicrosoftAuthenticationResult public enum MicrosoftAuthenticationFlowType { - Auto = 0, - EmbeddedWebView = 1, - SystemWebView = 2, - DeviceCode = 3 + EmbeddedWebView, + SystemWebView, + DeviceCode } public class MicrosoftAuthentication : AuthenticationBase, IMicrosoftAuthentication { + private const string GcmMacKeychainServiceName = "GitCredentialManager.MSAL"; + private const string GcmMacKeychainUserAccountName = "UserCache"; + private const string GcmMacKeychainAppAccountName = "AppCache"; + public static readonly string[] AuthorityIds = { "msa", "microsoft", "microsoftaccount", @@ -152,6 +155,27 @@ public async Task GetTokenForUserAsync( Context.Trace.WriteLine("MSA passthrough is enabled."); } + // Check if the user has specified a particular type of authentication flow + MicrosoftAuthenticationFlowType flowType = GetFlowType(redirectUri); + Context.Trace.WriteLine($"Flow type is: '{flowType}'."); + + // If we are going to use anything *other than* the system webview, we ignore + // the provided redirect URI and set it to the default for a native client. + // The broker is used above all else, if enabled, but that has a fallback to + // the system browser if there is a problem. + // We must continue to pass through the provided redirect URI if we're going to + // try the system webview, as the system webview requires a real loopback redirect + // URI that is registered with the application. + if (!useBroker && flowType != MicrosoftAuthenticationFlowType.SystemWebView) + { + Context.Trace.WriteLine("Using default redirect URI."); + redirectUri = null; // null to signal the default redirect URI + } + else + { + Context.Trace.WriteLine($"Redirect URI is '{redirectUri}'."); + } + try { // Create the public client application for authentication @@ -214,27 +238,17 @@ public async Task GetTokenForUserAsync( Context.Trace.WriteLine("Performing interactive auth with broker..."); result = await app.AcquireTokenInteractive(scopes) .WithPrompt(Prompt.SelectAccount) - // We must configure the system webview as a fallback + // We must configure the system webview as a fallback in case + // the broker is not available on this system. .WithSystemWebViewOptions(GetSystemWebViewOptions()) .ExecuteAsync(); } } else { - // Check for a user flow preference if they've specified one - MicrosoftAuthenticationFlowType flowType = GetFlowType(); + // Respect the user's flow preference switch (flowType) { - case MicrosoftAuthenticationFlowType.Auto: - if (CanUseEmbeddedWebView()) - goto case MicrosoftAuthenticationFlowType.EmbeddedWebView; - - if (CanUseSystemWebView(app, redirectUri)) - goto case MicrosoftAuthenticationFlowType.SystemWebView; - - // Fall back to device code flow - goto case MicrosoftAuthenticationFlowType.DeviceCode; - case MicrosoftAuthenticationFlowType.EmbeddedWebView: Context.Trace.WriteLine("Performing interactive auth with embedded web view..."); EnsureCanUseEmbeddedWebView(); @@ -247,7 +261,7 @@ public async Task GetTokenForUserAsync( case MicrosoftAuthenticationFlowType.SystemWebView: Context.Trace.WriteLine("Performing interactive auth with system web view..."); - EnsureCanUseSystemWebView(app, redirectUri); + EnsureCanUseSystemWebView(redirectUri); result = await app.AcquireTokenInteractive(scopes) .WithPrompt(Prompt.SelectAccount) .WithSystemWebViewOptions(GetSystemWebViewOptions()) @@ -263,7 +277,7 @@ public async Task GetTokenForUserAsync( break; default: - goto case MicrosoftAuthenticationFlowType.Auto; + goto case MicrosoftAuthenticationFlowType.DeviceCode; // safe default } } } @@ -457,7 +471,7 @@ await AvaloniaUi.ShowViewAsync( } } - internal MicrosoftAuthenticationFlowType GetFlowType() + internal MicrosoftAuthenticationFlowType GetFlowType(Uri redirectUri) { if (Context.Settings.TryGetSetting( Constants.EnvironmentVariables.MsAuthFlow, @@ -469,7 +483,7 @@ internal MicrosoftAuthenticationFlowType GetFlowType() switch (valueStr.ToLowerInvariant()) { case "auto": - return MicrosoftAuthenticationFlowType.Auto; + return Auto(); case "embedded": return MicrosoftAuthenticationFlowType.EmbeddedWebView; case "system": @@ -483,7 +497,21 @@ internal MicrosoftAuthenticationFlowType GetFlowType() Context.Streams.Error.WriteLine($"warning: unknown Microsoft Authentication flow type '{valueStr}'; using 'auto'"); } - return MicrosoftAuthenticationFlowType.Auto; + return Auto(); + + // Resolve the 'auto' flow type based on the redirect URI and platform capabilities + MicrosoftAuthenticationFlowType Auto() + { + // Prefer embedded webview + if (CanUseEmbeddedWebView()) + return MicrosoftAuthenticationFlowType.EmbeddedWebView; + + if (CanUseSystemWebView(redirectUri)) + return MicrosoftAuthenticationFlowType.SystemWebView; + + // Fall back to device code flow + return MicrosoftAuthenticationFlowType.DeviceCode; + } } /// @@ -550,9 +578,21 @@ private async Task CreatePublicClientApplicationAsync( var appBuilder = PublicClientApplicationBuilder.Create(clientId) .WithAuthority(authority) - .WithRedirectUri(redirectUri.ToString()) .WithHttpClientFactory(httpFactoryAdaptor); + // Use the default redirect URI if one is not provided + if (redirectUri is null) + { + // Uses "https://login.microsoftonline.com/common/oauth2/nativeclient" on .NET Framework + // but "http://localhost" on .NET Core. This is because there is no embedded webview support + // in .NET Core and thus the system webview is the only option. + appBuilder.WithDefaultRedirectUri(); + } + else + { + appBuilder.WithRedirectUri(redirectUri.ToString()); + } + // Listen to MSAL logs if GCM_TRACE_MSAUTH is set if (Context.Settings.IsMsalTracingEnabled) { @@ -683,8 +723,8 @@ private async Task RegisterTokenCacheAsync(ITokenCache cache, StoragePropertiesB return; } - // We use the MSAL extension library to provide us consistent cache file access semantics (synchronisation, etc) - // as other GCM processes, and other Microsoft developer tools such as the Azure PowerShell CLI. + // We use the MSAL extension library to provide consistent cache file access semantics (synchronisation, etc) + // between GCM processes. On Windows and Linux this cache is also shared with other Microsoft developer tools. MsalCacheHelper helper = null; try { @@ -735,32 +775,45 @@ private async Task RegisterTokenCacheAsync(ITokenCache cache, StoragePropertiesB /// /// Create the properties for the user token cache. This is used by public client applications only. - /// This cache is shared between GCM processes, and also other Microsoft developer tools such as the Azure - /// PowerShell CLI. + /// This cache is shared between GCM processes. On Windows and Linux it is also shared with other Microsoft + /// developer tools such as the Azure PowerShell CLI. /// /// /// internal StorageCreationProperties CreateUserTokenCacheProps(bool useLinuxFallback) { - const string cacheFileName = "msal.cache"; + string cacheFileName; string cacheDirectory; - if (PlatformUtils.IsWindows()) + StorageCreationPropertiesBuilder builder; + + if (PlatformUtils.IsMacOS()) { - // The shared MSAL cache is located at "%LocalAppData%\.IdentityService\msal.cache" on Windows. - cacheDirectory = Path.Combine( - Environment.GetFolderPath(Environment.SpecialFolder.LocalApplicationData), - ".IdentityService" - ); + // Keep the macOS cache isolated from other Microsoft developer tools. Those tools can recreate the + // shared Keychain item and discard GCM's access control entry, causing repeated password prompts. + cacheFileName = "user.cache"; + cacheDirectory = Path.Combine(Context.FileSystem.UserDataDirectoryPath, "msal"); + builder = new StorageCreationPropertiesBuilder(cacheFileName, cacheDirectory) + .WithMacKeyChain(GcmMacKeychainServiceName, GcmMacKeychainUserAccountName); } else { - // The shared MSAL cache metadata is located at "~/.local/.IdentityService/msal.cache" on UNIX. - cacheDirectory = Path.Combine(Context.FileSystem.UserHomePath, ".local", ".IdentityService"); - } + cacheFileName = "msal.cache"; + if (PlatformUtils.IsWindows()) + { + // The shared MSAL cache is located at "%LocalAppData%\.IdentityService\msal.cache" on Windows. + cacheDirectory = Path.Combine( + Environment.GetFolderPath(Environment.SpecialFolder.LocalApplicationData), + ".IdentityService" + ); + } + else + { + // The shared MSAL cache metadata is located at "~/.local/.IdentityService/msal.cache" on Linux. + cacheDirectory = Path.Combine(Context.FileSystem.UserHomePath, ".local", ".IdentityService"); + } - // The keychain is used on macOS with the following service & account names - var builder = new StorageCreationPropertiesBuilder(cacheFileName, cacheDirectory) - .WithMacKeyChain("Microsoft.Developer.IdentityService", "MSALCache"); + builder = new StorageCreationPropertiesBuilder(cacheFileName, cacheDirectory); + } if (useLinuxFallback) { @@ -836,7 +889,7 @@ internal StorageCreationProperties CreateAppTokenCacheProps(bool useLinuxFallbac // The keychain is used on macOS with the following service & account names var builder = new StorageCreationPropertiesBuilder(cacheFileName, cacheDirectory) - .WithMacKeyChain("GitCredentialManager.MSAL", "AppCache"); + .WithMacKeyChain(GcmMacKeychainServiceName, GcmMacKeychainAppAccountName); if (useLinuxFallback) { @@ -988,7 +1041,7 @@ private void EnsureCanUseEmbeddedWebView() #endif } - private bool CanUseSystemWebView(IPublicClientApplication app, Uri redirectUri) + private bool CanUseSystemWebView(Uri redirectUri) { // // MSAL requires the application redirect URI is a loopback address to use the System WebView @@ -1000,7 +1053,7 @@ private bool CanUseSystemWebView(IPublicClientApplication app, Uri redirectUri) return Context.SessionManager.IsWebBrowserAvailable && redirectUri.IsLoopback; } - private void EnsureCanUseSystemWebView(IPublicClientApplication app, Uri redirectUri) + private void EnsureCanUseSystemWebView(Uri redirectUri) { if (!Context.SessionManager.IsWebBrowserAvailable) { diff --git a/src/shared/Core/Authentication/OAuth/IOAuth2WebBrowser.cs b/src/shared/Core/Authentication/OAuth/IOAuth2WebBrowser.cs index a9dbdf519a..72a30de613 100644 --- a/src/shared/Core/Authentication/OAuth/IOAuth2WebBrowser.cs +++ b/src/shared/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/shared/Core/Authentication/OAuth/OAuth2Client.cs b/src/shared/Core/Authentication/OAuth/OAuth2Client.cs index 27834d2aaf..75120522cc 100644 --- a/src/shared/Core/Authentication/OAuth/OAuth2Client.cs +++ b/src/shared/Core/Authentication/OAuth/OAuth2Client.cs @@ -73,6 +73,7 @@ public class OAuth2Client : IOAuth2Client private readonly ITrace2 _trace2; private readonly string _clientSecret; private readonly bool _addAuthHeader; + private readonly OAuth2ResponseMode _responseMode; private IOAuth2CodeGenerator _codeGenerator; @@ -82,7 +83,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; @@ -91,6 +93,7 @@ public OAuth2Client(HttpClient httpClient, _redirectUri = redirectUri; _clientSecret = clientSecret; _addAuthHeader = addAuthHeader; + _responseMode = responseMode; } public IOAuth2CodeGenerator CodeGenerator @@ -119,6 +122,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) @@ -157,25 +167,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/shared/Core/Authentication/OAuth/OAuth2Constants.cs b/src/shared/Core/Authentication/OAuth/OAuth2Constants.cs index 0b96a60476..a1c0ca90a8 100644 --- a/src/shared/Core/Authentication/OAuth/OAuth2Constants.cs +++ b/src/shared/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/shared/Core/Authentication/OAuth/OAuth2ResponseMode.cs b/src/shared/Core/Authentication/OAuth/OAuth2ResponseMode.cs new file mode 100644 index 0000000000..2f5ef0f13f --- /dev/null +++ b/src/shared/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/shared/Core/Authentication/OAuth/OAuth2SystemWebBrowser.cs b/src/shared/Core/Authentication/OAuth/OAuth2SystemWebBrowser.cs index 05843f9df2..4f55072a47 100644 --- a/src/shared/Core/Authentication/OAuth/OAuth2SystemWebBrowser.cs +++ b/src/shared/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/shared/Core/Constants.cs b/src/shared/Core/Constants.cs index 6fecc2b38d..d906d3a55c 100644 --- a/src/shared/Core/Constants.cs +++ b/src/shared/Core/Constants.cs @@ -129,6 +129,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"; @@ -145,6 +146,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 @@ -191,6 +196,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/shared/Core/Diagnostics/MicrosoftAuthenticationDiagnostic.cs b/src/shared/Core/Diagnostics/MicrosoftAuthenticationDiagnostic.cs index e4dba08224..ad64b7f810 100644 --- a/src/shared/Core/Diagnostics/MicrosoftAuthenticationDiagnostic.cs +++ b/src/shared/Core/Diagnostics/MicrosoftAuthenticationDiagnostic.cs @@ -17,7 +17,15 @@ protected override async Task RunInternalAsync(StringBuilder log, IList 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/shared/Core/GenericOAuthConfig.cs b/src/shared/Core/GenericOAuthConfig.cs index 522d89fec3..098541babb 100644 --- a/src/shared/Core/GenericOAuthConfig.cs +++ b/src/shared/Core/GenericOAuthConfig.cs @@ -134,6 +134,23 @@ public static bool TryGet(ITrace trace, ISettings settings, InputArguments input 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, InputArguments input 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/shared/Core/ISessionManager.cs b/src/shared/Core/ISessionManager.cs index 0ad0204c4b..8ee291f300 100644 --- a/src/shared/Core/ISessionManager.cs +++ b/src/shared/Core/ISessionManager.cs @@ -67,7 +67,13 @@ public void OpenBrowser(Uri uri) throw new ArgumentException("Can only open HTTP/HTTPS URIs", nameof(uri)); } - OpenBrowserInternal(uri.ToString()); + // 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); } protected virtual void OpenBrowserInternal(string url) diff --git a/src/shared/TestInfrastructure/Objects/TestOAuth2WebBrowser.cs b/src/shared/TestInfrastructure/Objects/TestOAuth2WebBrowser.cs index 547aaf360b..86f011cddc 100644 --- a/src/shared/TestInfrastructure/Objects/TestOAuth2WebBrowser.cs +++ b/src/shared/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(); } } }