From 3f6eeb4d7daafdac30b0ef0bf7c098ead797a792 Mon Sep 17 00:00:00 2001 From: "Juan Picado @jotadeveloper" Date: Sun, 29 Apr 2018 21:50:10 +0200 Subject: [PATCH] fix: update uplinks auth header * increases unit test coverage * remove dead code * update documentation * light refactoring fix #670 --- docs/uplinks.md | 5 +- flow-typed/npm/supertest_vx.x.x.js | 52 +++++++++++++++++ src/api/endpoint/index.js | 1 - src/lib/auth.js | 91 ++++++++---------------------- test/unit/api.spec.js | 64 ++++++++++++++++++++- test/unit/partials/config/index.js | 9 +++ 6 files changed, 148 insertions(+), 74 deletions(-) create mode 100644 flow-typed/npm/supertest_vx.x.x.js diff --git a/docs/uplinks.md b/docs/uplinks.md index 11e8a21f2..9e70d291c 100644 --- a/docs/uplinks.md +++ b/docs/uplinks.md @@ -34,14 +34,15 @@ maxage | string | No |10m | all | limit maximun failure request | 2m fail_timeout | string | No |10m | all | defines max time when a request becomes a failure | 5m max_fails | number | No |2 | all | limit maximun failure request | 2 cache | boolean | No |[true,false] | >= 2.1 | avoid cache tarballs | true -auth | list | No | type: [bearer,basic], [token: "token",token_env: [true,\]] | >= 2.5 | assigns the header 'Authorization' see: http://blog.npmjs.org/post/118393368555/deploying-with-npm-private-modules | disabled -headers | list | No | authorization: "Basic YourBase64EncodedCredentials==" | all | list of custom headers for the uplink | disabled +auth | list | No | type: [bearer], [token: "token",token_env: [true,\]] | >= 2.5 | assigns the header 'Authorization' see: http://blog.npmjs.org/post/118393368555/deploying-with-npm-private-modules | disabled +headers | list | No | authorization: "Bearer SecretJWToken==" | all | list of custom headers for the uplink | disabled strict_ssl |boolean | No | [true,false] | >= 3.0 | If true, requires SSL certificates be valid. | true > The `auth` property allows you to use a auth token via an environment variable, [clik here for an example](https://github.com/verdaccio/verdaccio/releases/tag/v2.5.0). ### You Must know +* Verdaccio does not use Basic Authentication since version `v2.3.0`. All tokens generated by verdaccio are based on JWT ([JSON Web Token](https://jwt.io/)) * Uplinks must be registries compatible with the `npm` endpoints. Eg: *verdaccio*, `sinopia@1.4.0`, *npmjs registry*, *yarn registry*, *JFrog*, *Nexus* and more. * Setting `cache` to false will help to save space in your hard drive. This will avoid store `tarballs` but [it will keep metadata in folders](https://github.com/verdaccio/verdaccio/issues/391). * Exceed with multiple uplinks might slow down the lookup of your packages due for each request a npm client does, verdaccio does 1 call for each uplink. diff --git a/flow-typed/npm/supertest_vx.x.x.js b/flow-typed/npm/supertest_vx.x.x.js new file mode 100644 index 000000000..65ae129cb --- /dev/null +++ b/flow-typed/npm/supertest_vx.x.x.js @@ -0,0 +1,52 @@ +// flow-typed signature: 92c970084ff90673c82c72604aa26f27 +// flow-typed version: <>/supertest_v3.x.x/flow_v0.69.0 + +/** + * This is an autogenerated libdef stub for: + * + * 'supertest' + * + * Fill this stub out by replacing all the `any` types. + * + * Once filled out, we encourage you to share your work with the + * community by sending a pull request to: + * https://github.com/flowtype/flow-typed + */ + +declare module 'supertest' { + declare module.exports: any; +} + +/** + * We include stubs for each file inside this npm package in case you need to + * require those files directly. Feel free to delete any files that aren't + * needed. + */ +declare module 'supertest/lib/agent' { + declare module.exports: any; +} + +declare module 'supertest/lib/test' { + declare module.exports: any; +} + +declare module 'supertest/test/supertest' { + declare module.exports: any; +} + +// Filename aliases +declare module 'supertest/index' { + declare module.exports: $Exports<'supertest'>; +} +declare module 'supertest/index.js' { + declare module.exports: $Exports<'supertest'>; +} +declare module 'supertest/lib/agent.js' { + declare module.exports: $Exports<'supertest/lib/agent'>; +} +declare module 'supertest/lib/test.js' { + declare module.exports: $Exports<'supertest/lib/test'>; +} +declare module 'supertest/test/supertest.js' { + declare module.exports: $Exports<'supertest/test/supertest'>; +} diff --git a/src/api/endpoint/index.js b/src/api/endpoint/index.js index b9af5d3fd..cbe659fd4 100644 --- a/src/api/endpoint/index.js +++ b/src/api/endpoint/index.js @@ -38,7 +38,6 @@ export default function(config: Config, auth: IAuth, storage: IStorageHandler) { app.param('anything', match(/.*/)); app.use(auth.basic_middleware()); - // app.use(auth.bearer_middleware()) app.use(bodyParser.json({strict: false, limit: config.max_body_size || '10mb'})); app.use(anti_loop(config)); // encode / in a scoped package name to be matched as a single parameter in routes diff --git a/src/lib/auth.js b/src/lib/auth.js index 7ecc3de11..de2628834 100644 --- a/src/lib/auth.js +++ b/src/lib/auth.js @@ -41,7 +41,7 @@ class Auth { _applyDefaultPlugins() { const allow_action = function(action) { return function(user, pkg, cb) { - let ok = pkg[action].reduce(function(prev, curr) { + const ok = pkg[action].reduce(function(prev, curr) { if (user.name === curr || user.groups.indexOf(curr) !== -1) return true; return prev; }, false); @@ -185,21 +185,19 @@ class Auth { })(); } + /** * Set up a basic middleware. * @return {Function} */ basic_middleware() { - let self = this; - let credentials; - return function(req: $RequestExtend, res: $Response, _next: NextFunction) { + return (req: $RequestExtend, res: $Response, _next: NextFunction) => { req.pause(); const next = function(err) { req.resume(); // uncomment this to reject users with bad auth headers // return _next.apply(null, arguments) - // swallow error, user remains unauthorized // set remoteUserError to indicate that user was attempting authentication if (err) { @@ -213,26 +211,19 @@ class Auth { } req.remote_user = buildAnonymousUser(); - let authorization = req.headers.authorization; + const authorization = req.headers.authorization; if (authorization == null) { return next(); } - let parts = authorization.split(' '); - + const parts = authorization.split(' '); if (parts.length !== 2) { return next( ErrorCode.get400('bad authorization header') ); } - const scheme = parts[0]; - if (scheme === 'Basic') { - credentials = new Buffer(parts[1], 'base64').toString(); - } else if (scheme === 'Bearer') { - credentials = self.aes_decrypt(new Buffer(parts[1], 'base64')).toString('utf8'); - if (!credentials) { - return next(); - } - } else { + const credentials = this._parseCredentials(parts); + + if (!credentials) { return next(); } @@ -241,10 +232,10 @@ class Auth { return next(); } - const user = credentials.slice(0, index); - const pass = credentials.slice(index + 1); + const user: string = credentials.slice(0, index); + const pass: string = credentials.slice(index + 1); - self.authenticate(user, pass, function(err, user) { + this.authenticate(user, pass, function(err, user) { if (!err) { req.remote_user = user; next(); @@ -256,55 +247,19 @@ class Auth { }; } - /** - * Set up the bearer middleware. - * @return {Function} - */ - bearer_middleware() { - let self = this; - return function(req: $RequestExtend, res: $Response, _next: NextFunction) { - req.pause(); - const next = function(_err) { - req.resume(); - /* eslint prefer-spread: "off" */ - /* eslint prefer-rest-params: "off" */ - return _next.apply(null, arguments); - }; - - if (req.remote_user != null && req.remote_user.name !== undefined) { - return next(); + _parseCredentials(parts: Array) { + let credentials; + const scheme = parts[0]; + if (scheme.toUpperCase() === 'BASIC') { + credentials = new Buffer(parts[1], 'base64').toString(); + this.logger.warn('basic authentication is deprecated, please use JWT instead'); + return credentials; + } else if (scheme.toUpperCase() === 'BEARER') { + credentials = this.aes_decrypt(new Buffer(parts[1], 'base64')).toString('utf8'); + return credentials; + } else { + return; } - req.remote_user = buildAnonymousUser(); - - let authorization = req.headers.authorization; - if (authorization == null) { - return next(); - } - - let parts = authorization.split(' '); - - if (parts.length !== 2) { - return next( ErrorCode.get400('bad authorization header') ); - } - - let scheme = parts[0]; - let token = parts[1]; - - if (scheme !== 'Bearer') { - return next(); - } - let user; - try { - user = self.decode_token(token); - } catch(err) { - return next(err); - } - - req.remote_user = authenticatedUser(user.u, user.g); - // $FlowFixMe - req.remote_user.token = token; - next(); - }; } /** diff --git a/test/unit/api.spec.js b/test/unit/api.spec.js index 037e4aa65..c7fb95b47 100644 --- a/test/unit/api.spec.js +++ b/test/unit/api.spec.js @@ -36,7 +36,7 @@ describe('endpoint unit test', () => { describe('Registry API Endpoints', () => { describe('should test ping api', () => { - test('should test endpoint /-/ping', (done) => { + test('should test endpoint /-/ping', (done) => { request(app) .get('/-/ping') .expect('Content-Type', /json/) @@ -79,6 +79,49 @@ describe('endpoint unit test', () => { }); describe('should test user api', () => { + + describe('should test authorization headers with tokens only errors', () => { + test('should fails on protected endpoint /-/auth-package bad format', (done) => { + request(app) + .get('/auth-package') + .set('authorization', 'FakeHader') + .expect('Content-Type', /json/) + .expect(403) + .end(function(err, res) { + expect(res.body.error).toBeDefined(); + expect(res.body.error).toMatch(/unregistered users are not allowed to access package auth-package/); + done(); + }); + }); + + test('should fails on protected endpoint /-/auth-package bad JWT Bearer format', (done) => { + request(app) + .get('/auth-package') + .set('authorization', 'Bearer') + .expect('Content-Type', /json/) + .expect(403) + .end(function(err, res) { + expect(res.body.error).toBeDefined(); + expect(res.body.error).toMatch(/unregistered users are not allowed to access package auth-package/); + done(); + }); + }); + + test('should fails on protected endpoint /-/auth-package well JWT Bearer', (done) => { + request(app) + .get('/auth-package') + .set('authorization', 'Bearer 12345') + .expect('Content-Type', /json/) + .expect(403) + .end(function(err, res) { + expect(res.body.error).toBeDefined(); + expect(res.body.error).toMatch(/unregistered users are not allowed to access package auth-package/); + done(); + }); + }); + }); + + test('should test add a new user', (done) => { request(app) .put('/-/user/org.couchdb.user:jota') @@ -89,10 +132,25 @@ describe('endpoint unit test', () => { if (err) { return done(err); } - expect(res.body.ok).toBeDefined(); + expect(res.body.token).toBeDefined(); + const token = res.body.token; + expect(typeof token).toBe('string'); expect(res.body.ok).toMatch(`user '${credentials.name}' created`); - done(); + + // testing JWT auth headers with token + // we need it here, because token is required + request(app) + .get('/vue') + .set('authorization', `Bearer ${token}`) + .expect('Content-Type', /json/) + .expect(200) + .end(function(err, res) { + expect(err).toBeNull(); + expect(res.body).toBeDefined(); + expect(res.body.name).toMatch(/vue/); + done(); + }); }); }); diff --git a/test/unit/partials/config/index.js b/test/unit/partials/config/index.js index 99262b33e..694fa3ab1 100644 --- a/test/unit/partials/config/index.js +++ b/test/unit/partials/config/index.js @@ -31,6 +31,15 @@ const config = { allow_publish: '$all', proxy: 'npmjs' }, + 'auth-package': { + allow_access: '$authenticated', + allow_publish: '$authenticated' + }, + 'vue': { + allow_access: '$authenticated', + allow_publish: '$authenticated', + proxy: 'npmjs' + }, '*': { allow_access: '$all', allow_publish: '$all'