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

Non-anonymous users rejected from downloading packages unless they are in config.yaml #17

Closed
iambrandonn opened this issue Dec 3, 2013 · 8 comments

Comments

@iambrandonn
Copy link
Contributor

I've found that anonymous users ("npm adduser" has not yet been run) are allowed to download packages from Sinopia... which is what I would expect. However, I've also found that if a user has run "npm adduser", possibly because of using registry.npmjs.org instead of Sinopia, AND that user is not set up in the config file... the user is rejected from even downloading packages. I would expect rejection for publishing, but not downloading.
I see two possible solutions. If the basic_auth middleware were placed lower in index.js after all the routes that accept anonymous users but before any route that required auth... that might solve the problem. The other idea is to not fail a request immediately on bad auth, but let the route check when and if the user is needed. At that point the request could fail if anonymous users are not ok.
I am willing to work on either solution and submit a PR if you like, but want to get your opinion before putting any work into it.

@rlidwka
Copy link
Owner

rlidwka commented Dec 3, 2013

Not a bug. It matches the behaviour of couchdb registry:

$ curl -H 'Authorization: Basic dGVzdDoxMjM0NTYK' http://registry.npmjs.org/sinopia
{"error":"unauthorized","reason":"Name or password is incorrect."}

However, it can be changed if it causes trouble. Does it? What should it do if user is in config, but password is wrong?

@iambrandonn
Copy link
Contributor Author

The problem is that authorization between npmjs.org and Sinopia isn't shared (nor should it be) which means that a user with a valid account on npmjs.org may or may not be a valid account on a Sinopia instance. I have two concerns. One is that it can be a maintenance headache to add every possible user to the config.yaml file. In my particular scenario I may have 5-10 users who are allowed to publish (and thus are in config.yaml), but I may have several hundred users who are only using Sinopia as consumers. My other concern is the inconsistency of allowing anonymous users to download packages but denying users who have authorized. It feels like at worst they should be treated equally with anonymous users.

To more directly answer your question about what I think it should do...
A user with a valid username and matching password should be able to publish, download, and do whatever the config file has granted them. Someone with a valid username but invalid password should be treated as an anonymous user. Anonymous users can only download packages.

@rlidwka
Copy link
Owner

rlidwka commented Dec 4, 2013

My only concern here is security. Passwords are transferred in cleartext, and if somebody is requesting something from sinopia with valid npmjs credentials, that's a big security hole to say the least, they should be warned against that somehow.

@iambrandonn
Copy link
Contributor Author

True. So what is the recommended practice for using multiple registries with the same npm client?

@rlidwka
Copy link
Owner

rlidwka commented Dec 4, 2013

True. So what is the recommended practice for using multiple registries with the same npm client?

No idea, maybe using this script is:
https://github.com/deoxxa/npmrc

Anyway, I think I'll change this behaviour, because it makes it harder to use it and doesn't really prevent anything.

@iambrandonn
Copy link
Contributor Author

Maybe it could be configurable with a default to not allow downloads from users who aren't in the config file. If turned on it it would allow anyone to download.

@rlidwka rlidwka closed this as completed in a257fc3 Dec 6, 2013
@rlidwka
Copy link
Owner

rlidwka commented Dec 6, 2013

It should work now, and there is a description in comments how to bring old behaviour back.

Configuration options can be bad if there are too many of them. I'd prefer no configuration at all, sadly it isn't an option.

I'll try to fix it in npm somehow, and after that I might think about reverting it when npm stops sending plaintext passwords everywhere. But it could be years.

@rlidwka
Copy link
Owner

rlidwka commented Feb 18, 2014

opened an issue in npm repo, npm/npm#4711

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

No branches or pull requests

2 participants