Murmele / Murmele/Gittyup

Broken state of git-credential implementation (w.r.t. non-native helpers)

Open
#511 4 comments 3 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

enhancement
Dominant language
C++
Stars
2.3k
Forks
171
Avg merge
3d 19h
Merged PRs (30d)
10

Description

Credentials are currently handled internally for various credential.helpers, effectively re-implementing the git functionality.

When encountering an unsupported value in credential.helper, the generic GitCredential class is supposed to call through to the actual helper binary to do the work. There are a number of issues with the current implementation:

  1. Only the Gittyup install dir is searched, which is unlikely to produce a result
  2. Implementations that come with their own UI (e.g. Git Credential Manager) may not actually save the credentials
  3. It is perfectly valid to have multiple credential.helpers configured. As far as I can tell, this is not taken into account at all.
  4. Longer-running credential.helper implementations (such as those with built-in UI) will block the Gittyup UI

Details

1. Search Path

This is a rather simple fix, just search PATH and falling back to some hard-coded default locations. I have a working implementation for this, but it isn't particularly useful without addressing the other issues (Since the most prominent use would be GCM)

2. External UI support

The way the external UI is implemented is by blocking the helper's get subcommand on user input (login dialog or similar) and returning the login information. This is especially useful for implementing non-basic authentication schemes (like OAuth). Note that this may NOT actually save the credentials to the helper's store yet (this is the case at least with GCM).

I believe this is as intended by the git credential.helper protocol, and it is expected that the helper be called again with the store subcommand to confirm that the given credentials were acceptable and should be persisted. This is exactly what git itself does when running e.g. a git fetch on a https remote.

Gittyup currently doesn't do this and assumes that, if the helper returns credentials, that they are always taken from a persistent store and need not be sent back to be persisted. Ideally, this should only be done after verifying the credentials work.

3. Multiple credential helpers

Git supports specifying credential.helper multiple times to query multiple helpers for a stored password (See here). They should be tried in turn, with the first one to give a full username+password combination winning out. I have not tested whether or not the store subcommand goes to all or only some of the configured helpers, but I would expect the former.

While I'm not sure whether or not this is a commonly used feature, at the very least non-support should be documented and a runtime warning emitted when encountering a config with multiple helpers. Correctly implementing it would probably not be particularly hard, but needs to be tested against the actual git implementation.

4. Blocking UI

There is no guarantee that the helper returns (with or without info) right away, so blocking the UI on it is not great UX. While this is mostly a cosmetic issue, having non-responding UI doesn't look good.

The credential implementation should therefore be asynchronous.

Proposal

While it is certainly possible to fix these issues and have a functional re-implementation of the git credentials mechanism in Gittyup, I'm not sure this is a good allocation of time.

Instead, I'd like to propose completely replacing the current credential system with a new (asynchronous) implementation written from scratch. It wouldn't try to re-implement git internals and instead always delegate to the public interface git provides for this purpose, git-credential. Additionally, it is probably desirable to provide a GIT_ASKPASS implementation calling back into Gittyup to keep UX with non-UI helpers consistent. git credential fill does not have an option to return without asking the user when no stored password is found AFAICT.

It may make sense to add an additional (non-persistent) credentials cache inside Gittyup on top of this to reduce the need for repeatedly calling external processes, but this can be added at a later point.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by tracing Gittyup's current credential handling and its generic GitCredential path, then compare it with the public git-credential interface. A complete change would address external helper discovery, multiple helpers, persistence after successful authentication, and asynchronous execution; no repository files or tests are named in the issue.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp, git
Domain
authentication, desktop
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.