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

[FR]: Basic auth for npm registry #623

Closed
1 task
LavaToaster opened this issue Nov 18, 2022 · 10 comments · Fixed by #646
Closed
1 task

[FR]: Basic auth for npm registry #623

LavaToaster opened this issue Nov 18, 2022 · 10 comments · Fixed by #646
Labels
enhancement New feature or request

Comments

@LavaToaster
Copy link
Contributor

What is the current behavior?

The npm authentication implemented by this repository only supports token-based authentication.

Describe the feature

My company uses Azure DevOps Artifacts. When configuring your local .npmrc file it requires us to use npm's basic auth when fetching packages. You can see their guide on how to configure your .npmrc file here - https://learn.microsoft.com/en-us/azure/devops/artifacts/npm/scopes?view=azure-devops#set-up-credentials

Without this, our pnpm local installs work, but anything downloaded from the rules here do not.

Fund our work

@LavaToaster LavaToaster added the enhancement New feature or request label Nov 18, 2022
@LavaToaster
Copy link
Contributor Author

LavaToaster commented Nov 18, 2022

I do have some initial work on this that you can use as a guideline to implement this, but you would need to be able to base64 decode the password field in the npmrc file - which I cannot figure out how to do in Starlark.

Click here for some code in diff format
diff --git a/npm/npm_import.bzl b/npm/npm_import.bzl
index 55579c0..4199055 100644
--- a/npm/npm_import.bzl
+++ b/npm/npm_import.bzl
@@ -414,6 +414,8 @@ def npm_import(
         patches = [],
         custom_postinstall = "",
         npm_auth = "",
+        npm_auth_username = "",
+        npm_auth_password = "",
         bins = {},
         **kwargs):
     """Import a single npm package into Bazel.
@@ -632,6 +634,8 @@ def npm_import(
         patches = patches,
         custom_postinstall = custom_postinstall,
         npm_auth = npm_auth,
+        npm_auth_username = npm_auth_username,
+        npm_auth_password = npm_auth_password,
         run_lifecycle_hooks = run_lifecycle_hooks,
         extra_build_content = (
             extra_build_content if type(extra_build_content) == "string" else "\n".join(extra_build_content)
diff --git a/npm/private/npm_import.bzl b/npm/private/npm_import.bzl
index 1dc55cf..c466816 100644
--- a/npm/private/npm_import.bzl
+++ b/npm/private/npm_import.bzl
@@ -356,7 +356,13 @@ def _impl(rctx):
             "pattern": "Bearer <password>",
             "password": rctx.attr.npm_auth,
         },
-    } if rctx.attr.npm_auth else {}
+    } if rctx.attr.npm_auth else {
+        download_url: {
+            "type": "basic",
+            "login": rctx.attr.npm_auth_username,
+            "password": rctx.attr.npm_auth_password,
+        },
+    } if rctx.attr.npm_auth_username and rctx.attr.npm_auth_password else {}
 
     rctx.download(
         output = _TARBALL_FILENAME,
@@ -709,6 +715,8 @@ _ATTRS = dicts.add(_COMMON_ATTRS, {
     "link_workspace": attr.string(),
     "url": attr.string(),
     "npm_auth": attr.string(),
+    "npm_auth_username": attr.string(),
+    "npm_auth_password": attr.string(),
     "generate_bzl_library_targets": attr.bool(),
 })
 
diff --git a/npm/private/npm_translate_lock.bzl b/npm/private/npm_translate_lock.bzl
index bf61e3f..b1dc8c1 100644
--- a/npm/private/npm_translate_lock.bzl
+++ b/npm/private/npm_translate_lock.bzl
@@ -56,7 +56,7 @@ _NPM_IMPORT_TMPL = \
         version = "{version}",
         url = "{url}",
         lifecycle_hooks_no_sandbox = {lifecycle_hooks_no_sandbox},
-        npm_translate_lock_repo = "{npm_translate_lock_repo}",{maybe_generate_bzl_library_targets}{maybe_integrity}{maybe_deps}{maybe_transitive_closure}{maybe_patches}{maybe_patch_args}{maybe_run_lifecycle_hooks}{maybe_custom_postinstall}{maybe_lifecycle_hooks_env}{maybe_lifecycle_hooks_execution_requirements}{maybe_bins}{maybe_npm_auth}
+        npm_translate_lock_repo = "{npm_translate_lock_repo}",{maybe_generate_bzl_library_targets}{maybe_integrity}{maybe_deps}{maybe_transitive_closure}{maybe_patches}{maybe_patch_args}{maybe_run_lifecycle_hooks}{maybe_custom_postinstall}{maybe_lifecycle_hooks_env}{maybe_lifecycle_hooks_execution_requirements}{maybe_bins}{maybe_npm_auth}{maybe_npm_auth_username}{maybe_npm_auth_password}
     )
 """
 
@@ -185,9 +185,12 @@ def get_npm_auth(npmrc, npmrc_path, environ):
     """
 
     _NPM_TOKEN_KEY = ":_authtoken"
+    _NPM_USERNAME = ":username"
+    _NPM_PASSWORD = ":_password"
     _NPM_PKG_SCOPE_KEY = ":registry"
     tokens = {}
     registries = {}
+    basic_auth = {}
 
     for (k, v) in npmrc.items():
         if k.find(_NPM_TOKEN_KEY) != -1:
@@ -222,7 +225,29 @@ WARNING: Issue while reading "{npmrc}". Failed to replace env in config: ${{{tok
             registry = _to_registry_url(v)
             registries[scope] = registry
 
-    return (tokens, registries)
+        if k.find(_NPM_USERNAME) != -1:
+            # //somewhere-else.com/myorg/:username=someone
+            # registry: somewhere-else.com/myorg
+            # username: someone
+            registry = k.removeprefix("//").removesuffix("/{}".format(_NPM_USERNAME))
+
+            if registry not in basic_auth:
+                basic_auth[registry] = {"username": "", "password": ""}
+
+            basic_auth[registry]["username"] = v
+
+        if k.find(_NPM_PASSWORD) != -1:
+            # //somewhere-else.com/myorg/:_password=aHVudGVyMg==
+            # registry: somewhere-else.com/myorg
+            # _password: aHVudGVyMg==
+            registry = k.removeprefix("//").removesuffix("/{}".format(_NPM_PASSWORD))
+
+            if registry not in basic_auth:
+                basic_auth[registry] = {"username": "", "password": ""}
+
+            basic_auth[registry]["password"] = v
+
+    return (tokens, registries, basic_auth)
 
 def _to_registry_url(url):
     return "%s://%s" % (DEFAULT_REGISTRY_PROTOCOL, url) if url.find("//") == -1 else url
@@ -494,6 +519,7 @@ def _impl(rctx):
     lockfile_description = None
     npm_tokens = {}
     npm_registries = {}
+    basic_auth = {}
     default_registry = DEFAULT_REGISTRY
 
     bzlmod_supported = is_bazel_6_or_greater()
@@ -502,7 +528,7 @@ def _impl(rctx):
     if rctx.attr.npmrc:
         npmrc_path = rctx.path(rctx.attr.npmrc)
         npmrc = parse_ini(rctx.read(npmrc_path))
-        (npm_tokens, npm_registries) = get_npm_auth(npmrc, npmrc_path, rctx.os.environ)
+        (npm_tokens, npm_registries, basic_auth) = get_npm_auth(npmrc, npmrc_path, rctx.os.environ)
 
         if "registry" in npmrc:
             default_registry = _to_registry_url(npmrc["registry"])
@@ -828,16 +854,29 @@ load("@aspect_rules_js//npm/private:npm_package_store.bzl", _npm_package_store =
         maybe_generate_bzl_library_targets = ("""
         generate_bzl_library_targets = True,""") if rctx.attr.generate_bzl_library_targets else ""
 
-        _registry = url.split("//", 1)[-1]
+        _registry = url.split("//", 1)[-1].lower()
         npm_token = None
+        npm_auth_username = None
+        npm_auth_password = None
         match_len = 0
         for (auth_registry, auth_token) in npm_tokens.items():
             if _registry.startswith(auth_registry) and len(auth_registry) > match_len:
                 npm_token = auth_token
                 match_len = len(auth_registry)
 
+        if not npm_token:
+            for auth_registry, auth_info in basic_auth.items():
+                if _registry.startswith(auth_registry) and len(auth_registry) > match_len:
+                    npm_auth_username = auth_info["username"]
+                    npm_auth_password = auth_info["password"]
+                    match_len = len(auth_registry)
+
         maybe_npm_auth = ("""
         npm_auth = "%s",""" % npm_token) if npm_token else ""
+        maybe_npm_auth_username = ("""
+        npm_auth_username = "%s",""" % npm_auth_username) if npm_auth_username else ""
+        maybe_npm_auth_password = ("""
+        npm_auth_password = "%s",""" % npm_auth_password) if npm_auth_password else ""
 
         repositories_bzl.append(_NPM_IMPORT_TMPL.format(
             lifecycle_hooks_no_sandbox = rctx.attr.lifecycle_hooks_no_sandbox,
@@ -851,6 +890,8 @@ load("@aspect_rules_js//npm/private:npm_package_store.bzl", _npm_package_store =
             maybe_lifecycle_hooks_env = maybe_lifecycle_hooks_env,
             maybe_lifecycle_hooks_execution_requirements = maybe_lifecycle_hooks_execution_requirements,
             maybe_npm_auth = maybe_npm_auth,
+            maybe_npm_auth_username = maybe_npm_auth_username,
+            maybe_npm_auth_password = maybe_npm_auth_password,
             maybe_patch_args = maybe_patch_args,
             maybe_patches = maybe_patches,
             maybe_run_lifecycle_hooks = maybe_run_lifecycle_hooks,

@alexeagle
Copy link
Member

I guess this wasn't resolved by the PR in #383 ?

Looking through history it seems like @pedrobarco knows what to do, I'm not sure why #499 was closed.

@LavaToaster
Copy link
Contributor Author

LavaToaster commented Nov 18, 2022

No not fully.

If you look at what the npm cli itself uses, then you'll there's about 3 auth mechanisms with an additional mTLS setup. https://github.com/npm/npm-registry-fetch/blob/main/lib/auth.js#L124-L141

From what I can see, rules_js only has one of these implemented. I'm not sure if mTLS has been implemented or not.

If you're able to give me an example of how to decode base64 in Starlark I'd be happy to give this a proper go.

@LavaToaster
Copy link
Contributor Author

Hey @alexeagle @gregmagolan I'd love to help you guys with this, any chance of pointing me in the right direction of how to deal with the base64 decoding in starlark?

@alexeagle
Copy link
Member

I doubt there is any library for that in Bazel starlark. I think the npm_translate_lock should have the result of base64 decoding the value from .npmrc and then a test could verify that it's done correctly. That's our usual workaround: precompute a value outside of Bazel that's too hard to derive at runtime

@LavaToaster
Copy link
Contributor Author

LavaToaster commented Nov 23, 2022

I have a version of this kind of working with the following, but I'm not sure it's quite what you might envision as it certainly feels hacky, nor if it's the right way to call out to node since I'm unsure if this is using the node provided by the toolchain or on the system path.

            result = rctx.execute([
                "node",
                "-e",
                "process.stdout.write(Buffer.from(process.argv[1], 'base64').toString('utf8'))",
                "--",
                v,
            ])
Updated patch to see it in context
diff --git a/npm/npm_import.bzl b/npm/npm_import.bzl
index e6ebb7d..394d1b5 100644
--- a/npm/npm_import.bzl
+++ b/npm/npm_import.bzl
@@ -427,6 +427,8 @@ def npm_import(
         patches = [],
         custom_postinstall = "",
         npm_auth = "",
+        npm_auth_username = "",
+        npm_auth_password = "",
         bins = {},
         **kwargs):
     """Import a single npm package into Bazel.
@@ -658,6 +660,8 @@ def npm_import(
         patches = patches,
         custom_postinstall = custom_postinstall,
         npm_auth = npm_auth,
+        npm_auth_username = npm_auth_username,
+        npm_auth_password = npm_auth_password,
         run_lifecycle_hooks = run_lifecycle_hooks,
         extra_build_content = (
             extra_build_content if type(extra_build_content) == "string" else "\n".join(extra_build_content)
diff --git a/npm/private/npm_import.bzl b/npm/private/npm_import.bzl
index caa3f9b..3948943 100644
--- a/npm/private/npm_import.bzl
+++ b/npm/private/npm_import.bzl
@@ -382,7 +382,13 @@ def _download_and_extract_archive(rctx):
             "pattern": "Bearer <password>",
             "password": rctx.attr.npm_auth,
         },
-    } if rctx.attr.npm_auth else {}
+    } if rctx.attr.npm_auth else {
+        download_url: {
+            "type": "basic",
+            "login": rctx.attr.npm_auth_username,
+            "password": rctx.attr.npm_auth_password,
+        },
+    } if rctx.attr.npm_auth_username and rctx.attr.npm_auth_password else {}
 
     rctx.download(
         output = _TARBALL_FILENAME,
@@ -743,6 +749,8 @@ _ATTRS = dicts.add(_COMMON_ATTRS, {
     "url": attr.string(),
     "commit": attr.string(),
     "npm_auth": attr.string(),
+    "npm_auth_username": attr.string(),
+    "npm_auth_password": attr.string(),
     "generate_bzl_library_targets": attr.bool(),
 })
 
diff --git a/npm/private/npm_translate_lock.bzl b/npm/private/npm_translate_lock.bzl
index c2883ab..88e6f01 100644
--- a/npm/private/npm_translate_lock.bzl
+++ b/npm/private/npm_translate_lock.bzl
@@ -57,7 +57,7 @@ _NPM_IMPORT_TMPL = \
         version = "{version}",
         url = "{url}",
         lifecycle_hooks_no_sandbox = {lifecycle_hooks_no_sandbox},
-        npm_translate_lock_repo = "{npm_translate_lock_repo}",{maybe_commit}{maybe_generate_bzl_library_targets}{maybe_integrity}{maybe_deps}{maybe_transitive_closure}{maybe_patches}{maybe_patch_args}{maybe_run_lifecycle_hooks}{maybe_custom_postinstall}{maybe_lifecycle_hooks_env}{maybe_lifecycle_hooks_execution_requirements}{maybe_bins}{maybe_npm_auth}
+        npm_translate_lock_repo = "{npm_translate_lock_repo}",{maybe_commit}{maybe_generate_bzl_library_targets}{maybe_integrity}{maybe_deps}{maybe_transitive_closure}{maybe_patches}{maybe_patch_args}{maybe_run_lifecycle_hooks}{maybe_custom_postinstall}{maybe_lifecycle_hooks_env}{maybe_lifecycle_hooks_execution_requirements}{maybe_bins}{maybe_npm_auth}{maybe_npm_auth_username}{maybe_npm_auth_password}
     )
 """
 
@@ -145,7 +145,7 @@ def _gather_values_from_matching_names(keyed_lists, *names):
                 result.append(v)
     return result
 
-def get_npm_auth(npmrc, npmrc_path, environ):
+def get_npm_auth(npmrc, npmrc_path, environ, rctx):
     """Parses npm tokens, registries and scopes from `.npmrc`.
 
     - creates a token by registry dict: {registry: token}
@@ -187,9 +187,12 @@ def get_npm_auth(npmrc, npmrc_path, environ):
     """
 
     _NPM_TOKEN_KEY = ":_authtoken"
+    _NPM_USERNAME = ":username"
+    _NPM_PASSWORD = ":_password"
     _NPM_PKG_SCOPE_KEY = ":registry"
     tokens = {}
     registries = {}
+    basic_auth = {}
 
     for (k, v) in npmrc.items():
         if k.find(_NPM_TOKEN_KEY) != -1:
@@ -224,7 +227,45 @@ WARNING: Issue while reading "{npmrc}". Failed to replace env in config: ${{{tok
             registry = _to_registry_url(v)
             registries[scope] = registry
 
-    return (tokens, registries)
+        if k.find(_NPM_USERNAME) != -1:
+            # //somewhere-else.com/myorg/:username=someone
+            # registry: somewhere-else.com/myorg
+            # username: someone
+            registry = k.removeprefix("//").removesuffix("/{}".format(_NPM_USERNAME))
+
+            if registry not in basic_auth:
+                basic_auth[registry] = {"username": "", "password": ""}
+
+            basic_auth[registry]["username"] = v
+
+        if k.find(_NPM_PASSWORD) != -1:
+            # //somewhere-else.com/myorg/:_password=aHVudGVyMg==
+            # registry: somewhere-else.com/myorg
+            # _password: aHVudGVyMg==
+            registry = k.removeprefix("//").removesuffix("/{}".format(_NPM_PASSWORD))
+
+            if registry not in basic_auth:
+                basic_auth[registry] = {"username": "", "password": ""}
+
+            result = rctx.execute([
+                "node",
+                "-e",
+                "process.stdout.write(Buffer.from(process.argv[1], 'base64').toString('utf8'))",
+                "--",
+                v,
+            ])
+
+            if result.stderr:
+                # buildifier: disable=print
+                print("""
+WARNING: Issue while reading "{npmrc}". Failed to decode base64 encoded password.
+""".format(
+                    npmrc = npmrc_path,
+                ))
+            else:
+                basic_auth[registry]["password"] = result.stdout
+
+    return (tokens, registries, basic_auth)
 
 def _to_registry_url(url):
     return "%s://%s" % (DEFAULT_REGISTRY_PROTOCOL, url) if url.find("//") == -1 else url
@@ -519,15 +560,33 @@ def _impl(rctx):
     lockfile_description = None
     npm_tokens = {}
     npm_registries = {}
+    npm_basic_auth = {}
     default_registry = DEFAULT_REGISTRY
 
     bzlmod_supported = is_bazel_6_or_greater()
 
+    if "HOME" in rctx.os.environ and not rctx.os.name.startswith("windows"):
+        npmrc_path = "%s/.npmrc" % (rctx.os.environ["HOME"])
+        if rctx.execute(["test", "-f", npmrc_path]).return_code == 0:
+            npmrc = parse_ini(rctx.read(npmrc_path))
+
+            (npm_tokens, _, npm_basic_auth) = get_npm_auth(npmrc, npmrc_path, rctx.os.environ, rctx)
+
+    if "USERPROFILE" in rctx.os.environ and rctx.os.name.startswith("windows"):
+        npmrc_path = "%s/.npmrc" % (rctx.os.environ["USERPROFILE"])
+        if rctx.path(npmrc_path).exists:
+            npmrc = parse_ini(rctx.read(npmrc_path))
+
+            (npm_tokens, _, npm_basic_auth) = get_npm_auth(npmrc, npmrc_path, rctx.os.environ, rctx)
+
     # Read tokens from npmrc label
     if rctx.attr.npmrc:
         npmrc_path = rctx.path(rctx.attr.npmrc)
         npmrc = parse_ini(rctx.read(npmrc_path))
-        (npm_tokens, npm_registries) = get_npm_auth(npmrc, npmrc_path, rctx.os.environ)
+        (local_npm_tokens, npm_registries, local_npm_basic_auth) = get_npm_auth(npmrc, npmrc_path, rctx.os.environ, rctx)
+
+        npm_tokens = dicts.add(npm_tokens, local_npm_tokens)
+        npm_basic_auth = dicts.add(npm_basic_auth, local_npm_basic_auth)
 
         if "registry" in npmrc:
             default_registry = _to_registry_url(npmrc["registry"])
@@ -864,16 +923,29 @@ load("@aspect_rules_js//npm/private:npm_package_store.bzl", _npm_package_store =
         maybe_commit = """
         commit = "%s",""" % _import.commit if _import.commit else ""
 
-        _registry = url.split("//", 1)[-1]
+        _registry = url.split("//", 1)[-1].lower()
         npm_token = None
+        npm_auth_username = None
+        npm_auth_password = None
         match_len = 0
         for (auth_registry, auth_token) in npm_tokens.items():
             if _registry.startswith(auth_registry) and len(auth_registry) > match_len:
                 npm_token = auth_token
                 match_len = len(auth_registry)
 
+        if not npm_token:
+            for auth_registry, auth_info in npm_basic_auth.items():
+                if _registry.startswith(auth_registry) and len(auth_registry) > match_len:
+                    npm_auth_username = auth_info["username"]
+                    npm_auth_password = auth_info["password"]
+                    match_len = len(auth_registry)
+
         maybe_npm_auth = ("""
         npm_auth = "%s",""" % npm_token) if npm_token else ""
+        maybe_npm_auth_username = ("""
+        npm_auth_username = "%s",""" % npm_auth_username) if npm_auth_username else ""
+        maybe_npm_auth_password = ("""
+        npm_auth_password = "%s",""" % npm_auth_password) if npm_auth_password else ""
 
         repositories_bzl.append(_NPM_IMPORT_TMPL.format(
             lifecycle_hooks_no_sandbox = rctx.attr.lifecycle_hooks_no_sandbox,
@@ -888,6 +960,8 @@ load("@aspect_rules_js//npm/private:npm_package_store.bzl", _npm_package_store =
             maybe_lifecycle_hooks_env = maybe_lifecycle_hooks_env,
             maybe_lifecycle_hooks_execution_requirements = maybe_lifecycle_hooks_execution_requirements,
             maybe_npm_auth = maybe_npm_auth,
+            maybe_npm_auth_username = maybe_npm_auth_username,
+            maybe_npm_auth_password = maybe_npm_auth_password,
             maybe_patch_args = maybe_patch_args,
             maybe_patches = maybe_patches,
             maybe_run_lifecycle_hooks = maybe_run_lifecycle_hooks,

@gregmagolan
Copy link
Member

Hmm.. the trouble with that approach is that repository rules can't easily hook into toolchains so

rctx.execute([
                "node",

depends on there being a node installed on the local machine

@LavaToaster
Copy link
Contributor Author

I investigated using the same method you guys used for yq but that appeared to be cyclic and I couldn't seem to get it working.

Do you have any other suggestions on how to approach this? Happy to follow any other examples.

@gregmagolan
Copy link
Member

I'll put some thought into it today for what runtime to use to perform the base64. Other than that your patch looks great.

FYI. I landed a fix to parse_ini. It's called parse_npmrc now so you'll just need to tweak that when you rebase to HEAD.

@gregmagolan
Copy link
Member

gregmagolan commented Nov 25, 2022

Hmm. Maybe the answer is to just do it in native starlark.

There are lots of reference implementations in the wild for python:

https://gist.github.com/trondhumbor/ce57c0c2816bb45a8fbb
https://stackoverflow.com/questions/29050582/own-implement-encoding-and-decoding-base64-files-in-python

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants