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.

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

LibGit2 user/password authentication with ENV variables #20769

Closed
wants to merge 1 commit into from

Conversation

omus
Copy link
Member

@omus omus commented Feb 23, 2017

I'm working with a custom METADATA.jl repo that includes packages which are hosted on a private server. I have a GitLab CI setup working but in order to have Julia's Pkg.resolve() work with private packages I need some way of providing automated authentication. The GitLab CI setup provides a temporary token which can be used for HTTPS access to the private packages but currently Julia provides no way for me to use this token in an automated way.

I hope that this PR can make it in before the 0.6 feature freeze as without this I may have to manually have to resolve package dependencies without it.

@omus omus added the libgit2 The libgit2 library or the LibGit2 stdlib module label Feb 23, 2017
@omus
Copy link
Member Author

omus commented Feb 23, 2017

Note that originally I was hoping to have #20725 get merged before the feature freeze (that work also solves this problem) but the introduction of a new test framework and refactoring work make it appear unlikely that the necessary work will be merged before the freeze.

@@ -229,7 +229,7 @@ function credentials_callback(libgit2credptr::Ptr{Ptr{Void}}, url_ptr::Cstring,
end

if isset(allowed_types, Cuint(Consts.CREDTYPE_USERPASS_PLAINTEXT))
defaultcreds = reset!(UserPasswordCredentials(true), -1)
defaultcreds = reset!(UserPasswordCredentials(true), 1)
Copy link
Member Author

Choose a reason for hiding this comment

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

Was set to -1 to trigger a prompt right away. Setting it to 1 allows the ENV variables (also cached creds) be attempted before a prompt occurs

@@ -156,7 +156,7 @@ function authenticate_userpass(creds::UserPasswordCredentials, libgit2credptr::P
isnull(res) && return Cint(Error.EAUTH)
username, userpass = Base.get(res)
end
elseif isusedcreds
elseif isempty(username) || isempty(userpass) || isusedcreds
Copy link
Member Author

Choose a reason for hiding this comment

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

Make a prompt occur right away if the cached credentials are empty

@omus omus requested a review from tkelman February 23, 2017 20:17
@tkelman
Copy link
Contributor

tkelman commented Feb 23, 2017

Note that originally I was hoping to have #20725 get merged before the feature freeze

That PR really should have been opened at the latest in December if you wanted to be sure of that, there's been a huge amount of advance notice. (not to be mean, I hate that this isn't working as much as you do and we'll need to fix it, but these things take time and rushing anything in last-second isn't ideal)

Does gitlab support setting https remotes like https://<token>:[email protected]/org/repo the way github does?

(edited because I can never spell oauth correctly)

@omus
Copy link
Member Author

omus commented Feb 23, 2017

I believe GitLab supports that URL syntax. Unfortunately Julia's credential_callback doesn't use passwords specified in the URL.

Maybe I'm not understanding what you're suggesting?

@tkelman
Copy link
Contributor

tkelman commented Feb 23, 2017

If you set the remote url in a way that has the token in it, why would it need to go through the credential callback at all?

username = creds.user
userpass = creds.pass
username = Base.get(ENV, "AUTH_USER", creds.user)
userpass = Base.get(ENV, "AUTH_PASS", creds.pass)
Copy link
Contributor

Choose a reason for hiding this comment

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

are these standard / common names? should the option here be documented? seems like there's potential for collision with other applications if these are always favored/tried by default

Copy link
Member Author

Choose a reason for hiding this comment

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

I've found some examples of these being used in other applications. Mostly I was following the naming conventions set by authenticate_ssh which uses SSL_KEY_PATH, SSH_PUB_KEY_PATH and SSH_KEY_PASS. I'm fine with making these Julia specific: JULIA_AUTH_USER and JULIA_AUTH_PASS.

@omus
Copy link
Member Author

omus commented Feb 23, 2017

Note that originally I was hoping to have #20725 get merged before the feature freeze

That PR really should have been opened at the latest in December if you wanted to be sure of that, there's been a huge amount of advance notice. (not to be mean, I hate that this isn't working as much as you do and we'll need to fix it, but these things take time and rushing anything in last-second isn't ideal)

I agree it isn't ideal. It was overly optimistic of me to think I could get that PR in so close to the freeze. These changes only became necessary as of #17057 which is why I started on this work so late.

@omus
Copy link
Member Author

omus commented Feb 23, 2017

If you set the remote url in a way that has the token in it, why would it need to go through the credential callback at all?

Ah, I see what you're getting at. When running a GitLab CI job I'm given a CI_BUILD_TOKEN which is only valid for the duration of build/run. In order to use that with Pkg.resolve() I would need to modify the URLs in the private METADATA.jl for all of the private packages. That could work as a solution but it definitely isn't ideal.

@tkelman
Copy link
Contributor

tkelman commented Feb 23, 2017

there's a small chance the url rewriting functionality in setprotocol! might be flexible enough to let you prepend a dynamically changing token? nevermind, looks like that can't help you here

@omus
Copy link
Member Author

omus commented Feb 23, 2017

Too bad for setprotocol!. A good idea though. I'll look into how practical doing a string replacement is on the entire METADATA.jl is. We have separate METADATA repos for every CI job so this may be a viable solution.

@omus
Copy link
Member Author

omus commented Feb 24, 2017

Doing the in-place replacement does work:

find $(julia -e 'println(Pkg.dir("METADATA"))') -maxdepth 2 -name url | xargs sed -i "s;https://gitlab-hostname/;https://gitlab-ci-token:$CI_BUILD_TOKEN@gitlab-hostname/;"

It's definitely a suboptimal solution but it is a solution. This PR is no longer a requirement for me for Julia 0.6 and I'll close this PR in favor of eventually getting the features added in #20725.

@omus omus closed this Feb 24, 2017
@omus omus deleted the cv/libgit2-env-userpass branch February 24, 2017 15:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libgit2 The libgit2 library or the LibGit2 stdlib module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants