Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add LibGit2 CredentialPayload struct #23189

Merged
merged 5 commits into from
Aug 16, 2017
Merged

Add LibGit2 CredentialPayload struct #23189

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5ef2ca9
Add minimal CredentialPayload implementation
omus Aug 6, 2017
cebf09a
Correct SSH explicit test
omus Aug 12, 2017
ecd4285
Error on incompatible explicit credential
omus Aug 11, 2017
3d4ff9d
Store parsed callback URL in payload
omus Aug 7, 2017
5cd220c
Make credential handling more consistent
omus Aug 6, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
102 changes: 64 additions & 38 deletions base/libgit2/callbacks.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,8 @@ function user_abort()
return Cint(Error.EAUTH)
end

function authenticate_ssh(creds::SSHCredentials, libgit2credptr::Ptr{Ptr{Void}},
username_ptr, schema, host)
function authenticate_ssh(libgit2credptr::Ptr{Ptr{Void}}, p::CredentialPayload, username_ptr)
creds = Base.get(p.credential)::SSHCredentials
isusedcreds = checkused!(creds)

# Note: The same SSHCredentials can be used to authenticate separate requests using the
Expand All @@ -75,7 +75,7 @@ function authenticate_ssh(creds::SSHCredentials, libgit2credptr::Ptr{Ptr{Void}},
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
Expand All @@ -85,7 +85,7 @@ function authenticate_ssh(creds::SSHCredentials, libgit2credptr::Ptr{Ptr{Void}},
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")
Expand Down Expand Up @@ -167,28 +167,28 @@ 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}},
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)
Expand All @@ -211,7 +211,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.
Expand All @@ -238,48 +238,74 @@ 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)
explicit = false

# 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
# get `CredentialPayload` object from payload pointer
@assert payload_ptr != C_NULL
creds = unsafe_pointer_to_objref(payload_ptr)
explicit = !isnull(creds[]) && !isa(Base.get(creds[]), CachedCredentials)
p = unsafe_pointer_to_objref(payload_ptr)[]::CredentialPayload

# 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]

# 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://$host", reset!(SSHCredentials(true), -1))
if isa(sshcreds, SSHCredentials)
err = authenticate_ssh(sshcreds, libgit2credptr, username_ptr, schema, host)
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)
Copy link
Member Author

@omus omus Aug 9, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previously, if user supplied a UserPasswordCredentials type and SSH was one of the allowed types we would just skip SSH authentication. The PR code changes this to attempt to first attempt to do an SSH authentication then do a user/password authentication. In the process the passed in user credentials will be overwritten and not used.

We may want to revise the logic further to better support when authentication can use either SSH or user/password but I'd like to avoid trying doing that at the moment as I don't have any real-world examples of this behaviour.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That seems unintuitive, wouldn't UserPasswordCredentials only be passed if the user expected them to be used?

Are any of the methods here visible externally, would this need deprecations anywhere?

Copy link
Member Author

@omus omus Aug 9, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That seems unintuitive, wouldn't UserPasswordCredentials only be passed if the user expected them to be used?

Yes, the user should be expecting them to be used but it is possible they cannot be used. Let me go over a quick example. If you run:

LibGit2.clone("ssh://git@github.com/user/my-private-repo.git", payload=Nullable(LibGit2.UserPasswordCredentials("foo", "bar")))

The credential callback will be requesting making a request for SSH credentials. But since the user passed in UserPasswordCredentials we can't do anything with these and we need to ignore these credentials (PR behaviour) or abend (original behaviour).

In a completely hypothetical scenario where the allowed_types mask stated that either a SSH credential or user/pass credential could be returned (I have yet to see this) and we ran the same code above we would end up prompting the user for SSH credentials even though we could just try using the passed in UserPassCredentials. This is the scenario I was referring to in my earlier comment.

Are any of the methods here visible externally, would this need deprecations anywhere?

I don't think any deprecations are required as the change now falls back to prompting the user for credentials instead of abending when the explicit credential fails.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there still a way to force error-on-bad-credentials behavior? prompting could be a bad idea in some non-interactive uses

Copy link
Member Author

@omus omus Aug 9, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a PR in the pipeline which will allow the option of disallowing prompting for all credentials. For now however I'll try to restore the original abending behaviour for this PR.

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(true), -1)
credid = "$(isempty(schema) ? "ssh" : schema)://$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(Base.get(creds[]), CachedCredentials) && (Base.get(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
return authenticate_userpass(upcreds, libgit2credptr, schema, host, urlusername)
err = authenticate_userpass(libgit2credptr, p)
err == 0 && return err
end

# 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
Expand Down
46 changes: 42 additions & 4 deletions base/libgit2/types.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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

"""
Expand Down Expand Up @@ -920,3 +926,35 @@ 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}
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})
CredentialPayload(credential, Nullable{CachedCredentials}())
end

function CredentialPayload(cache::Nullable{CachedCredentials})
CredentialPayload(Nullable{AbstractCredentials}(), cache)
end

function CredentialPayload()
CredentialPayload(Nullable{AbstractCredentials}(), Nullable{CachedCredentials}())
end
35 changes: 22 additions & 13 deletions test/libgit2-helpers.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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.
Expand All @@ -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

Expand All @@ -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
Loading