feat: make role name unique and remove key usage

This commit is contained in:
Ali BARIN
2024-09-04 11:07:45 +00:00
parent b089069b8e
commit 63dfb6947e
33 changed files with 51 additions and 55 deletions

View File

@@ -10,7 +10,7 @@ import process from 'process';
async function fetchAdminRole() { async function fetchAdminRole() {
const role = await Role.query() const role = await Role.query()
.where({ .where({
key: 'admin', name: 'Admin',
}) })
.limit(1) .limit(1)
.first(); .first();

View File

@@ -15,7 +15,7 @@ describe('POST /api/v1/admin/apps/:appKey/auth-clients', () => {
beforeEach(async () => { beforeEach(async () => {
vi.spyOn(license, 'hasValidLicense').mockResolvedValue(true); vi.spyOn(license, 'hasValidLicense').mockResolvedValue(true);
adminRole = await createRole({ key: 'admin' }); adminRole = await createRole({ name: 'Admin' });
currentUser = await createUser({ roleId: adminRole.id }); currentUser = await createUser({ roleId: adminRole.id });
token = await createAuthTokenByUserId(currentUser.id); token = await createAuthTokenByUserId(currentUser.id);

View File

@@ -15,7 +15,7 @@ describe('POST /api/v1/admin/apps/:appKey/config', () => {
beforeEach(async () => { beforeEach(async () => {
vi.spyOn(license, 'hasValidLicense').mockResolvedValue(true); vi.spyOn(license, 'hasValidLicense').mockResolvedValue(true);
adminRole = await createRole({ key: 'admin' }); adminRole = await createRole({ name: 'Admin' });
currentUser = await createUser({ roleId: adminRole.id }); currentUser = await createUser({ roleId: adminRole.id });
token = await createAuthTokenByUserId(currentUser.id); token = await createAuthTokenByUserId(currentUser.id);

View File

@@ -15,7 +15,7 @@ describe('GET /api/v1/admin/apps/:appKey/auth-clients/:appAuthClientId', () => {
beforeEach(async () => { beforeEach(async () => {
vi.spyOn(license, 'hasValidLicense').mockResolvedValue(true); vi.spyOn(license, 'hasValidLicense').mockResolvedValue(true);
adminRole = await createRole({ key: 'admin' }); adminRole = await createRole({ name: 'Admin' });
currentUser = await createUser({ roleId: adminRole.id }); currentUser = await createUser({ roleId: adminRole.id });
currentAppAuthClient = await createAppAuthClient({ currentAppAuthClient = await createAppAuthClient({

View File

@@ -14,7 +14,7 @@ describe('GET /api/v1/admin/apps/:appKey/auth-clients', () => {
beforeEach(async () => { beforeEach(async () => {
vi.spyOn(license, 'hasValidLicense').mockResolvedValue(true); vi.spyOn(license, 'hasValidLicense').mockResolvedValue(true);
adminRole = await createRole({ key: 'admin' }); adminRole = await createRole({ name: 'Admin' });
currentUser = await createUser({ roleId: adminRole.id }); currentUser = await createUser({ roleId: adminRole.id });
token = await createAuthTokenByUserId(currentUser.id); token = await createAuthTokenByUserId(currentUser.id);

View File

@@ -17,7 +17,7 @@ describe('PATCH /api/v1/admin/apps/:appKey/auth-clients', () => {
beforeEach(async () => { beforeEach(async () => {
vi.spyOn(license, 'hasValidLicense').mockResolvedValue(true); vi.spyOn(license, 'hasValidLicense').mockResolvedValue(true);
adminRole = await createRole({ key: 'admin' }); adminRole = await createRole({ name: 'Admin' });
currentUser = await createUser({ roleId: adminRole.id }); currentUser = await createUser({ roleId: adminRole.id });
token = await createAuthTokenByUserId(currentUser.id); token = await createAuthTokenByUserId(currentUser.id);

View File

@@ -15,7 +15,7 @@ describe('PATCH /api/v1/admin/apps/:appKey/config', () => {
beforeEach(async () => { beforeEach(async () => {
vi.spyOn(license, 'hasValidLicense').mockResolvedValue(true); vi.spyOn(license, 'hasValidLicense').mockResolvedValue(true);
adminRole = await createRole({ key: 'admin' }); adminRole = await createRole({ name: 'Admin' });
currentUser = await createUser({ roleId: adminRole.id }); currentUser = await createUser({ roleId: adminRole.id });
token = await createAuthTokenByUserId(currentUser.id); token = await createAuthTokenByUserId(currentUser.id);

View File

@@ -14,7 +14,7 @@ describe('PATCH /api/v1/admin/config', () => {
beforeEach(async () => { beforeEach(async () => {
vi.spyOn(license, 'hasValidLicense').mockResolvedValue(true); vi.spyOn(license, 'hasValidLicense').mockResolvedValue(true);
adminRole = await createRole({ key: 'admin' }); adminRole = await createRole({ name: 'Admin' });
currentUser = await createUser({ roleId: adminRole.id }); currentUser = await createUser({ roleId: adminRole.id });
token = await createAuthTokenByUserId(currentUser.id); token = await createAuthTokenByUserId(currentUser.id);

View File

@@ -11,7 +11,7 @@ describe('GET /api/v1/admin/permissions/catalog', () => {
let role, currentUser, token; let role, currentUser, token;
beforeEach(async () => { beforeEach(async () => {
role = await createRole({ key: 'admin' }); role = await createRole({ name: 'Admin' });
currentUser = await createUser({ roleId: role.id }); currentUser = await createUser({ roleId: role.id });
token = await createAuthTokenByUserId(currentUser.id); token = await createAuthTokenByUserId(currentUser.id);

View File

@@ -1,4 +1,3 @@
import kebabCase from 'lodash/kebabCase.js';
import { renderObject } from '../../../../../helpers/renderer.js'; import { renderObject } from '../../../../../helpers/renderer.js';
import Role from '../../../../../models/role.js'; import Role from '../../../../../models/role.js';
@@ -14,10 +13,8 @@ export default async (request, response) => {
const roleParams = (request) => { const roleParams = (request) => {
const { name, description, permissions } = request.body; const { name, description, permissions } = request.body;
const key = kebabCase(name);
return { return {
key,
name, name,
description, description,
permissions, permissions,

View File

@@ -14,7 +14,7 @@ describe('POST /api/v1/admin/roles', () => {
beforeEach(async () => { beforeEach(async () => {
vi.spyOn(license, 'hasValidLicense').mockResolvedValue(true); vi.spyOn(license, 'hasValidLicense').mockResolvedValue(true);
role = await createRole({ key: 'admin' }); role = await createRole({ name: 'Admin' });
currentUser = await createUser({ roleId: role.id }); currentUser = await createUser({ roleId: role.id });
token = await createAuthTokenByUserId(currentUser.id); token = await createAuthTokenByUserId(currentUser.id);
@@ -41,7 +41,7 @@ describe('POST /api/v1/admin/roles', () => {
const createdRole = await Role.query() const createdRole = await Role.query()
.withGraphFetched({ permissions: true }) .withGraphFetched({ permissions: true })
.findOne({ key: 'viewer' }) .findOne({ name: 'Viewer' })
.throwIfNotFound(); .throwIfNotFound();
const expectedPayload = await createRoleMock( const expectedPayload = await createRoleMock(
@@ -61,7 +61,7 @@ describe('POST /api/v1/admin/roles', () => {
expect(response.body).toEqual(expectedPayload); expect(response.body).toEqual(expectedPayload);
}); });
it('should return unprocessable entity response for invalid data', async () => { it('should return unprocessable entity response for invalid role data', async () => {
const roleData = { const roleData = {
description: '', description: '',
permissions: [], permissions: [],
@@ -76,7 +76,6 @@ describe('POST /api/v1/admin/roles', () => {
expect(response.body).toStrictEqual({ expect(response.body).toStrictEqual({
errors: { errors: {
name: ["must have required property 'name'"], name: ["must have required property 'name'"],
key: ['must NOT have fewer than 1 characters'],
}, },
meta: { meta: {
type: 'ModelValidation', type: 'ModelValidation',
@@ -85,8 +84,10 @@ describe('POST /api/v1/admin/roles', () => {
}); });
it('should return unprocessable entity response for duplicate role', async () => { it('should return unprocessable entity response for duplicate role', async () => {
await createRole({ name: 'Viewer' });
const roleData = { const roleData = {
name: 'admin', name: 'Viewer',
permissions: [], permissions: [],
}; };
@@ -98,7 +99,7 @@ describe('POST /api/v1/admin/roles', () => {
expect(response.body).toStrictEqual({ expect(response.body).toStrictEqual({
errors: { errors: {
key: ["'key' must be unique."], name: ["'name' must be unique."],
}, },
meta: { meta: {
type: 'UniqueViolationError', type: 'UniqueViolationError',

View File

@@ -13,7 +13,7 @@ describe('GET /api/v1/admin/roles/:roleId', () => {
let role, currentUser, token, permissionOne, permissionTwo; let role, currentUser, token, permissionOne, permissionTwo;
beforeEach(async () => { beforeEach(async () => {
role = await createRole({ key: 'admin' }); role = await createRole({ name: 'Admin' });
permissionOne = await createPermission({ roleId: role.id }); permissionOne = await createPermission({ roleId: role.id });
permissionTwo = await createPermission({ roleId: role.id }); permissionTwo = await createPermission({ roleId: role.id });
currentUser = await createUser({ roleId: role.id }); currentUser = await createUser({ roleId: role.id });

View File

@@ -11,8 +11,8 @@ describe('GET /api/v1/admin/roles', () => {
let roleOne, roleTwo, currentUser, token; let roleOne, roleTwo, currentUser, token;
beforeEach(async () => { beforeEach(async () => {
roleOne = await createRole({ key: 'admin' }); roleOne = await createRole({ name: 'Admin' });
roleTwo = await createRole({ key: 'user' }); roleTwo = await createRole({ name: 'User' });
currentUser = await createUser({ roleId: roleOne.id }); currentUser = await createUser({ roleId: roleOne.id });
token = await createAuthTokenByUserId(currentUser.id); token = await createAuthTokenByUserId(currentUser.id);

View File

@@ -13,7 +13,7 @@ describe('POST /api/v1/admin/saml-auth-provider', () => {
beforeEach(async () => { beforeEach(async () => {
vi.spyOn(license, 'hasValidLicense').mockResolvedValue(true); vi.spyOn(license, 'hasValidLicense').mockResolvedValue(true);
role = await createRole({ key: 'admin' }); role = await createRole({ name: 'Admin' });
currentUser = await createUser({ roleId: role.id }); currentUser = await createUser({ roleId: role.id });
token = await createAuthTokenByUserId(currentUser.id); token = await createAuthTokenByUserId(currentUser.id);

View File

@@ -13,7 +13,7 @@ describe('GET /api/v1/admin/saml-auth-providers/:samlAuthProviderId/role-mapping
let roleMappingOne, roleMappingTwo, samlAuthProvider, currentUser, token; let roleMappingOne, roleMappingTwo, samlAuthProvider, currentUser, token;
beforeEach(async () => { beforeEach(async () => {
const role = await createRole({ key: 'admin' }); const role = await createRole({ name: 'Admin' });
currentUser = await createUser({ roleId: role.id }); currentUser = await createUser({ roleId: role.id });
samlAuthProvider = await createSamlAuthProvider(); samlAuthProvider = await createSamlAuthProvider();

View File

@@ -13,7 +13,7 @@ describe('GET /api/v1/admin/saml-auth-provider/:samlAuthProviderId', () => {
let samlAuthProvider, currentUser, token; let samlAuthProvider, currentUser, token;
beforeEach(async () => { beforeEach(async () => {
const role = await createRole({ key: 'admin' }); const role = await createRole({ name: 'Admin' });
currentUser = await createUser({ roleId: role.id }); currentUser = await createUser({ roleId: role.id });
samlAuthProvider = await createSamlAuthProvider(); samlAuthProvider = await createSamlAuthProvider();

View File

@@ -12,7 +12,7 @@ describe('GET /api/v1/admin/saml-auth-providers', () => {
let samlAuthProviderOne, samlAuthProviderTwo, currentUser, token; let samlAuthProviderOne, samlAuthProviderTwo, currentUser, token;
beforeEach(async () => { beforeEach(async () => {
const role = await createRole({ key: 'admin' }); const role = await createRole({ name: 'Admin' });
currentUser = await createUser({ roleId: role.id }); currentUser = await createUser({ roleId: role.id });
samlAuthProviderOne = await createSamlAuthProvider(); samlAuthProviderOne = await createSamlAuthProvider();

View File

@@ -15,7 +15,7 @@ describe('PATCH /api/v1/admin/saml-auth-provider/:samlAuthProviderId', () => {
beforeEach(async () => { beforeEach(async () => {
vi.spyOn(license, 'hasValidLicense').mockResolvedValue(true); vi.spyOn(license, 'hasValidLicense').mockResolvedValue(true);
role = await createRole({ key: 'admin' }); role = await createRole({ name: 'Admin' });
currentUser = await createUser({ roleId: role.id }); currentUser = await createUser({ roleId: role.id });
token = await createAuthTokenByUserId(currentUser.id); token = await createAuthTokenByUserId(currentUser.id);

View File

@@ -10,7 +10,7 @@ describe('DELETE /api/v1/admin/users/:userId', () => {
let currentUser, currentUserRole, anotherUser, token; let currentUser, currentUserRole, anotherUser, token;
beforeEach(async () => { beforeEach(async () => {
currentUserRole = await createRole({ key: 'admin' }); currentUserRole = await createRole({ name: 'Admin' });
currentUser = await createUser({ roleId: currentUserRole.id }); currentUser = await createUser({ roleId: currentUserRole.id });
anotherUser = await createUser(); anotherUser = await createUser();

View File

@@ -12,7 +12,7 @@ describe('GET /api/v1/admin/users/:userId', () => {
let currentUser, currentUserRole, anotherUser, anotherUserRole, token; let currentUser, currentUserRole, anotherUser, anotherUserRole, token;
beforeEach(async () => { beforeEach(async () => {
currentUserRole = await createRole({ key: 'admin' }); currentUserRole = await createRole({ name: 'Admin' });
currentUser = await createUser({ roleId: currentUserRole.id }); currentUser = await createUser({ roleId: currentUserRole.id });
anotherUser = await createUser(); anotherUser = await createUser();

View File

@@ -10,7 +10,7 @@ describe('GET /api/v1/admin/users', () => {
let currentUser, currentUserRole, anotherUser, anotherUserRole, token; let currentUser, currentUserRole, anotherUser, anotherUserRole, token;
beforeEach(async () => { beforeEach(async () => {
currentUserRole = await createRole({ key: 'admin' }); currentUserRole = await createRole({ name: 'Admin' });
currentUser = await createUser({ currentUser = await createUser({
roleId: currentUserRole.id, roleId: currentUserRole.id,
@@ -18,7 +18,6 @@ describe('GET /api/v1/admin/users', () => {
}); });
anotherUserRole = await createRole({ anotherUserRole = await createRole({
key: 'anotherUser',
name: 'Another user role', name: 'Another user role',
}); });

View File

@@ -11,7 +11,7 @@ describe('PATCH /api/v1/admin/users/:userId', () => {
let currentUser, adminRole, token; let currentUser, adminRole, token;
beforeEach(async () => { beforeEach(async () => {
adminRole = await createRole({ key: 'admin' }); adminRole = await createRole({ name: 'Admin' });
currentUser = await createUser({ roleId: adminRole.id }); currentUser = await createUser({ roleId: adminRole.id });
token = await createAuthTokenByUserId(currentUser.id); token = await createAuthTokenByUserId(currentUser.id);

View File

@@ -13,8 +13,7 @@ describe('POST /api/v1/installation/users', () => {
beforeEach(async () => { beforeEach(async () => {
adminRole = await createRole({ adminRole = await createRole({
name: 'Admin', name: 'Admin',
key: 'admin', });
})
}); });
describe('for incomplete installations', () => { describe('for incomplete installations', () => {
@@ -26,7 +25,7 @@ describe('POST /api/v1/installation/users', () => {
.send({ .send({
email: 'user@automatisch.io', email: 'user@automatisch.io',
password: 'password', password: 'password',
fullName: 'Initial admin' fullName: 'Initial admin',
}) })
.expect(204); .expect(204);
@@ -48,7 +47,7 @@ describe('POST /api/v1/installation/users', () => {
.send({ .send({
email: 'user@automatisch.io', email: 'user@automatisch.io',
password: 'password', password: 'password',
fullName: 'Initial admin' fullName: 'Initial admin',
}) })
.expect(403); .expect(403);
@@ -71,7 +70,7 @@ describe('POST /api/v1/installation/users', () => {
.send({ .send({
email: 'user@automatisch.io', email: 'user@automatisch.io',
password: 'password', password: 'password',
fullName: 'Initial admin' fullName: 'Initial admin',
}) })
.expect(403); .expect(403);
@@ -80,5 +79,5 @@ describe('POST /api/v1/installation/users', () => {
expect(user).toBeUndefined(); expect(user).toBeUndefined();
expect(await Config.isInstallationCompleted()).toBe(true); expect(await Config.isInstallationCompleted()).toBe(true);
}); });
}) });
}); });

View File

@@ -1,11 +1,11 @@
export async function up(knex) { export async function up(knex) {
return await knex.schema.alterTable('roles', (table) => { return await knex.schema.alterTable('roles', (table) => {
table.unique('key'); table.unique('name');
}); });
} }
export async function down(knex) { export async function down(knex) {
return await knex.schema.alterTable('roles', function (table) { return await knex.schema.alterTable('roles', function (table) {
table.dropUnique('key'); table.dropUnique('name');
}); });
} }

View File

@@ -0,0 +1,11 @@
export async function up(knex) {
return await knex.schema.alterTable('roles', (table) => {
table.dropColumn('key');
});
}
export async function down(knex) {
return await knex.schema.alterTable('roles', function (table) {
table.string('key').notNullable();
});
}

View File

@@ -32,7 +32,7 @@ const createUser = async (_parent, params, context) => {
userPayload.roleId = params.input.role.id; userPayload.roleId = params.input.role.id;
} catch { } catch {
// void // void
const role = await Role.query().findOne({ key: 'admin' }); const role = await Role.findAdmin();
userPayload.roleId = role.id; userPayload.roleId = role.id;
} }

View File

@@ -15,7 +15,7 @@ const registerUser = async (_parent, params) => {
throw new Error('User already exists!'); throw new Error('User already exists!');
} }
const role = await Role.query().findOne({ key: 'user' }); const role = await Role.query().findOne({ name: 'User' });
const user = await User.query().insert({ const user = await User.query().insert({
fullName, fullName,

View File

@@ -7,22 +7,17 @@ class Role extends Base {
static jsonSchema = { static jsonSchema = {
type: 'object', type: 'object',
required: ['name', 'key'], required: ['name'],
properties: { properties: {
id: { type: 'string', format: 'uuid' }, id: { type: 'string', format: 'uuid' },
name: { type: 'string', minLength: 1 }, name: { type: 'string', minLength: 1 },
key: { type: 'string', minLength: 1 },
description: { type: ['string', 'null'], maxLength: 255 }, description: { type: ['string', 'null'], maxLength: 255 },
createdAt: { type: 'string' }, createdAt: { type: 'string' },
updatedAt: { type: 'string' }, updatedAt: { type: 'string' },
}, },
}; };
static get virtualAttributes() {
return ['isAdmin'];
}
static relationMappings = () => ({ static relationMappings = () => ({
users: { users: {
relation: Base.HasManyRelation, relation: Base.HasManyRelation,
@@ -43,11 +38,11 @@ class Role extends Base {
}); });
get isAdmin() { get isAdmin() {
return this.key === 'admin'; return this.name === 'Admin';
} }
static async findAdmin() { static async findAdmin() {
return await this.query().findOne({ key: 'admin' }); return await this.query().findOne({ name: 'Admin' });
} }
} }

View File

@@ -3,10 +3,8 @@ import Role from '../../src/models/role';
export const createRole = async (params = {}) => { export const createRole = async (params = {}) => {
const name = faker.lorem.word(); const name = faker.lorem.word();
const key = name.toLowerCase();
params.name = params?.name || name; params.name = params?.name || name;
params.key = params?.key || key;
const role = await Role.query().insertAndFetch(params); const role = await Role.query().insertAndFetch(params);

View File

@@ -1,7 +1,6 @@
const createRoleMock = async (role, permissions = []) => { const createRoleMock = async (role, permissions = []) => {
const data = { const data = {
id: role.id, id: role.id,
key: role.key,
name: role.name, name: role.name,
isAdmin: role.isAdmin, isAdmin: role.isAdmin,
description: role.description, description: role.description,

View File

@@ -1,7 +1,6 @@
const getRoleMock = async (role, permissions) => { const getRoleMock = async (role, permissions) => {
const data = { const data = {
id: role.id, id: role.id,
key: role.key,
name: role.name, name: role.name,
isAdmin: role.isAdmin, isAdmin: role.isAdmin,
description: role.description, description: role.description,

View File

@@ -2,7 +2,6 @@ const getRolesMock = async (roles) => {
const data = roles.map((role) => { const data = roles.map((role) => {
return { return {
id: role.id, id: role.id,
key: role.key,
name: role.name, name: role.name,
isAdmin: role.isAdmin, isAdmin: role.isAdmin,
description: role.description, description: role.description,

View File

@@ -9,7 +9,6 @@ const updateUserMock = (user, role) => {
updatedAt: user.updatedAt.getTime(), updatedAt: user.updatedAt.getTime(),
role: { role: {
id: role.id, id: role.id,
key: role.key,
name: role.name, name: role.name,
isAdmin: role.isAdmin, isAdmin: role.isAdmin,
createdAt: role.createdAt.getTime(), createdAt: role.createdAt.getTime(),