From 4fb1bcb52451613616b5d3348ecbe7ed5796329d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ivar=20Conradi=20=C3=98sthus?= Date: Fri, 29 Oct 2021 10:25:42 +0200 Subject: [PATCH] feat: Disable password based login (#1046) This commit will introduce a new setting used to disbaled simple password based authention. The setting itself is an enterprise setting. --- src/lib/routes/admin-api/config.ts | 33 +++++++++++---- src/lib/server-impl.ts | 2 + src/lib/services/index.ts | 3 +- src/lib/services/setting-service.ts | 2 +- src/lib/services/user-service.test.ts | 42 +++++++++++++++++++ src/lib/services/user-service.ts | 17 ++++++++ src/lib/types/authentication-required.ts | 12 +++++- .../types/settings/simple-auth-settings.ts | 4 ++ src/lib/types/stores/settings-store.ts | 2 +- src/test/e2e/api/admin/config.e2e.test.ts | 37 ++++++++++++++++ .../reset-password-controller.e2e.test.ts | 7 ++++ .../services/reset-token-service.e2e.test.ts | 7 ++++ .../e2e/services/user-service.e2e.test.ts | 20 +++++++++ src/test/fixtures/fake-setting-store.ts | 3 +- 14 files changed, 176 insertions(+), 15 deletions(-) create mode 100644 src/lib/types/settings/simple-auth-settings.ts create mode 100644 src/test/e2e/api/admin/config.e2e.test.ts diff --git a/src/lib/routes/admin-api/config.ts b/src/lib/routes/admin-api/config.ts index 93f7575816..fbc4867d7e 100644 --- a/src/lib/routes/admin-api/config.ts +++ b/src/lib/routes/admin-api/config.ts @@ -3,15 +3,31 @@ import { IUnleashServices } from '../../types/services'; import { IUnleashConfig } from '../../types/option'; import version from '../../util/version'; -const Controller = require('../controller'); +import Controller from '../controller'; +import VersionService from '../../services/version-service'; +import SettingService from '../../services/setting-service'; +import { + simpleAuthKey, + SimpleAuthSettings, +} from '../../types/settings/simple-auth-settings'; class ConfigController extends Controller { + private versionService: VersionService; + + private settingService: SettingService; + + private uiConfig: any; + constructor( config: IUnleashConfig, - { versionService }: Pick, + { + versionService, + settingService, + }: Pick, ) { super(config); this.versionService = versionService; + this.settingService = settingService; const authenticationType = config.authentication && config.authentication.type; this.uiConfig = { @@ -26,13 +42,12 @@ class ConfigController extends Controller { async getUIConfig(req: Request, res: Response): Promise { const config = this.uiConfig; - if (this.versionService) { - const versionInfo = this.versionService.getVersionInfo(); - res.json({ ...config, versionInfo }); - } else { - res.json(config); - } + const simpleAuthSettings = + await this.settingService.get(simpleAuthKey); + + const versionInfo = this.versionService.getVersionInfo(); + const disablePasswordAuth = simpleAuthSettings?.disabled; + res.json({ ...config, versionInfo, disablePasswordAuth }); } } export default ConfigController; -module.exports = ConfigController; diff --git a/src/lib/server-impl.ts b/src/lib/server-impl.ts index 0c892e6c87..c298e7b14e 100644 --- a/src/lib/server-impl.ts +++ b/src/lib/server-impl.ts @@ -24,6 +24,7 @@ import { IAuthRequest } from './routes/unleash-types'; import * as permissions from './types/permissions'; import * as eventType from './types/events'; import { RoleName } from './types/model'; +import { SimpleAuthSettings } from './types/settings/simple-auth-settings'; async function createApp( config: IUnleashConfig, @@ -177,4 +178,5 @@ export type { IUser, IUnleashServices, IAuthRequest, + SimpleAuthSettings, }; diff --git a/src/lib/services/index.ts b/src/lib/services/index.ts index 8832d78f9c..bc713d7c2e 100644 --- a/src/lib/services/index.ts +++ b/src/lib/services/index.ts @@ -47,15 +47,16 @@ export const createServices = ( const tagTypeService = new TagTypeService(stores, config); const addonService = new AddonService(stores, config, tagTypeService); const sessionService = new SessionService(stores, config); + const settingService = new SettingService(stores, config); const userService = new UserService(stores, config, { accessService, resetTokenService, emailService, sessionService, + settingService, }); const versionService = new VersionService(stores, config); const healthService = new HealthService(stores, config); - const settingService = new SettingService(stores, config); const userFeedbackService = new UserFeedbackService(stores, config); const featureToggleServiceV2 = new FeatureToggleServiceV2(stores, config); const environmentService = new EnvironmentService(stores, config); diff --git a/src/lib/services/setting-service.ts b/src/lib/services/setting-service.ts index 4d7a8d3b0e..282b86b944 100644 --- a/src/lib/services/setting-service.ts +++ b/src/lib/services/setting-service.ts @@ -16,7 +16,7 @@ export default class SettingService { this.settingStore = settingStore; } - async get(id: string): Promise { + async get(id: string): Promise { return this.settingStore.get(id); } diff --git a/src/lib/services/user-service.test.ts b/src/lib/services/user-service.test.ts index 84485a35f2..126fd478de 100644 --- a/src/lib/services/user-service.test.ts +++ b/src/lib/services/user-service.test.ts @@ -11,6 +11,8 @@ import SessionService from './session-service'; import FakeSessionStore from '../../test/fixtures/fake-session-store'; import User from '../types/user'; import FakeResetTokenStore from '../../test/fixtures/fake-reset-token-store'; +import SettingService from './setting-service'; +import FakeSettingStore from '../../test/fixtures/fake-setting-store'; const config: IUnleashConfig = createTestConfig(); @@ -28,12 +30,17 @@ test('Should create new user', async () => { const sessionStore = new FakeSessionStore(); const sessionService = new SessionService({ sessionStore }, config); const emailService = new EmailService(config.email, config.getLogger); + const settingService = new SettingService( + { settingStore: new FakeSettingStore() }, + config, + ); const service = new UserService({ userStore, eventStore }, config, { accessService, resetTokenService, emailService, sessionService, + settingService, }); const user = await service.createUser( { @@ -63,12 +70,17 @@ test('Should create default user', async () => { const emailService = new EmailService(config.email, config.getLogger); const sessionStore = new FakeSessionStore(); const sessionService = new SessionService({ sessionStore }, config); + const settingService = new SettingService( + { settingStore: new FakeSettingStore() }, + config, + ); const service = new UserService({ userStore, eventStore }, config, { accessService, resetTokenService, emailService, sessionService, + settingService, }); await service.initAdminUser(); @@ -90,12 +102,17 @@ test('Should be a valid password', async () => { const emailService = new EmailService(config.email, config.getLogger); const sessionStore = new FakeSessionStore(); const sessionService = new SessionService({ sessionStore }, config); + const settingService = new SettingService( + { settingStore: new FakeSettingStore() }, + config, + ); const service = new UserService({ userStore, eventStore }, config, { accessService, resetTokenService, emailService, sessionService, + settingService, }); const valid = service.validatePassword('this is a strong password!'); @@ -115,12 +132,17 @@ test('Password must be at least 10 chars', async () => { const emailService = new EmailService(config.email, config.getLogger); const sessionStore = new FakeSessionStore(); const sessionService = new SessionService({ sessionStore }, config); + const settingService = new SettingService( + { settingStore: new FakeSettingStore() }, + config, + ); const service = new UserService({ userStore, eventStore }, config, { accessService, resetTokenService, emailService, sessionService, + settingService, }); expect(() => service.validatePassword('admin')).toThrow( 'The password must be at least 10 characters long.', @@ -142,12 +164,17 @@ test('The password must contain at least one uppercase letter.', async () => { const emailService = new EmailService(config.email, config.getLogger); const sessionStore = new FakeSessionStore(); const sessionService = new SessionService({ sessionStore }, config); + const settingService = new SettingService( + { settingStore: new FakeSettingStore() }, + config, + ); const service = new UserService({ userStore, eventStore }, config, { accessService, resetTokenService, emailService, sessionService, + settingService, }); expect(() => service.validatePassword('qwertyabcde')).toThrowError( @@ -171,12 +198,17 @@ test('The password must contain at least one number', async () => { const emailService = new EmailService(config.email, config.getLogger); const sessionStore = new FakeSessionStore(); const sessionService = new SessionService({ sessionStore }, config); + const settingService = new SettingService( + { settingStore: new FakeSettingStore() }, + config, + ); const service = new UserService({ userStore, eventStore }, config, { accessService, resetTokenService, emailService, sessionService, + settingService, }); expect(() => service.validatePassword('qwertyabcdE')).toThrowError( @@ -199,12 +231,17 @@ test('The password must contain at least one special character', async () => { const emailService = new EmailService(config.email, config.getLogger); const sessionStore = new FakeSessionStore(); const sessionService = new SessionService({ sessionStore }, config); + const settingService = new SettingService( + { settingStore: new FakeSettingStore() }, + config, + ); const service = new UserService({ userStore, eventStore }, config, { accessService, resetTokenService, emailService, sessionService, + settingService, }); expect(() => service.validatePassword('qwertyabcdE2')).toThrowError( @@ -227,12 +264,17 @@ test('Should be a valid password with special chars', async () => { const emailService = new EmailService(config.email, config.getLogger); const sessionStore = new FakeSessionStore(); const sessionService = new SessionService({ sessionStore }, config); + const settingService = new SettingService( + { settingStore: new FakeSettingStore() }, + config, + ); const service = new UserService({ userStore, eventStore }, config, { accessService, resetTokenService, emailService, sessionService, + settingService, }); const valid = service.validatePassword('this is a strong password!'); diff --git a/src/lib/services/user-service.ts b/src/lib/services/user-service.ts index 382d586213..2bb5778641 100644 --- a/src/lib/services/user-service.ts +++ b/src/lib/services/user-service.ts @@ -21,6 +21,9 @@ import { USER_UPDATED, USER_CREATED, USER_DELETED } from '../types/events'; import { IEventStore } from '../types/stores/event-store'; import { IUserSearch, IUserStore } from '../types/stores/user-store'; import { RoleName } from '../types/model'; +import SettingService from './setting-service'; +import { SimpleAuthSettings } from '../server-impl'; +import { simpleAuthKey } from '../types/settings/simple-auth-settings'; const systemUser = new User({ id: -1, username: 'system' }); @@ -77,6 +80,8 @@ class UserService { private emailService: EmailService; + private settingService: SettingService; + constructor( stores: Pick, { @@ -88,6 +93,7 @@ class UserService { resetTokenService: ResetTokenService; emailService: EmailService; sessionService: SessionService; + settingService: SettingService; }, ) { this.logger = getLogger('service/user-service.js'); @@ -97,6 +103,7 @@ class UserService { this.resetTokenService = services.resetTokenService; this.emailService = services.emailService; this.sessionService = services.sessionService; + this.settingService = services.settingService; if (authentication && authentication.createAdminUser) { process.nextTick(() => this.initAdminUser()); } @@ -241,6 +248,16 @@ class UserService { } async loginUser(usernameOrEmail: string, password: string): Promise { + const settings = await this.settingService.get( + simpleAuthKey, + ); + + if (settings && settings.disabled) { + throw new Error( + 'Logging in with username/password has been disabled.', + ); + } + const idQuery = isEmail(usernameOrEmail) ? { email: usernameOrEmail } : { username: usernameOrEmail }; diff --git a/src/lib/types/authentication-required.ts b/src/lib/types/authentication-required.ts index 74b7ba391f..e3d8519df2 100644 --- a/src/lib/types/authentication-required.ts +++ b/src/lib/types/authentication-required.ts @@ -2,6 +2,7 @@ interface IBaseOptions { type: string; path: string; message: string; + defaultHidden?: boolean; } interface IOptions extends IBaseOptions { @@ -15,13 +16,22 @@ class AuthenticationRequired { private message: string; + private defaultHidden: boolean; + private options?: IBaseOptions[]; - constructor({ type, path, message, options }: IOptions) { + constructor({ + type, + path, + message, + options, + defaultHidden = false, + }: IOptions) { this.type = type; this.path = path; this.message = message; this.options = options; + this.defaultHidden = defaultHidden; } } diff --git a/src/lib/types/settings/simple-auth-settings.ts b/src/lib/types/settings/simple-auth-settings.ts new file mode 100644 index 0000000000..eb63658910 --- /dev/null +++ b/src/lib/types/settings/simple-auth-settings.ts @@ -0,0 +1,4 @@ +export const simpleAuthKey = 'unleash.auth.simple'; +export interface SimpleAuthSettings { + disabled: boolean; +} diff --git a/src/lib/types/stores/settings-store.ts b/src/lib/types/stores/settings-store.ts index ef9b396dd7..edb8476edc 100644 --- a/src/lib/types/stores/settings-store.ts +++ b/src/lib/types/stores/settings-store.ts @@ -6,6 +6,6 @@ export interface ISettingInsert { } export interface ISettingStore extends Store { - insert(name: string, content: any): Promise; + insert(name: string, content: T): Promise; updateRow(name: string, content: any): Promise; } diff --git a/src/test/e2e/api/admin/config.e2e.test.ts b/src/test/e2e/api/admin/config.e2e.test.ts new file mode 100644 index 0000000000..895953e70a --- /dev/null +++ b/src/test/e2e/api/admin/config.e2e.test.ts @@ -0,0 +1,37 @@ +import dbInit, { ITestDb } from '../../helpers/database-init'; +import { setupApp } from '../../helpers/test-helper'; +import getLogger from '../../../fixtures/no-logger'; +import { simpleAuthKey } from '../../../../lib/types/settings/simple-auth-settings'; + +let db: ITestDb; +let app; + +beforeAll(async () => { + db = await dbInit('config_api_serial', getLogger); + app = await setupApp(db.stores); +}); + +afterAll(async () => { + await app.destroy(); + await db.destroy(); +}); + +test('gets ui config', async () => { + const { body } = await app.request + .get('/api/admin/ui-config') + .expect('Content-Type', /json/) + .expect(200); + + expect(body.unleashUrl).toBe('http://localhost:4242'); + expect(body.version).toBeDefined(); +}); + +test('gets ui config with disablePasswordAuth', async () => { + await db.stores.settingStore.insert(simpleAuthKey, { disabled: true }); + const { body } = await app.request + .get('/api/admin/ui-config') + .expect('Content-Type', /json/) + .expect(200); + + expect(body.disablePasswordAuth).toBe(true); +}); diff --git a/src/test/e2e/api/auth/reset-password-controller.e2e.test.ts b/src/test/e2e/api/auth/reset-password-controller.e2e.test.ts index 6c09ef8b03..57889f3b3c 100644 --- a/src/test/e2e/api/auth/reset-password-controller.e2e.test.ts +++ b/src/test/e2e/api/auth/reset-password-controller.e2e.test.ts @@ -13,6 +13,8 @@ import { EmailService } from '../../../../lib/services/email-service'; import SessionStore from '../../../../lib/db/session-store'; import SessionService from '../../../../lib/services/session-service'; import { RoleName } from '../../../../lib/types/model'; +import SettingService from '../../../../lib/services/setting-service'; +import FakeSettingStore from '../../../fixtures/fake-setting-store'; let app; let stores; @@ -53,11 +55,16 @@ beforeAll(async () => { config.getLogger, ); const sessionService = new SessionService({ sessionStore }, config); + const settingService = new SettingService( + { settingStore: new FakeSettingStore() }, + config, + ); userService = new UserService(stores, config, { accessService, resetTokenService, emailService, sessionService, + settingService, }); resetTokenService = new ResetTokenService(stores, config); const adminRole = await accessService.getRootRole(RoleName.ADMIN); diff --git a/src/test/e2e/services/reset-token-service.e2e.test.ts b/src/test/e2e/services/reset-token-service.e2e.test.ts index 4b9c173352..5631a834e8 100644 --- a/src/test/e2e/services/reset-token-service.e2e.test.ts +++ b/src/test/e2e/services/reset-token-service.e2e.test.ts @@ -9,6 +9,8 @@ import { createTestConfig } from '../../config/test-config'; import SessionService from '../../../lib/services/session-service'; import InvalidTokenError from '../../../lib/error/invalid-token-error'; import { IUser } from '../../../lib/types/user'; +import SettingService from '../../../lib/services/setting-service'; +import FakeSettingStore from '../../fixtures/fake-setting-store'; const config: IUnleashConfig = createTestConfig(); @@ -28,12 +30,17 @@ beforeAll(async () => { resetTokenService = new ResetTokenService(stores, config); sessionService = new SessionService(stores, config); const emailService = new EmailService(undefined, config.getLogger); + const settingService = new SettingService( + { settingStore: new FakeSettingStore() }, + config, + ); userService = new UserService(stores, config, { accessService, resetTokenService, emailService, sessionService, + settingService, }); adminUser = await userService.createUser({ diff --git a/src/test/e2e/services/user-service.e2e.test.ts b/src/test/e2e/services/user-service.e2e.test.ts index 3dd1dcda42..0934ef699b 100644 --- a/src/test/e2e/services/user-service.e2e.test.ts +++ b/src/test/e2e/services/user-service.e2e.test.ts @@ -10,6 +10,8 @@ import SessionService from '../../../lib/services/session-service'; import NotFoundError from '../../../lib/error/notfound-error'; import { IRole } from '../../../lib/types/stores/access-store'; import { RoleName } from '../../../lib/types/model'; +import SettingService from '../../../lib/services/setting-service'; +import { simpleAuthKey } from '../../../lib/types/settings/simple-auth-settings'; let db; let stores; @@ -27,12 +29,14 @@ beforeAll(async () => { const resetTokenService = new ResetTokenService(stores, config); const emailService = new EmailService(undefined, config.getLogger); sessionService = new SessionService(stores, config); + const settingService = new SettingService(stores, config); userService = new UserService(stores, config, { accessService, resetTokenService, emailService, sessionService, + settingService, }); userStore = stores.userStore; const rootRoles = await accessService.getRootRoles(); @@ -93,6 +97,22 @@ test('should create user with password', async () => { expect(user.username).toBe('test'); }); +test('should not login user if simple auth is disabled', async () => { + await db.stores.settingStore.insert(simpleAuthKey, { disabled: true }); + + await userService.createUser({ + username: 'test_no_pass', + password: 'A very strange P4ssw0rd_', + rootRole: adminRole.id, + }); + + await expect(async () => { + await userService.loginUser('test_no_pass', 'A very strange P4ssw0rd_'); + }).rejects.toThrowError( + 'Logging in with username/password has been disabled.', + ); +}); + test('should login for user _without_ password', async () => { const email = 'some@test.com'; await userService.createUser({ diff --git a/src/test/fixtures/fake-setting-store.ts b/src/test/fixtures/fake-setting-store.ts index fb0f0d9d9d..4fd87e1f98 100644 --- a/src/test/fixtures/fake-setting-store.ts +++ b/src/test/fixtures/fake-setting-store.ts @@ -1,5 +1,4 @@ import { ISettingStore } from '../../lib/types/stores/settings-store'; -import NotFoundError from '../../lib/error/notfound-error'; export default class FakeSettingStore implements ISettingStore { settings: Map = new Map(); @@ -23,7 +22,7 @@ export default class FakeSettingStore implements ISettingStore { if (setting) { return setting; } - throw new NotFoundError(`Could not find setting with key ${key}`); + return undefined; } async getAll(): Promise {