From 85c1bd1f764d52749eb27599c92c7caf01f4b992 Mon Sep 17 00:00:00 2001 From: "Juan Picado @jotadeveloper" Date: Thu, 13 Jun 2019 06:58:43 +0200 Subject: [PATCH] fix(api): force authenticate on login (#1347) When a user has a valid token and tries to login with other credentials the endpoint returns 201. The reason was if another user logged previously and had a valid token stored in the terminal. We must authenticate any user that tries to log in even if the token stored is valid. We must check credentials again and return a new token, if the credentials are wrong we reject the login. Furthermore, the new token will update the list of groups. --- src/api/endpoint/api/user.js | 28 ++++++++++++++++-------- test/unit/api/__api-helper.js | 23 ++++++++++++++++++-- test/unit/api/api.jwt.spec.js | 40 ++++++++++++++++++++++++++++------- 3 files changed, 72 insertions(+), 19 deletions(-) diff --git a/src/api/endpoint/api/user.js b/src/api/endpoint/api/user.js index 60e09c3f1..3143a2d15 100644 --- a/src/api/endpoint/api/user.js +++ b/src/api/endpoint/api/user.js @@ -8,9 +8,10 @@ import Cookies from 'cookies'; import { ErrorCode } from '../../../lib/utils'; import { API_ERROR, API_MESSAGE, HTTP_STATUS } from '../../../lib/constants'; -import { createSessionToken, getApiToken, getAuthenticatedMessage, validatePassword } from '../../../lib/auth-utils'; +import { createRemoteUser, createSessionToken, getApiToken, getAuthenticatedMessage, validatePassword } from '../../../lib/auth-utils'; +import logger from '../../../lib/logger'; -import type { Config } from '@verdaccio/types'; +import type { Config, RemoteUser } from '@verdaccio/types'; import type { $Response, Router } from 'express'; import type { $RequestExtend, $ResponseExtend, $NextFunctionVer, IAuth } from '../../../../types'; @@ -22,17 +23,26 @@ export default function(route: Router, auth: IAuth, config: Config) { }); }); - route.put('/-/user/:org_couchdb_user/:_rev?/:revision?', async function(req: $RequestExtend, res: $Response, next: $NextFunctionVer) { + route.put('/-/user/:org_couchdb_user/:_rev?/:revision?', function(req: $RequestExtend, res: $Response, next: $NextFunctionVer) { const { name, password } = req.body; + const remoteName = req.remote_user.name; - if (_.isNil(req.remote_user.name) === false) { - const token = name && password ? await getApiToken(auth, config, req.remote_user, password) : undefined; + if (_.isNil(remoteName) === false && _.isNil(name) === false && remoteName === name) { + auth.authenticate(name, password, async function callbackAuthenticate(err, groups) { + if (err) { + logger.logger.trace({ name, err }, 'authenticating for user @{username} failed. Error: @{err.message}'); + return next(ErrorCode.getCode(HTTP_STATUS.UNAUTHORIZED, API_ERROR.BAD_USERNAME_PASSWORD)); + } - res.status(HTTP_STATUS.CREATED); + const restoredRemoteUser: RemoteUser = createRemoteUser(name, groups); + const token = await getApiToken(auth, config, restoredRemoteUser, password); - return next({ - ok: getAuthenticatedMessage(req.remote_user.name), - token, + res.status(HTTP_STATUS.CREATED); + + return next({ + ok: getAuthenticatedMessage(req.remote_user.name), + token, + }); }); } else { if (validatePassword(password) === false) { diff --git a/test/unit/api/__api-helper.js b/test/unit/api/__api-helper.js index df4159561..2b09d6fb2 100644 --- a/test/unit/api/__api-helper.js +++ b/test/unit/api/__api-helper.js @@ -1,6 +1,7 @@ // @flow -import {HEADER_TYPE, HEADERS, HTTP_STATUS} from '../../../src/lib/constants'; +import {HEADER_TYPE, HEADERS, HTTP_STATUS, TOKEN_BEARER} from '../../../src/lib/constants'; +import {buildToken} from "../../../src/lib/utils"; export function getPackage( request: any, @@ -19,6 +20,24 @@ export function getPackage( }); } +export function loginUserToken(request: any, + user: string, + credentials: any, + token: string, + statusCode: number = HTTP_STATUS.CREATED) { + // $FlowFixMe + return new Promise((resolve) => { + request.put(`/-/user/org.couchdb.user:${user}`) + .send(credentials) + .set('authorization', buildToken(TOKEN_BEARER, token)) + .expect(HEADER_TYPE.CONTENT_TYPE, HEADERS.JSON_CHARSET) + .expect(statusCode) + .end(function(err, res) { + return resolve([err, res]); + }); + }); +} + export function addUser(request: any, user: string, credentials: any, statusCode: number = HTTP_STATUS.CREATED) { // $FlowFixMe @@ -50,7 +69,7 @@ export function getProfile(request: any, token: string, statusCode: number = HTT // $FlowFixMe return new Promise((resolve) => { request.get(`/-/npm/v1/user`) - .set('authorization', `Bearer ${token}`) + .set('authorization', buildToken(TOKEN_BEARER, token)) .expect(HEADER_TYPE.CONTENT_TYPE, HEADERS.JSON_CHARSET) .expect(statusCode) .end(function(err, res) { diff --git a/test/unit/api/api.jwt.spec.js b/test/unit/api/api.jwt.spec.js index 196efc4fe..6518d3271 100644 --- a/test/unit/api/api.jwt.spec.js +++ b/test/unit/api/api.jwt.spec.js @@ -7,12 +7,12 @@ import rimraf from 'rimraf'; import endPointAPI from '../../../src/api/index'; -import {HEADERS, HTTP_STATUS, HEADER_TYPE} from '../../../src/lib/constants'; +import {HEADERS, HTTP_STATUS, HEADER_TYPE, TOKEN_BASIC, TOKEN_BEARER, API_ERROR} from '../../../src/lib/constants'; import {mockServer} from './mock'; import {DOMAIN_SERVERS} from '../../functional/config.functional'; -import {parseConfigFile} from '../../../src/lib/utils'; +import {buildToken, parseConfigFile} from '../../../src/lib/utils'; import {parseConfigurationFile} from '../__helper'; -import {addUser, getPackage} from './__api-helper'; +import {addUser, getPackage, loginUserToken} from './__api-helper'; import {setup} from '../../../src/lib/logger'; import {buildUserBuffer} from '../../../src/lib/auth-utils'; @@ -65,17 +65,19 @@ describe('endpoint user auth JWT unit test', () => { expect(err).toBeNull(); expect(res.body.ok).toBeDefined(); expect(res.body.token).toBeDefined(); - const token = res.body.token; + + const { token } = res.body; expect(typeof token).toBe('string'); expect(res.body.ok).toMatch(`user '${credentials.name}' created`); + // testing JWT auth headers with token // we need it here, because token is required - const [err1, resp1] = await getPackage(request(app), `Bearer ${token}`, 'vue'); + const [err1, resp1] = await getPackage(request(app), buildToken(TOKEN_BEARER, token), 'vue'); expect(err1).toBeNull(); expect(resp1.body).toBeDefined(); expect(resp1.body.name).toMatch('vue'); - const [err2, resp2] = await getPackage(request(app), `Bearer fake`, 'vue', HTTP_STATUS.UNAUTHORIZED); + const [err2, resp2] = await getPackage(request(app), buildToken(TOKEN_BEARER, 'fake'), 'vue', HTTP_STATUS.UNAUTHORIZED); expect(err2).toBeNull(); expect(resp2.statusCode).toBe(HTTP_STATUS.UNAUTHORIZED); expect(resp2.body.error).toMatch(FORBIDDEN_VUE); @@ -88,27 +90,49 @@ describe('endpoint user auth JWT unit test', () => { await addUser(request(app), credentials.name, credentials); // it should fails conflict 409 await addUser(request(app), credentials.name, credentials, HTTP_STATUS.CONFLICT); + // npm will try to sign in sending credentials via basic auth header const token = buildUserBuffer(credentials.name, credentials.password).toString('base64'); request(app).put(`/-/user/org.couchdb.user:${credentials.name}/-rev/undefined`) .send(credentials) - .set('authorization', `Basic ${token}`) + .set('authorization', buildToken(TOKEN_BASIC, token)) .expect(HEADER_TYPE.CONTENT_TYPE, HEADERS.JSON_CHARSET) .expect(HTTP_STATUS.CREATED) .end(function(err, res) { expect(err).toBeNull(); expect(res.body.ok).toBeDefined(); expect(res.body.token).toBeDefined(); + done(); }); }); test('should fails on try to access with corrupted token', async (done) => { - const [err2, resp2] = await getPackage(request(app), `Bearer fake`, 'vue', HTTP_STATUS.UNAUTHORIZED); + const [err2, resp2] = await getPackage(request(app), buildToken(TOKEN_BEARER, 'fake'), 'vue', HTTP_STATUS.UNAUTHORIZED); expect(err2).toBeNull(); expect(resp2.statusCode).toBe(HTTP_STATUS.UNAUTHORIZED); expect(resp2.body.error).toMatch(FORBIDDEN_VUE); done(); }); + test('should fails on login if user credentials are invalid even if jwt valid token is provided', async (done) => { + const credentials = { name: 'newFailsUser', password: 'secretPass' }; + const [err, res] = await addUser(request(app), credentials.name, credentials); + expect(err).toBeNull(); + expect(res.body.ok).toBeDefined(); + expect(res.body.token).toBeDefined(); + + const { token } = res.body; + expect(typeof token).toBe('string'); + expect(res.body.ok).toMatch(`user '${credentials.name}' created`); + + // we login when token is valid + const newCredentials = { name: 'newFailsUser', password: 'BAD_PASSWORD' }; + const [err2, resp2] = await loginUserToken(request(app), newCredentials.name, newCredentials, token, HTTP_STATUS.UNAUTHORIZED); + expect(err2).toBeNull(); + expect(resp2.statusCode).toBe(HTTP_STATUS.UNAUTHORIZED); + expect(resp2.body.error).toMatch(API_ERROR.BAD_USERNAME_PASSWORD); + + done(); + }); });