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

feat: add support for profile cli command #392 (change password) #1034

Merged
merged 23 commits into from
Oct 12, 2018

Conversation

juanpicado
Copy link
Member

@juanpicado juanpicado commented Sep 30, 2018

Type: feat

The following has been addressed in the PR:

Description:

  • it allows updating password npm profile set password
  • display current profile npm profile get

https://docs.npmjs.com/cli/profile

⚠️ Depends on

Resolves #392
Refers to #913

ℹ️ Currently only some plugins (verdaccio-htpasswd) implement the changePassword feature, thus, if you are using an authentification plugin that does not support this feature, I'd suggest open a request in their repositories.

- it allows to update password npm profile set password
- display current profile npm profile get

https://docs.npmjs.com/cli/profile
@juanpicado juanpicado added this to the 4.0.0 milestone Sep 30, 2018
@juanpicado juanpicado added the WIP label Sep 30, 2018
on npm by defaul is min 7 characters, this might be configurable in the future.
@juanpicado juanpicado added this to In Progress in Verdaccio 4 Sep 30, 2018
@juanpicado juanpicado changed the title feat: add support for profile cli command #392 feat: add support for profile cli command #392 (change password) Oct 1, 2018
Copy link
Member

@sergiohgz sergiohgz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM but only one minor change 😅

if (validatePassword(password) === false) {
/* eslint new-cap:off */
return next(ErrorCode.getCode(HTTP_STATUS.BAD_REQUEST, API_ERROR.PASSWORD_SHORT()));
/* eslint new-cap:off */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you tried to re-enable new-cap rule, but cp was wrong 😂

@juanpicado
Copy link
Member Author

This still needs more test, functional and increase coverage. But the basic functionality works.

@juanpicado
Copy link
Member Author

It's ready for CR, I'll defer the functional test for a future PR, I need to upgrade first verdaccio-auth-memory

@juanpicado juanpicado mentioned this pull request Oct 6, 2018
@juanpicado juanpicado requested a review from a team October 11, 2018 16:18
Copy link
Member

@ayusharma ayusharma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me 👍 Tested locally as well. 😃

@juanpicado juanpicado merged commit f1416ed into 4.x Oct 12, 2018
Verdaccio 4 automation moved this from In Progress to Done Oct 12, 2018
@delete-merged-branch delete-merged-branch bot deleted the fix-392 branch October 12, 2018 09:07
@lock
Copy link

lock bot commented Jan 11, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
No open projects
Verdaccio 4
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants