From c264f944fbcd72adffaa7c8ecb7cde69d972605c Mon Sep 17 00:00:00 2001 From: "Juan Picado @jotadeveloper" Date: Sat, 10 Aug 2019 13:38:06 +0200 Subject: [PATCH] fix: unpublish and add or remove star colision (#1434) * fix: unpublish and add or remove star colision The issue was the npm star use a similar payload, but we did not check properly the shape of the payload, this fix and allow unpublish correctly. Improve unit testing for publishing and unpublishing Add new code documentation for future changes. * chore: update secrets baseline * chore: add missing type this will requires update types in the future --- .secrets-baseline | 27 +-- src/api/endpoint/api/publish.ts | 124 ++++++++-- src/api/endpoint/api/star.ts | 2 + src/api/middleware.ts | 12 +- src/lib/auth-utils.ts | 19 +- src/lib/auth.ts | 44 ++-- src/lib/constants.ts | 2 + src/lib/local-storage.ts | 6 + src/lib/storage-utils.ts | 15 +- src/lib/utils.ts | 10 +- test/unit/__helper/api.ts | 88 ++++++- test/unit/__helper/expects.ts | 20 ++ test/unit/__helper/utils.ts | 120 +++++++++- .../api/__snapshots__/publish.spec.ts.snap | 30 +-- test/unit/modules/api/api.spec.ts | 216 ++++++++++++------ test/unit/modules/api/publish.spec.ts | 54 ++--- test/unit/modules/auth/jwt.spec.ts | 21 +- test/unit/modules/cli/cli.spec.ts | 2 + test/unit/partials/config/yaml/api.spec.yaml | 41 +++- test/unit/partials/star-api.js | 7 - types/index.ts | 9 +- yarn.lock | Bin 363048 -> 362651 bytes 22 files changed, 631 insertions(+), 238 deletions(-) create mode 100644 test/unit/__helper/expects.ts delete mode 100644 test/unit/partials/star-api.js diff --git a/.secrets-baseline b/.secrets-baseline index ef855b871..0a2b7ff69 100644 --- a/.secrets-baseline +++ b/.secrets-baseline @@ -3,7 +3,7 @@ "files": null, "lines": null }, - "generated_at": "2019-07-27T21:44:36Z", + "generated_at": "2019-08-10T11:09:03Z", "plugins_used": [ { "name": "AWSKeyDetector" @@ -40,31 +40,26 @@ "src/lib/auth-utils.ts": [ { "hashed_secret": "6947818ac409551f11fbaa78f0ea6391960aa5b8", - "line_number": 10, + "line_number": 12, "type": "Secret Keyword" }, { "hashed_secret": "ecb252044b5ea0f679ee78ec1a12904739e2904d", - "line_number": 174, + "line_number": 187, "type": "Secret Keyword" }, { "hashed_secret": "f35dd4c51c0a89bd055b5ad30c162c778981306d", - "line_number": 179, + "line_number": 192, "type": "Secret Keyword" }, { "hashed_secret": "45c43fe97e3a06ab078b0eeff6fbe622cc417a25", - "line_number": 197, + "line_number": 210, "type": "Secret Keyword" } ], "src/lib/auth.ts": [ - { - "hashed_secret": "3812d6abc055424d0556b35f48774c7b0044eac2", - "line_number": 32, - "type": "Secret Keyword" - }, { "hashed_secret": "6981afa9890d125c05133d13053201f32292ec9f", "line_number": 38, @@ -91,12 +86,12 @@ "src/lib/constants.ts": [ { "hashed_secret": "f34fbc9a9769ba9eff5aff3d008a6b49f85c08b1", - "line_number": 14, + "line_number": 15, "type": "Secret Keyword" }, { "hashed_secret": "b9343f1143ccb83555b450eb54dde96a05522ccc", - "line_number": 115, + "line_number": 116, "type": "Secret Keyword" } ], @@ -270,12 +265,12 @@ "test/unit/modules/api/api.spec.ts": [ { "hashed_secret": "97752a468368b0d6b192140d6a140c38fd0cbd8b", - "line_number": 293, + "line_number": 304, "type": "Secret Keyword" }, { "hashed_secret": "364bdf2ed77a8544d3b711a03b69eeadcc63c9d7", - "line_number": 802, + "line_number": 828, "type": "Secret Keyword" } ], @@ -304,12 +299,12 @@ "test/unit/modules/auth/jwt.spec.ts": [ { "hashed_secret": "364bdf2ed77a8544d3b711a03b69eeadcc63c9d7", - "line_number": 121, + "line_number": 118, "type": "Secret Keyword" }, { "hashed_secret": "eaacdf2d9ed66df2601c8b51ab4084db14336d11", - "line_number": 132, + "line_number": 129, "type": "Secret Keyword" } ], diff --git a/src/api/endpoint/api/publish.ts b/src/api/endpoint/api/publish.ts index 4322d0b42..4191df1cb 100644 --- a/src/api/endpoint/api/publish.ts +++ b/src/api/endpoint/api/publish.ts @@ -3,7 +3,7 @@ import Path from 'path'; import mime from 'mime'; import { API_MESSAGE, HEADERS, DIST_TAGS, API_ERROR, HTTP_STATUS } from '../../../lib/constants'; -import { validateMetadata, isObject, ErrorCode } from '../../../lib/utils'; +import {validateMetadata, isObject, ErrorCode, hasDiffOneKey} from '../../../lib/utils'; import { media, expectJson, allow } from '../../middleware'; import { notify } from '../../../lib/notify'; import star from './star'; @@ -12,14 +12,80 @@ import { Router } from 'express'; import { Config, Callback, MergeTags, Version, Package } from '@verdaccio/types'; import { IAuth, $ResponseExtend, $RequestExtend, $NextFunctionVer, IStorageHandler } from '../../../../types'; import { logger } from '../../../lib/logger'; +import {isPublishablePackage} from "../../../lib/storage-utils"; export default function publish(router: Router, auth: IAuth, storage: IStorageHandler, config: Config) { const can = allow(auth); - // publishing a package - router.put('/:package/:_rev?/:revision?', can('publish'), media(mime.getType('json')), expectJson, publishPackage(storage, config)); + /** + * Publish a package / update package / un/start a package + * + * There are multiples scenarios here to considere: + * + * 1. Publish scenario + * + * Publish a package consist of at least 1 step (PUT) with a metadata payload. + * When a package is published, an _attachment property is present that contains the data + * of the tarball. + * + * Example flow of publish. + * + * npm http fetch PUT 201 http://localhost:4873/@scope%2ftest1 9627ms + npm info lifecycle @scope/test1@1.0.1~publish: @scope/test1@1.0.1 + npm info lifecycle @scope/test1@1.0.1~postpublish: @scope/test1@1.0.1 + + @scope/test1@1.0.1 + npm verb exit [ 0, true ] + * + * + * 2. Unpublish scenario + * + * Unpublish consist in 3 steps. + * 1. Try to fetch metadata -> if it fails, return 404 + * 2. Compute metadata locally (client side) and send a mutate payload excluding the version to be unpublished + * eg: if metadata reflects 1.0.1, 1.0.2 and 1.0.3, the computed metadata won't include 1.0.3. + * 3. Once the second step has been succesfully finished, delete the tarball. + * + * All these steps are consecutive and required, there is no transacions here, if step 3 fails, metadata might + * get corrupted. + * + * Note the unpublish call will suffix in the url a /-rev/14-5d500cfce92f90fd revision number, this not + * used internally. + * + * + * Example flow of unpublish. + * + * npm http fetch GET 200 http://localhost:4873/@scope%2ftest1?write=true 1680ms + npm http fetch PUT 201 http://localhost:4873/@scope%2ftest1/-rev/14-5d500cfce92f90fd 956606ms attempt #2 + npm http fetch GET 200 http://localhost:4873/@scope%2ftest1?write=true 1601ms + npm http fetch DELETE 201 http://localhost:4873/@scope%2ftest1/-/test1-1.0.3.tgz/-rev/16-e11c8db282b2d992 19ms + * + * 3. Star a package + * + * Permissions: start a package depends of the publish and unpublish permissions, there is no specific flag for star or un start. + * The URL for star is similar to the unpublish (change package format) + * + * npm has no enpoint for star a package, rather mutate the metadata and acts as, the difference is the + * users property which is part of the payload and the body only includes + * + * { + "_id": pkgName, + "_rev": "3-b0cdaefc9bdb77c8", + "users": { + [username]: boolean value (true, false) + } + } + * + */ + router.put('/:package/:_rev?/:revision?', can('publish'), media(mime.getType('json')), expectJson, publishPackage(storage, config, auth)); - // un-publishing an entire package + /** + * Un-publishing an entire package. + * + * This scenario happens when the first call detect there is only one version remaining + * in the metadata, then the client decides to DELETE the resource + * npm http fetch GET 304 http://localhost:4873/@scope%2ftest1?write=true 1076ms (from cache) + npm http fetch DELETE 201 http://localhost:4873/@scope%2ftest1/-rev/18-d8ebe3020bd4ac9c 22ms + */ router.delete('/:package/-rev/*', can('unpublish'), unPublishPackage(storage)); // removing a tarball @@ -35,10 +101,13 @@ export default function publish(router: Router, auth: IAuth, storage: IStorageHa /** * Publish a package */ -export function publishPackage(storage: IStorageHandler, config: Config) { +export function publishPackage(storage: IStorageHandler, config: Config, auth: IAuth) { const starApi = star(storage); return function(req: $RequestExtend, res: $ResponseExtend, next: $NextFunctionVer) { const packageName = req.params.package; + + logger.debug({packageName} , `publishing or updating a new version for @{packageName}`); + /** * Write tarball of stream data from package clients. */ @@ -72,9 +141,10 @@ export function publishPackage(storage: IStorageHandler, config: Config) { const afterChange = function(error, okMessage, metadata) { const metadataCopy: Package = { ...metadata }; + const { _attachments, versions } = metadataCopy; - // old npm behavior, if there is no attachments + // if the is no attachments, it is change, it is a new package. if (_.isNil(_attachments)) { if (error) { return next(error); @@ -89,10 +159,14 @@ export function publishPackage(storage: IStorageHandler, config: Config) { // npm-registry-client 0.3+ embeds tarball into the json upload // https://github.com/isaacs/npm-registry-client/commit/e9fbeb8b67f249394f735c74ef11fe4720d46ca0 // issue https://github.com/rlidwka/sinopia/issues/31, dealing with it here: - if (isObject(_attachments) === false || Object.keys(_attachments).length !== 1 || isObject(versions) === false || Object.keys(versions).length !== 1) { + const isInvalidBodyFormat = isObject(_attachments) === false || hasDiffOneKey(_attachments) || + isObject(versions) === false || hasDiffOneKey(versions); + + if (isInvalidBodyFormat) { // npm is doing something strange again // if this happens in normal circumstances, report it as a bug - return next(ErrorCode.getBadRequest('unsupported registry call')); + logger.info({ packageName }, `wrong package format on publish a package @{packageName}`); + return next(ErrorCode.getBadRequest(API_ERROR.UNSUPORTED_REGISTRY_CALL)); } if (error && error.status !== HTTP_STATUS.CONFLICT) { @@ -100,7 +174,7 @@ export function publishPackage(storage: IStorageHandler, config: Config) { } // at this point document is either created or existed before - const firstAttachmentKey = Object.keys(_attachments)[0]; + const [firstAttachmentKey] = Object.keys(_attachments); createTarball(Path.basename(firstAttachmentKey), _attachments[firstAttachmentKey], function(error) { if (error) { @@ -134,23 +208,34 @@ export function publishPackage(storage: IStorageHandler, config: Config) { }); }; - if (Object.prototype.hasOwnProperty.call(req.body, '_rev') && isObject(req.body.users)) { + if (isPublishablePackage(req.body) === false && isObject(req.body.users)) { return starApi(req, res, next); } try { const metadata = validateMetadata(req.body, packageName); if (req.params._rev) { - storage.changePackage(packageName, metadata, req.params.revision, function(error) { - afterChange(error, API_MESSAGE.PKG_CHANGED, metadata); + logger.debug({packageName} , `updating a new version for @{packageName}`); + // we check unpublish permissions, an update is basically remove versions + const remote = req.remote_user; + auth.allow_unpublish({packageName}, remote, (error, allowed) => { + if (error) { + logger.debug({packageName} , `not allowed to unpublish a version for @{packageName}`); + return next(error); + } + + storage.changePackage(packageName, metadata, req.params.revision, function(error) { + afterChange(error, API_MESSAGE.PKG_CHANGED, metadata); + }); }); } else { + logger.debug({packageName} , `adding a new version for @{packageName}`); storage.addPackage(packageName, metadata, function(error) { afterChange(error, API_MESSAGE.PKG_CREATED, metadata); }); } } catch (error) { - logger.error({error}, 'error on publish, bad package data @{error}'); + logger.error({packageName}, 'error on publish, bad package data for @{packageName}'); return next(ErrorCode.getBadData(API_ERROR.BAD_PACKAGE_DATA)); } }; @@ -161,7 +246,10 @@ export function publishPackage(storage: IStorageHandler, config: Config) { */ export function unPublishPackage(storage: IStorageHandler) { return function(req: $RequestExtend, res: $ResponseExtend, next: $NextFunctionVer) { - storage.removePackage(req.params.package, function(err) { + const packageName = req.params.package; + + logger.debug({packageName} , `unpublishing @{packageName}`); + storage.removePackage(packageName, function(err) { if (err) { return next(err); } @@ -176,11 +264,17 @@ export function unPublishPackage(storage: IStorageHandler) { */ export function removeTarball(storage: IStorageHandler) { return function(req: $RequestExtend, res: $ResponseExtend, next: $NextFunctionVer) { - storage.removeTarball(req.params.package, req.params.filename, req.params.revision, function(err) { + const packageName = req.params.package; + const {filename, revision} = req.params; + + logger.debug({packageName, filename, revision} , `removing a tarball for @{packageName}-@{tarballName}-@{revision}`); + storage.removeTarball(packageName, filename, revision, function(err) { if (err) { return next(err); } res.status(HTTP_STATUS.CREATED); + + logger.debug({packageName, filename, revision} , `success remove tarball for @{packageName}-@{tarballName}-@{revision}`); return next({ ok: API_MESSAGE.TARBALL_REMOVED }); }); }; diff --git a/src/api/endpoint/api/star.ts b/src/api/endpoint/api/star.ts index 2e8ac6ac9..8dfe865cc 100644 --- a/src/api/endpoint/api/star.ts +++ b/src/api/endpoint/api/star.ts @@ -4,6 +4,7 @@ import { USERS, HTTP_STATUS } from '../../../lib/constants'; import {Response} from 'express'; import {$RequestExtend, $NextFunctionVer, IStorageHandler} from '../../../../types'; import _ from 'lodash'; +import { logger } from '../../../lib/logger'; export default function(storage: IStorageHandler) { const validateInputs = (newUsers, localUsers, username, isStar) => { @@ -21,6 +22,7 @@ export default function(storage: IStorageHandler) { return (req: $RequestExtend, res: Response, next: $NextFunctionVer): void => { const name = req.params.package; + logger.debug({name}, 'starring a package for @{name}'); const afterChangePackage = function(err?: Error) { if (err) { return next(err); diff --git a/src/api/middleware.ts b/src/api/middleware.ts index ee4959bac..98067a854 100644 --- a/src/api/middleware.ts +++ b/src/api/middleware.ts @@ -1,15 +1,10 @@ -/** - * @prettier - * @flow - */ - import _ from 'lodash'; import { validateName as utilValidateName, validatePackage as utilValidatePackage, getVersionFromTarball, isObject, ErrorCode } from '../lib/utils'; import { API_ERROR, HEADER_TYPE, HEADERS, HTTP_STATUS, TOKEN_BASIC, TOKEN_BEARER } from '../lib/constants'; import { stringToMD5 } from '../lib/crypto-utils'; import { $ResponseExtend, $RequestExtend, $NextFunctionVer, IAuth } from '../../types'; -import { Config, Package } from '@verdaccio/types'; +import { Config, Package, RemoteUser } from '@verdaccio/types'; import { logger } from '../lib/logger'; import { VerdaccioError } from '@verdaccio/commons-api'; @@ -108,9 +103,10 @@ export function allow(auth: IAuth): Function { req.pause(); const packageName = req.params.scope ? `@${req.params.scope}/${req.params.package}` : req.params.package; const packageVersion = req.params.filename ? getVersionFromTarball(req.params.filename) : undefined; + const remote: RemoteUser = req.remote_user; + logger.trace({ action, user: remote.name }, `[middleware/allow][@{action}] allow for @{user}`); - // $FlowFixMe - auth['allow_' + action]({ packageName, packageVersion }, req.remote_user, function(error, allowed): void { + auth['allow_' + action]({ packageName, packageVersion }, remote, function(error, allowed): void { req.resume(); if (error) { next(error); diff --git a/src/lib/auth-utils.ts b/src/lib/auth-utils.ts index 399d9c8cd..1cfeda4dd 100644 --- a/src/lib/auth-utils.ts +++ b/src/lib/auth-utils.ts @@ -6,6 +6,8 @@ import { RemoteUser, Package, Callback, Config, Security, APITokenOptions, JWTOp import { CookieSessionToken, IAuthWebUI, AuthMiddlewarePayload, AuthTokenHeader, BasicPayload } from '../../types'; import { aesDecrypt, verifyPayload } from './crypto-utils'; +import { logger } from '../lib/logger'; + export function validatePassword(password: string, minLength: number = DEFAULT_MIN_LIMIT_PASSWORD): boolean { return typeof password === 'string' && password.length >= minLength; } @@ -40,10 +42,14 @@ export function createAnonymousRemoteUser(): RemoteUser { export function allow_action(action: string): Function { return function(user: RemoteUser, pkg: Package, callback: Callback): void { + logger.trace({remote: user.name}, `[auth/allow_action]: user: @{user.name}`); const { name, groups } = user; - const hasPermission = pkg[action].some(group => name === group || groups.includes(group)); + const groupAccess = pkg[action]; + const hasPermission = groupAccess.some(group => name === group || groups.includes(group)); + logger.trace({pkgName: pkg.name, hasPermission, remote: user.name, groupAccess}, `[auth/allow_action]: hasPermission? @{hasPermission} for user: @{user}`); if (hasPermission) { + logger.trace({remote: user.name}, `auth/allow_action: access granted to: @{user}`); return callback(null, true); } @@ -55,15 +61,22 @@ export function allow_action(action: string): Function { }; } +/** + * + */ export function handleSpecialUnpublish(): any { return function(user: RemoteUser, pkg: Package, callback: Callback): void { const action = 'unpublish'; - const hasSupport: boolean = _.isNil(pkg[action]) === false ? pkg[action] : false; + // verify whether the unpublish prop has been defined + const isUnpublishMissing: boolean = _.isNil(pkg[action]); + const hasGroups: boolean = isUnpublishMissing ? false : pkg[action].length > 0; + logger.trace({user: user.name, name: pkg.name, hasGroups}, `fallback unpublish for @{name} has groups: @{hasGroups} for @{user}`); - if (hasSupport === false) { + if (isUnpublishMissing || hasGroups === false) { return callback(null, undefined); } + logger.trace({user: user.name, name: pkg.name, action, hasGroups}, `allow_action for @{action} for @{name} has groups: @{hasGroups} for @{user}`); return allow_action(action)(user, pkg, callback); }; } diff --git a/src/lib/auth.ts b/src/lib/auth.ts index 0da251a02..c7db7bcc9 100644 --- a/src/lib/auth.ts +++ b/src/lib/auth.ts @@ -71,25 +71,30 @@ class Auth implements IAuth { } for (const plugin of validPlugins) { - this.logger.trace({ username }, 'updating password for @{username}'); - plugin.changePassword( - username, - password, - newPassword, - (err, profile): void => { - if (err) { - this.logger.error( - { username, err }, - `An error has been produced - updating the password for @{username}. Error: @{err.message}` - ); - return cb(err); - } + if (_.isNil(plugin) || _.isFunction(plugin.changePassword) === false) { + this.logger.trace('auth plugin does not implement changePassword, trying next one'); + continue; + } else { + this.logger.trace({username}, 'updating password for @{username}'); + plugin.changePassword!( + username, + password, + newPassword, + (err, profile): void => { + if (err) { + this.logger.error( + {username, err}, + `An error has been produced + updating the password for @{username}. Error: @{err.message}` + ); + return cb(err); + } - this.logger.trace({ username }, 'updated password for @{username} was successful'); - return cb(null, profile); - } - ); + this.logger.trace({username}, 'updated password for @{username} was successful'); + return cb(null, profile); + } + ); + } } } @@ -152,7 +157,7 @@ class Auth implements IAuth { // p.add_user() execution plugin[method](user, password, function(err, ok): void { if (err) { - self.logger.trace({ user, err }, 'the user @{user} could not being added. Error: @{err}'); + self.logger.trace({ user, err: err.message }, 'the user @{user} could not being added. Error: @{err}'); return cb(err); } if (ok) { @@ -203,6 +208,7 @@ class Auth implements IAuth { for (const plugin of this.plugins) { if (_.isNil(plugin) || _.isFunction(plugin.allow_unpublish) === false) { + this.logger.trace({ packageName }, 'allow unpublish for @{packageName} plugin does not implement allow_unpublish'); continue; } else { plugin.allow_unpublish!( diff --git a/src/lib/constants.ts b/src/lib/constants.ts index b0eda189b..acf576481 100644 --- a/src/lib/constants.ts +++ b/src/lib/constants.ts @@ -10,6 +10,7 @@ export const DEFAULT_DOMAIN = 'localhost'; export const TIME_EXPIRATION_24H = '24h'; export const TIME_EXPIRATION_7D = '7d'; export const DIST_TAGS = 'dist-tags'; +export const LATEST = 'latest'; export const USERS = 'users'; export const DEFAULT_MIN_LIMIT_PASSWORD = 3; export const DEFAULT_USER = 'Anonymous'; @@ -128,6 +129,7 @@ export const API_ERROR = { MAX_USERS_REACHED: 'maximum amount of users reached', VERSION_NOT_EXIST: "this version doesn't exist", FILE_NOT_FOUND: 'File not found', + UNSUPORTED_REGISTRY_CALL: 'unsupported registry call', BAD_STATUS_CODE: 'bad status code', PACKAGE_EXIST: 'this package is already present', BAD_AUTH_HEADER: 'bad authorization header', diff --git a/src/lib/local-storage.ts b/src/lib/local-storage.ts index 9e81542da..5f89c4da0 100644 --- a/src/lib/local-storage.ts +++ b/src/lib/local-storage.ts @@ -58,6 +58,7 @@ class LocalStorage implements IStorage { */ public removePackage(name: string, callback: Callback): void { const storage: any = this._getLocalStorage(name); + this.logger.debug({ name }, `[storage] removing package @{name}`); if (_.isNil(storage)) { return callback(ErrorCode.getNotFound()); @@ -77,6 +78,8 @@ class LocalStorage implements IStorage { this.localData.remove(name, (removeFailed: Error): void => { if (removeFailed) { // This will happen when database is locked + this.logger.debug({ name }, `[storage/removePackage] the database is locked, removed has failed for @{name}`); + return callback(ErrorCode.getBadData(removeFailed.message)); } @@ -309,9 +312,11 @@ class LocalStorage implements IStorage { */ public changePackage(name: string, incomingPkg: Package, revision: string | void, callback: Callback): void { if (!isObject(incomingPkg.versions) || !isObject(incomingPkg[DIST_TAGS])) { + this.logger.debug({name}, `changePackage bad data for @{name}`); return callback(ErrorCode.getBadData()); } + this.logger.debug({name}, `changePackage udapting package for @{name}`); this._updatePackage( name, (localData, cb): void => { @@ -740,6 +745,7 @@ class LocalStorage implements IStorage { } private _deleteAttachments(storage: any, attachments: string[], callback: Callback): void { + this.logger.debug({l: attachments.length }, `[storage/_deleteAttachments] delete attachments total: @{l}`); const unlinkNext = function(cb): void { if (_.isEmpty(attachments)) { return cb(); diff --git a/src/lib/storage-utils.ts b/src/lib/storage-utils.ts index 79044942c..6cb2eea74 100644 --- a/src/lib/storage-utils.ts +++ b/src/lib/storage-utils.ts @@ -1,8 +1,3 @@ -/** - * @prettier - * @flow - */ - import _ from 'lodash'; import { ErrorCode, isObject, normalizeDistTags, semverSort } from './utils'; import Search from './search'; @@ -249,3 +244,13 @@ export function prepareSearchPackage(data: Package, time: unknown): any { return pkg; } } + +/** + * Check whether the package metadta has enough data to be published + * @param pkg metadata + */ +export function isPublishablePackage(pkg: Package): boolean { + const keys: string[] = Object.keys(pkg); + + return _.includes(keys, 'versions'); +} diff --git a/src/lib/utils.ts b/src/lib/utils.ts index d6d17cbdc..eddc1502e 100644 --- a/src/lib/utils.ts +++ b/src/lib/utils.ts @@ -182,7 +182,7 @@ export function getLocalRegistryTarballUri(uri: string, pkgName: string, req: Re const protocol = getWebProtocol(req.get(HEADERS.FORWARDED_PROTO), req.protocol); const domainRegistry = combineBaseUrl(protocol, headers.host, urlPrefix); - return `${domainRegistry}/${pkgName.replace(/\//g, '%2f')}/-/${tarballName}`; + return `${domainRegistry}/${encodeScopedUri(pkgName)}/-/${tarballName}`; } /** @@ -591,3 +591,11 @@ export function pad(str, max): string { } return str; } + +export function encodeScopedUri(packageName) { + return packageName.replace(/\//g, '%2f'); +} + +export function hasDiffOneKey(versions) { + return Object.keys(versions).length !== 1; +} diff --git a/test/unit/__helper/api.ts b/test/unit/__helper/api.ts index fe7e8e2b9..337c27d69 100644 --- a/test/unit/__helper/api.ts +++ b/test/unit/__helper/api.ts @@ -1,6 +1,11 @@ +import _ from 'lodash'; +import request from 'supertest'; + import {HEADER_TYPE, HEADERS, HTTP_STATUS, TOKEN_BEARER} from '../../../src/lib/constants'; -import {buildToken} from '../../../src/lib/utils'; +import {buildToken, encodeScopedUri} from '../../../src/lib/utils'; import { Package } from '@verdaccio/types'; +import {getTaggedVersionFromPackage} from "./expects"; +import {generateRandomHexString} from "../../../src/lib/crypto-utils"; // API Helpers @@ -13,13 +18,20 @@ import { Package } from '@verdaccio/types'; export function putPackage( request: any, pkgName: string, - publishMetadata: Package -): Promise { + publishMetadata: Package, + token?: string, + httpStatus: number = HTTP_STATUS.CREATED): Promise { return new Promise((resolve) => { - request.put(pkgName) + let put = request.put(pkgName) .set(HEADER_TYPE.CONTENT_TYPE, HEADERS.JSON) - .send(JSON.stringify(publishMetadata)) - .set('accept', 'gzip') + .send(JSON.stringify(publishMetadata)); + + if (_.isEmpty(token) === false ) { + expect(token).toBeDefined(); + put.set(HEADERS.AUTHORIZATION, buildToken(TOKEN_BEARER, token as string)) + } + + put.set('accept', 'gzip') .set('accept-encoding', HEADERS.JSON) .expect(HTTP_STATUS.CREATED) .end(function(err, res) { @@ -28,15 +40,40 @@ export function putPackage( }); } +export function deletePackage( + request: any, + pkgName: string, + token?: string +): Promise { + return new Promise((resolve) => { + let del = request.put(`/${encodeScopedUri(pkgName)}/-rev/${generateRandomHexString(8)}`) + .set(HEADER_TYPE.CONTENT_TYPE, HEADERS.JSON); + + if (_.isNil(token) === false ) { + del.set(HEADERS.AUTHORIZATION, buildToken(TOKEN_BEARER, token as string)) + } + + del.set('accept-encoding', HEADERS.JSON) + .expect(HTTP_STATUS.CREATED) + .end(function(err, res) { + resolve([err, res]); + }); + }); +} + export function getPackage( request: any, - header: string, - pkg: string, + token: string, + pkgName: string, statusCode: number = HTTP_STATUS.OK): Promise { - // $FlowFixMe return new Promise((resolve) => { - request.get(`/${pkg}`) - .set(HEADERS.AUTHORIZATION, header) + let getRequest = request.get(`/${pkgName}`); + + if (_.isNil(token) === false || _.isEmpty(token) === false) { + getRequest.set(HEADERS.AUTHORIZATION, buildToken(TOKEN_BEARER, token)); + } + + getRequest .expect(HEADER_TYPE.CONTENT_TYPE, HEADERS.JSON_CHARSET) .expect(statusCode) .end(function(err, res) { @@ -116,3 +153,32 @@ export function postProfile(request: any, body: any, token: string, statusCode: }); }); } + +export async function fetchPackageByVersionAndTag(app, encodedPkgName, pkgName, version, tag = 'latest') { + // we retrieve the package to verify + const [err, resp]= await getPackage(request(app), '', encodedPkgName); + + expect(err).toBeNull(); + + // we check whether the latest version match with the previous published one + return getTaggedVersionFromPackage(resp.body, pkgName, tag, version); +} + +export async function isExistPackage(app, packageName) { + const [err]= await getPackage(request(app), '', encodeScopedUri(packageName), HTTP_STATUS.OK); + + return _.isNull(err); +} + +export async function verifyPackageVersionDoesExist(app, packageName, version, token?: string) { + const [, res]= await getPackage(request(app), token as string, encodeScopedUri(packageName), HTTP_STATUS.OK); + + const { versions } = res.body; + const versionsKeys = Object.keys(versions); + + return versionsKeys.includes(version) === false; +} + +export function generateUnPublishURI(pkgName) { + return `/${encodeScopedUri(pkgName)}/-rev/${generateRandomHexString(8)}`; +} diff --git a/test/unit/__helper/expects.ts b/test/unit/__helper/expects.ts new file mode 100644 index 000000000..dd975aaf2 --- /dev/null +++ b/test/unit/__helper/expects.ts @@ -0,0 +1,20 @@ +import {DIST_TAGS, LATEST} from "../../../src/lib/constants"; + + +/** + * Verify whether the package tag match with the desired version. + */ +export function getTaggedVersionFromPackage(pkg, pkgName, tag: string = LATEST, version: string) { + // extract the tagged version + const taggedVersion = pkg[DIST_TAGS][tag]; + expect(taggedVersion).toBeDefined(); + expect(taggedVersion).toEqual(version); + + // the version must exist + const latestPkg = pkg.versions[taggedVersion]; + expect(latestPkg).toBeDefined(); + // the name must match + expect(latestPkg.name).toEqual(pkgName); + + return latestPkg; +} diff --git a/test/unit/__helper/utils.ts b/test/unit/__helper/utils.ts index 06eecfe6b..bd3b0e9d4 100644 --- a/test/unit/__helper/utils.ts +++ b/test/unit/__helper/utils.ts @@ -1,17 +1,123 @@ import { Package } from "@verdaccio/types"; -export function generatePackageMetadata(pkgName: string): Package { +export function generateAttachment(pkgName, version) { + return { + "content_type": "application\/octet-stream", + "data": "H4sIAAAAAAAAE+2W32vbMBDH85y\/QnjQp9qxLEeBMsbGlocNBmN7bFdQ5WuqxJaEpGQdo\/\/79KPeQsnIw5KUDX\/9IOvurLuz\/DHSjK\/YAiY6jcXSKjk6sMqypHWNdtmD6hlBI0wqQmo8nVbVqMR4OsNoVB66kF1aW8eML+Vv10m9oF\/jP6IfY4QyyTrILlD2eqkcm+gVzpdrJrPz4NuAsULJ4MZFWdBkbcByI7R79CRjx0ScCdnAvf+SkjUFWu8IubzBgXUhDPidQlfZ3BhlLpBUKDiQ1cDFrYDmKkNnZwjuhUM4808+xNVW8P2bMk1Y7vJrtLC1u1MmLPjBF40+Cc4ahV6GDmI\/DWygVRpMwVX3KtXUCg7Sxp7ff3nbt6TBFy65gK1iffsN41yoEHtdFbOiisWMH8bPvXUH0SP3k+KG3UBr+DFy7OGfEJr4x5iWVeS\/pLQe+D+FIv\/agIWI6GX66kFuIhT+1gDjrp\/4d7WAvAwEJPh0u14IufWkM0zaW2W6nLfM2lybgJ4LTJ0\/jWiAK8OcMjt8MW3OlfQppcuhhQ6k+2OgkK2Q8DssFPi\/IHpU9fz3\/+xj5NjDf8QFE39VmE4JDfzPCBn4P4X6\/f88f\/Pu47zomiPk2Lv\/dOv8h+P\/34\/D\/p9CL+Kp67mrGDRo0KBBp9ZPsETQegASAAA=", + "length": 512 + } +} + +export function generateVersion(pkgName, version) { + return { + "name": pkgName, + "version": version, + "description": "some foo dependency", + "main": "index.js", + "scripts": { + "test": "echo \"Error: no test specified\" && exit 1" + }, + "keywords": [], + "author": { + "name": "User NPM", + "email": "user@domain.com" + }, + "license": "ISC", + "dependencies": { + "verdaccio": "^4.0.0" + }, + "readme": "# test", + "readmeFilename": "README.md", + "_id": `${pkgName}@${version}`, + "_npmVersion": "5.5.1", + "_npmUser": { + 'name': 'foo', + }, + "dist": { + "integrity": "sha512-6gHiERpiDgtb3hjqpQH5\/i7zRmvYi9pmCjQf2ZMy3QEa9wVk9RgdZaPWUt7ZOnWUPFjcr9cmE6dUBf+XoPoH4g==", + "shasum": "2c03764f651a9f016ca0b7620421457b619151b9", // pragma: allowlist secret + "tarball": `http:\/\/localhost:5555\/${pkgName}\/-\/${pkgName}-${version}.tgz` + } + } +} + +/** + * Generates a metadata body including attachments. + * If you intent to build a body for npm publish, please include only one version. + * if you intent to to generate a complete metadata include multiple versions. + */ +export function generatePackageBody(pkgName: string, _versions: string[] = ['1.0.0']): Package { + const latest: string = _versions[_versions.length - 1]; + const versions = _versions.reduce((cat, version) => { + cat[version] = generateVersion(pkgName, version); + return cat; + }, {}); + + const attachtment = _versions.reduce((cat, version) => { + cat[`${pkgName}-${version}.tgz`] = generateAttachment(pkgName, version); + return cat; + }, {}); + + // @ts-ignore + return { + "_id": pkgName, + "name": pkgName, + "readme": "# test", + "dist-tags": { + "latest": latest + }, + "versions": versions, + "_attachments": attachtment + } +} + +/** + * The metadata that comes from npm unpublish only contains the versions won't be removed and + * also does not includes any _attachment. + * @param pkgName + * @param _versions + */ +export function generatePackageUnpublish(pkgName: string, _versions: string[] = ['1.0.0']): Package { + const latest: string = _versions[_versions.length - 1]; + const versions = _versions.reduce((cat, version) => { + cat[version] = generateVersion(pkgName, version); + return cat; + }, {}); + + // @ts-ignore + return { + "_id": pkgName, + "name": pkgName, + "readme": "# test", + // users usually is present when run npm star [pkg] + "users": {}, + "dist-tags": { + "latest": latest + }, + "versions": versions, + } +} + +export function generateStarMedatada(pkgName: string, users): any { + return { + "_id": pkgName, + "_rev": "3-b0cdaefc9bdb77c8", + "users": users + } +} + +export function generatePackageMetadata(pkgName: string, version: string = '1.0.0'): Package { // @ts-ignore return { "_id": pkgName, "name": pkgName, "dist-tags": { - "latest": "1.0.0" + "latest": version }, "versions": { - "1.0.0": { + [version]: { "name": pkgName, - "version": "1.0.0", + "version": version, "description": "", "main": "index.js", "scripts": { @@ -30,7 +136,7 @@ export function generatePackageMetadata(pkgName: string): Package { }, "readme": "# test", "readmeFilename": "README.md", - "_id": `${pkgName}@1.0.0`, + "_id": `${pkgName}@${version}`, "_npmVersion": "5.5.1", "_npmUser": { 'name': 'foo', @@ -38,13 +144,13 @@ export function generatePackageMetadata(pkgName: string): Package { "dist": { "integrity": "sha512-6gHiERpiDgtb3hjqpQH5\/i7zRmvYi9pmCjQf2ZMy3QEa9wVk9RgdZaPWUt7ZOnWUPFjcr9cmE6dUBf+XoPoH4g==", "shasum": "2c03764f651a9f016ca0b7620421457b619151b9", // pragma: allowlist secret - "tarball": `http:\/\/localhost:5555\/${pkgName}\/-\/${pkgName}-1.0.0.tgz` + "tarball": `http:\/\/localhost:5555\/${pkgName}\/-\/${pkgName}-${version}.tgz` } } }, "readme": "# test", "_attachments": { - [`${pkgName}-1.0.0.tgz`]: { + [`${pkgName}-${version}.tgz`]: { "content_type": "application\/octet-stream", "data": "H4sIAAAAAAAAE+2W32vbMBDH85y\/QnjQp9qxLEeBMsbGlocNBmN7bFdQ5WuqxJaEpGQdo\/\/79KPeQsnIw5KUDX\/9IOvurLuz\/DHSjK\/YAiY6jcXSKjk6sMqypHWNdtmD6hlBI0wqQmo8nVbVqMR4OsNoVB66kF1aW8eML+Vv10m9oF\/jP6IfY4QyyTrILlD2eqkcm+gVzpdrJrPz4NuAsULJ4MZFWdBkbcByI7R79CRjx0ScCdnAvf+SkjUFWu8IubzBgXUhDPidQlfZ3BhlLpBUKDiQ1cDFrYDmKkNnZwjuhUM4808+xNVW8P2bMk1Y7vJrtLC1u1MmLPjBF40+Cc4ahV6GDmI\/DWygVRpMwVX3KtXUCg7Sxp7ff3nbt6TBFy65gK1iffsN41yoEHtdFbOiisWMH8bPvXUH0SP3k+KG3UBr+DFy7OGfEJr4x5iWVeS\/pLQe+D+FIv\/agIWI6GX66kFuIhT+1gDjrp\/4d7WAvAwEJPh0u14IufWkM0zaW2W6nLfM2lybgJ4LTJ0\/jWiAK8OcMjt8MW3OlfQppcuhhQ6k+2OgkK2Q8DssFPi\/IHpU9fz3\/+xj5NjDf8QFE39VmE4JDfzPCBn4P4X6\/f88f\/Pu47zomiPk2Lv\/dOv8h+P\/34\/D\/p9CL+Kp67mrGDRo0KBBp9ZPsETQegASAAA=", "length": 512 diff --git a/test/unit/modules/api/__snapshots__/publish.spec.ts.snap b/test/unit/modules/api/__snapshots__/publish.spec.ts.snap index 60d034d41..e7a752a69 100644 --- a/test/unit/modules/api/__snapshots__/publish.spec.ts.snap +++ b/test/unit/modules/api/__snapshots__/publish.spec.ts.snap @@ -1,6 +1,8 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`Publish endpoints - publish package should add a new package 1`] = ` +exports[`Publish endpoints - publish package should change the existing package 1`] = `[MockFunction]`; + +exports[`Publish endpoints - publish package should publish a new a new package 1`] = ` [MockFunction] { "calls": Array [ Array [ @@ -23,31 +25,7 @@ exports[`Publish endpoints - publish package should add a new package 1`] = ` } `; -exports[`Publish endpoints - publish package should change the existing package 1`] = ` -[MockFunction] { - "calls": Array [ - Array [ - "verdaccio", - Object { - "dist-tags": Object {}, - "name": "verdaccio", - "time": Object {}, - "versions": Object {}, - }, - undefined, - [Function], - ], - ], - "results": Array [ - Object { - "type": "return", - "value": undefined, - }, - ], -} -`; - -exports[`Publish endpoints - publish package should star a package 1`] = ` +exports[`Publish endpoints - publish package test start should star a package 1`] = ` [MockFunction] { "calls": Array [ Array [ diff --git a/test/unit/modules/api/api.spec.ts b/test/unit/modules/api/api.spec.ts index 98910d96c..fa631a555 100644 --- a/test/unit/modules/api/api.spec.ts +++ b/test/unit/modules/api/api.spec.ts @@ -5,19 +5,30 @@ import rimraf from 'rimraf'; import configDefault from '../../partials/config'; import publishMetadata from '../../partials/publish-api'; -import starMetadata from '../../partials/star-api'; import endPointAPI from '../../../../src/api'; -import {HEADERS, API_ERROR, HTTP_STATUS, HEADER_TYPE, API_MESSAGE, TOKEN_BEARER} from '../../../../src/lib/constants'; +import { + HEADERS, + API_ERROR, + HTTP_STATUS, + HEADER_TYPE, + API_MESSAGE, + TOKEN_BEARER, +} from '../../../../src/lib/constants'; import {mockServer} from '../../__helper/mock'; import {DOMAIN_SERVERS} from '../../../functional/config.functional'; -import {buildToken} from '../../../../src/lib/utils'; -import {getNewToken, putPackage} from '../../__helper/api'; -import { generatePackageMetadata } from '../../__helper/utils'; +import {buildToken, encodeScopedUri} from '../../../../src/lib/utils'; +import { + getNewToken, + putPackage, + verifyPackageVersionDoesExist, generateUnPublishURI +} from '../../__helper/api'; +import {generatePackageMetadata, generatePackageUnpublish, generateStarMedatada} from '../../__helper/utils'; require('../../../../src/lib/logger').setup([ - { type: 'stdout', format: 'pretty', level: 'info' } + { type: 'stdout', format: 'pretty', level: 'warn' } ]); + const credentials = { name: 'jota', password: 'secretPass' }; const putVersion = (app, name, publishMetadata) => { @@ -625,62 +636,122 @@ describe('endpoint unit test', () => { }); describe('should test publish/unpublish api', () => { - test('should publish a new package with no credentials', async (done) => { - // const token = await putPackage('@scope%2fpk1-test'); + /** + * It publish 2 versions and unpublish the latest one, then verifies + * the version do not exist anymore in the body of the metadata. + */ + const runPublishUnPublishFlow = async (pkgName: string, done, token?: string) => { + const version = '2.0.0'; + const pkg = generatePackageMetadata(pkgName, version); - request(app) - .put('/@scope%2fpk1-test') - .set(HEADER_TYPE.CONTENT_TYPE, HEADERS.JSON) - .send(JSON.stringify(publishMetadata)) - .expect(HTTP_STATUS.CREATED) - .end(function(err, res) { - if (err) { - expect(err).toBeNull(); - return done(err); - } - expect(res.body.ok).toBeDefined(); - expect(res.body.success).toBeDefined(); - expect(res.body.success).toBeTruthy(); - expect(res.body.ok).toMatch(API_MESSAGE.PKG_CREATED); - done(); - }); + const [err] = await putPackage(request(app), `/${encodeScopedUri(pkgName)}`, pkg, token); + if (err) { + expect(err).toBeNull(); + return done(err); + } + + const newVersion = '2.0.1'; + const [newErr] = await putPackage(request(app), `/${encodeScopedUri(pkgName)}`, + generatePackageMetadata(pkgName, newVersion), token); + if (newErr) { + expect(newErr).toBeNull(); + return done(newErr); + } + + const deletePayload = generatePackageUnpublish(pkgName, ['2.0.0']); + const [err2, res2] = await putPackage(request(app), generateUnPublishURI(pkgName), deletePayload, token); + + expect(err2).toBeNull(); + expect(res2.body.ok).toMatch(API_MESSAGE.PKG_CHANGED); + + const existVersion = await verifyPackageVersionDoesExist(app, pkgName, newVersion, token); + expect(existVersion).toBeTruthy(); + + return done(); + }; + + describe('un/publish scenarios with credentials', () => { + test('should flow with no credentials', async (done) => { + const pkgName = '@public-anyone-can-publish/pk1-test'; + runPublishUnPublishFlow(pkgName, done, undefined); + }); + + test('should flow with credentials', async (done) => { + const credentials = { name: 'jota_unpublish', password: 'secretPass' }; + const token = await getNewToken(request(app), credentials); + const pkgName = '@only-one-can-publish/pk1-test'; + + runPublishUnPublishFlow(pkgName, done, token); + }); }); - test('should unpublish a new package with credentials', async (done) => { + describe('test error handling', () => { + test('should fail if user is not allowed to unpublish', async (done) => { + /** + * Context: + * + * 'non-unpublish': + access: $authenticated + publish: jota_unpublish_fail + # There is some conditions to keep on mind here + # - If unpublish is empty, fallback with the publish value + # - If the user has permissions to publish and this empty it will be allowed to unpublish + # - If we want to forbid anyone to unpublish, just write here any unexisting user + unpublish: none - const credentials = { name: 'jota_unpublish', password: 'secretPass' }; - const token = await getNewToken(request(app), credentials); - //FUTURE: for some reason it does not remove the scope folder - request(app) - .del('/@scope%2fpk1-test/-rev/4-6abcdb4efd41a576') - .set(HEADER_TYPE.CONTENT_TYPE, HEADERS.JSON) - .set(HEADERS.AUTHORIZATION, buildToken(TOKEN_BEARER, token)) - .expect(HTTP_STATUS.CREATED) - .end(function(err, res) { - if (err) { - expect(err).toBeNull(); - return done(err); - } - expect(res.body.ok).toBeDefined(); - expect(res.body.ok).toMatch(API_MESSAGE.PKG_REMOVED); - done(); - }); - }); + The result of this test should fail and even if jota_unpublish_fail is allowed to publish. - test('should fail due non-unpublish nobody can unpublish', async (done) => { - const credentials = { name: 'jota_unpublish_fail', password: 'secretPass' }; - const token = await getNewToken(request(app), credentials); - request(app) - .del('/non-unpublish/-rev/4-6abcdb4efd41a576') - .set(HEADER_TYPE.CONTENT_TYPE, HEADERS.JSON) - .set(HEADERS.AUTHORIZATION, buildToken(TOKEN_BEARER, token)) - .expect(HTTP_STATUS.FORBIDDEN) - .end(function(err, res) { - expect(err).toBeNull(); - expect(res.body.error).toBeDefined(); - expect(res.body.error).toMatch(/user jota_unpublish_fail is not allowed to unpublish package non-unpublish/); - done(); - }); + * + */ + const credentials = { name: 'jota_unpublish_fail', password: 'secretPass' }; + const pkgName = 'non-unpublish'; + const newVersion = '1.0.0'; + const token = await getNewToken(request(app), credentials); + + const [newErr] = await putPackage(request(app), `/${encodeScopedUri(pkgName)}`, + generatePackageMetadata(pkgName, newVersion), token); + if (newErr) { + expect(newErr).toBeNull(); + return done(newErr); + } + + const deletePayload = generatePackageUnpublish(pkgName, ['2.0.0']); + const [err2, res2] = await putPackage(request(app), generateUnPublishURI(pkgName), deletePayload, token, HTTP_STATUS.FORBIDDEN); + + expect(err2).not.toBeNull(); + expect(res2.body.error).toMatch(/user jota_unpublish_fail is not allowed to unpublish package non-unpublish/); + done(); + }); + + test('should fail if publish prop is not defined', async (done) => { + /** + * Context: + * + * 'non-unpublish': + access: $authenticated + publish: jota_unpublish_fail + # There is some conditions to keep on mind here + # - If unpublish is empty, fallback with the publish value + # - If the user has permissions to publish and this empty it will be allowed to unpublish + # - If we want to forbid anyone to unpublish, just write here any unexisting user + unpublish: none + + The result of this test should fail and even if jota_unpublish_fail is allowed to publish. + + * + */ + const credentials = { name: 'jota_only_unpublish_fail', password: 'secretPass' }; + const pkgName = 'only-unpublish'; + const newVersion = '1.0.0'; + const token = await getNewToken(request(app), credentials); + + const [newErr, resp] = await putPackage(request(app), `/${encodeScopedUri(pkgName)}`, + generatePackageMetadata(pkgName, newVersion), token); + + expect(newErr).not.toBeNull(); + expect(resp.body.error).toMatch(/user jota_only_unpublish_fail is not allowed to publish package only-unpublish/); + done(); + }); }); test('should be able to publish/unpublish by only super_admin user', async (done) => { @@ -753,22 +824,23 @@ describe('endpoint unit test', () => { }); describe('should test star and stars api', () => { - const pkgName = '@scope/starPackage'; + const pkgName = '@scope/starPackage'; + const credentials = { name: 'jota_star', password: 'secretPass' }; + let token = ''; beforeAll(async (done) =>{ - await putPackage(request(app), `/${pkgName}`, generatePackageMetadata(pkgName)); - done(); + token = await getNewToken(request(app), credentials); + await putPackage(request(app), `/${pkgName}`, generatePackageMetadata(pkgName), token); + done(); }); test('should star a package', (done) => { request(app) .put(`/${pkgName}`) + .set(HEADERS.AUTHORIZATION, buildToken(TOKEN_BEARER, token)) .set(HEADER_TYPE.CONTENT_TYPE, HEADERS.JSON) - .send(JSON.stringify({ - ...starMetadata, - users: { - [credentials.name]: true - } - })) + .send(JSON.stringify(generateStarMedatada(pkgName, { + [credentials.name]: true + }))) .expect(HTTP_STATUS.OK) .end(function(err, res) { if (err) { @@ -785,7 +857,8 @@ describe('endpoint unit test', () => { request(app) .put(`/${pkgName}`) .set(HEADER_TYPE.CONTENT_TYPE, HEADERS.JSON) - .send(JSON.stringify(starMetadata)) + .set(HEADERS.AUTHORIZATION, buildToken(TOKEN_BEARER, token)) + .send(JSON.stringify(generateStarMedatada(pkgName, {}))) .expect(HTTP_STATUS.OK) .end(function(err, res) { if (err) { @@ -799,18 +872,11 @@ describe('endpoint unit test', () => { }); test('should retrieve stars list with credentials', async (done) => { - const credentials = { name: 'star_user', password: 'secretPass' }; - const token = await getNewToken(request(app), credentials); request(app) .put(`/${pkgName}`) .set(HEADERS.AUTHORIZATION, buildToken(TOKEN_BEARER, token)) .set(HEADER_TYPE.CONTENT_TYPE, HEADERS.JSON) - .send(JSON.stringify({ - ...starMetadata, - users: { - [credentials.name]: true - } - })) + .send(generateStarMedatada(pkgName, {[credentials.name]: true})) .expect(HTTP_STATUS.OK).end(function(err) { if (err) { expect(err).toBeNull(); diff --git a/test/unit/modules/api/publish.spec.ts b/test/unit/modules/api/publish.spec.ts index cb87397a8..00725e749 100644 --- a/test/unit/modules/api/publish.spec.ts +++ b/test/unit/modules/api/publish.spec.ts @@ -244,7 +244,7 @@ describe('Publish endpoints - publish package', () => { expect(storage.changePackage).toMatchSnapshot(); }); - test('should add a new package', () => { + test('should publish a new a new package', () => { const storage = { addPackage: jest.fn(), }; @@ -266,32 +266,34 @@ describe('Publish endpoints - publish package', () => { expect(next).toHaveBeenCalledWith(new Error(API_ERROR.BAD_PACKAGE_DATA)); }); - test('should star a package', () => { - const storage = { - changePackage: jest.fn(), - getPackage({ name, req, callback }) { - callback(null, { - users: {}, - }); - }, - }; - req = { - params: { - package: 'verdaccio', - }, - body: { - _rev: REVISION_MOCK, - users: { - verdaccio: true, + describe('test start', () => { + test('should star a package', () => { + const storage = { + changePackage: jest.fn(), + getPackage({ name, req, callback }) { + callback(null, { + users: {}, + }); }, - }, - remote_user: { - name: 'verdaccio', - }, - }; + }; + req = { + params: { + package: 'verdaccio', + }, + body: { + _rev: REVISION_MOCK, + users: { + verdaccio: true, + }, + }, + remote_user: { + name: 'verdaccio', + }, + }; - // @ts-ignore - publishPackage(storage)(req, res, next); - expect(storage.changePackage).toMatchSnapshot(); + // @ts-ignore + publishPackage(storage)(req, res, next); + expect(storage.changePackage).toMatchSnapshot(); + }); }); }); diff --git a/test/unit/modules/auth/jwt.spec.ts b/test/unit/modules/auth/jwt.spec.ts index 3961dc5a5..cc9722609 100644 --- a/test/unit/modules/auth/jwt.spec.ts +++ b/test/unit/modules/auth/jwt.spec.ts @@ -1,5 +1,4 @@ import request from 'supertest'; -import _ from 'lodash'; import path from 'path'; import rimraf from 'rimraf'; @@ -8,19 +7,15 @@ import endPointAPI from '../../../../src/api'; import {HEADERS, HTTP_STATUS, HEADER_TYPE, TOKEN_BEARER, TOKEN_BASIC, API_ERROR} from '../../../../src/lib/constants'; import {mockServer} from '../../__helper/mock'; import {DOMAIN_SERVERS} from '../../../functional/config.functional'; -import {buildToken, parseConfigFile} from '../../../../src/lib/utils'; -import {parseConfigurationFile} from '../../__helper'; +import {buildToken} from '../../../../src/lib/utils'; import {addUser, getPackage, loginUserToken} from '../../__helper/api'; import {setup} from '../../../../src/lib/logger'; +import configDefault from '../../partials/config'; import {buildUserBuffer} from '../../../../src/lib/auth-utils'; setup([]); const credentials = { name: 'JotaJWT', password: 'secretPass' }; -const parseConfigurationJWTFile = () => { - return parseConfigurationFile(`api-jwt/jwt`); -}; - const FORBIDDEN_VUE = 'authorization required to access package vue'; describe('endpoint user auth JWT unit test', () => { @@ -33,8 +28,7 @@ describe('endpoint user auth JWT unit test', () => { const store = path.join(__dirname, '../../partials/store/test-jwt-storage'); const mockServerPort = 55546; rimraf(store, async () => { - const confS = parseConfigFile(parseConfigurationJWTFile()); - const configForTest = _.assign({}, _.cloneDeep(confS), { + const configForTest = configDefault({ storage: store, uplinks: { npmjs: { @@ -46,8 +40,11 @@ describe('endpoint user auth JWT unit test', () => { htpasswd: { file: './test-jwt-storage/.htpasswd_jwt_auth' } - } - }); + }, + logs: [ + { type: 'stdout', format: 'pretty', level: 'warn' } + ] + }, 'api-jwt/jwt.yaml'); app = await endPointAPI(configForTest); mockRegistry = await mockServer(mockServerPort).init(); @@ -72,7 +69,7 @@ describe('endpoint user auth JWT unit test', () => { // testing JWT auth headers with token // we need it here, because token is required - const [err1, resp1] = await getPackage(request(app), buildToken(TOKEN_BEARER, token), 'vue'); + const [err1, resp1] = await getPackage(request(app), token, 'vue'); expect(err1).toBeNull(); expect(resp1.body).toBeDefined(); expect(resp1.body.name).toMatch('vue'); diff --git a/test/unit/modules/cli/cli.spec.ts b/test/unit/modules/cli/cli.spec.ts index 58278c9cb..cdb039db5 100644 --- a/test/unit/modules/cli/cli.spec.ts +++ b/test/unit/modules/cli/cli.spec.ts @@ -14,6 +14,8 @@ jest.mock('../../../../src/lib/logger', () => ({ logger: { child: jest.fn(), warn: jest.fn(), + trace: jest.fn(), + debug: jest.fn(), error: jest.fn(), fatal: jest.fn() } diff --git a/test/unit/partials/config/yaml/api.spec.yaml b/test/unit/partials/config/yaml/api.spec.yaml index c3094c1fc..be28a2fd6 100644 --- a/test/unit/partials/config/yaml/api.spec.yaml +++ b/test/unit/partials/config/yaml/api.spec.yaml @@ -3,22 +3,51 @@ uplinks: npmjs: url: http://localhost:4873/ packages: + '@public-anyone-can-publish/*': + access: $anonymous jota_unpublish + publish: $anonymous jota_unpublish + unpublish: $anonymous jota_unpublish + '@scope/starPackage': + access: $all + publish: jota_star + unpublish: jota_star + '@only-one-can-publish/*': + access: jota_unpublish + publish: jota_unpublish + unpublish: jota_unpublish + '@jquery/*': + access: $all + publish: $all + proxy: npmjs + '@scope/*': + access: test + publish: dsadsa + proxy: npmjs '@*/*': access: $all publish: $all unpublish: $authenticated proxy: npmjs - '@jquery/*': - access: $all - publish: $all - proxy: npmjs 'auth-package': access: $authenticated publish: $authenticated + 'only-you-can-publish': + access: $authenticated + publish: you + unpublish: you 'non-unpublish': access: $authenticated - publish: $authenticated - # this is intended, empty block + publish: jota_unpublish_fail + # There is some conditions to keep on mind here + # - If unpublish is empty, fallback with the publish value + # - If the user has permissions to publish and this empty it will be allowed to unpublish + # - If we want to forbid anyone to unpublish, just write here any unexisting user + unpublish: some_unexisting_user_defined_here_might_be_a_hash + 'only-unpublish': + access: $authenticated + # comment out is intended, we want to test if publish prop is not defined + # publish: jota_unpublish_fail + # unpublish: 'super-admin-can-unpublish': access: $authenticated diff --git a/test/unit/partials/star-api.js b/test/unit/partials/star-api.js deleted file mode 100644 index ee206b9ef..000000000 --- a/test/unit/partials/star-api.js +++ /dev/null @@ -1,7 +0,0 @@ -const json = { - "_id": "@scope\/pk1-test", - "_rev": "4-6abcdb4efd41a576", - "users": {} -} - - module.exports = json; \ No newline at end of file diff --git a/types/index.ts b/types/index.ts index 608570281..28c5087fe 100644 --- a/types/index.ts +++ b/types/index.ts @@ -15,7 +15,13 @@ import { JWTSignOptions, PackageAccess, ILocalData, - StringValue as verdaccio$StringValue, IReadTarball, Package, IPluginStorageFilter, Author} from '@verdaccio/types'; + StringValue as verdaccio$StringValue, + IReadTarball, + Package, + IPluginStorageFilter, + Author, + AuthPluginPackage +} from '@verdaccio/types'; import lunrMutable from 'lunr-mutable-indexes'; import {NextFunction, Request, Response} from 'express'; @@ -111,6 +117,7 @@ export interface IAuth extends IBasicAuth, IAuthMiddleware, IAuthWebUI { secret: string; // eslint-disable-next-line @typescript-eslint/no-explicit-any plugins: any[]; + allow_unpublish(pkg: AuthPluginPackage, user: RemoteUser, callback: Callback): void; } export interface IWebSearch { diff --git a/yarn.lock b/yarn.lock index a7a99c5cecbd74628c1807f1018bb16594f817da..26ebf16bf078ed0d38891c3c750dbebbfea740e1 100644 GIT binary patch delta 47 zcmZ26MQrv&v4$4LElhV#ZGUx&iBp70M`3!w119I~D)*TJ8QVD?F#$32c8*6Z3M&B5 CY!X}m delta 58 zcmbO|QEbH&v4$4LElhV#F>C24Y`=JlNknA(&wETwjMLxTW3rpR;2abG^oTo5((M8F OnShvid%%4b@09@C{}}85