-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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 npm owner #4582
base: master
Are you sure you want to change the base?
Conversation
Next would be addressing Note 1 and Note 2 from above. |
I didn't have much focus time to finish the review but I have no objections so far, great job I'll back to this next week (I'm afk lately) |
Todos:
|
This is it 😃 If you set PS: I will update docs in separate PR. |
Still half way to finish the deep testing but yeah that was one of my questions I had so far :) sorry I need to focus on the cipher issue but I will be back on this. |
fixing getTarballDetails, then back to this |
Rebased. Secrets are related to tests. Just ignore? Will these findings show on all PRs? |
solved :) the GitGuardian allows set some files as tests, so I did. |
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.
Great stuff, I tested thoughtfully and I have only two small request
- Update config files
- Pending question regarding possible undefined.
- Optional (https://github.com/verdaccio/verdaccio/blob/master/website/docs/config.md#advanced-settings-advanced-settings) only for next version (no 5.x or 6.x yet) I still need to figure out how to replace storage dependency.
@@ -196,6 +196,7 @@ export interface Security { | |||
|
|||
export interface PublishOptions { | |||
allow_offline: boolean; | |||
check_owners: boolean; |
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.
don't forget also update the official config yaml file like
@@ -1390,6 +1467,10 @@ class Storage { | |||
}, | |||
}; | |||
|
|||
// Set initial package owner | |||
// TODO: Add email of user | |||
packageData.maintainers = [{ name: username || '', email: '' }]; |
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.
What if username
is undefined?
Thanks for the feedback! I have busy week speaking at a conference. I will get back to it afterwards. |
Here we go...
Overview (based on
npm owner
)This PR will allow executing the
npm owner
command on a verdaccio registry.The current owners of a package are stored using the
maintainers
property ofpackage.json
. Each version will also contain the historical list of owners at the time of publishing in themaintainers
property of the version.Note 1
Verdaccio does not store user emails. Therefore, maintainers are added using just the username (and an empty email field). There's also no email invitation flow for added owners as with the npm registry. Adding emails to user profiles might be a future enhancement (
npm profile
).Note 2
By default, this PR will not impact on permissions for accessing packages. A new config option can be used to activate, checking package ownership when trying publish a new or unpublish an old version of a package:
If
check_owners
istrue
, then changing a package (publish/unpublish), will require that your user is listed as an owner of the package i.e. included in themaintainers
array of the packument.Technical details (based on
npm cli
)npm owner list
get /:package
endpoint to retrieve the manifest and maintainersnpm owner add/rm
get /-/user/:org_couchdb_user
endpoint to get the users name and email (see below)put /:package
endpoint (via request body)npm star
get /-/user/:org_couchdb_user
{ name: <username>, email: '' }
to the response if user is logged in (required bynpm owner
){ ok: false }
if user is not logged in (to mirror npm registry)put /-/user/:org_couchdb_user/:_rev?/:revision?
put /-/user/org.couchdb.user:tom
will raise an error whenname: 'jerry'
is contained in the bodyput /:package
npm publish
will setmaintainers: [ { name: <logged_in_user>, email: '' }]
for new packagesnpm owner
will updatedmaintainers
as provided bynpm cli
adding or removing ownersStatus
npm owner list
npm owner add/rm
npm publish
, update packagenpm publish
, update version metadataExample