From 5ef2ca966f595d73136007d912c110ffed3bf280 Mon Sep 17 00:00:00 2001
From: Curtis Vogt <curtis.vogt@gmail.com>
Date: Sun, 6 Aug 2017 18:05:58 -0500
Subject: [PATCH 1/5] Add minimal CredentialPayload implementation

LibGit2 credential callback now always uses a CredentialPayload struct
which allows us keep callback state separate from credentials.
Implementation at this point is minimal to allow us to more thoroughly
test the existing behaviour.
---
 base/libgit2/callbacks.jl |  32 ++++--
 base/libgit2/types.jl     |  38 ++++++-
 test/libgit2-helpers.jl   |  35 ++++---
 test/libgit2.jl           | 207 ++++++++++++++++++++++++++++++++++++++
 4 files changed, 285 insertions(+), 27 deletions(-)

diff --git a/base/libgit2/callbacks.jl b/base/libgit2/callbacks.jl
index 74fb15bd17946..8e79db06ef4ea 100644
--- a/base/libgit2/callbacks.jl
+++ b/base/libgit2/callbacks.jl
@@ -48,8 +48,9 @@ function user_abort()
     return Cint(Error.EAUTH)
 end
 
-function authenticate_ssh(creds::SSHCredentials, libgit2credptr::Ptr{Ptr{Void}},
+function authenticate_ssh(libgit2credptr::Ptr{Ptr{Void}}, p::CredentialPayload,
         username_ptr, schema, host)
+    creds = Base.get(p.credential)::SSHCredentials
     isusedcreds = checkused!(creds)
 
     # Note: The same SSHCredentials can be used to authenticate separate requests using the
@@ -167,8 +168,9 @@ function authenticate_ssh(creds::SSHCredentials, libgit2credptr::Ptr{Ptr{Void}},
                  libgit2credptr, creds.user, creds.pubkey, creds.prvkey, creds.pass)
 end
 
-function authenticate_userpass(creds::UserPasswordCredentials, libgit2credptr::Ptr{Ptr{Void}},
+function authenticate_userpass(libgit2credptr::Ptr{Ptr{Void}}, p::CredentialPayload,
         schema, host, urlusername)
+    creds = Base.get(p.credential)::UserPasswordCredentials
     isusedcreds = checkused!(creds)
 
     if creds.prompt_if_incorrect
@@ -211,7 +213,7 @@ 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.AbstractCredentials` object.
+If a payload is provided then `payload_ptr` should contain a `LibGit2.CredentialPayload` object.
 
 For `LibGit2.Consts.CREDTYPE_USERPASS_PLAINTEXT` type, if the payload contains fields:
 `user` & `pass`, they are used to create authentication credentials.
@@ -240,21 +242,30 @@ function credentials_callback(libgit2credptr::Ptr{Ptr{Void}}, url_ptr::Cstring,
     err = Cint(0)
     url = unsafe_string(url_ptr)
 
+    # get `CredentialPayload` object from payload pointer
+    @assert payload_ptr != C_NULL
+    p = unsafe_pointer_to_objref(payload_ptr)[]::CredentialPayload
+
     # parse url for schema and host
     urlparts = match(URL_REGEX, url)
     schema = urlparts[:scheme] === nothing ? "" : urlparts[:scheme]
     urlusername = urlparts[:user] === nothing ? "" : urlparts[:user]
     host = urlparts[:host]
 
-    # get credentials object from payload pointer
-    @assert payload_ptr != C_NULL
-    creds = unsafe_pointer_to_objref(payload_ptr)
-    explicit = !isnull(creds[]) && !isa(Base.get(creds[]), CachedCredentials)
+    if !isnull(p.credential)
+        creds = unsafe_get(p.credential)
+        explicit = true
+    else
+        creds = Base.get(p.cache, nothing)
+        explicit = false
+    end
+
     # use ssh key or ssh-agent
     if isset(allowed_types, Cuint(Consts.CREDTYPE_SSH_KEY))
         sshcreds = get_creds!(creds, "ssh://$host", reset!(SSHCredentials(true), -1))
         if isa(sshcreds, SSHCredentials)
-            err = authenticate_ssh(sshcreds, libgit2credptr, username_ptr, schema, host)
+            p.credential = Nullable(sshcreds)
+            err = authenticate_ssh(libgit2credptr, p, username_ptr, schema, host)
             err == 0 && return err
         end
     end
@@ -268,9 +279,10 @@ function credentials_callback(libgit2credptr::Ptr{Ptr{Void}}, url_ptr::Cstring,
         # credentials
         if !isa(upcreds, UserPasswordCredentials)
             upcreds = defaultcreds
-            isa(Base.get(creds[]), CachedCredentials) && (Base.get(creds[]).creds[credid] = upcreds)
+            isa(creds, CachedCredentials) && (creds.creds[credid] = upcreds)
         end
-        return authenticate_userpass(upcreds, libgit2credptr, schema, host, urlusername)
+        p.credential = Nullable(upcreds)
+        return authenticate_userpass(libgit2credptr, p, schema, host, urlusername)
     end
 
     # No authentication method we support succeeded. The most likely cause is
diff --git a/base/libgit2/types.jl b/base/libgit2/types.jl
index 65a6cbc65ff60..c223ef8416146 100644
--- a/base/libgit2/types.jl
+++ b/base/libgit2/types.jl
@@ -161,6 +161,8 @@ Matches the [`git_checkout_options`](https://libgit2.github.com/libgit2/#HEAD/ty
     perfdata_payload::Ptr{Void}
 end
 
+abstract type Payload end
+
 """
     LibGit2.RemoteCallbacks
 
@@ -183,12 +185,16 @@ Matches the [`git_remote_callbacks`](https://libgit2.github.com/libgit2/#HEAD/ty
     payload::Ptr{Void}
 end
 
-function RemoteCallbacks(credentials::Ptr{Void}, payload::Ref{Nullable{AbstractCredentials}})
-    RemoteCallbacks(credentials=credentials, payload=pointer_from_objref(payload))
+function RemoteCallbacks(credentials_cb::Ptr{Void}, payload::Ref{<:Payload})
+    RemoteCallbacks(credentials=credentials_cb, payload=pointer_from_objref(payload))
+end
+
+function RemoteCallbacks(credentials_cb::Ptr{Void}, payload::Payload)
+    RemoteCallbacks(credentials_cb, Ref(payload))
 end
 
-function RemoteCallbacks(credentials::Ptr{Void}, payload::Nullable{<:AbstractCredentials})
-    RemoteCallbacks(credentials, Ref{Nullable{AbstractCredentials}}(payload))
+function RemoteCallbacks(credentials_cb::Ptr{Void}, credentials)
+    RemoteCallbacks(credentials_cb, CredentialPayload(credentials))
 end
 
 """
@@ -920,3 +926,27 @@ function securezero!(p::CachedCredentials)
     foreach(securezero!, values(p.cred))
     return p
 end
+
+"""
+    LibGit2.CredentialPayload
+
+Retains state between multiple calls to the credential callback. A single
+`CredentialPayload` instance will be used when authentication fails for a URL but different
+instances will be used when the URL has changed.
+"""
+mutable struct CredentialPayload <: Payload
+    credential::Nullable{AbstractCredentials}
+    cache::Nullable{CachedCredentials}
+end
+
+function CredentialPayload(credential::Nullable{<:AbstractCredentials})
+    CredentialPayload(credential, Nullable{CachedCredentials}())
+end
+
+function CredentialPayload(cache::Nullable{CachedCredentials})
+    CredentialPayload(Nullable{AbstractCredentials}(), cache)
+end
+
+function CredentialPayload()
+    CredentialPayload(Nullable{AbstractCredentials}(), Nullable{CachedCredentials}())
+end
diff --git a/test/libgit2-helpers.jl b/test/libgit2-helpers.jl
index 5b02c4346cf8d..13e5f163569a1 100644
--- a/test/libgit2-helpers.jl
+++ b/test/libgit2-helpers.jl
@@ -1,6 +1,7 @@
 # This file is a part of Julia. License is MIT: https://julialang.org/license
 
-import Base.LibGit2: AbstractCredentials, UserPasswordCredentials, SSHCredentials, CachedCredentials
+import Base.LibGit2: AbstractCredentials, UserPasswordCredentials, SSHCredentials,
+    CachedCredentials, CredentialPayload, Payload
 
 """
 Emulates the LibGit2 credential loop to allows testing of the credential_callback function
@@ -11,10 +12,10 @@ function credential_loop(
         url::AbstractString,
         user::Nullable{<:AbstractString},
         allowed_types::UInt32,
-        cache::CachedCredentials=CachedCredentials())
+        payload::CredentialPayload)
     cb = Base.LibGit2.credentials_cb()
     libgitcred_ptr_ptr = Ref{Ptr{Void}}(C_NULL)
-    payload_ptr = Ref(Nullable{AbstractCredentials}(cache))
+    payload_ptr = Ref(payload)
 
     # Number of times credentials were authenticated against. With the real LibGit2
     # credential loop this would be how many times we sent credentials to the remote.
@@ -29,7 +30,7 @@ function credential_loop(
         num_authentications += 1
 
         # Check if the callback provided us with valid credentials
-        if length(cache.cred) == 1 && first(values(cache.cred)) == valid_credential
+        if !isnull(payload.credential) && get(payload.credential) == valid_credential
             break
         end
 
@@ -44,38 +45,46 @@ end
 function credential_loop(
         valid_credential::UserPasswordCredentials,
         url::AbstractString,
-        user::Nullable{<:AbstractString}=Nullable{String}())
-    credential_loop(valid_credential, url, user, 0x000001)
+        user::Nullable{<:AbstractString}=Nullable{String}(),
+        payload::CredentialPayload=CredentialPayload())
+    credential_loop(valid_credential, url, user, 0x000001, payload)
 end
 
 function credential_loop(
         valid_credential::SSHCredentials,
         url::AbstractString,
-        user::Nullable{<:AbstractString}=Nullable{String}();
+        user::Nullable{<:AbstractString}=Nullable{String}(),
+        payload::CredentialPayload=CredentialPayload();
         use_ssh_agent::Bool=false)
-    cache = CachedCredentials()
 
     if !use_ssh_agent
+        if isnull(payload.cache)
+            payload.cache = Nullable(CachedCredentials())
+        end
+        cache = get(payload.cache)
+
         m = match(LibGit2.URL_REGEX, url)
         default_cred = LibGit2.reset!(SSHCredentials(true), -1)
         default_cred.usesshagent = "N"
         LibGit2.get_creds!(cache, "ssh://$(m[:host])", default_cred)
     end
 
-    credential_loop(valid_credential, url, user, 0x000046, cache)
+    credential_loop(valid_credential, url, user, 0x000046, payload)
 end
 
 function credential_loop(
         valid_credential::UserPasswordCredentials,
         url::AbstractString,
-        user::AbstractString)
-    credential_loop(valid_credential, url, Nullable(user))
+        user::AbstractString,
+        payload::CredentialPayload=CredentialPayload())
+    credential_loop(valid_credential, url, Nullable(user), payload)
 end
 
 function credential_loop(
         valid_credential::SSHCredentials,
         url::AbstractString,
-        user::AbstractString;
+        user::AbstractString,
+        payload::CredentialPayload=CredentialPayload();
         use_ssh_agent::Bool=false)
-    credential_loop(valid_credential, url, Nullable(user), use_ssh_agent=use_ssh_agent)
+    credential_loop(valid_credential, url, Nullable(user), payload, use_ssh_agent=use_ssh_agent)
 end
diff --git a/test/libgit2.jl b/test/libgit2.jl
index e63f989a7a7d4..d5a3ba483bf27 100644
--- a/test/libgit2.jl
+++ b/test/libgit2.jl
@@ -1613,6 +1613,10 @@ mktempdir() do dir
             LibGit2.Error.Callback, LibGit2.Error.EAUTH,
             "Aborting, user cancelled credential request.")
 
+        eauth_error = LibGit2.GitError(
+            LibGit2.Error.None, LibGit2.Error.EAUTH,
+            "No errors")
+
         @testset "SSH credential prompt" begin
             url = "git@github.com:test/package.jl"
 
@@ -1896,6 +1900,209 @@ mktempdir() do dir
             @test err == 0
             @test auth_attempts == 5
         end
+
+        @testset "SSH explicit credentials" begin
+            url = "git@github.com:test/package.jl"
+
+            invalid_key = joinpath(KEY_DIR, "invalid")
+            valid_p_key = joinpath(KEY_DIR, "valid-passphrase")
+            username = "git"
+            passphrase = "secret"
+
+            valid_cmd = """
+            include("$LIBGIT2_HELPER_PATH")
+            valid_cred = LibGit2.SSHCredentials("$username", "$passphrase", "$valid_p_key", "$valid_p_key.pub")
+            payload = CredentialPayload(Nullable(valid_cred))
+            err, auth_attempts = credential_loop(valid_cred, "$url", "$username", payload)
+            (err < 0 ? LibGit2.GitError(err) : err, auth_attempts)
+            """
+
+            invalid_cmd = """
+            include("$LIBGIT2_HELPER_PATH")
+            valid_cred = LibGit2.SSHCredentials("$username", "$passphrase", "$valid_p_key", "$valid_p_key.pub")
+            invalid_cred = LibGit2.SSHCredentials("$username", "", "$invalid_key", "$invalid_key.pub")
+            payload = CredentialPayload(Nullable(invalid_cred))
+            err, auth_attempts = credential_loop(valid_cred, "$url", "$username", payload)
+            (err < 0 ? LibGit2.GitError(err) : err, auth_attempts)
+            """
+
+            # Explicitly provided credential is correct
+            err, auth_attempts = challenge_prompt(valid_cmd, [])
+            @test err == 0
+            @test auth_attempts == 1
+
+            # TODO: Currently infinite loops
+            # Explicitly provided credential is incorrect
+            #=
+            err, auth_attempts = challenge_prompt(invalid_cmd, [])
+            @test err == 0
+            @test auth_attempts == 1
+            =#
+        end
+
+        @testset "HTTPS explicit credentials" begin
+            url = "https://github.com/test/package.jl"
+
+            valid_username = "julia"
+            valid_password = randstring(16)
+            invalid_username = "alice"
+            invalid_password = randstring(15)
+
+            valid_cmd = """
+            include("$LIBGIT2_HELPER_PATH")
+            valid_cred = LibGit2.UserPasswordCredentials("$valid_username", "$valid_password")
+            payload = CredentialPayload(Nullable(valid_cred))
+            err, auth_attempts = credential_loop(valid_cred, "$url", "", payload)
+            (err < 0 ? LibGit2.GitError(err) : err, auth_attempts)
+            """
+
+            invalid_cmd = """
+            include("$LIBGIT2_HELPER_PATH")
+            valid_cred = LibGit2.UserPasswordCredentials("$valid_username", "$valid_password")
+            invalid_cred = LibGit2.UserPasswordCredentials("$invalid_username", "$invalid_password")
+            payload = CredentialPayload(Nullable(invalid_cred))
+            err, auth_attempts = credential_loop(valid_cred, "$url", "", payload)
+            (err < 0 ? LibGit2.GitError(err) : err, auth_attempts)
+            """
+
+            # Explicitly provided credential is correct
+            err, auth_attempts = challenge_prompt(valid_cmd, [])
+            @test err == 0
+            @test auth_attempts == 1
+
+            # Explicitly provided credential is incorrect
+            err, auth_attempts = challenge_prompt(invalid_cmd, [])
+            @test err == eauth_error
+            @test auth_attempts == 4
+        end
+
+        @testset "Cached credentials" begin
+            url = "https://github.com/test/package.jl"
+            cred_id = "https://github.com"
+
+            valid_username = "julia"
+            valid_password = randstring(16)
+            invalid_username = "alice"
+            invalid_password = randstring(15)
+
+            valid_cmd = """
+            include("$LIBGIT2_HELPER_PATH")
+            valid_cred = LibGit2.UserPasswordCredentials("$valid_username", "$valid_password")
+            cache = CachedCredentials()
+            LibGit2.get_creds!(cache, "$cred_id", valid_cred)
+            payload = CredentialPayload(Nullable(cache))
+            err, auth_attempts = credential_loop(valid_cred, "$url", "", payload)
+            (err < 0 ? LibGit2.GitError(err) : err, auth_attempts)
+            """
+
+            add_cmd = """
+            include("$LIBGIT2_HELPER_PATH")
+            valid_cred = LibGit2.UserPasswordCredentials("$valid_username", "$valid_password")
+            cache = CachedCredentials()
+            payload = CredentialPayload(Nullable(cache))
+            err, auth_attempts = credential_loop(valid_cred, "$url", "", payload)
+            (err < 0 ? LibGit2.GitError(err) : err, auth_attempts, cache)
+            """
+
+            replace_cmd = """
+            include("$LIBGIT2_HELPER_PATH")
+            valid_cred = LibGit2.UserPasswordCredentials("$valid_username", "$valid_password")
+            invalid_cred = LibGit2.UserPasswordCredentials("$invalid_username", "$invalid_password", true)
+            cache = CachedCredentials()
+            LibGit2.get_creds!(cache, "$cred_id", invalid_cred)
+            payload = CredentialPayload(Nullable(cache))
+            err, auth_attempts = credential_loop(valid_cred, "$url", "", payload)
+            (err < 0 ? LibGit2.GitError(err) : err, auth_attempts, cache)
+            """
+
+            # Cache contains a correct credential
+            err, auth_attempts = challenge_prompt(valid_cmd, [])
+            @test err == 0
+            @test auth_attempts == 1
+
+            # Add a credential into the cache
+            challenges = [
+                "Username for 'https://github.com':" => "$valid_username\n",
+                "Password for 'https://$valid_username@github.com':" => "$valid_password\n",
+            ]
+            expected_cred = LibGit2.UserPasswordCredentials(valid_username, valid_password)
+            err, auth_attempts, cache = challenge_prompt(add_cmd, challenges)
+            @test err == 0
+            @test auth_attempts == 1
+            @test typeof(cache) == LibGit2.CachedCredentials
+            @test cache.cred == Dict(cred_id => expected_cred)
+
+            # Replace a credential in the cache
+            challenges = [
+                "Username for 'https://github.com' [alice]:" => "$valid_username\n",
+                "Password for 'https://$valid_username@github.com':" => "$valid_password\n",
+            ]
+            expected_cred = LibGit2.UserPasswordCredentials(valid_username, valid_password)
+            err, auth_attempts, cache = challenge_prompt(replace_cmd, challenges)
+            @test err == 0
+            @test auth_attempts == 4
+            @test typeof(cache) == LibGit2.CachedCredentials
+            @test cache.cred == Dict(cred_id => expected_cred)
+        end
+
+        @testset "Incompatible explicit credentials" begin
+            # User provides a user/password credential where a SSH credential is required.
+            ssh_cmd = """
+            include("$LIBGIT2_HELPER_PATH")
+            valid_cred = LibGit2.UserPasswordCredentials("foo", "bar")
+            payload = CredentialPayload(Nullable(valid_cred))
+            err, auth_attempts = credential_loop(valid_cred, "ssh://github.com/repo",
+                Nullable(""), Cuint(LibGit2.Consts.CREDTYPE_SSH_KEY), payload)
+            (err < 0 ? LibGit2.GitError(err) : err, auth_attempts)
+            """
+
+            # User provides a SSH credential where a user/password credential is required.
+            https_cmd = """
+            include("$LIBGIT2_HELPER_PATH")
+            valid_cred = LibGit2.SSHCredentials("foo", "", "", "")
+            payload = CredentialPayload(Nullable(valid_cred))
+            err, auth_attempts = credential_loop(valid_cred, "https://github.com/repo",
+                Nullable(""), Cuint(LibGit2.Consts.CREDTYPE_USERPASS_PLAINTEXT), payload)
+            (err < 0 ? LibGit2.GitError(err) : err, auth_attempts)
+            """
+
+            # TODO: Currently a warning is reported about the explicit credential being
+            # incompatible with the authentication method. We should change this to an
+            # error.
+            err, auth_attempts = challenge_prompt(ssh_cmd, [])
+            @test err == eauth_error
+            @test auth_attempts == 1
+
+            # TODO: Providing an explicit SSH credential which is incompatible with the
+            # authentication method triggers a prompt...
+            challenges = [
+                "Username for 'https://github.com':" => "\x04",
+            ]
+            err, auth_attempts = challenge_prompt(https_cmd, challenges)
+            @test err == abort_prompt
+            @test auth_attempts == 1
+        end
+
+        # A hypothetical scenario where the the allowed authentication can either be
+        # SSH or username/password.
+        @testset "SSH & HTTPS authentication" begin
+            allowed_types = Cuint(LibGit2.Consts.CREDTYPE_SSH_KEY) |
+                Cuint(LibGit2.Consts.CREDTYPE_USERPASS_PLAINTEXT)
+
+            # User provides a user/password credential where a SSH credential is required.
+            cmd = """
+            include("$LIBGIT2_HELPER_PATH")
+            valid_cred = LibGit2.UserPasswordCredentials("foo", "bar")
+            payload = CredentialPayload(Nullable(valid_cred))
+            err, auth_attempts = credential_loop(valid_cred, "foo://github.com/repo",
+                Nullable(""), Cuint($allowed_types), payload)
+            (err < 0 ? LibGit2.GitError(err) : err, auth_attempts)
+            """
+
+            err, auth_attempts = challenge_prompt(cmd, [])
+            @test err == 0
+            @test auth_attempts == 1
+        end
     end
 
     #= temporarily disabled until working on the buildbots, ref https://github.com/JuliaLang/julia/pull/17651#issuecomment-238211150

From cebf09ae74016cd158bb7be93afd1a9d3b15debb Mon Sep 17 00:00:00 2001
From: Curtis Vogt <curtis.vogt@gmail.com>
Date: Fri, 11 Aug 2017 23:45:39 -0500
Subject: [PATCH 2/5] Correct SSH explicit test

---
 test/libgit2.jl | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/test/libgit2.jl b/test/libgit2.jl
index d5a3ba483bf27..ede2ba1445822 100644
--- a/test/libgit2.jl
+++ b/test/libgit2.jl
@@ -1921,6 +1921,7 @@ mktempdir() do dir
             include("$LIBGIT2_HELPER_PATH")
             valid_cred = LibGit2.SSHCredentials("$username", "$passphrase", "$valid_p_key", "$valid_p_key.pub")
             invalid_cred = LibGit2.SSHCredentials("$username", "", "$invalid_key", "$invalid_key.pub")
+            invalid_cred.usesshagent = "N"  # Disable SSH agent use
             payload = CredentialPayload(Nullable(invalid_cred))
             err, auth_attempts = credential_loop(valid_cred, "$url", "$username", payload)
             (err < 0 ? LibGit2.GitError(err) : err, auth_attempts)
@@ -1931,13 +1932,11 @@ mktempdir() do dir
             @test err == 0
             @test auth_attempts == 1
 
-            # TODO: Currently infinite loops
             # Explicitly provided credential is incorrect
-            #=
+            # TODO: Unless the SSH agent is disabled we may get caught in an infinite loop
             err, auth_attempts = challenge_prompt(invalid_cmd, [])
-            @test err == 0
-            @test auth_attempts == 1
-            =#
+            @test err == eauth_error
+            @test auth_attempts == 4
         end
 
         @testset "HTTPS explicit credentials" begin

From ecd4285c13557d4394074e2fee010d6791f89097 Mon Sep 17 00:00:00 2001
From: Curtis Vogt <curtis.vogt@gmail.com>
Date: Fri, 11 Aug 2017 09:23:52 -0500
Subject: [PATCH 3/5] Error on incompatible explicit credential

---
 base/libgit2/callbacks.jl |  7 ++++---
 test/libgit2.jl           | 18 ++++++++++--------
 2 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/base/libgit2/callbacks.jl b/base/libgit2/callbacks.jl
index 8e79db06ef4ea..e4e3a59b899a7 100644
--- a/base/libgit2/callbacks.jl
+++ b/base/libgit2/callbacks.jl
@@ -287,11 +287,12 @@ function credentials_callback(libgit2credptr::Ptr{Ptr{Void}}, url_ptr::Cstring,
 
     # No authentication method we support succeeded. The most likely cause is
     # that explicit credentials were passed in, but said credentials are incompatible
-    # with the remote host.
+    # with the requested authentication method.
     if err == 0
         if explicit
-            warn("The explicitly provided credentials were incompatible with " *
-                 "the server's supported authentication methods")
+            ccall((:giterr_set_str, :libgit2), Void, (Cint, Cstring), Cint(Error.Callback),
+                  "The explicitly provided credential is incompatible with the requested " *
+                  "authentication methods.")
         end
         err = Cint(Error.EAUTH)
     end
diff --git a/test/libgit2.jl b/test/libgit2.jl
index ede2ba1445822..ba6b06d3a7477 100644
--- a/test/libgit2.jl
+++ b/test/libgit2.jl
@@ -1613,6 +1613,11 @@ mktempdir() do dir
             LibGit2.Error.Callback, LibGit2.Error.EAUTH,
             "Aborting, user cancelled credential request.")
 
+        incompatible_error = LibGit2.GitError(
+            LibGit2.Error.Callback, LibGit2.Error.EAUTH,
+            "The explicitly provided credential is incompatible with the requested " *
+            "authentication methods.")
+
         eauth_error = LibGit2.GitError(
             LibGit2.Error.None, LibGit2.Error.EAUTH,
             "No errors")
@@ -2046,7 +2051,7 @@ mktempdir() do dir
 
         @testset "Incompatible explicit credentials" begin
             # User provides a user/password credential where a SSH credential is required.
-            ssh_cmd = """
+            expect_ssh_cmd = """
             include("$LIBGIT2_HELPER_PATH")
             valid_cred = LibGit2.UserPasswordCredentials("foo", "bar")
             payload = CredentialPayload(Nullable(valid_cred))
@@ -2056,7 +2061,7 @@ mktempdir() do dir
             """
 
             # User provides a SSH credential where a user/password credential is required.
-            https_cmd = """
+            expect_https_cmd = """
             include("$LIBGIT2_HELPER_PATH")
             valid_cred = LibGit2.SSHCredentials("foo", "", "", "")
             payload = CredentialPayload(Nullable(valid_cred))
@@ -2065,11 +2070,8 @@ mktempdir() do dir
             (err < 0 ? LibGit2.GitError(err) : err, auth_attempts)
             """
 
-            # TODO: Currently a warning is reported about the explicit credential being
-            # incompatible with the authentication method. We should change this to an
-            # error.
-            err, auth_attempts = challenge_prompt(ssh_cmd, [])
-            @test err == eauth_error
+            err, auth_attempts = challenge_prompt(expect_ssh_cmd, [])
+            @test err == incompatible_error
             @test auth_attempts == 1
 
             # TODO: Providing an explicit SSH credential which is incompatible with the
@@ -2077,7 +2079,7 @@ mktempdir() do dir
             challenges = [
                 "Username for 'https://github.com':" => "\x04",
             ]
-            err, auth_attempts = challenge_prompt(https_cmd, challenges)
+            err, auth_attempts = challenge_prompt(expect_https_cmd, challenges)
             @test err == abort_prompt
             @test auth_attempts == 1
         end

From 3d4ff9d1ef9406292239d4c4d779d123368e80e7 Mon Sep 17 00:00:00 2001
From: Curtis Vogt <curtis.vogt@gmail.com>
Date: Sun, 6 Aug 2017 21:52:54 -0500
Subject: [PATCH 4/5] Store parsed callback URL in payload

---
 base/libgit2/callbacks.jl | 44 ++++++++++++++++++++-------------------
 base/libgit2/types.jl     |  8 +++++++
 2 files changed, 31 insertions(+), 21 deletions(-)

diff --git a/base/libgit2/callbacks.jl b/base/libgit2/callbacks.jl
index e4e3a59b899a7..2d6e54320dcb4 100644
--- a/base/libgit2/callbacks.jl
+++ b/base/libgit2/callbacks.jl
@@ -48,8 +48,7 @@ function user_abort()
     return Cint(Error.EAUTH)
 end
 
-function authenticate_ssh(libgit2credptr::Ptr{Ptr{Void}}, p::CredentialPayload,
-        username_ptr, schema, host)
+function authenticate_ssh(libgit2credptr::Ptr{Ptr{Void}}, p::CredentialPayload, username_ptr)
     creds = Base.get(p.credential)::SSHCredentials
     isusedcreds = checkused!(creds)
 
@@ -76,7 +75,7 @@ function authenticate_ssh(libgit2credptr::Ptr{Ptr{Void}}, p::CredentialPayload,
         username = username_ptr != Cstring(C_NULL) ? unsafe_string(username_ptr) : ""
         if isempty(username)
             uname = creds.user # check if credentials were already used
-            prompt_url = git_url(scheme=schema, host=host)
+            prompt_url = git_url(scheme=p.scheme, host=p.host)
             if !isusedcreds
                 username = uname
             else
@@ -86,7 +85,7 @@ function authenticate_ssh(libgit2credptr::Ptr{Ptr{Void}}, p::CredentialPayload,
             end
         end
 
-        prompt_url = git_url(scheme=schema, host=host, username=username)
+        prompt_url = git_url(scheme=p.scheme, host=p.host, username=username)
 
         # For SSH we need a private key location
         privatekey = if haskey(ENV,"SSH_KEY_PATH")
@@ -168,29 +167,28 @@ function authenticate_ssh(libgit2credptr::Ptr{Ptr{Void}}, p::CredentialPayload,
                  libgit2credptr, creds.user, creds.pubkey, creds.prvkey, creds.pass)
 end
 
-function authenticate_userpass(libgit2credptr::Ptr{Ptr{Void}}, p::CredentialPayload,
-        schema, host, urlusername)
+function authenticate_userpass(libgit2credptr::Ptr{Ptr{Void}}, p::CredentialPayload)
     creds = Base.get(p.credential)::UserPasswordCredentials
     isusedcreds = checkused!(creds)
 
     if creds.prompt_if_incorrect
         username = creds.user
         userpass = creds.pass
-        prompt_url = git_url(scheme=schema, host=host)
+        prompt_url = git_url(scheme=p.scheme, host=p.host)
         if Sys.iswindows()
             if isempty(username) || isempty(userpass) || isusedcreds
                 response = Base.winprompt("Please enter your credentials for '$prompt_url'", "Credentials required",
-                    isempty(username) ? urlusername : username; prompt_username = true)
+                    isempty(username) ? p.username : username; prompt_username = true)
                 isnull(response) && return user_abort()
                 username, userpass = unsafe_get(response)
             end
         elseif isusedcreds
             response = Base.prompt("Username for '$prompt_url'",
-                default=isempty(username) ? urlusername : username)
+                default=isempty(username) ? p.username : username)
             isnull(response) && return user_abort()
             username = unsafe_get(response)
 
-            prompt_url = git_url(scheme=schema, host=host, username=username)
+            prompt_url = git_url(scheme=p.scheme, host=p.host, username=username)
             response = Base.prompt("Password for '$prompt_url'", password=true)
             isnull(response) && return user_abort()
             userpass = unsafe_get(response)
@@ -240,17 +238,21 @@ function credentials_callback(libgit2credptr::Ptr{Ptr{Void}}, url_ptr::Cstring,
                               username_ptr::Cstring,
                               allowed_types::Cuint, payload_ptr::Ptr{Void})
     err = Cint(0)
-    url = unsafe_string(url_ptr)
 
     # get `CredentialPayload` object from payload pointer
     @assert payload_ptr != C_NULL
     p = unsafe_pointer_to_objref(payload_ptr)[]::CredentialPayload
 
-    # parse url for schema and host
-    urlparts = match(URL_REGEX, url)
-    schema = urlparts[:scheme] === nothing ? "" : urlparts[:scheme]
-    urlusername = urlparts[:user] === nothing ? "" : urlparts[:user]
-    host = urlparts[:host]
+    # Parse URL only during the first call to this function. Future calls will use the
+    # information cached inside the payload.
+    if isempty(p.host)
+        url = match(URL_REGEX, unsafe_string(url_ptr))
+
+        p.scheme = url[:scheme] === nothing ? "" : url[:scheme]
+        p.username = url[:user] === nothing ? "" : url[:user]
+        p.host = url[:host]
+        p.path = url[:path]
+    end
 
     if !isnull(p.credential)
         creds = unsafe_get(p.credential)
@@ -262,17 +264,17 @@ function credentials_callback(libgit2credptr::Ptr{Ptr{Void}}, url_ptr::Cstring,
 
     # use ssh key or ssh-agent
     if isset(allowed_types, Cuint(Consts.CREDTYPE_SSH_KEY))
-        sshcreds = get_creds!(creds, "ssh://$host", reset!(SSHCredentials(true), -1))
+        sshcreds = get_creds!(creds, "ssh://$(p.host)", reset!(SSHCredentials(p.username, "", true), -1))
         if isa(sshcreds, SSHCredentials)
             p.credential = Nullable(sshcreds)
-            err = authenticate_ssh(libgit2credptr, p, username_ptr, schema, host)
+            err = authenticate_ssh(libgit2credptr, p, username_ptr)
             err == 0 && return err
         end
     end
 
     if isset(allowed_types, Cuint(Consts.CREDTYPE_USERPASS_PLAINTEXT))
-        defaultcreds = reset!(UserPasswordCredentials(true), -1)
-        credid = "$(isempty(schema) ? "ssh" : schema)://$host"
+        defaultcreds = reset!(UserPasswordCredentials(p.username, "", true), -1)
+        credid = "$(isempty(p.scheme) ? "ssh" : p.scheme)://$(p.host)"
         upcreds = get_creds!(creds, credid, defaultcreds)
         # If there were stored SSH credentials, but we ended up here that must
         # mean that something went wrong. Replace the SSH credentials by user/pass
@@ -282,7 +284,7 @@ function credentials_callback(libgit2credptr::Ptr{Ptr{Void}}, url_ptr::Cstring,
             isa(creds, CachedCredentials) && (creds.creds[credid] = upcreds)
         end
         p.credential = Nullable(upcreds)
-        return authenticate_userpass(libgit2credptr, p, schema, host, urlusername)
+        return authenticate_userpass(libgit2credptr, p)
     end
 
     # No authentication method we support succeeded. The most likely cause is
diff --git a/base/libgit2/types.jl b/base/libgit2/types.jl
index c223ef8416146..f061de12f00bb 100644
--- a/base/libgit2/types.jl
+++ b/base/libgit2/types.jl
@@ -937,6 +937,14 @@ instances will be used when the URL has changed.
 mutable struct CredentialPayload <: Payload
     credential::Nullable{AbstractCredentials}
     cache::Nullable{CachedCredentials}
+    scheme::String
+    username::String
+    host::String
+    path::String
+
+    function CredentialPayload(credential::Nullable{<:AbstractCredentials}, cache::Nullable{CachedCredentials})
+        new(credential, cache, "", "", "", "")
+    end
 end
 
 function CredentialPayload(credential::Nullable{<:AbstractCredentials})

From 5cd220c23db14635389c030de6104832207dcab1 Mon Sep 17 00:00:00 2001
From: Curtis Vogt <curtis.vogt@gmail.com>
Date: Sun, 6 Aug 2017 18:05:58 -0500
Subject: [PATCH 5/5] Make credential handling more consistent

The two code paths for handling the the SSH and username/password
`allowed_types` differ enough that some bugs have cropped up. These
changes make the two code paths nearly identical which should remove
any inconsistences.
---
 base/libgit2/callbacks.jl | 57 +++++++++++++++++++++++----------------
 test/libgit2.jl           |  9 ++-----
 2 files changed, 36 insertions(+), 30 deletions(-)

diff --git a/base/libgit2/callbacks.jl b/base/libgit2/callbacks.jl
index 2d6e54320dcb4..4923318738b95 100644
--- a/base/libgit2/callbacks.jl
+++ b/base/libgit2/callbacks.jl
@@ -238,6 +238,7 @@ function credentials_callback(libgit2credptr::Ptr{Ptr{Void}}, url_ptr::Cstring,
                               username_ptr::Cstring,
                               allowed_types::Cuint, payload_ptr::Ptr{Void})
     err = Cint(0)
+    explicit = false
 
     # get `CredentialPayload` object from payload pointer
     @assert payload_ptr != C_NULL
@@ -252,39 +253,49 @@ function credentials_callback(libgit2credptr::Ptr{Ptr{Void}}, url_ptr::Cstring,
         p.username = url[:user] === nothing ? "" : url[:user]
         p.host = url[:host]
         p.path = url[:path]
-    end
 
-    if !isnull(p.credential)
-        creds = unsafe_get(p.credential)
-        explicit = true
-    else
-        creds = Base.get(p.cache, nothing)
-        explicit = false
+        # 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
+        # modification only is in effect for the first callback since `allowed_types` cannot
+        # be mutated.
+        if !isnull(p.credential)
+            explicit = true
+            cred = unsafe_get(p.credential)
+            if isa(cred, SSHCredentials)
+                allowed_types &= Cuint(Consts.CREDTYPE_SSH_KEY)
+            elseif isa(cred, UserPasswordCredentials)
+                allowed_types &= Cuint(Consts.CREDTYPE_USERPASS_PLAINTEXT)
+            else
+                allowed_types &= Cuint(0)  # Unhandled credential type
+            end
+        end
     end
 
     # use ssh key or ssh-agent
     if isset(allowed_types, Cuint(Consts.CREDTYPE_SSH_KEY))
-        sshcreds = get_creds!(creds, "ssh://$(p.host)", reset!(SSHCredentials(p.username, "", true), -1))
-        if isa(sshcreds, SSHCredentials)
-            p.credential = Nullable(sshcreds)
-            err = authenticate_ssh(libgit2credptr, p, username_ptr)
-            err == 0 && return err
+        if isnull(p.credential) || !isa(unsafe_get(p.credential), SSHCredentials)
+            creds = reset!(SSHCredentials(p.username, "", true), -1)
+            if !isnull(p.cache)
+                credid = "ssh://$(p.host)"
+                creds = get_creds!(unsafe_get(p.cache), credid, creds)
+            end
+            p.credential = Nullable(creds)
         end
+        err = authenticate_ssh(libgit2credptr, p, username_ptr)
+        err == 0 && return err
     end
 
     if isset(allowed_types, Cuint(Consts.CREDTYPE_USERPASS_PLAINTEXT))
-        defaultcreds = reset!(UserPasswordCredentials(p.username, "", true), -1)
-        credid = "$(isempty(p.scheme) ? "ssh" : p.scheme)://$(p.host)"
-        upcreds = get_creds!(creds, credid, defaultcreds)
-        # If there were stored SSH credentials, but we ended up here that must
-        # mean that something went wrong. Replace the SSH credentials by user/pass
-        # credentials
-        if !isa(upcreds, UserPasswordCredentials)
-            upcreds = defaultcreds
-            isa(creds, CachedCredentials) && (creds.creds[credid] = upcreds)
+        if isnull(p.credential) || !isa(unsafe_get(p.credential), UserPasswordCredentials)
+            creds = reset!(UserPasswordCredentials(p.username, "", true), -1)
+            if !isnull(p.cache)
+                credid = "$(isempty(p.scheme) ? "ssh" : p.scheme)://$(p.host)"
+                creds = get_creds!(unsafe_get(p.cache), credid, creds)
+            end
+            p.credential = Nullable(creds)
         end
-        p.credential = Nullable(upcreds)
-        return authenticate_userpass(libgit2credptr, p)
+        err = authenticate_userpass(libgit2credptr, p)
+        err == 0 && return err
     end
 
     # No authentication method we support succeeded. The most likely cause is
diff --git a/test/libgit2.jl b/test/libgit2.jl
index ba6b06d3a7477..72c964cb9be0a 100644
--- a/test/libgit2.jl
+++ b/test/libgit2.jl
@@ -2074,13 +2074,8 @@ mktempdir() do dir
             @test err == incompatible_error
             @test auth_attempts == 1
 
-            # TODO: Providing an explicit SSH credential which is incompatible with the
-            # authentication method triggers a prompt...
-            challenges = [
-                "Username for 'https://github.com':" => "\x04",
-            ]
-            err, auth_attempts = challenge_prompt(expect_https_cmd, challenges)
-            @test err == abort_prompt
+            err, auth_attempts = challenge_prompt(expect_https_cmd, [])
+            @test err == incompatible_error
             @test auth_attempts == 1
         end