Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GCM release 2.3.0 #1361

Merged
merged 15 commits into from
Aug 1, 2023
Merged

GCM release 2.3.0 #1361

merged 15 commits into from
Aug 1, 2023

Conversation

mjcheetham and others added 15 commits July 10, 2023 10:52
Let the caller in to the `IMicrosoftAuthentication` component decide if
Microsoft Account Passthrough (MSA-PT) behaviour should be used.

Azure DevOps requires MSA-PT, so set that to `true` in usages.
When we have a Microsoft Account (MSA) in the cache and attempt to do a
silent authentication, if we're an MSA-PT app we need to specify the
special MSA transfer tenant ID to make sure we get the a token silently,
correctly. See the issue [1] in the MSAL repo for more information.

[1] AzureAD/microsoft-authentication-library-for-dotnet#3077
Since 2 versions of Git have released since the rename of the executable
from git-credential-manager-core to git-credential-manager, remove the
associated symlinks and warnings, (as outlined in docs/rename.md).
Currently, failure to access http://example.com causes failure of the
Networking Diagnostic portion of the diagnose command. To improve the
experience for users who are unable to access http://example.com, this
change:

1. Adds a fallback URI - if accessing http://example.com throws an
exception, we now try http://httpforever.com.
2. Prints a warning when either the primary or both the primary and
fallback uris throw an exception (instead of failing the Networking
Diagnostic).
These exceptions were discovered while exploring trace2 settings with a
full Git client.

Git can take a directory location as a trace2 target and will create new
files for every process. GCM currently throws a
DirectoryNotFoundException given such a parameter.

Git processes somehow can append to the same file across multiple
subprocesses. When GCM attempts to append to the file, it gets an
UnauthorizedAccessException on Windows due to multiple writers being
problematic. This exception could also happen on other platforms if the
setting is pointing to a file with restricted permissions.

In both of these cases, we chose to do nothing. The traces are lost, but
that's better than crashing the process.

Future directions could include:

1. Sending a warning over stderr if these exceptions occur, to make it
   clear why trace2 are not showing up.

2. Directories could be noticed as a different kind of trace target and
   we create a new file for the process before passing it to the
   Trace2FileWriter.

3. Perhaps there is a way for Git to pass the handle to the trace file
   so we can append to the file that Git was using. (Alternatively, Git
   could close the file and then reopen it after running the GCM
   subprocess.)

Each of these issues are a bit complicated, so this quick fix is chosen
as a stop gap to avoid this problem for current users.
These exceptions were discovered while exploring trace2 settings with a
full Git client.

Git can take a directory location as a trace2 target and will create new
files for every process. GCM currently throws a
DirectoryNotFoundException given such a parameter.

Git processes somehow can append to the same file across multiple
subprocesses. When GCM attempts to append to the file, it gets an
UnauthorizedAccessException on Windows due to multiple writers being
problematic. This exception could also happen on other platforms if the
setting is pointing to a file with restricted permissions.

In both of these cases, we chose to do nothing. The traces are lost, but
that's better than crashing the process.

Future directions could include:

1. Sending a warning over stderr if these exceptions occur, to make it
clear why trace2 are not showing up.

2. Directories could be noticed as a different kind of trace target and
we create a new file for the process before passing it to the
Trace2FileWriter.

3. Perhaps there is a way for Git to pass the handle to the trace file
so we can append to the file that Git was using. (Alternatively, Git
could close the file and then reopen it after running the GCM
subprocess.)

Each of these issues are a bit complicated, so this quick fix is chosen
as a stop gap to avoid this problem for current users.
Change the example command `git-credential-manager github --help` to `git credential-manager github --help` to ensure compatibility with GCM bundled with Git for Windows.
Since 2 versions of Git have released since the rename of the executable
from `git-credential-manager-core` to `git-credential-manager`, remove
the associated symlinks and warnings, (as outlined in `docs/rename.md`).
Currently, failure to access http://example.com causes failure of the
Networking Diagnostic portion of the `diagnose` command. To improve the
experience for users who are unable to access http://example.com, this
change:

1. Adds a fallback URI - if accessing http://example.com throws an
exception, we now try http://httpforever.com.
2. Prints a warning when either the primary or both the primary and
fallback uris throw an exception (instead of failing the Networking
Diagnostic).

Fixes #1215
Update the version notation because since the release of git-credential-manager version 2.2.1, the sdk version required for installation is .NET 7.
Update the version notation because since the release of
git-credential-manager version 2.2.1, [the sdk version required for
installation is .NET
7](https://www.nuget.org/packages/git-credential-manager/2.2.1).
When we have a Microsoft Account (MSA) in the cache and attempt to do a
silent authentication, if we're an MSA-PT app we need to specify the
special MSA transfer tenant ID to make sure we get the a token silently,
correctly.

See the
[issue](AzureAD/microsoft-authentication-library-for-dotnet#3077)
in the MSAL repo for more information.

Fixes: #1297
@ldennington ldennington self-assigned this Aug 1, 2023
@ldennington ldennington merged commit 58e34e3 into release Aug 1, 2023
16 checks passed
@ldennington ldennington temporarily deployed to release August 7, 2023 16:10 — with GitHub Actions Inactive
@ldennington ldennington temporarily deployed to release August 7, 2023 16:10 — with GitHub Actions Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants