Skip to content

Commit

Permalink
fix: duplicated groups on use jwt tokens (#3151)
Browse files Browse the repository at this point in the history
* fix: duplicated groups on use jwt tokens

* chore: format

* chore: fix jest ci
  • Loading branch information
juanpicado committed May 4, 2022
1 parent 34b7394 commit 51803c3
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 16 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/e2e-jest-workflow.yml
Expand Up @@ -140,7 +140,7 @@ jobs:
yarn jest module.test.js
pnpm6:
name: 'pnpm:next-6:jest example'
name: 'pnpm:latest-6:jest example'
runs-on: ubuntu-latest

steps:
Expand All @@ -151,7 +151,7 @@ jobs:
with:
node-version: 12.x
- name: 'install latest pnpm'
run: npm i -g pnpm@next-6
run: npm i -g pnpm@latest-6
- name: Install Dependencies
run: yarn install
- name: 'Run verdaccio in the background'
Expand Down
2 changes: 1 addition & 1 deletion src/lib/auth-utils.ts
Expand Up @@ -24,7 +24,7 @@ export function validatePassword(
*/
export function createRemoteUser(name: string, pluginGroups: string[]): RemoteUser {
const isGroupValid: boolean = Array.isArray(pluginGroups);
const groups = (isGroupValid ? pluginGroups : []).concat([ROLES.$ALL, ROLES.$AUTH, ROLES.DEPRECATED_ALL, ROLES.DEPRECATED_AUTH, ROLES.ALL]);
const groups = Array.from(new Set((isGroupValid ? pluginGroups : []).concat([ROLES.$ALL, ROLES.$AUTH, ROLES.DEPRECATED_ALL, ROLES.DEPRECATED_AUTH, ROLES.ALL])));

return {
name,
Expand Down
2 changes: 1 addition & 1 deletion src/lib/auth.ts
Expand Up @@ -420,7 +420,7 @@ class Auth implements IAuth {
public async jwtEncrypt(user: RemoteUser, signOptions: JWTSignOptions): Promise<string> {
const { real_groups, name, groups } = user;
const realGroupsValidated = _.isNil(real_groups) ? [] : real_groups;
const groupedGroups = _.isNil(groups) ? real_groups : groups.concat(realGroupsValidated);
const groupedGroups = _.isNil(groups) ? real_groups : Array.from(new Set([...groups.concat(realGroupsValidated)]));
const payload: RemoteUser = {
real_groups: realGroupsValidated,
name,
Expand Down
54 changes: 42 additions & 12 deletions test/unit/modules/auth/auth-utils.spec.ts
Expand Up @@ -3,7 +3,15 @@ import _ from 'lodash';
import { Config, RemoteUser, Security } from '@verdaccio/types';

import Auth from '../../../../src/lib/auth';
import { buildUserBuffer, getApiToken, getAuthenticatedMessage, getMiddlewareCredentials, getSecurity } from '../../../../src/lib/auth-utils';
import {
buildUserBuffer,
createAnonymousRemoteUser,
createRemoteUser,
getApiToken,
getAuthenticatedMessage,
getMiddlewareCredentials,
getSecurity,
} from '../../../../src/lib/auth-utils';
import AppConfig from '../../../../src/lib/config';
import { CHARACTER_ENCODING, TOKEN_BEARER } from '../../../../src/lib/constants';
import { aesDecrypt, verifyPayload } from '../../../../src/lib/crypto-utils';
Expand Down Expand Up @@ -41,8 +49,8 @@ describe('Auth utilities', () => {
const spyNotCalled = jest.spyOn(auth, methodNotBeenCalled);
const user: RemoteUser = {
name: username,
real_groups: [],
groups: [],
real_groups: ['test', '$all', '$authenticated', '@all', '@authenticated', 'all'],
groups: ['company-role1', 'company-role2'],
};
const token = await getApiToken(auth, config, user, password);
expect(spy).toHaveBeenCalled();
Expand All @@ -57,7 +65,9 @@ describe('Auth utilities', () => {
const payload = verifyPayload(token, secret);
expect(payload.name).toBe(user);
expect(payload.groups).toBeDefined();
expect(payload.groups).toEqual(['company-role1', 'company-role2', 'test', '$all', '$authenticated', '@all', '@authenticated', 'all']);
expect(payload.real_groups).toBeDefined();
expect(payload.real_groups).toEqual(['test', '$all', '$authenticated', '@all', '@authenticated', 'all']);
};

const verifyAES = (token: string, user: string, password: string, secret: string) => {
Expand All @@ -68,6 +78,30 @@ describe('Auth utilities', () => {
expect(content[0]).toBe(password);
};

describe('createRemoteUser', () => {
test('create remote user', () => {
expect(createRemoteUser('test', [])).toEqual({
name: 'test',
real_groups: [],
groups: ['$all', '$authenticated', '@all', '@authenticated', 'all'],
});
});
test('create remote user with groups', () => {
expect(createRemoteUser('test', ['group1', 'group2'])).toEqual({
name: 'test',
real_groups: ['group1', 'group2'],
groups: ['group1', 'group2', '$all', '$authenticated', '@all', '@authenticated', 'all'],
});
});
test('create anonymous remote user', () => {
expect(createAnonymousRemoteUser()).toEqual({
name: undefined,
real_groups: [],
groups: ['$all', '$anonymous', '@all', '@anonymous'],
});
});
});

describe('getApiToken test', () => {
test('should sign token with aes and security missing', async () => {
const token = await signCredentials('security-missing', 'test', 'test', '1234567', 'aesEncrypt', 'jwtEncrypt');
Expand Down Expand Up @@ -190,15 +224,13 @@ describe('Auth utilities', () => {
test('should return anonymous whether token is corrupted', () => {
const config: Config = getConfig('security-jwt', '12345');
const security: Security = getSecurity(config);
const credentials = getMiddlewareCredentials(security, '12345', buildToken(TOKEN_BEARER, 'fakeToken'));
const credentials = getMiddlewareCredentials(security, '12345', buildToken(TOKEN_BEARER, 'fakeToken')) as RemoteUser;

expect(credentials).toBeDefined();
// @ts-ignore
expect(credentials.name).not.toBeDefined();
// @ts-ignore
expect(credentials.real_groups).toBeDefined();
// @ts-ignore
expect(credentials.real_groups).toEqual([]);
expect(credentials.groups).toEqual(['$all', '$anonymous', '@all', '@anonymous']);
});

test('should return anonymous whether token and scheme are corrupted', () => {
Expand All @@ -215,14 +247,12 @@ describe('Auth utilities', () => {
const config: Config = getConfig('security-jwt', secret);
const token = await signCredentials('security-jwt', user, 'secretTest', secret, 'jwtEncrypt', 'aesEncrypt');
const security: Security = getSecurity(config);
const credentials = getMiddlewareCredentials(security, secret, buildToken(TOKEN_BEARER, token));
const credentials = getMiddlewareCredentials(security, secret, buildToken(TOKEN_BEARER, token)) as RemoteUser;
expect(credentials).toBeDefined();
// @ts-ignore
expect(credentials.name).toEqual(user);
// @ts-ignore
expect(credentials.real_groups).toBeDefined();
// @ts-ignore
expect(credentials.real_groups).toEqual([]);
expect(credentials.real_groups).toEqual(['test', '$all', '$authenticated', '@all', '@authenticated', 'all']);
expect(credentials.groups).toEqual(['company-role1', 'company-role2', 'test', '$all', '$authenticated', '@all', '@authenticated', 'all']);
});
});
});
Expand Down

0 comments on commit 51803c3

Please sign in to comment.