Merge pull request #23690 from JuliaLang/cv/libgit2-allow-keywords

Add settings to CredentialPayload

NEWS.md

@@ -445,6 +445,10 @@ Deprecated or removed
 
   * `contains(eq, itr, item)` is deprecated in favor of `any` with a predicate ([#23716]).
 
+  * Constructors for `LibGit2.UserPasswordCredentials` and `LibGit2.SSHCredentials` which take a
+    `prompt_if_incorrect` argument are deprecated. Instead, prompting behavior is controlled using
+    the `allow_prompt` keyword in the `LibGit2.CredentialPayload` constructor ([#23690]).
+
 Command-line option changes
 ---------------------------
 

base/deprecated.jl

@@ -1806,6 +1806,10 @@ end
 
 @deprecate contains(eq::Function, itr, x) any(y->eq(y,x), itr)
 
+# PR #23690
+# `SSHCredentials` and `UserPasswordCredentials` constructors using `prompt_if_incorrect`
+# are deprecated in base/libgit2/types.jl.
+
 # END 0.7 deprecations
 
 # BEGIN 1.0 deprecations

base/libgit2/callbacks.jl

@@ -57,18 +57,15 @@ function authenticate_ssh(libgit2credptr::Ptr{Ptr{Void}}, p::CredentialPayload,
     end
 
     # first try ssh-agent if credentials support its usage
-    if p.use_ssh_agent == 'Y' && username_ptr != Cstring(C_NULL)
+    if p.use_ssh_agent && username_ptr != Cstring(C_NULL)
         err = ccall((:git_cred_ssh_key_from_agent, :libgit2), Cint,
                     (Ptr{Ptr{Void}}, Cstring), libgit2credptr, username_ptr)
-        if err == 0
-            p.use_ssh_agent = 'U'  # used ssh-agent only one time
-            return Cint(0)
-        else
-            p.use_ssh_agent = 'E'
-        end
+
+        p.use_ssh_agent = false  # use ssh-agent only one time
+        err == 0 && return Cint(0)
     end
 
-    if creds.prompt_if_incorrect
+    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)
@@ -166,7 +163,7 @@ function authenticate_userpass(libgit2credptr::Ptr{Ptr{Void}}, p::CredentialPayl
         creds.pass = ""
     end
 
-    if creds.prompt_if_incorrect
+    if p.allow_prompt
         username = creds.user
         userpass = creds.pass
         if isempty(username) || isempty(userpass)
@@ -266,7 +263,7 @@ 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))
         if isnull(p.credential) || !isa(unsafe_get(p.credential), SSHCredentials)
-            creds = SSHCredentials(p.username, "", true)
+            creds = SSHCredentials(p.username)
             if !isnull(p.cache)
                 credid = "ssh://$(p.host)"
                 creds = get_creds!(unsafe_get(p.cache), credid, creds)
@@ -279,7 +276,7 @@ function credentials_callback(libgit2credptr::Ptr{Ptr{Void}}, url_ptr::Cstring,
 
     if isset(allowed_types, Cuint(Consts.CREDTYPE_USERPASS_PLAINTEXT))
         if isnull(p.credential) || !isa(unsafe_get(p.credential), UserPasswordCredentials)
-            creds = UserPasswordCredentials(p.username, "", true)
+            creds = UserPasswordCredentials(p.username)
             if !isnull(p.cache)
                 credid = "$(isempty(p.scheme) ? "ssh" : p.scheme)://$(p.host)"
                 creds = get_creds!(unsafe_get(p.cache), credid, creds)

base/libgit2/types.jl

@@ -1066,13 +1066,21 @@ abstract type AbstractCredentials end
 mutable struct UserPasswordCredentials <: AbstractCredentials
     user::String
     pass::String
-    prompt_if_incorrect::Bool    # Whether to allow interactive prompting if the credentials are incorrect
-    function UserPasswordCredentials(u::AbstractString,p::AbstractString,prompt_if_incorrect::Bool=false)
-        c = new(u,p,prompt_if_incorrect)
+    function UserPasswordCredentials(user::AbstractString="", pass::AbstractString="")
+        c = new(user, pass)
         finalizer(c, securezero!)
         return c
     end
-    UserPasswordCredentials(prompt_if_incorrect::Bool=false) = UserPasswordCredentials("","",prompt_if_incorrect)
+
+    # Deprecated constructors
+    function UserPasswordCredentials(u::AbstractString,p::AbstractString,prompt_if_incorrect::Bool)
+        Base.depwarn(string(
+            "`UserPasswordCredentials` no longer supports the `prompt_if_incorrect` parameter. ",
+            "Use the `allow_prompt` keyword in supported by `LibGit2.CredentialPayload` ",
+            "instead."), :UserPasswordCredentials)
+        UserPasswordCredentials(u, p)
+    end
+    UserPasswordCredentials(prompt_if_incorrect::Bool) = UserPasswordCredentials("","",prompt_if_incorrect)
 end
 
 function securezero!(cred::UserPasswordCredentials)
@@ -1091,14 +1099,23 @@ mutable struct SSHCredentials <: AbstractCredentials
     pass::String
     prvkey::String
     pubkey::String
-    prompt_if_incorrect::Bool    # Whether to allow interactive prompting if the credentials are incorrect
-    function SSHCredentials(u::AbstractString,p::AbstractString,prvkey::AbstractString,pubkey::AbstractString,prompt_if_incorrect::Bool=false)
-        c = new(u,p,prvkey,pubkey,prompt_if_incorrect)
+    function SSHCredentials(user::AbstractString="", pass::AbstractString="",
+                            prvkey::AbstractString="", pubkey::AbstractString="")
+        c = new(user, pass, prvkey, pubkey)
         finalizer(c, securezero!)
         return c
     end
-    SSHCredentials(u::AbstractString,p::AbstractString,prompt_if_incorrect::Bool=false) = SSHCredentials(u,p,"","",prompt_if_incorrect)
-    SSHCredentials(prompt_if_incorrect::Bool=false) = SSHCredentials("","","","",prompt_if_incorrect)
+
+    # Deprecated constructors
+    function SSHCredentials(u::AbstractString,p::AbstractString,prvkey::AbstractString,pubkey::AbstractString,prompt_if_incorrect::Bool)
+        Base.depwarn(string(
+            "`SSHCredentials` no longer supports the `prompt_if_incorrect` parameter. ",
+            "Use the `allow_prompt` keyword in supported by `LibGit2.CredentialPayload` ",
+            "instead."), :SSHCredentials)
+        SSHCredentials(u, p, prvkey, pubkey)
+    end
+    SSHCredentials(u::AbstractString, p::AbstractString, prompt_if_incorrect::Bool) = SSHCredentials(u,p,"","",prompt_if_incorrect)
+    SSHCredentials(prompt_if_incorrect::Bool) = SSHCredentials("","","","",prompt_if_incorrect)
 end
 
 function securezero!(cred::SSHCredentials)
@@ -1130,39 +1147,41 @@ 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.
+Retains the state between multiple calls to the credential callback for the same URL.
+A `CredentialPayload` instance is expected to be `reset!` whenever it will be used with a
+different URL.
 """
 mutable struct CredentialPayload <: Payload
     explicit::Nullable{AbstractCredentials}
     cache::Nullable{CachedCredentials}
+    allow_ssh_agent::Bool  # Allow the use of the SSH agent to get credentials
+    allow_prompt::Bool     # Allow prompting the user for credentials
 
     # Ephemeral state fields
     credential::Nullable{AbstractCredentials}
     first_pass::Bool
-    use_ssh_agent::Char
+    use_ssh_agent::Bool
     scheme::String
     username::String
     host::String
     path::String
 
-    function CredentialPayload(credential::Nullable{<:AbstractCredentials}, cache::Nullable{CachedCredentials})
-        payload = new(credential, cache)
+    function CredentialPayload(
+            credential::Nullable{<:AbstractCredentials}=Nullable{AbstractCredentials}(),
+            cache::Nullable{CachedCredentials}=Nullable{CachedCredentials}();
+            allow_ssh_agent::Bool=true,
+            allow_prompt::Bool=true)
+        payload = new(credential, cache, allow_ssh_agent, allow_prompt)
         return reset!(payload)
     end
 end
 
-function CredentialPayload(credential::AbstractCredentials)
-    CredentialPayload(Nullable(credential), Nullable{CachedCredentials}())
-end
-
-function CredentialPayload(cache::CachedCredentials)
-    CredentialPayload(Nullable{AbstractCredentials}(), Nullable(cache))
+function CredentialPayload(credential::AbstractCredentials; kwargs...)
+    CredentialPayload(Nullable(credential), Nullable{CachedCredentials}(); kwargs...)
 end
 
-function CredentialPayload()
-    CredentialPayload(Nullable{AbstractCredentials}(), Nullable{CachedCredentials}())
+function CredentialPayload(cache::CachedCredentials; kwargs...)
+    CredentialPayload(Nullable{AbstractCredentials}(), Nullable(cache); kwargs...)
 end
 
 """
@@ -1174,7 +1193,7 @@ the credential callback.
 function reset!(p::CredentialPayload)
     p.credential = Nullable{AbstractCredentials}()
     p.first_pass = true
-    p.use_ssh_agent = 'Y'
+    p.use_ssh_agent = p.allow_ssh_agent
     p.scheme = ""
     p.username = ""
     p.host = ""

test/libgit2-helpers.jl

@@ -61,29 +61,14 @@ function credential_loop(
         valid_credential::SSHCredentials,
         url::AbstractString,
         user::Nullable{<:AbstractString}=Nullable{String}(),
-        payload::CredentialPayload=CredentialPayload();
-        use_ssh_agent::Bool=false)
-
-    if !use_ssh_agent
-        payload.use_ssh_agent = 'N'
-    end
-
+        payload::CredentialPayload=CredentialPayload(allow_ssh_agent=false))
     credential_loop(valid_credential, url, user, 0x000046, payload)
 end
 
 function credential_loop(
-        valid_credential::UserPasswordCredentials,
+        valid_credential::AbstractCredentials,
         url::AbstractString,
         user::AbstractString,
-        payload::CredentialPayload=CredentialPayload())
+        payload::CredentialPayload=CredentialPayload(allow_ssh_agent=false))
     credential_loop(valid_credential, url, Nullable(user), payload)
 end
-
-function credential_loop(
-        valid_credential::SSHCredentials,
-        url::AbstractString,
-        user::AbstractString,
-        payload::CredentialPayload=CredentialPayload();
-        use_ssh_agent::Bool=false)
-    credential_loop(valid_credential, url, Nullable(user), payload, use_ssh_agent=use_ssh_agent)
-end

test/libgit2-online.jl

@@ -9,7 +9,8 @@ mktempdir() do dir
     @testset "Cloning repository" begin
         @testset "with 'https' protocol" begin
             repo_path = joinpath(dir, "Example1")
-            repo = LibGit2.clone(repo_url, repo_path)
+            payload = LibGit2.CredentialPayload(allow_prompt=false)
+            repo = LibGit2.clone(repo_url, repo_path, payload=payload)
             try
                 @test isdir(repo_path)
                 @test isdir(joinpath(repo_path, ".git"))
@@ -23,7 +24,7 @@ mktempdir() do dir
                 repo_path = joinpath(dir, "Example2")
                 # credentials are required because github tries to authenticate on unknown repo
                 cred = LibGit2.UserPasswordCredentials("JeffBezanson", "hunter2") # make sure Jeff is using a good password :)
-                payload = LibGit2.CredentialPayload(cred)
+                payload = LibGit2.CredentialPayload(cred, allow_prompt=false)
                 LibGit2.clone(repo_url*randstring(10), repo_path, payload=payload)
                 error("unexpected")
             catch ex
@@ -37,7 +38,7 @@ mktempdir() do dir
                 repo_path = joinpath(dir, "Example3")
                 # credentials are required because github tries to authenticate on unknown repo
                 cred = LibGit2.UserPasswordCredentials("","") # empty credentials cause authentication error
-                payload = LibGit2.CredentialPayload(cred)
+                payload = LibGit2.CredentialPayload(cred, allow_prompt=false)
                 LibGit2.clone(repo_url*randstring(10), repo_path, payload=payload)
                 error("unexpected")
             catch ex

test/libgit2.jl

@@ -1936,26 +1936,23 @@ mktempdir() do dir
             function gen_ex(; username="git")
                 quote
                     include($LIBGIT2_HELPER_PATH)
-                    credential_loop($valid_cred, $url, Nullable{String}($username), use_ssh_agent=true)
+                    payload = CredentialPayload(allow_ssh_agent=true, allow_prompt=false)
+                    credential_loop($valid_cred, $url, Nullable{String}($username), payload)
                 end
             end
 
-            challenges = [
-                "Username for 'github.com':" => "\x04",
-            ]
-
             # An empty string username_ptr
             ex = gen_ex(username="")
-            err, auth_attempts = challenge_prompt(ex, challenges)
-            @test err == abort_prompt  # TODO: `eauth_error` when we can disable prompting
+            err, auth_attempts = challenge_prompt(ex, [])
+            @test err == eauth_error
             @test auth_attempts == 2
 
             # 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, challenges)
-            @test err == abort_prompt  # TODO: `eauth_error` when we can disable prompting
-            @test auth_attempts == 1
+            err, auth_attempts = challenge_prompt(ex, [])
+            @test err == eauth_error
+            @test auth_attempts == 2
         end
 
         @testset "SSH explicit credentials" begin
@@ -1969,22 +1966,22 @@ mktempdir() do dir
             invalid_key = joinpath(KEY_DIR, "invalid")
             invalid_cred = LibGit2.SSHCredentials(username, "", invalid_key, invalid_key * ".pub")
 
-            function gen_ex(cred)
+            function gen_ex(cred; allow_prompt=true)
                 quote
                     include($LIBGIT2_HELPER_PATH)
-                    payload = CredentialPayload($cred)
-                    credential_loop($valid_cred, $url, $username, payload, use_ssh_agent=false)
+                    payload = CredentialPayload($cred, allow_ssh_agent=false, allow_prompt=$allow_prompt)
+                    credential_loop($valid_cred, $url, $username, payload)
                 end
             end
 
             # Explicitly provided credential is correct
-            ex = gen_ex(valid_cred)
+            ex = gen_ex(valid_cred, allow_prompt=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)
+            ex = gen_ex(invalid_cred, allow_prompt=false)
             err, auth_attempts = challenge_prompt(ex, [])
             @test err == eauth_error
             @test auth_attempts == 2
@@ -1996,22 +1993,22 @@ mktempdir() do dir
             valid_cred = LibGit2.UserPasswordCredentials("julia", randstring(16))
             invalid_cred = LibGit2.UserPasswordCredentials("alice", randstring(15))
 
-            function gen_ex(cred)
+            function gen_ex(cred; allow_prompt=true)
                 quote
                     include($LIBGIT2_HELPER_PATH)
-                    payload = CredentialPayload($cred)
+                    payload = CredentialPayload($cred, allow_prompt=$allow_prompt)
                     credential_loop($valid_cred, $url, "", payload)
                 end
             end
 
             # Explicitly provided credential is correct
-            ex = gen_ex(valid_cred)
+            ex = gen_ex(valid_cred, allow_prompt=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)
+            ex = gen_ex(invalid_cred, allow_prompt=false)
             err, auth_attempts = challenge_prompt(ex, [])
             @test err == eauth_error
             @test auth_attempts == 2
@@ -2023,11 +2020,11 @@ mktempdir() do dir
 
             valid_username = "julia"
             valid_password = randstring(16)
-            valid_cred = LibGit2.UserPasswordCredentials(valid_username, valid_password, true)
+            valid_cred = LibGit2.UserPasswordCredentials(valid_username, valid_password)
 
             invalid_username = "alice"
             invalid_password = randstring(15)
-            invalid_cred = LibGit2.UserPasswordCredentials(invalid_username, invalid_password, true)
+            invalid_cred = LibGit2.UserPasswordCredentials(invalid_username, invalid_password)
 
             function gen_ex(; cached_cred=nothing)
                 quote
@@ -2076,7 +2073,7 @@ mktempdir() do dir
             expect_ssh_ex = quote
                 include($LIBGIT2_HELPER_PATH)
                 valid_cred = LibGit2.UserPasswordCredentials("foo", "bar")
-                payload = CredentialPayload(valid_cred)
+                payload = CredentialPayload(valid_cred, allow_ssh_agent=false)
                 credential_loop(valid_cred, "ssh://github.com/repo", Nullable(""),
                     Cuint(LibGit2.Consts.CREDTYPE_SSH_KEY), payload)
             end
@@ -2089,7 +2086,7 @@ mktempdir() do dir
             expect_https_ex = quote
                 include($LIBGIT2_HELPER_PATH)
                 valid_cred = LibGit2.SSHCredentials("foo", "", "", "")
-                payload = CredentialPayload(valid_cred)
+                payload = CredentialPayload(valid_cred, allow_ssh_agent=false)
                 credential_loop(valid_cred, "https://github.com/repo", Nullable(""),
                     Cuint(LibGit2.Consts.CREDTYPE_USERPASS_PLAINTEXT), payload)
             end
@@ -2109,7 +2106,7 @@ mktempdir() do dir
             ex = quote
                 include($LIBGIT2_HELPER_PATH)
                 valid_cred = LibGit2.UserPasswordCredentials("foo", "bar")
-                payload = CredentialPayload(valid_cred)
+                payload = CredentialPayload(valid_cred, allow_ssh_agent=false)
                 credential_loop(valid_cred, "foo://github.com/repo", Nullable(""),
                     $allowed_types, payload)
             end