change destinationGitRepositoryPersonalAccessToken to not be required

[milliamp]

The documentation inconsistently declares "Destination Repository - Personal Access Token" as both required and optional in different sections. The code already appears to handle destinationGitRepositoryPersonalAccessToken being undefined so this simply updates the one mention that the field is required and changes the task.json to make it optional. This allows pushing to repositories that don't have authentication.

NB: The other mention that destinationGitRepositoryPersonalAccessToken can be omitted is in README.md @ line 63

Haven't added any extra tests as this only alters the task.json declaration, not the functionality and the getAuthenticatedGitUri tests already cover this case.

Changes

Summary of changes included in this PR:

• destinationGitRepositoryPersonalAccessToken is now optional

CC - @traviskosarek @calebcartwright

[codecov]

Codecov Report

Merging #63 into master will not change coverage.
The diff coverage is n/a.

@@          Coverage Diff          @@
##           master    #63   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           1      1           
  Lines          75     75           
  Branches       10     10           
=====================================
  Hits           75     75

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 91f21b0...7e3b70e. Read the comment docs.

[calebcartwright]

Thanks for the PR @milliamp!

Do you have any examples/instances of git repositories that don't require any authentication or authorization for pushes/writes? Completely agreed on the need to rectify the docs regardless, but I'm having trouble picturing a scenario where auth wouldn't be required for the destination repo

[milliamp]

I am running this on an Azure DevOps Server 2019 installation which does not accept credentials using the form https://<user>:<password>@host/. In the interim I am supplying authentication by adding it to the Git Credential Manager on the agent (in this case Windows agent, so Windows Credential Manager) which allows authentication to pass.

The authentication on our Azure DevOps Server does work if the auth is passed using git like so:
git -c http.extraheader="AUTHORIZATION: Basic <base64-encoded-colon-followed-by-PAT>" clone
reference: https://stackoverflow.com/questions/53106546/cannot-clone-git-from-azure-devops-using-pat

This may simply be a difference between Azure DevOps (online) and Azure DevOps Server or could be configuration difference on our installation, however, as others seem to have this issue, it could be something like an Azure DevOps bug that is fixed online.

The next step would probably be to write this type of auth into the build task but this seemed like an acceptable first step before diving into that.

[calebcartwright]

Azure DevOps Server 2019 installation which does not accept credentials using the form https://:@host/

AzDO Services (SaaS) doesn't accept user:password either, but does AzDO Server not accept https://<user>:<pat>@host/?

[milliamp]

AzDO Services (SaaS) doesn't accept user:password either, but does AzDO Server not accept https://<user>:<pat>@host/?

I have tried the following combinations <user>:<password>, <user>:<pat>, :<pat>, made-up-user:<pat>, <pat>. Which all fail but in different ways that seem sane (PAT alone asks for a password and fails due to non-interactive, other just give 40X error)
Looking into this, my original assumption was that URL encoded authentication simply turned into basic authentication header client side. I discounted that because the same username and password works when sending the Basic authentication using the header directly so assumed it must be something server side but it is possible something more fishy is going on.

On a side note, found this article about Basic Auth on Wikipedia that suggests URL encoded auth was deprecated by section 3.2.1 of RFC3986 so there could also be some problems arising from software changes related to that.

[calebcartwright]

The docs heavily imply that https://<anything>:<pat>@.... should work for AzDO Server as well 🤔

https://docs.microsoft.com/en-us/azure/devops/organizations/accounts/use-personal-access-tokens-to-authenticate?view=azure-devops&tabs=preview-page#use-your-personal-access-token (Server 2019 is listed at the top as well).

However, in my experience it's not uncommon for the docs to be outdated and/or inaccurate, and that doesn't change the fact that it isn't working as described at all for you.

I was talking to @traviskosarek offline and we came to a consensus that we'd rather extend the task to support both URI and header approaches for authentication instead of dropping the requirement for the destination auth info. We considered switching to the auth header outright, but decided that carried to0 many unknowns/risks (would it break other users/target platforms, etc.).

Would you have any interest in implementing that @milliamp? No worries if not!

[js-hdl]

We have a situation where we are connecting to a repository on TFS 2015, which has no PAT support, so we are using SSH. Considering the Git Credential Manager and SSH options, as well as Windows Authentication for build agents running under specific domain service accounts, there are several real-case scenarios where it is handy to use authentication that is negotiated outside of the individual git push call. Requiring the auth token as input does not increase any security, as that is set up on the repository. However, having that requirement does prevent the use of these other "external" authentication schemes. I'm not a huge fan of magic strings, but perhaps a token value of "external" or "integrated" could be used to indicate that yes, the user has taken care of authentication in a different way, and the URI should not be modified.

[calebcartwright]

there are several real-case scenarios where it is handy to use authentication that is negotiated outside of the individual git push call.

Thanks for sharing your perspective @js-hdl! To elaborate a bit on the preceding discussion..

We're well aware that there's multiple ways of authenticating with a git host, but the questions and discussion were in reference to this line: This allows pushing to repositories that don't have authentication. from the PR description which implied that a git hosting platform would accept anonymous writes which still strikes me as unlikely.

A large number of the usage patterns with this extension is within the Azure DevOps (Services) ecosystem and utilizing the task in pipelines that are running on the Microsoft-hosted agent pools. In such cases it is, in fact, absolutely necessary for our users to be able to supply authentication info to use for their target repository (and in some cases destination) in these types of ephemeral, vendor managed environments. From a UX perspective, it's also beneficial for the task to indicate to users what fields are required to ensure that invalid configurations cannot be saved.

Yes, TFS/AzDO Server users and users with non-elastic self-hosted agents for AzDO Services can indeed pre-configure authorization locally on the host device and make it available to the running agents, thus obviating the need for the auth info to be included in the mirrored push invoked by the task.

However, although that's possible and is a use case we'd like to support, it is comparatively more of an edge case and the problem with the changes proposed in this PR is that they optimize for that edge case at the detriment of the more common use cases which isn't something we're willing to accept.

We described an alternative approach above that we'd be happy to support, and I've opened up #86 and #85 to describe the path we're willing to take in more detail. If you (or anyone else) would be interested in working on it then that would be great!

I am going to go ahead and close this one though given the above and the other issues. Thanks again for opening this to start the dialog though @milliamp!