Skip to content

Commit

Permalink
Make credential handling more consistent
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
omus committed Aug 15, 2017
1 parent 3d4ff9d commit 5cd220c
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 30 deletions.
57 changes: 34 additions & 23 deletions base/libgit2/callbacks.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
9 changes: 2 additions & 7 deletions test/libgit2.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down

0 comments on commit 5cd220c

Please sign in to comment.