From b0e6d8c363c35e5474d1170f87d49ba291734b25 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ivar=20Conradi=20=C3=98sthus?= Date: Thu, 22 Apr 2021 23:40:52 +0200 Subject: [PATCH] fix: User should require a ID field set (#799) --- src/lib/db/user-store.ts | 18 ++++--- .../middleware/api-token-middleware.test.ts | 8 ++- src/lib/middleware/api-token-middleware.ts | 10 ++-- src/lib/middleware/demo-authentication.js | 7 --- src/lib/middleware/no-authentication.js | 5 +- src/lib/middleware/rbac-middleware.test.ts | 14 +++--- src/lib/middleware/rbac-middleware.ts | 4 +- .../routes/admin-api/api-token-controller.ts | 7 +-- src/lib/routes/admin-api/user.test.js | 4 +- src/lib/routes/admin-api/user.ts | 2 +- .../auth/simple-password-provider.test.js | 6 +-- src/lib/routes/logout.test.js | 4 +- src/lib/routes/unleash-types.ts | 2 +- src/lib/server-impl.ts | 2 +- src/lib/services/access-service.ts | 3 +- src/lib/services/api-token-service.ts | 21 ++------ src/lib/services/project-service.ts | 2 +- src/lib/services/user-service.ts | 20 ++++---- src/lib/types/api-user.ts | 24 +++++++++ src/lib/types/core.ts | 2 +- src/lib/{user.test.js => types/user.test.ts} | 25 +++++----- src/lib/{ => types}/user.ts | 35 ++++++------- src/test/e2e/api/admin/user-admin.e2e.test.ts | 12 ++--- .../reset-password-controller.e2e.test.ts | 2 +- .../e2e/services/access-service.e2e.test.js | 45 +++++++++-------- .../e2e/services/project-service.e2e.test.js | 50 +++++++++++-------- .../services/reset-token-service.e2e.test.ts | 2 +- .../e2e/services/user-service.e2e.test.ts | 4 +- src/test/e2e/stores/user-store.e2e.test.js | 47 ++++------------- src/test/fixtures/access-service-mock.ts | 2 +- src/test/fixtures/fake-user-store.ts | 2 +- 31 files changed, 188 insertions(+), 203 deletions(-) create mode 100644 src/lib/types/api-user.ts rename src/lib/{user.test.js => types/user.test.ts} (73%) rename src/lib/{ => types}/user.ts (75%) diff --git a/src/lib/db/user-store.ts b/src/lib/db/user-store.ts index 4360a6d4b3..c2bc9327d0 100644 --- a/src/lib/db/user-store.ts +++ b/src/lib/db/user-store.ts @@ -2,7 +2,7 @@ import { Knex } from 'knex'; import { Logger, LogProvider } from '../logger'; -import User from '../user'; +import User from '../types/user'; const NotFoundError = require('../error/notfound-error'); @@ -14,7 +14,6 @@ const USER_COLUMNS = [ 'username', 'email', 'image_url', - 'permissions', // TODO: remove in v4 'login_attempts', 'seen_at', 'created_at', @@ -29,14 +28,20 @@ const emptify = value => { return value; }; -const mapUserToColumns = user => ({ +const mapUserToColumns = (user: ICreateUser) => ({ name: user.name, username: user.username, email: user.email, image_url: user.imageUrl, - permissions: user.permissions ? JSON.stringify(user.permissions) : null, }); +interface ICreateUser { + name?: string; + username?: string; + email?: string; + imageUrl?: string; +} + const rowToUser = row => { if (!row) { throw new NotFoundError('No user found'); @@ -48,7 +53,6 @@ const rowToUser = row => { email: emptify(row.email), imageUrl: emptify(row.image_url), loginAttempts: row.login_attempts, - permissions: row.permissions, seenAt: row.seen_at, createdAt: row.created_at, }); @@ -88,14 +92,14 @@ class UserStore { return this.get({ id }); } - async insert(user: User): Promise { + async insert(user: ICreateUser): Promise { const rows = await this.db(TABLE) .insert(mapUserToColumns(user)) .returning(USER_COLUMNS); return rowToUser(rows[0]); } - async upsert(user: User): Promise { + async upsert(user: ICreateUser): Promise { const id = await this.hasUser(user); if (id) { diff --git a/src/lib/middleware/api-token-middleware.test.ts b/src/lib/middleware/api-token-middleware.test.ts index 74370613ed..1ee74ac094 100644 --- a/src/lib/middleware/api-token-middleware.test.ts +++ b/src/lib/middleware/api-token-middleware.test.ts @@ -4,9 +4,9 @@ import sinon from 'sinon'; import apiTokenMiddleware from './api-token-middleware'; import getLogger from '../../test/fixtures/no-logger'; -import User from '../user'; import { CLIENT } from '../permissions'; import { createTestConfig } from '../../test/config/test-config'; +import ApiUser from '../types/api-user'; let config: any; @@ -60,8 +60,7 @@ test('should not add user if unknown token', async t => { }); test('should add user if unknown token', async t => { - const apiUser = new User({ - isAPI: true, + const apiUser = new ApiUser({ username: 'default', permissions: [CLIENT], }); @@ -86,8 +85,7 @@ test('should add user if unknown token', async t => { }); test('should not add user if disabled', async t => { - const apiUser = new User({ - isAPI: true, + const apiUser = new ApiUser({ username: 'default', permissions: [CLIENT], }); diff --git a/src/lib/middleware/api-token-middleware.ts b/src/lib/middleware/api-token-middleware.ts index e5a78ceb51..ef597eea06 100644 --- a/src/lib/middleware/api-token-middleware.ts +++ b/src/lib/middleware/api-token-middleware.ts @@ -16,15 +16,15 @@ const apiAccessMiddleware = ( } return (req, res, next) => { - if (req.user) { + if (req.apiUser) { return next(); } try { - const userToken = req.header('authorization'); - const user = apiTokenService.getUserForToken(userToken); - if (user) { - req.user = user; + const apiToken = req.header('authorization'); + const apiUser = apiTokenService.getUserForToken(apiToken); + if (apiUser) { + req.user = apiUser; } } catch (error) { logger.error(error); diff --git a/src/lib/middleware/demo-authentication.js b/src/lib/middleware/demo-authentication.js index 2683ac18ef..c2be33eb61 100644 --- a/src/lib/middleware/demo-authentication.js +++ b/src/lib/middleware/demo-authentication.js @@ -1,5 +1,3 @@ -const auth = require('basic-auth'); -const User = require('../user'); const AuthenticationRequired = require('../authentication-required'); function demoAuthentication(app, basePath = '', { userService }) { @@ -15,11 +13,6 @@ function demoAuthentication(app, basePath = '', { userService }) { app.use(`${basePath}/api/admin/`, (req, res, next) => { if (req.session.user && req.session.user.email) { req.user = req.session.user; - } else if (req.header('authorization')) { - const user = auth(req); - if (user && user.name) { - req.user = new User({ username: user.name }); - } } next(); }); diff --git a/src/lib/middleware/no-authentication.js b/src/lib/middleware/no-authentication.js index dd9347ff0d..a08b013101 100644 --- a/src/lib/middleware/no-authentication.js +++ b/src/lib/middleware/no-authentication.js @@ -1,14 +1,13 @@ 'use strict'; const { ADMIN } = require('../permissions'); -const User = require('../user'); +const ApiUser = require('../types/api-user'); function noneAuthentication(basePath = '', app) { app.use(`${basePath}/api/admin/`, (req, res, next) => { if (!req.user) { - req.user = new User({ + req.user = new ApiUser({ username: 'unknown', - isAPI: true, permissions: [ADMIN], }); } diff --git a/src/lib/middleware/rbac-middleware.test.ts b/src/lib/middleware/rbac-middleware.test.ts index 31947fa5bc..21a0419f62 100644 --- a/src/lib/middleware/rbac-middleware.test.ts +++ b/src/lib/middleware/rbac-middleware.test.ts @@ -4,10 +4,11 @@ import sinon from 'sinon'; import rbacMiddleware from './rbac-middleware'; import ffStore from '../../test/fixtures/fake-feature-toggle-store'; -import User from '../user'; +import User from '../types/user'; import perms from '../permissions'; import { IUnleashConfig } from '../types/option'; import { createTestConfig } from '../../test/config/test-config'; +import ApiUser from '../types/api-user'; let config: IUnleashConfig; let featureToggleStore: any; @@ -43,10 +44,9 @@ test('should give api-user ADMIN permission', async t => { const cb = sinon.fake(); const req: any = { - user: new User({ + user: new ApiUser({ username: 'api', permissions: [perms.ADMIN], - isAPI: true, }), }; @@ -66,10 +66,9 @@ test('should not give api-user ADMIN permission', async t => { const cb = sinon.fake(); const req: any = { - user: new User({ + user: new ApiUser({ username: 'api', permissions: [perms.CLIENT], - isAPI: true, }), }; @@ -90,10 +89,9 @@ test('should not allow user to miss userId', async t => { const cb = sinon.fake(); const req: any = { - user: new User({ + user: { username: 'user', - permissions: [perms.ADMIN], - }), + }, }; func(req, undefined, cb); diff --git a/src/lib/middleware/rbac-middleware.ts b/src/lib/middleware/rbac-middleware.ts index a0a8eb536b..7ec8df7d6c 100644 --- a/src/lib/middleware/rbac-middleware.ts +++ b/src/lib/middleware/rbac-middleware.ts @@ -5,9 +5,10 @@ import { DELETE_FEATURE, ADMIN, } from '../permissions'; +import ApiUser from '../types/api-user'; import { IUnleashConfig } from '../types/option'; import { IUnleashStores } from '../types/stores'; -import User from '../user'; +import User from '../types/user'; interface PermissionChecker { hasPermission( @@ -34,7 +35,6 @@ const rbacMiddleware = ( return false; } - // Support ADMIN API tokens for enterpriseAuthentication. if (user.isAPI) { return user.permissions.includes(ADMIN); } diff --git a/src/lib/routes/admin-api/api-token-controller.ts b/src/lib/routes/admin-api/api-token-controller.ts index 8bb66b4e36..a522be9dc3 100644 --- a/src/lib/routes/admin-api/api-token-controller.ts +++ b/src/lib/routes/admin-api/api-token-controller.ts @@ -12,8 +12,9 @@ import { Logger } from '../../logger'; import { ApiTokenType } from '../../db/api-token-store'; import { AccessService } from '../../services/access-service'; import { IAuthRequest } from '../unleash-types'; -import User from '../../user'; +import User from '../../types/user'; import { IUnleashConfig } from '../../types/option'; +import ApiUser from '../../types/api-user'; interface IServices { apiTokenService: ApiTokenService; @@ -39,8 +40,8 @@ class ApiTokenController extends Controller { this.delete('/:token', this.deleteApiToken, DELETE_API_TOKEN); } - private async isTokenAdmin(user: User) { - if (user.isAPI) { + private async isTokenAdmin(user: User | ApiUser) { + if (user instanceof ApiUser) { return user.permissions.includes(ADMIN); } diff --git a/src/lib/routes/admin-api/user.test.js b/src/lib/routes/admin-api/user.test.js index 90214832b8..566c9791d9 100644 --- a/src/lib/routes/admin-api/user.test.js +++ b/src/lib/routes/admin-api/user.test.js @@ -8,11 +8,11 @@ const supertest = require('supertest'); const { EventEmitter } = require('events'); const store = require('../../../test/fixtures/store'); const getApp = require('../../app'); -const User = require('../../user'); +const User = require('../../types/user'); const eventBus = new EventEmitter(); -const currentUser = new User({ email: 'test@mail.com' }); +const currentUser = new User({ id: 1337, email: 'test@mail.com' }); function getSetup() { const base = `/random${Math.round(Math.random() * 1000)}`; diff --git a/src/lib/routes/admin-api/user.ts b/src/lib/routes/admin-api/user.ts index 3d04a375e3..8cfbe40502 100644 --- a/src/lib/routes/admin-api/user.ts +++ b/src/lib/routes/admin-api/user.ts @@ -7,7 +7,7 @@ import { AccessService } from '../../services/access-service'; import { IUnleashConfig } from '../../types/option'; import { IUnleashServices } from '../../types/services'; import UserService from '../../services/user-service'; -import User from '../../user'; +import User from '../../types/user'; import { Logger } from '../../logger'; import { handleErrors } from './util'; diff --git a/src/lib/routes/auth/simple-password-provider.test.js b/src/lib/routes/auth/simple-password-provider.test.js index 299ec676de..b7b6f2551c 100644 --- a/src/lib/routes/auth/simple-password-provider.test.js +++ b/src/lib/routes/auth/simple-password-provider.test.js @@ -1,7 +1,7 @@ const test = require('ava'); const request = require('supertest'); const express = require('express'); -const User = require('../../user'); +const User = require('../../types/user'); const PasswordProvider = require('./simple-password-provider'); const getLogger = () => ({ info: () => {}, error: () => {} }); @@ -24,7 +24,7 @@ test('Should require password', async t => { test('Should login user', async t => { const username = 'ola'; const password = 'simplepass'; - const user = new User({ username, permissions: ['ADMIN'] }); + const user = new User({ id: 123, username }); const app = express(); app.use(express.json()); @@ -55,7 +55,7 @@ test('Should login user', async t => { test('Should not login user with wrong password', async t => { const username = 'ola'; const password = 'simplepass'; - const user = new User({ username, permissions: ['ADMIN'] }); + const user = new User({ id: 133, username }); const app = express(); app.use(express.json()); diff --git a/src/lib/routes/logout.test.js b/src/lib/routes/logout.test.js index e62fec3d2f..ba031f5ea7 100644 --- a/src/lib/routes/logout.test.js +++ b/src/lib/routes/logout.test.js @@ -8,11 +8,11 @@ const supertest = require('supertest'); const { EventEmitter } = require('events'); const store = require('../../test/fixtures/store'); const getApp = require('../app'); -const User = require('../user'); +const User = require('../types/user'); const eventBus = new EventEmitter(); -const currentUser = new User({ email: 'test@mail.com' }); +const currentUser = new User({ id: 1337, email: 'test@mail.com' }); function getSetup() { const base = `/random${Math.round(Math.random() * 1000)}`; diff --git a/src/lib/routes/unleash-types.ts b/src/lib/routes/unleash-types.ts index 2b9c5e1d5c..2362110d1f 100644 --- a/src/lib/routes/unleash-types.ts +++ b/src/lib/routes/unleash-types.ts @@ -1,5 +1,5 @@ import { Request } from 'express'; -import User from '../user'; +import User from '../types/user'; export interface IAuthRequest extends Request { user: User; diff --git a/src/lib/server-impl.ts b/src/lib/server-impl.ts index 0d21e3b863..4a46bc6475 100644 --- a/src/lib/server-impl.ts +++ b/src/lib/server-impl.ts @@ -10,7 +10,7 @@ import { createMetricsMonitor } from './metrics'; import { createStores } from './db'; import { createServices } from './services'; import { createConfig } from './create-config'; -import User from './user'; +import User from './types/user'; import permissions from './permissions'; import AuthenticationRequired from './authentication-required'; diff --git a/src/lib/services/access-service.ts b/src/lib/services/access-service.ts index 53041c877f..f76a5863fb 100644 --- a/src/lib/services/access-service.ts +++ b/src/lib/services/access-service.ts @@ -5,7 +5,7 @@ import { IUserRole, } from '../db/access-store'; import permissions from '../permissions'; -import User from '../user'; +import User from '../types/user'; export const ALL_PROJECTS = '*'; @@ -65,6 +65,7 @@ enum PermissionType { } export enum RoleName { + // eslint-disable-next-line @typescript-eslint/no-shadow ADMIN = 'Admin', EDITOR = 'Editor', VIEWER = 'Viewer', diff --git a/src/lib/services/api-token-service.ts b/src/lib/services/api-token-service.ts index 2e64d80da0..0fbd2108e7 100644 --- a/src/lib/services/api-token-service.ts +++ b/src/lib/services/api-token-service.ts @@ -1,23 +1,13 @@ import crypto from 'crypto'; import { ApiTokenStore, IApiToken, ApiTokenType } from '../db/api-token-store'; -import { Logger, LogProvider } from '../logger'; +import { Logger } from '../logger'; import { ADMIN, CLIENT } from '../permissions'; -import User from '../user'; import { IUnleashStores } from '../types/stores'; import { IUnleashConfig } from '../types/option'; +import ApiUser from '../types/api-user'; const ONE_MINUTE = 60_000; -interface IStores { - apiTokenStore: ApiTokenStore; - settingStore: any; -} - -interface IConfig { - getLogger: LogProvider; - baseUriPath: string; -} - interface CreateTokenRequest { username: string; type: ApiTokenType; @@ -63,14 +53,13 @@ export class ApiTokenService { return this.store.getAllActive(); } - public getUserForToken(secret: string): User | undefined { + public getUserForToken(secret: string): ApiUser | undefined { const token = this.activeTokens.find(t => t.secret === secret); if (token) { const permissions = token.type === ApiTokenType.ADMIN ? [ADMIN] : [CLIENT]; - return new User({ - isAPI: true, + return new ApiUser({ username: token.username, permissions, }); @@ -101,7 +90,7 @@ export class ApiTokenService { return crypto.randomBytes(32).toString('hex'); } - destroy() { + destroy(): void { clearInterval(this.timer); this.timer = null; } diff --git a/src/lib/services/project-service.ts b/src/lib/services/project-service.ts index 5ef3c601c2..5a895a5440 100644 --- a/src/lib/services/project-service.ts +++ b/src/lib/services/project-service.ts @@ -1,4 +1,4 @@ -import User from '../user'; +import User from '../types/user'; import { AccessService, IUserWithRole, RoleName } from './access-service'; import ProjectStore, { IProject } from '../db/project-store'; import EventStore from '../db/event-store'; diff --git a/src/lib/services/user-service.ts b/src/lib/services/user-service.ts index b7b4f9520c..3747ece605 100644 --- a/src/lib/services/user-service.ts +++ b/src/lib/services/user-service.ts @@ -6,7 +6,7 @@ import Joi from 'joi'; import { URL } from 'url'; import UserStore, { IUserSearch } from '../db/user-store'; import { Logger } from '../logger'; -import User, { IUser } from '../user'; +import User, { IUser } from '../types/user'; import isEmail from '../util/is-email'; import { AccessService, RoleName } from './access-service'; import { ADMIN } from '../permissions'; @@ -110,12 +110,9 @@ class UserService { this.logger.info( 'Creating default user "admin" with password "admin"', ); - const user = await this.store.insert( - new User({ - username: 'admin', - permissions: [ADMIN], // TODO: remove in v4 - }), - ); + const user = await this.store.insert({ + username: 'admin', + }); const passwordHash = await bcrypt.hash('admin', saltRounds); await this.store.setPasswordHash(user.id, passwordHash); @@ -180,10 +177,11 @@ class UserService { throw new Error('User already exists'); } - const user = await this.store.insert( - // TODO: remove permission in v4. - new User({ username, email, name, permissions: [ADMIN] }), - ); + const user = await this.store.insert({ + username, + email, + name, + }); await this.accessService.setUserRootRole(user.id, rootRole); diff --git a/src/lib/types/api-user.ts b/src/lib/types/api-user.ts new file mode 100644 index 0000000000..4957cb30ee --- /dev/null +++ b/src/lib/types/api-user.ts @@ -0,0 +1,24 @@ +import { CLIENT } from '../permissions'; + +interface IApiUserData { + username: string; + permissions?: string[]; +} + +export default class ApiUser { + isAPI: boolean = true; + + username: string; + + permissions: string[]; + + constructor({ username, permissions = [CLIENT] }: IApiUserData) { + if (!username) { + throw new TypeError('username is required'); + } + this.username = username; + this.permissions = permissions; + } +} + +module.exports = ApiUser; diff --git a/src/lib/types/core.ts b/src/lib/types/core.ts index 3c20124169..bd7aa83b63 100644 --- a/src/lib/types/core.ts +++ b/src/lib/types/core.ts @@ -2,7 +2,7 @@ import { Request } from 'express'; import EventEmitter from 'events'; import * as https from 'https'; import * as http from 'http'; -import User from '../user'; +import User from './user'; import { IUnleashConfig } from './option'; import { IUnleashStores } from './stores'; import { IUnleashServices } from './services'; diff --git a/src/lib/user.test.js b/src/lib/types/user.test.ts similarity index 73% rename from src/lib/user.test.js rename to src/lib/types/user.test.ts index 104cc3434a..9602a52bb2 100644 --- a/src/lib/user.test.js +++ b/src/lib/types/user.test.ts @@ -1,10 +1,8 @@ -'use strict'; - -const test = require('ava'); -const User = require('./user'); +import test from 'ava'; +import User from './user'; test('should create user', t => { - const user = new User({ name: 'ole', email: 'some@email.com' }); + const user = new User({ id: 11, name: 'ole', email: 'some@email.com' }); t.is(user.name, 'ole'); t.is(user.email, 'some@email.com'); t.is( @@ -15,15 +13,14 @@ test('should create user', t => { test('should create user, all fields', t => { const user = new User({ + id: 11, name: 'Admin', username: 'admin', email: 'some@email.com', - permissions: ['admin', 'client'], }); t.is(user.name, 'Admin'); t.is(user.username, 'admin'); t.is(user.email, 'some@email.com'); - t.deepEqual(user.permissions, ['admin', 'client']); t.is( user.imageUrl, 'https://gravatar.com/avatar/d8ffeba65ee5baf57e4901690edc8e1b?size=42&default=retro', @@ -33,7 +30,7 @@ test('should create user, all fields', t => { test('should require email or username', t => { const error = t.throws( () => { - const user = new User(); // eslint-disable-line + const user = new User({ id: 11 }); // eslint-disable-line }, { instanceOf: Error }, ); @@ -42,7 +39,7 @@ test('should require email or username', t => { }); test('Should create user with only email defined', t => { - const user = new User({ email: 'some@email.com' }); + const user = new User({ id: 123, email: 'some@email.com' }); t.is(user.email, 'some@email.com'); }); @@ -50,7 +47,7 @@ test('Should create user with only email defined', t => { test('Should require valid email', t => { const error = t.throws( () => { - new User({ email: 'some@' }); // eslint-disable-line + new User({ id: 11, email: 'some@' }); // eslint-disable-line }, { instanceOf: Error }, ); @@ -59,7 +56,7 @@ test('Should require valid email', t => { }); test('Should create user with only username defined', t => { - const user = new User({ username: 'some-user' }); + const user = new User({ id: 133, username: 'some-user' }); t.is(user.username, 'some-user'); t.is( user.imageUrl, @@ -68,6 +65,10 @@ test('Should create user with only username defined', t => { }); test('Should create user with only username defined and undefined email', t => { - const user = new User({ username: 'some-user', email: undefined }); + const user = new User({ + id: 1447, + username: 'some-user', + email: undefined, + }); t.is(user.username, 'some-user'); }); diff --git a/src/lib/user.ts b/src/lib/types/user.ts similarity index 75% rename from src/lib/user.ts rename to src/lib/types/user.ts index c04e08246e..a32887b0c8 100644 --- a/src/lib/user.ts +++ b/src/lib/types/user.ts @@ -2,12 +2,10 @@ import gravatarUrl from 'gravatar-url'; import Joi from 'joi'; export interface UserData { - id?: number; - isAPI?: boolean; + id: number; name?: string; username?: string; email?: string; - permissions?: string[]; imageUrl?: string; seenAt?: Date; loginAttempts?: number; @@ -25,8 +23,6 @@ export interface IUser { export default class User implements IUser { id: number; - isAPI: boolean; - name: string; username: string; @@ -43,20 +39,19 @@ export default class User implements IUser { createdAt: Date; - constructor( - { - id, - isAPI, - name, - email, - username, - imageUrl, - permissions, - seenAt, - loginAttempts, - createdAt, - }: UserData = { isAPI: false }, - ) { + constructor({ + id, + name, + email, + username, + imageUrl, + seenAt, + loginAttempts, + createdAt, + }: UserData) { + if (!id) { + throw new TypeError('Id is required'); + } if (!username && !email) { throw new TypeError('Username or Email is required'); } @@ -65,11 +60,9 @@ export default class User implements IUser { Joi.assert(name, Joi.string(), 'Name'); this.id = id; - this.isAPI = isAPI; this.name = name; this.username = username; this.email = email; - this.permissions = permissions; this.imageUrl = imageUrl || this.generateImageUrl(); this.seenAt = seenAt; this.loginAttempts = loginAttempts; diff --git a/src/test/e2e/api/admin/user-admin.e2e.test.ts b/src/test/e2e/api/admin/user-admin.e2e.test.ts index 1edcd03c09..6d97852e14 100644 --- a/src/test/e2e/api/admin/user-admin.e2e.test.ts +++ b/src/test/e2e/api/admin/user-admin.e2e.test.ts @@ -2,7 +2,7 @@ import test from 'ava'; import { setupApp } from '../../helpers/test-helper'; import dbInit from '../../helpers/database-init'; import getLogger from '../../../fixtures/no-logger'; -import User from '../../../../lib/user'; +import User from '../../../../lib/types/user'; import UserStore from '../../../../lib/db/user-store'; import { AccessStore, IRole } from '../../../../lib/db/access-store'; import { RoleName } from '../../../../lib/services/access-service'; @@ -164,7 +164,7 @@ test.serial('update user name', async t => { test.serial('should delete user', async t => { t.plan(0); - const user = await userStore.insert(new User({ email: 'some@mail.com' })); + const user = await userStore.insert({ email: 'some@mail.com' }); const request = await setupApp(stores); return request.delete(`/api/admin/user-admin/${user.id}`).expect(200); @@ -193,7 +193,7 @@ test.serial('validator should accept strong password', async t => { test.serial('should change password', async t => { t.plan(0); - const user = await userStore.insert(new User({ email: 'some@mail.com' })); + const user = await userStore.insert({ email: 'some@mail.com' }); const request = await setupApp(stores); return request @@ -205,9 +205,9 @@ test.serial('should change password', async t => { test.serial('should search for users', async t => { t.plan(2); - await userStore.insert(new User({ email: 'some@mail.com' })); - await userStore.insert(new User({ email: 'another@mail.com' })); - await userStore.insert(new User({ email: 'another2@mail.com' })); + await userStore.insert({ email: 'some@mail.com' }); + await userStore.insert({ email: 'another@mail.com' }); + await userStore.insert({ email: 'another2@mail.com' }); const request = await setupApp(stores); return request 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 24a2d9b076..af91f3e327 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 @@ -11,7 +11,7 @@ import ResetTokenService from '../../../../lib/services/reset-token-service'; import UserService from '../../../../lib/services/user-service'; import { setupApp } from '../../helpers/test-helper'; import { EmailService } from '../../../../lib/services/email-service'; -import User from '../../../../lib/user'; +import User from '../../../../lib/types/user'; import { IUnleashConfig } from '../../../../lib/types/option'; import { createTestConfig } from '../../../config/test-config'; diff --git a/src/test/e2e/services/access-service.e2e.test.js b/src/test/e2e/services/access-service.e2e.test.js index 6c3689b905..16a8593db0 100644 --- a/src/test/e2e/services/access-service.e2e.test.js +++ b/src/test/e2e/services/access-service.e2e.test.js @@ -9,7 +9,6 @@ const { ALL_PROJECTS, } = require('../../../lib/services/access-service'); const permissions = require('../../../lib/permissions'); -const User = require('../../../lib/user'); let db; let stores; @@ -23,16 +22,17 @@ let readRole; const createUserEditorAccess = async (name, email) => { const { userStore } = stores; - const user = await userStore.insert(new User({ name, email })); + const user = await userStore.insert({ name, email }); await accessService.addUserToRole(user.id, editorRole.id); return user; }; const createSuperUser = async () => { const { userStore } = stores; - const user = await userStore.insert( - new User({ name: 'Alice Admin', email: 'admin@getunleash.io' }), - ); + const user = await userStore.insert({ + name: 'Alice Admin', + email: 'admin@getunleash.io', + }); await accessService.addUserToRole(user.id, adminRole.id); return user; }; @@ -293,9 +293,10 @@ test.serial('should not get access if not specifying project', async t => { test.serial('should remove user from role', async t => { const { userStore } = stores; - const user = await userStore.insert( - new User({ name: 'Some User', email: 'random123@getunleash.io' }), - ); + const user = await userStore.insert({ + name: 'Some User', + email: 'random123@getunleash.io', + }); await accessService.addUserToRole(user.id, editorRole.id); @@ -311,9 +312,10 @@ test.serial('should remove user from role', async t => { test.serial('should return role with users', async t => { const { userStore } = stores; - const user = await userStore.insert( - new User({ name: 'Some User', email: 'random2223@getunleash.io' }), - ); + const user = await userStore.insert({ + name: 'Some User', + email: 'random2223@getunleash.io', + }); await accessService.addUserToRole(user.id, editorRole.id); @@ -327,9 +329,10 @@ test.serial('should return role with users', async t => { test.serial('should return role with permissions and users', async t => { const { userStore } = stores; - const user = await userStore.insert( - new User({ name: 'Some User', email: 'random2244@getunleash.io' }), - ); + const user = await userStore.insert({ + name: 'Some User', + email: 'random2244@getunleash.io', + }); await accessService.addUserToRole(user.id, editorRole.id); @@ -368,9 +371,10 @@ test.serial('should return list of permissions', async t => { test.serial('should set root role for user', async t => { const { userStore } = stores; - const user = await userStore.insert( - new User({ name: 'Some User', email: 'random2255@getunleash.io' }), - ); + const user = await userStore.insert({ + name: 'Some User', + email: 'random2255@getunleash.io', + }); await accessService.setUserRootRole(user.id, editorRole.id); @@ -382,9 +386,10 @@ test.serial('should set root role for user', async t => { test.serial('should switch root role for user', async t => { const { userStore } = stores; - const user = await userStore.insert( - new User({ name: 'Some User', email: 'random22Read@getunleash.io' }), - ); + const user = await userStore.insert({ + name: 'Some User', + email: 'random22Read@getunleash.io', + }); await accessService.setUserRootRole(user.id, editorRole.id); await accessService.setUserRootRole(user.id, readRole.id); diff --git a/src/test/e2e/services/project-service.e2e.test.js b/src/test/e2e/services/project-service.e2e.test.js index cee0eb17de..1de056cecc 100644 --- a/src/test/e2e/services/project-service.e2e.test.js +++ b/src/test/e2e/services/project-service.e2e.test.js @@ -6,7 +6,6 @@ const { AccessService, RoleName, } = require('../../../lib/services/access-service'); -const User = require('../../../lib/user'); const { UPDATE_PROJECT } = require('../../../lib/permissions'); const NotFoundError = require('../../../lib/error/notfound-error'); @@ -20,9 +19,10 @@ let user; test.before(async () => { db = await dbInit('project_service_serial', getLogger); stores = db.stores; - user = await stores.userStore.insert( - new User({ name: 'Some Name', email: 'test@getunleash.io' }), - ); + user = await stores.userStore.insert({ + name: 'Some Name', + email: 'test@getunleash.io', + }); const config = { getLogger, experimental: { rbac: true } }; accessService = new AccessService(stores, config); projectService = new ProjectService(stores, config, accessService); @@ -240,12 +240,14 @@ test.serial('should add a member user to the project', async t => { }; await projectService.createProject(project, user); - const projectMember1 = await stores.userStore.insert( - new User({ name: 'Some Member', email: 'member1@getunleash.io' }), - ); - const projectMember2 = await stores.userStore.insert( - new User({ name: 'Some Member 2', email: 'member2@getunleash.io' }), - ); + const projectMember1 = await stores.userStore.insert({ + name: 'Some Member', + email: 'member1@getunleash.io', + }); + const projectMember2 = await stores.userStore.insert({ + name: 'Some Member 2', + email: 'member2@getunleash.io', + }); const roles = await stores.accessStore.getRolesForProject(project.id); const memberRole = roles.find(r => r.name === RoleName.MEMBER); @@ -271,12 +273,14 @@ test.serial('should add admin users to the project', async t => { }; await projectService.createProject(project, user); - const projectAdmin1 = await stores.userStore.insert( - new User({ name: 'Some Member', email: 'admin1@getunleash.io' }), - ); - const projectAdmin2 = await stores.userStore.insert( - new User({ name: 'Some Member 2', email: 'admin2@getunleash.io' }), - ); + const projectAdmin1 = await stores.userStore.insert({ + name: 'Some Member', + email: 'admin1@getunleash.io', + }); + const projectAdmin2 = await stores.userStore.insert({ + name: 'Some Member 2', + email: 'admin2@getunleash.io', + }); const projectRoles = await stores.accessStore.getRolesForProject( project.id, @@ -320,9 +324,10 @@ test.serial('add user should fail if user already have access', async t => { }; await projectService.createProject(project, user); - const projectMember1 = await stores.userStore.insert( - new User({ name: 'Some Member', email: 'member42@getunleash.io' }), - ); + const projectMember1 = await stores.userStore.insert({ + name: 'Some Member', + email: 'member42@getunleash.io', + }); const roles = await stores.accessStore.getRolesForProject(project.id); const memberRole = roles.find(r => r.name === RoleName.MEMBER); @@ -352,9 +357,10 @@ test.serial('should remove user from the project', async t => { }; await projectService.createProject(project, user); - const projectMember1 = await stores.userStore.insert( - new User({ name: 'Some Member', email: 'member99@getunleash.io' }), - ); + const projectMember1 = await stores.userStore.insert({ + name: 'Some Member', + email: 'member99@getunleash.io', + }); const roles = await stores.accessStore.getRolesForProject(project.id); const memberRole = roles.find(r => r.name === RoleName.MEMBER); 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 46ba35753a..35f29b2835 100644 --- a/src/test/e2e/services/reset-token-service.e2e.test.ts +++ b/src/test/e2e/services/reset-token-service.e2e.test.ts @@ -6,7 +6,7 @@ import UserService from '../../../lib/services/user-service'; import { AccessService } from '../../../lib/services/access-service'; import NotFoundError from '../../../lib/error/notfound-error'; import { EmailService } from '../../../lib/services/email-service'; -import User from '../../../lib/user'; +import User from '../../../lib/types/user'; import { IUnleashConfig } from '../../../lib/types/option'; import { createTestConfig } from '../../config/test-config'; diff --git a/src/test/e2e/services/user-service.e2e.test.ts b/src/test/e2e/services/user-service.e2e.test.ts index bd59dc1a82..95ddc78c67 100644 --- a/src/test/e2e/services/user-service.e2e.test.ts +++ b/src/test/e2e/services/user-service.e2e.test.ts @@ -4,7 +4,7 @@ import getLogger from '../../fixtures/no-logger'; import UserService from '../../../lib/services/user-service'; import { AccessService, RoleName } from '../../../lib/services/access-service'; import UserStore from '../../../lib/db/user-store'; -import User from '../../../lib/user'; +import User from '../../../lib/types/user'; import { IRole } from '../../../lib/db/access-store'; import ResetTokenService from '../../../lib/services/reset-token-service'; import { EmailService } from '../../../lib/services/email-service'; @@ -51,7 +51,7 @@ test.serial('should create initial admin user', async t => { }); test.serial('should not be allowed to create existing user', async t => { - await userStore.insert(new User({ username: 'test', name: 'Hans Mola' })); + await userStore.insert({ username: 'test', name: 'Hans Mola' }); await t.throwsAsync( userService.createUser({ username: 'test', rootRole: adminRole.id }), ); diff --git a/src/test/e2e/stores/user-store.e2e.test.js b/src/test/e2e/stores/user-store.e2e.test.js index 00f365e7e3..bb3b9a2292 100644 --- a/src/test/e2e/stores/user-store.e2e.test.js +++ b/src/test/e2e/stores/user-store.e2e.test.js @@ -1,12 +1,6 @@ 'use strict'; const test = require('ava'); -const User = require('../../../lib/user'); -const { - CREATE_FEATURE, - DELETE_FEATURE, - UPDATE_FEATURE, -} = require('../../../lib/permissions'); const NotFoundError = require('../../../lib/error/notfound-error'); const dbInit = require('../helpers/database-init'); const getLogger = require('../../fixtures/no-logger'); @@ -29,7 +23,7 @@ test.serial('should have no users', async t => { }); test.serial('should insert new user with email', async t => { - const user = new User({ email: 'me2@mail.com' }); + const user = { email: 'me2@mail.com' }; await stores.userStore.upsert(user); const users = await stores.userStore.getAll(); t.deepEqual(users[0].email, user.email); @@ -53,14 +47,14 @@ test.serial('should not allow two users with same email', async t => { }); test.serial('should insert new user with email and return it', async t => { - const user = new User({ email: 'me2@mail.com' }); + const user = { email: 'me2@mail.com' }; const newUser = await stores.userStore.upsert(user); t.deepEqual(newUser.email, user.email); t.truthy(newUser.id); }); test.serial('should insert new user with username', async t => { - const user = new User({ username: 'admin' }); + const user = { username: 'admin' }; await stores.userStore.upsert(user); const dbUser = await stores.userStore.get(user); t.deepEqual(dbUser.username, user.username); @@ -79,7 +73,7 @@ test('Should require email or username', async t => { test.serial('should set password_hash for user', async t => { const store = stores.userStore; - const user = await store.insert(new User({ email: 'admin@mail.com' })); + const user = await store.insert({ email: 'admin@mail.com' }); await store.setPasswordHash(user.id, 'rubbish'); const hash = await store.getPasswordHash(user.id); @@ -100,7 +94,7 @@ test.serial('should not get password_hash for unknown userId', async t => { test.serial('should update loginAttempts for user', async t => { const store = stores.userStore; - const user = new User({ email: 'admin@mail.com' }); + const user = { email: 'admin@mail.com' }; await store.upsert(user); await store.incLoginAttempts(user); await store.incLoginAttempts(user); @@ -111,9 +105,9 @@ test.serial('should update loginAttempts for user', async t => { test.serial('should not increment for user unknwn user', async t => { const store = stores.userStore; - const user = new User({ email: 'another@mail.com' }); + const user = { email: 'another@mail.com' }; await store.upsert(user); - await store.incLoginAttempts(new User({ email: 'unknown@mail.com' })); + await store.incLoginAttempts({ email: 'unknown@mail.com' }); const storedUser = await store.get(user); t.is(storedUser.loginAttempts, 0); @@ -121,9 +115,7 @@ test.serial('should not increment for user unknwn user', async t => { test.serial('should reset user after successful login', async t => { const store = stores.userStore; - const user = await store.insert( - new User({ email: 'anotherWithResert@mail.com' }), - ); + const user = await store.insert({ email: 'anotherWithResert@mail.com' }); await store.incLoginAttempts(user); await store.incLoginAttempts(user); @@ -134,37 +126,20 @@ test.serial('should reset user after successful login', async t => { t.true(storedUser.seenAt >= user.seenAt); }); -test.serial('should store and get permissions', async t => { - const store = stores.userStore; - const email = 'userWithPermissions@mail.com'; - const user = new User({ - email, - permissions: [CREATE_FEATURE, UPDATE_FEATURE, DELETE_FEATURE], - }); - - await store.upsert(user); - - const storedUser = await store.get({ email }); - - t.deepEqual(storedUser.permissions, user.permissions); -}); - test.serial('should only update specified fields on user', async t => { const store = stores.userStore; const email = 'userTobeUpdated@mail.com'; - const user = new User({ + const user = { email, username: 'test', - permissions: [CREATE_FEATURE, UPDATE_FEATURE, DELETE_FEATURE], - }); + }; await store.upsert(user); - await store.upsert({ username: 'test', permissions: [CREATE_FEATURE] }); + await store.upsert({ username: 'test' }); const storedUser = await store.get({ email }); t.deepEqual(storedUser.email, user.email); t.deepEqual(storedUser.username, user.username); - t.deepEqual(storedUser.permissions, [CREATE_FEATURE]); }); diff --git a/src/test/fixtures/access-service-mock.ts b/src/test/fixtures/access-service-mock.ts index 764632fddc..e77d32a813 100644 --- a/src/test/fixtures/access-service-mock.ts +++ b/src/test/fixtures/access-service-mock.ts @@ -8,7 +8,7 @@ import { IPermission, IRoleData, } from '../../lib/services/access-service'; -import User from '../../lib/user'; +import User from '../../lib/types/user'; import noLoggerProvider from './no-logger'; class AccessServiceMock extends AccessService { diff --git a/src/test/fixtures/fake-user-store.ts b/src/test/fixtures/fake-user-store.ts index 77abcaa347..911396d3e0 100644 --- a/src/test/fixtures/fake-user-store.ts +++ b/src/test/fixtures/fake-user-store.ts @@ -1,5 +1,5 @@ import UserStore, { IUserLookup } from '../../lib/db/user-store'; -import User from '../../lib/user'; +import User from '../../lib/types/user'; import noLoggerProvider from './no-logger'; class UserStoreMock extends UserStore {