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

Bridge credentials should be read from the full configuration chain #406

Closed
sudoforge opened this issue Jun 17, 2020 · 14 comments
Closed

Bridge credentials should be read from the full configuration chain #406

sudoforge opened this issue Jun 17, 2020 · 14 comments

Comments

@sudoforge
Copy link

sudoforge commented Jun 17, 2020

git-bug does not currently read bridge credentials from anything except the user configuration file, e.g. ${HOME}/.gitconfig. It would be awesome if the full configuration chain could be supported, in order of precedence:

  • Local configuration -- ${REPO_ROOT}/.git/config
  • User configuration -- ${HOME}/.gitconfig or ${XDG_CONFIG_HOME}/git/config
  • System configuration -- /etc/gitconfig

Additionally, following include directives would be useful -- the main application for which is likely keeping the authentication configuration out of the default user-scoped conffile which is stored in a repository for public sharing.

@MichaelMure
Copy link
Owner

So git-bug read the git config with git itself, through the git config command.

Bridges configurations are stored in the local git config (--local is added to the command).
Bridges credentials are stored in the global git config (--global is added to the command).

Based on this, you should be able to use whatever git foo or files as long as it works for git itself. Isn't that the case ?

@sudoforge sudoforge changed the title Bridge authentication should read from the full configuration chain Bridge credentials should be read from the full configuration chain Jun 17, 2020
@sudoforge
Copy link
Author

I updated the description and title of this issue for additional clarity.

Yes, the configuration for bridges is read from the local configuration, and credentials are read from global. This issue pitches that credentials should be read from the entire chain.

@sudoforge
Copy link
Author

Also, note that there is a bug with the way credentials are read from the global configuration. If my global configuration contains a block like:

[include]
  path = ~/.gitconfig.foo

and ~/.gitconfig.foo contains the git-bug credential configuration, then you end up getting:

➜ git bug bridge {pull,push,etc}
Error: no token found for the default login "<git-bug.bridge.default.default-login>"

@MichaelMure
Copy link
Owner

But why is that the case ? Can you read those with the git config command ? git-bug doesn't do anything special there.

@sudoforge
Copy link
Author

Yeah, if you have the follow file structure:

~/.gitconfig

[include]
  path = "~/.gitconfig.local"

~/.gitconfig.local

[foo]
  bar = "baz"

then git config --get foo.bar returns baz as expected, however explicitly defining --global doesn't follow the include statement and thus will return an empty value.

@sudoforge
Copy link
Author

git-bug should probably not care about where the credentials are coming from and rely upon Git's configuration loading by simply calling:

git config --get <key...>

instead of explicitly calling --global, --local, or --system.

@sudoforge
Copy link
Author

I don't have time to familiarize myself with the codebase today, but this is a fairly trivial patch (if you approve of it) and I'd be happy to take care of this over the next few days / weekend.

@MichaelMure
Copy link
Owner

I'm torn. This looks like a git bug really, because ~/.gitconfig.local in your example is still the global config right ? So those entries should show up.

One thing that could be done is to use --global when writing but not for reading, but that blur the line between local and global config. That could lead to other bugs.

Some pointers:
https://github.com/MichaelMure/git-bug/blob/master/repository/config_git.go
https://github.com/MichaelMure/git-bug/blob/master/repository/git.go#L37-L45

I want to eventually migrate to using go-git instead of relying on the command line (git-bug would directly interact with the repo instead of running git commands) so this problem might just be irrelevant in the future.

@MichaelMure
Copy link
Owner

Also, storing credentials in the git config might just be a bad idea: #327

@sudoforge
Copy link
Author

Agreed re: #327


It's not really a bug in git -- using --global or --local are specifying a specific file. You can follow includes by using --include:

--[no-]includes
  Respect include.*  directives in config files when looking up values. Defaults to off when a specific file is given (e.g., using --file, --global, etc) and on when searching all config files.

Although, again, I think removing the file specification altogether makes the most sense.

@MichaelMure
Copy link
Owner

So just adding --include would work for now ? That would be trivial to do.

@sudoforge
Copy link
Author

That would do it, yes, but I'd suggest removing the --global and --local flags when reading instead. This is, in my opinion, a much more idiomatic way to parse configuration values, as well as allowing the user to manage where their configuration is stored.

For writes, writing to the global configuration is probably fine. Better yet, we could allow a flag to choose a particular configuration file for writing (e.g. --global, --local, --file <path>).

@MichaelMure
Copy link
Owner

@sudoforge actually this has already been done with #371 but no released version include it yet.

That said, I'm looking at migrating to go-git instead of launching CLI commands. One of the limitation is that go-git doesn't support includes. I opened an issue at go-git/go-git#110

@MichaelMure
Copy link
Owner

Closing as it's now outdated. git-bug now use go-git and doesn't use the git config for credentials anymore.

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

No branches or pull requests

2 participants