From bc7b824c723b973e3bb11b3174c4a61e32a434d6 Mon Sep 17 00:00:00 2001
From: Curtis Vogt <curtis.vogt@gmail.com>
Date: Wed, 20 Sep 2017 22:30:27 -0500
Subject: [PATCH 01/18] Rename internal variable creds to cred

---
 base/libgit2/callbacks.jl | 42 +++++++++++++++++++--------------------
 1 file changed, 21 insertions(+), 21 deletions(-)

diff --git a/base/libgit2/callbacks.jl b/base/libgit2/callbacks.jl
index 196d8651fcfbe..c39e638f7217c 100644
--- a/base/libgit2/callbacks.jl
+++ b/base/libgit2/callbacks.jl
@@ -49,11 +49,11 @@ function user_abort()
 end
 
 function authenticate_ssh(libgit2credptr::Ptr{Ptr{Void}}, p::CredentialPayload, username_ptr)
-    creds = Base.get(p.credential)::SSHCredentials
+    cred = Base.get(p.credential)::SSHCredentials
 
     # Reset password on sucessive calls
     if !p.first_pass
-        creds.pass = ""
+        cred.pass = ""
     end
 
     # first try ssh-agent if credentials support its usage
@@ -69,29 +69,29 @@ function authenticate_ssh(libgit2credptr::Ptr{Ptr{Void}}, p::CredentialPayload,
 
     privatekey = Base.get(ENV, "SSH_KEY_PATH") do
         default = joinpath(homedir(), ".ssh", "id_rsa")
-        if isempty(creds.prvkey) && isfile(default)
+        if isempty(cred.prvkey) && isfile(default)
             default
         else
-            creds.prvkey
+            cred.prvkey
         end
     end
 
     publickey = Base.get(ENV, "SSH_PUB_KEY_PATH") do
         default = privatekey * ".pub"
-        if isempty(creds.pubkey) && isfile(default)
+        if isempty(cred.pubkey) && isfile(default)
             default
         else
-            creds.pubkey
+            cred.pubkey
         end
     end
 
-    passphrase = Base.get(ENV, "SSH_KEY_PASS", creds.pass)
+    passphrase = Base.get(ENV, "SSH_KEY_PASS", cred.pass)
 
     if p.allow_prompt
         # if username is not provided or empty, then prompt for it
         if isempty(username)
             prompt_url = git_url(scheme=p.scheme, host=p.host)
-            response = Base.prompt("Username for '$prompt_url'", default=creds.user)
+            response = Base.prompt("Username for '$prompt_url'", default=cred.user)
             isnull(response) && return user_abort()
             username = unsafe_get(response)
         end
@@ -106,7 +106,7 @@ function authenticate_ssh(libgit2credptr::Ptr{Ptr{Void}}, p::CredentialPayload,
             privatekey = unsafe_get(response)
 
             # Only update the public key if the private key changed
-            if privatekey != creds.prvkey
+            if privatekey != cred.prvkey
                 publickey = privatekey * ".pub"
             end
         end
@@ -135,30 +135,30 @@ function authenticate_ssh(libgit2credptr::Ptr{Ptr{Void}}, p::CredentialPayload,
             end
         end
 
-        creds.user = username # save credentials
-        creds.prvkey = privatekey # save credentials
-        creds.pubkey = publickey # save credentials
-        creds.pass = passphrase
+        cred.user = username # save credentials
+        cred.prvkey = privatekey # save credentials
+        cred.pubkey = publickey # save credentials
+        cred.pass = passphrase
     elseif !p.first_pass
         return Cint(Error.EAUTH)
     end
 
     return ccall((:git_cred_ssh_key_new, :libgit2), Cint,
                  (Ptr{Ptr{Void}}, Cstring, Cstring, Cstring, Cstring),
-                 libgit2credptr, creds.user, creds.pubkey, creds.prvkey, creds.pass)
+                 libgit2credptr, cred.user, cred.pubkey, cred.prvkey, cred.pass)
 end
 
 function authenticate_userpass(libgit2credptr::Ptr{Ptr{Void}}, p::CredentialPayload)
-    creds = Base.get(p.credential)::UserPasswordCredentials
+    cred = Base.get(p.credential)::UserPasswordCredentials
 
     # Reset password on sucessive calls
     if !p.first_pass
-        creds.pass = ""
+        cred.pass = ""
     end
 
     if p.allow_prompt
-        username = creds.user
-        userpass = creds.pass
+        username = cred.user
+        userpass = cred.pass
         if isempty(username) || isempty(userpass)
             prompt_url = git_url(scheme=p.scheme, host=p.host)
             if Sys.iswindows()
@@ -180,15 +180,15 @@ function authenticate_userpass(libgit2credptr::Ptr{Ptr{Void}}, p::CredentialPayl
                 isempty(userpass) && return user_abort()  # Ambiguous if EOF or newline
             end
         end
-        creds.user = username # save credentials
-        creds.pass = userpass # save credentials
+        cred.user = username # save credentials
+        cred.pass = userpass # save credentials
     elseif !p.first_pass
         return Cint(Error.EAUTH)
     end
 
     return ccall((:git_cred_userpass_plaintext_new, :libgit2), Cint,
                  (Ptr{Ptr{Void}}, Cstring, Cstring),
-                 libgit2credptr, creds.user, creds.pass)
+                 libgit2credptr, cred.user, cred.pass)
 end
 
 

From 3894ef1ecc4e75c317797998ca358aa7116df894 Mon Sep 17 00:00:00 2001
From: Curtis Vogt <curtis.vogt@gmail.com>
Date: Mon, 28 Aug 2017 09:41:58 -0500
Subject: [PATCH 02/18] Save modifications directory to temp credential

The temporary credential object is independent from any cached or stored
credential until it is explicitly approved.
---
 base/libgit2/callbacks.jl | 71 ++++++++++++++++++---------------------
 1 file changed, 32 insertions(+), 39 deletions(-)

diff --git a/base/libgit2/callbacks.jl b/base/libgit2/callbacks.jl
index c39e638f7217c..4645d33a7353f 100644
--- a/base/libgit2/callbacks.jl
+++ b/base/libgit2/callbacks.jl
@@ -65,9 +65,7 @@ function authenticate_ssh(libgit2credptr::Ptr{Ptr{Void}}, p::CredentialPayload,
         err == 0 && return Cint(0)
     end
 
-    username = username_ptr != Cstring(C_NULL) ? unsafe_string(username_ptr) : ""
-
-    privatekey = Base.get(ENV, "SSH_KEY_PATH") do
+    cred.prvkey = Base.get(ENV, "SSH_KEY_PATH") do
         default = joinpath(homedir(), ".ssh", "id_rsa")
         if isempty(cred.prvkey) && isfile(default)
             default
@@ -76,8 +74,8 @@ function authenticate_ssh(libgit2credptr::Ptr{Ptr{Void}}, p::CredentialPayload,
         end
     end
 
-    publickey = Base.get(ENV, "SSH_PUB_KEY_PATH") do
-        default = privatekey * ".pub"
+    cred.pubkey = Base.get(ENV, "SSH_PUB_KEY_PATH") do
+        default = cred.prvkey * ".pub"
         if isempty(cred.pubkey) && isfile(default)
             default
         else
@@ -85,60 +83,59 @@ function authenticate_ssh(libgit2credptr::Ptr{Ptr{Void}}, p::CredentialPayload,
         end
     end
 
-    passphrase = Base.get(ENV, "SSH_KEY_PASS", cred.pass)
+    cred.pass = Base.get(ENV, "SSH_KEY_PASS", cred.pass)
 
     if p.allow_prompt
         # if username is not provided or empty, then prompt for it
+        username = username_ptr != Cstring(C_NULL) ? unsafe_string(username_ptr) : ""
         if isempty(username)
             prompt_url = git_url(scheme=p.scheme, host=p.host)
             response = Base.prompt("Username for '$prompt_url'", default=cred.user)
             isnull(response) && return user_abort()
-            username = unsafe_get(response)
+            cred.user = unsafe_get(response)
+        else
+            cred.user = username
         end
 
-        prompt_url = git_url(scheme=p.scheme, host=p.host, username=username)
+        prompt_url = git_url(scheme=p.scheme, host=p.host, username=cred.user)
 
         # For SSH we need a private key location
-        if !isfile(privatekey)
+        if !isfile(cred.prvkey)
             response = Base.prompt("Private key location for '$prompt_url'",
-                default=privatekey)
+                default=cred.prvkey)
             isnull(response) && return user_abort()
-            privatekey = unsafe_get(response)
+            last_private_key = cred.prvkey
+            cred.prvkey = unsafe_get(response)
 
             # Only update the public key if the private key changed
-            if privatekey != cred.prvkey
-                publickey = privatekey * ".pub"
+            if cred.prvkey != last_private_key
+                cred.pubkey = cred.prvkey * ".pub"
             end
         end
 
         # For SSH we need a public key location. Avoid asking about the public key as
         # typically this will just annoy users.
-        if !isfile(publickey) && isfile(privatekey)
+        if !isfile(cred.pubkey) && isfile(cred.prvkey)
             response = Base.prompt("Public key location for '$prompt_url'",
-                default=publickey)
+                default=cred.pubkey)
             isnull(response) && return user_abort()
-            publickey = unsafe_get(response)
+            cred.pubkey = unsafe_get(response)
         end
 
-        if isempty(passphrase) && is_passphrase_required(privatekey)
+        if isempty(cred.pass) && is_passphrase_required(cred.prvkey)
             if Sys.iswindows()
                 response = Base.winprompt(
                     "Your SSH Key requires a password, please enter it now:",
-                    "Passphrase required", privatekey; prompt_username=false)
+                    "Passphrase required", cred.prvkey; prompt_username=false)
                 isnull(response) && return user_abort()
-                passphrase = unsafe_get(response)[2]
+                cred.pass = unsafe_get(response)[2]
             else
-                response = Base.prompt("Passphrase for $privatekey", password=true)
+                response = Base.prompt("Passphrase for $(cred.prvkey)", password=true)
                 isnull(response) && return user_abort()
-                passphrase = unsafe_get(response)
-                isempty(passphrase) && return user_abort()  # Ambiguous if EOF or newline
+                cred.pass = unsafe_get(response)
+                isempty(cred.pass) && return user_abort()  # Ambiguous if EOF or newline
             end
         end
-
-        cred.user = username # save credentials
-        cred.prvkey = privatekey # save credentials
-        cred.pubkey = publickey # save credentials
-        cred.pass = passphrase
     elseif !p.first_pass
         return Cint(Error.EAUTH)
     end
@@ -157,31 +154,27 @@ function authenticate_userpass(libgit2credptr::Ptr{Ptr{Void}}, p::CredentialPayl
     end
 
     if p.allow_prompt
-        username = cred.user
-        userpass = cred.pass
-        if isempty(username) || isempty(userpass)
+        if isempty(cred.user) || isempty(cred.pass)
             prompt_url = git_url(scheme=p.scheme, host=p.host)
             if Sys.iswindows()
                 response = Base.winprompt(
                     "Please enter your credentials for '$prompt_url'", "Credentials required",
-                    isempty(username) ? p.username : username; prompt_username=true)
+                    isempty(cred.user) ? p.username : cred.user; prompt_username=true)
                 isnull(response) && return user_abort()
-                username, userpass = unsafe_get(response)
+                cred.user, cred.pass = unsafe_get(response)
             else
                 response = Base.prompt("Username for '$prompt_url'",
-                    default=isempty(username) ? p.username : username)
+                    default=isempty(cred.user) ? p.username : cred.user)
                 isnull(response) && return user_abort()
-                username = unsafe_get(response)
+                cred.user = unsafe_get(response)
 
-                prompt_url = git_url(scheme=p.scheme, host=p.host, username=username)
+                prompt_url = git_url(scheme=p.scheme, host=p.host, username=cred.user)
                 response = Base.prompt("Password for '$prompt_url'", password=true)
                 isnull(response) && return user_abort()
-                userpass = unsafe_get(response)
-                isempty(userpass) && return user_abort()  # Ambiguous if EOF or newline
+                cred.pass = unsafe_get(response)
+                isempty(cred.pass) && return user_abort()  # Ambiguous if EOF or newline
             end
         end
-        cred.user = username # save credentials
-        cred.pass = userpass # save credentials
     elseif !p.first_pass
         return Cint(Error.EAUTH)
     end

From 5d10ce45c2b7896f1e8b98089c922b8b1b2fe603 Mon Sep 17 00:00:00 2001
From: Curtis Vogt <curtis.vogt@gmail.com>
Date: Mon, 28 Aug 2017 15:27:14 -0500
Subject: [PATCH 03/18] Create use_env check

---
 base/libgit2/callbacks.jl | 32 +++++++++++++++++---------------
 base/libgit2/types.jl     |  2 ++
 2 files changed, 19 insertions(+), 15 deletions(-)

diff --git a/base/libgit2/callbacks.jl b/base/libgit2/callbacks.jl
index 4645d33a7353f..2978ef54e76d1 100644
--- a/base/libgit2/callbacks.jl
+++ b/base/libgit2/callbacks.jl
@@ -65,25 +65,27 @@ function authenticate_ssh(libgit2credptr::Ptr{Ptr{Void}}, p::CredentialPayload,
         err == 0 && return Cint(0)
     end
 
-    cred.prvkey = Base.get(ENV, "SSH_KEY_PATH") do
-        default = joinpath(homedir(), ".ssh", "id_rsa")
-        if isempty(cred.prvkey) && isfile(default)
-            default
-        else
-            cred.prvkey
+    if p.use_env
+        cred.prvkey = Base.get(ENV, "SSH_KEY_PATH") do
+            default = joinpath(homedir(), ".ssh", "id_rsa")
+            if isempty(cred.prvkey) && isfile(default)
+                default
+            else
+                cred.prvkey
+            end
         end
-    end
 
-    cred.pubkey = Base.get(ENV, "SSH_PUB_KEY_PATH") do
-        default = cred.prvkey * ".pub"
-        if isempty(cred.pubkey) && isfile(default)
-            default
-        else
-            cred.pubkey
+        cred.pubkey = Base.get(ENV, "SSH_PUB_KEY_PATH") do
+            default = cred.prvkey * ".pub"
+            if isempty(cred.pubkey) && isfile(default)
+                default
+            else
+                cred.pubkey
+            end
         end
-    end
 
-    cred.pass = Base.get(ENV, "SSH_KEY_PASS", cred.pass)
+        cred.pass = Base.get(ENV, "SSH_KEY_PASS", cred.pass)
+    end
 
     if p.allow_prompt
         # if username is not provided or empty, then prompt for it
diff --git a/base/libgit2/types.jl b/base/libgit2/types.jl
index 564b0536cb81b..7def5fbafebe9 100644
--- a/base/libgit2/types.jl
+++ b/base/libgit2/types.jl
@@ -1177,6 +1177,7 @@ mutable struct CredentialPayload <: Payload
     credential::Nullable{AbstractCredentials}
     first_pass::Bool
     use_ssh_agent::Bool
+    use_env::Bool
     url::String
     scheme::String
     username::String
@@ -1211,6 +1212,7 @@ function reset!(p::CredentialPayload)
     p.credential = Nullable{AbstractCredentials}()
     p.first_pass = true
     p.use_ssh_agent = p.allow_ssh_agent
+    p.use_env = true
     p.url = ""
     p.scheme = ""
     p.username = ""

From 822b45e99bdff4dd6e223604c78886cd563a7602 Mon Sep 17 00:00:00 2001
From: Curtis Vogt <curtis.vogt@gmail.com>
Date: Mon, 28 Aug 2017 09:02:06 -0500
Subject: [PATCH 04/18] Create isfilled function

---
 base/libgit2/types.jl      | 16 ++++++++++++++++
 doc/src/devdocs/libgit2.md |  1 +
 2 files changed, 17 insertions(+)

diff --git a/base/libgit2/types.jl b/base/libgit2/types.jl
index 7def5fbafebe9..eb15c76d28994 100644
--- a/base/libgit2/types.jl
+++ b/base/libgit2/types.jl
@@ -1062,6 +1062,13 @@ import Base.securezero!
 "Abstract credentials payload"
 abstract type AbstractCredentials end
 
+"""
+    isfilled(cred::AbstractCredentials) -> Bool
+
+Verifies that a credential is ready for use in authentication.
+"""
+isfilled(::AbstractCredentials)
+
 "Credentials that support only `user` and `password` parameters"
 mutable struct UserPasswordCredentials <: AbstractCredentials
     user::String
@@ -1093,6 +1100,10 @@ function Base.:(==)(a::UserPasswordCredentials, b::UserPasswordCredentials)
     a.user == b.user && a.pass == b.pass
 end
 
+function isfilled(cred::UserPasswordCredentials)
+    !isempty(cred.user) && !isempty(cred.pass)
+end
+
 "SSH credentials type"
 mutable struct SSHCredentials <: AbstractCredentials
     user::String
@@ -1130,6 +1141,11 @@ function Base.:(==)(a::SSHCredentials, b::SSHCredentials)
     a.user == b.user && a.pass == b.pass && a.prvkey == b.prvkey && a.pubkey == b.pubkey
 end
 
+function isfilled(cred::SSHCredentials)
+    !isempty(cred.user) && isfile(cred.prvkey) && isfile(cred.pubkey) &&
+    (!isempty(cred.pass) || !is_passphrase_required(cred.prvkey))
+end
+
 "Credentials that support caching"
 struct CachedCredentials <: AbstractCredentials
     cred::Dict{String,AbstractCredentials}
diff --git a/doc/src/devdocs/libgit2.md b/doc/src/devdocs/libgit2.md
index f31eed7e6b396..e8e95d82d1ab1 100644
--- a/doc/src/devdocs/libgit2.md
+++ b/doc/src/devdocs/libgit2.md
@@ -103,6 +103,7 @@ Base.LibGit2.isbinary
 Base.LibGit2.iscommit
 Base.LibGit2.isdiff
 Base.LibGit2.isdirty
+Base.LibGit2.isfilled
 Base.LibGit2.isorphan
 Base.LibGit2.isset
 Base.LibGit2.iszero

From 335c5aa534da542984a6e90f7cfd28754a532a95 Mon Sep 17 00:00:00 2001
From: Curtis Vogt <curtis.vogt@gmail.com>
Date: Mon, 28 Aug 2017 17:14:33 -0500
Subject: [PATCH 05/18] Error when credentials are not modified

Credentials should be used when they are filled in and alternatives
should be tried when they are not. These changes allow use to enable
the commented out tests which use invalid credentials.

Note when prompting is disabled we now attempt to authenticate using the
default ~/.ssh/id_rsa key.
---
 base/libgit2/callbacks.jl | 73 +++++++++++++++++++++++----------------
 test/libgit2-online.jl    |  8 +----
 test/libgit2.jl           | 18 +++++-----
 3 files changed, 53 insertions(+), 46 deletions(-)

diff --git a/base/libgit2/callbacks.jl b/base/libgit2/callbacks.jl
index 2978ef54e76d1..d2a36362231aa 100644
--- a/base/libgit2/callbacks.jl
+++ b/base/libgit2/callbacks.jl
@@ -50,9 +50,12 @@ end
 
 function authenticate_ssh(libgit2credptr::Ptr{Ptr{Void}}, p::CredentialPayload, username_ptr)
     cred = Base.get(p.credential)::SSHCredentials
+    revised = false
 
-    # Reset password on sucessive calls
-    if !p.first_pass
+    # Use a filled credential as-is on the first pass. Reset password on sucessive calls.
+    if p.first_pass && isfilled(cred)
+        revised = true
+    elseif !p.first_pass
         cred.pass = ""
     end
 
@@ -65,7 +68,7 @@ function authenticate_ssh(libgit2credptr::Ptr{Ptr{Void}}, p::CredentialPayload,
         err == 0 && return Cint(0)
     end
 
-    if p.use_env
+    if p.use_env && (!revised || !isfilled(cred))
         cred.prvkey = Base.get(ENV, "SSH_KEY_PATH") do
             default = joinpath(homedir(), ".ssh", "id_rsa")
             if isempty(cred.prvkey) && isfile(default)
@@ -85,9 +88,12 @@ function authenticate_ssh(libgit2credptr::Ptr{Ptr{Void}}, p::CredentialPayload,
         end
 
         cred.pass = Base.get(ENV, "SSH_KEY_PASS", cred.pass)
+
+        revised = true
+        p.use_env = false
     end
 
-    if p.allow_prompt
+    if p.allow_prompt && (!revised || !isfilled(cred))
         # if username is not provided or empty, then prompt for it
         username = username_ptr != Cstring(C_NULL) ? unsafe_string(username_ptr) : ""
         if isempty(username)
@@ -102,7 +108,7 @@ function authenticate_ssh(libgit2credptr::Ptr{Ptr{Void}}, p::CredentialPayload,
         prompt_url = git_url(scheme=p.scheme, host=p.host, username=cred.user)
 
         # For SSH we need a private key location
-        if !isfile(cred.prvkey)
+        if !isfile(cred.prvkey) || !revised
             response = Base.prompt("Private key location for '$prompt_url'",
                 default=cred.prvkey)
             isnull(response) && return user_abort()
@@ -138,7 +144,11 @@ function authenticate_ssh(libgit2credptr::Ptr{Ptr{Void}}, p::CredentialPayload,
                 isempty(cred.pass) && return user_abort()  # Ambiguous if EOF or newline
             end
         end
-    elseif !p.first_pass
+
+        revised = true
+    end
+
+    if !revised
         return Cint(Error.EAUTH)
     end
 
@@ -149,35 +159,40 @@ end
 
 function authenticate_userpass(libgit2credptr::Ptr{Ptr{Void}}, p::CredentialPayload)
     cred = Base.get(p.credential)::UserPasswordCredentials
+    revised = false
 
-    # Reset password on sucessive calls
-    if !p.first_pass
+    # Use a filled credential as-is on the first pass. Reset password on sucessive calls.
+    if p.first_pass && isfilled(cred)
+        revised = true
+    elseif !p.first_pass
         cred.pass = ""
     end
 
-    if p.allow_prompt
-        if isempty(cred.user) || isempty(cred.pass)
-            prompt_url = git_url(scheme=p.scheme, host=p.host)
-            if Sys.iswindows()
-                response = Base.winprompt(
-                    "Please enter your credentials for '$prompt_url'", "Credentials required",
-                    isempty(cred.user) ? p.username : cred.user; prompt_username=true)
-                isnull(response) && return user_abort()
-                cred.user, cred.pass = unsafe_get(response)
-            else
-                response = Base.prompt("Username for '$prompt_url'",
-                    default=isempty(cred.user) ? p.username : cred.user)
-                isnull(response) && return user_abort()
-                cred.user = unsafe_get(response)
+    if p.allow_prompt && (!revised || !isfilled(cred))
+        prompt_url = git_url(scheme=p.scheme, host=p.host)
+        if Sys.iswindows()
+            response = Base.winprompt(
+                "Please enter your credentials for '$prompt_url'", "Credentials required",
+                isempty(cred.user) ? p.username : cred.user; prompt_username=true)
+            isnull(response) && return user_abort()
+            cred.user, cred.pass = unsafe_get(response)
+        else
+            response = Base.prompt("Username for '$prompt_url'",
+                default=isempty(cred.user) ? p.username : cred.user)
+            isnull(response) && return user_abort()
+            cred.user = unsafe_get(response)
 
-                prompt_url = git_url(scheme=p.scheme, host=p.host, username=cred.user)
-                response = Base.prompt("Password for '$prompt_url'", password=true)
-                isnull(response) && return user_abort()
-                cred.pass = unsafe_get(response)
-                isempty(cred.pass) && return user_abort()  # Ambiguous if EOF or newline
-            end
+            prompt_url = git_url(scheme=p.scheme, host=p.host, username=cred.user)
+            response = Base.prompt("Password for '$prompt_url'", password=true)
+            isnull(response) && return user_abort()
+            cred.pass = unsafe_get(response)
+            isempty(cred.pass) && return user_abort()  # Ambiguous if EOF or newline
         end
-    elseif !p.first_pass
+
+        revised = true
+    end
+
+    if !revised
         return Cint(Error.EAUTH)
     end
 
diff --git a/test/libgit2-online.jl b/test/libgit2-online.jl
index 5424ef31bd9dc..39c31a5b1946d 100644
--- a/test/libgit2-online.jl
+++ b/test/libgit2-online.jl
@@ -43,13 +43,7 @@ mktempdir() do dir
                 error("unexpected")
             catch ex
                 @test isa(ex, LibGit2.Error.GitError)
-                if Sys.iswindows() && LibGit2.version() >= v"0.26.0"
-                    # see #22681 and https://github.com/libgit2/libgit2/pull/4055
-                    @test_broken ex.code == LibGit2.Error.EAUTH
-                    @test ex.code == LibGit2.Error.ERROR
-                else
-                    @test ex.code == LibGit2.Error.EAUTH
-                end
+                @test ex.code == LibGit2.Error.EAUTH
             end
         end
     end
diff --git a/test/libgit2.jl b/test/libgit2.jl
index ac290a189d516..ab84089a5aa94 100644
--- a/test/libgit2.jl
+++ b/test/libgit2.jl
@@ -1734,7 +1734,7 @@ mktempdir() do dir
                 # could also just re-call the credential callback like they do for HTTP.
                 challenges = [
                     "Passphrase for $valid_p_key:" => "foo\n",
-                    # "Private key location for 'git@github.com' [$valid_p_key]:" => "\n",
+                    "Private key location for 'git@github.com' [$valid_p_key]:" => "\n",
                     "Passphrase for $valid_p_key:" => "$passphrase\n",
                 ]
                 err, auth_attempts = challenge_prompt(ssh_p_ex, challenges)
@@ -1795,6 +1795,7 @@ mktempdir() do dir
                 challenges = [
                     "Username for 'github.com':" => "foo\n",
                     "Username for 'github.com' [foo]:" => "\n",
+                    "Private key location for 'foo@github.com' [$valid_key]:" => "\n",
                     "Username for 'github.com' [foo]:" => "\x04",  # Need to manually abort
                 ]
                 err, auth_attempts = challenge_prompt(ssh_u_ex, challenges)
@@ -1802,7 +1803,7 @@ mktempdir() do dir
                 @test auth_attempts == 3
 
                 # Credential callback is given an empty string in the `username_ptr`
-                # instead of the typical C_NULL.
+                # instead of the C_NULL in the other missing username tests.
                 ssh_user_empty_ex = gen_ex(valid_cred, username="")
                 challenges = [
                     "Username for 'github.com':" => "$username\n",
@@ -1859,12 +1860,10 @@ mktempdir() do dir
                 @test auth_attempts == 2
             end
 
-            # TODO: Tests are currently broken. Credential callback currently infinite loops
-            # and never prompts user to change private keys.
-            #=
             # Explicitly setting these env variables to an existing but invalid key pair
             # means the user will be given a prompt with that defaults to the given values.
-            withenv("SSH_KEY_PATH" => invalid_key, "SSH_PUB_KEY_PATH" => invalid_key * ".pub") do
+            withenv("SSH_KEY_PATH" => invalid_key,
+                    "SSH_PUB_KEY_PATH" => invalid_key * ".pub") do
                 challenges = [
                     "Private key location for 'git@github.com' [$invalid_key]:" => "$valid_key\n",
                 ]
@@ -1872,7 +1871,6 @@ mktempdir() do dir
                 @test err == git_ok
                 @test auth_attempts == 2
             end
-            =#
 
             # Explicitly set the public key ENV variable to a non-existent file.
             withenv("SSH_KEY_PATH" => valid_key,
@@ -1903,7 +1901,7 @@ mktempdir() do dir
                 ]
                 err, auth_attempts = challenge_prompt(ssh_ex, challenges)
                 @test err == git_ok
-                @test auth_attempts == 2
+                @test auth_attempts == 1
             end
             =#
         end
@@ -1987,7 +1985,7 @@ mktempdir() do dir
             ex = gen_ex(username="")
             err, auth_attempts = challenge_prompt(ex, [])
             @test err == eauth_error
-            @test auth_attempts == 2
+            @test auth_attempts == 3
 
             # A null username_ptr passed into `git_cred_ssh_key_from_agent` can cause a
             # segfault.
@@ -2026,7 +2024,7 @@ mktempdir() do dir
             ex = gen_ex(invalid_cred, allow_prompt=false)
             err, auth_attempts = challenge_prompt(ex, [])
             @test err == eauth_error
-            @test auth_attempts == 2
+            @test auth_attempts == 3
         end
 
         @testset "HTTPS explicit credentials" begin

From d6e0c5b66741812ded0a3b7809f24b849355578c Mon Sep 17 00:00:00 2001
From: Curtis Vogt <curtis.vogt@gmail.com>
Date: Tue, 29 Aug 2017 09:11:46 -0500
Subject: [PATCH 06/18] Prompt for public key when invalid

Previously if the public key file exists and the private key was
correct we could get into a scenario where we would never prompt for the
public key to be changed.
---
 base/libgit2/callbacks.jl | 6 ++++--
 test/libgit2.jl           | 6 +-----
 2 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/base/libgit2/callbacks.jl b/base/libgit2/callbacks.jl
index d2a36362231aa..a153b11241ae9 100644
--- a/base/libgit2/callbacks.jl
+++ b/base/libgit2/callbacks.jl
@@ -108,11 +108,11 @@ function authenticate_ssh(libgit2credptr::Ptr{Ptr{Void}}, p::CredentialPayload,
         prompt_url = git_url(scheme=p.scheme, host=p.host, username=cred.user)
 
         # For SSH we need a private key location
+        last_private_key = cred.prvkey
         if !isfile(cred.prvkey) || !revised
             response = Base.prompt("Private key location for '$prompt_url'",
                 default=cred.prvkey)
             isnull(response) && return user_abort()
-            last_private_key = cred.prvkey
             cred.prvkey = unsafe_get(response)
 
             # Only update the public key if the private key changed
@@ -123,13 +123,15 @@ function authenticate_ssh(libgit2credptr::Ptr{Ptr{Void}}, p::CredentialPayload,
 
         # For SSH we need a public key location. Avoid asking about the public key as
         # typically this will just annoy users.
-        if !isfile(cred.pubkey) && isfile(cred.prvkey)
+        stale = !p.first_pass && cred.prvkey == last_private_key && cred.pubkey != cred.prvkey * ".pub"
+        if isfile(cred.prvkey) && (stale || !isfile(cred.pubkey))
             response = Base.prompt("Public key location for '$prompt_url'",
                 default=cred.pubkey)
             isnull(response) && return user_abort()
             cred.pubkey = unsafe_get(response)
         end
 
+        # Ask for a passphrase when the private key exists and requires a passphrase
         if isempty(cred.pass) && is_passphrase_required(cred.prvkey)
             if Sys.iswindows()
                 response = Base.winprompt(
diff --git a/test/libgit2.jl b/test/libgit2.jl
index ab84089a5aa94..980bca1acb173 100644
--- a/test/libgit2.jl
+++ b/test/libgit2.jl
@@ -1886,9 +1886,6 @@ mktempdir() do dir
                 @test auth_attempts == 1
             end
 
-            # TODO: Tests are currently broken. Credential callback currently infinite loops
-            # and never prompts user to change private keys.
-            #=
             # Explicitly set the public key ENV variable to a public key that doesn't match
             # the private key.
             withenv("SSH_KEY_PATH" => valid_key,
@@ -1901,9 +1898,8 @@ mktempdir() do dir
                 ]
                 err, auth_attempts = challenge_prompt(ssh_ex, challenges)
                 @test err == git_ok
-                @test auth_attempts == 1
+                @test auth_attempts == 2
             end
-            =#
         end
 
         @testset "HTTPS credential prompt" begin

From 3f5b1f6b40b18c7354607bd466095709cd8640d2 Mon Sep 17 00:00:00 2001
From: Curtis Vogt <curtis.vogt@gmail.com>
Date: Tue, 29 Aug 2017 15:32:21 -0500
Subject: [PATCH 07/18] Add prompt limit

Note the prompt limit is an EAUTH error since hitting the prompt limit
should cause saved credential to be rejected.
---
 base/libgit2/callbacks.jl | 22 +++++++++++++++++-----
 base/libgit2/types.jl     |  3 +++
 test/libgit2.jl           | 35 +++++++++++++++++++++++++++++++----
 3 files changed, 51 insertions(+), 9 deletions(-)

diff --git a/base/libgit2/callbacks.jl b/base/libgit2/callbacks.jl
index a153b11241ae9..383d099ea773c 100644
--- a/base/libgit2/callbacks.jl
+++ b/base/libgit2/callbacks.jl
@@ -42,12 +42,18 @@ end
 function user_abort()
     # Note: Potentially it could be better to just throw a Julia error.
     ccall((:giterr_set_str, :libgit2), Void,
-          (Cint, Cstring),
-          Cint(Error.Callback), "Aborting, user cancelled credential request.")
-
+          (Cint, Cstring), Cint(Error.Callback),
+          "Aborting, user cancelled credential request.")
     return Cint(Error.EUSER)
 end
 
+function prompt_limit()
+    ccall((:giterr_set_str, :libgit2), Void,
+          (Cint, Cstring), Cint(Error.Callback),
+          "Aborting, maximum number of prompts reached.")
+    return Cint(Error.EAUTH)
+end
+
 function authenticate_ssh(libgit2credptr::Ptr{Ptr{Void}}, p::CredentialPayload, username_ptr)
     cred = Base.get(p.credential)::SSHCredentials
     revised = false
@@ -93,7 +99,7 @@ function authenticate_ssh(libgit2credptr::Ptr{Ptr{Void}}, p::CredentialPayload,
         p.use_env = false
     end
 
-    if p.allow_prompt && (!revised || !isfilled(cred))
+    if p.remaining_prompts > 0 && (!revised || !isfilled(cred))
         # if username is not provided or empty, then prompt for it
         username = username_ptr != Cstring(C_NULL) ? unsafe_string(username_ptr) : ""
         if isempty(username)
@@ -148,6 +154,9 @@ function authenticate_ssh(libgit2credptr::Ptr{Ptr{Void}}, p::CredentialPayload,
         end
 
         revised = true
+
+        p.remaining_prompts -= 1
+        p.remaining_prompts <= 0 && return prompt_limit()
     end
 
     if !revised
@@ -170,7 +179,7 @@ function authenticate_userpass(libgit2credptr::Ptr{Ptr{Void}}, p::CredentialPayl
         cred.pass = ""
     end
 
-    if p.allow_prompt && (!revised || !isfilled(cred))
+    if p.remaining_prompts > 0 && (!revised || !isfilled(cred))
         prompt_url = git_url(scheme=p.scheme, host=p.host)
         if Sys.iswindows()
             response = Base.winprompt(
@@ -192,6 +201,9 @@ function authenticate_userpass(libgit2credptr::Ptr{Ptr{Void}}, p::CredentialPayl
         end
 
         revised = true
+
+        p.remaining_prompts -= 1
+        p.remaining_prompts <= 0 && return prompt_limit()
     end
 
     if !revised
diff --git a/base/libgit2/types.jl b/base/libgit2/types.jl
index eb15c76d28994..e2d673025f574 100644
--- a/base/libgit2/types.jl
+++ b/base/libgit2/types.jl
@@ -1194,6 +1194,8 @@ mutable struct CredentialPayload <: Payload
     first_pass::Bool
     use_ssh_agent::Bool
     use_env::Bool
+    remaining_prompts::Int
+
     url::String
     scheme::String
     username::String
@@ -1229,6 +1231,7 @@ function reset!(p::CredentialPayload)
     p.first_pass = true
     p.use_ssh_agent = p.allow_ssh_agent
     p.use_env = true
+    p.remaining_prompts = p.allow_prompt ? 3 : 0
     p.url = ""
     p.scheme = ""
     p.username = ""
diff --git a/test/libgit2.jl b/test/libgit2.jl
index 980bca1acb173..95ca90f7de972 100644
--- a/test/libgit2.jl
+++ b/test/libgit2.jl
@@ -1673,6 +1673,10 @@ mktempdir() do dir
             LibGit2.Error.Callback, LibGit2.Error.EUSER,
             "Aborting, user cancelled credential request.")
 
+        prompt_limit = LibGit2.GitError(
+            LibGit2.Error.Callback, LibGit2.Error.EAUTH,
+            "Aborting, maximum number of prompts reached.")
+
         incompatible_error = LibGit2.GitError(
             LibGit2.Error.Callback, LibGit2.Error.EAUTH,
             "The explicitly provided credential is incompatible with the requested " *
@@ -1858,6 +1862,17 @@ mktempdir() do dir
                 err, auth_attempts = challenge_prompt(ssh_ex, challenges)
                 @test err == abort_prompt
                 @test auth_attempts == 2
+
+                # User provides an invalid private key until prompt limit reached.
+                # Note: the prompt should not supply an invalid default.
+                challenges = [
+                    "Private key location for 'git@github.com':" => "foo\n",
+                    "Private key location for 'git@github.com' [foo]:" => "foo\n",
+                    "Private key location for 'git@github.com' [foo]:" => "foo\n",
+                ]
+                err, auth_attempts = challenge_prompt(ssh_ex, challenges)
+                @test err == prompt_limit
+                @test auth_attempts == 3
             end
 
             # Explicitly setting these env variables to an existing but invalid key pair
@@ -1870,6 +1885,16 @@ mktempdir() do dir
                 err, auth_attempts = challenge_prompt(ssh_ex, challenges)
                 @test err == git_ok
                 @test auth_attempts == 2
+
+                # User repeatedly chooses the default invalid private key until prompt limit reached
+                challenges = [
+                    "Private key location for 'git@github.com' [$invalid_key]:" => "\n",
+                    "Private key location for 'git@github.com' [$invalid_key]:" => "\n",
+                    "Private key location for 'git@github.com' [$invalid_key]:" => "\n",
+                ]
+                err, auth_attempts = challenge_prompt(ssh_ex, challenges)
+                @test err == prompt_limit
+                @test auth_attempts == 4
             end
 
             # Explicitly set the public key ENV variable to a non-existent file.
@@ -1955,12 +1980,14 @@ mktempdir() do dir
             challenges = [
                 "Username for 'https://github.com':" => "foo\n",
                 "Password for 'https://foo@github.com':" => "bar\n",
-                "Username for 'https://github.com' [foo]:" => "$valid_username\n",
-                "Password for 'https://$valid_username@github.com':" => "$valid_password\n",
+                "Username for 'https://github.com' [foo]:" => "foo\n",
+                "Password for 'https://foo@github.com':" => "bar\n",
+                "Username for 'https://github.com' [foo]:" => "foo\n",
+                "Password for 'https://foo@github.com':" => "bar\n",
             ]
             err, auth_attempts = challenge_prompt(https_ex, challenges)
-            @test err == git_ok
-            @test auth_attempts == 2
+            @test err == prompt_limit
+            @test auth_attempts == 3
         end
 
         @testset "SSH agent username" begin

From bfda7d9fbf845fee03125e8406768d19c04c5192 Mon Sep 17 00:00:00 2001
From: Curtis Vogt <curtis.vogt@gmail.com>
Date: Tue, 29 Aug 2017 18:23:54 -0500
Subject: [PATCH 08/18] Remove path from CredentialPayload

---
 base/libgit2/callbacks.jl | 1 -
 base/libgit2/types.jl     | 2 --
 2 files changed, 3 deletions(-)

diff --git a/base/libgit2/callbacks.jl b/base/libgit2/callbacks.jl
index 383d099ea773c..4f879cb28565c 100644
--- a/base/libgit2/callbacks.jl
+++ b/base/libgit2/callbacks.jl
@@ -256,7 +256,6 @@ function credentials_callback(libgit2credptr::Ptr{Ptr{Void}}, url_ptr::Cstring,
         p.scheme = m[:scheme] === nothing ? "" : m[:scheme]
         p.username = m[:user] === nothing ? "" : m[:user]
         p.host = m[:host]
-        p.path = m[:path]
 
         # When an explicit credential is supplied we will make sure to use the given
         # credential during the first callback by modifying the allowed types. The
diff --git a/base/libgit2/types.jl b/base/libgit2/types.jl
index e2d673025f574..0dd8e7704076d 100644
--- a/base/libgit2/types.jl
+++ b/base/libgit2/types.jl
@@ -1200,7 +1200,6 @@ mutable struct CredentialPayload <: Payload
     scheme::String
     username::String
     host::String
-    path::String
 
     function CredentialPayload(
             credential::Nullable{<:AbstractCredentials}=Nullable{AbstractCredentials}(),
@@ -1236,7 +1235,6 @@ function reset!(p::CredentialPayload)
     p.scheme = ""
     p.username = ""
     p.host = ""
-    p.path = ""
 
     return p
 end

From 3b63e6133c31ccb64731d912444367b9693b7a26 Mon Sep 17 00:00:00 2001
From: Curtis Vogt <curtis.vogt@gmail.com>
Date: Tue, 29 Aug 2017 19:50:43 -0500
Subject: [PATCH 09/18] Incorporate expanduser

---
 base/libgit2/callbacks.jl |  4 ++--
 test/libgit2.jl           | 50 ++++++++++++++++++++++++++++++++++++++-
 2 files changed, 51 insertions(+), 3 deletions(-)

diff --git a/base/libgit2/callbacks.jl b/base/libgit2/callbacks.jl
index 4f879cb28565c..18fe20a760a3d 100644
--- a/base/libgit2/callbacks.jl
+++ b/base/libgit2/callbacks.jl
@@ -119,7 +119,7 @@ function authenticate_ssh(libgit2credptr::Ptr{Ptr{Void}}, p::CredentialPayload,
             response = Base.prompt("Private key location for '$prompt_url'",
                 default=cred.prvkey)
             isnull(response) && return user_abort()
-            cred.prvkey = unsafe_get(response)
+            cred.prvkey = expanduser(unsafe_get(response))
 
             # Only update the public key if the private key changed
             if cred.prvkey != last_private_key
@@ -134,7 +134,7 @@ function authenticate_ssh(libgit2credptr::Ptr{Ptr{Void}}, p::CredentialPayload,
             response = Base.prompt("Public key location for '$prompt_url'",
                 default=cred.pubkey)
             isnull(response) && return user_abort()
-            cred.pubkey = unsafe_get(response)
+            cred.pubkey = expanduser(unsafe_get(response))
         end
 
         # Ask for a passphrase when the private key exists and requires a passphrase
diff --git a/test/libgit2.jl b/test/libgit2.jl
index 95ca90f7de972..d480b9d5c0be8 100644
--- a/test/libgit2.jl
+++ b/test/libgit2.jl
@@ -7,6 +7,7 @@ const LIBGIT2_MIN_VER = v"0.23.0"
 const LIBGIT2_HELPER_PATH = joinpath(@__DIR__, "libgit2-helpers.jl")
 
 const KEY_DIR = joinpath(@__DIR__, "libgit2")
+const HOME = Sys.iswindows() ? "USERPROFILE" : "HOME"  # Environment variable name for home
 
 function get_global_dir()
     buf = Ref(LibGit2.Buffer())
@@ -1822,7 +1823,7 @@ mktempdir() do dir
             withenv("SSH_KEY_PATH" => nothing,
                     "SSH_PUB_KEY_PATH" => nothing,
                     "SSH_KEY_PASS" => nothing,
-                    (Sys.iswindows() ? "USERPROFILE" : "HOME") => tempdir()) do
+                    HOME => dir) do
 
                 # Set the USERPROFILE / HOME above to be a directory that does not contain
                 # the "~/.ssh/id_rsa" file. If this file exists the credential callback
@@ -2018,6 +2019,53 @@ mktempdir() do dir
             @test auth_attempts == 2
         end
 
+         @testset "SSH expand tilde" begin
+            url = "git@github.com:test/package.jl"
+
+            valid_key = joinpath(KEY_DIR, "valid")
+            valid_cred = LibGit2.SSHCredentials("git", "", valid_key, valid_key * ".pub")
+
+            invalid_key = joinpath(KEY_DIR, "invalid")
+
+            ssh_ex = quote
+                include($LIBGIT2_HELPER_PATH)
+                payload = CredentialPayload(allow_ssh_agent=false, allow_prompt=true)
+                err, auth_attempts = credential_loop($valid_cred, $url, "git", payload)
+                (err, auth_attempts, payload.credential)
+            end
+
+            withenv("SSH_KEY_PATH" => nothing,
+                    "SSH_PUB_KEY_PATH" => nothing,
+                    "SSH_KEY_PASS" => nothing,
+                    HOME => KEY_DIR) do
+
+                # Expand tilde during the private key prompt
+                challenges = [
+                    "Private key location for 'git@github.com':" => "~/valid\n",
+                ]
+                err, auth_attempts, credential = challenge_prompt(ssh_ex, challenges)
+                @test err == git_ok
+                @test auth_attempts == 1
+                @test get(credential).prvkey == abspath(valid_key)
+            end
+
+            withenv("SSH_KEY_PATH" => valid_key,
+                    "SSH_PUB_KEY_PATH" => invalid_key * ".pub",
+                    "SSH_KEY_PASS" => nothing,
+                    HOME => KEY_DIR) do
+
+                # Expand tilde during the public key prompt
+                challenges = [
+                    "Private key location for 'git@github.com' [$valid_key]:" => "\n",
+                    "Public key location for 'git@github.com' [$invalid_key.pub]:" => "~/valid.pub\n",
+                ]
+                err, auth_attempts, credential = challenge_prompt(ssh_ex, challenges)
+                @test err == git_ok
+                @test auth_attempts == 2
+                @test get(credential).pubkey == abspath(valid_key * ".pub")
+            end
+        end
+
         @testset "SSH explicit credentials" begin
             url = "git@github.com:test/package.jl"
             username = "git"

From e9940f81a7291543a11c8c7298d46f17012a7668 Mon Sep 17 00:00:00 2001
From: Curtis Vogt <curtis.vogt@gmail.com>
Date: Wed, 30 Aug 2017 11:50:42 -0500
Subject: [PATCH 10/18] Set error message when auth fails

Without setting an error message the message will be whatever error
message came before.
---
 base/libgit2/callbacks.jl | 11 +++++++++--
 test/libgit2.jl           | 16 ++++++++--------
 2 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/base/libgit2/callbacks.jl b/base/libgit2/callbacks.jl
index 18fe20a760a3d..8f9d2fa276d41 100644
--- a/base/libgit2/callbacks.jl
+++ b/base/libgit2/callbacks.jl
@@ -54,6 +54,13 @@ function prompt_limit()
     return Cint(Error.EAUTH)
 end
 
+function exhausted_abort()
+    ccall((:giterr_set_str, :libgit2), Void,
+          (Cint, Cstring), Cint(Error.Callback),
+          "All authentication methods have failed.")
+    return Cint(Error.EAUTH)
+end
+
 function authenticate_ssh(libgit2credptr::Ptr{Ptr{Void}}, p::CredentialPayload, username_ptr)
     cred = Base.get(p.credential)::SSHCredentials
     revised = false
@@ -160,7 +167,7 @@ function authenticate_ssh(libgit2credptr::Ptr{Ptr{Void}}, p::CredentialPayload,
     end
 
     if !revised
-        return Cint(Error.EAUTH)
+        return exhausted_abort()
     end
 
     return ccall((:git_cred_ssh_key_new, :libgit2), Cint,
@@ -207,7 +214,7 @@ function authenticate_userpass(libgit2credptr::Ptr{Ptr{Void}}, p::CredentialPayl
     end
 
     if !revised
-        return Cint(Error.EAUTH)
+        return exhausted_abort()
     end
 
     return ccall((:git_cred_userpass_plaintext_new, :libgit2), Cint,
diff --git a/test/libgit2.jl b/test/libgit2.jl
index d480b9d5c0be8..859acf7d84989 100644
--- a/test/libgit2.jl
+++ b/test/libgit2.jl
@@ -1683,9 +1683,9 @@ mktempdir() do dir
             "The explicitly provided credential is incompatible with the requested " *
             "authentication methods.")
 
-        eauth_error = LibGit2.GitError(
-            LibGit2.Error.None, LibGit2.Error.EAUTH,
-            "No errors")
+        exhausted_error = LibGit2.GitError(
+            LibGit2.Error.Callback, LibGit2.Error.EAUTH,
+            "All authentication methods have failed.")
 
         @testset "SSH credential prompt" begin
             url = "git@github.com:test/package.jl"
@@ -2008,14 +2008,14 @@ mktempdir() do dir
             # An empty string username_ptr
             ex = gen_ex(username="")
             err, auth_attempts = challenge_prompt(ex, [])
-            @test err == eauth_error
+            @test err == exhausted_error
             @test auth_attempts == 3
 
             # A null username_ptr passed into `git_cred_ssh_key_from_agent` can cause a
             # segfault.
             ex = gen_ex(username=nothing)
             err, auth_attempts = challenge_prompt(ex, [])
-            @test err == eauth_error
+            @test err == exhausted_error
             @test auth_attempts == 2
         end
 
@@ -2094,7 +2094,7 @@ mktempdir() do dir
             # Explicitly provided credential is incorrect
             ex = gen_ex(invalid_cred, allow_prompt=false)
             err, auth_attempts = challenge_prompt(ex, [])
-            @test err == eauth_error
+            @test err == exhausted_error
             @test auth_attempts == 3
         end
 
@@ -2121,7 +2121,7 @@ mktempdir() do dir
             # Explicitly provided credential is incorrect
             ex = gen_ex(invalid_cred, allow_prompt=false)
             err, auth_attempts = challenge_prompt(ex, [])
-            @test err == eauth_error
+            @test err == exhausted_error
             @test auth_attempts == 2
         end
 
@@ -2193,7 +2193,7 @@ mktempdir() do dir
             # An EAUTH error should remove credentials from the cache
             ex = gen_ex(cached_cred=invalid_cred, allow_prompt=false)
             err, auth_attempts, cache = challenge_prompt(ex, [])
-            @test err == eauth_error
+            @test err == exhausted_error
             @test auth_attempts == 2
             @test typeof(cache) == LibGit2.CachedCredentials
             @test cache.cred == Dict()

From b26c2caadb1eccff1bddc8e8b330479e04debdfd Mon Sep 17 00:00:00 2001
From: Curtis Vogt <curtis.vogt@gmail.com>
Date: Tue, 19 Sep 2017 10:01:20 -0500
Subject: [PATCH 11/18] Remove old unused SSH testset

The test has been superseded by all of the new credential_loop tests.
---
 test/libgit2.jl | 176 ------------------------------------------------
 1 file changed, 176 deletions(-)

diff --git a/test/libgit2.jl b/test/libgit2.jl
index 859acf7d84989..f4c94197853f7 100644
--- a/test/libgit2.jl
+++ b/test/libgit2.jl
@@ -2286,182 +2286,6 @@ mktempdir() do dir
         end
     end
 
-    #= temporarily disabled until working on the buildbots, ref https://github.com/JuliaLang/julia/pull/17651#issuecomment-238211150
-    @testset "SSH" begin
-        sshd_command = ""
-        ssh_repo = joinpath(dir, "Example.SSH")
-        if !Sys.iswindows()
-            try
-                # SSHD needs to be executed by its full absolute path
-                sshd_command = strip(read(`which sshd`, String))
-            catch
-                warn("Skipping SSH tests (Are `which` and `sshd` installed?)")
-            end
-        end
-        if !isempty(sshd_command)
-            mktempdir() do fakehomedir
-                mkdir(joinpath(fakehomedir,".ssh"))
-                # Unsetting the SSH agent serves two purposes. First, we make
-                # sure that we don't accidentally pick up an existing agent,
-                # and second we test that we fall back to using a key file
-                # if the agent isn't present.
-                withenv("HOME"=>fakehomedir,"SSH_AUTH_SOCK"=>nothing) do
-                    # Generate user file, first an unencrypted one
-                    wait(spawn(`ssh-keygen -N "" -C juliatest@localhost -f $fakehomedir/.ssh/id_rsa`))
-
-                    # Generate host keys
-                    wait(spawn(`ssh-keygen -f $fakehomedir/ssh_host_rsa_key -N '' -t rsa`))
-                    wait(spawn(`ssh-keygen -f $fakehomedir/ssh_host_dsa_key -N '' -t dsa`))
-
-                    our_ssh_port = rand(13000:14000) # Chosen arbitrarily
-
-                    key_option = "AuthorizedKeysFile $fakehomedir/.ssh/id_rsa.pub"
-                    pidfile_option = "PidFile $fakehomedir/sshd.pid"
-                    sshp = agentp = nothing
-                    logfile = tempname()
-                    ssh_debug = false
-                    function spawn_sshd()
-                        debug_flags = ssh_debug ? `-d -d` : ``
-                        _p = open(logfile, "a") do logfilestream
-                            spawn(pipeline(pipeline(`$sshd_command
-                            -e -f /dev/null $debug_flags
-                            -h $fakehomedir/ssh_host_rsa_key
-                            -h $fakehomedir/ssh_host_dsa_key -p $our_ssh_port
-                            -o $pidfile_option
-                            -o 'Protocol 2'
-                            -o $key_option
-                            -o 'UsePrivilegeSeparation no'
-                            -o 'StrictModes no'`,STDOUT),stderr=logfilestream))
-                        end
-                        # Give the SSH server 5 seconds to start up
-                        yield(); sleep(5)
-                        _p
-                    end
-                    sshp = spawn_sshd()
-
-                    TIOCSCTTY_str = "ccall(:ioctl, Void, (Cint, Cint, Int64), 0,
-                        (Sys.isbsd() || Sys.isapple()) ? 0x20007461 : Sys.islinux() ? 0x540E :
-                        error(\"Fill in TIOCSCTTY for this OS here\"), 0)"
-
-                    # To fail rather than hang
-                    function killer_task(p, master)
-                        @async begin
-                            sleep(10)
-                            kill(p)
-                            if isopen(master)
-                                nb_available(master) > 0 &&
-                                    write(logfile,
-                                        readavailable(master))
-                                close(master)
-                            end
-                        end
-                    end
-
-                    try
-                        function try_clone(challenges = [])
-                            cmd = """
-                            repo = nothing
-                            try
-                                $TIOCSCTTY_str
-                                reponame = "ssh://$(ENV["USER"])@localhost:$our_ssh_port$cache_repo"
-                                repo = LibGit2.clone(reponame, "$ssh_repo")
-                            catch err
-                                open("$logfile","a") do f
-                                    println(f,"HOME: ",ENV["HOME"])
-                                    println(f, err)
-                                end
-                            finally
-                                close(repo)
-                            end
-                            """
-                            # We try to be helpful by desperately looking for
-                            # a way to prompt the password interactively. Pretend
-                            # to be a TTY to suppress those shenanigans. Further, we
-                            # need to detach and change the controlling terminal with
-                            # TIOCSCTTY, since getpass opens the controlling terminal
-                            TestHelpers.with_fake_pty() do slave, master
-                                err = Base.Pipe()
-                                let p = spawn(detach(
-                                    `$(Base.julia_cmd()) --startup-file=no -e $cmd`),slave,slave,STDERR)
-                                    killer_task(p, master)
-                                    for (challenge, response) in challenges
-                                        readuntil(master, challenge)
-                                        sleep(1)
-                                        print(master, response)
-                                    end
-                                    sleep(2)
-                                    wait(p)
-                                    close(master)
-                                end
-                            end
-                            @test isfile(joinpath(ssh_repo,"testfile"))
-                            rm(ssh_repo, recursive = true)
-                        end
-
-                        # Should use the default files, no interaction required.
-                        try_clone()
-                        ssh_debug && (kill(sshp); sshp = spawn_sshd())
-
-                        # Ok, now encrypt the file and test with that (this also
-                        # makes sure that we don't accidentally fall back to the
-                        # unencrypted version)
-                        wait(spawn(`ssh-keygen -p -N "xxxxx" -f $fakehomedir/.ssh/id_rsa`))
-
-                        # Try with the encrypted file. Needs a password.
-                        try_clone(["Passphrase"=>"xxxxx\r\n"])
-                        ssh_debug && (kill(sshp); sshp = spawn_sshd())
-
-                        # Move the file. It should now ask for the location and
-                        # then the passphrase
-                        mv("$fakehomedir/.ssh/id_rsa","$fakehomedir/.ssh/id_rsa2")
-                        cp("$fakehomedir/.ssh/id_rsa.pub","$fakehomedir/.ssh/id_rsa2.pub")
-                        try_clone(["location"=>"$fakehomedir/.ssh/id_rsa2\n",
-                                   "Passphrase"=>"xxxxx\n"])
-                        mv("$fakehomedir/.ssh/id_rsa2","$fakehomedir/.ssh/id_rsa")
-                        rm("$fakehomedir/.ssh/id_rsa2.pub")
-
-                        # Ok, now start an agent
-                        agent_sock = tempname()
-                        agentp = spawn(`ssh-agent -a $agent_sock -d`)
-                        while stat(agent_sock).mode == 0 # Wait until the agent is started
-                            sleep(1)
-                        end
-
-                        # fake pty is required for the same reason as in try_clone
-                        # above
-                        withenv("SSH_AUTH_SOCK" => agent_sock) do
-                            TestHelpers.with_fake_pty() do slave, master
-                                cmd = """
-                                    $TIOCSCTTY_str
-                                    run(pipeline(`ssh-add $fakehomedir/.ssh/id_rsa`,
-                                        stderr = DevNull))
-                                """
-                                addp = spawn(detach(`$(Base.julia_cmd()) --startup-file=no -e $cmd`),
-                                        slave, slave, STDERR)
-                                killer_task(addp, master)
-                                sleep(2)
-                                write(master, "xxxxx\n")
-                                wait(addp)
-                            end
-
-                            # Should now use the agent
-                            try_clone()
-                        end
-                    catch err
-                        println("SSHD logfile contents follows:")
-                        println(read(logfile, String))
-                        rethrow(err)
-                    finally
-                        rm(logfile)
-                        sshp !== nothing && kill(sshp)
-                        agentp !== nothing && kill(agentp)
-                    end
-                end
-            end
-        end
-    end
-    =#
-
     # Note: Tests only work on linux as SSL_CERT_FILE is only respected on linux systems.
     @testset "Hostname verification" begin
         openssl_installed = false

From 22ed0aed52362a3ed4b22c0c332352e0759c1b1e Mon Sep 17 00:00:00 2001
From: Curtis Vogt <curtis.vogt@gmail.com>
Date: Tue, 19 Sep 2017 10:42:19 -0500
Subject: [PATCH 12/18] Add additional docstrings

---
 base/libgit2/types.jl      | 12 ++++++++++++
 doc/src/devdocs/libgit2.md | 14 +++++++++-----
 2 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/base/libgit2/types.jl b/base/libgit2/types.jl
index 0dd8e7704076d..6a8ade8e76cd3 100644
--- a/base/libgit2/types.jl
+++ b/base/libgit2/types.jl
@@ -1239,6 +1239,12 @@ function reset!(p::CredentialPayload)
     return p
 end
 
+"""
+    approve(payload::CredentialPayload) -> Void
+
+Store the `payload` credential for re-use in a future authentication. Should only be called
+when authentication was successful.
+"""
 function approve(p::CredentialPayload)
     isnull(p.credential) && return  # No credentials were used
     cred = unsafe_get(p.credential)
@@ -1248,6 +1254,12 @@ function approve(p::CredentialPayload)
     end
 end
 
+"""
+    reject(payload::CredentialPayload) -> Void
+
+Discard the `payload` credential from begin re-used in future authentication. Should only be
+called when authentication was unsuccessful.
+"""
 function reject(p::CredentialPayload)
     isnull(p.credential) && return  # No credentials were used
     cred = unsafe_get(p.credential)
diff --git a/doc/src/devdocs/libgit2.md b/doc/src/devdocs/libgit2.md
index e8e95d82d1ab1..3b6c49065691f 100644
--- a/doc/src/devdocs/libgit2.md
+++ b/doc/src/devdocs/libgit2.md
@@ -12,9 +12,7 @@ For more information on some of the objects and methods referenced here, consult
 [libgit2 API reference](https://libgit2.github.com/libgit2/#v0.25.1).
 
 ```@docs
-Base.LibGit2.AbstractCredentials
 Base.LibGit2.Buffer
-Base.LibGit2.CachedCredentials
 Base.LibGit2.CheckoutOptions
 Base.LibGit2.CloneOptions
 Base.LibGit2.DescribeOptions
@@ -49,13 +47,11 @@ Base.LibGit2.PushOptions
 Base.LibGit2.RebaseOperation
 Base.LibGit2.RebaseOptions
 Base.LibGit2.RemoteCallbacks
-Base.LibGit2.SSHCredentials
 Base.LibGit2.SignatureStruct
 Base.LibGit2.StatusEntry
 Base.LibGit2.StatusOptions
 Base.LibGit2.StrArrayStruct
 Base.LibGit2.TimeStruct
-Base.LibGit2.UserPasswordCredentials
 Base.LibGit2.add!
 Base.LibGit2.add_fetch!
 Base.LibGit2.add_push!
@@ -92,6 +88,7 @@ Base.LibGit2.features
 Base.LibGit2.filename
 Base.LibGit2.filemode
 Base.LibGit2.gitdir
+Base.LibGit2.git_url
 Base.LibGit2.@githash_str
 Base.LibGit2.head
 Base.LibGit2.head!
@@ -103,7 +100,6 @@ Base.LibGit2.isbinary
 Base.LibGit2.iscommit
 Base.LibGit2.isdiff
 Base.LibGit2.isdirty
-Base.LibGit2.isfilled
 Base.LibGit2.isorphan
 Base.LibGit2.isset
 Base.LibGit2.iszero
@@ -154,4 +150,12 @@ Base.LibGit2.with
 Base.LibGit2.with_warn
 Base.LibGit2.workdir
 Base.LibGit2.GitObject(::Base.LibGit2.GitTreeEntry)
+Base.LibGit2.AbstractCredentials
+Base.LibGit2.UserPasswordCredentials
+Base.LibGit2.SSHCredentials
+Base.LibGit2.isfilled
+Base.LibGit2.CachedCredentials
+Base.LibGit2.CredentialPayload
+Base.LibGit2.approve
+Base.LibGit2.reject
 ```

From d1084ec38cabf6786f7ee3c5dbdbd48d21d090b6 Mon Sep 17 00:00:00 2001
From: Curtis Vogt <curtis.vogt@gmail.com>
Date: Wed, 20 Sep 2017 22:43:37 -0500
Subject: [PATCH 13/18] Formatting fix

---
 base/libgit2/callbacks.jl | 26 ++++++++++++--------------
 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/base/libgit2/callbacks.jl b/base/libgit2/callbacks.jl
index 8f9d2fa276d41..9144d22b223d4 100644
--- a/base/libgit2/callbacks.jl
+++ b/base/libgit2/callbacks.jl
@@ -110,21 +110,20 @@ function authenticate_ssh(libgit2credptr::Ptr{Ptr{Void}}, p::CredentialPayload,
         # if username is not provided or empty, then prompt for it
         username = username_ptr != Cstring(C_NULL) ? unsafe_string(username_ptr) : ""
         if isempty(username)
-            prompt_url = git_url(scheme=p.scheme, host=p.host)
-            response = Base.prompt("Username for '$prompt_url'", default=cred.user)
+            url = git_url(scheme=p.scheme, host=p.host)
+            response = Base.prompt("Username for '$url'", default=cred.user)
             isnull(response) && return user_abort()
             cred.user = unsafe_get(response)
         else
             cred.user = username
         end
 
-        prompt_url = git_url(scheme=p.scheme, host=p.host, username=cred.user)
+        url = git_url(scheme=p.scheme, host=p.host, username=cred.user)
 
         # For SSH we need a private key location
         last_private_key = cred.prvkey
         if !isfile(cred.prvkey) || !revised
-            response = Base.prompt("Private key location for '$prompt_url'",
-                default=cred.prvkey)
+            response = Base.prompt("Private key location for '$url'", default=cred.prvkey)
             isnull(response) && return user_abort()
             cred.prvkey = expanduser(unsafe_get(response))
 
@@ -138,8 +137,7 @@ function authenticate_ssh(libgit2credptr::Ptr{Ptr{Void}}, p::CredentialPayload,
         # typically this will just annoy users.
         stale = !p.first_pass && cred.prvkey == last_private_key && cred.pubkey != cred.prvkey * ".pub"
         if isfile(cred.prvkey) && (stale || !isfile(cred.pubkey))
-            response = Base.prompt("Public key location for '$prompt_url'",
-                default=cred.pubkey)
+            response = Base.prompt("Public key location for '$url'", default=cred.pubkey)
             isnull(response) && return user_abort()
             cred.pubkey = expanduser(unsafe_get(response))
         end
@@ -187,21 +185,21 @@ function authenticate_userpass(libgit2credptr::Ptr{Ptr{Void}}, p::CredentialPayl
     end
 
     if p.remaining_prompts > 0 && (!revised || !isfilled(cred))
-        prompt_url = git_url(scheme=p.scheme, host=p.host)
+        url = git_url(scheme=p.scheme, host=p.host)
+        username = isempty(cred.user) ? p.username : cred.user
         if Sys.iswindows()
             response = Base.winprompt(
-                "Please enter your credentials for '$prompt_url'", "Credentials required",
-                isempty(cred.user) ? p.username : cred.user; prompt_username=true)
+                "Please enter your credentials for '$url'", "Credentials required",
+                username; prompt_username=true)
             isnull(response) && return user_abort()
             cred.user, cred.pass = unsafe_get(response)
         else
-            response = Base.prompt("Username for '$prompt_url'",
-                default=isempty(cred.user) ? p.username : cred.user)
+            response = Base.prompt("Username for '$url'", default=username)
             isnull(response) && return user_abort()
             cred.user = unsafe_get(response)
 
-            prompt_url = git_url(scheme=p.scheme, host=p.host, username=cred.user)
-            response = Base.prompt("Password for '$prompt_url'", password=true)
+            url = git_url(scheme=p.scheme, host=p.host, username=cred.user)
+            response = Base.prompt("Password for '$url'", password=true)
             isnull(response) && return user_abort()
             cred.pass = unsafe_get(response)
             isempty(cred.pass) && return user_abort()  # Ambiguous if EOF or newline

From e55b9a695c3e9829a35239de5e7d179b89fce723 Mon Sep 17 00:00:00 2001
From: Curtis Vogt <curtis.vogt@gmail.com>
Date: Thu, 21 Sep 2017 11:49:31 -0500
Subject: [PATCH 14/18] Add SSH id_rsa test

---
 test/libgit2.jl | 39 ++++++++++++++++++++++++++++++++++++---
 1 file changed, 36 insertions(+), 3 deletions(-)

diff --git a/test/libgit2.jl b/test/libgit2.jl
index f4c94197853f7..02820296aea09 100644
--- a/test/libgit2.jl
+++ b/test/libgit2.jl
@@ -1717,14 +1717,14 @@ mktempdir() do dir
             # sure a users will actually have these files. Instead we will use the ENV
             # variables to set the default values.
 
-            # Default credentials are valid
+            # ENV credentials are valid
             withenv("SSH_KEY_PATH" => valid_key) do
                 err, auth_attempts = challenge_prompt(ssh_ex, [])
                 @test err == git_ok
                 @test auth_attempts == 1
             end
 
-            # Default credentials are valid but requires a passphrase
+            # ENV credentials are valid but requires a passphrase
             withenv("SSH_KEY_PATH" => valid_p_key) do
                 challenges = [
                     "Passphrase for $valid_p_key:" => "$passphrase\n",
@@ -1763,6 +1763,7 @@ mktempdir() do dir
                 @test auth_attempts == 1
             end
 
+            # ENV credential requiring passphrase
             withenv("SSH_KEY_PATH" => valid_p_key, "SSH_KEY_PASS" => passphrase) do
                 err, auth_attempts = challenge_prompt(ssh_p_ex, [])
                 @test err == git_ok
@@ -2019,7 +2020,39 @@ mktempdir() do dir
             @test auth_attempts == 2
         end
 
-         @testset "SSH expand tilde" begin
+        @testset "SSH default" begin
+            mktempdir() do home_dir
+                url = "github.com:test/package.jl"
+
+                default_key = joinpath(home_dir, ".ssh", "id_rsa")
+                default_cred = LibGit2.SSHCredentials("git", "", default_key, default_key * ".pub")
+
+                # Copy the stored private/public
+                key = joinpath(KEY_DIR, "valid")
+                mkdir(dirname(default_key))
+                cp(key, default_key)
+                cp(key * ".pub", default_key * ".pub")
+
+                ssh_ex = quote
+                    include($LIBGIT2_HELPER_PATH)
+                    credential_loop($default_cred, $url, "git")
+                end
+
+                withenv("SSH_KEY_PATH" => nothing,
+                        "SSH_PUB_KEY_PATH" => nothing,
+                        "SSH_KEY_PASS" => nothing,
+                        HOME => home_dir) do
+
+                    @test isfile(joinpath(homedir(), ".ssh", "id_rsa"))
+
+                    err, auth_attempts = challenge_prompt(ssh_ex, [])
+                    @test err == git_ok
+                    @test auth_attempts == 1
+                end
+            end
+        end
+
+        @testset "SSH expand tilde" begin
             url = "git@github.com:test/package.jl"
 
             valid_key = joinpath(KEY_DIR, "valid")

From 3f3da52c5b26dd7919ffd985728a3b0e596be805 Mon Sep 17 00:00:00 2001
From: Curtis Vogt <curtis.vogt@gmail.com>
Date: Thu, 21 Sep 2017 18:18:30 -0500
Subject: [PATCH 15/18] Make LibGit2 retry when SSH passphrase is wrong

According to the LibGit2 authentication guide
(https://libgit2.github.com/docs/guides/authentication/) the callback
is suppose to be retried if the server doesn't accept the credentials.
In the case where we use SSH and the passphrase is invalid LibGit2
would just give a generic:

  GitError(Code:ERROR, Class:SSH, Failed to authenticate SSH session: Callback returned error)
---
 deps/libgit2.mk                     |  8 +++++++-
 deps/patches/libgit2-ssh-loop.patch | 24 ++++++++++++++++++++++++
 2 files changed, 31 insertions(+), 1 deletion(-)
 create mode 100644 deps/patches/libgit2-ssh-loop.patch

diff --git a/deps/libgit2.mk b/deps/libgit2.mk
index 1196c0f6d0ac8..5d16aee5063fd 100644
--- a/deps/libgit2.mk
+++ b/deps/libgit2.mk
@@ -85,6 +85,11 @@ $(LIBGIT2_SRC_PATH)/libgit2-mbedtls-fixup.patch-applied: $(LIBGIT2_SRC_PATH)/sou
 		patch -p1 -f < $(SRCDIR)/patches/libgit2-mbedtls-fixup.patch
 	echo 1 > $@
 
+$(LIBGIT2_SRC_PATH)/libgit2-ssh-loop.patch-applied: $(LIBGIT2_SRC_PATH)/source-extracted | $(LIBGIT2_SRC_PATH)/libgit2-mbedtls-fixup.patch-applied
+	cd $(LIBGIT2_SRC_PATH) && \
+		patch -p1 -f < $(SRCDIR)/patches/libgit2-ssh-loop.patch
+	echo 1 > $@
+
 $(build_datarootdir)/julia/cert.pem: $(CERTFILE)
 	mkdir -p $(build_datarootdir)/julia
 	cp -f $(CERTFILE) $@
@@ -94,7 +99,8 @@ $(BUILDDIR)/$(LIBGIT2_SRC_DIR)/build-configured: \
 	$(LIBGIT2_SRC_PATH)/libgit2-ssh.patch-applied \
 	$(LIBGIT2_SRC_PATH)/libgit2-agent-nonfatal.patch-applied \
 	$(LIBGIT2_SRC_PATH)/libgit2-mbedtls-verify.patch-applied \
-	$(LIBGIT2_SRC_PATH)/libgit2-mbedtls-fixup.patch-applied
+	$(LIBGIT2_SRC_PATH)/libgit2-mbedtls-fixup.patch-applied \
+	$(LIBGIT2_SRC_PATH)/libgit2-ssh-loop.patch-applied \
 
 ifneq ($(CERTFILE),)
 $(BUILDDIR)/$(LIBGIT2_SRC_DIR)/build-configured: $(build_datarootdir)/julia/cert.pem
diff --git a/deps/patches/libgit2-ssh-loop.patch b/deps/patches/libgit2-ssh-loop.patch
new file mode 100644
index 0000000000000..dfc0ac632c95b
--- /dev/null
+++ b/deps/patches/libgit2-ssh-loop.patch
@@ -0,0 +1,24 @@
+commit eac62497aec204568a494743f829d922787d69c5
+Author: Curtis Vogt <curtis.vogt@gmail.com>
+Date:   Thu Sep 21 15:51:52 2017 -0500
+
+    Ask for credentials again when passphrase is wrong
+    
+    When trying to decode the private key it looks like LibSSH2 returns a
+    LIBSSH2_ERROR_PUBLICKEY_UNVERIFIED when the passphrase is incorrect.
+
+diff --git a/src/transports/ssh.c b/src/transports/ssh.c
+index 172ef413c..ec3b0b6ff 100644
+--- a/src/transports/ssh.c
++++ b/src/transports/ssh.c
+@@ -420,8 +420,8 @@ static int _git_ssh_authenticate_session(
+ 		}
+ 	} while (LIBSSH2_ERROR_EAGAIN == rc || LIBSSH2_ERROR_TIMEOUT == rc);
+ 
+-        if (rc == LIBSSH2_ERROR_PASSWORD_EXPIRED || rc == LIBSSH2_ERROR_AUTHENTICATION_FAILED)
+-                return GIT_EAUTH;
++	if (rc == LIBSSH2_ERROR_PASSWORD_EXPIRED || rc == LIBSSH2_ERROR_AUTHENTICATION_FAILED || rc == LIBSSH2_ERROR_PUBLICKEY_UNVERIFIED)
++		return GIT_EAUTH;
+ 
+ 	if (rc != LIBSSH2_ERROR_NONE) {
+ 		if (!giterr_last())

From 3f97c24c7b7e196653a2e6fb9581c60349dd1ad8 Mon Sep 17 00:00:00 2001
From: Curtis Vogt <curtis.vogt@gmail.com>
Date: Thu, 21 Sep 2017 19:14:45 -0500
Subject: [PATCH 16/18] Additional comments

---
 base/libgit2/callbacks.jl | 28 ++++++++++++++++------------
 test/libgit2.jl           |  2 +-
 2 files changed, 17 insertions(+), 13 deletions(-)

diff --git a/base/libgit2/callbacks.jl b/base/libgit2/callbacks.jl
index 9144d22b223d4..7e8bb93e1641b 100644
--- a/base/libgit2/callbacks.jl
+++ b/base/libgit2/callbacks.jl
@@ -221,27 +221,31 @@ function authenticate_userpass(libgit2credptr::Ptr{Ptr{Void}}, p::CredentialPayl
 end
 
 
-"""Credentials callback function
-
-Function provides different credential acquisition functionality w.r.t. a connection protocol.
-If a payload is provided then `payload_ptr` should contain a `LibGit2.CredentialPayload` object.
+"""
+    credential_callback(...) -> Cint
 
-For `LibGit2.Consts.CREDTYPE_USERPASS_PLAINTEXT` type, if the payload contains fields:
-`user` & `pass`, they are used to create authentication credentials.
+A LibGit2 credential callback function which provides different credential acquisition
+functionality w.r.t. a connection protocol. The `payload_ptr` is required to contain a
+`LibGit2.CredentialPayload` object which will keep track of state and settings.
 
-For `LibGit2.Consts.CREDTYPE_SSH_KEY` type, if the payload contains fields:
-`user`, `prvkey`, `pubkey` & `pass`, they are used to create authentication credentials.
+The `allowed_types` contains a bitmask of `LibGit2.Consts.GIT_CREDTYPE` values specifying
+which authentication methods should be attempted.
 
-Typing `^D` (control key together with the `d` key) will abort the credential prompt.
+Credential authentication is done in the following order (if supported):
+- SSH agent
+- SSH private/public key pair
+- Username/password plain text
 
-Credentials are checked in the following order (if supported):
-- ssh key pair (`ssh-agent` if specified in payload's `use_ssh_agent` field)
-- plain text
+If a user is presented with a credential prompt they can abort the prompt by typing `^D`
+(pressing the control key together with the `d` key).
 
 **Note**: Due to the specifics of the `libgit2` authentication procedure, when
 authentication fails, this function is called again without any indication whether
 authentication was successful or not. To avoid an infinite loop from repeatedly
 using the same faulty credentials, we will keep track of state using the payload.
+
+For addition details see the LibGit2 guide on
+[authenticating against a server](https://libgit2.github.com/docs/guides/authentication/).
 """
 function credentials_callback(libgit2credptr::Ptr{Ptr{Void}}, url_ptr::Cstring,
                               username_ptr::Cstring,
diff --git a/test/libgit2.jl b/test/libgit2.jl
index 02820296aea09..c3815e721aeb3 100644
--- a/test/libgit2.jl
+++ b/test/libgit2.jl
@@ -1644,7 +1644,7 @@ mktempdir() do dir
 
         @test !haskey(cache, cred_id)
 
-        # Reject a credential which wasn't stored
+        # Attempt to reject a credential which wasn't stored
         LibGit2.reject(cache, cred, url)
         @test !haskey(cache, cred_id)
         @test cred.user == "julia"

From 773d226b2ba0b470112d76854437109beaa73543 Mon Sep 17 00:00:00 2001
From: Curtis Vogt <curtis.vogt@gmail.com>
Date: Thu, 21 Sep 2017 19:20:33 -0500
Subject: [PATCH 17/18] Skip SSH agent when using cached credentials

---
 base/libgit2/callbacks.jl |  2 +-
 test/libgit2.jl           | 12 +++++++-----
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/base/libgit2/callbacks.jl b/base/libgit2/callbacks.jl
index 7e8bb93e1641b..87e7c05435d22 100644
--- a/base/libgit2/callbacks.jl
+++ b/base/libgit2/callbacks.jl
@@ -73,7 +73,7 @@ function authenticate_ssh(libgit2credptr::Ptr{Ptr{Void}}, p::CredentialPayload,
     end
 
     # first try ssh-agent if credentials support its usage
-    if p.use_ssh_agent && username_ptr != Cstring(C_NULL)
+    if p.use_ssh_agent && username_ptr != Cstring(C_NULL) && (!revised || !isfilled(cred))
         err = ccall((:git_cred_ssh_key_from_agent, :libgit2), Cint,
                     (Ptr{Ptr{Void}}, Cstring), libgit2credptr, username_ptr)
 
diff --git a/test/libgit2.jl b/test/libgit2.jl
index c3815e721aeb3..1f8a73f211a7a 100644
--- a/test/libgit2.jl
+++ b/test/libgit2.jl
@@ -2110,22 +2110,24 @@ mktempdir() do dir
             invalid_key = joinpath(KEY_DIR, "invalid")
             invalid_cred = LibGit2.SSHCredentials(username, "", invalid_key, invalid_key * ".pub")
 
-            function gen_ex(cred; allow_prompt=true)
+            function gen_ex(cred; allow_prompt=true, allow_ssh_agent=false)
                 quote
                     include($LIBGIT2_HELPER_PATH)
-                    payload = CredentialPayload($cred, allow_ssh_agent=false, allow_prompt=$allow_prompt)
+                    payload = CredentialPayload($cred, allow_ssh_agent=$allow_ssh_agent,
+                        allow_prompt=$allow_prompt)
                     credential_loop($valid_cred, $url, $username, payload)
                 end
             end
 
-            # Explicitly provided credential is correct
-            ex = gen_ex(valid_cred, allow_prompt=true)
+            # Explicitly provided credential is correct. Note: allowing prompting and
+            # SSH agent to ensure they are skipped.
+            ex = gen_ex(valid_cred, allow_prompt=true, allow_ssh_agent=true)
             err, auth_attempts = challenge_prompt(ex, [])
             @test err == git_ok
             @test auth_attempts == 1
 
             # Explicitly provided credential is incorrect
-            ex = gen_ex(invalid_cred, allow_prompt=false)
+            ex = gen_ex(invalid_cred, allow_prompt=false, allow_ssh_agent=false)
             err, auth_attempts = challenge_prompt(ex, [])
             @test err == exhausted_error
             @test auth_attempts == 3

From 23d2fd2058d67958aad4a7f42a39c68b48e5f55f Mon Sep 17 00:00:00 2001
From: Curtis Vogt <curtis.vogt@gmail.com>
Date: Thu, 21 Sep 2017 20:46:03 -0500
Subject: [PATCH 18/18] Prompt for private if SSH_KEY_PATH unset

Allows the user to confirm they want to use the default private key.
During development it was noticed that the username was only being
filled in if prompting was allowed.
---
 base/libgit2/callbacks.jl | 12 +++++-----
 test/libgit2.jl           | 49 ++++++++++++++++++++++++++++++---------
 2 files changed, 44 insertions(+), 17 deletions(-)

diff --git a/base/libgit2/callbacks.jl b/base/libgit2/callbacks.jl
index 87e7c05435d22..9267bbb2982cf 100644
--- a/base/libgit2/callbacks.jl
+++ b/base/libgit2/callbacks.jl
@@ -82,6 +82,10 @@ function authenticate_ssh(libgit2credptr::Ptr{Ptr{Void}}, p::CredentialPayload,
     end
 
     if p.use_env && (!revised || !isfilled(cred))
+        if isempty(cred.user) && username_ptr != Cstring(C_NULL)
+            cred.user = unsafe_string(username_ptr)
+        end
+
         cred.prvkey = Base.get(ENV, "SSH_KEY_PATH") do
             default = joinpath(homedir(), ".ssh", "id_rsa")
             if isempty(cred.prvkey) && isfile(default)
@@ -107,22 +111,18 @@ function authenticate_ssh(libgit2credptr::Ptr{Ptr{Void}}, p::CredentialPayload,
     end
 
     if p.remaining_prompts > 0 && (!revised || !isfilled(cred))
-        # if username is not provided or empty, then prompt for it
-        username = username_ptr != Cstring(C_NULL) ? unsafe_string(username_ptr) : ""
-        if isempty(username)
+        if isempty(cred.user) || username_ptr == Cstring(C_NULL)
             url = git_url(scheme=p.scheme, host=p.host)
             response = Base.prompt("Username for '$url'", default=cred.user)
             isnull(response) && return user_abort()
             cred.user = unsafe_get(response)
-        else
-            cred.user = username
         end
 
         url = git_url(scheme=p.scheme, host=p.host, username=cred.user)
 
         # For SSH we need a private key location
         last_private_key = cred.prvkey
-        if !isfile(cred.prvkey) || !revised
+        if !isfile(cred.prvkey) || !revised || !haskey(ENV, "SSH_KEY_PATH")
             response = Base.prompt("Private key location for '$url'", default=cred.prvkey)
             isnull(response) && return user_abort()
             cred.prvkey = expanduser(unsafe_get(response))
diff --git a/test/libgit2.jl b/test/libgit2.jl
index 1f8a73f211a7a..83149d7bb5327 100644
--- a/test/libgit2.jl
+++ b/test/libgit2.jl
@@ -2025,17 +2025,34 @@ mktempdir() do dir
                 url = "github.com:test/package.jl"
 
                 default_key = joinpath(home_dir, ".ssh", "id_rsa")
-                default_cred = LibGit2.SSHCredentials("git", "", default_key, default_key * ".pub")
-
-                # Copy the stored private/public
-                key = joinpath(KEY_DIR, "valid")
                 mkdir(dirname(default_key))
-                cp(key, default_key)
-                cp(key * ".pub", default_key * ".pub")
 
-                ssh_ex = quote
-                    include($LIBGIT2_HELPER_PATH)
-                    credential_loop($default_cred, $url, "git")
+                valid_key = joinpath(KEY_DIR, "valid")
+                valid_cred = LibGit2.SSHCredentials("git", "", valid_key, valid_key * ".pub")
+
+                valid_p_key = joinpath(KEY_DIR, "valid-passphrase")
+                passphrase = "secret"
+                valid_p_cred = LibGit2.SSHCredentials("git", passphrase, valid_p_key, valid_p_key * ".pub")
+
+                function gen_ex(cred)
+                    quote
+                        valid_cred = $cred
+
+                        default_cred = deepcopy(valid_cred)
+                        default_cred.prvkey = $default_key
+                        default_cred.pubkey = $default_key * ".pub"
+
+                        cp(valid_cred.prvkey, default_cred.prvkey)
+                        cp(valid_cred.pubkey, default_cred.pubkey)
+
+                        try
+                            include($LIBGIT2_HELPER_PATH)
+                            credential_loop(default_cred, $url, "git")
+                        finally
+                            rm(default_cred.prvkey)
+                            rm(default_cred.pubkey)
+                        end
+                    end
                 end
 
                 withenv("SSH_KEY_PATH" => nothing,
@@ -2043,9 +2060,19 @@ mktempdir() do dir
                         "SSH_KEY_PASS" => nothing,
                         HOME => home_dir) do
 
-                    @test isfile(joinpath(homedir(), ".ssh", "id_rsa"))
+                    # Automatically use the default key
+                    ex = gen_ex(valid_cred)
+                    err, auth_attempts = challenge_prompt(ex, [])
+                    @test err == git_ok
+                    @test auth_attempts == 1
 
-                    err, auth_attempts = challenge_prompt(ssh_ex, [])
+                    # Confirm the private key if any other prompting is required
+                    ex = gen_ex(valid_p_cred)
+                    challenges = [
+                        "Private key location for 'git@github.com' [$default_key]:" => "\n",
+                        "Passphrase for $default_key:" => "$passphrase\n",
+                    ]
+                    err, auth_attempts = challenge_prompt(ex, challenges)
                     @test err == git_ok
                     @test auth_attempts == 1
                 end