From eca62597e8e2efbadee925a0692320cf788d9eab Mon Sep 17 00:00:00 2001 From: Marc Bernard <59966492+mbtools@users.noreply.github.com> Date: Sun, 1 Dec 2024 09:44:00 -0500 Subject: [PATCH] fix: error E409 "username is already registered" (adduser) (#4957) * fix: E409 username is already registered (adduser) * update tests --- .changeset/green-eagles-boil.md | 6 +++ packages/api/src/user.ts | 6 ++- packages/auth/src/utils.ts | 4 +- packages/auth/test/auth-utils.spec.ts | 7 ++-- packages/auth/test/auth.spec.ts | 55 ++++++++++++++++----------- 5 files changed, 51 insertions(+), 27 deletions(-) create mode 100644 .changeset/green-eagles-boil.md diff --git a/.changeset/green-eagles-boil.md b/.changeset/green-eagles-boil.md new file mode 100644 index 000000000..bf1ee71eb --- /dev/null +++ b/.changeset/green-eagles-boil.md @@ -0,0 +1,6 @@ +--- +'@verdaccio/auth': patch +'@verdaccio/api': patch +--- + +fix: E409 username is already registered (adduser) diff --git a/packages/api/src/user.ts b/packages/api/src/user.ts index 05b58c42e..e42f4ba41 100644 --- a/packages/api/src/user.ts +++ b/packages/api/src/user.ts @@ -28,7 +28,11 @@ export default function (route: Router, auth: Auth, config: Config, logger: Logg function (req: $RequestExtend, res: Response, next: $NextFunctionVer): void { debug('verifying user'); - if (typeof req.remote_user.name !== 'string' || req.remote_user.name === '') { + if ( + !req.remote_user || + typeof req.remote_user.name !== 'string' || + req.remote_user.name === '' + ) { debug('user not logged in'); res.status(HTTP_STATUS.OK); return next({ ok: false }); diff --git a/packages/auth/src/utils.ts b/packages/auth/src/utils.ts index fa5929c8b..3b3328d50 100644 --- a/packages/auth/src/utils.ts +++ b/packages/auth/src/utils.ts @@ -166,7 +166,9 @@ export function getDefaultPlugins(logger: Logger): pluginUtils.Auth { adduser(_user: string, _password: string, cb: pluginUtils.AuthUserCallback): void { debug('triggered default adduser method'); - return cb(errorUtils.getConflict(API_ERROR.BAD_USERNAME_PASSWORD)); + // since adduser is not implemented but optional, continue without error + // this assumes that the user is added by an external system + cb(null, true); }, // @ts-ignore diff --git a/packages/auth/test/auth-utils.spec.ts b/packages/auth/test/auth-utils.spec.ts index 5eba75f73..65d8d5d20 100644 --- a/packages/auth/test/auth-utils.spec.ts +++ b/packages/auth/test/auth-utils.spec.ts @@ -123,11 +123,12 @@ describe('Auth utilities', () => { }); }); - test('add user should fail by default (default)', () => { + test('add user should not fail by default (default)', () => { const plugin = getDefaultPlugins({ trace: vi.fn() }); // @ts-ignore - plugin.adduser('foo', 'bar', (error: any) => { - expect(error).toEqual(errorUtils.getForbidden(API_ERROR.BAD_USERNAME_PASSWORD)); + plugin.adduser('foo', 'bar', (error: any, access: any) => { + expect(error).toEqual(null); + expect(access).toEqual(true); }); }); }); diff --git a/packages/auth/test/auth.spec.ts b/packages/auth/test/auth.spec.ts index d2dd88912..982562dcf 100644 --- a/packages/auth/test/auth.spec.ts +++ b/packages/auth/test/auth.spec.ts @@ -4,14 +4,7 @@ import supertest from 'supertest'; import { describe, expect, test, vi } from 'vitest'; import { Config as AppConfig, ROLES, createRemoteUser, getDefaultConfig } from '@verdaccio/config'; -import { - API_ERROR, - HEADERS, - HTTP_STATUS, - SUPPORT_ERRORS, - TOKEN_BEARER, - errorUtils, -} from '@verdaccio/core'; +import { HEADERS, HTTP_STATUS, SUPPORT_ERRORS, TOKEN_BEARER, errorUtils } from '@verdaccio/core'; import { logger, setup } from '@verdaccio/logger'; import { errorReportingMiddleware, final, handleError } from '@verdaccio/middleware'; import { Config } from '@verdaccio/types'; @@ -259,7 +252,7 @@ describe('AuthTest', () => { describe('no custom allow_access implementation provided', () => { // when allow_access is not implemented, the groups must match // exactly with the packages access group - test('should fails if groups do not match exactly', async () => { + test('should fail if groups do not match exactly', async () => { const config: Config = new AppConfig({ ...authProfileConf }); config.checkSecretKey('12345'); const auth: Auth = new Auth(config, logger); @@ -308,7 +301,7 @@ describe('AuthTest', () => { describe('no custom allow_publish implementation provided', () => { // when allow_access is not implemented, the groups must match // exactly with the packages access group - test('should fails if groups do not match exactly', async () => { + test('should fail if groups do not match exactly', async () => { const config: Config = new AppConfig({ ...authProfileConf }); config.checkSecretKey('12345'); const auth: Auth = new Auth(config, logger); @@ -354,7 +347,7 @@ describe('AuthTest', () => { }); describe('allow_unpublish', () => { describe('no custom allow_unpublish implementation provided', () => { - test('should fails if groups do not match exactly', async () => { + test('should fail if groups do not match exactly', async () => { const config: Config = new AppConfig({ ...authProfileConf }); config.checkSecretKey('12345'); @@ -439,7 +432,7 @@ describe('AuthTest', () => { describe('error handling', () => { // when allow_access is not implemented, the groups must match // exactly with the packages access group - test('should fails with bad password if adduser is not implemented', async () => { + test('should not fail if adduser is not implemented', async () => { const config: Config = new AppConfig({ ...authProfileConf }); config.checkSecretKey('12345'); const auth: Auth = new Auth(config, logger); @@ -451,12 +444,21 @@ describe('AuthTest', () => { auth.add_user('juan', 'password', callback); expect(callback).toHaveBeenCalledTimes(1); - expect(callback).toHaveBeenCalledWith( - errorUtils.getConflict(API_ERROR.BAD_USERNAME_PASSWORD) - ); + expect(callback).toHaveBeenCalledWith(null, { + groups: [ + 'test', + ROLES.$ALL, + ROLES.$AUTH, + ROLES.DEPRECATED_ALL, + ROLES.DEPRECATED_AUTH, + ROLES.ALL, + ], + name: 'juan', + real_groups: ['test'], + }); }); - test('should fails if adduser fails internally (exception)', async () => { + test('should fail if adduser fails internally (exception)', async () => { const config: Config = new AppConfig({ ...getDefaultConfig(), plugins: path.join(__dirname, './partials/plugin'), @@ -471,14 +473,14 @@ describe('AuthTest', () => { const callback = vi.fn(); - // note: fail uas username make plugin fails + // note: username to make plugin fail auth.add_user('fail', 'password', callback); expect(callback).toHaveBeenCalledTimes(1); expect(callback).toHaveBeenCalledWith(new Error('bad username')); }); - test('should skip to the next plugin and fails', async () => { + test('should skip to the next plugin and fail', async () => { const config: Config = new AppConfig({ ...getDefaultConfig(), plugins: path.join(__dirname, './partials/plugin'), @@ -495,13 +497,22 @@ describe('AuthTest', () => { const callback = vi.fn(); - // note: fail uas username make plugin fails + // note: username to make plugin fail auth.add_user('skip', 'password', callback); expect(callback).toHaveBeenCalledTimes(1); - expect(callback).toHaveBeenCalledWith( - errorUtils.getConflict(API_ERROR.BAD_USERNAME_PASSWORD) - ); + expect(callback).toHaveBeenCalledWith(null, { + groups: [ + 'test', + ROLES.$ALL, + ROLES.$AUTH, + ROLES.DEPRECATED_ALL, + ROLES.DEPRECATED_AUTH, + ROLES.ALL, + ], + name: 'skip', + real_groups: ['test'], + }); }); }); test('should success if adduser', async () => {