-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
fix(plugin-npm-cli): fix login with Verdaccio #5983
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I find it a bit odd that the login
command performs user registration, that doesn't seem right.
Ref #1848 (comment)
Just to clarify, I 100% agree that it's weird, but it's how the npm registry has always worked. I updated my PR to fix typechecking. The PR works and follows the logic of the officiel client, but I'm forcing a basic auth authorization header manually (It corresponds to this part of the official client). I could try pushing the auth header logic to |
fe51c67
to
f77f75e
Compare
The diff sounds reasonable to me. I suspect we don't actually support the register flow since we don't send the email, so this would address the "can we avoid doing both" part 🤔 |
This commit fixes `yarn npm login` when the remote registry is Verdaccio. When a user already exists, the registry replies with `409 Conflict`. The official npm client then retrieves the latest user state and inserts a revision, using HTTP basic authentication. This step was missing, and this commits adds it. The change was tested to work with a private Verdaccio registry. It should now be as reliable as the official npm client. - Closes yarnpkg#1044 - Closes yarnpkg#1848 - Closes verdaccio/verdaccio#1737
The registry does odd things I suppose.
Hello! The logic for the old path did not change at all (token acquired during the first request) so there should be no regression. |
Thanks for reminding me! I'll look to make a patch release later this week. |
Thank you very much 🙂 I know that Yarn does regular releases, so don't feel pressured to release just for this fix. Having it on master and knowing that it will be in the next patch is good enough already for me. |
**What's the problem this PR addresses?** This commit fixes `yarn npm login` when the remote registry is Verdaccio. - Closes #1044 - Closes #1848 - Closes verdaccio/verdaccio#1737 ... **How did you fix it?** When a user already exists, the registry replies with `409 Conflict`. The official npm client then retrieves the latest user state and inserts a revision, using HTTP basic authentication. This step was missing, and this commits adds it. The change was tested to work with a private Verdaccio registry. It should now be as reliable as the official npm client. ... **Checklist** <!--- Don't worry if you miss something, chores are automatically tested. --> <!--- This checklist exists to help you remember doing the chores when you submit a PR. --> <!--- Put an `x` in all the boxes that apply. --> - [x] I have read the [Contributing Guide](https://yarnpkg.com/advanced/contributing). <!-- See https://yarnpkg.com/advanced/contributing#preparing-your-pr-to-be-released for more details. --> <!-- Check with `yarn version check` and fix with `yarn version check -i` --> - [x] I have set the packages that need to be released for my changes to be effective. <!-- The "Testing chores" workflow validates that your PR follows our guidelines. --> <!-- If it doesn't pass, click on it to see details as to what your PR might be missing. --> - [x] I will check that all automated PR checks pass before the PR gets reviewed. (cherry picked from commit db6210f)
What's the problem this PR addresses?
This commit fixes
yarn npm login
when the remote registry is Verdaccio....
How did you fix it?
When a user already exists, the registry replies with
409 Conflict
. The official npm client then retrieves the latest user state and inserts a revision, using HTTP basic authentication. This step was missing, and this commits adds it.The change was tested to work with a private Verdaccio registry. It should now be as reliable as the official npm client.
...
Checklist