http_archive rule now has an ability to authenticate to github using .netrc file as storage for credentials #7978

genrym · 2019-04-08T22:22:23Z

Hi.

Need an opinion for the general direction.

It is still missing the default path for .netrc file to be set to home directory, which is now set as root directory

googlebot · 2019-04-08T22:22:30Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.

What to do if you already signed the CLA

Individual signers

It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

.netrc file as storage for credentials

googlebot · 2019-04-09T08:30:58Z

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

aehlig

Generally I like the idea of adding .netrc authentication, but please follow the procedure required by our design review process.

aehlig · 2019-04-09T12:44:12Z

...a/com/google/devtools/build/lib/bazel/repository/downloader/AuthorizationHeaderProvider.java

+    private static final Map<String, AuthorizationProtocol> TYPE_TO_PROTOCOL = new HashMap<>();
+
+    public AuthorizationHeaderProvider() {
+        TYPE_TO_PROTOCOL.put("github", new GithubAuthorizationProtocol());


So the "github" authentication protocol is just sticking "token " in front of the password? And that is only necessary because the .netrc format does not allow to quote spaces?

Is that something generic (so that changing the name would be appropriate) or something really used by github only? What I'm worried about is that, over time, we might accumulate special casing for each and every hosting site.

So the "github" authentication protocol is just sticking "token " in front of the password? And that is only necessary because the .netrc format does not allow to quote spaces?

That is right

Is that something generic (so that changing the name would be appropriate) or something really used by github only? What I'm worried about is that, over time, we might accumulate special casing for each and every hosting site.

Unfortunately, github uses a non-standard Authorization protocol, so this is specific for github.

OAuth2 states:
Authorization: Bearer <token>

Github states:
Authorization: token <token>

It is possible in future to add generic protocols, such as OAuth2, Basic authentication, etc.

So the "github" authentication protocol is just sticking "token " in front of the password? And that is only necessary because the .netrc format does not allow to quote spaces?

That is right

Thanks for clarifying. That does not improve my opinion on the .netrc format, but if it is popular, we'll have to support it.

Unfortunately, github uses a non-standard Authorization protocol, so this is specific for github.

OAuth2 states:
Authorization: Bearer <token>

Github states:
Authorization: token <token>

OK, so we have to special case it, unless we define a parametric WithPrefixAuthenticationProtocol.

It is possible in future to add generic protocols, such as OAuth2, Basic authentication, etc.

I would appreciate if bazel wouldn't add authentication for GitHub only.

aehlig · 2019-04-09T12:46:09Z

...main/java/com/google/devtools/build/lib/bazel/repository/downloader/CredentialsProvider.java

+
+    Optional<Credentials> getCredentials(String host);
+
+    class Credentials {


Are Credentials always user name and pasword? I.e., will a credential provide always be limited to essentially basic auth?

At the moment I am not aware of more information needed for authentication and authorization either than user/pass or api key (token - which we use password to state in netrc file).
However, if would be required, the interface can be evolved using default methods

aehlig · 2019-04-09T12:47:23Z

...java/com/google/devtools/build/lib/bazel/repository/downloader/HttpConnectorMultiplexer.java

@@ -27,6 +25,9 @@
 import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.events.EventHandler;
 import com.google.devtools.build.lib.util.Sleeper;
+
+import javax.annotation.Nullable;
+import javax.annotation.concurrent.GuardedBy;
 import java.io.IOException;


This rearrangement of imports seems unrelated (and will be changed back anyway upon import into the authoritative repository).

That's just FYI, there is nothing to do on your side.

aehlig · 2019-04-09T12:54:54Z

...va/com/google/devtools/build/lib/skylarkbuildapi/repository/SkylarkRepositoryContextApi.java

@@ -459,6 +461,24 @@ public void extract(Object archive, Object output, String stripPrefix, Location
                    + " <code>build_file</code>, this field can be used to strip it from extracted"
                    + " files."),
        @Param(


This is a change of a public Starlark build API interface, for which the bazel policy requires a design discussion; see https://github.com/bazelbuild/proposals/

Will post one, thanks :)

Will post one, thanks :)

Thanks. So I'll wait for that design document being written and discussed before processing with this pull request.

@aehlig added the proposal: bazelbuild/proposals#115
Should I assign you as a reviewer?

aehlig · 2019-04-09T12:58:15Z

...va/com/google/devtools/build/lib/skylarkbuildapi/repository/SkylarkRepositoryContextApi.java

@@ -459,6 +461,24 @@ public void extract(Object archive, Object output, String stripPrefix, Location
                    + " <code>build_file</code>, this field can be used to strip it from extracted"
                    + " files."),
        @Param(
+            name = "is_netrc_auth_enabled",


Isn't that flag implicit by whether or not the appropriate key is found in netrc_domain_auth_types?

You are right, we can remove it

aehlig · 2019-04-09T12:59:29Z

...va/com/google/devtools/build/lib/skylarkbuildapi/repository/SkylarkRepositoryContextApi.java

+            type = SkylarkDict.class,
+            defaultValue = "{}",
+            named = true,
+            doc = "the authorization type which is the host in netrc file for now support \"github\""),


What about true basic auth? Isn't that provided by the default authorization protocol anyway?

We can add a BasicAuthenticationProtocol implementation by type basic if needed. I might be missing, is there another implementation for basic authentication already?

genrym · 2019-04-09T13:43:21Z

...a/com/google/devtools/build/lib/bazel/repository/downloader/AuthorizationHeaderProvider.java

+    private static final Map<String, AuthorizationProtocol> TYPE_TO_PROTOCOL = new HashMap<>();
+
+    public AuthorizationHeaderProvider() {
+        TYPE_TO_PROTOCOL.put("github", new GithubAuthorizationProtocol());


So the "github" authentication protocol is just sticking "token " in front of the password? And that is only necessary because the .netrc format does not allow to quote spaces?

That is right

genrym · 2019-04-09T13:50:11Z

...a/com/google/devtools/build/lib/bazel/repository/downloader/AuthorizationHeaderProvider.java

+    private static final Map<String, AuthorizationProtocol> TYPE_TO_PROTOCOL = new HashMap<>();
+
+    public AuthorizationHeaderProvider() {
+        TYPE_TO_PROTOCOL.put("github", new GithubAuthorizationProtocol());


Is that something generic (so that changing the name would be appropriate) or something really used by github only? What I'm worried about is that, over time, we might accumulate special casing for each and every hosting site.

Unfortunately, github uses a non-standard Authorization protocol, so this is specific for github.

OAuth2 states:
Authorization: Bearer <token>

Github states:
Authorization: token <token>

It is possible in future to add generic protocols, such as OAuth2, Basic authentication, etc.

genrym · 2019-04-09T13:58:24Z

...main/java/com/google/devtools/build/lib/bazel/repository/downloader/CredentialsProvider.java

+
+    Optional<Credentials> getCredentials(String host);
+
+    class Credentials {


At the moment I am not aware of more information needed for authentication and authorization either than user/pass or api key (token - which we use password to state in netrc file).
However, if would be required, the interface can be evolved using default methods

genrym · 2019-04-09T14:04:10Z

...va/com/google/devtools/build/lib/skylarkbuildapi/repository/SkylarkRepositoryContextApi.java

@@ -459,6 +461,24 @@ public void extract(Object archive, Object output, String stripPrefix, Location
                    + " <code>build_file</code>, this field can be used to strip it from extracted"
                    + " files."),
        @Param(


Will post one, thanks :)

genrym · 2019-04-09T14:06:05Z

...va/com/google/devtools/build/lib/skylarkbuildapi/repository/SkylarkRepositoryContextApi.java

@@ -459,6 +461,24 @@ public void extract(Object archive, Object output, String stripPrefix, Location
                    + " <code>build_file</code>, this field can be used to strip it from extracted"
                    + " files."),
        @Param(
+            name = "is_netrc_auth_enabled",


You are right, we can remove it

genrym · 2019-04-09T14:08:34Z

...va/com/google/devtools/build/lib/skylarkbuildapi/repository/SkylarkRepositoryContextApi.java

+            type = SkylarkDict.class,
+            defaultValue = "{}",
+            named = true,
+            doc = "the authorization type which is the host in netrc file for now support \"github\""),


We can add a BasicAuthenticationProtocol implementation by type basic if needed. I might be missing, is there another implementation for basic authentication already?

genrym · 2019-04-09T14:11:12Z

src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/HttpDownloader.java

@@ -78,6 +90,7 @@ public void setDistdir(List<Path> distdir) {
   * @param clientEnv environment variables in shell issuing this command
   * @param repo the name of the external repository for which the file was fetched; used only for
   *     reporting
+   * @param authorization TBD


will add description

jin · 2019-04-30T17:54:39Z

Is this blocked on a proposal?

genrym · 2019-04-30T18:14:16Z

I am changing the implementation according to comments, which seems like also changing the api a bit, then will send a proposal for api

genrym · 2019-05-01T12:11:43Z

proposal submitted: bazelbuild/proposals#115

genrym · 2019-05-01T17:13:43Z

only now saw that there is an issue for that already, which this PR tries to solve: #7770

genrym · 2019-07-26T08:01:29Z

@aehlig (thanks to you :) ) we have already a solution for http_archive rule will respect by default .netrc stored credentials and use HTTP basic authentication for the related domain/host which will be available at version 0.29.
Proposing to close this one.

jin · 2019-08-01T20:08:00Z

Closing this PR as @aehlig has integrated netrc w/ http_* rules. Thanks @aehlig :)

googlebot added the cla: no label Apr 8, 2019

http_archive rule now has an ability to authenticate to github using

4eba284

.netrc file as storage for credentials

genrym force-pushed the http-netrc-auth branch from 40652bb to 4eba284 Compare April 9, 2019 08:30

googlebot added cla: yes and removed cla: no labels Apr 9, 2019

aehlig reviewed Apr 9, 2019

View reviewed changes

genrym commented Apr 9, 2019

View reviewed changes

removed redundant property

434fada

jin assigned aehlig Apr 19, 2019

jin added the team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. label Apr 19, 2019

ittaiz mentioned this pull request May 2, 2019

http_archive should support authentication #7770

Closed

jin closed this Aug 1, 2019

keith mentioned this pull request Sep 4, 2019

[Security] netrc support passes original credentials on HTTP redirects #9327

Closed

philwo added the team-OSS Issues for the Bazel OSS team: installation, release processBazel packaging, website label Jun 15, 2020

philwo unassigned aehlig Jun 15, 2020

philwo removed the team-OSS Issues for the Bazel OSS team: installation, release processBazel packaging, website label Nov 29, 2021


		Optional<Credentials> getCredentials(String host);

		class Credentials {

http_archive rule now has an ability to authenticate to github using .netrc file as storage for credentials #7978

http_archive rule now has an ability to authenticate to github using .netrc file as storage for credentials #7978

Conversation

genrym commented Apr 8, 2019

googlebot commented Apr 8, 2019

What to do if you already signed the CLA

Individual signers

Corporate signers

googlebot commented Apr 9, 2019

aehlig left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jin commented Apr 30, 2019

genrym commented Apr 30, 2019

genrym commented May 1, 2019

genrym commented May 1, 2019 • edited Loading

genrym commented Jul 26, 2019 • edited Loading

jin commented Aug 1, 2019

genrym commented May 1, 2019 •

edited

Loading

genrym commented Jul 26, 2019 •

edited

Loading