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

[BUG] npm whoami allow empty username #5867

Closed
2 tasks done
sosoba opened this issue Nov 18, 2022 · 3 comments · Fixed by #5884
Closed
2 tasks done

[BUG] npm whoami allow empty username #5867

sosoba opened this issue Nov 18, 2022 · 3 comments · Fixed by #5884
Labels
Bug thing that needs fixing Needs Triage needs review for next steps Release 8.x work is associated with a specific npm 8 release

Comments

@sosoba
Copy link
Contributor

sosoba commented Nov 18, 2022

Is there an existing issue for this?

  • I have searched the existing issues

This issue exists in the latest npm version

  • I am using the latest npm

Current Behavior

npm whoami

undefined

// No username, but we have other credentials; fetch the username from registry
if (creds.token || creds.certfile && creds.keyfile) {
const registryData = await npmFetch.json('/-/whoami', { ...opts })
return registryData.username
}
// At this point, even if they have a credentials object, it doesn't have a
// valid token.
throw Object.assign(
new Error('This command requires you to be logged in.'),

Expected Behavior

npm whoami

npm ERR! This command requires you to be logged in.

It seems to me, that there should be condition on return at line 15

if (registryData?.username) {
  return registryData.username;
}

Steps To Reproduce

  1. Config registry=https://verdaccio.example.com
  2. Run 'npm whoami'
  3. See result

Environment

  • npm: 8.19.2
  • Node.js: 18.12.1
  • OS Name: Windows 11
  • System Model Name: Other
  • npm config:
registry=https://verdaccio.example.com
@sosoba sosoba added Bug thing that needs fixing Needs Triage needs review for next steps Release 8.x work is associated with a specific npm 8 release labels Nov 18, 2022
@wraithgar
Copy link
Member

Is verdaccio returning a 2xx response when you are not logged in? npm assumes that whoami will return an http error if you are not logged in. This seems like a verdaccio bug if it is not properly telling clients the situation.

@sosoba
Copy link
Contributor Author

sosoba commented Nov 22, 2022

Is verdaccio returning a 2xx response when you are not logged in? npm assumes that whoami will return an http error if you are not logged in. This seems like a verdaccio bug if it is not properly telling clients the situation.

Yes. You are right, This third-party registy has different behaviour that registry.npmjs.org. But in general the client should not assume that the server always returns valid values. It should validate them.

@juanpicado
Copy link
Contributor

I think is a mix both, I will also include some improvements in verdaccio. Thanks @sosoba .

@fritzy fritzy closed this as completed Nov 23, 2022
fritzy pushed a commit that referenced this issue Nov 30, 2022
Fix for #5867 (prevent undefined username)

Co-authored-by: nlf <nlf@github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug thing that needs fixing Needs Triage needs review for next steps Release 8.x work is associated with a specific npm 8 release
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants