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

add npm_client_version config option #188

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mstorus
Copy link

@mstorus mstorus commented Jan 9, 2015

  • to restrict which npm clients can login

I'd like the ability to disallow npm login / npm adduser for certain versions of the npm client.
This information can be read from the version request header.

Use case: I only want to allow login for users who have npm >= 1.5, since npm < 1.5 stores the user's base64-encoded password in plaintext under their local ~/.npmrc file. This is particularly important if you are using the sinopia-ldap plugin, since this password is the user's LDAP password.

see: #187

- to restrict which npm clients can login
@mstorus mstorus force-pushed the npm_client_version branch from ea16133 to 886809d Compare January 10, 2015 00:29
@mstorus
Copy link
Author

mstorus commented Feb 3, 2015

any thoughts on this PR?
does it seem ok, or would it make more sense to expose a middleware API that provides access to the full request headers?

@dzoba
Copy link

dzoba commented Apr 2, 2015

👍 We could really use this too - storing plaintext ldap passwords is a deal breaker.

@lencioni
Copy link

LGTM! @rlidwka is there anything else that you'd like to see before we get this merged in and released?

@dgaya
Copy link

dgaya commented Mar 17, 2016

What if you make it simpler:

if (config.npm_client_version && !Semver.satisfies(req.header('version'), config.npm_client_version)) {
    return next( Error[422]("...") )
}

then there's no need to modify lib/config.js and give any meaning to "*".

If you PR to https://github.com/fl4re/sinopia I will merge ASAP.

Regards,
David

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

Successfully merging this pull request may close these issues.

4 participants