generator icon indicating copy to clipboard operation
generator copied to clipboard

feat: :sparkles: added support for using current npm registry

Open rluvaton opened this issue 4 years ago • 15 comments

Description Use the current NPM registry

Related issue(s) Fixes #538

rluvaton avatar Oct 06 '21 19:10 rluvaton

I would be happy if you could add the hacktoberfest-accepted label to this PR 😄

I would update the tests shortly

rluvaton avatar Oct 06 '21 19:10 rluvaton

I would be happy if you could add the hacktoberfest-accepted label to this PR

is it needed if after all the whole repo has hacktoberfest topi?

There are 2 challenges I have with proposed solution:

  • libnpmconfig is an archived project and one of its dependencies probably won't work with npm7, and even if it will, maintainers are pretty clear about the library status. This is not a critical package, but if its dependencies already have mixed status than long term, any security updates will cause lots of issues 😞
  • we now use Arborist, work with npm programmatically without using npm installation, and this solution basically expects you have npm installed as your npm config is read. Why do you think it is better than enabling authorization on library API level?

derberg avatar Oct 07 '21 07:10 derberg

I would be happy if you could add the hacktoberfest-accepted label to this PR

is it needed if after all the whole repo has hacktoberfest topi?

I haven't saw that, my bad, it's not needed than.

There are 2 challenges I have with proposed solution:

  • libnpmconfig is an archived project and one of its dependencies probably won't work with npm7, and even if it will, maintainers are pretty clear about the library status. This is not a critical package, but if its dependencies already have mixed status than long term, any security updates will cause lots of issues 😞

Yeah I saw and I thought about it too but the library is not deprecated so instead of running child process with npm config get registry (which I'm trying to avoid) I thought it's better to do this instead, but I'm on the fence there.

There is also npm/config package that NPM use it in their CLI code but it seems like it only getting you the config at specific place

  • we now use Arborist, work with npm programmatically without using npm installation, and this solution basically expects you have npm installed as your npm config is read. Why do you think it is better than enabling authorization on library API level?

I don't really understand what you mean. Anyway, Artborist doesn't use the currently used npm registry.

rluvaton avatar Oct 07 '21 07:10 rluvaton

@derberg just pinging you in case you haven't saw my comment

rluvaton avatar Oct 12 '21 18:10 rluvaton

@rluvaton ah, you are right, I missed you comment, thanks for pinging me 🙇🏼

My point is, based of course on my understanding of Arborist, is to use Arborist for authentication with npm registries. So you can pass to generator library and cli an environment variable, like NPM_TOKEN and once it is present, we use it in the Arborist API. Look at their docs 👇🏼 we can enable different environment variables for different auth methods and use them in the generator + enable users to pass the custom registry URL through API/CLI

const arb = new Arborist({
  // options object

  // where we're doing stuff.  defaults to cwd.
  path: '/path/to/package/root',

  // url to the default registry.  defaults to npm's default registry
  registry: 'https://registry.npmjs.org',

  // scopes can be mapped to a different registry
  '@foo:registry': 'https://registry.foo.com/',

  // Auth can be provided in a couple of different ways.  If none are
  // provided, then requests are anonymous, and private packages will 404.
  // Arborist doesn't do anything with these, it just passes them down
  // the chain to pacote and npm-registry-fetch.

  // Safest: a bearer token provided by a registry:
  // 1. an npm auth token, used with the default registry
  token: 'deadbeefcafebad',
  // 2. an alias for the same thing:
  _authToken: 'deadbeefcafebad',

  // insecure options:
  // 3. basic auth, username:password, base64 encoded
  auth: 'aXNhYWNzOm5vdCBteSByZWFsIHBhc3N3b3Jk',
  // 4. username and base64 encoded password
  username: 'isaacs',
  password: 'bm90IG15IHJlYWwgcGFzc3dvcmQ=',

  // auth configs can also be scoped to a given registry with this
  // rather unusual pattern:
  '//registry.foo.com:token': 'blahblahblah',
  '//basic.auth.only.foo.com:_auth': 'aXNhYWNzOm5vdCBteSByZWFsIHBhc3N3b3Jk',
  '//registry.foo.com:always-auth': true,
})

does it make sense? it gives you more flexibility imho

derberg avatar Oct 13 '21 07:10 derberg

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

sonarqubecloud[bot] avatar Oct 13 '21 10:10 sonarqubecloud[bot]

IMO I think we should use the currently used registry and maybe let the user pass custom registry, this way we don't need to handle authentication and stuff, if the user wants to use custom registry only for this he can create an .npmrc file.

WDYT?

rluvaton avatar Oct 13 '21 10:10 rluvaton

Also, doing this we need to support every change in the NPM capabilities, I suggest that we don't use the Arborist package at all as it doesn't use by default the local machine configuration and if we only use it to download packages we better use better solution


EDIT: I see that npm CLI uses Arborist so I take that back https://github.com/npm/cli/blob/latest/lib/install.js#L169-L170

But I still think we should use the current configuration rather than implement it by ourselfs

rluvaton avatar Oct 13 '21 11:10 rluvaton

@rluvaton the reason we switched to Arborist as it is the most latest and modern approach to manage NPM modules installation. We had too many issues with reusing NPM clients. Thus my request is for us to use Arborist for private/custom registries. Using the config file could work as a fallback

derberg avatar Oct 18 '21 14:10 derberg

😀 so what we need to do is add those flags: --registry, --username, --password, --auth-token, right?

rluvaton avatar Oct 18 '21 14:10 rluvaton

@rluvaton yes, and have it in Generator library of course.

Critical to remember is security. By default, we must support passing secret data using environment variables. Flag for password and token should be there to just overwrite env variables.

derberg avatar Oct 18 '21 15:10 derberg

Hi, I was wondering if this issue is handled, for I have met the same question.

zmt-Eason avatar Oct 26 '21 07:10 zmt-Eason

@rluvaton what do you need to continue here?

jonaslagoni avatar Jan 31 '22 19:01 jonaslagoni

I'm sorry, crazy times, if someone wants she/he can continue this PR...

rluvaton avatar Jan 31 '22 19:01 rluvaton

This pull request has been automatically marked as stale because it has not had recent activity :sleeping:

It will be closed in 120 days if no further activity occurs. To unstale this pull request, add a comment with detailed explanation.

There can be many reasons why some specific pull request has no activity. The most probable cause is lack of time, not lack of interest. AsyncAPI Initiative is a Linux Foundation project not owned by a single for-profit company. It is a community-driven initiative ruled under open governance model.

Let us figure out together how to push this pull request forward. Connect with us through one of many communication channels we established here.

Thank you for your patience :heart:

github-actions[bot] avatar Jun 01 '22 00:06 github-actions[bot]

This pull request has been automatically marked as stale because it has not had recent activity :sleeping:

It will be closed in 120 days if no further activity occurs. To unstale this pull request, add a comment with detailed explanation.

There can be many reasons why some specific pull request has no activity. The most probable cause is lack of time, not lack of interest. AsyncAPI Initiative is a Linux Foundation project not owned by a single for-profit company. It is a community-driven initiative ruled under open governance model.

Let us figure out together how to push this pull request forward. Connect with us through one of many communication channels we established here.

Thank you for your patience :heart:

github-actions[bot] avatar Nov 26 '22 00:11 github-actions[bot]