Skip to content

Commit

Permalink
fix(api): force authenticate on login (#1347)
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@juanpicado
juanpicado committed Jun 13, 2019
1 parent 192fb77 commit 85c1bd1
Show file tree
Hide file tree
Showing 3 changed files with 72 additions and 19 deletions.
28 changes: 19 additions & 9 deletions src/api/endpoint/api/user.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -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);

res.status(HTTP_STATUS.CREATED);

return next({
ok: getAuthenticatedMessage(req.remote_user.name),
token,
return next({
ok: getAuthenticatedMessage(req.remote_user.name),
token,
});
});
} else {
if (validatePassword(password) === false) {
Expand Down
23 changes: 21 additions & 2 deletions test/unit/api/__api-helper.js
View file
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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
Expand Down Expand Up @@ -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) {
Expand Down
40 changes: 32 additions & 8 deletions test/unit/api/api.jwt.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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);
Expand All @@ -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();
});
});

0 comments on commit 85c1bd1

Please sign in to comment.