feat(AppConfig): iterate how apps are managed

- auth clients are always shared, cannot be disabled
- custom connections are enabled by default, can be disabled
- any existing connections can be reconnected regardless of its AppConfig or AppAuthClient states
This commit is contained in:
Ali BARIN
2024-12-09 17:46:51 +00:00
parent 00e80f1fba
commit a57134a89c
48 changed files with 192 additions and 563 deletions

View File

@@ -3,17 +3,9 @@
exports[`AppConfig model > jsonSchema should have correct validations 1`] = `
{
"properties": {
"connectionAllowed": {
"default": false,
"type": "boolean",
},
"createdAt": {
"type": "string",
},
"customConnectionAllowed": {
"default": false,
"type": "boolean",
},
"disabled": {
"default": false,
"type": "boolean",
@@ -25,13 +17,13 @@ exports[`AppConfig model > jsonSchema should have correct validations 1`] = `
"key": {
"type": "string",
},
"shared": {
"default": false,
"type": "boolean",
},
"updatedAt": {
"type": "string",
},
"useOnlyPredefinedAuthClients": {
"default": false,
"type": "boolean",
},
},
"required": [
"key",

View File

@@ -60,39 +60,26 @@ class AppAuthClient extends Base {
return this.authDefaults ? true : false;
}
async triggerAppConfigUpdate() {
const appConfig = await this.$relatedQuery('appConfig');
// This is a workaround to update connection allowed column for AppConfig
await appConfig?.$query().patch({
key: appConfig.key,
shared: appConfig.shared,
disabled: appConfig.disabled,
});
}
// TODO: Make another abstraction like beforeSave instead of using
// beforeInsert and beforeUpdate separately for the same operation.
async $beforeInsert(queryContext) {
await super.$beforeInsert(queryContext);
this.encryptData();
}
async $afterInsert(queryContext) {
await super.$afterInsert(queryContext);
await this.triggerAppConfigUpdate();
}
async $beforeUpdate(opt, queryContext) {
await super.$beforeUpdate(opt, queryContext);
this.encryptData();
}
async $afterUpdate(opt, queryContext) {
await super.$afterUpdate(opt, queryContext);
await this.triggerAppConfigUpdate();
}
async $afterFind() {

View File

@@ -164,63 +164,6 @@ describe('AppAuthClient model', () => {
});
});
describe('triggerAppConfigUpdate', () => {
it('should trigger an update in related app config', async () => {
await createAppConfig({ key: 'gitlab' });
const appAuthClient = await createAppAuthClient({
appKey: 'gitlab',
});
const appConfigBeforeUpdateSpy = vi.spyOn(
AppConfig.prototype,
'$beforeUpdate'
);
await appAuthClient.triggerAppConfigUpdate();
expect(appConfigBeforeUpdateSpy).toHaveBeenCalledOnce();
});
it('should update related AppConfig after creating an instance', async () => {
const appConfig = await createAppConfig({
key: 'gitlab',
disabled: false,
shared: true,
});
await createAppAuthClient({
appKey: 'gitlab',
active: true,
});
const refetchedAppConfig = await appConfig.$query();
expect(refetchedAppConfig.connectionAllowed).toBe(true);
});
it('should update related AppConfig after updating an instance', async () => {
const appConfig = await createAppConfig({
key: 'gitlab',
disabled: false,
shared: true,
});
const appAuthClient = await createAppAuthClient({
appKey: 'gitlab',
active: false,
});
let refetchedAppConfig = await appConfig.$query();
expect(refetchedAppConfig.connectionAllowed).toBe(false);
await appAuthClient.$query().patchAndFetch({ active: true });
refetchedAppConfig = await appConfig.$query();
expect(refetchedAppConfig.connectionAllowed).toBe(true);
});
});
it('$beforeInsert should call AppAuthClient.encryptData', async () => {
const appAuthClientBeforeInsertSpy = vi.spyOn(
AppAuthClient.prototype,
@@ -232,17 +175,6 @@ describe('AppAuthClient model', () => {
expect(appAuthClientBeforeInsertSpy).toHaveBeenCalledOnce();
});
it('$afterInsert should call AppAuthClient.triggerAppConfigUpdate', async () => {
const appAuthClientAfterInsertSpy = vi.spyOn(
AppAuthClient.prototype,
'triggerAppConfigUpdate'
);
await createAppAuthClient();
expect(appAuthClientAfterInsertSpy).toHaveBeenCalledOnce();
});
it('$beforeUpdate should call AppAuthClient.encryptData', async () => {
const appAuthClient = await createAppAuthClient();
@@ -256,19 +188,6 @@ describe('AppAuthClient model', () => {
expect(appAuthClientBeforeUpdateSpy).toHaveBeenCalledOnce();
});
it('$afterUpdate should call AppAuthClient.triggerAppConfigUpdate', async () => {
const appAuthClient = await createAppAuthClient();
const appAuthClientAfterUpdateSpy = vi.spyOn(
AppAuthClient.prototype,
'triggerAppConfigUpdate'
);
await appAuthClient.$query().patchAndFetch({ name: 'sample' });
expect(appAuthClientAfterUpdateSpy).toHaveBeenCalledOnce();
});
it('$afterFind should call AppAuthClient.decryptData', async () => {
const appAuthClient = await createAppAuthClient();

View File

@@ -39,39 +39,6 @@ class AppConfig extends Base {
return await App.findOneByKey(this.key);
}
async computeAndAssignConnectionAllowedProperty() {
this.connectionAllowed = await this.computeConnectionAllowedProperty();
}
async computeConnectionAllowedProperty() {
const appAuthClients = await this.$relatedQuery('appAuthClients');
const hasSomeActiveAppAuthClients =
appAuthClients?.some((appAuthClient) => appAuthClient.active) || false;
const conditions = [
hasSomeActiveAppAuthClients,
this.shared,
!this.disabled,
];
const connectionAllowed = conditions.every(Boolean);
return connectionAllowed;
}
async $beforeInsert(queryContext) {
await super.$beforeInsert(queryContext);
await this.computeAndAssignConnectionAllowedProperty();
}
async $beforeUpdate(opt, queryContext) {
await super.$beforeUpdate(opt, queryContext);
await this.computeAndAssignConnectionAllowedProperty();
}
}
export default AppConfig;

View File

@@ -1,11 +1,9 @@
import { vi, describe, it, expect } from 'vitest';
import { describe, it, expect } from 'vitest';
import Base from './base.js';
import AppConfig from './app-config.js';
import App from './app.js';
import AppAuthClient from './app-auth-client.js';
import { createAppConfig } from '../../test/factories/app-config.js';
import { createAppAuthClient } from '../../test/factories/app-auth-client.js';
describe('AppConfig model', () => {
it('tableName should return correct name', () => {
@@ -55,126 +53,4 @@ describe('AppConfig model', () => {
expect(app).toStrictEqual(expectedApp);
});
});
describe('computeAndAssignConnectionAllowedProperty', () => {
it('should call computeConnectionAllowedProperty and assign the result', async () => {
const appConfig = await createAppConfig();
const computeConnectionAllowedPropertySpy = vi
.spyOn(appConfig, 'computeConnectionAllowedProperty')
.mockResolvedValue(true);
await appConfig.computeAndAssignConnectionAllowedProperty();
expect(computeConnectionAllowedPropertySpy).toHaveBeenCalled();
expect(appConfig.connectionAllowed).toBe(true);
});
});
describe('computeConnectionAllowedProperty', () => {
it('should return true when app is enabled, shared and allows custom connection with an active app auth client', async () => {
await createAppAuthClient({
appKey: 'deepl',
active: true,
});
await createAppAuthClient({
appKey: 'deepl',
active: false,
});
const appConfig = await createAppConfig({
disabled: false,
customConnectionAllowed: true,
shared: true,
key: 'deepl',
});
const connectionAllowed =
await appConfig.computeConnectionAllowedProperty();
expect(connectionAllowed).toBe(true);
});
it('should return false if there is no active app auth client', async () => {
await createAppAuthClient({
appKey: 'deepl',
active: false,
});
const appConfig = await createAppConfig({
disabled: false,
customConnectionAllowed: true,
shared: true,
key: 'deepl',
});
const connectionAllowed =
await appConfig.computeConnectionAllowedProperty();
expect(connectionAllowed).toBe(false);
});
it('should return false if there is no app auth clients', async () => {
const appConfig = await createAppConfig({
disabled: false,
customConnectionAllowed: true,
shared: true,
key: 'deepl',
});
const connectionAllowed =
await appConfig.computeConnectionAllowedProperty();
expect(connectionAllowed).toBe(false);
});
it('should return false when app is disabled', async () => {
const appConfig = await createAppConfig({
disabled: true,
customConnectionAllowed: true,
});
const connectionAllowed =
await appConfig.computeConnectionAllowedProperty();
expect(connectionAllowed).toBe(false);
});
it(`should return false when app doesn't allow custom connection`, async () => {
const appConfig = await createAppConfig({
disabled: false,
customConnectionAllowed: false,
});
const connectionAllowed =
await appConfig.computeConnectionAllowedProperty();
expect(connectionAllowed).toBe(false);
});
});
it('$beforeInsert should call computeAndAssignConnectionAllowedProperty', async () => {
const computeAndAssignConnectionAllowedPropertySpy = vi
.spyOn(AppConfig.prototype, 'computeAndAssignConnectionAllowedProperty')
.mockResolvedValue(true);
await createAppConfig();
expect(computeAndAssignConnectionAllowedPropertySpy).toHaveBeenCalledOnce();
});
it('$beforeUpdate should call computeAndAssignConnectionAllowedProperty', async () => {
const appConfig = await createAppConfig();
const computeAndAssignConnectionAllowedPropertySpy = vi
.spyOn(AppConfig.prototype, 'computeAndAssignConnectionAllowedProperty')
.mockResolvedValue(true);
await appConfig.$query().patch({
key: 'deepl',
});
expect(computeAndAssignConnectionAllowedPropertySpy).toHaveBeenCalledOnce();
});
});

View File

@@ -33,10 +33,6 @@ class Connection extends Base {
},
};
static get virtualAttributes() {
return ['reconnectable'];
}
static relationMappings = () => ({
user: {
relation: Base.BelongsToOneRelation,
@@ -83,18 +79,6 @@ class Connection extends Base {
},
});
get reconnectable() {
if (this.appAuthClientId) {
return this.appAuthClient.active;
}
if (this.appConfig) {
return !this.appConfig.disabled && this.appConfig.customConnectionAllowed;
}
return true;
}
encryptData() {
if (!this.eligibleForEncryption()) return;
@@ -144,19 +128,13 @@ class Connection extends Base {
);
}
if (!appConfig.customConnectionAllowed && this.formattedData) {
if (appConfig.useOnlyPredefinedAuthClients && this.formattedData) {
throw new NotAuthorizedError(
`New custom connections have been disabled for ${app.name}!`
);
}
if (!appConfig.shared && this.appAuthClientId) {
throw new NotAuthorizedError(
'The connection with the given app auth client is not allowed!'
);
}
if (appConfig.shared && !this.formattedData) {
if (!this.formattedData) {
const authClient = await appConfig
.$relatedQuery('appAuthClients')
.findById(this.appAuthClientId)

View File

@@ -23,14 +23,6 @@ describe('Connection model', () => {
expect(Connection.jsonSchema).toMatchSnapshot();
});
it('virtualAttributes should return correct attributes', () => {
const virtualAttributes = Connection.virtualAttributes;
const expectedAttributes = ['reconnectable'];
expect(virtualAttributes).toStrictEqual(expectedAttributes);
});
describe('relationMappings', () => {
it('should return correct associations', () => {
const relationMappings = Connection.relationMappings();
@@ -92,78 +84,6 @@ describe('Connection model', () => {
});
});
describe('reconnectable', () => {
it('should return active status of app auth client when created via app auth client', async () => {
const appAuthClient = await createAppAuthClient({
active: true,
formattedAuthDefaults: {
clientId: 'sample-id',
},
});
const connection = await createConnection({
appAuthClientId: appAuthClient.id,
formattedData: {
token: 'sample-token',
},
});
const connectionWithAppAuthClient = await connection
.$query()
.withGraphFetched({
appAuthClient: true,
});
expect(connectionWithAppAuthClient.reconnectable).toBe(true);
});
it('should return true when app config is not disabled and allows custom connection', async () => {
const appConfig = await createAppConfig({
key: 'gitlab',
disabled: false,
customConnectionAllowed: true,
});
const connection = await createConnection({
key: appConfig.key,
formattedData: {
token: 'sample-token',
},
});
const connectionWithAppAuthClient = await connection
.$query()
.withGraphFetched({
appConfig: true,
});
expect(connectionWithAppAuthClient.reconnectable).toBe(true);
});
it('should return false when app config is disabled or does not allow custom connection', async () => {
const connection = await createConnection({
key: 'gitlab',
formattedData: {
token: 'sample-token',
},
});
await createAppConfig({
key: 'gitlab',
disabled: true,
customConnectionAllowed: false,
});
const connectionWithAppAuthClient = await connection
.$query()
.withGraphFetched({
appConfig: true,
});
expect(connectionWithAppAuthClient.reconnectable).toBe(false);
});
});
describe('encryptData', () => {
it('should return undefined if eligibleForEncryption is not true', async () => {
vi.spyOn(Connection.prototype, 'eligibleForEncryption').mockReturnValue(
@@ -366,6 +286,7 @@ describe('Connection model', () => {
);
});
// TODO: update test case name
it('should throw an error when app config does not allow custom connection with formatted data', async () => {
vi.spyOn(Connection.prototype, 'getApp').mockResolvedValue({
name: 'gitlab',
@@ -373,7 +294,7 @@ describe('Connection model', () => {
vi.spyOn(Connection.prototype, 'getAppConfig').mockResolvedValue({
disabled: false,
customConnectionAllowed: false,
useOnlyPredefinedAuthClients: true,
});
const connection = new Connection();
@@ -386,32 +307,10 @@ describe('Connection model', () => {
);
});
it('should throw an error when app config is not shared with app auth client', async () => {
vi.spyOn(Connection.prototype, 'getApp').mockResolvedValue({
name: 'gitlab',
});
vi.spyOn(Connection.prototype, 'getAppConfig').mockResolvedValue({
disabled: false,
shared: false,
});
const connection = new Connection();
connection.appAuthClientId = 'sample-id';
await expect(() =>
connection.checkEligibilityForCreation()
).rejects.toThrow(
'The connection with the given app auth client is not allowed!'
);
});
it('should apply app auth client auth defaults when creating with shared app auth client', async () => {
await createAppConfig({
key: 'gitlab',
disabled: false,
customConnectionAllowed: true,
shared: true,
});
const appAuthClient = await createAppAuthClient({