Skip to content

lfsapi/creds: teach about GIT_ASKPASS and core.askpass #2500

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.

Sign up for GitHub

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

Jump to bottom
Merged
ttaylorr merged 10 commits into from
Aug 11, 2017
Merged

lfsapi/creds: teach about GIT_ASKPASS and core.askpass #2500

ttaylorr merged 10 commits into from
Aug 11, 2017

Conversation

@ttaylorr
Copy link
Contributor

@ttaylorr ttaylorr commented Aug 10, 2017

This pull request teaches the authentication mechanism in package lfsapi to execute and share credentials with GIT_ASKPASS (and core.askpass) as described[1]:

  1. If the GIT_ASKPASS environment variable is set, the program specified by the variable is invoked. A suitable prompt is provided to the program on the command line, and the user’s input is read from its standard output.

  2. Otherwise, if the core.askPass configuration variable is set, its value is used as above.

Here's a breakdown of how things happened:

  1. 12380c2: extract a getCredentialHelper() function to anticipate additional complication in determining which set of CredentialHelper implementations to use.
  2. 04cec03: implement a CredentialHelpers type (defined as type CredentialHelpers []CredentialHelper) which acts like an io.MultiReader or io.MultiWriter for CredentialHelpers.
  3. c95fcdd: implement a AskPassCredentialHelper to execute the program and arguments given as GIT_ASKPASS or core.askpass.
  4. 9b36cc3: use the above two to properly implement GIT_ASKPASS and core.askpass.

Closes: #2285.

/cc @git-lfs/core

@ttaylorr ttaylorr added this to the v2.3.0 milestone Aug 10, 2017
@ttaylorr ttaylorr requested a review from rubyist August 10, 2017 22:58
@ttaylorr
Copy link
Contributor Author

ttaylorr commented Aug 11, 2017

Travis consistently fails the core.askPass test, most likely due to an environment variable that we don't know about, or a system-wide .gitconfig. For now, skip the test in 9da65c0, since it passes locally on my Ubuntu Trusty Tahr Docker image:

root@885fc0ba109a:/usr/local/src/github.com/git-lfs/git-lfs# lsb_release -a
No LSB modules are available.
Distributor ID: Ubuntu
Description:    Ubuntu 14.04.5 LTS
Release:        14.04
Codename:       trusty

root@885fc0ba109a:/usr/local/src/github.com/git-lfs/git-lfs# ./test/test-askpass.sh
# ...

test: askpass: push with GIT_ASKPASS ...                           OK
test: askpass: push with core.askPass ...                          OK

@ttaylorr ttaylorr merged commit 5737ed1 into master Aug 11, 2017
@ttaylorr ttaylorr deleted the git-askpass branch August 11, 2017 18:11
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Sep 10, 2024
@chrisd8088
t/t-{askpass,clone}.sh: drop checks for Travis CI
943497c 
In commit d2c1c0f of PR git-lfs#1067 the
"cloneSSL" test in what is now our t/t-clone.sh test script was changed
with the intent that the test should be skipped when it was run in the
Travis CI environment.  The commit description from that time notes that
we could not determine why the test was failing only on that platform,
so the decision was made to just ignore the test in that case.

Subsequently, similar checks were added to three other tests.  First,
the "clone ClientCert" test was introduced in PR git-lfs#1893, and as of
commit b471566 in that PR it has
included an initial check for a Travis CI environment with the intent
of skipping the test when it is run there.

Next, in commits 9da65c0 of PR git-lfs#2500
and e53e34e of PR git-lfs#2609 the "askpass:
push with core.askPass" and "askpass: push with SSH_ASKPASS" tests
in what is now our t/t-askpass.sh tests script were amended to skip
execution in the Travis CI context.

In the case of the latter two tests, this check works as designed;
however, in the case of the two tests in t/t-clone.sh, the check is
written as a simple "if $TRAVIS" shell statement, rather than one
which uses a shell test command or builtin ("test" or "[" or "[[").
As described in git-lfs#5658, the result is that when the TRAVIS variable
is undefined, these statements always succeed, and so their conditional
blocks run, meaning these tests are skipped on all platforms, at
least since we migrated our CI jobs to GitHub Actions in PR git-lfs#3808.

In previous commits in this PR we have addressed all the latent
bugs that have accumulated in these tests since this problem was
first introduced, and so we are now in a position to remove both
the valid and invalid checks for the TRAVIS environment variable
from all of our tests.  This should ensure that the full set of
tests in both t/t-askpass.sh and t/t-clone.sh run on all our CI
and build platforms.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Sep 15, 2024
@chrisd8088
t/t-{askpass,clone}.sh: drop checks for Travis CI
5b27d69 
In commit d2c1c0f of PR git-lfs#1067 the
"cloneSSL" test in what is now our t/t-clone.sh test script was changed
with the intent that the test should be skipped when it was run in the
Travis CI environment.  The commit description from that time notes that
we could not determine why the test was failing only on that platform,
so the decision was made to just ignore the test in that case.

Subsequently, similar checks were added to three other tests.  First,
the "clone ClientCert" test was introduced in PR git-lfs#1893, and as of
commit b471566 in that PR it has
included an initial check for a Travis CI environment with the intent
of skipping the test when it is run there.

Next, in commits 9da65c0 of PR git-lfs#2500
and e53e34e of PR git-lfs#2609 the "askpass:
push with core.askPass" and "askpass: push with SSH_ASKPASS" tests
in what is now our t/t-askpass.sh tests script were amended to skip
execution in the Travis CI context.

In the case of the latter two tests, this check works as designed;
however, in the case of the two tests in t/t-clone.sh, the check is
written as a simple "if $TRAVIS" shell statement, rather than one
which uses a shell test command or builtin ("test" or "[" or "[[").
As described in git-lfs#5658, the result is that when the TRAVIS variable
is undefined, these statements always succeed, and so their conditional
blocks run, meaning these tests are skipped on all platforms, at
least since we migrated our CI jobs to GitHub Actions in PR git-lfs#3808.

In previous commits in this PR we have addressed all the latent
bugs that have accumulated in these tests since this problem was
first introduced, and so we are now in a position to remove both
the valid and invalid checks for the TRAVIS environment variable
from all of our tests.  This should ensure that the full set of
tests in both t/t-askpass.sh and t/t-clone.sh run on all our CI
and build platforms.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Oct 7, 2024
@chrisd8088
t/t-{askpass,clone}.sh: drop checks for Travis CI
7c64bc0 
In commit d2c1c0f of PR git-lfs#1067 the
"cloneSSL" test in what is now our t/t-clone.sh test script was changed
with the intent that the test should be skipped when it was run in the
Travis CI environment.  The commit description from that time notes that
we could not determine why the test was failing only on that platform,
so the decision was made to just ignore the test in that case.

Subsequently, similar checks were added to three other tests.  First,
the "clone ClientCert" test was introduced in PR git-lfs#1893, and as of
commit b471566 in that PR it has
included an initial check for a Travis CI environment with the intent
of skipping the test when it is run there.

Next, in commits 9da65c0 of PR git-lfs#2500
and e53e34e of PR git-lfs#2609 the "askpass:
push with core.askPass" and "askpass: push with SSH_ASKPASS" tests
in what is now our t/t-askpass.sh tests script were amended to skip
execution in the Travis CI context.

In the case of the latter two tests, this check works as designed;
however, in the case of the two tests in t/t-clone.sh, the check is
written as a simple "if $TRAVIS" shell statement, rather than one
which uses a shell test command or builtin ("test" or "[" or "[[").
As described in git-lfs#5658, the result is that when the TRAVIS variable
is undefined, these statements always succeed, and so their conditional
blocks run, meaning these tests are skipped on all platforms, at
least since we migrated our CI jobs to GitHub Actions in PR git-lfs#3808.

In previous commits in this PR we have addressed all the latent
bugs that have accumulated in these tests since this problem was
first introduced, and so we are now in a position to remove both
the valid and invalid checks for the TRAVIS environment variable
from all of our tests.  This should ensure that the full set of
tests in both t/t-askpass.sh and t/t-clone.sh run on all our CI
and build platforms.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Oct 9, 2024
@chrisd8088
t/t-{askpass,clone}.sh: drop checks for Travis CI
9ba4341 
In commit d2c1c0f of PR git-lfs#1067 the
"cloneSSL" test in what is now our t/t-clone.sh test script was changed
with the intent that the test should be skipped when it was run by the
Travis CI service.  The commit description from that time notes that
we could not determine why the test was failing on just that platform,
so the decision was made to simply ignore the test in the Travis CI
environment.

Subsequently, similar checks were added to three other tests.  First,
the "clone ClientCert" test was introduced in PR git-lfs#1893, and since
commit b471566 in that PR it has
included an initial check for a Travis CI environment with the intent
of skipping the test when it is run there.

Next, in commits 9da65c0 of PR git-lfs#2500
and e53e34e of PR git-lfs#2609 the "askpass:
push with core.askPass" and "askpass: push with SSH_ASKPASS" tests
in what is now our t/t-askpass.sh test script were amended to also
skip executing in the Travis CI context.

In the case of the latter two tests, this check works as designed;
however, in the case of the two tests in t/t-clone.sh, the check is
written as a simple "if $TRAVIS" shell statement, rather than one
which uses a shell test command or builtin (i.e., "test" or "[" or
"[[").  As described in git-lfs#5658, the result is that when the TRAVIS
variable is undefined, these statements always succeed, and so their
conditional blocks run, meaning these tests are skipped on every
platform and system.

In previous commits in this PR we have addressed all the latent bugs
that have accumulated in these tests since this problem was first
introduced, and so we are now in a position to remove both the valid
and invalid checks for the TRAVIS environment variable from all four
of the tests.  This will ensure that our CI jobs always run the
entire set of tests from both of the t/t-askpass.sh and t/t-clone.sh
test scripts.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Oct 9, 2024
@chrisd8088
t/t-{askpass,clone}.sh: drop checks for Travis CI
e5c1081 
In commit d2c1c0f of PR git-lfs#1067 the
"cloneSSL" test in what is now our t/t-clone.sh test script was changed
with the intent that the test should be skipped when it was run by the
Travis CI service.  The commit description from that time notes that
we could not determine why the test was failing on just that platform,
so the decision was made to simply ignore the test in the Travis CI
environment.

Subsequently, similar checks were added to three other tests.  First,
the "clone ClientCert" test was introduced in PR git-lfs#1893, and since
commit b471566 in that PR it has
included an initial check for a Travis CI environment with the intent
of skipping the test when it is run there.

Next, in commits 9da65c0 of PR git-lfs#2500
and e53e34e of PR git-lfs#2609 the "askpass:
push with core.askPass" and "askpass: push with SSH_ASKPASS" tests
in what is now our t/t-askpass.sh test script were amended to also
skip executing in the Travis CI context.

In the case of the latter two tests, this check works as designed;
however, in the case of the two tests in t/t-clone.sh, the check is
written as a simple "if $TRAVIS" shell statement, rather than one
which uses a shell test command or builtin (i.e., "test" or "[" or
"[[").  As described in git-lfs#5658, the result is that when the TRAVIS
variable is undefined, these statements always succeed, and so their
conditional blocks run, meaning these tests are skipped on every
platform and system.

In previous commits in this PR we have addressed all the latent bugs
that have accumulated in these tests since this problem was first
introduced, and so we are now in a position to remove both the valid
and invalid checks for the TRAVIS environment variable from all four
of the tests.  This will ensure that our CI jobs always run the
entire set of tests from both of the t/t-askpass.sh and t/t-clone.sh
test scripts.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Oct 29, 2024
@chrisd8088
t/t-{askpass,clone}.sh: drop checks for Travis CI
9ea802e 
In commit d2c1c0f of PR git-lfs#1067 the
"cloneSSL" test in what is now our t/t-clone.sh test script was changed
with the intent that the test should be skipped when it was run by the
Travis CI service.  The commit description from that time notes that
we could not determine why the test was failing on just that platform,
so the decision was made to simply ignore the test in the Travis CI
environment.

Subsequently, similar checks were added to three other tests.  First,
the "clone ClientCert" test was introduced in PR git-lfs#1893, and since
commit b471566 in that PR it has
included an initial check for a Travis CI environment with the intent
of skipping the test when it is run there.

Next, in commits 9da65c0 of PR git-lfs#2500
and e53e34e of PR git-lfs#2609 the "askpass:
push with core.askPass" and "askpass: push with SSH_ASKPASS" tests
in what is now our t/t-askpass.sh test script were amended to also
skip executing in the Travis CI context.

In the case of the latter two tests, this check works as designed;
however, in the case of the two tests in t/t-clone.sh, the check is
written as a simple "if $TRAVIS" shell statement, rather than one
which uses a shell test command or builtin (i.e., "test" or "[" or
"[[").  As described in git-lfs#5658, the result is that when the TRAVIS
variable is undefined, these statements always succeed, and so their
conditional blocks run, meaning these tests are skipped on every
platform and system.

In previous commits in this PR we have addressed all the latent bugs
that have accumulated in these tests since this problem was first
introduced, and so we are now in a position to remove both the valid
and invalid checks for the TRAVIS environment variable from all four
of the tests.  This will ensure that our CI jobs always run the
entire set of tests from both of the t/t-askpass.sh and t/t-clone.sh
test scripts.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@rubyist rubyist rubyist approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

review

Projects

None yet

Milestone

v2.3.0

Development

Successfully merging this pull request may close these issues.

3 participants

@ttaylorr @rubyist